Re: [PATCH 2/2] dt-bindings: iio: dac: Add DAC8163

[Lukas Metz]

Thanks a lot for the review. This is my first time submitting a
patch so im grateful for the detailed comments and suggestions.

On Tue, Jun 23, 2026 at 02:17:04PM -0500, David Lechner wrote:
> It is more logical to put the dt-bindings patch first in the series
> before the driver that makes use of it.

I will reorder the commits in v2.

> There are a couple of more SPI properties needed since this is not a "normal"
> SPI device. We can only write and not read because there is no D_OUT pin. So
> 
> spi-rx-bus-width:
>   items:
>     - const: 0
> 
> will describe this. 

I will the add the suggested changes to v2. Are there any other poperties 
i have missed? Same for the other comments regarding vendor-prefix, 
spi-max-frequency, avdd-supply and vref supply name.

> We also want the binding to be complete even if the driver doesn't all of it, so
> `clear-gpios` and `sync-gpios` probably make sense too.

SYNC pin is the chip select pin of the device as described below. In
that case i dont need to add it here right?

> Usually, we don't bother with a property like this since it is redundant.
> If an external reference supply is given, then it gets used, otherwise
> the internal reference is used.

That sounds logical. I will remove the property completly.

> These chips don't appear to have a chip select pin, so this comment
> doesn't make sense to me. More logical would be to just use dac@0
> and reg = <0>; since it should just be ignored.

The SYNC pin on the device acts like a chip select pin.
According to the datasheet: when the pin goes low it enables the input shift
register. At least that was my understanding. On my board i have tested the 
driver with the chip select signal connected to the SYNC pin. The
example comes straight from my own device tree where i have two devices
on the bus. Thats why i used reg<1> here but i can change it to 0 and
remove the comment.
 
> The pin is marked active low in the datasheet, so I would expect
> this to be GPIO_ACTIVE_LOW.

I wasnt sure about that. The pin needs to be held low continuously. I
thought when the pin is marked active low and i initialize the pin with
GPIOD_OUT_LOW the result will be that the pin is held high. To match the
datasheet description seems logical though.