Re: [PATCH 07/24] xc5000: properly report i2c write failures

Re: [PATCH 07/24] xc5000: properly report i2c write failures

[Joonyoung Shim]

Hi,

Sorry for response about the past post.

> The logic as written would *never* actually return an error condition, since
> the loop would run until the counter hit zero but the check was for a value
> less than zero.
>
> Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
> ---
>   drivers/media/common/tuners/xc5000.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
> diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
> index f660e33..a7fa17e 100644
> --- a/drivers/media/common/tuners/xc5000.c
> +++ b/drivers/media/common/tuners/xc5000.c
> @@ -341,7 +341,7 @@    static int xc_write_reg(struct xc5000_priv *priv, u16 regAddr, u16 i2cData)
>   			}
>   		}
>   	}
> -	if (WatchDogTimer < 0)
> +	if (WatchDogTimer <= 0)

I can't load firmware like error of below link.

https://bugs.launchpad.net/ubuntu/+source/linux-firmware-nonfree/+bug/1263837 
<https://bugs.launchpad.net/ubuntu/+source/linux-firmware-nonfree/+bug/1263837>

This error is related with this patch. This fix is right but above error 
is created after this fix
because my device makes WatchDogTimer to 0 when load firmware.
Maybe it will be related with XREG_BUSY register but i can't check it.

I removed this fix, but i have faced at other error with "xc5000: PLL 
not running after fwload"
So i have commented like below.

static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
         .name = XC5000A_FIRMWARE,
         .size = 12401,
         //.pll_reg = 0x806c,
};

Then, xc5000 device works well.

I don't have xc5000 datasheet so i can't debug xc5000 driver anymore.

Any help?

Thanks.

>   		result = XC_RESULT_I2C_WRITE_FAILURE;
>   
>   	return result;
>

[PATCH 00/24] Various HVR-950q and xc5000 fixes

[Devin Heitmueller]

This patch series contains fixes for a variety of problems found in the
HVR-950q as well as the xc5000 driver.

Details can be found in the individual patches, but it is worth mentioning
specifically that this addresses the MythTV problem causing BUG() to occur,
firmware loading is now significantly improved, and we now have a
redistributable version for the xc5000c firmware.

Devin Heitmueller (24):
  au8522: fix intermittent lockup of analog video decoder
  au8522: Fix off-by-one in SNR table for QAM256
  au8522: properly recover from the au8522 delivering misaligned TS
    streams
  au0828: Make the s_reg and g_reg advanced debug calls work against
    the bridge
  xc5000: properly show quality register values
  xc5000: add support for showing the SNR and gain in the debug output
  xc5000: properly report i2c write failures
  au0828: fix race condition that causes xc5000 to not bind for digital
  au0828: make sure video standard is setup in tuner-core
  au8522: fix regression in logging introduced by separation of modules
  xc5000: don't invoke auto calibration unless we really did reset
    tuner
  au0828: prevent i2c gate from being kept open while in analog mode
  au0828: fix case where STREAMOFF being called on stopped stream
    causes BUG()
  au0828: speed up i2c clock when doing xc5000 firmware load
  au0828: remove control buffer from send_control_msg
  au0828: tune retry interval for i2c interaction
  au0828: fix possible race condition in usage of dev->ctrlmsg
  xc5000: reset device if encountering PLL lock failure
  xc5000: add support for firmware load check and init status
  au0828: tweak workaround for i2c clock stretching bug
  xc5000: show debug version fields in decimal instead of hex
  au0828: fix a couple of missed edge cases for i2c gate with analog
  au0828: make xc5000 firmware speedup apply to the xc5000c as well
  xc5000: change filename to production/redistributable xc5000c
    firmware

 drivers/media/common/tuners/xc5000.c         |  161 +++++++++++++++++++++-----
 drivers/media/dvb/frontends/au8522_common.c  |   22 +++-
 drivers/media/dvb/frontends/au8522_decoder.c |   11 +-
 drivers/media/dvb/frontends/au8522_dig.c     |   98 ++++++++--------
 drivers/media/dvb/frontends/au8522_priv.h    |   29 ++++-
 drivers/media/video/au0828/au0828-cards.c    |    4 +-
 drivers/media/video/au0828/au0828-core.c     |   59 ++++------
 drivers/media/video/au0828/au0828-dvb.c      |   54 ++++++++-
 drivers/media/video/au0828/au0828-i2c.c      |   21 +++-
 drivers/media/video/au0828/au0828-reg.h      |    1 +
 drivers/media/video/au0828/au0828-video.c    |   76 +++++++++---
 drivers/media/video/au0828/au0828.h          |    2 +
 12 files changed, 379 insertions(+), 159 deletions(-)

[PATCH 07/24] xc5000: properly report i2c write failures

[Devin Heitmueller]

The logic as written would *never* actually return an error condition, since
the loop would run until the counter hit zero but the check was for a value
less than zero.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/common/tuners/xc5000.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index f660e33..a7fa17e 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -341,7 +341,7 @@ static int xc_write_reg(struct xc5000_priv *priv, u16 regAddr, u16 i2cData)
 			}
 		}
 	}
-	if (WatchDogTimer < 0)
+	if (WatchDogTimer <= 0)
 		result = XC_RESULT_I2C_WRITE_FAILURE;
 
 	return result;
-- 
1.7.1

Re: [PATCH 07/24] xc5000: properly report i2c write failures

[Devin Heitmueller]

