Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
On Tue, Oct 30, 2012 at 12:34 PM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Sun, Oct 28, 2012 at 09:12:52PM +0100, Linus Walleij wrote: Moving this handling to bus code or anywhere else invariably implies that resource acquisition/release order does not matter, and my point is that it does. Doing this in the buses is definitely wrong, as you say it's not bus specific. I do however think we can usefully do this stuff in a SoC specific place like a power domain, keeping the SoC integration code together and out of the drivers. IME the SoCs where you need to do different things for different IPs shoudl mostly still get some reuse out of such an approach. Talking to Kevin Hilman today he was also stressing that power domains is a good thing for handling resources, especially when replacing prior hacks in the custom clk code. However he pointed specifically to clocks and voltages, which may be true. What worries me is when knowledge of the hardware which is traditionally a concern for the device driver start to bubble up to the power domain (or better renamed resource domain if use like this, as Felipe points out). I worry that we will end up with power/resource domain code that start to look like this: suspend() switch (device.id) { case DEV_FOO: clk_disable(); pinctrl_set_state(); power_domain_off(); case DEV_BAR: pinctrl_set_state(); clk_disable(); // Always-on domain case DEV_BAZ: pinctrl_set_state(); clk_disable(); power_domain_off(); case ... Mutate the above with silicon errata, specific tweaks etc that Felipe was mentioning. What is happening is that device-specific behaviour which traditionally handled in the driver is now inside the power/resource domain. I agree that if the domain was doing the same thing for each piece of hardware, this would be the right thing to do, and I think the in-kernel examples are all simple, e.g. arch/arm/mach-omap2/powerdomain* is all about power domains and nothing else, and the notifiers used by SHmobile is all about clock gating and nothing else. Another effect is that moving all resource handling to power domains is that if we do that for a widely shared device driver like the PL011 that mandates that power domains need to be implemented for U300, Ux500, Nomadik, SPEAr 13xx, 3xx, 6xx, Versatile, Versatile Express, Integrator (AP CP) and BCM2835. Probably more. None of which has power domains (upstream) as of today. Some of which (U300, Ux500, Nomadik, SPEAr, Integrator, BCM2835) implement pin control. Basically power (resource) domains have the side-effect of in this light not doing one thing (power domains) but instead imposing all-or-nothing imperialistic characteristics. While avoiding a set of distributed, optional pinctrl hooks, it mandates a central piece of upfront powerdomain boilerplate to be present in each and every platform that wants to control its pins. I think the lesser of two evils is the distributed approach, and then I'm talking about pinctrl only, disregarding the fact that clocks and power domains are basically subject to the same kind of argument. I still buy into the concept of using power domains for exactly power domains only. Arguably this is an elegance opinion... I worry that the per-SoC power domain implementation which will live in arch/arm/mach-* as of today will become the new board file problem, overburdening the arch/* tree. Maybe I'm mistaken as to the size of these things, but just doing ls arch/arm/mach-omap2/powerdomain* makes me start worrying. Yours, Linus Walleij -- 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/
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
On Tue, Oct 30, 2012 at 12:49 PM, Felipe Balbi ba...@ti.com wrote: On Tue, Oct 30, 2012 at 11:24:10AM +, Mark Brown wrote: We need some place to put the SoC integration; power domains seem like the obvious place to me but YMMV. Nothing about having this out of the except that pin muxing has nothing to do with power domain. To me that sounds like an abuse of the API. It could be renamed to power resources or something as long as it's related to resource handling related to the PM calls. But I worry that it violates the Unix principle to do one thing and one thing only. A device power resource framework goes in the opposite direction, trying to do a lot of unrelated things in a central place as opposed to distributing the task. drivers requires that this be done by individual subsystems in isolation from each other. Half the point here is that for the reusable IPs this stuff often isn't driver specific at all, it's often more about the SoC integration than it is about the driver and so you'll get a consistent pattern for most IPs on the SoC. and all of that SoC-specific detail is already hidden behind power domains, runtime PM, pinctrl, clk API, regulator framework, etc. I agree. pinctrl has already done a fair job at trying to be abstract in the states requested from the core, in linux/pinctrl/pinctrl-state.h. And I accept the idea to try to centralize more as well, maybe as a helpful struct and some inlines for the pinctrl core. I think this is enough, and pushing all handles into central code creates a problem elsewhere. (But I'm not so certain ... so I might just change opinion one of those days depending on what arguments will be made.) Yours, Linus Walleij -- 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/
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
On Tue, Oct 30, 2012 at 3:07 PM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: Essentially all the patches I'm seeing adding pinctrl are totally mindless, most of them are just doing the initial setup on boot and not doing any active management at runtime at all. None of the Ux500 pinctrl patches are like that. I concludes in some other mails that all Ux500 drivers will need to handle atleast two states (default and sleep) and some a third state (idle). And this is what we're doing in our patches. Arguably it can all be pushed to power domains, but that will as said mandate all affected systems to implement power domains, and effectively moving code from drivers/* to arch/arm/* in our case atleast. Yours, Linus Walleij -- 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/
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
On Tue, Oct 30, 2012 at 7:37 PM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: More seriously the amount of time we seem to have been spending recently on changes which end up requiring us to go through essentially every driver and add code to them (often several times) doesn't seem like we're doing a good job here. If this is your main concern you should be made aware that there are people out there planning to supplant the existing DT probe paths that are now being added to each and every ARM-related driver with an ACPI probe path as ARM servers come into the picture. pinctrl is really noticable because it's new but it's not the only thing. As a subsystem maintainer this code just makes me want to add new subsystem features to pull the code out of drivers but obviously that's not something that should be being done at the subsystem level. We did manage to drag the power/voltage domain per se out of the AMBA bus, and recommend that people (like us) do that business using the power domains. I think most people (including OMAP) have bought into the concept of using the runtime PM framework and power domains to control the power domain switches. It's this wider concept of using the loose concept PM resource domains to control also clocks and pins that is at stake, and so far the runtime PM core people (Rafael and Magnus) has not said much so I think we need some kind of indication from them as to what is to happen, long-term, with drivers handling their own clocks and pins. Should it be centralized or not? If it's to be centralized it needs to become a large piece of infrastructure refactoring and needs the attention of Linaro and the like to happen. I've CC:ed a few people into this thread so we get some traction, we need more subsystem maintainers in here. Yours, Linus Walleij -- 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/
[GIT PULL] GPIO fixes for v3.7
Hi Linus, here is a set of GPIO fixes accumulated since -rc1 that have been trickling in. Condensed description of the contents is in the tag. Please pull them in! Yours, Linus Walleij The following changes since commit ddffeb8c4d0331609ef2581d84de4d763607bd37: Linux 3.7-rc1 (2012-10-14 14:41:04 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git tags/gpio-fixes-v3.7-rc4 for you to fetch changes up to 8fcff5f13773aa3898df1d13a1615d468079cb15: GPIO: mvebu-gpio: Don't initialize the mask_cache (2012-10-30 22:34:20 +0100) Some GPIO fixes for the v3.7 series since -rc1: - Fix a potential bit wrap issue in the Timberdale driver - Fix up the buffer allocation size in the 74x164 driver - Set the value in direction_output() right in the mvebu driver - Return proper error codes for invalid GPIOs - Fix an off-mode bug for the OMAP - Don't initialized the mask_cach on the mvebu driver Andrew Lunn (1): GPIO: mvebu-gpio: Don't initialize the mask_cache Dan Carpenter (1): gpio-timberdale: fix a potential wrapping issue Jon Hunter (1): gpio/omap: fix off-mode bug: clear debounce settings on free/reset Mathias Nyman (1): gpiolib: Don't return -EPROBE_DEFER to sysfs, or for invalid gpios Roland Stigge (1): gpio-74x164: Fix buffer allocation size Thomas Petazzoni (1): gpio: mvebu: correctly set the value in direction_output() drivers/gpio/gpio-74x164.c | 2 +- drivers/gpio/gpio-mvebu.c | 4 +++- drivers/gpio/gpio-omap.c | 35 +++ drivers/gpio/gpio-timberdale.c | 4 ++-- drivers/gpio/gpiolib.c | 10 +++--- 5 files changed, 48 insertions(+), 7 deletions(-) -- 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/
Re: [PATCH RESEND 1/5 v6] gpio: Add a block GPIO API to gpiolib
-nchip; i++) + kfree(block-gbc[i].remap); + kfree(block-gpio); + kfree(block-gbc); + kfree(block); +} +EXPORT_SYMBOL_GPL(gpio_block_free); So if we only had a real struct device inside the gpiochip all this boilerplate would go away, as devm_* take care of releasing the resources. +unsigned long gpio_block_get(const struct gpio_block *block) +{ + struct gpio_block_chip *gbc; + int i, j; + unsigned long values = 0; + + for (i = 0; i block-nchip; i++) { + unsigned long remapped = 0; + + gbc = block-gbc[i]; + + if (gbc-gc-get_block) { + remapped = gbc-gc-get_block(gbc-gc, gbc-mask); + } else { + /* emulate */ + for_each_set_bit(j, gbc-mask, BITS_PER_LONG) + remapped |= gbc-gc-get(gbc-gc, + gbc-gc-base + j) j; + } + + for (j = 0; j gbc-nremap; j++) { + struct gpio_remap *gr = gbc-remap[j]; + + values |= (remapped gr-offset) gr-mask; + } + } + + return values; +} +EXPORT_SYMBOL_GPL(gpio_block_get); + +void gpio_block_set(struct gpio_block *block, unsigned long values) +{ + struct gpio_block_chip *gbc; + int i, j; + + for (i = 0; i block-nchip; i++) { + unsigned long remapped = 0; + + gbc = block-gbc[i]; + + for (j = 0; j gbc-nremap; j++) { + struct gpio_remap *gr = gbc-remap[j]; + + remapped |= (values gr-mask) gr-offset; + } + if (gbc-gc-set_block) { + gbc-gc-set_block(gbc-gc, gbc-mask, remapped); + } else { + /* emulate */ + for_each_set_bit(j, gbc-mask, BITS_PER_LONG) + gbc-gc-set(gbc-gc, gbc-gc-base + j, +(remapped j) 1); + } + } +} +EXPORT_SYMBOL_GPL(gpio_block_set); + +struct gpio_block *gpio_block_find_by_name(const char *name) +{ + struct gpio_block *i; + + list_for_each_entry(i, gpio_block_list, list) + if (!strcmp(i-name, name)) + return i; And here is a list anyway, I'm getting confused of this partitioning between using dynamic arrays and lists. Just use a list please. This part looks good. Yours, Linus Walleij -- 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/
[PATCH] ARM: plat-versatile: move FPGA irq driver to drivers/irqchip
This moves the Versatile FPGA interrupt controller driver, used in the Integrator/AP, Integrator/CP and some Versatile boards, out of arch/arm/plat-versatile and down to drivers/irqchip where we have consensus that such drivers belong. The header file is consequently moved to linux/platform_data/irq-versatile-fpga.h. Signed-off-by: Linus Walleij linus.wall...@linaro.org --- arch/arm/Kconfig | 4 +- arch/arm/mach-integrator/integrator_ap.c | 3 +- arch/arm/mach-integrator/integrator_cp.c | 2 +- arch/arm/mach-versatile/core.c | 2 +- arch/arm/plat-versatile/Kconfig| 9 - arch/arm/plat-versatile/Makefile | 1 - drivers/irqchip/Kconfig| 9 +- drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-arm-fpga.c | 204 + .../irqchip/irq-versatile-fpga.c | 4 +- .../linux/platform_data/irq-versatile-fpga.h | 0 11 files changed, 220 insertions(+), 19 deletions(-) create mode 100644 drivers/irqchip/irq-arm-fpga.c rename arch/arm/plat-versatile/fpga-irq.c = drivers/irqchip/irq-versatile-fpga.c (97%) rename arch/arm/plat-versatile/include/plat/fpga-irq.h = include/linux/platform_data/irq-versatile-fpga.h (100%) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 73067ef..2205e3e 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -284,8 +284,8 @@ config ARCH_INTEGRATOR select MULTI_IRQ_HANDLER select NEED_MACH_MEMORY_H select PLAT_VERSATILE - select PLAT_VERSATILE_FPGA_IRQ select SPARSE_IRQ + select VERSATILE_FPGA_IRQ help Support for ARM's Integrator platform. @@ -318,7 +318,7 @@ config ARCH_VERSATILE select PLAT_VERSATILE select PLAT_VERSATILE_CLCD select PLAT_VERSATILE_CLOCK - select PLAT_VERSATILE_FPGA_IRQ + select VERSATILE_FPGA_IRQ help This enables support for ARM Ltd Versatile board. diff --git a/arch/arm/mach-integrator/integrator_ap.c b/arch/arm/mach-integrator/integrator_ap.c index 4f13bc5..caa279f 100644 --- a/arch/arm/mach-integrator/integrator_ap.c +++ b/arch/arm/mach-integrator/integrator_ap.c @@ -34,6 +34,7 @@ #include linux/mtd/physmap.h #include linux/clk.h #include linux/platform_data/clk-integrator.h +#include linux/platform_data/irq-versatile-fpga.h #include linux/of_irq.h #include linux/of_address.h #include linux/of_platform.h @@ -56,8 +57,6 @@ #include asm/mach/pci.h #include asm/mach/time.h -#include plat/fpga-irq.h - #include common.h /* diff --git a/arch/arm/mach-integrator/integrator_cp.c b/arch/arm/mach-integrator/integrator_cp.c index 4423bc8..b50fdc7 100644 --- a/arch/arm/mach-integrator/integrator_cp.c +++ b/arch/arm/mach-integrator/integrator_cp.c @@ -23,6 +23,7 @@ #include linux/gfp.h #include linux/mtd/physmap.h #include linux/platform_data/clk-integrator.h +#include linux/platform_data/irq-versatile-fpga.h #include linux/of_irq.h #include linux/of_address.h #include linux/of_platform.h @@ -46,7 +47,6 @@ #include asm/hardware/timer-sp.h #include plat/clcd.h -#include plat/fpga-irq.h #include plat/sched_clock.h #include common.h diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c index 5b5c1ee..46bfb8c 100644 --- a/arch/arm/mach-versatile/core.c +++ b/arch/arm/mach-versatile/core.c @@ -35,6 +35,7 @@ #include linux/gfp.h #include linux/clkdev.h #include linux/mtd/physmap.h +#include linux/platform_data/irq-versatile-fpga.h #include asm/irq.h #include asm/hardware/arm_timer.h @@ -51,7 +52,6 @@ #include asm/hardware/timer-sp.h #include plat/clcd.h -#include plat/fpga-irq.h #include plat/sched_clock.h #include core.h diff --git a/arch/arm/plat-versatile/Kconfig b/arch/arm/plat-versatile/Kconfig index 2a4ae8a..619f0fa 100644 --- a/arch/arm/plat-versatile/Kconfig +++ b/arch/arm/plat-versatile/Kconfig @@ -6,15 +6,6 @@ config PLAT_VERSATILE_CLOCK config PLAT_VERSATILE_CLCD bool -config PLAT_VERSATILE_FPGA_IRQ - bool - select IRQ_DOMAIN - -config PLAT_VERSATILE_FPGA_IRQ_NR - int - default 4 - depends on PLAT_VERSATILE_FPGA_IRQ - config PLAT_VERSATILE_LEDS def_bool y if NEW_LEDS depends on ARCH_REALVIEW || ARCH_VERSATILE diff --git a/arch/arm/plat-versatile/Makefile b/arch/arm/plat-versatile/Makefile index 74cfd94..f88d448 100644 --- a/arch/arm/plat-versatile/Makefile +++ b/arch/arm/plat-versatile/Makefile @@ -2,7 +2,6 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include obj-$(CONFIG_PLAT_VERSATILE_CLOCK) += clock.o obj-$(CONFIG_PLAT_VERSATILE_CLCD) += clcd.o -obj-$(CONFIG_PLAT_VERSATILE_FPGA_IRQ) += fpga-irq.o obj-$(CONFIG_PLAT_VERSATILE_LEDS) += leds.o obj-$(CONFIG_PLAT_VERSATILE_SCHED_CLOCK) += sched-clock.o obj-$(CONFIG_SMP) += headsmp.o platsmp.o diff --git
Fwd: [PATCHv2] Input: omap4-keypad: Add pinctrl support
On Wed, Oct 31, 2012 at 9:10 PM, Kevin Hilman khil...@deeprootsystems.com wrote: Linus Walleij linus.wall...@linaro.org writes: piece of hardware, this would be the right thing to do, and I think the in-kernel examples are all simple, e.g. arch/arm/mach-omap2/powerdomain* is all about power domains and nothing else, FYI... that code isn't the same as PM domain. This sort of points to a core problem here. Our terminologies are ambiguous that we cannot understand each others code. As long as linux/pm_domain.h begins: /* * pm_domain.h - Definitions and headers related to device power domains. * But arguably that should just be patched (I think there are a few remnants in the code still implying that these things are only about power). That code is for the *hardware* powerdomains, not the software concept of PM domain. In OMAP, PM domain is implmented at the omap_device level. And omap_device is the abstraction of an IP block that knows about all the PM related register settings, clocks, HW powerdomain, voltage domain, PM related pin-muxing etc. etc.All of these things are abstracted in an omap_device, so that the PM domain implementation for OMAP looks rather simple (c.f. omap_device_pm_domain in arch/arm/plat-omap/omap_device.c.) OK following now... I think the lesser of two evils is the distributed approach, The pinctrl examples I've seen mentioned so far are all PM related (sleep, idle, wakeup, etc.) so to me I think they still belong in PM domains (and that's how we handle the PM related pins in OMAP.) Well, the pinctrl grabbers in these drivers are using these states also for platforms that do not even select CONFIG_PM. For example mach-nomadik is quite happy that the PL011 driver is thusly muxing in its pins. And would require refactoring to use PM domains. So basically this requirement comes down to: - When dealing with a SoC IP block driver - That need to multiplex pins - Then your SoC must select CONFIG_PM and CONFIG_PM_RUNTIME and CONFIG_PM_GENERIC_DOMAINS and implement proper domain handling hooks. Is this correct? And for Mark, Dmitry, does this correspond to your view? It's actually something that needs to be acknowledged by the ARM SoC maintainers, because they will be the ones telling all subarch maintainers to go implement full PM handling with these three frameworks whenever an SoC driver want to handle pins. Bascially all subsystem maintainers dealing with embedded SoCs need to be aligned on this as well... And IIUC not only pins but also silicon block clocks? I can surely fix these for my systems, but it really needs to be enforced widely or it will be a mess. I worry that the per-SoC power domain implementation which will live in arch/arm/mach-* as of today will become the new board file problem, overburdening the arch/* tree. Maybe I'm mistaken as to the size of these things, but just doing ls arch/arm/mach-omap2/powerdomain* makes me start worrying. Yes, I agree that this means more code/data in arch/arm/mach-*, but IMO, that's really where it belongs. It really is SoC integration details, and driver should really not know about it. OK we need feedback from ARM SoC on this. Yours, Linus Walleij -- 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/
Re: [PATCH] ARM: plat-versatile: move FPGA irq driver to drivers/irqchip
On Thu, Nov 1, 2012 at 2:02 AM, Rob Herring robherri...@gmail.com wrote: On 10/31/2012 04:31 PM, Linus Walleij wrote: rename arch/arm/plat-versatile/include/plat/fpga-irq.h = include/linux/platform_data/irq-versatile-fpga.h (100%) I think include/linux/irqchip/ is the right place. OK I'll fix. Ideally we would not need the header at all. You can remove some of the function declarations if you base this on Thomas Petazzoni's series to have a common init function for DT and also move the fpga_handle_irq init into the fpga_irq_init function. Sounds like a separate patch but surely we can do this. Is Thomas' stuff on a branch somewhere that I can then rebase upon to get it upstream? I was planning to get this series as such to the ARM SoC maintainers soon-ish. Yours, Linus Walleij -- 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/
Re: Fwd: [PATCHv2] Input: omap4-keypad: Add pinctrl support
On Thu, Nov 1, 2012 at 12:42 PM, Kevin Hilman khil...@deeprootsystems.com wrote: [Me] Well, the pinctrl grabbers in these drivers are using these states also for platforms that do not even select CONFIG_PM. For example mach-nomadik is quite happy that the PL011 driver is thusly muxing in its pins. And would require refactoring to use PM domains. If CONFIG_PM is disabled, then is it safe to assume that the pins in question are probably only done once at init time. I assume during -probe(). ? Sadly no. Consider drivers/tty/serial/amba-pl011.c Many ARM platforms have several instances of PL011, and not all of them have CONFIG_PM friends, so it's a good example. Here the driver will probe and currently fetch a pinctrl handle and looks up two states: default, which refers to the situation you describe, and optionally sleep which will put pins into a low-power state. The driver will currently put the pins into the sleep state when .shutdown() is called by something (userspace or in-kernel users). So in the new suggested scheme using runtime PM, this would have to be replaced by pm_runtime_get[_sync]() and pm_runtime_put() hints and the current pin handling deleted, and for each platform using this driver instead implement a PM domain to do the same thing. Else you loose this runtime power optimization. This is what I refer to the all-or-nothing charcter of runtime PM domains... but maybe it's a good thing, I haven't quite made my mind up about it. (...) if what we want/need are only ways to introduce SoC-specific integration details into non-PM transitions (e.g. probe/remove), maybe bus notifiers would suffice here. e.g. you'd get a bus notifier when the device is added/attached and any init-time pinctrl setup could be done then. This still keeps drivers clean of SoC-specific integration data/code, and also allows that to happen whether or not PM features are enabled. It doesn't cut it for any of our drivers as shown above, but it would work for the patch in $SUBJECT. It sounds like the way silicon clocks are handled on SH am I right? Yours, Linus Walleij -- 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/
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
On Thu, Nov 1, 2012 at 1:07 PM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Thu, Nov 01, 2012 at 09:54:00AM +0100, Linus Walleij wrote: For the pin hogging I'd actually been thinking separately that we should just have the device core do a devm_pinctrl_get_set_default() prior to probing the device and store the result in the struct device. That would immediately remove almost all of the current pinctrl users, users that do need to do things with the data or check the result can then pick up the pinctrl pointer from the device struct. I never thought of that. This sounds like it would work. And the good thing is that this is a clean cut so we will centralized code without having to decide right now how to handle the pm idle/sleep cases. Talking here with Kevin Hilman on my left and Grant Likely on my right (they're physically here) there is some worry about stashing stuff into struct device. What if I retrieve this in the pinctrl subsystem using bus notifiers and then expose the struct pinctrl * to the clients by using pinctrl_get() and when you get such a handle in your probe() you know that the pinctrl subsystem has already fetched the handle and set it to default at that point? I just worry whether there is a fringe case where the default state is not be be default-selected in probe(), the API semantics does not mandate that. But I think this is the case for all in-kernel drivers so we wouldn't be breaking anything by doing this right now. And platforms can just leave the default state undefined in that case. It's actually something that needs to be acknowledged by the ARM SoC maintainers, because they will be the ones telling all subarch maintainers to go implement full PM handling with these three frameworks whenever an SoC driver want to handle pins. Well, they're going to have to implement it somewhere anyway - either in the drivers or in the SoC stuff. Sure I just worry about it being done is several different ways and creating a mess so they need to be involved to block other approaches. I can surely fix these for my systems, but it really needs to be enforced widely or it will be a mess. We definitely need to decide if it's something that should be open coded everywhere. If I prepare a patch to move the fetch+set defaul to the pinctrl core using notifiers, we centralize one piece and we get the currently floating patches out of the way. Then what to do with sleep and idle is a question we need to handle next. Requiring PM domains for this is one approach, albeit a bit controversial. Yours, Linus Walleij -- 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/
Re: [PATCH] ARM: plat-versatile: move FPGA irq driver to drivers/irqchip
On Thu, Nov 1, 2012 at 10:12 AM, Thomas Petazzoni thomas.petazz...@free-electrons.com wrote: On Thu, 1 Nov 2012 10:00:19 +0100, Linus Walleij wrote: Sounds like a separate patch but surely we can do this. Is Thomas' stuff on a branch somewhere that I can then rebase upon to get it upstream? I was planning to get this series as such to the ARM SoC maintainers soon-ish. Not at the moment. But do you want me to put that in a branch, and agglomerate all the related patches (posted by Rob for GIC/VIC and by you for the FPGA IRQ controller), and then later send a pull request to Arnd with those changes? Whatever I can base on ... I would just push the stuff you consider stable to ARM SoC as quickly as possible so we can grab it from there and base development on it. Then each of us can just request the ARM SoC people to pull it and state that it is based on that branch so they need to pull it into the same place. Also, again, the whole point of the initial infrastructure in drivers/irqchip/ was to avoid adding per-driver header files in include/linux/irqchip/, so there should at least be a long term plan on how to remove those headers file, either by moving more platforms to DT, or my extending the irqchip infrastructure to cover more features. So the header in this case looks like this: #ifndef PLAT_FPGA_IRQ_H #define PLAT_FPGA_IRQ_H struct device_node; struct pt_regs; void fpga_handle_irq(struct pt_regs *regs); void fpga_irq_init(void __iomem *, const char *, int, int, u32, struct device_node *node); int fpga_irq_of_init(struct device_node *node, struct device_node *parent); #endif So this is the stuff that needs to be called from the machine descriptor, nothing else. Example: MACHINE_START(INTEGRATOR, ARM-Integrator) /* Maintainer: ARM Ltd/Deep Blue Solutions Ltd */ .atag_offset= 0x100, .reserve= integrator_reserve, .map_io = ap_map_io, .init_early = ap_init_early, .init_irq = ap_init_irq, .handle_irq = fpga_handle_irq, .timer = ap_timer, .init_machine = ap_init, .restart= integrator_restart, MACHINE_END The .init_irq hooks above contain some other stuff apart from just calling these directly, but the problem remains: how to cross-call these functions from the machine start since the IRQs are needed by say the timer and everything else. include/linux/irqchip/bcm2835.h look exactly the same (just one function instead of separete DT/non-DT versions) so there isn't exactly a precedent on how to solve this in an elegant way. But maybe your patch set contains the silver bullet that will decouple this and fix everything? Then I can do a patch to convert this and the BCM2835 too probably... Yours, Linus Walleij -- 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/
[PATCH v2] ARM: plat-versatile: move FPGA irq driver to drivers/irqchip
This moves the Versatile FPGA interrupt controller driver, used in the Integrator/AP, Integrator/CP and some Versatile boards, out of arch/arm/plat-versatile and down to drivers/irqchip where we have consensus that such drivers belong. The header file is consequently moved to linux/platform_data/irq-versatile-fpga.h. Signed-off-by: Linus Walleij linus.wall...@linaro.org --- ChangeLog v1-v2: - Moved include to linux/irqchip/versatile-fpga.h - Deleted merge/moval artifact --- arch/arm/Kconfig| 4 +- arch/arm/mach-integrator/integrator_ap.c| 3 +- arch/arm/mach-integrator/integrator_cp.c| 2 +- arch/arm/mach-versatile/core.c | 2 +- arch/arm/plat-versatile/Kconfig | 9 -- arch/arm/plat-versatile/Makefile| 1 - arch/arm/plat-versatile/fpga-irq.c | 204 arch/arm/plat-versatile/include/plat/fpga-irq.h | 13 -- drivers/irqchip/Kconfig | 9 +- drivers/irqchip/Makefile| 1 + drivers/irqchip/irq-versatile-fpga.c| 204 include/linux/irqchip/versatile-fpga.h | 13 ++ 12 files changed, 231 insertions(+), 234 deletions(-) delete mode 100644 arch/arm/plat-versatile/fpga-irq.c delete mode 100644 arch/arm/plat-versatile/include/plat/fpga-irq.h create mode 100644 drivers/irqchip/irq-versatile-fpga.c create mode 100644 include/linux/irqchip/versatile-fpga.h diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 73067ef..2205e3e 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -284,8 +284,8 @@ config ARCH_INTEGRATOR select MULTI_IRQ_HANDLER select NEED_MACH_MEMORY_H select PLAT_VERSATILE - select PLAT_VERSATILE_FPGA_IRQ select SPARSE_IRQ + select VERSATILE_FPGA_IRQ help Support for ARM's Integrator platform. @@ -318,7 +318,7 @@ config ARCH_VERSATILE select PLAT_VERSATILE select PLAT_VERSATILE_CLCD select PLAT_VERSATILE_CLOCK - select PLAT_VERSATILE_FPGA_IRQ + select VERSATILE_FPGA_IRQ help This enables support for ARM Ltd Versatile board. diff --git a/arch/arm/mach-integrator/integrator_ap.c b/arch/arm/mach-integrator/integrator_ap.c index 4f13bc5..e67a9fe 100644 --- a/arch/arm/mach-integrator/integrator_ap.c +++ b/arch/arm/mach-integrator/integrator_ap.c @@ -31,6 +31,7 @@ #include linux/clockchips.h #include linux/interrupt.h #include linux/io.h +#include linux/irqchip/versatile-fpga.h #include linux/mtd/physmap.h #include linux/clk.h #include linux/platform_data/clk-integrator.h @@ -56,8 +57,6 @@ #include asm/mach/pci.h #include asm/mach/time.h -#include plat/fpga-irq.h - #include common.h /* diff --git a/arch/arm/mach-integrator/integrator_cp.c b/arch/arm/mach-integrator/integrator_cp.c index 4423bc8..acecf04 100644 --- a/arch/arm/mach-integrator/integrator_cp.c +++ b/arch/arm/mach-integrator/integrator_cp.c @@ -20,6 +20,7 @@ #include linux/amba/clcd.h #include linux/amba/mmci.h #include linux/io.h +#include linux/irqchip/versatile-fpga.h #include linux/gfp.h #include linux/mtd/physmap.h #include linux/platform_data/clk-integrator.h @@ -46,7 +47,6 @@ #include asm/hardware/timer-sp.h #include plat/clcd.h -#include plat/fpga-irq.h #include plat/sched_clock.h #include common.h diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c index 5b5c1ee..5d59294 100644 --- a/arch/arm/mach-versatile/core.c +++ b/arch/arm/mach-versatile/core.c @@ -32,6 +32,7 @@ #include linux/amba/mmci.h #include linux/amba/pl022.h #include linux/io.h +#include linux/irqchip/versatile-fpga.h #include linux/gfp.h #include linux/clkdev.h #include linux/mtd/physmap.h @@ -51,7 +52,6 @@ #include asm/hardware/timer-sp.h #include plat/clcd.h -#include plat/fpga-irq.h #include plat/sched_clock.h #include core.h diff --git a/arch/arm/plat-versatile/Kconfig b/arch/arm/plat-versatile/Kconfig index 2a4ae8a..619f0fa 100644 --- a/arch/arm/plat-versatile/Kconfig +++ b/arch/arm/plat-versatile/Kconfig @@ -6,15 +6,6 @@ config PLAT_VERSATILE_CLOCK config PLAT_VERSATILE_CLCD bool -config PLAT_VERSATILE_FPGA_IRQ - bool - select IRQ_DOMAIN - -config PLAT_VERSATILE_FPGA_IRQ_NR - int - default 4 - depends on PLAT_VERSATILE_FPGA_IRQ - config PLAT_VERSATILE_LEDS def_bool y if NEW_LEDS depends on ARCH_REALVIEW || ARCH_VERSATILE diff --git a/arch/arm/plat-versatile/Makefile b/arch/arm/plat-versatile/Makefile index 74cfd94..f88d448 100644 --- a/arch/arm/plat-versatile/Makefile +++ b/arch/arm/plat-versatile/Makefile @@ -2,7 +2,6 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include obj-$(CONFIG_PLAT_VERSATILE_CLOCK) += clock.o obj-$(CONFIG_PLAT_VERSATILE_CLCD) += clcd.o -obj-$(CONFIG_PLAT_VERSATILE_FPGA_IRQ) += fpga-irq.o obj-$(CONFIG_PLAT_VERSATILE_LEDS
Re: [PATCH] gpio-lpc32xx: Fix value handling of gpio_direction_output()
On Thu, Sep 20, 2012 at 10:48 AM, Roland Stigge sti...@antcom.de wrote: For GPIOs of gpio-lpc32xx, gpio_direction_output() ignores the value argument (initial value of output). This patch fixes this by setting the level accordingly. Signed-off-by: Roland Stigge sti...@antcom.de Applied with Alexandre's ACK, should go into stable right? So tagging this with Cc: stable and sending for the -rc:s. Thanks, Linus Walleij -- 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/
Re: [PATCH 1/1] mfd: Provide the PRCMU with its own IRQ domain
On Mon, Sep 24, 2012 at 10:11 AM, Lee Jones lee.jo...@linaro.org wrote: The PRCMU has its own USB, Thermal, GPIO, Modem, HSI and RTC drivers, amongst other things. This patch allows those subordinate devices to use it as an interrupt controller as and when they are DT enabled. CC: Samuel Ortiz sa...@linux.intel.com Signed-off-by: Lee Jones lee.jo...@linaro.org This is good stuff. Acked-by: Linus Walleij linus.wall...@linaro.org Sam, please pick this up, IRQdomains are a pathway to many good things. Yours, Linus Walleij -- 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/
[PATCH] ARM: ux500: 8500: update I2C sleep states pinctrl
From: Patrice Chotard patrice.chot...@stericsson.com This defines the proper sleep states for all the I2C pins of the MOP500 DB8500 ASIC setting. Signed-off-by: Patrice Chotard patrice.chot...@stericsson.com Signed-off-by: Linus Walleij linus.wall...@linaro.org --- Requesting ARM SoC ACKs to take this through pinctrl for ease-of-merge going forward, as dependecies are in there. --- arch/arm/mach-ux500/board-mop500-pins.c | 30 -- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/arch/arm/mach-ux500/board-mop500-pins.c b/arch/arm/mach-ux500/board-mop500-pins.c index 722523c..099369e 100644 --- a/arch/arm/mach-ux500/board-mop500-pins.c +++ b/arch/arm/mach-ux500/board-mop500-pins.c @@ -30,7 +30,6 @@ static enum custom_pin_cfg_t pinsfor; #define BIAS(a,b) static unsigned long a[] = { b } BIAS(pd, PIN_PULL_DOWN); -BIAS(slpm_gpio_nopull, PIN_SLPM_GPIO|PIN_SLPM_INPUT_NOPULL); BIAS(in_nopull, PIN_INPUT_NOPULL); BIAS(in_nopull_slpm_nowkup, PIN_INPUT_NOPULL|PIN_SLPM_WAKEUP_DISABLE); BIAS(in_pu, PIN_INPUT_PULLUP); @@ -55,12 +54,16 @@ BIAS(slpm_out_hi_wkup_pdis, PIN_SLEEPMODE_ENABLED|PIN_SLPM_OUTPUT_HIGH|PIN_SLPM_ BIAS(slpm_out_wkup_pdis, PIN_SLEEPMODE_ENABLED|PIN_SLPM_WAKEUP_ENABLE|PIN_SLPM_PDIS_DISABLED); BIAS(slpm_out_lo_wkup, PIN_SLEEPMODE_ENABLED|PIN_SLPM_OUTPUT_LOW|PIN_SLPM_WAKEUP_ENABLE); BIAS(slpm_out_lo_wkup_pdis, PIN_SLEEPMODE_ENABLED|PIN_SLPM_OUTPUT_LOW|PIN_SLPM_WAKEUP_ENABLE|PIN_SLPM_PDIS_DISABLED); +BIAS(slpm_in_nopull_wkup_pdis, PIN_SLEEPMODE_ENABLED|PIN_SLPM_INPUT_NOPULL|PIN_SLPM_WAKEUP_ENABLE|PIN_SLPM_PDIS_DISABLED); /* We use these to define hog settings that are always done on boot */ #define DB8500_MUX_HOG(group,func) \ PIN_MAP_MUX_GROUP_HOG_DEFAULT(pinctrl-db8500, group, func) #define DB8500_PIN_HOG(pin,conf) \ PIN_MAP_CONFIGS_PIN_HOG_DEFAULT(pinctrl-db8500, pin, conf) +#define DB8500_PIN_SLEEP(pin, conf, dev) \ + PIN_MAP_CONFIGS_PIN(dev, PINCTRL_STATE_SLEEP, pinctrl-db8500, \ + pin, conf) /* These are default states associated with device and changed runtime */ #define DB8500_MUX(group,func,dev) \ @@ -160,19 +163,26 @@ static struct pinctrl_map __initdata mop500_family_pinmap[] = { DB8500_MUX(lcdaclk_b_1, lcda, mcde-tvout), /* Mux in LCD VSI1 and pull it up for MCDE HDMI output */ DB8500_MUX(lcdvsi1_a_1, lcd, av8100-hdmi), - /* Mux in I2C blocks, put pins into GPIO in sleepmode no pull-up */ + /* Mux in i2c0 block, default state */ DB8500_MUX(i2c0_a_1, i2c0, nmk-i2c.0), - DB8500_PIN(GPIO147_C15, slpm_gpio_nopull, nmk-i2c.0), - DB8500_PIN(GPIO148_B16, slpm_gpio_nopull, nmk-i2c.0), + /* i2c0 sleep state */ + DB8500_PIN_SLEEP(GPIO147_C15, slpm_in_nopull_wkup_pdis, nmk-i2c.0), /* SDA */ + DB8500_PIN_SLEEP(GPIO148_B16, slpm_in_nopull_wkup_pdis, nmk-i2c.0), /* SCL */ + /* Mux in i2c1 block, default state */ DB8500_MUX(i2c1_b_2, i2c1, nmk-i2c.1), - DB8500_PIN(GPIO16_AD3, slpm_gpio_nopull, nmk-i2c.1), - DB8500_PIN(GPIO17_AD4, slpm_gpio_nopull, nmk-i2c.1), + /* i2c1 sleep state */ + DB8500_PIN_SLEEP(GPIO16_AD3, slpm_in_nopull_wkup_pdis, nmk-i2c.1), /* SDA */ + DB8500_PIN_SLEEP(GPIO17_AD4, slpm_in_nopull_wkup_pdis, nmk-i2c.1), /* SCL */ + /* Mux in i2c2 block, default state */ DB8500_MUX(i2c2_b_2, i2c2, nmk-i2c.2), - DB8500_PIN(GPIO10_AF5, slpm_gpio_nopull, nmk-i2c.2), - DB8500_PIN(GPIO11_AG4, slpm_gpio_nopull, nmk-i2c.2), + /* i2c2 sleep state */ + DB8500_PIN_SLEEP(GPIO10_AF5, slpm_in_nopull_wkup_pdis, nmk-i2c.2), /* SDA */ + DB8500_PIN_SLEEP(GPIO11_AG4, slpm_in_nopull_wkup_pdis, nmk-i2c.2), /* SCL */ + /* Mux in i2c3 block, default state */ DB8500_MUX(i2c3_c_2, i2c3, nmk-i2c.3), - DB8500_PIN(GPIO229_AG7, slpm_gpio_nopull, nmk-i2c.3), - DB8500_PIN(GPIO230_AF7, slpm_gpio_nopull, nmk-i2c.3), + /* i2c3 sleep state */ + DB8500_PIN_SLEEP(GPIO229_AG7, slpm_in_nopull_wkup_pdis, nmk-i2c.3), /* SDA */ + DB8500_PIN_SLEEP(GPIO230_AF7, slpm_in_nopull_wkup_pdis, nmk-i2c.3), /* SCL */ /* Mux in SDI0 (here called MC0) used for removable MMC/SD/SDIO cards */ DB8500_MUX(mc0_a_1, mc0, sdi0), DB8500_PIN(GPIO18_AC2, out_hi, sdi0), /* CMDDIR */ -- 1.7.11.3 -- 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/
Re: [PATCH] ARM: ux500: Move regulator-name properties out to board DTS files
On Tue, Sep 25, 2012 at 1:37 PM, Lee Jones lee.jo...@linaro.org wrote: Regulator supply names should be allocated by board rather than per SoC, as the same SoC could be wired differently on varying hardware. Here we push all regulator-name allocation out to the dbx5x0 subordinate board files; HREF and Snowball. Requested-by: Mark Brown broo...@opensource.wolfsonmicro.com Signed-off-by: Lee Jones lee.jo...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org Thanks, Linus Walleij -- 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/
Re: [PATCH RESEND] gpio-lpc32xx: Fix value handling of gpio_direction_output()
On Tue, Sep 25, 2012 at 9:53 AM, Roland Stigge sti...@antcom.de wrote: For GPIOs of gpio-lpc32xx, gpio_direction_output() ignores the value argument (initial value of output). This patch fixes this by setting the level accordingly. Signed-off-by: Roland Stigge sti...@antcom.de Acked-by: Alexandre Pereira da Silva aletes@gmail.com --- Applies to v3.6-rc7 I will attempt to funnel this to Torvalds today. Thanks, Linus Walleij -- 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/
Re: [PATCH v1 01/12] clk: davinci - add Main PLL clock driver
On Tue, Sep 25, 2012 at 12:20 AM, Murali Karicheri m-kariche...@ti.com wrote: +struct clk_davinci_pll_data { + /* physical addresses set by platform code */ + u32 phy_pllm; + /* if PLL has a prediv register this should be non zero */ + u32 phy_prediv; + /* if PLL has a postdiv register this should be non zero */ + u32 phy_postdiv; + /* mapped addresses. should be initialized by */ + void __iomem *pllm; + void __iomem *prediv; + void __iomem *postdiv; + u32 pllm_mask; + u32 prediv_mask; + u32 postdiv_mask; + u32 num; + /* framework flags */ + u32 flags; + /* pll flags */ + u32 pll_flags; + u32 fixed_prediv; /* use this value for prediv */ + u32 pllm_multiplier;/* multiply PLLM by this factor. By default +* most SOC set this to zero that translates +* to a multiplier of 1 and incrementer of 1. +* To override default, set this factor */ +}; OMG this commenting style hurt my eyes ;-) Please use good old kerneldoc above the struct instead. Yours, Linus Walleij -- 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/
Re: [PATCH] pinctrl: Fix potential memory leak in pinctrl_register_one_pin()
On Tue, Sep 25, 2012 at 9:11 AM, Sachin Kamat sachin.ka...@linaro.org wrote: 'pindesc' was not freed when returning from an error induced exit path. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Thanks, patch applied! Linus Walleij -- 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/
Re: [PATCH] ARM: ux500: Fix initialisation order for UIBs
On Tue, Sep 25, 2012 at 5:10 PM, Lee Jones lee.jo...@linaro.org wrote: An earlier change prevented User Interface Boards (UIBs) from being initialised on boards which did not support them. This change had the undesired effect of reordering the UIB initialisation calls with I2C registration. Here we ensure UIBs are only setup after all required infrastructure is already in place. Signed-off-by: Lee Jones lee.jo...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- 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/
Re: [RFC PATCH 2/17] input: RMI4 core bus and sensor drivers.
On Wed, Sep 26, 2012 at 1:53 AM, Christopher Heiny che...@synaptics.com wrote: +/* Remove access to raw format string versions */ +/*#undef simple_show_union_struct +#undef show_union_struct_unsigned +#undef show_store_union_struct +#undef show_repeated_union_struct +#undef show_store_repeated_union_struct*/ This looks like trying to reimplement ioctl() in sysfs. If what you want is to send big structs in/out of the kernel, use either ioctl() on device nodes (should be trivial since input is using real device nodes) or use configfs. I'm a little confused. There's repeated emphasis in the kernel doc that you shouldn't use ioctl() anymore - use sysfs instead. So we've been using sysfs, though it seems somewhat klutzy. If it's actually OK to use ioctl(), that could simplify things. On the other hand, using configfs might be more appropriate. OK yes configfs is said to be ideal for large configuration chunks, I haven't really used it. sysfs has this concept of one value per file, and that turns into the above serialization/marshalling code if followed, so it doesn't look good. Maybe configfs is the silver bullet. Yours, Linus Walleij -- 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/
[PATCH] pinctrl: clarify idle vs sleep states
From: Linus Walleij linus.wall...@linaro.org This pure documentation fix tries to align the idle and sleep pin states to the idle and suspend states from runtime PM. Signed-off-by: Linus Walleij linus.wall...@linaro.org --- include/linux/pinctrl/pinctrl-state.h | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/include/linux/pinctrl/pinctrl-state.h b/include/linux/pinctrl/pinctrl-state.h index 634608dc..2dbf71d 100644 --- a/include/linux/pinctrl/pinctrl-state.h +++ b/include/linux/pinctrl/pinctrl-state.h @@ -6,13 +6,15 @@ * @PINCTRL_STATE_DEFAULT: the state the pinctrl handle shall be put * into as default, usually this means the pins are up and ready to * be used by the device driver. This state is commonly used by - * hogs to configure muxing and pins at boot. + * hogs to configure muxing and pins at boot, and also as a state + * to go into when returning from sleep and idle in + * .pm_runtime_resume() or ordinary .resume() for example. * @PINCTRL_STATE_IDLE: the state the pinctrl handle shall be put into * when the pins are idle. Could typically be set from a - * pm_runtime_suspend() operation. + * pm_runtime_idle() operation. * @PINCTRL_STATE_SLEEP: the state the pinctrl handle shall be put into * when the pins are sleeping. Could typically be set from a - * common suspend() function. + * common pm_runtime_suspend() or ordinary .suspend() function. */ #define PINCTRL_STATE_DEFAULT default #define PINCTRL_STATE_IDLE idle -- 1.7.11.3 -- 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/
Re: replacement for /sys/kernel/debug/omap_mux in DT/pinctrl land ?
On Wed, Sep 26, 2012 at 2:56 PM, Matt Porter mpor...@ti.com wrote: Adding Linus W. and lkml. On Wed, Sep 26, 2012 at 01:46:45PM +0200, Koen Kooi wrote: Hi, With a patched 3.6rc7 on my beaglebone I can set the pinmux for pins using pinctrl and that seems to work. On the 3.2 vendor tree there was the omap_mux driver with an awesome debugfs interface: # cat /sys/kernel/debug/omap_mux/lcd_data0 name: lcd_data0.ehrpwm2A (0x44e108a0/0x8a0 = 0x0003), b NA, t NA mode: OMAP_PIN_OUTPUT | OMAP_MUX_MODE3 signals: lcd_data0 | gpmc_a0 | pr1_mii_mt0_clk | ehrpwm2A | NA | pr1_pru1_pru_r30_0 | pr1_pru1_pru_r31_0 | gpio2_6 Notice how it tells me that it's muxed the PWM in 2 ways: signal name (ehrpwm2A) and register content (0x0003). Compare to pinctrl: root@bone-mainline:/sys/kernel/debug/pinctrl/44e10800.pinmux# grep 8a0 * pinconf-pins:pin 40 (44e108a0): pingroups:pin 40 (44e108a0) pinmux-pins:pin 40 (44e108a0): 4a30.pruss (GPIO UNCLAIMED) function pinmux_pruss_led_pins group pinmux_pruss_led_pins pins:pin 40 (44e108a0) pinctrl-single What is that pin muxed to? It is part of the 'pinmux_pruss_led_pins' in the DT, but debugfs remains mute on how pin 40 is muxed. It does seem like a pretty big gap in the pinctrl/pinmux debugfs interface when viewed from an OMAP perspective. Ideally there would be a pinctrl/pinmux hook to the pinmux driver to provide the detailed h/w specific pin state info. So add the hooks you need? I assume you are using Tony's pinctrl-single driver, so Tony is the one to ask. Yours, Linus Walleij -- 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/
[PATCH] pinctrl/nomadik: allocate IRQ descriptors dynamically
From: Linus Walleij linus.wall...@linaro.org This allocates the IRQ descriptors for the Nomadik pin controller dynamically so that we don't have to rely on some other mechanism doing it, and moving a step closer to a linear IRQ domain. Cc: Rob Herring rob.herr...@calxeda.com Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/pinctrl/pinctrl-nomadik.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c index 3dde653..29921d1 100644 --- a/drivers/pinctrl/pinctrl-nomadik.c +++ b/drivers/pinctrl/pinctrl-nomadik.c @@ -1185,6 +1185,8 @@ static int __devinit nmk_gpio_probe(struct platform_device *dev) struct clk *clk; int secondary_irq; void __iomem *base; + int irq_start; + int irq_base; int irq; int ret; @@ -1288,9 +1290,22 @@ static int __devinit nmk_gpio_probe(struct platform_device *dev) platform_set_drvdata(dev, nmk_chip); + /* +* Allocate descriptors and IRQ domain using the legacy model, this +* should eventually be replaced with a linear IRQ domain as we get +* independent from the irq numbers with the switch to device tree. +*/ + irq_start = NOMADIK_GPIO_TO_IRQ(pdata-first_gpio); + irq_base = irq_alloc_descs(irq_start, 0, NMK_GPIO_PER_CHIP, + numa_node_id()); + if (IS_ERR_VALUE(irq_base)) { + WARN(1, Cannot allocate irq_descs @ IRQ%d, assuming pre-allocated\n, +irq_start); + irq_base = irq_start; + } nmk_chip-domain = irq_domain_add_legacy(np, NMK_GPIO_PER_CHIP, - NOMADIK_GPIO_TO_IRQ(pdata-first_gpio), - 0, nmk_gpio_irq_simple_ops, nmk_chip); + irq_base, 0, + nmk_gpio_irq_simple_ops, nmk_chip); if (!nmk_chip-domain) { dev_err(dev-dev, failed to create irqdomain\n); ret = -ENOSYS; -- 1.7.11.3 -- 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/
[GIT PULL] late gpio fix for v3.6
Hi Linus, here is a late fix for the GPIO subsystem from Roland Stigge. This one is going into the stable series so no point in holding it back. Please pull it in! Yours, Linus Walleij The following changes since commit 979570e02981d4a8fc20b3cc8fd651856c98ee9d: Linux 3.6-rc7 (2012-09-23 18:10:57 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git tags/gpio-fixes-v3.6 for you to fetch changes up to b1268d3737c6316016026245eef276eda6b0a621: gpio-lpc32xx: Fix value handling of gpio_direction_output() (2012-09-24 21:56:01 +0200) A lates GPIO fix: Roland Stigge found a problem in the LPC32xx driver where a callback ignores one of its arguments. It needs to go into stable too so sending this upstream immediately. Roland Stigge (1): gpio-lpc32xx: Fix value handling of gpio_direction_output() drivers/gpio/gpio-lpc32xx.c | 5 + 1 file changed, 5 insertions(+) -- 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/
Re: [PATCH] pinctrl: pinctrl-single: add debugfs pin h/w state info
On Wed, Sep 26, 2012 at 9:07 PM, Matt Porter mpor...@ti.com wrote: Adds support for displaying the individual pin h/w config state. Signed-off-by: Matt Porter mpor...@ti.com Acked-by: Tony Lindgren t...@atomide.com Thanks, patch applied. Linus Walleij -- 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/
Re: [PATCH] pinctrl/nomadik: allocate IRQ descriptors dynamically
On Thu, Sep 27, 2012 at 7:13 AM, Stephen Warren swar...@nvidia.com wrote: On 09/26/2012 11:18 AM, Linus Walleij wrote: + irq_start = NOMADIK_GPIO_TO_IRQ(pdata-first_gpio); Presumably that's a 1:1 mapping... Yep, albeit on a legacy domain to be able to keep the start irq. + irq_base = irq_alloc_descs(irq_start, 0, NMK_GPIO_PER_CHIP, +numa_node_id()); + if (IS_ERR_VALUE(irq_base)) { + WARN(1, Cannot allocate irq_descs @ IRQ%d, assuming pre-allocated\n, + irq_start); + irq_base = irq_start; + } Hmmm. Is this code targetting the DT case or the non-DT case or both? I think typically you'd call irq_domain_add_linear without forcing a particular IRQ base for the DT case, and only call irq_alloc_descs() for the non-DT case, right? This is indeed the non-DT case. The irqdomain stuff here doesn't seem to be properly addressing the DT usecase. I'll cook a v2 version doing a linear domain for DT and have Lee review it. Thanks! Linus Walleij -- 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/
Re: [PATCH] pinctrl/nomadik: allocate IRQ descriptors dynamically
On Thu, Sep 27, 2012 at 2:37 PM, Rob Herring robherri...@gmail.com wrote: nmk_chip-domain = irq_domain_add_legacy(np, NMK_GPIO_PER_CHIP, You might as well change to irq_domain_add_simple here. True. I'll fix. Perhaps we should just add irq_alloc_descs call into irq_domain_add_simple in the legacy case. It may need to be conditioned on SPARSE_IRQ. There's currently no callers, so it wouldn't break anything. OK get ready to ACK, I'll fix. Yours, Linus Walleij -- 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/
[PATCH 2/4] irqdomain: augment add_simple() to allocate descs
From: Linus Walleij linus.wall...@linaro.org Currently we rely on all IRQ chip instances to dynamically allocate their IRQ descriptors unless they use the linear IRQ domain. So for irqdomain_add_legacy() and irqdomain_add_simple() the caller need to make sure that descriptors are allocated. Let's slightly augment the yet unused irqdomain_add_simple() to also allocate descriptors as a means to simplify usage and avoid code duplication throughout the kernel. We warn if descriptors cannot be allocated, e.g. if a platform has the bad habit of hogging descriptors at boot time. Cc: Rob Herring rob.herr...@calxeda.com Cc: Thomas Gleixner t...@linutronix.de Cc: Grant Likely grant.lik...@secretlab.ca Cc: Russell King li...@arm.linux.org.uk Cc: Lee Jones lee.jo...@linaro.org Signed-off-by: Linus Walleij linus.wall...@linaro.org --- Rob/Grant/Thomas if you ACK this I will take it through the pinctrl tree since I introduce its only user right there. --- kernel/irq/irqdomain.c | 21 - 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index 49a7772..a0655b6 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -148,7 +148,8 @@ static unsigned int irq_domain_legacy_revmap(struct irq_domain *domain, * @host_data: Controller private data pointer * * Allocates a legacy irq_domain if irq_base is positive or a linear - * domain otherwise. + * domain otherwise. For the legacy domain, IRQ descriptors will also + * be allocated. * * This is intended to implement the expected behaviour for most * interrupt controllers which is that a linear mapping should @@ -162,11 +163,21 @@ struct irq_domain *irq_domain_add_simple(struct device_node *of_node, const struct irq_domain_ops *ops, void *host_data) { - if (first_irq 0) - return irq_domain_add_legacy(of_node, size, first_irq, 0, + if (first_irq 0) { + int irq_base; + + irq_base = irq_alloc_descs(first_irq, 0, size, numa_node_id()); + if (irq_base 0) { + WARN(1, Cannot allocate irq_descs @ IRQ%d, assuming pre-allocated\n, +first_irq); + irq_base = first_irq; + } + return irq_domain_add_legacy(of_node, size, irq_base, 0, ops, host_data); - else - return irq_domain_add_linear(of_node, size, ops, host_data); + } + + /* A linear domain is the default */ + return irq_domain_add_linear(of_node, size, ops, host_data); } /** -- 1.7.11.3 -- 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/
[PATCH 4/4] ARM: nomadik/ux500: convert to SPARSE_IRQ
From: Linus Walleij linus.wall...@linaro.org This converts the Nomadik and Ux500 over to using sparse IRQ, including some pokes around the pinctrl driver. To avoid referencing unnecessary header files, the plat-nomadik timer driver is augmented to pass an irq number at init time, and the change is applied across both platforms simultaneously for this reason. Cc: Lee Jones lee.jo...@linaro.org Signed-off-by: Linus Walleij linus.wall...@linaro.org --- arch/arm/Kconfig | 2 ++ arch/arm/mach-nomadik/board-nhk8815.c | 4 ++-- arch/arm/mach-nomadik/include/mach/irqs.h | 2 +- arch/arm/mach-ux500/board-mop500.c| 1 - arch/arm/mach-ux500/cpu-db8500.c | 1 + arch/arm/mach-ux500/devices-common.c | 1 + arch/arm/mach-ux500/devices-db8500.c | 1 + arch/arm/mach-ux500/devices-db8500.h | 1 + arch/arm/mach-ux500/include/mach/irqs.h | 2 +- arch/arm/mach-ux500/timer.c | 2 +- arch/arm/plat-nomadik/include/plat/mtu.h | 2 +- arch/arm/plat-nomadik/timer.c | 4 ++-- drivers/pinctrl/pinctrl-nomadik.c | 1 + 13 files changed, 15 insertions(+), 9 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 2f88d8d..bec5d08 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -945,6 +945,7 @@ config ARCH_U8500 select ARCH_HAS_CPUFREQ select HAVE_SMP select MIGHT_HAVE_CACHE_L2X0 + select SPARSE_IRQ help Support for ST-Ericsson's Ux500 architecture @@ -958,6 +959,7 @@ config ARCH_NOMADIK select PINCTRL select MIGHT_HAVE_CACHE_L2X0 select ARCH_REQUIRE_GPIOLIB + select SPARSE_IRQ help Support for the Nomadik platform by ST-Ericsson diff --git a/arch/arm/mach-nomadik/board-nhk8815.c b/arch/arm/mach-nomadik/board-nhk8815.c index f4535a7..287b349 100644 --- a/arch/arm/mach-nomadik/board-nhk8815.c +++ b/arch/arm/mach-nomadik/board-nhk8815.c @@ -27,7 +27,6 @@ #include asm/sizes.h #include asm/mach-types.h #include asm/mach/arch.h -#include asm/mach/irq.h #include asm/mach/flash.h #include asm/mach/time.h @@ -36,6 +35,7 @@ #include mach/nand.h #include mach/fsmc.h +#include mach/irqs.h #include cpu-8815.h @@ -260,7 +260,7 @@ static void __init nomadik_timer_init(void) src_cr |= SRC_CR_INIT_VAL; writel(src_cr, io_p2v(NOMADIK_SRC_BASE)); - nmdk_timer_init(io_p2v(NOMADIK_MTU0_BASE)); + nmdk_timer_init(io_p2v(NOMADIK_MTU0_BASE), IRQ_MTU0); } static struct sys_timer nomadik_timer = { diff --git a/arch/arm/mach-nomadik/include/mach/irqs.h b/arch/arm/mach-nomadik/include/mach/irqs.h index a118e61..b549d05 100644 --- a/arch/arm/mach-nomadik/include/mach/irqs.h +++ b/arch/arm/mach-nomadik/include/mach/irqs.h @@ -72,7 +72,7 @@ #define NOMADIK_NR_GPIO128 /* last 4 not wired to pins */ #define NOMADIK_GPIO_TO_IRQ(gpio) ((gpio) + NOMADIK_GPIO_OFFSET) #define NOMADIK_IRQ_TO_GPIO(irq) ((irq) - NOMADIK_GPIO_OFFSET) -#define NR_IRQS NOMADIK_GPIO_TO_IRQ(NOMADIK_NR_GPIO) +#define NOMADIK_NR_IRQS NOMADIK_GPIO_TO_IRQ(NOMADIK_NR_GPIO) /* Following two are used by entry_macro.S, to access our dual-vic */ #define VIC_REG_IRQSR0 0 diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c index a534d88..da2fd05 100644 --- a/arch/arm/mach-ux500/board-mop500.c +++ b/arch/arm/mach-ux500/board-mop500.c @@ -1,4 +1,3 @@ - /* * Copyright (C) 2008-2009 ST-Ericsson * diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c index db3c52d..0a09647 100644 --- a/arch/arm/mach-ux500/cpu-db8500.c +++ b/arch/arm/mach-ux500/cpu-db8500.c @@ -26,6 +26,7 @@ #include mach/devices.h #include mach/usb.h #include mach/db8500-regs.h +#include mach/irqs.h #include devices-db8500.h #include ste-dma40-db8500.h diff --git a/arch/arm/mach-ux500/devices-common.c b/arch/arm/mach-ux500/devices-common.c index dfdd4a5..0923dbd 100644 --- a/arch/arm/mach-ux500/devices-common.c +++ b/arch/arm/mach-ux500/devices-common.c @@ -15,6 +15,7 @@ #include plat/gpio-nomadik.h #include mach/hardware.h +#include mach/irqs.h #include devices-common.h diff --git a/arch/arm/mach-ux500/devices-db8500.c b/arch/arm/mach-ux500/devices-db8500.c index 91754a8..bec92a7 100644 --- a/arch/arm/mach-ux500/devices-db8500.c +++ b/arch/arm/mach-ux500/devices-db8500.c @@ -17,6 +17,7 @@ #include mach/hardware.h #include mach/setup.h +#include mach/irqs.h #include ste-dma40-db8500.h diff --git a/arch/arm/mach-ux500/devices-db8500.h b/arch/arm/mach-ux500/devices-db8500.h index 3c8010f..4b24c99 100644 --- a/arch/arm/mach-ux500/devices-db8500.h +++ b/arch/arm/mach-ux500/devices-db8500.h @@ -8,6 +8,7 @@ #ifndef __DEVICES_DB8500_H #define __DEVICES_DB8500_H +#include mach/irqs.h #include devices-common.h struct ske_keypad_platform_data; diff --git a/arch/arm/mach-ux500/include
[PATCH 1/4] pinctrl/nomadik: use irq_find_mapping()
From: Linus Walleij linus.wall...@linaro.org The code was using a homegrown method of looking up the offset from the irq domain, not to be encouraged. Use the proper irq_find_mapping() call instead. Cc: Lee Jones lee.jo...@linaro.org Cc: Rob Herring rob.herr...@calxeda.com Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/pinctrl/pinctrl-nomadik.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c index 3dde653..e031c84 100644 --- a/drivers/pinctrl/pinctrl-nomadik.c +++ b/drivers/pinctrl/pinctrl-nomadik.c @@ -826,16 +826,14 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc, { struct nmk_gpio_chip *nmk_chip; struct irq_chip *host_chip = irq_get_chip(irq); - unsigned int first_irq; chained_irq_enter(host_chip, desc); nmk_chip = irq_get_handler_data(irq); - first_irq = nmk_chip-domain-revmap_data.legacy.first_irq; while (status) { int bit = __ffs(status); - generic_handle_irq(first_irq + bit); + generic_handle_irq(irq_find_mapping(nmk_chip-domain, bit)); status = ~BIT(bit); } -- 1.7.11.3 -- 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/
[PATCH 3/4] pinctrl/nomadik: use simple or linear IRQ domain
From: Linus Walleij linus.wall...@linaro.org This alters the Nomadik pinctrl driver to: - Call irqdomain_add_linear() for the DT case so we get all independent from IRQ numbers in this case. - Call irqdomain_add_simple() for the legacy case, which allocates the IRQ descriptors for the Nomadik pin controller dynamically. Cc: Lee Jones lee.jo...@linaro.org Cc: Rob Herring rob.herr...@calxeda.com Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/pinctrl/pinctrl-nomadik.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c index e031c84..6aab107 100644 --- a/drivers/pinctrl/pinctrl-nomadik.c +++ b/drivers/pinctrl/pinctrl-nomadik.c @@ -1286,9 +1286,19 @@ static int __devinit nmk_gpio_probe(struct platform_device *dev) platform_set_drvdata(dev, nmk_chip); - nmk_chip-domain = irq_domain_add_legacy(np, NMK_GPIO_PER_CHIP, - NOMADIK_GPIO_TO_IRQ(pdata-first_gpio), - 0, nmk_gpio_irq_simple_ops, nmk_chip); + if (np) { + /* The DT case will just grab a set of IRQ numbers */ + nmk_chip-domain = irq_domain_add_linear(np, NMK_GPIO_PER_CHIP, + nmk_gpio_irq_simple_ops, nmk_chip); + } else { + /* Non-DT legacy mode, use hardwired IRQ numbers */ + int irq_start; + + irq_start = NOMADIK_GPIO_TO_IRQ(pdata-first_gpio); + nmk_chip-domain = irq_domain_add_simple(NULL, + NMK_GPIO_PER_CHIP, irq_start, + nmk_gpio_irq_simple_ops, nmk_chip); + } if (!nmk_chip-domain) { dev_err(dev-dev, failed to create irqdomain\n); ret = -ENOSYS; -- 1.7.11.3 -- 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/
Re: [PATCH 01/13] clk: davinci - add Main PLL clock driver
On Wed, Sep 26, 2012 at 8:07 PM, Murali Karicheri m-kariche...@ti.com wrote: +struct clk_davinci_pll_data { + /* physical addresses set by platform code */ + u32 phy_pllm; + /* if PLL has a prediv register this should be non zero */ + u32 phy_prediv; + /* if PLL has a postdiv register this should be non zero */ + u32 phy_postdiv; + /* mapped addresses. should be initialized by */ + void __iomem *pllm; + void __iomem *prediv; + void __iomem *postdiv; + u32 pllm_mask; + u32 prediv_mask; + u32 postdiv_mask; + u32 num; + /* framework flags */ + u32 flags; + /* pll flags */ + u32 pll_flags; + /* use this value for prediv */ + u32 fixed_prediv; + /* multiply PLLM by this factor. By default most SOC set this to zero +* that translates to a multiplier of 1 and incrementer of 1. +* To override default, set this factor +*/ + u32 pllm_multiplier; +}; + No, that's not what I meant. I meant like this: /** * struct clk_davinci_pll_data - struct holding the PLL data * phy_pllm: physical addresses set by platform code * phy_prediv: ... (...) */ struct clk_davinci_pll_data { u32 phy_pllm; u32 phy_prediv; (...) }; Yours, Linus Walleij -- 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/
[PATCH v2] pinctrl: clarify idle vs sleep states
From: Linus Walleij linus.wall...@linaro.org This pure documentation fix tries to align the idle and sleep pin states to the idle and suspend states from runtime PM. Cc: Patrice Chotard patrice.chot...@st.com Signed-off-by: Linus Walleij linus.wall...@linaro.org --- ChangeLog v1-v2: - I changed my mind slightly after experimentation in practice (empirical stuff is the best). So now I think idle is for runtime_suspend(), and sleep is for suspend() proper. This works best in practice for us. I'm trying to set some model here that will be easy to understand and usable for typical cases. --- include/linux/pinctrl/pinctrl-state.h | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/include/linux/pinctrl/pinctrl-state.h b/include/linux/pinctrl/pinctrl-state.h index 634608dc..b5919f8 100644 --- a/include/linux/pinctrl/pinctrl-state.h +++ b/include/linux/pinctrl/pinctrl-state.h @@ -6,13 +6,18 @@ * @PINCTRL_STATE_DEFAULT: the state the pinctrl handle shall be put * into as default, usually this means the pins are up and ready to * be used by the device driver. This state is commonly used by - * hogs to configure muxing and pins at boot. + * hogs to configure muxing and pins at boot, and also as a state + * to go into when returning from sleep and idle in + * .pm_runtime_resume() or ordinary .resume() for example. * @PINCTRL_STATE_IDLE: the state the pinctrl handle shall be put into - * when the pins are idle. Could typically be set from a - * pm_runtime_suspend() operation. + * when the pins are idle. This is a state where the system is relaxed + * but not fully sleeping - some power may be on but clocks gated for + * example. Could typically be set from a pm_runtime_suspend() or + * pm_runtime_idle() operation. * @PINCTRL_STATE_SLEEP: the state the pinctrl handle shall be put into - * when the pins are sleeping. Could typically be set from a - * common suspend() function. + * when the pins are sleeping. This is a state where the system is in + * its lowest sleep state. Could typically be set from an + * ordinary .suspend() function. */ #define PINCTRL_STATE_DEFAULT default #define PINCTRL_STATE_IDLE idle -- 1.7.11.3 -- 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/
Re: [PATCH 17/17] ARM: soc: dependency warnings for errata
On Tue, Oct 2, 2012 at 6:36 PM, Arnd Bergmann a...@arndb.de wrote: The PL310_ERRATA_753970 and ARM_ERRATA_764369 symbols only make sense when the base features for them are enabled, so select them conditionally in Kconfig to avoid warnings like: warning: (UX500_SOC_COMMON) selects PL310_ERRATA_753970 which has unmet direct dependencies (CACHE_PL310) warning: (ARCH_TEGRA_2x_SOC ARCH_TEGRA_3x_SOC UX500_SOC_COMMON) selects ARM_ERRATA_764369 which has unmet direct dependencies (CPU_V7 SMP) Signed-off-by: Arnd Bergmann a...@arndb.de Cc: Linus Walleij linus.wall...@linaro.org Cc: Stephen Warren swar...@wwwdotorg.org Makes perfect sense. Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- 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/
Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib
On Sun, Sep 30, 2012 at 12:50 PM, Roland Stigge sti...@antcom.de wrote: Besides what I discussed with JC and Linus, I find the unsigned int (i.e. u32 or u64, depending on the arch) quite appealing. It is a nice compromise between my general bit mapped data model (variable size *u8 array) and the bool *values list. Even maps easily onto a single sysfs entry for values, by abstracting a gpio list to an actual data word. What do others think? JC? Linus? I'm considering this (unsigned int data) a valid option. I think we mostly use an unsigned long for such stuff as IRQ flags and ioctl() parameters in the kernel. In this case it has the upside that it will be 32bit on 32bit systems and 64bit on 64bit systems if I'm not mistaken. Yours, Linus Walleij -- 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/
Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib
On Sun, Sep 30, 2012 at 5:46 PM, Roland Stigge sti...@antcom.de wrote: On 30/09/12 17:19, Stijn Devriendt wrote: Rules are rules, but why make the interface overly complex when the multi-value file is saner, cleaner and simpler? Simply because they won't (and probably shouldn't) accept it mainline. We, including you and Stijn *are* the mainline ... ;-) The only reason I really dislike it is that the GPIO sysfs interface is scary as it is, so I don't want to add to it if we can instead push to reform it into something more sane. Linus Walleij -- 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/
Re: [PATCH 1/3] mfd: dbx500: Export prmcu_request_ape_opp_100_voltage
On Mon, Sep 24, 2012 at 4:43 PM, Ulf Hansson ulf.hans...@stericsson.com wrote: From: Ulf Hansson ulf.hans...@linaro.org This function needs to be exported to let clients be able to request the ape opp 100 voltage. Cc: Samuel Ortiz sa...@linux.intel.com Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- 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/
Re: [PATCH][GPIO] Add IRQ edge setter to gpiolib
On Fri, Oct 5, 2012 at 2:20 PM, Drasko DRASKOVIC drasko.drasko...@gmail.com wrote: please find a patch that adds IRQ edge set-up mechanism to sysfs that can be called from the kernel. This functionality can be very useful for embedded systems, as it permits kernel to do GPIO set-up during boot stage. Configuration which defines pins behavior is often kept in NVRAM, and during boot stage these structures can be parsed and executed by the kernel, so that when user processes already find all sysfs environment ready and correctly set-up. While at the present it is possible to export GPIO pins to sysfs (and correct direction / value), it is not possible to export IRQ configuration as well, so this must be done in user space (most often via command line). this patch implements missing functionality, so that gpio_sysfs_set_edge() function can be called directly from the kernel. Why not put the above text into the patch commit blurb? I really won't touch this unless I get a comment from Grant and/or Thomas Gleixner about it. The common GPIO sysfs is hairy enough as it is, and not universally liked. If I understand correctly the below more or less exports struct irq_chip to userspace, trying to hide it by instead exposing a property of the containing struct gpio_chip and it worries me. One possible comment is that you shouldn't add this to gpiolib, if you want to mess around with the irq_chip you should create sysfs entries for the irq_chip per se, then we can have a symbolic link from the gpio_chip to its irq_chip in sysfs, and you can follow that to change trigger flanks, right? Part of me doesn't like it when userspace is messing around with this kind of thing. Why can't this be set up from the kernel itself by some jam table? I can understand it if this is some lab board with GPIO or so, if it's some embedded GPIO controller within a laptop or something it surely should be in kernelspace. So please detail your usecase a bit... what is the code daemon etc in userspace poking around at these pins? What is is doing and why? Yours, Linus Walleij -- 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/
Re: [PATCH 6/6] gpio: gpio-da9052: Convert to the new da9052 interrupt functions
On Thu, Oct 4, 2012 at 5:15 AM, Fabio Estevam feste...@gmail.com wrote: From: Fabio Estevam fabio.este...@freescale.com Convert to the new da9052 interrupt functions, so that we can get rid of irq_base references. Cc: Grant Likely grant.lik...@secretlab.ca Cc: Linus Walleij linus.wall...@linaro.org Signed-off-by: Fabio Estevam fabio.este...@freescale.com Applied to my GPIO tree with Mark's ACK. Yours, Linus Walleij -- 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/
Re: [PATCH] gpio: add TS-5500 DIO headers support
On Fri, Oct 5, 2012 at 1:18 AM, Vivien Didelot vivien.dide...@gmail.com wrote: Grant, Linus, any feedback? On the entire driver or on the config fragment? The latter looks OK, I'll look at the driver per se now. (I was busy with the merge window you know...) Yours, Linus Walleij -- 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/
Re: [PATCH] gpio: add TS-5500 DIO headers support
) line.value_bit); It's quite a common way to clamp a numeral to a bool int. (...) +static int ts5500_gpio_to_irq(struct gpio_chip *chip, unsigned offset) +{ + const struct ts5500_dio line = ts5500_dios[offset]; + + /* Only a few lines are IRQ-capable */ + if (line.irq != NO_IRQ) + return line.irq; + + /* This allows to bridge a line with the IRQ line of the same header */ + if (dio1_irq offset 13) + return ts5500_dios[13].irq; + if (dio2_irq offset 13 offset 26) + return ts5500_dios[26].irq; + if (lcd_irq offset 26 offset 37) + return ts5500_dios[37].irq; Don't do this. Please use irqdomain for converting physical IRQ numbers to Linux IRQ numbers. (Consult other GPIO drivers for examples.) These magic constants (13, 26, 37) are scary too. You should not try to handle each block as a single IRQ, instead instatiate a struct irq_chip in the driver and let that use irqdomain do demux the IRQ and register a range of Linux IRQ numbers, on per pin, so the GPIO driver will handle the physical IRQ line, then dispatch to a fan-out handler, so drivers that need IRQs from the GPIO chip just register IRQ handlers like anyone else. (...) +static int __devinit ts5500_gpio_probe(struct platform_device *pdev) +{ + int ret; + unsigned long flags; + struct ts5500_gpio_platform_data *pdata = pdev-dev.platform_data; This is where you should allocate you state container dynamically. (...) + /* Enable IRQ generation */ + spin_lock_irqsave(lock, flags); + io_set_bit(7, 0x7a); /* DIO1_13 on IRQ7 */ + io_set_bit(7, 0x7d); /* DIO2_13 on IRQ6 */ + if (lcd_dio) { + io_clear_bit(4, 0x7d); /* LCD Header usage as DIO */ + io_set_bit(6, 0x7d); /* LCD_RS on IRQ1 */ + } This is pincontrol ... but well. It's very very little after all. +/** + * struct ts5500_gpio_platform_data - TS-5500 GPIO configuration + * @base: The GPIO base number to use. + * @lcd_dio: Use the LCD port as 11 additional digital I/O lines. + * @lcd_irq: Return IRQ1 for every line of LCD DIO header. + * @dio1_irq: Return IRQ7 for every line of DIO1 header. + * @dio2_irq: Return IRQ6 for every line of DIO2 header. + */ +struct ts5500_gpio_platform_data { + int base; + bool lcd_dio; + bool lcd_irq; + bool dio1_irq; + bool dio2_irq; +}; So I don't like how this hardcodes IRQ 1, 7, 6 to be used for this purpose, it's better to pass that in as resources from the platform. Yours, Linus Walleij -- 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/
Re: [PATCH 2/2] gpio: do not call __gpio_xxx under !CONFIG_GPIOLIB
On Wed, Oct 31, 2012 at 8:00 AM, Yuanhan Liu yuanhan@linux.intel.com wrote: Those functions are availabe only when CONFIG_GPIOLIB is set. So, we should not call them under !CONFIG_GPIOLIB block. This would fix following build errros: include/asm-generic/gpio.h: In function 'gpio_get_value_cansleep': include/asm-generic/gpio.h:220:2: error: implicit declaration of function '__gpio_get_value' include/asm-generic/gpio.h: In function 'gpio_set_value_cansleep': nclude/asm-generic/gpio.h:226:2: error: implicit declaration of function '__gpio_set_value' OK... static inline int gpio_get_value_cansleep(unsigned gpio) { - might_sleep(); So why are you deleting this very useful might_sleep() runtime semantic check? - return __gpio_get_value(gpio); + WARN_ON(1); + return 0; } static inline void gpio_set_value_cansleep(unsigned gpio, int value) { - might_sleep(); - __gpio_set_value(gpio, value); + WARN_ON(1); } Yours, Linus Walleij -- 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/
Re: [PATCH] gpio: em: Fix build errors
On Wed, Oct 31, 2012 at 10:03 AM, Axel Lin axel@ingics.com wrote: Fix below build errors: Thanks for fixing my screwups! Patch applied to my devel branch where the patch causing the problems was. Yours, Linus Walleij -- 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/
Re: How about a gpio_get(device *, char *) function?
On Wed, Oct 31, 2012 at 10:04 AM, Alex Courbot acour...@nvidia.com wrote: Would anyone be opposed to having a gpio_get() function that works similarly to e.g. regulator_get() and clk_get()? I understand the concept and why you want to do this. However I think the global GPIO numberspace defeats the purpose. gpio_get() should get an abstract handle just like clk_get() or regulator_get(), not a fixed numeral. That is the only way to really transit away from the global GPIO numberspace. The proper refactoring I can see is to introduce an orthogonal mechanism that will return something like a struct gpio_handle * when you call gpio_get(), and then use a new set of accessor functions to manipulate the GPIO, like gpio_handle_set()/ etc for all known GPIO operations. So defined in a new #ifdef CONFIG_GPIO_HANDLES or so in a new linux/gpio-handles.h header and like the other subsystems handling abstract resources just passing an opaque pointer cookie around. Then when migrating a driver to use this mechanism, only include linux/gpio-handles.h and make that one driver only use the abstract interface and get rid of the global numberspace. Then we can have drivers that under a transition period will support both the global numberspace and this new handle-based access pattern. Then migrate all clients over to linux/gpio-handle.h and finally kill off the old global numberspace and only use handles with that driver. So a much more thorough refactoring is needed, just doing a simple int lookup returning global GPIO numbers will likely only make the current mess worse. This more ambitious refactoring path is going to be a large endavour and will likely go through a lot of reviews and debate, but something like this is badly needed for the GPIO subsystem. Maybe Grant will have different opinions though so this is not the final word on this issue... Yours, Linus Walleij -- 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/
Re: [PATCH] gpio: adnp: Depend on OF_GPIO instead of OF
On Thu, Nov 1, 2012 at 11:22 AM, Thierry Reding thierry.red...@avionic-design.de wrote: The driver accesses the of_node field of struct gpio_chip, which is only available if OF_GPIO is selected. This solves a build issue on SPARC which conflicts with OF_GPIO and therefore does not provide this field. Signed-off-by: Thierry Reding thierry.red...@avionic-design.de Thanks! Patch applied. Yours, Linus Walleij -- 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/
Re: [PATCH 1/2] Input: nomadik-ske-keypad - fixup use of clk
On Thu, Nov 1, 2012 at 3:20 PM, Ulf Hansson ulf.hans...@stericsson.com wrote: From: Ulf Hansson ulf.hans...@linaro.org Do proper error handling for clk and make sure clocks are being prepared|unprepared as well as enabled|disabled. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- 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/
Re: [PATCH 2/2] Input: nomadik-ske-keypad - start using the apb_pclk
On Thu, Nov 1, 2012 at 3:20 PM, Ulf Hansson ulf.hans...@stericsson.com wrote: From: Ulf Hansson ulf.hans...@linaro.org Previously this clock was handled internally by the clockdriver, but now this is separate clk. So we need take care of it. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org So this is a silicon block clock and falls into the category of things we've been discussing... If I understand correctly, the only real solution is to implement the PM domains and have these enable the clocks. An alternative may be to move this driver over to the AMBA bus, because I think this device actually has primecell registers. Then the bus will take care of the pclk for starters. Yours, Linus Walleij -- 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/
Re: [PATCH] gpio-mcp23s08: Build I2C support even when CONFIG_I2C=m
On Fri, Nov 2, 2012 at 1:09 AM, Daniel M. Weeks d...@danweeks.net wrote: The driver has both SPI and I2C pieces. The appropriate pieces are built based on whether SPI and/or I2C is/are enabled. However, it was only checking if I2C was built-in, never if it was built as a module. This patch checks for either since building both this driver and I2C as modules is possible. Good catch! -#ifdef CONFIG_I2C +#if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE) But don't do it like this, there are already helpers for exactly this. Do: #if IS_ENABLED(CONFIG_I2C) Read include/linux/kconfig.h if in doubt... Looking forward to v2! Yours, Linus Walleij -- 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/
Re: [PATCH] gpio-pch: Set parent dev for gpio chip
On Fri, Nov 2, 2012 at 4:02 PM, Alexander Stein alexander.st...@systec-electronic.com wrote: This will show the gpio chip as a child node under /sys/bus/pci/devices/:xx:xx.x/ Signed-off-by: Alexander Stein alexander.st...@systec-electronic.com Thanks, patch applied! Yours, Linus Walleij -- 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/
Re: [PATCH 0/3] ACPI 5 support for GPIO, SPI and I2C
On Sat, Nov 3, 2012 at 8:46 AM, Mika Westerberg mika.westerb...@linux.intel.com wrote: With ACPI 5 we can now describe how devices are connected to their bus using new resources: SPISerialBus and I2CSerialBus. Also it is now possible to add GPIO connections for the devices with the help of GpioIO and GpioInt resources. This series adds support for these new resources. I would very much like Grant to review and merge these patches, since he is way more familiar with these concepts than me, but I'll have a quick glance for syntax and semantics... Yours, Linus Walleij -- 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/
Re: [PATCH] pinctrl: exynos: Add terminating entry for of_device_id table
On Fri, Nov 2, 2012 at 2:46 PM, Axel Lin axel@ingics.com wrote: The of_device_id table is supposed to be zero-terminated. Signed-off-by: Axel Lin axel@ingics.com Thanks Axel, patch applied. Yours, Linus Walleij -- 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/
Re: [PATCH] pinctrl: nomadik: Add terminating entry for platform_device_id table
On Sun, Nov 4, 2012 at 4:30 PM, Axel Lin axel@ingics.com wrote: The platform_device_id table is supposed to be zero-terminated. Signed-off-by: Axel Lin axel@ingics.com Thanks Axel, patch applied. Yours, Linus Walleij -- 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/
Re: [PATCH 1/3] gpio / ACPI: add ACPI support
On Sat, Nov 3, 2012 at 8:46 AM, Mika Westerberg mika.westerb...@linux.intel.com wrote: +/** + * acpi_get_gpio() - Translate ACPI GPIO pin to GPIO number usable with GPIO API + * @path: ACPI GPIO controller full path name, (e.g. \\_SB.GPO1) + * @pin: ACPI GPIO pin number (0-based, controller-relative) + * + * Returns GPIO number to use with Linux generic GPIO API, or errno error value + */ So by just looking at that we can see that this is yet another instance of papering over the fact that the Linux GPIO numbers are global to the kernel and not per-chip, as would be preferred. Can you please contribute to the parallel discussion on how to get rid of the global GPIO numberspace, thread named: How about a gpio_get(device *, char *) function? Yours, Linus Walleij -- 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/
Re: How about a gpio_get(device *, char *) function?
On Mon, Nov 5, 2012 at 8:31 AM, Alex Courbot acour...@nvidia.com wrote: Interesting. I see you already gave the whole thing a thought. What I don't understand however is what is so wrong with the current GPIO numberspace that you want to replace it? Whether we use simple integers or blind pointers, the adressable space will basically remain the same. GPIO numbers can actually be considered as handles, and actually I would not mind typedef'ing int to a GPIO handle type in order to add more opacity to the framework. So the basic problem is scalability and multiplatform support. Currently we have a global roof on GPIO numbers, ARCH_NR_GPIOS, which is helpfully set to 256 in include/asm-generic/gpio.h. In the embedded case ARCH is a particular board for a particular SoC so this number is actually roof:ed for some arbitrary combination of SoC + electronics, at compile time, for that very machine. So what happens when we try to achieve multiplatform support in the ARM tree (same for others, I expect ACPI etc to run into the same problem) is that this has to be set to some arbitrarily high value so that all GPIOs will fit, and they are not sparse, so if you're just using GPIOs 0 .. 15 and 199935..20 you will need to reserve 20 * sizeof(int) bytes. So to avoid this, arch maintainers try with different clever macros to pack the GPIO number assignments per board downward to begin at 0 and keep the array at minimum size, and do not dynamically grab GPIO numbers as they like. Contrast this to IRQs which are in the CONFIG_SPARSE_IRQ case stored in a radix tree and IRQs can be sparse like this without wasting much memory. So IRQdomains can for example just grab a number of free IRQ descriptors and the actual numbers returned do not matter. Using static number assignments also has the side effect that developers will just int mygpio = 42; directly in the code to cut corners and invites such laziness instead of allocating and propagating resources. With abstract handles this does not happen. So moving forward we need a dynamic, radix tree of sparse GPIOs, and we need to follow the design pattern of IRQ descriptors to keep the kernel consistent, for example pinctrl does this with pins - these are sparse and dynamically allocated. They do not have integer numbers other than locally for a particular pin controller. Also the current DT bindings will likely continue to require the legacy API anyway, so I am not sure we can make it go away. We can make any DT business go away as long as all DTS files are maintained in the kernel and especially if their structure is not future-proof. The DT contract to make bindings stable and everlasting is being broken every day as things are right now. Not that I know the DT stuff by heart, but if the way we're doing GPIO in DT is requiring the OS to use a certain type of static array implementation it must be wrong since DT is supposed to be OS-neutral. My initial thought was to build something on top of the existing scheme to address my immediate needs - what you are talking about is much more scary. :) Could you elaborate on your motivations for such a radical direction? See above. I suspect Grant have similar thoughts, but let's see... Yours, Linus Walleij -- 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/
Re: [PATCH 2/2] Input: nomadik-ske-keypad - start using the apb_pclk
On Mon, Nov 5, 2012 at 1:25 PM, Ulf Hansson ulf.hans...@linaro.org wrote: On 4 November 2012 19:12, Linus Walleij linus.wall...@linaro.org wrote: If I understand correctly, the only real solution is to implement the PM domains and have these enable the clocks. Agree. Although, since the pm_domain not yet exist, this as a way forward for now - to fix what is broken. True. Fixing regressions is more important. Acked-by: Linus Walleij linus.wall...@linaro.org When the pm_domain is in place and when we decide to fold in the clock handling in there, we can move it. OK for me. An alternative may be to move this driver over to the AMBA bus, because I think this device actually has primecell registers. Then the bus will take care of the pclk for starters. You are definitely right, this driver can be converted into using the AMBA bus. Although, do you think that should be done _instead_ of going ahead with this patch or do you want that to be handled as a next and a separate step? No, just a suggestion of possibilities... Yours, Linus Walleij -- 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/
Re: [PATCH 1/3] gpio / ACPI: add ACPI support
On Mon, Nov 5, 2012 at 1:46 PM, Rafael J. Wysocki r...@sisk.pl wrote: On Monday, November 05, 2012 02:14:21 PM Mathias Nyman wrote: per-chip based numbering sounds saner, but this deals with what we currently have. And we need something to hook up drivers to right now. So speaking of it I have these drivers consuming pinctrl that I need to hook up right now and the subsystem maintainers are NACKing the patches because they want a centralized solution instead of per-driver hooks and I get the red light... Do you think they will change their mind and give me green light if I tell them I just need to do it right now? ;-) I suggest to take it easy and prolong the deadline, but none of my business primarily, you will have to go through Grant who is probably way more reasonable than me. Yours, Linus Walleij -- 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/
Re: [PATCH 2/3] spi / ACPI: add ACPI enumeration support
On Mon, Nov 5, 2012 at 2:15 PM, Mika Westerberg mika.westerb...@linux.intel.com wrote: On Mon, Nov 05, 2012 at 01:59:23PM +0100, Rafael J. Wysocki wrote: On Monday, November 05, 2012 01:23:50 PM Jean Delvare wrote: On Mon, 5 Nov 2012 14:02:19 +0200, Mika Westerberg wrote: On Mon, Nov 05, 2012 at 11:56:39AM +0100, Mark Brown wrote: (...) Yeah, I just went through DSDT table of one of our machines and found a device that actually has two I2CSerialBus connectors (and those are to the same controller). What I'm not sure is that is it used to select between two different addresses or doest the device really have two physical I2C connections. Neither would make sense from a hardware perspective. Well, interesting. :-) It looks like some PMICs for example have two I2C control interfaces, like TI's TWL6030 if I'm not mistaken. If both are put behind the same I2C controller with different address, you have the situation like above. This is quite common. So for example the AB8500 (drivers/mfd/ab8500-core.c) has this. The reason is usually that the device has more than 256 registers to the address space simply is not big enough. What we do is refer to the subaddresses as banks and they happen to always be at the next consecutive address so n+1. So the same device appear behind several addresses just to support a lot of registers. So you're not actually modelling the devices on the other end but the multiple addresses of a single device. If the addresses are consecutive like ours it makes sense to just instantiate one device and have the driver know that it will also be able to access address +1, +2 ... +n. So is it possible to group the consecutive related addresses after each other here at the acpi-spi level and just create a single device? If the addresses are not consecutive I guess you need to have one device driver bind to several devices and return -EPROBE_DEFER until the driver has been probed for all of them or something like that, this is what multi-block GPIO drivers sometimes do. Yours, Linus Walleij -- 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/
Re: [PATCH 1/3] gpio / ACPI: add ACPI support
On Mon, Nov 5, 2012 at 2:19 PM, Rafael J. Wysocki r...@sisk.pl wrote: On Monday, November 05, 2012 02:11:03 PM Linus Walleij wrote: Do you think they will change their mind and give me green light if I tell them I just need to do it right now? ;-) Well, it's just a matter of fairness to me, actually. If you allowed somebody to do something in the past, it's simply unfair to forbid someone else to do a similar thing later, isn't it? So if some subsystem has previously merged clk_get() for the silicon clock pertaining to a driver the maintainer should be fair to the pinctrl people and merge pinctrl_get() as well. Well, they don't. But it wasn't any of your subsystems so I don't blame you. I have been stand-in-maintaining the GPIO subsystem for the last two merge windows and I am worrying about the long term viability of the subsystem if we keep doing this without facing the real problem of the global GPIO numberspace. But since Grant merged the gpiolib-of.c thing and is still the main maintainer I can atleast chicken out by referring to him this time ... hit me back if he doesn't respond and I will have to refactor the world or something. Yours, Linus Walleij -- 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/
Re: [PATCH] pinctrl: at91: Staticize non-exported symbols
On Mon, Nov 5, 2012 at 2:23 PM, Axel Lin axel@ingics.com wrote: Signed-off-by: Axel Lin axel@ingics.com Merged to my AT91 branch, unless Jean-Christophe says something... Thanks! Linus Walleij -- 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/
Re: [PATCH 1/3] gpio / ACPI: add ACPI support
On Mon, Nov 5, 2012 at 2:50 PM, Rafael J. Wysocki r...@sisk.pl wrote: On Monday, November 05, 2012 02:28:45 PM Linus Walleij wrote: But since Grant merged the gpiolib-of.c thing and is still the main maintainer I can atleast chicken out by referring to him this time ... hit me back if he doesn't respond and I will have to refactor the world or something. No need to be grumpy. :-) Just a little bit ... sorry. I forgot to mention that we want to hook up _existing_ drivers to those things, and they already use the global GPIO numbers, don't they? Yes they do, usually this is either passed from the platform using platform data or handled by device tree lookups to individual drivers. So you will have to modify each such existing driver to do ACPI probe akin to the DT codepath and call acpi_get_gpio() on every pin they need going forward. But that is the plan I guess. Yours, Linus Walleij -- 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/
Re: [PATCH 2/3] spi / ACPI: add ACPI enumeration support
On Mon, Nov 5, 2012 at 3:03 PM, Jean Delvare kh...@linux-fr.org wrote: (...) If the addresses are consecutive like ours it makes sense to just instantiate one device and have the driver know that it will also be able to access address +1, +2 ... +n. So is it possible to group the consecutive related addresses after each other here at the acpi-spi level and just create a single device? We were actually discussing I2C here, although I admit not in the right thread, and maybe some of this applies to SPI as well. This is my typo. The AB8500 example is indeed I2C. So if multiple slave addresses must be supported, please do not limit this support to consecutive addresses. I buy that ... in Nomadik we even have an instance of a single device with two discrete *physical* interfaces connected to it, on both of them it responds to the same address, but with totally different register layouts ... hardware engineers rule! :-) There really is no reason to limit us that way anyway, as i2c-core supports attaching any additional slave address to en existing i2c_client using i2c_new_dummy(). Yes, this is how we do it in AB8500. We just take advantage of the fact that the addresses are consecutive. Note for DDR3 DIMM SPD chips we currently have two different drivers handling the two slave addresses (eeprom/at24 and jc42.) I don't know if this is something that could be instantiated from ACPI. So it seems we really have two different cases when a single chip replies to multiple slave addresses: either they refer to the same function and we want a single driver for all of them, or they are for unrelated functions and we want separate drivers (and thus separate i2c_clients.) Not sure how we can always handle that properly on the ACPI side. If they can be accessed concurrently and do not share any locks or code they should be separate I think? When they have strong dependencies we immediately sort of fall into the MFD class where you need a mediator I think. If the addresses are not consecutive I guess you need to have one device driver bind to several devices and return -EPROBE_DEFER until the driver has been probed for all of them or something like that, this is what multi-block GPIO drivers sometimes do. Consecutive or not makes no difference really, as long as the driver can deduce the additional addresses from the main address or register contents accessible from the main address. This has always been the case so far. You're right. Yours, Linus Walleij -- 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/
Re: [PATCH] pinctrl: u300: Staticize non-exported symbols
On Mon, Nov 5, 2012 at 2:42 PM, Axel Lin axel@ingics.com wrote: Staticize u300_pin_config_get() and u300_pin_config_set() functions. Signed-off-by: Axel Lin axel@ingics.com OK it still builds after this, patch applied! Thanks! Linus Walleij -- 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/
Re: [PATCH v3 3/5] gpio: add viperboard gpio driver
On Mon, Nov 5, 2012 at 3:48 PM, Lars Poeschel la...@wh2.tu-dresden.de wrote: From: Lars Poeschel poesc...@lemonage.de This adds the mfd cell to use the gpio a and gpio b part of the Nano River Technologies viperboard. Signed-off-by: Lars Poeschel poesc...@lemonage.de OK I think we can maintain this. Reviewed-by: Linus Walleij linus.wall...@linaro.org Please get Sam to merge this through his MFD tree. Yours, Linus Walleij -- 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/
Re: [PATCH] pinctrl: sirf: Staticize non-exported symbol
On Mon, Nov 5, 2012 at 2:44 PM, Axel Lin axel@ingics.com wrote: Staticize sirfsoc_gpio_irq_map() function. Signed-off-by: Axel Lin axel@ingics.com Applied with Barry's ACK, thanks! Linus Walleij -- 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/
Re: [PATCH v2 1/2] mailbox: OMAP: introduce mailbox framework
On Tue, Nov 6, 2012 at 4:40 AM, Stephen Warren swar...@wwwdotorg.org wrote: Is this a public interface to the driver? If so, shouldn't the header be in include/linux somewhere? I think the split out of the public header is done in patch 2/2. Yours, Linus Walleij -- 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/
Re: [PATCH v2] pinctrl: samsung: Add ifdef CONFIG_OF_GPIO guard for gc-of_node
On Tue, Nov 6, 2012 at 6:03 AM, Thomas Abraham thomas.abra...@linaro.org wrote: On 5 November 2012 21:14, Axel Lin axel@ingics.com wrote: +#if defined(CONFIG_OF_GPIO) gc-of_node = bank-of_node; +#endif The samsung pinctrl driver supports only device tree enabled platforms. So instead of adding the above #if, would it not be better to add 'select OF_GPIO' for Samsung pinctrl driver in drivers/pinctrl/Kconfig. ? Or edit the Samsung driver to depend on OF_GPIO, this is probably better. Yours, Linus Walleij -- 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/
Re: [PATCH] pinctrl: PINCTRL_SAMSUNG and PINCTRL_EXYNOS4 need to depend on OF GPIOLIB
On Tue, Nov 6, 2012 at 8:04 AM, Axel Lin axel@ingics.com wrote: This patch fixes below build error when !CONFIG_OF_GPIO. CC drivers/pinctrl/pinctrl-samsung.o drivers/pinctrl/pinctrl-samsung.c: In function 'samsung_pinctrl_parse_dt_pins': drivers/pinctrl/pinctrl-samsung.c:557:19: warning: unused variable 'prop' [-Wunused-variable] drivers/pinctrl/pinctrl-samsung.c: In function 'samsung_gpiolib_register': drivers/pinctrl/pinctrl-samsung.c:797:5: error: 'struct gpio_chip' has no member named 'of_node' make[2]: *** [drivers/pinctrl/pinctrl-samsung.o] Error 1 make[1]: *** [drivers/pinctrl] Error 2 make: *** [drivers] Error 2 The samsung pinctrl driver supports only device tree enabled platforms. Thus make PINCTRL_SAMSUNG depend on OF GPIOLIB. The reason to depend on GPIOLIB is CONFIG_OF_GPIO only available when GPIOLIB is selected. Since PINCTRL_EXYNOS4 select PINCTRL_SAMSUNG, thus also make PINCTRL_EXYNOS4 depend on OF GPIOLIB. Signed-off-by: Axel Lin axel@ingics.com This looks good (maybe it should depend on OF_GPIO but who cares). Patch applied to my fixes branch after som patch -p1 foo.patch mockery. Yours, Linus Walleij -- 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/
Re: [PATCH 1/3] gpio / ACPI: add ACPI support
On Tue, Nov 6, 2012 at 10:39 AM, Mika Westerberg mika.westerb...@linux.intel.com wrote: On Mon, Nov 05, 2012 at 03:40:14PM +0100, Linus Walleij wrote: I forgot to mention that we want to hook up _existing_ drivers to those things, and they already use the global GPIO numbers, don't they? Yes they do, usually this is either passed from the platform using platform data or handled by device tree lookups to individual drivers. So you will have to modify each such existing driver to do ACPI probe akin to the DT codepath and call acpi_get_gpio() on every pin they need going forward. But that is the plan I guess. Yes, that's the plan. Do you think it is OK to go with this implementation (acpi_get_gpio()) for now? We will try to make sure that the gpio_get() (or whatever it will be called that time) supports ACPI as well. Yes I'll be OK with it but I don't dare to merge it unless Grant ACKs it. Yours, Linus Walleij -- 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/
Re: [PATCH] ARM: dts: add missing ux500 device trees
On Tue, Nov 6, 2012 at 2:51 PM, Fabio Baltieri fabio.balti...@linaro.org wrote: This adds hrefprev60, hrefv60plus and ccu9540 to device trees compiled during build. Cc: Linus Walleij linus.wall...@linaro.org Acked-by: Lee Jones lee.jo...@linaro.org Signed-off-by: Fabio Baltieri fabio.balti...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- 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/
[PATCH 1/5] gpiolib: fix up function prototypes etc
From: Linus Walleij linus.wall...@linaro.org Commit 69e1601bca88809dc118abd1becb02c15a02ec71 gpiolib: provide provision to register pin ranges Got most of it's function prototypes wrong, so fix this up by: - Moving the void declarations into static inlines in linux/gpio.h (previously the actual prototypes were declared here...) - Declare the gpiochip_add_pin_range() and gpiochip_remove_pin_ranges() functions in asm-generic/gpio.h together with the pin range struct declaration itself. - Actually only implement these very functions in gpiolib.c if CONFIG_PINCTRL is set. - Additionally export the symbols since modules will need to be able to do this. Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/gpio/gpiolib.c | 10 +- include/asm-generic/gpio.h | 6 ++ include/linux/gpio.h | 24 ++-- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 3e84796..10fc9c3 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1183,6 +1183,7 @@ struct gpio_chip *gpiochip_find(void *data, EXPORT_SYMBOL_GPL(gpiochip_find); #ifdef CONFIG_PINCTRL + void gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, unsigned int pin_base, unsigned int npins) { @@ -1204,6 +1205,7 @@ void gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, list_add_tail(pin_range-node, chip-pin_ranges); } +EXPORT_SYMBOL_GPL(gpiochip_add_pin_range); void gpiochip_remove_pin_ranges(struct gpio_chip *chip) { @@ -1215,11 +1217,9 @@ void gpiochip_remove_pin_ranges(struct gpio_chip *chip) pin_range-range); } } -#else -void gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, - unsigned int pin_base, unsigned int npins) {} -void gpiochip_remove_pin_ranges(struct gpio_chip *chip) {} -#endif +EXPORT_SYMBOL_GPL(gpiochip_remove_pin_ranges); + +#endif /* CONFIG_PINCTRL */ /* These optional allocation calls help prevent drivers from stomping * on each other, and help provide better diagnostics in debugfs. diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 92e5c43..2e60de4 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -49,6 +49,7 @@ struct module; struct device_node; #ifdef CONFIG_PINCTRL + /** * struct gpio_pin_range - pin range controlled by a gpio chip * @head: list for maintaining set of pin ranges, used internally @@ -61,6 +62,11 @@ struct gpio_pin_range { struct pinctrl_dev *pctldev; struct pinctrl_gpio_range range; }; + +void gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, + unsigned int pin_base, unsigned int npins); +void gpiochip_remove_pin_ranges(struct gpio_chip *chip); + #endif /** diff --git a/include/linux/gpio.h b/include/linux/gpio.h index a284459..21d28b9 100644 --- a/include/linux/gpio.h +++ b/include/linux/gpio.h @@ -72,9 +72,9 @@ static inline int irq_to_gpio(unsigned int irq) return -EINVAL; } -#endif +#endif /* ! CONFIG_ARCH_HAVE_CUSTOM_GPIO_H */ -#else +#else /* ! CONFIG_GENERIC_GPIO */ #include linux/kernel.h #include linux/types.h @@ -231,9 +231,21 @@ static inline int irq_to_gpio(unsigned irq) return -EINVAL; } -void gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, - unsigned int pin_base, unsigned int npins); -void gpiochip_remove_pin_ranges(struct gpio_chip *chip); -#endif +#ifdef CONFIG_PINCTRL + +static inline void +gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, + unsigned int pin_base, unsigned int npins) +{ +} + +static inline void +gpiochip_remove_pin_ranges(struct gpio_chip *chip) +{ +} + +#endif /* CONFIG_PINCTRL */ + +#endif /* ! CONFIG_GENERIC_GPIO */ #endif /* __LINUX_GPIO_H */ -- 1.7.11.3 -- 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/
[PATCH 2/5] gpiolib-of: staticize the pin range calls
From: Linus Walleij linus.wall...@linaro.org Commit 69e1601bca88809dc118abd1becb02c15a02ec71 gpiolib: provide provision to register pin ranges Declared the of_gpiochip_[add|remove]_pin_range() global while they should be static as they are only ever used in this file. Let's convert them to static. Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/gpio/gpiolib-of.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index a5b90c8..220caa5 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -218,7 +218,7 @@ err0: EXPORT_SYMBOL(of_mm_gpiochip_add); #ifdef CONFIG_PINCTRL -void of_gpiochip_add_pin_range(struct gpio_chip *chip) +static void of_gpiochip_add_pin_range(struct gpio_chip *chip) { struct device_node *np = chip-of_node; struct gpio_pin_range *pin_range; @@ -254,7 +254,7 @@ void of_gpiochip_add_pin_range(struct gpio_chip *chip) } while (index++); } -void of_gpiochip_remove_pin_range(struct gpio_chip *chip) +static void of_gpiochip_remove_pin_range(struct gpio_chip *chip) { struct gpio_pin_range *pin_range, *tmp; @@ -265,8 +265,8 @@ void of_gpiochip_remove_pin_range(struct gpio_chip *chip) } } #else -void of_gpiochip_add_pin_range(struct gpio_chip *chip) {} -void of_gpiochip_remove_pin_range(struct gpio_chip *chip) {} +static void of_gpiochip_add_pin_range(struct gpio_chip *chip) {} +static void of_gpiochip_remove_pin_range(struct gpio_chip *chip) {} #endif void of_gpiochip_add(struct gpio_chip *chip) -- 1.7.11.3 -- 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/
[PATCH 3/5] gpiolib: remove duplicate pin range code
From: Linus Walleij linus.wall...@linaro.org Commit 69e1601bca88809dc118abd1becb02c15a02ec71 gpiolib: provide provision to register pin ranges Introduced both of_gpiochip_remove_pin_range() and gpiochip_remove_pin_ranges(). But the contents are exactly the same so remove the OF one and rely on the range deletion in the core. Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/gpio/gpiolib-of.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index 220caa5..67403e4 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -254,19 +254,8 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip) } while (index++); } -static void of_gpiochip_remove_pin_range(struct gpio_chip *chip) -{ - struct gpio_pin_range *pin_range, *tmp; - - list_for_each_entry_safe(pin_range, tmp, chip-pin_ranges, node) { - list_del(pin_range-node); - pinctrl_remove_gpio_range(pin_range-pctldev, - pin_range-range); - } -} #else static void of_gpiochip_add_pin_range(struct gpio_chip *chip) {} -static void of_gpiochip_remove_pin_range(struct gpio_chip *chip) {} #endif void of_gpiochip_add(struct gpio_chip *chip) @@ -288,7 +277,7 @@ void of_gpiochip_add(struct gpio_chip *chip) void of_gpiochip_remove(struct gpio_chip *chip) { - of_gpiochip_remove_pin_range(chip); + gpiochip_remove_pin_ranges(chip); if (chip-of_node) of_node_put(chip-of_node); -- 1.7.11.3 -- 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/
[PATCH 4/5] gpiolib: call pin removal in chip removal function
From: Linus Walleij linus.wall...@linaro.org This makes us call gpiochio_remove_pin_ranges() in the gpiochip_remove() function, so we get rid of ranges when freeing the chip. Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/gpio/gpiolib.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 10fc9c3..fd7280f 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1125,6 +1125,7 @@ int gpiochip_remove(struct gpio_chip *chip) spin_lock_irqsave(gpio_lock, flags); + gpiochip_remove_pin_ranges(chip); of_gpiochip_remove(chip); for (id = chip-base; id chip-base + chip-ngpio; id++) { -- 1.7.11.3 -- 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/
[PATCH 5/5] gpiolib: separation of pin concerns
From: Linus Walleij linus.wall...@linaro.org The fact that of_gpiochip_add_pin_range() and gpiochip_add_pin_range() share too much code is fragile and will invariably mean that bugs need to be fixed in two places instead of one. So separate the concerns of gpiolib.c and gpiolib-of.c and have the latter call the former as back-end. This is necessary also when going forward with other device descriptions such as ACPI. This is done by: - Adding a return code to gpiochip_add_pin_range() so we can reliably check whether this succeeds. - Get rid of the custom of_pinctrl_add_gpio_range() from pinctrl. Instead create of_pinctrl_get() to just retrive the pin controller per se from an OF node. This composite function was just begging to be deleted, it was way to purpose-specific. - Use pinctrl_dev_get_name() to get the name of the retrieved pin controller and use that to call back into the generic gpiochip_add_pin_range(). Now the pin range is only allocated and tied to a pin controller from the core implementation in gpiolib.c. Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/gpio/gpiolib-of.c | 23 +-- drivers/gpio/gpiolib.c | 8 +--- drivers/pinctrl/devicetree.c| 4 +--- include/asm-generic/gpio.h | 4 ++-- include/linux/gpio.h| 2 +- include/linux/pinctrl/pinctrl.h | 7 ++- 6 files changed, 20 insertions(+), 28 deletions(-) diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index 67403e4..a40cd84 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -221,8 +221,8 @@ EXPORT_SYMBOL(of_mm_gpiochip_add); static void of_gpiochip_add_pin_range(struct gpio_chip *chip) { struct device_node *np = chip-of_node; - struct gpio_pin_range *pin_range; struct of_phandle_args pinspec; + struct pinctrl_dev *pctldev; int index = 0, ret; if (!np) @@ -234,22 +234,17 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip) if (ret) break; - pin_range = devm_kzalloc(chip-dev, sizeof(*pin_range), - GFP_KERNEL); - if (!pin_range) { - pr_err(%s: GPIO chip: failed to allocate pin ranges\n, - chip-label); + pctldev = of_pinctrl_get(pinspec.np); + if (!pctldev) break; - } - pin_range-range.name = chip-label; - pin_range-range.base = chip-base; - pin_range-range.pin_base = pinspec.args[0]; - pin_range-range.npins = pinspec.args[1]; - pin_range-pctldev = of_pinctrl_add_gpio_range(pinspec.np, - pin_range-range); + ret = gpiochip_add_pin_range(chip, +pinctrl_dev_get_name(pctldev), +pinspec.args[0], +pinspec.args[1]); - list_add_tail(pin_range-node, chip-pin_ranges); + if (ret) + break; } while (index++); } diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index fd7280f..addbabb 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1185,8 +1185,8 @@ EXPORT_SYMBOL_GPL(gpiochip_find); #ifdef CONFIG_PINCTRL -void gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, - unsigned int pin_base, unsigned int npins) +int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, + unsigned int pin_base, unsigned int npins) { struct gpio_pin_range *pin_range; @@ -1194,7 +1194,7 @@ void gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, if (!pin_range) { pr_err(%s: GPIO chip: failed to allocate pin ranges\n, chip-label); - return; + return -ENOMEM; } pin_range-range.name = chip-label; @@ -1205,6 +1205,8 @@ void gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, pin_range-range); list_add_tail(pin_range-node, chip-pin_ranges); + + return 0; } EXPORT_SYMBOL_GPL(gpiochip_add_pin_range); diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c index 6728ec7..fe2d1af 100644 --- a/drivers/pinctrl/devicetree.c +++ b/drivers/pinctrl/devicetree.c @@ -106,8 +106,7 @@ static struct pinctrl_dev *find_pinctrl_by_of_node(struct device_node *np) return NULL; } -struct pinctrl_dev *of_pinctrl_add_gpio_range(struct device_node *np, - struct pinctrl_gpio_range *range) +struct pinctrl_dev *of_pinctrl_get(struct device_node *np) { struct pinctrl_dev *pctldev; @@ -115,7 +114,6 @@ struct pinctrl_dev
[PATCH] gpiolib: iron out include ladder mistakes
From: Linus Walleij linus.wall...@linaro.org The */gpio.h includes are updated again: now we need to account for the problem introduced by commit: 595679a8038584df7b9398bf34f61db3c038bfea gpiolib: fix up function prototypes etc Actually we need static inlines in include/asm-generic/gpio.h as well since we may have GPIOLIB but not PINCTRL. And we need to keep the static inlines in linux/gpio.h but here for the !CONFIG_GENERIC_GPIO case, and then we may as well throw in a few warnings like the other prototypes there, if someone would have the bad taste of compiling without GENERIC_GPIO even. Signed-off-by: Linus Walleij linus.wall...@linaro.org --- include/asm-generic/gpio.h | 13 + include/linux/gpio.h | 7 +++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 50d995e..b546edb 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -67,6 +67,19 @@ int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, unsigned int pin_base, unsigned int npins); void gpiochip_remove_pin_ranges(struct gpio_chip *chip); +#else + +static inline int +gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, + unsigned int pin_base, unsigned int npins) +{ +} + +static inline void +gpiochip_remove_pin_ranges(struct gpio_chip *chip) +{ +} + #endif /** diff --git a/include/linux/gpio.h b/include/linux/gpio.h index 81bbfe5..7ba2762 100644 --- a/include/linux/gpio.h +++ b/include/linux/gpio.h @@ -231,21 +231,20 @@ static inline int irq_to_gpio(unsigned irq) return -EINVAL; } -#ifdef CONFIG_PINCTRL - static inline int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, unsigned int pin_base, unsigned int npins) { + WARN_ON(1); + return -EINVAL; } static inline void gpiochip_remove_pin_ranges(struct gpio_chip *chip) { + WARN_ON(1); } -#endif /* CONFIG_PINCTRL */ - #endif /* ! CONFIG_GENERIC_GPIO */ #endif /* __LINUX_GPIO_H */ -- 1.7.11.3 -- 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/
Re: How about a gpio_get(device *, char *) function?
On Tue, Nov 6, 2012 at 2:33 AM, Alex Courbot acour...@nvidia.com wrote: How about, in a first time (and because I'd also like to get the power seqs moving on), a typedef from int to gpio_handle_t and a first implementation of the gpio_handle_*() API that would just call the existing integer-based API (apart from gpio_handle_get())? That way things will not break when we switch to a real handle. I'm afraid of typedef:ing gpio_handle_t to int because it sort of encourages non-handlers to be used mixed with the old integers. I would prefer to create, e.g. in linux/gpio/consumer.h something like: struct gpio; struct gpio *gpio_get(struct device *dev, const char *name); int gpio_get_value(struct gpio *g); Nothing more! I.e. struct gpio is an opaque cookie, nothing to be known about it. And then have the drivers using this *ONLY* include linux/gpio/consumer.h and not linux/gpio.h. So drivers have no clue what struct gpio is and will only operate on it using functions. This follows the pattern set by Thomas Gleixner for e.g. IRQ descriptors, and the style was also used in the redesign of the struct clk *. Of course the cross-referencing implementation can in e.g. drivers/gpio/gpiodev.c internally define that like this: #include linux/gpio.h /** * @gpio: pointer to global GPIO number */ struct gpio { int gpio; }; struct gpio *gpio_get(struct device *dev, const char *name) { /* Lookup in cross-ref table etc */ } int gpioh_get_value(struct gpio *g) { return gpio_get_value(g-gpio); } (...) Then we can work from there. I think it adds the proper opaqueness factor. I don't really like the gpioh_* prefix instead of just gpio_* but I guess there is not polymorphism to exploit as transition path here :-P Yours, Linus Walleij -- 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/
Re: How about a gpio_get(device *, char *) function?
On Mon, Nov 5, 2012 at 6:35 PM, Stephen Warren swar...@wwwdotorg.org wrote: [Me] gpio_get() should get an abstract handle just like clk_get() or regulator_get(), not a fixed numeral. I don't really see why the return type of gpio_get() influences whether it can be implemented or not. It doesn't influence that, but I want to follow the opaqueness design pattern from irq descriptors and struct clk. With board files, some gpio map table would simply contain the same int GPIO ID value the table as is used anywhere else already. With DT, the same xlate function would translate from DT GPIO-chip-relative IDs/specifiers into the global number space in the same way that we do today via other APIs. Yes, this part I buy into, just want to see how we can move forward from there. The coplete nightmare is to introduce something into DT that nails down a global GPIO numberspace... but I think that is not the case atleast. If the GPIO subsystem were reworked as you propose, this API could be reworked in exactly the same way, or if implemented after the rework, it would return whatever handle type was in use at the time. Yes, I just think we should return an opaque struct from day 1, so just a little, little bit more to shield us. Yours, Linus Walleij -- 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/
[GIT PULL] pinctrl fixes for v3.7 rc:s
Hi Linus, here is a set of pinctrl fixes for the current -rc series. Details in the signed tag. Please pull it in! Yours, Linus Walleij The following changes since commit 8f0d8163b50e01f398b14bcd4dc039ac5ab18d64: Linux 3.7-rc3 (2012-10-28 12:24:48 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git tags/pinctrl-for-v3.7-rc5 for you to fetch changes up to 924da31416f20a8ee7a9008dd4e6e6054bc36b1b: pinctrl: samsung and exynos need to depend on OF GPIOLIB (2012-11-06 10:02:14 +0100) Some pinctrl fixes for the v3.7 series: - A set of SPEAr pinctrl fixes that recently arrived - A fixup for the Samsung/Exynos Kconfig deps Axel Lin (1): pinctrl: samsung and exynos need to depend on OF GPIOLIB Deepak Sikri (2): pinctrl: SPEAr320: Correct pad mux entries for rmii/smii pinctrl: SPEAr1340: Make DDR reset clock pads as gpio Shiraz Hashim (3): pinctrl: SPEAr3xx: correct register space to configure pwm pinctrl: SPEAr1310: fix clcd high resolution pin group name pinctrl: SPEAr1310: add register entries for enabling pad direction Vipul Kumar Samar (3): pinctrl: SPEAr1310: Fix value of PERIP_CFG reigster and MCIF_SEL_SHIFT pinctrl: SPEAr1310: Separate out pci pins from pcie_sata pin group pinctrl: SPEAr1340: Add clcd sleep mode pin configuration Viresh Kumar (1): pinctrl: SPEAr: Don't update all non muxreg bits on pinctrl_disable drivers/pinctrl/Kconfig | 2 + drivers/pinctrl/spear/pinctrl-spear.c | 2 +- drivers/pinctrl/spear/pinctrl-spear1310.c | 365 ++ drivers/pinctrl/spear/pinctrl-spear1340.c | 41 +++- drivers/pinctrl/spear/pinctrl-spear320.c | 8 +- drivers/pinctrl/spear/pinctrl-spear3xx.h | 1 + 6 files changed, 369 insertions(+), 50 deletions(-) -- 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/
[PATCH 1/2] ARM: ux500: add PRCM register base for pinctrl
From: Jonas Aaberg jonas.ab...@stericsson.com This adds the PRCM register range base as a resource to the pinctrl driver do we can break the dependency to the PRCMU driver and handle these registers in the driver alone. Cc: a...@kernel.org Signed-off-by: Jonas Aaberg jonas.ab...@stericsson.com Signed-off-by: Linus Walleij linus.wall...@linaro.org --- ARM SoC guys: this patch is better contained in the pinctrl tree, can I have your ACK to push it through pinctrl? Thanks. --- arch/arm/mach-ux500/cpu-db8500.c | 2 +- arch/arm/mach-ux500/devices-common.h | 8 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c index 87a8f9f..113d9c4 100644 --- a/arch/arm/mach-ux500/cpu-db8500.c +++ b/arch/arm/mach-ux500/cpu-db8500.c @@ -158,7 +158,7 @@ static void __init db8500_add_gpios(struct device *parent) dbx500_add_gpios(parent, ARRAY_AND_SIZE(db8500_gpio_base), IRQ_DB8500_GPIO0, pdata); - dbx500_add_pinctrl(parent, pinctrl-db8500); + dbx500_add_pinctrl(parent, pinctrl-db8500, U8500_PRCMU_BASE); } static int usb_db8500_rx_dma_cfg[] = { diff --git a/arch/arm/mach-ux500/devices-common.h b/arch/arm/mach-ux500/devices-common.h index 7fbf0ba..96fa4ac 100644 --- a/arch/arm/mach-ux500/devices-common.h +++ b/arch/arm/mach-ux500/devices-common.h @@ -129,12 +129,18 @@ void dbx500_add_gpios(struct device *parent, resource_size_t *base, int num, int irq, struct nmk_gpio_platform_data *pdata); static inline void -dbx500_add_pinctrl(struct device *parent, const char *name) +dbx500_add_pinctrl(struct device *parent, const char *name, + resource_size_t base) { + struct resource res[] = { + DEFINE_RES_MEM(base, SZ_8K), + }; struct platform_device_info pdevinfo = { .parent = parent, .name = name, .id = -1, + .res = res, + .num_res = ARRAY_SIZE(res), }; platform_device_register_full(pdevinfo); -- 1.7.11.3 -- 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/
[PATCH 2/2] pinctrl/nomadik: make independent of prcmu driver
From: Jonas Aaberg jonas.ab...@stericsson.com Currently there are some unnecessary criss-cross dependencies between the PRCMU driver in MFD and a lot of other drivers, mainly because other drivers need to poke around in the PRCM register range. In cases like this there are actually just a few select registers that the pinctrl driver need to read/modify/write, and it turns out that no other driver is actually using these registers, so there are no concurrency issues whatsoever. So: don't let the location of the register range complicate things, just poke into these registers directly and skip a layer of indirection. Cc: Loic Pallardy loic.palla...@st.com Signed-off-by: Jonas Aaberg jonas.ab...@stericsson.com Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/pinctrl/pinctrl-nomadik-db8500.c | 4 +-- drivers/pinctrl/pinctrl-nomadik-db8540.c | 4 +-- drivers/pinctrl/pinctrl-nomadik-stn8815.c | 4 +-- drivers/pinctrl/pinctrl-nomadik.c | 52 ++- drivers/pinctrl/pinctrl-nomadik.h | 14 + 5 files changed, 45 insertions(+), 33 deletions(-) diff --git a/drivers/pinctrl/pinctrl-nomadik-db8500.c b/drivers/pinctrl/pinctrl-nomadik-db8500.c index 6de52e7..e73d75e 100644 --- a/drivers/pinctrl/pinctrl-nomadik-db8500.c +++ b/drivers/pinctrl/pinctrl-nomadik-db8500.c @@ -1230,7 +1230,7 @@ static const u16 db8500_prcm_gpiocr_regs[] = { [PRCM_IDX_GPIOCR2] = 0x574, }; -static const struct nmk_pinctrl_soc_data nmk_db8500_soc = { +static struct nmk_pinctrl_soc_data nmk_db8500_soc = { .gpio_ranges = nmk_db8500_ranges, .gpio_num_ranges = ARRAY_SIZE(nmk_db8500_ranges), .pins = nmk_db8500_pins, @@ -1245,7 +1245,7 @@ static const struct nmk_pinctrl_soc_data nmk_db8500_soc = { }; void __devinit -nmk_pinctrl_db8500_init(const struct nmk_pinctrl_soc_data **soc) +nmk_pinctrl_db8500_init(struct nmk_pinctrl_soc_data **soc) { *soc = nmk_db8500_soc; } diff --git a/drivers/pinctrl/pinctrl-nomadik-db8540.c b/drivers/pinctrl/pinctrl-nomadik-db8540.c index 52fc301..1276ba3 100644 --- a/drivers/pinctrl/pinctrl-nomadik-db8540.c +++ b/drivers/pinctrl/pinctrl-nomadik-db8540.c @@ -1240,7 +1240,7 @@ static const u16 db8540_prcm_gpiocr_regs[] = { [PRCM_IDX_GPIOCR3] = 0x2bc, }; -static const struct nmk_pinctrl_soc_data nmk_db8540_soc = { +static struct nmk_pinctrl_soc_data nmk_db8540_soc = { .gpio_ranges = nmk_db8540_ranges, .gpio_num_ranges = ARRAY_SIZE(nmk_db8540_ranges), .pins = nmk_db8540_pins, @@ -1255,7 +1255,7 @@ static const struct nmk_pinctrl_soc_data nmk_db8540_soc = { }; void __devinit -nmk_pinctrl_db8540_init(const struct nmk_pinctrl_soc_data **soc) +nmk_pinctrl_db8540_init(struct nmk_pinctrl_soc_data **soc) { *soc = nmk_db8540_soc; } diff --git a/drivers/pinctrl/pinctrl-nomadik-stn8815.c b/drivers/pinctrl/pinctrl-nomadik-stn8815.c index 7d432c3..ed5b144 100644 --- a/drivers/pinctrl/pinctrl-nomadik-stn8815.c +++ b/drivers/pinctrl/pinctrl-nomadik-stn8815.c @@ -339,7 +339,7 @@ static const struct nmk_function nmk_stn8815_functions[] = { FUNCTION(i2cusb), }; -static const struct nmk_pinctrl_soc_data nmk_stn8815_soc = { +static struct nmk_pinctrl_soc_data nmk_stn8815_soc = { .gpio_ranges = nmk_stn8815_ranges, .gpio_num_ranges = ARRAY_SIZE(nmk_stn8815_ranges), .pins = nmk_stn8815_pins, @@ -351,7 +351,7 @@ static const struct nmk_pinctrl_soc_data nmk_stn8815_soc = { }; void __devinit -nmk_pinctrl_stn8815_init(const struct nmk_pinctrl_soc_data **soc) +nmk_pinctrl_stn8815_init(struct nmk_pinctrl_soc_data **soc) { *soc = nmk_stn8815_soc; } diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c index 22f6937..33c614e 100644 --- a/drivers/pinctrl/pinctrl-nomadik.c +++ b/drivers/pinctrl/pinctrl-nomadik.c @@ -30,20 +30,6 @@ #include linux/pinctrl/pinconf.h /* Since we request GPIOs from ourself */ #include linux/pinctrl/consumer.h -/* - * For the U8500 archs, use the PRCMU register interface, for the older - * Nomadik, provide some stubs. The functions using these will only be - * called on the U8500 series. - */ -#ifdef CONFIG_ARCH_U8500 -#include linux/mfd/dbx500-prcmu.h -#else -static inline u32 prcmu_read(unsigned int reg) { - return 0; -} -static inline void prcmu_write(unsigned int reg, u32 value) {} -static inline void prcmu_write_masked(unsigned int reg, u32 mask, u32 value) {} -#endif #include linux/platform_data/pinctrl-nomadik.h #include asm/mach/irq.h @@ -85,7 +71,7 @@ struct nmk_gpio_chip { struct nmk_pinctrl { struct device *dev; struct pinctrl_dev *pctl; - const struct nmk_pinctrl_soc_data *soc; + struct nmk_pinctrl_soc_data *soc; }; static struct nmk_gpio_chip * @@ -247,6 +233,15 @@ nmk_gpio_disable_lazy_irq(struct nmk_gpio_chip *nmk_chip, unsigned offset) dev_dbg(nmk_chip-chip.dev, %d: clearing interrupt mask\n, gpio
Re: [PATCH 1/5] gpiolib: fix up function prototypes etc
On Wed, Nov 7, 2012 at 6:09 AM, Viresh Kumar viresh.ku...@linaro.org wrote: On 6 November 2012 20:46, Linus Walleij linus.wall...@stericsson.com -void gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, - unsigned int pin_base, unsigned int npins); -void gpiochip_remove_pin_ranges(struct gpio_chip *chip); -#endif +#ifdef CONFIG_PINCTRL Shouldn't this be ifndef?? Probably, fixed up in later patches... Yours, Linus Walleij -- 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/
Re: [PATCH 3/5] gpiolib: remove duplicate pin range code
On Wed, Nov 7, 2012 at 6:14 AM, viresh kumar viresh.ku...@linaro.org wrote: On Tue, Nov 6, 2012 at 8:46 PM, Linus Walleij linus.wall...@stericsson.com wrote: From: Linus Walleij linus.wall...@linaro.org Commit 69e1601bca88809dc118abd1becb02c15a02ec71 gpiolib: provide provision to register pin ranges Introduced both of_gpiochip_remove_pin_range() and gpiochip_remove_pin_ranges(). But the contents are exactly the same so remove the OF one and rely on the range deletion in the core. Signed-off-by: Linus Walleij linus.wall...@linaro.org I can't believe that i did this :( Don't worry, it's impossible to get these things right. I am trying to fix it properly here and just introduce new bugs for every fix I try to make. Maybe I'll soon have something that actually doesn't break x86... Yours, Linus Walleij -- 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/
Re: [PATCH] gpiolib: iron out include ladder mistakes
On Tue, Nov 6, 2012 at 10:40 PM, Stephen Warren swar...@wwwdotorg.org wrote: On 11/06/2012 09:21 AM, Linus Walleij wrote: And we need to keep the static inlines in linux/gpio.h but here for the !CONFIG_GENERIC_GPIO case, and then we may as well throw in a few warnings like the other prototypes there, if someone would have the bad taste of compiling without GENERIC_GPIO even. Hmm. Is there way to avoid the duplication of the dummy implementations? Having a prototype and a truly dummy implementation in one place, but a WARNing/failing dummy implementation elsewhere, seems like it'll cause issues. Yeah :-/ This is not exactly elegant and is some side effect of the split between CONFIG_GENERIC_GPIO and CONFIG_GPIOLIB, the real fix is to get rid of all GENERIC_GPIO implementations in the kernel and switch everyone over to GPIOLIB. Not that easy though :-( can't think of any nice fix. Does this patch mean the previous series causes git bisect failures? Yeah once I have something that doesn't break x86 I might just squash collapse all of this into the gpioranges patch. Yours, Linus Walleij -- 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/
Re: linux-next: build failure after merge of the final tree (pinctrl tree related)
On Wed, Nov 7, 2012 at 5:48 AM, Stephen Rothwell s...@canb.auug.org.au wrote: Caused by commit e8321df59155 (gpiolib: iron out include ladder mistakes) (and some earlier ones) from the pinctrl tree. I added this patch for today (there may be a better return value from gpiochip_add_pin_range): (..) +struct gpio_chip; + static inline int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, unsigned int pin_base, unsigned int npins) { + return 0; } Bah, I happened to put these prototypes just above the actual definition of struct gpio_chip :-P Fixing up and respinning the patch, I've added the compulsory return value as well. Yours, Linus Walleij -- 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/
[PATCH v2] gpiolib: iron out include ladder mistakes
From: Linus Walleij linus.wall...@linaro.org The */gpio.h includes are updated again: now we need to account for the problem introduced by commit: 595679a8038584df7b9398bf34f61db3c038bfea gpiolib: fix up function prototypes etc Actually we need static inlines in include/asm-generic/gpio.h as well since we may have GPIOLIB but not PINCTRL. Make sure to move all the CONFIG_PINCTRL business to the end of the file so we are sure we have declared struct gpio_chip. And we need to keep the static inlines in linux/gpio.h but here for the !CONFIG_GENERIC_GPIO case, and then we may as well throw in a few warnings like the other prototypes there, if someone would have the bad taste of compiling without GENERIC_GPIO even. Signed-off-by: Linus Walleij linus.wall...@linaro.org --- ChangeLog v1-v2: - Move function prototypes below all other #ifdefs at the bottom of the file so we know we have struct irq_chip defines - Provide a proper return value from the gpiochip_add_pin_range() stub - Thanks to Stephen Rothwell for pointing this out. --- include/asm-generic/gpio.h | 56 +- include/linux/gpio.h | 7 +++--- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 50d995e..2b84fc3 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -48,27 +48,6 @@ struct seq_file; struct module; struct device_node; -#ifdef CONFIG_PINCTRL - -/** - * struct gpio_pin_range - pin range controlled by a gpio chip - * @head: list for maintaining set of pin ranges, used internally - * @pctldev: pinctrl device which handles corresponding pins - * @range: actual range of pins controlled by a gpio controller - */ - -struct gpio_pin_range { - struct list_head node; - struct pinctrl_dev *pctldev; - struct pinctrl_gpio_range range; -}; - -int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, - unsigned int pin_base, unsigned int npins); -void gpiochip_remove_pin_ranges(struct gpio_chip *chip); - -#endif - /** * struct gpio_chip - abstract a GPIO controller * @label: for diagnostics @@ -288,4 +267,39 @@ static inline void gpio_unexport(unsigned gpio) } #endif /* CONFIG_GPIO_SYSFS */ +#ifdef CONFIG_PINCTRL + +/** + * struct gpio_pin_range - pin range controlled by a gpio chip + * @head: list for maintaining set of pin ranges, used internally + * @pctldev: pinctrl device which handles corresponding pins + * @range: actual range of pins controlled by a gpio controller + */ + +struct gpio_pin_range { + struct list_head node; + struct pinctrl_dev *pctldev; + struct pinctrl_gpio_range range; +}; + +int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, + unsigned int pin_base, unsigned int npins); +void gpiochip_remove_pin_ranges(struct gpio_chip *chip); + +#else + +static inline int +gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, + unsigned int pin_base, unsigned int npins) +{ + return 0; +} + +static inline void +gpiochip_remove_pin_ranges(struct gpio_chip *chip) +{ +} + +#endif /* CONFIG_PINCTRL */ + #endif /* _ASM_GENERIC_GPIO_H */ diff --git a/include/linux/gpio.h b/include/linux/gpio.h index 81bbfe5..7ba2762 100644 --- a/include/linux/gpio.h +++ b/include/linux/gpio.h @@ -231,21 +231,20 @@ static inline int irq_to_gpio(unsigned irq) return -EINVAL; } -#ifdef CONFIG_PINCTRL - static inline int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name, unsigned int pin_base, unsigned int npins) { + WARN_ON(1); + return -EINVAL; } static inline void gpiochip_remove_pin_ranges(struct gpio_chip *chip) { + WARN_ON(1); } -#endif /* CONFIG_PINCTRL */ - #endif /* ! CONFIG_GENERIC_GPIO */ #endif /* __LINUX_GPIO_H */ -- 1.7.11.3 -- 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/
Re: [PATCHv2] gpio-mcp23s08: Build I2C support even when CONFIG_I2C=m
On Wed, Nov 7, 2012 at 5:51 AM, Daniel M. Weeks d...@danweeks.net wrote: The driver has both SPI and I2C pieces. The appropriate pieces are built based on whether SPI and/or I2C is/are enabled. However, it was only checking if I2C was built-in, never if it was built as a module. This patch checks for either since building both this driver and I2C as modules is possible. Signed-off-by: Daniel M. Weeks d...@danweeks.net Thanks, patch merged to my fixes branch. Yours, Linus Walleij -- 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/
Re: [PATCH 2/2] gpio: tegra: fix suspend/resume apis
On Wed, Nov 7, 2012 at 4:01 PM, Laxman Dewangan ldewan...@nvidia.com wrote: Following are changes done to fix the suspend/resume functionality of tegra gpio driver: - Protect suspend/resume callbacks with CONFIG_PM_SLEEP because CONFIG_PM doesn't actually enable any of the PM callbacks, it only allows to enable CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME. This means if CONFIG_PM is used to protect system sleep callbacks then it may end up unreferenced if only runtime PM is enabled. - Fix the suspend/resume APIs declaration as per callback prototype. Signed-off-by: Laxman Dewangan ldewan...@nvidia.com OK patch applied to my devel branch, with Stephens ACK and I also fixed the whitespace issue he pointed out. Yours, Linus Walleij -- 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/
Re: [PATCH V2] gpio: tegra: fix suspend/resume apis
On Thu, Nov 8, 2012 at 5:22 AM, Laxman Dewangan ldewan...@nvidia.com wrote: Changes from V1: - It was 2/2/ for prev patch and dropping 1/2. - nit cleanup in change. - Added ack from Stephen as it is acked in 2/2. Oh I didn't see you respun it yourself. Anyway it's applied now. Yours, Linus Walleij -- 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/
Re: [PATCH 1/2] gpio: tegra: Staticize non-exported symbols
On Thu, Nov 8, 2012 at 3:45 AM, Axel Lin axel@ingics.com wrote: Both tegra_gpio_request() and tegra_gpio_free() are not referenced outside of this file, make them static. Signed-off-by: Axel Lin axel@ingics.com Applied with Stephen's ACK. Yours, Linus Walleij -- 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/
Re: [PATCH 2/2] gpio: tegra: Drop exporting static functions
On Thu, Nov 8, 2012 at 3:47 AM, Axel Lin axel@ingics.com wrote: Both tegra_gpio_enable() and tegra_gpio_disable() are static functions, it does not make sense to export them. Signed-off-by: Axel Lin axel@ingics.com Applied with Stephen's ACK. Yours, Linus Walleij -- 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/
Re: [PATCH] gpio: mvebu: Set free callback for gpio_chip
On Thu, Nov 8, 2012 at 3:27 AM, Axel Lin axel@ingics.com wrote: We call pinctrl_request_gpio() in request callback, thus we need to call pinctrl_free_gpio() in free callback. Both mvebu_gpio_request() and mvebu_gpio_free() are not referenced outside of this file, make them static. Signed-off-by: Axel Lin axel@ingics.com Applied with Thomas' ACK, thanks! Yours, Linus Walleij -- 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/
Re: [RFC PATCH] gpiolib: Refactor gpio_export
On Wed, Oct 17, 2012 at 1:41 AM, Ryan Mallon rmal...@gmail.com wrote: The gpio_export function uses nested if statements and the status variable to handle the failure cases. This makes the function logic difficult to follow. Refactor the code to abort immediately on failure using goto. This makes the code slightly longer, but significantly reduces the nesting and number of split lines and makes the code easier to read. Signed-off-by: Ryan Mallon Very good initiative! +++ b/drivers/gpio/gpiolib.c @@ -702,68 +702,74 @@ int gpio_export(unsigned gpio, bool direction_may_change) { unsigned long flags; struct gpio_desc*desc; - int status = -EINVAL; + int status; const char *ioname = NULL; + struct device *dev; /* can't export until sysfs is available ... */ if (!gpio_class.p) { pr_debug(%s: called too early!\n, __func__); - return -ENOENT; + status = -ENOENT; + goto fail; Why bother with all the goto:s here since there are no resources to clean up? Just pr_debug() and return -ENOENT; is good enough. I don't quite see the point. Arguably this should be pr_err() or something BTW, just debug() may hide serious bugs. - if (!gpio_is_valid(gpio)) - goto done; + if (!gpio_is_valid(gpio)) { + status = -EINVAL; + goto fail; + } Why not just pr_err(..); return -EINVAL; ? - if (test_bit(FLAG_REQUESTED, desc-flags) -!test_bit(FLAG_EXPORT, desc-flags)) { - status = 0; - if (!desc-chip-direction_input - || !desc-chip-direction_output) - direction_may_change = false; + if (!test_bit(FLAG_REQUESTED, desc-flags) || +test_bit(FLAG_EXPORT, desc-flags)) { + spin_unlock_irqrestore(gpio_lock, flags); + status = -ENOENT; + goto fail; I would just print and return here as well. + + if (!desc-chip-direction_input || !desc-chip-direction_output) + direction_may_change = false; spin_unlock_irqrestore(gpio_lock, flags); if (desc-chip-names desc-chip-names[gpio - desc-chip-base]) ioname = desc-chip-names[gpio - desc-chip-base]; - if (status == 0) { - struct device *dev; - - dev = device_create(gpio_class, desc-chip-dev, MKDEV(0, 0), - desc, ioname ? ioname : gpio%u, gpio); - if (!IS_ERR(dev)) { - status = sysfs_create_group(dev-kobj, - gpio_attr_group); - - if (!status direction_may_change) - status = device_create_file(dev, - dev_attr_direction); - - if (!status gpio_to_irq(gpio) = 0 -(direction_may_change - || !test_bit(FLAG_IS_OUT, - desc-flags))) - status = device_create_file(dev, - dev_attr_edge); - - if (status != 0) - device_unregister(dev); - } else - status = PTR_ERR(dev); - if (status == 0) - set_bit(FLAG_EXPORT, desc-flags); + dev = device_create(gpio_class, desc-chip-dev, MKDEV(0, 0), + desc, ioname ? ioname : gpio%u, gpio); + if (IS_ERR(dev)) { + status = PTR_ERR(dev); + goto fail_unlock; Since this involves clean-up in the form of unlocking the mutex this goto is fine however. Same for unregister_device:. Yours, Linus Walleij -- 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/
Re: [PATCH V3] gpio: samsung: use pr_* instead of printk
On Wed, Oct 17, 2012 at 3:52 AM, Jingoo Han jg1@samsung.com wrote: This patch uses pr_* instead of printk. Also, gpio_dbg is replaced with pr_debug. Signed-off-by: Jingoo Han jg1@samsung.com Reviewed-by: Linus Walleij linus.wall...@linaro.org - NAK Please consult Documentation/SubmittingPatches as to the conditions that apply when you add Reviewed-by tags. Hint: it doesn't mean I know Linus looked at this patch. Yours, Linus Walleij -- 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/
Re: [PATCH 6/7] gpio/langwell: find the irq domain mapping
On Wed, Oct 17, 2012 at 9:26 AM, Mika Westerberg mika.westerb...@linux.intel.com wrote: On Tue, Oct 16, 2012 at 09:23:23PM +0200, Linus Walleij wrote: Switch from creating the IRQ domain mapping to finding it. In this case we know very well that the driver has created the apropriate mapping, we just need to locate it, no need to create any on-the-fly mappings. I may be missing something but where in the driver we create the mappings then? I couldn't find any place where it is done. I think I got this backwards in my head. I was under the impression that descriptors were pre-allocated in the DT case. But now, you're right: for linear maps we have to call create* atleast once. Sorry... dropping this crap. Yours, Linus Walleij -- 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/
Re: [PATCH 7/7] gpio/tegra: convert to use linear irqdomain
On Wed, Oct 17, 2012 at 8:32 PM, Stephen Warren swar...@wwwdotorg.org wrote: On 10/16/2012 04:33 PM, Stephen Warren wrote: On 10/16/2012 01:23 PM, Linus Walleij wrote: The MXS driver tries to do the work of irq_domain_add_linear() by reserving a bunch of descriptors somewhere and keeping track of the base offset, then calling irq_domain_add_legacy(). Let's stop doing that and simply use the linear IRQ domain. This /looks/ fine, but appears to break users of GPIOs from this module, and causes a backtrace when cat /sys/kernel/debug/gpio: ... The following additional diff makes it work: diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c index 0234162..c7c175a 100644 --- a/drivers/gpio/gpio-tegra.c +++ b/drivers/gpio/gpio-tegra.c @@ -460,7 +460,7 @@ static int __devinit tegra_gpio_probe(struct platform_device *pdev) gpiochip_add(tegra_gpio_chip); for (gpio = 0; gpio tegra_gpio_chip.ngpio; gpio++) { - int irq = irq_find_mapping(irq_domain, gpio); + int irq = irq_create_mapping(irq_domain, gpio); /* No validity check; all Tegra GPIOs are valid IRQs */ bank = tegra_gpio_banks[GPIO_BANK(gpio)]; I wonder if perhaps the entirety of that loop and perhaps the one after it should be in the IRQ domain's map op - is that how all this is intended to work? I've been asking the same kind of question... Basically as far as I understand: - If you use a linear IRQ domain you need to call irq_create_mapping() atleast once, and at that point the IRQ descriptor will be dynamically allocated. On subsequent calls, irq_find_mapping() can be used. - If using legacy IRQ domains, descriptors are supposed to be allocated elsewhere, and you can just use irq_find_mapping(), but irq_create_mapping() does not hurt, because that call will check if a descriptor is already available. - With the simple domain (as augmented by myself) the mappins will be dynamically created if the passed IRQ base is = 0 else it falls through to creating a linear domain, so in this case to be certain irq_create_mapping() should also be called at least once. I'm not overly happy with these semantics :-( Basically irq_find_mapping() may be a fragile and dangerous optimization path. I've folded the above into my patch. Yours, Linus Walleij -- 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/
Re: [PATCH 4/7] gpio/mxs: convert to use linear irqdomain
On Thu, Oct 18, 2012 at 8:22 AM, Shawn Guo shawn@linaro.org wrote: /* gpio-mxs can be a generic irq chip */ mxs_gpio_init_gc(port, irq_base); So I know this one is not compile-tested. No, which defconfig shall I use for this driver? This is exactly the reason why I have to use irq_domain_add_legacy other than irq_domain_add_linear when I add irqdomain support for the driver. The driver uses generic-irq infrastructural which needs irq_base for setup. So sadly, before generic-irq gets improved, any irq chip that uses generic-irq will have to use irq_domain_add_legacy. Bah, I'm dropping this and a lot of the other patches as well. I obviously do not understand the inner transcendental meaning of irqdomains yet. Yours, Linus Walleij -- 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/
Re: [PATCH RFC 2/6 v3] gpio: Add sysfs support to block GPIO API
On Thu, Oct 18, 2012 at 6:38 AM, Daniel Glöckner daniel...@gmx.net wrote: On Mon, Oct 15, 2012 at 10:30:15PM +0200, Linus Walleij wrote: Another patch that is circulating concerns edge triggers and similar, and it appear that some parts of the GPIO sysfs is for example redefining and exporting IRQchip properties like trigger edge in sysfs, while the settings of the irqchip actually used by the driver is not reflected in the other direction. So you can *set* these things by writing in the GPIO sysfs, but never trust what you *read* from there. And you can set what edges an IRQ will trigger on a certain GPIO, and the way to handle the IRQs from usespace is to poll on a value. This is not really documented but well ... Part of this sounds like you are not familiar with the GPIOlib sysfs IRQ stuff. Yeah you bet :-) I'm just trying to maintain it, I think I need your help with this. Do you think it'd be possible for you to augment the Documentation/gpio.txt file with some userspace code examples? I think this could be very useful. The trigger edge set in sysfs is only used when userspace polls the GPIO via sysfs. Drivers that want to register a gpio IRQ should IMHO always explicitly request the edge/level to trigger on and they should request the gpio beforehand. This prevents the gpio from being exported to userspace. Only IRQ triggers accepted by the irq chip are settable in sysfs, so you can trust the value read from that file. OK. Daniel: are you interested in helping us fixing the GPIOlib sysfs ABI and kernel internals? I'm a bit afraid of it. Actually I don't know what you want to change to fix the existing sysfs ABI. Personally I'd like to see the following things changed: - /sys/gpio/.../direction does not correspond to hardware before first use of gpio_direction_* due to lack of gpio_get_direction. - Names given to gpios by the chip should just result in symlinks to the usual gpioX directories or (un)exporting of gpios should accept names. Please send a patch! I'll merge. Actually I'm mostly referring to another floating patch, I will try to dig it up and CC you. Yours, Linus Walleij -- 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/
Re: [PATCH RFC 02/15 v5] gpio: Add sysfs support to block GPIO API
On Thu, Oct 18, 2012 at 12:07 PM, Roland Stigge sti...@antcom.de wrote: On 10/17/2012 09:05 PM, Greg KH wrote: +if (value != exported) { +if (value) +status = gpio_block_value_export(block); +else +status = gpio_block_value_unexport(block); That looks like a recipie for disaster. Why do you allow userspace to do this? Exporting for gpio blocks is done as follows: writing 1 to the exported _device_ attribute of the gpio block creates the values attribute and at the same time requests the whole block (including all of its gpios) as sysfs. To me it reads like Greg's comment is basically pinpointing a flaw in Brownell's initial design of gpio sysfs: that new sysfs files are created and destroyed by writing into sysfs */export files from userspace? See commit: d8f388d8dc8d4f36539dd37c1fff62cc404ea0fc The block GPIO stuff is just following that design pattern. Yours, Linus Walleij -- 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/