Re: [U-Boot] [PATCH v2] gpio: xilinx: Convert driver to DM
Hi Michal,
Am 23.07.2018 um 13:43 schrieb Michal Simek:
On 20.7.2018 22:05, Stefan Herbrechtsmeier wrote:
Am 13.07.2018 um 17:20 schrieb Michal Simek:
This patch is enabling GPIO_DM support to have an option to use this
driver together with zynq gpio driver.
!DM part is kept there till Microblaze is cleanup which will be done
hopefully soon.
Just a note:
There is no reason to initialize uc-priv->name because it is completely
unused.
Signed-off-by: Michal Simek
---
Changes in v2:
- Show value in set_value when debug is enabled
- Implement xlate function
- Remove tabs from structures for alignment (to be consistent with the
rest of code)
drivers/gpio/xilinx_gpio.c | 265
-
1 file changed, 264 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/xilinx_gpio.c b/drivers/gpio/xilinx_gpio.c
index 74c5be0865d1..48b52c985a55 100644
--- a/drivers/gpio/xilinx_gpio.c
+++ b/drivers/gpio/xilinx_gpio.c
[snip]
+static int xilinx_gpio_probe(struct udevice *dev)
+{
+ struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
+ struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+
+ uc_priv->bank_name = dev->name;
Have you check the "gpio status -a" output? Maybe you could use a
gpio-bank-name from the device tree.
The same as for zynq. gpio-bank-name is not in Linux. And yes I was
checking output. If you know better way please let me know.
Please see my other answer.
+
+ uc_priv->gpio_count = platdata->bank_max[0] + platdata->bank_max[1];
+
+ return 0;
+}
+
+static int xilinx_gpio_ofdata_to_platdata(struct udevice *dev)
+{
+ struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
+ int is_dual;
+
+ platdata->regs = (struct gpio_regs *)dev_read_addr(dev);
+
+ platdata->bank_max[0] = dev_read_u32_default(dev,
+ "xlnx,gpio-width", 0);
The default value of the Linux driver is 32.
but not in binding doc.
Okay
+ platdata->bank_input[0] = dev_read_u32_default(dev,
+ "xlnx,all-inputs", 0);
This isn't supported by the Linux driver but documented in the device
tree bindings.
correct.
+ platdata->bank_output[0] = dev_read_u32_default(dev,
+ "xlnx,all-outputs", 0);
This isn't supported by the Linux driver neither documented in the
device tree bindings.
This IP, driver and dt binding was done pretty long time ago. I could be
one of the first dt driver that's why there could issues with dt bindings.
DTG is generating all these properties from day one and in Linux only
documented property where that one which are used by Linux.
All that old dt binding docs should be checked again and there is
actually 22 patches sent to gpio mailing list
https://patchwork.ozlabs.org/patch/947371/
but I haven't looked at them yet.
Okay
+
+ is_dual = dev_read_u32_default(dev, "xlnx,is-dual", 0);
+ if (is_dual) {
+ platdata->bank_max[1] = dev_read_u32_default(dev,
+ "xlnx,gpio2-width", 0);
+ platdata->bank_input[1] = dev_read_u32_default(dev,
+ "xlnx,all-inputs-2", 0);
+ platdata->bank_output[1] = dev_read_u32_default(dev,
+ "xlnx,all-outputs-2", 0);
+ }
+
+ return 0;
+}
+
+static const struct udevice_id xilinx_gpio_ids[] = {
+ { .compatible = "xlnx,xps-gpio-1.00.a",},
+ { }
+};
+
+U_BOOT_DRIVER(xilinx_gpio) = {
+ .name = "xlnx_gpio",
+ .id = UCLASS_GPIO,
+ .ops = _gpio_ops,
+ .of_match = xilinx_gpio_ids,
+ .ofdata_to_platdata = xilinx_gpio_ofdata_to_platdata,
+ .probe = xilinx_gpio_probe,
+ .platdata_auto_alloc_size = sizeof(struct xilinx_gpio_platdata),
+};
+#endif
Maybe the Xilinx AXI GPIO LogiCore IP driver could be integrated into
the generic gpio-mmio driver. This driver is compatible to the hardware
and doesn't need a default value for the data register in the device tree.
There is no support for irq that's why I don't think this is going to fly.
I wasn't aware of the irq because the irq support is missing in the
xilinx driver at the moment. I only realized that the xilinx driver use
a subset of the generic gpio driver with a different binding.
Best regards
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] gpio: xilinx: Convert driver to DM
Hi,
On 20.7.2018 22:05, Stefan Herbrechtsmeier wrote:
> Hi Michal,
>
> Am 13.07.2018 um 17:20 schrieb Michal Simek:
>> This patch is enabling GPIO_DM support to have an option to use this
>> driver together with zynq gpio driver.
>> !DM part is kept there till Microblaze is cleanup which will be done
>> hopefully soon.
>>
>> Just a note:
>> There is no reason to initialize uc-priv->name because it is completely
>> unused.
>>
>> Signed-off-by: Michal Simek
>> ---
>>
>> Changes in v2:
>> - Show value in set_value when debug is enabled
>> - Implement xlate function
>> - Remove tabs from structures for alignment (to be consistent with the
>> rest of code)
>>
>> drivers/gpio/xilinx_gpio.c | 265
>> -
>> 1 file changed, 264 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpio/xilinx_gpio.c b/drivers/gpio/xilinx_gpio.c
>> index 74c5be0865d1..48b52c985a55 100644
>> --- a/drivers/gpio/xilinx_gpio.c
>> +++ b/drivers/gpio/xilinx_gpio.c
>> @@ -1,6 +1,6 @@
>> // SPDX-License-Identifier: GPL-2.0+
>> /*
>> - * Copyright (c) 2013 Xilinx, Michal Simek
>> + * Copyright (c) 2013 - 2018 Xilinx, Michal Simek
>> */
>> #include
>> @@ -9,6 +9,7 @@
>> #include
>> #include
>> #include
>> +#include
>> static LIST_HEAD(gpio_list);
>> @@ -23,6 +24,8 @@ struct gpio_regs {
>> u32 gpiodir;
>> };
>> +#if !defined(CONFIG_DM_GPIO)
>> +
>> #define GPIO_NAME_SIZE 10
>> struct gpio_names {
>> @@ -345,3 +348,263 @@ int gpio_alloc_dual(u32 baseaddr, const char
>> *name, u32 gpio_no0, u32 gpio_no1)
>> /* Return the first gpio allocated for this device */
>> return ret;
>> }
>> +#else
>> +#include
>> +
>> +#define XILINX_GPIO_MAX_BANK 2
>> +
>> +struct xilinx_gpio_platdata {
>> + struct gpio_regs *regs;
>> + int bank_max[XILINX_GPIO_MAX_BANK];
>> + int bank_input[XILINX_GPIO_MAX_BANK];
>> + int bank_output[XILINX_GPIO_MAX_BANK];
>> +};
>> +
>> +static int xilinx_gpio_get_bank_pin(unsigned offset, u32 *bank_num,
>> + u32 *bank_pin_num, struct udevice *dev)
>> +{
>> + struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
>> + u32 bank, max_pins;
>> + /* the first gpio is 0 not 1 */
>> + u32 pin_num = offset;
>> +
>> + for (bank = 0; bank < XILINX_GPIO_MAX_BANK; bank++) {
>> + max_pins = platdata->bank_max[bank];
>> + if (pin_num < max_pins) {
>> + debug("%s: found at bank 0x%x pin 0x%x\n", __func__,
>> + bank, pin_num);
>> + *bank_num = bank;
>> + *bank_pin_num = pin_num;
>> + return 0;
>> + }
>> + pin_num -= max_pins;
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static int xilinx_gpio_set_value(struct udevice *dev, unsigned offset,
>> + int value)
>> +{
>> + struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
>> + int val, ret;
>> + u32 bank, pin;
>> +
>> + ret = xilinx_gpio_get_bank_pin(offset, , , dev);
>> + if (ret)
>> + return ret;
>> +
>> + debug("%s: regs: %lx, value: %x, gpio: %x, bank %x, pin %x\n",
>> + __func__, (ulong)platdata->regs, value, offset, bank, pin);
>> +
>> + if (value) {
>> + val = readl(>regs->gpiodata + bank * 2);
>> + val = val | (1 << pin);
>> + writel(val, >regs->gpiodata + bank * 2);
>> + } else {
>> + val = readl(>regs->gpiodata + bank * 2);
>> + val = val & ~(1 << pin);
>> + writel(val, >regs->gpiodata + bank * 2);
>> + }
>
> The old driver doesn't read the value from the register because this
> values represents the external status of the inputs and not the value of
> the outputs. This implementation doesn't works with a wired-OR.
> Additionally you could simplify the code.
ah ok. This needs to be fixed.
>
>> +
>> + return val;
>> +};
>> +
>> +static int xilinx_gpio_get_value(struct udevice *dev, unsigned offset)
>> +{
>> + struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
>> + int val, ret;
>> + u32 bank, pin;
>> +
>> + ret = xilinx_gpio_get_bank_pin(offset, , , dev);
>> + if (ret)
>> + return ret;
>> +
>> + debug("%s: regs: %lx, gpio: %x, bank %x, pin %x\n", __func__,
>> + (ulong)platdata->regs, offset, bank, pin);
>> +
>> + val = readl(>regs->gpiodata + bank * 2);
>> + val = !!(val & (1 << pin));
>> +
>> + return val;
>> +};
>> +
>> +static int xilinx_gpio_get_function(struct udevice *dev, unsigned
>> offset)
>> +{
>> + struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
>> + int val, ret;
>> + u32 bank, pin;
>> +
>> + /* Check if all pins are inputs */
>> + if (platdata->bank_input[bank])
>> + return GPIOF_INPUT;
>> +
>> + /* Check if all pins are outputs */
>> + if (platdata->bank_output[bank])
>> + return GPIOF_OUTPUT;
>> +
>> + ret = xilinx_gpio_get_bank_pin(offset, , , dev);
>> + if (ret)
>> + return ret;
Re: [U-Boot] [PATCH v2] gpio: xilinx: Convert driver to DM
Hi Michal,
Am 13.07.2018 um 17:20 schrieb Michal Simek:
This patch is enabling GPIO_DM support to have an option to use this
driver together with zynq gpio driver.
!DM part is kept there till Microblaze is cleanup which will be done
hopefully soon.
Just a note:
There is no reason to initialize uc-priv->name because it is completely
unused.
Signed-off-by: Michal Simek
---
Changes in v2:
- Show value in set_value when debug is enabled
- Implement xlate function
- Remove tabs from structures for alignment (to be consistent with the
rest of code)
drivers/gpio/xilinx_gpio.c | 265 -
1 file changed, 264 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/xilinx_gpio.c b/drivers/gpio/xilinx_gpio.c
index 74c5be0865d1..48b52c985a55 100644
--- a/drivers/gpio/xilinx_gpio.c
+++ b/drivers/gpio/xilinx_gpio.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
/*
- * Copyright (c) 2013 Xilinx, Michal Simek
+ * Copyright (c) 2013 - 2018 Xilinx, Michal Simek
*/
#include
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
static LIST_HEAD(gpio_list);
@@ -23,6 +24,8 @@ struct gpio_regs {
u32 gpiodir;
};
+#if !defined(CONFIG_DM_GPIO)
+
#define GPIO_NAME_SIZE10
struct gpio_names {
@@ -345,3 +348,263 @@ int gpio_alloc_dual(u32 baseaddr, const char *name, u32
gpio_no0, u32 gpio_no1)
/* Return the first gpio allocated for this device */
return ret;
}
+#else
+#include
+
+#define XILINX_GPIO_MAX_BANK 2
+
+struct xilinx_gpio_platdata {
+ struct gpio_regs *regs;
+ int bank_max[XILINX_GPIO_MAX_BANK];
+ int bank_input[XILINX_GPIO_MAX_BANK];
+ int bank_output[XILINX_GPIO_MAX_BANK];
+};
+
+static int xilinx_gpio_get_bank_pin(unsigned offset, u32 *bank_num,
+ u32 *bank_pin_num, struct udevice *dev)
+{
+ struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
+ u32 bank, max_pins;
+ /* the first gpio is 0 not 1 */
+ u32 pin_num = offset;
+
+ for (bank = 0; bank < XILINX_GPIO_MAX_BANK; bank++) {
+ max_pins = platdata->bank_max[bank];
+ if (pin_num < max_pins) {
+ debug("%s: found at bank 0x%x pin 0x%x\n", __func__,
+ bank, pin_num);
+ *bank_num = bank;
+ *bank_pin_num = pin_num;
+ return 0;
+ }
+ pin_num -= max_pins;
+ }
+
+ return -EINVAL;
+}
+
+static int xilinx_gpio_set_value(struct udevice *dev, unsigned offset,
+int value)
+{
+ struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
+ int val, ret;
+ u32 bank, pin;
+
+ ret = xilinx_gpio_get_bank_pin(offset, , , dev);
+ if (ret)
+ return ret;
+
+ debug("%s: regs: %lx, value: %x, gpio: %x, bank %x, pin %x\n",
+ __func__, (ulong)platdata->regs, value, offset, bank, pin);
+
+ if (value) {
+ val = readl(>regs->gpiodata + bank * 2);
+ val = val | (1 << pin);
+ writel(val, >regs->gpiodata + bank * 2);
+ } else {
+ val = readl(>regs->gpiodata + bank * 2);
+ val = val & ~(1 << pin);
+ writel(val, >regs->gpiodata + bank * 2);
+ }
The old driver doesn't read the value from the register because this
values represents the external status of the inputs and not the value of
the outputs. This implementation doesn't works with a wired-OR.
Additionally you could simplify the code.
+
+ return val;
+};
+
+static int xilinx_gpio_get_value(struct udevice *dev, unsigned offset)
+{
+ struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
+ int val, ret;
+ u32 bank, pin;
+
+ ret = xilinx_gpio_get_bank_pin(offset, , , dev);
+ if (ret)
+ return ret;
+
+ debug("%s: regs: %lx, gpio: %x, bank %x, pin %x\n", __func__,
+ (ulong)platdata->regs, offset, bank, pin);
+
+ val = readl(>regs->gpiodata + bank * 2);
+ val = !!(val & (1 << pin));
+
+ return val;
+};
+
+static int xilinx_gpio_get_function(struct udevice *dev, unsigned offset)
+{
+ struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
+ int val, ret;
+ u32 bank, pin;
+
+ /* Check if all pins are inputs */
+ if (platdata->bank_input[bank])
+ return GPIOF_INPUT;
+
+ /* Check if all pins are outputs */
+ if (platdata->bank_output[bank])
+ return GPIOF_OUTPUT;
+
+ ret = xilinx_gpio_get_bank_pin(offset, , , dev);
+ if (ret)
+ return ret;
This must be called before the checks otherwise the bank is undefined.
+
+ /* FIXME test on dual */
+ val = readl(>regs->gpiodir + bank * 2);
+ val = !(val & (1 << pin));
+
+ /*
[U-Boot] [PATCH v2] gpio: xilinx: Convert driver to DM
This patch is enabling GPIO_DM support to have an option to use this
driver together with zynq gpio driver.
!DM part is kept there till Microblaze is cleanup which will be done
hopefully soon.
Just a note:
There is no reason to initialize uc-priv->name because it is completely
unused.
Signed-off-by: Michal Simek
---
Changes in v2:
- Show value in set_value when debug is enabled
- Implement xlate function
- Remove tabs from structures for alignment (to be consistent with the
rest of code)
drivers/gpio/xilinx_gpio.c | 265 -
1 file changed, 264 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/xilinx_gpio.c b/drivers/gpio/xilinx_gpio.c
index 74c5be0865d1..48b52c985a55 100644
--- a/drivers/gpio/xilinx_gpio.c
+++ b/drivers/gpio/xilinx_gpio.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
/*
- * Copyright (c) 2013 Xilinx, Michal Simek
+ * Copyright (c) 2013 - 2018 Xilinx, Michal Simek
*/
#include
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
static LIST_HEAD(gpio_list);
@@ -23,6 +24,8 @@ struct gpio_regs {
u32 gpiodir;
};
+#if !defined(CONFIG_DM_GPIO)
+
#define GPIO_NAME_SIZE 10
struct gpio_names {
@@ -345,3 +348,263 @@ int gpio_alloc_dual(u32 baseaddr, const char *name, u32
gpio_no0, u32 gpio_no1)
/* Return the first gpio allocated for this device */
return ret;
}
+#else
+#include
+
+#define XILINX_GPIO_MAX_BANK 2
+
+struct xilinx_gpio_platdata {
+ struct gpio_regs *regs;
+ int bank_max[XILINX_GPIO_MAX_BANK];
+ int bank_input[XILINX_GPIO_MAX_BANK];
+ int bank_output[XILINX_GPIO_MAX_BANK];
+};
+
+static int xilinx_gpio_get_bank_pin(unsigned offset, u32 *bank_num,
+ u32 *bank_pin_num, struct udevice *dev)
+{
+ struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
+ u32 bank, max_pins;
+ /* the first gpio is 0 not 1 */
+ u32 pin_num = offset;
+
+ for (bank = 0; bank < XILINX_GPIO_MAX_BANK; bank++) {
+ max_pins = platdata->bank_max[bank];
+ if (pin_num < max_pins) {
+ debug("%s: found at bank 0x%x pin 0x%x\n", __func__,
+ bank, pin_num);
+ *bank_num = bank;
+ *bank_pin_num = pin_num;
+ return 0;
+ }
+ pin_num -= max_pins;
+ }
+
+ return -EINVAL;
+}
+
+static int xilinx_gpio_set_value(struct udevice *dev, unsigned offset,
+int value)
+{
+ struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
+ int val, ret;
+ u32 bank, pin;
+
+ ret = xilinx_gpio_get_bank_pin(offset, , , dev);
+ if (ret)
+ return ret;
+
+ debug("%s: regs: %lx, value: %x, gpio: %x, bank %x, pin %x\n",
+ __func__, (ulong)platdata->regs, value, offset, bank, pin);
+
+ if (value) {
+ val = readl(>regs->gpiodata + bank * 2);
+ val = val | (1 << pin);
+ writel(val, >regs->gpiodata + bank * 2);
+ } else {
+ val = readl(>regs->gpiodata + bank * 2);
+ val = val & ~(1 << pin);
+ writel(val, >regs->gpiodata + bank * 2);
+ }
+
+ return val;
+};
+
+static int xilinx_gpio_get_value(struct udevice *dev, unsigned offset)
+{
+ struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
+ int val, ret;
+ u32 bank, pin;
+
+ ret = xilinx_gpio_get_bank_pin(offset, , , dev);
+ if (ret)
+ return ret;
+
+ debug("%s: regs: %lx, gpio: %x, bank %x, pin %x\n", __func__,
+ (ulong)platdata->regs, offset, bank, pin);
+
+ val = readl(>regs->gpiodata + bank * 2);
+ val = !!(val & (1 << pin));
+
+ return val;
+};
+
+static int xilinx_gpio_get_function(struct udevice *dev, unsigned offset)
+{
+ struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
+ int val, ret;
+ u32 bank, pin;
+
+ /* Check if all pins are inputs */
+ if (platdata->bank_input[bank])
+ return GPIOF_INPUT;
+
+ /* Check if all pins are outputs */
+ if (platdata->bank_output[bank])
+ return GPIOF_OUTPUT;
+
+ ret = xilinx_gpio_get_bank_pin(offset, , , dev);
+ if (ret)
+ return ret;
+
+ /* FIXME test on dual */
+ val = readl(>regs->gpiodir + bank * 2);
+ val = !(val & (1 << pin));
+
+ /* input is 1 in reg but GPIOF_INPUT is 0 */
+ /* output is 0 in reg but GPIOF_OUTPUT is 1 */
+
+ return val;
+}
+
+static int xilinx_gpio_direction_output(struct udevice *dev, unsigned offset,
+ int value)
+{
+ struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
+ int val, ret;
+ u32 bank, pin;
+
+ ret = xilinx_gpio_get_bank_pin(offset, , ,