> I can't load firmware like error of below link.
>
> https://bugs.launchpad.net/ubuntu/+source/linux-firmware-nonfree/+bug/1263837
>
> This error is related with this patch. This fix is right but above error is
> created after this fix
> because my device makes WatchDogTimer to 0 when load firmware.
> Maybe it will be related with XREG_BUSY register but i can't check it.
>
> I removed this fix, but i have faced at other error with "xc5000: PLL not
> running after fwload"
> So i have commented like below.
>
> static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
>         .name = XC5000A_FIRMWARE,
>         .size = 12401,
>         //.pll_reg = 0x806c,
> };
>
> Then, xc5000 device works well.
>
> I don't have xc5000 datasheet so i can't debug xc5000 driver anymore.

Hi Joonyoung,

Assuming this is the DViCO FusionHDTV7 device that uses the
au0828/au8522, I suspect that what's happening here is your I2C
controller is not stable.  The I2C clock stretching done by the xc5000
often exposed bugs in various bridge drivers and the au0828 was no
exception.  I had to work around these hardware bugs in the au0828
driver but I made them specific to the HVR-950q since that was the
only device I could test with.

In other words, the xc5000 is most likely doing exactly what it is
supposed to, and the increased robustness of the tuner driver with
those two patches exposed intermittent I2C failures in au0828 that
were previously being silently discarded (resulting in indeterminate
behavior).

I would recommending looking at the changes in au0828-cards.c for the
HVR-950q and add the code necessary to make them also apply for the
DVICO device, and that should resolve your problems (in particular the
i2c_clk_divider field should be set).

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH 07/24] xc5000: properly report i2c write failures

[Joonyoung Shim]

Hi Devin,

2014-02-07 22:46 GMT+09:00 Devin Heitmueller <dheitmueller@kernellabs.com>:
>
> > I can't load firmware like error of below link.
> >
> > https://bugs.launchpad.net/ubuntu/+source/linux-firmware-nonfree/+bug/1263837
> >
> > This error is related with this patch. This fix is right but above error is
> > created after this fix
> > because my device makes WatchDogTimer to 0 when load firmware.
> > Maybe it will be related with XREG_BUSY register but i can't check it.
> >
> > I removed this fix, but i have faced at other error with "xc5000: PLL not
> > running after fwload"
> > So i have commented like below.
> >
> > static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
> >         .name = XC5000A_FIRMWARE,
> >         .size = 12401,
> >         //.pll_reg = 0x806c,
> > };
> >
> > Then, xc5000 device works well.
> >
> > I don't have xc5000 datasheet so i can't debug xc5000 driver anymore.
>
> Hi Joonyoung,
>
> Assuming this is the DViCO FusionHDTV7 device that uses the
> au0828/au8522, I suspect that what's happening here is your I2C
> controller is not stable.  The I2C clock stretching done by the xc5000
> often exposed bugs in various bridge drivers and the au0828 was no
> exception.  I had to work around these hardware bugs in the au0828
> driver but I made them specific to the HVR-950q since that was the
> only device I could test with.
>
> In other words, the xc5000 is most likely doing exactly what it is
> supposed to, and the increased robustness of the tuner driver with
> those two patches exposed intermittent I2C failures in au0828 that
> were previously being silently discarded (resulting in indeterminate
> behavior).
>
> I would recommending looking at the changes in au0828-cards.c for the
> HVR-950q and add the code necessary to make them also apply for the
> DVICO device, and that should resolve your problems (in particular the
> i2c_clk_divider field should be set).
>

As you said, i modified like below patch and it is working well.

Thanks for your advice.

diff --git a/drivers/media/usb/au0828/au0828-cards.c
b/drivers/media/usb/au0828/au0828-cards.c
index 0cb7c28..9936875 100644
--- a/drivers/media/usb/au0828/au0828-cards.c
+++ b/drivers/media/usb/au0828/au0828-cards.c
@@ -108,7 +108,7 @@ struct au0828_board au0828_boards[] = {
         .name    = "DViCO FusionHDTV USB",
         .tuner_type = UNSET,
         .tuner_addr = ADDR_UNSET,
-        .i2c_clk_divider = AU0828_I2C_CLK_250KHZ,
+        .i2c_clk_divider = AU0828_I2C_CLK_20KHZ,
     },
     [AU0828_BOARD_HAUPPAUGE_WOODBURY] = {
         .name = "Hauppauge Woodbury",


- Joonyoung Shim

Re: [PATCH 07/24] xc5000: properly report i2c write failures

[Devin Heitmueller]

On Mon, Feb 10, 2014 at 3:25 AM, Joonyoung Shim <dofmind@gmail.com> wrote:
> As you said, i modified like below patch and it is working well.
>
> Thanks for your advice.
>
> diff --git a/drivers/media/usb/au0828/au0828-cards.c
> b/drivers/media/usb/au0828/au0828-cards.c
> index 0cb7c28..9936875 100644
> --- a/drivers/media/usb/au0828/au0828-cards.c
> +++ b/drivers/media/usb/au0828/au0828-cards.c
> @@ -108,7 +108,7 @@ struct au0828_board au0828_boards[] = {
>          .name    = "DViCO FusionHDTV USB",
>          .tuner_type = UNSET,
>          .tuner_addr = ADDR_UNSET,
> -        .i2c_clk_divider = AU0828_I2C_CLK_250KHZ,
> +        .i2c_clk_divider = AU0828_I2C_CLK_20KHZ,
>      },
>      [AU0828_BOARD_HAUPPAUGE_WOODBURY] = {
>          .name = "Hauppauge Woodbury",

Great.  Feel free to submit a patch to the mailing list with your SOB,
and we'll merge that change upstream.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com