Re: [GIT PULL] TI clock driver changes for 3.18 merge window
Quoting Tero Kristo (2014-09-29 09:13:09) Hi Mike, The following changes since commit 7d1311b93e58ed55f3a31cc8f94c4b8fe988a2b9: Linux 3.17-rc1 (2014-08-16 10:40:26 -0600) are available in the git repository at: g...@github.com:t-kristo/linux-pm.git for-v3.18/ti-clk-driver for you to fetch changes up to 04ed831f224d4553682f48e1b4a6b68f2622b68e: clk: ti: dra7-atl-clock: Mark the device as pm_runtime_irq_safe (2014-09-29 11:51:14 +0300) Pulled! Thanks, Mike Mostly fixes, except for the of_clk_init() change for the clock driver. Boot tested on pretty much every board I have access to, also merge tested with latest master to see if there are any conflicts with anything. Behan Webster (1): clk: ti: LLVMLinux: Move __init outside of type definition Peter Ujfalusi (1): clk: ti: dra7-atl-clock: Mark the device as pm_runtime_irq_safe Sebastian Andrzej Siewior (1): clk: ti: consider the fact that of_clk_get() might return an error Tero Kristo (2): clk: ti: change clock init to use generic of_clk_init clk: ti: dra7-atl-clock: fix a memory leak arch/arm/mach-omap2/io.c | 12 +-- arch/arm/mach-omap2/prm_common.c |2 -- drivers/clk/ti/clk-dra7-atl.c|2 ++ drivers/clk/ti/clk.c | 68 -- drivers/clk/ti/clockdomain.c |5 +++ drivers/clk/ti/divider.c |4 +-- include/linux/clk/ti.h |1 + 7 files changed, 63 insertions(+), 31 deletions(-) -- 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: prevent erronous parsing of children during rate change
Quoting Tero Kristo (2014-09-29 01:09:24) On 09/27/2014 02:24 AM, Mike Turquette wrote: Quoting Tero Kristo (2014-09-26 00:18:55) On 09/26/2014 04:35 AM, Stephen Boyd wrote: On 09/23/14 06:38, Tero Kristo wrote: On 09/22/2014 10:18 PM, Stephen Boyd wrote: On 08/21, Tero Kristo wrote: /* Skip children who will be reparented to another clock */ if (child-new_parent child-new_parent != clk) continue; Are we not hitting the new_parent check here? I don't understand how we can be changing parents here unless the check is being avoided, in which case I wonder why determine_rate isn't being used. It depends how the clock underneath handles the situation. The error I am seeing actually happens with a SoC specific compound clock (DPLL) which integrates set_rate + mux functionality into a single clock node. A call to the clk_set_rate changes the parent of this clock (from bypass clock to reference clock), in addition to changing the rate (tune the mul+div.) I looked at using the determine rate call with this type but it breaks everything up... the parent gets changed but not the clock rate, in addition to some other issues. Ok. Is this omap3_noncore_dpll_set_rate()? Yes. Can we use determine_rate + clk_set_parent_and_rate()? At least clk_set_parent_and_rate() would allow us to do the mult+div and the parent in the same op call, although I don't understand why setting the parent and then setting the rate is not going to work. Well, setting parent first, then rate later causes problems with the DPLL ending up running with illegal (non-specified) rate, the M+N values are most likely wrong if you just switch from bypass clock to reference clock first without programming the M+N first. I took a quick look and it still seems to me that the OMAP DPLLs are still not modeled properly as mux clocks. Is this correct? Yeah, they are not mux clocks, but rather a compound of mux + DPLL multiplier/divider logic. Changing the DPLL to be a separate mux + DPLL div/mult clock will still have overlapping usage of the DPLL_EN field, I'm not talking about splitting up the clock into two separate clocks. If memory serves the DPLL clock implementation cheats and hides the bypass_clk info from the clock framework. To be explicit, from the perspective of Linux clock framework DPLL clocks only have one parent. In reality a typical DPLL should have at least 2 parents (and in some cases starting with OMAP4, some of the DPLL output clocks should have a second HSD parent). But the implementation does not reflect this. as the DPLL must be in bypass mode during M+N change. Or, should the DPLL rate change only be allowed if the mux is in bypass setting? Several drivers still depend on direct dpll clk_set_rate working 'properly' (there are some other issues currently present also which have nothing to do with the mux behavior.) This issue has been lingering for a long time and we can't use determine_rate unless that clock has multiple parents. Simply hacking knowledge of the parent bypass clock into the .set_rate callback is not enough. If you believe this _must_ be changed, I can take a look at this for next merge window, but this will cause a DT data compatibility break if nothing else (personally I don't care about this as I always rebuild DT blob with kernel, but lots of other people seem to do.) Well I guess the question is how long will we put up with the many small headaches caused by incorrectly modeling the clock? determine_rate and clk_set_parent_and_rate should be sufficient for the OMAP DPLLs but only if they are correctly modeled in the framework. Regards, Mike -Tero Regards, Mike I'm interested in the other issues that you mentioned too. Mostly these were side-effects from the illegal DPLL setup I guess, like boot hang, failed drivers etc. I didn't really investigate this that much as it is much more simpler just to use safe list iteration here. -Tero -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx
On Mon, Sep 29, 2014 at 12:30:43PM +0200, Frans Klaver wrote: On Mon, Sep 29, 2014 at 11:54:40AM +0200, Sebastian Andrzej Siewior wrote: For your too much work for irq problem: Could you add trace_printk() in tx/rx dma start/complete, and irq routine? The interresting part is what is the irq routine doing once entered. It might be a condition that is ignored at first and acked later while serving another event. Or it is really doing something and this is more or less legal. Here's some trace output I get. I hope branches become clear by the calls they do. uart_test-482 [000] .ns.17.860139: __dma_rx_do_complete: error: 0, uart_bug_dma: 32 uart_test-482 [000] .ns.17.860141: __dma_rx_do_complete: rx_dma(p, 0) uart_test-480 [000] ..s.17.860260: __dma_rx_do_complete: error: 0, uart_bug_dma: 32 uart_test-480 [000] ..s.17.860263: __dma_rx_do_complete: rx_dma(p, 0) uart_test-478 [000] ..s.17.860369: __dma_rx_do_complete: error: 0, uart_bug_dma: 32 uart_test-478 [000] ..s.17.860372: __dma_rx_do_complete: rx_dma(p, 0) kworker/0:1-10[000] ..s.17.860508: __dma_rx_do_complete: error: 0, uart_bug_dma: 32 kworker/0:1-10[000] ..s.17.860512: __dma_rx_do_complete: rx_dma(p, 0) uart_test-477 [000] ..s.17.860634: __dma_rx_do_complete: error: 0, uart_bug_dma: 32 uart_test-477 [000] ..s.17.860642: __dma_rx_do_complete: rx_dma(p, 0) uart_test-477 [000] ..s.17.860768: __dma_rx_do_complete: error: 0, uart_bug_dma: 32 uart_test-477 [000] ..s.17.860772: __dma_rx_do_complete: rx_dma(p, 0) kworker/0:1-10[000] ..s.17.860900: __dma_rx_do_complete: error: 0, uart_bug_dma: 32 kworker/0:1-10[000] ..s.17.860904: __dma_rx_do_complete: rx_dma(p, 0) uart_test-483 [000] dnh.17.861018: serial8250_interrupt: irq 89 uart_test-483 [000] dnh.17.861026: serial8250_interrupt: 89 e uart_test-483 [000] .ns.17.861045: __dma_rx_do_complete: error: 0, uart_bug_dma: 32 uart_test-483 [000] .ns.17.861047: __dma_rx_do_complete: rx_dma(p, 0) uart_test-479 [000] d.H.17.861124: serial8250_interrupt: irq 89 uart_test-479 [000] d.H.17.861133: serial8250_handle_irq: l1 IIR cc LSR 61 uart_test-479 [000] d.H.17.861135: serial8250_handle_irq: rx_dma(up, iir) uart_test-479 [000] d.H.17.861139: serial8250_rx_dma: l1, UART_IIR_RX_TIMEOUT uart_test-479 [000] d.H.17.861147: __dma_rx_do_complete: error: 1, uart_bug_dma: 32 uart_test-479 [000] d.H.17.861150: serial8250_handle_irq: rx_chars(up, status) uart_test-479 [000] d.H.17.861190: serial8250_handle_irq: rx_dma(up, 0) uart_test-479 [000] d.H.17.861205: serial8250_interrupt: 89 e kworker/0:1-10[000] dnH.17.864949: serial8250_interrupt: irq 89 So far so good. We're just entered interrupt where stuff goes awry. The following pattern repeats over 600 times: kworker/0:1-10[000] dnH.17.865198: serial8250_handle_irq: l1 IIR cc LSR 61 kworker/0:1-10[000] dnH.17.865254: serial8250_handle_irq: rx_dma(up, iir) kworker/0:1-10[000] dnH.17.865333: serial8250_rx_dma: l1, UART_IIR_RX_TIMEOUT kworker/0:1-10[000] dnH.17.865626: __dma_rx_do_complete: error: 1, uart_bug_dma: 32 kworker/0:1-10[000] dnH.17.865747: serial8250_handle_irq: rx_chars(up, status) kworker/0:1-10[000] dnH.17.868797: serial8250_handle_irq: rx_dma(up, 0) ending with: kworker/0:1-10[000] dnH.20.027093: serial8250_interrupt: serial8250: too much work for irq89 kworker/0:1-10[000] dnH.20.027181: serial8250_interrupt: 89 e This clogs the entire system until I disconnect the communication lines. So we get an RX timeout. The receiver fifo trigger level was not reached and 8250_core is left to copy the remaining data. I would expect that if the trigger level wasn't reached, we wouldn't need to be doing this that often. On the other hand, could we be trapped reading data from rx without dma helping us? And how could we resolve this? Frans -- 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: prevent erronous parsing of children during rate change
On 09/30/2014 10:07 AM, Mike Turquette wrote: Quoting Tero Kristo (2014-09-29 01:09:24) On 09/27/2014 02:24 AM, Mike Turquette wrote: Quoting Tero Kristo (2014-09-26 00:18:55) On 09/26/2014 04:35 AM, Stephen Boyd wrote: On 09/23/14 06:38, Tero Kristo wrote: On 09/22/2014 10:18 PM, Stephen Boyd wrote: On 08/21, Tero Kristo wrote: /* Skip children who will be reparented to another clock */ if (child-new_parent child-new_parent != clk) continue; Are we not hitting the new_parent check here? I don't understand how we can be changing parents here unless the check is being avoided, in which case I wonder why determine_rate isn't being used. It depends how the clock underneath handles the situation. The error I am seeing actually happens with a SoC specific compound clock (DPLL) which integrates set_rate + mux functionality into a single clock node. A call to the clk_set_rate changes the parent of this clock (from bypass clock to reference clock), in addition to changing the rate (tune the mul+div.) I looked at using the determine rate call with this type but it breaks everything up... the parent gets changed but not the clock rate, in addition to some other issues. Ok. Is this omap3_noncore_dpll_set_rate()? Yes. Can we use determine_rate + clk_set_parent_and_rate()? At least clk_set_parent_and_rate() would allow us to do the mult+div and the parent in the same op call, although I don't understand why setting the parent and then setting the rate is not going to work. Well, setting parent first, then rate later causes problems with the DPLL ending up running with illegal (non-specified) rate, the M+N values are most likely wrong if you just switch from bypass clock to reference clock first without programming the M+N first. I took a quick look and it still seems to me that the OMAP DPLLs are still not modeled properly as mux clocks. Is this correct? Yeah, they are not mux clocks, but rather a compound of mux + DPLL multiplier/divider logic. Changing the DPLL to be a separate mux + DPLL div/mult clock will still have overlapping usage of the DPLL_EN field, I'm not talking about splitting up the clock into two separate clocks. If memory serves the DPLL clock implementation cheats and hides the bypass_clk info from the clock framework. To be explicit, from the perspective of Linux clock framework DPLL clocks only have one parent. In reality a typical DPLL should have at least 2 parents (and in some cases starting with OMAP4, some of the DPLL output clocks should have a second HSD parent). But the implementation does not reflect this. No, this is not the DPLLs are modelled. Each DPLL has currently two parents, ref-clk and bypass-clk, which are both modelled as separate clock nodes, and the DPLL switches parents based on bypass/lock mode. The bypass clock is also usually a mux clock, which further selects separate bypass parent, resulting in 3 or more parents for a certain DPLL. as the DPLL must be in bypass mode during M+N change. Or, should the DPLL rate change only be allowed if the mux is in bypass setting? Several drivers still depend on direct dpll clk_set_rate working 'properly' (there are some other issues currently present also which have nothing to do with the mux behavior.) This issue has been lingering for a long time and we can't use determine_rate unless that clock has multiple parents. Simply hacking knowledge of the parent bypass clock into the .set_rate callback is not enough. If you believe this _must_ be changed, I can take a look at this for next merge window, but this will cause a DT data compatibility break if nothing else (personally I don't care about this as I always rebuild DT blob with kernel, but lots of other people seem to do.) Well I guess the question is how long will we put up with the many small headaches caused by incorrectly modeling the clock? Well, its not kind of incorrectly modelled, it is just modelled in such way that clk_set_rate doesn't cope too well with it. determine_rate and clk_set_parent_and_rate should be sufficient for the OMAP DPLLs but only if they are correctly modeled in the framework. Do we have implementation for clk_set_parent_and_rate someplace? I looked at rc7 and didn't find this. I think this would fix the issues I am seeing combined with determine_rate, if clk_set_rate would internally handle changing both rate + parent. -Tero Regards, Mike -Tero Regards, Mike I'm interested in the other issues that you mentioned too. Mostly these were side-effects from the illegal DPLL setup I guess, like boot hang, failed drivers etc. I didn't really investigate this that much as it is much more simpler just to use safe list iteration here. -Tero -- 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/2] LLVMLinux: Patches to enable the kernel to be compiled with clang/LLVM
On 27/09/14 04:10, Behan Webster wrote: Replace the use of nested functions where a normal function will suffice. Nested functions are not liked by upstream kernel developers in general. Their use breaks the use of clang as a compiler, and doesn't make the code any better. This code now works for both gcc and clang. The LLVMLinux project aims to fully build the Linux kernel using both gcc and clang (the C front end for the LLVM compiler infrastructure project). Behan Webster (2): arm, fbdev, omap2, LLVMLinux: Remove nested function from omap2 dss arm, fbdev, omap2, LLVMLinux: Remove nested function from omapfb drivers/video/fbdev/omap2/dss/dispc-compat.c | 9 + drivers/video/fbdev/omap2/dss/manager-sysfs.c | 16 +--- drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 14 +++--- 3 files changed, 21 insertions(+), 18 deletions(-) I have to say I do like small helper funcs as nested functions, as they are restricted inside the parent function's scope. But, of course, nested funcs have their issues. So looks fine to me, queuing for 3.18. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
On 09/30/2014 03:26 PM, Wolfram Sang wrote: On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: Some TI SoCs like DRA7 have a RAMINIT register specification different from the other AMxx SoCs and as expected by the existing driver. To add more insanity, this register is shared with other IPs like DSS, PCIe and PWM. Provides a more generic mechanism to specify the RAMINIT register location and START/DONE bit position and use the syscon/regmap framework to access the register. Signed-off-by: Roger Quadros rog...@ti.com --- .../devicetree/bindings/net/can/c_can.txt | 7 ++ drivers/net/can/c_can/c_can.h | 11 ++- drivers/net/can/c_can/c_can_platform.c | 109 +++-- 3 files changed, 95 insertions(+), 32 deletions(-) diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt index 8f1ae81..e12d1a1 100644 --- a/Documentation/devicetree/bindings/net/can/c_can.txt +++ b/Documentation/devicetree/bindings/net/can/c_can.txt @@ -13,6 +13,13 @@ Optional properties: - ti,hwmods : Must be d_cann or c_cann, n being the instance number +- ti,raminit-syscon : Handle to system control region that contains the + RAMINIT register. If specified, the second memory resource + in the reg property must index into the RAMINIT + register within the syscon region There seems to be a simple syscon property these days. +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register This should not be encoded in DT! This is not describing hardware setup. The driver should know where the bits are for the syscon phandle, depending on which SoC it runs... Is the register shared by more than one core? If so the information has to go somewhere. Using an alias in the DT is probably the wrong approach. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
On 09/30/2014 04:52 PM, Wolfram Sang wrote: +- ti,raminit-syscon : Handle to system control region that contains the +RAMINIT register. If specified, the second memory resource +in the reg property must index into the RAMINIT +register within the syscon region There seems to be a simple syscon property these days. I had used plain syscon in the earlier revisions but was asked to make it a TI specific property since only TI uses this mechanism. I see, when was that? Currently, it looks messy :( Grepping through the dts files in 3.17-rc7, I see that bcm7445 uses syscon and variants with prefixes, STE uses it, too. Samsung uses samsung,syscon-phandle, st uses st,syscon... The MACID readout patches for AM335x (just applied right now) also use syscon: https://patchwork.ozlabs.org/patch/394289/ I'd vote for a generic syscon to be OK, yet I guess the DT maintainers will have the final word. Now that I actually checked, it was never just syscon but just without the ti vendor prefix. Sorry for the misinformation. As just TI is using this out of band RAMINIT mechanism, should it be ti,syscon or just syscon? cheers, -roger -- 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] net: can: c_can: Add syscon/regmap RAMINIT mechanism
On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: Some TI SoCs like DRA7 have a RAMINIT register specification different from the other AMxx SoCs and as expected by the existing driver. To add more insanity, this register is shared with other IPs like DSS, PCIe and PWM. Provides a more generic mechanism to specify the RAMINIT register location and START/DONE bit position and use the syscon/regmap framework to access the register. Signed-off-by: Roger Quadros rog...@ti.com --- .../devicetree/bindings/net/can/c_can.txt | 7 ++ drivers/net/can/c_can/c_can.h | 11 ++- drivers/net/can/c_can/c_can_platform.c | 109 +++-- 3 files changed, 95 insertions(+), 32 deletions(-) diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt index 8f1ae81..e12d1a1 100644 --- a/Documentation/devicetree/bindings/net/can/c_can.txt +++ b/Documentation/devicetree/bindings/net/can/c_can.txt @@ -13,6 +13,13 @@ Optional properties: - ti,hwmods : Must be d_cann or c_cann, n being the instance number +- ti,raminit-syscon : Handle to system control region that contains the + RAMINIT register. If specified, the second memory resource + in the reg property must index into the RAMINIT + register within the syscon region There seems to be a simple syscon property these days. +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register +- ti,raminit-done-bit: Bit position of DONE bit in the RAMINIT register This should not be encoded in DT! This is not describing hardware setup. The driver should know where the bits are for the syscon phandle, depending on which SoC it runs... signature.asc Description: Digital signature
Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
On 09/30/2014 04:26 PM, Wolfram Sang wrote: On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: Some TI SoCs like DRA7 have a RAMINIT register specification different from the other AMxx SoCs and as expected by the existing driver. To add more insanity, this register is shared with other IPs like DSS, PCIe and PWM. Provides a more generic mechanism to specify the RAMINIT register location and START/DONE bit position and use the syscon/regmap framework to access the register. Signed-off-by: Roger Quadros rog...@ti.com --- .../devicetree/bindings/net/can/c_can.txt | 7 ++ drivers/net/can/c_can/c_can.h | 11 ++- drivers/net/can/c_can/c_can_platform.c | 109 +++-- 3 files changed, 95 insertions(+), 32 deletions(-) diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt index 8f1ae81..e12d1a1 100644 --- a/Documentation/devicetree/bindings/net/can/c_can.txt +++ b/Documentation/devicetree/bindings/net/can/c_can.txt @@ -13,6 +13,13 @@ Optional properties: - ti,hwmods : Must be d_cann or c_cann, n being the instance number +- ti,raminit-syscon : Handle to system control region that contains the + RAMINIT register. If specified, the second memory resource + in the reg property must index into the RAMINIT + register within the syscon region There seems to be a simple syscon property these days. I had used plain syscon in the earlier revisions but was asked to make it a TI specific property since only TI uses this mechanism. +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register This should not be encoded in DT! This is not describing hardware setup. The driver should know where the bits are for the syscon phandle, depending on which SoC it runs... OK. I'll think of matching the compatible ID with SOC specific data in the driver. cheers, -roger -- 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] net: can: c_can: Add syscon/regmap RAMINIT mechanism
On 09/30/2014 04:45 PM, Marc Kleine-Budde wrote: On 09/30/2014 03:26 PM, Wolfram Sang wrote: On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: Some TI SoCs like DRA7 have a RAMINIT register specification different from the other AMxx SoCs and as expected by the existing driver. To add more insanity, this register is shared with other IPs like DSS, PCIe and PWM. Provides a more generic mechanism to specify the RAMINIT register location and START/DONE bit position and use the syscon/regmap framework to access the register. Signed-off-by: Roger Quadros rog...@ti.com --- .../devicetree/bindings/net/can/c_can.txt | 7 ++ drivers/net/can/c_can/c_can.h | 11 ++- drivers/net/can/c_can/c_can_platform.c | 109 +++-- 3 files changed, 95 insertions(+), 32 deletions(-) diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt index 8f1ae81..e12d1a1 100644 --- a/Documentation/devicetree/bindings/net/can/c_can.txt +++ b/Documentation/devicetree/bindings/net/can/c_can.txt @@ -13,6 +13,13 @@ Optional properties: - ti,hwmods: Must be d_cann or c_cann, n being the instance number +- ti,raminit-syscon: Handle to system control region that contains the + RAMINIT register. If specified, the second memory resource + in the reg property must index into the RAMINIT + register within the syscon region There seems to be a simple syscon property these days. +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register This should not be encoded in DT! This is not describing hardware setup. The driver should know where the bits are for the syscon phandle, depending on which SoC it runs... Is the register shared by more than one core? If so the information has to go somewhere. Using an alias in the DT is probably the wrong approach. In case of the DRA7xx Soc, this syscon register is shared by multiple cores. (CAN, Display, etc), sigh!! For AM335x and AM43xx SoCs, the register is used only for DCAN. but shared by 2 DCAN instances. Shouldn't the information for the CAN specific bits lie in the CAN driver? cheers, -roger -- 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] net: can: c_can: Add syscon/regmap RAMINIT mechanism
As just TI is using this out of band RAMINIT mechanism, should it be ti,syscon or just syscon? Yes, only TI uses this out-of-band RAMINIT (currently, at least). So, we need an (optional) way to describe that. However, accessing syscon registers in general is not TI specific and a generic way to do this should be used. Which looks to me like the syscon property to allow access to the register. Still, we should ask DT maintainers about it, maybe they prefer a more precise property like syscon-raminit to allow for further syscon extensions later or something... signature.asc Description: Digital signature
Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
On 09/30/2014 04:02 PM, Roger Quadros wrote: On 09/30/2014 04:45 PM, Marc Kleine-Budde wrote: On 09/30/2014 03:26 PM, Wolfram Sang wrote: On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: Some TI SoCs like DRA7 have a RAMINIT register specification different from the other AMxx SoCs and as expected by the existing driver. To add more insanity, this register is shared with other IPs like DSS, PCIe and PWM. Provides a more generic mechanism to specify the RAMINIT register location and START/DONE bit position and use the syscon/regmap framework to access the register. Signed-off-by: Roger Quadros rog...@ti.com --- .../devicetree/bindings/net/can/c_can.txt | 7 ++ drivers/net/can/c_can/c_can.h | 11 ++- drivers/net/can/c_can/c_can_platform.c | 109 +++-- 3 files changed, 95 insertions(+), 32 deletions(-) diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt index 8f1ae81..e12d1a1 100644 --- a/Documentation/devicetree/bindings/net/can/c_can.txt +++ b/Documentation/devicetree/bindings/net/can/c_can.txt @@ -13,6 +13,13 @@ Optional properties: - ti,hwmods : Must be d_cann or c_cann, n being the instance number +- ti,raminit-syscon : Handle to system control region that contains the +RAMINIT register. If specified, the second memory resource +in the reg property must index into the RAMINIT +register within the syscon region There seems to be a simple syscon property these days. +- ti,raminit-start-bit: Bit posistion of START bit in the RAMINIT register +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register This should not be encoded in DT! This is not describing hardware setup. The driver should know where the bits are for the syscon phandle, depending on which SoC it runs... Is the register shared by more than one core? If so the information has to go somewhere. Using an alias in the DT is probably the wrong approach. In case of the DRA7xx Soc, this syscon register is shared by multiple cores. (CAN, Display, etc), sigh!! For AM335x and AM43xx SoCs, the register is used only for DCAN. but shared by 2 DCAN instances. Shouldn't the information for the CAN specific bits lie in the CAN driver? As there are more than one DCAN instance on the same SOC the information which DCAN instance uses which bits in the register has to go somewhere. We might add these bits as the 3rd and 4th parameter to the syscon phandle. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
+- ti,raminit-syscon : Handle to system control region that contains the +RAMINIT register. If specified, the second memory resource +in the reg property must index into the RAMINIT +register within the syscon region There seems to be a simple syscon property these days. I had used plain syscon in the earlier revisions but was asked to make it a TI specific property since only TI uses this mechanism. I see, when was that? Currently, it looks messy :( Grepping through the dts files in 3.17-rc7, I see that bcm7445 uses syscon and variants with prefixes, STE uses it, too. Samsung uses samsung,syscon-phandle, st uses st,syscon... The MACID readout patches for AM335x (just applied right now) also use syscon: https://patchwork.ozlabs.org/patch/394289/ I'd vote for a generic syscon to be OK, yet I guess the DT maintainers will have the final word. +- ti,raminit-start-bit: Bit posistion of START bit in the RAMINIT register +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register This should not be encoded in DT! This is not describing hardware setup. The driver should know where the bits are for the syscon phandle, depending on which SoC it runs... OK. I'll think of matching the compatible ID with SOC specific data in the driver. Great, thanks! signature.asc Description: Digital signature
Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
On 09/30/2014 04:19 PM, Wolfram Sang wrote: As just TI is using this out of band RAMINIT mechanism, should it be ti,syscon or just syscon? Yes, only TI uses this out-of-band RAMINIT (currently, at least). So, we need an (optional) way to describe that. However, accessing syscon registers in general is not TI specific and a generic way to do this should be used. Which looks to me like the syscon property to allow access to the register. Still, we should ask DT maintainers about it, maybe they prefer a more precise property like syscon-raminit to allow for further syscon extensions later or something... What do you think about putting the bit information in the syscon-raminit phandle as additional arguments? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
On Tue, Sep 30, 2014 at 04:22:08PM +0200, Marc Kleine-Budde wrote: On 09/30/2014 04:19 PM, Wolfram Sang wrote: As just TI is using this out of band RAMINIT mechanism, should it be ti,syscon or just syscon? Yes, only TI uses this out-of-band RAMINIT (currently, at least). So, we need an (optional) way to describe that. However, accessing syscon registers in general is not TI specific and a generic way to do this should be used. Which looks to me like the syscon property to allow access to the register. Still, we should ask DT maintainers about it, maybe they prefer a more precise property like syscon-raminit to allow for further syscon extensions later or something... What do you think about putting the bit information in the syscon-raminit phandle as additional arguments? Then we'd have n syscon phandles for n instances? Also, judging from Markus patch [1] there is already some infrastructure, namely syscon_regmap_lookup_by_phandle(). From a glimpse, it doesn't look viable to add such a support to it. So, I'd rather drop additional arguments. Why would you like to have it encoded in DT? signature.asc Description: Digital signature
Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
On 09/30/2014 04:49 PM, Wolfram Sang wrote: On Tue, Sep 30, 2014 at 04:22:08PM +0200, Marc Kleine-Budde wrote: On 09/30/2014 04:19 PM, Wolfram Sang wrote: As just TI is using this out of band RAMINIT mechanism, should it be ti,syscon or just syscon? Yes, only TI uses this out-of-band RAMINIT (currently, at least). So, we need an (optional) way to describe that. However, accessing syscon registers in general is not TI specific and a generic way to do this should be used. Which looks to me like the syscon property to allow access to the register. Still, we should ask DT maintainers about it, maybe they prefer a more precise property like syscon-raminit to allow for further syscon extensions later or something... What do you think about putting the bit information in the syscon-raminit phandle as additional arguments? Then we'd have n syscon phandles for n instances? Also, judging from Markus patch [1] there is already some infrastructure, namely syscon_regmap_lookup_by_phandle(). From a glimpse, it doesn't look viable to add such a support to it. Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for additional parameters. Have a look at: drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c First get the regmap, then the 1st argument is the offset in the regmap, the 2nd and 3rd could be the bits. So, I'd rather drop additional arguments. Why would you like to have it encoded in DT? Where put the information then? Into the driver, but where do you get the reference which instance of the DCAN you are, so that you can look up the correct bits? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for additional parameters. Have a look at: drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c First get the regmap, then the 1st argument is the offset in the regmap, the 2nd and 3rd could be the bits. So, for one driver the extra arguments are: reg start_bit stop_bit For another driver (the stmmac example): reg_offset reg_shift Phew... Then we should really have a syscon-raminit property probably, so that at least plain syscon has a consistent syntax? So, I'd rather drop additional arguments. Why would you like to have it encoded in DT? Where put the information then? Into the driver, but where do you get the reference which instance of the DCAN you are, so that you can look up the correct bits? Agreed. I thought we had this information in the driver already, but we haven't... signature.asc Description: Digital signature
[PATCH] regmap: Allow read_reg_mask to be 0
There may be spi devices that do not require a register read mask to read the registers. Currently the code sets the read mask based on a non-zero value passed in from the driver or if that value is 0 sets the read mask to 0x80. A mask of 0 is a valid mask as well. Create a define to indicate that the read mask can be zero and separate out the read flag mask and the write flag mask. Signed-off-by: Dan Murphy dmur...@ti.com --- drivers/base/regmap/regmap.c | 11 +++ include/linux/regmap.h |2 ++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index 78f43fb..8c0a9b8 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -528,12 +528,15 @@ struct regmap *regmap_init(struct device *dev, INIT_LIST_HEAD(map-async_free); init_waitqueue_head(map-async_waitq); - if (config-read_flag_mask || config-write_flag_mask) { + if (config-read_flag_mask == REGMAP_NO_READ_MASK) + map-read_flag_mask = 0x00; + else if (config-read_flag_mask) map-read_flag_mask = config-read_flag_mask; - map-write_flag_mask = config-write_flag_mask; - } else if (bus) { + else if (bus) map-read_flag_mask = bus-read_flag_mask; - } + + if (config-write_flag_mask) + map-write_flag_mask = config-write_flag_mask; if (!bus) { map-reg_read = config-reg_read; diff --git a/include/linux/regmap.h b/include/linux/regmap.h index c5ed83f..f1cdfe5 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -18,6 +18,8 @@ #include linux/err.h #include linux/bug.h +#define REGMAP_NO_READ_MASK0xff + struct module; struct device; struct i2c_client; -- 1.7.9.5 -- 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] net: can: c_can: Add syscon/regmap RAMINIT mechanism
On 09/30/2014 05:25 PM, Wolfram Sang wrote: Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for additional parameters. Have a look at: drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c First get the regmap, then the 1st argument is the offset in the regmap, the 2nd and 3rd could be the bits. So, for one driver the extra arguments are: reg start_bit stop_bit For another driver (the stmmac example): reg_offset reg_shift The DCAN's reg is a reg_offset as in the stmmc. Roger, can we derive both start and done bit from a common reg_shift? Phew... Then we should really have a syscon-raminit property probably, so that at least plain syscon has a consistent syntax? I think^whope we can have the same syntax as the stmmc :D So, I'd rather drop additional arguments. Why would you like to have it encoded in DT? Where put the information then? Into the driver, but where do you get the reference which instance of the DCAN you are, so that you can look up the correct bits? Agreed. I thought we had this information in the driver already, but we haven't... The current driver relies on the of_alias_get_id(), which isn't considered best practice, is it? So I want to avoid this when switching to syscon. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCHv6 0/5] hwspinlock core/omap dt support
Hi Ohad, This is an update to the hwspinlock dt support series. The series is rebased onto v3.17-rc3, and addresses the review comments on the previous v5 series. I have also split and left out the RFC patches about the support for reserved locks (will post these as a separate series) and return code convention changes in the hwspinlock core (will not be needed anymore). The support for deferred probing of clients is supported in the new of_hwspin_lock_get_id() function itself. Following are the main changes in v6 w.r.t v5: - [Patch 1] Updated generic hwspinlock bindings after folding back the hwlock-base-id property from v5 Patch 8 [1]. [Patch 1] - [Patch 4] Updated the common OF helpers patch based on review comments on v5. of_hwspin_lock_request_specific() is replaced with of_hwspin_lock_get_id(). of_hwspin_lock_simple_xlate() is made internal, and of_hwspin_lock_get_base_id() is added. - [Patch 5] Updated the OMAP hwspinlock DT support patch to address review comments about hard-coded base-id, it is parsed from DT if present. - Patches 2 and 3 are unchanged from previous version. - Support patches for AM335x and AM437x SoCs (v5 patches 6 and 7) have been dropped as they are merged in separately for 3.17 - RFC patches (v5 patches 9 through 15) adding the concept of reserved locks and return code change convention dropped. I had looked into dropping the patch to maintain the list of registered spinlocks, but had to retain the patch as it was needed to perform the validity of the args-specifier value in of_hwspin_lock_get_id(). Ping, do you have any comments on this revised series? If not, can we queue this for 3.18? regards Suman The validation logs on all the applicable OMAP SoCs are at: OMAP4 - http://pastebin.ubuntu.com/8329228 OMAP5 - http://pastebin.ubuntu.com/8329301 DRA74x - http://pastebin.ubuntu.com/8329261 AM33xx - http://pastebin.ubuntu.com/8329199 AM43xx - http://pastebin.ubuntu.com/8329273 The above logs are generated with some additional test patches staged here for reference: https://github.com/sumananna/omap-kernel/commits/hwspinlock/test/3.17-rc3-dt-v6 https://github.com/sumananna/omap-kernel/commits/hwspinlock/submit/3.17-rc3-dt-v6 regards Suman [1] https://patchwork.kernel.org/patch/4096741/ --- v5: http://marc.info/?l=linux-omapm=139890478402807w=2 - Rebased v4 patches plus additional RFC patches. - Added back the patch to support DT-based hwlock-base-id property, for registration purposes. - RFC patches introducing the concept of reserved locks. - Staged patches for converting return convention to better support deferred probing of client drivers. v4: - The DT bindings are split into separate patches, and updated to add comments about #hwlock-cells - Fixed a registration issue with repeated module installation and removal. - Added a new OF helper to support #hwlock-cells in addition to the previous OF functions. The OMAP adaptation patch is updated to use the default translate function - Updated hwspinlock documentation to adjust for the structure changes and the new api additions. - Added build support for AM335x, AM43xx and DRA7xx http://marc.info/?l=linux-omapm=138965904015225w=2 v3: - Removed the DT property hwlock-base-id and associated OF helper - Added changes in core to support requesting a specific hwlock using phandle + args approach - Revised both the common and OMAP DT bindings document http://marc.info/?l=linux-omapm=138143992932197w=2 v2: - Added a new common DT binding documentation and OF helpers. - Revised OMAP DT parse support to use the new OF helper (Patch2) - OMAP5 hwspinlock support including the hwmod entry and DT node - Add AM335x support to OMAP hwspinlock driver, including a fix needed in driver given that AM335 spinlock module requires s/w wakeup - AM335 DT node for spinlock, and a hwmod change to enable smart-idle for AM335. - OMAP4 DT node patch is unchanged http://marc.info/?l=linux-omapm=137944644112727w=2 v1: - Add DT parse support to OMAP hwspinlock driver - Add OMAP4 DT node and bindings information http://marc.info/?l=linux-omapm=137823082308009w=2 --- Suman Anna (5): Documentation: dt: add common bindings for hwspinlock Documentation: dt: add the omap hwspinlock bindings document hwspinlock/core: maintain a list of registered hwspinlock banks hwspinlock/core: add common OF helpers hwspinlock/omap: add support for dt nodes .../devicetree/bindings/hwlock/hwlock.txt | 55 +++ .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 24 +++ Documentation/hwspinlock.txt | 28 MAINTAINERS| 1 - arch/arm/mach-omap2/Makefile | 3 - arch/arm/mach-omap2/hwspinlock.c | 60 --- drivers/hwspinlock/hwspinlock_core.c | 173
Re: [PATCH] regmap: Allow read_reg_mask to be 0
On Tue, Sep 30, 2014 at 11:07:00AM -0500, Dan Murphy wrote: - if (config-read_flag_mask || config-write_flag_mask) { + if (config-read_flag_mask == REGMAP_NO_READ_MASK) + map-read_flag_mask = 0x00; + else if (config-read_flag_mask) This breaks the symmetry in handling of read and write masks which isn't great, please make the equivalent update for the write mask too. +#define REGMAP_NO_READ_MASK 0xff An actual out of band value might be preferable here though that'd involve changing the type and more checking so perhaps inessential. signature.asc Description: Digital signature
Re: [PATCH] clk: prevent erronous parsing of children during rate change
Quoting Tero Kristo (2014-09-30 01:48:49) On 09/30/2014 10:07 AM, Mike Turquette wrote: Quoting Tero Kristo (2014-09-29 01:09:24) On 09/27/2014 02:24 AM, Mike Turquette wrote: Quoting Tero Kristo (2014-09-26 00:18:55) On 09/26/2014 04:35 AM, Stephen Boyd wrote: On 09/23/14 06:38, Tero Kristo wrote: On 09/22/2014 10:18 PM, Stephen Boyd wrote: On 08/21, Tero Kristo wrote: /* Skip children who will be reparented to another clock */ if (child-new_parent child-new_parent != clk) continue; Are we not hitting the new_parent check here? I don't understand how we can be changing parents here unless the check is being avoided, in which case I wonder why determine_rate isn't being used. It depends how the clock underneath handles the situation. The error I am seeing actually happens with a SoC specific compound clock (DPLL) which integrates set_rate + mux functionality into a single clock node. A call to the clk_set_rate changes the parent of this clock (from bypass clock to reference clock), in addition to changing the rate (tune the mul+div.) I looked at using the determine rate call with this type but it breaks everything up... the parent gets changed but not the clock rate, in addition to some other issues. Ok. Is this omap3_noncore_dpll_set_rate()? Yes. Can we use determine_rate + clk_set_parent_and_rate()? At least clk_set_parent_and_rate() would allow us to do the mult+div and the parent in the same op call, although I don't understand why setting the parent and then setting the rate is not going to work. Well, setting parent first, then rate later causes problems with the DPLL ending up running with illegal (non-specified) rate, the M+N values are most likely wrong if you just switch from bypass clock to reference clock first without programming the M+N first. I took a quick look and it still seems to me that the OMAP DPLLs are still not modeled properly as mux clocks. Is this correct? Yeah, they are not mux clocks, but rather a compound of mux + DPLL multiplier/divider logic. Changing the DPLL to be a separate mux + DPLL div/mult clock will still have overlapping usage of the DPLL_EN field, I'm not talking about splitting up the clock into two separate clocks. If memory serves the DPLL clock implementation cheats and hides the bypass_clk info from the clock framework. To be explicit, from the perspective of Linux clock framework DPLL clocks only have one parent. In reality a typical DPLL should have at least 2 parents (and in some cases starting with OMAP4, some of the DPLL output clocks should have a second HSD parent). But the implementation does not reflect this. No, this is not the DPLLs are modelled. Each DPLL has currently two parents, ref-clk and bypass-clk, which are both modelled as separate clock nodes, and the DPLL switches parents based on bypass/lock mode. The bypass clock is also usually a mux clock, which further selects separate bypass parent, resulting in 3 or more parents for a certain DPLL. I stand corrected. I thought it was still done the old way where the machine-specific clock struct was holding the pointer to the ref_clk and bypass_clk. I'm glad that is not the case any more. as the DPLL must be in bypass mode during M+N change. Or, should the DPLL rate change only be allowed if the mux is in bypass setting? Several drivers still depend on direct dpll clk_set_rate working 'properly' (there are some other issues currently present also which have nothing to do with the mux behavior.) This issue has been lingering for a long time and we can't use determine_rate unless that clock has multiple parents. Simply hacking knowledge of the parent bypass clock into the .set_rate callback is not enough. If you believe this _must_ be changed, I can take a look at this for next merge window, but this will cause a DT data compatibility break if nothing else (personally I don't care about this as I always rebuild DT blob with kernel, but lots of other people seem to do.) Well I guess the question is how long will we put up with the many small headaches caused by incorrectly modeling the clock? Well, its not kind of incorrectly modelled, it is just modelled in such way that clk_set_rate doesn't cope too well with it. determine_rate and clk_set_parent_and_rate should be sufficient for the OMAP DPLLs but only if they are correctly modeled in the framework. Do we have implementation for clk_set_parent_and_rate someplace? I looked at rc7 and didn't find this. I think this would fix the issues I am seeing combined with determine_rate, if clk_set_rate would internally handle changing both rate + parent. I made it hard for you to find it because I typo'd. It's not a clk api but a clk_op: int (*set_rate_and_parent)(struct clk_hw *hw, unsigned long rate,
[PATCH 0/3] input: tsc2007: Extend for pre-calibration, flipping and rotation
Following series add support to tsc2007 touchscreen driver for pre-calibration, flipping and rotation. Added bindings are documented and used in gta04 device tree. Marek Belisko (3): input: tsc2007: Add pre-calibration, flipping and rotation Documentation: dt: input: tsc2007: Document new parameters arm: dts: omap3-gta04: Extend touchscreen configs .../bindings/input/touchscreen/tsc2007.txt | 17 - arch/arm/boot/dts/omap3-gta04.dtsi | 6 ++ drivers/input/touchscreen/tsc2007.c| 89 +- include/linux/i2c/tsc2007.h| 6 ++ 4 files changed, 111 insertions(+), 7 deletions(-) -- 1.9.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 2/3] Documentation: dt: input: tsc2007: Document new parameters
Signed-off-by: Marek Belisko ma...@goldelico.com --- .../devicetree/bindings/input/touchscreen/tsc2007.txt | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt index ec365e1..46b086c 100644 --- a/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt @@ -13,14 +13,25 @@ Optional properties: (see interrupt binding[0]). - interrupts: (gpio) interrupt to which the chip is connected (see interrupt binding[0]). -- ti,max-rt: maximum pressure. +- ti,swap-xy: if present, swap x and y values. Rotation left/right is + achieved by combination with flipping of x or y. +- ti,min-x: minimum x (default 0). Use this for a coarse calibration of the + touch if there is no user space option (e.g. Android, X11). +- ti,max-x: maximum x (default 4095). If max-x is smaller than min-x the + axis is swapped. +- ti,min-y: minimum y (default 0). +- ti,max-y: maximum y (default 4095). +- ti,min-rt: minimum pressure (default 0). +- ti,max-rt: maximum pressure (default 4095). Depending on your needs, you + can also invert the pressure value since the raw rt value reports the + resistance and not the pressure (i.e. becomes lower for higher pressure). - ti,fuzzx: specifies the absolute input fuzz x value. If set, it will permit noise in the data up to +- the value given to the fuzz - parameter, that is used to filter noise from the event stream. + parameter, that is used to filter noise from the event stream (default 0). - ti,fuzzy: specifies the absolute input fuzz y value. - ti,fuzzz: specifies the absolute input fuzz z value. - ti,poll-period: how much time to wait (in milliseconds) before reading again the - values from the tsc2007. + values from the tsc2007 (default 1). [0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt [1]: Documentation/devicetree/bindings/gpio/gpio.txt -- 1.9.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] regmap: Allow read_reg_mask to be 0
Mark Thanks for the review On 09/30/2014 12:25 PM, Mark Brown wrote: On Tue, Sep 30, 2014 at 11:07:00AM -0500, Dan Murphy wrote: -if (config-read_flag_mask || config-write_flag_mask) { +if (config-read_flag_mask == REGMAP_NO_READ_MASK) +map-read_flag_mask = 0x00; +else if (config-read_flag_mask) This breaks the symmetry in handling of read and write masks which isn't great, please make the equivalent update for the write mask too. Hmmm. If I make the similar change for the write mask I will be adding dead code as write_flag_mask is not defaulted by the bus like the read_flag_mask which is defaulted to 0x80 in the regmap-spi code. The i2c code does not make this assumption. The original code did not seem to have symmetry as the only instance that write_flag_mask is modified is if it is non-zero. Let me know what you think. I can add the code to make it more symmetrical but we may be adding code that is dead. If I remove the defaulted read_flag_mask value out of the spi driver then I will need to find every spi device that needs that flag and set it. IMHO that would be really messy and probably mess us some drivers. +#define REGMAP_NO_READ_MASK 0xff An actual out of band value might be preferable here though that'd involve changing the type and more checking so perhaps inessential. Thought about making this -1 with a variable change but that seemed really drastic where a value of 0xff seems to be a value that no one should use. Dan -- -- Dan Murphy -- 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 3/3] arm: dts: omap3-gta04: Extend touchscreen configs
Adding min/max values for various touschscreen properties. Signed-off-by: Marek Belisko ma...@goldelico.com --- arch/arm/boot/dts/omap3-gta04.dtsi | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi index fd34f91..2dafac3b 100644 --- a/arch/arm/boot/dts/omap3-gta04.dtsi +++ b/arch/arm/boot/dts/omap3-gta04.dtsi @@ -284,6 +284,12 @@ interrupts = 0 IRQ_TYPE_EDGE_FALLING; gpios = gpio6 0 GPIO_ACTIVE_LOW; ti,x-plate-ohms = 600; + ti,min-x = 0x100; + ti,max-x = 0xf00; + ti,min-y = 0xf00; + ti,max-y = 0x100; + ti,min-rt = 0xfff; + ti,max-rt = 0; }; }; -- 1.9.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 1/3] input: tsc2007: Add pre-calibration, flipping and rotation
This patch adds new parameters that allow to address typical hardware design differences: touch screens may be wired or oriented differently (portrait or landscape). And usually the active area of the touch is a little larger than the active area of the LCD. This results in the touch coordinates that have some significant deviation from LCD coordinates. Usually this is addressed in user space by a calibration tool (e.g. tslib or xinput-calibrator) but some systems don't have these tools or require that the screen is already roughly calibrated (e.g. Replicant) to operate the device until a better calibration can be done. And, some systems react very strangely if the touch event stream reports coordinates outside of the active area. This makes it necessry to be able to configure: 1. swapping x and y wires (coordinate values) 2. flipping of x (left - right) or y (top - bottom) or even both 3. define an active area so that an uncalibrated screen already roughly matches the LCD to be useful. 4. clip reported coordinates to the active area. If none of the new parameters is defined (in DT) or set in a board file, the driver behaves the same as without this patch. Author (12): H. Nikolaus Schaller h...@goldelico.com Author (34): Paul Kocialkowski cont...@paulk.fr Signed-off-by: H. Nikolaus Schaller h...@goldelico.com --- drivers/input/touchscreen/tsc2007.c | 89 +++-- include/linux/i2c/tsc2007.h | 6 +++ 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c index 1bf9906..cc0cc3c 100644 --- a/drivers/input/touchscreen/tsc2007.c +++ b/drivers/input/touchscreen/tsc2007.c @@ -74,6 +74,12 @@ struct tsc2007 { u16 model; u16 x_plate_ohms; + boolswap_xy;/* swap x and y axis */ + u16 min_x; + u16 min_y; + u16 min_rt; + u16 max_x; + u16 max_y; u16 max_rt; unsigned long poll_period; int fuzzx; @@ -193,11 +199,50 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle) break; } - if (rt = ts-max_rt) { + if (rt = max(ts-min_rt, ts-max_rt)) { dev_dbg(ts-client-dev, DOWN point(%4d,%4d), pressure (%4u)\n, tc.x, tc.y, rt); + if (ts-swap_xy) { + /* swap before applying the range limits */ + u16 h = tc.x; + + tc.x = tc.y; + tc.y = h; + } + + /* flip and/or clip X */ + if (ts-max_x ts-min_x) + tc.x = (ts-min_x - tc.x) + ts-max_x; + + if (tc.x max(ts-min_x, ts-max_x)) + tc.x = max(ts-min_x, ts-max_x); + else if (tc.x min(ts-min_x, ts-max_x)) + tc.x = min(ts-min_x, ts-max_x); + + /* flip and/or clip Y */ + if (ts-max_y ts-min_y) + tc.y = (ts-min_y - tc.y) + ts-max_y; + + if (tc.y max(ts-min_y, ts-max_y)) + tc.y = max(ts-min_y, ts-max_y); + else if (tc.y min(ts-min_y, ts-max_y)) + tc.y = min(ts-min_y, ts-max_y); + + /* clip Z */ + if (ts-max_rt ts-min_rt) + rt = (ts-min_rt - rt) + ts-max_rt; + + if (rt max(ts-min_rt, ts-max_rt)) + rt = max(ts-min_rt, ts-max_rt); + else if (rt min(ts-min_rt, ts-max_rt)) + rt = min(ts-min_rt, ts-max_rt); + + dev_dbg(ts-client-dev, + shaped point(%4d,%4d), pressure (%4u)\n, + tc.x, tc.y, rt); + input_report_key(input, BTN_TOUCH, 1); input_report_abs(input, ABS_X, tc.x); input_report_abs(input, ABS_Y, tc.y); @@ -299,6 +344,24 @@ static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts) return -EINVAL; } + ts-swap_xy = of_property_read_bool(np, ti,swap-xy); + + if (!of_property_read_u32(np, ti,min-x, val32)) + ts-min_x = val32; + if (!of_property_read_u32(np, ti,max-x, val32)) + ts-max_x = val32; + else + ts-max_x = MAX_12BIT; + + if
Re: [PATCH] regmap: Allow read_reg_mask to be 0
On 09/30/2014 12:25 PM, Mark Brown wrote: On Tue, Sep 30, 2014 at 11:07:00AM -0500, Dan Murphy wrote: -if (config-read_flag_mask || config-write_flag_mask) { +if (config-read_flag_mask == REGMAP_NO_READ_MASK) +map-read_flag_mask = 0x00; +else if (config-read_flag_mask) This breaks the symmetry in handling of read and write masks which isn't great, please make the equivalent update for the write mask too. Hmmm. If I make the similar change for the write mask I will be adding dead code as write_flag_mask is not defaulted by the bus like the read_flag_mask which is defaulted to 0x80 in the regmap-spi code. The i2c code does not make this assumption. The original code did not seem to have symmetry as the only instance that write_flag_mask is modified is if it is non-zero. Let me know what you think. I can add the code to make it more symmetrical but we may be adding code that is dead. If I remove the defaulted read_flag_mask value out of the spi driver then I will need to find every spi device that needs that flag and set it. IMHO that would be really messy and probably mess us some drivers. +#define REGMAP_NO_READ_MASK 0xff An actual out of band value might be preferable here though that'd involve changing the type and more checking so perhaps inessential. Thought about making this -1 with a variable change but that seemed really drastic where a value of 0xff seems to be a value that no one should use. Dan -- -- Dan Murphy -- 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 02/13] tty: serial: 8250: make serial8250_console_setup() non _init
On 09/29/2014 02:06 PM, Sebastian Andrzej Siewior wrote: if I boot with console=ttyS0 and the omap driver is module I end up with | console [ttyS0] disabled Note to self: make ISA config optional. | omap8250 44e09000.serial: ttyS0 at MMIO 0x44e09000 (irq = 88, base_baud = 300) is a 8250 | Unable to handle kernel paging request at virtual address c07a9de0 | Modules linked in: 8250_omap(+) | CPU: 0 PID: 908 Comm: modprobe Not tainted 3.17.0-rc5+ #1593 | PC is at serial8250_console_setup+0x0/0xc8 | LR is at register_console+0x13c/0x3a4 | [c0078788] (register_console) from [c02d0340] (uart_add_one_port+0x3cc/0x420) | [c02d0340] (uart_add_one_port) from [c02d38a4] (serial8250_register_8250_port+0x298/0x39c) | [c02d38a4] (serial8250_register_8250_port) from [bf006274] (omap8250_probe+0x218/0x3dc [8250_omap]) | [bf006274] (omap8250_probe [8250_omap]) from [c02e3424] (platform_drv_probe+0x2c/0x5c) | [c02e3424] (platform_drv_probe) from [c02e1eac] (driver_probe_device+0x104/0x228) … | [c009fa48] (SyS_init_module) from [c000e6e0] (ret_fast_syscall+0x0/0x30) | Code: 7823603b f8314620 051b3013 491ed416 (44792204) because serial8250_console_setup() is already gone. Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de Reviewed-by: Peter Hurley pe...@hurleysoftware.com -- 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] regmap: Allow read_reg_mask to be 0
Mark Thanks for the quick review On 09/30/2014 12:25 PM, Mark Brown wrote: On Tue, Sep 30, 2014 at 11:07:00AM -0500, Dan Murphy wrote: -if (config-read_flag_mask || config-write_flag_mask) { +if (config-read_flag_mask == REGMAP_NO_READ_MASK) +map-read_flag_mask = 0x00; +else if (config-read_flag_mask) This breaks the symmetry in handling of read and write masks which isn't great, please make the equivalent update for the write mask too. Hmmm. If I make the similar change for the write mask I will be adding dead code as write_flag_mask is not defaulted by the bus like the read_flag_mask which is defaulted to 0x80 in the regmap-spi code. The i2c code does not make this assumption. The original code did not seem to have symmetry as the only instance that write_flag_mask is modified is if it is non-zero. Let me know what you think. I can add the code to make it more symmetrical but we may be adding code that is dead. If I remove the defaulted read_flag_mask value out of the spi driver then I will need to find every spi device that needs that flag and set it. IMHO that would be really messy and probably mess us some drivers. +#define REGMAP_NO_READ_MASK 0xff An actual out of band value might be preferable here though that'd involve changing the type and more checking so perhaps inessential. Thought about making this -1 with a variable change but that seemed really drastic where a value of 0xff seems to be a value that no one should use. Dan -- -- Dan Murphy -- 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: [PATCHv6 0/5] hwspinlock core/omap dt support
On Fri, Sep 12, 2014 at 1:24 PM, Suman Anna s-a...@ti.com wrote: Hi Ohad, This is an update to the hwspinlock dt support series. The series is rebased onto v3.17-rc3, and addresses the review comments on the previous v5 series. I have also split and left out the RFC patches about the support for reserved locks (will post these as a separate series) and return code convention changes in the hwspinlock core (will not be needed anymore). The support for deferred probing of clients is supported in the new of_hwspin_lock_get_id() function itself. Thanks for your reply to me, I had missed that you continued this work. I find it somewhat awkward to have to call both of_hwspin_lock_get_id() and then hwspin_lock_request_specific(), but I found the request from Ohad, so let's stick with it. Am I right that hwlock-num-locks and hwlock-base-id are optional from the frameworks perspective and only there to aid the hwspin drivers? If so it is strange to have in the common binding and have the helper functions in the core for simply reading hwspin device specific properties. Otherwise I think it looks sane, although I haven't spend that much time reviewing it. I did throw it into my tree and gave it a testrun with the Qualcomm code I've been working on. So for the non-omap parts: Tested-by: Bjorn Andersson bjorn.anders...@sonymobile.com Regards, Bjorn -- 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] regmap: Allow read_reg_mask to be 0
On 09/30/2014 06:07 PM, Dan Murphy wrote: There may be spi devices that do not require a register read mask to read the registers. Currently the code sets the read mask based on a non-zero value passed in from the driver or if that value is 0 sets the read mask to 0x80. It only sets it to the bus default if both read_flag_mask and write_flag_mask are 0. The assumption is that both of them being zero is a invalid configuration and either of them (or both) have to be non-zero for proper operation, since otherwise the device can't tell the difference between a read and a write. Do you have a device where both the read and the write mask is 0? - Lars -- 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 11/17] iommu/omap: Make pagetable debugfs entry read-only
Remove the writeability on the 'pagetable' debugfs entry, so that the mapping/unmapping into an OMAP IOMMU is only limited to actual client devices/drivers at kernel-level. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu-debug.c | 48 ++-- 1 file changed, 2 insertions(+), 46 deletions(-) diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c index a520438..28de657 100644 --- a/drivers/iommu/omap-iommu-debug.c +++ b/drivers/iommu/omap-iommu-debug.c @@ -24,8 +24,6 @@ #include omap-iopgtable.h #include omap-iommu.h -#define MAXCOLUMN 100 /* for short messages */ - static DEFINE_MUTEX(iommu_debug_lock); static struct dentry *iommu_debug_root; @@ -82,39 +80,6 @@ static ssize_t debug_read_tlb(struct file *file, char __user *userbuf, return bytes; } -static ssize_t debug_write_pagetable(struct file *file, -const char __user *userbuf, size_t count, loff_t *ppos) -{ - struct iotlb_entry e; - struct cr_regs cr; - int err; - struct device *dev = file-private_data; - struct omap_iommu *obj = dev_to_omap_iommu(dev); - char buf[MAXCOLUMN], *p = buf; - - count = min(count, sizeof(buf)); - - mutex_lock(iommu_debug_lock); - if (copy_from_user(p, userbuf, count)) { - mutex_unlock(iommu_debug_lock); - return -EFAULT; - } - - sscanf(p, %x %x, cr.cam, cr.ram); - if (!cr.cam || !cr.ram) { - mutex_unlock(iommu_debug_lock); - return -EINVAL; - } - - omap_iotlb_cr_to_e(cr, e); - err = omap_iopgtable_store_entry(obj, e); - if (err) - dev_err(obj-dev, %s: fail to store cr\n, __func__); - - mutex_unlock(iommu_debug_lock); - return count; -} - #define dump_ioptable_entry_one(lv, da, val) \ ({ \ int __err = 0; \ @@ -202,14 +167,6 @@ static ssize_t debug_read_pagetable(struct file *file, char __user *userbuf, return bytes; } -#define DEBUG_FOPS(name) \ - static const struct file_operations debug_##name##_fops = { \ - .open = simple_open,\ - .read = debug_read_##name, \ - .write = debug_write_##name,\ - .llseek = generic_file_llseek, \ - }; - #define DEBUG_FOPS_RO(name)\ static const struct file_operations debug_##name##_fops = { \ .open = simple_open,\ @@ -219,7 +176,7 @@ static ssize_t debug_read_pagetable(struct file *file, char __user *userbuf, DEBUG_FOPS_RO(regs); DEBUG_FOPS_RO(tlb); -DEBUG_FOPS(pagetable); +DEBUG_FOPS_RO(pagetable); #define __DEBUG_ADD_FILE(attr, mode) \ { \ @@ -230,7 +187,6 @@ DEBUG_FOPS(pagetable); return -ENOMEM; \ } -#define DEBUG_ADD_FILE(name) __DEBUG_ADD_FILE(name, 0600) #define DEBUG_ADD_FILE_RO(name) __DEBUG_ADD_FILE(name, 0400) static int iommu_debug_register(struct device *dev, void *data) @@ -263,7 +219,7 @@ static int iommu_debug_register(struct device *dev, void *data) DEBUG_ADD_FILE_RO(regs); DEBUG_ADD_FILE_RO(tlb); - DEBUG_ADD_FILE(pagetable); + DEBUG_ADD_FILE_RO(pagetable); return 0; -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/17] iommu/omap: Remove couple of unused exported functions
The exported functions omap_foreach_iommu_device() and omap_iotlb_cr_to_e() have been deleted, as they are no longer needed. The function omap_foreach_iommu_device() is not required after the consolidation of the OMAP IOMMU debug module, and the function omap_iotlb_cr_to_e() is not required after making the debugfs entry 'pagetable' read-only. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu.c | 21 - drivers/iommu/omap-iommu.h | 5 - 2 files changed, 26 deletions(-) diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c index 717d654..b3bc087 100644 --- a/drivers/iommu/omap-iommu.c +++ b/drivers/iommu/omap-iommu.c @@ -203,20 +203,6 @@ static void iommu_disable(struct omap_iommu *obj) /* * TLB operations */ -void omap_iotlb_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e) -{ - BUG_ON(!cr || !e); - - e-da = cr-cam MMU_CAM_VATAG_MASK; - e-pa = cr-ram MMU_RAM_PADDR_MASK; - e-valid= cr-cam MMU_CAM_V; - e-pgsz = cr-cam MMU_CAM_PGSZ_MASK; - e-endian = cr-ram MMU_RAM_ENDIAN_MASK; - e-elsz = cr-ram MMU_RAM_ELSZ_MASK; - e-mixed= cr-ram MMU_RAM_MIXED; -} -EXPORT_SYMBOL_GPL(omap_iotlb_cr_to_e); - static inline int iotlb_cr_valid(struct cr_regs *cr) { if (!cr) @@ -593,13 +579,6 @@ size_t omap_dump_tlb_entries(struct omap_iommu *obj, char *buf, ssize_t bytes) } EXPORT_SYMBOL_GPL(omap_dump_tlb_entries); -int omap_foreach_iommu_device(void *data, int (*fn)(struct device *, void *)) -{ - return driver_for_each_device(omap_iommu_driver.driver, - NULL, data, fn); -} -EXPORT_SYMBOL_GPL(omap_foreach_iommu_device); - /* * H/W pagetable operations */ diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h index 6e9a8d2..2659139 100644 --- a/drivers/iommu/omap-iommu.h +++ b/drivers/iommu/omap-iommu.h @@ -190,14 +190,9 @@ static inline struct omap_iommu *dev_to_omap_iommu(struct device *dev) /* * global functions */ -extern void omap_iotlb_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e); - extern int omap_iopgtable_store_entry(struct omap_iommu *obj, struct iotlb_entry *e); -extern int omap_foreach_iommu_device(void *data, - int (*fn)(struct device *, void *)); - extern ssize_t omap_iommu_dump_ctx(struct omap_iommu *obj, char *buf, ssize_t len); extern size_t -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/17] iommu/omap: Remove duplicate declarations
The omap_iommu_save_ctx() and omap_iommu_restore_ctx() declarations are defined in include/linux/omap-iommu.h and do not belong in the internal drivers/iommu/omap-iommu.h header, so remove them. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h index 18a0f3a..4fc51c8 100644 --- a/drivers/iommu/omap-iommu.h +++ b/drivers/iommu/omap-iommu.h @@ -197,9 +197,6 @@ extern void omap_iotlb_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e); extern int omap_iopgtable_store_entry(struct omap_iommu *obj, struct iotlb_entry *e); -extern void omap_iommu_save_ctx(struct device *dev); -extern void omap_iommu_restore_ctx(struct device *dev); - extern int omap_foreach_iommu_device(void *data, int (*fn)(struct device *, void *)); -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/17] iommu/omap: Integrate omap-iommu-debug into omap-iommu
The debugfs support for OMAP IOMMU is currently implemented as a module, warranting certain OMAP-specific IOMMU API to be exported. The OMAP IOMMU, when enabled, can only be built-in into the kernel, so integrate the OMAP IOMMU debug module into the OMAP IOMMU driver. This helps in eliminating the need to export most of the current OMAP IOMMU API. The following are the main changes: - The OMAP_IOMMU_DEBUG Kconfig option is eliminated, and the OMAP IOMMU debugfs support is built alongside the OMAP IOMMU driver, and enabled automatically only when debugfs is enabled. - The debugfs directory and entry creation logic is reversed, the calls are invoked by the OMAP IOMMU driver now. - The current iffy circular logic of adding IOMMU archdata to the IOMMU devices itself to get a pointer to the omap_iommu object in the debugfs support code is replaced by directly using the omap_iommu structure while creating the debugfs entries. - The debugfs root directory is renamed from the generic name iommu to a specific name omap_iommu. - Unneeded headers have also been cleaned up while at this. - There will no longer be a omap-iommu-debug.ko module after this patch. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/Kconfig| 9 drivers/iommu/Makefile | 3 +- drivers/iommu/omap-iommu-debug.c | 102 --- drivers/iommu/omap-iommu.c | 11 +++-- drivers/iommu/omap-iommu.h | 7 +++ 5 files changed, 45 insertions(+), 87 deletions(-) diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index dd51122..9724d4a 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -143,15 +143,6 @@ config OMAP_IOMMU depends on ARCH_OMAP2PLUS select IOMMU_API -config OMAP_IOMMU_DEBUG - tristate Export OMAP IOMMU internals in DebugFS - depends on OMAP_IOMMU DEBUG_FS - help - Select this to see extensive information about - the internal state of OMAP IOMMU in debugfs. - - Say N unless you know you need this. - config TEGRA_IOMMU_GART bool Tegra GART IOMMU Support depends on ARCH_TEGRA_2x_SOC diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile index 18fa446..e0a4fed 100644 --- a/drivers/iommu/Makefile +++ b/drivers/iommu/Makefile @@ -10,8 +10,7 @@ obj-$(CONFIG_DMAR_TABLE) += dmar.o obj-$(CONFIG_INTEL_IOMMU) += iova.o intel-iommu.o obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o obj-$(CONFIG_IRQ_REMAP) += intel_irq_remapping.o irq_remapping.o -obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o -obj-$(CONFIG_OMAP_IOMMU_DEBUG) += omap-iommu-debug.o +obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o omap-iommu-debug.o obj-$(CONFIG_TEGRA_IOMMU_GART) += tegra-gart.o obj-$(CONFIG_TEGRA_IOMMU_SMMU) += tegra-smmu.o obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c index 28de657..54a0dc6 100644 --- a/drivers/iommu/omap-iommu-debug.c +++ b/drivers/iommu/omap-iommu-debug.c @@ -10,15 +10,11 @@ * published by the Free Software Foundation. */ -#include linux/module.h #include linux/err.h -#include linux/clk.h #include linux/io.h #include linux/slab.h #include linux/uaccess.h -#include linux/platform_device.h #include linux/debugfs.h -#include linux/omap-iommu.h #include linux/platform_data/iommu-omap.h #include omap-iopgtable.h @@ -31,8 +27,7 @@ static struct dentry *iommu_debug_root; static ssize_t debug_read_regs(struct file *file, char __user *userbuf, size_t count, loff_t *ppos) { - struct device *dev = file-private_data; - struct omap_iommu *obj = dev_to_omap_iommu(dev); + struct omap_iommu *obj = file-private_data; char *p, *buf; ssize_t bytes; @@ -55,8 +50,7 @@ static ssize_t debug_read_regs(struct file *file, char __user *userbuf, static ssize_t debug_read_tlb(struct file *file, char __user *userbuf, size_t count, loff_t *ppos) { - struct device *dev = file-private_data; - struct omap_iommu *obj = dev_to_omap_iommu(dev); + struct omap_iommu *obj = file-private_data; char *p, *buf; ssize_t bytes, rest; @@ -141,8 +135,7 @@ out: static ssize_t debug_read_pagetable(struct file *file, char __user *userbuf, size_t count, loff_t *ppos) { - struct device *dev = file-private_data; - struct omap_iommu *obj = dev_to_omap_iommu(dev); + struct omap_iommu *obj = file-private_data; char *p, *buf; size_t bytes; @@ -181,93 +174,58 @@ DEBUG_FOPS_RO(pagetable); #define __DEBUG_ADD_FILE(attr, mode) \ { \ struct dentry *dent;\ - dent = debugfs_create_file(#attr, mode, parent, \ -
[PATCH 10/17] iommu/omap: Fix the permissions on nr_tlb_entries
The permissions on the debugfs entry nr_tlb_entries should have been octal, not decimal, so fix it. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu-debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c index 0fb92aa..a520438 100644 --- a/drivers/iommu/omap-iommu-debug.c +++ b/drivers/iommu/omap-iommu-debug.c @@ -256,7 +256,7 @@ static int iommu_debug_register(struct device *dev, void *data) goto nomem; parent = d; - d = debugfs_create_u8(nr_tlb_entries, 400, parent, + d = debugfs_create_u8(nr_tlb_entries, 0400, parent, (u8 *)obj-nr_tlb_entries); if (!d) goto nomem; -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/17] OMAP IOMMU Cleanup Consolidation
Hi, The OMAP IOMMU driver is currently designed as three different pieces - a core OMAP IOMMU driver that implements the generic IOMMU API ops, a OMAP arch-specific layer that implements the OMAP specific arch iommu_functions, and an independent debugfs module. The former two are always built-in, while the last one is tristate. The driver supports OMAP2+ SoCs only, and this series performs a whole lot of cleanup and consolidates all the above pieces into a single built-in driver. The OMAP arch-specific layer is eliminated completely (absorbed into the core OMAP driver), and the debugfs portion is still maintained as a separate file, but not a separate module anymore. This cleanup and consolidation eliminates most of the currently exported functions from the OMAP IOMMU driver. The series also includes couple of fixes to the OMAP IOMMU debugfs entries - permissions on one debugfs entry, eliminate bus errors on debugfs access on disabled IOMMUs; and one enhancement to list all valid page table entries instead of listing only a page worth of data. The patches are baselined on 3.17-rc3 and on top of the OMAP IOMMU patches staged for 3.18 [1]; a reference branch with all these patches is available is hosted at [2]. I am posting this series for review with the intention to have them make it to 3.19, so I can repost once rc1 is out if needed. [1] http://git.kernel.org/cgit/linux/kernel/git/joro/iommu.git/log/?h=arm/omap [2] https://github.com/sumananna/omap-kernel/commits/iommu/submit/3.17-rc3-cleanup-consolidation regards Suman Suman Anna (17): iommu/omap: Remove refcount field from omap_iommu object iommu/omap: Remove unused isr_priv field from omap_iommu iommu/omap: Remove duplicate declarations iommu/omap: Remove conditional definition of dev_to_omap_iommu() iommu/omap: Remove ver debugfs entry iommu/omap: Remove omap_iommu_arch_version() and version field iommu/omap: Remove bogus version check in context save/restore iommu/omap: Simplify omap2_iommu_fault_isr() iommu/omap: Consolidate OMAP IOMMU modules iommu/omap: Fix the permissions on nr_tlb_entries iommu/omap: Make pagetable debugfs entry read-only iommu/omap: Integrate omap-iommu-debug into omap-iommu iommu/omap: Remove couple of unused exported functions iommu/omap: Do not export unneeded functions iommu/omap: Reset the domain field upon detaching iommu/omap: Fix bus error on debugfs access of unattached IOMMU iommu/omap: Switch pagetable debugfs entry to use seq_file drivers/iommu/Kconfig| 9 -- drivers/iommu/Makefile | 4 +- drivers/iommu/omap-iommu-debug.c | 244 drivers/iommu/omap-iommu.c | 304 ++- drivers/iommu/omap-iommu.h | 90 +-- drivers/iommu/omap-iommu2.c | 337 --- 6 files changed, 298 insertions(+), 690 deletions(-) delete mode 100644 drivers/iommu/omap-iommu2.c -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/17] iommu/omap: Remove ver debugfs entry
The debugfs entry 'ver' to read the OMAP IOMMU version is not much useful for developers, so it has been removed. The same can be deduced from the register dump, provided by the debugfs entry 'regs', REVISION register. This also allows us to remove the omap_iommu_arch_revision() which is currently returning a fixed value. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu-debug.c | 13 - 1 file changed, 13 deletions(-) diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c index 531658d..0fb92aa 100644 --- a/drivers/iommu/omap-iommu-debug.c +++ b/drivers/iommu/omap-iommu-debug.c @@ -30,17 +30,6 @@ static DEFINE_MUTEX(iommu_debug_lock); static struct dentry *iommu_debug_root; -static ssize_t debug_read_ver(struct file *file, char __user *userbuf, - size_t count, loff_t *ppos) -{ - u32 ver = omap_iommu_arch_version(); - char buf[MAXCOLUMN], *p = buf; - - p += sprintf(p, H/W version: %d.%d\n, (ver 4) 0xf , ver 0xf); - - return simple_read_from_buffer(userbuf, count, ppos, buf, p - buf); -} - static ssize_t debug_read_regs(struct file *file, char __user *userbuf, size_t count, loff_t *ppos) { @@ -228,7 +217,6 @@ static ssize_t debug_read_pagetable(struct file *file, char __user *userbuf, .llseek = generic_file_llseek, \ }; -DEBUG_FOPS_RO(ver); DEBUG_FOPS_RO(regs); DEBUG_FOPS_RO(tlb); DEBUG_FOPS(pagetable); @@ -273,7 +261,6 @@ static int iommu_debug_register(struct device *dev, void *data) if (!d) goto nomem; - DEBUG_ADD_FILE_RO(ver); DEBUG_ADD_FILE_RO(regs); DEBUG_ADD_FILE_RO(tlb); DEBUG_ADD_FILE(pagetable); -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/17] iommu/omap: Remove omap_iommu_arch_version() and version field
The function omap_iommu_arch_version() is not used anymore, and is not required either, so remove it. The .version field in struct iommu_functions that this function uses is also removed, as it is not really an ops to retrieve a version and there won't be any usage for this field either. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu.c | 9 - drivers/iommu/omap-iommu.h | 4 drivers/iommu/omap-iommu2.c | 2 -- 3 files changed, 15 deletions(-) diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c index ed26701..332bfb9 100644 --- a/drivers/iommu/omap-iommu.c +++ b/drivers/iommu/omap-iommu.c @@ -138,15 +138,6 @@ void omap_iommu_restore_ctx(struct device *dev) } EXPORT_SYMBOL_GPL(omap_iommu_restore_ctx); -/** - * omap_iommu_arch_version - Return running iommu arch version - **/ -u32 omap_iommu_arch_version(void) -{ - return arch_iommu-version; -} -EXPORT_SYMBOL_GPL(omap_iommu_arch_version); - static int iommu_enable(struct omap_iommu *obj) { int err; diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h index d7c5132..45fe67d 100644 --- a/drivers/iommu/omap-iommu.h +++ b/drivers/iommu/omap-iommu.h @@ -70,8 +70,6 @@ struct cr_regs { /* architecture specific functions */ struct iommu_functions { - unsigned long version; - int (*enable)(struct omap_iommu *obj); void (*disable)(struct omap_iommu *obj); void (*set_twl)(struct omap_iommu *obj, bool on); @@ -191,8 +189,6 @@ static inline struct omap_iommu *dev_to_omap_iommu(struct device *dev) /* * global functions */ -extern u32 omap_iommu_arch_version(void); - extern void omap_iotlb_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e); extern int diff --git a/drivers/iommu/omap-iommu2.c b/drivers/iommu/omap-iommu2.c index 5e1ea3b..2f6a9f7 100644 --- a/drivers/iommu/omap-iommu2.c +++ b/drivers/iommu/omap-iommu2.c @@ -297,8 +297,6 @@ static void omap2_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e) } static const struct iommu_functions omap2_iommu_ops = { - .version= IOMMU_ARCH_VERSION, - .enable = omap2_iommu_enable, .disable= omap2_iommu_disable, .set_twl= omap2_iommu_set_twl, -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/17] iommu/omap: Simplify omap2_iommu_fault_isr()
The function omap2_iommu_fault_isr() does an unnecessary recomputation of the return value. The logic relies on setting the same bit fields as the MMU fault error status bits, so simplify this function and remove the unneeded macros. These macros were originally exported to notify MMU faults to users prior to the IOMMU framework adaptation, but are now redundant. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu2.c | 20 +--- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/drivers/iommu/omap-iommu2.c b/drivers/iommu/omap-iommu2.c index 372141b..ce2fff3 100644 --- a/drivers/iommu/omap-iommu2.c +++ b/drivers/iommu/omap-iommu2.c @@ -53,13 +53,6 @@ ((pgsz) == MMU_CAM_PGSZ_64K) ? 0x :\ ((pgsz) == MMU_CAM_PGSZ_4K) ? 0xf000 : 0) -/* IOMMU errors */ -#define OMAP_IOMMU_ERR_TLB_MISS(1 0) -#define OMAP_IOMMU_ERR_TRANS_FAULT (1 1) -#define OMAP_IOMMU_ERR_EMU_MISS(1 2) -#define OMAP_IOMMU_ERR_TBLWALK_FAULT (1 3) -#define OMAP_IOMMU_ERR_MULTIHIT_FAULT (1 4) - static void __iommu_set_twl(struct omap_iommu *obj, bool on) { u32 l = iommu_read_reg(obj, MMU_CNTL); @@ -122,7 +115,6 @@ static void omap2_iommu_set_twl(struct omap_iommu *obj, bool on) static u32 omap2_iommu_fault_isr(struct omap_iommu *obj, u32 *ra) { u32 stat, da; - u32 errs = 0; stat = iommu_read_reg(obj, MMU_IRQSTATUS); stat = MMU_IRQ_MASK; @@ -134,19 +126,9 @@ static u32 omap2_iommu_fault_isr(struct omap_iommu *obj, u32 *ra) da = iommu_read_reg(obj, MMU_FAULT_AD); *ra = da; - if (stat MMU_IRQ_TLBMISS) - errs |= OMAP_IOMMU_ERR_TLB_MISS; - if (stat MMU_IRQ_TRANSLATIONFAULT) - errs |= OMAP_IOMMU_ERR_TRANS_FAULT; - if (stat MMU_IRQ_EMUMISS) - errs |= OMAP_IOMMU_ERR_EMU_MISS; - if (stat MMU_IRQ_TABLEWALKFAULT) - errs |= OMAP_IOMMU_ERR_TBLWALK_FAULT; - if (stat MMU_IRQ_MULTIHITFAULT) - errs |= OMAP_IOMMU_ERR_MULTIHIT_FAULT; iommu_write_reg(obj, stat, MMU_IRQSTATUS); - return errs; + return stat; } static void omap2_tlb_read_cr(struct omap_iommu *obj, struct cr_regs *cr) -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 17/17] iommu/omap: Switch pagetable debugfs entry to use seq_file
The debugfs entry 'pagetable' that shows the page table entry (PTE) data currently outputs only data that can be fit into a page. Switch the entry to use the seq_file interface so that it can show all the valid page table entries. The patch also corrected the output for L2 entries, and prints the proper L2 PTE instead of the previous L1 page descriptor pointer. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu-debug.c | 81 ++-- 1 file changed, 28 insertions(+), 53 deletions(-) diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c index e7b8aca..096947c5 100644 --- a/drivers/iommu/omap-iommu-debug.c +++ b/drivers/iommu/omap-iommu-debug.c @@ -85,95 +85,70 @@ static ssize_t debug_read_tlb(struct file *file, char __user *userbuf, return bytes; } -#define dump_ioptable_entry_one(lv, da, val) \ - ({ \ - int __err = 0; \ - ssize_t bytes; \ - const int maxcol = 22; \ - const char *str = %d: %08x %08x\n;\ - bytes = snprintf(p, maxcol, str, lv, da, val); \ - p += bytes; \ - len -= bytes; \ - if (len maxcol) \ - __err = -ENOMEM;\ - __err; \ - }) - -static ssize_t dump_ioptable(struct omap_iommu *obj, char *buf, ssize_t len) +static void dump_ioptable(struct seq_file *s) { - int i; - u32 *iopgd; - char *p = buf; + int i, j; + u32 da; + u32 *iopgd, *iopte; + struct omap_iommu *obj = s-private; spin_lock(obj-page_table_lock); iopgd = iopgd_offset(obj, 0); for (i = 0; i PTRS_PER_IOPGD; i++, iopgd++) { - int j, err; - u32 *iopte; - u32 da; - if (!*iopgd) continue; if (!(*iopgd IOPGD_TABLE)) { da = i IOPGD_SHIFT; - - err = dump_ioptable_entry_one(1, da, *iopgd); - if (err) - goto out; + seq_printf(s, 1: 0x%08x 0x%08x\n, da, *iopgd); continue; } iopte = iopte_offset(iopgd, 0); - for (j = 0; j PTRS_PER_IOPTE; j++, iopte++) { if (!*iopte) continue; da = (i IOPGD_SHIFT) + (j IOPTE_SHIFT); - err = dump_ioptable_entry_one(2, da, *iopgd); - if (err) - goto out; + seq_printf(s, 2: 0x%08x 0x%08x\n, da, *iopte); } } -out: - spin_unlock(obj-page_table_lock); - return p - buf; + spin_unlock(obj-page_table_lock); } -static ssize_t debug_read_pagetable(struct file *file, char __user *userbuf, - size_t count, loff_t *ppos) +static int debug_read_pagetable(struct seq_file *s, void *data) { - struct omap_iommu *obj = file-private_data; - char *p, *buf; - size_t bytes; + struct omap_iommu *obj = s-private; if (is_omap_iommu_detached(obj)) return -EPERM; - buf = (char *)__get_free_page(GFP_KERNEL); - if (!buf) - return -ENOMEM; - p = buf; - - p += sprintf(p, L: %8s %8s\n, da:, pa:); - p += sprintf(p, -\n); - mutex_lock(iommu_debug_lock); - bytes = PAGE_SIZE - (p - buf); - p += dump_ioptable(obj, p, bytes); - - bytes = simple_read_from_buffer(userbuf, count, ppos, buf, p - buf); + seq_printf(s, L: %8s %8s\n, da:, pte:); + seq_puts(s, --\n); + dump_ioptable(s); mutex_unlock(iommu_debug_lock); - free_page((unsigned long)buf); - return bytes; + return 0; } +#define DEBUG_SEQ_FOPS_RO(name) \ + static int debug_open_##name(struct inode *inode, struct file *file) \ + { \ + return single_open(file, debug_read_##name, inode-i_private); \ + } \ + \ + static const struct file_operations debug_##name##_fops = {\ + .open = debug_open_##name, \ +
[PATCH 04/17] iommu/omap: Remove conditional definition of dev_to_omap_iommu()
The dev_to_omap_iommu() is local to the OMAP IOMMU modules, and need not be defined conditionally. The CONFIG_IOMMU_API dependency check was added in the past to fix a compilation issue back when the header resided in the arch/arm layers, and is no longer needed. While at this, fix the header against double inclusion as well. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu.h | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h index 4fc51c8..d7c5132 100644 --- a/drivers/iommu/omap-iommu.h +++ b/drivers/iommu/omap-iommu.h @@ -10,6 +10,9 @@ * published by the Free Software Foundation. */ +#ifndef _OMAP_IOMMU_H +#define _OMAP_IOMMU_H + #if defined(CONFIG_ARCH_OMAP1) #error iommu for this processor not implemented yet #endif @@ -92,7 +95,6 @@ struct iommu_functions { ssize_t (*dump_ctx)(struct omap_iommu *obj, char *buf, ssize_t len); }; -#ifdef CONFIG_IOMMU_API /** * dev_to_omap_iommu() - retrieves an omap iommu object from a user device * @dev: iommu client device @@ -103,7 +105,6 @@ static inline struct omap_iommu *dev_to_omap_iommu(struct device *dev) return arch_data-iommu_dev; } -#endif /* * MMU Register offsets @@ -220,3 +221,5 @@ static inline void iommu_write_reg(struct omap_iommu *obj, u32 val, size_t offs) { __raw_writel(val, obj-regbase + offs); } + +#endif /* _OMAP_IOMMU_H */ -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 14/17] iommu/omap: Do not export unneeded functions
The following functions were exported previously for usage by the OMAP IOMMU debug module: omap_iommu_dump_ctx() omap_dump_tlb_entries() omap_iopgtable_store_entry() These functions need not be exported anymore as the OMAP IOMMU debugfs code is integrated with the OMAP IOMMU driver, and there won't be external users for these functions. So, remove the EXPORT_SYMBOL_GPL on these. The omap_iopgtable_store_entry() is also made internal only, after making the 'pagetable' debugfs entry read-only. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu.c | 6 ++ drivers/iommu/omap-iommu.h | 3 --- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c index b3bc087..a140873 100644 --- a/drivers/iommu/omap-iommu.c +++ b/drivers/iommu/omap-iommu.c @@ -509,7 +509,6 @@ ssize_t omap_iommu_dump_ctx(struct omap_iommu *obj, char *buf, ssize_t bytes) return bytes; } -EXPORT_SYMBOL_GPL(omap_iommu_dump_ctx); static int __dump_tlb_entries(struct omap_iommu *obj, struct cr_regs *crs, int num) @@ -577,7 +576,6 @@ size_t omap_dump_tlb_entries(struct omap_iommu *obj, char *buf, ssize_t bytes) return p - buf; } -EXPORT_SYMBOL_GPL(omap_dump_tlb_entries); /* * H/W pagetable operations @@ -760,7 +758,8 @@ iopgtable_store_entry_core(struct omap_iommu *obj, struct iotlb_entry *e) * @obj: target iommu * @e: an iommu tlb entry info **/ -int omap_iopgtable_store_entry(struct omap_iommu *obj, struct iotlb_entry *e) +static int +omap_iopgtable_store_entry(struct omap_iommu *obj, struct iotlb_entry *e) { int err; @@ -770,7 +769,6 @@ int omap_iopgtable_store_entry(struct omap_iommu *obj, struct iotlb_entry *e) prefetch_iotlb_entry(obj, e); return err; } -EXPORT_SYMBOL_GPL(omap_iopgtable_store_entry); /** * iopgtable_lookup_entry - Lookup an iommu pte entry diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h index 2659139..f32ddc3 100644 --- a/drivers/iommu/omap-iommu.h +++ b/drivers/iommu/omap-iommu.h @@ -190,9 +190,6 @@ static inline struct omap_iommu *dev_to_omap_iommu(struct device *dev) /* * global functions */ -extern int -omap_iopgtable_store_entry(struct omap_iommu *obj, struct iotlb_entry *e); - extern ssize_t omap_iommu_dump_ctx(struct omap_iommu *obj, char *buf, ssize_t len); extern size_t -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 15/17] iommu/omap: Reset the domain field upon detaching
The .domain field in omap_iommu struct is set properly when the OMAP IOMMU device is attached to, but is never reset properly on detach. Reset this properly so that the OMAP IOMMU debugfs logic can depend on this field before allowing the debugfs operations. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c index a140873..3769dc0 100644 --- a/drivers/iommu/omap-iommu.c +++ b/drivers/iommu/omap-iommu.c @@ -1202,6 +1202,7 @@ static void _omap_iommu_detach_dev(struct omap_iommu_domain *omap_domain, omap_domain-iommu_dev = arch_data-iommu_dev = NULL; omap_domain-dev = NULL; + oiommu-domain = NULL; } static void omap_iommu_detach_dev(struct iommu_domain *domain, -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/17] iommu/omap: Remove bogus version check in context save/restore
The omap2_iommu_save_ctx() and omap2_iommu_restore_ctx() performs a sanity version check against a fixed value that is correct only for OMAP2/OMAP3 IOMMUs. This fixed check does not scale for all OMAP2+ IOMMUs and is not absolutely required, so it has been removed. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu2.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/iommu/omap-iommu2.c b/drivers/iommu/omap-iommu2.c index 2f6a9f7..372141b 100644 --- a/drivers/iommu/omap-iommu2.c +++ b/drivers/iommu/omap-iommu2.c @@ -26,8 +26,6 @@ /* * omap2 architecture specific register bit definitions */ -#define IOMMU_ARCH_VERSION 0x0011 - /* IRQSTATUS IRQENABLE */ #define MMU_IRQ_MULTIHITFAULT (1 4) #define MMU_IRQ_TABLEWALKFAULT (1 3) @@ -268,8 +266,6 @@ static void omap2_iommu_save_ctx(struct omap_iommu *obj) p[i] = iommu_read_reg(obj, i * sizeof(u32)); dev_dbg(obj-dev, %s\t[%02d] %08x\n, __func__, i, p[i]); } - - BUG_ON(p[0] != IOMMU_ARCH_VERSION); } static void omap2_iommu_restore_ctx(struct omap_iommu *obj) @@ -281,8 +277,6 @@ static void omap2_iommu_restore_ctx(struct omap_iommu *obj) iommu_write_reg(obj, p[i], i * sizeof(u32)); dev_dbg(obj-dev, %s\t[%02d] %08x\n, __func__, i, p[i]); } - - BUG_ON(p[0] != IOMMU_ARCH_VERSION); } static void omap2_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e) -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 16/17] iommu/omap: Fix bus error on debugfs access of unattached IOMMU
Any debugfs access on an OMAP IOMMU that is not enabled (done during attach) results in a bus error due to access of registers without the clock or the reset enabled for the respective IOMMU. So, add a check to make sure the IOMMU is enabled/attached by a client device. This gracefully prints a Operation not permitted trace when the corresponding IOMMU is not enabled. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu-debug.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c index 54a0dc6..e7b8aca 100644 --- a/drivers/iommu/omap-iommu-debug.c +++ b/drivers/iommu/omap-iommu-debug.c @@ -24,6 +24,11 @@ static DEFINE_MUTEX(iommu_debug_lock); static struct dentry *iommu_debug_root; +static inline bool is_omap_iommu_detached(struct omap_iommu *obj) +{ + return !obj-domain; +} + static ssize_t debug_read_regs(struct file *file, char __user *userbuf, size_t count, loff_t *ppos) { @@ -31,6 +36,9 @@ static ssize_t debug_read_regs(struct file *file, char __user *userbuf, char *p, *buf; ssize_t bytes; + if (is_omap_iommu_detached(obj)) + return -EPERM; + buf = kmalloc(count, GFP_KERNEL); if (!buf) return -ENOMEM; @@ -54,6 +62,9 @@ static ssize_t debug_read_tlb(struct file *file, char __user *userbuf, char *p, *buf; ssize_t bytes, rest; + if (is_omap_iommu_detached(obj)) + return -EPERM; + buf = kmalloc(count, GFP_KERNEL); if (!buf) return -ENOMEM; @@ -139,6 +150,9 @@ static ssize_t debug_read_pagetable(struct file *file, char __user *userbuf, char *p, *buf; size_t bytes; + if (is_omap_iommu_detached(obj)) + return -EPERM; + buf = (char *)__get_free_page(GFP_KERNEL); if (!buf) return -ENOMEM; -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/17] iommu/omap: Consolidate OMAP IOMMU modules
The OMAP IOMMU driver was originally designed as modules, and split into a core module and a thin arch-specific module through the OMAP arch-specific struct iommu_functions, to scale for both OMAP1 and OMAP2+ IOMMU variants. The driver can only be built for OMAP2+ platforms currently, and also can only be built-in after the adaptation to generic IOMMU API. The OMAP1 variant was never added and will most probably be never added (the code for the only potential user, its parent, DSP processor has already been cleaned up). So, consolidate the OMAP2 specific omap-iommu2 module into the core OMAP IOMMU driver - this eliminates the arch-specific ops structure and simplifies the driver into a single module that only implements the generic IOMMU API's iommu_ops. The following are the main changes: - omap-iommu2 module is completely eliminated, with the common definitions moved to the internal omap-iommu.h, and the ops implementations moved into omap-iommu.c - OMAP arch-specific struct iommu_functions is also eliminated, with the ops implementations directly absorbed into the calling functions - iotlb_alloc_cr() is no longer inlined and defined only when PREFETCH_IOTLB is defined - iotlb_dump_cr() is similarly defined only when CONFIG_OMAP_IOMMU_DEBUG is defined - Elimination of the OMAP IOMMU exported functions to register the arch ops, omap_install_iommu_arch() omap_uninstall_iommu_arch() - Any stale comments about OMAP1 are also cleaned up Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/Makefile | 1 - drivers/iommu/omap-iommu.c | 263 ++--- drivers/iommu/omap-iommu.h | 61 + drivers/iommu/omap-iommu2.c | 311 4 files changed, 217 insertions(+), 419 deletions(-) delete mode 100644 drivers/iommu/omap-iommu2.c diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile index 16edef7..18fa446 100644 --- a/drivers/iommu/Makefile +++ b/drivers/iommu/Makefile @@ -11,7 +11,6 @@ obj-$(CONFIG_INTEL_IOMMU) += iova.o intel-iommu.o obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o obj-$(CONFIG_IRQ_REMAP) += intel_irq_remapping.o irq_remapping.o obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o -obj-$(CONFIG_OMAP_IOMMU) += omap-iommu2.o obj-$(CONFIG_OMAP_IOMMU_DEBUG) += omap-iommu-debug.o obj-$(CONFIG_TEGRA_IOMMU_GART) += tegra-gart.o obj-$(CONFIG_TEGRA_IOMMU_SMMU) += tegra-smmu.o diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c index 332bfb9..9ace5557 100644 --- a/drivers/iommu/omap-iommu.c +++ b/drivers/iommu/omap-iommu.c @@ -76,53 +76,23 @@ struct iotlb_lock { short vict; }; -/* accommodate the difference between omap1 and omap2/3 */ -static const struct iommu_functions *arch_iommu; - static struct platform_driver omap_iommu_driver; static struct kmem_cache *iopte_cachep; /** - * omap_install_iommu_arch - Install archtecure specific iommu functions - * @ops: a pointer to architecture specific iommu functions - * - * There are several kind of iommu algorithm(tlb, pagetable) among - * omap series. This interface installs such an iommu algorighm. - **/ -int omap_install_iommu_arch(const struct iommu_functions *ops) -{ - if (arch_iommu) - return -EBUSY; - - arch_iommu = ops; - return 0; -} -EXPORT_SYMBOL_GPL(omap_install_iommu_arch); - -/** - * omap_uninstall_iommu_arch - Uninstall archtecure specific iommu functions - * @ops: a pointer to architecture specific iommu functions - * - * This interface uninstalls the iommu algorighm installed previously. - **/ -void omap_uninstall_iommu_arch(const struct iommu_functions *ops) -{ - if (arch_iommu != ops) - pr_err(%s: not your arch\n, __func__); - - arch_iommu = NULL; -} -EXPORT_SYMBOL_GPL(omap_uninstall_iommu_arch); - -/** * omap_iommu_save_ctx - Save registers for pm off-mode support * @dev: client device **/ void omap_iommu_save_ctx(struct device *dev) { struct omap_iommu *obj = dev_to_omap_iommu(dev); + u32 *p = obj-ctx; + int i; - arch_iommu-save_ctx(obj); + for (i = 0; i (MMU_REG_SIZE / sizeof(u32)); i++) { + p[i] = iommu_read_reg(obj, i * sizeof(u32)); + dev_dbg(obj-dev, %s\t[%02d] %08x\n, __func__, i, p[i]); + } } EXPORT_SYMBOL_GPL(omap_iommu_save_ctx); @@ -133,20 +103,75 @@ EXPORT_SYMBOL_GPL(omap_iommu_save_ctx); void omap_iommu_restore_ctx(struct device *dev) { struct omap_iommu *obj = dev_to_omap_iommu(dev); + u32 *p = obj-ctx; + int i; - arch_iommu-restore_ctx(obj); + for (i = 0; i (MMU_REG_SIZE / sizeof(u32)); i++) { + iommu_write_reg(obj, p[i], i * sizeof(u32)); + dev_dbg(obj-dev, %s\t[%02d] %08x\n, __func__, i, p[i]); + } } EXPORT_SYMBOL_GPL(omap_iommu_restore_ctx); +static void __iommu_set_twl(struct omap_iommu *obj, bool on) +{ + u32 l = iommu_read_reg(obj, MMU_CNTL); + + if
Re: [PATCH] regmap: Allow read_reg_mask to be 0
Lars On 09/30/2014 04:03 PM, Lars-Peter Clausen wrote: On 09/30/2014 06:07 PM, Dan Murphy wrote: There may be spi devices that do not require a register read mask to read the registers. Currently the code sets the read mask based on a non-zero value passed in from the driver or if that value is 0 sets the read mask to 0x80. It only sets it to the bus default if both read_flag_mask and write_flag_mask are 0. The assumption is that both of them being zero is a invalid configuration and either of them (or both) have to be non-zero for proper operation, since otherwise the device can't tell the difference between a read and a write. Do you have a device where both the read and the write mask is 0? - Lars Yes I do have a device that the read/write mask are both 0. The device, which is already in production, has a specific control register that sets either the reading or writing of the rest of the registers. Here is the data sheet http://www.ti.com/lit/ds/symlink/afe4403.pdf See page 61 control0. Driver is written for this part just want to get this lead patch in or maybe an alternate solution. Dan -- -- Dan Murphy -- 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 01/17] iommu/omap: Remove refcount field from omap_iommu object
The refcount field in omap_iommu object is primarily used to check if an IOMMU device has already been enabled, but this is already implicit in the omap_iommu_attach_dev() which ensures that only a single device can attach to an IOMMU. This field is redundant, and so has been cleaned up. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu.c | 15 +++ drivers/iommu/omap-iommu.h | 1 - 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c index 4b432c4..ed26701 100644 --- a/drivers/iommu/omap-iommu.c +++ b/drivers/iommu/omap-iommu.c @@ -819,8 +819,9 @@ static irqreturn_t iommu_fault_handler(int irq, void *data) u32 *iopgd, *iopte; struct omap_iommu *obj = data; struct iommu_domain *domain = obj-domain; + struct omap_iommu_domain *omap_domain = domain-priv; - if (!obj-refcount) + if (!omap_domain-iommu_dev) return IRQ_NONE; errs = iommu_report_fault(obj, da); @@ -880,13 +881,6 @@ static struct omap_iommu *omap_iommu_attach(const char *name, u32 *iopgd) spin_lock(obj-iommu_lock); - /* an iommu device can only be attached once */ - if (++obj-refcount 1) { - dev_err(dev, %s: already attached!\n, obj-name); - err = -EBUSY; - goto err_enable; - } - obj-iopgd = iopgd; err = iommu_enable(obj); if (err) @@ -899,7 +893,6 @@ static struct omap_iommu *omap_iommu_attach(const char *name, u32 *iopgd) return obj; err_enable: - obj-refcount--; spin_unlock(obj-iommu_lock); return ERR_PTR(err); } @@ -915,9 +908,7 @@ static void omap_iommu_detach(struct omap_iommu *obj) spin_lock(obj-iommu_lock); - if (--obj-refcount == 0) - iommu_disable(obj); - + iommu_disable(obj); obj-iopgd = NULL; spin_unlock(obj-iommu_lock); diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h index 4f1b68c..5c14000 100644 --- a/drivers/iommu/omap-iommu.h +++ b/drivers/iommu/omap-iommu.h @@ -33,7 +33,6 @@ struct omap_iommu { void*isr_priv; struct iommu_domain *domain; - unsigned intrefcount; spinlock_t iommu_lock; /* global for this whole object */ /* -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/17] iommu/omap: Remove unused isr_priv field from omap_iommu
The isr_priv field is a left-over from before the IOMMU API adaptation, this was used to store the callback data. This is no longer relevant, so remove it. Signed-off-by: Suman Anna s-a...@ti.com --- drivers/iommu/omap-iommu.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h index 5c14000..18a0f3a 100644 --- a/drivers/iommu/omap-iommu.h +++ b/drivers/iommu/omap-iommu.h @@ -30,7 +30,6 @@ struct omap_iommu { const char *name; void __iomem*regbase; struct device *dev; - void*isr_priv; struct iommu_domain *domain; spinlock_t iommu_lock; /* global for this whole object */ -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv6 0/5] hwspinlock core/omap dt support
Hi Bjorn, On 09/30/2014 03:54 PM, Bjorn Andersson wrote: On Fri, Sep 12, 2014 at 1:24 PM, Suman Anna s-a...@ti.com wrote: Hi Ohad, This is an update to the hwspinlock dt support series. The series is rebased onto v3.17-rc3, and addresses the review comments on the previous v5 series. I have also split and left out the RFC patches about the support for reserved locks (will post these as a separate series) and return code convention changes in the hwspinlock core (will not be needed anymore). The support for deferred probing of clients is supported in the new of_hwspin_lock_get_id() function itself. Thanks for your reply to me, I had missed that you continued this work. I find it somewhat awkward to have to call both of_hwspin_lock_get_id() and then hwspin_lock_request_specific(), but I found the request from Ohad, so let's stick with it. Am I right that hwlock-num-locks and hwlock-base-id are optional from the frameworks perspective and only there to aid the hwspin drivers? If so it is strange to have in the common binding and have the helper functions in the core for simply reading hwspin device specific properties. The hwlock-num-locks and hwlock-base-id would be common features to all the hwspinlock drivers, so they are added as common bindings which the individual implementations should use instead of defining their own properties. These are added based on discussion way back on v1. You ought to replace the qcom,num-locks with the first one. I will respond to your v4 with a few comments so that we don't loose the context in that thread. Otherwise I think it looks sane, although I haven't spend that much time reviewing it. I did throw it into my tree and gave it a testrun with the Qualcomm code I've been working on. So for the non-omap parts: Tested-by: Bjorn Andersson bjorn.anders...@sonymobile.com Thanks for testing this with the new Qualcomm driver. regards Suman -- 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] regmap: Allow read_reg_mask to be 0
On 09/30/2014 11:18 PM, Dan Murphy wrote: Lars On 09/30/2014 04:03 PM, Lars-Peter Clausen wrote: On 09/30/2014 06:07 PM, Dan Murphy wrote: There may be spi devices that do not require a register read mask to read the registers. Currently the code sets the read mask based on a non-zero value passed in from the driver or if that value is 0 sets the read mask to 0x80. It only sets it to the bus default if both read_flag_mask and write_flag_mask are 0. The assumption is that both of them being zero is a invalid configuration and either of them (or both) have to be non-zero for proper operation, since otherwise the device can't tell the difference between a read and a write. Do you have a device where both the read and the write mask is 0? - Lars Yes I do have a device that the read/write mask are both 0. The device, which is already in production, has a specific control register that sets either the reading or writing of the rest of the registers. Here is the data sheet http://www.ti.com/lit/ds/symlink/afe4403.pdf See page 61 control0. Driver is written for this part just want to get this lead patch in or maybe an alternate solution. Looking at this the generic SPI regmap implementation might not necessarily be the best thing to use here and you are probably better of implementing either your own regmap bus or reg_read/reg_write callbacks that automatically set/clear the SPI_READ bit in the control register depending on the operation. - Lars -- 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