[PATCH 0/3] iio: frequency: Fix checkpatch warnings

[PATCH 0/3] iio: frequency: Fix checkpatch warnings

[Roberta Dobrescu]

This patchset fixes warnings detected using checkpatch.pl,
such as: useless 'out of memory' message, unnecessary braces around single
statement block, inadequate usage of msleep function.


Roberta Dobrescu (3):
  iio: frequency: Remove 'out of memory' message
  iio: frequency: Remove unnecessary braces around single statement block
  iio: frequency: Use usleep_range instead of msleep

 drivers/iio/frequency/ad9523.c  | 2 +-
 drivers/iio/frequency/adf4350.c | 7 ++-----
 2 files changed, 3 insertions(+), 6 deletions(-)

-- 
1.9.1

[PATCH 1/3] iio: frequency: Remove 'out of memory' message

[Roberta Dobrescu]

This patch fixes the following checkpatch.pl warning:
WARNING: Possible unnecessary 'out of memory' message

Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
---
 drivers/iio/frequency/adf4350.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
index 63a25d9..2b301eb 100644
--- a/drivers/iio/frequency/adf4350.c
+++ b/drivers/iio/frequency/adf4350.c
@@ -387,10 +387,8 @@ static struct adf4350_platform_data *adf4350_parse_dt(struct device *dev)
 	int ret;
 
 	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata) {
-		dev_err(dev, "could not allocate memory for platform data\n");
+	if (!pdata)
 		return NULL;
-	}
 
 	strncpy(&pdata->name[0], np->name, SPI_NAME_SIZE - 1);
 
-- 
1.9.1

Re: [PATCH 1/3] iio: frequency: Remove 'out of memory' message

[Daniel Baluta]

On Tue, Dec 16, 2014 at 11:11 AM, Roberta Dobrescu
<roberta.dobrescu@gmail.com> wrote:
> This patch fixes the following checkpatch.pl warning:
> WARNING: Possible unnecessary 'out of memory' message
>
> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>

Reviewed-by: Daniel Baluta <daniel.baluta@intel.com>

> ---
>  drivers/iio/frequency/adf4350.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
> index 63a25d9..2b301eb 100644
> --- a/drivers/iio/frequency/adf4350.c
> +++ b/drivers/iio/frequency/adf4350.c
> @@ -387,10 +387,8 @@ static struct adf4350_platform_data *adf4350_parse_dt(struct device *dev)
>         int ret;
>
>         pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> -       if (!pdata) {
> -               dev_err(dev, "could not allocate memory for platform data\n");
> +       if (!pdata)
>                 return NULL;
> -       }
>
>         strncpy(&pdata->name[0], np->name, SPI_NAME_SIZE - 1);
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] iio: frequency: Remove 'out of memory' message

[Lars-Peter Clausen]

On 12/16/2014 10:11 AM, Roberta Dobrescu wrote:
> This patch fixes the following checkpatch.pl warning:
> WARNING: Possible unnecessary 'out of memory' message
>
> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

Re: [PATCH 1/3] iio: frequency: Remove 'out of memory' message

[Hartmut Knaack]

