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 <kh...@linux-fr.org>
> Cc: Peter Tyser <pty...@xes-inc.com>
> Cc: Aaron Sierra <asie...@xes-inc.com>
> Cc: Grant Likely <grant.lik...@secretlab.ca>
> Cc: Samuel Ortiz <sa...@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));
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to