Re: [PATCH] gpio: axp: Add support for devicetree

[Yixun Lan <dlan@gentoo.org>]

Hi Denis,

On 15:50 Fri 19 Jun     , Andre Przywara wrote:
> Hi Denis,
> 
> thanks for sending this patch!
> 
> On 1/21/26 12:54, Denis Burkov wrote:
> 
> I am afraid you need to put some commit message in here. You can refer 
> to the comment you are removing, so that there *is* now DT support for 
> the AXP GPIOs. But also please mention what this fixes - why do we need 
> this patch? If you can provide an example usage, it would be easier to 
> take this patch.
> 
I agree, describe what's problem to solve.. mainly I think this patch
coverting the driver into device tree aware, so it will probe axp-gpio
based on if the target dts support it or not..

> > Signed-off-by: Denis Burkov <hitechshell@mail.ru>
> > ---
> >   board/sunxi/board.c     |  4 ----
> >   drivers/gpio/axp_gpio.c | 28 +++++++++++-----------------
> >   2 files changed, 11 insertions(+), 21 deletions(-)
> > 
> > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > index 2929bc17f08..3c61bfb8822 100644
> > --- a/board/sunxi/board.c
> > +++ b/board/sunxi/board.c
> > @@ -226,10 +226,6 @@ int board_init(void)
> >   	}
> >   #endif /* !CONFIG_ARM64 && !CONFIG_MACH_SUNIV */
> >   
> > -	ret = axp_gpio_init();
> > -	if (ret)
> > -		return ret;
> > -
> >   	eth_init_board();
> >   
> >   	return 0;
> > diff --git a/drivers/gpio/axp_gpio.c b/drivers/gpio/axp_gpio.c
> > index 181c53bfe72..1c5882a25d4 100644
> > --- a/drivers/gpio/axp_gpio.c
> > +++ b/drivers/gpio/axp_gpio.c
> > @@ -98,35 +98,29 @@ static const struct dm_gpio_ops gpio_axp_ops = {
> >   
> >   static int gpio_axp_probe(struct udevice *dev)
> >   {
> > +	int ret;
> >   	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> >   
> >   	/* Tell the uclass how many GPIOs we have */
> >   	uc_priv->bank_name = AXP_GPIO_PREFIX;
> >   	uc_priv->gpio_count = AXP_GPIO_COUNT;
> >   
> > +	ret = pmic_bus_init();
> > +	if (ret)
> > +		return ret;
> > +
> >   	return 0;
> >   }
> >   
> > +static const struct udevice_id gpio_axp_match[] = {
> > +	{ .compatible = "x-powers,axp209-gpio" },
> 
> Why is it only this compatible string? What about the others? I guess 
> the driver was just not enabled on newer AXPs before?
> Can you maybe look what it takes to support the other PMICs? If you look 
> at the Linux driver, that's mostly about what pins each AXP supports.
> 
For Linux driver, I think Andre refer to the AXP209 GPIO driver [1]
and two more compatibles "x-powers,axp221-gpio", "x-powers,axp813-gpio" are needed

Please also check the ".gpio_status_offset" which differ in device data

Link: https://elixir.bootlin.com/linux/v7.1/source/drivers/pinctrl/pinctrl-axp209.c#L429 [1]
> And peeking into the driver here, there are some #ifdef's in there 
> (AXP_GPIO[23]_CTRL), I think now collides with the DT description, and 
> whatever this was doing should be detected at runtime, potentially by 
> supporting multiple compatible strings.
> 
Agree, should fold these info into compatible data with multiple compatible

-- 
Yixun Lan (dlan)