WIP: tegra: Use driver model for GPIO driver
Subsystems (3)
PATCH 1/1 WIP: tegra: Use driver model for GPIO driver
This is an implementation of GPIOs for Tegra that uses driver model. It is written for comment and need work and testing before it is ready to use.
Specific points for discussion:
1. I can't find much in the way of GPIO device tree bindings, so ended up just creating the GPIO devices
3. Driver model understand the concept of a bank of GPIOs, but this is equivalent to 'port' in Tegra. So it is somewhat confusing. Need to think about this.
3. Since we need a separate device for each named bank, there is a top-level GPIO device type which creates the GPIO banks. The GPIO bank is the actual device.
4. There are a few minor changes in the driver model framework to make things easier. I'll turn these into patches in the end.
Please take a look and let me know what you think.
---
arch/arm/dts/tegra20.dtsi | 142 ++++++++++++ arch/arm/include/asm/arch-tegra/gpio.h | 14 +- board/nvidia/seaboard/seaboard.c | 2 +- drivers/core/lists.c | 1 + drivers/core/root.c | 7 +- drivers/core/uclass.c | 2 +- drivers/gpio/tegra_gpio.c | 390 ++++++++++++++++++++++++--------- include/configs/tegra-common.h | 3 + include/dm/device-internal.h | 4 + test/dm/Makefile | 2 + 10 files changed, 453 insertions(+), 114 deletions(-)
arch/arm/dts/tegra20.dtsi +142 -0
@@ -143,6 +143,148 @@ interrupts = < 64 65 66 67 87 119 121 >; #gpio-cells = <2>; gpio-controller; + #address-cells = <1>; + #size-cells = <0>; + port at 0 { + reg = <0>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "a"; + }; + port at 1 { + reg = <1>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "b"; + }; + port at 2 { + reg = <2>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "c"; + }; + port at 3 { + reg = <3>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "d"; + }; + port at 4 { + reg = <4>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "e"; + }; + port at 5 { + reg = <5>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "f"; + }; + port at 6 { + reg = <6>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "g"; + }; + port at 7 { + reg = <7>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "h"; + }; + port at 8 { + reg = <8>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "i"; + }; + port at 9 { + reg = <9>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "j"; + }; + port at 10 { + reg = <10>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "k"; + }; + port at 11 { + reg = <11>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "l"; + }; + port at 12 { + reg = <12>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "m"; + }; + port at 13 { + reg = <13>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "n"; + }; + port at 14 { + reg = <14>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "o"; + }; + port at 15 { + reg = <15>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "p"; + }; + port at 16 { + reg = <16>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "q"; + }; + port at 17 { + reg = <17>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "r"; + }; + port at 18 { + reg = <18>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "s"; + }; + port at 19 { + reg = <19>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "t"; + }; + port at 20 { + reg = <20>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "u"; + }; + port at 21 { + reg = <21>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "v"; + }; + port at 22 { + reg = <22>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "w"; + }; + port at 23 { + reg = <23>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "x"; + }; + port at 24 { + reg = <24>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "y"; + }; + port at 25 { + reg = <25>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "z"; + }; + port at 26 { + reg = <26>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "aa"; + }; + port at 27 { + reg = <27>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "bb"; + }; }; pinmux: pinmux at 70000000 {
arch/arm/include/asm/arch-tegra/gpio.h +9 -5
@@ -6,6 +6,8 @@#ifndef _TEGRA_GPIO_H_#define _TEGRA_GPIO_H_ +#define TEGRA_GPIOS_PER_PORT 8 +#define TEGRA_PORTS_PER_BANK 4#define MAX_NUM_GPIOS (TEGRA_GPIO_PORTS * TEGRA_GPIO_BANKS * 8)#define GPIO_NAME_SIZE 20/* gpio_request max label len */
@@ -14,11 +16,13 @@#define GPIO_FULLPORT (x) ((x) >> 3)#define GPIO_BIT (x) ((x) & 0x7) -/* -* Tegra-specific GPIO API +/** +* tegra_spl_gpio_direction_output() - set the output value of a GPIO +* +* This function is only used from SPL on seaboard, which needs to enable a +* GPIO to get the UART running. It could be done in U-Boot rather than SPL, +* but for now, this gets it working */ +int tegra_spl_gpio_direction_output (int gpio,int value); -void gpio_info (void ); - -#define gpio_status () gpio_info()#endif /* TEGRA_GPIO_H_ */
board/nvidia/seaboard/seaboard.c +1 -1
@@ -22,7 +22,7 @@ void gpio_early_init_uart(void)#ifndef CONFIG_SPL_BUILD gpio_request (GPIO_PI3 ,NULL );#endif - gpio_direction_output(GPIO_PI3, 0); + tegra_spl_gpio_direction_output(GPIO_PI3, 0); }#endif
drivers/core/lists.c +1 -0
@@ -14,6 +14,7 @@#include <dm/platdata.h> #include <dm/uclass.h> #include <dm/util.h> +#include <fdtdec.h> #include <linux/compiler.h> struct driver *lists_driver_lookup_name (const char *name)
drivers/core/root.c +4 -3
@@ -10,6 +10,7 @@#include <common.h> #include <errno.h> #include <malloc.h> +#include <libfdt.h> #include <dm/device.h> #include <dm/device-internal.h> #include <dm/lists.h>
@@ -42,9 +43,9 @@ int dm_init(void)dm_warn ("Virtual root driver already exists!\n" );return -EINVAL ; } -INIT_LIST_HEAD (&gd->uclass_root); +INIT_LIST_HEAD (&DM_UCLASS_ROOT ()); - ret =device_bind_by_name (NULL , &root_info, &gd->dm_root); + ret =device_bind_by_name (NULL , &root_info, &DM_ROOT ());if (ret)return ret;
@@ -55,7 +56,7 @@ int dm_scan_platdata(void) {int ret; - ret =lists_bind_drivers (gd->dm_root); + ret =lists_bind_drivers (DM_ROOT ());if (ret == -ENOENT ) {dm_warn ("Some drivers were not found\n" ); ret =0 ;
drivers/core/uclass.c +1 -1
@@ -75,7 +75,7 @@ static int uclass_add(enum uclass_id id, struct uclass **ucp) uc->uc_drv = uc_drv;INIT_LIST_HEAD (&uc->sibling_node);INIT_LIST_HEAD (&uc->dev_head); -list_add (&uc->sibling_node, &gd->uclass_root); +list_add (&uc->sibling_node, &DM_UCLASS_ROOT ());if (uc_drv->init) { ret = uc_drv->init (uc);
drivers/gpio/tegra_gpio.c +286 -104
515 lines not on this page · raw message
include/configs/tegra-common.h +3 -0
@@ -19,6 +19,9 @@#include <asm/arch/tegra.h> /* get chip and board defs */ +#define CONFIG_DM +#define CONFIG_DM_GPIO +#define CONFIG_SYS_TIMER_RATE 1000000#define CONFIG_SYS_TIMER_COUNTER NV_PA_TMRUS_BASE
include/dm/device-internal.h +4 -0
@@ -84,4 +84,8 @@ int device_remove(struct device *dev);*/ int device_unbind (struct device *dev); +/* Cast away any volatile pointer */ +#define DM_ROOT () (((gd_t *)gd)->dm_root) +#define DM_UCLASS_ROOT () (((gd_t *)gd)->uclass_root) +#endif
test/dm/Makefile +2 -0
@@ -15,4 +15,6 @@ obj-$(CONFIG_DM_TEST) += ut.o # subsystem you must add sandbox tests here. obj-$(CONFIG_DM_TEST) += core.o obj-$(CONFIG_DM_TEST) += ut.o +ifneq ($(CONFIG_DM_TEST),) obj-$(CONFIG_DM_GPIO) += gpio.o +endif
Signature · 1 line
1.9.1.423.g4596e3a
PATCH 1/1 WIP: tegra: Use driver model for GPIO driver
Quoted 8 lines · Simon Glass
This is an implementation of GPIOs for Tegra that uses driver model. It is written for comment and need work and testing before it is ready to use.
Specific points for discussion:
1. I can't find much in the way of GPIO device tree bindings, so ended up just creating the GPIO devices
The binding is already defined in the Linux kernel at:
Documentation/devicetree/bindings/gpio/gpio.txt Documentation/devicetree/bindings/gpio/nvidia,tegra20-gpio.txt
An example is in:
arch/arm/boot/dts/tegra20.dtsi
Quoted 3 lines
3. Driver model understand the concept of a bank of GPIOs, but this is equivalent to 'port' in Tegra. So it is somewhat confusing. Need to think about this.
There's no need at all to expose the banks separately. This is purely an implementation detail of the internal register layout of the HW, and not something that anyone outside the GPIO driver need concern itself with. Tegra simply has N GPIOs numbered 0..n-1. Admittedly the GPIOs also have textual names derived from the banked register layout, but this has no practical consequence, and need not be represented anywhere.
I would imagine this is true of any GPIO controller. Why does the driver model know/care about GPIO banks?
Quoted 5 lines
arch/arm/dts/tegra20.dtsi +0
-0
arch/arm/dts/tegra20.dtsi line 152
Quoted 10 lines
arch/arm/dts/tegra20.dtsi +7
-0@@ -143,6 +143,148 @@ interrupts = < 64 65 66 67 87 119 121 >; #gpio-cells = <2>; gpio-controller; + #address-cells = <1>; + #size-cells = <0>; + port at 0 { + reg = <0>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "a"; + };
We definitely shouldn't add these port child nodes.
PATCH 1/1 WIP: tegra: Use driver model for GPIO driver
Hi Stephen,
Quoted 10 lines · Stephen Warren
Quoted 8 lines · Simon Glass
This is an implementation of GPIOs for Tegra that uses driver model. It is written for comment and need work and testing before it is ready to use.
Specific points for discussion:
1. I can't find much in the way of GPIO device tree bindings, so ended up just creating the GPIO devices
Quoted 8 lines
The binding is already defined in the Linux kernel at:
Documentation/devicetree/bindings/gpio/gpio.txtDocumentation/devicetree/bindings/gpio/nvidia,tegra20-gpio.txtAn example is in:
arch/arm/boot/dts/tegra20.dtsi
Yes but all the parameters are hard-coded in the driver, not in the device tree. I ended up doing the same thing, as you probably noticed.
Quoted 13 lines
Quoted 5 lines
3. Driver model understand the concept of a bank of GPIOs, but this is equivalent to 'port' in Tegra. So it is somewhat confusing. Need to think about this.
There's no need at all to expose the banks separately. This is purely an implementation detail of the internal register layout of the HW, and not something that anyone outside the GPIO driver need concern itself with. Tegra simply has N GPIOs numbered 0..n-1. Admittedly the GPIOs also have textual names derived from the banked register layout, but this has no practical consequence, and need not be represented anywhere.
Quoted 2 lines
I would imagine this is true of any GPIO controller. Why does the driver model know/care about GPIO banks?
For naming - A, B, C, etc. Each of these is considered a 'bank'. At present each is a separate GPIO device, also. This will allow us to support I2C extenders and other ways of adding GPIOs.
Quoted 10 lines
Quoted 10 lines
arch/arm/dts/tegra20.dtsi +1
-0@@ -143,6 +143,148 @@ interrupts = < 64 65 66 67 87 119 121 >; #gpio-cells = <2>; gpio-controller; + #address-cells = <1>;
Quoted 8 lines
arch/arm/dts/tegra20.dtsi line 152
Quoted 6 lines
arch/arm/dts/tegra20.dtsi +6
-0+ #size-cells = <0>; + port at 0 { + reg = <0>; + compatible = "nvidia,tegra20-gpio-bank"; + gpio-bank-name = "a"; + };We definitely shouldn't add these port child nodes.
Yes, that was just something I was playing with - it's not used in the driver as you probably saw.
I see that U-Boot's GPIO stuff is out of date for tegra, so I'll update that. I'll see if I can remove the need for the second-level bank devices also. Not quite sure what I'll end up with. Ideas welcome.
Regards,
Simon
PATCH 1/1 WIP: tegra: Use driver model for GPIO driver
Quoted 18 lines · Simon Glass
Hi Stephen,
Quoted 15 lines · Stephen Warren
Quoted 8 lines · Simon Glass
This is an implementation of GPIOs for Tegra that uses driver model. It is written for comment and need work and testing before it is ready to use.
Specific points for discussion:
1. I can't find much in the way of GPIO device tree bindings, so ended up just creating the GPIO devices
The binding is already defined in the Linux kernel at:
Documentation/devicetree/bindings/gpio/gpio.txtDocumentation/devicetree/bindings/gpio/nvidia,tegra20-gpio.txt
Quoted 6 lines
Quoted 3 lines
An example is in:
arch/arm/boot/dts/tegra20.dtsiYes but all the parameters are hard-coded in the driver, not in the device tree. I ended up doing the same thing, as you probably noticed.
To be honest, since I saw all the DT binding changes and mentions of "banks", I didn't look at the driver, since I assumed it'd have to be re-written to solve those issues first. If the driver doesn't actually use any of that stuff, then perhaps I should take another look at the patch.
Quoted 11 lines
Quoted 11 lines
Quoted 3 lines
3. Driver model understand the concept of a bank of GPIOs, but this is equivalent to 'port' in Tegra. So it is somewhat confusing. Need to think about this.
There's no need at all to expose the banks separately. This is purely an implementation detail of the internal register layout of the HW, and not something that anyone outside the GPIO driver need concern itself with. Tegra simply has N GPIOs numbered 0..n-1. Admittedly the GPIOs also have textual names derived from the banked register layout, but this has no practical consequence, and need not be represented anywhere.
Quoted 6 lines
Quoted 2 lines
I would imagine this is true of any GPIO controller. Why does the driver model know/care about GPIO banks?
For naming - A, B, C, etc. Each of these is considered a 'bank'. At present each is a separate GPIO device, also. This will allow us to support I2C extenders and other ways of adding GPIOs.
I don't think individual GPIO controllers have to be broken down into banks in order for U-Boot to support multiple GPIO controllers. U-Boot should know that multiple controller instances exist. U-Boot shouldn't dictate the internal structure of each controller; if it wants to expose 1000 GPIOs in a flat naming space, that should be OK.
Or, when you mentioned "bank", did you mean "HW module" or "GPIO expander chip" i.e. what the Linux kernel calls a struct gpio_chip, rather than a "bank" being a sub-division of a whole HW module or GPIO expander chip?
PATCH 1/1 WIP: tegra: Use driver model for GPIO driver
Hi Stephen,
Quoted 24 lines · Stephen Warren
Quoted 22 lines · Simon Glass
Hi Stephen,
Quoted 18 lines · Stephen Warren
Quoted 8 lines · Simon Glass
This is an implementation of GPIOs for Tegra that uses driver model. It is written for comment and need work and testing before it is ready to use.
Specific points for discussion:
1. I can't find much in the way of GPIO device tree bindings, so ended up just creating the GPIO devices
The binding is already defined in the Linux kernel at:
Documentation/devicetree/bindings/gpio/gpio.txtDocumentation/devicetree/bindings/gpio/nvidia,tegra20-gpio.txtAn example is in:
arch/arm/boot/dts/tegra20.dtsi
Quoted 7 lines
Quoted 2 lines
Yes but all the parameters are hard-coded in the driver, not in the device tree. I ended up doing the same thing, as you probably noticed.
To be honest, since I saw all the DT binding changes and mentions of "banks", I didn't look at the driver, since I assumed it'd have to be re-written to solve those issues first. If the driver doesn't actually use any of that stuff, then perhaps I should take another look at the patch.
Well...you could :-) I sent an updated and cleaned-up version that has actually been tested.
Quoted 26 lines
Quoted 13 lines
Quoted 13 lines
Quoted 5 lines
3. Driver model understand the concept of a bank of GPIOs, but this is equivalent to 'port' in Tegra. So it is somewhat confusing. Need to think about this.
There's no need at all to expose the banks separately. This is purely an implementation detail of the internal register layout of the HW, and not something that anyone outside the GPIO driver need concern itself with. Tegra simply has N GPIOs numbered 0..n-1. Admittedly the GPIOs also have textual names derived from the banked register layout, but this has no practical consequence, and need not be represented anywhere.
Quoted 6 lines
Quoted 2 lines
I would imagine this is true of any GPIO controller. Why does the driver model know/care about GPIO banks?
For naming - A, B, C, etc. Each of these is considered a 'bank'. At present each is a separate GPIO device, also. This will allow us to support I2C extenders and other ways of adding GPIOs.
I don't think individual GPIO controllers have to be broken down into banks in order for U-Boot to support multiple GPIO controllers. U-Boot should know that multiple controller instances exist. U-Boot shouldn't dictate the internal structure of each controller; if it wants to expose 1000 GPIOs in a flat naming space, that should be OK.
Quoted 4 lines
Or, when you mentioned "bank", did you mean "HW module" or "GPIO expander chip" i.e. what the Linux kernel calls a struct
gpio_chip, rather than a "bank" being a sub-division of a whole HW module or GPIO expander chip?
I suppose I mean a conceptual 'bank', such as is used to name GPIOs in the datasheet. The current GPIO uclass implementation uses one device per bank, which has the virtue of simplicity, and it's easy to use 'dm tree' to see the available GPIO banks. But it remains to be seen whether this fits best with actual SOCs.
Regards,
Simon