linux-kernel - Re: [PATCH -next v2] staging: comedi: Remove unused variable ‘min_full_scale’ and function 'get_min_full

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YJ5lEJNaiKfYjFPV@kroah.com>
Date:   Fri, 14 May 2021 13:54:56 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Ian Abbott <abbotti@....co.uk>
Cc:     Bixuan Cui <cuibixuan@...wei.com>, linux-kernel@...r.kernel.org,
        hsweeten@...ionengravers.com, grandmaster@...klimov.de
Subject: Re: [PATCH -next v2] staging: comedi: Remove unused variable ‘min_full_scale’ and function 'get_min_full_scales'

On Fri, May 14, 2021 at 12:16:11PM +0100, Ian Abbott wrote:
> On 14/05/2021 08:52, Greg KH wrote:
> > 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 <cuibixuan@...wei.com>
> > > ---
> > > 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.
> 
> I don't think removing this would do any harm.  The `get_s16` function is
> just a simple wrapper around `readl` that converts the type of the returned
> value to `s16`.  The registers for minimum full scale and maximum full scale
> are just informational, and there is nothing in the manual[1] about reading
> registers in a particular order.  The driver used to print these values to
> the kernel log, but that code was removed in commit a1d16659538a ("staging:
> comedi: jr3_pci: remove noisy printk").
> 
> [1] 5907E - Legacy PCI Receiver Manual
> <https://www.jr3.com/component/phocadownload/category/6-product-manuals?download=36:5907e-legacy-pci-receiver-manual>
> 

Ok, can I get a "reviewed-by:" then?

thanks,

greg k-h