linux-kernel - Re: [PATCH v1 2/2] iio: adc: adding support for pac193x

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
> >   
>