[PATCH 1/7] af9015 Slow down download firmware

[PATCH 1/7] af9015 Slow down download firmware

[Malcolm Priestley]

It is noticed that sometimes the device fails to download parts of the firmware.

Since there is no ack from firmware write a 250u second delay has been added.

Signed-off-by: Malcolm Priestley <tvboxspy@gmail.com>
---
 drivers/media/dvb/dvb-usb/af9015.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
index c6c275b..dc6e4ec 100644
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -698,6 +698,7 @@ static int af9015_download_firmware(struct usb_device *udev,
 			err("firmware download failed:%d", ret);
 			goto error;
 		}
+		udelay(250);
 	}
 
 	/* firmware loaded, request boot */
-- 
1.7.5.4

Re: [PATCH 1/7] af9015 Slow down download firmware

[Antti Palosaari]

On 11/12/2011 05:54 PM, Malcolm Priestley wrote:
> It is noticed that sometimes the device fails to download parts of the firmware.
>
> Since there is no ack from firmware write a 250u second delay has been added.
>
> Signed-off-by: Malcolm Priestley<tvboxspy@gmail.com>
> ---
>   drivers/media/dvb/dvb-usb/af9015.c |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
> index c6c275b..dc6e4ec 100644
> --- a/drivers/media/dvb/dvb-usb/af9015.c
> +++ b/drivers/media/dvb/dvb-usb/af9015.c
> @@ -698,6 +698,7 @@ static int af9015_download_firmware(struct usb_device *udev,
>   			err("firmware download failed:%d", ret);
>   			goto error;
>   		}
> +		udelay(250);
>   	}
>
>   	/* firmware loaded, request boot */

That sleep is not critical as all, so defining it as udelay() is wrong 
in my understanding. Refer Kernel documentation about delays.

regards
Antti

-- 
http://palosaari.fi/

Re: [PATCH 1/7] af9015 Slow down download firmware

[Malcolm Priestley]

On Sat, 2011-11-12 at 18:14 +0200, Antti Palosaari wrote:
> On 11/12/2011 05:54 PM, Malcolm Priestley wrote:
> > It is noticed that sometimes the device fails to download parts of the firmware.
> >
> > Since there is no ack from firmware write a 250u second delay has been added.
> >
> > Signed-off-by: Malcolm Priestley<tvboxspy@gmail.com>
> > ---
> >   drivers/media/dvb/dvb-usb/af9015.c |    1 +
> >   1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
> > index c6c275b..dc6e4ec 100644
> > --- a/drivers/media/dvb/dvb-usb/af9015.c
> > +++ b/drivers/media/dvb/dvb-usb/af9015.c
> > @@ -698,6 +698,7 @@ static int af9015_download_firmware(struct usb_device *udev,
> >   			err("firmware download failed:%d", ret);
> >   			goto error;
> >   		}
> > +		udelay(250);
> >   	}
> >
> >   	/* firmware loaded, request boot */
> 
> That sleep is not critical as all, so defining it as udelay() is wrong 
> in my understanding. Refer Kernel documentation about delays.

So we just go faster and faster, without acknowledgements and  due
respect for the hardware?

Typical download time is about 100ms, download on some systems was less
than 50ms and failing.

A 250uS wait brought the time back up to arround 100ms.

Regards

Malcolm

Re: [PATCH 1/7] af9015 Slow down download firmware

[Antti Palosaari]

On 11/12/2011 08:10 PM, Malcolm Priestley wrote:
> On Sat, 2011-11-12 at 18:14 +0200, Antti Palosaari wrote:
>> On 11/12/2011 05:54 PM, Malcolm Priestley wrote:
>>> It is noticed that sometimes the device fails to download parts of the firmware.
>>>
>>> Since there is no ack from firmware write a 250u second delay has been added.
>>>
>>> Signed-off-by: Malcolm Priestley<tvboxspy@gmail.com>
>>> ---
>>>    drivers/media/dvb/dvb-usb/af9015.c |    1 +
>>>    1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
>>> index c6c275b..dc6e4ec 100644
>>> --- a/drivers/media/dvb/dvb-usb/af9015.c
>>> +++ b/drivers/media/dvb/dvb-usb/af9015.c
>>> @@ -698,6 +698,7 @@ static int af9015_download_firmware(struct usb_device *udev,
>>>    			err("firmware download failed:%d", ret);
>>>    			goto error;
>>>    		}
>>> +		udelay(250);
>>>    	}
>>>
>>>    	/* firmware loaded, request boot */
>>
>> That sleep is not critical as all, so defining it as udelay() is wrong
>> in my understanding. Refer Kernel documentation about delays.
>
> So we just go faster and faster, without acknowledgements and  due
> respect for the hardware?
>
> Typical download time is about 100ms, download on some systems was less
> than 50ms and failing.
>
> A 250uS wait brought the time back up to arround 100ms.

I said you should not use udelay() since that sleep is not critical. 
udelay() have bad effect in system level. You didn't looked 
documentation as I asked, here it is: 
Documentation/timers/timers-howto.txt . I think usleep_range() is 
correct function.

Antti


-- 
http://palosaari.fi/