On Sun, 15 May 2016 23:29:27 +0200, Robert Jarzmik wrote:
Takashi Iwai tiwai@suse.de writes:
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.)
Ah yes, I remember now, the on-stack generated device, indeed ugly.
IMO, a cleaner way would be to define the ops passed with both controller and codec objects as arguments, and pass NULL codec here.
It's rather unusual to need both the device and its controller in bus operations. I must admit I have no better idea so far, so I'll try that just to see how it looks like, and let's see next ...
Thinking of this again, I wonder now why we need to pass the codec object at all. It's the read/write ops via ac97, so we just need the ac97_controller object and the address slot of the accessed codec?
Takashi