Re: [PATCH RFT v2 2/2] gpio: dw: make deep probe compatible

[David Picard <david.picard@clermont.in2p3.fr>]

Tested-by:David Picard <david.picard@clermont.in2p3.fr>



Le 06/08/2025 à 12:15, Ahmad Fatoum a écrit :
> When deep probing GPIO controller, we expect struct gpio_chip::dev to be
> bound to a driver.
>
> This is currently not the case as the driver for the parent nodes
> creates devices for the child nodes and never binds a driver to them.
>
> As the child nodes have compatibles themselves too, let's match against
> proper drivers.
>
> Reported-by: David Picard <david.picard@clermont.in2p3.fr>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> v1 -> v2:
>    - unchanged
> ---
>   drivers/gpio/gpio-dw.c | 44 ++++++++++++++++++++++++------------------
>   1 file changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpio/gpio-dw.c b/drivers/gpio/gpio-dw.c
> index 98668c0a2d54..688b2536be2b 100644
> --- a/drivers/gpio/gpio-dw.c
> +++ b/drivers/gpio/gpio-dw.c
> @@ -6,6 +6,7 @@
>    */
>   
>   #include <common.h>
> +#include <linux/device.h>
>   #include <errno.h>
>   #include <io.h>
>   #include <gpio.h>
> @@ -104,9 +105,9 @@ static struct gpio_ops dw_gpio_ops = {
>   	.set = dw_gpio_set,
>   };
>   
> -static int dw_gpio_add_port(struct device *dev, struct device_node *node,
> -			    struct dw_gpio *parent)
> +static int dw_gpio_add_port(struct device *dev)
>   {
> +	struct dw_gpio *parent = dev_get_drvdata(dev->parent);
>   	struct dw_gpio_instance *chip;
>   	uint32_t config1, config2;
>   	int ngpio, ret;
> @@ -130,17 +131,7 @@ static int dw_gpio_add_port(struct device *dev, struct device_node *node,
>   
>   	chip->parent = parent;
>   	chip->chip.ngpio = ngpio;
> -	chip->chip.dev = add_generic_device("dw-port", DEVICE_ID_DYNAMIC, NULL,
> -					    dev->resource[0].start,
> -					    resource_size(&dev->resource[0]),
> -					    IORESOURCE_MEM, NULL);
> -
> -	if (!chip->chip.dev) {
> -		dev_err(dev, "unable to add device\n");
> -		return -ENODEV;
> -	}
> -
> -	chip->chip.dev->of_node = node;
> +	chip->chip.dev = dev;
>   
>   	ret = gpiochip_add(&chip->chip);
>   	if (ret)
> @@ -152,11 +143,22 @@ static int dw_gpio_add_port(struct device *dev, struct device_node *node,
>   	return 0;
>   }
>   
> +static struct of_device_id dwgpio_port_match[] = {
> +	{ .compatible = "snps,dw-apb-gpio-port" },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, dwgpio_port_match);
> +
> +static struct driver dwgpio_port_driver = {
> +	.name = "dw-apb-gpio-port",
> +	.probe = dw_gpio_add_port,
> +	.of_compatible = dwgpio_port_match,
> +};
> +
>   static int dw_gpio_probe(struct device *dev)
>   {
>   	struct resource *iores;
>   	struct dw_gpio *gpio;
> -	struct device_node *node;
>   
>   	gpio = xzalloc(sizeof(*gpio));
>   
> @@ -165,13 +167,12 @@ static int dw_gpio_probe(struct device *dev)
>   		return PTR_ERR(iores);
>   	gpio->regs = IOMEM(iores->start);
>   
> -	for_each_child_of_node(dev->of_node, node)
> -		dw_gpio_add_port(dev, node, gpio);
> +	dev_set_drvdata(dev, gpio);
>   
> -	return 0;
> +	return of_platform_populate(dev->of_node, NULL, dev);
>   }
>   
> -static __maybe_unused struct of_device_id dwgpio_match[] = {
> +static struct of_device_id dwgpio_match[] = {
>   	{
>   		.compatible = "snps,dw-apb-gpio",
>   	}, {
> @@ -186,4 +187,9 @@ static struct driver dwgpio_driver = {
>   	.of_compatible = dwgpio_match,
>   };
>   
> -postcore_platform_driver(dwgpio_driver);
> +static int __init dwgpio_driver_register(void)
> +{
> +	platform_driver_register(&dwgpio_port_driver);
> +	return platform_driver_register(&dwgpio_driver);
> +}
> +postcore_initcall(dwgpio_driver_register);