linux-kernel - Re: [PATCH v5 2/2] iio: temperature: add support for MCP998X

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] [day] [month] [year] [list]

Message-ID: <42a376a9451ed340e0aa7f1c01a16554298c0693.camel@microchip.com>
Date: Wed, 24 Sep 2025 12:47:31 +0000
From: <Victor.Duicu@...rochip.com>
To: <jic23@...nel.org>
CC: <dlechner@...libre.com>, <nuno.sa@...log.com>,
	<linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>, <robh@...nel.org>,
	<linux-kernel@...r.kernel.org>, <andy@...nel.org>, <krzk+dt@...nel.org>,
	<conor+dt@...nel.org>, <Marius.Cristea@...rochip.com>
Subject: Re: [PATCH v5 2/2] iio: temperature: add support for MCP998X

On Sat, 2025-09-20 at 11:55 +0100, Jonathan Cameron wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 

Hi Jonathan,

> On Thu, 18 Sep 2025 14:19:37 +0300
> <victor.duicu@...rochip.com> wrote:
> 
> > From: Victor Duicu <victor.duicu@...rochip.com>
> > 
> > This is the driver for Microchip MCP998X/33 and MCP998XD/33D
> > Multichannel
> > Automotive Temperature Monitor Family.
> > 
> > Signed-off-by: Victor Duicu <victor.duicu@...rochip.com>
> Hi Victor,
> 
> Various minor comments inline.
> Given the build warnings I didn't elect to just tidy these up whilst
> applying.
> Seemed like there was slightly too high a risk of me messing it up!
> Also we have lots of time as IIO is closed for this cycle now.
> 
> Jonathan
> 
...

> 
> 
> > +/**
> > + * Bit flags and their meaning
> 
> As below. I don't think it is worth encoding these in a bitmap. Just
> use
> 5 bools to represent the state.
> 
> > + * @RECD34_ENABLE:           state of Resistance Error
> > Correction(REC) on channels 3 and 4
> > + * @RECD12_ENABLE:           state of Resistance Error
> > Correction(REC) on channels 1 and 2
> > + * @APDD_ENABLE:             state of anti-parallel diode mode
> > + * @RUN_STATE:                       chip is in run state,
> > otherwise is in standby state
> > + * @WAIT_BEFORE_READ:                whether we need to wait a
> > delay before reading a new value
> > + */
> > +#define RECD34_ENABLE                                0
> > +#define RECD12_ENABLE                                1
> > +#define APDD_ENABLE                          2
> > +#define RUN_STATE                            3
> > +#define WAIT_BEFORE_READ                     4
> > +#define USE_PREVIOUS_FREQ                    5
> > +

Considering that I am planning to add new features to this driver,
I think that it would be useful to keep the flags.

> > +#define MCP9982_CHAN(index, si, __address)
> > (                                         \
> Why the outer set of ()?

Without the outer () compiler returns error "Macros with complex values
should be enclosed in parentheses."

> > +     (struct iio_chan_spec)
> > {                                                        \
> > +             .type =
> > IIO_TEMP,                                                       \
> > +             .info_mask_separate =
> > BIT(IIO_CHAN_INFO_RAW),                           \
> > +             .info_mask_shared_by_all_available =
> > BIT(IIO_CHAN_INFO_SAMP_FREQ) |     \
> > +            
> > BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),                  

Kind Regards,
Victor