Re: [PATCH] pinctrl:Convert the composition of devm_request_mem_region and devm_ioremap to a single call
On Thu, Dec 10, 2015 at 11:31 PM, Tony Lindgren <t...@atomide.com> wrote: >> > >> > I think we need to add ourselves to MAINTAINERS for this driver, >> > otherwise we'll keep on missing emails. >> >> Good idea! Patches accepted. > > How about this one below? Patch applied for fixes! Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 004/182] gpio: generic: factor into gpio_chip struct
On Thu, Dec 10, 2015 at 12:08 AM, Michael Welling <mwell...@ieee.org> wrote:
> On Wed, Dec 09, 2015 at 02:12:40PM +0100, Linus Walleij wrote:
> ...
>> - ret = gpiochip_add(>gc);
>> + ret = gpiochip_add_data(gc, NULL);
>> if (ret) {
>
> gpiochip_add is still mentioned in the dev_err below.
I explicitly left text string updates aside, I am anyways going to have to go
over all drivers for a second refactoring with gpiodev_alloc() etc.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/39] pinctrl: Move am4372 and dra7 macros to the the SoC header files
On Wed, Nov 25, 2015 at 7:39 PM, Tony Lindgren <t...@atomide.com> wrote: > Linus, (...) >> > OK, I believe he was waiting for yours to pick the series though ;) >> >> Yeah probably best to keep this series together if you're OK with that. > > Care to ack this one? I'd like to apply this series for v4.5 within next > few days.. Sorry for the delay. ACK. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 004/182] gpio: generic: factor into gpio_chip struct
The separate struct bgpio_chip has been a pain to handle, both by being confusingly similar in name to struct gpio_chip and for being contained inside a struct so that struct gpio_chip is contained in a struct contained in a struct, making several steps of dereferencing necessary. Make things simpler: include the fields directly into , #ifdef:ed for CONFIG_GENERIC_GPIO, and get rid of the altogether. Prefix some of the member variables with bgpio_* and add proper kerneldoc while we're at it. Modify all users to handle the change and use a struct gpio_chip directly. And while we're at it: replace all container_of() dereferencing by gpiochip_get_data() and registering the gpio_chip with gpiochip_add_data(). Cc: a...@kernel.org Cc: Lee Jones <lee.jo...@linaro.org> Cc: Alexander Shiyan <shc_w...@mail.ru> Cc: Shawn Guo <shawn...@kernel.org> Cc: Sascha Hauer <ker...@pengutronix.de> Cc: Tony Lindgren <t...@atomide.com> Cc: Kukjin Kim <kg...@kernel.org> Cc: Krzysztof Kozlowski <k.kozlow...@samsung.com> Cc: Alexandre Courbot <gnu...@gmail.com> Cc: Gregory Fong <gregory.0...@gmail.com> Cc: Brian Norris <computersforpe...@gmail.com> Cc: Florian Fainelli <f.faine...@gmail.com> Cc: Liviu Dudau <liviu.du...@arm.com> Cc: Sudeep Holla <sudeep.ho...@arm.com> Cc: Lorenzo Pieralisi <lorenzo.pieral...@arm.com> Cc: Nicolas Pitre <nicolas.pi...@linaro.org> Cc: Olof Johansson <o...@lixom.net> Cc: Vladimir Zapolskiy <vladimir_zapols...@mentor.com> Cc: Rabin Vincent <ra...@rab.in> Cc: linux-arm-ker...@lists.infradead.org Cc: linux-omap@vger.kernel.org Cc: linux-samsung-...@vger.kernel.org Cc: bcm-kernel-feedback-l...@broadcom.com Signed-off-by: Linus Walleij <linus.wall...@linaro.org> --- ARM SoC folks and Lee: it would be great if you could ACK the few lines hitting arch/arm/* and drivers/mfd/* in this so I can take it through the GPIO tree. --- arch/arm/mach-clps711x/board-autcpu12.c | 2 +- arch/arm/mach-clps711x/board-p720t.c| 2 +- arch/arm/mach-imx/mach-mx21ads.c| 2 +- arch/arm/mach-omap1/board-ams-delta.c | 2 +- arch/arm/mach-s3c64xx/mach-crag6410.c | 2 +- drivers/gpio/gpio-74xx-mmio.c | 37 ++-- drivers/gpio/gpio-brcmstb.c | 80 - drivers/gpio/gpio-clps711x.c| 28 +-- drivers/gpio/gpio-dwapb.c | 92 +- drivers/gpio/gpio-ep93xx.c | 25 +-- drivers/gpio/gpio-etraxfs.c | 49 +++--- drivers/gpio/gpio-ge.c | 24 +-- drivers/gpio/gpio-generic.c | 292 +++- drivers/gpio/gpio-grgpio.c | 73 drivers/gpio/gpio-moxart.c | 29 ++-- drivers/gpio/gpio-mxc.c | 27 ++- drivers/gpio/gpio-mxs.c | 33 ++-- drivers/gpio/gpio-sodaville.c | 13 +- drivers/gpio/gpio-xgene-sb.c| 40 ++--- drivers/mfd/vexpress-sysreg.c | 8 +- include/linux/basic_mmio_gpio.h | 80 - include/linux/gpio/driver.h | 54 ++ 22 files changed, 442 insertions(+), 552 deletions(-) delete mode 100644 include/linux/basic_mmio_gpio.h diff --git a/arch/arm/mach-clps711x/board-autcpu12.c b/arch/arm/mach-clps711x/board-autcpu12.c index c3d964221767..ba3d7d1b28f8 100644 --- a/arch/arm/mach-clps711x/board-autcpu12.c +++ b/arch/arm/mach-clps711x/board-autcpu12.c @@ -31,7 +31,7 @@ #include #include #include -#include +#include #include #include diff --git a/arch/arm/mach-clps711x/board-p720t.c b/arch/arm/mach-clps711x/board-p720t.c index e68dd629bda2..80a16a8b3776 100644 --- a/arch/arm/mach-clps711x/board-p720t.c +++ b/arch/arm/mach-clps711x/board-p720t.c @@ -28,7 +28,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/arch/arm/mach-imx/mach-mx21ads.c b/arch/arm/mach-imx/mach-mx21ads.c index 703ce31d7379..9986f9a697c8 100644 --- a/arch/arm/mach-imx/mach-mx21ads.c +++ b/arch/arm/mach-imx/mach-mx21ads.c @@ -17,7 +17,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c index a95499ea8706..97e66558c238 100644 --- a/arch/arm/mach-omap1/board-ams-delta.c +++ b/arch/arm/mach-omap1/board-ams-delta.c @@ -11,7 +11,7 @@ * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ -#include +#include #include #include #include diff --git a/arch/arm/mach-s3c64xx/mach-crag6410.c b/arch/arm/mach-s3c64xx/mach-crag6410.c index f776adcdaee8..723f47fefc81 100644 --- a/arch/arm/mach-s3c64xx/mach-crag6410.c +++ b/arch/arm/mach-s3c64xx/mach-crag6410.c @@ -29,7 +29,7 @@ #include #include #include -#include +#include #include #include diff --git a/drivers/gpio/gpio-74xx-mmio.c b/drivers/gp
Re: [4.4-rc][PATCH] gpio: omap: drop omap1 mpuio specific irq_mask/unmask callbacks
On Fri, Nov 20, 2015 at 2:35 PM, Grygorii Strashko
<grygorii.stras...@ti.com> wrote:
> Originally OMAP MPUIO GPIO irqchip was implemented using Generic irq
> chip, but after set of reworks Generic irq chip code was replaced by
> common OMAP GPIO implementation and finally removed by
> commit d2d05c65c40e ("gpio: omap: Fix regression for MPUIO interrupts").
> Unfortunately, above commit left .irq_mask/unmask callbacks assigned
> as below for MPUIO GPIO case:
> irqc->irq_mask = irq_gc_mask_set_bit;
> irqc->irq_unmask = irq_gc_mask_clr_bit;
>
> This now causes boot failure on OMAP1 platforms, after
> commit 450fa54cfd66 ("gpio: omap: convert to use generic irq handler")
> which forces these callbacks to be called during GPIO IRQs mapping
> from gpiochip_irq_map:
>
> Unable to handle kernel NULL pointer dereference at virtual address
> pgd = c0004000
> [] *pgd=
> Internal error: Oops: 75 [#1] ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted
> 4.4.0-rc1-e3-los_afe0c+-2-g25379c0-dirty #1
> Hardware name: Amstrad E3 (Delta)
> task: c1836000 ti: c1838000 task.ti: c1838000
> PC is at irq_gc_mask_set_bit+0x1c/0x60
> LR is at __irq_do_set_handler+0x118/0x15c
> pc : []lr : []psr: 60d3
> sp : c1839c90 ip : c1862c64 fp : c1839c9c
> r10: r9 : c0411950 r8 : c0411bbc
> r7 : r6 : c185c310 r5 : c00444e8 r4 : c185c300
> r3 : c1854b50 r2 : r1 : r0 : c185c310
> Flags: nZCv IRQs off FIQs off Mode SVC_32 ISA ARM Segment kernel
> Control: 317f Table: 10004000 DAC: 0057
> Process swapper (pid: 1, stack limit = 0xc1838190)
> Stack: (0xc1839c90 to 0xc183a000)
>
> [...]
>
> Backtrace:
> [] (irq_gc_mask_set_bit) from []
> (__irq_do_set_handler+0x118/0x15c)
> [] (__irq_do_set_handler) from []
> (__irq_set_handler+0x44/0x5c)
> r6: r5:c00444e8 r4:c185c300
> [] (__irq_set_handler) from []
> (irq_set_chip_and_handler_name+0x30/0x34)
> r7:0050 r6: r5:c00444e8 r4:0050
> [] (irq_set_chip_and_handler_name) from []
> (gpiochip_irq_map+0x3c/0x8c)
> r7:0050 r6: r5:0050 r4:c1862c64
> [] (gpiochip_irq_map) from []
> (irq_domain_associate+0x7c/0x1c4)
> r5:c185c310 r4:c185cb00
> [] (irq_domain_associate) from []
> (irq_domain_add_simple+0x98/0xc0)
> r8:c0411bbc r7:c185cb00 r6:0050 r5:0010 r4:0001
> [] (irq_domain_add_simple) from []
> (_gpiochip_irqchip_add+0x64/0x10c)
> r7:c1862c64 r6:c0419280 r5:c1862c64 r4:c1854b50
> [] (_gpiochip_irqchip_add) from []
> (omap_gpio_probe+0x2fc/0x63c)
> r5:c1854b50 r4:c1862c10
> [] (omap_gpio_probe) from []
> (platform_drv_probe+0x2c/0x64)
> r10: r9:c03e45e8 r8: r7:c0419294 r6:c0411984 r5:c0419294
> r4:c0411950
> [] (platform_drv_probe) from [] (really_probe+0x160/0x29c)
>
> Hence, fix it by remove obsolete callbacks assignment. After this
> change omap_gpio_mask_irq()/omap_gpio_unmask_irq() will be used
> for MPUIO IRQs masking, but this now happens anyway from
> omap_gpio_irq_startup/shutdown().
>
> Cc: Tony Lindgren <t...@atomide.com>
> Fixes: commit d2d05c65c40e ("gpio: omap: Fix regression for MPUIO interrupts")
> Reported-by: Aaro Koskinen <aaro.koski...@iki.fi>
> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com>
Patch applied for fixes.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/39] pinctrl: Move am4372 and dra7 macros to the the SoC header files
On Fri, Nov 13, 2015 at 5:53 AM, Javier Martinez Canillas <jav...@osg.samsung.com> wrote: > The header file defines a set of macros > for different SoCs families that falls under the OMAP sub-arch, that > allow to define the padconf register physical address instead of the > register offset from the padconf base. > > But the am43xx and dra7xx SoCs families have their own pinctrl header > file so the DTS using these SoCs aren't able to use the AM4372_IOPAD() > and DRA7XX_CORE_IOPAD() macros since is > not included. > > Move the macros to the correct header files so can be used by the DTS. > > Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com> I need Tony's ACK on this. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: omap: fix static checker warning
On Fri, Sep 25, 2015 at 9:06 PM, Grygorii Strashko <grygorii.stras...@ti.com> wrote: > This patch fixes below static checker warning by changing > type of irq field in struct gpio_bank from u16 to int. Nobody's saying anything, I'll just apply this. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] gpio: omap: move pm runtime in irq_chip.irq_bus_lock/sync_unlock
On Fri, Sep 25, 2015 at 12:28 PM, Grygorii Strashko
<grygorii.stras...@ti.com> wrote:
> The PM runtime API can't be used in atomic contex on -RT even if
> it's configured as irqsafe. As result, below error report can
> be seen when PM runtime API called from IRQ chip's callbacks
> irq_startup/irq_shutdown/irq_set_type, because they are
> protected by RAW spinlock:
>
> BUG: sleeping function called from invalid context at
> kernel/locking/rtmutex.c:917
> in_atomic(): 1, irqs_disabled(): 128, pid: 96, name: insmod
> 3 locks held by insmod/96:
> #0: (>mutex){..}, at: [] __driver_attach+0x54/0xa0
> #1: (>mutex){..}, at: [] __driver_attach+0x60/0xa0
> #2: (class){..}, at: [] __irq_get_desc_lock+0x60/0xa4
> irq event stamp: 1834
> hardirqs last enabled at (1833): []
> _raw_spin_unlock_irqrestore+0x88/0x90
> hardirqs last disabled at (1834): []
> _raw_spin_lock_irqsave+0x2c/0x64
> softirqs last enabled at (0): [] copy_process.part.52+0x410/0x19d8
> softirqs last disabled at (0): [< (null)>] (null)
> Preemption disabled at:[< (null)>] (null)
>
> CPU: 1 PID: 96 Comm: insmod Tainted: GW O
> 4.1.3-rt3-00618-g57e2387-dirty #184
> Hardware name: Generic DRA74X (Flattened Device Tree)
> [] (unwind_backtrace) from [] (show_stack+0x20/0x24)
> [] (show_stack) from [] (dump_stack+0x88/0xdc)
> [] (dump_stack) from [] (___might_sleep+0x198/0x2a8)
> [] (___might_sleep) from [] (rt_spin_lock+0x30/0x70)
> [] (rt_spin_lock) from [] (__pm_runtime_resume+0x68/0xa4)
> [] (__pm_runtime_resume) from []
> (omap_gpio_irq_type+0x188/0x1d8)
> [] (omap_gpio_irq_type) from []
> (__irq_set_trigger+0x68/0x130)
> [] (__irq_set_trigger) from []
> (irq_set_irq_type+0x44/0x6c)
> [] (irq_set_irq_type) from []
> (irq_create_of_mapping+0x120/0x174)
> [] (irq_create_of_mapping) from [] (of_irq_get+0x48/0x58)
> [] (of_irq_get) from [] (i2c_device_probe+0x54/0x15c)
> [] (i2c_device_probe) from []
> (driver_probe_device+0x184/0x2c8)
> [] (driver_probe_device) from []
> (__driver_attach+0x9c/0xa0)
> [] (__driver_attach) from [] (bus_for_each_dev+0x7c/0xb0)
> [] (bus_for_each_dev) from [] (driver_attach+0x28/0x30)
> [] (driver_attach) from [] (bus_add_driver+0x154/0x200)
> [] (bus_add_driver) from [] (driver_register+0x88/0x108)
> [] (driver_register) from []
> (i2c_register_driver+0x3c/0x90)
> [] (i2c_register_driver) from [] (pcf857x_init+0x18/0x24
> [gpio_pcf857x])
> [] (pcf857x_init [gpio_pcf857x]) from []
> (do_one_initcall+0x128/0x1e8)
> [] (do_one_initcall) from [] (do_init_module+0x6c/0x1bc)
> [] (do_init_module) from [] (load_module+0x18e8/0x21c4)
> [] (load_module) from [] (SyS_init_module+0xfc/0x158)
> [] (SyS_init_module) from [] (ret_fast_syscall+0x0/0x54)
>
> The IRQ chip interface defines only two callbacks which are executed in
> non-atomic contex - irq_bus_lock/irq_bus_sync_unlock, so lets move
> PM runtime calls there.
>
> Tested-by: Tony Lindgren <t...@atomide.com>
> Tested-by: Austin Schuh <aus...@peloton-tech.com>
> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com>
Patch applied with Santosh's ACK.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: Remove __ref on hotplug cpu die path
On Mon, Sep 14, 2015 at 5:23 PM, Stephen Boyd <sb...@codeaurora.org> wrote: > Now that __cpuinit has been removed, the __ref markings on these > functions are useless. Remove them. This also reduces the size of > the multi_v7_defconfig image: > > $ size before after >textdata bss dec hex filename >126835781470996 348904 14503478 dd4e36 before >126832741470996 348904 14503174 dd4d06 after > > presumably because now we don't have to jump to code in the > .ref.text section and/or the noinline marking is removed. > > Cc: Tony Lindgren <t...@atomide.com> > Cc: Barry Song <bao...@kernel.org> > Cc: Andy Gross <agr...@codeaurora.org> > Cc: Viresh Kumar <vire...@kernel.org> > Cc: Shiraz Hashim <shiraz.linux.ker...@gmail.com> > Cc: Stephen Warren <swar...@wwwdotorg.org> > Cc: Thierry Reding <thierry.red...@gmail.com> > Cc: Alexandre Courbot <gnu...@gmail.com> > Cc: Linus Walleij <linus.wall...@linaro.org> > Cc: Sudeep Holla <sudeep.ho...@arm.com> > Cc: Lorenzo Pieralisi <lorenzo.pieral...@arm.com> > Cc: Will Deacon <will.dea...@arm.com> > Cc: Mark Rutland <mark.rutl...@arm.com> > Cc: <linux-omap@vger.kernel.org> > Cc: <linux-arm-...@vger.kernel.org> > Cc: <spear-de...@list.st.com> > Cc: <linux-te...@vger.kernel.org> > Signed-off-by: Stephen Boyd <sb...@codeaurora.org> Acked-by: Linus Walleij <linus.wall...@linaro.org> Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] gpio: omap: convert to use generic irq handler
On Fri, Sep 25, 2015 at 12:28 PM, Grygorii Strashko <grygorii.stras...@ti.com> wrote: > This patch converts TI OMAP GPIO driver to use generic irq handler > instead of chained IRQ handler. This way OMAP GPIO driver will be > compatible with RT kernel where it will be forced thread IRQ handler > while in non-RT kernel it still will be executed in HW IRQ context. > As part of this change the IRQ wakeup configuration is applied to > GPIO Bank IRQ as it now will be under control of IRQ PM Core during > suspend. > > There are also additional benefits: > - on-RT kernel there will be no complains any more about PM runtime usage >in atomic context "BUG: sleeping function called from invalid context"; > - GPIO bank IRQs will appear in /proc/interrupts and its usage statistic > will be visible; > - GPIO bank IRQs could be configured through IRQ proc_fs interface and, >as result, could be a part of IRQ balancing process if needed; > - GPIO bank IRQs will be under control of IRQ PM Core during >suspend to RAM. > > Disadvantage: > - additional runtime overhed as call chain till >omap_gpio_irq_handler() will be longer now > - necessity to use wa_lock in omap_gpio_irq_handler() to W/A warning >in handle_irq_event_percpu() >WARNING: CPU: 1 PID: 35 at kernel/irq/handle.c:149 > handle_irq_event_percpu+0x51c/0x638() > > This patch doesn't fully follows recommendations provided by Sebastian > Andrzej Siewior [1], because It's required to go through and check all > GPIO IRQ pin states as fast as possible and pass control to handle_level_irq > or handle_edge_irq. handle_level_irq or handle_edge_irq will perform actions > specific for IRQ triggering type and wakeup corresponding registered > threaded IRQ handler (at least it's expected to be threaded). > IRQs can be lost if handle_nested_irq() will be used, because excecution > time of some pin specific GPIO IRQ handler can be very significant and > require accessing ext. devices (I2C). > > Idea of such kind reworking was also discussed in [2]. > > [1] http://www.spinics.net/lists/linux-omap/msg120665.html > [2] http://www.spinics.net/lists/linux-omap/msg119516.html > > Tested-by: Tony Lindgren <t...@atomide.com> > Tested-by: Austin Schuh <aus...@peloton-tech.com> > Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> Patch applied. I'm thinking that we need some recommendations on how to write IRQ handlers in order to be RT-compatible. Can you help me lining up the requirements in Documentation/gpio/driver.txt? I will write an RFC patch and let you write some additional text to it in response then we can iterate it a bit. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 6/7] gpio: omap: move pm runtime in irq_chip.irq_bus_lock/sync_unlock
On Tue, Aug 18, 2015 at 4:10 AM, Grygorii Strashko <grygorii.stras...@ti.com> wrote: > The PM runtime API can't be used in atomic contex on -RT even if > it's configured as irqsafe. As result, below error report can > be seen when PM runtime API called from IRQ chip's callbacks > irq_startup/irq_shutdown/irq_set_type, because they are > protected by RAW spinlock: Grygorri I have a massive backlog of mail but if patch 6&7 are still applicable, can you rebase and send me these for the v4.3-rc2+ tree? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: omap: Fix GPIO numbering for deferred probe
On Thu, Sep 3, 2015 at 7:31 PM, Tony Lindgren <t...@atomide.com> wrote: > If gpio-omap probe fails with -EPROBE_DEFER, the GPIO numbering > keeps increasing. Only increase the gpio count if gpiochip_add() > was successful as otherwise the numbers will increase for each > probe attempt. > > Cc: Grygorii Strashko <grygorii.stras...@ti.com> > Cc: Javier Martinez Canillas <jav...@dowhile0.org> > Cc: Kevin Hilman <khil...@deeprootsystems.com> > Cc: Santosh Shilimkar <ssant...@kernel.org> > Signed-off-by: Tony Lindgren <t...@atomide.com> Patch applied with Grygorii's review tag. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pinctrl: core: Warn about NULL gpio_chip in pinctrl_ready_for_gpio_range()
On Thu, Sep 3, 2015 at 7:34 PM, Tony Lindgren <t...@atomide.com> wrote: > If the gpio driver is confused about the numbers for gpio-ranges, > pinctrl_ready_for_gpio_range() may get called with invalid GPIO > causing a NULL pointer exception. Let's instead provide a warning > that allows fixing the problem and return with error. > > Signed-off-by: Tony Lindgren <t...@atomide.com> Patch applied for fixes. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: omap: Fix gpiochip_add() handling for deferred probe
On Fri, Aug 28, 2015 at 8:44 PM, Tony Lindgren <t...@atomide.com> wrote:
> Currently we gpio-omap breaks if gpiochip_add() returns -EPROBE_DEFER:
>
> [0.57] gpiochip_add: GPIOs 0..31 (gpio) failed to register
> [0.57] omap_gpio 4831.gpio: Could not register gpio chip -517
> ...
> [3.67] omap_gpio 4831.gpio: Unbalanced pm_runtime_enable!
>
> Let's fix the issue by adding the missing pm_runtime_put() on error.
>
> Cc: Grygorii Strashko <grygorii.stras...@ti.com>
> Cc: Javier Martinez Canillas <jav...@dowhile0.org>
> Cc: Kevin Hilman <khil...@deeprootsystems.com>
> Cc: Santosh Shilimkar <ssant...@kernel.org>
> Signed-off-by: Tony Lindgren <t...@atomide.com>
> ---
> drivers/gpio/gpio-omap.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index b0c57d5..f09bf0b 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1232,8 +1232,11 @@ static int omap_gpio_probe(struct platform_device
> *pdev)
> omap_gpio_mod_init(bank);
>
> ret = omap_gpio_chip_init(bank, irqc);
> - if (ret)
> + if (ret) {
> + pm_runtime_put_sync(bank->dev);
> + pm_runtime_disable(bank->dev);
> return ret;
> + }
>
> omap_gpio_show_rev(bank);
Patch applied for fixes with Santosh's ACK.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] gpiolib: add description for gpio irqchip fields in struct gpio_chip
On Mon, Aug 17, 2015 at 2:35 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: Add missed description for GPIO irqchip fields in struct gpio_chip. Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- Changes in v2: - New patch. Patch applied. Thanks for writing docs, it's very helpful. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] gpio: omap: remove wrong irq_domain_remove usage in probe
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: The bank-chip.irqdomain is uninitialized at the moment when irq_domain_remove() is called, so remove this call. Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com Obviously correct, patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] gpio: omap: protect regs access in omap_gpio_irq_handler
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: The access to HW registers has to be be protected in omap_gpio_irq_handler(), as it may race with code executed on another CPUs. Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] gpio: omap: fix clk_prepare/unprepare usage
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: As per CCF documentation (clk.txt) the clk_prepare/unprepare APIs are not allowed in atomic context. But now OMAP GPIO driver uses them while applying debounce settings and as part of PM runtime irqsafe operations: - omap_gpio_debounce() is holding the lock with IRQs off. + omap2_set_gpio_debounce() + clk_prepare_enable() + clk_prepare() this one might sleep. - pm_runtime_get_sync() is holding the lock with IRQs off + omap_gpio_runtime_suspend() + raw_spin_lock_irqsave() + omap_gpio_dbck_disable() + clk_disable_unprepare() Hence, fix it by moeving dbclk prepare/unprepare in OMAP GPIO omap_gpio_probe/omap_gpio_remove. Also, while here, ensure that debounce functionality is disabled if clk_get() failed, because otherwise kernel will carsh in omap2_set_gpio_debounce(). Reported-by: Sebastian Andrzej Siewior bige...@linutronix.de Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] gpio: omap: fix omap2_set_gpio_debounce
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: According to TRMs: Required input line stable = (the value of the GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME + 1) × 31, where the value of the GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME bit field is from 0 to 255. But now omap2_set_gpio_debounce() will calculate debounce time and behave incorrectly in the following cases: 1) requested debounce time is !0 and 32 calculated DEBOUNCETIME = 0x1 == 62 us; expected value of DEBOUNCETIME = 0x0 == 31us 2) requested debounce time is 0 calculated DEBOUNCETIME = 0x1 == 62 us; expected: disable debounce and DEBOUNCETIME = 0x0 3) requested debounce time is 32 and 63 calculated DEBOUNCETIME = 0x0 and debounce will be disabled; expected: enable debounce and DEBOUNCETIME = 0x1 == 62 us Hence, rework omap2_set_gpio_debounce() to fix above cases: 1) introduce local variable enable and use it to identify when debounce need to be enabled or disabled. Disable debounce if requested debounce time is 0. 2) use below formula for debounce time calculation: debounce = (DIV_ROUND_UP(debounce, 31) - 1) 0xFF; Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com Very hightech. Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] gpio: omap: switch to use platform_get_irq
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: Switch OMAP GPIO driver to use platform_get_irq(), because it is not recommened to use platform_get_resource(pdev, IORESOURCE_IRQ, ..) for requesting IRQ resources any more, as they can be not ready yet in case of DT-boot. Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com Sane handling of deffred probe. Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] gpio: omap: fixes and improvements
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: This patch series contains set of trivial fixes and improvements, and also patches which fixes wrong APIs usage in atomic context as for -RT as for non-RT kernel. The final goal of this series is to make TI OMAP GPIO driver compatible with -RT kernel as much as possible. Patch 1-4: trivial fixes and improvements Patch 5: fixes wrong CLK clk_prepare/unprepare APIs usage in atomic contexet I've applied patches 1-5 with Santosh's ACK and Tony's Tested-by. Patch 6(rfc): required to be compatible with -RT kernel, because PM runtime can't be used in atimic context on -RT. Patch 7(rfc): This patch converts TI OMAP GPIO driver to use generic irq handler instead of chained IRQ handler. This way OMAP GPIO driver will be compatible with RT kernel where it will be forced thread IRQ handler while in non-RT kernel it still will be executed in HW IRQ context. Waiting for more feedback here. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpiolib: irqchip: use different lockdep class for each gpio irqchip
On Fri, Aug 14, 2015 at 2:40 PM, Lars-Peter Clausen l...@metafoo.de wrote: On 08/14/2015 02:34 PM, Linus Walleij wrote: [...] Every chip will get their own lock class on the heap. But I think it is a bit kludgy. Is it not possible to have the lock key in struct gpio_chip be a real member instead of a pointer and get a per-chip lock that way? (...) struct lock_class_key lock_key; instead of: struct lock_class_key *lock_key; - problem solved, without kludgy header defines? Lock keys need to be in persistent memory since they have a unlimited life time. Once registered it is expected to exist until the system is reset. Aha I see. OK if we fix the documentation comment I guess we can go with this, even if it makes the header file somewhat hard to read. I have a bit of problem with it, because lockdep instrumentation is supposed to make development easier, not harder, now it helps in one end with lock validation and screws up in another end by creating API headers that are hopeless to read :( Linus -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] gpiolib: irqchip: use different lockdep class for each gpio irqchip
On Mon, Aug 17, 2015 at 2:35 PM, Grygorii Strashko grygorii.stras...@ti.com wrote: Since IRQ chip helpers were introduced drivers lose ability to register separate lockdep classes for each registered GPIO IRQ chip and the gpiolib now is using shared lockdep class for all GPIO IRQ chips (gpiochip_irq_lock_class). As result, lockdep will produce warning when there are min two stacked GPIO chips and all of them are interrupt controllers. HW configuration which generates lockdep warning (TI dra7-evm): (...) Cc: Geert Uytterhoeven ge...@linux-m68k.org Cc: Roger Quadros rog...@ti.com Reported-by: Roger Quadros rog...@ti.com Tested-by: Roger Quadros rog...@ti.com Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- Changes in v2: - removed accidental change in gpio chip structure description. This v2 patch applied. I see no better alternative. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpiolib: irqchip: use different lockdep class for each gpio irqchip
On Thu, Aug 13, 2015 at 4:58 PM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
Since IRQ chip helpers were introduced drivers lose ability to
register separate lockdep classes for each registered GPIO IRQ
chip and the gpiolib now is using shared lockdep class for
all GPIO IRQ chips (gpiochip_irq_lock_class).
As result, lockdep will produce warning when there are min two
stacked GPIO chips and all of them are interrupt controllers.
HW configuration which generates lockdep warning (TI dra7-evm):
(...)
Cc: Geert Uytterhoeven ge...@linux-m68k.org
Cc: Roger Quadros rog...@ti.com
Reported-by: Roger Quadros rog...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
Ah, I see...
* implies that if the chip supports IRQs, these IRQs need to be threaded
* as the chip access may sleep when e.g. reading out the IRQ status
* registers.
+ * @exported: flags if the gpiochip is exported for use from sysfs. Private.
* @irq_not_threaded: flag must be set if @can_sleep is set but the
* IRQs don't need to be threaded
*
@@ -126,6 +128,7 @@ struct gpio_chip {
irq_flow_handler_t irq_handler;
unsigned intirq_default_type;
int irq_parent;
+ struct lock_class_key *lock_key;
There is something weird with the kerneldoc. It is documenting something
else but not documenting the new member.
Anyway, so here:
+int _gpiochip_irqchip_add(struct gpio_chip *gpiochip,
+ struct irq_chip *irqchip,
+ unsigned int first_irq,
+ irq_flow_handler_t handler,
+ unsigned int type,
+ struct lock_class_key *lock_key);
+
+#ifdef CONFIG_LOCKDEP
+#define gpiochip_irqchip_add(...) \
+( \
+ ({ \
+ static struct lock_class_key _key; \
+ _gpiochip_irqchip_add(__VA_ARGS__, _key); \
+ }) \
+)
+#else
+#define gpiochip_irqchip_add(...) \
+ _gpiochip_irqchip_add(__VA_ARGS__, NULL)
+#endif
Every chip will get their own lock class on the heap.
But I think it is a bit kludgy.
Is it not possible to have the lock key in struct gpio_chip
be a real member instead of a pointer and get a per-chip
lock that way?
(...)
struct lock_class_key lock_key;
instead of:
struct lock_class_key *lock_key;
- problem solved, without kludgy header defines?
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: omap: Fix missing raw locks conversion
On Wed, Aug 5, 2015 at 4:37 PM, Axel Lin axel@ingics.com wrote: Fix below build warning: CC drivers/gpio/gpio-omap.o drivers/gpio/gpio-omap.c: In function 'omap_gpio_irq_type': drivers/gpio/gpio-omap.c:504:3: warning: passing argument 1 of 'spin_unlock_irqrestore' from incompatible pointer type [enabled by default] include/linux/spinlock.h:360:29: note: expected 'struct spinlock_t *' but argument is of type 'struct raw_spinlock_t *' Fixes: commit 4dbada2be460 (gpio: omap: use raw locks for locking) Signed-off-by: Axel Lin axel@ingics.com Fixed by merging in -rc4 and applying this patch for next. Thanks! Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [4.2-rc1][PATCH] gpio: omap: add missed spin_unlock_irqrestore in omap_gpio_irq_type
On Fri, Aug 7, 2015 at 9:34 AM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
Hi Tony,
On 08/07/2015 06:36 AM, Tony Lindgren wrote:
* Linus Walleij linus.wall...@linaro.org [150716 01:38]:
On Wed, Jun 24, 2015 at 4:54 PM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
From: Grygorii Strashko grygorii.stras...@linaro.org
Add missed spin_unlock_irqrestore in omap_gpio_irq_type when
omap_set_gpio_triggering() is failed.
It fixes static checker warning:
drivers/gpio/gpio-omap.c:523 omap_gpio_irq_type()
warn: inconsistent returns 'spin_lock:bank-lock'.
This fixes commit:
1562e4618ded ('gpio: omap: fix error handling in omap_gpio_irq_type')
Reported-by: Javier Martinez Canillas jav...@dowhile0.org
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
Patch applied for fixes.
Linus, looks like we now have a new build warning in Linux next
with the fixes and raw_spinlock_t change, so a merge or a commit
is needed. If you prefer a patch, here's one below.
Yes. It seems merge/rebase issue between fixes next:
- this patch went through fixes and RAW spinlock conversation
patch through -next, and without merge conflicts.
and patch has been posted already by Axel Lin:
http://www.spinics.net/lists/linux-omap/msg121031.html
I merged v4.2-rc4 into my devel branch and applied Axel's
patch to fix this mess. Check that it looks OK now...
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: omap: use raw locks for locking
On Tue, Jul 21, 2015 at 6:26 PM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: This patch converts gpio_bank.lock from a spin_lock into a raw_spin_lock. The call path is to access this lock is always under a raw_spin_lock, for instance - __setup_irq() holds desc-lock with irq off + __irq_set_trigger() + omap_gpio_irq_type() - handle_level_irq() (runs with irqs off therefore raw locks) + mask_ack_irq() + omap_gpio_mask_irq() This fixes the obvious backtrace on -RT. However the locking vs context is not and this is not limited to -RT: - omap_gpio_irq_type() is called with IRQ off and has an conditional call to pm_runtime_get_sync() which may sleep. Either it may happen or it may not happen but pm_runtime_get_sync() should not be called with irqs off. - omap_gpio_debounce() is holding the lock with IRQs off. + omap2_set_gpio_debounce() + clk_prepare_enable() + clk_prepare() this one might sleep. The number of users of gpiod_set_debounce() / gpio_set_debounce() looks low but still this is not good. Acked-by: Javier Martinez Canillas jav...@dowhile0.org Acked-by: Santosh Shilimkar ssant...@kernel.org Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de Patch applied. Now this question appear in my head: Is drivers/gpio full of stuff that will not work with the -RT kernel, and is this a change that should be done mutatis mutandis on all the GPIO drivers? Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [4.2-rc1][PATCH] gpio: omap: add missed spin_unlock_irqrestore in omap_gpio_irq_type
On Wed, Jun 24, 2015 at 4:54 PM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
From: Grygorii Strashko grygorii.stras...@linaro.org
Add missed spin_unlock_irqrestore in omap_gpio_irq_type when
omap_set_gpio_triggering() is failed.
It fixes static checker warning:
drivers/gpio/gpio-omap.c:523 omap_gpio_irq_type()
warn: inconsistent returns 'spin_lock:bank-lock'.
This fixes commit:
1562e4618ded ('gpio: omap: fix error handling in omap_gpio_irq_type')
Reported-by: Javier Martinez Canillas jav...@dowhile0.org
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
Patch applied for fixes.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pinctrl: single: ensure pcs irq will not be forced threaded
] driver_probe_device+0x70/0x254 [c0458810] __driver_attach+0x9c/0xa0 [c045674c] bus_for_each_dev+0x78/0xac [c0458030] driver_attach+0x2c/0x30 [c0457c78] bus_add_driver+0x15c/0x204 [c0458ee0] driver_register+0x88/0x108 [c045a168] __platform_driver_register+0x64/0x6c [c0a8170c] omap_hsmmc_driver_init+0x1c/0x20 [c0008a94] do_one_initcall+0x110/0x170 [c0a44d48] kernel_init_freeable+0x140/0x1e4 [c06fcc24] kernel_init+0x1c/0xf8 [c000eee8] ret_from_fork+0x14/0x20 } ... key at: [c1088a8c] __key.18572+0x0/0x8 ... acquired at: [c008cdd4] mark_lock+0x388/0x76c [c008df40] __lock_acquire+0x6d0/0x1f98 [c0090040] lock_acquire+0x9c/0x158 [c07065c8] _raw_spin_lock+0x48/0x58 [c0375b54] pcs_irq_handle+0x48/0x9c [c0375c5c] pcs_irq_handler+0x1c/0x28 [c009c458] irq_forced_thread_fn+0x30/0x74 [c009c784] irq_thread+0x158/0x1c4 [c0063fc4] kthread+0xd4/0xe8 [c000eee8] ret_from_fork+0x14/0x20 stack backtrace: CPU: 1 PID: 927 Comm: irq/369-pinctrl Not tainted 3.14.43-rt42-00360-g96ff499-dirty #24 [c00177e0] (unwind_backtrace) from [c00130b0] (show_stack+0x20/0x24) [c00130b0] (show_stack) from [c0702958] (dump_stack+0x84/0xd0) [c0702958] (dump_stack) from [c008bcfc] (print_irq_inversion_bug+0x1d0/0x21c) [c008bcfc] (print_irq_inversion_bug) from [c008bf18] (check_usage_backwards+0xb4/0x11c) [c008bf18] (check_usage_backwards) from [c008cdd4] (mark_lock+0x388/0x76c) [c008cdd4] (mark_lock) from [c008df40] (__lock_acquire+0x6d0/0x1f98) [c008df40] (__lock_acquire) from [c0090040] (lock_acquire+0x9c/0x158) [c0090040] (lock_acquire) from [c07065c8] (_raw_spin_lock+0x48/0x58) [c07065c8] (_raw_spin_lock) from [c0375b54] (pcs_irq_handle+0x48/0x9c) [c0375b54] (pcs_irq_handle) from [c0375c5c] (pcs_irq_handler+0x1c/0x28) [c0375c5c] (pcs_irq_handler) from [c009c458] (irq_forced_thread_fn+0x30/0x74) [c009c458] (irq_forced_thread_fn) from [c009c784] (irq_thread+0x158/0x1c4) [c009c784] (irq_thread) from [c0063fc4] (kthread+0xd4/0xe8) [c0063fc4] (kthread) from [c000eee8] (ret_from_fork+0x14/0x20) To fix it use IRQF_NO_THREAD to ensure that pcs irq will not be forced threaded. Cc: Tony Lindgren t...@atomide.com Cc: Sebastian Andrzej Siewior bige...@linutronix.de Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com OK queued for fixes with Tony's ACK. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pinctrl: single: dra7: remove PCS_QUIRK_SHARED_IRQ
On Mon, Jul 6, 2015 at 5:11 PM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
On DRA7 there is one pinctrl domain (dra7_pmx_core) and
PRCM wake-up IRQ is not shared, so remove quirk.
Cc: Nishanth Menon n...@ti.com
Cc: Tony Lindgren t...@atomide.com
Fixes: 31320beaa3d3 ('pinctrl: single: Add DRA7 pinctrl compatibility')
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
Patch applied with the ACKs.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: omap: prevent module from being unloaded while in use
On Thu, Jun 25, 2015 at 5:13 PM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
OMAP GPIO driver allowed to be built as loadable module, but it
doesn't set owner field in GPIO chip structure. As result,
module_get/put() API is not working and it's possible to unload
OMAP driver while in use:
omap_gpio 48051000.gpio: REMOVING GPIOCHIP WITH GPIOS STILL REQUESTED
Hence, add missing configuration.
Cc: Tony Lindgren t...@atomide.com
Fixes: cac089f9026e ('gpio: omap: Allow building as a loadable module')
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
Hi Linus,
Seems this one is for 4.2-rc.
Yup applied for fixes with Alex' ACK.
The bigger fix is applied for devel and the best way to handle this
is open for discussion.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: omap: use raw locks for locking
On Thu, Feb 12, 2015 at 5:10 PM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: This patch converts gpio_bank.lock from a spin_lock into a raw_spin_lock. The call path to access this lock is always under a raw_spin_lock, for instance - __setup_irq() holds desc-lock with irq off + __irq_set_trigger() + omap_gpio_irq_type() - handle_level_irq() (runs with irqs off therefore raw locks) + mask_ack_irq() + omap_gpio_mask_irq() This fixes the obvious backtrace on -RT. However I noticed two cases where it looks wrong and this is not limited to -RT: - omap_gpio_irq_type() is called with IRQ off and has an conditional call to pm_runtime_get_sync() which may sleep. Either it may happen or it may not happen but pm_runtime_get_sync() should not be called in an atomic section. - omap_gpio_debounce() is holding the lock with IRQs off. + omap2_set_gpio_debounce() + clk_prepare_enable() + clk_prepare() this one might sleep. The number of users of gpiod_set_debounce() / gpio_set_debounce() looks low but still this is not good. Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de This doesn't apply to the current development tree. Can you rebase this patch in Torvalds' HEAD, add Javier's ACK and resend? Please put me on the To: line, I have no time to read all mail that I'm only CC on. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc3: pci: make better use of gpiod API
On Fri, Jun 12, 2015 at 9:10 AM, Uwe Kleine-König u.kleine-koe...@pengutronix.de wrote: Since 39b2bbe3d715 (gpio: add flags argument to gpiod_get*() functions) which appeared in v3.17-rc1, the gpiod_get* functions take an additional parameter that allows to specify direction and initial value for output. Furthermore there is devm_gpiod_get_optional which is designed to get optional gpios. Also use devm_gpiod_get* because otherwise the gpio might be grabbed by a different driver. Simplify driver accordingly. Signed-off-by: Uwe Kleine-König u.kleine-koe...@pengutronix.de Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/15] mfd: kill off set_irq_flags usage
On Tue, Jun 9, 2015 at 8:26 PM, Rob Herring r...@kernel.org wrote: set_irq_flags is ARM specific with custom flags which have genirq equivalents. Convert drivers to use the genirq interfaces directly, so we can kill off set_irq_flags. The translation of flags is as follows: IRQF_VALID - !IRQ_NOREQUEST IRQF_PROBE - !IRQ_NOPROBE IRQF_NOAUTOEN - IRQ_NOAUTOEN For IRQs managed by an irqdomain, the irqdomain core code handles clearing and setting IRQ_NOREQUEST already, so there is no need to do this in .map() functions and we can simply remove the set_irq_flags calls. Some users also set IRQ_NOPROBE and this has been maintained although it is not clear that is really needed. There appears to be a great deal of blind copy and paste of this code. Signed-off-by: Rob Herring r...@kernel.org Cc: Samuel Ortiz sa...@linux.intel.com Cc: Lee Jones lee.jo...@linaro.org Cc: Linus Walleij linus.wall...@linaro.org Cc: Milo Kim milo@ti.com Cc: Kumar Gala ga...@codeaurora.org Cc: Andy Gross agr...@codeaurora.org Cc: David Brown dav...@codeaurora.org Cc: Tony Lindgren t...@atomide.com Cc: linux-arm-ker...@lists.infradead.org Cc: patc...@opensource.wolfsonmicro.com Cc: linux-arm-...@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: linux-omap@vger.kernel.org Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT v2 45/48] genirq, gpio: Kill the first parameter 'irq' of irq_flow_handler_t
On Thu, Jun 4, 2015 at 6:13 AM, Jiang Liu jiang@linux.intel.com wrote: Now most IRQ flow handlers make no use of the first parameter 'irq'. And for those who do make use of 'irq', we could easily get the irq number through irq_desc-irq_data-irq. So kill the first parameter 'irq' of irq_flow_handler_t. To ease review, I have split the changes into several parts, though they should be merge as one to support bisecting. Signed-off-by: Jiang Liu jiang@linux.intel.com Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] gpio: omap: fix omap_gpio_free to not clean up irq configuration
On Fri, May 22, 2015 at 4:35 PM, Grygorii Strashko grygorii.stras...@linaro.org wrote: This patch fixes following issue: - GPIOn is used as IRQ by some dev, for example PCF8575.INT - gpio6.11 - PCFx driver knows nothing about type of IRQ line (GPIO or not) so it doesn't request gpio and just do request_irq() - If gpio6.11 will be exported through the sysfs and then un-xeported then IRQs from PCFx will not be received any more, because IRQ configuration for gpio6.11 will be cleaned up unconditionally in omap_gpio_free. Fix this by removing all GPIO IRQ specific code from omap_gpio_free() and also do GPIO clean up (change direction to 'in' and disable debounce) only if corresponding GPIO is not used as IRQ too. GPIO IRQ will be properly cleaned up by GPIO irqchip code. Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org Can I get an ACK or comment from one of the three (!) maintainers on atleast these non-RFC patches? Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v1 05/25] gpio: Use irq_desc_get_xxx() to avoid redundant lookup of irq_desc
On Wed, May 20, 2015 at 11:59 AM, Jiang Liu jiang@linux.intel.com wrote: Use irq_desc_get_xxx() to avoid redundant lookup of irq_desc while we already have a pointer to corresponding irq_desc. Signed-off-by: Jiang Liu jiang@linux.intel.com Acked-by: Linus Walleij linus.wall...@linaro.org Are there dependencies on this patch or can I just apply it directly to the GPIO tree? Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/RFT PATCH 0/7] gpio: omap: rework and fixes
On Fri, May 22, 2015 at 9:03 PM, Tony Lindgren t...@atomide.com wrote: * Grygorii Strashko grygorii.stras...@linaro.org [150522 07:37]: Patches 1-5 seem to work for me, patch 6 does not. So for patches 1-5, please feel free to add: Tested-by: Tony Lindgren t...@atomide.com OK I take this as an excuse to apply patches 1-5 with Tony's test tag. The maintainers can cheer in if they want, I will anyway take the OMAP maintainers test tag as a good quality indication. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/8] gpio: omap: drop 'gpio' param from omap_gpio_init_irq()
On Mon, Mar 23, 2015 at 1:18 PM, grygorii.stras...@linaro.org wrote: From: Grygorii Strashko grygorii.stras...@linaro.org The 'gpio' parameter isn't needed any more as it duplicates 'offset' parameter, so drop it. Tested-by: Tony Lindgren t...@atomide.com Tested-by: Aaro Koskinen aaro.koski...@iki.fi Acked-by: Santosh Shilimkar ssant...@kernel.org Acked-by: Javier Martinez Canillas jav...@dowhile0.org Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/8] gpio: omap: convert omap_gpio_is_input() to use gpio offset
On Mon, Mar 23, 2015 at 1:18 PM, grygorii.stras...@linaro.org wrote: From: Grygorii Strashko grygorii.stras...@linaro.org Convert omap_gpio_is_input() to use GPIO offset instead of mask and, in such way, make code simpler and remove few lines of code. Tested-by: Tony Lindgren t...@atomide.com Tested-by: Aaro Koskinen aaro.koski...@iki.fi Acked-by: Santosh Shilimkar ssant...@kernel.org Acked-by: Javier Martinez Canillas jav...@dowhile0.org Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 8/8] gpio: omap: get rid of GPIO_INDEX() macro
On Mon, Mar 23, 2015 at 1:18 PM, grygorii.stras...@linaro.org wrote: From: Grygorii Strashko grygorii.stras...@linaro.org Now OMAP GPIO driver prepared for GPIO_INDEX() macro removing. Do It ;) Tested-by: Tony Lindgren t...@atomide.com Tested-by: Aaro Koskinen aaro.koski...@iki.fi Acked-by: Santosh Shilimkar ssant...@kernel.org Acked-by: Javier Martinez Canillas jav...@dowhile0.org Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/8] gpio: omap: convert gpio irq functions to use GPIO offset
On Mon, Mar 23, 2015 at 1:18 PM, grygorii.stras...@linaro.org wrote: From: Grygorii Strashko grygorii.stras...@linaro.org Convert GPIO IRQ functions to use GPIO offset instead of system GPIO numbers. This allows to drop unneeded conversations between system GPIO - GPIO offset which are done in many places and many times. It is safe to do now because: - gpiolib always passes GPIO offset to GPIO controller - OMAP GPIO driver converted to use IRQ domain, so struct irq_data-hwirq contains GPIO offset This is preparation step before removing: #define GPIO_INDEX(bank, gpio) #define GPIO_BIT(bank, gpio) int omap_irq_to_gpio() Tested-by: Tony Lindgren t...@atomide.com Tested-by: Aaro Koskinen aaro.koski...@iki.fi Acked-by: Santosh Shilimkar ssant...@kernel.org Acked-by: Javier Martinez Canillas jav...@dowhile0.org Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/8] gpio: omap: get rid of GPIO_BIT() macro
On Mon, Mar 23, 2015 at 1:18 PM, grygorii.stras...@linaro.org wrote: From: Grygorii Strashko grygorii.stras...@linaro.org Now OMAP GPIO driver prepared for GPIO_BIT() macro removing. Do it ;) Tested-by: Tony Lindgren t...@atomide.com Tested-by: Aaro Koskinen aaro.koski...@iki.fi Acked-by: Santosh Shilimkar ssant...@kernel.org Acked-by: Javier Martinez Canillas jav...@dowhile0.org Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] gpio: omap: get rid of omap_irq_to_gpio()
On Mon, Mar 23, 2015 at 1:18 PM, grygorii.stras...@linaro.org wrote: From: Grygorii Strashko grygorii.stras...@linaro.org Now OMAP GPIO driver prepared for omap_irq_to_gpio() removing. Do it ;) Tested-by: Tony Lindgren t...@atomide.com Tested-by: Aaro Koskinen aaro.koski...@iki.fi Acked-by: Santosh Shilimkar ssant...@kernel.org Acked-by: Javier Martinez Canillas jav...@dowhile0.org Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/8] gpio: omap: simplify omap_set_gpio_dataout_x()
On Mon, Mar 23, 2015 at 1:18 PM, grygorii.stras...@linaro.org wrote: From: Grygorii Strashko grygorii.stras...@linaro.org Both functions omap_set_gpio_dataout_reg() and omap_set_gpio_dataout_mask() accept GPIO offset as 'gpio' input parameter, so rename it to 'offset' and drop usage of GPIO_BIT() macro. Tested-by: Tony Lindgren t...@atomide.com Tested-by: Aaro Koskinen aaro.koski...@iki.fi Acked-by: Santosh Shilimkar ssant...@kernel.org Acked-by: Javier Martinez Canillas jav...@dowhile0.org Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/8] gpio: omap: convert debounce functions switch to use gpio offset
On Mon, Mar 23, 2015 at 1:18 PM, grygorii.stras...@linaro.org wrote: From: Grygorii Strashko grygorii.stras...@linaro.org Convert debounce functions to use GPIO offset instead of system GPIO numbers. This allows to drop unneeded conversations between system GPIO - GPIO offset which are done in many places and many times. It is safe to do now because: - gpiolib always passes GPIO offset to GPIO controller - OMAP GPIO driver converted to use IRQ domain This is preparation step before removing: #define GPIO_INDEX(bank, gpio) #define GPIO_BIT(bank, gpio) int omap_irq_to_gpio() Tested-by: Tony Lindgren t...@atomide.com Tested-by: Aaro Koskinen aaro.koski...@iki.fi Acked-by: Santosh Shilimkar ssant...@kernel.org Acked-by: Javier Martinez Canillas jav...@dowhile0.org Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: pcf857x: restore the initial line state of all pcf lines
On Wed, Mar 18, 2015 at 2:21 PM, Kishon Vijay Abraham I kis...@ti.com wrote: [Me] Specify exactly what stuff may happen after the reboot notifier. okay, it's assumed the device may be used (active) till the shutdown handler of that particular device is called. In this particular case we are restoring the initial line state of all pcf lines even though we don't know if the devices connected to it are still being active, which might cause a contention as explained by NM [1] OK I understand this, just that I want to know if it is a practical problem, i.e. do you run into it? Does the system lock up? If the reboot notifier gets invoked after the shutdown handler then we could be sure that restoring the initial line state of the pcf lines wouldn't affect the devices connected to it. OK so does it? ftrace is your friend. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: pcf857x: restore the initial line state of all pcf lines
On Mon, Mar 16, 2015 at 9:46 AM, Kishon Vijay Abraham I kis...@ti.com wrote:
On Wednesday 14 January 2015 05:28 PM, Linus Walleij wrote:
#include linux/reboot.h
static int foo_reboot_handler(struct notifier_block *this,
unsigned long code,
void *unused)
{
pr_crit(do some last minute stuff\n);
return NOTIFY_OK;
}
static struct notifier_block foo_reboot_notifier = {
.notifier_call = foo_reboot_handler,
};
register_reboot_notifier(foo_reboot_notifier);
Added debug prints and found the reboot notifier gets invoked before the
shutdown handler which means some stuff can be done after this reboot
notifier:-(
Specify exactly what stuff may happen after the reboot notifier.
Of course stuff happens both before and after the reboot
notifier, the question is exactly what, that may conflict with this.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] pinctrl: bindings: pinctrl: Add support for TI's IODelay configuration
On Tue, Mar 10, 2015 at 7:33 PM, Nishanth Menon n...@ti.com wrote:
On 03/10/2015 12:31 PM, Tony Lindgren wrote:
Yes except I'd make use of some kind of #pinctrl-cells here just like
interrupt controller has #interrupt-cells. Then you can have the values
seprate and the controller knows what to do with them based on the
compatible flag and #pinctrl-cells.
Something like the following I suppose, where pinctrl-cells is optional?
dra7_pmx_core: pinmux@4a003400 {
compatible = ti,dra7-padconf, pinctrl-single;
reg = 0x4a003400 0x0464;
#address-cells = 1;
#size-cells = 0;
#interrupt-cells = 1;
interrupt-controller;
pinctrl-single,register-width = 32;
pinctrl-single,function-mask = 0x3fff;
};
dra7_iodelay_core: padconf@4844a000 {
compatible = ti,dra7-iodelay;
reg = 0x4844a000 0x0d1c;
#address-cells = 1;
#size-cells = 0;
#pinctrl-cells = 2;
};
Linus,
I hope you are ok with the above?
Hm depends on where the documentation hits I guess?
Such a generic cell count property has to be to the generic
pinctrl-bindings.txt document if I read it right.
Overall I guess this will be acceptable but you really need to
reuse some more code between this driver and pinctrl-single.c
if I read it right.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] pinctrl: bindings: pinctrl: Add support for TI's IODelay configuration
On Wed, Mar 4, 2015 at 1:00 AM, Nishanth Menon n...@ti.com wrote:
+Configuration definition follows similar model as the pinctrl-single:
+The groups of pin configuration are defined under pinctrl-single,pins
+
+dra7_iodelay_core {
+ mmc2_iodelay_3v3_conf: mmc2_iodelay_3v3_conf {
+ pinctrl-single,pins =
+ 0x18c (A_DELAY(0) | G_DELAY(120)) /*
CFG_GPMC_A19_IN */
+ 0x1a4 (A_DELAY(265) | G_DELAY(360)) /*
CFG_GPMC_A20_IN */
+ 0x1b0 (A_DELAY(0) | G_DELAY(120)) /*
CFG_GPMC_A21_IN */
+ 0x1bc (A_DELAY(0) | G_DELAY(120)) /*
CFG_GPMC_A22_IN */
+ 0x1c8 (A_DELAY(287) | G_DELAY(420)) /*
CFG_GPMC_A23_IN */
+ 0x1d4 (A_DELAY(144) | G_DELAY(240)) /*
CFG_GPMC_A24_IN */
+ 0x1e0 (A_DELAY(0) | G_DELAY(0)) /*
CFG_GPMC_A25_IN */
+ 0x1ec (A_DELAY(120) | G_DELAY(0)) /*
CFG_GPMC_A26_IN */
+ 0x1f8 (A_DELAY(120) | G_DELAY(180)) /*
CFG_GPMC_A27_IN */
+ 0x360 (A_DELAY(0) | G_DELAY(0)) /*
CFG_GPMC_CS1_IN */
+ ;
+ };
+};
But wait.
The promise when we merged pinctrl-single was that this driver was to be used
when the system had a one-register-per-pin layout and it was easy to do device
trees based on that.
We were very reluctant to accept that even though we didn't even have the
generic pin control bindings in place, the argument being that the driver
should know the detailed register layout, it should not be described in the
device tree. We eventually caved in and accepted it as an exception.
Since this pin controller is not using pinctrl-single it does not enjoy that
privilege and you need to explain why this pin controller cannot use the
generic bindings like so many other pin controllers have since started
to do. (It is in the same SoC is not an acceptable argument.)
What is wrong with this:
Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
We can add generic delay bindings to the list, it even seems like
a good idea to do so, as it is likely something that will come up in
other hardware and will be needed for ACPI etc going forward.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] pinctrl: Introduce TI IOdelay configuration driver
PLL or something?
+ r = regmap_read(iod-regmap, reg-reg_coarse_offset, val);
+ if (r)
+ return r;
+ ival-coarse_ref_count =
+ ti_iodelay_extract(val, reg-coarse_ref_count_mask);
+ ival-coarse_delay_count =
+ ti_iodelay_extract(val, reg-coarse_delay_count_mask);
+ if (!ival-coarse_delay_count) {
+ dev_err(dev, Invalid Coarse delay count (0) (reg=0x%08x)\n,
+ val);
+ return -EINVAL;
+ }
+ ival-cdpe = ti_iodelay_compute_dpe(ival-ref_clk_period,
+ ival-coarse_ref_count,
+ ival-coarse_delay_count, 88);
88? What does that mean... This should be #defined I think.
+static int ti_iodelay_dt_node_to_map(struct pinctrl_dev *pctldev,
+struct device_node *np,
+struct pinctrl_map **map,
+unsigned *num_maps)
If you use the generic bindings this quite complex parsing and
everything associated with it goes away into the core.
+static void ti_iodelay_dt_free_map(struct pinctrl_dev *pctldev,
+ struct pinctrl_map *map, unsigned num_maps)
Dito.
BTW even if you proceed this way I suspect these are verbatim
copies from pinctrl-single so they shold obviouslyt be abstracted
out as library functions in that case.
+/**
+ * ti_iodelay_pinctrl_get_group_pins() - get group pins
+ * @pctldev: pinctrl device representing IODelay device
+ * @gselector: Group selector
+ * @pins: pointer to the pins
+ * @npins: number of pins
+ *
+ * Dummy implementation since we do not track pins, we track configurations
+ * Forced by pinctrl's pinctrl_check_ops()
+ *
+ * Return: -EINVAL
+ */
+static int ti_iodelay_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
+unsigned gselector,
+const unsigned **pins,
+unsigned *npins)
+{
+ /* Dummy implementation - we dont do pin mux */
+ return -EINVAL;
+}
This is not about muxing. It is about enumerating the pins in the
group that will be affected by the setting, which is something you
will want to do when using the generic bindings.
+ group = ti_iodelay_get_group(iod, gselector);
And this seems to be your re-implementation of exactly that pin control
core functionality. Which again should be shared with pinctrl-single if
you anyway go down this path.
I'll halt review here pending discussion on the bindings stuff.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] gpio: omap: irq_shutdown: remove unnecessary call of gpiochip_unlock_as_irq
On Fri, Mar 6, 2015 at 8:26 PM, grygorii.stras...@linaro.org wrote: From: Grygorii Strashko grygorii.stras...@linaro.org GPIOLib core implemnts irqchip-irq_request/release_resources callbacks internally and these callbacks already contain clalls of gpiochip_lock/unlock_as_irq(). Hence, remove unnecessary call of gpiochip_unlock_as_irq() from omap_gpio_irq_shutdown(). Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org Looks like a simple oversight. Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] gpio: omap: Fix bad device access with setup_irq()
On Fri, Jan 16, 2015 at 11:50 PM, Tony Lindgren t...@atomide.com wrote: Similar to omap_gpio_irq_type() let's make sure that the GPIO is usable as an interrupt if the platform init code did not call gpio_request(). Otherwise we can get invalid device access after setup_irq(): WARNING: CPU: 0 PID: 1 at drivers/bus/omap_l3_noc.c:147 l3_interrupt_handler+0x214/0x340() 4400.ocp:L3 Custom Error: MASTER MPU TARGET L4CFG (Idle): Data Access in Supervisor mode during Functional access ... [c05f21e4] (__irq_svc) from [c05f1974] (_raw_spin_unlock_irqrestore+0x34/0x44) [c05f1974] (_raw_spin_unlock_irqrestore) from [c00914a8] (__setup_irq+0x244/0x530) [c00914a8] (__setup_irq) from [c00917d4] (setup_irq+0x40/0x8c) [c00917d4] (setup_irq) from [c0039c8c] (omap_system_dma_probe+0x1d4/0x2b4) [c0039c8c] (omap_system_dma_probe) from [c03b2200] (platform_drv_probe+0x44/0xa4) ... We can fix this the same way omap_gpio_irq_type() is handling it. Note that the long term solution is to change the gpio-omap driver to handle the banks as separate driver instances. This will allow us to rely on just runtime PM for tracking the bank specific state. Reported-by: Russell King rmk+ker...@arm.linux.org.uk Cc: Felipe Balbi ba...@ti.com Cc: Javier Martinez Canillas jav...@dowhile0.org Cc: Kevin Hilman khil...@kernel.org Cc: Linus Walleij linus.wall...@linaro.org Cc: Russell King - ARM Linux li...@arm.linux.org.uk Cc: Santosh Shilimkar ssant...@kernel.org Signed-off-by: Tony Lindgren t...@atomide.com Patch applied for fixes with Felipe's Tested-by. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: pcf857x: restore the initial line state of all pcf lines
On Mon, Jan 5, 2015 at 7:26 AM, Kishon Vijay Abraham I kis...@ti.com wrote:
On Thursday 18 December 2014 07:41 PM, Nishanth Menon wrote:
On 12/18/2014 12:18 AM, Kishon Vijay Abraham I wrote:
On Tuesday 16 December 2014 02:20 AM, Nishanth Menon wrote:
On 12/12/2014 02:06 AM, Kishon Vijay Abraham I wrote:
The reset values for all the PCF lines are high and hence on shutdown
we should drive all the lines high in order to bring it to the reset
state.
This is actually required since pcf doesn't have a reset line and even
after
warm reset (by invoking reboot in prompt) the pcf lines maintains it's
previous programmed state. This becomes a problem if the boards are
designed
to work with the default initial state.
DRA7XX_evm uses PCF8575 and one of the PCF output lines feeds to MMC/SD
and
this line should be driven high in order for the MMC/SD to be detected.
This line is modelled as regulator and the hsmmc driver takes care of
enabling
and disabling it. In the case of 'reboot', during shutdown path as part
of it's
cleanup process the hsmmc driver disables this regulator. This makes MMC
boot
not functional.
Fixed it by driving high all the pcf lines.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
drivers/gpio/gpio-pcf857x.c |9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 236708a..00b15b2 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -448,6 +448,14 @@ static int pcf857x_remove(struct i2c_client *client)
return status;
}
+static void pcf857x_shutdown(struct i2c_client *client)
+{
+ struct pcf857x *gpio = i2c_get_clientdata(client);
+
+ /* Drive all the I/O lines high */
+ gpio-write(gpio-client, BIT(gpio-chip.ngpio) - 1);
you might force a contention here - depending on System configuration.
example:
+---+
| |
| U1 | +--+ +---+
| +- | | |
+---+ | | | |
| Switch-+SoC|
+---+ | | | |
| | | | | |
| U2-+--^---+ +---+
| ||
| ||
+---+|
+--+--+
| |
| PCF |
| |
+-+
At low, SoC pin is connected to U2 as drive. when reset to high, you
now have U1 driving to the same pin that SoC has, potentially
resulting in contention.
Unfortunately, at this level, you do not know what the state of the
system is, blindly forcing a pin level will potentially cause
contention risk depending on pin configuration.
Assume we are doing a reset when the system is powered on, irrespective of
the
state of the system, we'll be forcing the pin level to the default state.
Yes, I dont deny that system will be fine *after* reset sequence is
started or completed. However there is a duration between the pcf
shutdown handler is called and the final reset handler is invoked -
that is the duration when the contention might cause device behavior.
Essentially ignoring the state various drivers have asked PCF to setup
the pins and doing a hands down configuration may have side effects we
cant properly expect.
The solution might be to invoke the shutdown handler of the various drivers
using the PCF before the shutdown handler of the PCF driver itself gets
invoked? But I'm not sure how can that be achieved in linux kernel :-(
#include linux/reboot.h
static int foo_reboot_handler(struct notifier_block *this,
unsigned long code,
void *unused)
{
pr_crit(do some last minute stuff\n);
return NOTIFY_OK;
}
static struct notifier_block foo_reboot_notifier = {
.notifier_call = foo_reboot_handler,
};
register_reboot_notifier(foo_reboot_notifier);
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 14/21] ARM: imx6: convert GPC to stacked domains
On Mon, Jan 12, 2015 at 7:26 PM, Marc Zyngier marc.zyng...@arm.com wrote: IMX6 has been (ab)using the gic_arch_extn to provide wakeup from suspend, and it makes a lot of sense to convert this code to use stacked domains instead. This patch does just this, updating the DT files to actually reflect what the HW provides. BIG FAT WARNING: because the DTs were so far lying by not exposing the fact that the GPC block is actually the first interrupt controller in the chain, kernels with this patch applied wont have any suspend-resume facility when booted with old DTs, and old kernels with updated DTs won't even boot. Tested-by: Stefan Agner ste...@agner.ch Acked-by: Stefan Agner ste...@agner.ch Signed-off-by: Marc Zyngier marc.zyng...@arm.com (...) +static int imx_gpc_domain_alloc(struct irq_domain *domain, + unsigned int virq, Nutcase nitpick on this nice patch series: every time I see virq my OCD triggers, as I think the v in virq stand for virtual. These irqs are no more virtual than any other Linux irq numbers, hwirq is more to the point. I just refer to these as irq (sans v) in any code I write. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio / PM: Replace CONFIG_PM_RUNTIME with CONFIG_PM
On Wed, Dec 3, 2014 at 2:50 AM, Rafael J. Wysocki r...@rjwysocki.net wrote: From: Rafael J. Wysocki rafael.j.wyso...@intel.com After commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if PM_SLEEP is selected) PM_RUNTIME is always set if PM is set, so #ifdef blocks depending on CONFIG_PM_RUNTIME may now be changed to depend on CONFIG_PM. Replace CONFIG_PM_RUNTIME with CONFIG_PM in drivers/gpio/gpio-omap.c. Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com --- Note: This depends on commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if PM_SLEEP is selected) which is only in linux-next at the moment (via the linux-pm tree). Please let me know if it is OK to take this one into linux-pm. Go ahead. Should be no collissions. Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] usb: musb: Pass fifo_mode in platform data
On Mon, Nov 24, 2014 at 8:05 PM, Tony Lindgren t...@atomide.com wrote: This allows setting the correct fifo_mode when multiple MUSB glue layers are built-in. Cc: Fabio Baltieri fabio.balti...@linaro.org Cc: Lee Jones lee.jo...@linaro.org Cc: Linus Walleij linus.wall...@linaro.org Cc: Apelete Seketeli apel...@seketeli.net Cc: Lars-Peter Clausen l...@metafoo.de Signed-off-by: Tony Lindgren t...@atomide.com Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/8] usb: musb: Change end point selection to use new IO access
On Mon, Nov 24, 2014 at 8:05 PM, Tony Lindgren t...@atomide.com wrote: This allows the endpoints to work when multiple MUSB glue layers are built in. Cc: Fabio Baltieri fabio.balti...@linaro.org Cc: Lee Jones lee.jo...@linaro.org Cc: Linus Walleij linus.wall...@linaro.org Cc: Apelete Seketeli apel...@seketeli.net Cc: Lars-Peter Clausen l...@metafoo.de Signed-off-by: Tony Lindgren t...@atomide.com Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 6/8] net: can: c_can: Disable pins when CAN interface is down
On Fri, Nov 14, 2014 at 4:40 PM, Roger Quadros rog...@ti.com wrote: DRA7 CAN IP suffers from a problem which causes it to be prevented from fully turning OFF (i.e. stuck in transition) if the module was disabled while there was traffic on the CAN_RX line. To work around this issue we select the SLEEP pin state by default on probe and use the DEFAULT pin state on CAN up and back to the SLEEP pin state on CAN down. Signed-off-by: Roger Quadros rog...@ti.com Reviewed-by: Linus Walleij linus.wall...@linaro.org I see you figured it out all by yourselves :D (Sorry for being absent.) +#include linux/pinctrl/consumer.h + pinctrl_pm_select_default_state(dev-dev.parent); + pinctrl_pm_select_sleep_state(dev-dev.parent); + pinctrl_pm_select_sleep_state(dev-dev.parent); NB: in drivers/base/pinctrl.c: #ifdef CONFIG_PM /* * If power management is enabled, we also look for the optional * sleep and idle pin states, with semantics as defined in * linux/pinctrl/pinctrl-state.h */ dev-pins-sleep_state = pinctrl_lookup_state(dev-pins-p, PINCTRL_STATE_SLEEP); So if these states are necessary for the driver to work, put depends on PM or select PM in the Kconfig. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 6/8] net: can: c_can: Disable pins when CAN interface is down
On Fri, Nov 14, 2014 at 2:43 PM, Roger Quadros rog...@ti.com wrote: On 11/13/2014 06:03 PM, Marc Kleine-Budde wrote: On 11/13/2014 04:23 PM, Roger Quadros wrote: I just stumbled over pinctrl_pm_select_sleep_state(), is it possible to integrate this into runtime pm? http://lxr.free-electrons.com/source/drivers/pinctrl/core.c#L1282 I think those functions are there for the same reason but not sure why aren't they used in runtime pm core. Linus W. any hints? It is not used from PM core because there are cases where you may want to put pins to sleep for completely PM-core unrelated things. Things like turning off a serial port from userspace, for example. That should put the pins to sleep. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] usb: musb: Change to use new IO access
On Mon, Nov 24, 2014 at 8:05 PM, Tony Lindgren t...@atomide.com wrote: Change to use new IO access. This allows us to build in multiple MUSB glue layers. Cc: Fabio Baltieri fabio.balti...@linaro.org Cc: Lee Jones lee.jo...@linaro.org Cc: Linus Walleij linus.wall...@linaro.org Signed-off-by: Tony Lindgren t...@atomide.com Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] kbuild: remove unnecessary variable initializaions
On Tue, Sep 9, 2014 at 12:26 PM, Masahiro Yamada yamad...@jp.panasonic.com wrote: Clearing obj-y, obj-m, obj-n, obj- in each Makefile is a useless habit. They are non-exported variables; therefore they are always empty whenever descending into each subdirectory. (Moreorver, obj-y and obj-m are also set to empty at the beginning of scripts/Makefile.build) Signed-off-by: Masahiro Yamada yamad...@jp.panasonic.com Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH] gpio: omap: Fix interrupt names
On Fri, Sep 5, 2014 at 9:52 PM, Nishanth Menon n...@ti.com wrote: When viewing the /proc/interrupts, there is no information about which GPIO bank a specific gpio interrupt is hooked on to. This is more than a bit irritating as such information can esily be provided back to the user and at times, can be crucial for debug. So, instead of displaying something like: 31: 0 0 GPIO 0 palmas 32: 0 0 GPIO 27 mmc0 Display the following with appropriate device name: 31: 0 0 4ae1.gpio 0 palmas 32: 0 0 4805d000.gpio 27 mmc0 This requires that we create irq_chip instance specific for each GPIO bank which is trivial to achieve. Signed-off-by: Nishanth Menon n...@ti.com Acked-by: Santosh Shilimkar santosh.shilim...@ti.com Acked-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Acked-by: Kevin Hilman khil...@linaro.org --- Requested to be resend by Javier with linux-gpio maintainers in CC. Original V1 of the patch: https://patchwork.kernel.org/patch/4757891/ Probably belongs to 3.18 kernel series at this point in time. Changes since v1: just picked up Acks. Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] pinctrl: bindings: Add OMAP pinctrl binding
On Mon, Aug 25, 2014 at 9:03 PM, Nishanth Menon n...@ti.com wrote: ---8--- From 74121c6a2524048eb02c3b33a25e13261edd2e99 Mon Sep 17 00:00:00 2001 From: Nishanth Menon n...@ti.com Date: Thu, 22 May 2014 23:32:09 -0500 Subject: [PATCH V2] pinctrl: bindings: Add OMAP pinctrl binding Add basic skeleton of OMAP pinctrl bindings. This is compatible with pinctrl,single bindings and is meant purely as a reference point. Acked-by: Tony Lindgren t...@atomide.com Signed-off-by: Nishanth Menon n...@ti.com Applied this inline v2 version. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] pinctrl: single: AM437x: Add pinctrl compatibility
On Fri, Aug 22, 2014 at 4:01 PM, Nishanth Menon n...@ti.com wrote: From: Keerthy j-keer...@ti.com AM437x pinctrl definitions now differ from traditional 16 bit OMAP pin ctrl definitions, in that all 32 bits are used to describe a single pin Also the location of wakeupenable and event bits have changed. Signed-off-by: Keerthy j-keer...@ti.com [n...@ti.com: minor updates] Signed-off-by: Nishanth Menon n...@ti.com Patch applied with Tony's ACK. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] pinctrl: single: Add DRA7 pinctrl compatibility
On Fri, Aug 22, 2014 at 4:01 PM, Nishanth Menon n...@ti.com wrote: DRA7 pinctrl definitions now differ from traditional 16 bit OMAP pin ctrl definitions, in that all 32 bits are used to describe a single pin Also the location of wakeupenable and event bits have changed. Signed-off-by: Nishanth Menon n...@ti.com Patch applied with Tony's ACK. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] gpio: omap: Remove unnecessary lockdep class
On Fri, Jun 27, 2014 at 10:17 PM, Javier Martinez Canillas jav...@dowhile0.org wrote: GPIO irqchips assign to the cascaded IRQs their own lock class in order to avoid warnings about lockdep recursions since that allow the lockdep core to keep track of things. Since commit e45d1c80 (gpio: put GPIO IRQs into their own lock class) there is no need to do this in a driver if it's using the GPIO irqchip helpers since gpiolib already assigns a lockdep class. Signed-off-by: Javier Martinez Canillas jmarti...@softcrates.net That's right. Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] gpio: omap: Remove unneeded include
On Fri, Jun 27, 2014 at 10:17 PM, Javier Martinez Canillas jav...@dowhile0.org wrote: The linux/irqchip/chained_irq.h header is already included when selecting GPIOLIB_IRQCHIP so there is no need to do it in the driver. This is a left over from commit fb655f5 (gpio: omap: convert driver to use gpiolib irqchip). Signed-off-by: Javier Martinez Canillas jmarti...@softcrates.net Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] gpio: omap: Add an omap prefix to all functions
On Fri, Jun 27, 2014 at 10:17 PM, Javier Martinez Canillas jav...@dowhile0.org wrote: The GPIO OMAP driver didn't have a consistent naming scheme for all its functions. Some of them had an omap prefix while others didn't. There are many advantages on having a separate namespace for driver functions so let's add an omap prefix to all of them. Signed-off-by: Javier Martinez Canillas jmarti...@softcrates.net Yeah that was very disturbing, patch applied, thanks! Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/6] ARM: mm: cache-l2x0: Add base address argument to write_sec callback
On Wed, Jun 25, 2014 at 3:37 PM, Tomasz Figa t.f...@samsung.com wrote: For certain platforms (e.g. Exynos) it is necessary to read back some values from registers before they can be written (i.e. SMC calls that set multiple registers per call), so base address of L2C controller is needed for .write_sec operation. This patch adds base argument to .write_sec callback so that its implementation can also access registers directly. Signed-off-by: Tomasz Figa t.f...@samsung.com arch/arm/mach-ux500/cache-l2x0.c | 3 ++- In our case just changing the signature of the function I see so: Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] gpio: of: Allow -gpio suffix for property names
On Tue, Jun 3, 2014 at 1:14 AM, Tony Lindgren t...@atomide.com wrote: Looks like something like below fixes the issue. Regards, Tony 8 --- From: Tony Lindgren t...@atomide.com Date: Mon, 2 Jun 2014 16:13:46 -0700 Subject: [PATCH] gpio: of: Fix handling for deferred probe for -gpio suffix Commit dd34c37aa3e (gpio: of: Allow -gpio suffix for property names) added parsing for both -gpio and -gpios suffix but also changed the handling for deferred probe unintentionally. Because of the looping the second name will now return -ENOENT instead of -EPROBE_DEFER. Fix the issue by breaking out of the loop if -EPROBE_DEFER is encountered. Signed-off-by: Tony Lindgren t...@atomide.com Patch applied for fixes with Thierry's Reveiwed-by tag. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Accessing GPIOs from userspace using recent kernels
Hi Peter, I think you have already understood from the rest of the conversation that pin control configuration shall be done in the device tree and not from userspace, which is a good start. As shown by Javier many things people sometimes do in userspace should rather be done in kernelspace, such as controlling LEDs and reading pushbuttons etc. On Fri, May 16, 2014 at 12:15 PM, Peter TB Brett pe...@peter-b.co.uk wrote: On 2014-05-15 19:54, Javier Martinez Canillas wrote: 0x5e (PIN_INPUT_PULLUP | MUX_MODE0) /* hdmi_sda.hdmi_sda */ 0x12e (PIN_OUTPUT | MUX_MODE5) /* dispc2_data15 */ Yes, that's my mistake. I took the pin control register addresses from the OMAP4 documentation and forgot that you have to subtract 0x40 to get the correct address for use in the device tree. The correct snippet has 0x1e and 0xee. I find the pinctrl-single hexdigit syntax infinitely complex and confusing, but it was chosen by its designers. Most drivers use strings to configure muxing and biasing etc. Is what you shared your real change or just an example that does not apply to the Pandaboard? Could you please share your complete DTS? The attached .dts file sort of works-ish. It's an ugly hack, but I don't have the time to do any more investigation into this now, unfortunately. I guess my main question is: if I use /sys/class/gpio/export to export a GPIO for userspace control, Which you should avoid, if possible. it would make sense for the kernel to try and ensure that the GPIO is actually connected to something! How should we do that? The physics of that request evades me. The pin control subsystem will however refuse to use the pin if it is used for something else. The current call chain seems to be: gpiod_export() -- gpiod_request() -- omap_gpio_request(). Looking at other GPIO drivers, it seems like omap_gpio_request() should eventually call pinctrl_request_gpio(). Would be useful if someone who knows about OMAP4/gpio/pinctrl could take a look at this. That is Tony Lindgren and the linux-omap mailing list. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] gpio: omap: prepare and unprepare the debounce clock
On Thu, May 8, 2014 at 9:06 AM, Rajendra Nayak rna...@ti.com wrote: Do you mind picking this fix up via the GPIO tree? Yes, it's merged to my devel branch now with the ACKs. Alternatively you could Ack this if you are fine and we can take both Patch 1/2 and Patch 2/2 from this series via the OMAP tree. This probably will not work as I have a set of other changes to this driver in my tree. Patch 2/2 has a dependency on Patch 1/2 and they need to go in in that order else gpio would break. More discussions are here [1]. Tell me if I should prepare an immutable tag on my branch that you can pull in. I want an explicit handshake with the platform maintainer for this kind of stuff. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: always enable GPIO_OMAP on ARCH_OMAP
On Mon, Apr 28, 2014 at 11:07 AM, Arnd Bergmann a...@arndb.de wrote: Commit 4df42de9d3e gpio: omap: add a GPIO_OMAP option instead of using ARCH_OMAP made it possible to build OMAP kernels without the GPIO driver, which at least on OMAP2 and OMAP3 causes build errors because of functions used by the platform power management code: arch/arm/mach-omap2/built-in.o: In function `omap_sram_idle': arch/arm/mach-omap2/pm24xx.c:129: undefined reference to `omap2_gpio_prepare_for_idle' arch/arm/mach-omap2/pm24xx.c:129: undefined reference to `omap2_gpio_resume_after_idle' We presumably always want the GPIO driver on OMAP, so this adds a slightly broader dependency and only allows disabling the driver only when no OMAP2PLUS platform is selected. However, it seems entirely reasonable to include the driver in build tests on other platforms, so we should also allow building it for COMPILE_TEST builds and select the required GENERIC_IRQ_CHIP that may not already be enabled on other platforms. Signed-off-by: Arnd Bergmann a...@arndb.de Patch applied to my devel branch, this is not a fix to Torvalds HEAD AFAICT, but a fix to v3.16? Else poke me. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] backlight: gpio-backlight: Fix warning when the GPIO is on a I2C chip
On Fri, May 9, 2014 at 5:42 AM, Jingoo Han jg1@samsung.com wrote: Linus Walleij, Is there any reason to keep these two functions such as gpiod_set_raw_value_cansleep() and gpiod_set_raw_value()? Yes, the former can *not* be called from interrupt context, and thus erroneous usage can be detected with runtime checks. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] gpio: omap: implement get_direction
On Thu, Apr 24, 2014 at 8:57 AM, yegorsli...@googlemail.com wrote:
From: Yegor Yefremov yegorsli...@googlemail.com
This patch implements gpio_chip's get_direction() routine, that
lets other drivers get particular GPIOs direction using
struct gpio_desc.
Signed-off-by: Yegor Yefremov yegorsli...@googlemail.com
Acked-by: Javier Martinez Canillas jav...@dowhile0.org
---
Changes:
v3: get rid of _get_gpio_direction() (Linus Walleij)
v2: rework return value calculation
Looks good to me, Kevin, Santosh?
Part of me wants to list Javier as maintainer for this driver.
+static int gpio_get_direction(struct gpio_chip *chip, unsigned offset)
+{
+ struct gpio_bank *bank;
+ unsigned long flags;
+ void __iomem *reg;
+ int dir;
That is a bool, actually. But no big deal.
+
+ bank = container_of(chip, struct gpio_bank, chip);
+ reg = bank-base + bank-regs-direction;
+ spin_lock_irqsave(bank-lock, flags);
+ dir = !!(readl_relaxed(reg) BIT(offset));
+ spin_unlock_irqrestore(bank-lock, flags);
+ return dir;
+}
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] gpio: omap: implement get_direction
On Thu, Apr 24, 2014 at 8:57 AM, yegorsli...@googlemail.com wrote: From: Yegor Yefremov yegorsli...@googlemail.com This patch implements gpio_chip's get_direction() routine, that lets other drivers get particular GPIOs direction using struct gpio_desc. Signed-off-by: Yegor Yefremov yegorsli...@googlemail.com Acked-by: Javier Martinez Canillas jav...@dowhile0.org --- Changes: v3: get rid of _get_gpio_direction() (Linus Walleij) v2: rework return value calculation This v3 version applied with Santosh's ACK. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regressions in linux-next?
On Wed, Apr 23, 2014 at 9:24 AM, Javier Martinez Canillas jav...@dowhile0.org wrote: Linus, what do you think of the following patch? From ede333e85e0320d32e8c2d123560808ed7e43ece Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas javier.marti...@collabora.co.uk Date: Wed, 23 Apr 2014 09:13:54 +0200 Subject: [PATCH 1/1] gpio: don't call irq_set_irq_type() on IRQ domain map function (...) So no setting a default type in the mapping function... - irq_set_irq_type(irq, chip-irq_default_type); But there are drivers exploiting this to set up the hardware to some default state :-( What about this: if (chip-irq_default_type != IRQ_TYPE_NONE) irq_set_irq_type(irq, chip-irq_default_type); This way you can pass IRQ_TYPE_NONE and nothing happens in the mapping. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] gpio: omap: implement get_direction
On Tue, Apr 22, 2014 at 3:31 PM, yegorsli...@googlemail.com wrote:
From: Yegor Yefremov yegorsli...@googlemail.com
This patch implements gpio_chip's get_direction() routine, that
lets other drivers get particular GPIOs direction using
struct gpio_desc.
Signed-off-by: Yegor Yefremov yegorsli...@googlemail.com
Acked-by: Javier Martinez Canillas jav...@dowhile0.org
---
Changes:
v2: rework return value calculation
(...)
I don't see why this need to be a broken-out function? Can it not
be factored into gpio_get_direction?
+static int _get_gpio_direction(struct gpio_bank *bank, int gpio)
+{
+ void __iomem *reg = bank-base;
+ u32 l;
+
+ reg += bank-regs-direction;
+ l = (readl_relaxed(reg) gpio);
+
+ return (l 0x0001);
Rewrite the last two statements like this:
#include linux/bitops.h
return !!(readl_relaxed(reg) BIT(gpio));
+static int gpio_get_direction(struct gpio_chip *chip, unsigned offset)
+{
+ struct gpio_bank *bank;
+ unsigned long flags;
+ int dir;
+
+ bank = container_of(chip, struct gpio_bank, chip);
+ spin_lock_irqsave(bank-lock, flags);
+ dir = _get_gpio_direction(bank, offset);
+ spin_unlock_irqrestore(bank-lock, flags);
+ return dir;
+}
So since _get_gpio_direction is never called from unlocked context,
can it not just be part of this function then?
(I make a mental note to prefix all functions in this driver
with omap_*)
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/11] pinctrl: single: Clear pin interrupts enabled by bootloader
On Tue, Apr 22, 2014 at 6:10 PM, Tony Lindgren t...@atomide.com wrote: Shall I apply this patch or will you funnel it through ARM SoC due to deps? No deps except boards hanging without it.. Please feel free to take this one, prererrably as a fix for the -rc series if no objections. Yes this is -rc material. Queued for fixes. Thanks! Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regressions in linux-next?
On Wed, Apr 23, 2014 at 3:29 PM, Nishanth Menon n...@ti.com wrote: On 04/23/2014 08:01 AM, Linus Walleij wrote: What about this: if (chip-irq_default_type != IRQ_TYPE_NONE) irq_set_irq_type(irq, chip-irq_default_type); This way you can pass IRQ_TYPE_NONE and nothing happens in the mapping. What if these drivers depend on IRQ_TYPE_NONE to do something for the GPIO pins? Yeah :-( would you expect these drivers to pass IRQ_TYPE_DEFAULT? Actually that sounds like a good idea. Maybe we can go over the few drivers that pass IRQ_TYPE_NONE and see what they actually want. There are not *too* many users of this call yet. OR I wonder if we could pass some flag like -1 for platforms that dont care? The flags parameter to gpiochip_irqchip_add() is unsigned... Switching to IRQ_TYPE_DEFAULT for drivers that want this is likely the sane thing to do. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gpio: do not set up hardware for IRQ_TYPE_NONE
Some GPIO irqchip drivers exploit the irqdomain mapping function to set up the IRQ default type in the hardware, make sure that if we pass IRQ_TYPE_NONE, no hardware setup whatsoever takes place (this should be the norm) until later when the IRQ gets utilized. Cc: Nishanth Menon n...@ti.com Cc: Peter Ujfalusi peter.ujfal...@ti.com Cc: Ezequiel Garcia ezequiel.gar...@free-electrons.com Cc: Javier Martinez Canillas jav...@dowhile0.org Cc: Tony Lindgren t...@atomide.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: linux-omap linux-omap@vger.kernel.org Signed-off-by: Linus Walleij linus.wall...@linaro.org --- TI folks: can you provide a Tested-by tag if this makes your OMAPs work? I am pretty sure the other platforms will be unaffected, if they aren't I will switch them over to react to IRQ_TYPE_DEFAULT. --- drivers/gpio/gpiolib.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index ee1819fdcf35..97d173e9aa2d 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1409,7 +1409,12 @@ static int gpiochip_irq_map(struct irq_domain *d, unsigned int irq, #else irq_set_noprobe(irq); #endif - irq_set_irq_type(irq, chip-irq_default_type); + /* +* No set-up of the hardware will happen if IRQ_TYPE_NONE +* is passed as default type. +*/ + if (chip-irq_default_type != IRQ_TYPE_NONE) + irq_set_irq_type(irq, chip-irq_default_type); return 0; } @@ -1490,7 +1495,8 @@ static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip) * @first_irq: if not dynamically assigned, the base (first) IRQ to * allocate gpiochip irqs from * @handler: the irq handler to use (often a predefined irq core function) - * @type: the default type for IRQs on this irqchip + * @type: the default type for IRQs on this irqchip, pass IRQ_TYPE_NONE + * to have the core avoid setting up any default type in the hardware. * * This function closely associates a certain irqchip with a certain * gpiochip, providing an irq domain to translate the local IRQs to -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regressions in linux-next?
On Wed, Apr 23, 2014 at 4:50 PM, Javier Martinez Canillas jav...@dowhile0.org wrote: What do you think about the following patch? Although I agree that if we can use IRQ_TYPE_NONE as you propose then tht is a much better and efficient solution that this patch. Let's try IRQ_TYPE_NONE first and if that fails we go back to this. I've pushed the current patch to linux-next. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/5] add gpio_chip_ops to hold GPIO operations
On Tue, Apr 8, 2014 at 8:20 PM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: So this is an RFC patch-set to add a virtual table to be used by GPIO chip controllers and consist of the following patches: Overall I like this. However I don't want to see any transitional phase. I prefer a BIG fat patch converting everyone and its dog to the new vtable and removing the old function pointers. This can be based on the HEAD of my GPIO devel branch. It may be a good idea to use coccinelle for this refactoring in order not to miss any users. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/11] pinctrl: single: Clear pin interrupts enabled by bootloader
On Fri, Apr 11, 2014 at 1:47 AM, Tony Lindgren t...@atomide.com wrote: Since we set up device wake-up interrupts as pinctrl-single interrupts, we now must use the standard request_irq and related functions to manage them. If the pin interrupts are enabled for some pins at boot, the wake-up events can show up as constantly pending at least on omaps and will hang the system unless the related device driver clears the event at the device. To fix this, let's clear the interrupt flags during init, and print out a warning so the board maintainers can update their drivers to do proper request_irq for the driver specific wake-up events. Cc: Haojian Zhuang haojian.zhu...@linaro.org Cc: Linus Walleij linus.wall...@linaro.org Signed-off-by: Tony Lindgren t...@atomide.com Looks clean. Acked-by: Linus Walleij linus.wall...@linaro.org Shall I apply this patch or will you funnel it through ARM SoC due to deps? Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] GPIO OMAP driver changes for v3.16
On Fri, Apr 11, 2014 at 5:03 PM, Javier Martinez Canillas
jav...@dowhile0.org wrote:
Hello Aaro,
On Thu, Apr 10, 2014 at 11:22 PM, Aaro Koskinen aaro.koski...@iki.fi wrote:
Hi,
On Thu, Apr 10, 2014 at 10:17:44PM +0200, Javier Martinez Canillas wrote:
The same happens also on Nokia 770:
[0.118896] genirq: Setting trigger mode 0 for irq 128 failed
(gpio_irq_type+0x0/0x220)
I don't have those errors when booting on my DM3730 IGEPv2 board but
it seems that for some reason on omap1 __irq_set_trigger() complains
when IRQ_TYPE_NONE is used as a default flag when calling
gpiochip_irqchip_add()
Could you please test the following patch and tell me if your board
still works and makes the errors go away?
Now it complains about mode 8...
[0.118835] genirq: Setting trigger mode 8 for irq 128 failed
(gpio_irq_type
+0x0/0x220)
A.
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 8cc9e91..5bc8aec 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1122,7 +1122,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)
ret = gpiochip_irqchip_add(bank-chip, gpio_irq_chip,
irq_base, gpio_irq_handler,
- IRQ_TYPE_NONE);
+ IRQ_TYPE_LEVEL_LOW);
if (ret) {
dev_err(bank-dev, Couldn't add irqchip to gpiochip
%d\n, ret);
Best regards,
Javier
Thanks for testing. Unfortunately I'm out of ideas on why that error
could be shown and I don't have a way to further debug it without an
omap1 board. I wonder why that pr_err() message is shown or why it is
still working when an error happens.
Maybe Linus or Santosh could give us a hint on what is happening here.
Isn't an edge IRQ more apropriate as default then?
The code contains this:
if (!bank-regs-leveldetect0
(type (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH)))
return -EINVAL;
Meaning sometimes the banks don't support level IRQs.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regressions in linux-next?
On Tue, Apr 22, 2014 at 5:52 PM, Javier Martinez Canillas jav...@dowhile0.org wrote: I've revised the patch again and I couldn't find the reason why certain boards are failing to boot. I can't reproduce this issue since I only have a DM3730 IGEPv2 board which boots fine but I should have access to an AM3354 IGEP Aquila board which is similar to the am335x-evmsk so I may be able to debug it. It would really REALLY appreciate if some of the people maintaining and using OMAP1 would help Javier out in this refactoring operation. I'd really *hate* to have to drop his patches because of a lack of boards. This refactoring is necessary to handle the exploding multitude of GPIO drivers moving forward. We even tried to get an Innovator to boot just to be able to refactor OMAP stuff but fell short on some special JTAG reflash snag so we are dependent on maintainers to help out here :-/ Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] GPIO OMAP driver changes for v3.16
On Sun, Apr 6, 2014 at 4:58 PM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: Now that you have sent your changes for v3.15 to Torvalds, here are some changes for the OMAP GPIO driver targeted to v3.16. Mostly improvements so nothing here is -rc material. I like this series so I have applied them for v3.16, pending some ACK from Kevin | Santosh. The biggest change is Patch 4 that converts the driver to use the newly introduced generic irqchip helpers in the gpiolib core which allows to remove some driver specific logic that should really be generic. Excellent, I will take a closer look at this, it seems there is some cruft still in but let me look. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] gpio: omap: convert driver to use gpiolib irqchip
On Thu, Apr 10, 2014 at 7:39 PM, Santosh Shilimkar
santosh.shilim...@ti.com wrote:
On Sunday 06 April 2014 10:58 AM, Javier Martinez Canillas wrote:
+#ifdef CONFIG_ARCH_OMAP1
+ /*
+ * REVISIT: Once we have OMAP1 supporting SPARSE_IRQ, we can drop
+ * irq_alloc_descs() since a base IRQ offset will no longer be needed.
+ */
+ irq_base = irq_alloc_descs(-1, 0, bank-width, 0);
+ if (irq_base 0) {
+ dev_err(bank-dev, Couldn't allocate IRQ numbers\n);
+ return -ENODEV;
+ }
+#endif
+
Do we still need above after first patch ? Would be good
to get rid of above ugly #ifdef on the middle of the code.
I don't think so actually, simple irqdomain adds descriptors
for irqbase != 0, and the gpiochip irqchip helpers call
irq_create_mapping() on all offsets.
Preferably a separate patch on top of this removing that code
though so it's bisectable.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 49/75] ARM: l2c: fix register naming
On Fri, Mar 28, 2014 at 4:18 PM, Russell King rmk+ker...@arm.linux.org.uk wrote: We have a mixture of different devices with different register layouts, but we group all the bits together in an opaque mess. Split them out into those which are L2C-310 specific and ones which refer to earlier devices. Provide full auxiliary control register definitions. Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk Acked-by: Linus Walleij linus.wall...@linaro.org For ux500. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] power_supply: Introduce generic psy charging driver
On Thu, Mar 13, 2014 at 10:12 AM, Pavel Machek pa...@ucw.cz wrote: Hi! 30*HZ means 30 seconds in the kernel... what is hard to understand about it? Well I might be picky, but since it is a charging algorithm dealing with ampères, volts, constant-current/constant-voltage, watchdogs and timeouts, all stated in SI units, it would be nice if all such constants were specified in simple units instead of kernel-specific terms. I agree HZ is badly named, but hopefully anyone working on kernel is already familiar with it. But... what would actually help: I believe we should introduce milivolt_t, miliamp_t, milisec_t etc... types. Storing milivolts in int, then having comment saying milivolts is just wrong. Hm! I bet the regulator subsystem maintainers have opinions on that. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 2/4] power_supply: Introduce generic psy charging driver
On Mon, Mar 10, 2014 at 5:33 AM, Jenny Tc jenny...@intel.com wrote: On Fri, Mar 07, 2014 at 09:25:20PM +0100, Pavel Machek wrote: + /* sort based on priority. 0 has the highest priority */ + for (i = 0; i cnt; ++i) + for (j = 0; j cnt; ++j) + if (psy_prioirty(psy_lst[j]) psy_prioirty(psy_lst[i])) + swap(psy_lst[j], psy_lst[i]); + WTF? Bubble sort in kernel? Yes, it's bubble sort. Since the number of power supply objects in real systems (max 4) are limited, I feel the complexity would be as same as any other sorting algorithms. Any suggestions? You already have a kernel quicksort implementation in lib/sort.c. Please restructure the code to make use of this as it is already compiled into every kernel. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] power_supply: Introduce generic psy charging driver
On Fri, Mar 7, 2014 at 9:10 PM, Pavel Machek pa...@ucw.cz wrote: On Fri 2014-03-07 11:04:59, Linus Walleij wrote: On Fri, Feb 28, 2014 at 6:01 PM, Pavel Machek pa...@ucw.cz wrote: On Thu 2014-02-27 21:08:01, Linus Walleij wrote: On Thu, Feb 20, 2014 at 6:53 AM, Jenny TC jenny...@intel.com wrote: +++ b/include/linux/power/power_supply_charger.h +#define MAX_CUR_VOLT_SAMPLES 3 +#define DEF_CUR_VOLT_SAMPLE_JIFF (30*HZ) Why are things defined in Jiffies like this insead of seconds, milliseconds etc? This will vary with the current operating frequency of the system, why should physical measurements do that? It is actually ok. The define is relative to jiffies, and that's what interface expects. So consider the option that the interface is wrong. Stating something like a sample period in system-specific jiffies instead of period time T is just weird. What control systems guy would understand this? 30*HZ means 30 seconds in the kernel... what is hard to understand about it? Well I might be picky, but since it is a charging algorithm dealing with ampères, volts, constant-current/constant-voltage, watchdogs and timeouts, all stated in SI units, it would be nice if all such constants were specified in simple units instead of kernel-specific terms. One reason is that this kind of code actually needs review from non-programmers, people like chemists and physicists. I know it may be far fetched so no strong preference for sure. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 2/4] power_supply: Introduce generic psy charging driver
On Fri, Mar 7, 2014 at 6:29 AM, Jenny TC jenny...@intel.com wrote:
+enum psy_charger_cable_type {
+ PSY_CHARGER_CABLE_TYPE_NONE = 0,
+ PSY_CHARGER_CABLE_TYPE_USB_SDP = 1 0,
+ PSY_CHARGER_CABLE_TYPE_USB_DCP = 1 1,
+ PSY_CHARGER_CABLE_TYPE_USB_CDP = 1 2,
+ PSY_CHARGER_CABLE_TYPE_USB_ACA = 1 3,
+ PSY_CHARGER_CABLE_TYPE_AC = 1 4,
+ PSY_CHARGER_CABLE_TYPE_ACA_DOCK = 1 5,
+ PSY_CHARGER_CABLE_TYPE_ACA_A = 1 6,
+ PSY_CHARGER_CABLE_TYPE_ACA_B = 1 7,
+ PSY_CHARGER_CABLE_TYPE_ACA_C = 1 8,
+ PSY_CHARGER_CABLE_TYPE_SE1 = 1 9,
+ PSY_CHARGER_CABLE_TYPE_MHL = 1 10,
+ PSY_CHARGER_CABLE_TYPE_B_DEVICE = 1 11,
+};
I still disagree with using an enum as bitfield.
Atleast
#include linux/bitops.h
and use BIT(0), BIT(1) etc to define the bits.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] power_supply: Introduce generic psy charging driver
On Fri, Feb 28, 2014 at 12:27 PM, Jenny Tc jenny...@intel.com wrote:
On Thu, Feb 27, 2014 at 09:08:01PM +0100, Linus Walleij wrote:
On Thu, Feb 20, 2014 at 6:53 AM, Jenny TC jenny...@intel.com wrote:
+++ b/include/linux/power/power_supply_charger.h
+#define MAX_CUR_VOLT_SAMPLES 3
+#define DEF_CUR_VOLT_SAMPLE_JIFF (30*HZ)
Why are things defined in Jiffies like this insead of seconds, milliseconds
etc? This will vary with the current operating frequency of the system,
why should physical measurements do that?
Is it fine if I use msecs_to_jiffies(3)?
Keep the
#define DEF_CUR_VOLT_SAMPLE_PERIOD 3
Then use msecs_to_jiffies(DEF_CUR_VOLT_SAMPLE_PERIOD)
in the call site.
+enum psy_charger_cable_event {
+ PSY_CHARGER_CABLE_EVENT_DISCONNECT = 0,
+ PSY_CHARGER_CABLE_EVENT_CONNECT,
+ PSY_CHARGER_CABLE_EVENT_UPDATE,
+ PSY_CHARGER_CABLE_EVENT_RESUME,
+ PSY_CHARGER_CABLE_EVENT_SUSPEND,
+};
+
+enum psy_charger_cable_type {
+ PSY_CHARGER_CABLE_TYPE_NONE = 0,
+ PSY_CHARGER_CABLE_TYPE_USB_SDP = 1 0,
+ PSY_CHARGER_CABLE_TYPE_USB_DCP = 1 1,
+ PSY_CHARGER_CABLE_TYPE_USB_CDP = 1 2,
+ PSY_CHARGER_CABLE_TYPE_USB_ACA = 1 3,
+ PSY_CHARGER_CABLE_TYPE_AC = 1 4,
+ PSY_CHARGER_CABLE_TYPE_ACA_DOCK = 1 5,
+ PSY_CHARGER_CABLE_TYPE_ACA_A = 1 6,
+ PSY_CHARGER_CABLE_TYPE_ACA_B = 1 7,
+ PSY_CHARGER_CABLE_TYPE_ACA_C = 1 8,
+ PSY_CHARGER_CABLE_TYPE_SE1 = 1 9,
+ PSY_CHARGER_CABLE_TYPE_MHL = 1 10,
+ PSY_CHARGER_CABLE_TYPE_B_DEVICE = 1 11,
+};
Why is this even an enum? It is clearly bitfields. I would just:
#include linux/bitops.h
#define PSY_CHARGER_CABLE_TYPE_NONE 0x0
#define PSY_CHARGER_CABLE_TYPE_USB_SDP BIT(0)
#define PSY_CHARGER_CABLE_TYPE_USB_DCP BIT(1)
(etc)
This is to ensure type checks when the cable types are handled, #defines will
not help in type checks.
Type checks with static code check tools? But misrepresenting
a bitfield as an enum just to satisfy a static code checker is not
OK IMO.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] power_supply: Introduce generic psy charging driver
On Fri, Feb 28, 2014 at 6:01 PM, Pavel Machek pa...@ucw.cz wrote: On Thu 2014-02-27 21:08:01, Linus Walleij wrote: On Thu, Feb 20, 2014 at 6:53 AM, Jenny TC jenny...@intel.com wrote: +++ b/include/linux/power/power_supply_charger.h +#define MAX_CUR_VOLT_SAMPLES 3 +#define DEF_CUR_VOLT_SAMPLE_JIFF (30*HZ) Why are things defined in Jiffies like this insead of seconds, milliseconds etc? This will vary with the current operating frequency of the system, why should physical measurements do that? It is actually ok. The define is relative to jiffies, and that's what interface expects. So consider the option that the interface is wrong. Stating something like a sample period in system-specific jiffies instead of period time T is just weird. What control systems guy would understand this? Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] power_supply: Introduce PSE compliant algorithm
On Fri, Feb 28, 2014 at 11:07 AM, Jenny Tc jenny...@intel.com wrote: On Thu, Feb 27, 2014 at 09:18:57PM +0100, Linus Walleij wrote: On Tue, Feb 4, 2014 at 6:12 AM, Jenny TC jenny...@intel.com wrote: +static inline bool __is_battery_full + (long volt, long cur, long iterm, unsigned long cv) Overall I wonder if you've run checkpatch on these patches, but why are you naming this one function with a double __underscore? Just is_battery_full_check() or something would work fine I guess? Just to convey that is_battery_full is a local function and not generic. You can find similar usage in power_supply_core.c (__power_supply_changed_work) and in other drivers. Isn't it advised to have __ prefixes? The preference is different, usually __ is for compiler things, but while I dislike it (disturbs my perception) I can sure live with it. Why are you packing these structs? If no real reason, remove it. The compiler will pack what it thinks is appropriate anyway. The structure is part of the battery charging profile which can be read directly from an EEPROM or from secondary storage (emmc). So it should be packed to keep it align with the stored format. OK I buy that. Make sure this is noted somewhere (or maybe I missed it). Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] power_supply: Introduce PSE compliant algorithm
On Wed, Feb 26, 2014 at 3:54 AM, Jenny Tc jenny...@intel.com wrote: The idea is to allow pluggable charging algorithms. Currently we have only one charging algorithm proposed, but can have other charging algorithms (like pulse charging, rule based charging etc.). Based on the platform need, the algorithms can be selected. So this should be a user configurable option. I can add more explanation on when to select this option. Do you see a generic framework for pluggable algorithms on an abstracted level, so that it could be used for the CC/CV charging, measurement and temperature check algorithm that is found in e.g. drivers/power/abx500_chargalg.c drivers/power/ab8500_charger.c etc, or do you envision a set of pluggable algorithms for this one hardware? I'm asking because these drivers are a massive set of code and we may need to start to abstract out and define library functions and frameworks already now before it becomes impossible to contain. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] power_supply: Introduce generic psy charging driver
power_supply_register_charger(struct power_supply_charger *psyc);
+extern int power_supply_unregister_charger(struct power_supply_charger
*psyc);
+extern int power_supply_register_charging_algo(struct psy_charging_algo *);
+extern int power_supply_unregister_charging_algo(struct psy_charging_algo *);
+extern int psy_get_battery_prop(struct psy_batt_chrg_prof *batt_prop);
+extern void psy_battery_prop_changed(int battery_conn_stat,
+ struct psy_batt_chrg_prof *batt_prop);
+
+#else
+
+static int power_supply_register_charger(struct power_supply_charger *psyc)
+{ return 0; }
+static int power_supply_unregister_charger(struct power_supply_charger
*psyc)
+{ return 0; }
+static int power_supply_register_charging_algo(struct psy_charging_algo
*algo)
+{ return 0; }
+static int power_supply_unregister_charging_algo(struct psy_charging_algo
*algo)
+{ return 0; }
Why do these return 0? Should they not just fail if the power supply
charger support is not compiled in, like return -EINVAL etc?
Sorry for just making some random review of the header files, but
this caught my attention and I couldn't resist.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] power_supply: Introduce PSE compliant algorithm
On Tue, Feb 4, 2014 at 6:12 AM, Jenny TC jenny...@intel.com wrote:
+static inline bool __is_battery_full
+ (long volt, long cur, long iterm, unsigned long cv)
Overall I wonder if you've run checkpatch on these patches, but why
are you naming this one function with a double __underscore?
Just is_battery_full_check() or something would work fine I guess?
(...)
+/* Parameters defining the charging range */
+struct psy_ps_temp_chg_table {
+ /* upper temperature limit for each zone */
+ short int temp_up_lim; /* Degree Celsius */
+
+ /* charge current and voltage */
+ short int full_chrg_vol; /* mV */
+ short int full_chrg_cur; /* mA */
+
+ /*
+ * Maintenance charging thresholds.
+ * Maintenance charging voltage lower limit - Once battery hits full,
+ * charging will be resumed when battery voltage = this voltage
+ */
+ short int maint_chrg_vol_ll; /* mV */
+
+ /* Charge current and voltage in maintenance charging mode */
+ short int maint_chrg_vol_ul; /* mV */
+ short int maint_chrg_cur; /* mA */
+} __packed;
Why are you packing these structs? If no real reason, remove it.
The compiler will pack what it thinks is appropriate anyway.
Convert all comments to kerneldoc.
+#define BATTID_STR_LEN 8
+#define BATT_TEMP_NR_RNG 6
+
+struct psy_pse_chrg_prof {
+ /* battery id */
+ char batt_id[BATTID_STR_LEN];
+ u16 battery_type; /* Defined as POWER_SUPPLY_TECHNOLOGY_* */
Use a named enum by patching that in linux/power_supply.h?
+ u16 capacity; /* mAh */
+ u16 voltage_max; /* mV */
+ /* charge termination current */
+ u16 chrg_term_mA;
+ /* Low battery level voltage */
+ u16 low_batt_mV;
+ /* upper and lower temperature limits on discharging */
+ s8 disch_temp_ul; /* Degree Celsius */
+ s8 disch_temp_ll; /* Degree Celsius */
+ /* number of temperature monitoring ranges */
+ u16 temp_mon_ranges;
+ struct psy_ps_temp_chg_table temp_mon_range[BATT_TEMP_NR_RNG];
+ /* lowest temperature supported */
+ s8 temp_low_lim;
+} __packed;
Why packed, and convert to kerneldoc for this struct.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
