[PATCH] gpio-ich: Share ownership of GPIO groups

[PATCH] gpio-ich: Share ownership of GPIO groups

[Jean Delvare]

The ICH chips have their GPIO pins organized in 2 or 3 independent
groups of 32 GPIO pins. It can happen that the ACPI BIOS wants to make
use of pins in one group, preventing the OS to access these. This does
not prevent the OS from accessing the other group(s).

This is the case for example on my Asus Z8NA-D6 board. The ACPI BIOS
wants to control GPIO 18 (group 1), while I (the OS) need to control
GPIO 52 and 53 (group 2) for SMBus multiplexing.

So instead of checking for ACPI resource conflict on the whole I/O
range, check on a per-group basis, and consider it a success if at
least one of the groups is available for the OS to use.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Peter Tyser <ptyser@xes-inc.com>
Cc: Aaron Sierra <asierra@xes-inc.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Samuel Ortiz <sameo@linux.intel.com>
---
That's probably not the nicest code you've seen, but everything else I
could think of either couldn't work or was looking worse. If anyone can
think of a better approach, I'm all ears.

 drivers/gpio/gpio-ich.c     |   79 +++++++++++++++++++++++++++++++++++++------
 drivers/mfd/lpc_ich.c       |   29 ++++++++++++++-
 include/linux/mfd/lpc_ich.h |    1 
 3 files changed, 97 insertions(+), 12 deletions(-)

--- linux-3.5.orig/drivers/mfd/lpc_ich.c	2012-07-23 14:41:30.794134320 +0200
+++ linux-3.5/drivers/mfd/lpc_ich.c	2012-07-23 14:57:05.006073824 +0200
@@ -683,6 +683,30 @@ static void __devinit lpc_ich_finalize_c
 	cell->pdata_size = sizeof(struct lpc_ich_info);
 }
 
+/*
+ * We don't check for resource conflict globally. There are 2 or 3 independent
+ * GPIO groups and it's enough to have access to one of these to instantiate
+ * the device.
+ */
+static int __devinit lpc_ich_check_conflict_gpio(struct resource *res)
+{
+	int ret;
+	u8 use_gpio = 0;
+
+	if (resource_size(res) >= 0x50 &&
+	    !acpi_check_region(res->start + 0x40, 0x10, "LPC ICH GPIO3"))
+		use_gpio |= 1 << 2;
+
+	if (!acpi_check_region(res->start + 0x30, 0x10, "LPC ICH GPIO2"))
+		use_gpio |= 1 << 1;
+
+	ret = acpi_check_region(res->start + 0x00, 0x30, "LPC ICH GPIO1");
+	if (!ret)
+		use_gpio |= 1 << 0;
+
+	return use_gpio ? use_gpio : ret;
+}
+
 static int __devinit lpc_ich_init_gpio(struct pci_dev *dev,
 				const struct pci_device_id *id)
 {
@@ -740,12 +764,13 @@ gpe0_done:
 		break;
 	}
 
