linux-kernel - Re: [PATCH v2 3/5] dt-bindings: iio: adc: Add adi,ad4052

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: <vchomz3iazgdmotcs3jskrugi2qmdxyo74t4ruo2fsc7cjwtqb@7rtdmdkxobvg>
Date: Mon, 2 Jun 2025 11:17:39 +0200
From: Jorge Marques <gastmaier@...il.com>
To: David Lechner <dlechner@...libre.com>
Cc: Jorge Marques <jorge.marques@...log.com>, 
	Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>, 
	Michael Hennerich <Michael.Hennerich@...log.com>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Jonathan Corbet <corbet@....net>, Nuno Sá <nuno.sa@...log.com>, 
	Andy Shevchenko <andy@...nel.org>, Uwe Kleine-König <ukleinek@...nel.org>, 
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-doc@...r.kernel.org, linux-pwm@...r.kernel.org
Subject: Re: [PATCH v2 3/5] dt-bindings: iio: adc: Add adi,ad4052

On Tue, Apr 29, 2025 at 10:45:20AM -0500, David Lechner wrote:
> On 4/29/25 8:48 AM, Jorge Marques wrote:
> > Hi David, 
> > 
> > I didn't went through your's and Jonathan's ad4052.c review yet,
> > but for the trigger-source-cells I need to dig deeper and make
> > considerable changes to the driver, as well as hardware tests.
> > My idea was to have a less customizable driver, but I get that it is
> > more interesting to make it user-definable.
> 
> We don't need to make the driver support all possibilities, but the devicetree
> needs to be as complete as possible since it can't be as easily changed in the
> future.
> 

Ack.

I see that the node goes in the spi controller (the parent). To use the
same information in the driver I need to look-up the parent node, then
the node. I don't plan to do that in the version of the driver, just an
observation.

There is something else I want to discuss on the dt-bindings actually.
According to the schema, the spi-max-frequency is:

  > Maximum SPI clocking speed of the device in Hz.

The ad4052 has 2 maximum speeds: Configuration mode (lower) and ADC Mode
(higher, depends on VIO). The solution I came up, to not require a
custom regmap spi bus, is to have spi-max-frequency bound the
Configuration mode speed, and have ADC Mode set by VIO regulator
voltage, through spi_transfer.speed_hz. At the end of the day, both are
bounded by the spi controller maximum speed.

My concern is that having ADC mode speed higher than spi-max-frequency
may be counter-intuitive, still, it allows to achieve the max data sheet
speed considering VIO voltage with the lowest code boilerplate.

Let me know if I can proceed this way before submitting V3.

> ...
> 
> >>
> >> Assuming the diagram at [1] is correct, for SPI offload use, we are missing:
> >>
> >>   #trigger-source-cells:
> >>     const: 2
> >>     description: |
> >>       Output pins used as trigger source.
> >>
> >>       Cell 0 defines which pin:
> >>       * 0 = GP0
> >>       * 1 = GP1
> >>
> >>       Cell 1 defines the event:
> >>       * 0 = Data ready
> >>       * 1 = Min threshold
> >>       * 2 = Max threshold
> >>       * 3 = Either threshold
> >>       * 4 = Device ready
> >>       * 5 = Device enable
> >>       * 6 = Chop control
> >>
> >> Bonus points for adding a header with macros for the arbitrary event values.
> > 
> > In the sense of describing the device and not what the driver does, I
> > believe the proper mapping would be:
> > 
> >   Cell 1 defines the event:
> >   * 0 = Disabled
> >   * 1 = Data ready
> >   * 2 = Min threshold
> >   * 3 = Max threshold
> >   * 4 = Either threshold
> >   * 5 = CHOP control
> >   * 6 = Device enable
> >   * 7 = Device ready (only GP1)
> > 
> > I will investigate further this.
> > 
> >>
> 
> 0 = Disabled doesn't make sense to me. One would just not wire up a
> trigger-source in that case.

Ack.


Regards,
Jorge