| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20120801111359.6839bec7@endymion.delvare>
Date: Wed, 1 Aug 2012 11:13:59 +0200
From: Jean Delvare <khali@...ux-fr.org>
To: Grant Likely <grant.likely@...retlab.ca>,
Samuel Ortiz <sameo@...ux.intel.com>,
Linus Walleij <linus.walleij@...ricsson.com>
Cc: LKML <linux-kernel@...r.kernel.org>,
Peter Tyser <ptyser@...-inc.com>,
Aaron Sierra <asierra@...-inc.com>
Subject: Re: [PATCH] gpio-ich: Share ownership of GPIO groups
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@...ux-fr.org>
> Cc: Peter Tyser <ptyser@...-inc.com>
> Cc: Aaron Sierra <asierra@...-inc.com>
> Cc: Grant Likely <grant.likely@...retlab.ca>
> Cc: Samuel Ortiz <sameo@...ux.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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists