| Message ID | 20210414135128.180980-1-jacopo+renesas@jmondi.org |
|---|---|
| Headers | show |
| Series | arm64: dts: renesas: Enable GMSL on R8A77970 V3M Eagle | expand |
Hi Jacopo and Kieran, Thank you for the patch. On Wed, Apr 14, 2021 at 03:51:26PM +0200, Jacopo Mondi wrote: > From: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Enable the MAX9286 GMSL deserializer on the Eagle-V3M board. > > Connected cameras should be defined in a device-tree overlay or included > after these definitions. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > .../arm64/boot/dts/renesas/r8a77970-eagle.dts | 119 ++++++++++++++++++ > 1 file changed, 119 insertions(+) > > diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > index 874a7fc2730b..d2b6368d1e72 100644 > --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > @@ -6,6 +6,8 @@ > * Copyright (C) 2017 Cogent Embedded, Inc. > */ > > +#include <dt-bindings/gpio/gpio.h> > + > /dts-v1/; > #include "r8a77970.dtsi" > > @@ -188,6 +190,11 @@ i2c0_pins: i2c0 { > function = "i2c0"; > }; > > + i2c3_pins: i2c3 { > + groups = "i2c3_a"; > + function = "i2c3"; > + }; > + > qspi0_pins: qspi0 { > groups = "qspi0_ctrl", "qspi0_data4"; > function = "qspi0"; > @@ -266,6 +273,118 @@ &rwdt { > status = "okay"; > }; > > +&csi40 { > + status = "okay"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + csi40_in: endpoint { > + clock-lanes = <0>; > + data-lanes = <1 2 3 4>; > + remote-endpoint = <&max9286_out0>; > + }; > + }; > + }; > +}; > + > +&i2c3 { > + pinctrl-0 = <&i2c3_pins>; > + pinctrl-names = "default"; > + > + status = "okay"; > + clock-frequency = <400000>; > + > + gmsl: gmsl-deserializer@48 { > + compatible = "maxim,max9286"; > + reg = <0x48>; > + > + maxim,gpio-poc = <0 GPIO_ACTIVE_LOW>; > + > + /* eagle-pca9654-max9286-pwdn */ > + enable-gpios = <&io_expander 0 GPIO_ACTIVE_HIGH>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + max9286_in0: endpoint { > + }; > + }; > + > + port@1 { > + reg = <1>; > + max9286_in1: endpoint { > + }; > + }; > + > + port@2 { > + reg = <2>; > + max9286_in2: endpoint { > + }; > + }; > + > + port@3 { > + reg = <3>; > + max9286_in3: endpoint { > + }; > + }; > + > + port@4 { > + reg = <4>; > + max9286_out0: endpoint { > + clock-lanes = <0>; > + data-lanes = <1 2 3 4>; > + remote-endpoint = <&csi40_in>; > + }; > + }; > + }; > + > + i2c-mux { > + #address-cells = <1>; > + #size-cells = <0>; > + > + i2c@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0>; > + > + status = "disabled"; > + }; > + > + i2c@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <1>; > + > + status = "disabled"; > + }; > + > + i2c@2 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <2>; > + > + status = "disabled"; > + }; > + > + i2c@3 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <3>; > + > + status = "disabled"; > + }; > + }; > + }; > +}; > + > &scif0 { > pinctrl-0 = <&scif0_pins>; > pinctrl-names = "default";
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 -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds