* Re: [PATCH 1/3] dt-bindings: gpio: add Axiado SGPIO controller [not found] ` <CAD++jL=yc4rmNELLKUpreUqRbQ1Krg95C-o1xSrnD9Aicm4wgw@mail.gmail.com> @ 2026-05-07 8:05 ` Petar Stepanovic 2026-05-07 9:44 ` Linus Walleij 0 siblings, 1 reply; 5+ messages in thread From: Petar Stepanovic @ 2026-05-07 8:05 UTC (permalink / raw) To: Linus Walleij Cc: Tzu-Hao Wei, Swark Yang, Prasad Bolisetty, Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Harshit Shah, SriNavmani A, linux-gpio, devicetree, linux-arm-kernel, linux-kernel Hi Linus, thank you for the detailed review. I will go through the comments step by step. Replies inline below. On 4/24/2026 9:26 AM, Linus Walleij wrote: > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > Hi Petar, > > thanks for your patch! > > On Tue, Apr 14, 2026 at 3:49 PM Petar Stepanovic <pstepanovic@axiado.com> wrote: > >> Add device tree binding for the Axiado SGPIO controller. >> >> The SGPIO controller provides a serialized interface for >> controlling multiple GPIO signals over a limited number of >> physical lines. It supports configurable data direction and >> interrupt handling. >> >> The binding describes the properties required to instantiate >> the controller and register it as a GPIO provider. >> >> Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com> > (...) > >> +description: | >> + The SGPIO controller provides a serialized interface for controlling >> + multiple GPIO signals over a limited number of physical lines. >> + It supports configurable data direction and interrupt handling. > This is pretty generic, can you write some details on how this happens? Yes, I will improve the description and add details about the SGPIO operation and data flow. > >> + '#gpio-cells': >> + const: 2 > Are you sure you don't want to use 3 here instead and split the 128 > GPIOs into 4 "banks" second cell being the bank number? > <&gpio 2 4>; ? > > Maybe this also solves the 512 GPIO by grouping the GPIOs into > 8 banks...? Thank you for the suggestion. We would prefer to keep #gpio-cells = <2> to stay aligned with existing SGPIO drivers and current DTS usage. A single linear offset is sufficient to identify each GPIO, so introducing a bank cell would add additional complexity without a clear benefit. Any internal bank handling can remain within the driver if needed. >> + '#interrupt-cells': >> + const: 2 > Same there. > >> + design-variant: >> + description: SGPIO design variant size in bits (e.g. 128 or 512). >> + enum: [128, 512] >> + $ref: /schemas/types.yaml#/definitions/uint32 > Just use two different compatible strings and infer the variant from > that string instead. > >> + ngpios: >> + description: The number of gpios this controller has. >> + $ref: /schemas/types.yaml#/definitions/uint32 > Same here, certainly the 128 variant has 128 gpios and > the 512 has 512 GPIOs? Just use the compatible string > to infer this. This seems to be platform-specific rather than strictly hardware-dependent. We were considering keeping it as a separate property (possibly renamed to |axiado,sgpio-ngpios|). Would you prefer that, or deriving it from the compatible string? >> + bus-frequency: >> + description: The SGPIO shift clock frequency in Hz. >> + $ref: /schemas/types.yaml#/definitions/uint32 > Don't you want to use the clock bindings and a clk property > for this? Yes, I will rework this to use the standard clock binding instead of a custom bus-frequency propert >> + apb-frequency: >> + description: The APB bus frequency in Hz. >> + $ref: /schemas/types.yaml#/definitions/uint32 > Dito. > >> + dout-init: >> + description: Initial values for the dout registers. >> + $ref: /schemas/types.yaml#/definitions/uint32-array >> + minItems: 4 >> + maxItems: 4 > In: > Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml > > you find: > > lines-initial-states: > $ref: /schemas/types.yaml#/definitions/uint32 > description: > Bitmask that specifies the initial state of each line. > When a bit is set to zero, the corresponding line will be initialized to > the input (pulled-up) state. > When the bit is set to one, the line will be initialized to the > low-level output state. > If the property is not specified all lines will be initialized to the > input state. > > If this is what you want, use this standard binding instead. In our case, the hardware provides dedicated DOUT registers where each bit directly controls the output level (0 = low, 1 = high). The lines-initial-states property also encodes input state semantics, so it does not map directly to this hardware. Would you prefer adapting to lines-initial-states despite this, or using a separate property for output initialization? > > Yours, > Linus Walleij Thanks again for the guidance. Best regards, Petar Stepanovic ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] dt-bindings: gpio: add Axiado SGPIO controller 2026-05-07 8:05 ` [PATCH 1/3] dt-bindings: gpio: add Axiado SGPIO controller Petar Stepanovic @ 2026-05-07 9:44 ` Linus Walleij 2026-05-08 7:57 ` Petar Stepanovic 0 siblings, 1 reply; 5+ messages in thread From: Linus Walleij @ 2026-05-07 9:44 UTC (permalink / raw) To: Petar Stepanovic Cc: Tzu-Hao Wei, Swark Yang, Prasad Bolisetty, Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Harshit Shah, SriNavmani A, linux-gpio, devicetree, linux-arm-kernel, linux-kernel Hi Petar, On Thu, May 7, 2026 at 10:06 AM Petar Stepanovic <pstepanovic@axiado.com> wrote: > >> + '#gpio-cells': > >> + const: 2 > > Are you sure you don't want to use 3 here instead and split the 128 > > GPIOs into 4 "banks" second cell being the bank number? > > <&gpio 2 4>; ? > > > > Maybe this also solves the 512 GPIO by grouping the GPIOs into > > 8 banks...? > > Thank you for the suggestion. We would prefer to keep #gpio-cells = <2> > to stay aligned with existing SGPIO drivers and current DTS usage. > A single linear offset is sufficient to identify each GPIO, so introducing a > bank cell would add additional complexity without a clear benefit. > Any internal bank handling can remain within the driver if needed. If each bank also has its own associated IRQ line, for instance, then this also reflects the hardware in a better way. But it seems this controller has just one single IRQ line for all GPIOs, so maybe this is better. > >> + ngpios: > >> + description: The number of gpios this controller has. > >> + $ref: /schemas/types.yaml#/definitions/uint32 > > > > Same here, certainly the 128 variant has 128 gpios and > > the 512 has 512 GPIOs? Just use the compatible string > > to infer this. > > This seems to be platform-specific rather than strictly hardware-dependent. > We were considering keeping it as a separate property (possibly renamed to |axiado,sgpio-ngpios|). > Would you prefer that, or deriving it from the compatible string? In this case it is fine to use ngpios. ngpios is used when the hardware can actually do more GPIO lines, but they are not routed out on the package of the silicon, for example. > >> + dout-init: > >> + description: Initial values for the dout registers. > >> + $ref: /schemas/types.yaml#/definitions/uint32-array > >> + minItems: 4 > >> + maxItems: 4 > > In: > > Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml > > > > you find: > > > > lines-initial-states: > > $ref: /schemas/types.yaml#/definitions/uint32 > > description: > > Bitmask that specifies the initial state of each line. > > When a bit is set to zero, the corresponding line will be initialized to > > the input (pulled-up) state. > > When the bit is set to one, the line will be initialized to the > > low-level output state. > > If the property is not specified all lines will be initialized to the > > input state. > > > > If this is what you want, use this standard binding instead. > > In our case, the hardware provides dedicated DOUT registers where > each bit directly controls the output level (0 = low, 1 = high). > > The lines-initial-states property also encodes input state semantics, > so it does not map directly to this hardware. > > Would you prefer adapting to lines-initial-states despite this, > or using a separate property for output initialization? Please use lines-initial-states, support also input mode setting and write more than one register if necessary. Setting up the dout-states for lines which are supposed to be used as inputs just doesn't make sense does it? It is better if the device tree has this deeper semantic which provides useful information for the developer and makes the author of the device tree be more careful and detail-oriented around the actual usecase. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] dt-bindings: gpio: add Axiado SGPIO controller 2026-05-07 9:44 ` Linus Walleij @ 2026-05-08 7:57 ` Petar Stepanovic 2026-05-11 8:36 ` Linus Walleij 0 siblings, 1 reply; 5+ messages in thread From: Petar Stepanovic @ 2026-05-08 7:57 UTC (permalink / raw) To: Linus Walleij Cc: Tzu-Hao Wei, Swark Yang, Prasad Bolisetty, Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Harshit Shah, SriNavmani A, linux-gpio, devicetree, linux-arm-kernel, linux-kernel Hi Linus, Thanks for the feedback. On 5/7/2026 11:44 AM, Linus Walleij wrote: > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > Hi Petar, > > On Thu, May 7, 2026 at 10:06 AM Petar Stepanovic <pstepanovic@axiado.com> wrote: > >>>> + '#gpio-cells': >>>> + const: 2 >>> Are you sure you don't want to use 3 here instead and split the 128 >>> GPIOs into 4 "banks" second cell being the bank number? >>> <&gpio 2 4>; ? >>> >>> Maybe this also solves the 512 GPIO by grouping the GPIOs into >>> 8 banks...? >> Thank you for the suggestion. We would prefer to keep #gpio-cells = <2> >> to stay aligned with existing SGPIO drivers and current DTS usage. >> A single linear offset is sufficient to identify each GPIO, so introducing a >> bank cell would add additional complexity without a clear benefit. >> Any internal bank handling can remain within the driver if needed. > If each bank also has its own associated IRQ line, for instance, then > this also reflects the hardware in a better way. But it seems this > controller has just one single IRQ line for all GPIOs, so maybe > this is better. > >>>> + ngpios: >>>> + description: The number of gpios this controller has. >>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> Same here, certainly the 128 variant has 128 gpios and >>> the 512 has 512 GPIOs? Just use the compatible string >>> to infer this. >> This seems to be platform-specific rather than strictly hardware-dependent. >> We were considering keeping it as a separate property (possibly renamed to |axiado,sgpio-ngpios|). >> Would you prefer that, or deriving it from the compatible string? > In this case it is fine to use ngpios. > > ngpios is used when the hardware can actually do more > GPIO lines, but they are not routed out on the package of > the silicon, for example. > >>>> + dout-init: >>>> + description: Initial values for the dout registers. >>>> + $ref: /schemas/types.yaml#/definitions/uint32-array >>>> + minItems: 4 >>>> + maxItems: 4 >>> In: >>> Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml >>> >>> you find: >>> >>> lines-initial-states: >>> $ref: /schemas/types.yaml#/definitions/uint32 >>> description: >>> Bitmask that specifies the initial state of each line. >>> When a bit is set to zero, the corresponding line will be initialized to >>> the input (pulled-up) state. >>> When the bit is set to one, the line will be initialized to the >>> low-level output state. >>> If the property is not specified all lines will be initialized to the >>> input state. >>> >>> If this is what you want, use this standard binding instead. >> In our case, the hardware provides dedicated DOUT registers where >> each bit directly controls the output level (0 = low, 1 = high). >> >> The lines-initial-states property also encodes input state semantics, >> so it does not map directly to this hardware. >> >> Would you prefer adapting to lines-initial-states despite this, >> or using a separate property for output initialization? > Please use lines-initial-states, support also input mode setting > and write more than one register if necessary. > > Setting up the dout-states for lines which are supposed to be used > as inputs just doesn't make sense does it? > > It is better if the device tree has this deeper semantic which > provides useful information for the developer and makes the > author of the device tree be more careful and detail-oriented > around the actual usecase. For example, when SGPIO is configured for 128 lines, the hardware provides 128 input bits (DIN) and 128 output bits (DOUT). If modeled directly, this corresponds to 256 GPIOs in Linux, since the input and output signals are independent and are not bidirectional. Similar to the gpio-aspeed-sgpio.c driver, the input and output paths are fixed by hardware and cannot be configured dynamically per line. These are not interchangeable directions of the same GPIO line; they are separate input and output signals. Because of that, combining them into a single logical GPIO abstraction would not accurately represent the hardware model. Because the direction is fixed by hardware, the standard lines-initial-states property, which encodes both direction and initial state, does not map cleanly to this design. For the output lines (DOUT), should their initial values be described in the device tree, or should they be configured by userspace, with the driver only providing default initialization? > > Yours, > Linus Walleij Thanks again for the guidance. Best regards, Petar Stepanovic ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] dt-bindings: gpio: add Axiado SGPIO controller 2026-05-08 7:57 ` Petar Stepanovic @ 2026-05-11 8:36 ` Linus Walleij 2026-05-13 8:59 ` Petar Stepanovic 0 siblings, 1 reply; 5+ messages in thread From: Linus Walleij @ 2026-05-11 8:36 UTC (permalink / raw) To: Petar Stepanovic Cc: Tzu-Hao Wei, Swark Yang, Prasad Bolisetty, Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Harshit Shah, SriNavmani A, linux-gpio, devicetree, linux-arm-kernel, linux-kernel On Fri, May 8, 2026 at 9:57 AM Petar Stepanovic <pstepanovic@axiado.com> wrote: > For example, when SGPIO is configured for 128 lines, the hardware provides > 128 input bits (DIN) and 128 output bits (DOUT). If modeled directly, this > corresponds to 256 GPIOs in Linux, since the input and output signals are > independent and are not bidirectional. I don't get it. Linux internals are modeled after physical GPIO lines, actual rails you can control. ngpios for example means the number of controllable physical lines. What kind of bits exist in some registers does not concern this concept. Please check this presentation page 24 for example: https://www.df.lth.se/~triad/papers/pincontrol.pdf The fact that there exist many weird things inside the SoC doesn't alter the fact that "a GPIO" is an abstraction for a single physical I/O entity such as a line/pad/pin. > Similar to the gpio-aspeed-sgpio.c driver, the input and output paths are > fixed by hardware and cannot be configured dynamically per line. These are > not interchangeable directions of the same GPIO line; Are they connected to the same physical output line/pin or not? That is the only thing that matters. If they in the end control the same physical entitiy, it *is* the same GPIO line from Linux' point of view. > Because the direction is fixed by hardware, the standard > lines-initial-states property, which encodes both direction and initial state, > does not map cleanly to this design. GPIOs with fixed direction is nothing new for Linux, we've had that for ages. I would just have the driver reject configurations that does not apply and bail out. If you absolutely want to enforce the lines-initial-states to match what the hardware can do, then use YAML schema restriuctions on what values can be encoded into that array. > For the output lines (DOUT), should their initial values be described in the > device tree, or should they be configured by userspace, with the driver only > providing default initialization? I don't see why userspace should deal with that. The Linux userspace ABI is for hacking and odd usecases (like industrial). The nominal use is kernel-internal consumers and those must be able to request their GPIOs as well without any userspace shenanigans. But avoiding to deal with initial line states at all is a solution of course. What I don't understand is what purpose this dout-init actually does and why it cannot be set dynamically by the driver at runtime. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] dt-bindings: gpio: add Axiado SGPIO controller 2026-05-11 8:36 ` Linus Walleij @ 2026-05-13 8:59 ` Petar Stepanovic 0 siblings, 0 replies; 5+ messages in thread From: Petar Stepanovic @ 2026-05-13 8:59 UTC (permalink / raw) To: Linus Walleij Cc: Tzu-Hao Wei, Swark Yang, Prasad Bolisetty, Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Harshit Shah, SriNavmani A, linux-gpio, devicetree, linux-arm-kernel, linux-kernel On 5/11/2026 10:36 AM, Linus Walleij wrote: > Are they connected to the same physical output line/pin or > not? That is the only thing that matters. If they in the end control > the same physical entitiy, it *is* the same GPIO line from Linux' > point of view. No, they are not connected to the same physical line/pin. DIN and DOUT are separate physical SGPIO signals in this hardware. DIN signals are input-only and DOUT signals are output-only; they are not bidirectional or interchangeable paths for the same physical pin. So I agree that Linux should model physical GPIO entities rather than internal register bits. My previous wording was not clear enough: the intention was to describe separate physical SGPIO signals, not just separate register fields. >> Because the direction is fixed by hardware, the standard >> lines-initial-states property, which encodes both direction and initial state, >> does not map cleanly to this design. > GPIOs with fixed direction is nothing new for Linux, we've had > that for ages. > > I would just have the driver reject configurations that does > not apply and bail out. > > If you absolutely want to enforce the lines-initial-states to match what the > hardware can do, then use YAML schema restriuctions on what > values can be encoded into that array. > >> For the output lines (DOUT), should their initial values be described in the >> device tree, or should they be configured by userspace, with the driver only >> providing default initialization? > I don't see why userspace should deal with that. The Linux userspace > ABI is for hacking and odd usecases (like industrial). The nominal > use is kernel-internal consumers and those must be able to > request their GPIOs as well without any userspace shenanigans. > > But avoiding to deal with initial line states at all is a solution > of course. > > What I don't understand is what purpose this dout-init actually > does and why it cannot be set dynamically by the driver at runtime. Some SGPIO outputs may control host-critical signals. For example, if the BMC reboots while the host/server remains powered on, changing SGPIO output values during driver initialization could potentially reset or shut down the running host. The purpose of `dout-init` is to provide a deterministic safe output state during SGPIO initialization, before any GPIO consumer has requested the line. That said, if the preferred approach is to preserve the existing hardware DOUT state during probe and only change the value when a GPIO consumer requests the line, I can rework the driver in that direction. Thnaks, Petar ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-13 8:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260414-axiado-ax3000-sgpio-controller-v1-0-b5c7e4c2e69b@axiado.com>
[not found] ` <20260414-axiado-ax3000-sgpio-controller-v1-1-b5c7e4c2e69b@axiado.com>
[not found] ` <CAD++jL=yc4rmNELLKUpreUqRbQ1Krg95C-o1xSrnD9Aicm4wgw@mail.gmail.com>
2026-05-07 8:05 ` [PATCH 1/3] dt-bindings: gpio: add Axiado SGPIO controller Petar Stepanovic
2026-05-07 9:44 ` Linus Walleij
2026-05-08 7:57 ` Petar Stepanovic
2026-05-11 8:36 ` Linus Walleij
2026-05-13 8:59 ` Petar Stepanovic
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox