| Message ID | 20210414135128.180980-2-jacopo+renesas@jmondi.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | arm64: dts: renesas: Enable GMSL on R8A77970 V3M Eagle | expand |
Hi Jacopo, Thank you for the patch. On Wed, Apr 14, 2021 at 03:51:24PM +0200, Jacopo Mondi wrote: > Define a new vendor property in the maxim,max9286 binding schema. > > The new property allows to declare that the remote camera > power-over-coax is controlled by one of the MAX9286 gpio lines. > > As it is currently not possible to establish a regulator as consumer > of the MAX9286 gpio controller for this purpose, the property allows to > declare that the camera power is controlled by the MAX9286 directly. > > The property accepts a gpio-index (0 or 1) and one line polarity > flag as defined by dt-bindings/gpio/gpio.h. > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > --- > .../bindings/media/i2c/maxim,max9286.yaml | 53 ++++++++++++++++++- > 1 file changed, 52 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > index ee16102fdfe7..480a491f3744 100644 > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > @@ -70,6 +70,24 @@ properties: > a remote serializer whose high-threshold noise immunity is not enabled > is 100000 micro volts > > + maxim,gpio-poc: I would have written poc-gpio to match the order of the GPIO bindings syntax. > + $ref: '/schemas/types.yaml#/definitions/uint32-array' > + minItems: 2 > + maxItems: 2 > + description: | > + Identifier of gpio line that controls Power over Coax to the cameras and I'd write "Index of the MAX9286 GPIO output that ..." to make it clear that this is not a generic GPIO. > + the associated polarity flag (GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW) > + as defined in <include/dt-bindings/gpio/gpio.h>. > + > + When the remote cameras power is controlled by one of the MAX9286 gpio > + lines, this property has to be used to specify which line among the two > + available ones controls the remote camera power enablement. > + > + When this property is used it is not possible to register a gpio > + controller as the gpio lines are controlled directly by the MAX9286 and > + not available for consumers, nor the 'poc-supply' property should be > + specified. Only one of the two lines would be controlled directly. Shouldn't we still register a GPIO controller for the other line ? Could you also mention somewhere that the first item in the array should be 0 or 1 ? It may be hard to express in a YAML schema, so I'm fine just documenting it in the description. I've been wondering whether this would be a common enough issue that it could justify support in the GPIO core to handle consumer-provider loops, but even if that happens at some point in the future, I think the proposal here is good enough and we won't need to switch. > + > ports: > $ref: /schemas/graph.yaml#/properties/ports > > @@ -182,7 +200,6 @@ required: > - reg > - ports > - i2c-mux > - - gpio-controller > > additionalProperties: false > > @@ -327,4 +344,38 @@ examples: > }; > }; > }; > + > + /* > + * Example of a deserializer that controls the camera Power over Coax > + * through one of its gpio lines. > + */ > + gmsl-deserializer@6c { > + compatible = "maxim,max9286"; > + reg = <0x6c>; > + enable-gpios = <&gpio 14 GPIO_ACTIVE_HIGH>; > + > + /* > + * The remote camera power is controlled by MAX9286 GPIO line #0. > + * No 'poc-supply' nor 'gpio-controller' are specified. > + */ > + maxim,gpio-poc = <0 GPIO_ACTIVE_LOW>; > + > + /* > + * Do not describe connections as they're the same as in the previous > + * example. > + */ > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@4 { > + reg = <4>; > + }; > + }; > + > + i2c-mux { > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + }; > }; It's customary to indent DT examples with 4 spaces. The existing examples use two spaces, so maybe a patch on top of this would be useful to increase readability ? Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Hi Laurent, On Thu, Apr 15, 2021 at 02:47:12AM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Wed, Apr 14, 2021 at 03:51:24PM +0200, Jacopo Mondi wrote: > > Define a new vendor property in the maxim,max9286 binding schema. > > > > The new property allows to declare that the remote camera > > power-over-coax is controlled by one of the MAX9286 gpio lines. > > > > As it is currently not possible to establish a regulator as consumer > > of the MAX9286 gpio controller for this purpose, the property allows to > > declare that the camera power is controlled by the MAX9286 directly. > > > > The property accepts a gpio-index (0 or 1) and one line polarity > > flag as defined by dt-bindings/gpio/gpio.h. > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > --- > > .../bindings/media/i2c/maxim,max9286.yaml | 53 ++++++++++++++++++- > > 1 file changed, 52 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > > index ee16102fdfe7..480a491f3744 100644 > > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > > @@ -70,6 +70,24 @@ properties: > > a remote serializer whose high-threshold noise immunity is not enabled > > is 100000 micro volts > > > > + maxim,gpio-poc: > > I would have written poc-gpio to match the order of the GPIO bindings > syntax. > That's what I had :) but then the property gets matched against the gpio schema and I get complains because it expects a phandle as first argument... Maybe there's a way I've missed to prevent the property to be matched with *-gpio ? > > + $ref: '/schemas/types.yaml#/definitions/uint32-array' > > + minItems: 2 > > + maxItems: 2 > > + description: | > > + Identifier of gpio line that controls Power over Coax to the cameras and > > I'd write "Index of the MAX9286 GPIO output that ..." to make it clear > that this is not a generic GPIO. > Ack > > + the associated polarity flag (GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW) > > + as defined in <include/dt-bindings/gpio/gpio.h>. > > + > > + When the remote cameras power is controlled by one of the MAX9286 gpio > > + lines, this property has to be used to specify which line among the two > > + available ones controls the remote camera power enablement. > > + > > + When this property is used it is not possible to register a gpio > > + controller as the gpio lines are controlled directly by the MAX9286 and > > + not available for consumers, nor the 'poc-supply' property should be > > + specified. > > Only one of the two lines would be controlled directly. Shouldn't we > still register a GPIO controller for the other line ? I considered that and thought it was a bit of an overkill (and I also had a bit of troubles identifying how to register only gpio #1, as it would be identified as gpio #0 if the actual #0 is not registered) > > Could you also mention somewhere that the first item in the array should > be 0 or 1 ? It may be hard to express in a YAML schema, so I'm fine just > documenting it in the description. > Sure, I tried identifying how to express that with yaml and failed :) > I've been wondering whether this would be a common enough issue that it > could justify support in the GPIO core to handle consumer-provider > loops, but even if that happens at some point in the future, I think the > proposal here is good enough and we won't need to switch. > Please note that with the suggestion offline from rob I will add to the next version: # If 'maxim,gpio-poc' is present, then 'poc-supply' and 'gpio-controller' # are not allowed. if: required: - maxim,gpio-poc then: allOf: - not: required: - poc-supply - not: required: - gpio-controller > > + > > ports: > > $ref: /schemas/graph.yaml#/properties/ports > > > > @@ -182,7 +200,6 @@ required: > > - reg > > - ports > > - i2c-mux > > - - gpio-controller > > > > additionalProperties: false > > > > @@ -327,4 +344,38 @@ examples: > > }; > > }; > > }; > > + > > + /* > > + * Example of a deserializer that controls the camera Power over Coax > > + * through one of its gpio lines. > > + */ > > + gmsl-deserializer@6c { > > + compatible = "maxim,max9286"; > > + reg = <0x6c>; > > + enable-gpios = <&gpio 14 GPIO_ACTIVE_HIGH>; > > + > > + /* > > + * The remote camera power is controlled by MAX9286 GPIO line #0. > > + * No 'poc-supply' nor 'gpio-controller' are specified. > > + */ > > + maxim,gpio-poc = <0 GPIO_ACTIVE_LOW>; > > + > > + /* > > + * Do not describe connections as they're the same as in the previous > > + * example. > > + */ > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@4 { > > + reg = <4>; > > + }; > > + }; > > + > > + i2c-mux { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + }; > > + }; > > }; > > It's customary to indent DT examples with 4 spaces. The existing > examples use two spaces, so maybe a patch on top of this would be useful > to increase readability ? > Ah weird! I can add a patch before this one! > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Thanks j > -- > Regards, > > Laurent Pinchart
Hi Jacopo, On Thu, Apr 15, 2021 at 8:53 AM Jacopo Mondi <jacopo@jmondi.org> wrote: > On Thu, Apr 15, 2021 at 02:47:12AM +0300, Laurent Pinchart wrote: > > On Wed, Apr 14, 2021 at 03:51:24PM +0200, Jacopo Mondi wrote: > > > Define a new vendor property in the maxim,max9286 binding schema. > > > > > > The new property allows to declare that the remote camera > > > power-over-coax is controlled by one of the MAX9286 gpio lines. > > > > > > As it is currently not possible to establish a regulator as consumer > > > of the MAX9286 gpio controller for this purpose, the property allows to > > > declare that the camera power is controlled by the MAX9286 directly. > > > > > > The property accepts a gpio-index (0 or 1) and one line polarity > > > flag as defined by dt-bindings/gpio/gpio.h. > > > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > > --- > > > .../bindings/media/i2c/maxim,max9286.yaml | 53 ++++++++++++++++++- > > > 1 file changed, 52 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > > > index ee16102fdfe7..480a491f3744 100644 > > > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > > > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > > > @@ -70,6 +70,24 @@ properties: > > > a remote serializer whose high-threshold noise immunity is not enabled > > > is 100000 micro volts > > > > > > + maxim,gpio-poc: > > > > I would have written poc-gpio to match the order of the GPIO bindings > > syntax. > > > > That's what I had :) but then the property gets matched against the > gpio schema and I get complains because it expects a phandle as first > argument... Maybe there's a way I've missed to prevent the property to > be matched with *-gpio ? GPIO hogs also use gpio properties lacking the phandle. Hence the way this is handled for hogs may (or may not, it's yaml after all ;-) inspire you how to handle this here. Gr{oetje,eeting}s, Geert
diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml index ee16102fdfe7..480a491f3744 100644 --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml @@ -70,6 +70,24 @@ properties: a remote serializer whose high-threshold noise immunity is not enabled is 100000 micro volts + maxim,gpio-poc: + $ref: '/schemas/types.yaml#/definitions/uint32-array' + minItems: 2 + maxItems: 2 + description: | + Identifier of gpio line that controls Power over Coax to the cameras and + the associated polarity flag (GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW) + as defined in <include/dt-bindings/gpio/gpio.h>. + + When the remote cameras power is controlled by one of the MAX9286 gpio + lines, this property has to be used to specify which line among the two + available ones controls the remote camera power enablement. + + When this property is used it is not possible to register a gpio + controller as the gpio lines are controlled directly by the MAX9286 and + not available for consumers, nor the 'poc-supply' property should be + specified. + ports: $ref: /schemas/graph.yaml#/properties/ports @@ -182,7 +200,6 @@ required: - reg - ports - i2c-mux - - gpio-controller additionalProperties: false @@ -327,4 +344,38 @@ examples: }; }; }; + + /* + * Example of a deserializer that controls the camera Power over Coax + * through one of its gpio lines. + */ + gmsl-deserializer@6c { + compatible = "maxim,max9286"; + reg = <0x6c>; + enable-gpios = <&gpio 14 GPIO_ACTIVE_HIGH>; + + /* + * The remote camera power is controlled by MAX9286 GPIO line #0. + * No 'poc-supply' nor 'gpio-controller' are specified. + */ + maxim,gpio-poc = <0 GPIO_ACTIVE_LOW>; + + /* + * Do not describe connections as they're the same as in the previous + * example. + */ + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@4 { + reg = <4>; + }; + }; + + i2c-mux { + #address-cells = <1>; + #size-cells = <0>; + }; + }; };
Define a new vendor property in the maxim,max9286 binding schema. The new property allows to declare that the remote camera power-over-coax is controlled by one of the MAX9286 gpio lines. As it is currently not possible to establish a regulator as consumer of the MAX9286 gpio controller for this purpose, the property allows to declare that the camera power is controlled by the MAX9286 directly. The property accepts a gpio-index (0 or 1) and one line polarity flag as defined by dt-bindings/gpio/gpio.h. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- .../bindings/media/i2c/maxim,max9286.yaml | 53 ++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-)