2021-05-14 10:04:15

by Bixuan Cui

Subject: [PATCH -next v2] staging: comedi: Remove unused variable ‘min_full_scale’ and function 'get _min_full_scales'

The variable ‘min_full_scale’ and function 'get_min_full_scales' are
not used, So delete them.

Signed-off-by: Bixuan Cui <[email protected]>
---
Changes from v2:
* Delete function 'get_min_full_scales'

drivers/comedi/drivers/jr3_pci.c | 15 ---------------
1 file changed, 15 deletions(-)

diff --git a/drivers/comedi/drivers/jr3_pci.c b/drivers/comedi/drivers/jr3_pci.c
index 7a02c4fa3cda..f963080dd61f 100644
--- a/drivers/comedi/drivers/jr3_pci.c
+++ b/drivers/comedi/drivers/jr3_pci.c
@@ -186,19 +186,6 @@ static void set_full_scales(struct jr3_sensor __iomem *sensor,
set_s16(&sensor->command_word0, 0x0a00);
}

-static struct six_axis_t get_min_full_scales(struct jr3_sensor __iomem *sensor)
-{
- struct six_axis_t result;
-
- result.fx = get_s16(&sensor->min_full_scale.fx);
- result.fy = get_s16(&sensor->min_full_scale.fy);
- result.fz = get_s16(&sensor->min_full_scale.fz);
- result.mx = get_s16(&sensor->min_full_scale.mx);
- result.my = get_s16(&sensor->min_full_scale.my);
- result.mz = get_s16(&sensor->min_full_scale.mz);
- return result;
-}
-
static struct six_axis_t get_max_full_scales(struct jr3_sensor __iomem *sensor)
{
struct six_axis_t result;
@@ -504,10 +491,8 @@ jr3_pci_poll_subdevice(struct comedi_subdevice *s)
result = poll_delay_min_max(20, 100);
} else {
/* Set full scale */
- struct six_axis_t min_full_scale;
struct six_axis_t max_full_scale;

- min_full_scale = get_min_full_scales(sensor);
max_full_scale = get_max_full_scales(sensor);
set_full_scales(sensor, max_full_scale);

--
2.17.1

2021-05-14 14:16:13

by Greg Kroah-Hartman

Subject: Re: [PATCH -next v2] staging: co medi: Remove unused variable ‘min_full_scale’ and function 'get_min_full_scales'

On Fri, May 14, 2021 at 04:52:14PM +0800, Bixuan Cui wrote:
> The variable ‘min_full_scale’ and function 'get_min_full_scales' are
> not used, So delete them.
>
> Signed-off-by: Bixuan Cui <[email protected]>
> ---
> Changes from v2:
> * Delete function 'get_min_full_scales'
>
> drivers/comedi/drivers/jr3_pci.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/drivers/comedi/drivers/jr3_pci.c b/drivers/comedi/drivers/jr3_pci.c
> index 7a02c4fa3cda..f963080dd61f 100644
> --- a/drivers/comedi/drivers/jr3_pci.c
> +++ b/drivers/comedi/drivers/jr3_pci.c
> @@ -186,19 +186,6 @@ static void set_full_scales(struct jr3_sensor __iomem *sensor,
> set_s16(&sensor->command_word0, 0x0a00);
> }
>
> -static struct six_axis_t get_min_full_scales(struct jr3_sensor __iomem *sensor)
> -{
> - struct six_axis_t result;
> -
> - result.fx = get_s16(&sensor->min_full_scale.fx);
> - result.fy = get_s16(&sensor->min_full_scale.fy);
> - result.fz = get_s16(&sensor->min_full_scale.fz);
> - result.mx = get_s16(&sensor->min_full_scale.mx);
> - result.my = get_s16(&sensor->min_full_scale.my);
> - result.mz = get_s16(&sensor->min_full_scale.mz);
> - return result;

Are you _SURE_ that this is ok to do? You are reading from the
hardware, and that is sometimes required depending on the
internal-to-the-device state machine.

Have you tested to verify that not doing these reads does not break
something?

This is a constant theme in these "remove variables that are not being
used" patches, please be aware that how hardware works is very specific
and when you mess with how it is accessed, that has the chance of
causing problems.

thanks,

greg k-h

2021-05-17 17:13:24

by Ian Abbott

Subject: Re: [PATCH -next v2] staging: comedi: Remove un used variable ‘min_full_scale’ and function 'get_min_full_scales'

On 14/05/2021 09:52, Bixuan Cui wrote:
> The variable ‘min_full_scale’ and function 'get_min_full_scales' are
> not used, So delete them.
>
> Signed-off-by: Bixuan Cui <[email protected]>
> ---
> Changes from v2:
> * Delete function 'get_min_full_scales'
>
> drivers/comedi/drivers/jr3_pci.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/drivers/comedi/drivers/jr3_pci.c b/drivers/comedi/drivers/jr3_pci.c
> index 7a02c4fa3cda..f963080dd61f 100644
> --- a/drivers/comedi/drivers/jr3_pci.c
> +++ b/drivers/comedi/drivers/jr3_pci.c
> @@ -186,19 +186,6 @@ static void set_full_scales(struct jr3_sensor __iomem *sensor,
> set_s16(&sensor->command_word0, 0x0a00);
> }
>
> -static struct six_axis_t get_min_full_scales(struct jr3_sensor __iomem *sensor)
> -{
> - struct six_axis_t result;
> -
> - result.fx = get_s16(&sensor->min_full_scale.fx);
> - result.fy = get_s16(&sensor->min_full_scale.fy);
> - result.fz = get_s16(&sensor->min_full_scale.fz);
> - result.mx = get_s16(&sensor->min_full_scale.mx);
> - result.my = get_s16(&sensor->min_full_scale.my);
> - result.mz = get_s16(&sensor->min_full_scale.mz);
> - return result;
> -}
> -
> static struct six_axis_t get_max_full_scales(struct jr3_sensor __iomem *sensor)
> {
> struct six_axis_t result;
> @@ -504,10 +491,8 @@ jr3_pci_poll_subdevice(struct comedi_subdevice *s)
> result = poll_delay_min_max(20, 100);
> } else {
> /* Set full scale */
> - struct six_axis_t min_full_scale;
> struct six_axis_t max_full_scale;
>
> - min_full_scale = get_min_full_scales(sensor);
> max_full_scale = get_max_full_scales(sensor);
> set_full_scales(sensor, max_full_scale);
>
>

I'm pretty sure this is OK. The min_full_scale registers are just
informational. The driver used to print the values, but not since
commit a1d16659538a ("staging: comedi: jr3_pci: remove noisy printk") so
this code is now redundant.

Reviewed-by: Ian Abbott <[email protected]>

--
-=( Ian Abbott <[email protected]> || MEV Ltd. is a company )=-
-=( registered in England & Wales. Regd. number: 02862268. )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || http://www.mev.co.uk )=-

2021-05-19 18:58:52

by Bixuan Cui

Subject: Re: [PATCH -next v2] staging: comedi: Remove un used variable ‘min_full_scale’ and function 'get_min_full_scales'

On 2021/5/17 17:44, Ian Abbott wrote:
> On 14/05/2021 09:52, Bixuan Cui wrote:
>> The variable ‘min_full_scale’ and function 'get_min_full_scales' are
>> not used, So delete them.
>>
>> Signed-off-by: Bixuan Cui <[email protected]>
>> ---
>> Changes from v2:
>> * Delete function 'get_min_full_scales'
>>
>>   drivers/comedi/drivers/jr3_pci.c | 15 ---------------
>>   1 file changed, 15 deletions(-)
>>
>> diff --git a/drivers/comedi/drivers/jr3_pci.c b/drivers/comedi/drivers/jr3_pci.c
>> index 7a02c4fa3cda..f963080dd61f 100644
>> --- a/drivers/comedi/drivers/jr3_pci.c
>> +++ b/drivers/comedi/drivers/jr3_pci.c
>> @@ -186,19 +186,6 @@ static void set_full_scales(struct jr3_sensor __iomem *sensor,
>>       set_s16(&sensor->command_word0, 0x0a00);
>>   }
>>   -static struct six_axis_t get_min_full_scales(struct jr3_sensor __iomem *sensor)
>> -{
>> -    struct six_axis_t result;
>> -
>> -    result.fx = get_s16(&sensor->min_full_scale.fx);
>> -    result.fy = get_s16(&sensor->min_full_scale.fy);
>> -    result.fz = get_s16(&sensor->min_full_scale.fz);
>> -    result.mx = get_s16(&sensor->min_full_scale.mx);
>> -    result.my = get_s16(&sensor->min_full_scale.my);
>> -    result.mz = get_s16(&sensor->min_full_scale.mz);
>> -    return result;
>> -}
>> -
>>   static struct six_axis_t get_max_full_scales(struct jr3_sensor __iomem *sensor)
>>   {
>>       struct six_axis_t result;
>> @@ -504,10 +491,8 @@ jr3_pci_poll_subdevice(struct comedi_subdevice *s)
>>               result = poll_delay_min_max(20, 100);
>>           } else {
>>               /* Set full scale */
>> -            struct six_axis_t min_full_scale;
>>               struct six_axis_t max_full_scale;
>>   -            min_full_scale = get_min_full_scales(sensor);
>>               max_full_scale = get_max_full_scales(sensor);
>>               set_full_scales(sensor, max_full_scale);
>>  
>
> I'm pretty sure this is OK.  The min_full_scale registers are just informational.  The driver used to print the values, but not since commit a1d16659538a ("staging: comedi: jr3_pci: remove noisy printk") so this code is now redundant.
Thanks for your reply.

Thanks
Bixuan Cui

>
> Reviewed-by: Ian Abbott <[email protected]>
>