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 <sergio.paracuellos@xxxxxxxxx>
> > ---
> >
> > 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.
True. I'll change this and send v2.
>
> 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));
> }
Mmmm... This is rebased on the top of staging-next and the existing code there
is not the one you are pointing out here. The only different code between
this patch and the existing code is the use of 'mediatek' instead of 'mtk' for
the bindings which are in my previous patch series which haven't be applied
yet. Should I join all of them with this stuff fixed and send them all again for
your better review?
Best regards,
Sergio Paracuellos
>
> Thanks!
>
> NeilBrown
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
