Re: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for remote GPIO data source

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 5620 bytes --]

On Mon, Apr 27, 2026 at 08:12:10AM +0000, G.N. Zhou (OSS) wrote:
> Hi Conor,
> 
> Thanks for your review.
> 
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Sent: Saturday, April 25, 2026 1:09 AM
> > To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>; Mauro Carvalho
> > Chehab <mchehab@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof
> > Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Frank
> > Li <frank.li@nxp.com>; Vladimir Zapolskiy <vz@mleia.com>; Linus Walleij
> > <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>; linux-
> > media@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org; imx@lists.linux.dev; linux-gpio@vger.kernel.org; G.N.
> > Zhou <guoniu.zhou@nxp.com>
> > Subject: Re: [PATCH v6 1/4] dt-bindings: media: ti,ds90ub953: Add support for
> > remote GPIO data source
> > 
> > On Fri, Apr 24, 2026 at 09:42:24AM +0800, Guoniu Zhou wrote:
> > > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> > >
> > > The DS90UB953 supports four pins, GPIO0 through GPIO3. When enabled as
> > > an output, it can be programed to output local data or remote data
> > > coming from the remote compatible deserializer.
> > >
> > > Add GPIO flag in second cell to select remote GPIO data source.
> > >
> > > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > > ---
> > > Changes in v6:
> > > - Added GPIO_DATA_SOURCE_REMOTE flag to dt-bindings/gpio/gpio.h
> > > - Updated dt-bindings documentation accordingly
> > > - Dropped Reviewed-by tag from Rob Herring due to significant binding
> > > change
> > >
> > > Changes in v5:
> > > - Improve the description for "#gpio-cells" as commented by Conor.
> > >
> > > Changes in v4:
> > > - Use folder block instead of literal block for #gpio-cell property description.
> > >
> > > Changes in v3:
> > > - Make GPIO range from 0-3 to 0-7 to support GPIO data from remote
> > >   compatible deserializer suggested by Rob instead of adding third
> > >   cell for GPIO controller.
> > >
> > > Changes in v2:
> > > - Remove new property ti,gpio-data
> > > - Add third cell for GPIO controller to select GPIO output source.
> > > ---
> > >  Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml | 6 ++++-
> > -
> > >  include/dt-bindings/gpio/gpio.h                               | 8 ++++++++
> > >  2 files changed, 12 insertions(+), 2 deletions(-)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > > b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > > index 2e129bf573b7..da63771bc236 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > > +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
> > > @@ -21,8 +21,10 @@ properties:
> > >    '#gpio-cells':
> > >      const: 2
> > >      description:
> > > -      First cell is the GPIO pin number, second cell is the flags. The GPIO pin
> > > -      number must be in range of [0, 3].
> > > +      First cell is the GPIO pin number (0-3) and the second cell is used
> > > +      to specify flags. See <dt-bindings/gpio/gpio.h> for available flags
> > > +      including GPIO_DATA_SOURCE_REMOTE for remote GPIO data source.
> > > +      Flags can be OR'd together.
> > >
> > >    gpio-controller: true
> > >
> > > diff --git a/include/dt-bindings/gpio/gpio.h
> > > b/include/dt-bindings/gpio/gpio.h index b5d531237448..d04a494d96ad
> > > 100644
> > > --- a/include/dt-bindings/gpio/gpio.h
> > > +++ b/include/dt-bindings/gpio/gpio.h
> > > @@ -42,4 +42,12 @@
> > >  /* Bit 6 express pull disable */
> > >  #define GPIO_PULL_DISABLE 64
> > >
> > > +/*
> > > + * Bit 24 indicates the GPIO data source is from a remote device.
> > 
> > Why 24, not 7?
> 
> Good question. I chose bit 24 for the following reasons:
> - There is no clear guidance in the documentation on how to add custom
>    GPIO flags, so I wanted to ensure sufficient separation from the generic
>    GPIO flags to avoid potential conflicts.
> - The generic GPIO flags currently use the lower bits (0-6). By using
>    bit 24, I'm leaving ample room (bits 7-23) for future expansion of
>    generic flags without risking collision.
> - This also reserves space (bits 25-31) for potential future custom
>    flags that may be needed by this or other drivers.
> 
> However, if there's a preferred convention or range for driver-specific
> GPIO flags that I'm not aware of, I'm happy to adjust this.
> 
> > 
> > > + * This is used in serializer/deserializer setups where the GPIO pin
> > > + * on the local device (e.g., TI DS90UB953 serializer) reflects the
> > > + * state of a GPIO on the remote device (e.g., TI DS90UB960 deserializer).
> > > + */
> > > +#define GPIO_DATA_SOURCE_REMOTE		0x01000000
> > 
> > And why the divergent formatting compared to other defines in this file?
> 
> You're right about the formatting inconsistency.
> 
> The existing decimal notation (8, 16, 32, 64) works well for lower bits, but
> 16777216 (decimal for bit 24) is much less readable than 0x01000000. It's
> also harder to verify correctness and more error-prone.
> 
> If consistency with the existing style is preferred, I can use decimal, but
> I'd suggest considering updating all these defines to use BIT() hex for better
> Readability and maintainability. What's your preference?

FWIW, I'm not ignoring you, but I won't reply until the more important
discussion with Linus has been resolved.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]