On Mon, 2022-09-05 at 11:54 +0200, Krzysztof Kozlowski wrote: > On 05/09/2022 11:21, Roger Quadros wrote: > > > > On 05/09/2022 12:14, Niedermayr, BENEDIKT wrote: > > > On Mon, 2022-09-05 at 11:56 +0300, Roger Quadros wrote: > > > > Hi Benedikt, > > > > > > > > On 05/09/2022 10:17, B. Niedermayr wrote: > > > > > From: Benedikt Niedermayr <benedikt.niedermayr@xxxxxxxxxxx> > > > > > > > > > > Add a new dt-binding for the wait-pin-polarity property > > > > > > > > > > Signed-off-by: Benedikt Niedermayr < > > > > > benedikt.niedermayr@xxxxxxxxxxx > > > > > --- > > > > > .../bindings/memory-controllers/ti,gpmc-child.yaml | > > > > > 7 > > > > > +++++++ > > > > > 1 file changed, 7 insertions(+) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/memory- > > > > > controllers/ti,gpmc-child.yaml > > > > > b/Documentation/devicetree/bindings/memory- > > > > > controllers/ti,gpmc- > > > > > child.yaml > > > > > index 6e3995bb1630..7c721206f10b 100644 > > > > > --- a/Documentation/devicetree/bindings/memory- > > > > > controllers/ti,gpmc- > > > > > child.yaml > > > > > +++ b/Documentation/devicetree/bindings/memory- > > > > > controllers/ti,gpmc- > > > > > child.yaml > > > > > @@ -230,6 +230,13 @@ properties: > > > > > Wait-pin used by client. Must be less than "gpmc,num- > > > > > waitpins". > > > > > $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > > > > > + gpmc,wait-pin-polarity: > > > > > + description: | > > > > > + Wait-pin polarity used by the clien. It relates to the > > > > > pin > > > > > defined > > > > > > > > did you mean "client?" > > > > Can you please specify what value is for Active Low vs Active > > > > High? > > > > > > Yes, that makes sense. And yes I meant "client". My typo..... > > > > > + with "gpmc,wait-pin". > > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > > > Why can't type be boolean? > > > > > > Of course we can use the boolean there. In that case I should > > > give the > > > property a more meaningful name e.g. wait-pin-active-high or > > > wait-pin- > > > active-low. > > > Since the default behavour of this pin is Active High, > > > a bool property "gpmc,wait-pin-active-low" would make more sense > > > for > > > backwards compatibility. > > > If the property is missing, than the polarity stays on Active > > > High like > > > before. > > > > > > > OK, in that case you don't have to clarify the polarity in > > description. > > I don't understand (and it is not explained in commit msg), why do > you > need such property instead of using standard GPIO flags. > > The driver should use standard GPIO descriptor and standard bindings. > If > it cannot, this has to be explained. > > Best regards, > Krzysztof I think this is beacause the GPMC controller itself is not respecting the GPIO flags. Instead the GPMC is reading the Line Level directly (high,low) and then evaluates the logic depending how the WAIT<x>PINPOLARITY bit is set in the GPMPC_CONFIG register. Until now gpiochip_request_own_desc() was hardcorded to GPIO_ACTIVE_HIGH. An the GPMC_CONFIG register configuration has no relation to the GPIO setting (in the current implementation). My first approach was to make this part configurable via a new device tree property (wait-pin-polarity). IMHO (correct me if I'm wrong) the current implementation also does not make ues of standart GPIO bindings and defines the wait pin via a separate "gpmc,waitpin" binding. E.g. gpmc,watipin = <0> or gpmc,waitpin=<1> The best solution would should be when setting the binding this way for example: gpmc,wait-pin = <&gpiox y ACTIVE_X> But I think the current omap-gpmc.c implementation does not offer such a usecase and as roger already mentioned: "GPMC wait_pin polarity logic is hard-wired and doesn't depend on GPIO subsystem for its polarity" cheers, benedikt
