Re: [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO)

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 lukma@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->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->reg_set);
  

BIT(), fix globally.

• else

• writel(1 << offset, &reg->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->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->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->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 */