MIME-Version: 1.0
References: <20230224113837.874264-1-jneanne@baylibre.com> <20230224113837.874264-2-jneanne@baylibre.com>
 <Y/iqhsEIvHgnZ+5l@surfacebook>
In-Reply-To: <Y/iqhsEIvHgnZ+5l@surfacebook>
From:   Jon Cormier <jcormier@criticallink.com>
Date:   Mon, 27 Feb 2023 14:20:15 -0500
Message-ID: <CADL8D3bm82+=LwYnve04FB2zsSJ6ceKQHycV9oNWoYFiOQnVCA@mail.gmail.com>
Subject: Re: [PATCH 1/2] gpio: tps65219: add GPIO support for TPS65219 PMIC
To:     andy.shevchenko@gmail.com
Cc:     Jerome Neanne <jneanne@baylibre.com>, linus.walleij@linaro.org,
        brgl@bgdev.pl, tony@atomide.com, lee@kernel.org,
        linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
        linux-omap@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Feb 24, 2023 at 7:16 AM <andy.shevchenko@gmail.com> wrote:
>
> Fri, Feb 24, 2023 at 12:38:36PM +0100, Jerome Neanne kirjoitti:
> > Add support for TPS65219 PMICs GPIO interface.
> >
> > 3 GPIO pins:
> > - GPIO0 only is IO but input mode reserved for MULTI_DEVICE_ENABLE usage
> > - GPIO1 and GPIO2 are Output only and referred as GPO1 and GPO2 in spec
> >
> > GPIO0 is statically configured as input or output prior to linux boot.
>
> Linux
>
> > it is used for MULTI_DEVICE_ENABLE function.
> > This setting is statically configured by NVM.
> > GPIO0 can't be used as a generic GPIO (specification Table 8-34).
> > It's either a GPO when MULTI_DEVICE_EN=0
> > or a GPI when MULTI_DEVICE_EN=1.
>
> Something wrong with the indentation.
>
> > Link: https://www.ti.com/lit/ds/symlink/tps65219.pdf
>
> Can it be Datasheet tag?
>
> > Signed-off-by: Jonathan Cormier <jcormier@criticallink.com>
> > Signed-off-by: Jerome Neanne <jneanne@baylibre.com>
>
> Not sure how to interpet this along with the From line.
Are two sign-offs not allowed/expected?  I wrote the initial
implementation of this driver and Jerome updated it and is handling
submitting it since he did the rest of the TPS65219 drivers.
>
> ...
>
> > +config GPIO_TPS65219
> > +     tristate "TPS65219 GPIO"
> > +     depends on MFD_TPS65219
> > +     help
> > +       Select this option to enable GPIO driver for the TPS65219
> > +       chip family.
> > +       GPIO0 is statically configured as input or output prior to linux boot.
> > +       it is used for MULTI_DEVICE_ENABLE function.
> > +       This setting is statically configured by NVM.
> > +       GPIO0 can't be used as a generic GPIO.
> > +       It's either a GPO when MULTI_DEVICE_EN=0
> > +       or a GPI when MULTI_DEVICE_EN=1.
>
> Similar issues as with commit message. Also if chosen as M, how will it be
> called?
Based on INPUT_TPS65219_PWRBUTTON, this should have:

To compile this driver as a module, choose M here. The module will
be called tps65219-gpio.


--
Jonathan Cormier
Software Engineer

Voice:  315.425.4045 x222


http://www.CriticalLink.com
6712 Brooklawn Parkway, Syracuse, NY 13211