On Wed, May 16, 2018 at 09:59:09AM +1000, NeilBrown wrote:
> On Tue, May 15 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 <[email protected]>
> > ---
> >
> > I have not tested this because I don't hardware to do it but I
> > have compile it just to be sure it compiles. This patch needs
>
> I've tested it. Doesn't work :-( Close though.
> mtk_gpio_w32() needs the gpio_data, so it cannot be called before
> devm_gpiochip_add_data() is called.
> This:
>
> diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c
> b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> index be27cbdcbfa8..48141d979059 100644
> --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
> +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> @@ -185,9 +185,6 @@ mediatek_gpio_bank_probe(struct platform_device *pdev,
> struct device_node *bank)
> rg->chip.to_irq = mediatek_gpio_to_irq;
> rg->bank = be32_to_cpu(*id);
>
> - /* set polarity to low for all gpios */
> - mtk_gpio_w32(rg, GPIO_REG_POL, 0);
> -
> 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",
> @@ -195,6 +192,9 @@ mediatek_gpio_bank_probe(struct platform_device *pdev,
> struct device_node *bank)
> return ret;
> }
>
> + /* set polarity to low for all gpios */
> + mtk_gpio_w32(rg, GPIO_REG_POL, 0);
> +
> dev_info(&pdev->dev, "registering %d gpios\n", rg->chip.ngpio);
>
> return gpiochip_add(&rg->chip);
>
>
> makes it work. It probably won't work once I try interrupts as
> mediatek_gpio_to_irq() also needs gpio_data, and that is currently
> called early.
>
> Also:
>
> >
> > 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);
> > }
> >
>
> This hunk doesn't apply.
> The existing code is:
>
> static inline u32
> mtk_gpio_r32(struct mtk_gc *rg, u8 reg)
> {
> return ioread32(mediatek_gpio_membase + (reg * 0x10) + (rg->bank *
> 0x4));
> }
>
> Thanks!
>
> NeilBrown
I have just sent v2 with things you pointed out here probably fixed and other
remaining
cleanups pointed in this thread.
Hope this helps,
Best regards,
Sergio Paracuellos
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