Roberta Dobrescu schrieb am 16.12.2014 um 10:11:
> This patch fixes the following checkpatch.pl warning:
> WARNING: Possible unnecessary 'out of memory' message
Hi Roberta,
I think this is slightly more complex. This function is used in adf4350_probe():

	if (spi->dev.of_node) {
		pdata = adf4350_parse_dt(&spi->dev);
		if (pdata == NULL)
			return -EINVAL;

Since pdata will only be NULL in the case, where you dropped the error message,
I think the EINVAL here should be changed to ENOMEM as well.
> 
> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
> ---
>  drivers/iio/frequency/adf4350.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
> index 63a25d9..2b301eb 100644
> --- a/drivers/iio/frequency/adf4350.c
> +++ b/drivers/iio/frequency/adf4350.c
> @@ -387,10 +387,8 @@ static struct adf4350_platform_data *adf4350_parse_dt(struct device *dev)
>  	int ret;
>  
>  	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> -	if (!pdata) {
> -		dev_err(dev, "could not allocate memory for platform data\n");
> +	if (!pdata)
>  		return NULL;
> -	}
>  
>  	strncpy(&pdata->name[0], np->name, SPI_NAME_SIZE - 1);
>  
> 

Re: [PATCH 1/3] iio: frequency: Remove 'out of memory' message

[Daniel Baluta]

On Tue, Dec 16, 2014 at 11:11 AM, Roberta Dobrescu
<roberta.dobrescu@gmail.com> wrote:
> This patch fixes the following checkpatch.pl warning:
> WARNING: Possible unnecessary 'out of memory' message
>
> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
> ---
>  drivers/iio/frequency/adf4350.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
> index 63a25d9..2b301eb 100644
> --- a/drivers/iio/frequency/adf4350.c
> +++ b/drivers/iio/frequency/adf4350.c
> @@ -387,10 +387,8 @@ static struct adf4350_platform_data *adf4350_parse_dt(struct device *dev)
>         int ret;
>
>         pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> -       if (!pdata) {
> -               dev_err(dev, "could not allocate memory for platform data\n");
> +       if (!pdata)
>                 return NULL;
> -       }
>
Based on Harmut's review we can return ERR_PTR(-ENOMEM) here
and then use IS_ERR and PTR_ERR inside adf4350_probe()

>         strncpy(&pdata->name[0], np->name, SPI_NAME_SIZE - 1);
>

thanks,
Daniel.

Re: [PATCH 1/3] iio: frequency: Remove 'out of memory' message

[Lars-Peter Clausen]

On 12/23/2014 09:51 AM, Daniel Baluta wrote:
> On Tue, Dec 16, 2014 at 11:11 AM, Roberta Dobrescu
> <roberta.dobrescu@gmail.com> wrote:
>> This patch fixes the following checkpatch.pl warning:
>> WARNING: Possible unnecessary 'out of memory' message
>>
>> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
>> ---
>>   drivers/iio/frequency/adf4350.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
>> index 63a25d9..2b301eb 100644
>> --- a/drivers/iio/frequency/adf4350.c
>> +++ b/drivers/iio/frequency/adf4350.c
>> @@ -387,10 +387,8 @@ static struct adf4350_platform_data *adf4350_parse_dt(struct device *dev)
>>          int ret;
>>
>>          pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> -       if (!pdata) {
>> -               dev_err(dev, "could not allocate memory for platform data\n");
>> +       if (!pdata)
>>                  return NULL;
>> -       }
>>
> Based on Harmut's review we can return ERR_PTR(-ENOMEM) here
> and then use IS_ERR and PTR_ERR inside adf4350_probe()

This patch is fine as it is. It removes the redundant error message. Every 
thing else should be done in a second patch.

Re: [PATCH 1/3] iio: frequency: Remove 'out of memory' message

[Daniel Baluta]

On Tue, Dec 23, 2014 at 11:07 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 12/23/2014 09:51 AM, Daniel Baluta wrote:
>>
>> On Tue, Dec 16, 2014 at 11:11 AM, Roberta Dobrescu
>> <roberta.dobrescu@gmail.com> wrote:
>>>
>>> This patch fixes the following checkpatch.pl warning:
>>> WARNING: Possible unnecessary 'out of memory' message
>>>
>>> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
>>> ---
>>>   drivers/iio/frequency/adf4350.c | 4 +---
>>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/iio/frequency/adf4350.c
>>> b/drivers/iio/frequency/adf4350.c
>>> index 63a25d9..2b301eb 100644
>>> --- a/drivers/iio/frequency/adf4350.c
>>> +++ b/drivers/iio/frequency/adf4350.c
>>> @@ -387,10 +387,8 @@ static struct adf4350_platform_data
>>> *adf4350_parse_dt(struct device *dev)
>>>          int ret;
>>>
>>>          pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>>> -       if (!pdata) {
>>> -               dev_err(dev, "could not allocate memory for platform
>>> data\n");
>>> +       if (!pdata)
>>>                  return NULL;
>>> -       }
>>>
>> Based on Harmut's review we can return ERR_PTR(-ENOMEM) here
>> and then use IS_ERR and PTR_ERR inside adf4350_probe()
>
>
> This patch is fine as it is. It removes the redundant error message. Every
> thing else should be done in a second patch.

That's better, indeed.

Daniel.

Re: [PATCH 1/3] iio: frequency: Remove 'out of memory' message

[Hartmut Knaack]

Lars-Peter Clausen schrieb am 23.12.2014 um 10:07:
> On 12/23/2014 09:51 AM, Daniel Baluta wrote:
>> On Tue, Dec 16, 2014 at 11:11 AM, Roberta Dobrescu
>> <roberta.dobrescu@gmail.com> wrote:
>>> This patch fixes the following checkpatch.pl warning:
>>> WARNING: Possible unnecessary 'out of memory' message
>>>
>>> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
>>> ---
>>>   drivers/iio/frequency/adf4350.c | 4 +---
>>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
>>> index 63a25d9..2b301eb 100644
>>> --- a/drivers/iio/frequency/adf4350.c
>>> +++ b/drivers/iio/frequency/adf4350.c
>>> @@ -387,10 +387,8 @@ static struct adf4350_platform_data *adf4350_parse_dt(struct device *dev)
>>>          int ret;
>>>
>>>          pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>>> -       if (!pdata) {
>>> -               dev_err(dev, "could not allocate memory for platform data\n");
>>> +       if (!pdata)
>>>                  return NULL;
>>> -       }
>>>
>> Based on Harmut's review we can return ERR_PTR(-ENOMEM) here
>> and then use IS_ERR and PTR_ERR inside adf4350_probe()
> 
> This patch is fine as it is. It removes the redundant error message. Every 
> thing else should be done in a second patch.

I can't follow on this one. From what I see, if memory allocation fails here,
we will get an appropriate error message here, and _probe will pass up -EINVAL,
causing a misleading error message. Where do you see redundancy?

> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/3] iio: frequency: Remove 'out of memory' message

[Lars-Peter Clausen]

On 12/23/2014 09:21 PM, Hartmut Knaack wrote:
> Lars-Peter Clausen schrieb am 23.12.2014 um 10:07:
>> On 12/23/2014 09:51 AM, Daniel Baluta wrote:
>>> On Tue, Dec 16, 2014 at 11:11 AM, Roberta Dobrescu
>>> <roberta.dobrescu@gmail.com> wrote:
>>>> This patch fixes the following checkpatch.pl warning:
>>>> WARNING: Possible unnecessary 'out of memory' message
>>>>
>>>> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
>>>> ---
>>>>    drivers/iio/frequency/adf4350.c | 4 +---
>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
>>>> index 63a25d9..2b301eb 100644
>>>> --- a/drivers/iio/frequency/adf4350.c
>>>> +++ b/drivers/iio/frequency/adf4350.c
>>>> @@ -387,10 +387,8 @@ static struct adf4350_platform_data *adf4350_parse_dt(struct device *dev)
>>>>           int ret;
>>>>
>>>>           pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>>>> -       if (!pdata) {
>>>> -               dev_err(dev, "could not allocate memory for platform data\n");
>>>> +       if (!pdata)
>>>>                   return NULL;
>>>> -       }
>>>>
>>> Based on Harmut's review we can return ERR_PTR(-ENOMEM) here
>>> and then use IS_ERR and PTR_ERR inside adf4350_probe()
>>
>> This patch is fine as it is. It removes the redundant error message. Every
>> thing else should be done in a second patch.
>
> I can't follow on this one. From what I see, if memory allocation fails here,
> we will get an appropriate error message here, and _probe will pass up -EINVAL,
> causing a misleading error message. Where do you see redundancy?

Unless you pass __GFP_NOWARN to k*alloc it will print a error message on its 
own if the allocation fails.

- Lars

Re: [PATCH 1/3] iio: frequency: Remove 'out of memory' message

[Jonathan Cameron]

On 23/12/14 21:46, Lars-Peter Clausen wrote:
> On 12/23/2014 09:21 PM, Hartmut Knaack wrote:
>> Lars-Peter Clausen schrieb am 23.12.2014 um 10:07:
>>> On 12/23/2014 09:51 AM, Daniel Baluta wrote:
>>>> On Tue, Dec 16, 2014 at 11:11 AM, Roberta Dobrescu
>>>> <roberta.dobrescu@gmail.com> wrote:
>>>>> This patch fixes the following checkpatch.pl warning:
>>>>> WARNING: Possible unnecessary 'out of memory' message
>>>>>
>>>>> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
>>>>> ---
>>>>>    drivers/iio/frequency/adf4350.c | 4 +---
>>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
>>>>> index 63a25d9..2b301eb 100644
>>>>> --- a/drivers/iio/frequency/adf4350.c
>>>>> +++ b/drivers/iio/frequency/adf4350.c
>>>>> @@ -387,10 +387,8 @@ static struct adf4350_platform_data *adf4350_parse_dt(struct device *dev)
>>>>>           int ret;
>>>>>
>>>>>           pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>>>>> -       if (!pdata) {
>>>>> -               dev_err(dev, "could not allocate memory for platform data\n");
>>>>> +       if (!pdata)
>>>>>                   return NULL;
>>>>> -       }
>>>>>
>>>> Based on Harmut's review we can return ERR_PTR(-ENOMEM) here
>>>> and then use IS_ERR and PTR_ERR inside adf4350_probe()
>>>
>>> This patch is fine as it is. It removes the redundant error message. Every
>>> thing else should be done in a second patch.
>>
>> I can't follow on this one. From what I see, if memory allocation fails here,
>> we will get an appropriate error message here, and _probe will pass up -EINVAL,
>> causing a misleading error message. Where do you see redundancy?
> 
> Unless you pass __GFP_NOWARN to k*alloc it will print a error message on its own if the allocation fails.
> 
Yes, separate issues - be it the case that they both want fixing.  Anyhow, have applied
this patch to the togreg branch of iio.git

Thanks

[PATCH 2/3] iio: frequency: Remove unnecessary braces around single statement block

[Roberta Dobrescu]

This patch fixes the following checkpatch.pl warning:
WARNING: braces {} are not necessary for single statement blocks

Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
---
 drivers/iio/frequency/adf4350.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
index 2b301eb..10a0dfc 100644
--- a/drivers/iio/frequency/adf4350.c
+++ b/drivers/iio/frequency/adf4350.c
@@ -611,9 +611,8 @@ static int adf4350_remove(struct spi_device *spi)
 	if (st->clk)
 		clk_disable_unprepare(st->clk);
 
-	if (!IS_ERR(reg)) {
+	if (!IS_ERR(reg))
 		regulator_disable(reg);
-	}
 
 	return 0;
 }
