Re: bbb kexec bug: Unhandled fault external abort on non-linefetch (0x1028) at 0xfa1ac140
On 12/28/2015 09:18 AM, Dave Young wrote: > On 12/27/15 at 03:38pm, Dave Young wrote: >> Here is what I get when I test kdump on Beagle bone black: >> >> Added a printk line at the begin of function omap_gpio_rmw: >> printk("## %lx, %x, %x\n", base, reg, mask); >> >> Any hints how to fix it? I tried call the machine_kexec_mask_interrupts >> at runtime kernel also panics so it may not limit to kdump case. >> >> [ 66.340168] ## fa1ac000, 140, 1 >> [ 66.344456] Unhandled fault: external abort on non-linefetch (0x1028) at >> 0xfa1ac140 >> [ 66.352142] pgd = dd9f [...] >> [ 66.727278] [] (omap_set_gpio_triggering) from [] >> (omap_gpio_mask_irq+0x29/0x34) Usually such back-trace means that you are trying to access HW which is disabled (powered off) already. Or this HW IP has never been enabled. >> [ 66.736457] [] (omap_gpio_mask_irq) from [] >> (machine_crash_shutdown+0xb9/0x104) >> [ 66.745551] [] (machine_crash_shutdown) from [] >> (crash_kexec+0x35/0x68) >> [ 66.753942] [] (crash_kexec) from [] (die+0x1b9/0x390) >> [ 66.760859] [] (die) from [] >> (__do_kernel_fault.part.0+0x4f/0x1cc) >> [ 66.768824] [] (__do_kernel_fault.part.0) from [] >> (do_page_fault+0x155/0x29c) >> [ 66.40] [] (do_page_fault) from [] >> (do_DataAbort+0x2f/0x88) >> [ 66.785432] [] (do_DataAbort) from [] >> (__dabt_svc+0x3b/0x80) >> [ 66.792858] Exception stack(0xddc39e58 to 0xddc39ea0) >> [ 66.797929] 9e40: >> 0063 df93647c >> [ 66.806144] 9e60: 1f26a000 0001 0063 0007 c0702e3c >> ddc38000 >> [ 66.814359] 9e80: 7f70d614 0030 ddc39ea8 c021e54b c021e54c >> 600e0033 >> [ 66.822575] [] (__dabt_svc) from [] >> (sysrq_handle_crash+0x18/0x1c) >> [ 66.830530] [] (sysrq_handle_crash) from [] >> (__handle_sysrq+0x79/0x10c) >> [ 66.838919] [] (__handle_sysrq) from [] >> (write_sysrq_trigger+0x45/0x50) >> [ 66.847310] [] (write_sysrq_trigger) from [] >> (proc_reg_write+0x43/0x68) >> [ 66.855700] [] (proc_reg_write) from [] >> (__vfs_write+0xf/0x8c) >> [ 66.863304] [] (__vfs_write) from [] >> (vfs_write+0x5f/0x128) >> [ 66.870646] [] (vfs_write) from [] >> (SyS_write+0x2b/0x68) >> [ 66.877729] [] (SyS_write) from [] >> (ret_fast_syscall+0x1/0x4c) >> [ 66.885332] Code: 443c 4643 f6a9 f9a1 (6823) 0732 >> [ 66.890145] ---[ end trace 5a39094ece4dc200 ]--- >> [ 66.894782] Kernel panic - not syncing: Fatal exception >> [ 66.900033] ---[ end Kernel panic - not syncing: Fatal exception >> -- regards, -grygorii -- 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/3] wlcore/wl12xx: spi: add power operation function
On 12/27/2015 07:14 PM, Uri Mashiach wrote: > Hello Grygorii, > > On 12/24/2015 06:32 PM, Grygorii Strashko wrote: >> On 12/24/2015 05:35 PM, Uri Mashiach wrote: >>> The power function uses a consumer regulator access to update the WiFi >>> enable GPIO value. >>> >>> Signed-off-by: Uri Mashiach <uri.mashi...@compulab.co.il> >>> --- >>> v1 -> v2: oops fix was removed to a separate fix. >>> >>> drivers/net/wireless/ti/wlcore/spi.c | 37 >>> >>> 1 file changed, 37 insertions(+) >>> >>> diff --git a/drivers/net/wireless/ti/wlcore/spi.c >>> b/drivers/net/wireless/ti/wlcore/spi.c >>> >> >> [...] >> >>> + >>> static struct wl1271_if_operations spi_ops = { >>> .read= wl12xx_spi_raw_read, >>> .write= wl12xx_spi_raw_write, >>> .reset= wl12xx_spi_reset, >>> .init= wl12xx_spi_init, >>> +.power= wl12xx_spi_set_power, >>> .set_block_size = NULL, >>> }; >>> >>> @@ -353,6 +384,12 @@ static int wl1271_probe(struct spi_device *spi) >>>* comes from the board-peripherals file */ >>> spi->bits_per_word = 32; >>> >>> +glue->reg = devm_regulator_get(>dev, "vwlan"); >>> +if (IS_ERR(glue->reg)) { >> >> It will be more correct to handle -EPROBE_DEFER here also. Like: >> if (PTR_ERR(glue->reg) == -EPROBE_DEFER) >> return PTR_ERR(glue->reg); >> > > It seems that the IS_ERR(glue->reg) condition covers the EPROBE_DEFER case. True. But this is not an error and it's common practice do not print any log messages in this case from driver :) > >>> +dev_err(glue->dev, "can't get regulator\n"); >>> +return PTR_ERR(glue->reg); >>> +} >>> + >>> ret = spi_setup(spi); >>> if (ret < 0) { >>> dev_err(glue->dev, "spi_setup failed\n"); >>> >> >> > -- regards, -grygorii -- 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] ARM: dts: am4372: fix irq type for arm twd and global timer
As per ARM documentation PPI(0) ID27 - global timer interrupt is rising-edge sensitive. PPI(2) ID29 - twd interrupt is rising-edge sensitive. and the same is proved by GIC distributor register value GIC_DIST_CONFIG(0xC04) = 0x7DC0. Hence, set IRQ triggering type to IRQ_TYPE_EDGE_RISING for ARM TWD and Global timers. Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- arch/arm/boot/dts/am4372.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi index de8791a..dfa1a29 100644 --- a/arch/arm/boot/dts/am4372.dtsi +++ b/arch/arm/boot/dts/am4372.dtsi @@ -72,7 +72,7 @@ global_timer: timer@48240200 { compatible = "arm,cortex-a9-global-timer"; reg = <0x48240200 0x100>; - interrupts = ; + interrupts = ; interrupt-parent = <>; clocks = <_periphclk>; }; @@ -80,7 +80,7 @@ local_timer: timer@48240600 { compatible = "arm,cortex-a9-twd-timer"; reg = <0x48240600 0x100>; - interrupts = ; + interrupts = ; interrupt-parent = <>; clocks = <_periphclk>; }; -- 2.6.4 -- 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] ARM: dts: am437x: pixcir_tangoc: use correct flags for irq types
Now IRQs for Pixcir Tangoc touchscreen are defined using IRQ_TYPE_NONE in am437x-gp-evm.dts and am43x-epos-evm.dts wich do not correspond HW. Hence, update am437x-gp-evm.dts and am43x-epos-evm.dts files and use correct flag IRQ_TYPE_EDGE_FALLING for irq types. While here, remove duplicated irq declaration for pixcir_ts@5c node in am437x-gp-evm.dts. Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- arch/arm/boot/dts/am437x-gp-evm.dts | 4 +--- arch/arm/boot/dts/am43x-epos-evm.dts | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts index d2450ab..fc4c371 100644 --- a/arch/arm/boot/dts/am437x-gp-evm.dts +++ b/arch/arm/boot/dts/am437x-gp-evm.dts @@ -590,8 +590,6 @@ pinctrl-names = "default"; pinctrl-0 = <_ts_pins>; reg = <0x5c>; - interrupt-parent = <>; - interrupts = <22 0>; attb-gpio = < 22 GPIO_ACTIVE_HIGH>; @@ -599,7 +597,7 @@ * 0x264 represents the offset of padconf register of * gpio3_22 from am43xx_pinmux base. */ - interrupts-extended = < 22 IRQ_TYPE_NONE>, + interrupts-extended = < 22 IRQ_TYPE_EDGE_FALLING>, <_pinmux 0x264>; interrupt-names = "tsc", "wakeup"; diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts index 47954ed..29c6580 100644 --- a/arch/arm/boot/dts/am43x-epos-evm.dts +++ b/arch/arm/boot/dts/am43x-epos-evm.dts @@ -491,7 +491,7 @@ pinctrl-0 = <_ts_pins>; reg = <0x5c>; interrupt-parent = <>; - interrupts = <17 0>; + interrupts = <17 IRQ_TYPE_EDGE_FALLING>; attb-gpio = < 17 GPIO_ACTIVE_HIGH>; -- 2.6.4 -- 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/3] wlcore/wl12xx: spi: add device tree support
On 12/24/2015 05:35 PM, Uri Mashiach wrote: > Add DT support for the wl1271 SPI WiFi. > > Add documentation file for the wl1271 SPI WiFi. > > Signed-off-by: Uri Mashiach> Acked-by: Igor Grinberg > --- > v1 -> v2: update interrupt documentation. >replace interrupts and interrupt-parent with interrupts-extended. > IRQ parameters retrieved from spi_device instead of DT. > remove redundant #ifdef CONFIG_OF > > .../bindings/net/wireless/ti,wlcore,spi.txt| 34 + > drivers/net/wireless/ti/wlcore/spi.c | 55 > -- > 2 files changed, 85 insertions(+), 4 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt > > diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt > b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt > new file mode 100644 > index 000..502c27e > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt > @@ -0,0 +1,34 @@ > +* Texas Instruments wl1271 wireless lan controller > + > +The wl1271 chip can be connected via SPI or via SDIO. This > +document describes the binding for the SPI connected chip. > + > +Required properties: > +- compatible : Should be "ti,wl1271" > +- reg : Chip select address of device > +- spi-max-frequency : Maximum SPI clocking speed of device in Hz > +- ref-clock-frequency : Reference clock frequency > +- interrupts-extended : Should contain parameters for 1 interrupt line. > +Interrupt parameters: parent, line number, type. > +- vwlan-supply :Point the node of the regulator that powers/enable > the wl1271 chip > + > +Optional properties: > +- clock-xtal : boolean, clock is generated from XTAL > + > +- Please consult Documentation/devicetree/bindings/spi/spi-bus.txt > + for optional SPI connection related properties, > + > +Examples: > + > + { > + wl1271@1 { > + compatible = "ti,wl1271"; > + > + reg = <1>; > + spi-max-frequency = <4800>; > + clock-xtal; > + ref-clock-frequency = <3840>; > + interrupts-extended = < 8 IRQ_TYPE_LEVEL_HIGH>; > + vwlan-supply = <_fixed>; > + }; > +}; > diff --git a/drivers/net/wireless/ti/wlcore/spi.c > b/drivers/net/wireless/ti/wlcore/spi.c > index d3a4bcb..e9e8d54 100644 > --- a/drivers/net/wireless/ti/wlcore/spi.c > +++ b/drivers/net/wireless/ti/wlcore/spi.c > @@ -30,6 +30,7 @@ > #include > #include > #include > +#include > #include > > #include "wlcore.h" > @@ -357,6 +358,46 @@ static struct wl1271_if_operations spi_ops = { > .set_block_size = NULL, > }; > > +#ifdef CONFIG_OF > +static const struct of_device_id wlcore_spi_of_match_table[] = { > + { .compatible = "ti,wl1271" }, > + { } > +}; > + > +/** > + * wlcore_probe_of - DT node parsing. > + * @spi: SPI slave device parameters. > + * @res: resource parameters. > + * @glue: wl12xx SPI bus to slave device glue parameters. > + * @pdev_data: wlcore device parameters > + */ > +static int wlcore_probe_of(struct spi_device *spi, struct wl12xx_spi_glue > *glue, > +struct wlcore_platdev_data *pdev_data) > +{ > + struct device_node *dt_node = spi->dev.of_node; > + int ret; > + > + if (of_find_property(dt_node, "clock-xtal", NULL)) > + pdev_data->ref_clock_xtal = true; > + > + ret = of_property_read_u32(dt_node, "ref-clock-frequency", > +_data->ref_clock_freq); > + if (IS_ERR_VALUE(ret)) { > + dev_err(glue->dev, > + "can't get reference clock frequency (%d)\n", ret); > + return ret; > + } > + > + return 0; > +} > +#else /* CONFIG_OF */ > +static int wlcore_probe_of(struct spi_device *spi, struct wl12xx_spi_glue > *glue, > +struct wlcore_platdev_data *pdev_data) > +{ > + return -ENODATA; > +} > +#endif /* CONFIG_OF */ My question for your v1 was related to all above ifdefs. If CONFIG_OF=n is not going to be supported then all this ifdefs can be dropped and proper dependency can be added to Kconfig instead. > + > static int wl1271_probe(struct spi_device *spi) > { > struct wl12xx_spi_glue *glue; > @@ -366,8 +407,6 @@ static int wl1271_probe(struct spi_device *spi) > > memset(_data, 0x00, sizeof(pdev_data)); > > - /* TODO: add DT parsing when needed */ > - > pdev_data.if_ops = _ops; > > glue = devm_kzalloc(>dev, sizeof(*glue), GFP_KERNEL); > @@ -390,6 +429,13 @@ static int wl1271_probe(struct spi_device *spi) > return PTR_ERR(glue->reg); > } > > + ret = wlcore_probe_of(spi, glue, _data); > + if (IS_ERR_VALUE(ret)) { > + dev_err(glue->dev, > + "can't get device
Re: [PATCH v2 1/3] wlcore/wl12xx: spi: add power operation function
On 12/24/2015 05:35 PM, Uri Mashiach wrote: The power function uses a consumer regulator access to update the WiFi enable GPIO value. Signed-off-by: Uri Mashiach--- v1 -> v2: oops fix was removed to a separate fix. drivers/net/wireless/ti/wlcore/spi.c | 37 1 file changed, 37 insertions(+) diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c [...] + static struct wl1271_if_operations spi_ops = { .read = wl12xx_spi_raw_read, .write = wl12xx_spi_raw_write, .reset = wl12xx_spi_reset, .init = wl12xx_spi_init, + .power = wl12xx_spi_set_power, .set_block_size = NULL, }; @@ -353,6 +384,12 @@ static int wl1271_probe(struct spi_device *spi) * comes from the board-peripherals file */ spi->bits_per_word = 32; + glue->reg = devm_regulator_get(>dev, "vwlan"); + if (IS_ERR(glue->reg)) { It will be more correct to handle -EPROBE_DEFER here also. Like: if (PTR_ERR(glue->reg) == -EPROBE_DEFER) return PTR_ERR(glue->reg); + dev_err(glue->dev, "can't get regulator\n"); + return PTR_ERR(glue->reg); + } + ret = spi_setup(spi); if (ret < 0) { dev_err(glue->dev, "spi_setup failed\n"); -- regards, -grygorii -- 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] wlcore/wl12xx: spi: fix NULL pointer dereference (Oops)
On 12/23/2015 10:35 AM, Uri Mashiach wrote: > The power function uses a consumer regulator access to update the WiFi > enable GPIO value. > > Fix the below Oops when trying to modprobe wlcore_spi. > The oops occurs because the wl1271_power_{off,on}() > function doesn't check the power() function pointer. > > [ 23.401447] Unable to handle kernel NULL pointer dereference at > virtual address > [ 23.409954] pgd = c0004000 > [ 23.412922] [] *pgd= > [ 23.416693] Internal error: Oops: 8007 [#1] SMP ARM > [ 23.422168] Modules linked in: wl12xx wlcore mac80211 cfg80211 > musb_dsps musb_hdrc usbcore usb_common snd_soc_simple_card evdev joydev > omap_rng wlcore_spi snd_soc_tlv320aic23_i2c rng_core snd_soc_tlv320aic23 > c_can_platform c_can can_dev snd_soc_davinci_mcasp snd_soc_edma > snd_soc_omap omap_wdt musb_am335x cpufreq_dt thermal_sys hwmon > [ 23.453253] CPU: 0 PID: 36 Comm: kworker/0:2 Not tainted > 4.2.0-2-g951efee-dirty #233 > [ 23.461720] Hardware name: Generic AM33XX (Flattened Device Tree) > [ 23.468123] Workqueue: events request_firmware_work_func > [ 23.473690] task: de32efc0 ti: de4ee000 task.ti: de4ee000 > [ 23.479341] PC is at 0x0 > [ 23.482112] LR is at wl12xx_set_power_on+0x28/0x124 [wlcore] Why can't you just add proper check in wl1271_power_on/wl1271_power_off() instead? -- regards, -grygorii -- 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] wlcore/wl12xx: spi: add device tree support
On 12/23/2015 10:35 AM, Uri Mashiach wrote: > Add DT support for the wl1271 SPI WiFi. > > Add documentation file for the wl1271 SPI WiFi. > > Signed-off-by: Uri Mashiach> Acked-by: Igor Grinberg > --- > .../bindings/net/wireless/ti,wlcore,spi.txt| 35 +++ > drivers/net/wireless/ti/wlcore/spi.c | 67 > +++--- > 2 files changed, 95 insertions(+), 7 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt > > diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt > b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt > new file mode 100644 > index 000..a3e7eb7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt > @@ -0,0 +1,35 @@ > +* Texas Instruments wl1271 wireless lan controller > + > +The wl1271 chip can be connected via SPI or via SDIO. This > +document describes the binding for the SPI connected chip. > + > +Required properties: > +- compatible : Should be "ti,wl1271" > +- reg : Chip select address of device > +- spi-max-frequency : Maximum SPI clocking speed of device in Hz > +- ref-clock-frequency : Reference clock frequency > +- interrupts : Should contain interrupt line and trigger type It's good to describe here number of IRQ lines your device have and IRQ types (edge/level) > +- interrupt-parent :Should be the phandle for the interrupt controller interrupt-parent is not required (also interrupts-extended can be used) > +that services interrupts for this device > +- vwlan-supply :Point the node of the regulator that controls the > wl1271 chip WLAN_EN pin Pls, use gpio if this is required just to toggle WLAN_EN pin. [ti,] power-gpio or en-gpio > + > +Optional properties: > +- clock-xtal : boolean, clock is generated from XTAL > + > +- Please consult Documentation/devicetree/bindings/spi/spi-bus.txt > + for optional SPI connection related properties,just > + > +Examples: > + > + { > + wl1271@1 { > + compatible = "ti,wl1271"; > + > + reg = <1>; > + spi-max-frequency = <4800>; > + clock-xtal; > + interrupt-parent = <>; > + interrupts = <8 IRQ_TYPE_LEVEL_HIGH>; > + wifi-supply = <_fixed>; > + }; > +}; > diff --git a/drivers/net/wireless/ti/wlcore/spi.c > b/drivers/net/wireless/ti/wlcore/spi.c > index d3a4bcb..7281f5a 100644 > --- a/drivers/net/wireless/ti/wlcore/spi.c > +++ b/drivers/net/wireless/ti/wlcore/spi.c > @@ -30,6 +30,7 @@ > #include > #include > #include > +#include > #include > > #include "wlcore.h" > @@ -357,6 +358,54 @@ static struct wl1271_if_operations spi_ops = { > .set_block_size = NULL, > }; > > +#ifdef CONFIG_OF > +static const struct of_device_id wlcore_spi_of_match_table[] = { > + { .compatible = "ti,wl1271" }, > + { } > +}; > + > +/** > + * wlcore_probe_of - DT node parsing. > + * @spi: SPI slave device parameters. > + * @res: resource parameters. > + * @glue: wl12xx SPI bus to slave device glue parameters. > + * @pdev_data: wlcore device parameters > + */ > +static int wlcore_probe_of(struct spi_device *spi, struct resource *res, > +struct wl12xx_spi_glue *glue, > +struct wlcore_platdev_data *pdev_data) > +{ > + struct device_node *dt_node = spi->dev.of_node; > + int ret; > + > + ret = of_irq_to_resource(dt_node, 0, [0]); > + if (spi->irq != ret) { > + dev_err(glue->dev, "can't get interrupt resource\n"); > + return -EINVAL; > + } SPI core will parse IRQ for you (see below) > + > + if (of_find_property(dt_node, "clock-xtal", NULL)) > + pdev_data->ref_clock_xtal = true; > + > + ret = of_property_read_u32(dt_node, "ref-clock-frequency", > +_data->ref_clock_freq); > + if (IS_ERR_VALUE(ret)) { > + dev_err(glue->dev, > + "can't get reference clock frequency (%d)\n", ret); > + return ret; > + } > + > + return 0; > +} > +#else /* CONFIG_OF */ > +static int wlcore_probe_of(struct spi_device *spi, struct resource *res, > +struct wl12xx_spi_glue *glue, > +struct wlcore_platdev_data *pdev_data) > +{ > + return -ENODATA; > +} > +#endif /* CONFIG_OF */ > + > static int wl1271_probe(struct spi_device *spi) > { > struct wl12xx_spi_glue *glue; > @@ -365,8 +414,7 @@ static int wl1271_probe(struct spi_device *spi) > int ret; > > memset(_data, 0x00, sizeof(pdev_data)); > - > - /* TODO: add DT parsing when needed */ > + memset(res, 0x00, sizeof(res)); > > pdev_data.if_ops = _ops; > > @@ -390,6 +438,13 @@ static int wl1271_probe(struct spi_device *spi)
Re: [RFC PATCH V2 2/8] irqdomain: Don't set type when mapping an IRQ
On 12/17/2015 12:48 PM, Jon Hunter wrote: > Some IRQ chips, such as GPIO controllers or secondary level interrupt > controllers, may require require additional runtime power management > control to ensure they are accessible. For such IRQ chips, it makes sense > to enable the IRQ chip when interrupts are requested and disabled them > again once all interrupts have been freed. > > When mapping an IRQ, the IRQ type settings are read and then programmed. > The mapping of the IRQ happens before the IRQ is requested and so the > programming of the type settings occurs before the IRQ is requested. This > is a problem for IRQ chips that require additional power management > control because they may not be accessible yet. Therefore, when mapping > the IRQ, don't program the type settings, just save them and then program > these saved settings when the IRQ is requested (so long as if they are not > overridden via the call to request the IRQ). > > Signed-off-by: Jon Hunter> --- > kernel/irq/irqdomain.c | 18 ++ > kernel/irq/manage.c| 7 +++ > 2 files changed, 21 insertions(+), 4 deletions(-) > > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c > index eae31e978ab2..4322d6fd0b8f 100644 > --- a/kernel/irq/irqdomain.c > +++ b/kernel/irq/irqdomain.c > @@ -570,6 +570,7 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec > *fwspec) > { > struct device_node *of_node; > struct irq_domain *domain; > + struct irq_data *irq_data; > irq_hw_number_t hwirq; > unsigned int cur_type = IRQ_TYPE_NONE; > unsigned int type = IRQ_TYPE_NONE; > @@ -634,10 +635,19 @@ unsigned int irq_create_fwspec_mapping(struct > irq_fwspec *fwspec) > return 0; > } > > - /* Set type if specified and different than the current one */ > - if (type != IRQ_TYPE_NONE && > - type != irq_get_trigger_type(virq)) > - irq_set_irq_type(virq, type); ^^^ note: Return code hes never ever been checked here ;) This patch has side effect - some boards which have ARM TWD timer will produce below backtrace on boot: genirq: Setting trigger mode 4 for irq 17 failed (gic_set_type+0x0/0x58) twd: can't register interrupt 17 (-22) [ cut here ] WARNING: CPU: 0 PID: 0 at arch/arm/kernel/smp_twd.c:405 twd_local_timer_of_register+0x68/0x7c() twd_local_timer_of_register failed (-22) Modules linked in: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.1.13-01909-g59e42df #53 Hardware name: Generic AM43 (Flattened Device Tree) Backtrace: [] (dump_backtrace) from [] (show_stack+0x18/0x1c) r7:c0797cec r6: r5:c08fa7bc r4: [] (show_stack) from [] (dump_stack+0x98/0xe0) [] (dump_stack) from [] (warn_slowpath_common+0x7c/0xb8) r7:c0797cec r6:0195 r5:0009 r4:c08b3f30 [] (warn_slowpath_common) from [] (warn_slowpath_fmt+0x38/0x40) r8:c0932000 r7:c08b48c0 r6: r5:ee5c9fb4 r4:c0797cc0 [] (warn_slowpath_fmt) from [] (twd_local_timer_of_register+0x68/0x7c) r3:ffea r2:c0797cc0 r4:c09322c4 [] (twd_local_timer_of_register) from [] (clocksource_of_init+0x50/0x90) r5:0001 r4:ee5c9fb4 [] (clocksource_of_init) from [] (omap4_local_timer_init+0x34/0x68) r5:c0932000 r4: [] (omap4_local_timer_init) from [] (time_init+0x24/0x38) [] (time_init) from [] (start_kernel+0x248/0x3e4) [] (start_kernel) from [<8000807c>] (0x8000807c) r10: r9:412fc09a r8:80004059 r7:c08b8884 r6:c089934c r5:c08b4970 r4:c0932214 ---[ end trace cb88537fdc8fa200 ]--- This happens, most probably, because TWD IRQs definitions in DT do not corresponds HW and gic_configure_irq() will return -EINVAL example (am4372.dtsi): local_timer: timer@48240600 { compatible = "arm,cortex-a9-twd-timer"; reg = <0x48240600 0x100>; interrupts = ; interrupt-parent = <>; clocks = <_periphclk>; status = "disabled"; }; So, some additional fixes might be required. Potentially problematic files are: arch/arm/boot/dts/bcm5301x.dtsi arch/arm/boot/dts/bcm63138.dtsi arch/arm/boot/dts/berlin2cd.dtsi arch/arm/boot/dts/berlin2.dtsi arch/arm/boot/dts/berlin2q.dtsi arch/arm/boot/dts/hip01.dtsi arch/arm/boot/dts/omap4.dtsi arch/arm/boot/dts/r8a7779.dts arch/arm/boot/dts/rk3xxx.dtsi arch/arm/boot/dts/sh73a0.dtsi arch/arm/boot/dts/socfpga_arria10.dtsi arch/arm/boot/dts/socfpga.dtsi arch/arm/boot/dts/spear13xx.dtsi arch/arm/boot/dts/ste-dbx5x0.dtsi arch/arm/boot/dts/tegra20.dtsi arch/arm/boot/dts/tegra30.dtsi arch/arm/boot/dts/uniphier-ph1-ld4.dtsi arch/arm/boot/dts/uniphier-ph1-.dtsi arch/arm/boot/dts/uniphier-proxstream2.dtsi arch/arm/boot/dts/vexpress-v2p-ca5s.dts arch/arm/boot/dts/vexpress-v2p-ca9.dts Any way, It seems working for me, but I've back-ported and tested it on kernel 4.1 and will try to do more tests this week. Thanks a lot. -- regards, -grygorii -- To unsubscribe from this list: send
Re: [PATCH v4] clocksource: arm_global_timer: fix suspend resume
Hi All, On 11/30/2015 08:25 PM, Grygorii Strashko wrote: > Now the System stall is observed on TI AM437x based board > (am437x-gp-evm) during resuming from System suspend when ARM Global > timer is selected as clocksource device (CPUIdle not enabled) - SysRq are > working, > but nothing else. > > The reason of stall is that ARM Global timer loses its contexts during > System suspend: > GT_CONTROL.TIMER_ENABLE = 0 (unbanked) > GT_COUNTERx = 0 > > Hence, update ARM Global timer driver to reflect above behaviour > - re-enable ARM Global timer on resume (GT_CONTROL.TIMER_ENABLE = 1) >if not enabled. > > CC: Arnd Bergmann <a...@arndb.de> > Cc: John Stultz <john.stu...@linaro.org> > Cc: Felipe Balbi <ba...@ti.com> > Cc: Tony Lindgren <t...@atomide.com> > Cc: Marc Zyngier <marc.zyng...@arm.com> > Reviewed-by: Santosh Shilimkar <ssant...@kernel.org> > Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> Are there any comments? Do I need to perform any additional actions to have it merged? -- regards, -grygorii -- 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 0/2] arm: omap2: AM43xx: enable ARM TWD timer
Now ARM TWD timer can be enabled for AM437x devices since all prerequisite patches have been merged already [1] [2]. But before finally enable ARM TWD timer for UP AM437x devices - the Broadcast event source and infrustructure need to be enabled properly. Otherwise CPUIdle will be broken. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-August/363989.html [2] http://www.spinics.net/lists/linux-omap/msg123940.html Felipe Balbi (1): arm: omap2: AM43xx: select ARM TWD timer Grygorii Strashko (1): ARM: OMAP: am43xx: enable GENERIC_CLOCKEVENTS_BROADCAST arch/arm/mach-omap2/Kconfig | 2 ++ arch/arm/mach-omap2/timer.c | 6 ++ 2 files changed, 8 insertions(+) -- 2.6.4 -- 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 1/2] ARM: OMAP: am43xx: enable GENERIC_CLOCKEVENTS_BROADCAST
System will misbehave in the following case: - AM43XX only build (UP); - CONFIG_CPU_IDLE=y - ARM TWD timer enabled and selected as clockevent device. In the above case, It's expected that broadcast timer will be used as backup timer when CPUIdle will put MPU in low power states where ARM TWD will stop and lose its context. But, the CONFIG_SMP might not be selected when kernel is built for AM43XX SoC only and, as result, GENERIC_CLOCKEVENTS_BROADCAST option will not be selected also. This will break CPUIdle and System will stuck in low power states. Hence, fix it by selecting GENERIC_CLOCKEVENTS_BROADCAST option for AM43XX SoCs always and add empty tick_broadcast() function implementation - no need to send any IPI on UP. After this change timer1 will be selected as broadcast timer the same way as for SMP, and CPUIdle will work properly. Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- arch/arm/mach-omap2/Kconfig | 1 + arch/arm/mach-omap2/timer.c | 6 ++ 2 files changed, 7 insertions(+) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index 4b4371d..32a0086 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -65,6 +65,7 @@ config SOC_AM43XX select MACH_OMAP_GENERIC select MIGHT_HAVE_CACHE_L2X0 select HAVE_ARM_SCU + select GENERIC_CLOCKEVENTS_BROADCAST config SOC_DRA7XX bool "TI DRA7XX" diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c index 3bfde44..f34a809 100644 --- a/arch/arm/mach-omap2/timer.c +++ b/arch/arm/mach-omap2/timer.c @@ -320,6 +320,12 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer, return r; } +#if !defined(CONFIG_SMP) && defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) +void tick_broadcast(const struct cpumask *mask) +{ +} +#endif + static void __init omap2_gp_clockevent_init(int gptimer_id, const char *fck_source, const char *property) -- 2.6.4 -- 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 v2 2/2] arm: omap2: AM43xx: select ARM TWD timer
From: Felipe Balbi <ba...@ti.com> Make sure to tell the kernel that AM437x devices have ARM TWD timer. Signed-off-by: Felipe Balbi <ba...@ti.com> [grygorii.stras...@ti.com: drop ARM Global timer selection, because it's incompatible with PM (cpuidle/cpufreq). So, it's unsafe to enable it unconditionally] Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- v1: http://www.spinics.net/lists/arm-kernel/msg459649.html arch/arm/mach-omap2/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index 32a0086..0517f0c 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -66,6 +66,7 @@ config SOC_AM43XX select MIGHT_HAVE_CACHE_L2X0 select HAVE_ARM_SCU select GENERIC_CLOCKEVENTS_BROADCAST + select HAVE_ARM_TWD config SOC_DRA7XX bool "TI DRA7XX" -- 2.6.4 -- 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 12/11/2015 06:57 PM, Sebastian Andrzej Siewior wrote: * Grygorii Strashko | 2015-10-15 19:33:43 [+0300]: Hi Thomas, On 10/13/2015 09:33 PM, Thomas Gleixner wrote: Grygorii, On Tue, 13 Oct 2015, Grygorii Strashko wrote: I'd very appreciate for any advice of how to better proceed with your request. - I can try to apply and re-send only patches marked by '*' - I can prepare branch with all above patches Please provide a branch on top of 4.1.10 which contains the whole lot. I have a look at it and decide then how to go from there. I've prepared branch linux-4.1.10-ti-gpio: in g...@git.ti.com:~gragst/ti-linux-kernel/gragsts-ti-linux-kernel.git could you please provide a git URL for that? git://git.ti.com/~gragst/ti-linux-kernel/gragsts-ti-linux-kernel.git based on top of git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git branch : linux-4.1.y commit : 27f1b7f Linux 4.1.10 -- regards, -grygorii -- 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] clk: ti: dra7: fix kernel boot with arg 'clocksource=gp_timer'
The OMAP Platform code provides possibility to select GP Timer as default clocksource instead of counter_32K by using bootcmd parameter 'clocksource', but the system will crash during early boot when this option is used on dra7 or omap5 platforms, because it will hit BUG() statement: omap2_gptimer_clocksource_init ->BUG_ON(res); This happens because clk_dev alias "sys_clkin_ck" is not registered. Hence, fix it by adding missing "sys_clkin_ck" clk_dev aliases definitions for omap5 and dra7. Cc: Tero Kristo <t-kri...@ti.com> Cc: Tony Lindgren <t...@atomide.com> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- drivers/clk/ti/clk-54xx.c | 1 + drivers/clk/ti/clk-7xx.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/clk/ti/clk-54xx.c b/drivers/clk/ti/clk-54xx.c index 59ce2fa..294bc03 100644 --- a/drivers/clk/ti/clk-54xx.c +++ b/drivers/clk/ti/clk-54xx.c @@ -210,6 +210,7 @@ static struct ti_dt_clk omap54xx_clks[] = { DT_CLK("usbhs_omap", "usbtll_fck", "dummy_ck"), DT_CLK("omap_wdt", "ick", "dummy_ck"), DT_CLK(NULL, "timer_32k_ck", "sys_32k_ck"), + DT_CLK(NULL, "sys_clkin_ck", "sys_clkin"), DT_CLK("4ae18000.timer", "timer_sys_ck", "sys_clkin"), DT_CLK("48032000.timer", "timer_sys_ck", "sys_clkin"), DT_CLK("48034000.timer", "timer_sys_ck", "sys_clkin"), diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c index a911d7d..87a87b5 100644 --- a/drivers/clk/ti/clk-7xx.c +++ b/drivers/clk/ti/clk-7xx.c @@ -289,6 +289,7 @@ static struct ti_dt_clk dra7xx_clks[] = { DT_CLK("usbhs_omap", "usbtll_fck", "dummy_ck"), DT_CLK("omap_wdt", "ick", "dummy_ck"), DT_CLK(NULL, "timer_32k_ck", "sys_32k_ck"), + DT_CLK(NULL, "sys_clkin_ck", "timer_sys_clk_div"), DT_CLK("4ae18000.timer", "timer_sys_ck", "timer_sys_clk_div"), DT_CLK("48032000.timer", "timer_sys_ck", "timer_sys_clk_div"), DT_CLK("48034000.timer", "timer_sys_ck", "timer_sys_clk_div"), -- 2.6.4 -- 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 v3] PCI: hosts: mark pcie/pci (msi) irq cascade handler as IRQF_NO_THREAD
On -RT and if kernel is booting with "threadirqs" cmd line parameter pcie/pci (msi) irq cascade handlers (like dra7xx_pcie_msi_irq_handler()) will be forced threaded and, as result, will generate warnings like: WARNING: CPU: 1 PID: 82 at kernel/irq/handle.c:150 handle_irq_event_percpu+0x14c/0x174() irq 460 handler irq_default_primary_handler+0x0/0x14 enabled interrupts Backtrace: (warn_slowpath_common) from [] (warn_slowpath_fmt+0x38/0x40) (warn_slowpath_fmt) from [] (handle_irq_event_percpu+0x14c/0x174) (handle_irq_event_percpu) from [] (handle_irq_event+0x84/0xb8) (handle_irq_event) from [] (handle_simple_irq+0x90/0x118) (handle_simple_irq) from [] (generic_handle_irq+0x30/0x44) (generic_handle_irq) from [] (dra7xx_pcie_msi_irq_handler+0x7c/0x8c) (dra7xx_pcie_msi_irq_handler) from [] (irq_forced_thread_fn+0x28/0x5c) (irq_forced_thread_fn) from [] (irq_thread+0x128/0x204) This happens because all of them invoke generic_handle_irq() from the requsted handler. generic_handle_irq grabs raw_locks and this needs to run in raw-irq context. This issue was originally reproduced on TI dra7-evem, but, as was identified during dicussion [1], other PCI(e) hosts can also suffer from this issue. So let's fix all them at once and mark pcie/pci (msi) irq cascade handlers IRQF_NO_THREAD explicitly. [1] https://lkml.org/lkml/2015/11/20/356 Cc: Kishon Vijay Abraham I <kis...@ti.com> Cc: Bjorn Helgaas <bhelg...@google.com> Cc: Jingoo Han <jingooh...@gmail.com> Cc: Kukjin Kim <kg...@kernel.org> Cc: Krzysztof Kozlowski <k.kozlow...@samsung.com> Cc: Richard Zhu <richard@freescale.com> Cc: Lucas Stach <l.st...@pengutronix.de> Cc: Thierry Reding <thierry.red...@gmail.com> Cc: Stephen Warren <swar...@wwwdotorg.org> Cc: Alexandre Courbot <gnu...@gmail.com> Cc: Simon Horman <ho...@verge.net.au> Cc: Pratyush Anand <pratyush.an...@gmail.com> Cc: Michal Simek <michal.si...@xilinx.com> Cc: "Sören Brinkmann" <soren.brinkm...@xilinx.com> Cc: Sebastian Andrzej Siewior <bige...@linutronix.de> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- Changes in v3: - change applied to all affected pci(e) host drivers in drivers/pci/hosts. After some invsetigation I've decided to not touch arch code - it is not easy to identify all places which need to be fixed. if it's still required - i can send separate patches for arch/mips/pci/msi-octeon.c and arch/sparc/kernel/pci_msi.c. Links v2: https://lkml.org/lkml/2015/11/20/356 v1: https://lkml.org/lkml/2015/11/5/593 ref: https://lkml.org/lkml/2015/11/3/660 drivers/pci/host/pci-dra7xx.c | 13 - drivers/pci/host/pci-exynos.c | 3 ++- drivers/pci/host/pci-imx6.c | 3 ++- drivers/pci/host/pci-tegra.c | 2 +- drivers/pci/host/pcie-rcar.c | 6 -- drivers/pci/host/pcie-spear13xx.c | 3 ++- drivers/pci/host/pcie-xilinx.c| 3 ++- 7 files changed, 25 insertions(+), 8 deletions(-) diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c index 8c36880..0415192 100644 --- a/drivers/pci/host/pci-dra7xx.c +++ b/drivers/pci/host/pci-dra7xx.c @@ -301,8 +301,19 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx, return -EINVAL; } + /* +* Mark dra7xx_pcie_msi IRQ as IRQF_NO_THREAD +* On -RT and if kernel is booting with "threadirqs" cmd line parameter +* the dra7xx_pcie_msi_irq_handler() will be forced threaded but, +* in the same time, it's IRQ dispatcher and calls generic_handle_irq(), +* which, in turn, will be resolved to handle_simple_irq() call. +* The handle_simple_irq() expected to be called with IRQ disabled, as +* result kernle will display warning: +* "irq XXX handler YYY+0x0/0x14 enabled interrupts". +*/ ret = devm_request_irq(>dev, pp->irq, - dra7xx_pcie_msi_irq_handler, IRQF_SHARED, + dra7xx_pcie_msi_irq_handler, + IRQF_SHARED | IRQF_NO_THREAD, "dra7-pcie-msi", pp); if (ret) { dev_err(>dev, "failed to request irq\n"); diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c index 01095e1..d997d22 100644 --- a/drivers/pci/host/pci-exynos.c +++ b/drivers/pci/host/pci-exynos.c @@ -522,7 +522,8 @@ static int __init exynos_add_pcie_port(struct pcie_port *pp, ret = devm_request_irq(>dev, pp->msi_irq, exynos_pcie_msi_irq_handler, - IRQF_SHARED, "exynos-pcie", pp); + IRQF_SHARED | IRQF_NO_THREAD, + "exynos-pcie", pp); if (ret) {
Re: [PATCH v2] PCI: dra7xx: mark dra7xx_pcie_msi irq as IRQF_NO_THREAD
On 12/09/2015 05:24 PM, Bjorn Helgaas wrote: On Tue, Dec 08, 2015 at 10:05:56AM +0100, Sebastian Andrzej Siewior wrote: * Bjorn Helgaas | 2015-12-04 12:46:19 [-0600]: The backtrace might be OK (maybe slightly overkill), but all the stack addresses are certainly irrelevant and distracting. We only need enough to recognize the problem. I don't think the modules list is relevant either. I would shorten it to the bare minimum. Also the patch description itself could be truncated to the required bits… diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c index 8c36880..0415192 100644 --- a/drivers/pci/host/pci-dra7xx.c +++ b/drivers/pci/host/pci-dra7xx.c @@ -301,8 +301,19 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx, return -EINVAL; } + /* +* Mark dra7xx_pcie_msi IRQ as IRQF_NO_THREAD +* On -RT and if kernel is booting with "threadirqs" cmd line parameter +* the dra7xx_pcie_msi_irq_handler() will be forced threaded but, +* in the same time, it's IRQ dispatcher and calls generic_handle_irq(), +* which, in turn, will be resolved to handle_simple_irq() call. +* The handle_simple_irq() expected to be called with IRQ disabled, as +* result kernle will display warning: +* "irq XXX handler YYY+0x0/0x14 enabled interrupts". +*/ …not to mention this piece. d7ce4377494a ("powerpc/fsl_msi: mark the msi cascade handler IRQF_NO_THREAD") fixes the same bug in arch/ppc so they bypassed you fixing it. ret = devm_request_irq(>dev, pp->irq, - dra7xx_pcie_msi_irq_handler, IRQF_SHARED, + dra7xx_pcie_msi_irq_handler, + IRQF_SHARED | IRQF_NO_THREAD, "dra7-pcie-msi", pp); There's similar code in exynos_add_pcie_port(), imx6_add_pcie_port(), and spear13xx_add_pcie_port(). Do they need similar changes? If not, why not? You are right. The request for the handler exynos_pcie_msi_irq_handler(), imx6_pcie_msi_handler() and spear13xx_pcie_irq_handler() needs same treatment. Additionally we have: arch/mips/pci/msi-octeon.c: if (request_irq(OCTEON_IRQ_PCI_MSI0, octeon_msi_interrupt0, arch/sparc/kernel/pci_msi.c:err = request_irq(irq, sparc64_msiq_interrupt, 0, drivers/pci/host/pci-tegra.c: err = request_irq(msi->irq, tegra_pcie_msi_irq, 0, drivers/pci/host/pcie-rcar.c: err = devm_request_irq(>dev, msi->irq1, rcar_pcie_msi_irq, drivers/pci/host/pcie-rcar.c: err = devm_request_irq(>dev, msi->irq2, rcar_pcie_msi_irq, drivers/pci/host/pcie-xilinx.c: err = devm_request_irq(dev, port->irq, xilinx_pcie_intr_handler, which require the same kind of fix… I see your discussion about DRA7 hardware design, but my impression is that this problem affects anybody who calls dw_handle_msi_irq() from a handler registered with IRQF_SHARED. … brecause all of them invoke generic_handle_irq() from the requsted handler. generic_handle_irq grabs raw_locks and this needs to run in raw-irq context. IRQF_SHARED could probably go away. The IRQ is mostlikely exclusive assigned in each SoC for MSI interrupt demux. It sounds like we should fix all these places at once. If you (Grygorii) work up a patch that does them all, with a more generic changelog, then we can solicit testing and acks. Sure, will do: - change will be applied for files listed in this thread only - IRQF_SHARED will be left unchanged - commit message will be made more generic - i prefer to keep comment in code for dra7 as is. Is it ok? -- regards, -grygorii -- 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.4-rc][PATCH v2] ARM: dts: am4372: fix clock source for arm twd and global timers
Hi Tony, Felipe, Tero, On 12/03/2015 08:04 PM, Grygorii Strashko wrote: On 12/03/2015 06:35 PM, Tony Lindgren wrote: * Grygorii Strashko <grygorii.stras...@ti.com> [151130 07:58]: ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2. But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result. Timekeeping core misbehaves. For example, execution of command "sleep 5" will take 10 sec instead of 5. Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use it for clocking ARM TWD and Global timer (same way as on OMAP4). Cc: Tony Lindgren <t...@atomide.com> Cc: Felipe Balbi <ba...@ti.com> Cc: Tero Kristo <t-kri...@ti.com> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and SCU nodes") Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> Is this fix alone enough or do you still need to disable other the ARM timers in the board specific dts file? This one is independent and it's required for -rc, because ARM TWD timer will be selected now for am43xx even if http://www.spinics.net/lists/arm-kernel/msg459649.html is not merged yet ;) in case of omap2plus_defconfig build. Are there any comments objection? Could this patch be merged as part of -rc cycle? -- regards, -grygorii -- 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: single: remove misuse of IRQF_NO_SUSPEND flag
On 12/03/2015 11:37 PM, Tony Lindgren wrote: * Grygorii Strashko <grygorii.stras...@ti.com> [151203 10:36]: I think, this patch should not break our wake-up functionality. It will just change the moment when pcs_irq_handler() will be called: before this change: - suspend_enter() - arch_suspend_enable_irqs(); - ^ right here after this change: - suspend_enter() dpm_resume_noirq() - resume_device_irqs() ^ here Correct? And as for me this is more safe. I think there's more to it though. With both applied, it produces this on coming back up from suspend: PM: noirq resume of devices complete after 18.127 msecs [ cut here ] WARNING: CPU: 0 PID: 123 at kernel/irq/manage.c:605 irq_set_irq_wake+0xbc/0xfc() Unbalanced IRQ 375 wake disable Modules linked in: ledtrig_default_on leds_gpio led_class rtc_twl twl4030_wdt CPU: 0 PID: 123 Comm: bash Tainted: GW 4.4.0-rc3-dirty #2682 Hardware name: Generic OMAP36xx (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) ] (show_stack) from [] (dump_stack+0x84/0x9c) [] (dump_stack) from [] (warn_slowpath_common+0x7c/0xb8) [] (warn_slowpath_common) from [] (warn_slowpath_fmt+0x30/0x40) [] (warn_slowpath_fmt) from [] (irq_set_irq_wake+0xbc/0xfc) [] (irq_set_irq_wake) from [] (device_wakeup_disarm_wake_irqs+0x70/0x12c) [] (device_wakeup_disarm_wake_irqs) from [] (dpm_resume_noirq+0x20c/0x2e4) [] (dpm_resume_noirq) from [] (suspend_devices_and_enter+0x1e4/0x6bc) [] (suspend_devices_and_enter) from [] (pm_suspend+0x358/0x4b8) [] (pm_suspend) from [] (state_store+0x64/0xb8) [] (state_store) from [] (kobj_attr_store+0x14/0x20) [] (kobj_attr_store) from [] (sysfs_kf_write+0x4c/0x50) [] (sysfs_kf_write) from [] (kernfs_fop_write+0xbc/0x1cc) [] (kernfs_fop_write) from [] (__vfs_write+0x24/0xd8) [] (__vfs_write) from [] (vfs_write+0x94/0x154) [] (vfs_write) from [] (SyS_write+0x40/0x94) [] (SyS_write) from [] (ret_fast_syscall+0x0/0x1c) ---[ end trace 321b51565e161bee ]--- And these both need to be applied together when we have a fix for the above as otherwise we'll get the lock recursion Sudeep mentioned in patch 2/2. Most probably below diff will fix above issue: diff --git a/arch/arm/mach-omap2/prm_common.c b/arch/arm/mach-omap2/prm_common.c index 3fc2cbe..69cde67 100644 --- a/arch/arm/mach-omap2/prm_common.c +++ b/arch/arm/mach-omap2/prm_common.c @@ -338,6 +338,7 @@ int omap_prcm_register_chain_handler(struct omap_prcm_irq_setup *irq_setup) ct->chip.irq_ack = irq_gc_ack_set_bit; ct->chip.irq_mask = irq_gc_mask_clr_bit; ct->chip.irq_unmask = irq_gc_mask_set_bit; + ct->chip.flags = IRQCHIP_SKIP_SET_WAKE; ct->regs.ack = irq_setup->ack + i * 4; ct->regs.mask = irq_setup->mask + i * 4; -- regards, -grygorii -- 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: single: remove misuse of IRQF_NO_SUSPEND flag
On 12/04/2015 12:54 PM, Sudeep Holla wrote: > Hi Grygorii, > > On 04/12/15 10:44, Grygorii Strashko wrote: >> On 12/03/2015 11:37 PM, Tony Lindgren wrote: > > [...] > >>> And these both need to be applied together when we have a fix for the >>> above >>> as otherwise we'll get the lock recursion Sudeep mentioned in patch 2/2. >>> >> >> Most probably below diff will fix above issue: >> >> diff --git a/arch/arm/mach-omap2/prm_common.c >> b/arch/arm/mach-omap2/prm_common.c >> index 3fc2cbe..69cde67 100644 >> --- a/arch/arm/mach-omap2/prm_common.c >> +++ b/arch/arm/mach-omap2/prm_common.c >> @@ -338,6 +338,7 @@ int omap_prcm_register_chain_handler(struct >> omap_prcm_irq_setup *irq_setup) >> ct->chip.irq_ack = irq_gc_ack_set_bit; >> ct->chip.irq_mask = irq_gc_mask_clr_bit; >> ct->chip.irq_unmask = irq_gc_mask_set_bit; >> + ct->chip.flags = IRQCHIP_SKIP_SET_WAKE; > > Thanks for testing. Sry, I've not tested it yet - it's just fast assumption :( In that case without this hunk, we should get error > from pcs_irq_set_wake in the suspend path. No ? May be driver is not > checking the error value and entering suspend. > Yep. Noone is checking return result from enable_irq_wake() in suspend path (see dev_pm_arm_wake_irq()). Actually, return result of enable_irq_wake() is checked only in ~30% of cases in kernel now :) -- regards, -grygorii -- 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: single: remove misuse of IRQF_NO_SUSPEND flag
On 12/04/2015 05:35 PM, Tony Lindgren wrote: > * Grygorii Strashko <grygorii.stras...@ti.com> [151204 02:45]: >> On 12/03/2015 11:37 PM, Tony Lindgren wrote: >>> * Grygorii Strashko <grygorii.stras...@ti.com> [151203 10:36]: >>>> >>>> I think, this patch should not break our wake-up functionality. >>>> It will just change the moment when pcs_irq_handler() will be called: >>>> >>>> before this change: >>>> - suspend_enter() >>>> >>>>- arch_suspend_enable_irqs(); >>>> - ^ right here >>>> >>>> after this change: >>>> - suspend_enter() >>>> >>>>dpm_resume_noirq() >>>>- resume_device_irqs() >>>> ^ here >>>> >>>> Correct? And as for me this is more safe. >>> >>> I think there's more to it though. With both applied, it produces this on >>> coming back up from suspend: >>> >>> PM: noirq resume of devices complete after 18.127 msecs >>> [ cut here ] >>> WARNING: CPU: 0 PID: 123 at kernel/irq/manage.c:605 >>> irq_set_irq_wake+0xbc/0xfc() >>> Unbalanced IRQ 375 wake disable >>> Modules linked in: ledtrig_default_on leds_gpio led_class rtc_twl >>> twl4030_wdt >>> CPU: 0 PID: 123 Comm: bash Tainted: GW 4.4.0-rc3-dirty #2682 >>> Hardware name: Generic OMAP36xx (Flattened Device Tree) >>> [] (unwind_backtrace) from [] (show_stack+0x10/0x14) >>> ] (show_stack) from [] (dump_stack+0x84/0x9c) >>> [] (dump_stack) from [] (warn_slowpath_common+0x7c/0xb8) >>> [] (warn_slowpath_common) from [] >>> (warn_slowpath_fmt+0x30/0x40) >>> [] (warn_slowpath_fmt) from [] >>> (irq_set_irq_wake+0xbc/0xfc) >>> [] (irq_set_irq_wake) from [] >>> (device_wakeup_disarm_wake_irqs+0x70/0x12c) >>> [] (device_wakeup_disarm_wake_irqs) from [] >>> (dpm_resume_noirq+0x20c/0x2e4) >>> [] (dpm_resume_noirq) from [] >>> (suspend_devices_and_enter+0x1e4/0x6bc) >>> [] (suspend_devices_and_enter) from [] >>> (pm_suspend+0x358/0x4b8) >>> [] (pm_suspend) from [] (state_store+0x64/0xb8) >>> [] (state_store) from [] (kobj_attr_store+0x14/0x20) >>> [] (kobj_attr_store) from [] (sysfs_kf_write+0x4c/0x50) >>> [] (sysfs_kf_write) from [] >>> (kernfs_fop_write+0xbc/0x1cc) >>> [] (kernfs_fop_write) from [] (__vfs_write+0x24/0xd8) >>> [] (__vfs_write) from [] (vfs_write+0x94/0x154) >>> [] (vfs_write) from [] (SyS_write+0x40/0x94) >>> [] (SyS_write) from [] (ret_fast_syscall+0x0/0x1c) >>> ---[ end trace 321b51565e161bee ]--- >>> >>> And these both need to be applied together when we have a fix for the above >>> as otherwise we'll get the lock recursion Sudeep mentioned in patch 2/2. >>> >> >> Most probably below diff will fix above issue: >> >> diff --git a/arch/arm/mach-omap2/prm_common.c >> b/arch/arm/mach-omap2/prm_common.c >> index 3fc2cbe..69cde67 100644 >> --- a/arch/arm/mach-omap2/prm_common.c >> +++ b/arch/arm/mach-omap2/prm_common.c >> @@ -338,6 +338,7 @@ int omap_prcm_register_chain_handler(struct >> omap_prcm_irq_setup *irq_setup) >> ct->chip.irq_ack = irq_gc_ack_set_bit; >> ct->chip.irq_mask = irq_gc_mask_clr_bit; >> ct->chip.irq_unmask = irq_gc_mask_set_bit; >> + ct->chip.flags = IRQCHIP_SKIP_SET_WAKE; >> >> ct->regs.ack = irq_setup->ack + i * 4; >> ct->regs.mask = irq_setup->mask + i * 4; >> >> > > That fixes the warning on resume, but adds a new one during init: > > [ cut here ] > WARNING: CPU: 0 PID: 1 at kernel/irq/pm.c:51 irq_pm_install_action+0x9c/0xec() > Modules linked in: > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.4.0-rc3-1-g6a5e5ec #2694 > Hardware name: Generic OMAP36xx (Flattened Device Tree) > [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > [] (show_stack) from [] (dump_stack+0x84/0x9c) > [] (dump_stack) from [] (warn_slowpath_common+0x7c/0xb8) > [] (warn_slowpath_common) from [] > (warn_slowpath_null+0x1c/0x24) > [] (warn_slowpath_null) from [] > (irq_pm_install_action+0x9c/0xec) > [] (irq_pm_install_action) from [] > (__setup_irq+0x434/0x5e0) > [] (__setup_irq) from [] (request_threaded_irq+0xc4/0x15c) > [] (request_threaded_irq) from [] > (omap3_pm_init+0x10c/0x400) > [] (omap3_pm_init) from [] (omap3_i
Re: [PATCH 2/2] pinctrl: single: remove misuse of IRQF_NO_SUSPEND flag
On 12/04/2015 05:44 PM, Sudeep Holla wrote: > > > On 04/12/15 15:40, Tony Lindgren wrote: >> * Tony Lindgren[151203 13:41]: >>> * Sudeep Holla [151203 11:00]: I have added irq_set_irq_wake(pcs_soc->irq, state) in pcs_irq_set_wake which ensures it's marked for wakeup. >>> >>> Hmm well see the error I pasted in this thread, maybe that provides >>> more clues. >> >> The irq_set_irq_wake(pcs_soc->irq, state) in pcs_irq_set_wake does not >> look right to me as pcs_irq_set_wake toggles the irq_wake for each pin >> separately, not for the whole controller. >> > > OK, my understanding was that this driver supports multiple single > pinmux with one main irq `pcs_soc->irq`. Hence I added the wakeup on > that irq. I now think that understand is wrong. > With this change, PCS parent IRQ will be marked as wake up source as many times as many pins were requested as wake up IRQs (protected by counter). Most of all GPIO IRQ chips work this way. Of course, if we will look on pinctrl-single.c from only OMAP point of view then Prent IRQ can be marked as wake up source from probe only once. But, since this driver expected to be generic - this patch is more correct, because other HW may require to perform some real HW re-configuration to enable/disable wake up capabilities for Parent IRQ in Parent IRQ controller. Any way, in my opinion, it's right and more safe to manage all wakeup IRQs through IRQ PM core and Device wakeirq framework. And this patch should just go together with platform changes and not alone. -- regards, -grygorii -- 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: single: remove misuse of IRQF_NO_SUSPEND flag
On 12/04/2015 06:11 PM, Sudeep Holla wrote: > > > On 04/12/15 15:59, Grygorii Strashko wrote: >> >> Sorry, I can't test it right now :( >> Potential fix below: > > I had posted similar patch a while ago which Tony rejected. > I might have made a mistake of not putting them together, though they > were part of the same series[1], patch 12 and 16 True. I've remembered that I saw smth. like this, but I was not able to find it :( -- regards, -grygorii -- 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] PCI: dra7xx: mark dra7xx_pcie_msi irq as IRQF_NO_THREAD
On 12/04/2015 08:46 PM, Bjorn Helgaas wrote: > Hi Grygorii, > > On Fri, Nov 20, 2015 at 03:59:26PM +0200, Grygorii Strashko wrote: >> On -RT, TI DRA7 PCIe driver always produces below backtrace when the >> first PCI interrupt is triggered: >> >> [ cut here ] >> WARNING: CPU: 1 PID: 82 at kernel/irq/handle.c:150 >> handle_irq_event_percpu+0x14c/0x174() >> irq 460 handler irq_default_primary_handler+0x0/0x14 enabled interrupts [...] >> [] (handle_simple_irq) from [] >> (generic_handle_irq+0x30/0x44) >> r5:ee4a9020 r4:01cc >> [] (generic_handle_irq) from [] >> (dra7xx_pcie_msi_irq_handler+0x7c/0x8c) >> r5:ee4a9020 r4:0001 >> [] (dra7xx_pcie_msi_irq_handler) from [] >> (irq_forced_thread_fn+0x28/0x5c) >> r5:ee1d0900 r4:ee4b59c0 >> [] (irq_forced_thread_fn) from [] >> (irq_thread+0x128/0x204) >> r7:0001 r6: r5:ee4d2000 r4:ee4b59e4 >> [] (irq_thread) from [] (kthread+0xd4/0xec) >> r10: r9: r8: r7:c00870b4 r6:ee4b59c0 r5:ee4b5a00 >> r4: >> [] (kthread) from [] (ret_from_fork+0x14/0x2c) >> r7: r6: r5:c005e228 r4:ee4b5a00 >> ---[ end trace 0002 ]--- > > The backtrace might be OK (maybe slightly overkill), but all the > stack addresses are certainly irrelevant and distracting. We only > need enough to recognize the problem. I don't think the modules list > is relevant either. I'll take this into account. Is this blocker for this patch now? > >> This backtrace is triggered because dra7xx_pcie_msi_irq_handler() >> forced to be threaded by default on -RT but, in the same time, it's >> IRQ dispatcher and calls generic_handle_irq(), which leads to >> handle_simple_irq() call. The handle_simple_irq() expected to be >> called with IRQ disabled. >> >> The same issue will also happen if kernel will boot with "threadirqs" >> cmdline parameter (CONFIG_IRQ_FORCED_THREADING). >> >> As discussed in [1], the current DRA7 PCIe hw configuration supports >> 32 interrupts, which cannot change because it's hardwired in silicon, >> this is a single status read and assuming that only a few (most of the >> time it will be exactly ONE) of those interrupts are pending at the >> same time is pretty much a sane assumption. And recommended fix for >> this issue is to request dra7xx_pcie_msi IRQ with IRQF_NO_THREAD flag. >> >> [1] https://lkml.org/lkml/2015/11/3/660 >> >> Cc: Thomas Gleixner <t...@linutronix.de> >> Cc: Sebastian Andrzej Siewior <bige...@linutronix.de> >> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> >> --- >> Changes in v2: >> - comment in code truncated >> Link on v1: >> https://lkml.org/lkml/2015/11/5/593 >> drivers/pci/host/pci-dra7xx.c | 13 - >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c >> index 8c36880..0415192 100644 >> --- a/drivers/pci/host/pci-dra7xx.c >> +++ b/drivers/pci/host/pci-dra7xx.c >> @@ -301,8 +301,19 @@ static int __init dra7xx_add_pcie_port(struct >> dra7xx_pcie *dra7xx, >> return -EINVAL; >> } >> >> +/* >> + * Mark dra7xx_pcie_msi IRQ as IRQF_NO_THREAD >> + * On -RT and if kernel is booting with "threadirqs" cmd line parameter >> + * the dra7xx_pcie_msi_irq_handler() will be forced threaded but, >> + * in the same time, it's IRQ dispatcher and calls generic_handle_irq(), >> + * which, in turn, will be resolved to handle_simple_irq() call. >> + * The handle_simple_irq() expected to be called with IRQ disabled, as >> + * result kernle will display warning: >> + * "irq XXX handler YYY+0x0/0x14 enabled interrupts". >> + */ >> ret = devm_request_irq(>dev, pp->irq, >> - dra7xx_pcie_msi_irq_handler, IRQF_SHARED, >> + dra7xx_pcie_msi_irq_handler, >> + IRQF_SHARED | IRQF_NO_THREAD, >> "dra7-pcie-msi", pp); > > There's similar code in exynos_add_pcie_port(), imx6_add_pcie_port(), > and spear13xx_add_pcie_port(). Do they need similar changes? If not, > why not? > > I see your discussion about DRA7 hardware design, but my impression is > that this problem affects anybody who calls dw_handle_msi_irq() from a > handler registered with IRQF_SHARED. Issue fixed by this patch is not related to IRQF_SHARED. It will happen on -RT
Re: [PATCH 2/2] ARM: OMAP2+: Change core_initcall levels to postcore_initcall
On 12/03/2015 06:00 PM, Tony Lindgren wrote: > * Tony Lindgren[151130 08:29]: >> We want to be able to probe a few selected device drivers before hwmod >> code populates the clocks in omap_hwmod_setup_all(). This allows us to >> convert most of the clock drivers into regular device drivers. if understand things right, ti clks now will be populated and initialized from __omap_sync32k_timer_init - omap_clk_init() - .. - of_clk_init() - .. - omap_clk_soc_init() and __omap_sync32k_timer_init(), in turn, will be called from: arch/arm/kernel/time.c - time_init() machine_desc->init_time(); (without your patch 1). So, I don't see real dependency here between clk initialization and hwmods :( >> >> We only need a few minimal clock drivers early for the system timers to >> select between the 32KiHz clock and the high frequency oscillator. >> >> With these changes, initializing the clock drivers can be just done at >> core_initcall time with something like: >> >> np = of_find_node_by_name(NULL, "plls"); >> if (np) >> of_platform_populate(np, NULL, NULL, NULL); >> >> And then these clocks will be available for the interconnect code to use. >> >> Having most of the clock drivers being regular device drivers allows >> us to use the nice things like devm_* functions and dev_err and dev_dbg. >> As an extra bonus, this also allows us to develop the clock drivers for >> new SoCs as loadable modules initially for cases where we can boot up >> the system based on the bootloader configured clocks. >> >> To do this, let's change the core_initcalls to postcore_initcall under >> mach-omap2. > > FYI I'm applying this one into omap-for-v4.5/soc today, the first patch > in this series needs more work as discussed. > To be honest I don't see how this will help to convert ti clks in regular devices right now. Wouldn't it be better to move forward with this patch together with future patches? So it will be clear what benefits will this approach provide. In other words, I think it should be a part of some bigger series. -- regards, -grygorii -- 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.4-rc][PATCH v2] ARM: dts: am4372: fix clock source for arm twd and global timers
On 12/03/2015 06:35 PM, Tony Lindgren wrote: > * Grygorii Strashko <grygorii.stras...@ti.com> [151130 07:58]: >> ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2. >> But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result. >> Timekeeping core misbehaves. For example, execution of command >> "sleep 5" will take 10 sec instead of 5. >> >> Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use >> it for clocking ARM TWD and Global timer (same way as on OMAP4). >> >> Cc: Tony Lindgren <t...@atomide.com> >> Cc: Felipe Balbi <ba...@ti.com> >> Cc: Tero Kristo <t-kri...@ti.com> >> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and SCU >> nodes") >> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> > > Is this fix alone enough or do you still need to disable other the ARM > timers in the board specific dts file? > This one is independent and it's required for -rc, because ARM TWD timer will be selected now for am43xx even if http://www.spinics.net/lists/arm-kernel/msg459649.html is not merged yet ;) in case of omap2plus_defconfig build. -- regards, -grygorii -- 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 00/10] Patches to get dm814x-evm booting to NFSroot
On 12/02/2015 01:38 AM, Tony Lindgren wrote: > Hi all, > > Here are some fixes for v4.5 merge window to get dm814x-evm booting. > While hp t410 boots based on the bootloader clocks, dm814x-evm needs > more things configured. Especially the clock dts entries were all > wrong and just happened to be harmless on hp t410. > > To boot, you probably want to use v4.4-rc3 because of commit 29f5b34ca1a1 > ("arm: omap2+: add missing HWMOD_NO_IDLEST in 81xx hwmod data") and also > manually apply commit 0db19b850468 ("net: cpsw: Fix ethernet regression > for dm814x") from Linux next. > > I have more changes coming up after this series after I clean them > up a bit. Here's a brief status update for people: > > What's working after this series on dm814x-evm and hp t410: > > - Timers > > - Serial > > - Ethernet > > - DMA > > - I2C (only tested so far with i2cdetect -r 0) > > - GPIO (only tested with additional MMC patches for card detect) > > I have the following additional patches coming soonish: > > - Basic ADPLL clock driver > > - MMC support > > - USB support > > - Minimal j5eco-evm support > > Should work with just configuration: > > - [PATCH 0/3] pwm: omap: Add PWM support using dual-mode timers > > I'm not working on any of the accelerators or graphics FYI. If somebody > has patches coming for those please notify on the linux-omap and > linux-arm-kernel mailings lists so we can avoid duplicate work. > > Tony Lindgren (10): >ARM: OMAP2+: Fix timer entries for dm814x >clk: ti: Add few dm814x clock aliases >ARM: OMAP2+: Add DPPLS clock manager for dm814x >ARM: OMAP2+: Enable GPIO for dm814x >ARM: OMAP2+: Disable GPIO softreset for dm81xx >ARM: OMAP2+: Remove useless check for legacy booting for dm814x >ARM: dts: Fix dm814x entries for pllss and prcm >ARM: dts: Fix some mux and divider clocks to get dm814x-evm booting >ARM: dts: Fix dm8148 control modules ranges >ARM: dts: Fix dm814x pinctrl address and mask I'm worry a bit, if you will apply this series in its current order - it will break git bisect. Patch one "ARM: OMAP2+: Fix timer entries for dm814x" will use timerX_fck, but those clocks will be added by patches 2 "clk: ti: Add few dm814x clock aliases" and 8 "ARM: dts: Fix some mux and divider clocks to get dm814x-evm booting" -- regards, -grygorii -- 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.4-rc][PATCH] ARM: dts: am4372: disable arm twd and global timer's nodes
On 12/03/2015 06:37 PM, Tony Lindgren wrote: > * Tony Lindgren[151203 08:34]: >> >> It seems we should apply this as a fix unless somebody has better ideas. > > Actually I think the fix for now is "[4.4-rc][PATCH v2] ARM: dts: am4372: fix > clock source for arm twd and global timers" until PM starts working? > Yes. if no PM it should work, but PM patches are on the fly for Am43. But, again, main reason I've posted this patch is that I do not see the way how to enable/disable or limit functionality of these timer by using Kconfig options in case of Muliplatform builds :( (or by boot parameters). Example 1 (this is how 4.4 kernel is working): 1) OMAP4 has ARM TWD timer and properly supports it, so in Kconfig we have "select HAVE_ARM_TWD" and corresponding "local-timer" is defined in DT. AM437x has ARM TWD timer, but does not support it yet, so corresponding "local-timer" defined in DT and in Kconfig HAVE_ARM_TWD is not selected. (patch http://www.spinics.net/lists/arm-kernel/msg459649.html is not merged yet) Result: ARM TWD timer will be enabled on AM437x !? :( Example 2: ARM Global timer can't be used as clocksource device if CPUFreq is enabled (this is not supported even by Timekeeping core now). So, there are two options: 1) update selection for config SOC_AM43XX + select ARM_GLOBAL_TIMER if !CPU_FREQ 2) updated ARM GT driver and don't register clocksource device if CPUFreq is enabled Both options will not work: 1) ARM_GLOBAL_TIMER will be selected if any other SoC in multiplatform build will select ARM_GLOBAL_TIMER 2) clocksource device will never be registered if at least one SoC will select CPU_FREQ. And this might break SoC which expect it to be registered, because CPU_FREQ is not enabled on these SoCs (no CPUFreq drivers registered). So, as alternative to this patch, ARM timers nodes can be disabled in platform DTS files, but then all platform DTS files will need to be updated. -- regards, -grygorii -- 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: single: remove misuse of IRQF_NO_SUSPEND flag
On 12/03/2015 08:13 PM, Tony Lindgren wrote: > * Linus Walleij[151201 06:07]: >> On Fri, Nov 27, 2015 at 6:21 PM, Sudeep Holla wrote: >> >>> From: Sudeep Holla >>> >>> The IRQF_NO_SUSPEND flag is used to identify the interrupts that should >>> be left enabled so as to allow them to work as expected during the >>> suspend-resume cycle, but doesn't guarantee that it will wake the system >>> from a suspended state, enable_irq_wake is recommended to be used for >>> the wakeup. >>> >>> This patch removes the use of IRQF_NO_SUSPEND flags replacing it with >>> irq_set_irq_wake instead. >>> >>> Cc: Linus Walleij >>> Cc: linux-g...@vger.kernel.org >>> Signed-off-by: Sudeep Holla >> >> I need Tony's ACK on this as well. > > At least on omaps, this controller is always powered and we never want to > suspend it as it handles wake-up events for all the IO pins. And that > usecase sounds exactly like what you're describing above. > > I don't quite follow what your suggested alternative for an interrupt > controller is? > > At least we need to have the alternative patched in with this chage before > just removing IRQF_NO_SUSPEND. > > The enable_irq_wake is naturally used for the consumer drivers of this > interrupt controller and actually mostly done automatically now with the > dev_pm_set_dedicated_wake_irq. > I think, this patch should not break our wake-up functionality. It will just change the moment when pcs_irq_handler() will be called: before this change: - suspend_enter() - arch_suspend_enable_irqs(); - ^ right here after this change: - suspend_enter() dpm_resume_noirq() - resume_device_irqs() ^ here Correct? And as for me this is more safe. -- regards, -grygorii -- 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] ARM: OMAP2+: Change core_initcall levels to postcore_initcall
On 12/03/2015 06:41 PM, Tony Lindgren wrote: > * Grygorii Strashko <grygorii.stras...@ti.com> [151203 08:35]: >> On 12/03/2015 06:00 PM, Tony Lindgren wrote: >>> * Tony Lindgren <t...@atomide.com> [151130 08:29]: >>>> We want to be able to probe a few selected device drivers before hwmod >>>> code populates the clocks in omap_hwmod_setup_all(). This allows us to >>>> convert most of the clock drivers into regular device drivers. >> >> if understand things right, ti clks now will be populated and initialized >> from >> __omap_sync32k_timer_init >> - omap_clk_init() >> - .. >> - of_clk_init() >> - .. >> - omap_clk_soc_init() >> >> and __omap_sync32k_timer_init(), in turn, will be called from: >> arch/arm/kernel/time.c >> - time_init() >> machine_desc->init_time(); >> (without your patch 1). > > Yes that's the current approach, but we can do better. We only need the > following > clocks for system timers at that point: > > - mux clocks to select between the 32k and hf oscillator source > > - clkctrl driver to gate the ocp clock > > All the other clocks can be initialized at core_initcall time with this > change. > >> So, I don't see real dependency here between clk initialization and hwmods :( > > You don't because it's only implemented so far for the dm814x ADPLL clock :) > > That I posted as "[PATCH 0/2] Clock driver for dm814x ADPLL". Note how it's > just a regular device driver that also works as loadable module on boards that > have all the necessary clocks enabled already by the bootloader. > Ah. Thanks. I see it now. -- regards, -grygorii -- 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] clocksource: ti-32k: convert to platform device
On 12/01/2015 06:07 PM, Tony Lindgren wrote: * Grygorii Strashko <grygorii.stras...@ti.com> [151201 07:09]: On 11/30/2015 06:28 PM, Tony Lindgren wrote: We should be able to make this into an early_platform_device and just have it depend on the source clock muxes. See the omap initcall changes patches I just posted. Sry, may be I've missed smth, but how early_platform_device will help us to get rid of platform code - We'd still need to power on manually early_platform_device's from platform code :( through hwmod. Having minimal platform code early is not a problem. The problem is that our early code is not minimal. For the system timers, we should only initialize the mux clocks needed early to select between 32k and hf oscillator source. This needs to be done using the clock framework, but we don't need the other clocks initialized early. The system timers we're using should be in the alwon power domain, if they are not, then we should change the timers around so we're using only timers in the alwon domain for system timers. Typically at least gpt1 and 12 are always powered. That allows us to leave out the hwmod dependency for system timers. Or am I forgetting some other dependency with our system timers? both counter32 and GP timer have to be enabled through sysc registers. They are in "Force idle" state after reset. The main reason why I've tried this is because clocksource will be really selected only at fs_initcall time - and at that time we have no restriction for using platform devices, Pm runtime APIs, etc. (exception/blocker is sched_clock :(). Right. But it seems we can leave out quite a bit of the dependencies for system timers. We already have gptimer probe not touching the system timers later on and can use shared gptimer functions after the clock muxing is done. -- regards, -grygorii -- 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] clocksource: ti-32k: convert to platform device
Hi Tony, On 11/30/2015 06:28 PM, Tony Lindgren wrote: > * Grygorii Strashko <grygorii.stras...@ti.com> [151127 12:11]: >> On 11/20/2015 08:21 PM, Felipe Balbi wrote: >>> Grygorii Strashko <grygorii.stras...@ti.com> writes: >>>> Since system clocksource is finally selected by Clocksource core at >>>> fs_initcall stage during boot there are no reasons to initialize >>>> ti_32k_timer at early boot stages. Hence, ti_32k_timer can be >>>> converted to use platform device/driver model and its PM can be >>>> implemented using PM runtime which is common for OMAP devices. >>>> >>>> Platform specific initialization code has to be disabled once as >>>> ti_32k_timer is converted to platform device - otherwise OMAP platform >>>> code will generate boot warnings. >>>> >>>> After this change, all counter_32k's platform code can be removed >>>> once all OMAP boards will be converted to DT. >>>> >>>> Cc: Tony Lindgren <t...@atomide.com> >>>> Cc: Felipe Balbi <ba...@ti.com> >>>> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> >>>> --- >> >> [...] >> >>>> + >>>> +static struct platform_driver ti_32k_driver __initdata = { >>>> + .probe = ti_32k_probe, >>>> + .driver = { >>>> + .name = "ti_32k_timer", >>>> + .of_match_table = of_match_ptr(ti_32k_of_table), >>>> + } >>>> +}; >>>> + >>>> +static int __init ti_32k_init(void) >>>> +{ >>>> + return platform_driver_register(_32k_driver); >>>>} >>>> -CLOCKSOURCE_OF_DECLARE(ti_32k_timer, "ti,omap-counter32k", >>>> - ti_32k_timer_init); >>>> + >>>> +subsys_initcall(ti_32k_init); >>>> + >>>> +MODULE_AUTHOR("Paul Mundt"); >>>> +MODULE_AUTHOR("Juha Yrjölä"); >>>> +MODULE_DESCRIPTION("OMAP2 32k Timer"); >>>> +MODULE_ALIAS("platform:ti_32k_timer"); >>>> +MODULE_LICENSE("GPL v2"); >>> >>> this will break clksource_of_init(), right ? Eventually, we want that to >>> be the only thing called by our .init_time method. I'll leave it to Tony >>> to decide, but IMO this is not a good path forward for timers. >>> >> >> Yeh :(. I did additional tests, and, unfortunately, this can't be used as >> is. >> But not because of clocksource_of_init() which will just produce boot >> warning. >> It can't be done because of sched_clock_register() which is expected to be >> called during early boot time only and with disabled IRQs. >> >> It was so tempting to try :) > > We should be able to make this into an early_platform_device and just > have it depend on the source clock muxes. See the omap initcall changes > patches I just posted. > Sry, may be I've missed smth, but how early_platform_device will help us to get rid of platform code - We'd still need to power on manually early_platform_device's from platform code :( through hwmod. The main reason why I've tried this is because clocksource will be really selected only at fs_initcall time - and at that time we have no restriction for using platform devices, Pm runtime APIs, etc. (exception/blocker is sched_clock :(). -- regards, -grygorii -- 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] ARM: OMAP2+: Initialize timers later with late_time_init
On 11/30/2015 06:26 PM, Tony Lindgren wrote: > We don't need timers right away and initializing them later gives us few > nice things like interrupts and kmalloc. As the timers have a dependency > to the clock framework, we're better off initializing things later rather > than early if things go wrong. And this allows us to make the mux clock > driver needed for system timers into early_platform drivers. > > Note that smp_prepare_cpus() will get called later on during the init so > we just need to local_irq_enable/disable for clocksource_probe(). > > Cc: Felipe Balbi <ba...@ti.com> > Cc: Grygorii Strashko <grygorii.stras...@ti.com> > Cc: Paul Walmsley <p...@pwsan.com> > Cc: Tero Kristo <t-kri...@ti.com> > Signed-off-by: Tony Lindgren <t...@atomide.com> > --- > arch/arm/mach-omap2/timer.c | 46 > +++-- > 1 file changed, 40 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c > index b18ebbe..68bf482 100644 > --- a/arch/arm/mach-omap2/timer.c > +++ b/arch/arm/mach-omap2/timer.c > @@ -478,36 +478,56 @@ static void __init __omap_sync32k_timer_init(int > clkev_nr, const char *clkev_src > omap2_gp_clockevent_init(clkev_nr, clkev_src, clkev_prop); > > /* Enable the use of clocksource="gp_timer" kernel parameter */ > + local_irq_disable(); > if (use_gptimer_clksrc || gptimer) > omap2_gptimer_clocksource_init(clksrc_nr, clksrc_src, > clksrc_prop); > else > omap2_sync32k_clocksource_init(); > + local_irq_enable(); So, this will be called now after sched_clock_postinit() and to W/A warnings you've added local_irq_disable()/local_irq_enable(). Am I right? Are you sure this is safe? -- regards, -grygorii -- 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
[4.4-rc][PATCH v2] ARM: dts: am4372: fix clock source for arm twd and global timers
ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2. But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result. Timekeeping core misbehaves. For example, execution of command "sleep 5" will take 10 sec instead of 5. Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use it for clocking ARM TWD and Global timer (same way as on OMAP4). Cc: Tony Lindgren <t...@atomide.com> Cc: Felipe Balbi <ba...@ti.com> Cc: Tero Kristo <t-kri...@ti.com> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and SCU nodes") Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- Changes in v2: - fix: mpu_periphclk should be clocked from dpll_mpu_m2_ck instead of dpll_mpu_ck. link on v1: http://www.spinics.net/lists/arm-kernel/msg463898.html arch/arm/boot/dts/am4372.dtsi| 4 ++-- arch/arm/boot/dts/am43xx-clocks.dtsi | 8 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi index d83ff9c..de8791a 100644 --- a/arch/arm/boot/dts/am4372.dtsi +++ b/arch/arm/boot/dts/am4372.dtsi @@ -74,7 +74,7 @@ reg = <0x48240200 0x100>; interrupts = ; interrupt-parent = <>; - clocks = <_mpu_m2_ck>; + clocks = <_periphclk>; }; local_timer: timer@48240600 { @@ -82,7 +82,7 @@ reg = <0x48240600 0x100>; interrupts = ; interrupt-parent = <>; - clocks = <_mpu_m2_ck>; + clocks = <_periphclk>; }; l2-cache-controller@48242000 { diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi b/arch/arm/boot/dts/am43xx-clocks.dtsi index cc88728..a38af2b 100644 --- a/arch/arm/boot/dts/am43xx-clocks.dtsi +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi @@ -259,6 +259,14 @@ ti,invert-autoidle-bit; }; + mpu_periphclk: mpu_periphclk { + #clock-cells = <0>; + compatible = "fixed-factor-clock"; + clocks = <_mpu_m2_ck>; + clock-mult = <1>; + clock-div = <2>; + }; + dpll_ddr_ck: dpll_ddr_ck { #clock-cells = <0>; compatible = "ti,am3-dpll-clock"; -- 2.6.3 -- 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: OMAP4: execute initcall to reserve SRAM for I688 only on OMAP4
On 11/30/2015 07:27 PM, Lucas Stach wrote: > Am Montag, den 16.11.2015, 14:24 +0200 schrieb Grygorii Strashko: >> On 11/16/2015 01:25 PM, Lucas Stach wrote: >>> omap_interconnect_sync() is the only user of the SRAM scratch area >>> allocated in the omap4_sram_init initcall. The interconnect sync is >>> used exclusively in the OMAP4 specific WFI implementation, so there >>> is no point in allocating the SRAM scratch on other SoC types. >>> >>> Bail out of the initcall if the kernel is not running on OMAP4 to >>> avoid a confusing warning about being unable to allocate the SRAM >>> needed for I688 handling. >>> >>> Signed-off-by: Lucas Stach <l.st...@pengutronix.de> >>> Tested-by: Bastian Stender <b...@pengutronix.de> >>> --- >>>arch/arm/mach-omap2/omap4-common.c | 3 +++ >>>1 file changed, 3 insertions(+) >>> >>> diff --git a/arch/arm/mach-omap2/omap4-common.c >>> b/arch/arm/mach-omap2/omap4-common.c >>> index 949696b6f17b..6db393a30a28 100644 >>> --- a/arch/arm/mach-omap2/omap4-common.c >>> +++ b/arch/arm/mach-omap2/omap4-common.c >>> @@ -131,6 +131,9 @@ static int __init omap4_sram_init(void) >>> struct device_node *np; >>> struct gen_pool *sram_pool; >>> >>> + if (!cpu_is_omap44xx()) >>> + return 0; >> >> This one affects on am43xx also >> > So you are saying this erratum is also present on AM43xx? I wasn't able > to deduce this from the information provided by Richard Woodruff. > "..SOCs using similar chassis components of OMAP4430 time are impacted..." "..But AM335x should be immune from this particular issue..." Advisory 11 Asynchronous Bridge Corruption http://www.ti.com/lit/er/sprz408b/sprz408b.pdf >> >>> + >>> np = of_find_compatible_node(NULL, NULL, "ti,omap4-mpu"); >>> if (!np) >>> pr_warn("%s:Unable to allocate sram needed to handle errata >>> I688\n", >> >> Since all OMAP4+ platforms are now DT based why can't we just return from >> here silently? >> > If we are unable to allocate the SRAM needed to work around I688 this is > a real error on platforms that expose this erratum, so silently bailing > out at this point may obscure a real issue. > SRAM is not allocated here - It's just check to understand do we need it or not in case of multiplatform build where CONFIG_OMAP_INTERCONNECT_BARRIER will be selected most probably. And if "ti,omap4-mpu" was not found - it just means that this, particular, platform is not affected by i688 errata. If someone misses corresponding node in DT - we can't do nothing :) -- regards, -grygorii -- 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 v4] clocksource: arm_global_timer: fix suspend resume
Now the System stall is observed on TI AM437x based board (am437x-gp-evm) during resuming from System suspend when ARM Global timer is selected as clocksource device (CPUIdle not enabled) - SysRq are working, but nothing else. The reason of stall is that ARM Global timer loses its contexts during System suspend: GT_CONTROL.TIMER_ENABLE = 0 (unbanked) GT_COUNTERx = 0 Hence, update ARM Global timer driver to reflect above behaviour - re-enable ARM Global timer on resume (GT_CONTROL.TIMER_ENABLE = 1) if not enabled. CC: Arnd Bergmann <a...@arndb.de> Cc: John Stultz <john.stu...@linaro.org> Cc: Felipe Balbi <ba...@ti.com> Cc: Tony Lindgren <t...@atomide.com> Cc: Marc Zyngier <marc.zyng...@arm.com> Reviewed-by: Santosh Shilimkar <ssant...@kernel.org> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- Changes in v4: - check current timer's state before enabling. Changes in v3: - dropped all DT specific code Changes in v2: - suspend/resume simplified: nothing is stored any more and ARM GT just re-enabled Links on prev versions: v3: https://lkml.org/lkml/2015/11/27/402 v2: https://lkml.org/lkml/2015/11/20/355 v1: https://lkml.org/lkml/2015/11/13/456 drivers/clocksource/arm_global_timer.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c index a2cb6fa..817a787 100644 --- a/drivers/clocksource/arm_global_timer.c +++ b/drivers/clocksource/arm_global_timer.c @@ -195,12 +195,23 @@ static cycle_t gt_clocksource_read(struct clocksource *cs) return gt_counter_read(); } +static void gt_resume(struct clocksource *cs) +{ + unsigned long ctrl; + + ctrl = readl(gt_base + GT_CONTROL); + if (!(ctrl & GT_CONTROL_TIMER_ENABLE)) + /* re-enable timer on resume */ + writel(GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL); +} + static struct clocksource gt_clocksource = { .name = "arm_global_timer", .rating = 300, .read = gt_clocksource_read, .mask = CLOCKSOURCE_MASK(64), .flags = CLOCK_SOURCE_IS_CONTINUOUS, + .resume = gt_resume, }; #ifdef CONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK -- 2.6.3 -- 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] clocksource: arm_global_timer: fix suspend resume
On 11/30/2015 02:29 AM, santosh.shilim...@oracle.com wrote: On 11/27/15 11:47 AM, Grygorii Strashko wrote: Now the System stall is observed on TI AM437x based board (am437x-gp-evm) during resuming from System suspend when ARM Global timer is selected as clocksource device (CPUIdle not enabled) - SysRq are working, but nothing else. The reason of stall is that ARM Global timer loses its contexts during System suspend: GT_CONTROL.TIMER_ENABLE = 0 (unbanked) GT_COUNTERx = 0 Hence, update ARM Global timer driver to reflect above behaviour - re-enable ARM Global timer on resume GT_CONTROL.TIMER_ENABLE = 1. CC: Arnd Bergmann <a...@arndb.de> Cc: John Stultz <john.stu...@linaro.org> Cc: Felipe Balbi <ba...@ti.com> Cc: Tony Lindgren <t...@atomide.com> Cc: Santosh Shilimkar <ssant...@kernel.org> Cc: Marc Zyngier <marc.zyng...@arm.com> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- Changes in v3: - dropped all DT specific code Changes in v2: - suspend/resume simplified: nothing is stored any more and ARM GT just re-enabled Link on v2: https://lkml.org/lkml/2015/11/20/355 Link on v1: https://lkml.org/lkml/2015/11/13/456 Looks reasonable to me. drivers/clocksource/arm_global_timer.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c index a2cb6fa..10d1417 100644 --- a/drivers/clocksource/arm_global_timer.c +++ b/drivers/clocksource/arm_global_timer.c @@ -195,12 +195,19 @@ static cycle_t gt_clocksource_read(struct clocksource *cs) return gt_counter_read(); } +static void gt_resume(struct clocksource *cs) +{ +/* re-enable timer on resume */ +writel(GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL); Check if its disabled before enabling it. Other than that, Reviewed-by: Santosh Shilimkar <ssant...@kernel.org> Sure, I'll update. Thanks. -- regards, -grygorii -- 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.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
On 11/30/2015 10:25 AM, Tero Kristo wrote: On 11/27/2015 09:44 PM, Grygorii Strashko wrote: ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2. But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result. Timekeeping core misbehaves. For example, execution of command "sleep 5" will take 10 sec instead of 5. Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use it for clocking ARM TWD and Global timer (same way as on OMAP4). Cc: Tony Lindgren <t...@atomide.com> Cc: Felipe Balbi <ba...@ti.com> Cc: Tero Kristo <t-kri...@ti.com> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and SCU nodes") Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- arch/arm/boot/dts/am4372.dtsi| 4 ++-- arch/arm/boot/dts/am43xx-clocks.dtsi | 8 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi index d83ff9c..de8791a 100644 --- a/arch/arm/boot/dts/am4372.dtsi +++ b/arch/arm/boot/dts/am4372.dtsi @@ -74,7 +74,7 @@ reg = <0x48240200 0x100>; interrupts = ; interrupt-parent = <>; -clocks = <_mpu_m2_ck>; +clocks = <_periphclk>; }; local_timer: timer@48240600 { @@ -82,7 +82,7 @@ reg = <0x48240600 0x100>; interrupts = ; interrupt-parent = <>; -clocks = <_mpu_m2_ck>; +clocks = <_periphclk>; }; l2-cache-controller@48242000 { diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi b/arch/arm/boot/dts/am43xx-clocks.dtsi index cc88728..2ff58b1 100644 --- a/arch/arm/boot/dts/am43xx-clocks.dtsi +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi @@ -259,6 +259,14 @@ ti,invert-autoidle-bit; }; +mpu_periphclk: mpu_periphclk { +#clock-cells = <0>; +compatible = "fixed-factor-clock"; +clocks = <_mpu_ck>; I don't think this is correct, ARM core is fed dpll_mpu_m2_ck, where the divisor value can potentially differ from 1. If you feed this clock directly from dpll_mpu_ck, you bypass this divisor. Sry. My mistake. I'll update it to use dpll_mpu_m2_ck. Did you check what is the impact of cpufreq on the ARM TWD/timers? TWD is cpufreq friendly, ARM GT is not. +clock-mult = <1>; +clock-div = <2>; +}; + dpll_ddr_ck: dpll_ddr_ck { #clock-cells = <0>; compatible = "ti,am3-dpll-clock"; -- regards, -grygorii -- 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.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
On 11/30/2015 03:32 PM, Tero Kristo wrote: > On 11/30/2015 01:53 PM, Grygorii Strashko wrote: >> On 11/30/2015 10:25 AM, Tero Kristo wrote: >>> On 11/27/2015 09:44 PM, Grygorii Strashko wrote: >>>> ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2. >>>> But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result. >>>> Timekeeping core misbehaves. For example, execution of command >>>> "sleep 5" will take 10 sec instead of 5. >>>> >>>> Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use >>>> it for clocking ARM TWD and Global timer (same way as on OMAP4). >>>> >>>> Cc: Tony Lindgren <t...@atomide.com> >>>> Cc: Felipe Balbi <ba...@ti.com> >>>> Cc: Tero Kristo <t-kri...@ti.com> >>>> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and >>>> SCU nodes") >>>> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> >>>> --- >>>> arch/arm/boot/dts/am4372.dtsi| 4 ++-- >>>> arch/arm/boot/dts/am43xx-clocks.dtsi | 8 >>>> 2 files changed, 10 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/arch/arm/boot/dts/am4372.dtsi >>>> b/arch/arm/boot/dts/am4372.dtsi >>>> index d83ff9c..de8791a 100644 >>>> --- a/arch/arm/boot/dts/am4372.dtsi >>>> +++ b/arch/arm/boot/dts/am4372.dtsi >>>> @@ -74,7 +74,7 @@ >>>> reg = <0x48240200 0x100>; >>>> interrupts = ; >>>> interrupt-parent = <>; >>>> -clocks = <_mpu_m2_ck>; >>>> +clocks = <_periphclk>; >>>> }; >>>> >>>> local_timer: timer@48240600 { >>>> @@ -82,7 +82,7 @@ >>>> reg = <0x48240600 0x100>; >>>> interrupts = ; >>>> interrupt-parent = <>; >>>> -clocks = <_mpu_m2_ck>; >>>> +clocks = <_periphclk>; >>>> }; >>>> >>>> l2-cache-controller@48242000 { >>>> diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi >>>> b/arch/arm/boot/dts/am43xx-clocks.dtsi >>>> index cc88728..2ff58b1 100644 >>>> --- a/arch/arm/boot/dts/am43xx-clocks.dtsi >>>> +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi >>>> @@ -259,6 +259,14 @@ >>>> ti,invert-autoidle-bit; >>>> }; >>>> >>>> +mpu_periphclk: mpu_periphclk { >>>> +#clock-cells = <0>; >>>> +compatible = "fixed-factor-clock"; >>>> +clocks = <_mpu_ck>; >>> >>> I don't think this is correct, ARM core is fed dpll_mpu_m2_ck, where the >>> divisor value can potentially differ from 1. If you feed this clock >>> directly from dpll_mpu_ck, you bypass this divisor. >> >> Sry. My mistake. I'll update it to use dpll_mpu_m2_ck. >> >>> >>> Did you check what is the impact of cpufreq on the ARM TWD/timers? >> >> TWD is cpufreq friendly, ARM GT is not. > > I think the TWD kick period changes with cpufreq also right? linux/arch/arm/kernel/smp_twd.c has code to handle cpufreq. > > How are the clocks handled with cpufreq? The user just needs to > understand that the timers will be screwed if he uses ARM GT? Should we > add some sort of dependency to disable the ARM GT if cpufreq is enabled? Yep. May be, but very good question is how to do that in case of OMAP multiplatform build which enables most of all config options at once. There two threads related to this: [1] http://www.spinics.net/lists/arm-kernel/msg459649.html [2] http://www.spinics.net/lists/arm-kernel/msg461141.html Personally, I've do not see better way than [2] right now. >>>> +clock-mult = <1>; >>>> +clock-div = <2>; >>>> +}; >>>> + >>>> dpll_ddr_ck: dpll_ddr_ck { >>>> #clock-cells = <0>; >>>> compatible = "ti,am3-dpll-clock"; >>>> By the way, does this patch is still correct taking into account dependency from cpufreq? Does it make sense update it to use dpll_mpu_ck and resend? -- regards, -grygorii -- 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 v3] clocksource: arm_global_timer: fix suspend resume
Now the System stall is observed on TI AM437x based board (am437x-gp-evm) during resuming from System suspend when ARM Global timer is selected as clocksource device (CPUIdle not enabled) - SysRq are working, but nothing else. The reason of stall is that ARM Global timer loses its contexts during System suspend: GT_CONTROL.TIMER_ENABLE = 0 (unbanked) GT_COUNTERx = 0 Hence, update ARM Global timer driver to reflect above behaviour - re-enable ARM Global timer on resume GT_CONTROL.TIMER_ENABLE = 1. CC: Arnd Bergmann <a...@arndb.de> Cc: John Stultz <john.stu...@linaro.org> Cc: Felipe Balbi <ba...@ti.com> Cc: Tony Lindgren <t...@atomide.com> Cc: Santosh Shilimkar <ssant...@kernel.org> Cc: Marc Zyngier <marc.zyng...@arm.com> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- Changes in v3: - dropped all DT specific code Changes in v2: - suspend/resume simplified: nothing is stored any more and ARM GT just re-enabled Link on v2: https://lkml.org/lkml/2015/11/20/355 Link on v1: https://lkml.org/lkml/2015/11/13/456 drivers/clocksource/arm_global_timer.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c index a2cb6fa..10d1417 100644 --- a/drivers/clocksource/arm_global_timer.c +++ b/drivers/clocksource/arm_global_timer.c @@ -195,12 +195,19 @@ static cycle_t gt_clocksource_read(struct clocksource *cs) return gt_counter_read(); } +static void gt_resume(struct clocksource *cs) +{ + /* re-enable timer on resume */ + writel(GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL); +} + static struct clocksource gt_clocksource = { .name = "arm_global_timer", .rating = 300, .read = gt_clocksource_read, .mask = CLOCKSOURCE_MASK(64), .flags = CLOCK_SOURCE_IS_CONTINUOUS, + .resume = gt_resume, }; #ifdef CONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK -- 2.6.3 -- 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
[4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2. But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result. Timekeeping core misbehaves. For example, execution of command "sleep 5" will take 10 sec instead of 5. Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use it for clocking ARM TWD and Global timer (same way as on OMAP4). Cc: Tony Lindgren <t...@atomide.com> Cc: Felipe Balbi <ba...@ti.com> Cc: Tero Kristo <t-kri...@ti.com> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and SCU nodes") Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- arch/arm/boot/dts/am4372.dtsi| 4 ++-- arch/arm/boot/dts/am43xx-clocks.dtsi | 8 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi index d83ff9c..de8791a 100644 --- a/arch/arm/boot/dts/am4372.dtsi +++ b/arch/arm/boot/dts/am4372.dtsi @@ -74,7 +74,7 @@ reg = <0x48240200 0x100>; interrupts = ; interrupt-parent = <>; - clocks = <_mpu_m2_ck>; + clocks = <_periphclk>; }; local_timer: timer@48240600 { @@ -82,7 +82,7 @@ reg = <0x48240600 0x100>; interrupts = ; interrupt-parent = <>; - clocks = <_mpu_m2_ck>; + clocks = <_periphclk>; }; l2-cache-controller@48242000 { diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi b/arch/arm/boot/dts/am43xx-clocks.dtsi index cc88728..2ff58b1 100644 --- a/arch/arm/boot/dts/am43xx-clocks.dtsi +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi @@ -259,6 +259,14 @@ ti,invert-autoidle-bit; }; + mpu_periphclk: mpu_periphclk { + #clock-cells = <0>; + compatible = "fixed-factor-clock"; + clocks = <_mpu_ck>; + clock-mult = <1>; + clock-div = <2>; + }; + dpll_ddr_ck: dpll_ddr_ck { #clock-cells = <0>; compatible = "ti,am3-dpll-clock"; -- 2.6.3 -- 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] clocksource: ti-32k: convert to platform device
Hi Felipe, On 11/20/2015 08:21 PM, Felipe Balbi wrote: > Grygorii Strashko <grygorii.stras...@ti.com> writes: >> Since system clocksource is finally selected by Clocksource core at >> fs_initcall stage during boot there are no reasons to initialize >> ti_32k_timer at early boot stages. Hence, ti_32k_timer can be >> converted to use platform device/driver model and its PM can be >> implemented using PM runtime which is common for OMAP devices. >> >> Platform specific initialization code has to be disabled once as >> ti_32k_timer is converted to platform device - otherwise OMAP platform >> code will generate boot warnings. >> >> After this change, all counter_32k's platform code can be removed >> once all OMAP boards will be converted to DT. >> >> Cc: Tony Lindgren <t...@atomide.com> >> Cc: Felipe Balbi <ba...@ti.com> >> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> >> --- [...] >> + >> +static struct platform_driver ti_32k_driver __initdata = { >> +.probe = ti_32k_probe, >> +.driver = { >> +.name = "ti_32k_timer", >> +.of_match_table = of_match_ptr(ti_32k_of_table), >> +} >> +}; >> + >> +static int __init ti_32k_init(void) >> +{ >> +return platform_driver_register(_32k_driver); >> } >> -CLOCKSOURCE_OF_DECLARE(ti_32k_timer, "ti,omap-counter32k", >> -ti_32k_timer_init); >> + >> +subsys_initcall(ti_32k_init); >> + >> +MODULE_AUTHOR("Paul Mundt"); >> +MODULE_AUTHOR("Juha Yrjölä"); >> +MODULE_DESCRIPTION("OMAP2 32k Timer"); >> +MODULE_ALIAS("platform:ti_32k_timer"); >> +MODULE_LICENSE("GPL v2"); > > this will break clksource_of_init(), right ? Eventually, we want that to > be the only thing called by our .init_time method. I'll leave it to Tony > to decide, but IMO this is not a good path forward for timers. > Yeh :(. I did additional tests, and, unfortunately, this can't be used as is. But not because of clocksource_of_init() which will just produce boot warning. It can't be done because of sched_clock_register() which is expected to be called during early boot time only and with disabled IRQs. It was so tempting to try :) Thanks for your comment. -- regards, -grygorii -- 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 v2] ARM: omap2+: enable REGULATOR_FIXED_VOLTAGE
Enable REGULATOR_FIXED_VOLTAGE for all OMAP2+ platforms otherwise system can't boot from SD-card when kernel is built for single SoC (for example, with CONFIG_SOC_DRA7XX=y only). It's also required for almost all TI SoC's platforms. Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- Link on v1: http://www.spinics.net/lists/linux-omap/msg12.html arch/arm/mach-omap2/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index d07887f..cda35a1 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -127,6 +127,7 @@ config ARCH_OMAP2PLUS_TYPICAL select NEON if CPU_V7 select PM select REGULATOR + select REGULATOR_FIXED_VOLTAGE select TWL4030_CORE if ARCH_OMAP3 || ARCH_OMAP4 select TWL4030_POWER if ARCH_OMAP3 || ARCH_OMAP4 select VFP @@ -207,7 +208,6 @@ config MACH_OMAP3_PANDORA depends on ARCH_OMAP3 default y select OMAP_PACKAGE_CBB - select REGULATOR_FIXED_VOLTAGE if REGULATOR config MACH_NOKIA_N810 bool -- 2.6.3 -- 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: omap2plus_defconfig: enable REGULATOR_FIXED_VOLTAGE
On 11/25/2015 10:57 PM, Tony Lindgren wrote: * Grygorii Strashko <grygorii.stras...@ti.com> [151125 12:26]: On 11/25/2015 08:25 PM, Tony Lindgren wrote: * Grygorii Strashko <grygorii.stras...@ti.com> [151105 08:45]: Enable REGULATOR_FIXED_VOLTAGE otherwise system can't boot from SD-card when kernel is buit with CONFIG_SOC_DRA7XX=y only. It's also required for almost all other TI SoC's platforms. This is already enabled by default when you do make omap2plus_defconfig? Yes. But only because it's hard-coded for MACH_OMAP3_PANDORA :( config MACH_OMAP3_PANDORA bool "OMAP3 Pandora" depends on ARCH_OMAP3 default y select OMAP_PACKAGE_CBB select REGULATOR_FIXED_VOLTAGE if REGULATOR But I can't boot from SD-card if I'm trying SoC specific build, like only SOC_DRA7XX=y above. Well let's move to under config ARCH_OMAP2PLUS in the Kconfig? Otherwise if somebody does make savedefconfig it will disappear again from omap2plus_defconfig. Agree. Done. -- regards, -grygorii -- 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: omap2plus_defconfig: enable REGULATOR_FIXED_VOLTAGE
On 11/25/2015 08:25 PM, Tony Lindgren wrote: > * Grygorii Strashko <grygorii.stras...@ti.com> [151105 08:45]: >> Enable REGULATOR_FIXED_VOLTAGE otherwise system can't boot from >> SD-card when kernel is buit with CONFIG_SOC_DRA7XX=y only. >> >> It's also required for almost all other TI SoC's platforms. > > This is already enabled by default when you do make omap2plus_defconfig? Yes. But only because it's hard-coded for MACH_OMAP3_PANDORA :( config MACH_OMAP3_PANDORA bool "OMAP3 Pandora" depends on ARCH_OMAP3 default y select OMAP_PACKAGE_CBB select REGULATOR_FIXED_VOLTAGE if REGULATOR But I can't boot from SD-card if I'm trying SoC specific build, like only SOC_DRA7XX=y above. -- regards, -grygorii -- 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 v2] PCI: dra7xx: mark dra7xx_pcie_msi irq as IRQF_NO_THREAD
On -RT, TI DRA7 PCIe driver always produces below backtrace when the first PCI interrupt is triggered: [ cut here ] WARNING: CPU: 1 PID: 82 at kernel/irq/handle.c:150 handle_irq_event_percpu+0x14c/0x174() irq 460 handler irq_default_primary_handler+0x0/0x14 enabled interrupts Modules linked in: ahci_platform(+) ti_vip(+) libahci_platform xhci_pci(+) c_can_platform(+) libahci xhci_hcd ti_vpe c_can libata v4l2_mem2mem can_dev gpio_keys leds_gpio snd_soc_simple_card usbcore spi_ti_qspi videobuf2_core extcon_usb_gpio omap_wdt ti_vpdma videobuf2_dma_contig ti_soc_thermal videobuf2_memops dwc3_omap extcon rtc_omap ov1063x v4l2_common videodev media snd_soc_tlv320aic3x omap_rng rng_core omap_remoteproc CPU: 1 PID: 82 Comm: irq/26-dra7-pci Not tainted 4.1.10-rt10-02638-g6486d7e-dirty #1 Hardware name: Generic DRA74X (Flattened Device Tree) Backtrace: [] (dump_backtrace) from [] (show_stack+0x18/0x1c) r7:c07acca0 r6: r5:c09034e4 r4: [] (show_stack) from [] (dump_stack+0x90/0xac) [] (dump_stack) from [] (warn_slowpath_common+0x7c/0xb8) r7:c07acca0 r6:0096 r5:0009 r4:ee4d3e38 [] (warn_slowpath_common) from [] (warn_slowpath_fmt+0x38/0x40) r8:ee3fcc00 r7:01cc r6: r5:0002 r4:c07acc78 [] (warn_slowpath_fmt) from [] (handle_irq_event_percpu+0x14c/0x174) r3:01cc r2:c07acc78 r4:ecfcdd80 [] (handle_irq_event_percpu) from [] (handle_irq_event+0x84/0xb8) r10:0001 r9:ee4b59c0 r8:ee1d0900 r7:0001 r6: r5:ecfcdd80 r4:ee3fcc00 [] (handle_irq_event) from [] (handle_simple_irq+0x90/0x118) r5:ee4a9020 r4:ee3fcc00 [] (handle_simple_irq) from [] (generic_handle_irq+0x30/0x44) r5:ee4a9020 r4:01cc [] (generic_handle_irq) from [] (dra7xx_pcie_msi_irq_handler+0x7c/0x8c) r5:ee4a9020 r4:0001 [] (dra7xx_pcie_msi_irq_handler) from [] (irq_forced_thread_fn+0x28/0x5c) r5:ee1d0900 r4:ee4b59c0 [] (irq_forced_thread_fn) from [] (irq_thread+0x128/0x204) r7:0001 r6: r5:ee4d2000 r4:ee4b59e4 [] (irq_thread) from [] (kthread+0xd4/0xec) r10: r9: r8: r7:c00870b4 r6:ee4b59c0 r5:ee4b5a00 r4: [] (kthread) from [] (ret_from_fork+0x14/0x2c) r7: r6: r5:c005e228 r4:ee4b5a00 ---[ end trace 0002 ]--- This backtrace is triggered because dra7xx_pcie_msi_irq_handler() forced to be threaded by default on -RT but, in the same time, it's IRQ dispatcher and calls generic_handle_irq(), which leads to handle_simple_irq() call. The handle_simple_irq() expected to be called with IRQ disabled. The same issue will also happen if kernel will boot with "threadirqs" cmdline parameter (CONFIG_IRQ_FORCED_THREADING). As discussed in [1], the current DRA7 PCIe hw configuration supports 32 interrupts, which cannot change because it's hardwired in silicon, this is a single status read and assuming that only a few (most of the time it will be exactly ONE) of those interrupts are pending at the same time is pretty much a sane assumption. And recommended fix for this issue is to request dra7xx_pcie_msi IRQ with IRQF_NO_THREAD flag. [1] https://lkml.org/lkml/2015/11/3/660 Cc: Thomas Gleixner <t...@linutronix.de> Cc: Sebastian Andrzej Siewior <bige...@linutronix.de> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- Changes in v2: - comment in code truncated Link on v1: https://lkml.org/lkml/2015/11/5/593 drivers/pci/host/pci-dra7xx.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c index 8c36880..0415192 100644 --- a/drivers/pci/host/pci-dra7xx.c +++ b/drivers/pci/host/pci-dra7xx.c @@ -301,8 +301,19 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx, return -EINVAL; } + /* +* Mark dra7xx_pcie_msi IRQ as IRQF_NO_THREAD +* On -RT and if kernel is booting with "threadirqs" cmd line parameter +* the dra7xx_pcie_msi_irq_handler() will be forced threaded but, +* in the same time, it's IRQ dispatcher and calls generic_handle_irq(), +* which, in turn, will be resolved to handle_simple_irq() call. +* The handle_simple_irq() expected to be called with IRQ disabled, as +* result kernle will display warning: +* "irq XXX handler YYY+0x0/0x14 enabled interrupts". +*/ ret = devm_request_irq(>dev, pp->irq, - dra7xx_pcie_msi_irq_handler, IRQF_SHARED, + dra7xx_pcie_msi_irq_handler, + IRQF_SHARED | IRQF_NO_THREAD, "dra7-pcie-msi", pp); if (ret) { dev_err(>dev, "failed to request irq\n"); -- 2.6.3 -- 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 v2] clocksource: arm_global_timer: fix suspend resume
Now the System stall is observed on TI AM437x based board (am437x-gp-evm) during resuming from System suspend when ARM Global timer is selected as clocksource device - SysRq are working, but nothing else. The reason of stall is that ARM Global timer loses its contexts. The reason of stall is that ARM Global timer loses its contexts during System suspend: GT_CONTROL.TIMER_ENABLE = 0 (unbanked) GT_COUNTERx = 0 Hence, update ARM Global timer driver to reflect above behaviour - re-enable ARM Global timer on resume GT_CONTROL.TIMER_ENABLE = 1 - ensure clocksource and clockevent devices have coresponding flags (CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set depending on presence of "always-on" DT property. CC: Arnd Bergmann <a...@arndb.de> Cc: John Stultz <john.stu...@linaro.org> Cc: Felipe Balbi <ba...@ti.com> Cc: Tony Lindgren <t...@atomide.com> Cc: Santosh Shilimkar <ssant...@kernel.org> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- Changes in v2: - suspend/resume simplified: nothing is stored any more and ARM GT just re-enabled Link on v1: https://lkml.org/lkml/2015/11/13/456 drivers/clocksource/arm_global_timer.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c index a2cb6fa..867e546 100644 --- a/drivers/clocksource/arm_global_timer.c +++ b/drivers/clocksource/arm_global_timer.c @@ -51,6 +51,7 @@ static void __iomem *gt_base; static unsigned long gt_clk_rate; static int gt_ppi; static struct clock_event_device __percpu *gt_evt; +static bool gt_always_on; /* * To get the value from the Global Timer Counter register proceed as follows: @@ -168,6 +169,9 @@ static int gt_clockevents_init(struct clock_event_device *clk) { int cpu = smp_processor_id(); + if (!gt_always_on) + clk->features |= CLOCK_EVT_FEAT_C3STOP; + clk->name = "arm_global_timer"; clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERCPU; @@ -195,12 +199,19 @@ static cycle_t gt_clocksource_read(struct clocksource *cs) return gt_counter_read(); } +static void gt_resume(struct clocksource *cs) +{ + /* re-enable timer on resume */ + writel(GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL); +} + static struct clocksource gt_clocksource = { .name = "arm_global_timer", .rating = 300, .read = gt_clocksource_read, .mask = CLOCKSOURCE_MASK(64), .flags = CLOCK_SOURCE_IS_CONTINUOUS, + .resume = gt_resume, }; #ifdef CONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK @@ -218,6 +229,9 @@ static void __init gt_clocksource_init(void) /* enables timer on all the cores */ writel(GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL); + if (gt_always_on) + gt_clocksource.flags |= CLOCK_SOURCE_SUSPEND_NONSTOP; + #ifdef CONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK sched_clock_register(gt_sched_clock_read, 64, gt_clk_rate); #endif @@ -289,6 +303,8 @@ static void __init global_timer_of_register(struct device_node *np) goto out_clk; } + gt_always_on = of_property_read_bool(np, "always-on"); + err = request_percpu_irq(gt_ppi, gt_clockevent_interrupt, "gt", gt_evt); if (err) { -- 2.6.3 -- 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
[4.4-rc][PATCH] gpio: omap: drop omap1 mpuio specific irq_mask/unmask callbacks
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> --- drivers/gpio/gpio-omap.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 56d2d02..f7fbb46 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -1122,8 +1122,6 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) /* MPUIO is a bit different, reading IRQ status clears it */ if (bank->is_mpuio) { irqc->irq_ack = dummy_irq_chip.irq_ack; - irqc->irq_mask = irq_gc_mask_set_bit; - irqc->irq_unmask = irq_gc_mask_clr_bit; if (!bank->regs->wkup_en) irqc->irq_set_wake = NULL; } -- 2.6.3 -- 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
[RFC PATCH] clocksource: ti-32k: convert to platform device
Since system clocksource is finally selected by Clocksource core at fs_initcall stage during boot there are no reasons to initialize ti_32k_timer at early boot stages. Hence, ti_32k_timer can be converted to use platform device/driver model and its PM can be implemented using PM runtime which is common for OMAP devices. Platform specific initialization code has to be disabled once as ti_32k_timer is converted to platform device - otherwise OMAP platform code will generate boot warnings. After this change, all counter_32k's platform code can be removed once all OMAP boards will be converted to DT. Cc: Tony Lindgren <t...@atomide.com> Cc: Felipe Balbi <ba...@ti.com> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- arch/arm/mach-omap2/timer.c| 16 +++ drivers/clocksource/timer-ti-32k.c | 58 -- 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c index b18ebbe..3bfde44 100644 --- a/arch/arm/mach-omap2/timer.c +++ b/arch/arm/mach-omap2/timer.c @@ -393,23 +393,15 @@ static const struct of_device_id omap_counter_match[] __initconst = { static int __init __maybe_unused omap2_sync32k_clocksource_init(void) { int ret; - struct device_node *np = NULL; struct omap_hwmod *oh; const char *oh_name = "counter_32k"; /* -* If device-tree is present, then search the DT blob -* to see if the 32kHz counter is supported. +* If device-tree is present, then just exit - +* 32kHz clocksource driver will handle it. */ - if (of_have_populated_dt()) { - np = omap_get_timer_dt(omap_counter_match, NULL); - if (!np) - return -ENODEV; - - of_property_read_string_index(np, "ti,hwmods", 0, _name); - if (!oh_name) - return -ENODEV; - } + if (of_have_populated_dt()) + return 0; /* * First check hwmod data is available for sync32k counter diff --git a/drivers/clocksource/timer-ti-32k.c b/drivers/clocksource/timer-ti-32k.c index 8518d9d..e71496f 100644 --- a/drivers/clocksource/timer-ti-32k.c +++ b/drivers/clocksource/timer-ti-32k.c @@ -39,8 +39,11 @@ #include #include #include +#include #include #include +#include +#include /* * 32KHz clocksource ... always available, on pretty most chips except @@ -88,15 +91,28 @@ static u64 notrace omap_32k_read_sched_clock(void) return ti_32k_read_cycles(_32k_timer.cs); } -static void __init ti_32k_timer_init(struct device_node *np) +static const struct of_device_id ti_32k_of_table[] = { + { .compatible = "ti,omap-counter32k" }, + { } +}; +MODULE_DEVICE_TABLE(of, ti_32k_of_table); + +static int __init ti_32k_probe(struct platform_device *pdev) { + struct device *dev = >dev; + struct resource *res; int ret; - ti_32k_timer.base = of_iomap(np, 0); - if (!ti_32k_timer.base) { - pr_err("Can't ioremap 32k timer base\n"); - return; - } + /* Static mapping, never released */ + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + ti_32k_timer.base = devm_ioremap_resource(dev, res); + if (IS_ERR(ti_32k_timer.base)) + return PTR_ERR(ti_32k_timer.base); + + pm_runtime_enable(dev); + ret = pm_runtime_get_sync(dev); + if (ret < 0) + goto probe_err; ti_32k_timer.counter = ti_32k_timer.base; @@ -116,11 +132,35 @@ static void __init ti_32k_timer_init(struct device_node *np) ret = clocksource_register_hz(_32k_timer.cs, 32768); if (ret) { pr_err("32k_counter: can't register clocksource\n"); - return; + goto probe_err; } sched_clock_register(omap_32k_read_sched_clock, 32, 32768); pr_info("OMAP clocksource: 32k_counter at 32768 Hz\n"); + return 0; + +probe_err: + pm_runtime_put_noidle(dev); + return ret; +}; + +static struct platform_driver ti_32k_driver __initdata = { + .probe = ti_32k_probe, + .driver = { + .name = "ti_32k_timer", + .of_match_table = of_match_ptr(ti_32k_of_table), + } +}; + +static int __init ti_32k_init(void) +{ + return platform_driver_register(_32k_driver); } -CLOCKSOURCE_OF_DECLARE(ti_32k_timer, "ti,omap-counter32k", - ti_32k_timer_init); + +subsys_initcall(ti_32k_init); + +MODULE_AUTHOR("Paul Mundt"); +MODULE_AUTHOR("Juha Yrjölä"); +MODULE_DESCRIPTION("OMAP2 32k Timer"); +MODULE_ALIAS("platform:ti_32k_timer"); +MODULE_LICENSE("GPL v2"); -- 2.6.3 -- 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] clocksource: arm_global_timer: fix suspend resume
Hi Santosh, On 11/20/2015 07:23 PM, santosh shilimkar wrote: > + Thomas, Marc > > On 11/20/2015 5:57 AM, Grygorii Strashko wrote: >> Now the System stall is observed on TI AM437x based board >> (am437x-gp-evm) during resuming from System suspend when ARM Global >> timer is selected as clocksource device - SysRq are working, but >> nothing else. The reason of stall is that ARM Global timer loses its >> contexts. >> >> The reason of stall is that ARM Global timer loses its contexts during >> System suspend: >> GT_CONTROL.TIMER_ENABLE = 0 (unbanked) >> GT_COUNTERx = 0 >> >> Hence, update ARM Global timer driver to reflect above behaviour >> - re-enable ARM Global timer on resume GT_CONTROL.TIMER_ENABLE = 1 >> - ensure clocksource and clockevent devices have coresponding flags >>(CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set >>depending on presence of "always-on" DT property. >> > Something which loses context in low power states can't be > called "always-on" Sry, it's kinda new area for me and I could make mistakes. While working on this patch I've: - re-used implementation from ARM arch timer commit 82a5619410d4c4df65c04272db198eca5a867c18 Author: Lorenzo Pieralisi <lorenzo.pieral...@arm.com> Date: Tue Apr 8 10:04:32 2014 +0100 clocksource: arch_arm_timer: Fix age-old arch timer C3STOP detection issue - and followed timekeeping.txt: "Typically the clock source is a monotonic, atomic counter which will provide n bits which count from 0 to 2^(n-1) and then wraps around to 0 and start over. It will ideally NEVER stop ticking as long as the system is running. It may stop during system suspend." ^^ And that exactly my use-case: I'd like to use ARM GT as clocksource and with CPUIdle = n. But System suspend has to be allowed. > > Also if the clock-soucre can't tick in the low power states > then that device shouldn't be used as a clock-source. Agree. clocksource can't (except with suspend). Have I missed something? > >> CC: Arnd Bergmann <a...@arndb.de> >> Cc: John Stultz <john.stu...@linaro.org> >> Cc: Felipe Balbi <ba...@ti.com> >> Cc: Tony Lindgren <t...@atomide.com> >> Cc: Santosh Shilimkar <ssant...@kernel.org> >> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> >> --- >> Changes in v2: >> - suspend/resume simplified: nothing is stored any more and >> ARM GT just re-enabled >> Link on v1: >> https://lkml.org/lkml/2015/11/13/456 >> >> drivers/clocksource/arm_global_timer.c | 16 >> 1 file changed, 16 insertions(+) >> >> diff --git a/drivers/clocksource/arm_global_timer.c >> b/drivers/clocksource/arm_global_timer.c >> index a2cb6fa..867e546 100644 >> --- a/drivers/clocksource/arm_global_timer.c >> +++ b/drivers/clocksource/arm_global_timer.c >> @@ -51,6 +51,7 @@ static void __iomem *gt_base; >> static unsigned long gt_clk_rate; >> static int gt_ppi; >> static struct clock_event_device __percpu *gt_evt; >> +static bool gt_always_on; >> >> /* >>* To get the value from the Global Timer Counter register proceed >> as follows: >> @@ -168,6 +169,9 @@ static int gt_clockevents_init(struct >> clock_event_device *clk) >> { >> int cpu = smp_processor_id(); >> >> +if (!gt_always_on) >> +clk->features |= CLOCK_EVT_FEAT_C3STOP; >> + I can drop ^ >> clk->name = "arm_global_timer"; >> clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | >> CLOCK_EVT_FEAT_PERCPU; and change this as clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERCPU | CLOCK_EVT_FEAT_C3STOP; >> @@ -195,12 +199,19 @@ static cycle_t gt_clocksource_read(struct >> clocksource *cs) >> return gt_counter_read(); >> } >> >> +static void gt_resume(struct clocksource *cs) >> +{ >> +/* re-enable timer on resume */ >> +writel(GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL); >> +} >> + >> static struct clocksource gt_clocksource = { >> .name= "arm_global_timer", >> .rating= 300, >> .read= gt_clocksource_read, >> .mask= CLOCKSOURCE_MASK(64), >> .flags= CLOCK_SOURCE_IS_CONTINUOUS, >> +.resume = gt_resume, >> }; >> >> #ifdef CONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK >> @@ -218,6 +229,9 @@ static void __init gt_clocksource_init(void) >>
Re: [PATCH v2] clocksource: arm_global_timer: fix suspend resume
On 11/20/2015 09:09 PM, John Stultz wrote: > On Fri, Nov 20, 2015 at 10:35 AM, Grygorii Strashko > <grygorii.stras...@ti.com> wrote: >> Hi Santosh, >> >> On 11/20/2015 07:23 PM, santosh shilimkar wrote: >>> + Thomas, Marc >>> >>> On 11/20/2015 5:57 AM, Grygorii Strashko wrote: >>>> Now the System stall is observed on TI AM437x based board >>>> (am437x-gp-evm) during resuming from System suspend when ARM Global >>>> timer is selected as clocksource device - SysRq are working, but >>>> nothing else. The reason of stall is that ARM Global timer loses its >>>> contexts. >>>> >>>> The reason of stall is that ARM Global timer loses its contexts during >>>> System suspend: >>>> GT_CONTROL.TIMER_ENABLE = 0 (unbanked) >>>> GT_COUNTERx = 0 >>>> >>>> Hence, update ARM Global timer driver to reflect above behaviour >>>> - re-enable ARM Global timer on resume GT_CONTROL.TIMER_ENABLE = 1 >>>> - ensure clocksource and clockevent devices have coresponding flags >>>> (CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set >>>> depending on presence of "always-on" DT property. >>>> >>> Something which loses context in low power states can't be >>> called "always-on" >> >> Sry, it's kinda new area for me and I could make mistakes. >> >> While working on this patch I've: >> - re-used implementation from ARM arch timer >> commit 82a5619410d4c4df65c04272db198eca5a867c18 >> Author: Lorenzo Pieralisi <lorenzo.pieral...@arm.com> >> Date: Tue Apr 8 10:04:32 2014 +0100 >> >> clocksource: arch_arm_timer: Fix age-old arch timer C3STOP detection >> issue >> >> >> - and followed timekeeping.txt: >> "Typically the clock source is a monotonic, atomic counter which will provide >> n bits which count from 0 to 2^(n-1) and then wraps around to 0 and start >> over. >> It will ideally NEVER stop ticking as long as the system is running. It >> may stop during system suspend." >> ^^ >> >> And that exactly my use-case: I'd like to use ARM GT as clocksource >> and with CPUIdle = n. But System suspend has to be allowed. >> >> >>> >>> Also if the clock-soucre can't tick in the low power states >>> then that device shouldn't be used as a clock-source. >> >> Agree. clocksource can't (except with suspend). Have I missed something? > > I think the point Stantosh is making is that if the clocksource stops > in suspend (which is allowed) you should not be setting > CLOCK_SOURCE_SUSPEND_NONSTOP (which promises the clocksource doesn't > stop in suspend, so it can be used for suspend timing as well). > Ok. Thanks. I got it now. Adding CLOCK_SOURCE_SUSPEND_NONSTOP is mistake. > The contradictory part in your patch is that you're also setting the > clockevent logic as CLOCK_EVT_FEAT_C3STOP, which flags that the > clockevent hardware might stop in low-power idle states (ie: not > suspend, but while the system is running). Usually hardware that > halts in low-power mode (like the apic on some x86 machines) is not > also used for timekeeping (instead we use the hpet/acpi there). Sry, I've set CLOCK_EVT_FEAT_C3STOP if "always-on" = *false* ^ this, I thought, might be valid because it will stop in low-power idle states and "always-on" was expected to indicate case when CPUIdle = n (no CPU PM), same way as Lorenzo did. Would it be correct to just set it always? and CLOCK_SOURCE_SUSPEND_NONSTOP if "always-on" = *true* ^ this part is mistake > > So its unlikely that the hardware both stays running through suspend, > but also might halt in idle. That would be "unique". > Thanks a lot. -- regards, -grygorii -- 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] clocksource: arm_global_timer: fix suspend resume
On 11/20/2015 08:52 PM, santosh shilimkar wrote: > On 11/20/2015 10:46 AM, Marc Zyngier wrote: >> On 20/11/15 18:35, Grygorii Strashko wrote: >>> Hi Santosh, >>> >>> On 11/20/2015 07:23 PM, santosh shilimkar wrote: >>>> + Thomas, Marc >>>> >>>> On 11/20/2015 5:57 AM, Grygorii Strashko wrote: >>>>> Now the System stall is observed on TI AM437x based board >>>>> (am437x-gp-evm) during resuming from System suspend when ARM Global >>>>> timer is selected as clocksource device - SysRq are working, but >>>>> nothing else. The reason of stall is that ARM Global timer loses its >>>>> contexts. >>>>> >>>>> The reason of stall is that ARM Global timer loses its contexts during >>>>> System suspend: >>>>> GT_CONTROL.TIMER_ENABLE = 0 (unbanked) >>>>> GT_COUNTERx = 0 >>>>> >>>>> Hence, update ARM Global timer driver to reflect above behaviour >>>>> - re-enable ARM Global timer on resume GT_CONTROL.TIMER_ENABLE = 1 >>>>> - ensure clocksource and clockevent devices have coresponding flags >>>>> (CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set >>>>> depending on presence of "always-on" DT property. >>>>> >>>> Something which loses context in low power states can't be >>>> called "always-on" >>> >>> Sry, it's kinda new area for me and I could make mistakes. >>> >>> While working on this patch I've: >>> - re-used implementation from ARM arch timer >>> commit 82a5619410d4c4df65c04272db198eca5a867c18 >>> Author: Lorenzo Pieralisi <lorenzo.pieral...@arm.com> >>> Date: Tue Apr 8 10:04:32 2014 +0100 >>> >>> clocksource: arch_arm_timer: Fix age-old arch timer C3STOP >>> detection issue >> >> [...] >> >> This patch has a very specific purpose: instructing the core code that >> this timer will never stop ticking, ever. It is really targeted at >> virtual machines, whose timer is backed by the host timer, even when the >> VM is not running. >> >> Using it on actual hardware is may not be the best idea, specially in >> the presence of PM. >> > Exactly. Thanks for clarifying the commit Marc. Thanks for your comments, but could you clarify pls - Can i use ARM GT for my use case CPUIDLE=n and SUSPEND=y, taking into account that System time can be corrected by RTC core during resume? (it will be used as clocksource only). And would it be reasonable to resend new version if i will drop all "always-on" related code? -- regards, -grygorii -- 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] clocksource: arm_global_timer: fix suspend resume
On 11/20/2015 09:32 PM, John Stultz wrote: > On Fri, Nov 20, 2015 at 11:28 AM, Grygorii Strashko > <grygorii.stras...@ti.com> wrote: >> On 11/20/2015 09:09 PM, John Stultz wrote: >>> On Fri, Nov 20, 2015 at 10:35 AM, Grygorii Strashko >>> <grygorii.stras...@ti.com> wrote: >>>> Hi Santosh, >>>> >>>> On 11/20/2015 07:23 PM, santosh shilimkar wrote: >>>>> + Thomas, Marc >>>>> >>>>> On 11/20/2015 5:57 AM, Grygorii Strashko wrote: >>>>>> Now the System stall is observed on TI AM437x based board >>>>>> (am437x-gp-evm) during resuming from System suspend when ARM Global >>>>>> timer is selected as clocksource device - SysRq are working, but >>>>>> nothing else. The reason of stall is that ARM Global timer loses its >>>>>> contexts. >>>>>> >>>>>> The reason of stall is that ARM Global timer loses its contexts during >>>>>> System suspend: >>>>>> GT_CONTROL.TIMER_ENABLE = 0 (unbanked) >>>>>> GT_COUNTERx = 0 >>>>>> >>>>>> Hence, update ARM Global timer driver to reflect above behaviour >>>>>> - re-enable ARM Global timer on resume GT_CONTROL.TIMER_ENABLE = 1 >>>>>> - ensure clocksource and clockevent devices have coresponding flags >>>>>> (CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set >>>>>> depending on presence of "always-on" DT property. >>>>>> >>>>> Something which loses context in low power states can't be >>>>> called "always-on" >>>> >>>> Sry, it's kinda new area for me and I could make mistakes. >>>> >>>> While working on this patch I've: >>>>- re-used implementation from ARM arch timer >>>> commit 82a5619410d4c4df65c04272db198eca5a867c18 >>>> Author: Lorenzo Pieralisi <lorenzo.pieral...@arm.com> >>>> Date: Tue Apr 8 10:04:32 2014 +0100 >>>> >>>> clocksource: arch_arm_timer: Fix age-old arch timer C3STOP detection >>>> issue >>>> >>>> >>>> - and followed timekeeping.txt: >>>> "Typically the clock source is a monotonic, atomic counter which will >>>> provide >>>>n bits which count from 0 to 2^(n-1) and then wraps around to 0 and >>>> start over. >>>> It will ideally NEVER stop ticking as long as the system is running. It >>>> may stop during system suspend." >>>> ^^ >>>> >>>> And that exactly my use-case: I'd like to use ARM GT as clocksource >>>> and with CPUIdle = n. But System suspend has to be allowed. >>>> >>>> >>>>> >>>>> Also if the clock-soucre can't tick in the low power states >>>>> then that device shouldn't be used as a clock-source. >>>> >>>> Agree. clocksource can't (except with suspend). Have I missed something? >>> >>> I think the point Stantosh is making is that if the clocksource stops >>> in suspend (which is allowed) you should not be setting >>> CLOCK_SOURCE_SUSPEND_NONSTOP (which promises the clocksource doesn't >>> stop in suspend, so it can be used for suspend timing as well). >>> >> >> Ok. Thanks. I got it now. Adding CLOCK_SOURCE_SUSPEND_NONSTOP is mistake. >> >>> The contradictory part in your patch is that you're also setting the >>> clockevent logic as CLOCK_EVT_FEAT_C3STOP, which flags that the >>> clockevent hardware might stop in low-power idle states (ie: not >>> suspend, but while the system is running). Usually hardware that >>> halts in low-power mode (like the apic on some x86 machines) is not >>> also used for timekeeping (instead we use the hpet/acpi there). >> >> Sry, I've set CLOCK_EVT_FEAT_C3STOP if "always-on" = *false* > > You might also consider renaming that value from always_on to > something more descriptive, given the subtlety of the different states > here. Maybe instead use a flag called halts_in_idle or something? > I'd better just drop it for now hence I'm still not sure what is more reasonable continue with DT property or just add this flag always. -- regards, -grygorii -- 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: [BISECTED] v4.4-rc1 GPIO regression on OMAP1
On 11/18/2015 08:31 PM, Aaro Koskinen wrote: > Hi, > > On Wed, Nov 18, 2015 at 02:58:38PM +0200, Grygorii Strashko wrote: >> On 11/17/2015 11:42 PM, Aaro Koskinen wrote: >>> Amstrad E3 computer (OMAP1) boot crashes early with v4.4-rc1. Bisected to: >>> >>> 450fa54cfd66e3dda6eda26256637ee8928af12a is the first bad commit >>> commit 450fa54cfd66e3dda6eda26256637ee8928af12a >>> Author: Grygorii Strashko <grygorii.stras...@ti.com> >>> Date: Fri Sep 25 12:28:03 2015 -0700 >>> >>> gpio: omap: convert to use generic irq handler >> >> Could you check pls if below change will solve this issue, pls? >> >> iff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c >> index 56d2d02..2905e0d 100644 >> --- a/drivers/gpio/gpio-omap.c >> +++ b/drivers/gpio/gpio-omap.c >> @@ -1129,7 +1129,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, >> struct irq_chip *irqc) >> } >> >> ret = gpiochip_irqchip_add(>chip, irqc, >> - irq_base, handle_bad_irq, >> + irq_base, handle_simple_irq, >> IRQ_TYPE_NONE); > Thanks for testing it. But I'd like to ask you for more help here if possible :) Above change is expected to fix the OMAP1 boot, but looking at the OMAP GPIO code I think that MPUIO IRQ are not working :( and this functionality has been broken long time ago - commit "gpio: omap: convert to use generic irq handler" has just made hidden issue visible. (I've found only one OMAP1 board which uses MPUIO GPIO as IRQ (board-osk.c)). So, could I ask you to test another change instead of previous one? --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -1122,8 +1122,6 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) /* MPUIO is a bit different, reading IRQ status clears it */ if (bank->is_mpuio) { irqc->irq_ack = dummy_irq_chip.irq_ack; - irqc->irq_mask = irq_gc_mask_set_bit; - irqc->irq_unmask = irq_gc_mask_clr_bit; if (!bank->regs->wkup_en) irqc->irq_set_wake = NULL; } In my opinion real test will be - request(any MPUIO GPIO IRQ, IRQF_TRIGGER_LOW/HIGH), - trigger MPUIO GPIO IRQ - free(MPUIO GPIO IRQ) -- regards, -grygorii -- 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.4-rc][PATCH] ARM: dts: am4372: disable arm twd and global timer's nodes
Hi Mark, On 11/18/2015 04:15 PM, Mark Rutland wrote: > On Wed, Nov 18, 2015 at 04:01:55PM +0200, Grygorii Strashko wrote: >> Keep ARM TWD and Global timer's nodes disabled by default - if someone >> would like to use them then those nodes have to be enabled explicitly >> in board file. >> >> The reason for this change is: >> - ARM TWD is not always-on timer on am437x and it will stop in low >>CPUIdle states and, therefore, broadcast timer has to configured >>properly if CPU_IDLE=y. >> - ARM Global timer is not always-on timer on am437x and it can't be >>used as clocksource device if CPU_IDLE=y. > > I don't understand. What timer do you use in the absence of a TWD, and > if it is better why is it not used even if TWD is present? OMAP GP timer as clockevent device "ti,omap-counter32k" as clocksource both are in wakeup (always-on) PM domain. I see some problems with selecting clocksource/event device (but may be I'm mistaken): - Current clock event will be selected using "rating" field in clock_event_device structure and now ARM TWD has higer value (350) vs OMAP GP timer (300), so ARM TWD will be always selected if it's present. - we have to force all am437x user to use kernel cmdline parameter "clocksource=" if ARM Global timer is present, but shouldn't be used. But even in this case, it will be selected as sched_clock. We can see benefits from using these timers when Power mangement is not the case, for example on -RT kernel where CPUIdle is not the target. By this change, and taking into account discovered issues, I want to make people, who would like to use these timers on AM437x based boards, responsible for such decision with assumption that they know what they are doing. And this is simplest way I found to disable those timers without modifying kernel. > >> - ARM Global timer driver doesn't support CPUfreq now. > > Surely that should be fixed in the driver (e.g. make it fail to probe if > CPUfreq is present)? It's broken for any other users too... May be. Also, there is additional issue with ARM global timer which I've not mentioned here, because I sent the fix for it already (in progress): https://lkml.org/lkml/2015/11/13/456 and there is ongoing discussion: http://www.spinics.net/lists/arm-kernel/msg459649.html -- regards, -grygorii -- 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: [BISECTED] v4.4-rc1 GPIO regression on OMAP1
Hi Aaro, On 11/17/2015 11:42 PM, Aaro Koskinen wrote: Amstrad E3 computer (OMAP1) boot crashes early with v4.4-rc1. Bisected to: 450fa54cfd66e3dda6eda26256637ee8928af12a is the first bad commit commit 450fa54cfd66e3dda6eda26256637ee8928af12a Author: Grygorii Strashko <grygorii.stras...@ti.com> Date: Fri Sep 25 12:28:03 2015 -0700 gpio: omap: convert to use generic irq handler Reverting the commit works (as a quick workaround at least). The crash log: [0.253612] Unable to handle kernel NULL pointer dereference at virtual address [0.262613] pgd = c0004000 [0.265754] [] *pgd= [0.269862] Internal error: Oops: 75 [#1] ARM [0.274734] Modules linked in: [0.278313] CPU: 0 PID: 1 Comm: swapper Not tainted 4.4.0-rc1-e3-los_afe0c+-2-g25379c0-dirty #1 [0.288246] Hardware name: Amstrad E3 (Delta) [0.293146] task: c1836000 ti: c1838000 task.ti: c1838000 [0.299155] PC is at irq_gc_mask_set_bit+0x1c/0x60 [0.304525] LR is at __irq_do_set_handler+0x118/0x15c [0.310165] pc : []lr : []psr: 60d3 [0.310165] sp : c1839c90 ip : c1862c64 fp : c1839c9c [0.322740] r10: r9 : c0411950 r8 : c0411bbc [0.328533] r7 : r6 : c185c310 r5 : c00444e8 r4 : c185c300 [0.335687] r3 : c1854b50 r2 : r1 : r0 : c185c310 [0.342858] Flags: nZCv IRQs off FIQs off Mode SVC_32 ISA ARM Segment kernel [0.351153] Control: 317f Table: 10004000 DAC: 0057 [0.357494] Process swapper (pid: 1, stack limit = 0xc1838190) [0.363917] Stack: (0xc1839c90 to 0xc183a000) [0.368837] 9c80: c1839cc4 c1839ca0 c0047d4c c0048480 [0.377916] 9ca0: 0050 c185c300 c00444e8 c1839cec c1839cc8 [0.386989] 9cc0: c0047dd4 c0047c44 c0046f40 a053 0050 c00444e8 0050 [0.396064] 9ce0: c1839d0c c1839cf0 c0047e1c c0047da0 c1862c64 0050 0050 [0.405139] 9d00: c1839d24 c1839d10 c01b345c c0047dfc c185cb00 c185c310 c1839d54 c1839d28 [0.414214] 9d20: c0049670 c01b3430 c0075888 c0043440 c1854b50 0001 0010 0050 [0.423293] 9d40: c185cb00 c0411bbc c1839d7c c1839d58 c0049894 c0049604 c1862c64 [0.432375] 9d60: c1854b50 c1862c64 c0419280 c1862c64 c1839d9c c1839d80 c01b3328 c004980c [0.441456] 9d80: c1862c64 c1862c64 c1862c10 c1854b50 c1839ddc c1839da0 c01b79f4 c01b32d4 [0.450530] 9da0: c0411958 c0419294 c0411950 c0419294 c0411984 [0.459605] 9dc0: c0419294 c03e45e8 c1839df4 c1839de0 c01fcf58 c01b7708 [0.468684] 9de0: c0411950 c045ce30 c1839e1c c1839df8 c01fb668 c01fcf3c c0411950 c0419294 [0.477759] 9e00: c0411984 c03e45e8 c1839e3c c1839e20 c01fb844 c01fb518 [0.486836] 9e20: c0419294 c01fb7a4 c1839e64 c1839e40 c01f9a90 c01fb7b4 [0.495913] 9e40: c185434c c185b450 c185b514 c0419294 c185b4e0 c041d9c8 c1839e74 c1839e68 [0.504991] 9e60: c01fb078 c01f9a28 c1839e9c c1839e78 c01facf8 c01fb068 c03931d0 c1839e88 [0.514069] 9e80: c0419294 c040bb80 c1804960 c03f5a08 c1839eb4 c1839ea0 c01fbdb0 c01fab7c [0.523150] 9ea0: c040bb80 c040bb80 c1839ec4 c1839eb8 c01fcf18 c01fbd40 c1839ed4 c1839ec8 [0.532227] 9ec0: c03f5a20 c01fcef0 c1839f4c c1839ed8 c000966c c03f5a18 c1839efc c1839ee8 [0.541304] 9ee0: c03e4604 c019b3b4 c03e2700 c1ffcd34 c1839f4c c1839f00 c00314ec c03e45f8 [0.550384] 9f00: 0002 0002 c03e2024 c038f2fc [0.559465] 9f20: c1839f4c 0002 c03ff81c 0002 c03ff820 005e c042ba80 c042ba80 [0.568543] 9f40: c1839f94 c1839f50 c03e4e14 c00095f0 0002 0002 c03e45e8 [0.577625] 9f60: 9b7bd08b c0406660 ad19cf47 c0305764 [0.586701] 9f80: c1839fac c1839f98 c0305774 c03e4d18 c0305764 [0.595775] 9fa0: c1839fb0 c000a440 c0305774 [0.604845] 9fc0: [0.613916] 9fe0: 0013 faef8a80 f438707c [0.622908] Backtrace: [0.625894] [] (irq_gc_mask_set_bit) from [] (__irq_do_set_handler+0x118/0x15c) [0.635897] [] (__irq_do_set_handler) from [] (__irq_set_handler+0x44/0x5c) [0.645458] r6: r5:c00444e8 r4:c185c300 [0.650746] [] (__irq_set_handler) from [] (irq_set_chip_and_handler_name+0x30/0x34) [0.661128] r7:0050 r6: r5:c00444e8 r4:0050 [0.667559] [] (irq_set_chip_and_handler_name) from [] (gpiochip_irq_map+0x3c/0x8c) [0.677869] r7:0050 r6: r5:0050 r4:c1862c64 [0.684300] [] (gpiochip_irq_map) from [] (irq_domain_associate+0x7c/0x1c4) [0.693885] r5:c185c310 r4:c185cb00 [0.
Re: [PATCH] arm: omap2: Kconfig: select TWD and global timer on AM43xx devices
On 11/13/2015 06:39 PM, santosh shilimkar wrote: > On 11/13/2015 5:07 AM, Mason wrote: >> On 13/11/2015 13:48, Grygorii Strashko wrote: >>> On 11/12/2015 08:06 PM, Felipe Balbi wrote: >>>> Make sure to tell the kernel that AM437x has >>>> TWD and global timers. >>>> >>>> Signed-off-by: Felipe Balbi <ba...@ti.com> >>>> --- >>>> >>>> Hi Tony, >>>> >>>> now that all dependencies are in place, we can >>>> finally enable twd and global_timer for AM437x. >>>> > Is AM437x SMP SOC ? No. But it has ARM TWD and GT > Is TWD on AM437x works in low power states ? No. But TWD, seems, is not a problem here if omap gp timer can be used as broadcast device. > Probably I haven't followed the recent updates, but does > the TWD supports C3STOP on UP systems ? The broadcast > code was SMP only in past, and TWD use to die in > low power state on past OMAP SOCs. > > If either of these are still the issue then > TWD shouldn't be used. > Yep. I see the problem with ARM Global timer here if CPUIdle is enabled and ARM Global timer is selected as clocksource device. I think, it will be right thing to disable "global_timer" and "local_timer" nodes in am437x.dtsi by default - if someone would like to use them then those nodes have to be enabled explicitly in board file. (TWD timer will be enabled on OMAP multi SoC build irrespectively of HAVE_ARM_TWD is selected for am437x or not, because it will be selected for omap4). I've just sent corresponding patch: http://www.spinics.net/lists/arm-kernel/msg461141.html Also, probably, it could be reasonable to: - make ARM_GLOBAL_TIMER fully selectable option and allow select it from defconfig. - or - update this patch as below - select ARM_GLOBAL_TIMER - select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK + select ARM_GLOBAL_TIMER if !CPU_IDLE + select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK if ARM_GLOBAL_TIMER Thanks. -- regards, -grygorii -- 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
[4.4-rc][PATCH] ARM: dts: am4372: disable arm twd and global timer's nodes
Keep ARM TWD and Global timer's nodes disabled by default - if someone would like to use them then those nodes have to be enabled explicitly in board file. The reason for this change is: - ARM TWD is not always-on timer on am437x and it will stop in low CPUIdle states and, therefore, broadcast timer has to configured properly if CPU_IDLE=y. - ARM Global timer is not always-on timer on am437x and it can't be used as clocksource device if CPU_IDLE=y. - ARM Global timer driver doesn't support CPUfreq now. Cc: Felipe Balbi <ba...@ti.com> Cc: Santosh Shilimkar <ssant...@kernel.org> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- arch/arm/boot/dts/am4372.dtsi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi index d83ff9c..11376e3 100644 --- a/arch/arm/boot/dts/am4372.dtsi +++ b/arch/arm/boot/dts/am4372.dtsi @@ -75,6 +75,7 @@ interrupts = ; interrupt-parent = <>; clocks = <_mpu_m2_ck>; + status = "disabled"; }; local_timer: timer@48240600 { @@ -83,6 +84,7 @@ interrupts = ; interrupt-parent = <>; clocks = <_mpu_m2_ck>; + status = "disabled"; }; l2-cache-controller@48242000 { -- 2.6.3 -- 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: OMAP4: execute initcall to reserve SRAM for I688 only on OMAP4
On 11/16/2015 01:25 PM, Lucas Stach wrote: > omap_interconnect_sync() is the only user of the SRAM scratch area > allocated in the omap4_sram_init initcall. The interconnect sync is > used exclusively in the OMAP4 specific WFI implementation, so there > is no point in allocating the SRAM scratch on other SoC types. > > Bail out of the initcall if the kernel is not running on OMAP4 to > avoid a confusing warning about being unable to allocate the SRAM > needed for I688 handling. > > Signed-off-by: Lucas Stach> Tested-by: Bastian Stender > --- > arch/arm/mach-omap2/omap4-common.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm/mach-omap2/omap4-common.c > b/arch/arm/mach-omap2/omap4-common.c > index 949696b6f17b..6db393a30a28 100644 > --- a/arch/arm/mach-omap2/omap4-common.c > +++ b/arch/arm/mach-omap2/omap4-common.c > @@ -131,6 +131,9 @@ static int __init omap4_sram_init(void) > struct device_node *np; > struct gen_pool *sram_pool; > > + if (!cpu_is_omap44xx()) > + return 0; This one affects on am43xx also > + > np = of_find_compatible_node(NULL, NULL, "ti,omap4-mpu"); > if (!np) > pr_warn("%s:Unable to allocate sram needed to handle errata > I688\n", Since all OMAP4+ platforms are now DT based why can't we just return from here silently? -- regards, -grygorii -- 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] ARM: OMAP4+: SMP: use lockless clkdm/pwrdm api in omap4_boot_secondary
OMAP CPU hotplug uses cpu1's clocks and power domains for CPU1 wake up from low power states (or turn on CPU1). This part of code is also part of system suspend (disable_nonboot_cpus()). >From other side, cpu1's clocks and power domains are used by CPUIdle. All above functionality is mutually exclusive and, therefore, lockless clkdm/pwrdm api can be used in omap4_boot_secondary(). This fixes below back-trace on -RT which is triggered by pwrdm_lock/unlock(): BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:917 in_atomic(): 1, irqs_disabled(): 0, pid: 118, name: sh 9 locks held by sh/118: #0: (sb_writers#4){.+.+.+}, at: [] vfs_write+0x13c/0x164 #1: (>mutex){+.+.+.}, at: [] kernfs_fop_write+0x48/0x19c #2: (s_active#24){.+.+.+}, at: [] kernfs_fop_write+0x50/0x19c #3: (device_hotplug_lock){+.+.+.}, at: [] lock_device_hotplug_sysfs+0xc/0x4c #4: (>mutex){..}, at: [] device_online+0x14/0x88 #5: (cpu_add_remove_lock){+.+.+.}, at: [] cpu_up+0x50/0x1a0 #6: (cpu_hotplug.lock){++}, at: [] cpu_hotplug_begin+0x0/0xc4 #7: (cpu_hotplug.lock#2){+.+.+.}, at: [] cpu_hotplug_begin+0x78/0xc4 #8: (boot_lock){+.+...}, at: [] omap4_boot_secondary+0x1c/0x178 Preemption disabled at:[< (null)>] (null) CPU: 0 PID: 118 Comm: sh Not tainted 4.1.12-rt11-01998-gb4a62c3-dirty #137 Hardware name: Generic DRA74X (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0x80/0x94) [] (dump_stack) from [] (rt_spin_lock+0x24/0x54) [] (rt_spin_lock) from [] (clkdm_wakeup+0x10/0x2c) [] (clkdm_wakeup) from [] (omap4_boot_secondary+0x88/0x178) [] (omap4_boot_secondary) from [] (__cpu_up+0xc4/0x164) [] (__cpu_up) from [] (cpu_up+0x15c/0x1a0) [] (cpu_up) from [] (device_online+0x64/0x88) [] (device_online) from [] (online_store+0x68/0x74) [] (online_store) from [] (kernfs_fop_write+0xb8/0x19c) [] (kernfs_fop_write) from [] (__vfs_write+0x20/0xd8) [] (__vfs_write) from [] (vfs_write+0x90/0x164) [] (vfs_write) from [] (SyS_write+0x44/0x9c) [] (SyS_write) from [] (ret_fast_syscall+0x0/0x54) CPU1: smp_ops.cpu_die() returned, trying to resuscitate Cc: Tero Kristo <t-kri...@ti.com> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- arch/arm/mach-omap2/omap-smp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/mach-omap2/omap-smp.c b/arch/arm/mach-omap2/omap-smp.c index 5305ec7..79e1f87 100644 --- a/arch/arm/mach-omap2/omap-smp.c +++ b/arch/arm/mach-omap2/omap-smp.c @@ -143,9 +143,9 @@ static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle) * Ensure that CPU power state is set to ON to avoid CPU * powerdomain transition on wfi */ - clkdm_wakeup(cpu1_clkdm); - omap_set_pwrdm_state(cpu1_pwrdm, PWRDM_POWER_ON); - clkdm_allow_idle(cpu1_clkdm); + clkdm_wakeup_nolock(cpu1_clkdm); + pwrdm_set_next_pwrst(cpu1_pwrdm, PWRDM_POWER_ON); + clkdm_allow_idle_nolock(cpu1_clkdm); if (IS_PM44XX_ERRATUM(PM_OMAP4_ROM_SMP_BOOT_ERRATUM_GICD)) { while (gic_dist_disabled()) { -- 2.6.3 -- 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] clocksource: arm_global_timer: fix suspend resume
On 11/13/2015 06:43 PM, Felipe Balbi wrote: > > Hi, > > Grygorii Strashko <grygorii.stras...@ti.com> writes: >> Now the System stall is observed on TI AM437x based board >> (am437x-gp-evm) during resuming from System suspend when ARM Global >> timer is selected as clocksource device - SysRq are working, but >> nothing else. The reason of stall is that ARM Global timer loses its >> contexts. >> >> The reason of stall is that ARM Global timer loses its contexts during >> System suspend: >> GT_CONTROL.TIMER_ENABLE = 0 (unbanked) >> GT_COUNTERx = 0 >> >> Hence, update ARM Global timer driver to reflect above behaviour >> - save GT_CONTROL.TIMER_ENABLE during suspend and restore on resume; >> - ensure clocksource and clockevent devices have coresponding flags >>(CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set >>depending on presence of "always-on" DT property. >> >> CC: Arnd Bergmann <a...@arndb.de> >> Cc: John Stultz <john.stu...@linaro.org> >> Cc: Felipe Balbi <ba...@ti.com> >> Cc: Tony Lindgren <t...@atomide.com> >> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> >> --- >> drivers/clocksource/arm_global_timer.c | 23 +++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/drivers/clocksource/arm_global_timer.c >> b/drivers/clocksource/arm_global_timer.c >> index a2cb6fa..1bbaf64 100644 >> --- a/drivers/clocksource/arm_global_timer.c >> +++ b/drivers/clocksource/arm_global_timer.c >> @@ -51,6 +51,8 @@ static void __iomem *gt_base; >> static unsigned long gt_clk_rate; >> static int gt_ppi; >> static struct clock_event_device __percpu *gt_evt; >> +static bool gt_always_on; >> +static u32 gt_control; >> >> /* >>* To get the value from the Global Timer Counter register proceed as >> follows: >> @@ -168,6 +170,9 @@ static int gt_clockevents_init(struct clock_event_device >> *clk) >> { >> int cpu = smp_processor_id(); >> >> +if (!gt_always_on) >> +clk->features |= CLOCK_EVT_FEAT_C3STOP; >> + >> clk->name = "arm_global_timer"; >> clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | >> CLOCK_EVT_FEAT_PERCPU; >> @@ -195,12 +200,25 @@ static cycle_t gt_clocksource_read(struct clocksource >> *cs) >> return gt_counter_read(); >> } >> >> +static void gt_suspend(struct clocksource *cs) >> +{ >> +gt_control = readl(gt_base + GT_CONTROL); >> +} >> + >> +static void gt_resume(struct clocksource *cs) >> +{ >> +/* enables timer on all the cores */ >> +writel(gt_control & GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL); > > do you really need to save context if all you restore is TIMER_ENABLE > bit ? seems like you could skip gt_suspend altogether. Is there really a > situation where this driver is running and GT isn't enabled ? Now It's not. It's always enabled. I did it because .suspend() is called for all registered clock sources regardless of their usage. So, potentially in the future, at the moment when .suspend() is called it might be disabled (for example, .enable/disable() callbacks can be added and, if ARM Global timer will not be registered as sched_clock, it will be possible to keep it disabled if not used now). But It's not essentially now - I can update it and drop save restore. Pls, confirm. -- regards, -grygorii -- 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: omap2: Kconfig: select TWD and global timer on AM43xx devices
On 11/13/2015 02:48 PM, Grygorii Strashko wrote: > On 11/12/2015 08:06 PM, Felipe Balbi wrote: >> Make sure to tell the kernel that AM437x has >> TWD and global timers. >> >> Signed-off-by: Felipe Balbi <ba...@ti.com> >> --- >> >> Hi Tony, >> >> now that all dependencies are in place, we can >> finally enable twd and global_timer for AM437x. >> > > I'd appreciated if someone can clarify if all described below is valid. > (may be some questions are dummy - sorry). > > After all last changes related to TI OMAP clock source/clock event/sched > clock's > devices configuration we will have the following for am437x case: > > clockevents: > "timer1", clockevent_gpt, .rating = 300, CLOCK_EVT_FEAT_PERIODIC | > CLOCK_EVT_FEAT_ONESHOT, ti,timer-alwon, > "arm,twd-timer",twd_evt,.rating = 350, CLOCK_EVT_FEAT_PERIODIC | > CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP > "arm_global_timer", gt_evt, rating = 300, CLOCK_EVT_FEAT_PERIODIC | > CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERCPU > > clocksources: > "jiffies", clocksource_jiffies, rating = 1 > if use_gptimer_clksrc > "timer2", clocksource_gpt, .rating = 300, CLOCK_SOURCE_IS_CONTINUOUS, > |-sched_clock_register(dmtimer_read_sched_clock, 32, clksrc.rate); > else > "ti,omap-counter32k", ti_32k_timer, .rating = 250, > CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP, > |-sched_clock_register(omap_32k_read_sched_clock, 32, 32768); > > "arm,global-timer", gt_clocksource, .rating = 300, > CLOCK_SOURCE_IS_CONTINUOUS > |-sched_clock_register(gt_sched_clock_read, 64, gt_clk_rate); > > 1) current clockevent device will be registered during registration of > clockevent devices > and it will be "arm,twd-timer" - processing sequence follows the way they > are listed above. > > 2) current clocksource will be selected from > fs_initcall(clocksource_done_booting); > and it will be "arm,global-timer" or "timer2" if use_gptimer_clksrc > > 3) sched clock: "arm,global-timer" will be selected as sched_clock *always*, > because it depend on Makefile order (if someone will decide to sort entries > in drivers/clocksource/Makefile then "ti,omap-counter32k" most probably will > be always selected as sched clock). Ok. Sry. Above is not completely true. There are no dependency on makefile and sched clock with Max freq will be selected - and never changed even if corresponding clocksource can be changed through sysfs. > > Uh.. > > As for me, "timer1" is selected as clockevent device incorrectly, because it's > ti,timer-alwon. > > I think, "ti,omap-counter32k" will crash if use_gptimer_clksrc == true, > because > corresponding HWmod will not be enabled (not tested this case). Ok. Below is confirmation for the case if use_gptimer_clksrc == true, so https://www.spinics.net/lists/linux-omap/msg122576.html "[PATCH 00/11] arm: omap: counter32k rework" introduces regression !? cat /sys/devices/system/clocksource/clocksource0/available_clocksource timer2 arm_global_timer 32k_counter / # echo 32k_counter > /sys/devices/system/clocksource/clocksource0/current_cloc ksource [ 102.194313] Unhandled fault: imprecise external abort (0x1406) at 0x0020ab5c [ 102.197862] pgd = ee22c000 -- regards, -grygorii -- 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] clocksource: arm_global_timer: fix suspend resume
Now the System stall is observed on TI AM437x based board (am437x-gp-evm) during resuming from System suspend when ARM Global timer is selected as clocksource device - SysRq are working, but nothing else. The reason of stall is that ARM Global timer loses its contexts. The reason of stall is that ARM Global timer loses its contexts during System suspend: GT_CONTROL.TIMER_ENABLE = 0 (unbanked) GT_COUNTERx = 0 Hence, update ARM Global timer driver to reflect above behaviour - save GT_CONTROL.TIMER_ENABLE during suspend and restore on resume; - ensure clocksource and clockevent devices have coresponding flags (CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set depending on presence of "always-on" DT property. CC: Arnd Bergmann <a...@arndb.de> Cc: John Stultz <john.stu...@linaro.org> Cc: Felipe Balbi <ba...@ti.com> Cc: Tony Lindgren <t...@atomide.com> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- drivers/clocksource/arm_global_timer.c | 23 +++ 1 file changed, 23 insertions(+) diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c index a2cb6fa..1bbaf64 100644 --- a/drivers/clocksource/arm_global_timer.c +++ b/drivers/clocksource/arm_global_timer.c @@ -51,6 +51,8 @@ static void __iomem *gt_base; static unsigned long gt_clk_rate; static int gt_ppi; static struct clock_event_device __percpu *gt_evt; +static bool gt_always_on; +static u32 gt_control; /* * To get the value from the Global Timer Counter register proceed as follows: @@ -168,6 +170,9 @@ static int gt_clockevents_init(struct clock_event_device *clk) { int cpu = smp_processor_id(); + if (!gt_always_on) + clk->features |= CLOCK_EVT_FEAT_C3STOP; + clk->name = "arm_global_timer"; clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERCPU; @@ -195,12 +200,25 @@ static cycle_t gt_clocksource_read(struct clocksource *cs) return gt_counter_read(); } +static void gt_suspend(struct clocksource *cs) +{ + gt_control = readl(gt_base + GT_CONTROL); +} + +static void gt_resume(struct clocksource *cs) +{ + /* enables timer on all the cores */ + writel(gt_control & GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL); +} + static struct clocksource gt_clocksource = { .name = "arm_global_timer", .rating = 300, .read = gt_clocksource_read, .mask = CLOCKSOURCE_MASK(64), .flags = CLOCK_SOURCE_IS_CONTINUOUS, + .suspend = gt_suspend, + .resume = gt_resume, }; #ifdef CONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK @@ -218,6 +236,9 @@ static void __init gt_clocksource_init(void) /* enables timer on all the cores */ writel(GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL); + if (gt_always_on) + gt_clocksource.flags |= CLOCK_SOURCE_SUSPEND_NONSTOP; + #ifdef CONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK sched_clock_register(gt_sched_clock_read, 64, gt_clk_rate); #endif @@ -289,6 +310,8 @@ static void __init global_timer_of_register(struct device_node *np) goto out_clk; } + gt_always_on = of_property_read_bool(np, "always-on"); + err = request_percpu_irq(gt_ppi, gt_clockevent_interrupt, "gt", gt_evt); if (err) { -- 2.6.3 -- 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] clocksource: arm_global_timer: fix suspend resume
Hi Felipe, On 11/13/2015 08:59 PM, Grygorii Strashko wrote: > On 11/13/2015 08:32 PM, Felipe Balbi wrote: >> Grygorii Strashko <grygorii.stras...@ti.com> writes: >>> On 11/13/2015 08:15 PM, Felipe Balbi wrote: >>>> Grygorii Strashko <grygorii.stras...@ti.com> writes: >>>>> On 11/13/2015 07:40 PM, Felipe Balbi wrote: >>>>>> Grygorii Strashko <grygorii.stras...@ti.com> writes: >>>>>>> On 11/13/2015 06:43 PM, Felipe Balbi wrote: >>>>>>>> GrygoCONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCKrii Strashko >>>>>>>> <grygorii.stras...@ti.com> writes: >>>>>>>>> Now the System stall is observed on TI AM437x based board >>>>>>>>> (am437x-gp-evm) during resuming from System suspend when ARM Global >>>>>>>>> timer is selected as clocksource device - SysRq are working, but >>>>>>>>> nothing else. The reason of stall is that ARM Global timer loses its >>>>>>>>> contexts. >>>>>>>>> >>>>>>>>> The reason of stall is that ARM Global timer loses its contexts during >>>>>>>>> System suspend: >>>>>>>>> GT_CONTROL.TIMER_ENABLE = 0 (unbanked) >>>>>>>>> GT_COUNTERx = 0 >>>>>>>>> >>>>>>>>> Hence, update ARM Global timer driver to reflect above behaviour >>>>>>>>> - save GT_CONTROL.TIMER_ENABLE during suspend and restore on resume; >>>>>>>>> - ensure clocksource and clockevent devices have coresponding flags >>>>>>>>>(CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set >>>>>>>>>depending on presence of "always-on" DT property. >>>>>>>>> >>>>>>>>> CC: Arnd Bergmann <a...@arndb.de> >>>>>>>>> Cc: John Stultz <john.stu...@linaro.org> >>>>>>>>> Cc: Felipe Balbi <ba...@ti.com> >>>>>>>>> Cc: Tony Lindgren <t...@atomide.com> >>>>>>>>> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> >>>>>>>>> --- >>>>>>>>> drivers/clocksource/arm_global_timer.c | 23 >>>>>>>>> +++ >>>>>>>>> 1 file changed, 23 insertions(+) >>>>>>>>> >>>>>>>>> diff --git a/drivers/clocksource/arm_global_timer.c >>>>>>>>> b/drivers/clocksource/arm_global_timer.c >>>>>>>>> index a2cb6fa..1bbaf64 100644 >>>>>>>>> --- a/drivers/clocksource/arm_global_timer.c >>>>>>>>> +++ b/drivers/clocksource/arm_global_timer.c >>>>>>>>> @@ -51,6 +51,8 @@ static void __iomem *gt_base; >>>>>>>>> static unsigned long gt_clk_rate; >>>>>>>>> static int gt_ppi; >>>>>>>>> static struct clock_event_device __percpu *gt_evt; >>>>>>>>> +static bool gt_always_on; >>>>>>>>> +static u32 gt_control; >>>>>>>>> >>>>>>>>> /* >>>>>>>>>* To get the value from the Global Timer Counter register >>>>>>>>> proceed as follows: >>>>>>>>> @@ -168,6 +170,9 @@ static int gt_clockevents_init(struct >>>>>>>>> clock_event_device *clk) >>>>>>>>> { >>>>>>>>> int cpu = smp_processor_id(); >>>>>>>>> >>>>>>>>> + if (!gt_always_on) >>>>>>>>> + clk->features |= CLOCK_EVT_FEAT_C3STOP; >>>>>>>>> + >>>>>>>>> clk->name = "arm_global_timer"; >>>>>>>>> clk->features = CLOCK_EVT_FEAT_PERIODIC | >>>>>>>>> CLOCK_EVT_FEAT_ONESHOT | >>>>>>>>> CLOCK_EVT_FEAT_PERCPU; >>>>>>>>> @@ -195,12 +200,25 @@ static cycle_t gt_clocksource_read(struct >>>>>>>>> clocksource *cs) >>>>>>>>> return gt_counter_read(); >>
Re: [PATCH] clocksource: arm_global_timer: fix suspend resume
On 11/13/2015 07:40 PM, Felipe Balbi wrote: Hi, Grygorii Strashko <grygorii.stras...@ti.com> writes: On 11/13/2015 06:43 PM, Felipe Balbi wrote: Hi, Grygorii Strashko <grygorii.stras...@ti.com> writes: Now the System stall is observed on TI AM437x based board (am437x-gp-evm) during resuming from System suspend when ARM Global timer is selected as clocksource device - SysRq are working, but nothing else. The reason of stall is that ARM Global timer loses its contexts. The reason of stall is that ARM Global timer loses its contexts during System suspend: GT_CONTROL.TIMER_ENABLE = 0 (unbanked) GT_COUNTERx = 0 Hence, update ARM Global timer driver to reflect above behaviour - save GT_CONTROL.TIMER_ENABLE during suspend and restore on resume; - ensure clocksource and clockevent devices have coresponding flags (CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set depending on presence of "always-on" DT property. CC: Arnd Bergmann <a...@arndb.de> Cc: John Stultz <john.stu...@linaro.org> Cc: Felipe Balbi <ba...@ti.com> Cc: Tony Lindgren <t...@atomide.com> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- drivers/clocksource/arm_global_timer.c | 23 +++ 1 file changed, 23 insertions(+) diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c index a2cb6fa..1bbaf64 100644 --- a/drivers/clocksource/arm_global_timer.c +++ b/drivers/clocksource/arm_global_timer.c @@ -51,6 +51,8 @@ static void __iomem *gt_base; static unsigned long gt_clk_rate; static int gt_ppi; static struct clock_event_device __percpu *gt_evt; +static bool gt_always_on; +static u32 gt_control; /* * To get the value from the Global Timer Counter register proceed as follows: @@ -168,6 +170,9 @@ static int gt_clockevents_init(struct clock_event_device *clk) { int cpu = smp_processor_id(); + if (!gt_always_on) + clk->features |= CLOCK_EVT_FEAT_C3STOP; + clk->name = "arm_global_timer"; clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERCPU; @@ -195,12 +200,25 @@ static cycle_t gt_clocksource_read(struct clocksource *cs) return gt_counter_read(); } +static void gt_suspend(struct clocksource *cs) +{ + gt_control = readl(gt_base + GT_CONTROL); +} + +static void gt_resume(struct clocksource *cs) +{ + /* enables timer on all the cores */ + writel(gt_control & GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL); do you really need to save context if all you restore is TIMER_ENABLE bit ? seems like you could skip gt_suspend altogether. Is there really a situation where this driver is running and GT isn't enabled ? Now It's not. It's always enabled. I did it because .suspend() is called for all registered clock sources regardless of their usage. So, potentially in the future, at the moment when .suspend() is called it might be disabled (for example, .enable/disable() callbacks can be added and, if ARM Global timer will not be registered as sched_clock, it will be possible to keep it disabled if not used now). But It's not essentially now - I can update it and drop save restore. Pls, confirm. I think it's best to skip suspend completely. You're not restoring anything you saved during suspend, unless you meant | where you used &. I didn't get it - I'm restoring one bit(0) only. -- regards, -grygorii -- 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] clocksource: arm_global_timer: fix suspend resume
On 11/13/2015 08:15 PM, Felipe Balbi wrote: Hi, Grygorii Strashko <grygorii.stras...@ti.com> writes: On 11/13/2015 07:40 PM, Felipe Balbi wrote: Hi, Grygorii Strashko <grygorii.stras...@ti.com> writes: On 11/13/2015 06:43 PM, Felipe Balbi wrote: Hi, Grygorii Strashko <grygorii.stras...@ti.com> writes: Now the System stall is observed on TI AM437x based board (am437x-gp-evm) during resuming from System suspend when ARM Global timer is selected as clocksource device - SysRq are working, but nothing else. The reason of stall is that ARM Global timer loses its contexts. The reason of stall is that ARM Global timer loses its contexts during System suspend: GT_CONTROL.TIMER_ENABLE = 0 (unbanked) GT_COUNTERx = 0 Hence, update ARM Global timer driver to reflect above behaviour - save GT_CONTROL.TIMER_ENABLE during suspend and restore on resume; - ensure clocksource and clockevent devices have coresponding flags (CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set depending on presence of "always-on" DT property. CC: Arnd Bergmann <a...@arndb.de> Cc: John Stultz <john.stu...@linaro.org> Cc: Felipe Balbi <ba...@ti.com> Cc: Tony Lindgren <t...@atomide.com> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- drivers/clocksource/arm_global_timer.c | 23 +++ 1 file changed, 23 insertions(+) diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c index a2cb6fa..1bbaf64 100644 --- a/drivers/clocksource/arm_global_timer.c +++ b/drivers/clocksource/arm_global_timer.c @@ -51,6 +51,8 @@ static void __iomem *gt_base; static unsigned long gt_clk_rate; static int gt_ppi; static struct clock_event_device __percpu *gt_evt; +static bool gt_always_on; +static u32 gt_control; /* * To get the value from the Global Timer Counter register proceed as follows: @@ -168,6 +170,9 @@ static int gt_clockevents_init(struct clock_event_device *clk) { int cpu = smp_processor_id(); + if (!gt_always_on) + clk->features |= CLOCK_EVT_FEAT_C3STOP; + clk->name = "arm_global_timer"; clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERCPU; @@ -195,12 +200,25 @@ static cycle_t gt_clocksource_read(struct clocksource *cs) return gt_counter_read(); } +static void gt_suspend(struct clocksource *cs) +{ + gt_control = readl(gt_base + GT_CONTROL); +} + +static void gt_resume(struct clocksource *cs) +{ + /* enables timer on all the cores */ + writel(gt_control & GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL); do you really need to save context if all you restore is TIMER_ENABLE bit ? seems like you could skip gt_suspend altogether. Is there really a situation where this driver is running and GT isn't enabled ? Now It's not. It's always enabled. I did it because .suspend() is called for all registered clock sources regardless of their usage. So, potentially in the future, at the moment when .suspend() is called it might be disabled (for example, .enable/disable() callbacks can be added and, if ARM Global timer will not be registered as sched_clock, it will be possible to keep it disabled if not used now). But It's not essentially now - I can update it and drop save restore. Pls, confirm. I think it's best to skip suspend completely. You're not restoring anything you saved during suspend, unless you meant | where you used &. I didn't get it - I'm restoring one bit(0) only. that's the point, if you know you're restoring only that bit. Why save anything at all ? i think there are difference between "restoring" and "re-enabling". "restoring" - assume saving smth.. then restore saving value. I'm saving & restoring one bit here. But I can do just "re-enabling" - writel(GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL); and then I don't need to save anything. It will work with current code. -- regards, -grygorii -- 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] clocksource: arm_global_timer: fix suspend resume
On 11/13/2015 08:32 PM, Felipe Balbi wrote: > > Hi, > > Grygorii Strashko <grygorii.stras...@ti.com> writes: >> On 11/13/2015 08:15 PM, Felipe Balbi wrote: >>> >>> Hi, >>> >>> Grygorii Strashko <grygorii.stras...@ti.com> writes: >>>> On 11/13/2015 07:40 PM, Felipe Balbi wrote: >>>>> >>>>> Hi, >>>>> >>>>> Grygorii Strashko <grygorii.stras...@ti.com> writes: >>>>>> On 11/13/2015 06:43 PM, Felipe Balbi wrote: >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> Grygorii Strashko <grygorii.stras...@ti.com> writes: >>>>>>>> Now the System stall is observed on TI AM437x based board >>>>>>>> (am437x-gp-evm) during resuming from System suspend when ARM Global >>>>>>>> timer is selected as clocksource device - SysRq are working, but >>>>>>>> nothing else. The reason of stall is that ARM Global timer loses its >>>>>>>> contexts. >>>>>>>> >>>>>>>> The reason of stall is that ARM Global timer loses its contexts during >>>>>>>> System suspend: >>>>>>>>GT_CONTROL.TIMER_ENABLE = 0 (unbanked) >>>>>>>>GT_COUNTERx = 0 >>>>>>>> >>>>>>>> Hence, update ARM Global timer driver to reflect above behaviour >>>>>>>> - save GT_CONTROL.TIMER_ENABLE during suspend and restore on resume; >>>>>>>> - ensure clocksource and clockevent devices have coresponding flags >>>>>>>> (CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set >>>>>>>> depending on presence of "always-on" DT property. >>>>>>>> >>>>>>>> CC: Arnd Bergmann <a...@arndb.de> >>>>>>>> Cc: John Stultz <john.stu...@linaro.org> >>>>>>>> Cc: Felipe Balbi <ba...@ti.com> >>>>>>>> Cc: Tony Lindgren <t...@atomide.com> >>>>>>>> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> >>>>>>>> --- >>>>>>>> drivers/clocksource/arm_global_timer.c | 23 >>>>>>>> +++ >>>>>>>> 1 file changed, 23 insertions(+) >>>>>>>> >>>>>>>> diff --git a/drivers/clocksource/arm_global_timer.c >>>>>>>> b/drivers/clocksource/arm_global_timer.c >>>>>>>> index a2cb6fa..1bbaf64 100644 >>>>>>>> --- a/drivers/clocksource/arm_global_timer.c >>>>>>>> +++ b/drivers/clocksource/arm_global_timer.c >>>>>>>> @@ -51,6 +51,8 @@ static void __iomem *gt_base; >>>>>>>> static unsigned long gt_clk_rate; >>>>>>>> static int gt_ppi; >>>>>>>> static struct clock_event_device __percpu *gt_evt; >>>>>>>> +static bool gt_always_on; >>>>>>>> +static u32 gt_control; >>>>>>>> >>>>>>>> /* >>>>>>>> * To get the value from the Global Timer Counter register >>>>>>>> proceed as follows: >>>>>>>> @@ -168,6 +170,9 @@ static int gt_clockevents_init(struct >>>>>>>> clock_event_device *clk) >>>>>>>> { >>>>>>>>int cpu = smp_processor_id(); >>>>>>>> >>>>>>>> + if (!gt_always_on) >>>>>>>> + clk->features |= CLOCK_EVT_FEAT_C3STOP; >>>>>>>> + >>>>>>>>clk->name = "arm_global_timer"; >>>>>>>>clk->features = CLOCK_EVT_FEAT_PERIODIC | >>>>>>>> CLOCK_EVT_FEAT_ONESHOT | >>>>>>>>CLOCK_EVT_FEAT_PERCPU; >>>>>>>> @@ -195,12 +200,25 @@ static cycle_t gt_clocksource_read(struct >>>>>>>> clocksource *cs) >>>>>>>>return gt_counter_read(); >>>>>>>> } >>>>>>>> >>>>>>>> +static void gt_suspend(struct clocksource *cs) >>>>>>>> +{ >>>>>>>> +
Re: [PATCH] PCI: dra7xx: mark dra7xx_pcie_msi irq as IRQF_NO_THREAD
On 11/12/2015 11:19 AM, Sebastian Andrzej Siewior wrote: > On 11/06/2015 08:59 PM, Grygorii Strashko wrote: >> Hi Sebastian, > > Hi Grygorii, > >> - IRQF_NO_THREAD is the first considered option for such kind of issues. >>But: Now in LKML there are ~60 occurrences of IRQF_NO_THREAD - most of >>them are used by Arch code. And It's only used by 6 drivers (drivers/*) >> [Addendum 2]. >>During past year, I've found only two threads related to IRQF_NO_THREAD >>and, in both cases, IRQF_NO_THREAD was added for arch specific IRQs which >>can't be threaded >> (https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-November/122659.html, >>https://lkml.org/lkml/2015/4/21/404). > > That powerpc patch you reference is doing the same thing you are doing > here. Probably. I don't know this hw, so my assumption was based on commits descriptions. > >> - ARM UP system: TI's am437xx SoCs for example. >> Here everything is started from /drivers/irqchip/irq-gic.c -> >> gic_handle_irq() >> > >> GIC IRQ handler gic_handle_irq() may process more than one IRQ without >> leaving HW IRQ mode >> (during my experiments I saw up to 6 IRQs processed in one cycle). > > not only GIC. But then what good does it do if it leaves and returns > immediately back to the routine? > >> As result, It was concluded, that having such current HW/SW and all IRQs >> forced threaded [1], >> it will be potentially possible to predict system behavior, because >> gic_handle_irq() will >> do the same things for most of processed IRQs. >> But, once there will be chained [2] or IRQF_NO_THREAD [3] IRQs - complete >> unpredictability. > > I would not go as far as "complete unpredictability". What you do (or > should do) is testing the system for longer period of time with > different behavior in order to estimate the worst case. > You can't predict the system anyway since it is way too complex. Just > try something that ensures that cyclictest is no longer cache hot and > see what happens then. I understand that. That's the current plan and work is in progress. The nearest target is to get rid of all -RT specific backtracks and ensure TI -RT kernel supports the same functionality as non-RT. next step - try to optimize. > >> So, It was selected as goal to have all PPI IRQs (forced) threaded. And if >> someone >> will require faster IRQ handling - IRQF_NO_THREAD can be always added, but >> it will >> be custom solution then. >> >> I'd be appreciated for your comments - if above problem is not a problem. >> Good - IRQF_NO_THREAD forever! > > Yes, we try to avoid IRQF_NO_THREAD under all circumstances. However it > is required for low-level arch code. This includes basically > everything that is using raw-locks which includes interrupt controller > (the "real" ones like GIC or cascading like MSI or GPIO). > Here it is simple - you have a cascading MSI-interrupt controller and > as such it should be IRQF_NO_THREAD marked. > The latency spikes in worst case are not huge as explained earlier: The > only thing your cascading controller is allowed to do is to mark > interrupt as pending (which is with threaded interrupts just a task > wakeup). > And this is not a -RT only problem: it is broken in vanilla linux with > threaded interrupts as well. > Ok. I've got it. IRQF_NO_THREAD will be a solution for reference code and for issues like this. I understand, that each, -RT based, real solution is unique and need to be specifically tuned, so if someone will have problem with IRQF_NO_THREAD - it can be removed easily and replaced with any sort of custom hacks/improvements. Thanks a lot for your comments. I'll apply your previous comments and re-send. -- regards, -grygorii -- 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: omap2: Kconfig: select TWD and global timer on AM43xx devices
On 11/12/2015 08:06 PM, Felipe Balbi wrote: > Make sure to tell the kernel that AM437x has > TWD and global timers. > > Signed-off-by: Felipe Balbi> --- > > Hi Tony, > > now that all dependencies are in place, we can > finally enable twd and global_timer for AM437x. > I'd appreciated if someone can clarify if all described below is valid. (may be some questions are dummy - sorry). After all last changes related to TI OMAP clock source/clock event/sched clock's devices configuration we will have the following for am437x case: clockevents: "timer1", clockevent_gpt, .rating = 300, CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, ti,timer-alwon, "arm,twd-timer", twd_evt,.rating = 350, CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP "arm_global_timer", gt_evt, rating = 300, CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERCPU clocksources: "jiffies", clocksource_jiffies, rating = 1 if use_gptimer_clksrc "timer2", clocksource_gpt, .rating = 300, CLOCK_SOURCE_IS_CONTINUOUS, |-sched_clock_register(dmtimer_read_sched_clock, 32, clksrc.rate); else "ti,omap-counter32k", ti_32k_timer, .rating = 250, CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP, |-sched_clock_register(omap_32k_read_sched_clock, 32, 32768); "arm,global-timer", gt_clocksource, .rating = 300, CLOCK_SOURCE_IS_CONTINUOUS |-sched_clock_register(gt_sched_clock_read, 64, gt_clk_rate); 1) current clockevent device will be registered during registration of clockevent devices and it will be "arm,twd-timer" - processing sequence follows the way they are listed above. 2) current clocksource will be selected from fs_initcall(clocksource_done_booting); and it will be "arm,global-timer" or "timer2" if use_gptimer_clksrc 3) sched clock: "arm,global-timer" will be selected as sched_clock *always*, because it depend on Makefile order (if someone will decide to sort entries in drivers/clocksource/Makefile then "ti,omap-counter32k" most probably will be always selected as sched clock). Uh.. As for me, "timer1" is selected as clockevent device incorrectly, because it's ti,timer-alwon. I think, "ti,omap-counter32k" will crash if use_gptimer_clksrc == true, because corresponding HWmod will not be enabled (not tested this case). Not sure if "timer2" is selected correctly when both "arm,global-timer" and GP timer registered as clocksources. ? Also, It looks like the way sched clock is selected is a little bit unsafe/unclear (especially taking into account that clocksource dev can be changed through the sysfs and TI multiSoC kernel). What is the policy/right way to configure all this clocks? - should only one clock source be selected for each of them in config file? - how to dial with sched clock if it depends only on Makefile order ? :( - should we convert all of them to modules and load depending on current hw? Could it be reasonable to configure sched clock together with clocksource dev registration/selection? - add read_sched field struct clocksource { u64 (*read_sched_clock)(void); - configure sched clock from clocksource core if read_sched != null Thanks. > arch/arm/mach-omap2/Kconfig | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig > index 5076d3f334d2..bb3daf0fa7f7 100644 > --- a/arch/arm/mach-omap2/Kconfig > +++ b/arch/arm/mach-omap2/Kconfig > @@ -65,6 +65,9 @@ config SOC_AM43XX > select MACH_OMAP_GENERIC > select MIGHT_HAVE_CACHE_L2X0 > select HAVE_ARM_SCU > + select HAVE_ARM_TWD > + select ARM_GLOBAL_TIMER > + select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK > > config SOC_DRA7XX > bool "TI DRA7XX" > -- regards, -grygorii -- 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 debounce time calculation
On 11/12/2015 08:09 PM, Felipe Balbi wrote: Hi, Grygorii Strashko <grygorii.stras...@ti.com> writes: On 11/12/2015 07:50 PM, Felipe Balbi wrote: According to TRM, debounce is measured in periods of the functional clock of the GPIO IP. This means that What TRM? link pls. http://www.ti.com/lit/ug/spruhl7d/spruhl7d.pdf 28.4.1.24 GPIO_DEBOUNCINGTIME Register (offset = 154h) [reset = 0h] The GPIO_DEBOUNCINGTIME register controls debouncing time (the value is global for all ports). The debouncing cell is running with the debouncing clock (32 kHz), this register represents the number of the clock cycle(s) (31 s long) to be used. Debouncing Value in 31 microsecond steps. Debouncing Value = (DEBOUNCETIME + 1) * 31 microseconds. DRA7xx: " 8-bit values specifying the debouncing time. It is n- periods of the muxed clock, which can come from either a true 32k oscillator/pad of from the system clock. It depends on which boot mode is selected. For more information see Chapter 32, Initialization. " See http://www.ti.com/lit/ug/spruhz6d/spruhz6d.pdf 27.4.3 General-Purpose Interface Clock Configuration 27.4.3.1 Clocking This completely unclear. Sry, I think this patch can't be used as is, first of all because of backward compatibility issues. -- regards, -grygorii -- 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 debounce time calculation
On 11/12/2015 07:50 PM, Felipe Balbi wrote: > According to TRM, debounce is measured in periods of > the functional clock of the GPIO IP. This means that What TRM? link pls. http://www.ti.com/lit/ug/spruhl7d/spruhl7d.pdf 28.4.1.24 GPIO_DEBOUNCINGTIME Register (offset = 154h) [reset = 0h] The GPIO_DEBOUNCINGTIME register controls debouncing time (the value is global for all ports). The debouncing cell is running with the debouncing clock (32 kHz), this register represents the number of the clock cycle(s) (31 s long) to be used. Debouncing Value in 31 microsecond steps. Debouncing Value = (DEBOUNCETIME + 1) * 31 microseconds. > we should divide by the rate of functional clock. > > Signed-off-by: Felipe Balbi> --- > drivers/gpio/gpio-omap.c | 24 +--- > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > index 56d2d026e62e..2b29fd195521 100644 > --- a/drivers/gpio/gpio-omap.c > +++ b/drivers/gpio/gpio-omap.c > @@ -217,15 +217,29 @@ static void omap2_set_gpio_debounce(struct gpio_bank > *bank, unsigned offset, > u32 val; > u32 l; > boolenable = !!debounce; > + unsigned long flags; > > if (!bank->dbck_flag) > return; > > if (enable) { > - debounce = DIV_ROUND_UP(debounce, 31) - 1; > + struct clk *clk; > + unsigned long rate; > + > + clk = clk_get(bank->dev, "fck"); > + if (IS_ERR(clk)) { > + dev_err(bank->dev, "can't get clock\n"); > + return; > + } > + > + rate = clk_get_rate(clk); > + clk_put(clk); > + > + debounce = DIV_ROUND_UP(debounce, rate); > debounce &= OMAP4_GPIO_DEBOUNCINGTIME_MASK; > } > > + raw_spin_lock_irqsave(>lock, flags); > l = BIT(offset); > > clk_enable(bank->dbck); > @@ -256,6 +270,7 @@ static void omap2_set_gpio_debounce(struct gpio_bank > *bank, unsigned offset, > bank->context.debounce = debounce; > bank->context.debounce_en = val; > } > + raw_spin_unlock_irqrestore(>lock, flags); > } > > /** > @@ -1002,14 +1017,9 @@ static int omap_gpio_output(struct gpio_chip *chip, > unsigned offset, int value) > static int omap_gpio_debounce(struct gpio_chip *chip, unsigned offset, > unsigned debounce) > { > - struct gpio_bank *bank; > - unsigned long flags; > - > - bank = container_of(chip, struct gpio_bank, chip); > + struct gpio_bank *bank = container_of(chip, struct gpio_bank, chip); > > - raw_spin_lock_irqsave(>lock, flags); > omap2_set_gpio_debounce(bank, offset, debounce); > - raw_spin_unlock_irqrestore(>lock, flags); > > return 0; > } > -- regards, -grygorii -- 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
[4.4-rc][PATCH] genirq / PM: restore system wake up from chained interrupts
Commit e509bd7da149 ("genirq: Allow migration of chained interrupts by installing default action") breaks PCS wake up IRQ behaviour on TI OMAP based platforms (dra7-evm). TI OMAP IRQ wake up configuration: GIC-irqchip->PCM_IRQ |- omap_prcm_register_chain_handler |- PRCM-irqchip -> PRCM_IO_IRQ |- pcs_irq_chain_handler |- pinctrl-irqchip -> PCS_uart1_wakeup_irq This happens because IRQ PM code (irq/pm.c) is expected to ignore chained interrupts by default: static bool suspend_device_irq(struct irq_desc *desc) { if (!desc->action || desc->no_suspend_depth) return false; - it's expected !desc->action = true for chained interrupts; but, after above change, all chained interrupt descriptors will have default action handler installed - chained_action. As result, chained interrupts will be silently disabled during system suspend. Hence, fix it by introducing helper function irq_desc_is_chained() and use it in suspend_device_irq() for chained interrupts identification and skip them, once detected. Cc: Mika Westerberg <mika.westerb...@linux.intel.com> Cc: Tony Lindgren <t...@atomide.com> Fixes: e509bd7da149 ("genirq: Allow migration of chained interrupts..") Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- kernel/irq/internals.h | 5 + kernel/irq/pm.c| 3 ++- kernel/irq/proc.c | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h index 05c2188..fcab63c 100644 --- a/kernel/irq/internals.h +++ b/kernel/irq/internals.h @@ -199,6 +199,11 @@ static inline int irq_desc_get_node(struct irq_desc *desc) return irq_common_data_get_node(>irq_common_data); } +static inline int irq_desc_is_chained(struct irq_desc *desc) +{ + return (desc->action && desc->action == _action); +} + #ifdef CONFIG_PM_SLEEP bool irq_pm_check_wakeup(struct irq_desc *desc); void irq_pm_install_action(struct irq_desc *desc, struct irqaction *action); diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c index e80c440..cea1de0 100644 --- a/kernel/irq/pm.c +++ b/kernel/irq/pm.c @@ -70,7 +70,8 @@ void irq_pm_remove_action(struct irq_desc *desc, struct irqaction *action) static bool suspend_device_irq(struct irq_desc *desc) { - if (!desc->action || desc->no_suspend_depth) + if (!desc->action || irq_desc_is_chained(desc) || + desc->no_suspend_depth) return false; if (irqd_is_wakeup_set(>irq_data)) { diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c index a916cf1..a2c02fd 100644 --- a/kernel/irq/proc.c +++ b/kernel/irq/proc.c @@ -475,7 +475,7 @@ int show_interrupts(struct seq_file *p, void *v) for_each_online_cpu(j) any_count |= kstat_irqs_cpu(i, j); action = desc->action; - if ((!action || action == _action) && !any_count) + if ((!action || irq_desc_is_chained(desc)) && !any_count) goto out; seq_printf(p, "%*d: ", prec, i); -- 2.6.2 -- 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] hw_random: omap3-rom-rng: convert timer to delayed work
On 11/06/2015 12:15 AM, Aaro Koskinen wrote: We cannot put the HW RNG to idle using a timer because we cannot disable clocks from atomic context. Use a delayed work instead. Fixes a warning with CONFIG_DEBUG_MUTEXES on Nokia N900 during boot. Reported-by: Sebastian ReichelSigned-off-by: Aaro Koskinen --- drivers/char/hw_random/omap3-rom-rng.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/char/hw_random/omap3-rom-rng.c b/drivers/char/hw_random/omap3-rom-rng.c index a405cdc..58191c6 100644 --- a/drivers/char/hw_random/omap3-rom-rng.c +++ b/drivers/char/hw_random/omap3-rom-rng.c @@ -29,11 +29,11 @@ /* param1: ptr, param2: count, param3: flag */ static u32 (*omap3_rom_rng_call)(u32, u32, u32); -static struct timer_list idle_timer; +static struct delayed_work idle_work; static int rng_idle; static struct clk *rng_clk; -static void omap3_rom_rng_idle(unsigned long data) +static void omap3_rom_rng_idle(struct work_struct *work) { int r; @@ -51,7 +51,7 @@ static int omap3_rom_rng_get_random(void *buf, unsigned int count) u32 r; u32 ptr; - del_timer_sync(_timer); + cancel_delayed_work_sync(_work); if (rng_idle) { clk_prepare_enable(rng_clk); r = omap3_rom_rng_call(0, 0, RNG_GEN_PRNG_HW_INIT); @@ -65,7 +65,7 @@ static int omap3_rom_rng_get_random(void *buf, unsigned int count) ptr = virt_to_phys(buf); r = omap3_rom_rng_call(ptr, count, RNG_GEN_HW); - mod_timer(_timer, jiffies + msecs_to_jiffies(500)); + schedule_delayed_work(_work, msecs_to_jiffies(500)); if (r != 0) return -EINVAL; return 0; @@ -102,7 +102,7 @@ static int omap3_rom_rng_probe(struct platform_device *pdev) return -EINVAL; } - setup_timer(_timer, omap3_rom_rng_idle, 0); + INIT_DELAYED_WORK(_work, omap3_rom_rng_idle); rng_clk = devm_clk_get(>dev, "ick"); if (IS_ERR(rng_clk)) { pr_err("unable to get RNG clock\n"); @@ -118,6 +118,7 @@ static int omap3_rom_rng_probe(struct platform_device *pdev) static int omap3_rom_rng_remove(struct platform_device *pdev) { + cancel_delayed_work_sync(_work); hwrng_unregister(_rom_rng_ops); clk_disable_unprepare(rng_clk); return 0; Sry, It looks like PM runtime autosuspend might work well here. What do you think? -- regards, -grygorii -- 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] PCI: dra7xx: mark dra7xx_pcie_msi irq as IRQF_NO_THREAD
Hi Sebastian, On 11/06/2015 10:53 AM, Sebastian Andrzej Siewior wrote: > On 11/05/2015 07:43 PM, Grygorii Strashko wrote: >> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c >> index 6589e7e..31460f4 100644 >> --- a/drivers/pci/host/pci-dra7xx.c >> +++ b/drivers/pci/host/pci-dra7xx.c >> @@ -302,8 +302,30 @@ static int __init dra7xx_add_pcie_port(struct >> dra7xx_pcie *dra7xx, >> return -EINVAL; >> } >> >> +/* >> + * Mark dra7xx_pcie_msi IRQ as IRQF_NO_THREAD >> + * On -RT and if kernel is booting with "threadirqs" cmd line parameter >> + * the dra7xx_pcie_msi_irq_handler() will be forced threaded but, >> + * in the same time, it's IRQ dispatcher and calls generic_handle_irq(), >> + * which, in turn, will be resolved to handle_simple_irq() call. >> + * The handle_simple_irq() expected to be called with IRQ disabled, as >> + * result kernle will display warning: >> + * "irq XXX handler YYY+0x0/0x14 enabled interrupts". >> + * >> + * Current DRA7 PCIe hw configuration supports 32 interrupts, >> + * which cannot change because it's hardwired in silicon, and can assume >> + * that only a few (most of the time it will be exactly ONE) of those >> + * interrupts are pending at the same time. So, It's sane way to dial >> + * with above issue by marking dra7xx_pcie_msi IRQ as IRQF_NO_THREAD. I can remove below text. >> + * if some platform will utilize a lot of MSI IRQs and will suffer >> + * form -RT latencies degradation because of that - IRQF_NO_THREAD can >> + * be removed and "Warning Annihilation" W/A can be applied [1] >> + * >> + * [1] https://lkml.org/lkml/2015/11/2/578 > > I don't think this novel is required. Also a reference to a patch which > was not accepted is not a smart idea (since people might think they > will improve their -RT latency by applying it without thinking). Agree. Honestly, I don't like both these "solution"/W/A... :( but have nothing better in mind. > > Do you have any *real* numbers where you can say this patch does > better/worse than what you suggester earlier? I'm simply asking because > each gpio-irq multiplexing driver does the same thing. > Indeed. And not only gpio :( Below you can find list of files which contain "generic_handle_irq" and do not contain "irq_set_chained_handler" (of course this list is inaccurate). [Addendum 1] Regarding *real* numbers, I've tested it only on TI dra7-evm + pcie USB3 expander card which generates legacy IRQ every 1 ms and didn't observe latency changes. I'm not sure that I'd be able to test any scenario soon (next week), which will be close to the potential "worst" case. Plus, I found acceptable assumption, that only few pci irqs might be pending at the same time (at least for the reference Kernel code). That's why I gave up without fight :( Seems, I need explain why I'm trying to avoid using IRQF_NO_THREAD by all means (I'm sorry, if below may sound dummy, and may be I'm overestimating/demonizing problem - my -RT experience is not really good [yet:)]). - IRQF_NO_THREAD is the first considered option for such kind of issues. But: Now in LKML there are ~60 occurrences of IRQF_NO_THREAD - most of them are used by Arch code. And It's only used by 6 drivers (drivers/*) [Addendum 2]. During past year, I've found only two threads related to IRQF_NO_THREAD and, in both cases, IRQF_NO_THREAD was added for arch specific IRQs which can't be threaded (https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-November/122659.html, https://lkml.org/lkml/2015/4/21/404). - ARM UP system: TI's am437xx SoCs for example. Here everything is started from /drivers/irqchip/irq-gic.c -> gic_handle_irq() static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs) { u32 irqstat, irqnr; struct gic_chip_data *gic = _data[0]; void __iomem *cpu_base = gic_data_cpu_base(gic); do { irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK); irqnr = irqstat & GICC_IAR_INT_ID_MASK; if (likely(irqnr > 15 && irqnr < 1021)) { if (static_key_true(_deactivate)) writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI); handle_domain_irq(gic->domain, irqnr, regs); |- struct pt_regs *old_regs = set_irq_regs(regs); * |- irq_enter(); |- generic_handle_irq(irq); [1]//*** -RT: all IRQ are threaded (except SPI, timers,..) *** |- handle_f
[PATCH] ARM: omap2plus_defconfig: enable REGULATOR_FIXED_VOLTAGE
Enable REGULATOR_FIXED_VOLTAGE otherwise system can't boot from SD-card when kernel is buit with CONFIG_SOC_DRA7XX=y only. It's also required for almost all other TI SoC's platforms. Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- arch/arm/configs/omap2plus_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig index 3f15a5c..6b4be64 100644 --- a/arch/arm/configs/omap2plus_defconfig +++ b/arch/arm/configs/omap2plus_defconfig @@ -286,6 +286,7 @@ CONFIG_REGULATOR_TPS65217=y CONFIG_REGULATOR_TPS65218=y CONFIG_REGULATOR_TPS65910=y CONFIG_REGULATOR_TWL4030=y +CONFIG_REGULATOR_FIXED_VOLTAGE=y CONFIG_FB=y CONFIG_FIRMWARE_EDID=y CONFIG_FB_MODE_HELPERS=y -- 2.6.2 -- 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] PCI: dra7xx: mark dra7xx_pcie_msi irq as IRQF_NO_THREAD
On -RT, TI DRA7 PCIe driver always produces below backtrace when the first PCI interrupt is triggered: [ cut here ] WARNING: CPU: 1 PID: 82 at kernel/irq/handle.c:150 handle_irq_event_percpu+0x14c/0x174() irq 460 handler irq_default_primary_handler+0x0/0x14 enabled interrupts Modules linked in: ahci_platform(+) ti_vip(+) libahci_platform xhci_pci(+) c_can_platform(+) libahci xhci_hcd ti_vpe c_can libata v4l2_mem2mem can_dev gpio_keys leds_gpio snd_soc_simple_card usbcore spi_ti_qspi videobuf2_core extcon_usb_gpio omap_wdt ti_vpdma videobuf2_dma_contig ti_soc_thermal videobuf2_memops dwc3_omap extcon rtc_omap ov1063x v4l2_common videodev media snd_soc_tlv320aic3x omap_rng rng_core omap_remoteproc CPU: 1 PID: 82 Comm: irq/26-dra7-pci Not tainted 4.1.10-rt10-02638-g6486d7e-dirty #1 Hardware name: Generic DRA74X (Flattened Device Tree) Backtrace: [] (dump_backtrace) from [] (show_stack+0x18/0x1c) r7:c07acca0 r6: r5:c09034e4 r4: [] (show_stack) from [] (dump_stack+0x90/0xac) [] (dump_stack) from [] (warn_slowpath_common+0x7c/0xb8) r7:c07acca0 r6:0096 r5:0009 r4:ee4d3e38 [] (warn_slowpath_common) from [] (warn_slowpath_fmt+0x38/0x40) r8:ee3fcc00 r7:01cc r6: r5:0002 r4:c07acc78 [] (warn_slowpath_fmt) from [] (handle_irq_event_percpu+0x14c/0x174) r3:01cc r2:c07acc78 r4:ecfcdd80 [] (handle_irq_event_percpu) from [] (handle_irq_event+0x84/0xb8) r10:0001 r9:ee4b59c0 r8:ee1d0900 r7:0001 r6: r5:ecfcdd80 r4:ee3fcc00 [] (handle_irq_event) from [] (handle_simple_irq+0x90/0x118) r5:ee4a9020 r4:ee3fcc00 [] (handle_simple_irq) from [] (generic_handle_irq+0x30/0x44) r5:ee4a9020 r4:01cc [] (generic_handle_irq) from [] (dra7xx_pcie_msi_irq_handler+0x7c/0x8c) r5:ee4a9020 r4:0001 [] (dra7xx_pcie_msi_irq_handler) from [] (irq_forced_thread_fn+0x28/0x5c) r5:ee1d0900 r4:ee4b59c0 [] (irq_forced_thread_fn) from [] (irq_thread+0x128/0x204) r7:0001 r6: r5:ee4d2000 r4:ee4b59e4 [] (irq_thread) from [] (kthread+0xd4/0xec) r10: r9: r8: r7:c00870b4 r6:ee4b59c0 r5:ee4b5a00 r4: [] (kthread) from [] (ret_from_fork+0x14/0x2c) r7: r6: r5:c005e228 r4:ee4b5a00 ---[ end trace 0002 ]--- This backtrace is triggered because dra7xx_pcie_msi_irq_handler() forced to be threaded by default on -RT but, in the same time, it's IRQ dispatcher and calls generic_handle_irq(), which leads to handle_simple_irq() call. The handle_simple_irq() expected to be called with IRQ disabled. The same issue will also happen if kernel will boot with "threadirqs" cmdline parameter (CONFIG_IRQ_FORCED_THREADING). As discussed in [1], the current DRA7 PCIe hw configuration supports 32 interrupts, which cannot change because it's hardwired in silicon, this is a single status read and assuming that only a few (most of the time it will be exactly ONE) of those interrupts are pending at the same time is pretty much a sane assumption. And recommended fix for this issue is to request dra7xx_pcie_msi IRQ with IRQF_NO_THREAD flag. [1] https://lkml.org/lkml/2015/11/3/660 Cc: Thomas Gleixner <t...@linutronix.de> Cc: Sebastian Andrzej Siewior <bige...@linutronix.de> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- drivers/pci/host/pci-dra7xx.c | 24 +++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c index 6589e7e..31460f4 100644 --- a/drivers/pci/host/pci-dra7xx.c +++ b/drivers/pci/host/pci-dra7xx.c @@ -302,8 +302,30 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx, return -EINVAL; } + /* +* Mark dra7xx_pcie_msi IRQ as IRQF_NO_THREAD +* On -RT and if kernel is booting with "threadirqs" cmd line parameter +* the dra7xx_pcie_msi_irq_handler() will be forced threaded but, +* in the same time, it's IRQ dispatcher and calls generic_handle_irq(), +* which, in turn, will be resolved to handle_simple_irq() call. +* The handle_simple_irq() expected to be called with IRQ disabled, as +* result kernle will display warning: +* "irq XXX handler YYY+0x0/0x14 enabled interrupts". +* +* Current DRA7 PCIe hw configuration supports 32 interrupts, +* which cannot change because it's hardwired in silicon, and can assume +* that only a few (most of the time it will be exactly ONE) of those +* interrupts are pending at the same time. So, It's sane way to dial +* with above issue by marking dra7xx_pcie_msi IRQ as IRQF_NO_THREAD. +* if some platform will utilize a lot of MSI IRQs and will suffer +* form -RT latencies degradation because of that - IRQF_NO_THREAD can +* be removed and "Warning Annihilation" W/A can be applied [1] +* +* [1] h
Re: [PATCH] spi: ti-qspi: improve ->remove() callback
On 11/02/2015 05:20 PM, Felipe Balbi wrote: > Grygorii Strashko <grygorii.stras...@ti.com> writes: > >> On 10/29/2015 03:57 PM, Felipe Balbi wrote: >>> there's no need to call pm_runtime_get_sync() >>> followed by pm_runtime_put(). We should, instead, >>> just call pm_runtime_put_sync() and pm_runtime_disable(). >> >> Sry, but why do we need to call pm_runtime_put[_sync]() here? >> >> My be just pm_runtime_disable() will be ok? > > and disable with unbalanced pm_runtime_get() ? > Which one is unbalanced pm_runtime_get()? There are no pm_runtime_get() in probe, so there you are going to introduce unbalanced pm_runtime_put_sync() actually :( -- regards, -grygorii -- 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] spi: ti-qspi: improve ->remove() callback
On 10/29/2015 03:57 PM, Felipe Balbi wrote: there's no need to call pm_runtime_get_sync() followed by pm_runtime_put(). We should, instead, just call pm_runtime_put_sync() and pm_runtime_disable(). Sry, but why do we need to call pm_runtime_put[_sync]() here? My be just pm_runtime_disable() will be ok? Signed-off-by: Felipe Balbi--- drivers/spi/spi-ti-qspi.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c index 69c1a95b0615..64318fcfacf2 100644 --- a/drivers/spi/spi-ti-qspi.c +++ b/drivers/spi/spi-ti-qspi.c @@ -554,16 +554,7 @@ free_master: static int ti_qspi_remove(struct platform_device *pdev) { - struct ti_qspi *qspi = platform_get_drvdata(pdev); - int ret; - - ret = pm_runtime_get_sync(qspi->dev); - if (ret < 0) { - dev_err(qspi->dev, "pm_runtime_get_sync() failed\n"); - return ret; - } - - pm_runtime_put(qspi->dev); + pm_runtime_put_sync(>dev); pm_runtime_disable(>dev); return 0; -- regards, -grygorii -- 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] spi: ti-qspi: improve ->remove() callback
On 11/02/2015 06:06 PM, Felipe Balbi wrote: > > hi, > > Grygorii Strashko <grygorii.stras...@ti.com> writes: >> On 11/02/2015 05:20 PM, Felipe Balbi wrote: >>> Grygorii Strashko <grygorii.stras...@ti.com> writes: >>> >>>> On 10/29/2015 03:57 PM, Felipe Balbi wrote: >>>>> there's no need to call pm_runtime_get_sync() >>>>> followed by pm_runtime_put(). We should, instead, >>>>> just call pm_runtime_put_sync() and pm_runtime_disable(). >>>> >>>> Sry, but why do we need to call pm_runtime_put[_sync]() here? >>>> >>>> My be just pm_runtime_disable() will be ok? >>> >>> and disable with unbalanced pm_runtime_get() ? >>> >> >> Which one is unbalanced pm_runtime_get()? >> There are no pm_runtime_get() in probe, so there you are >> going to introduce unbalanced pm_runtime_put_sync() actually :( > > look at ti_qspi_setup(). I _do_ see, however, that it calls > pm_runtime_put_autosuspend() in the same function; what happens if > driver is removed after ti_qspi_setup() runs but before > put_autosuspend() has time to actually run ? > Seems nothing :) If I understand code in __device_release_driver() right: the .remove() callback will be called after pm_runtime_put_sync() and device should be disabled at this moment. Also, note that ti_qspi_setup() will be called for each SPI device attached to SPI master and further PM management of SPI master is performed by SPI core from __spi_pump_messages(). -- regards, -grygorii -- 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: Question about TI Shared Transport
On 10/26/2015 03:58 PM, Adam Ford wrote: Gigi, I am working on modifying the DTS and DTSI files for the LogicPD Torpedo which Tony added in Kernel 4.2 There was a flurry of activity earlier this year regarding device tree bindings for the shared transport and using the Bluetooth with it as seen here: https://patchwork.ozlabs.org/patch/464777/ I was able to get the Logic PD device to work with Bluetooth and the Shared Transport using the instructions found http://processors.wiki.ti.com/index.php/Shared_Transport_Driver#Device_Tree_kernel When I attempted to submit patches for the btwilink, the check script noted that the documentation for btwlink and shared transport bindings were missing. It seems like there might be a new way to attach the BTWILINK to the UART, but I wasn't sure if there was a better solution available yet. I also noticed that using the shared transport mechanism in 4.2.x worked, but trying the same method in 4.3 resulted in a segmentation fault. Does anyone have any updated instructions for the proper way to use the BTWILINK? It is not worked because OF support was reverted: c0bd1b9 Revert "ti-st: add device tree support" -- regards, -grygorii -- 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: MUSB peripheral DMA regression caused by driver core runtime PM change
On 10/23/2015 07:48 PM, Felipe Balbi wrote: > > Hi, > > Tony Lindgren <t...@atomide.com> writes: >> From: Tony Lindgren <t...@atomide.com> >> Date: Fri, 23 Oct 2015 09:03:22 -0700 >> Subject: [PATCH] usb: musb: omap2430: Fix regression caused by driver core >> change >> >> Commit ddef08dd00f5 ("Driver core: wakeup the parent device before trying >> probe") started automatically ensuring the parent device is enabled when >> the child gets probed. >> >> This however caused a regression for MUSB omap2430 interface as the >> runtime PM for the parent device needs the child initialized to access >> the MUSB hardware registers. >> >> Let's delay the enabling of PM runtime for the parent until the child >> has been properly initialized as suggested in an earlier patch by >> Grygorii Strashko <grygorii.stras...@ti.com>. >> >> In addition to delaying pm_runtime_enable, we now also need to make sure >> the parent is enabled during omap2430_musb_init. We also want to propagate >> an error from omap2430_runtime_resume if struct musb is not initialized. >> >> Note that we use pm_runtime_put_noidle here for both the child and parent >> to prevent an extra runtime_suspend/resume cycle. >> >> Let's also add some comments to avoid confusion between the >> two different devices. >> >> Fixes: ddef08dd00f5 ("Driver core: wakeup the parent device before >> trying probe") >> Suggested-by: Grygorii Strashko <grygorii.stras...@ti.com> >> Signed-off-by: Tony Lindgren <t...@atomide.com> > > I'm fine with this patch to fix this v4.3 regression. Greg, do you want > a pull request or can you take this in as a patch ? In any case: > > Acked-by: Felipe Balbi <ba...@ti.com> Reviewed-by: Grygorii Strashko <grygorii.stras...@ti.com> It always fun when DD/PM core is updated to fix some driver/subsystem's specific PM issue :( > >> --- a/drivers/usb/musb/omap2430.c >> +++ b/drivers/usb/musb/omap2430.c >> @@ -391,9 +391,20 @@ static int omap2430_musb_init(struct musb *musb) >> } >> musb->isr = omap2430_musb_interrupt; >> >> +/* >> + * Enable runtime PM for musb parent (this driver). We can't >> + * do it earlier as struct musb is not yet allocated and we >> + * need to touch the musb registers for runtime PM. >> + */ >> +pm_runtime_enable(glue->dev); >> +status = pm_runtime_get_sync(glue->dev); >> +if (status < 0) >> +goto err1; >> + >> status = pm_runtime_get_sync(dev); Hm. My assumption was that *Parent* device (omap2430) will be enabled here :( But as I can see this will not happen: static int rpm_resume(struct device *dev, int rpmflags) {... if (!parent && dev->parent) { /* * Increment the parent's usage counter and resume it if * necessary. Not needed if dev is irq-safe; then the * parent is permanently resumed. */ parent = dev->parent; if (dev->power.irq_safe) goto skip_parent; ^^^ and musb device is irq_safe :( >> if (status < 0) { >> dev_err(dev, "pm_runtime_get_sync FAILED %d\n", status); >> +pm_runtime_put_sync(glue->dev); >> goto err1; >> } >> >> @@ -426,6 +437,7 @@ static int omap2430_musb_init(struct musb *musb) >> phy_power_on(musb->phy); >> >> pm_runtime_put_noidle(musb->controller); >> +pm_runtime_put_noidle(glue->dev); >> return 0; >> >> err1: >> @@ -626,7 +638,11 @@ static int omap2430_probe(struct platform_device *pdev) >> goto err2; >> } >> >> -pm_runtime_enable(>dev); >> +/* >> + * Note that we cannot enable PM runtime yet for this >> + * driver as we need struct musb initialized first. >> + * See omap2430_musb_init above. >> + */ >> >> ret = platform_device_add(musb); >> if (ret) { >> @@ -675,11 +691,12 @@ static int omap2430_runtime_resume(struct device *dev) >> struct omap2430_glue*glue = dev_get_drvdata(dev); >> struct musb *musb = glue_to_musb(glue); >> >> -if (musb) { >> -omap2430_low_level_init(musb); >> -musb_writel(musb->mregs, OTG_INTERFSEL, >> -musb->context.otg_interfsel); >> -} >> +if (!musb) >> +return -EPROBE_DEFER; >> + >> +omap2430_low_level_init(musb); >> +musb_writel(musb->mregs, OTG_INTERFSEL, >> +musb->context.otg_interfsel); >> >> return 0; >> } > -- regards, -grygorii -- 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: MUSB peripheral DMA regression caused by driver core runtime PM change
On 10/23/2015 02:01 AM, Tony Lindgren wrote: * Tony Lindgren[151022 11:03]: * Tony Lindgren [151021 16:44]: Hi all, I noticed a regresssino in v4.3-rc series to day with MUSB gadgets and DMA. Doing a git bisect between v4.2..v4.3-rc1 on it pointed to: ddef08dd00f5 ("Driver core: wakeup the parent device before trying probe") With the commit above reverted things work fine with DMA and USB gadgets. This is on omap3 with CONFIG_USB_INVENTRA_DMA selected. Selecting CONFIG_MUSB_PIO_ONLY still works even without reverting ddef08dd00f5. Anybody got ideas what might be wrong? Some wrong runtime PM usage under drivers/usb/musb? Here's some more debug info on where things are different initializing the USB gadgets. I added some printks and diffed the dmesg output. The added calls from commit ddef08dd00f5 start with dd: Well turns out the problem actually happens earlier. We end up calling omap2430_runtime_resume with NULL struct musb while EPROBE_DEFER probing. No ideas yet how it should be fixed though. I'm not sure, but below diff might help diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c index 70f2b8a..aba8ca7 100644 --- a/drivers/usb/musb/omap2430.c +++ b/drivers/usb/musb/omap2430.c @@ -391,6 +391,8 @@ static int omap2430_musb_init(struct musb *musb) } musb->isr = omap2430_musb_interrupt; + pm_runtime_enable(glue->dev); + status = pm_runtime_get_sync(dev); if (status < 0) { dev_err(dev, "pm_runtime_get_sync FAILED %d\n", status); @@ -626,8 +628,6 @@ static int omap2430_probe(struct platform_device *pdev) goto err2; } - pm_runtime_enable(>dev); - ret = platform_device_add(musb); if (ret) { dev_err(>dev, "failed to register musb device\n"); -- regards, -grygorii -- 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] drivers: net: cpsw: use module_platform_driver
There is no reasons to probe cpsw from late_initcall level and it's not recommended. Hence, use module_platform_driver() to register and probe cpsw driver from module_init() level. Cc: Tony Lindgren <t...@atomide.com> Acked-by: Mugunthan V N <mugunthan...@ti.com> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- drivers/net/ethernet/ti/cpsw.c | 12 +--- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index 8fc90f1..e35a34d 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -2578,17 +2578,7 @@ static struct platform_driver cpsw_driver = { .remove = cpsw_remove, }; -static int __init cpsw_init(void) -{ - return platform_driver_register(_driver); -} -late_initcall(cpsw_init); - -static void __exit cpsw_exit(void) -{ - platform_driver_unregister(_driver); -} -module_exit(cpsw_exit); +module_platform_driver(cpsw_driver); MODULE_LICENSE("GPL"); MODULE_AUTHOR("Cyril Chemparathy <cy...@ti.com>"); -- 2.6.2 -- 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] clk: ti: drop locking code from mux/divider drivers
On 10/01/2015 10:20 PM, Grygorii Strashko wrote: TI's mux and divider clock drivers do not require locking and they do not initialize internal spinlocks. This code was occasionally copy-posted from generic mux/divider drivers. So remove it. Cc: Tony Lindgren <t...@atomide.com> Cc: Sekhar Nori <nsek...@ti.com> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- Gentle ping. drivers/clk/ti/divider.c | 16 +++- drivers/clk/ti/mux.c | 15 +++ 2 files changed, 6 insertions(+), 25 deletions(-) -- regards, -grygorii -- 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
Hi Thomas, On 10/13/2015 09:33 PM, Thomas Gleixner wrote: Grygorii, On Tue, 13 Oct 2015, Grygorii Strashko wrote: I'd very appreciate for any advice of how to better proceed with your request. - I can try to apply and re-send only patches marked by '*' - I can prepare branch with all above patches Please provide a branch on top of 4.1.10 which contains the whole lot. I have a look at it and decide then how to go from there. I've prepared branch linux-4.1.10-ti-gpio: in g...@git.ti.com:~gragst/ti-linux-kernel/gragsts-ti-linux-kernel.git based on top of git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git branch : linux-4.1.y commit : 27f1b7f Linux 4.1.10 Also, I've tried to merge it in linux-4.1.y-rt and found that merge can be done without merge conflicts if below patch is reverted: " Revert "gpio: omap: use raw locks for locking" This reverts commit 26dc4f70eb09ff7d36b1e6cd720e65b3c69098ae " -- regards, -grygorii -- 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
Hi Thomas, On 10/12/2015 10:52 AM, Thomas Gleixner wrote: > Grygorii, > > can you please provide a patch set against 4.1-RT? That stuff rejects > left and right. > This is really not easy thing to do :( and I don't know how to do it the best way. This patches are based on top of big set of other patches. As result we've back-ported mostly all GPIO patches from upstream kernel to TI's 4.1 kernel to keep things consistent and avoid complicated conflicts during back-porting of fixes. The branch linux-4.1.y-rt is continuously merged in TI's 4.1 rt-kernel. So, on top of linux-4.1.y there is below set of patches in TI's kernel now: *bc6b549 gpio: omap: convert to use generic irq handler *d8b79f8 gpio: omap: move pm runtime in irq_chip.irq_bus_lock/sync_unlock c5d9d80 gpio: omap: fix static checker warning 8e3f97d gpio: omap: Fix GPIO numbering for deferred probe a5fafaa gpio: omap: Fix gpiochip_add() handling for deferred probe *a79afac gpio: omap: fix clk_prepare/unprepare usage *e967fb8 gpio: omap: protect regs access in omap_gpio_irq_handler 7f66a45 gpio: omap: fix omap2_set_gpio_debounce 225b622 gpio: omap: switch to use platform_get_irq 4ad92d9 gpio: omap: remove wrong irq_domain_remove usage in probe f76691a gpio: omap: Fix missing raw locks conversion *97e1a2c gpio: omap: use raw locks for locking 7a74d02 ARM: OMAP2: Drop the concept of certain power domains not being able to lose context. 76281a5 gpio: omap: prevent module from being unloaded while in use 7aa88c9 gpio: omap: add missed spin_unlock_irqrestore in omap_gpio_irq_type 79d3bc2 gpio: omap: rework omap_gpio_irq_startup to handle current pin state properly 81dcc40 gpio: omap: rework omap_gpio_request to touch only gpio specific registers e44665c gpio: omap: rework omap_x_irq_shutdown to touch only irqs specific registers 23c487f gpio: omap: fix error handling in omap_gpio_irq_type b328dfc gpio: omap: fix omap_gpio_free to not clean up irq configuration 2966641 gpio: omap: Allow building as a loadable module I'd very appreciate for any advice of how to better proceed with your request. - I can try to apply and re-send only patches marked by '*' - I can prepare branch with all above patches - smth. else -- regards, -grygorii -- 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: AM37x unable to drive output of some gpio lines (works with 2.6.37)
On 10/12/2015 04:12 PM, Ladislav Michl wrote: > On Fri, Oct 09, 2015 at 11:45:17AM +0200, Rolf Peukert wrote: >> On 09.10.2015 02:54, Ladislav Michl wrote: >>> Hi there! >>> >>> on custom AM37x board running 2.6.37 this was enough to enable gpio 67: >>> echo 71 > /sys/class/gpio/export >>> echo out > /sys/class/gpio/gpio71/direction >>> echo 1 > /sys/class/gpio/gpio71/value >>> >>> However, with DT configured linux-4.2 compiled using omap2plus_defconfig >>> this no longer works. >> [...] >> >> Is some other device driver using these pins? Pins 70 and 71 could be >> DSS or UART1. Maybe you need to set them to "disabled" in your DT. > > Only UART2 is enabled. This should not make difference anyway, as it should > be matter of mux config and gpio settings. Working GPIO 82 is on the same > bank as GPIO 71 and to make it work from U-Boot only these registers need > to be touched: > => mw.l 0x480020DC 0x40004 > => mw.l 0x480020F4 0x40004 > => mw.l 0x49052034 0xFFF41F3F > => mw.l 0x4905203C > First two writes are mux config, 3rd is direction and the last one is output. > Both GPIO 71 and 82 could be driven this way. But as long as kernel steps in, > GPIO 71 no longer works even if I use devmem utility to write relevant > registers. Just tried 3.19.8 and it does not work either, moving backward > to the past... > I think You might need to check pin muxing in DT-file. if you have smth like dss_dpi_pins_cm_t35x: pinmux_dss_dpi_pins_cm_t35x { pinctrl-single,pins = < OMAP3_CORE1_IOPAD(0x20dc, PIN_OUTPUT | MUX_MODE0) /* dss_data0.dss_data0 */ OMAP3_CORE1_IOPAD(0x20de, PIN_OUTPUT | MUX_MODE0) /* dss_data1.dss_data1 */ ^ it will not work as gpio. OMAP3_CORE1_IOPAD(0x20e0, PIN_OUTPUT | MUX_MODE0) /* dss_data2.dss_data2 */ OMAP3_CORE1_IOPAD(0x20e2, PIN_OUTPUT | MUX_MODE0) /* dss_data3.dss_data3 */ OMAP3_CORE1_IOPAD(0x20e4, PIN_OUTPUT | MUX_MODE0) /* dss_data4.dss_data4 */ OMAP3_CORE1_IOPAD(0x20e6, PIN_OUTPUT | MUX_MODE0) /* dss_data5.dss_data5 */ >; }; -- regards, -grygorii -- 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] genirq: export handle_bad_irq
Hi Arnd, On 10/06/2015 03:24 PM, Arnd Bergmann wrote: A cleanup of the omap gpio driver introduced a use of the handle_bad_irq() function in a device driver that can be a loadable module. This broke the ARM allmodconfig build: ERROR: "handle_bad_irq" [drivers/gpio/gpio-omap.ko] undefined! This patch exports the handle_bad_irq symbol in order to allow the use in modules. Thanks for fixing it and sorry for the mess. Tested-by: Grygorii Strashko <grygorii.stras...@ti.com> Signed-off-by: Arnd Bergmann <a...@arndb.de> Fixes: 450fa54cfd66 ("gpio: omap: convert to use generic irq handler") diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c index ea7b5fd99ba5..142bbf3b607f 100644 --- a/kernel/irq/handle.c +++ b/kernel/irq/handle.c @@ -35,6 +35,7 @@ void handle_bad_irq(struct irq_desc *desc) kstat_incr_irqs_this_cpu(desc); ack_bad_irq(irq); } +EXPORT_SYMBOL_GPL(handle_bad_irq); /* * Special, empty irq handler: -- regards, -grygorii -- 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 10/02/2015 03:17 PM, Linus Walleij wrote: 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. Thanks. 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. Sure. I'll try to help. -- regards, -grygorii -- 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] clk: ti: drop locking code from mux/divider drivers
TI's mux and divider clock drivers do not require locking and they do not initialize internal spinlocks. This code was occasionally copy-posted from generic mux/divider drivers. So remove it. Cc: Tony Lindgren <t...@atomide.com> Cc: Sekhar Nori <nsek...@ti.com> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- drivers/clk/ti/divider.c | 16 +++- drivers/clk/ti/mux.c | 15 +++ 2 files changed, 6 insertions(+), 25 deletions(-) diff --git a/drivers/clk/ti/divider.c b/drivers/clk/ti/divider.c index 5b17268..df25583 100644 --- a/drivers/clk/ti/divider.c +++ b/drivers/clk/ti/divider.c @@ -214,7 +214,6 @@ static int ti_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate, { struct clk_divider *divider; unsigned int div, value; - unsigned long flags = 0; u32 val; if (!hw || !rate) @@ -228,9 +227,6 @@ static int ti_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate, if (value > div_mask(divider)) value = div_mask(divider); - if (divider->lock) - spin_lock_irqsave(divider->lock, flags); - if (divider->flags & CLK_DIVIDER_HIWORD_MASK) { val = div_mask(divider) << (divider->shift + 16); } else { @@ -240,9 +236,6 @@ static int ti_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate, val |= value << divider->shift; ti_clk_ll_ops->clk_writel(val, divider->reg); - if (divider->lock) - spin_unlock_irqrestore(divider->lock, flags); - return 0; } @@ -256,8 +249,7 @@ static struct clk *_register_divider(struct device *dev, const char *name, const char *parent_name, unsigned long flags, void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags, -const struct clk_div_table *table, -spinlock_t *lock) +const struct clk_div_table *table) { struct clk_divider *div; struct clk *clk; @@ -288,7 +280,6 @@ static struct clk *_register_divider(struct device *dev, const char *name, div->shift = shift; div->width = width; div->flags = clk_divider_flags; - div->lock = lock; div->hw.init = div->table = table; @@ -421,7 +412,7 @@ struct clk *ti_clk_register_divider(struct ti_clk *setup) clk = _register_divider(NULL, setup->name, div->parent, flags, (void __iomem *)reg, div->bit_shift, - width, div_flags, table, NULL); + width, div_flags, table); if (IS_ERR(clk)) kfree(table); @@ -584,8 +575,7 @@ static void __init of_ti_divider_clk_setup(struct device_node *node) goto cleanup; clk = _register_divider(NULL, node->name, parent_name, flags, reg, - shift, width, clk_divider_flags, table, - NULL); + shift, width, clk_divider_flags, table); if (!IS_ERR(clk)) { of_clk_add_provider(node, of_clk_src_simple_get, clk); diff --git a/drivers/clk/ti/mux.c b/drivers/clk/ti/mux.c index 69f08a1..dab9ba8 100644 --- a/drivers/clk/ti/mux.c +++ b/drivers/clk/ti/mux.c @@ -69,7 +69,6 @@ static int ti_clk_mux_set_parent(struct clk_hw *hw, u8 index) { struct clk_mux *mux = to_clk_mux(hw); u32 val; - unsigned long flags = 0; if (mux->table) { index = mux->table[index]; @@ -81,9 +80,6 @@ static int ti_clk_mux_set_parent(struct clk_hw *hw, u8 index) index++; } - if (mux->lock) - spin_lock_irqsave(mux->lock, flags); - if (mux->flags & CLK_MUX_HIWORD_MASK) { val = mux->mask << (mux->shift + 16); } else { @@ -93,9 +89,6 @@ static int ti_clk_mux_set_parent(struct clk_hw *hw, u8 index) val |= index << mux->shift; ti_clk_ll_ops->clk_writel(val, mux->reg); - if (mux->lock) - spin_unlock_irqrestore(mux->lock, flags); - return 0; } @@ -109,7 +102,7 @@ static struct clk *_register_mux(struct device *dev, const char *name, const char **parent_names, u8 num_parents, unsigned long flags, void __iomem *reg, u8 shift, u32 mask, u8 clk_mux_flags, -u32 *table, spinlock_t *lock) +u32 *table) { struct clk_mux *mux; struct clk *clk; @@ -133,7 +126,6 @@ static struct clk *_register_mux(struct device *dev,
[PATCH 1/2] gpio: omap: move pm runtime in irq_chip.irq_bus_lock/sync_unlock
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> --- drivers/gpio/gpio-omap.c | 25 +++-- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 5236db1..a254691 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -496,9 +496,6 @@ static int omap_gpio_irq_type(struct irq_data *d, unsigned type) (type & (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH))) return -EINVAL; - if (!BANK_USED(bank)) - pm_runtime_get_sync(bank->dev); - raw_spin_lock_irqsave(>lock, flags); retval = omap_set_gpio_triggering(bank, offset, type); if (retval) { @@ -521,8 +518,6 @@ static int omap_gpio_irq_type(struct irq_data *d, unsigned type) return 0; error: - if (!BANK_USED(bank)) - pm_runtime_put(bank->dev); return retval; } @@ -797,9 +792,6 @@ static unsigned int omap_gpio_irq_startup(struct irq_data *d) unsigned long flags; unsigned offset = d->hwirq; - if (!BANK_USED(bank)) - pm_runtime_get_sync(bank->dev); - raw_spin_lock_irqsave(>lock, flags); if (!LINE_USED(bank->mod_usage, offset)) @@ -815,8 +807,6 @@ static unsigned int omap_gpio_irq_startup(struct irq_data *d) return 0; err: raw_spin_unlock_irqrestore(>lock, flags); - if (!BANK_USED(bank)) - pm_runtime_put(bank->dev); return -EINVAL; } @@ -835,6 +825,19 @@ static void omap_gpio_irq_shutdown(struct irq_data *d) omap_clear_gpio_debounce(bank, offset); omap_disable_gpio_module(bank, offset); raw_spin_unlock_irqrestore(>lock, flags); +} + +static void omap_gpio_irq_bus_lock(struct irq_data *data) +{ + struct gpio_bank *bank = omap_irq_data_get_bank(data); + + if (!BANK_USED(bank)) + pm_runtime_get_sync(bank->dev); +} + +static void gpio_irq_bus_sync_unlock(struct irq_data *data) +{ + struct gpio_bank *bank = omap_irq_data_get_bank(data); /* * If this is the last IRQ
[PATCH] gpio: omap: fix static checker warning
This patch fixes below static checker warning by changing type of irq field in struct gpio_bank from u16 to int. drivers/gpio/gpio-omap.c:1191 omap_gpio_probe() warn: assigning (-6) to unsigned variable 'bank->irq' drivers/gpio/gpio-omap.c 1188 bank->irq = platform_get_irq(pdev, 0); 1189 if (bank->irq <= 0) { bank->irq is u16. 1190 if (!bank->irq) 1191 bank->irq = -ENXIO; Does not work. 1192 if (bank->irq != -EPROBE_DEFER) Does not work. 1193 dev_err(dev, 1194 "can't get irq resource ret=%d\n", bank->irq); 1195 return bank->irq; 1196 } Fixes: commit 89d18e3af8b9: "gpio: omap: switch to use platform_get_irq" Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com> --- drivers/gpio/gpio-omap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 376827f..56d2d02 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -51,7 +51,7 @@ struct gpio_regs { struct gpio_bank { struct list_head node; void __iomem *base; - u16 irq; + int irq; u32 non_wakeup_gpios; u32 enabled_non_wakeup_gpios; struct gpio_regs context; -- 2.5.1 -- 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 2/2] gpio: omap: convert to use generic irq handler
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> --- drivers/gpio/gpio-omap.c | 55 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index a254691..376827f 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -59,6 +59,7 @@ struct gpio_bank { u32 level_mask; u32 toggle_mask; raw_spinlock_t lock; + raw_spinlock_t wa_lock; struct gpio_chip chip; struct clk *dbck; u32 mod_usage; @@ -649,8 +650,13 @@ static int omap_gpio_wake_enable(struct irq_data *d, unsigned int enable) { struct gpio_bank *bank = omap_irq_data_get_bank(d); unsigned offset = d->hwirq; + int ret; + + ret = omap_set_gpio_wakeup(bank, offset, enable); + if (!ret) + ret = irq_set_irq_wake(bank->irq, enable); - return omap_set_gpio_wakeup(bank, offset, enable); + return ret; } static int omap_gpio_request(struct gpio_chip *chip, unsigned offset) @@ -704,26 +710,21 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset) * line's interrupt handler has been run, we may miss some nested * interrupts. */ -static void omap_gpio_irq_handler(struct irq_desc *desc) +static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) { void __iomem *isr_reg = NULL; u32 isr; unsigned int bit; - struct gpio_bank *bank; - int unmasked = 0; - struct irq_chip *irqchip = irq_desc_get_chip(desc); - struct gpio_chip *chip = irq_desc_get_handler_data(desc); + struct gpio_bank *bank = gpiobank; + unsigned long wa_lock_flags; unsigned long lock_flags; - chained_irq_enter(irqchip, desc); - - bank = container_of(chip, struct gpio_bank, chip); isr_reg = bank->base + bank->regs->irqstatus; - pm_runtime_get_sync(bank->dev); - if (WARN_ON(!isr_reg)) goto exit; + pm_runtime_get_sync(bank->dev); + while (1) { u32 isr_saved, level_mask = 0; u32 enabled; @@ -745,13 +746,6 @@ static void omap_gpio_irq_handler(struct irq_desc *desc) raw_spin_unlock_irqrestore(>lock, lock_flags); - /* if there is only edge sensitive GPIO pin interrupts - configured, we could unmask GPIO bank interrupt immediately */ - if (!level_mask && !unmasked) { - unmasked = 1; - chained_irq_exit(irqchip, desc); - } - if (!isr) break; @@ -772,18 +766,18 @@ static void omap_gpio_irq_handler(struct irq_desc *desc) raw_spin_unlock_irqrestore(>lock, lock_f
[PATCH 0/2] convert to use generic irq handler
This patch series contains patches which fixes wrong APIs usage in atomic context on RT-kernel. The final goal is to make TI OMAP GPIO driver compatible with -RT kernel as much as possible. Patch 1: required to be compatible with -RT kernel, because PM runtime's irq_safe mode is incompatible with -RT. Patch 2: 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. Boot, basic gpio functionality tested on: dra7-evm, BeagleBone(white), am43xx-gpevm, am437x-sk Manually tested on dra7-evm including suspend/resume and wakeup. Links on RFC: https://lkml.org/lkml/2015/8/18/161 https://lkml.org/lkml/2015/8/18/162 Grygorii Strashko (2): gpio: omap: move pm runtime in irq_chip.irq_bus_lock/sync_unlock gpio: omap: convert to use generic irq handler drivers/gpio/gpio-omap.c | 80 +--- 1 file changed, 42 insertions(+), 38 deletions(-) -- 2.5.1 -- 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/3] ARM: dts: dra7-evm: enable leds and gpio-keys support
Hi Tony, On 08/27/2015 08:20 AM, Grygorii Strashko wrote: This series enables support for leds and gpio-keys which is available on dra7-evm. It also adds pcf8575 gpio expander (i2c1 addr 20) Grygorii Strashko (3): ARM: dts: dra7-evm: add pcf8575 gpio expander (i2c1 addr 20) ARM: dts: dra7-evm: add gpio leds support ARM: dts: dra7-evm: add gpio key support arch/arm/boot/dts/dra7-evm.dts | 58 ++ 1 file changed, 58 insertions(+) Are there any comments on this series? Could it be applied? -- regards, -grygorii -- 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 09/24/2015 03:28 PM, Linus Walleij wrote: 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? sure. Thanks. -- regards, -grygorii -- 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
+Cc: Austin, Philipp On 08/26/2015 10:53 AM, Linus Walleij wrote: 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 I've got some feedback here. Patches were tested on to of TI -RT tree ti-rt-linux-4.1.y. Forwarded Message Subject: Re: [PATCH 0/7] gpio: omap: fixes and improvements Date: Fri, 4 Sep 2015 21:42:03 + From: Austin Schuh <aus...@peloton-tech.com> To: grygorii.stras...@ti.com CC: Philipp Schrader <phil...@peloton-tech.com> Hi Grygorii, We are running the entire patchset on top of a7a6541db840cc4514a26fc12cc1bc94d1b9e873 from the git:// git.ti.com/ti-linux-kernel/ti-linux-kernel.git tree. I've pushed the system pretty hard overnight and haven't seen any instability (with lockdep enabled. We used to get splats pretty fast). I wasn't on lkml when you sent the patchset out, so I'm not sure how to best reply to the original thread with this info. Tested-by: Austin Schuh <aus...@peloton-tech.com> Austin Forwarded Message -- regards, -grygorii -- 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 09/03/2015 08:31 PM, Tony Lindgren 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> --- drivers/gpio/gpio-omap.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -1095,7 +1095,6 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) } else { bank->chip.label = "gpio"; bank->chip.base = gpio; - gpio += bank->width; } bank->chip.ngpio = bank->width; @@ -1105,6 +1104,9 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) return ret; } + if (!bank->is_mpuio) + gpio += bank->width; + #ifdef CONFIG_ARCH_OMAP1 /* * REVISIT: Once we have OMAP1 supporting SPARSE_IRQ, we can drop Reviewed-by: Grygorii Strashko <grygorii.stras...@ti.com> With hope that only GPIO0 will be deferred, otherwise there will be total mess in gpios enumeration ;) -- regards, -grygorii -- 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 02/15] mmc: host: omap_hsmmc: return on fatal errors from omap_hsmmc_reg_get
On 09/01/2015 12:14 AM, Tony Lindgren wrote: > * Tony Lindgren[150831 14:02]: >> >> And I must have tested next-20150827 instead of next-20150828. Or >> else it does not happen on every boot. In any case, I'm now getting >> the following on next-20150831 most of the time: >> >> [9.493133] omap_hsmmc 4809c000.mmc: using lookup tables for GPIO lookup >> [9.500274] omap_hsmmc 4809c000.mmc: lookup for GPIO wp failed >> [9.506378] [ cut here ] >> [9.508941] WARNING: CPU: 0 PID: 6 at drivers/bus/omap_l3_noc.c:147 >> l3_interrupt_handler+0x224/0x350() >> [9.520568] 4400.ocp:L3 Custom Error: MASTER MPU TARGET L4PER2 >> (Read): Data Access in User mode during Functional access >> [9.524810] Modules linked in: rtc_twl twl4030_wdt >> [9.534820] CPU: 0 PID: 6 Comm: kworker/u4:0 Not tainted >> 4.2.0-next-20150831-2-gf55bad8 #1113 >> [9.544830] Hardware name: Generic OMAP4 (Flattened Device Tree) >> [9.544830] Workqueue: deferwq deferred_probe_work_func >> [9.554809] [] (unwind_backtrace) from [] >> (show_stack+0x10/0x14) >> [9.564819] [] (show_stack) from [] >> (dump_stack+0x84/0x9c) >> [9.574829] [] (dump_stack) from [] >> (warn_slowpath_common+0x78/0xb4) >> [9.574951] [] (warn_slowpath_common) from [] >> (warn_slowpath_fmt+0x30/0x40) >> [9.584686] [] (warn_slowpath_fmt) from [] >> (l3_interrupt_handler+0x224/0x350) >> [9.594818] [] (l3_interrupt_handler) from [] >> (handle_irq_event_percpu+0x44/0x1f0) >> [9.604827] [] (handle_irq_event_percpu) from [] >> (handle_irq_event+0x40/0x64) >> [9.614807] [] (handle_irq_event) from [] >> (handle_fasteoi_irq+0xcc/0x1c4) >> [9.625396] [] (handle_fasteoi_irq) from [] >> (generic_handle_irq+0x28/0x3c) >> [9.638732] [] (generic_handle_irq) from [] >> (__handle_domain_irq+0x64/0xe0) >> [9.647827] [] (__handle_domain_irq) from [] >> (gic_handle_irq+0x40/0x8c) >> [9.654693] [] (gic_handle_irq) from [] >> (__irq_svc+0x58/0x78) >> [9.664367] Exception stack(0xee0cfd80 to 0xee0cfdc8) >> [9.665130] fd80: ee1ec010 c082f174 00d0 ee6b0800 ee6ae850 >> ee1ec000 ee1ec010 >> [9.674835] fda0: ee6b0cc0 00ee fa09c000 0003 ee0cfdd0 >> c04cd748 c04df4e0 >> [9.684814] fdc0: 2113 >> [9.684814] [] (__irq_svc) from [] >> (devm_clk_get+0x8/0x70) >> [9.694824] [] (devm_clk_get) from [] >> (omap_hsmmc_probe+0x2e8/0x9f0) >> [9.704833] [] (omap_hsmmc_probe) from [] >> (platform_drv_probe+0x44/0xac) >> [9.714691] [] (platform_drv_probe) from [] >> (driver_probe_device+0x1f4/0x2f0) >> [9.724548] [] (driver_probe_device) from [] >> (bus_for_each_drv+0x64/0x98) >> [9.733459] [] (bus_for_each_drv) from [] >> (__device_attach+0xb0/0x118) >> [9.734832] [] (__device_attach) from [] >> (bus_probe_device+0x88/0x90) >> [9.744812] [] (bus_probe_device) from [] >> (deferred_probe_work_func+0x60/0x90) >> [9.760040] [] (deferred_probe_work_func) from [] >> (process_one_work+0x1a4/0x558) >> [9.769470] [] (process_one_work) from [] >> (worker_thread+0x3c/0x514) >> [9.774688] [] (worker_thread) from [] >> (kthread+0xd4/0xf0) >> [9.785614] [] (kthread) from [] >> (ret_from_fork+0x14/0x24) >> [9.785614] ---[ end trace 402743bd8cfdde2f ]--- > > And with the (currently almost useless) l3 interrupt stuff taken out by > removing the ti,omap4-l3-noc compatible from omap4.dtsi, we get a real > trace that might be of some help to you: > > [8.440917] omap_hsmmc 4809c000.mmc: lookup for GPIO wp failed > [8.447418] Unhandled fault: imprecise external abort (0x1406) at > 0xbeafaa10 > [8.454925] pgd = c0004000 > [8.454986] [beafaa10] *pgd=/root/init: line 14: > /sys/devices/6800.ocp/4 > 8098000.spi/spi_master/spi1/spi1[8.461334] Internal error: : 1406 [#1] > SMP ARM > .2/backlight/acx565akm/brightness: No such file [8.473175] Modules linked > in:or directory > rtc_twl twl4030_wdt > [8.483520] CPU: 1 PID: 66 Comm: kworker/u4:1 Not tainted > 4.2.0-next-20150831-2-gf55bad8-dirty #1115 > [8.493652] Hardware name: Generic OMAP4 (Flattened Device Tree) > [8.498352] Workqueue: deferwq deferred_probe_work_func > [8.505493] task: ee5f4f40 ti: ee5f6000 task.ti: ee5f6000 > [8.510803] PC is at devm_clk_get+0x8/0x70 > [8.514801] LR is at omap_hsmmc_probe+0x2e8/0x9f0 > [8.520385] pc : []lr : []psr: 20010013 > [8.520385] sp : ee5f7dd0 ip : 0003 fp : fa09c000 > [8.532470] r10: 00ee r9 : ee6904c0 r8 : > [8.537963] r7 : ee1ec010 r6 : ee1ec000 r5 : ee6e7dd0 r4 : ee69 > [8.544769] r3 : r2 : 00d0 r1 : c082f184 r0 : ee1ec010 > [8.551666] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment > none > [8.559143] Control: 10c5387d Table: ae73804a DAC: 0051 > [8.564727] Process kworker/u4:1 (pid: 66, stack limit = 0xee5f6218)
Re: [PATCH v3 02/15] mmc: host: omap_hsmmc: return on fatal errors from omap_hsmmc_reg_get
On 09/01/2015 05:50 PM, Tony Lindgren wrote: * Grygorii Strashko <grygorii.stras...@ti.com> [150901 07:36]: On 09/01/2015 12:14 AM, Tony Lindgren wrote: * Tony Lindgren <t...@atomide.com> [150831 14:02]: And I must have tested next-20150827 instead of next-20150828. Or else it does not happen on every boot. In any case, I'm now getting the following on next-20150831 most of the time: [9.493133] omap_hsmmc 4809c000.mmc: using lookup tables for GPIO lookup [9.500274] omap_hsmmc 4809c000.mmc: lookup for GPIO wp failed [9.506378] [ cut here ] [9.508941] WARNING: CPU: 0 PID: 6 at drivers/bus/omap_l3_noc.c:147 l3_interrupt_handler+0x224/0x350() [9.520568] 4400.ocp:L3 Custom Error: MASTER MPU TARGET L4PER2 (Read): Data Access in User mode during Functional access [9.524810] Modules linked in: rtc_twl twl4030_wdt [9.534820] CPU: 0 PID: 6 Comm: kworker/u4:0 Not tainted 4.2.0-next-20150831-2-gf55bad8 #1113 [9.544830] Hardware name: Generic OMAP4 (Flattened Device Tree) [9.544830] Workqueue: deferwq deferred_probe_work_func [9.554809] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [9.564819] [] (show_stack) from [] (dump_stack+0x84/0x9c) [9.574829] [] (dump_stack) from [] (warn_slowpath_common+0x78/0xb4) [9.574951] [] (warn_slowpath_common) from [] (warn_slowpath_fmt+0x30/0x40) [9.584686] [] (warn_slowpath_fmt) from [] (l3_interrupt_handler+0x224/0x350) [9.594818] [] (l3_interrupt_handler) from [] (handle_irq_event_percpu+0x44/0x1f0) [9.604827] [] (handle_irq_event_percpu) from [] (handle_irq_event+0x40/0x64) [9.614807] [] (handle_irq_event) from [] (handle_fasteoi_irq+0xcc/0x1c4) [9.625396] [] (handle_fasteoi_irq) from [] (generic_handle_irq+0x28/0x3c) [9.638732] [] (generic_handle_irq) from [] (__handle_domain_irq+0x64/0xe0) [9.647827] [] (__handle_domain_irq) from [] (gic_handle_irq+0x40/0x8c) [9.654693] [] (gic_handle_irq) from [] (__irq_svc+0x58/0x78) [9.664367] Exception stack(0xee0cfd80 to 0xee0cfdc8) [9.665130] fd80: ee1ec010 c082f174 00d0 ee6b0800 ee6ae850 ee1ec000 ee1ec010 [9.674835] fda0: ee6b0cc0 00ee fa09c000 0003 ee0cfdd0 c04cd748 c04df4e0 [9.684814] fdc0: 2113 [9.684814] [] (__irq_svc) from [] (devm_clk_get+0x8/0x70) [9.694824] [] (devm_clk_get) from [] (omap_hsmmc_probe+0x2e8/0x9f0) [9.704833] [] (omap_hsmmc_probe) from [] (platform_drv_probe+0x44/0xac) [9.714691] [] (platform_drv_probe) from [] (driver_probe_device+0x1f4/0x2f0) [9.724548] [] (driver_probe_device) from [] (bus_for_each_drv+0x64/0x98) [9.733459] [] (bus_for_each_drv) from [] (__device_attach+0xb0/0x118) [9.734832] [] (__device_attach) from [] (bus_probe_device+0x88/0x90) [9.744812] [] (bus_probe_device) from [] (deferred_probe_work_func+0x60/0x90) [9.760040] [] (deferred_probe_work_func) from [] (process_one_work+0x1a4/0x558) [9.769470] [] (process_one_work) from [] (worker_thread+0x3c/0x514) [9.774688] [] (worker_thread) from [] (kthread+0xd4/0xf0) [9.785614] [] (kthread) from [] (ret_from_fork+0x14/0x24) [9.785614] ---[ end trace 402743bd8cfdde2f ]--- And with the (currently almost useless) l3 interrupt stuff taken out by removing the ti,omap4-l3-noc compatible from omap4.dtsi, we get a real trace that might be of some help to you: [8.440917] omap_hsmmc 4809c000.mmc: lookup for GPIO wp failed [8.447418] Unhandled fault: imprecise external abort (0x1406) at 0xbeafaa10 [8.454925] pgd = c0004000 [8.454986] [beafaa10] *pgd=/root/init: line 14: /sys/devices/6800.ocp/4 8098000.spi/spi_master/spi1/spi1[8.461334] Internal error: : 1406 [#1] SMP ARM .2/backlight/acx565akm/brightness: No such file [8.473175] Modules linked in:or directory rtc_twl twl4030_wdt [8.483520] CPU: 1 PID: 66 Comm: kworker/u4:1 Not tainted 4.2.0-next-20150831-2-gf55bad8-dirty #1115 [8.493652] Hardware name: Generic OMAP4 (Flattened Device Tree) [8.498352] Workqueue: deferwq deferred_probe_work_func [8.505493] task: ee5f4f40 ti: ee5f6000 task.ti: ee5f6000 [8.510803] PC is at devm_clk_get+0x8/0x70 [8.514801] LR is at omap_hsmmc_probe+0x2e8/0x9f0 [8.520385] pc : []lr : []psr: 20010013 [8.520385] sp : ee5f7dd0 ip : 0003 fp : fa09c000 [8.532470] r10: 00ee r9 : ee6904c0 r8 : [8.537963] r7 : ee1ec010 r6 : ee1ec000 r5 : ee6e7dd0 r4 : ee69 [8.544769] r3 : r2 : 00d0 r1 : c082f184 r0 : ee1ec010 [8.551666] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [8.559143] Control: 10c5387d Table: ae73804a DAC: 0051 [8.564727] Process kworker/u4:1 (pid: 66, stack limit = 0xee5f6218) [8.571441] Stack: (0xee5f7dd0 to 0xee5f8000) [8.576263] 7dc0: ee69 ee6e7dd0 ee1ec000 c04cd748 [8.