-	ret = acpi_check_resource_conflict(res);
-	if (ret) {
+	ret = lpc_ich_check_conflict_gpio(res);
+	if (ret < 0) {
 		/* this isn't necessarily fatal for the GPIO */
 		acpi_conflict = true;
 		goto gpio_done;
 	}
+	lpc_chipset_info[id->driver_data].use_gpio = ret;
 	lpc_ich_enable_gpio_space(dev);
 
 	lpc_ich_finalize_cell(&lpc_ich_cells[LPC_GPIO], id);
--- linux-3.5.orig/include/linux/mfd/lpc_ich.h	2012-07-23 14:41:30.795134320 +0200
+++ linux-3.5/include/linux/mfd/lpc_ich.h	2012-07-23 14:42:57.034135599 +0200
@@ -43,6 +43,7 @@ struct lpc_ich_info {
 	char name[32];
 	unsigned int iTCO_version;
 	unsigned int gpio_version;
+	u8 use_gpio;
 };
 
 #endif
--- linux-3.5.orig/drivers/gpio/gpio-ich.c	2012-07-23 14:41:30.795134320 +0200
+++ linux-3.5/drivers/gpio/gpio-ich.c	2012-07-23 15:08:57.699546528 +0200
@@ -49,6 +49,10 @@ static const u8 ichx_regs[3][3] = {
 	{0x0c, 0x38, 0x48},	/* LVL[1-3] offsets */
 };
 
+static const u8 ichx_reglen[3] = {
+	0x30, 0x10, 0x10,
+};
+
 #define ICHX_WRITE(val, reg, base_res)	outl(val, (reg) + (base_res)->start)
 #define ICHX_READ(reg, base_res)	inl((reg) + (base_res)->start)
 
@@ -75,6 +79,7 @@ static struct {
 	struct resource *pm_base;	/* Power Mangagment IO base */
 	struct ichx_desc *desc;	/* Pointer to chipset-specific description */
 	u32 orig_gpio_ctrl;	/* Orig CTRL value, used to restore on exit */
+	u8 use_gpio;		/* Which GPIO groups are usable */
 } ichx_priv;
 
 static int modparam_gpiobase = -1;	/* dynamic */
@@ -123,8 +128,16 @@ static int ichx_read_bit(int reg, unsign
 	return data & (1 << bit) ? 1 : 0;
 }
 
+static int ichx_gpio_check_available(struct gpio_chip *gpio, unsigned nr)
+{
+	return (ichx_priv.use_gpio & (1 << (nr / 32))) ? 0 : -ENXIO;
+}
+
 static int ichx_gpio_direction_input(struct gpio_chip *gpio, unsigned nr)
 {
+	if (!ichx_gpio_check_available(gpio, nr))
+		return -ENXIO;
+
 	/*
 	 * Try setting pin as an input and verify it worked since many pins
 	 * are output-only.
@@ -138,6 +151,9 @@ static int ichx_gpio_direction_input(str
 static int ichx_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
 					int val)
 {
+	if (!ichx_gpio_check_available(gpio, nr))
+		return -ENXIO;
+
 	/* Set GPIO output value. */
 	ichx_write_bit(GPIO_LVL, nr, val, 0);
 
@@ -153,6 +169,9 @@ static int ichx_gpio_direction_output(st
 
 static int ichx_gpio_get(struct gpio_chip *chip, unsigned nr)
 {
+	if (!ichx_gpio_check_available(chip, nr))
+		return -ENXIO;
+
 	return ichx_read_bit(GPIO_LVL, nr);
 }
 
@@ -161,6 +180,9 @@ static int ich6_gpio_get(struct gpio_chi
 	unsigned long flags;
 	u32 data;
 
+	if (!ichx_gpio_check_available(chip, nr))
+		return -ENXIO;
+
 	/*
 	 * GPI 0 - 15 need to be read from the power management registers on
 	 * a ICH6/3100 bridge.
@@ -291,6 +313,46 @@ static struct ichx_desc intel5_desc = {
 	.ngpio = 76,
 };
 
+static int __devinit ichx_gpio_request_regions(struct resource *res_base,
+						const char *name, u8 use_gpio)
+{
+	int i;
+
+	if (!res_base || !res_base->start || !res_base->end)
+		return -ENODEV;
+
+	for (i = 0; i < ARRAY_SIZE(ichx_regs[0]); i++) {
+		if (!(use_gpio & (1 << i)))
+			continue;
+		if (!request_region(res_base->start + ichx_regs[0][i],
+				    ichx_reglen[i], name))
+			goto request_err;
+	}
+	return 0;
+
+request_err:
+	/* Clean up: release already requested regions, if any */
+	for (i--; i >= 0; i--) {
+		if (!(use_gpio & (1 << i)))
+			continue;
+		release_region(res_base->start + ichx_regs[0][i],
+			       ichx_reglen[i]);
+	}
+	return -EBUSY;
+}
+
+static void ichx_gpio_release_regions(struct resource *res_base, u8 use_gpio)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ichx_regs[0]); i++) {
+		if (!(use_gpio & (1 << i)))
+			continue;
+		release_region(res_base->start + ichx_regs[0][i],
+			       ichx_reglen[i]);
+	}
+}
+
 static int __devinit ichx_gpio_probe(struct platform_device *pdev)
 {
 	struct resource *res_base, *res_pm;
@@ -329,12 +391,11 @@ static int __devinit ichx_gpio_probe(str
 	}
 
 	res_base = platform_get_resource(pdev, IORESOURCE_IO, ICH_RES_GPIO);
-	if (!res_base || !res_base->start || !res_base->end)
-		return -ENODEV;
-
-	if (!request_region(res_base->start, resource_size(res_base),
-				pdev->name))
-		return -EBUSY;
+	ichx_priv.use_gpio = ich_info->use_gpio;
+	err = ichx_gpio_request_regions(res_base, pdev->name,
+					ichx_priv.use_gpio);
+	if (err)
+		return err;
 
 	ichx_priv.gpio_base = res_base;
 
@@ -374,8 +435,7 @@ init:
 	return 0;
 
 add_err:
-	release_region(ichx_priv.gpio_base->start,
-			resource_size(ichx_priv.gpio_base));
+	ichx_gpio_release_regions(ichx_priv.gpio_base, ichx_priv.use_gpio);
 	if (ichx_priv.pm_base)
 		release_region(ichx_priv.pm_base->start,
 				resource_size(ichx_priv.pm_base));
@@ -393,8 +453,7 @@ static int __devexit ichx_gpio_remove(st
 		return err;
 	}
 
-	release_region(ichx_priv.gpio_base->start,
-				resource_size(ichx_priv.gpio_base));
+	ichx_gpio_release_regions(ichx_priv.gpio_base, ichx_priv.use_gpio);
 	if (ichx_priv.pm_base)
 		release_region(ichx_priv.pm_base->start,
 				resource_size(ichx_priv.pm_base));

-- 
Jean Delvare

Re: [PATCH] gpio-ich: Share ownership of GPIO groups

[Jean Delvare]

On Mon, 23 Jul 2012 17:34:15 +0200, Jean Delvare wrote:
> The ICH chips have their GPIO pins organized in 2 or 3 independent
> groups of 32 GPIO pins. It can happen that the ACPI BIOS wants to make
> use of pins in one group, preventing the OS to access these. This does
> not prevent the OS from accessing the other group(s).
> 
> This is the case for example on my Asus Z8NA-D6 board. The ACPI BIOS
> wants to control GPIO 18 (group 1), while I (the OS) need to control
> GPIO 52 and 53 (group 2) for SMBus multiplexing.
> 
> So instead of checking for ACPI resource conflict on the whole I/O
> range, check on a per-group basis, and consider it a success if at
> least one of the groups is available for the OS to use.
> 
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Peter Tyser <ptyser@xes-inc.com>
> Cc: Aaron Sierra <asierra@xes-inc.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> ---
> That's probably not the nicest code you've seen, but everything else I
> could think of either couldn't work or was looking worse. If anyone can
> think of a better approach, I'm all ears.
> 
>  drivers/gpio/gpio-ich.c     |   79 +++++++++++++++++++++++++++++++++++++------
>  drivers/mfd/lpc_ich.c       |   29 ++++++++++++++-
>  include/linux/mfd/lpc_ich.h |    1 
>  3 files changed, 97 insertions(+), 12 deletions(-)

Grant, Samuel, Linus (sorry for not including you on original
submission), any comment on this? I suppose it's too late for 3.6 but
can this be scheduled to be integrated in 3.7?

Thanks,
Jean

> --- linux-3.5.orig/drivers/mfd/lpc_ich.c	2012-07-23 14:41:30.794134320 +0200
> +++ linux-3.5/drivers/mfd/lpc_ich.c	2012-07-23 14:57:05.006073824 +0200
> @@ -683,6 +683,30 @@ static void __devinit lpc_ich_finalize_c
>  	cell->pdata_size = sizeof(struct lpc_ich_info);
>  }
>  
> +/*
> + * We don't check for resource conflict globally. There are 2 or 3 independent
> + * GPIO groups and it's enough to have access to one of these to instantiate
> + * the device.
> + */
> +static int __devinit lpc_ich_check_conflict_gpio(struct resource *res)
> +{
> +	int ret;
> +	u8 use_gpio = 0;
> +
> +	if (resource_size(res) >= 0x50 &&
> +	    !acpi_check_region(res->start + 0x40, 0x10, "LPC ICH GPIO3"))
> +		use_gpio |= 1 << 2;
> +
> +	if (!acpi_check_region(res->start + 0x30, 0x10, "LPC ICH GPIO2"))
> +		use_gpio |= 1 << 1;
> +
> +	ret = acpi_check_region(res->start + 0x00, 0x30, "LPC ICH GPIO1");
> +	if (!ret)
> +		use_gpio |= 1 << 0;
> +
> +	return use_gpio ? use_gpio : ret;
> +}
> +
>  static int __devinit lpc_ich_init_gpio(struct pci_dev *dev,
>  				const struct pci_device_id *id)
>  {
> @@ -740,12 +764,13 @@ gpe0_done:
>  		break;
>  	}
>  
> -	ret = acpi_check_resource_conflict(res);
> -	if (ret) {
> +	ret = lpc_ich_check_conflict_gpio(res);
> +	if (ret < 0) {
>  		/* this isn't necessarily fatal for the GPIO */
>  		acpi_conflict = true;
>  		goto gpio_done;
>  	}
> +	lpc_chipset_info[id->driver_data].use_gpio = ret;
>  	lpc_ich_enable_gpio_space(dev);
>  
>  	lpc_ich_finalize_cell(&lpc_ich_cells[LPC_GPIO], id);
> --- linux-3.5.orig/include/linux/mfd/lpc_ich.h	2012-07-23 14:41:30.795134320 +0200
> +++ linux-3.5/include/linux/mfd/lpc_ich.h	2012-07-23 14:42:57.034135599 +0200
> @@ -43,6 +43,7 @@ struct lpc_ich_info {
>  	char name[32];
>  	unsigned int iTCO_version;
>  	unsigned int gpio_version;
> +	u8 use_gpio;
>  };
>  
>  #endif
> --- linux-3.5.orig/drivers/gpio/gpio-ich.c	2012-07-23 14:41:30.795134320 +0200
> +++ linux-3.5/drivers/gpio/gpio-ich.c	2012-07-23 15:08:57.699546528 +0200
> @@ -49,6 +49,10 @@ static const u8 ichx_regs[3][3] = {
>  	{0x0c, 0x38, 0x48},	/* LVL[1-3] offsets */
>  };
>  
> +static const u8 ichx_reglen[3] = {
> +	0x30, 0x10, 0x10,
> +};
> +
>  #define ICHX_WRITE(val, reg, base_res)	outl(val, (reg) + (base_res)->start)
>  #define ICHX_READ(reg, base_res)	inl((reg) + (base_res)->start)
>  
> @@ -75,6 +79,7 @@ static struct {
>  	struct resource *pm_base;	/* Power Mangagment IO base */
>  	struct ichx_desc *desc;	/* Pointer to chipset-specific description */
>  	u32 orig_gpio_ctrl;	/* Orig CTRL value, used to restore on exit */
> +	u8 use_gpio;		/* Which GPIO groups are usable */
>  } ichx_priv;
>  
>  static int modparam_gpiobase = -1;	/* dynamic */
> @@ -123,8 +128,16 @@ static int ichx_read_bit(int reg, unsign
>  	return data & (1 << bit) ? 1 : 0;
>  }
>  
> +static int ichx_gpio_check_available(struct gpio_chip *gpio, unsigned nr)
> +{
> +	return (ichx_priv.use_gpio & (1 << (nr / 32))) ? 0 : -ENXIO;
> +}
> +
>  static int ichx_gpio_direction_input(struct gpio_chip *gpio, unsigned nr)
>  {
> +	if (!ichx_gpio_check_available(gpio, nr))
> +		return -ENXIO;
> +
>  	/*
>  	 * Try setting pin as an input and verify it worked since many pins
>  	 * are output-only.
> @@ -138,6 +151,9 @@ static int ichx_gpio_direction_input(str
>  static int ichx_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
>  					int val)
>  {
> +	if (!ichx_gpio_check_available(gpio, nr))
> +		return -ENXIO;
> +
>  	/* Set GPIO output value. */
>  	ichx_write_bit(GPIO_LVL, nr, val, 0);
>  
> @@ -153,6 +169,9 @@ static int ichx_gpio_direction_output(st
>  
>  static int ichx_gpio_get(struct gpio_chip *chip, unsigned nr)
>  {
> +	if (!ichx_gpio_check_available(chip, nr))
> +		return -ENXIO;
> +
>  	return ichx_read_bit(GPIO_LVL, nr);
>  }
>  
> @@ -161,6 +180,9 @@ static int ich6_gpio_get(struct gpio_chi
>  	unsigned long flags;
>  	u32 data;
>  
> +	if (!ichx_gpio_check_available(chip, nr))
> +		return -ENXIO;
> +
>  	/*
>  	 * GPI 0 - 15 need to be read from the power management registers on
>  	 * a ICH6/3100 bridge.
> @@ -291,6 +313,46 @@ static struct ichx_desc intel5_desc = {
>  	.ngpio = 76,
>  };
>  
> +static int __devinit ichx_gpio_request_regions(struct resource *res_base,
> +						const char *name, u8 use_gpio)
> +{
> +	int i;
> +
> +	if (!res_base || !res_base->start || !res_base->end)
> +		return -ENODEV;
> +
> +	for (i = 0; i < ARRAY_SIZE(ichx_regs[0]); i++) {
> +		if (!(use_gpio & (1 << i)))
> +			continue;
> +		if (!request_region(res_base->start + ichx_regs[0][i],
> +				    ichx_reglen[i], name))
> +			goto request_err;
> +	}
> +	return 0;
> +
> +request_err:
> +	/* Clean up: release already requested regions, if any */
> +	for (i--; i >= 0; i--) {
> +		if (!(use_gpio & (1 << i)))
> +			continue;
> +		release_region(res_base->start + ichx_regs[0][i],
> +			       ichx_reglen[i]);
> +	}
> +	return -EBUSY;
> +}
> +
> +static void ichx_gpio_release_regions(struct resource *res_base, u8 use_gpio)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ichx_regs[0]); i++) {
> +		if (!(use_gpio & (1 << i)))
> +			continue;
> +		release_region(res_base->start + ichx_regs[0][i],
> +			       ichx_reglen[i]);
> +	}
> +}
> +
>  static int __devinit ichx_gpio_probe(struct platform_device *pdev)
>  {
>  	struct resource *res_base, *res_pm;
> @@ -329,12 +391,11 @@ static int __devinit ichx_gpio_probe(str
>  	}
>  
>  	res_base = platform_get_resource(pdev, IORESOURCE_IO, ICH_RES_GPIO);
> -	if (!res_base || !res_base->start || !res_base->end)
> -		return -ENODEV;
> -
> -	if (!request_region(res_base->start, resource_size(res_base),
> -				pdev->name))
> -		return -EBUSY;
> +	ichx_priv.use_gpio = ich_info->use_gpio;
> +	err = ichx_gpio_request_regions(res_base, pdev->name,
> +					ichx_priv.use_gpio);
> +	if (err)
> +		return err;
>  
>  	ichx_priv.gpio_base = res_base;
>  
> @@ -374,8 +435,7 @@ init:
>  	return 0;
>  
>  add_err:
> -	release_region(ichx_priv.gpio_base->start,
> -			resource_size(ichx_priv.gpio_base));
> +	ichx_gpio_release_regions(ichx_priv.gpio_base, ichx_priv.use_gpio);
>  	if (ichx_priv.pm_base)
>  		release_region(ichx_priv.pm_base->start,
>  				resource_size(ichx_priv.pm_base));
> @@ -393,8 +453,7 @@ static int __devexit ichx_gpio_remove(st
>  		return err;
>  	}
>  
> -	release_region(ichx_priv.gpio_base->start,
> -				resource_size(ichx_priv.gpio_base));
> +	ichx_gpio_release_regions(ichx_priv.gpio_base, ichx_priv.use_gpio);
>  	if (ichx_priv.pm_base)
>  		release_region(ichx_priv.pm_base->start,
>  				resource_size(ichx_priv.pm_base));
> 

Re: [PATCH] gpio-ich: Share ownership of GPIO groups

[Samuel Ortiz]

Hi Jean,

On Wed, Aug 01, 2012 at 11:13:59AM +0200, Jean Delvare wrote:
> On Mon, 23 Jul 2012 17:34:15 +0200, Jean Delvare wrote:
> > The ICH chips have their GPIO pins organized in 2 or 3 independent
> > groups of 32 GPIO pins. It can happen that the ACPI BIOS wants to make
> > use of pins in one group, preventing the OS to access these. This does
> > not prevent the OS from accessing the other group(s).
> > 
> > This is the case for example on my Asus Z8NA-D6 board. The ACPI BIOS
> > wants to control GPIO 18 (group 1), while I (the OS) need to control
> > GPIO 52 and 53 (group 2) for SMBus multiplexing.
> > 
> > So instead of checking for ACPI resource conflict on the whole I/O
> > range, check on a per-group basis, and consider it a success if at
> > least one of the groups is available for the OS to use.
> > 
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > Cc: Peter Tyser <ptyser@xes-inc.com>
> > Cc: Aaron Sierra <asierra@xes-inc.com>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Samuel Ortiz <sameo@linux.intel.com>
> > ---
> > That's probably not the nicest code you've seen, but everything else I
> > could think of either couldn't work or was looking worse. If anyone can
> > think of a better approach, I'm all ears.
> > 
> >  drivers/gpio/gpio-ich.c     |   79 +++++++++++++++++++++++++++++++++++++------
> >  drivers/mfd/lpc_ich.c       |   29 ++++++++++++++-
> >  include/linux/mfd/lpc_ich.h |    1 
> >  3 files changed, 97 insertions(+), 12 deletions(-)
> 
> Grant, Samuel, Linus (sorry for not including you on original
> submission), any comment on this? I suppose it's too late for 3.6 but
> can this be scheduled to be integrated in 3.7?
I was planning to look at this one and queue it for 3.7 as I'll be working on
my for-next branch once rc1 is tagged.
As you said, this is not the nicest code ever, so I may have a few comments.
No time for that right now though.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH] gpio-ich: Share ownership of GPIO groups

