[2/3] mn88472: add work around for failing firmware loading
Commit Message
Sometimes the firmware fails to load because of an i2c error.
Work around that by adding retry logic. This kind of logic
also exist in the binary driver and failures have been observed
there also. Thus this seems to be a property of the hardware
and a fix like this is needed.
Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
---
drivers/staging/media/mn88472/mn88472.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
Comments
Hello
I am not sure if problem is I2C adapter/bus or that demodulator I2C
slave. If it is demod, then that workaround is correct place, but if it
is not, then that is wrong and I2C adapter repeating logic should be used.
I did some testing again... Loading mn88472 firmware 1000 times, it failed:
61 times RC polling disabled
68 times RC polling enabled
83 times RC polling enabled, but repeated failed message due to that patch
I don't want apply that patch until I find some time myself to examine
that problem - or someone else does some study to point out whats wrong.
There is many things to test in order to get better understanding.
regards
Antti
On 11/29/2015 04:10 AM, Benjamin Larsson wrote:
> Sometimes the firmware fails to load because of an i2c error.
> Work around that by adding retry logic. This kind of logic
> also exist in the binary driver and failures have been observed
> there also. Thus this seems to be a property of the hardware
> and a fix like this is needed.
>
> Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
> ---
> drivers/staging/media/mn88472/mn88472.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/mn88472/mn88472.c b/drivers/staging/media/mn88472/mn88472.c
> index cf2e96b..80c5807 100644
> --- a/drivers/staging/media/mn88472/mn88472.c
> +++ b/drivers/staging/media/mn88472/mn88472.c
> @@ -282,7 +282,7 @@ static int mn88472_init(struct dvb_frontend *fe)
> int ret, len, remaining;
> const struct firmware *fw = NULL;
> u8 *fw_file = MN88472_FIRMWARE;
> - unsigned int tmp;
> + unsigned int tmp, retry_cnt;
>
> dev_dbg(&client->dev, "\n");
>
> @@ -330,9 +330,22 @@ static int mn88472_init(struct dvb_frontend *fe)
> if (len > (dev->i2c_wr_max - 1))
> len = dev->i2c_wr_max - 1;
>
> + /* I2C transfers during firmware load might fail sometimes,
> + * just retry in that case. 4 consecutive failures have
> + * been observed thus a retry limit of 20 is used.
> + */
> + retry_cnt = 20;
> +retry:
> ret = regmap_bulk_write(dev->regmap[0], 0xf6,
> &fw->data[fw->size - remaining], len);
> if (ret) {
> + if (retry_cnt) {
> + dev_dbg(&client->dev,
> + "i2c error, retry %d triggered\n", retry_cnt);
> + retry_cnt--;
> + usleep_range(200, 10000);
> + goto retry;
> + }
> dev_err(&client->dev,
> "firmware download failed=%d\n", ret);
> goto firmware_release;
>
On 12/21/2015 03:52 AM, Antti Palosaari wrote:
> Hello
> I am not sure if problem is I2C adapter/bus or that demodulator I2C
> slave. If it is demod, then that workaround is correct place, but if it
> is not, then that is wrong and I2C adapter repeating logic should be used.
>
> I did some testing again... Loading mn88472 firmware 1000 times, it failed:
> 61 times RC polling disabled
> 68 times RC polling enabled
> 83 times RC polling enabled, but repeated failed message due to that patch
At least this confirms there is an issue.
>
> I don't want apply that patch until I find some time myself to examine
> that problem - or someone else does some study to point out whats wrong.
> There is many things to test in order to get better understanding.
>
> regards
> Antti
I do have other hardware with with a mn88472 demod on it. A CX23102
bridge and a dibcom (Xbox one tuner). I think that running the same test
on those hardware will tell where the issue is.
I know that Olli have worked on the Xbox one tuner, do you have any
support patches that could help testing this? And did you observe any
issues with the mn88472 demod when working on the Xbox one tuner ?
I am quite sure that I saw this on the mn88473 version of this hardware
also. I just haven't had the time to test it. But I will postpone that
until the tests on the other bridges are done.
MvH
Benjamin Larsson
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -282,7 +282,7 @@ static int mn88472_init(struct dvb_frontend *fe)
int ret, len, remaining;
const struct firmware *fw = NULL;
u8 *fw_file = MN88472_FIRMWARE;
- unsigned int tmp;
+ unsigned int tmp, retry_cnt;
dev_dbg(&client->dev, "\n");
@@ -330,9 +330,22 @@ static int mn88472_init(struct dvb_frontend *fe)
if (len > (dev->i2c_wr_max - 1))
len = dev->i2c_wr_max - 1;
+ /* I2C transfers during firmware load might fail sometimes,
+ * just retry in that case. 4 consecutive failures have
+ * been observed thus a retry limit of 20 is used.
+ */
+ retry_cnt = 20;
+retry:
ret = regmap_bulk_write(dev->regmap[0], 0xf6,
&fw->data[fw->size - remaining], len);
if (ret) {
+ if (retry_cnt) {
+ dev_dbg(&client->dev,
+ "i2c error, retry %d triggered\n", retry_cnt);
+ retry_cnt--;
+ usleep_range(200, 10000);
+ goto retry;
+ }
dev_err(&client->dev,
"firmware download failed=%d\n", ret);
goto firmware_release;