| Message ID | 20180530141133.3711-6-faiz_abbas@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Faiz Abbas <faiz_abbas@ti.com> [180530 14:12]: > The ti-sysc driver provides support for manipulating the idlemodes > and interconnect level resets. ... > --- a/arch/arm/boot/dts/dra76x.dtsi > +++ b/arch/arm/boot/dts/dra76x.dtsi > @@ -11,6 +11,25 @@ > / { > compatible = "ti,dra762", "ti,dra7"; > > + ocp { > + > + target-module@0x42c00000 { > + compatible = "ti,sysc-dra7-mcan"; > + ranges = <0x0 0x42c00000 0x2000>; > + #address-cells = <1>; > + #size-cells = <1>; > + reg = <0x42c01900 0x4>, > + <0x42c01904 0x4>, > + <0x42c01908 0x4>; > + reg-names = "rev", "sysc", "syss"; > + ti,sysc-mask = <(SYSC_OMAP4_SOFTRESET | > + SYSC_DRA7_MCAN_ENAWAKEUP)>; > + ti,syss-mask = <1>; > + clocks = <&wkupaon_clkctrl DRA7_ADC_CLKCTRL 0>; > + clock-names = "fck"; > + }; > + }; > + > }; Looks good to me except I think the reset won't do anything currently with ti-sysc.c unless you specfify also "ti,hwmods" for the module? Can you please check? It might be worth adding the reset function to ti-sysc.c for non "ti,hwmods" case and that just might remove the need for any hwmod code for this module. Regards, Tony
On Wed, May 30, 2018 at 07:41:32PM +0530, Faiz Abbas wrote: > The ti-sysc driver provides support for manipulating the idlemodes > and interconnect level resets. > > Add the generic interconnect target module node for MCAN to support > the same. > > CC: Tony Lindgren <tony@atomide.com> > Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> > --- > arch/arm/boot/dts/dra76x.dtsi | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi > index bfc82636999c..57b8dc0fe719 100644 > --- a/arch/arm/boot/dts/dra76x.dtsi > +++ b/arch/arm/boot/dts/dra76x.dtsi > @@ -11,6 +11,25 @@ > / { > compatible = "ti,dra762", "ti,dra7"; > > + ocp { > + > + target-module@0x42c00000 { Build your dtb with W=1 and fix warnings you add (drop '0x'). This is a CAN bus controller? If so, then use 'can' for node name. > + compatible = "ti,sysc-dra7-mcan"; > + ranges = <0x0 0x42c00000 0x2000>; > + #address-cells = <1>; > + #size-cells = <1>; > + reg = <0x42c01900 0x4>, > + <0x42c01904 0x4>, > + <0x42c01908 0x4>; > + reg-names = "rev", "sysc", "syss"; > + ti,sysc-mask = <(SYSC_OMAP4_SOFTRESET | > + SYSC_DRA7_MCAN_ENAWAKEUP)>; > + ti,syss-mask = <1>; > + clocks = <&wkupaon_clkctrl DRA7_ADC_CLKCTRL 0>; > + clock-names = "fck"; > + }; > + }; > + > }; > > /* MCAN interrupts are hard-wired to irqs 67, 68 */ > -- > 2.17.0 >
Hi, On Wednesday 30 May 2018 08:34 PM, Tony Lindgren wrote: > * Faiz Abbas <faiz_abbas@ti.com> [180530 14:12]: >> The ti-sysc driver provides support for manipulating the idlemodes >> and interconnect level resets. > ... >> --- a/arch/arm/boot/dts/dra76x.dtsi >> +++ b/arch/arm/boot/dts/dra76x.dtsi >> @@ -11,6 +11,25 @@ >> / { >> compatible = "ti,dra762", "ti,dra7"; >> >> + ocp { >> + >> + target-module@0x42c00000 { >> + compatible = "ti,sysc-dra7-mcan"; >> + ranges = <0x0 0x42c00000 0x2000>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + reg = <0x42c01900 0x4>, >> + <0x42c01904 0x4>, >> + <0x42c01908 0x4>; >> + reg-names = "rev", "sysc", "syss"; >> + ti,sysc-mask = <(SYSC_OMAP4_SOFTRESET | >> + SYSC_DRA7_MCAN_ENAWAKEUP)>; >> + ti,syss-mask = <1>; >> + clocks = <&wkupaon_clkctrl DRA7_ADC_CLKCTRL 0>; >> + clock-names = "fck"; >> + }; >> + }; >> + >> }; > > Looks good to me except I think the reset won't do anything currently > with ti-sysc.c unless you specfify also "ti,hwmods" for the module? > > Can you please check? It might be worth adding the reset function to > ti-sysc.c for non "ti,hwmods" case and that just might remove the > need for any hwmod code for this module. > If I understand correctly, this involves adding a (*reset_module) in ti_sysc_platform_data and defining a ti_sysc_reset_module() in ti-sysc.c similar to ti_sysc_idle_module(). Right? Thanks, Faiz
Hi, On Thursday 31 May 2018 09:36 AM, Rob Herring wrote: > On Wed, May 30, 2018 at 07:41:32PM +0530, Faiz Abbas wrote: >> The ti-sysc driver provides support for manipulating the idlemodes >> and interconnect level resets. >> >> Add the generic interconnect target module node for MCAN to support >> the same. >> >> CC: Tony Lindgren <tony@atomide.com> >> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> >> --- >> arch/arm/boot/dts/dra76x.dtsi | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi >> index bfc82636999c..57b8dc0fe719 100644 >> --- a/arch/arm/boot/dts/dra76x.dtsi >> +++ b/arch/arm/boot/dts/dra76x.dtsi >> @@ -11,6 +11,25 @@ >> / { >> compatible = "ti,dra762", "ti,dra7"; >> >> + ocp { >> + >> + target-module@0x42c00000 { > > Build your dtb with W=1 and fix warnings you add (drop '0x'). Sure, Will fix this. > > This is a CAN bus controller? If so, then use 'can' for node name. Yes but I am using m_can along the lines of dcan in other boards (For example, see arch/arm/boot/dts/am33xx.dtsi:1046). Are you saying all CAN controllers should only be called can? Thanks, Faiz
* Faiz Abbas <faiz_abbas@ti.com> [180531 11:27]: > On Thursday 31 May 2018 09:36 AM, Rob Herring wrote: > > On Wed, May 30, 2018 at 07:41:32PM +0530, Faiz Abbas wrote: > >> --- a/arch/arm/boot/dts/dra76x.dtsi > >> +++ b/arch/arm/boot/dts/dra76x.dtsi > >> @@ -11,6 +11,25 @@ > >> / { > >> compatible = "ti,dra762", "ti,dra7"; > >> > >> + ocp { > >> + > >> + target-module@0x42c00000 { > > > > Build your dtb with W=1 and fix warnings you add (drop '0x'). > > Sure, Will fix this. > > > > This is a CAN bus controller? If so, then use 'can' for node name. > > Yes but I am using m_can along the lines of dcan in other boards (For > example, see arch/arm/boot/dts/am33xx.dtsi:1046). Are you saying all CAN > controllers should only be called can? The module should be target-module@42c00000 and the child(ren) can@1a00 or mcan@1a00 if mcan is different from can. Regards, Tony
* Faiz Abbas <faiz_abbas@ti.com> [180531 10:22]: > On Wednesday 30 May 2018 08:34 PM, Tony Lindgren wrote: > > Looks good to me except I think the reset won't do anything currently > > with ti-sysc.c unless you specfify also "ti,hwmods" for the module? > > > > Can you please check? It might be worth adding the reset function to > > ti-sysc.c for non "ti,hwmods" case and that just might remove the > > need for any hwmod code for this module. > > > > If I understand correctly, this involves adding a (*reset_module) in > ti_sysc_platform_data and defining a ti_sysc_reset_module() in ti-sysc.c > similar to ti_sysc_idle_module(). Right? Well try moving "ti,hwmods" to the module level first. Then reset will happen with enable. Then for simple cases we can add reset directly to ti-sysc.c without pdata callbacks and and drop "ti,hwmods". For more complex cases we need to use reset-simple for the RSTCTRL registers. Regards, Tony
Hi, On Thursday 31 May 2018 07:15 PM, Tony Lindgren wrote: > * Faiz Abbas <faiz_abbas@ti.com> [180531 10:22]: >> On Wednesday 30 May 2018 08:34 PM, Tony Lindgren wrote: >>> Looks good to me except I think the reset won't do anything currently >>> with ti-sysc.c unless you specfify also "ti,hwmods" for the module? >>> >>> Can you please check? It might be worth adding the reset function to >>> ti-sysc.c for non "ti,hwmods" case and that just might remove the >>> need for any hwmod code for this module. >>> >> >> If I understand correctly, this involves adding a (*reset_module) in >> ti_sysc_platform_data and defining a ti_sysc_reset_module() in ti-sysc.c >> similar to ti_sysc_idle_module(). Right? > > Well try moving "ti,hwmods" to the module level first. Then reset will > happen with enable. Ok. Let me try that. Thanks, Faiz
Quoting Faiz Abbas (2018-05-30 07:11:32) > diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi > index bfc82636999c..57b8dc0fe719 100644 > --- a/arch/arm/boot/dts/dra76x.dtsi > +++ b/arch/arm/boot/dts/dra76x.dtsi > @@ -11,6 +11,25 @@ > / { > compatible = "ti,dra762", "ti,dra7"; > > + ocp { > + > + target-module@0x42c00000 { Drop the 0x on unit address. Also, it should match the first register property. And what is a "target-module"? > + compatible = "ti,sysc-dra7-mcan"; > + ranges = <0x0 0x42c00000 0x2000>; > + #address-cells = <1>; > + #size-cells = <1>; > + reg = <0x42c01900 0x4>, > + <0x42c01904 0x4>, >
Hi, On Saturday 02 June 2018 12:02 PM, Stephen Boyd wrote: > Quoting Faiz Abbas (2018-05-30 07:11:32) >> diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi >> index bfc82636999c..57b8dc0fe719 100644 >> --- a/arch/arm/boot/dts/dra76x.dtsi >> +++ b/arch/arm/boot/dts/dra76x.dtsi >> @@ -11,6 +11,25 @@ >> / { >> compatible = "ti,dra762", "ti,dra7"; >> >> + ocp { >> + >> + target-module@0x42c00000 { > > Drop the 0x on unit address. Also, it should match the first register > property. And what is a "target-module"? > Will fix it. And please see Documentation/devicetree/bindings/bus/ti-sysc.txt Thanks, Faiz
diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi index bfc82636999c..57b8dc0fe719 100644 --- a/arch/arm/boot/dts/dra76x.dtsi +++ b/arch/arm/boot/dts/dra76x.dtsi @@ -11,6 +11,25 @@ / { compatible = "ti,dra762", "ti,dra7"; + ocp { + + target-module@0x42c00000 { + compatible = "ti,sysc-dra7-mcan"; + ranges = <0x0 0x42c00000 0x2000>; + #address-cells = <1>; + #size-cells = <1>; + reg = <0x42c01900 0x4>, + <0x42c01904 0x4>, + <0x42c01908 0x4>; + reg-names = "rev", "sysc", "syss"; + ti,sysc-mask = <(SYSC_OMAP4_SOFTRESET | + SYSC_DRA7_MCAN_ENAWAKEUP)>; + ti,syss-mask = <1>; + clocks = <&wkupaon_clkctrl DRA7_ADC_CLKCTRL 0>; + clock-names = "fck"; + }; + }; + }; /* MCAN interrupts are hard-wired to irqs 67, 68 */
The ti-sysc driver provides support for manipulating the idlemodes and interconnect level resets. Add the generic interconnect target module node for MCAN to support the same. CC: Tony Lindgren <tony@atomide.com> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> --- arch/arm/boot/dts/dra76x.dtsi | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)