[U-Boot] [PATCH 1/2] x86: gpio: Make x86-pinctrl subnode of ich6-gpio
Instead of having x86-pinctrl work separately from ich6-gpio have it work underneath ich6-gpio. This removes redundant configuration and will allow the addition of shared bank settings in future commits.
Signed-off-by: George McCollister george.mccollister@gmail.com --- arch/x86/dts/minnowmax.dts | 96 +++++++++--------- arch/x86/include/asm/gpio.h | 1 - board/intel/minnowmax/minnowmax.c | 9 +- doc/device-tree-bindings/gpio/intel,ich6-gpio.txt | 14 +++ .../gpio/intel,x86-pinctrl.txt | 17 ++-- drivers/gpio/intel_ich6_gpio.c | 107 ++++++++------------- 6 files changed, 118 insertions(+), 126 deletions(-) create mode 100644 doc/device-tree-bindings/gpio/intel,ich6-gpio.txt
diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts index e917f0f..79ac26d 100644 --- a/arch/x86/dts/minnowmax.dts +++ b/arch/x86/dts/minnowmax.dts @@ -26,54 +26,6 @@ silent_console = <0>; };
- pch_pinctrl { - compatible = "intel,x86-pinctrl"; - io-base = <0x4c>; - - /* GPIO E0 */ - soc_gpio_s5_0@0 { - gpio-offset = <0x80 0>; - pad-offset = <0x1d0>; - mode-gpio; - output-value = <0>; - direction = <PIN_OUTPUT>; - }; - - /* GPIO E1 */ - soc_gpio_s5_1@0 { - gpio-offset = <0x80 1>; - pad-offset = <0x210>; - mode-gpio; - output-value = <0>; - direction = <PIN_OUTPUT>; - }; - - /* GPIO E2 */ - soc_gpio_s5_2@0 { - gpio-offset = <0x80 2>; - pad-offset = <0x1e0>; - mode-gpio; - output-value = <0>; - direction = <PIN_OUTPUT>; - }; - - pin_usb_host_en0@0 { - gpio-offset = <0x80 8>; - pad-offset = <0x260>; - mode-gpio; - output-value = <1>; - direction = <PIN_OUTPUT>; - }; - - pin_usb_host_en1@0 { - gpio-offset = <0x80 9>; - pad-offset = <0x250>; - mode-gpio; - output-value = <1>; - direction = <PIN_OUTPUT>; - }; - }; - gpioa { compatible = "intel,ich6-gpio"; u-boot,dm-pre-reloc; @@ -107,6 +59,54 @@ u-boot,dm-pre-reloc; reg = <0x80 0x20>; bank-name = "E"; + + pch_pinctrl@0 { + compatible = "intel,x86-pinctrl"; + io-base = <0x4c>; + + /* GPIO E0 */ + soc_gpio_s5_0@0 { + gpio-offset = <0>; + pad-offset = <0x1d0>; + mode-gpio; + output-value = <0>; + direction = <PIN_OUTPUT>; + }; + + /* GPIO E1 */ + soc_gpio_s5_1@0 { + gpio-offset = <1>; + pad-offset = <0x210>; + mode-gpio; + output-value = <0>; + direction = <PIN_OUTPUT>; + }; + + /* GPIO E2 */ + soc_gpio_s5_2@0 { + gpio-offset = <2>; + pad-offset = <0x1e0>; + mode-gpio; + output-value = <0>; + direction = <PIN_OUTPUT>; + }; + + pin_usb_host_en0@0 { + gpio-offset = <8>; + pad-offset = <0x260>; + mode-gpio; + output-value = <1>; + direction = <PIN_OUTPUT>; + }; + + pin_usb_host_en1@0 { + gpio-offset = <9>; + pad-offset = <0x250>; + mode-gpio; + output-value = <1>; + direction = <PIN_OUTPUT>; + }; + }; };
gpiof { diff --git a/arch/x86/include/asm/gpio.h b/arch/x86/include/asm/gpio.h index ed85b08..1099427 100644 --- a/arch/x86/include/asm/gpio.h +++ b/arch/x86/include/asm/gpio.h @@ -147,7 +147,6 @@ struct pch_gpio_map { } set3; };
-int gpio_ich6_pinctrl_init(void); void setup_pch_gpios(u16 gpiobase, const struct pch_gpio_map *gpio); void ich_gpio_set_gpio_map(const struct pch_gpio_map *map);
diff --git a/board/intel/minnowmax/minnowmax.c b/board/intel/minnowmax/minnowmax.c index 44e5bf4..ae06cc4 100644 --- a/board/intel/minnowmax/minnowmax.c +++ b/board/intel/minnowmax/minnowmax.c @@ -5,12 +5,17 @@ */
#include <common.h> +#include <dm.h> #include <asm/gpio.h>
int arch_early_init_r(void) { - /* do the pin-muxing */ - gpio_ich6_pinctrl_init(); + struct udevice *dev; + + /* Force initialization of gpio so initial states are set correctly. */ + for (uclass_first_device(UCLASS_GPIO, &dev); + dev; + uclass_next_device(&dev));
return 0; } diff --git a/doc/device-tree-bindings/gpio/intel,ich6-gpio.txt b/doc/device-tree-bindings/gpio/intel,ich6-gpio.txt new file mode 100644 index 0000000..23345b2 --- /dev/null +++ b/doc/device-tree-bindings/gpio/intel,ich6-gpio.txt @@ -0,0 +1,14 @@ +Intel x86 GPIO controller + +Each GPIO bank node can have the following properties: +- compatible - (required) must be set to "intel,x86-pinctrl" +- reg - (required) GPIO offset and length. +- bank-name - (required) Name of the bank. + +Example: +gpioa { + compatible = "intel,ich6-gpio"; + u-boot,dm-pre-reloc; + reg = <0 0x20>; + bank-name = "A"; +}; diff --git a/doc/device-tree-bindings/gpio/intel,x86-pinctrl.txt b/doc/device-tree-bindings/gpio/intel,x86-pinctrl.txt index 45ab1af..eb9c8da 100644 --- a/doc/device-tree-bindings/gpio/intel,x86-pinctrl.txt +++ b/doc/device-tree-bindings/gpio/intel,x86-pinctrl.txt @@ -1,7 +1,7 @@ -Intel x86 PINCTRL/GPIO controller +Intel x86 PINCTRL
-Pin-muxing on x86 can be described with a node for the PINCTRL master -node and a set of child nodes for each pin on the SoC. +Pin-muxing on x86 can be described under each GPIO bank with a node for the +PINCTRL master node and a set of child nodes for each pin on the SoC.
The PINCTRL master node requires the following properties: - compatible : "intel,x86-pinctrl" @@ -9,13 +9,12 @@ The PINCTRL master node requires the following properties: Pin nodes must be children of the pinctrl master node and can contain the following properties: - pad-offset - (required) offset in the IOBASE for the pin to configured. -- gpio-offset - (required) offset in the GPIOBASE for the pin to configured and - also the bit shift in this register. -- mode-gpio - (optional) standalone property to force the pin into GPIO mode. -- mode-func - (optional) function number to assign to the pin. if 'mode-gpio' +- gpio-offset - (required) bit shift in the GPIO register. +- mode-gpio - (optional) standalone property to force the pin into GPIO mode. +- mode-func - (optional) function number to assign to the pin. if 'mode-gpio' is set, this property will be ignored. in case of 'mode-gpio' property set: -- output-value - (optional) this set the default output value of the GPIO. +- output-value - (optional) this set the default output value of the GPIO. - direction - (optional) this set the direction of the gpio. - pull-str - (optional) this set the pull strength of the pin. - pull-assign - (optional) this set the pull assignement (up/down) of the pin. @@ -23,7 +22,7 @@ in case of 'mode-gpio' property set: Example:
pin_usb_host_en0@0 { - gpio-offset = <0x80 8>; + gpio-offset = <8>; pad-offset = <0x260>; mode-gpio; output-value = <1>; diff --git a/drivers/gpio/intel_ich6_gpio.c b/drivers/gpio/intel_ich6_gpio.c index 67bf0a2..18420f3 100644 --- a/drivers/gpio/intel_ich6_gpio.c +++ b/drivers/gpio/intel_ich6_gpio.c @@ -41,15 +41,11 @@ DECLARE_GLOBAL_DATA_PTR;
struct ich6_bank_priv { /* These are I/O addresses */ - uint16_t use_sel; - uint16_t io_sel; - uint16_t lvl; + u16 use_sel; + u16 io_sel; + u16 lvl; };
-#define GPIO_USESEL_OFFSET(x) (x) -#define GPIO_IOSEL_OFFSET(x) (x + 4) -#define GPIO_LVL_OFFSET(x) (x + 8) - #define IOPAD_MODE_MASK 0x7 #define IOPAD_PULL_ASSIGN_SHIFT 7 #define IOPAD_PULL_ASSIGN_MASK (0x3 << IOPAD_PULL_ASSIGN_SHIFT) @@ -147,92 +143,90 @@ static int gpio_ich6_get_base(unsigned long base) return tmplong & 1 ? tmplong & ~3 : tmplong & ~15; }
-static int _ich6_gpio_set_value(uint16_t base, unsigned offset, int value) + +static int ich6_gpio_set_value(struct udevice *dev, unsigned offset, + int value) { + struct ich6_bank_priv *bank = dev_get_priv(dev); u32 val;
- val = inl(base); + val = inl(bank->lvl); if (value) val |= (1UL << offset); else val &= ~(1UL << offset); - outl(val, base); + outl(val, bank->lvl);
return 0; }
-static int _ich6_gpio_set_function(uint16_t base, unsigned offset, int func) +static int ich6_gpio_set_function(struct udevice *dev, unsigned offset, + int func) { + struct ich6_bank_priv *bank = dev_get_priv(dev); u32 val;
if (func) { - val = inl(base); + val = inl(bank->use_sel); val |= (1UL << offset); - outl(val, base); + outl(val, bank->use_sel); } else { - val = inl(base); + val = inl(bank->use_sel); val &= ~(1UL << offset); - outl(val, base); + outl(val, bank->use_sel); }
return 0; }
-static int _ich6_gpio_set_direction(uint16_t base, unsigned offset, int dir) +static int ich6_gpio_set_direction(struct udevice *dev, unsigned offset, + int dir) { + struct ich6_bank_priv *bank = dev_get_priv(dev); u32 val;
if (!dir) { - val = inl(base); + val = inl(bank->io_sel); val |= (1UL << offset); - outl(val, base); + outl(val, bank->io_sel); } else { - val = inl(base); + val = inl(bank->io_sel); val &= ~(1UL << offset); - outl(val, base); + outl(val, bank->io_sel); }
return 0; }
-static int _gpio_ich6_pinctrl_cfg_pin(s32 gpiobase, s32 iobase, int pin_node) +static int _gpio_ich6_pinctrl_cfg_pin(struct udevice *dev, s32 iobase, + int pin_node) { - u32 gpio_offset[2]; + int gpio_offset; int pad_offset; int val; - int ret; const void *prop;
/* * GPIO node is not mandatory, so we only do the * pinmuxing if the node exist. */ - ret = fdtdec_get_int_array(gd->fdt_blob, pin_node, "gpio-offset", - gpio_offset, 2); - if (!ret) { + gpio_offset = fdtdec_get_int(gd->fdt_blob, pin_node, "gpio-offset", -1); + if (gpio_offset != -1) { /* Do we want to force the GPIO mode? */ prop = fdt_getprop(gd->fdt_blob, pin_node, "mode-gpio", NULL); if (prop) - _ich6_gpio_set_function(GPIO_USESEL_OFFSET - (gpiobase) + - gpio_offset[0], - gpio_offset[1], 1); + ich6_gpio_set_function(dev, gpio_offset, 1);
val = fdtdec_get_int(gd->fdt_blob, pin_node, "direction", -1); if (val != -1) - _ich6_gpio_set_direction(GPIO_IOSEL_OFFSET - (gpiobase) + - gpio_offset[0], - gpio_offset[1], val); + ich6_gpio_set_direction(dev, gpio_offset, val);
val = fdtdec_get_int(gd->fdt_blob, pin_node, "output-value", -1); if (val != -1) - _ich6_gpio_set_value(GPIO_LVL_OFFSET(gpiobase) - + gpio_offset[0], - gpio_offset[1], val); + ich6_gpio_set_value(dev, gpio_offset, val); }
/* if iobase is present, let's configure the pad */ @@ -286,30 +280,19 @@ static int _gpio_ich6_pinctrl_cfg_pin(s32 gpiobase, s32 iobase, int pin_node) return 0; }
-int gpio_ich6_pinctrl_init(void) +static int gpio_ich6_pinctrl_init(struct udevice *dev) { int pin_node; int node; int ret; - int gpiobase; int iobase_offset; int iobase = -1; - - /* - * Get the memory/io base address to configure every pins. - * IOBASE is used to configure the mode/pads - * GPIOBASE is used to configure the direction and default value - */ - gpiobase = gpio_ich6_get_base(PCI_CFG_GPIOBASE); - if (gpiobase < 0) { - debug("%s: invalid GPIOBASE address (%08x)\n", __func__, - gpiobase); - return -EINVAL; - } + int depth = 0;
/* This is not an error to not have a pinctrl node */ node = - fdtdec_next_compatible(gd->fdt_blob, 0, COMPAT_INTEL_X86_PINCTRL); + fdtdec_next_compatible_subnode(gd->fdt_blob, dev->of_offset, + COMPAT_INTEL_X86_PINCTRL, &depth); if (node <= 0) { debug("%s: no pinctrl node\n", __func__); return 0; @@ -335,7 +318,7 @@ int gpio_ich6_pinctrl_init(void) pin_node > 0; pin_node = fdt_next_subnode(gd->fdt_blob, pin_node)) { /* Configure the pin */ - ret = _gpio_ich6_pinctrl_cfg_pin(gpiobase, iobase, pin_node); + ret = _gpio_ich6_pinctrl_cfg_pin(dev, iobase, pin_node); if (ret != 0) { debug("%s: invalid configuration for the pin %d\n", __func__, pin_node); @@ -382,6 +365,8 @@ static int ich6_gpio_probe(struct udevice *dev) bank->io_sel = plat->base_addr + 4; bank->lvl = plat->base_addr + 8;
+ gpio_ich6_pinctrl_init(dev); + return 0; }
@@ -408,22 +393,19 @@ static int ich6_gpio_request(struct udevice *dev, unsigned offset,
static int ich6_gpio_direction_input(struct udevice *dev, unsigned offset) { - struct ich6_bank_priv *bank = dev_get_priv(dev); - - return _ich6_gpio_set_direction(bank->io_sel, offset, 0); + return ich6_gpio_set_direction(dev, offset, 0); }
static int ich6_gpio_direction_output(struct udevice *dev, unsigned offset, int value) { int ret; - struct ich6_bank_priv *bank = dev_get_priv(dev);
- ret = _ich6_gpio_set_direction(bank->io_sel, offset, 1); + ret = ich6_gpio_set_direction(dev, offset, 1); if (ret) return ret;
- return _ich6_gpio_set_value(bank->lvl, offset, value); + return ich6_gpio_set_value(dev, offset, value); }
static int ich6_gpio_get_value(struct udevice *dev, unsigned offset) @@ -437,13 +419,6 @@ static int ich6_gpio_get_value(struct udevice *dev, unsigned offset) return r; }
-static int ich6_gpio_set_value(struct udevice *dev, unsigned offset, - int value) -{ - struct ich6_bank_priv *bank = dev_get_priv(dev); - return _ich6_gpio_set_value(bank->lvl, offset, value); -} - static int ich6_gpio_get_function(struct udevice *dev, unsigned offset) { struct ich6_bank_priv *bank = dev_get_priv(dev);
Add a device-tree property use-lvl-write-cache that will cause writes to lvl to be cached instead of read from lvl before each write. This is required on some platforms that have the register implemented as dual read/write (such as Baytrail).
Prior to this fix the blue USB port on the Minnowboard Max was unusable since USB_HOST_EN0 was set high then immediately set low when USB_HOST_EN1 was written.
Signed-off-by: George McCollister george.mccollister@gmail.com --- arch/x86/dts/minnowmax.dts | 6 ++++++ doc/device-tree-bindings/gpio/intel,ich6-gpio.txt | 5 +++++ drivers/gpio/intel_ich6_gpio.c | 20 +++++++++++++++++++- 3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts index 79ac26d..5a76c8e 100644 --- a/arch/x86/dts/minnowmax.dts +++ b/arch/x86/dts/minnowmax.dts @@ -31,6 +31,7 @@ u-boot,dm-pre-reloc; reg = <0 0x20>; bank-name = "A"; + use-lvl-write-cache; };
gpiob { @@ -38,6 +39,7 @@ u-boot,dm-pre-reloc; reg = <0x20 0x20>; bank-name = "B"; + use-lvl-write-cache; };
gpioc { @@ -45,6 +47,7 @@ u-boot,dm-pre-reloc; reg = <0x40 0x20>; bank-name = "C"; + use-lvl-write-cache; };
gpiod { @@ -52,6 +55,7 @@ u-boot,dm-pre-reloc; reg = <0x60 0x20>; bank-name = "D"; + use-lvl-write-cache; };
gpioe { @@ -59,6 +63,7 @@ u-boot,dm-pre-reloc; reg = <0x80 0x20>; bank-name = "E"; + use-lvl-write-cache;
pch_pinctrl@0 { compatible = "intel,x86-pinctrl"; @@ -114,6 +119,7 @@ u-boot,dm-pre-reloc; reg = <0xA0 0x20>; bank-name = "F"; + use-lvl-write-cache; };
chosen { diff --git a/doc/device-tree-bindings/gpio/intel,ich6-gpio.txt b/doc/device-tree-bindings/gpio/intel,ich6-gpio.txt index 23345b2..dbdcff4 100644 --- a/doc/device-tree-bindings/gpio/intel,ich6-gpio.txt +++ b/doc/device-tree-bindings/gpio/intel,ich6-gpio.txt @@ -4,6 +4,10 @@ Each GPIO bank node can have the following properties: - compatible - (required) must be set to "intel,x86-pinctrl" - reg - (required) GPIO offset and length. - bank-name - (required) Name of the bank. +- use-lvl-write-cache - (optional) Cache last value written to lvl and use it + the next time lvl is written instead of + reading lvl prior to writing. Required + for some platforms (Baytrail).
Example: gpioa { @@ -11,4 +15,5 @@ gpioa { u-boot,dm-pre-reloc; reg = <0 0x20>; bank-name = "A"; + use-lvl-write-cache; }; diff --git a/drivers/gpio/intel_ich6_gpio.c b/drivers/gpio/intel_ich6_gpio.c index 18420f3..b239ab5 100644 --- a/drivers/gpio/intel_ich6_gpio.c +++ b/drivers/gpio/intel_ich6_gpio.c @@ -44,6 +44,8 @@ struct ich6_bank_priv { u16 use_sel; u16 io_sel; u16 lvl; + u32 lvl_write_cache; + bool use_lvl_write_cache; };
#define IOPAD_MODE_MASK 0x7 @@ -150,12 +152,17 @@ static int ich6_gpio_set_value(struct udevice *dev, unsigned offset, struct ich6_bank_priv *bank = dev_get_priv(dev); u32 val;
- val = inl(bank->lvl); + if (bank->use_lvl_write_cache) + val = bank->lvl_write_cache; + else + val = inl(bank->lvl); if (value) val |= (1UL << offset); else val &= ~(1UL << offset); outl(val, bank->lvl); + if (bank->use_lvl_write_cache) + bank->lvl_write_cache = val;
return 0; } @@ -353,6 +360,7 @@ static int ich6_gpio_probe(struct udevice *dev) struct ich6_bank_platdata *plat = dev_get_platdata(dev); struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); struct ich6_bank_priv *bank = dev_get_priv(dev); + const void *prop;
if (gd->arch.gpio_map) { setup_pch_gpios(plat->base_addr, gd->arch.gpio_map); @@ -365,6 +373,14 @@ static int ich6_gpio_probe(struct udevice *dev) bank->io_sel = plat->base_addr + 4; bank->lvl = plat->base_addr + 8;
+ prop = fdt_getprop(gd->fdt_blob, dev->of_offset, + "use-lvl-write-cache", NULL); + if (prop) + bank->use_lvl_write_cache = true; + else + bank->use_lvl_write_cache = false; + bank->lvl_write_cache = 0; + gpio_ich6_pinctrl_init(dev);
return 0; @@ -415,6 +431,8 @@ static int ich6_gpio_get_value(struct udevice *dev, unsigned offset) int r;
tmplong = inl(bank->lvl); + if (bank->use_lvl_write_cache) + tmplong |= bank->lvl_write_cache; r = (tmplong & (1UL << offset)) ? 1 : 0; return r; }
Hi George,
On 7 October 2015 at 16:29, George McCollister george.mccollister@gmail.com wrote:
Add a device-tree property use-lvl-write-cache that will cause writes to lvl to be cached instead of read from lvl before each write. This is required on some platforms that have the register implemented as dual read/write (such as Baytrail).
Prior to this fix the blue USB port on the Minnowboard Max was unusable since USB_HOST_EN0 was set high then immediately set low when USB_HOST_EN1 was written.
Thanks for figuring this out. I saw this support in the datasheet but it did not work. I think I misunderstood how the cache was implemented.
One nit below.
Signed-off-by: George McCollister george.mccollister@gmail.com
arch/x86/dts/minnowmax.dts | 6 ++++++ doc/device-tree-bindings/gpio/intel,ich6-gpio.txt | 5 +++++ drivers/gpio/intel_ich6_gpio.c | 20 +++++++++++++++++++- 3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts index 79ac26d..5a76c8e 100644 --- a/arch/x86/dts/minnowmax.dts +++ b/arch/x86/dts/minnowmax.dts @@ -31,6 +31,7 @@ u-boot,dm-pre-reloc; reg = <0 0x20>; bank-name = "A";
use-lvl-write-cache; }; gpiob {@@ -38,6 +39,7 @@ u-boot,dm-pre-reloc; reg = <0x20 0x20>; bank-name = "B";
use-lvl-write-cache; }; gpioc {@@ -45,6 +47,7 @@ u-boot,dm-pre-reloc; reg = <0x40 0x20>; bank-name = "C";
use-lvl-write-cache; }; gpiod {@@ -52,6 +55,7 @@ u-boot,dm-pre-reloc; reg = <0x60 0x20>; bank-name = "D";
use-lvl-write-cache; }; gpioe {@@ -59,6 +63,7 @@ u-boot,dm-pre-reloc; reg = <0x80 0x20>; bank-name = "E";
use-lvl-write-cache; pch_pinctrl@0 { compatible = "intel,x86-pinctrl";@@ -114,6 +119,7 @@ u-boot,dm-pre-reloc; reg = <0xA0 0x20>; bank-name = "F";
use-lvl-write-cache; }; chosen {diff --git a/doc/device-tree-bindings/gpio/intel,ich6-gpio.txt b/doc/device-tree-bindings/gpio/intel,ich6-gpio.txt index 23345b2..dbdcff4 100644 --- a/doc/device-tree-bindings/gpio/intel,ich6-gpio.txt +++ b/doc/device-tree-bindings/gpio/intel,ich6-gpio.txt @@ -4,6 +4,10 @@ Each GPIO bank node can have the following properties:
- compatible - (required) must be set to "intel,x86-pinctrl"
- reg - (required) GPIO offset and length.
- bank-name - (required) Name of the bank.
+- use-lvl-write-cache - (optional) Cache last value written to lvl and use it
the next time lvl is written instead ofreading lvl prior to writing. Requiredfor some platforms (Baytrail).Example: gpioa { @@ -11,4 +15,5 @@ gpioa { u-boot,dm-pre-reloc; reg = <0 0x20>; bank-name = "A";
use-lvl-write-cache;}; diff --git a/drivers/gpio/intel_ich6_gpio.c b/drivers/gpio/intel_ich6_gpio.c index 18420f3..b239ab5 100644 --- a/drivers/gpio/intel_ich6_gpio.c +++ b/drivers/gpio/intel_ich6_gpio.c @@ -44,6 +44,8 @@ struct ich6_bank_priv { u16 use_sel; u16 io_sel; u16 lvl;
u32 lvl_write_cache;bool use_lvl_write_cache;};
#define IOPAD_MODE_MASK 0x7 @@ -150,12 +152,17 @@ static int ich6_gpio_set_value(struct udevice *dev, unsigned offset, struct ich6_bank_priv *bank = dev_get_priv(dev); u32 val;
val = inl(bank->lvl);
if (bank->use_lvl_write_cache)val = bank->lvl_write_cache;elseval = inl(bank->lvl); if (value) val |= (1UL << offset); else val &= ~(1UL << offset); outl(val, bank->lvl);if (bank->use_lvl_write_cache)bank->lvl_write_cache = val; return 0;} @@ -353,6 +360,7 @@ static int ich6_gpio_probe(struct udevice *dev) struct ich6_bank_platdata *plat = dev_get_platdata(dev); struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); struct ich6_bank_priv *bank = dev_get_priv(dev);
const void *prop; if (gd->arch.gpio_map) { setup_pch_gpios(plat->base_addr, gd->arch.gpio_map);@@ -365,6 +373,14 @@ static int ich6_gpio_probe(struct udevice *dev) bank->io_sel = plat->base_addr + 4; bank->lvl = plat->base_addr + 8;
prop = fdt_getprop(gd->fdt_blob, dev->of_offset,"use-lvl-write-cache", NULL);
fdtdec_get_bool()
if (prop)bank->use_lvl_write_cache = true;elsebank->use_lvl_write_cache = false;bank->lvl_write_cache = 0;gpio_ich6_pinctrl_init(dev); return 0;@@ -415,6 +431,8 @@ static int ich6_gpio_get_value(struct udevice *dev, unsigned offset) int r;
tmplong = inl(bank->lvl);
if (bank->use_lvl_write_cache)tmplong |= bank->lvl_write_cache; r = (tmplong & (1UL << offset)) ? 1 : 0; return r;}
2.5.0
Regards, Simon
Hi George,
On 7 October 2015 at 16:29, George McCollister george.mccollister@gmail.com wrote:
Instead of having x86-pinctrl work separately from ich6-gpio have it work underneath ich6-gpio. This removes redundant configuration and will allow the addition of shared bank settings in future commits.
Signed-off-by: George McCollister george.mccollister@gmail.com
arch/x86/dts/minnowmax.dts | 96 +++++++++--------- arch/x86/include/asm/gpio.h | 1 - board/intel/minnowmax/minnowmax.c | 9 +- doc/device-tree-bindings/gpio/intel,ich6-gpio.txt | 14 +++ .../gpio/intel,x86-pinctrl.txt | 17 ++-- drivers/gpio/intel_ich6_gpio.c | 107 ++++++++------------- 6 files changed, 118 insertions(+), 126 deletions(-) create mode 100644 doc/device-tree-bindings/gpio/intel,ich6-gpio.txt
Conceptually pinmux and GPIO are different concepts. What is the need to make pinctrl a subset of GPIO? In some ways the opposite would make more sense.
Perhaps instead we should have a pinctrl driver?
Regards, Simon
On Thu, Oct 8, 2015 at 12:43 PM, Simon Glass sjg@chromium.org wrote:
Hi George,
On 7 October 2015 at 16:29, George McCollister george.mccollister@gmail.com wrote:
Instead of having x86-pinctrl work separately from ich6-gpio have it work underneath ich6-gpio. This removes redundant configuration and will allow the addition of shared bank settings in future commits.
Signed-off-by: George McCollister george.mccollister@gmail.com
arch/x86/dts/minnowmax.dts | 96 +++++++++--------- arch/x86/include/asm/gpio.h | 1 - board/intel/minnowmax/minnowmax.c | 9 +- doc/device-tree-bindings/gpio/intel,ich6-gpio.txt | 14 +++ .../gpio/intel,x86-pinctrl.txt | 17 ++-- drivers/gpio/intel_ich6_gpio.c | 107 ++++++++------------- 6 files changed, 118 insertions(+), 126 deletions(-) create mode 100644 doc/device-tree-bindings/gpio/intel,ich6-gpio.txt
Conceptually pinmux and GPIO are different concepts. What is the need to make pinctrl a subset of GPIO? In some ways the opposite would make more sense.
Perhaps instead we should have a pinctrl driver?
Yeah, I think you're right.
I think the only things you'd want the pinctrl driver to touch would be mode-func, pull-assign and pull-strength.
The gpio driver would unconditionally set do "ich6_gpio_set_function(dev, gpio_offset, 1);" if the gpio was setup and you'd just need a way of setting direction and output value in device tree.
Is there already sane structure defined we could copy or should I invent something? This is pretty low on my priority list but I might be able to get around to it in my spare time if you think it's the way to go.
Regards, Simon
+Masahiro
Hi George,
On 8 October 2015 at 20:44, George McCollister george.mccollister@gmail.com wrote:
On Thu, Oct 8, 2015 at 12:43 PM, Simon Glass sjg@chromium.org wrote:
Hi George,
On 7 October 2015 at 16:29, George McCollister george.mccollister@gmail.com wrote:
Instead of having x86-pinctrl work separately from ich6-gpio have it work underneath ich6-gpio. This removes redundant configuration and will allow the addition of shared bank settings in future commits.
Signed-off-by: George McCollister george.mccollister@gmail.com
arch/x86/dts/minnowmax.dts | 96 +++++++++--------- arch/x86/include/asm/gpio.h | 1 - board/intel/minnowmax/minnowmax.c | 9 +- doc/device-tree-bindings/gpio/intel,ich6-gpio.txt | 14 +++ .../gpio/intel,x86-pinctrl.txt | 17 ++-- drivers/gpio/intel_ich6_gpio.c | 107 ++++++++------------- 6 files changed, 118 insertions(+), 126 deletions(-) create mode 100644 doc/device-tree-bindings/gpio/intel,ich6-gpio.txt
Conceptually pinmux and GPIO are different concepts. What is the need to make pinctrl a subset of GPIO? In some ways the opposite would make more sense.
Perhaps instead we should have a pinctrl driver?
Yeah, I think you're right.
I think the only things you'd want the pinctrl driver to touch would be mode-func, pull-assign and pull-strength.
The gpio driver would unconditionally set do "ich6_gpio_set_function(dev, gpio_offset, 1);" if the gpio was setup and you'd just need a way of setting direction and output value in device tree.
Is there already sane structure defined we could copy or should I invent something? This is pretty low on my priority list but I might be able to get around to it in my spare time if you think it's the way to go.
Well there is a driver model uclass for pinctrl - Masahiro has a full implementation for Uniphier (patches on the list).
Regards, Simon
participants (2)
-
George McCollister -
Simon Glass