Re: [PATCH v3 1/3] mailbox/omap: Add ti,mbox-send-noirq quirk to fix AM33xx CPU Idle
On 10/21/2015 11:42 PM, Jassi Brar wrote: > On 22 October 2015 at 05:35, Suman Anna wrote: > >>> Anyways I am OK too, if you guys want to fix it with a platform >>> specific quirk. Let me know I'll pick this patch. >> >> I haven't gotten a chance to try #1, and I won't be able to look at it >> atleast for another month. I suggest that you go ahead and pick this >> patch up, as a quirk is needed in one form or the other for #2, and #1 >> is anyways a bigger change that will affect all our IPC stacks across >> all pairs of MPU - remote processors on different SoCs. >> > Yeah that is the reason I offered to pick this patch as such. > OK I'll take this patch. Thanks a lot. We will be a lot closer to get PM working on AM335/AM437x SoCs. regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] mailbox/omap: Add ti,mbox-send-noirq quirk to fix AM33xx CPU Idle
Hi Jassi, On 10/20/2015 11:44 PM, Jassi Brar wrote: > On Wed, Sep 23, 2015 at 5:44 AM, Dave Gerlach wrote: >> The mailbox framework controls the transmission queue and requires >> either its controller implementations or clients to run the state >> machine for the Tx queue. The OMAP mailbox controller uses a Tx-ready >> interrupt as the equivalent of a Tx-done interrupt to run this Tx >> queue state-machine. >> >> The WkupM3 processor on AM33xx and AM43xx SoCs is used to offload >> certain PM tasks, like doing the necessary operations for Device >> PM suspend/resume or for entering lower c-states during cpuidle. >> >> The CPUIdle on AM33xx requires the messages to be sent without >> having to trigger the Tx-ready interrupts, as the interrupt >> would immediately terminate the CPUIdle operation. Support for >> this has been added by introducing a DT quirk, "ti,mbox-send-noirq" >> and using it to modify the normal OMAP mailbox controller behavior >> on the sub-mailboxes used to communicate with the WkupM3 remote >> processor. This also requires the wkup_m3_ipc driver to adjust >> its mailbox usage logic to run the Tx state machine. >> > Probably Suman already updated you on what was discussed about this > patch at Connect-SFO. My suggestion was to drive TX poll-based (I > know, I know...) because the "interrupt-driven" is just an impression, > it is not really. You get the interrupt as soon as you enable it > because it is the "FIFO not-full" interrupt which is always because > there is always space left after writing to the fifo. The cons are > that it buffers messages hidden from the client while abusing the irq. > If you guys could break away from the "interrupt-driven" illusion and > use polling which it actually is, everything falls into place and you > avoid the "ti,mbox-send-noirq" quirk. Looking at this closely, even with that approach, we would still need a quirk to deal with the weird behavior of our wakeup m3. Essentially we have two independent things: 1. Tx state machine (currently interrupt driven) for ticking the mailbox core tx state machine. 2. A quirk that we need for dealing with Wakeup M3 behavior, MPU needs to take out the message on behalf of WkupM3 processor after transmission and clearing the IP-level interrupt intended for WkupM3. This will be irrespective of the Tx state machine choice, and this needs to be different only on the specific mbox channel used to talk with WkupM3 processor. Previously, we were doing #2 in the OMAP mailbox regular interrupt handler (handles both Rx and Tx interrupts) as part of the Rx interrupt handler with the rx mbox variables reflecting those of the WkupM3 processor. But when we remove the interrupt processing, we now actually need to add code to deal with this. > Anyways I am OK too, if you guys want to fix it with a platform > specific quirk. Let me know I'll pick this patch. I haven't gotten a chance to try #1, and I won't be able to look at it atleast for another month. I suggest that you go ahead and pick this patch up, as a quirk is needed in one form or the other for #2, and #1 is anyways a bigger change that will affect all our IPC stacks across all pairs of MPU - remote processors on different SoCs. regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REPOST PATCH 1/4] ARM: dts: DRA7: Add dsp1_system syscon node
Hi Tony,
On 10/12/2015 04:43 PM, Tony Lindgren wrote:
> * Suman Anna [151002 16:27]:
>> The DSP_SYSTEM sub-module is a dedicated system control logic
>> module present within a DRA7 DSP processor sub-system. This
>> module is responsible for power management, clock generation
>> and connection to the device PRCM module.
>>
>> Add a syscon node for this module for the DSP1 processor
>> sub-system. This is added as a syscon node as it is a common
>> configuration module that can be used by the different IOMMU
>> instances and the corresponding remoteproc device.
>>
>> The node is added to the common dra7.dtsi file, as the DSP1
>> processor sub-system is mostly common across all the variants
>> of the DRA7 SoC family.
>>
>> Signed-off-by: Suman Anna
>> ---
>> arch/arm/boot/dts/dra7.dtsi | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
>> index e289c706d27d..62055094e8d5 100644
>> --- a/arch/arm/boot/dts/dra7.dtsi
>> +++ b/arch/arm/boot/dts/dra7.dtsi
>> @@ -292,6 +292,11 @@
>> #thermal-sensor-cells = <1>;
>> };
>>
>> +dsp1_system: dsp_system@40d0 {
>> +compatible = "syscon";
>> +reg = <0x40d0 0x100>;
>> +};
>> +
>> sdma: dma-controller@4a056000 {
>> compatible = "ti,omap4430-sdma";
>> reg = <0x4a056000 0x1000>;
>
> Hmm so why would you want to set up a complete device as a syscon
> mapping rather than just doing ioremap on it?
>
> What drivers will be sharing access to these registers?
Two different instances of the MMU for now, both get probed
independently. But there are other registers which a remoteproc driver
will mostly be interested in (like DSP_SYS_STAT for knowing the C66x
idle/active status).
regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/4] ARM: dts: DRA7: Add timer12 node
On 10/06/2015 02:52 AM, Tony Lindgren wrote: > * Felipe Balbi [151005 17:51]: >> >> according to Tony we should avoid using status at all for in-SoC >> devices. >> >> Tony, can you confirm I understood you correctly ? > > Yes. With status = "disabled" kernel completely ignores the > device and struct device is not created at all even with the > device being there. In general we're better off trying to > probe the device and idle it. > > The only time we really want to mark something with > status = "disabled" is if some coprocessor firmware is > using that device and the kernel should not touch it at > all. Not always, since some of the PM clocking logic depends on the state machine variables within the kernel. We are also using this to deal with paper-spins (atleast in the DRA7 case) and the DTS include model, wherein certain instances may not be present on all variations of the SoC, and enabled specifically on the instances that matter. Obviously, it could be done the other way too, but as far as what Nishanth mentioned sometime back, we are following the former for DRA7. In anycase, the status property on the Timer12 node can be removed, it doesn't fall into the above category, and we are fixing it up properly on HS devices in the kernel. regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] ARM: dts: DRA7: Add timer12 node
Add the DT node for Timer12 present on DRA7 family of
SoCs. Timer12 is present in PD_WKUPAON power domain, and
has the same capabilities as the other timers, except for
the fact that it serves as a secure timer on HS devices
and is clocked only from the secure 32K clock.
The node is marked disabled for now, and the kernel should
refrain from using this secure timer on HS devices.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra7.dtsi | 10 ++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index e289c706d27d..37d632dad576 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -762,6 +762,16 @@
ti,hwmods = "timer11";
};
+ timer12: timer@4ae2 {
+ compatible = "ti,omap5430-timer";
+ reg = <0x4ae2 0x80>;
+ interrupts = ;
+ ti,hwmods = "timer12";
+ ti,timer-alwon;
+ ti,timer-secure;
+ status = "disabled";
+ };
+
timer13: timer@48828000 {
compatible = "ti,omap5430-timer";
reg = <0x48828000 0x80>;
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] ARM: DRA7: hwmod: Add data for GPTimer 12
Add the hwmod data for GPTimer 12. GPTimer 12 is present in
WKUPAON power domain and is clocked from a secure 32K clock.
GPTimer 12 serves as a secure timer on HS devices, but is
available for kernel on regular GP devices.
The hwmod link is registered only on GP devices. The hwmod data
also reused the existing timer class instead of reintroducing
the identical dra7xx_timer_secure_sysc class which was dropped
in commit edec17863362 ("ARM: DRA7: hwmod: Fix the hwmod class
for GPTimer4").
Signed-off-by: Suman Anna
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 36 +--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index 562247bced49..37a10f87fbcd 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1929,6 +1929,20 @@ static struct omap_hwmod dra7xx_timer11_hwmod = {
},
};
+/* timer12 */
+static struct omap_hwmod dra7xx_timer12_hwmod = {
+ .name = "timer12",
+ .class = &dra7xx_timer_hwmod_class,
+ .clkdm_name = "wkupaon_clkdm",
+ .main_clk = "secure_32k_clk_src_ck",
+ .prcm = {
+ .omap4 = {
+ .clkctrl_offs =
DRA7XX_CM_WKUPAON_TIMER12_CLKCTRL_OFFSET,
+ .context_offs =
DRA7XX_RM_WKUPAON_TIMER12_CONTEXT_OFFSET,
+ },
+ },
+};
+
/* timer13 */
static struct omap_hwmod dra7xx_timer13_hwmod = {
.name = "timer13",
@@ -3135,6 +3149,14 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per1__timer11
= {
.user = OCP_USER_MPU | OCP_USER_SDMA,
};
+/* l4_wkup -> timer12 */
+static struct omap_hwmod_ocp_if dra7xx_l4_wkup__timer12 = {
+ .master = &dra7xx_l4_wkup_hwmod,
+ .slave = &dra7xx_timer12_hwmod,
+ .clk= "wkupaon_iclk_mux",
+ .user = OCP_USER_MPU | OCP_USER_SDMA,
+};
+
/* l4_per3 -> timer13 */
static struct omap_hwmod_ocp_if dra7xx_l4_per3__timer13 = {
.master = &dra7xx_l4_per3_hwmod,
@@ -3429,6 +3451,13 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[]
__initdata = {
NULL,
};
+/* GP-only hwmod links */
+static struct omap_hwmod_ocp_if *dra7xx_gp_hwmod_ocp_ifs[] __initdata = {
+ &dra7xx_l4_wkup__timer12,
+ NULL,
+};
+
+/* SoC variant specific hwmod links */
static struct omap_hwmod_ocp_if *dra74x_hwmod_ocp_ifs[] __initdata = {
&dra7xx_l4_per3__usb_otg_ss4,
NULL,
@@ -3446,9 +3475,12 @@ int __init dra7xx_hwmod_init(void)
ret = omap_hwmod_register_links(dra7xx_hwmod_ocp_ifs);
if (!ret && soc_is_dra74x())
- return omap_hwmod_register_links(dra74x_hwmod_ocp_ifs);
+ ret = omap_hwmod_register_links(dra74x_hwmod_ocp_ifs);
else if (!ret && soc_is_dra72x())
- return omap_hwmod_register_links(dra72x_hwmod_ocp_ifs);
+ ret = omap_hwmod_register_links(dra72x_hwmod_ocp_ifs);
+
+ if (!ret && omap_type() == OMAP2_DEVICE_TYPE_GP)
+ ret = omap_hwmod_register_links(dra7xx_gp_hwmod_ocp_ifs);
return ret;
}
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] ARM: OMAP2+: timer: Remove secure timer for DRA7xx HS devices
Timer 12 on DRA7 SoCs is reserved for secure usage on high-secure (HS)
devices. The timer cannot be used by the kernel on HS devices, but is
available on regular general purpose (GP) devices. This is similar to the
behavior on OMAP3 devices, so extend the logic used in commit ad24bde8f102
("ARM: OMAP3: Dynamically disable secure timer nodes for secure devices")
to remove the secure timer on DRA7xx SoCs at run-time based on the SoC
device type.
Signed-off-by: Suman Anna
---
arch/arm/mach-omap2/timer.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index a55655127ef2..1e346aa0a687 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -193,8 +193,8 @@ static struct device_node * __init omap_get_timer_dt(const
struct of_device_id *
/**
* omap_dmtimer_init - initialisation function when device tree is used
*
- * For secure OMAP3 devices, timers with device type "timer-secure" cannot
- * be used by the kernel as they are reserved. Therefore, to prevent the
+ * For secure OMAP3/DRA7xx devices, timers with device type "timer-secure"
+ * cannot be used by the kernel as they are reserved. Therefore, to prevent the
* kernel registering these devices remove them dynamically from the device
* tree on boot.
*/
@@ -202,7 +202,7 @@ static void __init omap_dmtimer_init(void)
{
struct device_node *np;
- if (!cpu_is_omap34xx())
+ if (!cpu_is_omap34xx() && !soc_is_dra7xx())
return;
/* If we are a secure device, remove any secure timer nodes */
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/4] ARM: OMAP: dmtimer: check for fixed timers during config
The omap_dm_timer_set_source() function provides a means for client
users to configure the mux parent for a GPTimer's functional clock.
However, not all timers are configurable (Eg: Timer12 on DRA7 is fed
by an internal 32k oscillator clock, and does not have configurable
parent clocks). So, check for such cases and proceed with out throwing
an error.
Signed-off-by: Suman Anna
---
arch/arm/plat-omap/dmtimer.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 8ca94d379bc3..4c48b52c4a7c 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -36,6 +36,7 @@
*/
#include
+#include
#include
#include
#include
@@ -504,6 +505,12 @@ int omap_dm_timer_set_source(struct omap_dm_timer *timer,
int source)
if (IS_ERR(timer->fclk))
return -EINVAL;
+#if defined(CONFIG_COMMON_CLK)
+ /* Check if the clock has configurable parents */
+ if (clk_hw_get_num_parents(__clk_get_hw(timer->fclk)) < 2)
+ return 0;
+#endif
+
switch (source) {
case OMAP_TIMER_SRC_SYS_CLK:
parent_name = "timer_sys_ck";
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/4] DRA7 Timer12 Support
Hi Tony, This is a revised version of the DRA7 Timer12 support patch series [1]. The series mainly is respun to fix a build issue caused by Patch1 [2] with omap1_defconfig due to the absence of Common Clock framework for OMAP1. Only Patch 1 is modified from the previous series. I have also revised the check slightly to fix another issue seen when requesting Timer12 on OMAP3, as that is a fixed clock and does return back with a parent count of 1. Series baselined on v4.3-rc3 just like the original series, and tested on all generations (AMx3x, OMAP4, OMAP5 and DRA7) except OMAP1 and OMAP2 generations. regards Suman [1] http://marc.info/?l=linux-omap&m=144372799601354&w=2 [2] https://patchwork.kernel.org/patch/7311021/ Suman Anna (4): ARM: OMAP: dmtimer: check for fixed timers during config ARM: OMAP2+: timer: Remove secure timer for DRA7xx HS devices ARM: dts: DRA7: Add timer12 node ARM: DRA7: hwmod: Add data for GPTimer 12 arch/arm/boot/dts/dra7.dtsi | 10 + arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 36 +-- arch/arm/mach-omap2/timer.c | 6 +++--- arch/arm/plat-omap/dmtimer.c | 7 ++ 4 files changed, 54 insertions(+), 5 deletions(-) -- 2.6.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] ARM: OMAP: dmtimer: check for fixed timers during config
On 10/05/2015 11:58 AM, Tony Lindgren wrote: > * Suman Anna [151005 09:47]: >> Tony, >> >> On 10/03/2015 12:29 PM, kbuild test robot wrote: >>> Hi Suman, >>> >>> [auto build test results on v4.3-rc3 -- if it's inappropriate base, please >>> ignore] >>> >>> config: arm-omap1_defconfig (attached as .config) >>> reproduce: >>> wget >>> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross >>> -O ~/bin/make.cross >>> chmod +x ~/bin/make.cross >>> # save the attached .config to linux build tree >>> make.cross ARCH=arm >>> >>> All error/warnings (new ones prefixed by >>): >>> >>>arch/arm/plat-omap/dmtimer.c: In function 'omap_dm_timer_set_source': >>>>> arch/arm/plat-omap/dmtimer.c:509:2: error: implicit declaration of >>>>> function 'clk_hw_get_num_parents' [-Werror=implicit-function-declaration] >>> if (!clk_hw_get_num_parents(__clk_get_hw(timer->fclk))) >>> ^ >>>>> arch/arm/plat-omap/dmtimer.c:509:2: error: implicit declaration of >>>>> function '__clk_get_hw' [-Werror=implicit-function-declaration] >>>cc1: some warnings being treated as errors >>> >>> vim +/clk_hw_get_num_parents +509 arch/arm/plat-omap/dmtimer.c >>> >>>503 return pdata->set_timer_src(timer->pdev, >>> source); >>>504 >>>505 if (IS_ERR(timer->fclk)) >>>506 return -EINVAL; >>>507 >>>508 /* Check if the clock has parents if not no point >>> checking */ >>> > 509 if (!clk_hw_get_num_parents(__clk_get_hw(timer->fclk))) >>>510 return 0; >> >> CONFIG_COMMON_CLK is not defined for OMAP1. So, are you ok if I enclose >> this within #ifdef CONFIG_COMMON_CLK? > > Yes, or maybe you can just clk_get_rate() on the 32k oscillator? > > Boards with no 32k oscillator should set the rate for that 0 zero. Well, this API is about setting the timer clk's mux source, and not really dealing with the clk rate or just setting it to the 32k oscillator. A 32k is usually one of the mux parents (if the timer has them). If there is a way to find if the clk is a mux clock, then that check would be ideal. But since no such thing exists, so I will go ahead and fix this by adding the #ifdef check. regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] ARM: OMAP: dmtimer: check for fixed timers during config
Tony, On 10/03/2015 12:29 PM, kbuild test robot wrote: > Hi Suman, > > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please > ignore] > > config: arm-omap1_defconfig (attached as .config) > reproduce: > wget > https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross > -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=arm > > All error/warnings (new ones prefixed by >>): > >arch/arm/plat-omap/dmtimer.c: In function 'omap_dm_timer_set_source': >>> arch/arm/plat-omap/dmtimer.c:509:2: error: implicit declaration of function >>> 'clk_hw_get_num_parents' [-Werror=implicit-function-declaration] > if (!clk_hw_get_num_parents(__clk_get_hw(timer->fclk))) > ^ >>> arch/arm/plat-omap/dmtimer.c:509:2: error: implicit declaration of function >>> '__clk_get_hw' [-Werror=implicit-function-declaration] >cc1: some warnings being treated as errors > > vim +/clk_hw_get_num_parents +509 arch/arm/plat-omap/dmtimer.c > >503return pdata->set_timer_src(timer->pdev, > source); >504 >505if (IS_ERR(timer->fclk)) >506return -EINVAL; >507 >508/* Check if the clock has parents if not no point > checking */ > > 509if (!clk_hw_get_num_parents(__clk_get_hw(timer->fclk))) >510return 0; CONFIG_COMMON_CLK is not defined for OMAP1. So, are you ok if I enclose this within #ifdef CONFIG_COMMON_CLK? regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REPOST PATCH 1/4] ARM: dts: DRA7: Add dsp1_system syscon node
The DSP_SYSTEM sub-module is a dedicated system control logic
module present within a DRA7 DSP processor sub-system. This
module is responsible for power management, clock generation
and connection to the device PRCM module.
Add a syscon node for this module for the DSP1 processor
sub-system. This is added as a syscon node as it is a common
configuration module that can be used by the different IOMMU
instances and the corresponding remoteproc device.
The node is added to the common dra7.dtsi file, as the DSP1
processor sub-system is mostly common across all the variants
of the DRA7 SoC family.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra7.dtsi | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index e289c706d27d..62055094e8d5 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -292,6 +292,11 @@
#thermal-sensor-cells = <1>;
};
+ dsp1_system: dsp_system@40d0 {
+ compatible = "syscon";
+ reg = <0x40d0 0x100>;
+ };
+
sdma: dma-controller@4a056000 {
compatible = "ti,omap4430-sdma";
reg = <0x4a056000 0x1000>;
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REPOST PATCH 0/4] Add DRA7 IOMMU DT nodes
Hi Tony, The following series is a repost of the previous seried that added the basic DT nodes for the DSP and IPU IOMMU devices for the DRA7xx SoC family [1]. There are no code changes, but the patches have to be rebased to resolve slight merge conflicts as the dra7_ctrl_core and dra7_ctrl_general nodes have been removed since the last submission. This patch series is pending based on the equivalent bindings update series [2] which has also been reposted without any changes. Patches are based on 4.3-rc3. regards Suman [1] http://marc.info/?l=linux-omap&m=143752332808524&w=2 [2] http://marc.info/?l=linux-omap&m=144382697928741&w=2 Suman Anna (4): ARM: dts: DRA7: Add dsp1_system syscon node ARM: dts: DRA74x: Add dsp2_system syscon node ARM: dts: DRA7: Add common IOMMU nodes ARM: dts: DRA74x: Add IOMMU nodes for DSP2 arch/arm/boot/dts/dra7.dtsi | 45 +++ arch/arm/boot/dts/dra74x.dtsi | 25 2 files changed, 70 insertions(+) -- 2.6.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REPOST PATCH 2/4] ARM: dts: DRA74x: Add dsp2_system syscon node
The DSP_SYSTEM sub-module is a dedicated system control logic
module present within a DRA7 DSP processor sub-system. This
module is responsible for power management, clock generation
and connection to the device PRCM module.
Add a syscon node for this module for the DSP2 processor
sub-system. This is added as a syscon node as it is a common
configuration module that can be used by the different IOMMU
instances and the corresponding remoteproc device.
The node is added to the dra74x.dtsi file, as the DSP2 processor
subsystem is usually present only on the DRA74x variants of the
DRA7 SoC family.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra74x.dtsi | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/dra74x.dtsi b/arch/arm/boot/dts/dra74x.dtsi
index feea98e0a4b5..dfa29b7ba86a 100644
--- a/arch/arm/boot/dts/dra74x.dtsi
+++ b/arch/arm/boot/dts/dra74x.dtsi
@@ -52,6 +52,11 @@
};
ocp {
+ dsp2_system: dsp_system@4150 {
+ compatible = "syscon";
+ reg = <0x4150 0x100>;
+ };
+
omap_dwc3_4: omap_dwc3_4@4894 {
compatible = "ti,dwc3";
ti,hwmods = "usb_otg_ss4";
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REPOST PATCH 4/4] ARM: dts: DRA74x: Add IOMMU nodes for DSP2
The DRA74x family of SoCs have a second DSP, that also has
two MMUs just like the DSP1 subsystem. Add the IOMMU nodes
for this DSP2 subsystem in disabled state to the DRA74x
specific DTS file, the nodes would need to be enabled
appropriately in the respective board DTS files.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra74x.dtsi | 20
1 file changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/dra74x.dtsi b/arch/arm/boot/dts/dra74x.dtsi
index dfa29b7ba86a..2f7d313e91a0 100644
--- a/arch/arm/boot/dts/dra74x.dtsi
+++ b/arch/arm/boot/dts/dra74x.dtsi
@@ -81,6 +81,26 @@
dr_mode = "otg";
};
};
+
+ mmu0_dsp2: mmu@41501000 {
+ compatible = "ti,dra7-dsp-iommu";
+ reg = <0x41501000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu0_dsp2";
+ #iommu-cells = <0>;
+ ti,syscon-mmuconfig = <&dsp2_system 0x0>;
+ status = "disabled";
+ };
+
+ mmu1_dsp2: mmu@41502000 {
+ compatible = "ti,dra7-dsp-iommu";
+ reg = <0x41502000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu1_dsp2";
+ #iommu-cells = <0>;
+ ti,syscon-mmuconfig = <&dsp2_system 0x1>;
+ status = "disabled";
+ };
};
};
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REPOST PATCH 3/4] ARM: dts: DRA7: Add common IOMMU nodes
The DRA7xx family of SOCs have two IPUs and one DSP processor
subsystems in common. The IOMMU DT nodes have been added for
these processor subsystems, and have been disabled by default.
These MMUs are very similar to those on OMAP4 and OMAP5, with
the only difference being the presence of a second MMU within
the DSP subsystem for the EDMA port. The DSP IOMMUs also need
an additional 'ti,syscon-mmuconfig' property compared to the
IPU IOMMUs.
NOTE: The enabling of these nodes is left to the respective
board dts files.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra7.dtsi | 40
1 file changed, 40 insertions(+)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 62055094e8d5..9f6bafcad385 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -916,6 +916,46 @@
status = "disabled";
};
+ mmu0_dsp1: mmu@40d01000 {
+ compatible = "ti,dra7-dsp-iommu";
+ reg = <0x40d01000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu0_dsp1";
+ #iommu-cells = <0>;
+ ti,syscon-mmuconfig = <&dsp1_system 0x0>;
+ status = "disabled";
+ };
+
+ mmu1_dsp1: mmu@40d02000 {
+ compatible = "ti,dra7-dsp-iommu";
+ reg = <0x40d02000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu1_dsp1";
+ #iommu-cells = <0>;
+ ti,syscon-mmuconfig = <&dsp1_system 0x1>;
+ status = "disabled";
+ };
+
+ mmu_ipu1: mmu@58882000 {
+ compatible = "ti,dra7-iommu";
+ reg = <0x58882000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu_ipu1";
+ #iommu-cells = <0>;
+ ti,iommu-bus-err-back;
+ status = "disabled";
+ };
+
+ mmu_ipu2: mmu@55082000 {
+ compatible = "ti,dra7-iommu";
+ reg = <0x55082000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu_ipu2";
+ #iommu-cells = <0>;
+ ti,iommu-bus-err-back;
+ status = "disabled";
+ };
+
abb_mpu: regulator-abb-mpu {
compatible = "ti,abb-v3";
regulator-name = "abb_mpu";
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REPOST PATCH 1/2] Documentation: dt: Update OMAP iommu bindings for DRA7 DSPs
The DSP processor sub-systems on DRA7xx have two MMU instances
each, one for the processor core and the other for an internal
EDMA block. These MMUs need an additional shared register to be
programmed in the DSP_SYSTEM sub-module to be enabled properly.
The OMAP IOMMU bindings is updated to account for this additional
syscon property required for these DSP IOMMU instances on DRA7xx
SoCs. A new compatible "ti,dra7-dsp-iommu" is also defined to
distinguish these devices specifically from other DRA7 IOMMU
devices.
An example of the DRA7 DSP IOMMU nodes is also added to the
document for clarity.
Signed-off-by: Suman Anna
---
.../devicetree/bindings/iommu/ti,omap-iommu.txt| 27 ++
1 file changed, 27 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
index 869699925fd5..4bd10dd881b8 100644
--- a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
+++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
@@ -4,6 +4,7 @@ Required properties:
- compatible : Should be one of,
"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
+ "ti,dra7-dsp-iommu" for DRA7xx DSP IOMMU instances
"ti,dra7-iommu" for DRA7xx IOMMU instances
- ti,hwmods : Name of the hwmod associated with the IOMMU instance
- reg: Address space for the configuration registers
@@ -19,6 +20,13 @@ Optional properties:
Should be either 8 or 32 (default: 32)
- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing
back a bus error response on MMU faults.
+- ti,syscon-mmuconfig : Should be a pair of the phandle to the DSP_SYSTEM
+syscon node that contains the additional control
+register for enabling the MMU, and the MMU instance
+number (0-indexed) within the sub-system. This property
+is required for DSP IOMMU instances on DRA7xx SoCs. The
+instance number should be 0 for DSP MDMA MMUs and 1 for
+DSP EDMA MMUs.
Example:
/* OMAP3 ISP MMU */
@@ -30,3 +38,22 @@ Example:
ti,hwmods = "mmu_isp";
ti,#tlb-entries = <8>;
};
+
+ /* DRA74x DSP2 MMUs */
+ mmu0_dsp2: mmu@41501000 {
+ compatible = "ti,dra7-dsp-iommu";
+ reg = <0x41501000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu0_dsp2";
+ #iommu-cells = <0>;
+ ti,syscon-mmuconfig = <&dsp2_system 0x0>;
+ };
+
+ mmu1_dsp2: mmu@41502000 {
+ compatible = "ti,dra7-dsp-iommu";
+ reg = <0x41502000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu1_dsp2";
+ #iommu-cells = <0>;
+ ti,syscon-mmuconfig = <&dsp2_system 0x1>;
+ };
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REPOST PATCH 2/2] iommu/omap: Add support for configuring dsp iommus on DRA7xx
The DSP MMUs on DRA7xx SoC requires configuring an additional
MMU_CONFIG register present in the DSP_SYSTEM sub module. This
setting dictates whether the DSP Core's MDMA and EDMA traffic
is routed through the respective MMU or not. Add the support
to the OMAP iommu driver so that the traffic is not bypassed
when enabling the MMUs.
The MMU_CONFIG register has two different bits for enabling
each of these two MMUs present in the DSP processor sub-system
on DRA7xx. An id field is added to the OMAP iommu object to
identify and enable each IOMMU. The id information and the
DSP_SYSTEM.MMU_CONFIG register programming is achieved through
the processing of the optional "ti,syscon-mmuconfig" property.
A proper value is assigned to the id field only when this
property is present.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu.c | 58 ++
drivers/iommu/omap-iommu.h | 9 +++
2 files changed, 67 insertions(+)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 36d0033c2ccb..3dc5b65f3990 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -26,6 +26,8 @@
#include
#include
#include
+#include
+#include
#include
@@ -112,6 +114,18 @@ void omap_iommu_restore_ctx(struct device *dev)
}
EXPORT_SYMBOL_GPL(omap_iommu_restore_ctx);
+static void dra7_cfg_dspsys_mmu(struct omap_iommu *obj, bool enable)
+{
+ u32 val, mask;
+
+ if (!obj->syscfg)
+ return;
+
+ mask = (1 << (obj->id * DSP_SYS_MMU_CONFIG_EN_SHIFT));
+ val = enable ? mask : 0;
+ regmap_update_bits(obj->syscfg, DSP_SYS_MMU_CONFIG, mask, val);
+}
+
static void __iommu_set_twl(struct omap_iommu *obj, bool on)
{
u32 l = iommu_read_reg(obj, MMU_CNTL);
@@ -147,6 +161,8 @@ static int omap2_iommu_enable(struct omap_iommu *obj)
iommu_write_reg(obj, pa, MMU_TTB);
+ dra7_cfg_dspsys_mmu(obj, true);
+
if (obj->has_bus_err_back)
iommu_write_reg(obj, MMU_GP_REG_BUS_ERR_BACK_EN, MMU_GP_REG);
@@ -161,6 +177,7 @@ static void omap2_iommu_disable(struct omap_iommu *obj)
l &= ~MMU_CNTL_MASK;
iommu_write_reg(obj, l, MMU_CNTL);
+ dra7_cfg_dspsys_mmu(obj, false);
dev_dbg(obj->dev, "%s is shutting down\n", obj->name);
}
@@ -864,6 +881,42 @@ static void omap_iommu_detach(struct omap_iommu *obj)
dev_dbg(obj->dev, "%s: %s\n", __func__, obj->name);
}
+static int omap_iommu_dra7_get_dsp_system_cfg(struct platform_device *pdev,
+ struct omap_iommu *obj)
+{
+ struct device_node *np = pdev->dev.of_node;
+ int ret;
+
+ if (!of_device_is_compatible(np, "ti,dra7-dsp-iommu"))
+ return 0;
+
+ if (!of_property_read_bool(np, "ti,syscon-mmuconfig")) {
+ dev_err(&pdev->dev, "ti,syscon-mmuconfig property is
missing\n");
+ return -EINVAL;
+ }
+
+ obj->syscfg =
+ syscon_regmap_lookup_by_phandle(np, "ti,syscon-mmuconfig");
+ if (IS_ERR(obj->syscfg)) {
+ /* can fail with -EPROBE_DEFER */
+ ret = PTR_ERR(obj->syscfg);
+ return ret;
+ }
+
+ if (of_property_read_u32_index(np, "ti,syscon-mmuconfig", 1,
+ &obj->id)) {
+ dev_err(&pdev->dev, "couldn't get the IOMMU instance id within
subsystem\n");
+ return -EINVAL;
+ }
+
+ if (obj->id != 0 && obj->id != 1) {
+ dev_err(&pdev->dev, "invalid IOMMU instance id\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
/*
* OMAP Device MMU(IOMMU) detection
*/
@@ -907,6 +960,10 @@ static int omap_iommu_probe(struct platform_device *pdev)
if (IS_ERR(obj->regbase))
return PTR_ERR(obj->regbase);
+ err = omap_iommu_dra7_get_dsp_system_cfg(pdev, obj);
+ if (err)
+ return err;
+
irq = platform_get_irq(pdev, 0);
if (irq < 0)
return -ENODEV;
@@ -943,6 +1000,7 @@ static const struct of_device_id omap_iommu_of_match[] = {
{ .compatible = "ti,omap2-iommu" },
{ .compatible = "ti,omap4-iommu" },
{ .compatible = "ti,dra7-iommu" },
+ { .compatible = "ti,dra7-dsp-iommu" },
{},
};
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index a656df2f9e03..59628e5017b4 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -30,6 +30,7 @@ struct iotlb_entry {
struct omap_iommu {
const char *name;
void __iomem*regbase;
+ struct regmap *syscfg;
struct device *dev;
struct iommu_dom
[REPOST PATCH 0/2] DRA7 DSP MMU config support
Hi Joerg, This is a repost of the DRA7 DSP MMU config support patch series, baselined on the latest rc (4.3-rc3). Details of the MMUs are summarized in the previous series [1], and the discussion with Tony did not require any changes to the code from that series. As per your request, I have also summarized the testing details below. I have tested the patches on OMAP3 Beagle-XM, OMAP4 Panda Board, OMAP5 uEVM and DRA7 Beagle-X15 boards using a OMAP IOMMU unit test module [2]. The unit test basically requires a test node to be added to the respective nodes with it using the IOMMU under test. The test is done during the probe of the test platform driver, with it mainly attaching to the IOMMU and programming the MMU pagetable. The entire test branch is hosted [3] for your reference and the corresponding logs are at [4]. A few additional notes regarding testing: - OMAP3 IVA MMU node was disabled at the moment in kernel, so it had to be enabled in the test branch along with the test node. A fix [5] was required in the TI CLK driver for it to pass the test, and this fix will be incorporated into the current -rc cycle. - One MMU device is tested per boot, depending on the IOMMU used by the test node. - This is only a preparatory series in adding the IOMMU support for the DRA7 SoCs. Full enablement of DRA7 IOMMUs requires DTS nodes, hwmod data and pdata quirks needed for reset management. The DRA7 logs therefore do not list any MMUs and the OMAP IOMMU driver is not registered even due to the absence of any compatible DT nodes. This series also had a supplementary DTS patch series previously [6], and I will be reposting those as well after rebasing for Tony to pick them up through the linux-omap tree. regards Suman [1] http://lists.linuxfoundation.org/pipermail/iommu/2015-July/013704.html [2] https://github.com/sumananna/omap-test-iommu/blob/master/main_dt.c [3] https://github.com/sumananna/omap-kernel/commits/iommu/test/4.3-rc3-dra7-dsp-support [4] https://github.com/sumananna/kernel-test-logs/tree/master/4.3-rc3-dra7-dsp-support [5] http://marc.info/?l=linux-omap&m=144356627831318&w=2 [6] http://marc.info/?l=linux-omap&m=143752332808524&w=2 Suman Anna (2): Documentation: dt: Update OMAP iommu bindings for DRA7 DSPs iommu/omap: Add support for configuring dsp iommus on DRA7xx .../devicetree/bindings/iommu/ti,omap-iommu.txt| 27 ++ drivers/iommu/omap-iommu.c | 58 ++ drivers/iommu/omap-iommu.h | 9 3 files changed, 94 insertions(+) -- 2.6.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] ARM: DRA7: hwmod: Add data for GPTimer 12
Add the hwmod data for GPTimer 12. GPTimer 12 is present in
WKUPAON power domain and is clocked from a secure 32K clock.
GPTimer 12 serves as a secure timer on HS devices, but is
available for kernel on regular GP devices.
The hwmod link is registered only on GP devices. The hwmod data
also reused the existing timer class instead of reintroducing
the identical dra7xx_timer_secure_sysc class which was dropped
in commit edec17863362 ("ARM: DRA7: hwmod: Fix the hwmod class
for GPTimer4").
Signed-off-by: Suman Anna
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 36 +--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index 562247bced49..37a10f87fbcd 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1929,6 +1929,20 @@ static struct omap_hwmod dra7xx_timer11_hwmod = {
},
};
+/* timer12 */
+static struct omap_hwmod dra7xx_timer12_hwmod = {
+ .name = "timer12",
+ .class = &dra7xx_timer_hwmod_class,
+ .clkdm_name = "wkupaon_clkdm",
+ .main_clk = "secure_32k_clk_src_ck",
+ .prcm = {
+ .omap4 = {
+ .clkctrl_offs =
DRA7XX_CM_WKUPAON_TIMER12_CLKCTRL_OFFSET,
+ .context_offs =
DRA7XX_RM_WKUPAON_TIMER12_CONTEXT_OFFSET,
+ },
+ },
+};
+
/* timer13 */
static struct omap_hwmod dra7xx_timer13_hwmod = {
.name = "timer13",
@@ -3135,6 +3149,14 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per1__timer11
= {
.user = OCP_USER_MPU | OCP_USER_SDMA,
};
+/* l4_wkup -> timer12 */
+static struct omap_hwmod_ocp_if dra7xx_l4_wkup__timer12 = {
+ .master = &dra7xx_l4_wkup_hwmod,
+ .slave = &dra7xx_timer12_hwmod,
+ .clk= "wkupaon_iclk_mux",
+ .user = OCP_USER_MPU | OCP_USER_SDMA,
+};
+
/* l4_per3 -> timer13 */
static struct omap_hwmod_ocp_if dra7xx_l4_per3__timer13 = {
.master = &dra7xx_l4_per3_hwmod,
@@ -3429,6 +3451,13 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[]
__initdata = {
NULL,
};
+/* GP-only hwmod links */
+static struct omap_hwmod_ocp_if *dra7xx_gp_hwmod_ocp_ifs[] __initdata = {
+ &dra7xx_l4_wkup__timer12,
+ NULL,
+};
+
+/* SoC variant specific hwmod links */
static struct omap_hwmod_ocp_if *dra74x_hwmod_ocp_ifs[] __initdata = {
&dra7xx_l4_per3__usb_otg_ss4,
NULL,
@@ -3446,9 +3475,12 @@ int __init dra7xx_hwmod_init(void)
ret = omap_hwmod_register_links(dra7xx_hwmod_ocp_ifs);
if (!ret && soc_is_dra74x())
- return omap_hwmod_register_links(dra74x_hwmod_ocp_ifs);
+ ret = omap_hwmod_register_links(dra74x_hwmod_ocp_ifs);
else if (!ret && soc_is_dra72x())
- return omap_hwmod_register_links(dra72x_hwmod_ocp_ifs);
+ ret = omap_hwmod_register_links(dra72x_hwmod_ocp_ifs);
+
+ if (!ret && omap_type() == OMAP2_DEVICE_TYPE_GP)
+ ret = omap_hwmod_register_links(dra7xx_gp_hwmod_ocp_ifs);
return ret;
}
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] ARM: dts: DRA7: Add timer12 node
Add the DT node for Timer12 present on DRA7 family of
SoCs. Timer12 is present in PD_WKUPAON power domain, and
has the same capabilities as the other timers, except for
the fact that it serves as a secure timer on HS devices
and is clocked only from the secure 32K clock.
The node is marked disabled for now, and the kernel should
refrain from using this secure timer on HS devices.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra7.dtsi | 10 ++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index e289c706d27d..37d632dad576 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -762,6 +762,16 @@
ti,hwmods = "timer11";
};
+ timer12: timer@4ae2 {
+ compatible = "ti,omap5430-timer";
+ reg = <0x4ae2 0x80>;
+ interrupts = ;
+ ti,hwmods = "timer12";
+ ti,timer-alwon;
+ ti,timer-secure;
+ status = "disabled";
+ };
+
timer13: timer@48828000 {
compatible = "ti,omap5430-timer";
reg = <0x48828000 0x80>;
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] ARM: OMAP2+: timer: Remove secure timer for DRA7xx devices
Timer 12 on DRA7 SoCs is reserved for secure usage on high-secure (HS)
devices. The timer cannot be used by the kernel on HS devices, but is
available on regular general purpose (GP) devices. This is similar to the
behavior on OMAP3 devices, so extend the logic used in commit ad24bde8f102
("ARM: OMAP3: Dynamically disable secure timer nodes for secure devices")
to remove the secure timer on DRA7xx SoCs at run-time based on the SoC
device type.
Signed-off-by: Suman Anna
---
arch/arm/mach-omap2/timer.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index a55655127ef2..1e346aa0a687 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -193,8 +193,8 @@ static struct device_node * __init omap_get_timer_dt(const
struct of_device_id *
/**
* omap_dmtimer_init - initialisation function when device tree is used
*
- * For secure OMAP3 devices, timers with device type "timer-secure" cannot
- * be used by the kernel as they are reserved. Therefore, to prevent the
+ * For secure OMAP3/DRA7xx devices, timers with device type "timer-secure"
+ * cannot be used by the kernel as they are reserved. Therefore, to prevent the
* kernel registering these devices remove them dynamically from the device
* tree on boot.
*/
@@ -202,7 +202,7 @@ static void __init omap_dmtimer_init(void)
{
struct device_node *np;
- if (!cpu_is_omap34xx())
+ if (!cpu_is_omap34xx() && !soc_is_dra7xx())
return;
/* If we are a secure device, remove any secure timer nodes */
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] DRA7 Timer12 Support
Hi Tony, The DRA7 Timer12 is completely missing from the kernel. This series adds the support for it, so that all the Timers on DRA7 are represented in the kernel. Timer12 is a special Timer, and is not available for kernel on HS devices, very much similar to the equivalent timer on OMAP3. Patch 1 is a bit debatable, as I am not entirely sure if the clk API used are acceptable outside the drivers/clk folders. It keeps intact the omap_dm_timer_set_source() API though, and allows it to handle the non-configurability of Timer12 parent within that API. The need for this API is questionable, I am not a big fan as the dmtimer code blindly tries to set the source of every requested API to OMAP_TIMER_SRC_32_KHZ, and then the requestors would have to reset the proper parent source again. It also supports configuring only one of 3 parents (which are valid when added originally for the then SoCs), requiring specific clock aliases to work across different SoCs, but the DRA7 SoC does have more than 3 configurable clock parents, and the indexes may not necessarily match for different timers on different SoCs. Patch 1 won't be needed if we can kill the omap_dm_timer_set_source() API. Patches baselined on 4.3-rc3, but should apply just fine on -rc1 as well. regards Suman Suman Anna (4): ARM: OMAP: dmtimer: check for fixed timers during config ARM: OMAP2+: timer: Remove secure timer for DRA7xx devices ARM: dts: DRA7: Add timer12 node ARM: DRA7: hwmod: Add data for GPTimer 12 arch/arm/boot/dts/dra7.dtsi | 10 + arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 36 +-- arch/arm/mach-omap2/timer.c | 6 +++--- arch/arm/plat-omap/dmtimer.c | 5 + 4 files changed, 52 insertions(+), 5 deletions(-) -- 2.6.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] ARM: OMAP: dmtimer: check for fixed timers during config
The omap_dm_timer_set_source() function provides a means for client
users to configure the mux parent for a GPTimer's functional clock.
However, not all timers are configurable (Eg: Timer12 on DRA7 is fed
by an internal 32k oscillator clock, and does not have configurable
parent clocks). So, check for such cases and proceed with out throwing
an error.
Signed-off-by: Suman Anna
---
arch/arm/plat-omap/dmtimer.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 8ca94d379bc3..25693e722f1f 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -36,6 +36,7 @@
*/
#include
+#include
#include
#include
#include
@@ -504,6 +505,10 @@ int omap_dm_timer_set_source(struct omap_dm_timer *timer,
int source)
if (IS_ERR(timer->fclk))
return -EINVAL;
+ /* Check if the clock has parents if not no point checking */
+ if (!clk_hw_get_num_parents(__clk_get_hw(timer->fclk)))
+ return 0;
+
switch (source) {
case OMAP_TIMER_SRC_SYS_CLK:
parent_name = "timer_sys_ck";
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] ARM: dts: dra72-evm: Enable the system mailboxes 5 and 6
Enable the System Mailboxes 5 and 6 and the corresponding
child sub-mailbox (IPC 3.x) nodes for the DRA72 EVM board.
This is needed to enable communication with the respective
remote processors IPU1, IPU2, and DSP1 from the MPU.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra72-evm.dts | 17 +
1 file changed, 17 insertions(+)
diff --git a/arch/arm/boot/dts/dra72-evm.dts b/arch/arm/boot/dts/dra72-evm.dts
index 6f6bd98c98df..ccb535b5ef9b 100644
--- a/arch/arm/boot/dts/dra72-evm.dts
+++ b/arch/arm/boot/dts/dra72-evm.dts
@@ -695,3 +695,20 @@
};
};
};
+
+&mailbox5 {
+ status = "okay";
+ mbox_ipu1_ipc3x: mbox_ipu1_ipc3x {
+ status = "okay";
+ };
+ mbox_dsp1_ipc3x: mbox_dsp1_ipc3x {
+ status = "okay";
+ };
+};
+
+&mailbox6 {
+ status = "okay";
+ mbox_ipu2_ipc3x: mbox_ipu2_ipc3x {
+ status = "okay";
+ };
+};
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] ARM: dts: DRA74x: Add IPC sub-mailbox nodes for all IPUs & DSPs
Add the sub-mailbox nodes that are used to communicate between
MPU and the remote processors IPU1, IPU2, DSP1 and DSP2.
The sub-mailbox nodes utilize the System Mailbox instances 5 and 6.
These sub-mailbox nodes are added to match the hard-coded mailbox
configuration used within the TI IPC 3.x software package. The
Dual-Cortex M4 IPU1 and IPU2 processor sub-systems are assumed to
be running in SMP-mode, and hence only a single sub-mailbox node
is added for each.
All these sub-mailbox nodes are left in disabled state, and should
be enabled (and modified if needed) as per the individual product
configuration in the corresponding board dts files.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra74x.dtsi | 26 ++
1 file changed, 26 insertions(+)
diff --git a/arch/arm/boot/dts/dra74x.dtsi b/arch/arm/boot/dts/dra74x.dtsi
index feea98e0a4b5..33d11c2c8af3 100644
--- a/arch/arm/boot/dts/dra74x.dtsi
+++ b/arch/arm/boot/dts/dra74x.dtsi
@@ -93,3 +93,29 @@
<&dss_video2_clk>;
clock-names = "fck", "video1_clk", "video2_clk";
};
+
+&mailbox5 {
+ mbox_ipu1_ipc3x: mbox_ipu1_ipc3x {
+ ti,mbox-tx = <6 2 2>;
+ ti,mbox-rx = <4 2 2>;
+ status = "disabled";
+ };
+ mbox_dsp1_ipc3x: mbox_dsp1_ipc3x {
+ ti,mbox-tx = <5 2 2>;
+ ti,mbox-rx = <1 2 2>;
+ status = "disabled";
+ };
+};
+
+&mailbox6 {
+ mbox_ipu2_ipc3x: mbox_ipu2_ipc3x {
+ ti,mbox-tx = <6 2 2>;
+ ti,mbox-rx = <4 2 2>;
+ status = "disabled";
+ };
+ mbox_dsp2_ipc3x: mbox_dsp2_ipc3x {
+ ti,mbox-tx = <5 2 2>;
+ ti,mbox-rx = <1 2 2>;
+ status = "disabled";
+ };
+};
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] Add DRA7 sub-mailboxes for 4.4
Hi Tony, This series adds the sub-mailbox nodes and enable them for each of the TI DRA7 boards. These are the basic mailbox configuration nodes required by client users (remoteproc) to communicate with the various IPU and DSP processor devices on DRA74x and DRA72x SoCs using the TI IPC 3.x software foundation on the remote processor firmwares. The mailboxes used are unfortunately hardcoded in the TI IPC 3.x software, and hence the weird usage of Mailbox 5 and 6 rather than starting from Mailbox 1. Any third-party or derived boards can use their own configuration if running their own IPC stack, and that can be done by overwriting the properties or by using different sub-mailbox nodes altogether. The actual enablement of nodes is therefore done in the respective board DTS files. Patches are based on 4.3-rc1 and are intended for the 4.4 merge window. regards Suman Suman Anna (5): ARM: dts: DRA74x: Add IPC sub-mailbox nodes for all IPUs & DSPs ARM: dts: DRA72x: Add IPC sub-mailbox nodes for IPU1, IPU2 & DSP1 ARM: dts: dra7-evm: Enable the system mailboxes 5 and 6 ARM: dts: dra72-evm: Enable the system mailboxes 5 and 6 ARM: dts: beagle-x15: Enable the system mailboxes 5 and 6 arch/arm/boot/dts/am57xx-beagle-x15.dts | 20 arch/arm/boot/dts/dra7-evm.dts | 20 arch/arm/boot/dts/dra72-evm.dts | 17 + arch/arm/boot/dts/dra72x.dtsi | 21 + arch/arm/boot/dts/dra74x.dtsi | 26 ++ 5 files changed, 104 insertions(+) -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] ARM: dts: dra7-evm: Enable the system mailboxes 5 and 6
Enable the System Mailboxes 5 and 6 and the corresponding
child sub-mailbox (IPC 3.x) nodes for the DRA7 EVM board.
This is needed to enable communication with the respective
remote processors IPU1, IPU2, DSP1 and DSP2 from the MPU.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra7-evm.dts | 20
1 file changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index a6c82e5b64fe..718315ac3063 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -707,3 +707,23 @@
pinctrl-1 = <&dcan1_pins_sleep>;
pinctrl-2 = <&dcan1_pins_default>;
};
+
+&mailbox5 {
+ status = "okay";
+ mbox_ipu1_ipc3x: mbox_ipu1_ipc3x {
+ status = "okay";
+ };
+ mbox_dsp1_ipc3x: mbox_dsp1_ipc3x {
+ status = "okay";
+ };
+};
+
+&mailbox6 {
+ status = "okay";
+ mbox_ipu2_ipc3x: mbox_ipu2_ipc3x {
+ status = "okay";
+ };
+ mbox_dsp2_ipc3x: mbox_dsp2_ipc3x {
+ status = "okay";
+ };
+};
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] ARM: dts: DRA72x: Add IPC sub-mailbox nodes for IPU1, IPU2 & DSP1
Add the sub-mailbox nodes that are used to communicate between
MPU and the remote processors IPU1, IPU2 and DSP1. These match the
respective node definitions on DRA74x to maintain compatibility for
the equivalent remote processors. There is no DSP2 on DRA72x, and
so the corresponding sub-mailbox node is not added.
These sub-mailbox nodes are added to match the hard-coded mailbox
configuration used within the TI IPC 3.x software package. The
Dual-Cortex M4 IPU1 and IPU2 processor sub-systems are assumed to
be running in SMP-mode, and hence only a single sub-mailbox node
is added for each.
All these sub-mailbox nodes are left in disabled state, and should
be enabled (and modified if needed) as per the individual product
configuration in the corresponding board dts files.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra72x.dtsi | 21 +
1 file changed, 21 insertions(+)
diff --git a/arch/arm/boot/dts/dra72x.dtsi b/arch/arm/boot/dts/dra72x.dtsi
index eaca143faa77..70a217050a4c 100644
--- a/arch/arm/boot/dts/dra72x.dtsi
+++ b/arch/arm/boot/dts/dra72x.dtsi
@@ -45,3 +45,24 @@
<&dss_video1_clk>;
clock-names = "fck", "video1_clk";
};
+
+&mailbox5 {
+ mbox_ipu1_ipc3x: mbox_ipu1_ipc3x {
+ ti,mbox-tx = <6 2 2>;
+ ti,mbox-rx = <4 2 2>;
+ status = "disabled";
+ };
+ mbox_dsp1_ipc3x: mbox_dsp1_ipc3x {
+ ti,mbox-tx = <5 2 2>;
+ ti,mbox-rx = <1 2 2>;
+ status = "disabled";
+ };
+};
+
+&mailbox6 {
+ mbox_ipu2_ipc3x: mbox_ipu2_ipc3x {
+ ti,mbox-tx = <6 2 2>;
+ ti,mbox-rx = <4 2 2>;
+ status = "disabled";
+ };
+};
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] ARM: dts: beagle-x15: Enable the system mailboxes 5 and 6
Enable the System Mailboxes 5 and 6 and the corresponding child
sub-mailbox (IPC 3.x) nodes for the Beagle X15 EVM boards. This
is needed to enable communication with the respective remote
processors IPU1, IPU2, DSP1 and DSP2 from the MPU.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/am57xx-beagle-x15.dts | 20
1 file changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/am57xx-beagle-x15.dts
b/arch/arm/boot/dts/am57xx-beagle-x15.dts
index 3a05b94f59ed..4a13419ab025 100644
--- a/arch/arm/boot/dts/am57xx-beagle-x15.dts
+++ b/arch/arm/boot/dts/am57xx-beagle-x15.dts
@@ -696,3 +696,23 @@
&pcie1 {
gpios = <&gpio2 8 GPIO_ACTIVE_LOW>;
};
+
+&mailbox5 {
+ status = "okay";
+ mbox_ipu1_ipc3x: mbox_ipu1_ipc3x {
+ status = "okay";
+ };
+ mbox_dsp1_ipc3x: mbox_dsp1_ipc3x {
+ status = "okay";
+ };
+};
+
+&mailbox6 {
+ status = "okay";
+ mbox_ipu2_ipc3x: mbox_ipu2_ipc3x {
+ status = "okay";
+ };
+ mbox_dsp2_ipc3x: mbox_dsp2_ipc3x {
+ status = "okay";
+ };
+};
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] iommu/omap: Add support for configuring dsp iommus on DRA7xx
Hi Tony, Sorry for the long delay in getting back on this. I will repost the series once 4.3-rc1 is out, but at the moment I do not think any changes are required. Let me know if you still need any additional details. >>>> On 07/23/2015 02:24 AM, Tony Lindgren wrote: >>>>> * Suman Anna [150722 09:25]: >>>>>> On 07/22/2015 12:26 AM, Tony Lindgren wrote: >>>>>>> >>>>>>> I don't like using syscon for tinkering directly with SoC registers. >>>>>> >>>>>> This is not a SoC-level register, but a register within a sub-module of >>>>>> the DSP processor sub-system. The DSP_SYSTEM sub-module in general is >>>>>> described in Section 5.3.3 of the TRM [1], and it implements different >>>>>> functionalities like the PRCM handshaking, wakeup logic and DSP >>>>>> subsystem top-level configuration. It is a module present within the DSP >>>>>> processor sub-system, so can only be accessed when the sub-system is >>>>>> clocked and the appropriate reset is released. >>>>> >>>>> OK so if it's specific to the DSP driver along the lines of sysc and >>>>> syss registers. >>>> >>>> There will be those registers too within the MMU config register space, >>>> even for DRA7xx MMUs. This is different, think of it like a register in >>>> the Control module except that it is present within the DSP sub-system >>>> instead of at the SoC level. >>> >>> And what is taking care of pm_runtime_get here to ensure the module >>> is powered and clocked? >> >> pm_runtime_get_sync is indeed getting invoked, its just the current >> patch doesn't include the code block that invokes it. The function that >> invokes omap2_iommu_enable does so after a call to the >> pm_runtime_get_sync() call. > > OK > >>> I think you are missing a layer here and it's the Linux kernel side >>> device driver for DSP that initializes things. >> >> We already have separate drivers for MMUs (omap-iommu) and the processor >> management (omap-rproc). The former manages all the low-level >> programming sequences for the MMUs, while the latter manages the >> low-level reset etc, and is a client user of the respective IOMMU >> device. Both integrate into the respective frameworks. The IOMMU API >> invocations are handled in the remoteproc core, with the OMAP remoteproc >> driver publishing itself as having an MMU with the remoteproc core. The >> IOMMU API invoke the appropriate iommu_ops. >> >> You can lookup the functions rproc_enable_iommu()/rproc_disable_iommu() >> in the remoteproc core. The IOMMU enabling sequences happen within the >> iommu_attach_device() API. The call flow is >> iommu_attach_device()->omap_iommu_attach_dev()->omap_iommu_attach()-> >> iommu_enable()-> >>omap_device_deassert_hardreset, pm_runtime_get_sync, omap2_iommu_enable. > > OK. The thing to check here is that you have a separate device driver > for each sysc/syss device registers. This is because each hardware module > can be independently clocked and idled. Otherwise things will break at > some point. And no "things are configured for autoidle" or "we're not > doing PM is not a good excuse here" :) > > Can you please check that? If the remoteproc driver and iommu driver > are tinkering with registers in separate hardware modules, we have > a layering violation. My guess is that we have at least two hardware > modules involved here, one for the iommu and one for the DSP device. Only the IOMMU has a SYSC/SYSS device register. It has both a soft-reset as well as a hard-reset line. The IOMMU stays in reset, and its state machine/programming is controlled through the remoteproc driver. The clock source is also single clock going into the remote processor subsystem, and split up internally to feed the memories and the processors. For the PRCM to auto-gate the clocks, we also need the processor to be executing a IDLE (DSPs) or WFI/WFE (Cortex M3/M4s). Just the IOMMU will allow the OCP idle because of the associated sysc/syss registers. Depending on the remote processor, there might be a separate internal module (like the DSPs usually have a separate SYSC module), but that programming is left to the software running on the DSP. > >>>>> Typically we handle these registers by mapping them to the PM runtime >>>>> functions for the interconnect so we can reset and idle the hardware >>>>> modules even if there is no driver, see for example >&
Re: [PATCH v3 3/6] remoteproc: Supply controller driver for ST's Remote Processors
Hi Lee,
On 09/02/2015 10:38 AM, Lee Jones wrote:
> Signed-off-by: Ludovic Barre
> Signed-off-by: Lee Jones
> ---
> drivers/remoteproc/Kconfig | 9 ++
> drivers/remoteproc/Makefile| 1 +
> drivers/remoteproc/st_remoteproc.c | 307
> +
> 3 files changed, 317 insertions(+)
> create mode 100644 drivers/remoteproc/st_remoteproc.c
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 28c711f..72e97d7 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -77,4 +77,13 @@ config DA8XX_REMOTEPROC
> It's safe to say n here if you're not interested in multimedia
> offloading.
>
> +config ST_REMOTEPROC
> + tristate "ST remoteproc support"
> + depends on ARCH_STI
> + select REMOTEPROC
> + help
> + Say y here to support ST's adjunct processors via the remote
> + processor framework.
> + This can be either built-in or a loadable module.
> +
> endmenu
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 81b04d1..279cb2e 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_OMAP_REMOTEPROC) +=
> omap_remoteproc.o
> obj-$(CONFIG_STE_MODEM_RPROC)+= ste_modem_rproc.o
> obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o
> obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
> +obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o
> diff --git a/drivers/remoteproc/st_remoteproc.c
> b/drivers/remoteproc/st_remoteproc.c
> new file mode 100644
> index 000..24ae6d3
> --- /dev/null
> +++ b/drivers/remoteproc/st_remoteproc.c
> @@ -0,0 +1,307 @@
> +/*
> + * ST's Remote Processor Control Driver
> + *
> + * Copyright (C) 2015 STMicroelectronics - All Rights Reserved
> + *
> + * Author: Ludovic Barre
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct st_rproc_config {
> + boolsw_reset;
> + boolpwr_reset;
> + u32 bootaddr_mask;
> +};
> +
> +struct st_rproc {
> + struct st_rproc_config *config;
> + struct reset_control*sw_reset;
> + struct reset_control*pwr_reset;
> + struct clk *clk;
> + u32 clk_rate;
> + struct regmap *boot_base;
> + u32 boot_offset;
> +};
> +
> +static int st_rproc_stop(struct rproc *rproc)
> +{
> + struct st_rproc *ddata = rproc->priv;
> + int sw_err = 0, pwr_err = 0;
> +
> + if (ddata->config->sw_reset) {
> + sw_err = reset_control_assert(ddata->sw_reset);
> + if (sw_err)
> + dev_err(&rproc->dev, "Failed to assert S/W Reset\n");
> + }
> +
> + if (ddata->config->pwr_reset) {
> + pwr_err = reset_control_assert(ddata->pwr_reset);
> + if (pwr_err)
> + dev_err(&rproc->dev, "Failed to assert Power Reset\n");
> + }
> +
> + clk_disable(ddata->clk);
> +
> + return sw_err ?: pwr_err;
> +}
nit, can you please move st_rproc_stop function after st_rproc_start,
same order as in fw_ops, for better flow.
> +
> +static int st_rproc_start(struct rproc *rproc)
> +{
> + struct st_rproc *ddata = rproc->priv;
> + int err;
> +
> + regmap_update_bits(ddata->boot_base, ddata->boot_offset,
> +ddata->config->bootaddr_mask, rproc->bootaddr);
> +
> + err = clk_enable(ddata->clk);
> + if (err) {
> + dev_err(&rproc->dev, "Failed to enable clock\n");
> + return err;
> + }
> +
> + if (ddata->config->sw_reset) {
> + err = reset_control_deassert(ddata->sw_reset);
> + if (err) {
> + dev_err(&rproc->dev, "Failed to deassert S/W Reset\n");
> + goto sw_reset_fail;
> + }
> + }
> +
> + if (ddata->config->pwr_reset) {
> + err = reset_control_deassert(ddata->pwr_reset);
> + if (err) {
> + dev_err(&rproc->dev, "Failed to deassert Power
> Reset\n");
> + goto pwr_reset_fail;
> + }
> + }
> +
> + dev_info(&rproc->dev, "Started from 0x%x\n", rproc->bootaddr);
> +
> + return 0;
> +
> +
> +pwr_reset_fail:
> + reset_control_assert(ddata->sw_reset);
Should this be under a check for (ddata->config->sw_reset). It is not
clear from code whether the sw_reset and pwr_reset are exclusive of each
of other or have both.
> +sw_reset_fail:
> + clk_disable(ddata->clk);
> +
> + return e
Re: [PATCH v2 1/3] mailbox/omap: Add ti,mbox-send-noirq quirk to fix AM33xx CPU Idle
Hi Tony, On 08/05/2015 05:28 AM, Tony Lindgren wrote: > * Dave Gerlach [150717 13:59]: >> --- a/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt >> +++ b/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt >> @@ -75,6 +75,14 @@ data that represent the following: >> Cell #3 (usr_id) - mailbox user id for identifying the interrupt line >> associated with generating a tx/rx fifo interrupt. >> >> +Optional Properties: >> + >> +- ti,mbox-send-noirq: Quirk flag to allow the client user of this >> sub-mailbox >> +to send messages without triggering a Tx ready >> interrupt, >> +and to control the Tx ticker. Should be used only on >> +sub-mailboxes used to communicate with WkupM3 remote >> +processor on AM33xx/AM43xx SoCs. >> + > > Hmm can't you do this just by setting a flag in the device driver > based on the compatible string? We can't because there can be other users like PRUSS which will use a sub-mailbox in a regular fashion. The compatible serves the IP and there can be multiple sub-mailboxes underneath it, and this quirk is needed only on the one that's used by the WkupM3. Jassi, Do you have any comments on this? regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" 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] DRA7 DSP MMU config support
On 08/03/2015 11:31 AM, Joerg Roedel wrote: > On Tue, Jul 21, 2015 at 06:55:34PM -0500, Suman Anna wrote: >> The patches are baselined on 4.2-rc3 + the recent OMAP IOMMU >> cleanup series [1]. I will post the DTS patches separately to >> allow Tony to pick them up independently. > > From the discussion it looks like some more work is necessary here. > Before you repost, please also test on older omap hardware to make sure > nothing breaks there. Please also describe the testing you did in the > repost. Yes, sure will do. I will repost once the next -rc1 is out. regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] iommu/omap: Add support for configuring dsp iommus on DRA7xx
Hi Tony, On 07/23/2015 11:30 PM, Tony Lindgren wrote: > * Suman Anna [150723 09:25]: >> Hi Tony, >> >> On 07/23/2015 02:24 AM, Tony Lindgren wrote: >>> * Suman Anna [150722 09:25]: >>>> On 07/22/2015 12:26 AM, Tony Lindgren wrote: >>>>> >>>>> I don't like using syscon for tinkering directly with SoC registers. >>>> >>>> This is not a SoC-level register, but a register within a sub-module of >>>> the DSP processor sub-system. The DSP_SYSTEM sub-module in general is >>>> described in Section 5.3.3 of the TRM [1], and it implements different >>>> functionalities like the PRCM handshaking, wakeup logic and DSP >>>> subsystem top-level configuration. It is a module present within the DSP >>>> processor sub-system, so can only be accessed when the sub-system is >>>> clocked and the appropriate reset is released. >>> >>> OK so if it's specific to the DSP driver along the lines of sysc and >>> syss registers. >> >> There will be those registers too within the MMU config register space, >> even for DRA7xx MMUs. This is different, think of it like a register in >> the Control module except that it is present within the DSP sub-system >> instead of at the SoC level. > > And what is taking care of pm_runtime_get here to ensure the module > is powered and clocked? pm_runtime_get_sync is indeed getting invoked, its just the current patch doesn't include the code block that invokes it. The function that invokes omap2_iommu_enable does so after a call to the pm_runtime_get_sync() call. > > I think you are missing a layer here and it's the Linux kernel side > device driver for DSP that initializes things. We already have separate drivers for MMUs (omap-iommu) and the processor management (omap-rproc). The former manages all the low-level programming sequences for the MMUs, while the latter manages the low-level reset etc, and is a client user of the respective IOMMU device. Both integrate into the respective frameworks. The IOMMU API invocations are handled in the remoteproc core, with the OMAP remoteproc driver publishing itself as having an MMU with the remoteproc core. The IOMMU API invoke the appropriate iommu_ops. You can lookup the functions rproc_enable_iommu()/rproc_disable_iommu() in the remoteproc core. The IOMMU enabling sequences happen within the iommu_attach_device() API. The call flow is iommu_attach_device()->omap_iommu_attach_dev()->omap_iommu_attach()-> iommu_enable()-> omap_device_deassert_hardreset, pm_runtime_get_sync, omap2_iommu_enable. > >>> Typically we handle these registers by mapping them to the PM runtime >>> functions for the interconnect so we can reset and idle the hardware >>> modules even if there is no driver, see for example >>> omap54xx_mmu_dsp_hwmod. >> >> I haven't yet submitted the DRA7xx hwmods, but they will look almost >> exactly like the OMAP5 ones. The reset and idle on these are in general >> not effective at boot time since these are also controlled by a >> hard-reset line, so that's left to the drivers to deal with it through >> the omap_device_deassert_hardreset API. > > If the MMU configuration is one time init, it may make sense to add > it to the hwmod reset function. However, if the Linux kernel side > driver needs to configure things depending on the DSP firmware, it > should be done in the kernel side device driver. The MMU configuration comes into picture whenever the remoteproc driver is being booted and shut down, and also during suspend (no support for this yet in mainline on OMAP rproc). Today, the hwmod _enable/_idle/reset functions alone are not enough to power on the OMAP remoteproc/iommus. We need sequencing calls to both omap_device_assert/_deassert_hardreset (done through pdata-quirks) and pm_runtime API to achieve this. > >>>>> We should use some Linux generic framework for configuring these >>>>> bits to avoid nasty dependencies between various hardware modules >>>>> on the SoC. >>>>> >>>>> What does DSP_SYS_MMU_CONFIG register do? It seems it's probably >>>>> a regulator or a gate clock? If so, it should be set up as a >>>>> regulator or a clock and then the omap-iommu driver can just >>>>> use regulator or clcok framework to request the resource. >>>> >>>> No, its neither. It is a control bit that dictates whether the >>>> processor/EDMA addresses go through the respective MMU or not. The >>>> register currently has 4 bits (bit 0 in each nibble), one each for >>>> enabling each
Re: [PATCH 2/2] iommu/omap: Add support for configuring dsp iommus on DRA7xx
Hi Tony, On 07/23/2015 02:24 AM, Tony Lindgren wrote: > * Suman Anna [150722 09:25]: >> On 07/22/2015 12:26 AM, Tony Lindgren wrote: >>> >>> I don't like using syscon for tinkering directly with SoC registers. >> >> This is not a SoC-level register, but a register within a sub-module of >> the DSP processor sub-system. The DSP_SYSTEM sub-module in general is >> described in Section 5.3.3 of the TRM [1], and it implements different >> functionalities like the PRCM handshaking, wakeup logic and DSP >> subsystem top-level configuration. It is a module present within the DSP >> processor sub-system, so can only be accessed when the sub-system is >> clocked and the appropriate reset is released. > > OK so if it's specific to the DSP driver along the lines of sysc and > syss registers. There will be those registers too within the MMU config register space, even for DRA7xx MMUs. This is different, think of it like a register in the Control module except that it is present within the DSP sub-system instead of at the SoC level. > > Typically we handle these registers by mapping them to the PM runtime > functions for the interconnect so we can reset and idle the hardware > modules even if there is no driver, see for example > omap54xx_mmu_dsp_hwmod. I haven't yet submitted the DRA7xx hwmods, but they will look almost exactly like the OMAP5 ones. The reset and idle on these are in general not effective at boot time since these are also controlled by a hard-reset line, so that's left to the drivers to deal with it through the omap_device_deassert_hardreset API. > >>> We should use some Linux generic framework for configuring these >>> bits to avoid nasty dependencies between various hardware modules >>> on the SoC. >>> >>> What does DSP_SYS_MMU_CONFIG register do? It seems it's probably >>> a regulator or a gate clock? If so, it should be set up as a >>> regulator or a clock and then the omap-iommu driver can just >>> use regulator or clcok framework to request the resource. >> >> No, its neither. It is a control bit that dictates whether the >> processor/EDMA addresses go through the respective MMU or not. The >> register currently has 4 bits (bit 0 in each nibble), one each for >> enabling each MMU and requesting an MMU abort on each. The MMU >> integration and enablement notes are detailed in Section 5.3.6 of the >> TRM [1], and the DSP_SYS_MMU_CONFIG register layout is in Table 5-28 >> (Page 1641). > > OK yeah seems like it should be handled by the DSP driver during > probe after doing pm_runtime_get. Then the driver can configure > things like IOMMU and load firmware. Or am I missing something here? The DSP (remoteproc) driver uses the generic IOMMU API, and all the IOMMU enabling sequence is transparent to the DSP driver. So, any configuration/enabling sequence specific to the IOMMU has to be handled within the respective IOMMU platform driver that gets integrated into the IOMMU API. regards Suman > > >> [1] http://www.ti.com/lit/ug/spruhz6/spruhz6.pdf -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] iommu/omap: Add support for configuring dsp iommus on DRA7xx
Hi Tony,
On 07/22/2015 12:26 AM, Tony Lindgren wrote:
> * Suman Anna [150721 16:58]:
>> --- a/drivers/iommu/omap-iommu.c
>> +++ b/drivers/iommu/omap-iommu.c
>> @@ -26,6 +26,8 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>>
>> #include
>>
>> @@ -112,6 +114,18 @@ void omap_iommu_restore_ctx(struct device *dev)
>> }
>> EXPORT_SYMBOL_GPL(omap_iommu_restore_ctx);
>>
>> +static void dra7_cfg_dspsys_mmu(struct omap_iommu *obj, bool enable)
>> +{
>> +u32 val, mask;
>> +
>> +if (!obj->syscfg)
>> +return;
>> +
>> +mask = (1 << (obj->id * DSP_SYS_MMU_CONFIG_EN_SHIFT));
>> +val = enable ? mask : 0;
>> +regmap_update_bits(obj->syscfg, DSP_SYS_MMU_CONFIG, mask, val);
>> +}
>> +
>> static void __iommu_set_twl(struct omap_iommu *obj, bool on)
>
> I don't like using syscon for tinkering directly with SoC registers.
This is not a SoC-level register, but a register within a sub-module of
the DSP processor sub-system. The DSP_SYSTEM sub-module in general is
described in Section 5.3.3 of the TRM [1], and it implements different
functionalities like the PRCM handshaking, wakeup logic and DSP
subsystem top-level configuration. It is a module present within the DSP
processor sub-system, so can only be accessed when the sub-system is
clocked and the appropriate reset is released.
> We should use some Linux generic framework for configuring these
> bits to avoid nasty dependencies between various hardware modules
> on the SoC.
>
> What does DSP_SYS_MMU_CONFIG register do? It seems it's probably
> a regulator or a gate clock? If so, it should be set up as a
> regulator or a clock and then the omap-iommu driver can just
> use regulator or clcok framework to request the resource.
No, its neither. It is a control bit that dictates whether the
processor/EDMA addresses go through the respective MMU or not. The
register currently has 4 bits (bit 0 in each nibble), one each for
enabling each MMU and requesting an MMU abort on each. The MMU
integration and enablement notes are detailed in Section 5.3.6 of the
TRM [1], and the DSP_SYS_MMU_CONFIG register layout is in Table 5-28
(Page 1641).
regards
Suman
[1] http://www.ti.com/lit/ug/spruhz6/spruhz6.pdf
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] ARM: dts: DRA7: Add common IOMMU nodes
The DRA7xx family of SOCs have two IPUs and one DSP processor
subsystems in common. The IOMMU DT nodes have been added for
these processor subsystems, and have been disabled by default.
These MMUs are very similar to those on OMAP4 and OMAP5, with
the only difference being the presence of a second MMU within
the DSP subsystem for the EDMA port. The DSP IOMMUs also need
an additional 'ti,syscon-mmuconfig' property compared to the
IPU IOMMUs.
NOTE: The enabling of these nodes is left to the respective
board dts files.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra7.dtsi | 40
1 file changed, 40 insertions(+)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 1c96729641b6..0754df4ca59c 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -911,6 +911,46 @@
status = "disabled";
};
+ mmu0_dsp1: mmu@40d01000 {
+ compatible = "ti,dra7-dsp-iommu";
+ reg = <0x40d01000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu0_dsp1";
+ #iommu-cells = <0>;
+ ti,syscon-mmuconfig = <&dsp1_system 0x0>;
+ status = "disabled";
+ };
+
+ mmu1_dsp1: mmu@40d02000 {
+ compatible = "ti,dra7-dsp-iommu";
+ reg = <0x40d02000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu1_dsp1";
+ #iommu-cells = <0>;
+ ti,syscon-mmuconfig = <&dsp1_system 0x1>;
+ status = "disabled";
+ };
+
+ mmu_ipu1: mmu@58882000 {
+ compatible = "ti,dra7-iommu";
+ reg = <0x58882000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu_ipu1";
+ #iommu-cells = <0>;
+ ti,iommu-bus-err-back;
+ status = "disabled";
+ };
+
+ mmu_ipu2: mmu@55082000 {
+ compatible = "ti,dra7-iommu";
+ reg = <0x55082000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu_ipu2";
+ #iommu-cells = <0>;
+ ti,iommu-bus-err-back;
+ status = "disabled";
+ };
+
abb_mpu: regulator-abb-mpu {
compatible = "ti,abb-v3";
regulator-name = "abb_mpu";
--
2.4.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] ARM: dts: DRA74x: Add IOMMU nodes for DSP2
The DRA74x family of SoCs have a second DSP, that also has
two MMUs just like the DSP1 subsystem. Add the IOMMU nodes
for this DSP2 subsystem in disabled state to the DRA74x
specific DTS file, the nodes would need to be enabled
appropriately in the respective board DTS files.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra74x.dtsi | 20
1 file changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/dra74x.dtsi b/arch/arm/boot/dts/dra74x.dtsi
index 6cb112450ddd..612cc7e13bab 100644
--- a/arch/arm/boot/dts/dra74x.dtsi
+++ b/arch/arm/boot/dts/dra74x.dtsi
@@ -76,6 +76,26 @@
dr_mode = "otg";
};
};
+
+ mmu0_dsp2: mmu@41501000 {
+ compatible = "ti,dra7-dsp-iommu";
+ reg = <0x41501000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu0_dsp2";
+ #iommu-cells = <0>;
+ ti,syscon-mmuconfig = <&dsp2_system 0x0>;
+ status = "disabled";
+ };
+
+ mmu1_dsp2: mmu@41502000 {
+ compatible = "ti,dra7-dsp-iommu";
+ reg = <0x41502000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu1_dsp2";
+ #iommu-cells = <0>;
+ ti,syscon-mmuconfig = <&dsp2_system 0x1>;
+ status = "disabled";
+ };
};
};
--
2.4.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] ARM: dts: DRA74x: Add dsp2_system syscon node
The DSP_SYSTEM sub-module is a dedicated system control logic
module present within a DRA7 DSP processor sub-system. This
module is responsible for power management, clock generation
and connection to the device PRCM module.
Add a syscon node for this module for the DSP2 processor
sub-system. This is added as a syscon node as it is a common
configuration module that can be used by the different IOMMU
instances and the corresponding remoteproc device.
The node is added to the dra74x.dtsi file, as the DSP2 processor
subsystem is usually present only on the DRA74x variants of the
DRA7 SoC family.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra74x.dtsi | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/dra74x.dtsi b/arch/arm/boot/dts/dra74x.dtsi
index fa995d0ca1f2..6cb112450ddd 100644
--- a/arch/arm/boot/dts/dra74x.dtsi
+++ b/arch/arm/boot/dts/dra74x.dtsi
@@ -52,6 +52,11 @@
};
ocp {
+ dsp2_system: dsp_system@4150 {
+ compatible = "syscon";
+ reg = <0x4150 0x100>;
+ };
+
omap_dwc3_4: omap_dwc3_4@4894 {
compatible = "ti,dwc3";
ti,hwmods = "usb_otg_ss4";
--
2.4.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] ARM: dts: DRA7: Add dsp1_system syscon node
The DSP_SYSTEM sub-module is a dedicated system control logic
module present within a DRA7 DSP processor sub-system. This
module is responsible for power management, clock generation
and connection to the device PRCM module.
Add a syscon node for this module for the DSP1 processor
sub-system. This is added as a syscon node as it is a common
configuration module that can be used by the different IOMMU
instances and the corresponding remoteproc device.
The node is added to the common dra7.dtsi file, as the DSP1
processor sub-system is mostly common across all the variants
of the DRA7 SoC family.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra7.dtsi | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 8f1e25bcecbd..1c96729641b6 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -296,6 +296,11 @@
reg = <0x4a002e00 0x7c>;
};
+ dsp1_system: dsp_system@40d0 {
+ compatible = "syscon";
+ reg = <0x40d0 0x100>;
+ };
+
sdma: dma-controller@4a056000 {
compatible = "ti,omap4430-sdma";
reg = <0x4a056000 0x1000>;
--
2.4.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] Add DRA7 IOMMU DT nodes
Hi Tony, The following patches add the basic DT nodes for the DSP and IPU IOMMU devices for the DRA7xx SoC family. The first two patches add syscon nodes for DSP_SYSTEM sub-modules, while the last two add the IOMMU nodes. The IPU IOMMU nodes mostly are similar to existing ones on OMAP4 and OMAP5. The DSP sub-system though has two MMUs, with a dedicated MMU for the internal EDMA engine. On previous SoCs, a single MMU served both the DSP processor and the internal EDMA. The DSP IOMMU nodes require a new property to reference the syscon nodes, as they each need a specific bit to be set in a register within the corresponding DSP_SYSTEM sub-module. So, these are pending based on the equivalent bindings update series [1]. Please let me know if the binding looks ok to you, as this seems it can be done in couple of ways. Patches are based on 4.2-rc3. regards Suman [1] http://marc.info/?l=linux-omap&m=143752295508435&w=2 Suman Anna (4): ARM: dts: DRA7: Add dsp1_system syscon node ARM: dts: DRA74x: Add dsp2_system syscon node ARM: dts: DRA7: Add common IOMMU nodes ARM: dts: DRA74x: Add IOMMU nodes for DSP2 arch/arm/boot/dts/dra7.dtsi | 45 +++ arch/arm/boot/dts/dra74x.dtsi | 25 2 files changed, 70 insertions(+) -- 2.4.4 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] DRA7 DSP MMU config support
Hi, This series adds the basic support in the OMAP IOMMU driver to enable/disable DSP IOMMUs for the DRA7xx family of SoCs. The DRA7 family has two MMUs within the DSP processor subsystems. This is the first time this was designed so in the silicon compared to the equivalent ones on OMAP2+ SoCs. Each MMU requires a specific bit to be turned on within a register from a separate sub-module DSP_SYSTEM present within the respective processor subsystem. The DSP_SYSTEM sub-module is represented as a syscon node, and an additional property "ti,syscon-mmuconfig" is used alongside a unique compatible property for configuring these devices. The register and bit offset is coded up in the driver while the DT property references the syscon phandle the MMU instance number. The binding support could have been done in couple of other ways, like using separate compatible each for the DSP processor MMU & the DSP EDMA MMU that automatically identifies the instance number; and/or coding up the register offset & bit position/offset in the syscon property, so let me know if the current approach is fine. The patches are baselined on 4.2-rc3 + the recent OMAP IOMMU cleanup series [1]. I will post the DTS patches separately to allow Tony to pick them up independently. regards Suman [1] http://lists.linuxfoundation.org/pipermail/iommu/2015-July/013659.html Suman Anna (2): Documentation: dt: Update OMAP iommu bindings for DRA7 DSPs iommu/omap: Add support for configuring dsp iommus on DRA7xx .../devicetree/bindings/iommu/ti,omap-iommu.txt| 27 ++ drivers/iommu/omap-iommu.c | 58 ++ drivers/iommu/omap-iommu.h | 9 3 files changed, 94 insertions(+) -- 2.4.4 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] iommu/omap: Add support for configuring dsp iommus on DRA7xx
The DSP MMUs on DRA7xx SoC requires configuring an additional
MMU_CONFIG register present in the DSP_SYSTEM sub module. This
setting dictates whether the DSP Core's MDMA and EDMA traffic
is routed through the respective MMU or not. Add the support
to the OMAP iommu driver so that the traffic is not bypassed
when enabling the MMUs.
The MMU_CONFIG register has two different bits for enabling
each of these two MMUs present in the DSP processor sub-system
on DRA7xx. An id field is added to the OMAP iommu object to
identify and enable each IOMMU. The id information and the
DSP_SYSTEM.MMU_CONFIG register programming is achieved through
the processing of the optional "ti,syscon-mmuconfig" property.
A proper value is assigned to the id field only when this
property is present.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu.c | 58 ++
drivers/iommu/omap-iommu.h | 9 +++
2 files changed, 67 insertions(+)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index fe742c01a4f2..0849a5927732 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -26,6 +26,8 @@
#include
#include
#include
+#include
+#include
#include
@@ -112,6 +114,18 @@ void omap_iommu_restore_ctx(struct device *dev)
}
EXPORT_SYMBOL_GPL(omap_iommu_restore_ctx);
+static void dra7_cfg_dspsys_mmu(struct omap_iommu *obj, bool enable)
+{
+ u32 val, mask;
+
+ if (!obj->syscfg)
+ return;
+
+ mask = (1 << (obj->id * DSP_SYS_MMU_CONFIG_EN_SHIFT));
+ val = enable ? mask : 0;
+ regmap_update_bits(obj->syscfg, DSP_SYS_MMU_CONFIG, mask, val);
+}
+
static void __iommu_set_twl(struct omap_iommu *obj, bool on)
{
u32 l = iommu_read_reg(obj, MMU_CNTL);
@@ -147,6 +161,8 @@ static int omap2_iommu_enable(struct omap_iommu *obj)
iommu_write_reg(obj, pa, MMU_TTB);
+ dra7_cfg_dspsys_mmu(obj, true);
+
if (obj->has_bus_err_back)
iommu_write_reg(obj, MMU_GP_REG_BUS_ERR_BACK_EN, MMU_GP_REG);
@@ -161,6 +177,7 @@ static void omap2_iommu_disable(struct omap_iommu *obj)
l &= ~MMU_CNTL_MASK;
iommu_write_reg(obj, l, MMU_CNTL);
+ dra7_cfg_dspsys_mmu(obj, false);
dev_dbg(obj->dev, "%s is shutting down\n", obj->name);
}
@@ -864,6 +881,42 @@ static void omap_iommu_detach(struct omap_iommu *obj)
dev_dbg(obj->dev, "%s: %s\n", __func__, obj->name);
}
+static int omap_iommu_dra7_get_dsp_system_cfg(struct platform_device *pdev,
+ struct omap_iommu *obj)
+{
+ struct device_node *np = pdev->dev.of_node;
+ int ret;
+
+ if (!of_device_is_compatible(np, "ti,dra7-dsp-iommu"))
+ return 0;
+
+ if (!of_property_read_bool(np, "ti,syscon-mmuconfig")) {
+ dev_err(&pdev->dev, "ti,syscon-mmuconfig property is
missing\n");
+ return -EINVAL;
+ }
+
+ obj->syscfg =
+ syscon_regmap_lookup_by_phandle(np, "ti,syscon-mmuconfig");
+ if (IS_ERR(obj->syscfg)) {
+ /* can fail with -EPROBE_DEFER */
+ ret = PTR_ERR(obj->syscfg);
+ return ret;
+ }
+
+ if (of_property_read_u32_index(np, "ti,syscon-mmuconfig", 1,
+ &obj->id)) {
+ dev_err(&pdev->dev, "couldn't get the IOMMU instance id within
subsystem\n");
+ return -EINVAL;
+ }
+
+ if (obj->id != 0 && obj->id != 1) {
+ dev_err(&pdev->dev, "invalid IOMMU instance id\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
/*
* OMAP Device MMU(IOMMU) detection
*/
@@ -907,6 +960,10 @@ static int omap_iommu_probe(struct platform_device *pdev)
if (IS_ERR(obj->regbase))
return PTR_ERR(obj->regbase);
+ err = omap_iommu_dra7_get_dsp_system_cfg(pdev, obj);
+ if (err)
+ return err;
+
irq = platform_get_irq(pdev, 0);
if (irq < 0)
return -ENODEV;
@@ -943,6 +1000,7 @@ static const struct of_device_id omap_iommu_of_match[] = {
{ .compatible = "ti,omap2-iommu" },
{ .compatible = "ti,omap4-iommu" },
{ .compatible = "ti,dra7-iommu" },
+ { .compatible = "ti,dra7-dsp-iommu" },
{},
};
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index a656df2f9e03..59628e5017b4 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -30,6 +30,7 @@ struct iotlb_entry {
struct omap_iommu {
const char *name;
void __iomem*regbase;
+ struct regmap *syscfg;
struct device *dev;
struct iommu_dom
[PATCH 1/2] Documentation: dt: Update OMAP iommu bindings for DRA7 DSPs
The DSP processor sub-systems on DRA7xx have two MMU instances
each, one for the processor core and the other for an internal
EDMA block. These MMUs need an additional shared register to be
programmed in the DSP_SYSTEM sub-module to be enabled properly.
The OMAP IOMMU bindings is updated to account for this additional
syscon property required for these DSP IOMMU instances on DRA7xx
SoCs. A new compatible "ti,dra7-dsp-iommu" is also defined to
distinguish these devices specifically from other DRA7 IOMMU
devices.
An example of the DRA7 DSP IOMMU nodes is also added to the
document for clarity.
Signed-off-by: Suman Anna
---
.../devicetree/bindings/iommu/ti,omap-iommu.txt| 27 ++
1 file changed, 27 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
index 869699925fd5..4bd10dd881b8 100644
--- a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
+++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
@@ -4,6 +4,7 @@ Required properties:
- compatible : Should be one of,
"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
+ "ti,dra7-dsp-iommu" for DRA7xx DSP IOMMU instances
"ti,dra7-iommu" for DRA7xx IOMMU instances
- ti,hwmods : Name of the hwmod associated with the IOMMU instance
- reg: Address space for the configuration registers
@@ -19,6 +20,13 @@ Optional properties:
Should be either 8 or 32 (default: 32)
- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing
back a bus error response on MMU faults.
+- ti,syscon-mmuconfig : Should be a pair of the phandle to the DSP_SYSTEM
+syscon node that contains the additional control
+register for enabling the MMU, and the MMU instance
+number (0-indexed) within the sub-system. This property
+is required for DSP IOMMU instances on DRA7xx SoCs. The
+instance number should be 0 for DSP MDMA MMUs and 1 for
+DSP EDMA MMUs.
Example:
/* OMAP3 ISP MMU */
@@ -30,3 +38,22 @@ Example:
ti,hwmods = "mmu_isp";
ti,#tlb-entries = <8>;
};
+
+ /* DRA74x DSP2 MMUs */
+ mmu0_dsp2: mmu@41501000 {
+ compatible = "ti,dra7-dsp-iommu";
+ reg = <0x41501000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu0_dsp2";
+ #iommu-cells = <0>;
+ ti,syscon-mmuconfig = <&dsp2_system 0x0>;
+ };
+
+ mmu1_dsp2: mmu@41502000 {
+ compatible = "ti,dra7-dsp-iommu";
+ reg = <0x41502000 0x100>;
+ interrupts = ;
+ ti,hwmods = "mmu1_dsp2";
+ #iommu-cells = <0>;
+ ti,syscon-mmuconfig = <&dsp2_system 0x1>;
+ };
--
2.4.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] ARM: dts: OMAP4: Add #iommu-cells property to IOMMUs
Add missing #iommu-cells property to the DSP and IPU IOMMU nodes
for OMAP4 platforms. This property is required as per the generic
iommu binding.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/omap4.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index f884d6adb71e..7d31c6ff246f 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -551,6 +551,7 @@
reg = <0x4a066000 0x100>;
interrupts = ;
ti,hwmods = "mmu_dsp";
+ #iommu-cells = <0>;
};
mmu_ipu: mmu@55082000 {
@@ -558,6 +559,7 @@
reg = <0x55082000 0x100>;
interrupts = ;
ti,hwmods = "mmu_ipu";
+ #iommu-cells = <0>;
ti,iommu-bus-err-back;
};
--
2.4.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Add missing #iommu-cells on IOMMU nodes
Hi Tony,
This series adds the #iommu-cells property to the existing
IOMMU nodes on OMAP4 & OMAP5. It was already fixed for OMAP3
in commit 2055088b5e17 ("ARM: dts: omap3: Add #iommu-cells to
isp and iva iommu").
OMAP4 and OMAP5 do not yet have users for these IOMMUs (remoteproc
nodes), so the warning as seen with the fix added for OMAP3 is not
seen yet, but will as soon as a user is added.
Patches based on 4.2-rc1.
regards
Suman
Suman Anna (2):
ARM: dts: OMAP4: Add #iommu-cells property to IOMMUs
ARM: dts: OMAP5: Add #iommu-cells property to IOMMUs
arch/arm/boot/dts/omap4.dtsi | 2 ++
arch/arm/boot/dts/omap5.dtsi | 2 ++
2 files changed, 4 insertions(+)
--
2.4.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] ARM: dts: OMAP5: Add #iommu-cells property to IOMMUs
Add missing #iommu-cells property to the DSP and IPU IOMMU nodes
for OMAP5 platforms. This property is required as per the generic
iommu binding.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/omap5.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 7d24ae0306b5..c8fd648a7108 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -612,6 +612,7 @@
reg = <0x4a066000 0x100>;
interrupts = ;
ti,hwmods = "mmu_dsp";
+ #iommu-cells = <0>;
};
mmu_ipu: mmu@55082000 {
@@ -619,6 +620,7 @@
reg = <0x55082000 0x100>;
interrupts = ;
ti,hwmods = "mmu_ipu";
+ #iommu-cells = <0>;
ti,iommu-bus-err-back;
};
--
2.4.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] of: define of_find_node_by_phandle for !CONFIG_OF
On 06/17/2015 06:33 PM, Rob Herring wrote:
> On Wed, Jun 17, 2015 at 11:53 AM, Suman Anna wrote:
>> Define stub implementation for of_find_node_by_phandle() API
>> so that users of this API can build properly even when CONFIG_OF
>> is not defined.
>>
>> Signed-off-by: Suman Anna
>
> Do you have a user for this? If not, apply it when you do.
Yeah, ran into a randconfig build problem on x86 when we tried to add an
API to remoteproc core [1] that used this function.
regards
Suman
[1] http://marc.info/?l=devicetree&m=143232772426559&w=2
>
> Acked-by: Rob Herring
>
>> ---
>> include/linux/of.h | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/include/linux/of.h b/include/linux/of.h
>> index ddeaae6d2083..90dfdc47ae26 100644
>> --- a/include/linux/of.h
>> +++ b/include/linux/of.h
>> @@ -422,6 +422,11 @@ static inline struct device_node
>> *of_find_node_opts_by_path(const char *path,
>> return NULL;
>> }
>>
>> +static inline struct device_node *of_find_node_by_phandle(phandle handle)
>> +{
>> + return NULL;
>> +}
>> +
>> static inline struct device_node *of_get_parent(const struct device_node
>> *node)
>> {
>> return NULL;
>> --
>> 2.4.1
>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] of: define of_find_node_by_phandle for !CONFIG_OF
Define stub implementation for of_find_node_by_phandle() API
so that users of this API can build properly even when CONFIG_OF
is not defined.
Signed-off-by: Suman Anna
---
include/linux/of.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/linux/of.h b/include/linux/of.h
index ddeaae6d2083..90dfdc47ae26 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -422,6 +422,11 @@ static inline struct device_node
*of_find_node_opts_by_path(const char *path,
return NULL;
}
+static inline struct device_node *of_find_node_by_phandle(phandle handle)
+{
+ return NULL;
+}
+
static inline struct device_node *of_get_parent(const struct device_node *node)
{
return NULL;
--
2.4.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] ARM: dts: atlas7: use general dt-binding for hwspinlock
On 06/01/2015 12:31 AM, Barry Song wrote: > 2015-05-29 23:50 GMT+08:00 Bjorn Andersson : >> On Thu, May 28, 2015 at 2:30 PM, Suman Anna wrote: >> [..] >>>> reg = <0x1324 0x0001>; >>> >>> An unrelated question here, why the reg is same for all the child nodes >>> of the parent ipc node? If this is partitioned properly, then the >>> driver can be simplified a bit by using platform_get_resource and >>> devm_ioremap_resource? >>> >> >> Good catch Suman, I missed that. >> >> Barry, if these blocks represents various functionalities of the same >> hw block then you should consider moving them to be part of a >> simple-mfd. > > the hwspinlock and the IPC, which works for kicking interrupts between > multiple cores in the SoC, are in one bus node but there is no > overlapping register. > hwspinlock: > begin from 0x1324 + 0x400 > IPC: > begin from 0x1324, and end at begin from 0x1324 + 0x400 - 0x4 ? > > so i guess we can refine the dts memory region to avoid MFD? Yeah, that should be fine from the hwspinlock perspective, all its registers are self-contained. regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] ARM: dts: atlas7: use general dt-binding for hwspinlock
On 06/01/2015 12:33 AM, Barry Song wrote:
> 2015-05-29 5:30 GMT+08:00 Suman Anna :
>> Barry,
>>
>> On 05/26/2015 03:28 AM, Barry Song wrote:
>>> From: Wei Chen
>>>
>>> This patch moves to use generic dt-binding for hwspinlock providers and
>>> clients.
>>> add #hwlock-cells for the provider and hwlocks for clients.
>>>
>>> Cc: Suman Anna
>>> Cc: Bjorn Andersson
>>> Signed-off-by: Wei Chen
>>> Signed-off-by: Barry Song
>>> ---
>>> arch/arm/boot/dts/atlas7.dtsi | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/atlas7.dtsi b/arch/arm/boot/dts/atlas7.dtsi
>>> index a753178..66d3f0e 100644
>>> --- a/arch/arm/boot/dts/atlas7.dtsi
>>> +++ b/arch/arm/boot/dts/atlas7.dtsi
>>> @@ -84,17 +84,17 @@
>>> #address-cells = <1>;
>>> #size-cells = <1>;
>>>
>>> - hwspinlock {
>>> + hwlock: hwspinlock {
>>> compatible = "sirf,hwspinlock";
>>> reg = <0x1324 0x0001>;
>>
>> An unrelated question here, why the reg is same for all the child nodes
>> of the parent ipc node? If this is partitioned properly, then the
>> driver can be simplified a bit by using platform_get_resource and
>> devm_ioremap_resource?
>>
>>> -
>>> - num-spinlocks = <30>;
>>> + #hwlock-cells = <1>;
>>> };
>>>
>>> ns_m3_rproc@0 {
>>> compatible = "sirf,ns2m30-rproc";
>>> reg = <0x1324 0x0001>;
>>> interrupts = <0 123 0>;
>>> + hwlocks = <&hwlock 0>, <&hwlock 1>;
>>
>> Does this need to be added for the other nodes like ns_m3_rproc@1 as well?
>
> now the hwlock-cells are only added for the nodes who are really using
> it. other users have not used it.
OK, may be split it out then and add these in a subsequent patch with
proper usage description.
regards
Suman
>
>>
>> regards
>> Suman
>>
>>> };
>>>
>>> ns_m3_rproc@1 {
>>>
>>
>
> -barry
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] ARM: dts: atlas7: use general dt-binding for hwspinlock
Barry,
On 05/26/2015 03:28 AM, Barry Song wrote:
> From: Wei Chen
>
> This patch moves to use generic dt-binding for hwspinlock providers and
> clients.
> add #hwlock-cells for the provider and hwlocks for clients.
>
> Cc: Suman Anna
> Cc: Bjorn Andersson
> Signed-off-by: Wei Chen
> Signed-off-by: Barry Song
> ---
> arch/arm/boot/dts/atlas7.dtsi | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/boot/dts/atlas7.dtsi b/arch/arm/boot/dts/atlas7.dtsi
> index a753178..66d3f0e 100644
> --- a/arch/arm/boot/dts/atlas7.dtsi
> +++ b/arch/arm/boot/dts/atlas7.dtsi
> @@ -84,17 +84,17 @@
> #address-cells = <1>;
> #size-cells = <1>;
>
> - hwspinlock {
> + hwlock: hwspinlock {
> compatible = "sirf,hwspinlock";
> reg = <0x1324 0x0001>;
An unrelated question here, why the reg is same for all the child nodes
of the parent ipc node? If this is partitioned properly, then the
driver can be simplified a bit by using platform_get_resource and
devm_ioremap_resource?
> -
> - num-spinlocks = <30>;
> + #hwlock-cells = <1>;
> };
>
> ns_m3_rproc@0 {
> compatible = "sirf,ns2m30-rproc";
> reg = <0x1324 0x0001>;
> interrupts = <0 123 0>;
> + hwlocks = <&hwlock 0>, <&hwlock 1>;
Does this need to be added for the other nodes like ns_m3_rproc@1 as well?
regards
Suman
> };
>
> ns_m3_rproc@1 {
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/3] Documentation: dt: add the CSR atlas7 hwspinlock bindings document
On 05/26/2015 03:28 AM, Barry Song wrote: > From: Wei Chen > > The Hardware Spinlock device on atlas7 provides hardware assistance > for synchronization between the multiple processors in the system > (dual Cortex-A7, CAN bus Cortex-M3 and audio DSP). > This patch adds the DT bindings information for this hwspinlock > module. > > Cc: Suman Anna > Cc: Bjorn Andersson > Signed-off-by: Wei Chen > Signed-off-by: Barry Song > --- Reviewed-by: Suman Anna -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/3] drivers: hwspinlock: add CSR atlas7 implementation
On 05/26/2015 03:28 AM, Barry Song wrote: > From: Wei Chen > > Add hwspinlock support for the CSR atlas7 SoC. > > The Hardware Spinlock device on atlas7 provides hardware assistance > for synchronization between the multiple processors in the system > (dual Cortex-A7, CAN bus Cortex-M3 and audio DSP). > > Cc: Suman Anna > Cc: Bjorn Andersson > Signed-off-by: Wei Chen > Signed-off-by: Barry Song Reviewed-by: Suman Anna regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3 v3] drivers: hwspinlock: add CSR atlas7 implementation
On 05/25/2015 12:37 AM, Barry Song wrote:
> 2015-05-23 6:51 GMT+08:00 Suman Anna :
>> Hi Barry,
>>
>> On 05/19/2015 01:41 AM, Barry Song wrote:
>>> From: Wei Chen
>>>
>>> Add hwspinlock support for the CSR atlas7 SoC.
>>>
>>> The Hardware Spinlock device on atlas7 provides hardware assistance
>>> for synchronization between the multiple processors in the system
>>> (dual Cortex-A7, CAN bus Cortex-M3 and audio DSP).
>>>
>>> Cc: Suman Anna
>>> Cc: Bjorn Andersson
>>> Signed-off-by: Wei Chen
>>> Signed-off-by: Barry Song
>>> ---
>>> -v3:
>>> use #hwlock-cells and general hwspinlock dt-binding;
>>> drop relax();
>>> drop num-spinlocks in dts;
>>> re-order Kconfig and Makefile;
>>> other codingstyle issues.
>>> Thanks Suman, Bjorn and Ohad
>>>
>>> drivers/hwspinlock/Kconfig | 12
>>> drivers/hwspinlock/Makefile | 1 +
>>> drivers/hwspinlock/sirf_hwspinlock.c | 135
>>> +++
>>> 3 files changed, 148 insertions(+)
>>> create mode 100644 drivers/hwspinlock/sirf_hwspinlock.c
>>>
>>> diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig
>>> index b5b4f52..73a4016 100644
>>> --- a/drivers/hwspinlock/Kconfig
>>> +++ b/drivers/hwspinlock/Kconfig
>>> @@ -30,6 +30,18 @@ config HWSPINLOCK_QCOM
>>>
>>> If unsure, say N.
>>>
>>> +config HWSPINLOCK_SIRF
>>> + tristate "SIRF Hardware Spinlock device"
>>> + depends on ARCH_SIRF
>>> + select HWSPINLOCK
>>> + help
>>> + Say y here to support the SIRF Hardware Spinlock device, which
>>> + provides a synchronisation mechanism for the various processors
>>> + on the SoC.
>>> +
>>> + It's safe to say n here if you're not interested in SIRF hardware
>>> + spinlock or just want a bare minimum kernel.
>>> +
>>> config HSEM_U8500
>>> tristate "STE Hardware Semaphore functionality"
>>> depends on ARCH_U8500
>>> diff --git a/drivers/hwspinlock/Makefile b/drivers/hwspinlock/Makefile
>>> index 68f95d9..6b59cb5a 100644
>>> --- a/drivers/hwspinlock/Makefile
>>> +++ b/drivers/hwspinlock/Makefile
>>> @@ -5,4 +5,5 @@
>>> obj-$(CONFIG_HWSPINLOCK) += hwspinlock_core.o
>>> obj-$(CONFIG_HWSPINLOCK_OMAP)+= omap_hwspinlock.o
>>> obj-$(CONFIG_HWSPINLOCK_QCOM)+= qcom_hwspinlock.o
>>> +obj-$(CONFIG_HWSPINLOCK_SIRF)+= sirf_hwspinlock.o
>>> obj-$(CONFIG_HSEM_U8500) += u8500_hsem.o
>>> diff --git a/drivers/hwspinlock/sirf_hwspinlock.c
>>> b/drivers/hwspinlock/sirf_hwspinlock.c
>>> new file mode 100644
>>> index 000..e7e5ba6
>>> --- /dev/null
>>> +++ b/drivers/hwspinlock/sirf_hwspinlock.c
>>> @@ -0,0 +1,135 @@
>>> +/*
>>> + * SIRF hardware spinlock driver
>>> + *
>>> + * Copyright (c) 2014 Cambridge Silicon Radio Limited, a CSR plc group
>>> company.
>>
>> Not sure on this, but 2015 is here and now..
>>
>>> + *
>>> + * Licensed under GPLv2.
>>> + */
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +#include "hwspinlock_internal.h"
>>> +
>>> +struct sirf_hwspinlock {
>>> + void __iomem *io_base;
>>> + struct hwspinlock_device bank;
>>> +};
>>> +
>>> +/* Number of Hardware Spinlocks*/
>>> +#define HW_SPINLOCK_NUMBER 30
>>> +
>>> +/* Hardware spinlock register offsets */
>>> +#define HW_SPINLOCK_BASE 0x404
>>> +#define HW_SPINLOCK_OFFSET(x)(HW_SPINLOCK_BASE + 0x4 * (x))
>>> +
>>> +static int sirf_hwspinlock_trylock(struct hwspinlock *lock)
>>> +{
>>> + void __iomem *lock_addr = lock->priv;
>>> +
>>> + /* attempt to acquire the lock by reading value == 1 from it */
>>> + return !!readl(lock_addr);
>>> +}
>>> +
>>> +static void sirf_hwspinlock_unlock(struct hwspinlock *lock)
>>> +{
>>> + void __iomem *lock_ad
Re: [PATCH 2/3 v3] Documentation: dt: add the CSR atlas7 hwspinlock bindings document
On 05/25/2015 12:33 AM, Barry Song wrote: > 2015-05-23 6:44 GMT+08:00 Suman Anna : >> Hi Barry, >> >> On 05/19/2015 01:41 AM, Barry Song wrote: >>> From: Wei Chen >>> >>> The Hardware Spinlock device on atlas7 provides hardware assistance >>> for synchronization between the multiple processors in the system >>> (dual Cortex-A7, CAN bus Cortex-M3 and audio DSP). >>> This patch adds the DT bindings information for this hwspinlock >>> module. >>> >>> Cc: Suman Anna >>> Cc: Bjorn Andersson >>> Signed-off-by: Wei Chen >>> Signed-off-by: Barry Song >>> --- >>> .../devicetree/bindings/hwlock/sirf,hwspinlock.txt | 25 >>> ++ >>> 1 file changed, 25 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/hwlock/sirf,hwspinlock.txt >>> >>> diff --git a/Documentation/devicetree/bindings/hwlock/sirf,hwspinlock.txt >>> b/Documentation/devicetree/bindings/hwlock/sirf,hwspinlock.txt >>> new file mode 100644 >>> index 000..cc6d351 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/hwlock/sirf,hwspinlock.txt >>> @@ -0,0 +1,25 @@ >>> +SIRF Hardware spinlock device Binding >>> +--- >>> + >>> +Required properties : >>> +- compatible : shall contain only one of the following: >>> + "sirf,hwspinlock" >>> + >>> +- reg : the register address of hwspinlock >> >> Also suggest to document the value to be used for #hwlock-cells, the >> generic hwlock binding does not mention that. > > do you think whether we can put one line like? > > #hwlock-cells : <1>, as required by generic hwspinlock binding. The generic hwspinlock binding does not require a specific value, that is something determined by the individual platform implementation. So, something like "#hwlock-cells: Should be 1" is fine. regards Suman >> >>> + >>> +Please look at the generic hwlock binding for usage information for >>> consumers, >>> +"Documentation/devicetree/bindings/hwlock/hwlock.txt" >>> + >> >> regards >> Suman >> > > -barry > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3 v3] drivers: hwspinlock: add CSR atlas7 implementation
Hi Barry,
On 05/19/2015 01:41 AM, Barry Song wrote:
> From: Wei Chen
>
> Add hwspinlock support for the CSR atlas7 SoC.
>
> The Hardware Spinlock device on atlas7 provides hardware assistance
> for synchronization between the multiple processors in the system
> (dual Cortex-A7, CAN bus Cortex-M3 and audio DSP).
>
> Cc: Suman Anna
> Cc: Bjorn Andersson
> Signed-off-by: Wei Chen
> Signed-off-by: Barry Song
> ---
> -v3:
> use #hwlock-cells and general hwspinlock dt-binding;
> drop relax();
> drop num-spinlocks in dts;
> re-order Kconfig and Makefile;
> other codingstyle issues.
> Thanks Suman, Bjorn and Ohad
>
> drivers/hwspinlock/Kconfig | 12
> drivers/hwspinlock/Makefile | 1 +
> drivers/hwspinlock/sirf_hwspinlock.c | 135
> +++
> 3 files changed, 148 insertions(+)
> create mode 100644 drivers/hwspinlock/sirf_hwspinlock.c
>
> diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig
> index b5b4f52..73a4016 100644
> --- a/drivers/hwspinlock/Kconfig
> +++ b/drivers/hwspinlock/Kconfig
> @@ -30,6 +30,18 @@ config HWSPINLOCK_QCOM
>
> If unsure, say N.
>
> +config HWSPINLOCK_SIRF
> + tristate "SIRF Hardware Spinlock device"
> + depends on ARCH_SIRF
> + select HWSPINLOCK
> + help
> + Say y here to support the SIRF Hardware Spinlock device, which
> + provides a synchronisation mechanism for the various processors
> + on the SoC.
> +
> + It's safe to say n here if you're not interested in SIRF hardware
> + spinlock or just want a bare minimum kernel.
> +
> config HSEM_U8500
> tristate "STE Hardware Semaphore functionality"
> depends on ARCH_U8500
> diff --git a/drivers/hwspinlock/Makefile b/drivers/hwspinlock/Makefile
> index 68f95d9..6b59cb5a 100644
> --- a/drivers/hwspinlock/Makefile
> +++ b/drivers/hwspinlock/Makefile
> @@ -5,4 +5,5 @@
> obj-$(CONFIG_HWSPINLOCK) += hwspinlock_core.o
> obj-$(CONFIG_HWSPINLOCK_OMAP)+= omap_hwspinlock.o
> obj-$(CONFIG_HWSPINLOCK_QCOM)+= qcom_hwspinlock.o
> +obj-$(CONFIG_HWSPINLOCK_SIRF)+= sirf_hwspinlock.o
> obj-$(CONFIG_HSEM_U8500) += u8500_hsem.o
> diff --git a/drivers/hwspinlock/sirf_hwspinlock.c
> b/drivers/hwspinlock/sirf_hwspinlock.c
> new file mode 100644
> index 000..e7e5ba6
> --- /dev/null
> +++ b/drivers/hwspinlock/sirf_hwspinlock.c
> @@ -0,0 +1,135 @@
> +/*
> + * SIRF hardware spinlock driver
> + *
> + * Copyright (c) 2014 Cambridge Silicon Radio Limited, a CSR plc group
> company.
Not sure on this, but 2015 is here and now..
> + *
> + * Licensed under GPLv2.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "hwspinlock_internal.h"
> +
> +struct sirf_hwspinlock {
> + void __iomem *io_base;
> + struct hwspinlock_device bank;
> +};
> +
> +/* Number of Hardware Spinlocks*/
> +#define HW_SPINLOCK_NUMBER 30
> +
> +/* Hardware spinlock register offsets */
> +#define HW_SPINLOCK_BASE 0x404
> +#define HW_SPINLOCK_OFFSET(x)(HW_SPINLOCK_BASE + 0x4 * (x))
> +
> +static int sirf_hwspinlock_trylock(struct hwspinlock *lock)
> +{
> + void __iomem *lock_addr = lock->priv;
> +
> + /* attempt to acquire the lock by reading value == 1 from it */
> + return !!readl(lock_addr);
> +}
> +
> +static void sirf_hwspinlock_unlock(struct hwspinlock *lock)
> +{
> + void __iomem *lock_addr = lock->priv;
> +
> + /* release the lock by writing 0 to it */
> + writel(0, lock_addr);
> +}
> +
> +static const struct hwspinlock_ops sirf_hwspinlock_ops = {
> + .trylock = sirf_hwspinlock_trylock,
> + .unlock = sirf_hwspinlock_unlock,
> +};
> +
> +static int sirf_hwspinlock_probe(struct platform_device *pdev)
> +{
> + struct sirf_hwspinlock *hwspin;
> + struct hwspinlock *hwlock;
> + int idx, ret;
> +
> + if (!pdev->dev.of_node)
> + return -ENODEV;
> +
> + hwspin = devm_kzalloc(&pdev->dev, sizeof(*hwspin) +
> + sizeof(*hwlock) * HW_SPINLOCK_NUMBER, GFP_KERNEL);
> + if (!hwspin)
> + return -ENOMEM;
> +
> + /* retrieve io base */
> + hwspin->io_base = of_iomap(pdev->dev.of_node, 0);
> + if (!hwspin->io_base)
> + ret = -ENOMEM;
You are missing the bail out here.
> +
> + for (idx = 0; idx < HW_SPINLOCK_NUM
Re: [PATCH 2/3 v3] Documentation: dt: add the CSR atlas7 hwspinlock bindings document
Hi Barry, On 05/19/2015 01:41 AM, Barry Song wrote: > From: Wei Chen > > The Hardware Spinlock device on atlas7 provides hardware assistance > for synchronization between the multiple processors in the system > (dual Cortex-A7, CAN bus Cortex-M3 and audio DSP). > This patch adds the DT bindings information for this hwspinlock > module. > > Cc: Suman Anna > Cc: Bjorn Andersson > Signed-off-by: Wei Chen > Signed-off-by: Barry Song > --- > .../devicetree/bindings/hwlock/sirf,hwspinlock.txt | 25 > ++ > 1 file changed, 25 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/hwlock/sirf,hwspinlock.txt > > diff --git a/Documentation/devicetree/bindings/hwlock/sirf,hwspinlock.txt > b/Documentation/devicetree/bindings/hwlock/sirf,hwspinlock.txt > new file mode 100644 > index 000..cc6d351 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwlock/sirf,hwspinlock.txt > @@ -0,0 +1,25 @@ > +SIRF Hardware spinlock device Binding > +--- > + > +Required properties : > +- compatible : shall contain only one of the following: > + "sirf,hwspinlock" > + > +- reg : the register address of hwspinlock Also suggest to document the value to be used for #hwlock-cells, the generic hwlock binding does not mention that. > + > +Please look at the generic hwlock binding for usage information for > consumers, > +"Documentation/devicetree/bindings/hwlock/hwlock.txt" > + regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/4] remoteproc: introduce rproc_get_by_phandle API
Hi Ohad,
On 05/09/2015 02:39 AM, Ohad Ben-Cohen wrote:
> Hi Dave,
>
> On Wed, Apr 1, 2015 at 10:37 PM, Dave Gerlach wrote:
>> Allow users of remoteproc the ability to get a handle to an rproc by
>> passing a phandle supplied in the user's device tree node. This is
>> useful in situations that require manual booting of the rproc.
>>
>> This patch uses the code removed by commit 40e575b1d0b3 ("remoteproc:
>> remove the get_by_name/put API") for the ref counting a rproc klist
>> code but has rproc_get_by_name replaced with an rproc_get_by_phandle API.
>
> The general idea makes sense to me, but I'm not sure we really do need
> a klist here, since the usage profile of this list is expected to be
> super simple: very small number of accessors, looking for small number
> of list members a small number of times, and probably never do need to
> modify the list while accessing it.
>
> I suspect that the code would be simpler to maintain, debug and
> understand if we just use a simple list with a simple locking
> methodology here.
The klist usage is something that we restored from previous remoteproc
core code as used by the rproc_get_by_name() API. This was removed in
commit 40e575b1d0b3 ("remoteproc: remove the get_by_name/put API"). We
chose to use the code that had been present before rather than inventing
something new all over again. If you feel that a regular list is the way
to go forward, we can make the switch.
regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 2/9] mailbox: Make struct mbox_controller's ops field const
Hi Andrew,
On 04/27/2015 05:37 PM, Andrew Bresticker wrote:
> The mailbox controller's channel ops ought to be read-only.
We ought to change this on all the existing controllers as well.
regards
Suman
>
> Signed-off-by: Andrew Bresticker
> Cc: Jassi Brar
> ---
> No changes from v5/v6.
> New for v5.
> ---
> include/linux/mailbox_controller.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/mailbox_controller.h
> b/include/linux/mailbox_controller.h
> index d4cf96f..68c4245 100644
> --- a/include/linux/mailbox_controller.h
> +++ b/include/linux/mailbox_controller.h
> @@ -72,7 +72,7 @@ struct mbox_chan_ops {
> */
> struct mbox_controller {
> struct device *dev;
> - struct mbox_chan_ops *ops;
> + const struct mbox_chan_ops *ops;
> struct mbox_chan *chans;
> int num_chans;
> bool txdone_irq;
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 3/9] mailbox: Fix up error handling in mbox_request_channel()
On 04/27/2015 05:37 PM, Andrew Bresticker wrote:
> From: Benson Leung
>
> mbox_request_channel() currently returns EBUSY in the event the controller
> is not present or if of_xlate() fails, but in neither case is EBUSY really
> appropriate. Return EPROBE_DEFER if the controller is not yet present
> and change of_xlate() to return an ERR_PTR instead of NULL so that the
> error can be propagated back to the caller of mbox_request_channel().
>
> Signed-off-by: Benson Leung
> Signed-off-by: Andrew Bresticker
> Cc: Jassi Brar
> Cc: Suman Anna
> ---
> Changes from v6:
> - Update omap-mailbox's xlate() to return error codes.
> No changes from v5.
> New for v5.
> ---
> drivers/mailbox/mailbox.c | 11 ---
> drivers/mailbox/omap-mailbox.c | 6 +++---
For the OMAP mailbox portion,
Acked-by: Suman Anna
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> index 19b491d..c3c42d4 100644
> --- a/drivers/mailbox/mailbox.c
> +++ b/drivers/mailbox/mailbox.c
> @@ -318,7 +318,7 @@ struct mbox_chan *mbox_request_channel(struct mbox_client
> *cl, int index)
> return ERR_PTR(-ENODEV);
> }
>
> - chan = NULL;
> + chan = ERR_PTR(-EPROBE_DEFER);
> list_for_each_entry(mbox, &mbox_cons, node)
> if (mbox->dev->of_node == spec.np) {
> chan = mbox->of_xlate(mbox, &spec);
> @@ -327,7 +327,12 @@ struct mbox_chan *mbox_request_channel(struct
> mbox_client *cl, int index)
>
> of_node_put(spec.np);
>
> - if (!chan || chan->cl || !try_module_get(mbox->dev->driver->owner)) {
> + if (IS_ERR(chan)) {
> + mutex_unlock(&con_mutex);
> + return chan;
> + }
> +
> + if (chan->cl || !try_module_get(mbox->dev->driver->owner)) {
> dev_dbg(dev, "%s: mailbox not free\n", __func__);
> mutex_unlock(&con_mutex);
> return ERR_PTR(-EBUSY);
> @@ -390,7 +395,7 @@ of_mbox_index_xlate(struct mbox_controller *mbox,
> int ind = sp->args[0];
>
> if (ind >= mbox->num_chans)
> - return NULL;
> + return ERR_PTR(-EINVAL);
>
> return &mbox->chans[ind];
> }
> diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
> index 0f332c1..e0df27b 100644
> --- a/drivers/mailbox/omap-mailbox.c
> +++ b/drivers/mailbox/omap-mailbox.c
> @@ -639,18 +639,18 @@ static struct mbox_chan *omap_mbox_of_xlate(struct
> mbox_controller *controller,
>
> mdev = container_of(controller, struct omap_mbox_device, controller);
> if (WARN_ON(!mdev))
> - return NULL;
> + return ERR_PTR(-EINVAL);
>
> node = of_find_node_by_phandle(phandle);
> if (!node) {
> pr_err("%s: could not find node phandle 0x%x\n",
> __func__, phandle);
> - return NULL;
> + return ERR_PTR(-ENODEV);
> }
>
> mbox = omap_mbox_device_find(mdev, node->name);
> of_node_put(node);
> - return mbox ? mbox->chan : NULL;
> + return mbox ? mbox->chan : ERR_PTR(-ENOENT);
> }
>
> static int omap_mbox_probe(struct platform_device *pdev)
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bus: omap_l3_noc: Fix master id address decoding for OMAP5
On 04/24/2015 02:38 PM, Nishanth Menon wrote:
> On Fri, Apr 24, 2015 at 2:10 PM, Suman Anna wrote:
>> On 04/24/2015 01:33 PM, Nishanth Menon wrote:
>>> On 04/24/2015 12:54 PM, Suman Anna wrote:
>
> ...
>>>> -static struct l3_target_data omap_l3_target_data_clk3[] = {
>>>> -{0x0100, "EMUSS",},
>>>> -{0x0300, "DEBUG SOURCE",},
>>>> -{0x0, "HOST CLK3",},
> ^^ this was HOST CLK3
> ..
>>>>
>>>> +/* OMAP5 data */
>>>> +static struct l3_target_data omap5_l3_target_data_clk3[] = {
>>>> +{0x0100, "L3INSTR",},
>>>> +{0x0300, "DEBUGSS",},
>>>> +{0x0,"HOSTCLK3",},
>>>
>>> "HOST CLK"
>>
>> Why? I followed the convention used for the other two HOST CLKs for the
>
> so asked, if it should be HOSTCLK3
Aah ok, you missed the trailing number before. In anycase, this was
intentional to match HOSTCLK1 and HOSTCLK2 on OMAP4/5. Overall, the
names are somewhat non-standard, some use underscores and some others
strip out the underscore and do not use any spaces in between either.
"HOST CLK3" would be the only one to use a space for OMAP4, so I got rid
of it, so hope that's ok with you.
regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bus: omap_l3_noc: Fix master id address decoding for OMAP5
On 04/24/2015 01:33 PM, Nishanth Menon wrote: > On 04/24/2015 12:54 PM, Suman Anna wrote: >> The L3 Error handling on OMAP5 for the most part is very similar >> to that of OMAP4, and had leveraged common data structures and >> register layout definitions so far. Upon closer inspection, there >> are a few minor differences causing an incorrect decoding and >> reporting of the master NIU upon an error: >> >> 1. The L3_TARG_STDERRLOG_MSTADDR.STDERRLOG_MSTADDR occupies >> 11 bits on OMAP5 as against 8 bits on OMAP4, with the master >> NIU connID encoded in the 6 MSBs of the STDERRLOG_MSTADDR >> field. >> 2. The CLK3 FlagMux component has 1 input source on OMAP4 and 3 >> input sources on OMAP5. The common DEBUGSS source is at a >> different input on each SoC. >> >> Fix the above issues by using a OMAP5-specific compatible property >> and using SoC-specific data where there are differences. >> >> Cc: Nishanth Menon >> Signed-off-by: Suman Anna >> --- >> >> Some validation traces by adding couple of traces and intentionally >> creating L3 errors from DSP & IPU by accessing invalid Timers >> >> Before Patch: >> OMAP4 [Correct] >> IPU accessing Timer4 >> [ 46.548095] flagmux = 1, err_reg = 0x8000 err_src = 0xf >> [ 46.553344] mstaddr = 0x44 mask = 0xfc masterid = 0x11 >> [ 46.553955] [ cut here ] >> [ 46.563171] WARNING: CPU: 0 PID: 4 at drivers/bus/omap_l3_noc.c:149 >> l3_interrupt_handler+0x280/0x388() >> [ 46.564941] 4400.ocp:L3 Custom Error: MASTER DucatiM3 TARGET L4PER3 >> (Idle): Data Access in User mode during >> Functional access >> >> DSP accessing Timer5: >> [ 114.018524] flagmux = 0, err_reg = 0x4 err_src = 0x2 >> [ 114.023498] mstaddr = 0x20 mask = 0xfc masterid = 0x8 >> [ 114.028564] [ cut here ] >> [ 114.033233] WARNING: CPU: 0 PID: 0 at drivers/bus/omap_l3_noc.c:149 >> l3_interrupt_handler+0x280/0x388() >> [ 114.042572] 4400.ocp:L3 Custom Error: MASTER DSP TARGET ABE (Idle): >> Data Access in Supervisor mode during Functional >> access >> >> OMAP5 [Incorrect] >> IPU accessing Timer4: >> [ 29.579306] flagmux = 1, err_reg = 0x8000 err_src = 0xf >> [ 29.584550] mstaddr = 0x220 mask = 0xfc masterid = 0x8 >> [ 29.589705] [ cut here ] >> [ 29.594345] WARNING: CPU: 0 PID: 61 at drivers/bus/omap_l3_noc.c:149 >> l3_interrupt_handler+0x280/0x388() >> [ 29.603774] 4400.ocp:L3 Custom Error: MASTER DSP TARGET L4PER3 >> (Idle): Data Access in User mode during Functional >> access >> >> DSP accessing Timer5: >> [ 21.347105] flagmux = 0, err_reg = 0x4 err_src = 0x2 >> [ 21.352091] mstaddr = 0x100 mask = 0xfc masterid = 0x0 >> [ 21.357250] [ cut here ] >> [ 21.361896] WARNING: CPU: 0 PID: 0 at drivers/bus/omap_l3_noc.c:149 >> l3_interrupt_handler+0x280/0x388() >> [ 21.371242] 4400.ocp:L3 Custom Error: MASTER MPU TARGET ABE (Idle): >> Data Access in Supervisor mode during Functional >> access >> >> After Patch: >> OMAP4 same as above >> >> OMAP5 [Corrected] >> IPU accessing Timer4 >> [ 67.896693] flagmux = 1, err_reg = 0x8000 err_src = 0xf >> [ 67.901940] mstaddr = 0x220 mask = 0x7e0 masterid = 0x11 >> [ 67.907275] [ cut here ] >> [ 67.911924] WARNING: CPU: 0 PID: 61 at drivers/bus/omap_l3_noc.c:149 >> l3_interrupt_handler+0x280/0x388() >> [ 67.921357] 4400.ocp:L3 Custom Error: MASTER DucatiM3 TARGET L4PER3 >> (Idle): Data Access in User mode during >> Functional access >> >> DSP accessing Timer5 >> [ 24.452565] flagmux = 0, err_reg = 0x4 err_src = 0x2 >> [ 24.457552] mstaddr = 0x100 mask = 0x7e0 masterid = 0x8 >> [ 24.462798] [ cut here ] >> [ 24.467449] WARNING: CPU: 0 PID: 0 at drivers/bus/omap_l3_noc.c:149 >> l3_interrupt_handler+0x280/0x388() >> [ 24.476795] 4400.ocp:L3 Custom Error: MASTER DSP TARGET ABE (Idle): >> Data Access in Supervisor mode during Functional >> access >> >> >> .../devicetree/bindings/arm/omap/l3-noc.txt| 1 + >> arch/arm/boot/dts/omap5.dtsi | 2 +- >> drivers/bus/omap_l3_noc.c | 5 ++- >> drivers/bus/omap_l3_noc.h | 52 >> -- >> 4 files changed, 44 insertions(+), 16 deletions(-) >> >> diff --git a/Documentation/devicetre
[PATCH] bus: omap_l3_noc: Fix master id address decoding for OMAP5
The L3 Error handling on OMAP5 for the most part is very similar
to that of OMAP4, and had leveraged common data structures and
register layout definitions so far. Upon closer inspection, there
are a few minor differences causing an incorrect decoding and
reporting of the master NIU upon an error:
1. The L3_TARG_STDERRLOG_MSTADDR.STDERRLOG_MSTADDR occupies
11 bits on OMAP5 as against 8 bits on OMAP4, with the master
NIU connID encoded in the 6 MSBs of the STDERRLOG_MSTADDR
field.
2. The CLK3 FlagMux component has 1 input source on OMAP4 and 3
input sources on OMAP5. The common DEBUGSS source is at a
different input on each SoC.
Fix the above issues by using a OMAP5-specific compatible property
and using SoC-specific data where there are differences.
Cc: Nishanth Menon
Signed-off-by: Suman Anna
---
Some validation traces by adding couple of traces and intentionally
creating L3 errors from DSP & IPU by accessing invalid Timers
Before Patch:
OMAP4 [Correct]
IPU accessing Timer4
[ 46.548095] flagmux = 1, err_reg = 0x8000 err_src = 0xf
[ 46.553344] mstaddr = 0x44 mask = 0xfc masterid = 0x11
[ 46.553955] [ cut here ]
[ 46.563171] WARNING: CPU: 0 PID: 4 at drivers/bus/omap_l3_noc.c:149
l3_interrupt_handler+0x280/0x388()
[ 46.564941] 4400.ocp:L3 Custom Error: MASTER DucatiM3 TARGET L4PER3
(Idle): Data Access in User mode during
Functional access
DSP accessing Timer5:
[ 114.018524] flagmux = 0, err_reg = 0x4 err_src = 0x2
[ 114.023498] mstaddr = 0x20 mask = 0xfc masterid = 0x8
[ 114.028564] [ cut here ]
[ 114.033233] WARNING: CPU: 0 PID: 0 at drivers/bus/omap_l3_noc.c:149
l3_interrupt_handler+0x280/0x388()
[ 114.042572] 4400.ocp:L3 Custom Error: MASTER DSP TARGET ABE (Idle): Data
Access in Supervisor mode during Functional
access
OMAP5 [Incorrect]
IPU accessing Timer4:
[ 29.579306] flagmux = 1, err_reg = 0x8000 err_src = 0xf
[ 29.584550] mstaddr = 0x220 mask = 0xfc masterid = 0x8
[ 29.589705] [ cut here ]
[ 29.594345] WARNING: CPU: 0 PID: 61 at drivers/bus/omap_l3_noc.c:149
l3_interrupt_handler+0x280/0x388()
[ 29.603774] 4400.ocp:L3 Custom Error: MASTER DSP TARGET L4PER3 (Idle):
Data Access in User mode during Functional
access
DSP accessing Timer5:
[ 21.347105] flagmux = 0, err_reg = 0x4 err_src = 0x2
[ 21.352091] mstaddr = 0x100 mask = 0xfc masterid = 0x0
[ 21.357250] [ cut here ]
[ 21.361896] WARNING: CPU: 0 PID: 0 at drivers/bus/omap_l3_noc.c:149
l3_interrupt_handler+0x280/0x388()
[ 21.371242] 4400.ocp:L3 Custom Error: MASTER MPU TARGET ABE (Idle): Data
Access in Supervisor mode during Functional
access
After Patch:
OMAP4 same as above
OMAP5 [Corrected]
IPU accessing Timer4
[ 67.896693] flagmux = 1, err_reg = 0x8000 err_src = 0xf
[ 67.901940] mstaddr = 0x220 mask = 0x7e0 masterid = 0x11
[ 67.907275] [ cut here ]
[ 67.911924] WARNING: CPU: 0 PID: 61 at drivers/bus/omap_l3_noc.c:149
l3_interrupt_handler+0x280/0x388()
[ 67.921357] 4400.ocp:L3 Custom Error: MASTER DucatiM3 TARGET L4PER3
(Idle): Data Access in User mode during
Functional access
DSP accessing Timer5
[ 24.452565] flagmux = 0, err_reg = 0x4 err_src = 0x2
[ 24.457552] mstaddr = 0x100 mask = 0x7e0 masterid = 0x8
[ 24.462798] [ cut here ]
[ 24.467449] WARNING: CPU: 0 PID: 0 at drivers/bus/omap_l3_noc.c:149
l3_interrupt_handler+0x280/0x388()
[ 24.476795] 4400.ocp:L3 Custom Error: MASTER DSP TARGET ABE (Idle): Data
Access in Supervisor mode during Functional
access
.../devicetree/bindings/arm/omap/l3-noc.txt| 1 +
arch/arm/boot/dts/omap5.dtsi | 2 +-
drivers/bus/omap_l3_noc.c | 5 ++-
drivers/bus/omap_l3_noc.h | 52 --
4 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/omap/l3-noc.txt
b/Documentation/devicetree/bindings/arm/omap/l3-noc.txt
index 974624ea68f6..161448da959d 100644
--- a/Documentation/devicetree/bindings/arm/omap/l3-noc.txt
+++ b/Documentation/devicetree/bindings/arm/omap/l3-noc.txt
@@ -6,6 +6,7 @@ provided by Arteris.
Required properties:
- compatible : Should be "ti,omap3-l3-smx" for OMAP3 family
Should be "ti,omap4-l3-noc" for OMAP4 family
+ Should be "ti,omap5-l3-noc" for OMAP5 family
Should be "ti,dra7-l3-noc" for DRA7 family
Should be "ti,am4372-l3-noc" for AM43 family
- reg: Contains L3 register address range for each noc domain.
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index efe5f737f39b..7d24ae0306b5 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -128,7 +128,7 @@
* hierarchy.
*/
ocp {
-
Re: [PATCH v8 0/4] hwspinlock core & omap dt support
Mark, On 03/18/2015 04:57 PM, Suman Anna wrote: > Hi Mark, > > On 03/12/2015 04:24 AM, Ohad Ben-Cohen wrote: >> Hi Suman, >> >> On Thu, Mar 5, 2015 at 4:01 AM, Suman Anna wrote: >>> This is the latest version of the hwspinlock dt support series, >>> rebased onto v4.0-rc1 and addressing the long discussion on the >>> bindings in v7 [1]. I really hope that this series can make it >>> into 4.1. >> >> From a quick glance this looks great, thanks! >> >>> Mark, >>> Can you please provide your Acked-by for the binding documents >>> so that Ohad can pick up the patches for the next merge window? >> >> That would be perfect. Once we'll have it I could move forward with >> this towards 4.1. > > Gentle reminder. Can you please provide your ack on the bindings in this > series (Patches 1 & 3) for Ohad to queue up the series for 4.1. > Ping again, if you can provide your ack on these bindings and the qcom hwmutex bindings, then Ohad can pick up both the series for 4.1. Both series are blocked on getting the binding acks. regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 0/4] hwspinlock core & omap dt support
Hi Mark, On 03/12/2015 04:24 AM, Ohad Ben-Cohen wrote: > Hi Suman, > > On Thu, Mar 5, 2015 at 4:01 AM, Suman Anna wrote: >> This is the latest version of the hwspinlock dt support series, >> rebased onto v4.0-rc1 and addressing the long discussion on the >> bindings in v7 [1]. I really hope that this series can make it >> into 4.1. > > From a quick glance this looks great, thanks! > >> Mark, >> Can you please provide your Acked-by for the binding documents >> so that Ohad can pick up the patches for the next merge window? > > That would be perfect. Once we'll have it I could move forward with > this towards 4.1. Gentle reminder. Can you please provide your ack on the bindings in this series (Patches 1 & 3) for Ohad to queue up the series for 4.1. Thanks Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: dts: DRA7: Remove ti,timer-dsp and ti,timer-pwm properties
Remove the 'ti,timer-dsp' and 'ti,timer-pwm' properties from the timer
nodes that still have them. This seems to be copied from OMAP5, on
which only certain timers are capable of providing PWM functionality
or be able to interrupt the DSP. All the GPTimers On DRA7 are capable
of PWM and interrupting any core (due to the presence of Crossbar).
These properties were used by the driver to add capabilities to each
timer, and support requesting timers by capability. In the DT world,
we expect any users of timers to use phandles to the respective timer,
and use the omap_dm_timer_request_by_node() API. The API to request
using capabilities, omap_dm_timer_request_by_cap() API should be
deprecated eventually.
Signed-off-by: Suman Anna
---
arch/arm/boot/dts/dra7.dtsi | 7 ---
1 file changed, 7 deletions(-)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 5827fedafd43..318d9409e8cd 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -658,7 +658,6 @@
reg = <0x4882 0x80>;
interrupts = ;
ti,hwmods = "timer5";
- ti,timer-dsp;
};
timer6: timer@48822000 {
@@ -666,8 +665,6 @@
reg = <0x48822000 0x80>;
interrupts = ;
ti,hwmods = "timer6";
- ti,timer-dsp;
- ti,timer-pwm;
};
timer7: timer@48824000 {
@@ -675,7 +672,6 @@
reg = <0x48824000 0x80>;
interrupts = ;
ti,hwmods = "timer7";
- ti,timer-dsp;
};
timer8: timer@48826000 {
@@ -683,8 +679,6 @@
reg = <0x48826000 0x80>;
interrupts = ;
ti,hwmods = "timer8";
- ti,timer-dsp;
- ti,timer-pwm;
};
timer9: timer@4803e000 {
@@ -706,7 +700,6 @@
reg = <0x48088000 0x80>;
interrupts = ;
ti,hwmods = "timer11";
- ti,timer-pwm;
};
timer13: timer@48828000 {
--
2.3.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] ARM: dts: am33xx: Move wkup_m3 node to soc node and add ranges
On 03/11/2015 12:32 PM, Tony Lindgren wrote:
> * Suman Anna [150311 10:18]:
>> On 03/11/2015 11:26 AM, Tony Lindgren wrote:
>>> * Dave Gerlach [150310 12:55]:
>>>> Suman and I have been looking at this together, so I can comment here. An
>>>> implementation like this is what Suman is referring to:
>>>>
>>>> + l4_wkup: l4_wkup@44c0 {
>>>> + compatible = "am335-l4-wkup", "simple-bus";
>>>> + #address-cells = <2>;
>>>> + #size-cells = <1>;
>>>> + ranges = <0 0 0x44c0 0x10>,
>>>> +<1 0x0 0x44d0 0x4000>,
>>>> +<2 0x8 0x44d8 0x2000>;
>>
>> Actually, this would be slightly different, something like
>> + ranges = <0 00x44c0 0x10>,
>> +<1 00x44d0 0x10>,
>> +<2 00x44e0 0x4000>,
>> + <3 00x44e1 0x2000>;
>>
>> and the M3 DMEM entry below will be adjusted as <1 0x8 0x2000>.
>>
>>>> +
>>>> + wkup_m3: wkup_m3@1,0 {
>>>> + compatible = "ti,am3353-wkup-m3";
>>>> + reg = <1 0x0 0x4000>, /* M3 UMEM
>>>> */
>>>> + <2 0x8 0x2000>; /* M3 DMEM
>>>> */
>>>> +
>>>> + ti,hwmods = "wkup_m3";
>>>> + ti,pm-firmware = "am335x-pm-firmware.elf";
>>>> + };
>>>> + };
>>>> +
>>>>
>>>> The of_* layer automatically translates everything so the pdata-quirks can
>>>> still
>>>> match based on wkup_m3@44d0. The existing wkup_m3_rproc driver works
>>>> almost
>>>> entirely as is with this, all cpu addresses are read and mapped correctly
>>>> but
>>>> the driver no longer will read the actual device addresses correctly which
>>>> we
>>>> need for understanding where to load the firmware sections.
>>>
>>> OK. I still don't quite understand how these additional ranges make sense
>>> for other drivers connected to the l4_wkup. For wkup_m3, it makes sense if
>>> it allows you to translate directly to the m3 address space, but is that
>>> really the case here? Maybe you should have the ranges in wkup_m3 instead
>>> if you want addresses for the m3?
>>
>> The idea is to introduce an additional address element (first cell in
>> ranges) so that the immediate child nodes bus address is referenced as 0
>> (second cell) for translation for their child nodes. This is the
>> approach used by the current scm node in Tero's series for OMAP4+. This
>> will work tomorrow if we move the prcm, scrm node under l4_wkup with
>> changes only in those nodes, and have their child nodes reg properties
>> unchanged. I guess you can see the difference between the following two
>> patches from Tero's PRCM series,
>> https://patchwork.kernel.org/patch/5882831/ &
>> https://patchwork.kernel.org/patch/5882841/
>
> Well I just commented on Tero on that regarding the dra7 patch. I think
> we need to have separate scm instances for scm_device, scm_core and
> scm_wkup instead of doing multiple ranges. This based on looking at for
> example 5432 TRM "Figure 18-1. Control Module Overview".
>
> But here I think it's a different issue. You want to use ranges for getting
> the m3 address space for the firmware? I'm not convinced we should
> complicate the ranges for all l4_wkup drivers because of that.
Yes, to some extent that's true, as we want to compute the m3 address
space using the regs property. In anycase, the wkupm3 driver has to be
updated for both approaches, the existing patch wouldn't work as is with
the above convention of using 2 address cells. Since the l4_wkup node
would be added for the first time, we are trying to see what would be
the good approach w.r.t subsequent changes in the DTS if and when we
have to move other nodes to be under l4_wkup.
regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] ARM: dts: am33xx: Move wkup_m3 node to soc node and add ranges
Hi Tony,
On 03/11/2015 11:26 AM, Tony Lindgren wrote:
> * Dave Gerlach [150310 12:55]:
>> Tony,
>> On 03/10/2015 11:09 AM, Tony Lindgren wrote:
>>> * Suman Anna [150309 16:59]:
>>>> On 03/05/2015 10:57 AM, Tony Lindgren wrote:
>>>>> * Suman Anna [150305 08:47]:
>>>>>> On 03/05/2015 09:40 AM, Tony Lindgren wrote:
>>>>>>> * Dave Gerlach [150304 20:14]:
>>>>>> Dave,
>>>>>>
>>>>>> Looks like the commit message disappeared during your patch preparation.
>>>>>>
>>>>>>>> Signed-off-by: Suman Anna
>>>>>>>> Signed-off-by: Dave Gerlach
>>>>>>>> ---
>>>>>>>> arch/arm/boot/dts/am33xx.dtsi | 21 +
>>>>>>>> 1 file changed, 13 insertions(+), 8 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/boot/dts/am33xx.dtsi
>>>>>>>> b/arch/arm/boot/dts/am33xx.dtsi
>>>>>>>> index acd3705..086415c 100644
>>>>>>>> --- a/arch/arm/boot/dts/am33xx.dtsi
>>>>>>>> +++ b/arch/arm/boot/dts/am33xx.dtsi
>>>>>>>> @@ -77,10 +77,23 @@
>>>>>>>> */
>>>>>>>>soc {
>>>>>>>>compatible = "ti,omap-infra";
>>>>>>>> + #address-cells = <1>;
>>>>>>>> + #size-cells = <1>;
>>>>>>>> + ranges = <0x0 0x44d0 0x4000>,
>>>>>>>> + <0x8 0x44d8 0x2000>;
>>>>>>>> +
>>>>>>>
>>>>>>> I think putting the ranges here will cause issues for adding
>>>>>>> ranges for anything else.
>>>>>>>
>>>>>>> How about do something like this instead (untested):
>>>>>>>
>>>>>>> ocp {
>>>>>>> l4_wkup: l4_wkup@44c0 {
>>>>>>> compatible = "am335-l4-wkup", "simple-bus";
>>>>>>> ranges = <0 0x44c0 0x3f>;
>>>>>>>
>>>>>>> wkup_m3: wkup_m3@44d0 {
>>>>>>> compatible = "ti,am3353-wkup-m3";
>>>>>>> reg = <0x100 0x4000>, /* M3 UMEM */
>>>>>>> <0x18 0x2000>; /* M3 DMEM */
>>>>>>> ti,hwmods = "wkup_m3";
>>>>>>> ti,pm-firmware = "am335x-pm-firmware.elf";
>>>>>>> };
>>>>>>>
>>>>>>> ...
>>>>>>> };
>>>>>>> };
>>>>>>>
>>>>>>> That way we can start moving also the other l4_wkup components there
>>>>>>> eventuallly without having to redo the ranges again for wkup_m3.
>>>>>>>
>>>>>>> You can also look at how the scm_conf was done for dm816x.dtsi for an
>>>>>>> example, and the recent large set of patches posted by Tero.
>>>>
>>>> I have taken a look at both the above. The L4_WKUP range includes the
>>>> PRCM, Control Module, as well as a few peripherals like DMTimer0, UART0
>>>> etc. What all do we want to move here eventually?
>>>
>>> Well eventually all the children of L4_WKUP, but that can be done
>>> slowly as some of the drivers have weird hacks and may not work
>>> properly if moved around.
>>>
>>> For example, anything with reg entries for something like SCM area will
>>> break as that's not going to be in the L4_WKUP area ny longer :p And
>>> that's actually good as it will protect us from spaghetti code
>>> automatically later on for new code.
>>>
>>>> Depending on that, we may have to use 2 address cells like in Tero's
>>>> PRCM cleanup series rather than the single cell translation used by
>>>> you in dm816x.dtsi so that we can retain the relative addresses
>>>> w.r.t the existing node bases in the derivative child nodes.
>>>
>>> Hmm OK, care to paste a dts snippet example for that?
&
Re: [PATCH v2 2/2] ARM: dts: am33xx: Move wkup_m3 node to soc node and add ranges
Tony,
On 03/05/2015 10:57 AM, Tony Lindgren wrote:
> * Suman Anna [150305 08:47]:
>> On 03/05/2015 09:40 AM, Tony Lindgren wrote:
>>> * Dave Gerlach [150304 20:14]:
>> Dave,
>>
>> Looks like the commit message disappeared during your patch preparation.
>>
>>>> Signed-off-by: Suman Anna
>>>> Signed-off-by: Dave Gerlach
>>>> ---
>>>> arch/arm/boot/dts/am33xx.dtsi | 21 +
>>>> 1 file changed, 13 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
>>>> index acd3705..086415c 100644
>>>> --- a/arch/arm/boot/dts/am33xx.dtsi
>>>> +++ b/arch/arm/boot/dts/am33xx.dtsi
>>>> @@ -77,10 +77,23 @@
>>>> */
>>>>soc {
>>>>compatible = "ti,omap-infra";
>>>> + #address-cells = <1>;
>>>> + #size-cells = <1>;
>>>> + ranges = <0x0 0x44d0 0x4000>,
>>>> + <0x8 0x44d8 0x2000>;
>>>> +
>>>
>>> I think putting the ranges here will cause issues for adding
>>> ranges for anything else.
>>>
>>> How about do something like this instead (untested):
>>>
>>> ocp {
>>> l4_wkup: l4_wkup@44c0 {
>>> compatible = "am335-l4-wkup", "simple-bus";
>>> ranges = <0 0x44c0 0x3f>;
>>>
>>> wkup_m3: wkup_m3@44d0 {
>>> compatible = "ti,am3353-wkup-m3";
>>> reg = <0x100 0x4000>, /* M3 UMEM */
>>> <0x18 0x2000>; /* M3 DMEM */
>>> ti,hwmods = "wkup_m3";
>>> ti,pm-firmware = "am335x-pm-firmware.elf";
>>> };
>>>
>>> ...
>>> };
>>> };
>>>
>>> That way we can start moving also the other l4_wkup components there
>>> eventuallly without having to redo the ranges again for wkup_m3.
>>>
>>> You can also look at how the scm_conf was done for dm816x.dtsi for an
>>> example, and the recent large set of patches posted by Tero.
I have taken a look at both the above. The L4_WKUP range includes the
PRCM, Control Module, as well as a few peripherals like DMTimer0, UART0
etc. What all do we want to move here eventually? Depending on that, we
may have to use 2 address cells like in Tero's PRCM cleanup series
rather than the single cell translation used by you in dm816x.dtsi so
that we can retain the relative addresses w.r.t the existing node bases
in the derivative child nodes.
regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] ARM: dts: am33xx: Move wkup_m3 node to soc node and add ranges
On 03/05/2015 09:40 AM, Tony Lindgren wrote:
> * Dave Gerlach [150304 20:14]:
Dave,
Looks like the commit message disappeared during your patch preparation.
>> Signed-off-by: Suman Anna
>> Signed-off-by: Dave Gerlach
>> ---
>> arch/arm/boot/dts/am33xx.dtsi | 21 +
>> 1 file changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
>> index acd3705..086415c 100644
>> --- a/arch/arm/boot/dts/am33xx.dtsi
>> +++ b/arch/arm/boot/dts/am33xx.dtsi
>> @@ -77,10 +77,23 @@
>> */
>> soc {
>> compatible = "ti,omap-infra";
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +ranges = <0x0 0x44d0 0x4000>,
>> + <0x8 0x44d8 0x2000>;
>> +
>
> I think putting the ranges here will cause issues for adding
> ranges for anything else.
>
> How about do something like this instead (untested):
>
> ocp {
> l4_wkup: l4_wkup@44c0 {
> compatible = "am335-l4-wkup", "simple-bus";
> ranges = <0 0x44c0 0x3f>;
>
> wkup_m3: wkup_m3@44d0 {
> compatible = "ti,am3353-wkup-m3";
> reg = <0x100 0x4000>, /* M3 UMEM */
> <0x18 0x2000>; /* M3 DMEM */
> ti,hwmods = "wkup_m3";
> ti,pm-firmware = "am335x-pm-firmware.elf";
> };
>
> ...
> };
> };
>
> That way we can start moving also the other l4_wkup components there
> eventuallly without having to redo the ranges again for wkup_m3.
>
> You can also look at how the scm_conf was done for dm816x.dtsi for an
> example, and the recent large set of patches posted by Tero.
>
Tony,
Thanks, I will take a look at this. I initially tried adding to ocp node
directly, but obviously it had issues.
But in general, you are ok with the ranges approach right, rather than
having to define another property with virtual addresses. These devices
are not on a separate bus like PCI, so I placed them in the soc node,
and expect only special devices like processors to kinda go under that node.
regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 4/4] hwspinlock/omap: add support for dt nodes
HwSpinlock IP is present only on OMAP4 and other newer SoCs,
which are all device-tree boot only. This patch adds the
base support for parsing the DT nodes, and removes the code
dealing with the traditional platform device instantiation.
Signed-off-by: Suman Anna
[t...@atomide.com: ack for legacy file removal]
Acked-by: Tony Lindgren
---
v8: Updated to not rely on the retrieving base-id from DT node.
Did not use match data for base_id as OMAP will be using 0
as base id value anyways. This looks almost like the v5 version
without the added xlate ops.
MAINTAINERS | 1 -
arch/arm/mach-omap2/Makefile | 3 --
arch/arm/mach-omap2/hwspinlock.c | 60
drivers/hwspinlock/omap_hwspinlock.c | 17 +++---
4 files changed, 13 insertions(+), 68 deletions(-)
delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
diff --git a/MAINTAINERS b/MAINTAINERS
index ddc5a8cf9a8a..f735fb3dfdd7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7035,7 +7035,6 @@ M:Ohad Ben-Cohen
L: linux-o...@vger.kernel.org
S: Maintained
F: drivers/hwspinlock/omap_hwspinlock.c
-F: arch/arm/mach-omap2/hwspinlock.c
OMAP MMC SUPPORT
M: Jarkko Lavinen
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index b83f18fcec9b..209a54007657 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -281,8 +281,5 @@ obj-y += $(nand-m)
$(nand-y)
smsc911x-$(CONFIG_SMSC911X):= gpmc-smsc911x.o
obj-y += $(smsc911x-m) $(smsc911x-y)
-ifneq ($(CONFIG_HWSPINLOCK_OMAP),)
-obj-y += hwspinlock.o
-endif
obj-y += common-board-devices.o twl-common.o
dss-common.o
diff --git a/arch/arm/mach-omap2/hwspinlock.c b/arch/arm/mach-omap2/hwspinlock.c
deleted file mode 100644
index ef175acaeaa2..
--- a/arch/arm/mach-omap2/hwspinlock.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * OMAP hardware spinlock device initialization
- *
- * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
- *
- * Contact: Simon Que
- * Hari Kanigeri
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * General Public License for more details.
- */
-
-#include
-#include
-#include
-#include
-
-#include "soc.h"
-#include "omap_hwmod.h"
-#include "omap_device.h"
-
-static struct hwspinlock_pdata omap_hwspinlock_pdata __initdata = {
- .base_id = 0,
-};
-
-static int __init hwspinlocks_init(void)
-{
- int retval = 0;
- struct omap_hwmod *oh;
- struct platform_device *pdev;
- const char *oh_name = "spinlock";
- const char *dev_name = "omap_hwspinlock";
-
- /*
-* Hwmod lookup will fail in case our platform doesn't support the
-* hardware spinlock module, so it is safe to run this initcall
-* on all omaps
-*/
- oh = omap_hwmod_lookup(oh_name);
- if (oh == NULL)
- return -EINVAL;
-
- pdev = omap_device_build(dev_name, 0, oh, &omap_hwspinlock_pdata,
- sizeof(struct hwspinlock_pdata));
- if (IS_ERR(pdev)) {
- pr_err("Can't build omap_device for %s:%s\n", dev_name,
- oh_name);
- retval = PTR_ERR(pdev);
- }
-
- return retval;
-}
-/* early board code might need to reserve specific hwspinlock instances */
-omap_postcore_initcall(hwspinlocks_init);
diff --git a/drivers/hwspinlock/omap_hwspinlock.c
b/drivers/hwspinlock/omap_hwspinlock.c
index 47a275c6ece1..e49e5eb784a6 100644
--- a/drivers/hwspinlock/omap_hwspinlock.c
+++ b/drivers/hwspinlock/omap_hwspinlock.c
@@ -1,7 +1,7 @@
/*
* OMAP hardware spinlock driver
*
- * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2010-2015 Texas Instruments Incorporated - http://www.ti.com
*
* Contact: Simon Que
* Hari Kanigeri
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include "hwspinlock_internal.h"
@@ -80,14 +81,15 @@ static const struct hwspinlock_ops omap_hwspinlock_ops = {
static int omap_hwspinlock_probe(struct platform_device *pdev)
{
- struct hwspinlock_pdata *pdata = pdev->dev.platform_data;
+ struct device_node *node = pdev->dev.of_node;
struct hwspinlock_device *bank;
struct hwspinlock *hwlock;
struct resource *
[PATCH v8 0/4] hwspinlock core & omap dt support
Hi Ohad, This is the latest version of the hwspinlock dt support series, rebased onto v4.0-rc1 and addressing the long discussion on the bindings in v7 [1]. I really hope that this series can make it into 4.1. Mark, Can you please provide your Acked-by for the binding documents so that Ohad can pick up the patches for the next merge window? Following are the main changes in v8 w.r.t v7: - Revised the generic hwspinlock bindings to remove hwlock-base-id and hwlock-num-locks properties, and added the optional hwlock-names property. - Updated the core device tree patch to remove of_hwspin_lock_get_base_id() and of_hwspin_lock_get_num_locks() functions. Reworked the of_hwspin_lock_get_id() API to not use the list of registered hwspinlock banks, but rely on the hwspinlock radix tree itself to perform deferred probing and pargs lock specifier validation. The last of the two were added in v6, but dropped in v7, and are now restored. - Updated the OMAP hwspinlock binding and DT support patches for the absence of the v7's mandatory hwlock-base-id property. - Changed the order of patches slightly to lump the core changes together and the OMAP hwspinlock changes together. The validation logs on all the applicable OMAP SoCs are at: OMAP4 : http://pastebin.ubuntu.com/10533448/ OMAP5 : http://pastebin.ubuntu.com/10533710/ DRA7 (X15) : http://pastebin.ubuntu.com/10533486/ AM33xx : http://pastebin.ubuntu.com/10533623/ AM43xx : http://pastebin.ubuntu.com/10533538/ The above logs are generated with some additional test patches staged here for reference, https://github.com/sumananna/omap-kernel/commits/hwspinlock/test/4.0-rc1-dt-v8 regards Suman [1] https://patchwork.kernel.org/patch/5635201/ --- v7: http://marc.info/?l=linux-omap&m=142126914027417&w=2 - Dropped the patch "hwspinlock/core: maintain a list of registered hwspinlock banks" - Updated generic hwspinlock bindings to make hwlock-base-id property mandatory. - Updated the OMAP hwspinlock binding and DT support patches to correct for the mandatory hwlock-base-id property. - Updated the common OF helpers patch to move the of_hwspin_lock_get_base_id() and of_hwspin_lock_get_num_locks() functions into the internal header, these are no longer exported, but available for platform implementations. of_hwspin_lock_get_id() is also simplified now. v6: http://marc.info/?l=linux-omap&m=141055365213895&w=2 - 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. - Updated the OMAP hwspinlock DT support patch to assign base-id from DT if present - RFC patches adding the concept of reserved locks and return code change convention dropped. v5: http://marc.info/?l=linux-omap&m=139890478402807&w=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-omap&m=138965904015225&w=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-omap&m=138143992932197&w=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-omap&m=137944644112727&w=2 v1: - Add DT parse support to OMAP hwspinlock driver - Add OMAP4 DT node and bindings information http://marc.info/?l=linux-omap&m=137823082308009&w=2 --- Suman Anna (4): Documentation: dt: add common bindings for hwspinlock hwspinlock/core: add device tree support Documentation: dt: add the omap hwspinlock bindings document hwspinlock/omap: add support for dt nodes .../devicetree/bindings/hwlock/hwlock.
[PATCH v8 3/4] Documentation: dt: add the omap hwspinlock bindings document
HwSpinlock IP is present only on OMAP4 and other newer SoCs,
which are all device-tree boot only. This patch adds the
DT bindings information for OMAP hwspinlock module.
Cc: Rob Herring
Cc: Mark Rutland
Signed-off-by: Suman Anna
---
v8: Removed the previous information about hwlock-base-id, and added
a reference to the base binding document.
.../devicetree/bindings/hwlock/omap-hwspinlock.txt | 26 ++
1 file changed, 26 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
new file mode 100644
index ..2c9804f4f4ac
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
@@ -0,0 +1,26 @@
+OMAP4+ HwSpinlock Driver
+
+
+Required properties:
+- compatible: Should be "ti,omap4-hwspinlock" for
+ OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
+- reg: Contains the hwspinlock module register address space
+ (base address and length)
+- ti,hwmods: Name of the hwmod associated with the hwspinlock device
+- #hwlock-cells: Should be 1. The OMAP hwspinlock users will use a
+ 0-indexed relative hwlock number as the argument
+ specifier value for requesting a specific hwspinlock
+ within a hwspinlock bank.
+
+Please look at the generic hwlock binding for usage information for consumers,
+"Documentation/devicetree/bindings/hwlock/hwlock.txt"
+
+Example:
+
+/* OMAP4 */
+hwspinlock: spinlock@4a0f6000 {
+ compatible = "ti,omap4-hwspinlock";
+ reg = <0x4a0f6000 0x1000>;
+ ti,hwmods = "spinlock";
+ #hwlock-cells = <1>;
+};
--
2.3.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 1/4] Documentation: dt: add common bindings for hwspinlock
This patch adds the generic common bindings used to represent
a hwlock device and use/request locks in a device-tree build.
Each hwspinlock provider should have the '#hwlock-cells' property,
which represents the number of cells to be used for representing
a specific hwspinlock. Client users shall use the property
'hwlocks' for requesting specific lock(s).
Note that the document is named hwlock.txt deliberately to keep
it a bit more generic.
Cc: Rob Herring
Cc: Mark Rutland
Reviewed-by: Bjorn Andersson
Signed-off-by: Suman Anna
---
v8: Revised the binding completely, removed hwlock-num-locks and
hwlock-base-id properties. Added hwlock-names as optional
property. Revised patch description as well.
.../devicetree/bindings/hwlock/hwlock.txt | 59 ++
1 file changed, 59 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt
diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt
b/Documentation/devicetree/bindings/hwlock/hwlock.txt
new file mode 100644
index ..085d1f5c916a
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt
@@ -0,0 +1,59 @@
+Generic hwlock bindings
+===
+
+Generic bindings that are common to all the hwlock platform specific driver
+implementations.
+
+Please also look through the individual platform specific hwlock binding
+documentations for identifying any additional properties specific to that
+platform.
+
+hwlock providers:
+=
+
+Required properties:
+- #hwlock-cells:Specifies the number of cells needed to represent a
+specific lock.
+
+hwlock users:
+=
+
+Consumers that require specific hwlock(s) should specify them using the
+property "hwlocks", and an optional "hwlock-names" property.
+
+Required properties:
+- hwlocks: List of phandle to a hwlock provider node and an
+associated hwlock args specifier as indicated by
+#hwlock-cells. The list can have just a single hwlock
+or multiple hwlocks, with each hwlock represented by
+a phandle and a corresponding args specifier.
+
+Optional properties:
+- hwlock-names: List of hwlock name strings defined in the same order
+as the hwlocks, with one name per hwlock. Consumers can
+use the hwlock-names to match and get a specific
hwlock.
+
+
+1. Example of a node using a single specific hwlock:
+
+The following example has a node requesting a hwlock in the bank defined by
+the node hwlock1. hwlock1 is a hwlock provider with an argument specifier
+of length 1.
+
+ node {
+ ...
+ hwlocks = <&hwlock1 2>;
+ ...
+ };
+
+2. Example of a node using multiple specific hwlocks:
+
+The following example has a node requesting two hwlocks, a hwlock within
+the hwlock device node 'hwlock1' with #hwlock-cells value of 1, and another
+hwlock within the hwlock device node 'hwlock2' with #hwlock-cells value of 2.
+
+ node {
+ ...
+ hwlocks = <&hwlock1 2>, <&hwlock2 0 3>;
+ ...
+ };
--
2.3.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 2/4] hwspinlock/core: add device tree support
This patch adds a new OF-friendly API of_hwspin_lock_get_id()
for hwspinlock clients to use/request locks from a hwspinlock
device instantiated through a device-tree blob. This new API
can be used by hwspinlock clients to get the id for a specific
lock using the phandle + args specifier, so that it can be
requested using the available hwspin_lock_request_specific()
API.
Signed-off-by: Suman Anna
---
v8: Removed the previous OF helpers of_hwspin_lock_get_num_locks()
& of_hwspin_lock_get_base_id() due to the revised generic
bindings. Added back the support for deferred probing and
pargs specifier validation like in v6, but without using an
added list for storing registered hwspinlock devices.
Documentation/hwspinlock.txt | 10 +
drivers/hwspinlock/hwspinlock_core.c | 79
include/linux/hwspinlock.h | 7
3 files changed, 96 insertions(+)
diff --git a/Documentation/hwspinlock.txt b/Documentation/hwspinlock.txt
index 62f7d4ea6e26..61c1ee98e59f 100644
--- a/Documentation/hwspinlock.txt
+++ b/Documentation/hwspinlock.txt
@@ -48,6 +48,16 @@ independent, drivers.
ids for predefined purposes.
Should be called from a process context (might sleep).
+ int of_hwspin_lock_get_id(struct device_node *np, int index);
+ - retrieve the global lock id for an OF phandle-based specific lock.
+ This function provides a means for DT users of a hwspinlock module
+ to get the global lock id of a specific hwspinlock, so that it can
+ be requested using the normal hwspin_lock_request_specific() API.
+ The function returns a lock id number on success, -EPROBE_DEFER if
+ the hwspinlock device is not yet registered with the core, or other
+ error values.
+ Should be called from a process context (might sleep).
+
int hwspin_lock_free(struct hwspinlock *hwlock);
- free a previously-assigned hwspinlock; returns 0 on success, or an
appropriate error code on failure (e.g. -EINVAL if the hwspinlock
diff --git a/drivers/hwspinlock/hwspinlock_core.c
b/drivers/hwspinlock/hwspinlock_core.c
index 461a0d739d75..f8539f624e3d 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "hwspinlock_internal.h"
@@ -257,6 +258,84 @@ void __hwspin_unlock(struct hwspinlock *hwlock, int mode,
unsigned long *flags)
}
EXPORT_SYMBOL_GPL(__hwspin_unlock);
+/**
+ * of_hwspin_lock_simple_xlate - translate hwlock_spec to return a lock id
+ * @bank: the hwspinlock device bank
+ * @hwlock_spec: hwlock specifier as found in the device tree
+ *
+ * This is a simple translation function, suitable for hwspinlock platform
+ * drivers that only has a lock specifier length of 1.
+ *
+ * Returns a relative index of the lock within a specified bank on success,
+ * or -EINVAL on invalid specifier cell count.
+ */
+static inline int
+of_hwspin_lock_simple_xlate(const struct of_phandle_args *hwlock_spec)
+{
+ if (WARN_ON(hwlock_spec->args_count != 1))
+ return -EINVAL;
+
+ return hwlock_spec->args[0];
+}
+
+/**
+ * of_hwspin_lock_get_id() - get lock id for an OF phandle-based specific lock
+ * @np: device node from which to request the specific hwlock
+ * @index: index of the hwlock in the list of values
+ *
+ * This function provides a means for DT users of the hwspinlock module to
+ * get the global lock id of a specific hwspinlock using the phandle of the
+ * hwspinlock device, so that it can be requested using the normal
+ * hwspin_lock_request_specific() API.
+ *
+ * Returns the global lock id number on success, -EPROBE_DEFER if the
hwspinlock
+ * device is not yet registered, -EINVAL on invalid args specifier value or an
+ * appropriate error as returned from the OF parsing of the DT client node.
+ */
+int of_hwspin_lock_get_id(struct device_node *np, int index)
+{
+ struct of_phandle_args args;
+ struct hwspinlock *hwlock;
+ struct radix_tree_iter iter;
+ void **slot;
+ int id;
+ int ret;
+
+ ret = of_parse_phandle_with_args(np, "hwlocks", "#hwlock-cells", index,
+&args);
+ if (ret)
+ return ret;
+
+ /* perform a sanity check of the hwspinlock device */
+ ret = -EPROBE_DEFER;
+ rcu_read_lock();
+ radix_tree_for_each_slot(slot, &hwspinlock_tree, &iter, 0) {
+ hwlock = radix_tree_deref_slot(slot);
+ if (unlikely(!hwlock))
+ continue;
+
+ if (hwlock->bank->dev->of_node == args.np) {
+ ret = 0;
+ break;
+ }
+ }
+ rcu_read_unlock();
+ if (ret < 0)
+ goto out;
+
+ id = of_hwspin_lock_simple_xlate(&args);
+ if (id < 0 || id >= hwlock-&
Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock
Hi Bjorn, On 02/05/2015 05:01 PM, Bjorn Andersson wrote: > On Mon, Feb 2, 2015 at 1:07 PM, Suman Anna wrote: >> On 02/01/2015 11:55 AM, Bjorn Andersson wrote: >>> On Fri, Jan 30, 2015 at 9:41 PM, Ohad Ben-Cohen wrote: >>>> On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson wrote: >>>>> In a system where you have two hwlock blocks lckA and lckB, each >>>>> consisting of 8 locks and you have dspB that can only access lckB >>>> >>>> This is a good example - thanks. To be able to cope with such cases we >>>> will have to pass a hwlock block reference and its relative lock id. >>>> >>> >>> Correct, so the #hwlock-cells and hwlock part from the proposal are >>> the important one. Having an optional hwlock-names will make things >>> easier to read as well, but is not necessary. >> >> Right, if anything, it would be useful only for the clients, but the >> hwspinlock core itself would not need it. So, I would forgo adding the >> hwlock-names for now. >> >>> >>>> The DT binding should definitely be prepared for such cases (just kill >>>> the base-id field?), but let's see what it means about the Linux >>>> implementation. >>>> >>> >>> From the dt binding PoV, we should be able to skip num-locks as well. >>> It seems most hwlock blocks have a fixed amount of locks provided and >>> the drivers are reporting this to the core when registering. >> >> I added this originally based on the initial MSM HW Mutex block bindings. >> > > It's not entirely correct to have this in DT for the MSM HW, as the > hardware has a fixed number of mutexes. As soon as we have the binding > sorted out I will follow up with a new revision of the tcsr/sfpb-mutex > driver. > >>> >>> So I think we can reduce the binding to: >>> >>> Providers: >>> #hwlock-cells >>> >>> Consumers: >>> hwlocks >>> hwlock-names >>> >>> For the hardware where number of locks is actually variable (e.g. >>> different variants of same block) there can be driver specific entries >>> for this. >> >> Right, we should be able to drop this and use the driver match data. As >> it is, the field is used during registration of the block with the >> hwspinlock core. >> > > If we have certain systems where it actually is a property to be > configured then they can have individual properties, extending the > standard set. Either way, it's not a dynamic property shared by all > hwlock drivers, so it should not be in the common binding. > > Will you send out a new revision of the binding? I would love to get > this integrated so I can move on with the dependents. Yep, will do as soon as I hear from Ohad on what to do with the patch "hwspinlock/core: maintain a list of registered hwspinlock banks" that I dropped from v7. Without that and dropping hwlock-base-id, I can't get the translations done. regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock
On 02/01/2015 05:00 AM, Ohad Ben-Cohen wrote: > On Sat, Jan 31, 2015 at 7:41 AM, Ohad Ben-Cohen wrote: >> On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson wrote: >>> In a system where you have two hwlock blocks lckA and lckB, each >>> consisting of 8 locks and you have dspB that can only access lckB >> >> This is a good example - thanks. To be able to cope with such cases we >> will have to pass a hwlock block reference and its relative lock id. So, you mean lckB is only between the host and dspB. Obviously, if it were shared between dspA and dspB only, then the allocation and management would be completely outside the host Linux driver's scope. > > Additionally, to support such a scenario, we can no longer retain the > simple dynamic allocation API we have today, because it might end up > allocating dspB an hwlock from IckA. > > We will have to make sure hwlocks are allocated only from pools > visible to the user, something that will change not only the > hwspinlock API but also the way it maintains the hwlocks. Right, the current API definitely will not scale for that. It was designed around the concept that it's easier to exchange a single global id, rather than a lcbB:id or some other similar semantics that needs to be interpreted. > > I suspect we want to wait for such hardware to show up first, and only > then add framework support for it. Agreed. regards Suman > Regardless, we obviously do want to > make sure our DT binding is prepared for the worse, so we'll drop the > "base-id" field. > > Thanks, > Ohad. > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock
On 02/01/2015 11:55 AM, Bjorn Andersson wrote: > On Fri, Jan 30, 2015 at 9:41 PM, Ohad Ben-Cohen wrote: >> On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson wrote: >>> In a system where you have two hwlock blocks lckA and lckB, each >>> consisting of 8 locks and you have dspB that can only access lckB >> >> This is a good example - thanks. To be able to cope with such cases we >> will have to pass a hwlock block reference and its relative lock id. >> > > Correct, so the #hwlock-cells and hwlock part from the proposal are > the important one. Having an optional hwlock-names will make things > easier to read as well, but is not necessary. Right, if anything, it would be useful only for the clients, but the hwspinlock core itself would not need it. So, I would forgo adding the hwlock-names for now. > >> The DT binding should definitely be prepared for such cases (just kill >> the base-id field?), but let's see what it means about the Linux >> implementation. >> > > From the dt binding PoV, we should be able to skip num-locks as well. > It seems most hwlock blocks have a fixed amount of locks provided and > the drivers are reporting this to the core when registering. I added this originally based on the initial MSM HW Mutex block bindings. > > So I think we can reduce the binding to: > > Providers: > #hwlock-cells > > Consumers: > hwlocks > hwlock-names > > For the hardware where number of locks is actually variable (e.g. > different variants of same block) there can be driver specific entries > for this. Right, we should be able to drop this and use the driver match data. As it is, the field is used during registration of the block with the hwspinlock core. regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock
On 01/22/2015 12:56 PM, Mark Rutland wrote:
> On Wed, Jan 21, 2015 at 05:56:37PM +0000, Suman Anna wrote:
>> On 01/21/2015 06:41 AM, Ohad Ben-Cohen wrote:
>>> On Tue, Jan 20, 2015 at 8:05 PM, Tony Lindgren wrote:
>>>> How about default to Linux id space and allow overriding that with
>>>> a module param option if needed?
>>>
>>> I'm not sure I'm following.
>>>
>>> If the main point of contention is the base_id field, I'm also fine
>>> with removing it entirely, as I'm not aware of any actual user for it
>>> (Suman please confirm?).
>>
>> Yeah, well the current implementations that I am aware of only have a
>> single bank, so all of them would be using a value of 0. I am yet to see
>> a platform with multiple instances where the property really makes a
>> difference. v7 has the property mandatory, so all the implementations
>> would need to define this value even if it is 0.
>>
>> regards
>> Suman
>>
>>>
>>> Mark? Rob? Will you accept Suman's patches if the base_id field is removed?
>
> My concern is that the mapping of hwspinlock IDs doesn't seem to be
> explicit in the DT on a per-context basis, which is what I'd expect.
>
> e.g.
>
> lck: hwspinlock-device@f00 {
> ...
> #hwlock-cells = <1>;
> };
>
> some-other-os-interface {
> ...
> hwlocks = <&lck 0>, <&lck 1>, <&lck 2>, <&lck 3>;
> hwlock-names = "glbl", "pool0", "pool1", "pool2";
> };
>
> a-different-os-interface {
> ...
> hwlocks = <&lck 18>, <&lck 21>, <&lck 4>, <&lck 5>;
> hwlock-names = "init", "teardown", "pool0", "pool1";
> };
>
> That's the only way I would expect this to possibly remain a stable
> over time, and it's the entire reason for #hwlock-cells, no?
>
> How do you expect the other components sharing the hwspinlocks to be
> described?
Yes indeed, this is what any of the clients will use on Linux. But this is not
necessarily the semantics for exchanging hwlocks with the other processor(s)
which is where the global id space comes into picture.
regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] of/device: manage resources similar to platform_device_add
Hi Grant,
On 01/13/2015 05:04 PM, Suman Anna wrote:
> On 01/13/2015 04:00 PM, Rob Herring wrote:
>> On Tue, Jan 13, 2015 at 3:25 PM, Suman Anna wrote:
>>> Hi Rob,
>>>
>>> On 01/13/2015 02:38 PM, Rob Herring wrote:
>>>> On Wed, Jan 7, 2015 at 11:30 AM, Suman Anna wrote:
>>>>> Drivers can use of_platform_populate() to create platform devices
>>>>> for children of the device main node, and a complementary API
>>>>> of_platform_depopulate() is provided to delete these child platform
>>>>> devices. The of_platform_depopulate() leverages the platform API
>>>>> for performing the cleanup of these devices.
>>>>>
>>>>> The platform device resources are managed differently between
>>>>> of_device_add and platform_device_add, and this asymmetry causes
>>>>> a kernel oops in platform_device_del during removal of the resources.
>>>>> Manage the platform device resources similar to platform_device_add
>>>>> to fix this kernel oops.
>>>>
>>>> This is a known issue and has been attempted to be fixed before (I
>>>> believe there is a revert in mainline). The problem is there are known
>>>> devicetrees which have overlapping resources and they will break with
>>>> your change.
>>>
>>> Are you referring to 02bbde7849e6 (Revert "of: use
>>> platform_device_add")?
>>
>> I believe that's the one.
>>
>>> That one seems to be in registration path, and
>>> this crash is in the unregistration path. If so, to fix the crash,
>>> should we be skipping the release_resource() for now in
>>> platform_device_del for DT nodes, or replace platform_device_unregister
>>> with of_device_unregister in of_platform_device_destroy()?
>>
>> IIRC, the problem is inserting a resource twice on add from 2
>> different nodes, not the removal path. Perhaps we could make a
>> collision non-fatal for in the DT case.
>
> We may be talking two different things here, I understand that this
> patch would create an issue with inserting a resource twice in the
> devicetrees with overlapping resources (just like the commit that was
> reverted above), but the crash is on devices with resources whose
> parent, child, sibling pointers have never been initialized (the
> of_device_add path does not touch these at all), and get dereferenced in
> platform_device_del()->release_resource(). See the following that has a
> better explanation [1].
>
> regards
> Suman
>
> [1]
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/274412.html
>
>
>> Grant may have some ideas on
>> what's needed here.
Ping, any suggestions here? Do we ought to replace
platform_device_unregister() with of_device_unregister() similar to the
approach taken in 02bbde7849e6 (Revert "of: use platform_device_add")?
regards
Suman
>>
>>> This is a common crash and we cannot use of_platform_depopulate() today
>>> in drivers to complement of_platform_populate().
>>
>> Yes, I know.
>>
>>> Also, the platform_data crash is independent of this, I could reproduce
>>> that one even with using of_device_unregister in a loop in driver remove.
>>
>> Missed this one. I'll reply to that patch.
>>
>> Rob
>>
>>>
>>> regards
>>> Suman
>>>
>>>>
>>>> Rob
>>>>
>>>>>
>>>>> Signed-off-by: Suman Anna
>>>>> ---
>>>>> drivers/of/device.c | 38 +-
>>>>> 1 file changed, 37 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/of/device.c b/drivers/of/device.c
>>>>> index 46d6c75c1404..fa27c1c71f29 100644
>>>>> --- a/drivers/of/device.c
>>>>> +++ b/drivers/of/device.c
>>>>> @@ -50,6 +50,8 @@ EXPORT_SYMBOL(of_dev_put);
>>>>>
>>>>> int of_device_add(struct platform_device *ofdev)
>>>>> {
>>>>> + int i, ret;
>>>>> +
>>>>> BUG_ON(ofdev->dev.of_node == NULL);
>>>>>
>>>>> /* name and id have to be set so that the platform bus doesn't get
>>>>> @@ -63,7 +65,41 @@ int of_device_add(struct platform_device *ofdev)
>>>>> if (!ofdev->dev.parent)
>>>>> set_dev_node(&ofdev->dev,
>>>>> of_node_to_nid(ofdev->dev.of_node));
>>
Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock
On 01/21/2015 06:41 AM, Ohad Ben-Cohen wrote: > On Tue, Jan 20, 2015 at 8:05 PM, Tony Lindgren wrote: >> How about default to Linux id space and allow overriding that with >> a module param option if needed? > > I'm not sure I'm following. > > If the main point of contention is the base_id field, I'm also fine > with removing it entirely, as I'm not aware of any actual user for it > (Suman please confirm?). Yeah, well the current implementations that I am aware of only have a single bank, so all of them would be using a value of 0. I am yet to see a platform with multiple instances where the property really makes a difference. v7 has the property mandatory, so all the implementations would need to define this value even if it is 0. regards Suman > > Mark? Rob? Will you accept Suman's patches if the base_id field is removed? > > Thanks, > Ohad. > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock
On 01/16/2015 04:19 AM, Mark Rutland wrote: > On Thu, Jan 15, 2015 at 02:42:23PM +, Rob Herring wrote: >> On Thu, Jan 15, 2015 at 7:55 AM, Mark Rutland wrote: >>> On Thu, Jan 15, 2015 at 01:52:01PM +, Mark Rutland wrote: >>>> On Wed, Jan 14, 2015 at 08:58:18PM +, Suman Anna wrote: >>>>> This patch adds the generic common bindings used to represent >>>>> a hwlock device and use/request locks in a device-tree build. >>>>> >>>>> All the platform-specific hwlock driver implementations need the >>>>> number of locks and associated base id for registering the locks >>>>> present within the device with the driver core. This base id >>>>> needs to be unique across multiple IP instances of a hwspinlock >>>>> device, so that each hwlock can be represented uniquely in a >>>>> system. >>>>> >>>>> The number of locks is represented by 'hwlock-num-locks' property, >>>>> and the base id is represented by the 'hwlock-base-id' property. >>>>> The args specifier length is dependent on each vendor-specific >>>>> implementation and is represented through the '#hwlock-cells' >>>>> property. Client users need to use the property 'hwlocks' for >>>>> requesting specific lock(s). >>>>> >>>>> Note that the document is named hwlock.txt deliberately to keep >>>>> it a bit more generic. >>>>> >>>>> Cc: Rob Herring >>>>> Signed-off-by: Suman Anna >>>>> --- >>>>> v7: Revised binding info for hwlock-base-id, it is mandatory now >>>>> >>>>> .../devicetree/bindings/hwlock/hwlock.txt | 55 >>>>> ++ >>>>> 1 file changed, 55 insertions(+) >>>>> create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt >>>>> b/Documentation/devicetree/bindings/hwlock/hwlock.txt >>>>> new file mode 100644 >>>>> index ..8de7aaf878f9 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt >>>>> @@ -0,0 +1,55 @@ >>>>> +Generic hwlock bindings >>>>> +=== >>>>> + >>>>> +Generic bindings that are common to all the hwlock platform specific >>>>> driver >>>>> +implementations. >>>>> + >>>>> +The validity and need of these common properties may vary from one >>>>> platform >>>>> +implementation to another. The platform specific bindings should >>>>> explicitly >>>>> +state if an optional property is used. Please also look through the >>>>> individual >>>>> +platform specific hwlock binding documentations for identifying the >>>>> applicable >>>>> +properties. >>>>> + >>>>> +Common properties: >>>>> +- #hwlock-cells: Specifies the number of cells needed to represent a >>>>> + specific lock. This property is mandatory for all >>>>> + platform implementations. >>>>> +- hwlock-num-locks:Number of locks present in a hwlock device. >>>>> This >>>>> + property is needed on hwlock devices, where the number >>>>> + of supported locks within a hwlock device cannot be >>>>> + read from a register. >>>>> +- hwlock-base-id: An unique base Id for the locks for a particular >>>>> hwlock >>>>> + device. This property is mandatory for all platform >>>>> + implementations. >>>> >>>> This property makes no sense. The ID encoded in the hwlock cells is >>>> relative to the instance (identified by phandle), not global. So the DT >>>> has no global ID space. >>>> >>>> Why do you think you need this? >>> >>> Having looked at the way this proeprty is used, NAK. >>> >>> If you need to carve up a Linux-internal ID space, do that dynamically. >>> There is no need for this property. >> >> Better yet, don't create a Linux ID space for this. Everywhere we have >> one, we want to get rid of it. > > Agreed. A completely opaque token / desc structure would prevent a lot > of potential abuse and save us from painful breakage. The regular API to acquire or release a lock on Linux does indeed use opaque handles, but the id space is needed for communication with other processors as the handles have no meaning on non-Linux processors. The id space is the simplest to exchange which lock to use with other processors compared to the device instance plus the lock number within that device. regards Suman -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock
On 01/15/2015 08:42 AM, Rob Herring wrote: > On Thu, Jan 15, 2015 at 7:55 AM, Mark Rutland wrote: >> On Thu, Jan 15, 2015 at 01:52:01PM +, Mark Rutland wrote: >>> On Wed, Jan 14, 2015 at 08:58:18PM +, Suman Anna wrote: >>>> This patch adds the generic common bindings used to represent >>>> a hwlock device and use/request locks in a device-tree build. >>>> >>>> All the platform-specific hwlock driver implementations need the >>>> number of locks and associated base id for registering the locks >>>> present within the device with the driver core. This base id >>>> needs to be unique across multiple IP instances of a hwspinlock >>>> device, so that each hwlock can be represented uniquely in a >>>> system. >>>> >>>> The number of locks is represented by 'hwlock-num-locks' property, >>>> and the base id is represented by the 'hwlock-base-id' property. >>>> The args specifier length is dependent on each vendor-specific >>>> implementation and is represented through the '#hwlock-cells' >>>> property. Client users need to use the property 'hwlocks' for >>>> requesting specific lock(s). >>>> >>>> Note that the document is named hwlock.txt deliberately to keep >>>> it a bit more generic. >>>> >>>> Cc: Rob Herring >>>> Signed-off-by: Suman Anna >>>> --- >>>> v7: Revised binding info for hwlock-base-id, it is mandatory now >>>> >>>> .../devicetree/bindings/hwlock/hwlock.txt | 55 >>>> ++ >>>> 1 file changed, 55 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt >>>> b/Documentation/devicetree/bindings/hwlock/hwlock.txt >>>> new file mode 100644 >>>> index ..8de7aaf878f9 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt >>>> @@ -0,0 +1,55 @@ >>>> +Generic hwlock bindings >>>> +=== >>>> + >>>> +Generic bindings that are common to all the hwlock platform specific >>>> driver >>>> +implementations. >>>> + >>>> +The validity and need of these common properties may vary from one >>>> platform >>>> +implementation to another. The platform specific bindings should >>>> explicitly >>>> +state if an optional property is used. Please also look through the >>>> individual >>>> +platform specific hwlock binding documentations for identifying the >>>> applicable >>>> +properties. >>>> + >>>> +Common properties: >>>> +- #hwlock-cells: Specifies the number of cells needed to represent a >>>> + specific lock. This property is mandatory for all >>>> + platform implementations. >>>> +- hwlock-num-locks:Number of locks present in a hwlock device. >>>> This >>>> + property is needed on hwlock devices, where the number >>>> + of supported locks within a hwlock device cannot be >>>> + read from a register. >>>> +- hwlock-base-id: An unique base Id for the locks for a particular hwlock >>>> + device. This property is mandatory for all platform >>>> + implementations. >>> >>> This property makes no sense. The ID encoded in the hwlock cells is >>> relative to the instance (identified by phandle), not global. So the DT >>> has no global ID space. >>> >>> Why do you think you need this? >> >> Having looked at the way this proeprty is used, NAK. >> >> If you need to carve up a Linux-internal ID space, do that dynamically. >> There is no need for this property. > > Better yet, don't create a Linux ID space for this. Everywhere we have > one, we want to get rid of it. Hi Mark, Rob, Thank you for your comments. The id space is not limited to Linux, as this is indeed a global id space for all the processors in an SoC, as that is the primary usage of the individual locks in these IP blocks. It needs to be static across the SoC irrespective of whether a node was enabled or not. Now, it is debatable whether we do this in DT or do this in each individual implementation. This is something that the hwspinlock core cannot do, and probably would have to be done as static driver match data. I have gone from having this as an optional property, to not having it, and now back to having it as mandatory - because of the different perspectives from bindings vs driver subsystem maintainers. I brought this back mainly based on the reasonings in [1] Ohad, Do you have any comments or suggestions here? It looks like I have no choice but to bring back "hwspinlock/core: maintain a list of registered hwspinlock banks", if we were to have the bindings without a hwlock-base-id property. regards Suman [1] http://marc.info/?l=linux-omap&m=139510004009415&w=2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: dts: OMAP: Add hwlock-base-id property to hwspinlock nodes
Add the generic property "hwlock-base-id" to the hwspinlock DT nodes on all applicable OMAP SoCS (OMAP4, OMAP5, DRA7, AM33xx and AM43xx). This common property is required by the hwspinlock core to be able to translate client locks properly. All the current OMAP SoCs only have a single instance of the IP, and so will use a common value of 0. Cc: Ohad Ben-Cohen Signed-off-by: Suman Anna --- This is required for the OMAP Hwspinlock driver with the latest DT series, http://marc.info/?l=linux-omap&m=142126914027417&w=2 arch/arm/boot/dts/am33xx.dtsi | 1 + arch/arm/boot/dts/am4372.dtsi | 1 + arch/arm/boot/dts/dra7.dtsi | 1 + arch/arm/boot/dts/omap4.dtsi | 1 + arch/arm/boot/dts/omap5.dtsi | 1 + 5 files changed, 5 insertions(+) diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index acd37057bca9..9ee1f44f0f4b 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -334,6 +334,7 @@ compatible = "ti,omap4-hwspinlock"; reg = <0x480ca000 0x1000>; ti,hwmods = "spinlock"; + hwlock-base-id = <0>; #hwlock-cells = <1>; }; diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi index b62a1cd776cd..98eebedb1430 100644 --- a/arch/arm/boot/dts/am4372.dtsi +++ b/arch/arm/boot/dts/am4372.dtsi @@ -371,6 +371,7 @@ compatible = "ti,omap4-hwspinlock"; reg = <0x480ca000 0x1000>; ti,hwmods = "spinlock"; + hwlock-base-id = <0>; #hwlock-cells = <1>; }; diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi index 22771bc1643a..cf7ebdc95b4a 100644 --- a/arch/arm/boot/dts/dra7.dtsi +++ b/arch/arm/boot/dts/dra7.dtsi @@ -752,6 +752,7 @@ compatible = "ti,omap4-hwspinlock"; reg = <0x4a0f6000 0x1000>; ti,hwmods = "spinlock"; + hwlock-base-id = <0>; #hwlock-cells = <1>; }; diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi index 074147cebae4..77197e5454e8 100644 --- a/arch/arm/boot/dts/omap4.dtsi +++ b/arch/arm/boot/dts/omap4.dtsi @@ -344,6 +344,7 @@ compatible = "ti,omap4-hwspinlock"; reg = <0x4a0f6000 0x1000>; ti,hwmods = "spinlock"; + hwlock-base-id = <0>; #hwlock-cells = <1>; }; diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi index b321fdf42c9f..c2fc81724308 100644 --- a/arch/arm/boot/dts/omap5.dtsi +++ b/arch/arm/boot/dts/omap5.dtsi @@ -393,6 +393,7 @@ compatible = "ti,omap4-hwspinlock"; reg = <0x4a0f6000 0x1000>; ti,hwmods = "spinlock"; + hwlock-base-id = <0>; #hwlock-cells = <1>; }; -- 2.2.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 0/4] hwspinlock core & omap dt support
Hi Ohad, This is an updated version of the hwspinlock dt support series, rebased onto v3.19-rc3 and mainly addresses the continued discussion on the need to maintain a list of registered spinlock banks [1]. I have removed this patch as per your wish, and as a result the burden of the spinlock validation and deferred probing is pushed onto the hwspinlock clients. Sorry for the delay in refreshing this series, hopefully this can be the last revision. Following are the main changes in v7 w.r.t v6: - Dropped the patch "hwspinlock/core: maintain a list of registered hwspinlock banks" - Updated generic hwspinlock bindings to make hwlock-base-id property mandatory. - Updated the OMAP hwspinlock binding and DT support patches to correct for the mandatory hwlock-base-id property. - Updated the common OF helpers patch to move the of_hwspin_lock_get_base_id() and of_hwspin_lock_get_num_locks() functions into the internal header, these are no longer exported, but available for platform implementations. of_hwspin_lock_get_id() is also simplified now. The validation logs on all the applicable OMAP SoCs are at: OMAP4 - http://slexy.org/view/s21Mh1SqP8 OMAP5 - http://slexy.org/view/s21TYWxoKu DRA74x - http://slexy.org/view/s2dQdghLPr DRA72x - http://slexy.org/view/s2QajhhWYu AM33xx - http://slexy.org/view/s21DvKQOpa AM43xx - http://slexy.org/view/s21L3YB95Q The above logs are generated with some additional test patches staged here for reference, https://github.com/sumananna/omap-kernel/commits/hwspinlock/test/3.19-rc3-dt-v7 The test branch also includes a required patch that adds the hwlock-base-id to all the OMAP hwspinlock DT nodes (will submit that patch separately for Tony to pick it up). Bjorn, I didn't add your Tested-by: or Reviewed-by as I have modified the series a bit. Care to give those once again, thanks. regards Suman [1] https://patchwork.kernel.org/patch/4898041/ --- v6: http://marc.info/?l=linux-omap&m=141055365213895&w=2 - 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. - Updated the OMAP hwspinlock DT support patch to assign base-id from DT if present - RFC patches adding the concept of reserved locks and return code change convention dropped. v5: http://marc.info/?l=linux-omap&m=139890478402807&w=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-omap&m=138965904015225&w=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-omap&m=138143992932197&w=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-omap&m=137944644112727&w=2 v1: - Add DT parse support to OMAP hwspinlock driver - Add OMAP4 DT node and bindings information http://marc.info/?l=linux-omap&m=137823082308009&w=2 --- Suman Anna (4): Documentation: dt: add common bindings for hwspinlock Documentation: dt: add the omap hwspinlock bindings document 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 | 28 ++ Documentation/hwspinlock.txt | 25 + MAINTAINERS| 1 - arch/arm/mach-omap2/Makefile | 3 - arch/arm/mach-omap2/hwspinlock.c | 60 drivers/hwspinlock/hwspinlock_core.c | 65
[PATCH v7 4/4] hwspinlock/omap: add support for dt nodes
HwSpinlock IP is present only on OMAP4 and other newer SoCs,
which are all device-tree boot only. This patch adds the
base support for parsing the DT nodes, and removes the code
dealing with the traditional platform device instantiation.
Signed-off-by: Suman Anna
[t...@atomide.com: ack for legacy file removal]
Acked-by: Tony Lindgren
---
v7: hwlock-base-id is no longer optional, probe will fail now
if property is absent.
MAINTAINERS | 1 -
arch/arm/mach-omap2/Makefile | 3 --
arch/arm/mach-omap2/hwspinlock.c | 60
drivers/hwspinlock/omap_hwspinlock.c | 22 ++---
4 files changed, 17 insertions(+), 69 deletions(-)
delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
diff --git a/MAINTAINERS b/MAINTAINERS
index ddb9ac8d32b3..3de03037e90a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6861,7 +6861,6 @@ M:Ohad Ben-Cohen
L: linux-o...@vger.kernel.org
S: Maintained
F: drivers/hwspinlock/omap_hwspinlock.c
-F: arch/arm/mach-omap2/hwspinlock.c
OMAP MMC SUPPORT
M: Jarkko Lavinen
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 5d27dfdef66b..6fa36846d5b4 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -282,9 +282,6 @@ obj-y += $(nand-m)
$(nand-y)
smsc911x-$(CONFIG_SMSC911X):= gpmc-smsc911x.o
obj-y += $(smsc911x-m) $(smsc911x-y)
-ifneq ($(CONFIG_HWSPINLOCK_OMAP),)
-obj-y += hwspinlock.o
-endif
emac-$(CONFIG_TI_DAVINCI_EMAC) := am35xx-emac.o
obj-y += $(emac-m) $(emac-y)
diff --git a/arch/arm/mach-omap2/hwspinlock.c b/arch/arm/mach-omap2/hwspinlock.c
deleted file mode 100644
index ef175acaeaa2..
--- a/arch/arm/mach-omap2/hwspinlock.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * OMAP hardware spinlock device initialization
- *
- * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
- *
- * Contact: Simon Que
- * Hari Kanigeri
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * General Public License for more details.
- */
-
-#include
-#include
-#include
-#include
-
-#include "soc.h"
-#include "omap_hwmod.h"
-#include "omap_device.h"
-
-static struct hwspinlock_pdata omap_hwspinlock_pdata __initdata = {
- .base_id = 0,
-};
-
-static int __init hwspinlocks_init(void)
-{
- int retval = 0;
- struct omap_hwmod *oh;
- struct platform_device *pdev;
- const char *oh_name = "spinlock";
- const char *dev_name = "omap_hwspinlock";
-
- /*
-* Hwmod lookup will fail in case our platform doesn't support the
-* hardware spinlock module, so it is safe to run this initcall
-* on all omaps
-*/
- oh = omap_hwmod_lookup(oh_name);
- if (oh == NULL)
- return -EINVAL;
-
- pdev = omap_device_build(dev_name, 0, oh, &omap_hwspinlock_pdata,
- sizeof(struct hwspinlock_pdata));
- if (IS_ERR(pdev)) {
- pr_err("Can't build omap_device for %s:%s\n", dev_name,
- oh_name);
- retval = PTR_ERR(pdev);
- }
-
- return retval;
-}
-/* early board code might need to reserve specific hwspinlock instances */
-omap_postcore_initcall(hwspinlocks_init);
diff --git a/drivers/hwspinlock/omap_hwspinlock.c
b/drivers/hwspinlock/omap_hwspinlock.c
index 47a275c6ece1..18edb3b4ab32 100644
--- a/drivers/hwspinlock/omap_hwspinlock.c
+++ b/drivers/hwspinlock/omap_hwspinlock.c
@@ -1,7 +1,7 @@
/*
* OMAP hardware spinlock driver
*
- * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2010-2015 Texas Instruments Incorporated - http://www.ti.com
*
* Contact: Simon Que
* Hari Kanigeri
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include "hwspinlock_internal.h"
@@ -80,16 +81,20 @@ static const struct hwspinlock_ops omap_hwspinlock_ops = {
static int omap_hwspinlock_probe(struct platform_device *pdev)
{
- struct hwspinlock_pdata *pdata = pdev->dev.platform_data;
+ struct device_node *node = pdev->dev.of_node;
struct hwspinlock_device *bank;
struct hwspinlock *hwlock;
struct resource *res;
void __iomem *io_base;
- int num_locks, i, ret;
+ int num_locks, i, ret, base_id;
[PATCH v7 3/4] hwspinlock/core: add common OF helpers
This patch adds two new OF helper functions for platform
implementations and one new API to use/request locks from a
hwspinlock device instantiated through a device-tree blob.
1. The of_hwspin_lock_get_num_locks() is a common OF helper
function to read the 'hwlock-num-locks' property.
2. The of_hwspin_lock_get_base_id() is a common OF helper
function to read the 'hwlock-base-id' property.
3. The of_hwspin_lock_get_id() API can be used by hwspinlock
clients to get the id for a specific lock using the phandle
+ args specifier, so that it can be requested using the
available hwspin_lock_request_specific() API.
Signed-off-by: Suman Anna
---
v7:
- Moved of_hwspin_lock_get_base_id() and of_hwspin_lock_get_num_locks
into hwspinlock_internal.h
- Simplified of_hwspin_lock_get_id(), removed deferred probing and
args specifier validation
- updated comments and documentation
Documentation/hwspinlock.txt | 25
drivers/hwspinlock/hwspinlock_core.c | 65
drivers/hwspinlock/hwspinlock_internal.h | 47 +++
include/linux/hwspinlock.h | 7
4 files changed, 144 insertions(+)
diff --git a/Documentation/hwspinlock.txt b/Documentation/hwspinlock.txt
index 62f7d4ea6e26..a29bb47e4637 100644
--- a/Documentation/hwspinlock.txt
+++ b/Documentation/hwspinlock.txt
@@ -48,6 +48,16 @@ independent, drivers.
ids for predefined purposes.
Should be called from a process context (might sleep).
+ int of_hwspin_lock_get_id(struct device_node *np, int index);
+ - retrieve the global lock id for an OF phandle-based specific lock.
+ This function provides a means for DT users of a hwspinlock module
+ to get the global lock id of a specific hwspinlock, so that it can
+ be requested using the normal hwspin_lock_request_specific() API.
+ The function returns a lock id number on success, or other error
+ values. The function does not perform any validation of the args
+ specifier lock values, this burden is placed on the user.
+ Should be called from a process context (might sleep).
+
int hwspin_lock_free(struct hwspinlock *hwlock);
- free a previously-assigned hwspinlock; returns 0 on success, or an
appropriate error code on failure (e.g. -EINVAL if the hwspinlock
@@ -243,6 +253,21 @@ int hwspinlock_example2(void)
Returns the address of hwspinlock on success, or NULL on error (e.g.
if the hwspinlock is still in use).
+ int of_hwspin_lock_get_num_locks(struct device_node *dn);
+ - is a common OF helper function that can be used by some underlying
+ vendor-specific implementations. This can be used by implementations
+ that require and define the number of locks supported within a hwspinlock
+ bank as a device tree node property. This function should be called by
+ needed implementations before registering a hwspinlock device with the
+ hwspinlock core.
+
+ int of_hwspin_lock_get_base_id(struct device_node *dn);
+ - is a common OF helper function that can be used by the underlying
+ vendor-specific implementations. This function should be called by
+ implementations to retrieve the base index for a block of locks present
+ within a hwspinlock device for registering that device with the
+ hwspinlock core.
+
5. Important structs
struct hwspinlock_device is a device which usually contains a bank
diff --git a/drivers/hwspinlock/hwspinlock_core.c
b/drivers/hwspinlock/hwspinlock_core.c
index 461a0d739d75..8f107bc34281 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "hwspinlock_internal.h"
@@ -257,6 +258,70 @@ void __hwspin_unlock(struct hwspinlock *hwlock, int mode,
unsigned long *flags)
}
EXPORT_SYMBOL_GPL(__hwspin_unlock);
+/**
+ * of_hwspin_lock_simple_xlate - translate hwlock_spec to return a lock id
+ * @bank: the hwspinlock device bank
+ * @hwlock_spec: hwlock specifier as found in the device tree
+ *
+ * This is a simple translation function, suitable for hwspinlock platform
+ * drivers that only has a lock specifier length of 1.
+ *
+ * Returns a relative index of the lock within a specified bank on success,
+ * or -EINVAL on invalid specifier cell count.
+ */
+static inline int
+of_hwspin_lock_simple_xlate(const struct of_phandle_args *hwlock_spec)
+{
+ if (WARN_ON(hwlock_spec->args_count != 1))
+ return -EINVAL;
+
+ return hwlock_spec->args[0];
+}
+
+/**
+ * of_hwspin_lock_get_id() - get lock id for an OF phandle-based specific lock
+ * @np: device node from which to request the specific hwlock
+ * @index: index of the hwlock in the list of values
+ *
+ * This function provides a means for DT users of the hwspinlock module to
+ * get the global lock id of a specific hwspinlock using the phandle of the
+ * hwspinlock devic
[PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock
This patch adds the generic common bindings used to represent
a hwlock device and use/request locks in a device-tree build.
All the platform-specific hwlock driver implementations need the
number of locks and associated base id for registering the locks
present within the device with the driver core. This base id
needs to be unique across multiple IP instances of a hwspinlock
device, so that each hwlock can be represented uniquely in a
system.
The number of locks is represented by 'hwlock-num-locks' property,
and the base id is represented by the 'hwlock-base-id' property.
The args specifier length is dependent on each vendor-specific
implementation and is represented through the '#hwlock-cells'
property. Client users need to use the property 'hwlocks' for
requesting specific lock(s).
Note that the document is named hwlock.txt deliberately to keep
it a bit more generic.
Cc: Rob Herring
Signed-off-by: Suman Anna
---
v7: Revised binding info for hwlock-base-id, it is mandatory now
.../devicetree/bindings/hwlock/hwlock.txt | 55 ++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt
diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt
b/Documentation/devicetree/bindings/hwlock/hwlock.txt
new file mode 100644
index ..8de7aaf878f9
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt
@@ -0,0 +1,55 @@
+Generic hwlock bindings
+===
+
+Generic bindings that are common to all the hwlock platform specific driver
+implementations.
+
+The validity and need of these common properties may vary from one platform
+implementation to another. The platform specific bindings should explicitly
+state if an optional property is used. Please also look through the individual
+platform specific hwlock binding documentations for identifying the applicable
+properties.
+
+Common properties:
+- #hwlock-cells: Specifies the number of cells needed to represent a
+ specific lock. This property is mandatory for all
+ platform implementations.
+- hwlock-num-locks:Number of locks present in a hwlock device. This
+ property is needed on hwlock devices, where the number
+ of supported locks within a hwlock device cannot be
+ read from a register.
+- hwlock-base-id: An unique base Id for the locks for a particular hwlock
+ device. This property is mandatory for all platform
+ implementations.
+
+Hwlock Users:
+=
+
+Nodes that require specific hwlock(s) should specify them using the property
+"hwlocks", each containing a phandle to the hwlock node and an args specifier
+value as indicated by #hwlock-cells. Multiple hwlocks can be requested using
+an array of the phandle and hwlock number specifier tuple.
+
+1. Example of a node using a single specific hwlock:
+
+The following example has a node requesting a hwlock in the bank defined by
+the node hwlock1. hwlock1 is a hwlock provider with an argument specifier
+of length 1.
+
+ node {
+ ...
+ hwlocks = <&hwlock1 2>;
+ ...
+ };
+
+2. Example of a node using multiple specific hwlocks:
+
+The following example has a node requesting two hwlocks, a hwlock within
+the hwlock device node 'hwlock1' with #hwlock-cells value of 1, and another
+hwlock within the hwlock device node 'hwlock2' with #hwlock-cells value of 2.
+
+ node {
+ ...
+ hwlocks = <&hwlock1 2>, <&hwlock2 0 3>;
+ ...
+ };
--
2.2.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 2/4] Documentation: dt: add the omap hwspinlock bindings document
HwSpinlock IP is present only on OMAP4 and other newer SoCs,
which are all device-tree boot only. This patch adds the
DT bindings information for OMAP hwspinlock module.
Cc: Rob Herring
Signed-off-by: Suman Anna
---
v7: Added information about hwlock-base-id and updated example to use it
.../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++
1 file changed, 28 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
new file mode 100644
index ..935173ec6b58
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
@@ -0,0 +1,28 @@
+OMAP4+ HwSpinlock Driver
+
+
+Required properties:
+- compatible: Should be "ti,omap4-hwspinlock" for
+ OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
+- reg: Contains the hwspinlock module register address space
+ (base address and length)
+- ti,hwmods: Name of the hwmod associated with the hwspinlock device
+- hwlock-base-id: Should be 0 for the first IP block instance, or a proper
+ positive value for any subsequent instance (if present)
+ of the IP block.
+- #hwlock-cells: Should be 1. The OMAP hwspinlock users will use a
+ 0-indexed relative hwlock number as the argument
+ specifier value for requesting a specific hwspinlock
+ within a hwspinlock bank.
+
+
+Example:
+
+/* OMAP4 */
+hwspinlock: spinlock@4a0f6000 {
+ compatible = "ti,omap4-hwspinlock";
+ reg = <0x4a0f6000 0x1000>;
+ ti,hwmods = "spinlock";
+ hwlock-base-id = <0>;
+ #hwlock-cells = <1>;
+};
--
2.2.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] of/device: manage resources similar to platform_device_add
On 01/13/2015 04:00 PM, Rob Herring wrote:
> On Tue, Jan 13, 2015 at 3:25 PM, Suman Anna wrote:
>> Hi Rob,
>>
>> On 01/13/2015 02:38 PM, Rob Herring wrote:
>>> On Wed, Jan 7, 2015 at 11:30 AM, Suman Anna wrote:
>>>> Drivers can use of_platform_populate() to create platform devices
>>>> for children of the device main node, and a complementary API
>>>> of_platform_depopulate() is provided to delete these child platform
>>>> devices. The of_platform_depopulate() leverages the platform API
>>>> for performing the cleanup of these devices.
>>>>
>>>> The platform device resources are managed differently between
>>>> of_device_add and platform_device_add, and this asymmetry causes
>>>> a kernel oops in platform_device_del during removal of the resources.
>>>> Manage the platform device resources similar to platform_device_add
>>>> to fix this kernel oops.
>>>
>>> This is a known issue and has been attempted to be fixed before (I
>>> believe there is a revert in mainline). The problem is there are known
>>> devicetrees which have overlapping resources and they will break with
>>> your change.
>>
>> Are you referring to 02bbde7849e6 (Revert "of: use
>> platform_device_add")?
>
> I believe that's the one.
>
>> That one seems to be in registration path, and
>> this crash is in the unregistration path. If so, to fix the crash,
>> should we be skipping the release_resource() for now in
>> platform_device_del for DT nodes, or replace platform_device_unregister
>> with of_device_unregister in of_platform_device_destroy()?
>
> IIRC, the problem is inserting a resource twice on add from 2
> different nodes, not the removal path. Perhaps we could make a
> collision non-fatal for in the DT case.
We may be talking two different things here, I understand that this
patch would create an issue with inserting a resource twice in the
devicetrees with overlapping resources (just like the commit that was
reverted above), but the crash is on devices with resources whose
parent, child, sibling pointers have never been initialized (the
of_device_add path does not touch these at all), and get dereferenced in
platform_device_del()->release_resource(). See the following that has a
better explanation [1].
regards
Suman
[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/274412.html
> Grant may have some ideas on
> what's needed here.
>
>> This is a common crash and we cannot use of_platform_depopulate() today
>> in drivers to complement of_platform_populate().
>
> Yes, I know.
>
>> Also, the platform_data crash is independent of this, I could reproduce
>> that one even with using of_device_unregister in a loop in driver remove.
>
> Missed this one. I'll reply to that patch.
>
> Rob
>
>>
>> regards
>> Suman
>>
>>>
>>> Rob
>>>
>>>>
>>>> Signed-off-by: Suman Anna
>>>> ---
>>>> drivers/of/device.c | 38 +-
>>>> 1 file changed, 37 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/of/device.c b/drivers/of/device.c
>>>> index 46d6c75c1404..fa27c1c71f29 100644
>>>> --- a/drivers/of/device.c
>>>> +++ b/drivers/of/device.c
>>>> @@ -50,6 +50,8 @@ EXPORT_SYMBOL(of_dev_put);
>>>>
>>>> int of_device_add(struct platform_device *ofdev)
>>>> {
>>>> + int i, ret;
>>>> +
>>>> BUG_ON(ofdev->dev.of_node == NULL);
>>>>
>>>> /* name and id have to be set so that the platform bus doesn't get
>>>> @@ -63,7 +65,41 @@ int of_device_add(struct platform_device *ofdev)
>>>> if (!ofdev->dev.parent)
>>>> set_dev_node(&ofdev->dev,
>>>> of_node_to_nid(ofdev->dev.of_node));
>>>>
>>>> - return device_add(&ofdev->dev);
>>>> + for (i = 0; i < ofdev->num_resources; i++) {
>>>> + struct resource *p, *r = &ofdev->resource[i];
>>>> +
>>>> + if (!r->name)
>>>> + r->name = dev_name(&ofdev->dev);
>>>> +
>>>> + p = r->parent;
>>>> + if (!p) {
>>>> + if (resource_type(r) == IORESOURCE_MEM)
>>>> + p = &iomem_re
Re: [RFC PATCH 2/3] core: platform: fix an invalid kfree during of_platform_depopulate
Hi Rob, On 01/13/2015 04:27 PM, Rob Herring wrote: > On Wed, Jan 7, 2015 at 11:30 AM, Suman Anna wrote: >> Drivers can use of_platform_populate() to create platform devices >> for children of the device main node, and a complementary API >> of_platform_depopulate() is provided to delete these child devices. >> Any platform_data supplied for the OF devices through auxdata lookup >> data is populated directly in the device's platform_data field, unlike >> those created using platform API. The of_platform_depopulate() >> leverages the platform code for cleanup, and this will result in a >> kernel oops due to an invalid kfree on this direct populated >> platform_data. >> >> Fix this by resetting the platform data for OF devices during >> platform device cleanup. > > We should probably copy the platform_data like is done for non-OF > platform devices. I'm sure there was some reason for it. Yeah, that was my first thought too, but went with adding a checking here as I am not aware of the original reason for not copying it, and it seemed like unnecessary copying of static data without any real gain. > It looks strange doing this in release. > > However, I'm inclined to not fix this and force users to move off of > auxdata. That's intended to be a temporary migration path and there > are only 54 instances of it that have platform_data. What device do > you care about? I use this mainly for the remoteproc devices (mainly differentiating multiple instances of the same compatible type on the same SoC), but fair enough, I can rework my driver to use some lookup based match data instead. So far, none of the drivers who use of_platform_populate() did supply platform data, so this particular crash is not seen/common. platform_data does get used in the OMAP pdata-quirks, though of_platform_depopulate() won't be called on those, as this is called in init_machine. regards Suman > > Rob > >> >> Signed-off-by: Suman Anna >> --- >> drivers/base/platform.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/base/platform.c b/drivers/base/platform.c >> index 9421fed40905..129e69c8c894 100644 >> --- a/drivers/base/platform.c >> +++ b/drivers/base/platform.c >> @@ -200,6 +200,8 @@ static void platform_device_release(struct device *dev) >> struct platform_object *pa = container_of(dev, struct >> platform_object, >> pdev.dev); >> >> + if (pa->pdev.dev.of_node) >> + pa->pdev.dev.platform_data = NULL; >> of_device_node_put(&pa->pdev.dev); >> kfree(pa->pdev.dev.platform_data); >> kfree(pa->pdev.mfd_cell); >> -- >> 2.2.1 >> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] of/device: manage resources similar to platform_device_add
Hi Rob,
On 01/13/2015 02:38 PM, Rob Herring wrote:
> On Wed, Jan 7, 2015 at 11:30 AM, Suman Anna wrote:
>> Drivers can use of_platform_populate() to create platform devices
>> for children of the device main node, and a complementary API
>> of_platform_depopulate() is provided to delete these child platform
>> devices. The of_platform_depopulate() leverages the platform API
>> for performing the cleanup of these devices.
>>
>> The platform device resources are managed differently between
>> of_device_add and platform_device_add, and this asymmetry causes
>> a kernel oops in platform_device_del during removal of the resources.
>> Manage the platform device resources similar to platform_device_add
>> to fix this kernel oops.
>
> This is a known issue and has been attempted to be fixed before (I
> believe there is a revert in mainline). The problem is there are known
> devicetrees which have overlapping resources and they will break with
> your change.
Are you referring to 02bbde7849e6 (Revert "of: use
platform_device_add")? That one seems to be in registration path, and
this crash is in the unregistration path. If so, to fix the crash,
should we be skipping the release_resource() for now in
platform_device_del for DT nodes, or replace platform_device_unregister
with of_device_unregister in of_platform_device_destroy()?
This is a common crash and we cannot use of_platform_depopulate() today
in drivers to complement of_platform_populate().
Also, the platform_data crash is independent of this, I could reproduce
that one even with using of_device_unregister in a loop in driver remove.
regards
Suman
>
> Rob
>
>>
>> Signed-off-by: Suman Anna
>> ---
>> drivers/of/device.c | 38 +-
>> 1 file changed, 37 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/of/device.c b/drivers/of/device.c
>> index 46d6c75c1404..fa27c1c71f29 100644
>> --- a/drivers/of/device.c
>> +++ b/drivers/of/device.c
>> @@ -50,6 +50,8 @@ EXPORT_SYMBOL(of_dev_put);
>>
>> int of_device_add(struct platform_device *ofdev)
>> {
>> + int i, ret;
>> +
>> BUG_ON(ofdev->dev.of_node == NULL);
>>
>> /* name and id have to be set so that the platform bus doesn't get
>> @@ -63,7 +65,41 @@ int of_device_add(struct platform_device *ofdev)
>> if (!ofdev->dev.parent)
>> set_dev_node(&ofdev->dev,
>> of_node_to_nid(ofdev->dev.of_node));
>>
>> - return device_add(&ofdev->dev);
>> + for (i = 0; i < ofdev->num_resources; i++) {
>> + struct resource *p, *r = &ofdev->resource[i];
>> +
>> + if (!r->name)
>> + r->name = dev_name(&ofdev->dev);
>> +
>> + p = r->parent;
>> + if (!p) {
>> + if (resource_type(r) == IORESOURCE_MEM)
>> + p = &iomem_resource;
>> + else if (resource_type(r) == IORESOURCE_IO)
>> + p = &ioport_resource;
>> + }
>> +
>> + if (p && insert_resource(p, r)) {
>> + dev_err(&ofdev->dev, "failed to claim resource %d\n",
>> + i);
>> + ret = -EBUSY;
>> + goto failed;
>> + }
>> + }
>> +
>> + ret = device_add(&ofdev->dev);
>> + if (ret == 0)
>> + return ret;
>> +
>> +failed:
>> + while (--i >= 0) {
>> + struct resource *r = &ofdev->resource[i];
>> + unsigned long type = resource_type(r);
>> +
>> + if (type == IORESOURCE_MEM || type == IORESOURCE_IO)
>> + release_resource(r);
>> + }
>> + return ret;
>> }
>>
>> int of_device_register(struct platform_device *pdev)
>> --
>> 2.2.1
>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 1/3] of/device: manage resources similar to platform_device_add
Drivers can use of_platform_populate() to create platform devices
for children of the device main node, and a complementary API
of_platform_depopulate() is provided to delete these child platform
devices. The of_platform_depopulate() leverages the platform API
for performing the cleanup of these devices.
The platform device resources are managed differently between
of_device_add and platform_device_add, and this asymmetry causes
a kernel oops in platform_device_del during removal of the resources.
Manage the platform device resources similar to platform_device_add
to fix this kernel oops.
Signed-off-by: Suman Anna
---
drivers/of/device.c | 38 +-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 46d6c75c1404..fa27c1c71f29 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -50,6 +50,8 @@ EXPORT_SYMBOL(of_dev_put);
int of_device_add(struct platform_device *ofdev)
{
+ int i, ret;
+
BUG_ON(ofdev->dev.of_node == NULL);
/* name and id have to be set so that the platform bus doesn't get
@@ -63,7 +65,41 @@ int of_device_add(struct platform_device *ofdev)
if (!ofdev->dev.parent)
set_dev_node(&ofdev->dev, of_node_to_nid(ofdev->dev.of_node));
- return device_add(&ofdev->dev);
+ for (i = 0; i < ofdev->num_resources; i++) {
+ struct resource *p, *r = &ofdev->resource[i];
+
+ if (!r->name)
+ r->name = dev_name(&ofdev->dev);
+
+ p = r->parent;
+ if (!p) {
+ if (resource_type(r) == IORESOURCE_MEM)
+ p = &iomem_resource;
+ else if (resource_type(r) == IORESOURCE_IO)
+ p = &ioport_resource;
+ }
+
+ if (p && insert_resource(p, r)) {
+ dev_err(&ofdev->dev, "failed to claim resource %d\n",
+ i);
+ ret = -EBUSY;
+ goto failed;
+ }
+ }
+
+ ret = device_add(&ofdev->dev);
+ if (ret == 0)
+ return ret;
+
+failed:
+ while (--i >= 0) {
+ struct resource *r = &ofdev->resource[i];
+ unsigned long type = resource_type(r);
+
+ if (type == IORESOURCE_MEM || type == IORESOURCE_IO)
+ release_resource(r);
+ }
+ return ret;
}
int of_device_register(struct platform_device *pdev)
--
2.2.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 0/3] of_platform_depopulate crash fixes
sk.ti: de56a000 [ 156.331360] PC is at kfree+0x50/0x15c [ 156.335181] LR is at kfree+0x34/0x15c [ 156.339003] pc : []lr : []psr: 2093 [ 156.339003] sp : de56be80 ip : fp : [ 156.350985] r10: a013 r9 : de56a000 r8 : c039b5dc [ 156.356437] r7 : c097977c r6 : c03a0b38 r5 : bf021470 r4 : de521c10 [ 156.363249] r3 : 023dc4a4 r2 : dfa71000 r1 : e1e4d4a4 r0 : c0139240 [ 156.370063] Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user [ 156.377601] Control: 10c5387d Table: 9e4f0019 DAC: 0015 [ 156.383597] Process rmmod (pid: 77, stack limit = 0xde56a240) [ 156.389594] Stack: (0xde56be80 to 0xde56c000) [ 156.394145] be80: de521c10 de521c10 de4ae440 de521c18 c039b5dc c03a0b38 [ 156.402683] bea0: de521c18 c039b748 de521c34 c094a4f0 de485fc0 c03128ac de1100dc [ 156.411221] bec0: de56bef0 de4ae470 0001 c05c98c8 0010 c04d6858 [ 156.419759] bee0: 0081 c000e904 c039b7ec de1100c0 de12cc10 de12cc00 [ 156.428298] bf00: c0915564 c04d6840 de12cc10 bf01f154 de12cc10 bf02135c de12cc44 c03a0714 [ 156.436836] bf20: c03a06fc de12cc10 bf02135c c039ec8c bf02135c de12cc10 bf02135c c039f494 [ 156.445374] bf40: bf02135c bec3dbe8 0880 c039eb14 bf021480 c00b4198 c0164bc0 73757270 [ 156.453912] bf60: 65725f73 65746f6d 636f7270 0001 de4fb180 c000e7d0 0001 [ 156.462450] bf80: 10c5387d c0083074 000195dc 73757270 65725f73 e85c 000195dc 73757270 [ 156.470988] bfa0: 65725f73 c000e740 000195dc 73757270 bec3dbe8 0880 bec3dbe8 0880 [ 156.479526] bfc0: 000195dc 73757270 65725f73 0081 000aa7f8 d1b0 [ 156.488065] bfe0: bec3dbe0 bec3dbd0 00019368 b6ef8bc0 6010 bec3dbe8 2e676572 495f3043 [ 156.496622] [] (kfree) from [] (platform_device_release+0x18/0x3c) [ 156.504903] [] (platform_device_release) from [] (device_release+0x2c/0x90) [ 156.513996] [] (device_release) from [] (kobject_release+0x48/0x7c) [ 156.522361] [] (kobject_release) from [] (klist_next+0xb0/0x12c) [ 156.530452] [] (klist_next) from [] (device_for_each_child+0x40/0x74) [ 156.539003] [] (device_for_each_child) from [] (of_platform_depopulate+0x2c/0x44) [ 156.548644] [] (of_platform_depopulate) from [] (pruss_remove+0x28/0x58 [pruss_remoteproc]) [ 156.559185] [] (pruss_remove [pruss_remoteproc]) from [] (platform_drv_remove+0x18/0x30) [ 156.569449] [] (platform_drv_remove) from [] (__device_release_driver+0x70/0xc4) [ 156.578986] [] (__device_release_driver) from [] (driver_detach+0xb4/0xb8) [ 156.587980] [] (driver_detach) from [] (bus_remove_driver+0x4c/0x90) [ 156.596441] [] (bus_remove_driver) from [] (SyS_delete_module+0x118/0x1e0) [ 156.605445] [] (SyS_delete_module) from [] (ret_fast_syscall+0x0/0x48) [ 156.614075] Code: e0833183 e5922000 e1a03103 e0821003 (e7920003) [ 156.620441] ---[ end trace 8d15970ad8371606 ]--- Also, while trying to reproduce the same with the OF unittest, I noticed that the of_platform_populate tests are not really being executed completely, the last patch enables all the of_selftest_platform_populate test code to execute, this does expose some additional WARN_ONs while running the test. I was able to reproduce the pdata kfree crash with some changes, but wasn't able to convert the current reg properties into IOMEM resources. Following are the complete logs taken from running my tests on BeagleBone Black with 3.19-rc3 + my driver, release_resource crash : http://slexy.org/view/s29B8Wntji platform data kfree crash: http://slexy.org/view/s2mUgd09gm OF UnitTest with just Patch3 : http://slexy.org/view/s21xz88p6P regards Suman Suman Anna (3): of/device: manage resources similar to platform_device_add core: platform: fix an invalid kfree during of_platform_depopulate of/unittest: fix trailing semi-colons on conditional selftest drivers/base/platform.c | 2 ++ drivers/of/device.c | 38 +- drivers/of/unittest.c | 4 ++-- 3 files changed, 41 insertions(+), 3 deletions(-) -- 2.2.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 3/3] of/unittest: fix trailing semi-colons on conditional selftest
The of_platform_populate() and of_platform_depopulate() tests
are not really being tested because of some additional trailing
semi-colons after the conditional checks on couple of selftest
macro usage. Remove them to properly run all the platform
tests.
Fixes: 851da976dc1d (of/unittest: Remove test devices after adding them)
Signed-off-by: Suman Anna
---
drivers/of/unittest.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 844838e11ef1..c67e50264e82 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -765,11 +765,11 @@ static void __init of_selftest_platform_populate(void)
selftest(irq < 0 && irq != -EPROBE_DEFER, "device parsing error failed
- %d\n", irq);
if (selftest(np = of_find_node_by_path("/testcase-data/platform-tests"),
-"No testcase data in device tree\n"));
+"No testcase data in device tree\n"))
return;
if (selftest(!(rc = device_register(&test_bus)),
-"testbus registration failed; rc=%i\n", rc));
+"testbus registration failed; rc=%i\n", rc))
return;
for_each_child_of_node(np, child) {
--
2.2.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 2/3] core: platform: fix an invalid kfree during of_platform_depopulate
Drivers can use of_platform_populate() to create platform devices for children of the device main node, and a complementary API of_platform_depopulate() is provided to delete these child devices. Any platform_data supplied for the OF devices through auxdata lookup data is populated directly in the device's platform_data field, unlike those created using platform API. The of_platform_depopulate() leverages the platform code for cleanup, and this will result in a kernel oops due to an invalid kfree on this direct populated platform_data. Fix this by resetting the platform data for OF devices during platform device cleanup. Signed-off-by: Suman Anna --- drivers/base/platform.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 9421fed40905..129e69c8c894 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -200,6 +200,8 @@ static void platform_device_release(struct device *dev) struct platform_object *pa = container_of(dev, struct platform_object, pdev.dev); + if (pa->pdev.dev.of_node) + pa->pdev.dev.platform_data = NULL; of_device_node_put(&pa->pdev.dev); kfree(pa->pdev.dev.platform_data); kfree(pa->pdev.mfd_cell); -- 2.2.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH (resend)] mailbox: Add Altera mailbox driver
Hi Ley Foon,
On 12/16/2014 12:33 AM, Ley Foon Tan wrote:
> On Tue, Dec 16, 2014 at 4:54 AM, Suman Anna wrote:
>> Hi Ley Foon,
>>
>> On 12/12/2014 08:38 AM, Dinh Nguyen wrote:
>>>
>>>
>>> On 12/12/14, 4:04 AM, Ley Foon Tan wrote:
>>>> The Altera mailbox allows for interprocessor communication. It supports
>>>> only one channel and work as either sender or receiver.
>>
>> I have a few more comments in addition to those that Dinh provided.
>>
>>>>
>>>> Signed-off-by: Ley Foon Tan
>>>> ---
>>>> .../devicetree/bindings/mailbox/altera-mailbox.txt | 49 +++
>>>> drivers/mailbox/Kconfig| 6 +
>>>> drivers/mailbox/Makefile | 2 +
>>>> drivers/mailbox/mailbox-altera.c | 404
>>>> +
>>>> 4 files changed, 461 insertions(+)
>>>> create mode 100644
>>>> Documentation/devicetree/bindings/mailbox/altera-mailbox.txt
>>>> create mode 100644 drivers/mailbox/mailbox-altera.c
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mailbox/altera-mailbox.txt
>>>> b/Documentation/devicetree/bindings/mailbox/altera-mailbox.txt
>>>> new file mode 100644
>>>> index 000..c261979
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mailbox/altera-mailbox.txt
>>>> @@ -0,0 +1,49 @@
>>>> +Altera Mailbox Driver
>>>> +=
>>>> +
>>>> +Required properties:
>>>> +- compatible : "altr,mailbox-1.0".
>>
>> I am not sure on this convention, sounds like IP version number. Please
>> check this with a DT maintainer.
> Our other soft IPs compatible strings all have 1.0 version number. Eg:
> altr,juart-1.0.
> So, we want to keep it as same format.
>
>>>> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
>>>> index c04fed9..84325f2 100644
>>>> --- a/drivers/mailbox/Kconfig
>>>> +++ b/drivers/mailbox/Kconfig
>>>> @@ -45,4 +45,10 @@ config PCC
>>>>states). Select this driver if your platform implements the
>>>>PCC clients mentioned above.
>>>>
>>>> +config ALTERA_MBOX
>>>> +tristate "Altera Mailbox"
>>
>> You haven't used any depends on here, this driver is not applicable on
>> all platforms, right?
> It is a soft IP, so it is not limited to our platform only.
> Other soft IP drivers also don't have 'depend on', so I think should be fine.
Yeah, ok.
>
>
>>>> +
>>>> +static inline struct altera_mbox *to_altera_mbox(struct mbox_chan *chan)
>>>> +{
>>>> +if (!chan)
>>>> +return NULL;
>>>> +
>>>> +return container_of(chan, struct altera_mbox, chan);
>>>> +}
>>>> +
>>>> +static inline int altera_mbox_full(struct altera_mbox *mbox)
>>>> +{
>>>> +u32 status;
>>>> +
>>>> +status = __raw_readl(mbox->mbox_base + MAILBOX_STS_REG);
>>
>> You may want to replace all the __raw_readl/__raw_writel with regular
>> readl/writel or its equivalent relaxed versions depending on your needs.
> Okay.
>
>
>>>> +static int altera_mbox_send_data(struct mbox_chan *chan, void *data)
>>>> +{
>>>> +struct altera_mbox *mbox = to_altera_mbox(chan);
>>>> +u32 *udata = (u32 *)data;
>>>> +
>>>> +if (!mbox || !data)
>>>> +return -EINVAL;
>>>> +if (!mbox->is_sender) {
>>>> +dev_warn(mbox->dev,
>>>> +"failed to send. This is receiver mailbox.\n");
>>>> +return -EINVAL;
>>>> +}
>>>> +
>>>> +if (altera_mbox_full(mbox))
>>>> +return -EBUSY;
>>
>> I think this logic in general will conflict between interrupt and poll
>> mode. send_data is supposed to return -EBUSY only in polling mode and
>> not in interrupt mode.
> What is the recommended error code for this case? BTW, omap-mailbox.c
> also return -EBUSY if mailbox is full.
Looks like the mbox core doesn't care about the error, it's just
checking for non-success case. I made that comment based on the
documentation in last_tx_done. OMAP mailbox is always interrupt driven,
and it is very rare that we will ever h
Re: [PATCH (resend)] mailbox: Add Altera mailbox driver
Hi Ley Foon,
On 12/12/2014 08:38 AM, Dinh Nguyen wrote:
>
>
> On 12/12/14, 4:04 AM, Ley Foon Tan wrote:
>> The Altera mailbox allows for interprocessor communication. It supports
>> only one channel and work as either sender or receiver.
I have a few more comments in addition to those that Dinh provided.
>>
>> Signed-off-by: Ley Foon Tan
>> ---
>> .../devicetree/bindings/mailbox/altera-mailbox.txt | 49 +++
>> drivers/mailbox/Kconfig| 6 +
>> drivers/mailbox/Makefile | 2 +
>> drivers/mailbox/mailbox-altera.c | 404
>> +
>> 4 files changed, 461 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/mailbox/altera-mailbox.txt
>> create mode 100644 drivers/mailbox/mailbox-altera.c
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/altera-mailbox.txt
>> b/Documentation/devicetree/bindings/mailbox/altera-mailbox.txt
>> new file mode 100644
>> index 000..c261979
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/altera-mailbox.txt
>> @@ -0,0 +1,49 @@
>> +Altera Mailbox Driver
>> +=
>> +
>> +Required properties:
>> +- compatible : "altr,mailbox-1.0".
I am not sure on this convention, sounds like IP version number. Please
check this with a DT maintainer.
>> +- reg : physical base address of the mailbox and length of
>> +memory mapped region.
>> +- #mbox-cells: Common mailbox binding property to identify the number
>> +of cells required for the mailbox specifier. Should be 1.
>> +
>> +Optional properties:
>> +- interrupt-parent :interrupt source phandle.
>> +- interrupts : interrupt number. The interrupt specifier format
>> +depends on the interrupt controller parent.
>> +
>> +Example:
>> +mbox_tx: mailbox@0x100 {
>> +compatible = "altr,mailbox-1.0";
>> +reg = <0x100 0x8>;
>> +interrupt-parent = < &gic_0 >;
>> +interrupts = <5>;
>> +#mbox-cells = <1>;
>> +};
>> +
>> +mbox_rx: mailbox@0x200 {
>> +compatible = "altr,mailbox-1.0";
>> +reg = <0x200 0x8>;
>> +interrupt-parent = < &gic_0 >;
>> +interrupts = <6>;
>> +#mbox-cells = <1>;
>> +};
>> +
>> +Mailbox client
>> +===
>> +"mboxes" and the optional "mbox-names" (please see
>> +Documentation/devicetree/bindings/mailbox/mailbox.txt for details). Each
>> value
>> +of the mboxes property should contain a phandle to the mailbox controller
>> +device node and second argument is the channel index. It must be 0 (hardware
>> +support only one channel).The equivalent "mbox-names" property value can be
>> +used to give a name to the communication channel to be used by the client
>> user.
>> +
>> +Example:
>> +mclient0: mclient0@0x400 {
>> +compatible = "client-1.0";
>> +reg = <0x400 0x10>;
>> +mbox-names = "mbox-tx", "mbox-rx";
>> +mboxes = <&mbox_tx 0>,
>> + <&mbox_rx 0>;
>> +};
>> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
>> index c04fed9..84325f2 100644
>> --- a/drivers/mailbox/Kconfig
>> +++ b/drivers/mailbox/Kconfig
>> @@ -45,4 +45,10 @@ config PCC
>>states). Select this driver if your platform implements the
>>PCC clients mentioned above.
>>
>> +config ALTERA_MBOX
>> +tristate "Altera Mailbox"
You haven't used any depends on here, this driver is not applicable on
all platforms, right?
>> +help
>> + An implementation of the Altera Mailbox soft core. It is used
>> + to send message between processors. Say Y here if you want to use the
>> + Altera mailbox support.
>> endif
>> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
>> index dd412c2..2e79231 100644
>> --- a/drivers/mailbox/Makefile
>> +++ b/drivers/mailbox/Makefile
>> @@ -7,3 +7,5 @@ obj-$(CONFIG_PL320_MBOX) += pl320-ipc.o
>> obj-$(CONFIG_OMAP2PLUS_MBOX)+= omap-mailbox.o
>>
>> obj-$(CONFIG_PCC) += pcc.o
>> +
>> +obj-$(CONFIG_ALTERA_MBOX) += mailbox-altera.o
>> diff --git a/drivers/mailbox/mailbox-altera.c
>> b/drivers/mailbox/mailbox-altera.c
>> new file mode 100644
>> index 000..9ed10de
>> --- /dev/null
>> +++ b/drivers/mailbox/mailbox-altera.c
>> @@ -0,0 +1,404 @@
>> +/*
>> + * Copyright Altera Corporation (C) 2013-2014. All rights reserved
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
>> + * more details.
>> + *
>
