Re: [PATCH v1 4/4] gpio: dwapb: Get rid of legacy platform data

[Serge Semin <fancer.lancer@xxxxxxxxx>]

On Mon, Jul 26, 2021 at 03:54:36PM +0300, Andy Shevchenko wrote:
> Platform data is a legacy interface to supply device properties
> to the driver. In this case we don't have anymore in-kernel users
> for it. Just remove it for good.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpio/gpio-dwapb.c                | 28 +++++++++++++++---------
>  include/linux/platform_data/gpio-dwapb.h | 24 --------------------
>  2 files changed, 18 insertions(+), 34 deletions(-)
>  delete mode 100644 include/linux/platform_data/gpio-dwapb.h
> 
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index e3011d4e17b0..b9dd0ba812dc 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -16,7 +16,6 @@
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> -#include <linux/platform_data/gpio-dwapb.h>
>  #include <linux/platform_device.h>
>  #include <linux/property.h>
>  #include <linux/reset.h>
> @@ -48,6 +47,7 @@
>  
>  #define DWAPB_DRIVER_NAME	"gpio-dwapb"
>  #define DWAPB_MAX_PORTS		4
> +#define DWAPB_MAX_GPIOS		32
>  
>  #define GPIO_EXT_PORT_STRIDE	0x04 /* register stride 32 bits */
>  #define GPIO_SWPORT_DR_STRIDE	0x0c /* register stride 3*32 bits */
> @@ -63,6 +63,19 @@
>  
>  #define DWAPB_NR_CLOCKS		2
>  

> +struct dwapb_port_property {
> +	struct fwnode_handle *fwnode;
> +	unsigned int idx;
> +	unsigned int ngpio;
> +	unsigned int gpio_base;
> +	int irq[DWAPB_MAX_GPIOS];
> +};
> +
> +struct dwapb_platform_data {
> +	struct dwapb_port_property *properties;
> +	unsigned int nports;
> +};
> +
>  struct dwapb_gpio;

If you need to resend the series anyway could you please move the
structures declarations to being below the forward declaration of the
dwapb_gpio structure? Of course it's not that critical, but for the
sake of just not to have the later one left somewhere in the middle of
the unrelated structures and for at least to keep some order in the
declarations.

Then feel free to add:
Acked-by: Serge Semin <fancer.lancer@xxxxxxxxx>

The whole series has been tested on Baikal-T1 SoC:
Tested-by: Serge Semin <fancer.lancer@xxxxxxxxx>

-Sergey

>  
>  #ifdef CONFIG_PM_SLEEP
> @@ -670,17 +683,12 @@ static int dwapb_gpio_probe(struct platform_device *pdev)
>  	unsigned int i;
>  	struct dwapb_gpio *gpio;
>  	int err;
> +	struct dwapb_platform_data *pdata;
>  	struct device *dev = &pdev->dev;
> -	struct dwapb_platform_data *pdata = dev_get_platdata(dev);
> -
> -	if (!pdata) {
> -		pdata = dwapb_gpio_get_pdata(dev);
> -		if (IS_ERR(pdata))
> -			return PTR_ERR(pdata);
> -	}
>  
> -	if (!pdata->nports)
> -		return -ENODEV;
> +	pdata = dwapb_gpio_get_pdata(dev);
> +	if (IS_ERR(pdata))
> +		return PTR_ERR(pdata);
>  
>  	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
>  	if (!gpio)
> diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h
> deleted file mode 100644
> index 535e5ed549d9..000000000000
> --- a/include/linux/platform_data/gpio-dwapb.h
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * Copyright(c) 2014 Intel Corporation.
> - */
> -
> -#ifndef GPIO_DW_APB_H
> -#define GPIO_DW_APB_H
> -
> -#define DWAPB_MAX_GPIOS		32
> -
> -struct dwapb_port_property {
> -	struct fwnode_handle *fwnode;
> -	unsigned int	idx;
> -	unsigned int	ngpio;
> -	unsigned int	gpio_base;
> -	int		irq[DWAPB_MAX_GPIOS];
> -};
> -
> -struct dwapb_platform_data {
> -	struct dwapb_port_property *properties;
> -	unsigned int nports;
> -};
> -
> -#endif
> -- 
> 2.30.2
>