Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 3 Jun 2022 02:06:18 -0700
From:   Piyush Malgujar <pmalgujar@marvell.com>
To:     Linus Walleij <linus.walleij@linaro.org>
CC:     <linux-gpio@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <brgl@bgdev.pl>,
        <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
        <rric@kernel.org>, <cchavva@marvell.com>, <wsadowski@marvell.com>
Subject: Re: [PATCH 2/5] dt-bindings: gpio: gpio-thunderx: Describe pin-cfg
 option
Message-ID: <20220603090618.GA27121@Dell2s-9>
References: <20220427144620.9105-1-pmalgujar@marvell.com>
 <20220427144620.9105-3-pmalgujar@marvell.com>
 <CACRpkdaqeTs-jHPBmtdiz+LdMM0pz0zqt4diX=e+YpgaGr0Jbw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <CACRpkdaqeTs-jHPBmtdiz+LdMM0pz0zqt4diX=e+YpgaGr0Jbw@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

Hi Linus,

Thanks for reviewing.

On Mon, May 02, 2022 at 12:15:34AM +0200, Linus Walleij wrote:
> On Wed, Apr 27, 2022 at 4:47 PM Piyush Malgujar <pmalgujar@marvell.com> wrote:
> 
> > Add support for pin-cfg to configure GPIO Pins
> >
> > Signed-off-by: Piyush Malgujar <pmalgujar@marvell.com>
> > ---
> >  Documentation/devicetree/bindings/gpio/gpio-thunderx.txt | 4 ++++
> 
> Would be nice to rewrite this binding in YAML
> 

Sure, will take care in V2.

> >    - First cell is the GPIO pin number relative to the controller.
> >    - Second cell is triggering flags as defined in interrupts.txt.
> > +- pin-cfg: Configuration of pin's function, filters, XOR and output mode.
> > +  - First cell is the GPIO pin number
> > +  - Second cell is a value written to GPIO_BIT_CFG register at driver probe.
> 
> Just poking magic hex values into some random register as
> part of a binding is not a good idea.
> 
> This looks like trying to reinvent the pin config subsystem.
> 
> GPIO is using the standard pin configurations in the second cell of
> the handle, use them in this driver as well and add new ones if we
> need.
> 
> You find the existing flags here:
> include/dt-bindings/gpio/gpio.h
> 
> If you need something more sophisticated than a simple flag, I think
> you need to implement proper pin config.
> 
> Yours,
> Linus Walleij

The purpose of this pin-cfg entry is different than the standard GPIO pin config usage.
It is to write a value to GPIO_BIT_CFG register which is used to configure fields like
pin function, selecting which signal is reported to GPIO output or which signal GPIO
input need to connect, filters, XOR and output mode.
We will define new entry specific to thunderx GPIO usage, instead of pin-cfg.

Thanks,
Piyush