On 6/16/19 12:34 AM, Lukasz Majewski wrote:
> This commit
This is not a commit, it's a change. It only becomes a commit when applied.
> adds support for DM in the mxs_gpio.c driver when DM_GPIO
> is enabled in Kconfig.
But this also adds support for DT probing, which is orthogonal to DM
support, yet it's not documented in the commit message.
> Signed-off-by: Lukasz Majewski <lu...@denx.de>
>
> ---
>
> Changes in v3:
> - Set more apropriate tags
>
> Changes in v2:
> - Use #if !CONFIG_IS_ENABLED(DM_GPIO) instead of plain #ifdef CONFIG_DM_GPIO
>
> arch/arm/include/asm/arch-mxs/gpio.h | 28 +++++++
> drivers/gpio/mxs_gpio.c | 149
> +++++++++++++++++++++++++++++++++++
> 2 files changed, 177 insertions(+)
>
> diff --git a/arch/arm/include/asm/arch-mxs/gpio.h
> b/arch/arm/include/asm/arch-mxs/gpio.h
> index 34fa421945..0c152e25e2 100644
> --- a/arch/arm/include/asm/arch-mxs/gpio.h
> +++ b/arch/arm/include/asm/arch-mxs/gpio.h
> @@ -15,4 +15,32 @@ void mxs_gpio_init(void);
> inline void mxs_gpio_init(void) {}
> #endif
>
> +#if defined(CONFIG_MX28) && CONFIG_IS_ENABLED(DM_GPIO)
> +/*
> + * According to i.MX28 Reference Manual:
> + * 'i.MX28 Applications Processor Reference Manual, Rev. 1, 2010'
> + * The i.MX28 has following number of GPIOs available:
> + * Bank 0: 0-28 -> 29 PINS
> + * Bank 1: 0-31 -> 32 PINS
> + * Bank 2: 0-27 -> 28 PINS
> + * Bank 3: 0-30 -> 31 PINS
> + * Bank 4: 0-20 -> 21 PINS
> + */
> +#define IMX28_GPIO_BANK0_PIN_NR 29
> +#define IMX28_GPIO_BANK1_PIN_NR 32
> +#define IMX28_GPIO_BANK2_PIN_NR 28
> +#define IMX28_GPIO_BANK3_PIN_NR 31
> +#define IMX28_GPIO_BANK4_PIN_NR 21
> +#define IMX28_GPIO_BANK_NR 5
Please make a complete conversion, not partial one.
You want to use gpio-ranges in DT and then parse the size of each GPIO
bank from those gpio-ranges , not hard-code it into the driver.
> +struct mxs_gpio_platdata {
> + u32 gpio_nr_of_bank_pins[IMX28_GPIO_BANK_NR];
> + u32 gpio_base_nr[IMX28_GPIO_BANK_NR];
> + u32 ngpio;
> +};
> +
> +extern const struct mxs_gpio_platdata mxs_gpio_def;
> +#define IMX_GPIO_NR(port, index) (mxs_gpio_def.gpio_base_nr[(port)] +
> (index))
So this should be completely unnecessary when using the DM GPIO, this
was only needed for non-DM-GPIO .
> +#endif
> +
> #endif /* __MX28_GPIO_H__ */
> diff --git a/drivers/gpio/mxs_gpio.c b/drivers/gpio/mxs_gpio.c
> index c2c8a25886..f386235ff1 100644
> --- a/drivers/gpio/mxs_gpio.c
> +++ b/drivers/gpio/mxs_gpio.c
> @@ -51,6 +51,7 @@ void mxs_gpio_init(void)
> }
> }
>
> +#if !CONFIG_IS_ENABLED(DM_GPIO)
> int gpio_get_value(unsigned gpio)
> {
> uint32_t bank = PAD_BANK(gpio);
> @@ -127,3 +128,151 @@ int name_to_gpio(const char *name)
>
> return (bank << MXS_PAD_BANK_SHIFT) | (pin << MXS_PAD_PIN_SHIFT);
> }
> +#else /* CONFIG_DM_GPIO */
> +#include <dm.h>
> +#include <asm/gpio.h>
> +#include <asm/arch/gpio.h>
> +#ifdef CONFIG_MX28
> +const struct mxs_gpio_platdata mxs_gpio_def = {
> + .gpio_nr_of_bank_pins[0] = IMX28_GPIO_BANK0_PIN_NR,
> + .gpio_nr_of_bank_pins[1] = IMX28_GPIO_BANK1_PIN_NR,
> + .gpio_nr_of_bank_pins[2] = IMX28_GPIO_BANK2_PIN_NR,
> + .gpio_nr_of_bank_pins[3] = IMX28_GPIO_BANK3_PIN_NR,
> + .gpio_nr_of_bank_pins[4] = IMX28_GPIO_BANK4_PIN_NR,
> + .gpio_base_nr[0] = 0,
> + .gpio_base_nr[1] = IMX28_GPIO_BANK0_PIN_NR,
> + .gpio_base_nr[2] = (IMX28_GPIO_BANK0_PIN_NR + \
> + IMX28_GPIO_BANK1_PIN_NR),
> + .gpio_base_nr[3] = (IMX28_GPIO_BANK0_PIN_NR + \
> + IMX28_GPIO_BANK1_PIN_NR + \
> + IMX28_GPIO_BANK2_PIN_NR),
> + .gpio_base_nr[4] = (IMX28_GPIO_BANK0_PIN_NR + \
> + IMX28_GPIO_BANK1_PIN_NR + \
> + IMX28_GPIO_BANK2_PIN_NR + \
> + IMX28_GPIO_BANK3_PIN_NR),
> + .ngpio = (IMX28_GPIO_BANK0_PIN_NR + \
> + IMX28_GPIO_BANK1_PIN_NR + \
> + IMX28_GPIO_BANK2_PIN_NR + \
> + IMX28_GPIO_BANK3_PIN_NR + \
> + IMX28_GPIO_BANK4_PIN_NR),
> +};
So please use gpio-ranges in DT.
> +#else
> +#error "Only i.MX28 supported with DM_GPIO"
> +#endif
> +
> +struct mxs_gpio_priv {
> + unsigned int bank;
> +};
> +
> +static int mxs_gpio_get_value(struct udevice *dev, unsigned offset)
> +{
> + struct mxs_gpio_priv *priv = dev_get_priv(dev);
> + struct mxs_register_32 *reg =
> + (struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> + PINCTRL_DIN(priv->bank));
> +
> + return (readl(®->reg) >> offset) & 1;
> +}
> +
> +static int mxs_gpio_set_value(struct udevice *dev, unsigned offset,
> + int value)
> +{
> + struct mxs_gpio_priv *priv = dev_get_priv(dev);
> + struct mxs_register_32 *reg =
> + (struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> + PINCTRL_DOUT(priv->bank));
> + if (value)
> + writel(1 << offset, ®->reg_set);
BIT(), fix globally.
> + else
> + writel(1 << offset, ®->reg_clr);
> +
> + return 0;
> +}
> +
> +static int mxs_gpio_direction_input(struct udevice *dev, unsigned offset)
> +{
> + struct mxs_gpio_priv *priv = dev_get_priv(dev);
> + struct mxs_register_32 *reg =
> + (struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> + PINCTRL_DOE(priv->bank));
> +
> + writel(1 << offset, ®->reg_clr);
> +
> + return 0;
> +}
> +
> +static int mxs_gpio_direction_output(struct udevice *dev, unsigned offset,
> + int value)
> +{
> + struct mxs_gpio_priv *priv = dev_get_priv(dev);
> + struct mxs_register_32 *reg =
> + (struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> + PINCTRL_DOE(priv->bank));
> +
> + mxs_gpio_set_value(dev, offset, value);
> +
> + writel(1 << offset, ®->reg_set);
> +
> + return 0;
> +}
> +
> +static int mxs_gpio_get_function(struct udevice *dev, unsigned offset)
> +{
> + struct mxs_gpio_priv *priv = dev_get_priv(dev);
> + struct mxs_register_32 *reg =
> + (struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> + PINCTRL_DOE(priv->bank));
> + bool is_output = !!(readl(®->reg) >> offset);
> +
> + return is_output ? GPIOF_OUTPUT : GPIOF_INPUT;
> +}
> +
> +static const struct dm_gpio_ops gpio_mxs_ops = {
> + .direction_input = mxs_gpio_direction_input,
> + .direction_output = mxs_gpio_direction_output,
> + .get_value = mxs_gpio_get_value,
> + .set_value = mxs_gpio_set_value,
> + .get_function = mxs_gpio_get_function,
> +};
> +
> +static int mxs_gpio_probe(struct udevice *dev)
> +{
> + struct mxs_gpio_priv *priv = dev_get_priv(dev);
> + struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> + struct mxs_gpio_platdata *pdata =
> + (struct mxs_gpio_platdata *)dev_get_driver_data(dev);
> + char name[18], *str;
> + int ret;
> +
> + ret = dev_read_u32(dev, "reg", &priv->bank);
devfdt_get_addr()
> + if (ret) {
> + printf("%s: No 'reg' property defined!\n", __func__);
> + return ret;
> + }
> +
> + sprintf(name, "GPIO%d_", priv->bank);
Name cannot conceivably have more than 16 characters, why is the array
18 characters large ? Also, snprintf() would likely be better here.
> + str = strdup(name);
> + if (!str)
> + return -ENOMEM;
> +
> + uc_priv->bank_name = str;
> + uc_priv->gpio_count = pdata->gpio_nr_of_bank_pins[priv->bank];
> +
> + return 0;
> +}
> +
> +static const struct udevice_id mxs_gpio_ids[] = {
> + { .compatible = "fsl,imx28-gpio", .data = (ulong)&mxs_gpio_def },
> + { }
> +};
> +
> +U_BOOT_DRIVER(gpio_mxs) = {
> + .name = "gpio_mxs",
> + .id = UCLASS_GPIO,
> + .ops = &gpio_mxs_ops,
> + .probe = mxs_gpio_probe,
> + .priv_auto_alloc_size = sizeof(struct mxs_gpio_priv),
> + .of_match = mxs_gpio_ids,
> +};
> +#endif /* CONFIG_DM_GPIO */
>
--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
