On Sat, 2011-01-08 at 19:45 +0100, Lawrence Rust wrote: > On Sat, 2011-01-08 at 09:22 -0500, Andy Walls wrote: > Thanks for the info on the PVR-150. It largely confirmed what I had > surmised - that the two cards disagree about serial audio data format. > Before my patch, the wm8775 was programmed for Philips mode but the > CX25843 on the PVR-150 is setup for Sony I2S mode!! On the Nova-S, the > cx23883 is setup (in cx88-tvaudio.c) for Philips mode. The patch > changed the wm8775 to Sony I2S mode because the existing setup gave > noise, indicative of a mismatch. > > It is my belief that either the wm8775 datasheet is wrong or there are > inverters on the SCLK lines between the wm8775 and cx25843/23883. It is > also plausible that Conexant have it wrong and both their datasheets are > wrong. > > Anyway, I have revised the patch (attached) so that the wm8775 is kept > in Philips mode (to please the PVR-150) and the cx23883 on the Nove-S is > now switched to Sony I2S mode (like the PVR-150) and this works fine. I will try and review and test this coming weekend. > The change is trivial, just 2 lines, so they're shouldn't be any other > consequences. However, could this affect any other cards? That is the problem with code reuse, for multiple board models, in all the I2C modules. It makes code increasingly more difficult to maintain as more different board models are supported and tested. But now there is infrastructure in place to pass board specific info down to the I2C v4l2_subdevice drivers. If the wm8775 driver were changed to provide a wm8775-specific platform-data structure for bridge drivers to use, bridge drivers could fill it out and call v4l2_i2c_new_subdev_board() to instantiate the wm8775 device instance specific to the board: Nova-S, PVR-150, or whatever. See the interaction between the ivtv and cx25840 module in this patch as an example: https://patchwork.kernel.org/patch/465571/ (Not all of the details are visible in the patch, since some were already there for the .s_config call in cx25840.) Obviously, the wm8775 module would need code added to take different actions when passed different platform data. However, that the best way to make sure one doesn't accidentally affect other boards. > NB I have only tested this patch on my Nova-S, no other. I do see one problem with your patch at the moment: diff --git a/include/media/wm8775.c b/include/media/wm8775.c ... + sd->grp_id = WM8775_GID; /* subdev group id */ ... diff --git a/include/media/wm8775.h b/include/media/wm8775.h ... +/* subdev group ID */ +#define WM8775_GID (1 << 0) + ... The wm8775 module probably should not define WM8775_GID and definitely should not set sd->grp_id. The sd->grp_id is for the bridge driver's use for that v4l2_subdev instance. Some bridge drivers may expect it to be 0 unless they set it themselves. The group ID values should be defined in the bridge driver, and the sd->grp_id field should be set by the bridge driver. You would want to do that in cx88. See cx23885, ivtv, and cx18 as examples of bridge drivers that use the group id field. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