[Jean Delvare]

On Wed, 1 Aug 2012 12:16:46 +0200, Samuel Ortiz wrote:
> On Wed, Aug 01, 2012 at 11:13:59AM +0200, Jean Delvare wrote:
> > Grant, Samuel, Linus (sorry for not including you on original
> > submission), any comment on this? I suppose it's too late for 3.6 but
> > can this be scheduled to be integrated in 3.7?
>
> I was planning to look at this one and queue it for 3.7 as I'll be working on
> my for-next branch once rc1 is tagged.
> As you said, this is not the nicest code ever, so I may have a few comments.
> No time for that right now though.

Great, thank you.

-- 
Jean Delvare

Re: [PATCH] gpio-ich: Share ownership of GPIO groups

[Linus Walleij]

On Mon, Jul 23, 2012 at 5:34 PM, Jean Delvare <khali@linux-fr.org> wrote:

> The ICH chips have their GPIO pins organized in 2 or 3 independent
> groups of 32 GPIO pins. It can happen that the ACPI BIOS wants to make
> use of pins in one group, preventing the OS to access these. This does
> not prevent the OS from accessing the other group(s).
>
> This is the case for example on my Asus Z8NA-D6 board. The ACPI BIOS
> wants to control GPIO 18 (group 1), while I (the OS) need to control
> GPIO 52 and 53 (group 2) for SMBus multiplexing.
>
> So instead of checking for ACPI resource conflict on the whole I/O
> range, check on a per-group basis, and consider it a success if at
> least one of the groups is available for the OS to use.
>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Peter Tyser <ptyser@xes-inc.com>
> Cc: Aaron Sierra <asierra@xes-inc.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> ---
> That's probably not the nicest code you've seen, but everything else I
> could think of either couldn't work or was looking worse. If anyone can
> think of a better approach, I'm all ears.

