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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230225172255.7b79374a@jic23-huawei>
Date:   Sat, 25 Feb 2023 17:22:55 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Krzysztof Kozlowski <krzk@...nel.org>
Cc:     marius.cristea@...rochip.com, lars@...afoo.de, robh+dt@...nel.org,
        linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/2] iio: adc: adding support for pac193x

On Sat, 25 Feb 2023 17:19:54 +0000
Jonathan Cameron <jic23@...nel.org> wrote:

> On Tue, 21 Feb 2023 14:46:08 +0100
> Krzysztof Kozlowski <krzk@...nel.org> wrote:
> 
> > On 20/02/2023 13:32, marius.cristea@...rochip.com wrote:  
> > > From: Marius Cristea <marius.cristea@...rochip.com>
> > > 
> > > This is the iio driver for Microchip
> > > PAC193X series of Power Monitor with Accumulator chip family.
> > > 
> > > Signed-off-by: Marius Cristea <marius.cristea@...rochip.com>
> > > ---
> > >  MAINTAINERS               |    7 +
> > >  drivers/iio/adc/Kconfig   |   12 +
> > >  drivers/iio/adc/Makefile  |    1 +
> > >  drivers/iio/adc/pac193x.c | 2072 +++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 2092 insertions(+)
> > >  create mode 100644 drivers/iio/adc/pac193x.c
> > >     
> > 
> > 
> > Thank you for your patch. There is something to discuss/improve.
> >   
> > > +
> > > +#define PAC193X_NEG_PWR_CH1_BIDI(x)	((x) ? BIT(7) : 0)
> > > +#define PAC193X_NEG_PWR_CH2_BIDI(x)	((x) ? BIT(6) : 0)
> > > +#define PAC193X_NEG_PWR_CH3_BIDI(x)	((x) ? BIT(5) : 0)
> > > +#define PAC193X_NEG_PWR_CH4_BIDI(x)	((x) ? BIT(4) : 0)
> > > +#define PAC193X_NEG_PWR_CH1_BIDV(x)	((x) ? BIT(3) : 0)
> > > +#define PAC193X_NEG_PWR_CH2_BIDV(x)	((x) ? BIT(2) : 0)
> > > +#define PAC193X_NEG_PWR_CH3_BIDV(x)	((x) ? BIT(1) : 0)
> > > +#define PAC193X_NEG_PWR_CH4_BIDV(x)	((x) ? BIT(0) : 0)
> > > +
> > > +/*
> > > + * Universal Unique Identifier (UUID),
> > > + * 033771E0-1705-47B4-9535-D1BBE14D9A09, is
> > > + * reserved to Microchip for the PAC193X and must not be changed
> > > + */
> > > +#define PAC193X_DSM_UUID		"033771E0-1705-47B4-9535-D1BBE14D9A09"
> > > +
> > > +enum pac193x_ids {
> > > +	pac1934,
> > > +	pac1933,
> > > +	pac1932,
> > > +	pac1931    
> > 
> > Enums are usually uppercase.  
> 
> I'm not sure there is anything in coding standard around that and a grep finds
> a mixture of the two when it comes to ones used for IDs.  Mind you uppercase
> is fine :)
I take it back. Is indeed in coding style doc.  Glad checkpatch doesn't check for
this though as we'd get 1000s of 'fixes' if it did and in most cases it doesn't
hurt readability.  I'll try and be more consistent on this in review going forwards!

Thanks!

Jonathan

> 
> 
> > 
> > 
> > 
> > Best regards,
> > Krzysztof
> >   
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