On 05/09/2022 13:48, Niedermayr, BENEDIKT wrote: > 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> Yes and I am afraid this will grow instead of adding proper GPIO usage. Any reason why it cannot be a standard GPIO pin desc? > > 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" This part I don't get. You mean hard-wired in the driver or hard-wired in the hardware? If the first, please un-wire it. If the latter, your property makes no sense, right? Best regards, Krzysztof
