| Message ID | 1410176880-22524-1-git-send-email-mika.westerberg@linux.intel.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Mon, Sep 8, 2014 at 1:47 PM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > Some newer Intel SoCs like Braswell already have more than 256 GPIOs > available so the default limit is exceeded. In order to support these add > back the custom GPIO header with limit of 512 GPIOs for x86. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> Argh! This is the kind of stuff I want to get rid of .... Preferably gpio should be a subsystem without a lot of hooks all over the place with arch-specific modifications for this and that, including the max number of GPIOs. I would actually prefer if you bump the value in include/asm-generic/gpio.h to 512 over this. But better still, now that we have descriptors etc would be to define some new per-arch selectable config option like CONFIG_ONLY_DYNAMIC_GPIO that changes the GPIO core to use something like a radix tree to store and retrieve descriptors. I.e. in drivers/gpio/gpiolib.c get rid of this: static struct gpio_desc gpio_desc[ARCH_NR_GPIOS]; Replace it with a radix tree of descriptors. This however makes it *impossible* to use things like desc_to_gpio() and/or gpio_to_desc() so the code has to be augmented all over the place to avoid any uses of GPIO numbers on that architecture, but I am sure it *can* be done on pure ACPI or device tree systems, and that's what we should aim for. Comments? Yours, Linus Walleij -- 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
Isn't this exactly what ida is for? Sent from my tablet, pardon any formatting problems. > On Sep 9, 2014, at 0:24, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Mon, Sep 8, 2014 at 1:47 PM, Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > >> Some newer Intel SoCs like Braswell already have more than 256 GPIOs >> available so the default limit is exceeded. In order to support these add >> back the custom GPIO header with limit of 512 GPIOs for x86. >> >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Argh! This is the kind of stuff I want to get rid of .... > > Preferably gpio should be a subsystem without a lot of hooks all over > the place with arch-specific modifications for this and that, including > the max number of GPIOs. > > I would actually prefer if you bump the value in > include/asm-generic/gpio.h to 512 over this. > > But better still, now that we have descriptors etc would be to define > some new per-arch selectable config option like > CONFIG_ONLY_DYNAMIC_GPIO that changes the GPIO > core to use something like a radix tree to store and retrieve > descriptors. > > I.e. in drivers/gpio/gpiolib.c get rid of this: > static struct gpio_desc gpio_desc[ARCH_NR_GPIOS]; > > Replace it with a radix tree of descriptors. > > This however makes it *impossible* to use things like desc_to_gpio() > and/or gpio_to_desc() so the code has to be augmented all over the > place to avoid any uses of GPIO numbers on that architecture, > but I am sure it *can* be done on pure ACPI or device tree > systems, and that's what we should aim for. > > Comments? > > Yours, > Linus Walleij -- 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
On Tue, Sep 09, 2014 at 09:24:59AM +0200, Linus Walleij wrote: > On Mon, Sep 8, 2014 at 1:47 PM, Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > > Some newer Intel SoCs like Braswell already have more than 256 GPIOs > > available so the default limit is exceeded. In order to support these add > > back the custom GPIO header with limit of 512 GPIOs for x86. > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Argh! This is the kind of stuff I want to get rid of .... > > Preferably gpio should be a subsystem without a lot of hooks all over > the place with arch-specific modifications for this and that, including > the max number of GPIOs. > > I would actually prefer if you bump the value in > include/asm-generic/gpio.h to 512 over this. OK, this makes sense and I can do the change for the next revision of the patch. > But better still, now that we have descriptors etc would be to define > some new per-arch selectable config option like > CONFIG_ONLY_DYNAMIC_GPIO that changes the GPIO > core to use something like a radix tree to store and retrieve > descriptors. > > I.e. in drivers/gpio/gpiolib.c get rid of this: > static struct gpio_desc gpio_desc[ARCH_NR_GPIOS]; > > Replace it with a radix tree of descriptors. > > This however makes it *impossible* to use things like desc_to_gpio() > and/or gpio_to_desc() so the code has to be augmented all over the > place to avoid any uses of GPIO numbers on that architecture, > but I am sure it *can* be done on pure ACPI or device tree > systems, and that's what we should aim for. > > Comments? That's a rather big rework to the GPIO subsystem and its users. I agree that it should be the goal eventually. For x86 such conversion is not that simple because we have systems out there that have neither ACPI nor DT available. -- 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
I should have responded to that thread long ago, but I am currently on holidays and strangely tend to check my mail less often than I should. :P On Tue, Sep 9, 2014 at 4:24 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > Argh! This is the kind of stuff I want to get rid of .... > > Preferably gpio should be a subsystem without a lot of hooks all over > the place with arch-specific modifications for this and that, including > the max number of GPIOs. > > I would actually prefer if you bump the value in > include/asm-generic/gpio.h to 512 over this. > > But better still, now that we have descriptors etc would be to define > some new per-arch selectable config option like > CONFIG_ONLY_DYNAMIC_GPIO that changes the GPIO > core to use something like a radix tree to store and retrieve > descriptors. > > I.e. in drivers/gpio/gpiolib.c get rid of this: > static struct gpio_desc gpio_desc[ARCH_NR_GPIOS]; > > Replace it with a radix tree of descriptors. > > This however makes it *impossible* to use things like desc_to_gpio() > and/or gpio_to_desc() so the code has to be augmented all over the > place to avoid any uses of GPIO numbers on that architecture, > but I am sure it *can* be done on pure ACPI or device tree > systems, and that's what we should aim for. desc_to_gpio()/gpio_to_desc() could still work even if we remove the big array of GPIO descriptors. Actually that's what I intended to do when I first submitted the gpiod patches some time ago but it was rejected for performance reasons. desc_to_gpio() actually doesn't even access this array - it does its job using the chip base and the beginning address of its descriptors array. gpio_to_desc() would suffer a performance hit. What I initially proposed was to parse the linked list of GPIO chips and check if the requested number is in their assigned range. This is obviously slower than accessing an array, but if we consider that we generally don't have too many GPIO chips on a given hardware I don't think the hit would be that bad. It would also give some incentive for people to move to the gpiod interface. I also have a patch in my queue that enables multiple users on the same GPIO descriptor (something requested since some time already). What happens with it is that descriptors ownership is fully transferred to the gpio_chip instances, and the static array becomes a array of double-pointers, making it considerable smaller and reducing the impact of increasing its size. Maybe I should submit that change just for this case? -- 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
On Fri, Sep 19, 2014 at 04:20:22PM +0900, Alexandre Courbot wrote: > I should have responded to that thread long ago, but I am currently on > holidays and strangely tend to check my mail less often than I should. > :P > > On Tue, Sep 9, 2014 at 4:24 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > > Argh! This is the kind of stuff I want to get rid of .... > > > > Preferably gpio should be a subsystem without a lot of hooks all over > > the place with arch-specific modifications for this and that, including > > the max number of GPIOs. > > > > I would actually prefer if you bump the value in > > include/asm-generic/gpio.h to 512 over this. > > > > But better still, now that we have descriptors etc would be to define > > some new per-arch selectable config option like > > CONFIG_ONLY_DYNAMIC_GPIO that changes the GPIO > > core to use something like a radix tree to store and retrieve > > descriptors. > > > > I.e. in drivers/gpio/gpiolib.c get rid of this: > > static struct gpio_desc gpio_desc[ARCH_NR_GPIOS]; > > > > Replace it with a radix tree of descriptors. > > > > This however makes it *impossible* to use things like desc_to_gpio() > > and/or gpio_to_desc() so the code has to be augmented all over the > > place to avoid any uses of GPIO numbers on that architecture, > > but I am sure it *can* be done on pure ACPI or device tree > > systems, and that's what we should aim for. > > desc_to_gpio()/gpio_to_desc() could still work even if we remove the > big array of GPIO descriptors. Actually that's what I intended to do > when I first submitted the gpiod patches some time ago but it was > rejected for performance reasons. > > desc_to_gpio() actually doesn't even access this array - it does its > job using the chip base and the beginning address of its descriptors > array. > > gpio_to_desc() would suffer a performance hit. What I initially > proposed was to parse the linked list of GPIO chips and check if the > requested number is in their assigned range. This is obviously slower > than accessing an array, but if we consider that we generally don't > have too many GPIO chips on a given hardware I don't think the hit > would be that bad. It would also give some incentive for people to > move to the gpiod interface. From what I can tell based on x86 based systems, there is typically only one GPIO controller in the PCH/SoC that controls all the pins. A good example is the Braswell/Cherryview controller. So I agree with you that the performance hit would be negliglible. > I also have a patch in my queue that enables multiple users on the > same GPIO descriptor (something requested since some time already). > What happens with it is that descriptors ownership is fully > transferred to the gpio_chip instances, and the static array becomes a > array of double-pointers, making it considerable smaller and reducing > the impact of increasing its size. Maybe I should submit that change > just for this case? Go for it :) I can try to assist if any testing is needed. -- 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
On Fri, Sep 19, 2014 at 12:20 AM, Alexandre Courbot <gnurou@gmail.com> wrote: > On Tue, Sep 9, 2014 at 4:24 PM, Linus Walleij <linus.walleij@linaro.org> wrote: >> This however makes it *impossible* to use things like desc_to_gpio() >> and/or gpio_to_desc() so the code has to be augmented all over the >> place to avoid any uses of GPIO numbers on that architecture, >> but I am sure it *can* be done on pure ACPI or device tree >> systems, and that's what we should aim for. > > desc_to_gpio()/gpio_to_desc() could still work even if we remove the > big array of GPIO descriptors. Actually that's what I intended to do > when I first submitted the gpiod patches some time ago but it was > rejected for performance reasons. OK. I'm ready to revisit the subject. > desc_to_gpio() actually doesn't even access this array - it does its > job using the chip base and the beginning address of its descriptors > array. Right. > gpio_to_desc() would suffer a performance hit. What I initially > proposed was to parse the linked list of GPIO chips and check if the > requested number is in their assigned range. This is obviously slower > than accessing an array, but if we consider that we generally don't > have too many GPIO chips on a given hardware I don't think the hit > would be that bad. It would also give some incentive for people to > move to the gpiod interface. I think the performance hit is acceptable, because this should not be on a hot path anyway. I would say go ahead with this refactoring. > I also have a patch in my queue that enables multiple users on the > same GPIO descriptor (something requested since some time already). > What happens with it is that descriptors ownership is fully > transferred to the gpio_chip instances, and the static array becomes a > array of double-pointers, making it considerable smaller and reducing > the impact of increasing its size. Maybe I should submit that change > just for this case? Ummmmm I think that is an orthogonal thing, but honestly I'm not following the double-pointers thing, so I guess I need to see the patch. Yours, Linus Walleij -- 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
On Sat, Sep 20, 2014 at 2:48 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Fri, Sep 19, 2014 at 12:20 AM, Alexandre Courbot <gnurou@gmail.com> wrote: >> On Tue, Sep 9, 2014 at 4:24 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > >>> This however makes it *impossible* to use things like desc_to_gpio() >>> and/or gpio_to_desc() so the code has to be augmented all over the >>> place to avoid any uses of GPIO numbers on that architecture, >>> but I am sure it *can* be done on pure ACPI or device tree >>> systems, and that's what we should aim for. >> >> desc_to_gpio()/gpio_to_desc() could still work even if we remove the >> big array of GPIO descriptors. Actually that's what I intended to do >> when I first submitted the gpiod patches some time ago but it was >> rejected for performance reasons. > > OK. I'm ready to revisit the subject. > >> desc_to_gpio() actually doesn't even access this array - it does its >> job using the chip base and the beginning address of its descriptors >> array. > > Right. > >> gpio_to_desc() would suffer a performance hit. What I initially >> proposed was to parse the linked list of GPIO chips and check if the >> requested number is in their assigned range. This is obviously slower >> than accessing an array, but if we consider that we generally don't >> have too many GPIO chips on a given hardware I don't think the hit >> would be that bad. It would also give some incentive for people to >> move to the gpiod interface. > > I think the performance hit is acceptable, because this should > not be on a hot path anyway. I would say go ahead with this refactoring. Great! I will come with something once my holidays are over. It should not be a complex change. > >> I also have a patch in my queue that enables multiple users on the >> same GPIO descriptor (something requested since some time already). >> What happens with it is that descriptors ownership is fully >> transferred to the gpio_chip instances, and the static array becomes a >> array of double-pointers, making it considerable smaller and reducing >> the impact of increasing its size. Maybe I should submit that change >> just for this case? > > Ummmmm I think that is an orthogonal thing, but honestly I'm > not following the double-pointers thing, so I guess I need to see > the patch. Yes, this is completely orthogonal, and actually this won't be needed if we decide to get rid of that array. -- 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
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 778178f4c7d1..0bf6fe76e7ba 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -136,6 +136,7 @@ config X86 select HAVE_ACPI_APEI if ACPI select HAVE_ACPI_APEI_NMI if ACPI select ACPI_LEGACY_TABLES_LOOKUP if ACPI + select ARCH_HAVE_CUSTOM_GPIO_H config INSTRUCTION_DECODER def_bool y diff --git a/arch/x86/include/asm/gpio.h b/arch/x86/include/asm/gpio.h index b3799d88ffcf..152b71788f2d 100644 --- a/arch/x86/include/asm/gpio.h +++ b/arch/x86/include/asm/gpio.h @@ -1,4 +1,50 @@ -#ifndef __LINUX_GPIO_H -#warning Include linux/gpio.h instead of asm/gpio.h -#include <linux/gpio.h> -#endif +/* + * GPIO customization for x86. + * + * Based on the original code: + * + * Copyright (c) 2007-2008 MontaVista Software, Inc. + * Author: Anton Vorontsov <avorontsov@ru.mvista.com> + * + * Copyright (c) 2014, Intel Corporation. + * Author: Mika Westerberg <mika.westerberg@linux.intel.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef __ASM_X86_GPIO_H +#define __ASM_X86_GPIO_H + +#define ARCH_NR_GPIOS 512 +#include <asm-generic/gpio.h> + +#ifdef CONFIG_GPIOLIB +static inline int gpio_get_value(unsigned int gpio) +{ + return __gpio_get_value(gpio); +} + +static inline void gpio_set_value(unsigned int gpio, int value) +{ + __gpio_set_value(gpio, value); +} + +static inline int gpio_cansleep(unsigned int gpio) +{ + return __gpio_cansleep(gpio); +} + +static inline int gpio_to_irq(unsigned int gpio) +{ + return __gpio_to_irq(gpio); +} + +static inline int irq_to_gpio(unsigned int irq) +{ + return -EINVAL; +} +#endif /* CONFIG_GPIOLIB */ + +#endif /* __ASM_X86_GPIO_H */
Some newer Intel SoCs like Braswell already have more than 256 GPIOs available so the default limit is exceeded. In order to support these add back the custom GPIO header with limit of 512 GPIOs for x86. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- arch/x86/Kconfig | 1 + arch/x86/include/asm/gpio.h | 54 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 51 insertions(+), 4 deletions(-)