* [PATCH 1/3] gpio: gpio-ich: add support for Intel Baytrail
2016-01-23 16:32 [PATCH 0/3] gpio: Intel Baytrail support Antoine Tenart
@ 2016-01-23 16:32 ` Antoine Tenart
2016-01-25 12:43 ` Lee Jones
2016-01-23 16:32 ` [PATCH 2/3] mfd: lpc_ich: use a correct mask for the GPIO base address Antoine Tenart
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Antoine Tenart @ 2016-01-23 16:32 UTC (permalink / raw)
To: ptyser
Cc: Antoine Tenart, lee.jones, linus.walleij, gnurou, linux-gpio,
linux-kernel
This patch adds support for the Braytrail series, with the hardware
blink capability.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
drivers/gpio/gpio-ich.c | 12 ++++++++++++
include/linux/mfd/lpc_ich.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/drivers/gpio/gpio-ich.c b/drivers/gpio/gpio-ich.c
index 4ba7ed502131..6f5fcae32855 100644
--- a/drivers/gpio/gpio-ich.c
+++ b/drivers/gpio/gpio-ich.c
@@ -384,6 +384,15 @@ static struct ichx_desc avoton_desc = {
.use_outlvl_cache = true,
};
+/* Baytrail */
+static struct ichx_desc baytrail_desc = {
+ .ngpio = 96,
+ .regs = ichx_regs,
+ .reglen = ichx_reglen,
+ .have_blink = true,
+ .use_outlvl_cache = true,
+};
+
static int ichx_gpio_request_regions(struct resource *res_base,
const char *name, u8 use_gpio)
{
@@ -461,6 +470,9 @@ static int ichx_gpio_probe(struct platform_device *pdev)
case AVOTON_GPIO:
ichx_priv.desc = &avoton_desc;
break;
+ case ICH_BAYTRAIL_GPIO:
+ ichx_priv.desc = &baytrail_desc;
+ break;
default:
return -ENODEV;
}
diff --git a/include/linux/mfd/lpc_ich.h b/include/linux/mfd/lpc_ich.h
index 2b300b44f994..659a1cd875d8 100644
--- a/include/linux/mfd/lpc_ich.h
+++ b/include/linux/mfd/lpc_ich.h
@@ -34,6 +34,7 @@ enum {
ICH_V10CORP_GPIO,
ICH_V10CONS_GPIO,
AVOTON_GPIO,
+ ICH_BAYTRAIL_GPIO,
};
struct lpc_ich_info {
--
2.7.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/3] gpio: gpio-ich: add support for Intel Baytrail
2016-01-23 16:32 ` [PATCH 1/3] gpio: gpio-ich: add support for Intel Baytrail Antoine Tenart
@ 2016-01-25 12:43 ` Lee Jones
0 siblings, 0 replies; 12+ messages in thread
From: Lee Jones @ 2016-01-25 12:43 UTC (permalink / raw)
To: Antoine Tenart; +Cc: ptyser, linus.walleij, gnurou, linux-gpio, linux-kernel
On Sat, 23 Jan 2016, Antoine Tenart wrote:
> This patch adds support for the Braytrail series, with the hardware
> blink capability.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
> drivers/gpio/gpio-ich.c | 12 ++++++++++++
> include/linux/mfd/lpc_ich.h | 1 +
Acked-by: Lee Jones <lee.jones@linaro.org>
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/gpio/gpio-ich.c b/drivers/gpio/gpio-ich.c
> index 4ba7ed502131..6f5fcae32855 100644
> --- a/drivers/gpio/gpio-ich.c
> +++ b/drivers/gpio/gpio-ich.c
> @@ -384,6 +384,15 @@ static struct ichx_desc avoton_desc = {
> .use_outlvl_cache = true,
> };
>
> +/* Baytrail */
> +static struct ichx_desc baytrail_desc = {
> + .ngpio = 96,
> + .regs = ichx_regs,
> + .reglen = ichx_reglen,
> + .have_blink = true,
> + .use_outlvl_cache = true,
> +};
> +
> static int ichx_gpio_request_regions(struct resource *res_base,
> const char *name, u8 use_gpio)
> {
> @@ -461,6 +470,9 @@ static int ichx_gpio_probe(struct platform_device *pdev)
> case AVOTON_GPIO:
> ichx_priv.desc = &avoton_desc;
> break;
> + case ICH_BAYTRAIL_GPIO:
> + ichx_priv.desc = &baytrail_desc;
> + break;
> default:
> return -ENODEV;
> }
> diff --git a/include/linux/mfd/lpc_ich.h b/include/linux/mfd/lpc_ich.h
> index 2b300b44f994..659a1cd875d8 100644
> --- a/include/linux/mfd/lpc_ich.h
> +++ b/include/linux/mfd/lpc_ich.h
> @@ -34,6 +34,7 @@ enum {
> ICH_V10CORP_GPIO,
> ICH_V10CONS_GPIO,
> AVOTON_GPIO,
> + ICH_BAYTRAIL_GPIO,
> };
>
> struct lpc_ich_info {
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] mfd: lpc_ich: use a correct mask for the GPIO base address
2016-01-23 16:32 [PATCH 0/3] gpio: Intel Baytrail support Antoine Tenart
2016-01-23 16:32 ` [PATCH 1/3] gpio: gpio-ich: add support for Intel Baytrail Antoine Tenart
@ 2016-01-23 16:32 ` Antoine Tenart
2016-01-25 12:44 ` Lee Jones
2016-01-23 16:32 ` [PATCH 3/3] mfd: lpc_ich: add GPIO support for Baytrail Antoine Tenart
2016-01-25 11:49 ` [PATCH 0/3] gpio: Intel Baytrail support Mika Westerberg
3 siblings, 1 reply; 12+ messages in thread
From: Antoine Tenart @ 2016-01-23 16:32 UTC (permalink / raw)
To: ptyser
Cc: Antoine Tenart, lee.jones, linus.walleij, gnurou, linux-gpio,
linux-kernel
The GPIO base address is read from the GPIOBASE register. The first
bit must be cleared as it can be hardwired to 1 to represent the i/o
space. Other bits are either containing the base address of are
reserved. They should not be cleared as all the chipsets do not have
the same reserved bits.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
drivers/mfd/lpc_ich.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
index b514f3cf140d..f13a5ded3958 100644
--- a/drivers/mfd/lpc_ich.c
+++ b/drivers/mfd/lpc_ich.c
@@ -921,7 +921,10 @@ static int lpc_ich_init_gpio(struct pci_dev *dev)
gpe0_done:
/* Setup GPIO base register */
pci_read_config_dword(dev, priv->gbase, &base_addr_cfg);
- base_addr = base_addr_cfg & 0x0000ff80;
+
+ /* Clear the i/o flag */
+ base_addr = base_addr_cfg & ~BIT(0);
+
if (!base_addr) {
dev_notice(&dev->dev, "I/O space for GPIO uninitialized\n");
ret = -ENODEV;
--
2.7.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/3] mfd: lpc_ich: use a correct mask for the GPIO base address
2016-01-23 16:32 ` [PATCH 2/3] mfd: lpc_ich: use a correct mask for the GPIO base address Antoine Tenart
@ 2016-01-25 12:44 ` Lee Jones
2016-01-25 16:07 ` Peter Tyser
0 siblings, 1 reply; 12+ messages in thread
From: Lee Jones @ 2016-01-25 12:44 UTC (permalink / raw)
To: Antoine Tenart; +Cc: ptyser, linus.walleij, gnurou, linux-gpio, linux-kernel
On Sat, 23 Jan 2016, Antoine Tenart wrote:
> The GPIO base address is read from the GPIOBASE register. The first
> bit must be cleared as it can be hardwired to 1 to represent the i/o
> space. Other bits are either containing the base address of are
> reserved. They should not be cleared as all the chipsets do not have
> the same reserved bits.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
> drivers/mfd/lpc_ich.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
Applied, thanks.
> diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
> index b514f3cf140d..f13a5ded3958 100644
> --- a/drivers/mfd/lpc_ich.c
> +++ b/drivers/mfd/lpc_ich.c
> @@ -921,7 +921,10 @@ static int lpc_ich_init_gpio(struct pci_dev *dev)
> gpe0_done:
> /* Setup GPIO base register */
> pci_read_config_dword(dev, priv->gbase, &base_addr_cfg);
> - base_addr = base_addr_cfg & 0x0000ff80;
> +
> + /* Clear the i/o flag */
> + base_addr = base_addr_cfg & ~BIT(0);
> +
> if (!base_addr) {
> dev_notice(&dev->dev, "I/O space for GPIO uninitialized\n");
> ret = -ENODEV;
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/3] mfd: lpc_ich: use a correct mask for the GPIO base address
2016-01-25 12:44 ` Lee Jones
@ 2016-01-25 16:07 ` Peter Tyser
2016-02-11 17:12 ` Lee Jones
0 siblings, 1 reply; 12+ messages in thread
From: Peter Tyser @ 2016-01-25 16:07 UTC (permalink / raw)
To: Lee Jones
Cc: Antoine Tenart, linus.walleij, gnurou, linux-gpio, linux-kernel,
Mika Westerberg
On Mon, 2016-01-25 at 12:44 +0000, Lee Jones wrote:
> On Sat, 23 Jan 2016, Antoine Tenart wrote:
>
> > The GPIO base address is read from the GPIOBASE register. The first
> > bit must be cleared as it can be hardwired to 1 to represent the i/o
> > space. Other bits are either containing the base address of are
> > reserved. They should not be cleared as all the chipsets do not have
> > the same reserved bits.
> >
> > Signed-off-by: Antoine Tenart tenart@free-electrons.com>
> > ---
> > drivers/mfd/lpc_ich.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
>
> Applied, thanks.
Is it possible to hold off on the application of the change Lee?
> > diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
> > index b514f3cf140d..f13a5ded3958 100644
> > --- a/drivers/mfd/lpc_ich.c
> > +++ b/drivers/mfd/lpc_ich.c
> > @@ -921,7 +921,10 @@ static int lpc_ich_init_gpio(struct pci_dev *dev)
> > gpe0_done:
> > /* Setup GPIO base register */
> > pci_read_config_dword(dev, priv->gbase, &base_addr_cfg);
> > - base_addr = base_addr_cfg & 0x0000ff80;
> > +
> > + /* Clear the i/o flag */
> > + base_addr = base_addr_cfg & ~BIT(0);
> > +
Does this patch work around an issue you are seeing? Looking at the Bay
Trail EDS, the GPIO base address register looks like it should work fine
with the original code (it uses 0xff00 as a mask for the address, and
reserves 0x80 which reads as a 0). Also, Bay Trail bit 1 is an enable
flag, which this patch wouldn't mask off. Eg if the BIOS enables the GPIO
controller and sets the enable bit, I think things would break with this
patch.
It's also scary to not mask off the reserved bits on other Intel chipsets -
you're assuming they all read as 0 and I'm not sure if this is true or
not. The patch also doesn't make the same change to the other base
register reads either, and ideally they'd be kept in sync.
Seems like things should be left as-is, or use an accurate chip-specific
mask.
I'd leave as-is personally. Like Mika mentioned, Baytrail GPIO should
already be supported elsewhere, which should make this change unnecessary.
Regards,
Peter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] mfd: lpc_ich: use a correct mask for the GPIO base address
2016-01-25 16:07 ` Peter Tyser
@ 2016-02-11 17:12 ` Lee Jones
0 siblings, 0 replies; 12+ messages in thread
From: Lee Jones @ 2016-02-11 17:12 UTC (permalink / raw)
To: Peter Tyser
Cc: Antoine Tenart, linus.walleij, gnurou, linux-gpio, linux-kernel,
Mika Westerberg
On Mon, 25 Jan 2016, Peter Tyser wrote:
>
> On Mon, 2016-01-25 at 12:44 +0000, Lee Jones wrote:
> > On Sat, 23 Jan 2016, Antoine Tenart wrote:
> >
> > > The GPIO base address is read from the GPIOBASE register. The first
> > > bit must be cleared as it can be hardwired to 1 to represent the i/o
> > > space. Other bits are either containing the base address of are
> > > reserved. They should not be cleared as all the chipsets do not have
> > > the same reserved bits.
> > >
> > > Signed-off-by: Antoine Tenart tenart@free-electrons.com>
> > > ---
> > > drivers/mfd/lpc_ich.c | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > Applied, thanks.
>
> Is it possible to hold off on the application of the change Lee?
Patch unapplied.
> > > diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
> > > index b514f3cf140d..f13a5ded3958 100644
> > > --- a/drivers/mfd/lpc_ich.c
> > > +++ b/drivers/mfd/lpc_ich.c
> > > @@ -921,7 +921,10 @@ static int lpc_ich_init_gpio(struct pci_dev *dev)
> > > gpe0_done:
> > > /* Setup GPIO base register */
> > > pci_read_config_dword(dev, priv->gbase, &base_addr_cfg);
> > > - base_addr = base_addr_cfg & 0x0000ff80;
> > > +
> > > + /* Clear the i/o flag */
> > > + base_addr = base_addr_cfg & ~BIT(0);
> > > +
>
>
> Does this patch work around an issue you are seeing? Looking at the Bay
> Trail EDS, the GPIO base address register looks like it should work fine
> with the original code (it uses 0xff00 as a mask for the address, and
> reserves 0x80 which reads as a 0). Also, Bay Trail bit 1 is an enable
> flag, which this patch wouldn't mask off. Eg if the BIOS enables the GPIO
> controller and sets the enable bit, I think things would break with this
> patch.
>
> It's also scary to not mask off the reserved bits on other Intel chipsets -
> you're assuming they all read as 0 and I'm not sure if this is true or
> not. The patch also doesn't make the same change to the other base
> register reads either, and ideally they'd be kept in sync.
>
> Seems like things should be left as-is, or use an accurate chip-specific
> mask.
>
> I'd leave as-is personally. Like Mika mentioned, Baytrail GPIO should
> already be supported elsewhere, which should make this change unnecessary.
>
> Regards,
> Peter
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] mfd: lpc_ich: add GPIO support for Baytrail
2016-01-23 16:32 [PATCH 0/3] gpio: Intel Baytrail support Antoine Tenart
2016-01-23 16:32 ` [PATCH 1/3] gpio: gpio-ich: add support for Intel Baytrail Antoine Tenart
2016-01-23 16:32 ` [PATCH 2/3] mfd: lpc_ich: use a correct mask for the GPIO base address Antoine Tenart
@ 2016-01-23 16:32 ` Antoine Tenart
2016-01-25 12:45 ` Lee Jones
2016-01-28 10:34 ` Linus Walleij
2016-01-25 11:49 ` [PATCH 0/3] gpio: Intel Baytrail support Mika Westerberg
3 siblings, 2 replies; 12+ messages in thread
From: Antoine Tenart @ 2016-01-23 16:32 UTC (permalink / raw)
To: ptyser
Cc: Antoine Tenart, lee.jones, linus.walleij, gnurou, linux-gpio,
linux-kernel
This patch adds the GPIO support for the Baytrail family.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
drivers/mfd/lpc_ich.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
index f13a5ded3958..d86b496874df 100644
--- a/drivers/mfd/lpc_ich.c
+++ b/drivers/mfd/lpc_ich.c
@@ -508,6 +508,7 @@ static struct lpc_ich_info lpc_chipset_info[] = {
[LPC_BAYTRAIL] = {
.name = "Bay Trail SoC",
.iTCO_version = 3,
+ .gpio_version = ICH_BAYTRAIL_GPIO,
},
[LPC_COLETO] = {
.name = "Coleto Creek",
--
2.7.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/3] mfd: lpc_ich: add GPIO support for Baytrail
2016-01-23 16:32 ` [PATCH 3/3] mfd: lpc_ich: add GPIO support for Baytrail Antoine Tenart
@ 2016-01-25 12:45 ` Lee Jones
2016-01-28 10:34 ` Linus Walleij
1 sibling, 0 replies; 12+ messages in thread
From: Lee Jones @ 2016-01-25 12:45 UTC (permalink / raw)
To: Antoine Tenart; +Cc: ptyser, linus.walleij, gnurou, linux-gpio, linux-kernel
On Sat, 23 Jan 2016, Antoine Tenart wrote:
> This patch adds the GPIO support for the Baytrail family.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
> drivers/mfd/lpc_ich.c | 1 +
> 1 file changed, 1 insertion(+)
I can't take this yet, as it has a dependency on the GPIO patch.
For my own reference:
Acked-by: Lee Jones <lee.jones@linaro.org>
> diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
> index f13a5ded3958..d86b496874df 100644
> --- a/drivers/mfd/lpc_ich.c
> +++ b/drivers/mfd/lpc_ich.c
> @@ -508,6 +508,7 @@ static struct lpc_ich_info lpc_chipset_info[] = {
> [LPC_BAYTRAIL] = {
> .name = "Bay Trail SoC",
> .iTCO_version = 3,
> + .gpio_version = ICH_BAYTRAIL_GPIO,
> },
> [LPC_COLETO] = {
> .name = "Coleto Creek",
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/3] mfd: lpc_ich: add GPIO support for Baytrail
2016-01-23 16:32 ` [PATCH 3/3] mfd: lpc_ich: add GPIO support for Baytrail Antoine Tenart
2016-01-25 12:45 ` Lee Jones
@ 2016-01-28 10:34 ` Linus Walleij
2016-01-28 15:48 ` Mika Westerberg
1 sibling, 1 reply; 12+ messages in thread
From: Linus Walleij @ 2016-01-28 10:34 UTC (permalink / raw)
To: Antoine Tenart, Mika Westerberg
Cc: ptyser@xes-inc.com, Lee Jones, Alexandre Courbot,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org
On Sat, Jan 23, 2016 at 5:32 PM, Antoine Tenart
<antoine.tenart@free-electrons.com> wrote:
> This patch adds the GPIO support for the Baytrail family.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
I'm confused by this and Mika's comment on patch 0.
Is the BayTrail an ICH variant or not? Or does it have
two GPIO controllers? Or um ....
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] mfd: lpc_ich: add GPIO support for Baytrail
2016-01-28 10:34 ` Linus Walleij
@ 2016-01-28 15:48 ` Mika Westerberg
0 siblings, 0 replies; 12+ messages in thread
From: Mika Westerberg @ 2016-01-28 15:48 UTC (permalink / raw)
To: Linus Walleij
Cc: Antoine Tenart, ptyser@xes-inc.com, Lee Jones, Alexandre Courbot,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org
On Thu, Jan 28, 2016 at 11:34:40AM +0100, Linus Walleij wrote:
> On Sat, Jan 23, 2016 at 5:32 PM, Antoine Tenart
> <antoine.tenart@free-electrons.com> wrote:
>
> > This patch adds the GPIO support for the Baytrail family.
> >
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
>
> I'm confused by this and Mika's comment on patch 0.
>
> Is the BayTrail an ICH variant or not? Or does it have
> two GPIO controllers? Or um ....
It is not compatible with ICH and there is only one GPIO controller in
the SoC.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] gpio: Intel Baytrail support
2016-01-23 16:32 [PATCH 0/3] gpio: Intel Baytrail support Antoine Tenart
` (2 preceding siblings ...)
2016-01-23 16:32 ` [PATCH 3/3] mfd: lpc_ich: add GPIO support for Baytrail Antoine Tenart
@ 2016-01-25 11:49 ` Mika Westerberg
3 siblings, 0 replies; 12+ messages in thread
From: Mika Westerberg @ 2016-01-25 11:49 UTC (permalink / raw)
To: Antoine Tenart
Cc: ptyser, lee.jones, linus.walleij, gnurou, linux-gpio,
linux-kernel, Mathias Nyman
On Sat, Jan 23, 2016 at 05:32:06PM +0100, Antoine Tenart wrote:
> This series introduces the GPIO Intel Baytrail support, using both the
> lpc ich and the gpio ich drivers.
>
> Antoine Tenart (3):
> gpio: gpio-ich: add support for Intel Baytrail
Intel Baytrail support has been in mainline kernel for quite some time
already. The driver is drivers/pintctrl/intel/pinctrl-baytrail.c.
Also the ICH GPIO block is not compatible to Baytrail which is why there
is a separate driver in the first place. Have you tested this series on
Baytrail?
> mfd: lpc_ich: use a correct mask for the GPIO base address
> mfd: lpc_ich: add GPIO support for Baytrail
All Baytrails I've seen expose the GPIO device via ACPI namespace so
there should be no need for the above two patches.
^ permalink raw reply [flat|nested] 12+ messages in thread