-- 
1.9.1

Re: [PATCH 2/3] iio: frequency: Remove unnecessary braces around single statement block

[Daniel Baluta]

On Tue, Dec 16, 2014 at 11:11 AM, Roberta Dobrescu
<roberta.dobrescu@gmail.com> wrote:
> This patch fixes the following checkpatch.pl warning:
> WARNING: braces {} are not necessary for single statement blocks
>
> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>

Reviewed-by: Daniel Baluta <daniel.baluta@intel.com>

> ---
>  drivers/iio/frequency/adf4350.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
> index 2b301eb..10a0dfc 100644
> --- a/drivers/iio/frequency/adf4350.c
> +++ b/drivers/iio/frequency/adf4350.c
> @@ -611,9 +611,8 @@ static int adf4350_remove(struct spi_device *spi)
>         if (st->clk)
>                 clk_disable_unprepare(st->clk);
>
> -       if (!IS_ERR(reg)) {
> +       if (!IS_ERR(reg))
>                 regulator_disable(reg);
> -       }
>
>         return 0;
>  }
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iio: frequency: Remove unnecessary braces around single statement block

[Lars-Peter Clausen]

On 12/16/2014 10:11 AM, Roberta Dobrescu wrote:
> This patch fixes the following checkpatch.pl warning:
> WARNING: braces {} are not necessary for single statement blocks
>
> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

