On 6/17/19 10:37 AM, Lukasz Majewski wrote:
Hi Marek,
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.
Ok. Will rewrite the commit message to reflect the changes in the commit.
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.
I would have used the gpio-ranges, but the original Linux code [1] (imx28.dtsi) doesn't define them.
You can add them to imx28-u-boot.dtsi , and submit patch for mainline Linux, it's easy.
Also, the dts files which include [1] don't extend original gpio nodes to have one.
As it is not available in the Linux kernel, I don't see any good reason to add the gpio-ranges to U-Boot. The same approach, as presented here, is used in zynq_gpio.c file (which also uses DM/DTS).
Adding per u-boot property - like 'u-boot,mx28-gpio-ranges' is also less appealing than 24 lines of code, which can be then easily re-used with OF_PLATDATA (without DM/DTS suport) in SPL (u-boot.sb to be precise).
It is well possible the zynq DTs predate the gpio-ranges . Read the documentation for it at [2] . It even explicitly states it's used for interaction between pincontrol and gpio controllers.
[2] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bind...
+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 .
This is a compatibility layer - for some reason ALL iMX ports define it
- i.e. imx8, imx7 - which shall use DM/DTS for gpio from the outset.
With the in-board code the dm_gpio_* set of functions shall (and will) be used (as done in opos6ul.c file).
Then use them and drop this.
+#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.
Please see the above comment regarding gpio-ranges.
DTTO
+#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.
I took it from the original implementation :-).
Original implementation is very old code.
[...]