public inbox for linux-gpio@vger.kernel.org
 help / color / mirror / Atom feed
From: Chanhong Jung <happycpu@gmail.com>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Chanhong Jung <happycpu@gmail.com>,
	Linus Walleij <linusw@kernel.org>,
	Bartosz Golaszewski <brgl@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Maxime Ripard <mripard@kernel.org>,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/2] dt-bindings: gpio: fairchild,74hc595: add registers-default property
Date: Thu, 23 Apr 2026 19:33:47 +0900	[thread overview]
Message-ID: <20260423103347.4054425-1-happycpu@gmail.com> (raw)
In-Reply-To: <20260423-quantum-precious-mantis-2481d7@quoll>

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@gmail.com>

  reply	other threads:[~2026-04-23 10:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-22 16:05 [PATCH v1 0/2] gpio: 74x164: seed the chain from DT at probe time Chanhong Jung
2026-04-22 16:05 ` [PATCH v1 1/2] dt-bindings: gpio: fairchild,74hc595: add registers-default property Chanhong Jung
2026-04-23  8:59   ` Krzysztof Kozlowski
2026-04-23 10:33     ` Chanhong Jung [this message]
2026-04-22 16:05 ` [PATCH v1 2/2] gpio: 74x164: support 'registers-default' DT property for initial state Chanhong Jung

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260423103347.4054425-1-happycpu@gmail.com \
    --to=happycpu@gmail.com \
    --cc=brgl@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mripard@kernel.org \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox