Re: [PATCH v2] gpio: zx: Add ZTE zx296702 GPIO support

[Shawn Guo <shawn.guo@xxxxxxxxxx>]

Jun,

A couple of comments below.

On Wed, Jun 24, 2015 at 5:31 PM, Jun Nie <jun.nie@xxxxxxxxxx> wrote:
>
> Add ZTE zx296702 GPIO controller support
>
> Signed-off-by: Jun Nie <jun.nie@xxxxxxxxxx>
> ---
>  drivers/gpio/Kconfig         |   6 +
>  drivers/gpio/Makefile        |   1 +
>  drivers/gpio/gpio-zx296702.c | 324 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 331 insertions(+)
>  create mode 100644 drivers/gpio/gpio-zx296702.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index caefe80..aba5226 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -970,6 +970,12 @@ config GPIO_MC33880
>           SPI driver for Freescale MC33880 high-side/low-side switch.
>           This provides GPIO interface supporting inputs and outputs.
>
> +config GPIO_ZX296702
> +       bool "ZTE ZX296702 GPIO support"
> +       select GPIOLIB_IRQCHIP
> +       help
> +         Say yes here to support the ZTE ZX296702 GPIO device
> +

This GPIO block is available on ZTE ZX family SoCs, not only ZX296702.
I suggest you name the option GPIO_ZX and the driver gpio-zx.

[...]

> +static int zx_direction_output(struct gpio_chip *gc, unsigned offset,
> +               int value)
> +{
> +       struct zx_gpio *chip = to_zx(gc);
> +       unsigned long flags;
> +       unsigned char gpiodir;

It's a 8-bit variable.  Is it what you want?  I guess you need to use
unsigned short to get a 16-bit variable.

Shawn

> +
> +       if (offset >= gc->ngpio)
> +               return -EINVAL;
> +
> +       spin_lock_irqsave(&chip->lock, flags);
> +       gpiodir = readw_relaxed(chip->base + ZX_GPIO_DIR);
> +       gpiodir |= BIT(offset);
> +       writew_relaxed(gpiodir, chip->base + ZX_GPIO_DIR);
> +
> +       if (value)
> +               writew_relaxed(BIT(offset), chip->base + ZX_GPIO_DO1);
> +       else
> +               writew_relaxed(BIT(offset), chip->base + ZX_GPIO_DO0);
> +       spin_unlock_irqrestore(&chip->lock, flags);
> +
> +       return 0;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html