[U-Boot] [PATCH 2/2] x86: gpio: Add use-lvl-write-cache to fix baytrail

[Simon Glass]

Hi George,

On 7 October 2015 at 16:29, George McCollister
<george.mccollister at 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 at 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 at 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);

fdtdec_get_bool()

> +       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;
>  }
> --
> 2.5.0
>

Regards,
Simon