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+rene...@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.pinch...@ideasonboard.com>
--
Regards,
Laurent Pinchart
