Messages in this thread | | | Date | Tue, 1 Dec 2020 14:02:04 +0900 | From | Nobuhiro Iwamatsu <> | Subject | Re: [PATCH v2 2/4] gpio: visoconti: Add Toshiba Visconti GPIO support |
| |
Hi,
Thanks for your review.
On Tue, Nov 17, 2020 at 09:59:06PM +0100, Linus Walleij wrote: > Hi Nobuhiro, > > On Thu, Nov 12, 2020 at 12:42 AM Nobuhiro Iwamatsu > <nobuhiro1.iwamatsu@toshiba.co.jp> wrote: > > > Add the GPIO driver for Toshiba Visconti ARM SoCs. > > > > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp> > > I just noticed this: > > > +config GPIO_VISCONTI > > + tristate "Toshiba Visconti GPIO support" > > + depends on ARCH_VISCONTI || COMPILE_TEST > > + depends on OF_GPIO > > + select GPIOLIB_IRQCHIP > > + help > > + Say yes here to support GPIO on Tohisba Visconti. > > Add: > select GPIO_GENERIC
OK, I will add this.
> > Then all of these: > > > +static int visconti_gpio_get(struct gpio_chip *chip, unsigned int offset) > > +static void visconti_gpio_set(struct gpio_chip *chip, unsigned int offset, int value) > > +static int visconti_gpio_get_dir(struct gpio_chip *chip, unsigned int offset) > > +static int visconti_gpio_dir_in(struct gpio_chip *chip, unsigned int offset) > > +static int visconti_gpio_dir_out(struct gpio_chip *chip, unsigned int offset, int value) > > Can be implemented by the genric MMIO GPIO library.
I see. I will update using generic MMIO GPIO library.
> > > + gpio_chip = &priv->gpio_chip; > > + gpio_chip->label = name; > > + gpio_chip->owner = THIS_MODULE; > > + gpio_chip->parent = dev; > > + gpio_chip->request = gpiochip_generic_request; > > + gpio_chip->free = gpiochip_generic_free; > > + gpio_chip->get = visconti_gpio_get; > > + gpio_chip->set = visconti_gpio_set; > > + gpio_chip->get_direction = visconti_gpio_get_dir; > > + gpio_chip->direction_input = visconti_gpio_dir_in; > > + gpio_chip->direction_output = visconti_gpio_dir_out; > > + gpio_chip->base = 0; > > + gpio_chip->ngpio = VISCONTI_GPIO_NR; > > + gpio_chip->irq.init_valid_mask = visconti_init_irq_valid_mask; > > Initialized the generic helpers using the addresses of the > GPIO registers here by a call to bgpio_init(). > > Check this driver for an example: > drivers/gpio/gpio-ftgpio010.c > > If you get uncertain about the arguments to bgpio_init() > check drivers/gpio/gpio-mmio.c, there is kerneldoc for the > function. > > By doing this you get implementations of gpio_[get|set]_multiple() > for free.
Thanks for your suggestion. I see. I will check these and update.
> > Yours, > Linus Walleij
Best regards, Nobuhiro
| |