| Message ID | 20210315163028.173148-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 Mon, Mar 15, 2021 at 05:30:25PM +0100, Jacopo Mondi wrote: > The MAX9286 GMSL deserializer features gpio controller capabilities, > as it provides 2 GPIO lines. > > As establishing a regulator that uses one of the GPIO lines and > enabling/disabling it at run-time in the max9286 won't work due to > a circular dependency on the gpio-controller/regulator creation, allow > the usage of a gpio-hog for that purpose. > > The usage of the gpio-hog is required in designs where the MAX9286 > GPIO lines control the remote cameras power. > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> That's really a workaround until we can find a good solution, do we have to officially support it in the DT bindings ? > --- > .../bindings/media/i2c/maxim,max9286.yaml | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > index ee16102fdfe7..9038300e373c 100644 > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > @@ -177,6 +177,22 @@ properties: > > additionalProperties: false > > +patternProperties: > + "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$": > + type: object > + properties: > + gpio-hog: true > + gpios: true > + output-low: true > + line-name: true > + > + required: > + - gpio-hog > + - gpios > + - output-low > + > + additionalProperties: false > + > required: > - compatible > - reg
Hi Laurent, On Tue, Mar 16, 2021 at 12:15:16AM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Mon, Mar 15, 2021 at 05:30:25PM +0100, Jacopo Mondi wrote: > > The MAX9286 GMSL deserializer features gpio controller capabilities, > > as it provides 2 GPIO lines. > > > > As establishing a regulator that uses one of the GPIO lines and > > enabling/disabling it at run-time in the max9286 won't work due to > > a circular dependency on the gpio-controller/regulator creation, allow > > the usage of a gpio-hog for that purpose. > > > > The usage of the gpio-hog is required in designs where the MAX9286 > > GPIO lines control the remote cameras power. > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > That's really a workaround until we can find a good solution, do we have > to officially support it in the DT bindings ? > That's an interesting question. The 'good' solution implies resolving the circular dependency on the regulator/gpio-controller creation and I feel like it might take a while to find a proper solution. In the meantime, all designs like Eagle that control the camera power through a MAX9286 gpio have to rely on this. I'll go with the majority here: either we add this and upstream the gmsl .dtsi for eagle, or we keep out-of-tree patches :/ > > --- > > .../bindings/media/i2c/maxim,max9286.yaml | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > > index ee16102fdfe7..9038300e373c 100644 > > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > > @@ -177,6 +177,22 @@ properties: > > > > additionalProperties: false > > > > +patternProperties: > > + "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$": > > + type: object > > + properties: > > + gpio-hog: true > > + gpios: true > > + output-low: true > > + line-name: true > > + > > + required: > > + - gpio-hog > > + - gpios > > + - output-low > > + > > + additionalProperties: false > > + > > required: > > - compatible > > - reg > > -- > Regards, > > Laurent Pinchart
Hi Jacopo, On Wed, Mar 17, 2021 at 11:14:12AM +0100, Jacopo Mondi wrote: > On Tue, Mar 16, 2021 at 12:15:16AM +0200, Laurent Pinchart wrote: > > On Mon, Mar 15, 2021 at 05:30:25PM +0100, Jacopo Mondi wrote: > > > The MAX9286 GMSL deserializer features gpio controller capabilities, > > > as it provides 2 GPIO lines. > > > > > > As establishing a regulator that uses one of the GPIO lines and > > > enabling/disabling it at run-time in the max9286 won't work due to > > > a circular dependency on the gpio-controller/regulator creation, allow > > > the usage of a gpio-hog for that purpose. > > > > > > The usage of the gpio-hog is required in designs where the MAX9286 > > > GPIO lines control the remote cameras power. > > > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > > > That's really a workaround until we can find a good solution, do we have > > to officially support it in the DT bindings ? > > > > That's an interesting question. The 'good' solution implies resolving > the circular dependency on the regulator/gpio-controller creation and > I feel like it might take a while to find a proper solution. How about not using a regulator in that case ? If a MAX9286 GPIO is used to controlled the camera power, we could describe that specifically in DT, and handle everything within the MAX9286 driver in that case. It could be considered as a bit of a hack, but it would be very localized, and I think that trying to fix it in a way that would involve the regulator framework would actually be worse as the complexity would be bigger, for little gain. > In the meantime, all designs like Eagle that control the camera power > through a MAX9286 gpio have to rely on this. I'll go with the majority > here: either we add this and upstream the gmsl .dtsi for eagle, or we > keep out-of-tree patches :/ I don't mind having it upstream in the driver as a workaround, but I'd like to keep it out of the DT bindings as it shouldn't be considered as an officially supported option. Would that work for you ? > > > --- > > > .../bindings/media/i2c/maxim,max9286.yaml | 16 ++++++++++++++++ > > > 1 file changed, 16 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > > > index ee16102fdfe7..9038300e373c 100644 > > > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > > > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml > > > @@ -177,6 +177,22 @@ properties: > > > > > > additionalProperties: false > > > > > > +patternProperties: > > > + "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$": > > > + type: object > > > + properties: > > > + gpio-hog: true > > > + gpios: true > > > + output-low: true > > > + line-name: true > > > + > > > + required: > > > + - gpio-hog > > > + - gpios > > > + - output-low > > > + > > > + additionalProperties: false > > > + > > > required: > > > - compatible > > > - reg
diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml index ee16102fdfe7..9038300e373c 100644 --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml @@ -177,6 +177,22 @@ properties: additionalProperties: false +patternProperties: + "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$": + type: object + properties: + gpio-hog: true + gpios: true + output-low: true + line-name: true + + required: + - gpio-hog + - gpios + - output-low + + additionalProperties: false + required: - compatible - reg
The MAX9286 GMSL deserializer features gpio controller capabilities, as it provides 2 GPIO lines. As establishing a regulator that uses one of the GPIO lines and enabling/disabling it at run-time in the max9286 won't work due to a circular dependency on the gpio-controller/regulator creation, allow the usage of a gpio-hog for that purpose. The usage of the gpio-hog is required in designs where the MAX9286 GPIO lines control the remote cameras power. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- .../bindings/media/i2c/maxim,max9286.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)