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

[Antti Palosaari <crope@iki.fi>]

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/