Re: [PATCH v6 2/2] dt-bindings: memory-controllers: gpmc-child: add wait-pin polarity

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2022-09-30 at 22:01 +0200, Krzysztof Kozlowski wrote:
> On 30/09/2022 21:42, Rob Herring wrote:
> > On Thu, Sep 29, 2022 at 02:56:39PM +0200, B. Niedermayr wrote:
> > > From: Benedikt Niedermayr <benedikt.niedermayr@xxxxxxxxxxx>
> > > 
> > > The GPMC controller has the ability to configure the polarity for the
> > > wait pin. The current properties do not allow this configuration.
> > > This binding directly configures the WAITPIN<X>POLARITY bit
> > > in the GPMC_CONFIG register by setting the gpmc,wait-pin-polarity
> > > dt-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..477189973334 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:
> > 
> > 'gpmc' is not a vendor. Don't continue this bad pattern, use 'ti'.
> > 
> > > +    description: |
> > > +      Set the desired polarity for the selected wait pin.
> > > +      1 for active low, 0 for active high.
> > 
> > Well that looks backwards. I assume from the commit msg above, it's the 
> > register value, but that's not what the description says. Please go with 
> > the logical state here and do the inversion in the driver.
> 
> This was actually my suggestion to keep the same value as
> ACTIVE_HIGH/LOW in standard GPIO flags. The DTS could reuse the defines.
> 
Ok, but how to proceed know? IMHO it makes more sense to use the value which actually lands in the register since most 
people will use the value found in the Datasheet. 

We already had a discussion with Roger about the GPIO bindings vs. wait-pin binding. The point was that we do not use GPIO bindings
in this case, or? 

> Best regards,
> Krzysztof
> 

Cheers,
Benedikt




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux