Re: [PATCH v1 1/2] dt-bindings: gpio: fairchild,74hc595: add registers-default property

[Chanhong Jung]

Hi Krzysztof,

Thanks for the review.

> You can fix the driver not to do that. Or fix the driver to properly
> parse hogs. I don't understand why exactly this one driver needs
> default registers stored in DT, but all other drivers in the kernel
> don't.

I thought about both, but I don't think either works for this chip.

74HC595/74LVC594 is pure write-only: no MISO path, no register
read-back, and the data sheet defines the power-on state of the
parallel outputs as whatever /SRCLR and /OE do before the SPI master
is up. That's different from MCP23xxx, PCA953x, STMPE and the rest,
which are either readable (so the driver can RMW) or have a defined
reset value. None of those drivers need a seed property. This one
does, because there is nothing to read back and no defined state to
fall back on.

Partial updates aren't possible either. set_value() has to shift the
full chain every time - see __gen_74x164_write_config() - so whatever
is in chip->buffer at the first ->set() call gets published to every
output at once. If probe doesn't seed the buffer, the first
consumer's set() runs off the kzalloc'd 0x00 and publishes that.
On the board that motivated this series there are 16 active-low
indicator LEDs on the chain; the 0x00 publish briefly lights every
one of them before gpio-leds finishes walking its children. That
glitch is what the probe-time write was preventing.

gpio-hog isn't a substitute here either:

  - gpio_request() refuses the second claim on a line (EBUSY), so a
    hog and a gpio-leds / reset-gpios consumer can't coexist on the
    same line. This board already has gpio-leds consumers on every
    output of the chain, so there is no free line to hog. In general,
    if a line has no consumer its initial state doesn't matter; if
    it does have one, you can't put a hog on it.

  - Each hog is applied sequentially by of_gpiochip_scan_gpios(), and
    because partial updates aren't possible every hog produces a
    full-chain write from an intermediate shadow. Seeding
    {0xFF 0xFF 0x00 0x00} through 16 hogs walks the physical pins
    through 16 intermediate patterns. A u8-array property collapses
    that to one atomic 4-byte transfer on the first SPI transaction.

  - bindings/gpio/gpio.txt describes gpio-hog as a mechanism for
    lines no driver claims. Using it to paper over a missing
    probe-time seed is the wrong semantic for a chip whose only
    hardware primitive is "shift the whole chain", not "set one bit".

On the vendor prefix you're right - "registers-default" is too
generic. In v2 I'll rename it to "fairchild,registers-default"
(scoped to the fairchild,74hc595 compatible), which addresses the
can-of-worms concern without changing the semantics. If you'd prefer
a more descriptive name, "fairchild,initial-latch-state" or
"fairchild,power-on-pattern" both work for me - let me know which
one you'd ack and I'll send v2 accordingly.

Best regards,
Chanhong Jung
<happycpu@xxxxxxxxx>