On Wed, May 16 2018, Sergio Paracuellos wrote:
> Gpio driver have a some globals which can be avoided just
> using platform_data in a proper form. This commit adds a new
> struct mtk_data which includes all of those globals setting them
> using platform_set_drvdata and devm_gpiochip_add_data functions.
> With this properly set we are able to retrieve driver data along
> the code using kernel api's so globals are not needed anymore.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> ---
> drivers/staging/mt7621-gpio/gpio-mt7621.c | 85 +++++++++++++++++++++----------
> 1 file changed, 59 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> index 7d17949..c701259 100644
> --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
> +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> @@ -31,17 +31,20 @@ enum mediatek_gpio_reg {
> GPIO_REG_EDGE,
> };
>
> -static void __iomem *mediatek_gpio_membase;
> -static int mediatek_gpio_irq;
> -static struct irq_domain *mediatek_gpio_irq_domain;
> -
> -static struct mtk_gc {
> +struct mtk_gc {
> struct gpio_chip chip;
> spinlock_t lock;
> int bank;
> u32 rising;
> u32 falling;
> -} *gc_map[MTK_MAX_BANK];
> +};
> +
> +struct mtk_data {
> + void __iomem *gpio_membase;
> + int gpio_irq;
> + struct irq_domain *gpio_irq_domain;
> + struct mtk_gc *gc_map[MTK_MAX_BANK];
> +};
>
> static inline struct mtk_gc
> *to_mediatek_gpio(struct gpio_chip *chip)
> @@ -56,15 +59,19 @@ static inline struct mtk_gc
> static inline void
> mtk_gpio_w32(struct mtk_gc *rg, u8 reg, u32 val)
> {
> - iowrite32(val, mediatek_gpio_membase + (reg * 0x10) + (rg->bank * 0x4));
> + struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
> + u32 offset = (reg * 0x10) + (rg->bank * 0x4);
> +
> + iowrite32(val, gpio_data->gpio_membase + offset);
> }
>
> static inline u32
> mtk_gpio_r32(struct mtk_gc *rg, u8 reg)
> {
> + struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
> u32 offset = (reg * 0x10) + (rg->bank * 0x4);
>
> - return ioread32(mediatek_gpio_membase + offset);
> + return ioread32(gpio_data->gpio_membase + offset);
> }
>
> static void
> @@ -137,23 +144,26 @@ mediatek_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
> static int
> mediatek_gpio_to_irq(struct gpio_chip *chip, unsigned int pin)
> {
> + struct mtk_data *gpio_data = gpiochip_get_data(chip);
> struct mtk_gc *rg = to_mediatek_gpio(chip);
>
> - return irq_create_mapping(mediatek_gpio_irq_domain,
> + return irq_create_mapping(gpio_data->gpio_irq_domain,
> pin + (rg->bank * MTK_BANK_WIDTH));
> }
>
> static int
> mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node *bank)
> {
> + struct mtk_data *gpio_data = dev_get_drvdata(&pdev->dev);
> const __be32 *id = of_get_property(bank, "reg", NULL);
> struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
> sizeof(struct mtk_gc), GFP_KERNEL);
> + int ret;
>
> if (!rg || !id || be32_to_cpu(*id) > MTK_MAX_BANK)
> return -ENOMEM;
>
> - gc_map[be32_to_cpu(*id)] = rg;
> + gpio_data->gc_map[be32_to_cpu(*id)] = rg;
>
> memset(rg, 0, sizeof(struct mtk_gc));
>
> @@ -169,10 +179,17 @@ mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node *bank)
> rg->chip.get_direction = mediatek_gpio_get_direction;
> rg->chip.get = mediatek_gpio_get;
> rg->chip.set = mediatek_gpio_set;
> - if (mediatek_gpio_irq_domain)
> + if (gpio_data->gpio_irq_domain)
> rg->chip.to_irq = mediatek_gpio_to_irq;
> rg->bank = be32_to_cpu(*id);
>
> + ret = devm_gpiochip_add_data(&pdev->dev, &rg->chip, gpio_data);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Could not register gpio %d, ret=%d\n",
> + rg->chip.ngpio, ret);
> + return ret;
> + }
> +
Calling devm_gpiochip_add_data() here looks good.
Not removing
return gpiochip_add(&rg->chip);
from the end of the function isn't so good :-(
BTW interrupt definitely don't work yet. The calls
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
get NULL. I think some sort of irq_set_chip_data(irq,...) is needed
in mediatek_gpio_gpio_map(), but it is too late at night for it to
all make sense to me :-)
Thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