I'm not very good at Intel and SMBus so cannot really help either.

Acked-by: Linus Walleij <linus.walleij@linaro.org>

I guess you're bringing this through Sam's tree.

Yours,
Linus Walleij

Re: [PATCH] gpio-ich: Share ownership of GPIO groups

[Jean Delvare]

Hi Sam,

On Wed, 1 Aug 2012 12:16:46 +0200, Samuel Ortiz wrote:
> Hi Jean,
> 
> On Wed, Aug 01, 2012 at 11:13:59AM +0200, Jean Delvare wrote:
> > On Mon, 23 Jul 2012 17:34:15 +0200, Jean Delvare wrote:
> > > The ICH chips have their GPIO pins organized in 2 or 3 independent
> > > groups of 32 GPIO pins. It can happen that the ACPI BIOS wants to make
> > > use of pins in one group, preventing the OS to access these. This does
> > > not prevent the OS from accessing the other group(s).
> > > 
> > > This is the case for example on my Asus Z8NA-D6 board. The ACPI BIOS
> > > wants to control GPIO 18 (group 1), while I (the OS) need to control
> > > GPIO 52 and 53 (group 2) for SMBus multiplexing.
> > > 
> > > So instead of checking for ACPI resource conflict on the whole I/O
> > > range, check on a per-group basis, and consider it a success if at
> > > least one of the groups is available for the OS to use.
> > > 
> > > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > > Cc: Peter Tyser <ptyser@xes-inc.com>
> > > Cc: Aaron Sierra <asierra@xes-inc.com>
> > > Cc: Grant Likely <grant.likely@secretlab.ca>
> > > Cc: Samuel Ortiz <sameo@linux.intel.com>
> > > ---
> > > That's probably not the nicest code you've seen, but everything else I
> > > could think of either couldn't work or was looking worse. If anyone can
> > > think of a better approach, I'm all ears.
> > > 
> > >  drivers/gpio/gpio-ich.c     |   79 +++++++++++++++++++++++++++++++++++++------
> > >  drivers/mfd/lpc_ich.c       |   29 ++++++++++++++-
> > >  include/linux/mfd/lpc_ich.h |    1 
> > >  3 files changed, 97 insertions(+), 12 deletions(-)
> > 
> > Grant, Samuel, Linus (sorry for not including you on original
> > submission), any comment on this? I suppose it's too late for 3.6 but
> > can this be scheduled to be integrated in 3.7?
>
> I was planning to look at this one and queue it for 3.7 as I'll be working on
> my for-next branch once rc1 is tagged.
> As you said, this is not the nicest code ever, so I may have a few comments.
> No time for that right now though.

Any news on this? I'd like to get this patch (or an alternative
implementation of the same) into kernel 3.7, and its merge window is
approaching.

Thanks,
-- 
Jean Delvare

Re: [PATCH] gpio-ich: Share ownership of GPIO groups

[Linus Walleij]

On Tue, Sep 4, 2012 at 1:36 PM, Jean Delvare <khali@linux-fr.org> wrote:

> Any news on this? I'd like to get this patch (or an alternative
> implementation of the same) into kernel 3.7, and its merge window is
> approaching.

I have acked the GPIO part, the rest is up to Sam. He's often in
submarine mode but usually he appears just in due time for the merge
window...

Yours,
Linus Walleij

Re: [PATCH] gpio-ich: Share ownership of GPIO groups

[Samuel Ortiz]

Hi Jean,

On Mon, Jul 23, 2012 at 05:34:15PM +0200, Jean Delvare wrote:
> The ICH chips have their GPIO pins organized in 2 or 3 independent
> groups of 32 GPIO pins. It can happen that the ACPI BIOS wants to make
> use of pins in one group, preventing the OS to access these. This does
> not prevent the OS from accessing the other group(s).
> 
> This is the case for example on my Asus Z8NA-D6 board. The ACPI BIOS
> wants to control GPIO 18 (group 1), while I (the OS) need to control
> GPIO 52 and 53 (group 2) for SMBus multiplexing.
> 
> So instead of checking for ACPI resource conflict on the whole I/O
> range, check on a per-group basis, and consider it a success if at
> least one of the groups is available for the OS to use.
> 
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Peter Tyser <ptyser@xes-inc.com>
> Cc: Aaron Sierra <asierra@xes-inc.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> ---
> That's probably not the nicest code you've seen, but everything else I
> could think of either couldn't work or was looking worse. If anyone can
> think of a better approach, I'm all ears.
> 
>  drivers/gpio/gpio-ich.c     |   79 +++++++++++++++++++++++++++++++++++++------
>  drivers/mfd/lpc_ich.c       |   29 ++++++++++++++-
>  include/linux/mfd/lpc_ich.h |    1 
>  3 files changed, 97 insertions(+), 12 deletions(-)
Applied (finally) to my for-next branch, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH] gpio-ich: Share ownership of GPIO groups