Re: [PATCH 2/3] iio: frequency: Remove unnecessary braces around single statement block

[Jonathan Cameron]

On 18/12/14 16:13, Lars-Peter Clausen wrote:
> On 12/16/2014 10:11 AM, Roberta Dobrescu wrote:
>> This patch fixes the following checkpatch.pl warning:
>> WARNING: braces {} are not necessary for single statement blocks
>>
>> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
> 
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
> 
Applied to the togreg branch of iio.git - initially pushed out as testing
etc etc.

[PATCH 3/3] iio: frequency: Use usleep_range instead of msleep

[Roberta Dobrescu]

This patch fixes the following checkpatch.pl warning:
WARNING: msleep < 20ms can sleep for up to 20ms

Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
---
 drivers/iio/frequency/ad9523.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/frequency/ad9523.c b/drivers/iio/frequency/ad9523.c
index 7c5245d..2e41ca5 100644
--- a/drivers/iio/frequency/ad9523.c
+++ b/drivers/iio/frequency/ad9523.c
@@ -445,7 +445,7 @@ static int ad9523_store_eeprom(struct iio_dev *indio_dev)
 
 	tmp = 4;
 	do {
-		msleep(16);
+		usleep_range(16000, 17000);
 		ret = ad9523_read(indio_dev,
 				  AD9523_EEPROM_DATA_XFER_STATUS);
 		if (ret < 0)
-- 
1.9.1

Re: [PATCH 3/3] iio: frequency: Use usleep_range instead of msleep

[Lars-Peter Clausen]

On 12/16/2014 10:11 AM, Roberta Dobrescu wrote:
> This patch fixes the following checkpatch.pl warning:
> WARNING: msleep < 20ms can sleep for up to 20ms
>
> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
> ---
>   drivers/iio/frequency/ad9523.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/frequency/ad9523.c b/drivers/iio/frequency/ad9523.c
> index 7c5245d..2e41ca5 100644
> --- a/drivers/iio/frequency/ad9523.c
> +++ b/drivers/iio/frequency/ad9523.c
> @@ -445,7 +445,7 @@ static int ad9523_store_eeprom(struct iio_dev *indio_dev)
>
>   	tmp = 4;
>   	do {
> -		msleep(16);
> +		usleep_range(16000, 17000);

I don't think this is exactly timing critical, things will work fine if this 
sleeps a bit longer. If you want to silence the warning for the sake of 
silencing the warning, maybe just replace it with a msleep(20).

>   		ret = ad9523_read(indio_dev,
>   				  AD9523_EEPROM_DATA_XFER_STATUS);
>   		if (ret < 0)
>