linux-kernel - Re: [PATCH 2/2] iio: dac: adding support for Microchip MCP47FEB02

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: <ffbb73aa801111ff0a645cbd2a5c3d03db70ab3a.camel@gmail.com>
Date: Mon, 29 Sep 2025 06:44:30 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: David Lechner <dlechner@...libre.com>, Ariana Lazar
 <ariana.lazar@...rochip.com>, Nuno Sá
 <nuno.sa@...log.com>,  Andy Shevchenko <andy@...nel.org>, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>,  linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org,  linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] iio: dac: adding support for Microchip MCP47FEB02

On Sat, 2025-09-27 at 18:13 +0100, Jonathan Cameron wrote:
> On Tue, 23 Sep 2025 09:21:30 +0100
> Nuno Sá <noname.nuno@...il.com> wrote:
> 
> > On Mon, 2025-09-22 at 17:15 -0500, David Lechner wrote:
> > > On 9/22/25 3:10 PM, Nuno Sá wrote:  
> > > > Hi Ariana,
> > > > 
> > > > Thanks for your patches. Some initial comments from me...
> > > > 
> > > > On Mon, 2025-09-22 at 14:30 +0300, Ariana Lazar wrote:  
> > > 
> > > ...
> > >   
> > > > > +static IIO_DEVICE_ATTR(store_eeprom, 0200, NULL, mcp47feb02_store_eeprom,
> > > > > 0);
> > > > > +static struct attribute *mcp47feb02_attributes[] = {
> > > > > +	&iio_dev_attr_store_eeprom.dev_attr.attr,
> > > > > +	NULL,
> > > > > +};
> > > > > +  
> > > > 
> > > > Not going to argue about the ABI for now but I don't think this is a
> > > > standard one? So
> > > > if acceptable you need an ABI doc.
> 
> store_eeprom is existing ABI and documented in sysfs-bus-iio (2 drivers implement
> it from
> a quick grep)
> 

Ack. Next time I should bother in grepping the docs :)

> 
> > > >   
> > > Here's a random idea. (I would wait for Jonathan to weigh in first before
> > > assuming it is an acceptable idea though :-p)
> > > 
> > > The config registers are pretty much going to be a one-time deal. So those
> > > could be written to only if they need it during probe.
> > > 
> > > For the voltage output registers, we could add extra out_voltageY channels
> > > that are the power-on output state channels. So writing to out_voltageY_raw
> > > wouldn't change any real output but would just be written to EEPROM. This
> > > way these voltages could be controlled independently from the real outputs
> > > and it uses existing ABI.
> 
> In some devices I've come across, the eeprom write is a 'store all current
> settings'
> rather than individual register writes. For that a set of extra channels doesn't
> work.
> 
> Also eeproms have very limited write cycles so you really don't want to make this
> too easy to do and we want to shout it's an eeprom. 
> 
> 
> > > 
> > > In any case, it would be interesting to hear more about how this chips are
> > > actually used to better understand this EEPROM feature.  
> > 
> > I didn't really looked at the datasheet so this can be totally wrong. But we
> > have some LTC parts (mainly hwmon stuff) that are also packed with an EEPRON.
> > AFAIU, the usecase in there is to have some defaults you can program in the
> > chips (and there's a feature we can enable so the chip can save things into the
> > eeprom automatically). Now, in those drivers we don't really support doing
> > anything with the eeprom at runtime so I'm curious to see how this unfolds :)
> 
> Usecase for DACs is that on power on (usually at board power up not driver load)
> they will start outputting the saved values.  That might well be part of something
> fairly critical such as fan control or trip points so you don't want to wait for
> the driver to load. The driver on an eeprom equiped part should not configure
> anything on probe but rather just report back what was already there.
> 
> Userspace can then modify those values and 'commit' them via store_eeprom
> to apply on next power cycle as well as now (in some cases only on next power
> cycle).

Makes sense and it is a similar case of we have in hwmon given those LTC chips are
often controlling power of some fundamental blocks of the system.

- Nuno Sá
> 
> Jonathan
> 
> 
> > 
> > - Nuno Sá
>