14 May
2016
14 May
'16
5:13 p.m.
On Sat, 14 May 2016 11:50:50 +0200, Robert Jarzmik wrote:
+unsigned int ac97_bus_scan_one(struct ac97_controller *ac97,
int codec_num)+{
- struct ac97_codec_device codec;
- unsigned short vid1, vid2;
- int ret;
- codec.dev = *ac97->dev;
- codec.num = codec_num;
- ret = ac97->ops->read(&codec, AC97_VENDOR_ID1);
- vid1 = (ret & 0xffff);
- if (ret < 0)
return 0;Hmm. This looks pretty hackish and dangerous.
You mean returning 0 even if the read failed, right ?
No, my concern is that it's creating a dummy codec object temporarily on the stack just by copying some fields and calling the ops with it. (And actually the current code may work wrongly because lack of zero-clear of the object.)
IMO, a cleaner way would be to define the ops passed with both controller and codec objects as arguments, and pass NULL codec here.
Takashi