[Jean Delvare]

On Tue, 11 Sep 2012 12:41:16 +0200, Samuel Ortiz wrote:
> Hi Jean,
> 
> On Mon, Jul 23, 2012 at 05:34:15PM +0200, Jean Delvare wrote:
> > The ICH chips have their GPIO pins organized in 2 or 3 independent
> > groups of 32 GPIO pins. It can happen that the ACPI BIOS wants to make
> > use of pins in one group, preventing the OS to access these. This does
> > not prevent the OS from accessing the other group(s).
> > 
> > This is the case for example on my Asus Z8NA-D6 board. The ACPI BIOS
> > wants to control GPIO 18 (group 1), while I (the OS) need to control
> > GPIO 52 and 53 (group 2) for SMBus multiplexing.
> > 
> > So instead of checking for ACPI resource conflict on the whole I/O
> > range, check on a per-group basis, and consider it a success if at
> > least one of the groups is available for the OS to use.
> > 
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > Cc: Peter Tyser <ptyser@xes-inc.com>
> > Cc: Aaron Sierra <asierra@xes-inc.com>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Samuel Ortiz <sameo@linux.intel.com>
> > ---
> > That's probably not the nicest code you've seen, but everything else I
> > could think of either couldn't work or was looking worse. If anyone can
> > think of a better approach, I'm all ears.
> > 
> >  drivers/gpio/gpio-ich.c     |   79 +++++++++++++++++++++++++++++++++++++------
> >  drivers/mfd/lpc_ich.c       |   29 ++++++++++++++-
> >  include/linux/mfd/lpc_ich.h |    1 
> >  3 files changed, 97 insertions(+), 12 deletions(-)
> Applied (finally) to my for-next branch, thanks.

Excellent, thank you!

-- 
Jean Delvare