public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: linux-media@vger.kernel.org, Dinesh.Ram@cern.ch,
	edubezval@gmail.com, Hans Verkuil <hans.verkuil@cisco.com>
Subject: Re: [PATCHv2 11/11] si4713: coding style cleanups
Date: Mon, 09 Dec 2013 17:17:41 +0100	[thread overview]
Message-ID: <52A5ED25.4050500@xs4all.nl> (raw)
In-Reply-To: <20131209135144.17aa47f6@samsung.com>

On 12/09/2013 04:51 PM, Mauro Carvalho Chehab wrote:
> Em Fri,  6 Dec 2013 11:17:14 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Fix most checkpatch errors/warnings.
>>
>> It's mostly whitespace changes, except for replacing msleep with
>> usleep_range and the jiffies comparison with time_is_after_jiffies().
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  drivers/media/radio/si4713/radio-usb-si4713.c |   4 +-
>>  drivers/media/radio/si4713/si4713.c           | 104 +++++++++++++-------------
>>  2 files changed, 55 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/media/radio/si4713/radio-usb-si4713.c b/drivers/media/radio/si4713/radio-usb-si4713.c
>> index d978844..691e487 100644
>> --- a/drivers/media/radio/si4713/radio-usb-si4713.c
>> +++ b/drivers/media/radio/si4713/radio-usb-si4713.c
>> @@ -207,7 +207,7 @@ static int si4713_send_startup_command(struct si4713_usb_device *radio)
>>  		}
>>  		if (time_is_before_jiffies(until_jiffies))
>>  			return -EIO;
>> -		msleep(3);
>> +		usleep_range(3000, 5000);
>>  	}
>>  
>>  	return retval;
>> @@ -354,7 +354,7 @@ static int si4713_i2c_read(struct si4713_usb_device *radio, char *data, int len)
>>  			data[0] = 0;
>>  			return 0;
>>  		}
>> -		msleep(3);
>> +		usleep_range(3000, 5000);
>>  	}
>>  }
>>  
>> diff --git a/drivers/media/radio/si4713/si4713.c b/drivers/media/radio/si4713/si4713.c
>> index 6f28a2b..451b9c0 100644
>> --- a/drivers/media/radio/si4713/si4713.c
>> +++ b/drivers/media/radio/si4713/si4713.c
>> @@ -50,12 +50,12 @@ MODULE_VERSION("0.0.1");
>>  #define DEFAULT_RDS_PS_REPEAT_COUNT	0x0003
>>  #define DEFAULT_LIMITER_RTIME		0x1392
>>  #define DEFAULT_LIMITER_DEV		0x102CA
>> -#define DEFAULT_PILOT_FREQUENCY 	0x4A38
>> +#define DEFAULT_PILOT_FREQUENCY		0x4A38
>>  #define DEFAULT_PILOT_DEVIATION		0x1A5E
>>  #define DEFAULT_ACOMP_ATIME		0x0000
>>  #define DEFAULT_ACOMP_RTIME		0xF4240L
>>  #define DEFAULT_ACOMP_GAIN		0x0F
>> -#define DEFAULT_ACOMP_THRESHOLD 	(-0x28)
>> +#define DEFAULT_ACOMP_THRESHOLD		(-0x28)
>>  #define DEFAULT_MUTE			0x01
>>  #define DEFAULT_POWER_LEVEL		88
>>  #define DEFAULT_FREQUENCY		8800
>> @@ -252,8 +252,8 @@ static int si4713_send_command(struct si4713_device *sdev, const u8 command,
>>  
>>  		if (client->irq)
>>  			return -EBUSY;
>> -		msleep(1);
>> -	} while (jiffies <= until_jiffies);
>> +		usleep_range(1000, 2000);
>> +	} while (time_is_after_jiffies(until_jiffies));
> 
> Condition seems to be wrong here: it should be time_is_before_jiffies().

No, the condition is good. 'time_is_after_jiffies(until_jiffies)' means
'until_jiffies > jiffies' which is 'jiffies <= until_jiffies'.

> Also, the better is to put this on a separate patch.

OK. 

Regards,

	Hans

  reply	other threads:[~2013-12-09 16:18 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-06 10:17 [PATCHv2 00/11] si4713: add si4713 usb driver Hans Verkuil
2013-12-06 10:17 ` [PATCHv2 01/11] si4713: Reorganized drivers/media/radio directory Hans Verkuil
2013-12-06 10:17 ` [PATCHv2 02/11] si4713: Modified i2c driver to handle cases where interrupts are not used Hans Verkuil
2013-12-09 15:58   ` Mauro Carvalho Chehab
2013-12-06 10:17 ` [PATCHv2 03/11] si4713: Reorganized includes in si4713.c/h Hans Verkuil
2013-12-06 10:17 ` [PATCHv2 04/11] si4713: Bug fix for si4713_tx_tune_power() method in the i2c driver Hans Verkuil
2013-12-06 10:17 ` [PATCHv2 05/11] si4713: HID blacklist Si4713 USB development board Hans Verkuil
2013-12-06 10:17 ` [PATCHv2 06/11] si4713: Added the USB driver for Si4713 Hans Verkuil
2013-12-09 15:47   ` Mauro Carvalho Chehab
2013-12-09 16:22     ` Hans Verkuil
2013-12-13 13:26     ` Hans Verkuil
2013-12-06 10:17 ` [PATCHv2 07/11] si4713: Added MAINTAINERS entry for radio-usb-si4713 driver Hans Verkuil
2013-12-06 10:17 ` [PATCHv2 08/11] si4713: move supply list to si4713_platform_data Hans Verkuil
2013-12-06 10:17 ` [PATCHv2 09/11] si4713: si4713_set_rds_radio_text overwrites terminating \0 Hans Verkuil
2013-12-06 10:17 ` [PATCHv2 10/11] si4713: print product number Hans Verkuil
2013-12-06 10:17 ` [PATCHv2 11/11] si4713: coding style cleanups Hans Verkuil
2013-12-06 13:26   ` edubezval
2013-12-09 15:51   ` Mauro Carvalho Chehab
2013-12-09 16:17     ` Hans Verkuil [this message]
2013-12-09 18:37       ` Mauro Carvalho Chehab
2013-12-09 15:59 ` [PATCHv2 00/11] si4713: add si4713 usb driver Mauro Carvalho Chehab

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52A5ED25.4050500@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=Dinesh.Ram@cern.ch \
    --cc=edubezval@gmail.com \
    --cc=hans.verkuil@cisco.com \
    --cc=linux-media@vger.kernel.org \
    --cc=m.chehab@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox