Hi Kieran,
On Fri, Jun 12, 2020 at 01:47:58PM +0100, Kieran Bingham wrote:
> On 12/06/2020 13:35, Kieran Bingham wrote:
> > On 10/06/2020 16:16, Jacopo Mondi wrote:
> >> Hi Kieran
> >>
> >> On Wed, Jun 10, 2020 at 01:46:20PM +0100, Kieran Bingham wrote:
> >>> From: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
> >>>
> >>> The MAX9286 exposes a GPIO controller to control the two GPIO out pins
> >>> of the chip.
> >>>
> >>> These can in some configurations be used to control the power of the
> >>> cameras, and in the case of the V3M, it is used to enable power up
> >>> of the GMSL PoC regulator.
> >>>
> >>> The regulator can not (currently) be moddelled as a regulator due to
> >>> probe time issues, and instead are controlled through the use of a
> >>> gpio-hog.
> >>>
> >>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
> >>
> >> I have missed if this should be a required property or not..
> >
> > Hrm... I'm not sure. It isn't 'required' ... but the device does expose
> > gpio pins, which the driver provides access to (and is needed to be able
> > to expose a gpio-hog).
> >
> > If the device isn't marked as a gpio-controller, then the gpio-hog
> > framework doesn't work.
> >
> > But the gpio pins will ...
> >
> > Do you think I should add gpio-controller to the required section as well?:
> >
> > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > @@ -220,6 +220,7 @@ required:
> > - reg
> > - ports
> > - i2c-mux
> > + - gpio-controller
> >
> > additionalProperties: false
> >
> >
> > As it's only required for making gpio-hogs, I guess it's optional, and
> > doesn't need to be listed...
As you off-line explained me, this mean that if cameras power is
controlled by the gpio pins exposed by the max9286, without this
property we would not be able to control it.
I would then make it mandatory
> >
> > But the *hardware* has gpios... which are controllable...
>
> And of course adding to the requried properties, means the example needs
> ot be updated too, so it's actually:
oh yes :)
>
> Which I'll also add to v10, if you don't object.
>
>
>
> Author: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
> Date: Fri Jun 12 13:36:28 2020 +0100
>
> make gpio-controller non-optional
>
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
>
> diff --git
> a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> index 8307c41f2cae..c632a10a753a 100644
> --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> @@ -220,6 +220,7 @@ required:
> - reg
> - ports
> - i2c-mux
> + - gpio-controller
>
> additionalProperties: false
>
> @@ -239,6 +240,9 @@ examples:
> poc-supply = <&camera_poc_12v>;
> enable-gpios = <&gpio 13 GPIO_ACTIVE_HIGH>;
>
> + gpio-controller;
> + #gpio-cells = <2>;
> +
Looks good to me!
Thanks
j
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
>
>
> >
> > --
> > Kieran
> >
> >
> >>
> >>
> >>> ---
> >>> .../devicetree/bindings/media/i2c/maxim,max9286.yaml | 5 +++++
> >>> 1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> >>> index f9d3e5712c59..7d75c3b63c0b 100644
> >>> --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> >>> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> >>> @@ -46,6 +46,11 @@ properties:
> >>> description: GPIO connected to the \#PWDN pin with inverted polarity
> >>> maxItems: 1
> >>>
> >>> + gpio-controller: true
> >>> +
> >>> + '#gpio-cells':
> >>> + const: 2
> >>> +
> >>> ports:
> >>> type: object
> >>> description: |
> >>> --
> >>> 2.25.1
> >>>
> >
>
