The following patches add dts, hwmod and sysconfig support
for MCAN on TI's dra76 SOCs
The patches depend on the following series:
https://patchwork.kernel.org/patch/10221105/
Changes in v2:
1. Added Support for mcan in the ti-sysc driver
Also added the target-module node for the same
2. Added clkctrl data for mcan clocks
Faiz Abbas (3):
clk: ti: dra7: Add clkctrl clock data for the mcan clocks
bus: ti-sysc: Add support for using ti-sysc for MCAN on dra76x
ARM: dts: Add generic interconnect target module node for MCAN
Franklin S Cooper Jr (1):
ARM: dts: dra76x: Add MCAN node
Lokesh Vutla (2):
ARM: dra762: hwmod: Add MCAN support
ARM: dts: dra762: Add MCAN clock support
.../devicetree/bindings/bus/ti-sysc.txt | 1 +
arch/arm/boot/dts/dra76-evm.dts | 7 ++
arch/arm/boot/dts/dra76x.dtsi | 67 +++++++++++++++++++
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 32 +++++++++
drivers/bus/ti-sysc.c | 17 +++++
drivers/clk/ti/clk-7xx.c | 1 +
include/dt-bindings/bus/ti-sysc.h | 2 +
include/dt-bindings/clock/dra7.h | 1 +
include/linux/platform_data/ti-sysc.h | 1 +
9 files changed, 129 insertions(+)
--
2.17.0
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 <[email protected]>
Signed-off-by: Faiz Abbas <[email protected]>
---
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 {
+ 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
The dra76x MCAN generic interconnect module has a its own
format for the bits in the control registers.
Therefore add a new module type, new regbits and new capabilities
specific to the MCAN module.
CC: Tony Lindgren <[email protected]>
Signed-off-by: Faiz Abbas <[email protected]>
---
.../devicetree/bindings/bus/ti-sysc.txt | 1 +
drivers/bus/ti-sysc.c | 17 +++++++++++++++++
include/dt-bindings/bus/ti-sysc.h | 2 ++
include/linux/platform_data/ti-sysc.h | 1 +
4 files changed, 21 insertions(+)
diff --git a/Documentation/devicetree/bindings/bus/ti-sysc.txt b/Documentation/devicetree/bindings/bus/ti-sysc.txt
index 2957a9ae291f..ebbb11144b7b 100644
--- a/Documentation/devicetree/bindings/bus/ti-sysc.txt
+++ b/Documentation/devicetree/bindings/bus/ti-sysc.txt
@@ -36,6 +36,7 @@ Required standard properties:
"ti,sysc-omap-aes"
"ti,sysc-mcasp"
"ti,sysc-usb-host-fs"
+ "ti,sysc-dra7-mcan"
- reg shall have register areas implemented for the interconnect
target module in question such as revision, sysc and syss
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index 7cd2fd04b212..83b47974567a 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -1262,6 +1262,22 @@ static const struct sysc_capabilities sysc_omap4_usb_host_fs = {
.regbits = &sysc_regbits_omap4_usb_host_fs,
};
+static const struct sysc_regbits sysc_regbits_dra7_mcan = {
+ .dmadisable_shift = -ENODEV,
+ .midle_shift = -ENODEV,
+ .sidle_shift = -ENODEV,
+ .clkact_shift = -ENODEV,
+ .enwkup_shift = 4,
+ .srst_shift = 0,
+ .emufree_shift = -ENODEV,
+ .autoidle_shift = -ENODEV,
+};
+
+static const struct sysc_capabilities sysc_dra7_mcan = {
+ .type = TI_SYSC_DRA7_MCAN,
+ .regbits = &sysc_regbits_dra7_mcan,
+};
+
static int sysc_init_pdata(struct sysc *ddata)
{
struct ti_sysc_platform_data *pdata = dev_get_platdata(ddata->dev);
@@ -1441,6 +1457,7 @@ static const struct of_device_id sysc_match[] = {
{ .compatible = "ti,sysc-mcasp", .data = &sysc_omap4_mcasp, },
{ .compatible = "ti,sysc-usb-host-fs",
.data = &sysc_omap4_usb_host_fs, },
+ { .compatible = "ti,sysc-dra7-mcan", .data = &sysc_dra7_mcan, },
{ },
};
MODULE_DEVICE_TABLE(of, sysc_match);
diff --git a/include/dt-bindings/bus/ti-sysc.h b/include/dt-bindings/bus/ti-sysc.h
index 2c005376ac0e..7138384e2ef9 100644
--- a/include/dt-bindings/bus/ti-sysc.h
+++ b/include/dt-bindings/bus/ti-sysc.h
@@ -15,6 +15,8 @@
/* SmartReflex sysc found on 36xx and later */
#define SYSC_OMAP3_SR_ENAWAKEUP (1 << 26)
+#define SYSC_DRA7_MCAN_ENAWAKEUP (1 << 4)
+
/* SYSCONFIG STANDBYMODE/MIDLEMODE/SIDLEMODE supported by hardware */
#define SYSC_IDLE_FORCE 0
#define SYSC_IDLE_NO 1
diff --git a/include/linux/platform_data/ti-sysc.h b/include/linux/platform_data/ti-sysc.h
index 80ce28d40832..1ea3aab972b4 100644
--- a/include/linux/platform_data/ti-sysc.h
+++ b/include/linux/platform_data/ti-sysc.h
@@ -14,6 +14,7 @@ enum ti_sysc_module_type {
TI_SYSC_OMAP4_SR,
TI_SYSC_OMAP4_MCASP,
TI_SYSC_OMAP4_USB_HOST_FS,
+ TI_SYSC_DRA7_MCAN,
};
struct ti_sysc_cookie {
--
2.17.0
From: Franklin S Cooper Jr <[email protected]>
Add support for the MCAN peripheral which supports both classic
CAN messages along with the new CAN-FD message.
Add MCAN node to evm and enable it with a maximum datarate of 5 mbps
Signed-off-by: Faiz Abbas <[email protected]>
---
arch/arm/boot/dts/dra76-evm.dts | 7 +++++++
arch/arm/boot/dts/dra76x.dtsi | 15 +++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/arch/arm/boot/dts/dra76-evm.dts b/arch/arm/boot/dts/dra76-evm.dts
index 2deb96405d06..277765257410 100644
--- a/arch/arm/boot/dts/dra76-evm.dts
+++ b/arch/arm/boot/dts/dra76-evm.dts
@@ -404,3 +404,10 @@
phys = <&pcie1_phy>, <&pcie2_phy>;
phy-names = "pcie-phy0", "pcie-phy1";
};
+
+&m_can0 {
+ status = "okay";
+ can-transceiver {
+ max-bitrate = <5000000>;
+ };
+};
diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi
index 57b8dc0fe719..d7a8cc569808 100644
--- a/arch/arm/boot/dts/dra76x.dtsi
+++ b/arch/arm/boot/dts/dra76x.dtsi
@@ -27,6 +27,21 @@
ti,syss-mask = <1>;
clocks = <&wkupaon_clkctrl DRA7_ADC_CLKCTRL 0>;
clock-names = "fck";
+
+ m_can0: mcan@42c01a00 {
+ compatible = "bosch,m_can";
+ reg = <0x1a00 0x4000>, <0x0 0x18FC>;
+ reg-names = "m_can", "message_ram";
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "int0", "int1";
+ ti,hwmods = "mcan";
+ clocks = <&mcan_clk>, <&l3_iclk_div>;
+ clock-names = "cclk", "hclk";
+ bosch,mram-cfg = <0x0 0 0 32 0 0 1 1>;
+ status = "disabled";
+ };
};
};
--
2.17.0
From: Lokesh Vutla <[email protected]>
Add MCAN hwmod data and register it for dra762 silicons.
Signed-off-by: Lokesh Vutla <[email protected]>
Signed-off-by: Faiz Abbas <[email protected]>
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 32 +++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index 62352d1e6361..a2cd7f865a60 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1355,6 +1355,29 @@ static struct omap_hwmod dra7xx_mailbox13_hwmod = {
},
};
+/*
+ * 'mcan' class
+ *
+ */
+static struct omap_hwmod_class dra76x_mcan_hwmod_class = {
+ .name = "mcan",
+};
+
+/* mcan */
+static struct omap_hwmod dra76x_mcan_hwmod = {
+ .name = "mcan",
+ .class = &dra76x_mcan_hwmod_class,
+ .clkdm_name = "wkupaon_clkdm",
+ .main_clk = "mcan_clk",
+ .prcm = {
+ .omap4 = {
+ .clkctrl_offs = DRA7XX_CM_WKUPAON_ADC_CLKCTRL_OFFSET,
+ .context_offs = DRA7XX_RM_WKUPAON_ADC_CONTEXT_OFFSET,
+ .modulemode = MODULEMODE_SWCTRL,
+ },
+ },
+};
+
/*
* 'mcspi' class
*
@@ -3818,6 +3841,14 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss2 = {
.user = OCP_USER_MPU,
};
+/* l3_main_1 -> mcan */
+static struct omap_hwmod_ocp_if dra76x_l3_main_1__mcan = {
+ .master = &dra7xx_l3_main_1_hwmod,
+ .slave = &dra76x_mcan_hwmod,
+ .clk = "l3_iclk_div",
+ .user = OCP_USER_MPU | OCP_USER_SDMA,
+};
+
static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = {
&dra7xx_l3_main_1__dmm,
&dra7xx_l3_main_2__l3_instr,
@@ -3958,6 +3989,7 @@ static struct omap_hwmod_ocp_if *dra7xx_gp_hwmod_ocp_ifs[] __initdata = {
/* SoC variant specific hwmod links */
static struct omap_hwmod_ocp_if *dra76x_hwmod_ocp_ifs[] __initdata = {
&dra7xx_l4_per3__usb_otg_ss4,
+ &dra76x_l3_main_1__mcan,
NULL,
};
--
2.17.0
Add clkctrl data for the m_can clocks and register it within the
clkctrl driver
CC: Tero Kristo <[email protected]>
Signed-off-by: Faiz Abbas <[email protected]>
---
drivers/clk/ti/clk-7xx.c | 1 +
include/dt-bindings/clock/dra7.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
index fb249a1637a5..71a122b2dc67 100644
--- a/drivers/clk/ti/clk-7xx.c
+++ b/drivers/clk/ti/clk-7xx.c
@@ -708,6 +708,7 @@ static const struct omap_clkctrl_reg_data dra7_wkupaon_clkctrl_regs[] __initcons
{ DRA7_COUNTER_32K_CLKCTRL, NULL, 0, "wkupaon_iclk_mux" },
{ DRA7_UART10_CLKCTRL, dra7_uart10_bit_data, CLKF_SW_SUP, "wkupaon_cm:clk:0060:24" },
{ DRA7_DCAN1_CLKCTRL, dra7_dcan1_bit_data, CLKF_SW_SUP, "wkupaon_cm:clk:0068:24" },
+ { DRA7_ADC_CLKCTRL, NULL, CLKF_SW_SUP, "mcan_clk" },
{ 0 },
};
diff --git a/include/dt-bindings/clock/dra7.h b/include/dt-bindings/clock/dra7.h
index 5e1061b15aed..d7549c57cac3 100644
--- a/include/dt-bindings/clock/dra7.h
+++ b/include/dt-bindings/clock/dra7.h
@@ -168,5 +168,6 @@
#define DRA7_COUNTER_32K_CLKCTRL DRA7_CLKCTRL_INDEX(0x50)
#define DRA7_UART10_CLKCTRL DRA7_CLKCTRL_INDEX(0x80)
#define DRA7_DCAN1_CLKCTRL DRA7_CLKCTRL_INDEX(0x88)
+#define DRA7_ADC_CLKCTRL DRA7_CLKCTRL_INDEX(0xa0)
#endif
--
2.17.0
From: Lokesh Vutla <[email protected]>
MCAN is clocked by H14 divider of DPLL_GMAC. Unlike other
DPLL dividers this DPLL_GMAC H14 divider is controlled by
control module. Adding support for these clocks.
Signed-off-by: Lokesh Vutla <[email protected]>
Signed-off-by: Faiz Abbas <[email protected]>
---
arch/arm/boot/dts/dra76x.dtsi | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi
index 1c88c581ff18..bfc82636999c 100644
--- a/arch/arm/boot/dts/dra76x.dtsi
+++ b/arch/arm/boot/dts/dra76x.dtsi
@@ -17,3 +17,36 @@
&crossbar_mpu {
ti,irqs-skip = <10 67 68 133 139 140>;
};
+
+&scm_conf_clocks {
+ dpll_gmac_h14x2_ctrl_ck: dpll_gmac_h14x2_ctrl_ck@3fc {
+ #clock-cells = <0>;
+ compatible = "ti,divider-clock";
+ clocks = <&dpll_gmac_x2_ck>;
+ ti,max-div = <63>;
+ reg = <0x03fc>;
+ ti,bit-shift=<20>;
+ ti,latch-bit=<26>;
+ assigned-clocks = <&dpll_gmac_h14x2_ctrl_ck>;
+ assigned-clock-rates = <80000000>;
+ };
+
+ dpll_gmac_h14x2_ctrl_mux_ck: dpll_gmac_h14x2_ctrl_mux_ck@3fc {
+ #clock-cells = <0>;
+ compatible = "ti,mux-clock";
+ clocks = <&dpll_gmac_ck>, <&dpll_gmac_h14x2_ctrl_ck>;
+ reg = <0x3fc>;
+ ti,bit-shift = <29>;
+ ti,latch-bit=<26>;
+ assigned-clocks = <&dpll_gmac_h14x2_ctrl_mux_ck>;
+ assigned-clock-parents = <&dpll_gmac_h14x2_ctrl_ck>;
+ };
+
+ mcan_clk: mcan_clk@3fc {
+ #clock-cells = <0>;
+ compatible = "ti,gate-clock";
+ clocks = <&dpll_gmac_h14x2_ctrl_mux_ck>;
+ ti,bit-shift = <27>;
+ reg = <0x3fc>;
+ };
+};
--
2.17.0
* Faiz Abbas <[email protected]> [180530 14:12]:
> From: Lokesh Vutla <[email protected]>
>
> Add MCAN hwmod data and register it for dra762 silicons.
>
> Signed-off-by: Lokesh Vutla <[email protected]>
> Signed-off-by: Faiz Abbas <[email protected]>
> ---
> arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 32 +++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index 62352d1e6361..a2cd7f865a60 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1355,6 +1355,29 @@ static struct omap_hwmod dra7xx_mailbox13_hwmod = {
> },
> };
>
> +/*
> + * 'mcan' class
> + *
> + */
> +static struct omap_hwmod_class dra76x_mcan_hwmod_class = {
> + .name = "mcan",
> +};
> +
> +/* mcan */
> +static struct omap_hwmod dra76x_mcan_hwmod = {
> + .name = "mcan",
> + .class = &dra76x_mcan_hwmod_class,
> + .clkdm_name = "wkupaon_clkdm",
> + .main_clk = "mcan_clk",
> + .prcm = {
> + .omap4 = {
> + .clkctrl_offs = DRA7XX_CM_WKUPAON_ADC_CLKCTRL_OFFSET,
> + .context_offs = DRA7XX_RM_WKUPAON_ADC_CONTEXT_OFFSET,
> + .modulemode = MODULEMODE_SWCTRL,
> + },
> + },
> +};
You should be now able to leave out at least the clkctrl_offs and modulemode
here. Please also check if leaving out clkdm_name and main_clk now works :)
> @@ -3818,6 +3841,14 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss2 = {
> .user = OCP_USER_MPU,
> };
>
> +/* l3_main_1 -> mcan */
> +static struct omap_hwmod_ocp_if dra76x_l3_main_1__mcan = {
> + .master = &dra7xx_l3_main_1_hwmod,
> + .slave = &dra76x_mcan_hwmod,
> + .clk = "l3_iclk_div",
> + .user = OCP_USER_MPU | OCP_USER_SDMA,
> +};
I think this we still need though for the clk. Tero, do you have any comments
on what all clocks can now be left out?
Regards,
Tony
* Faiz Abbas <[email protected]> [180530 14:12]:
> The dra76x MCAN generic interconnect module has a its own
> format for the bits in the control registers.
...
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -1262,6 +1262,22 @@ static const struct sysc_capabilities sysc_omap4_usb_host_fs = {
> .regbits = &sysc_regbits_omap4_usb_host_fs,
> };
>
> +static const struct sysc_regbits sysc_regbits_dra7_mcan = {
> + .dmadisable_shift = -ENODEV,
> + .midle_shift = -ENODEV,
> + .sidle_shift = -ENODEV,
> + .clkact_shift = -ENODEV,
> + .enwkup_shift = 4,
> + .srst_shift = 0,
> + .emufree_shift = -ENODEV,
> + .autoidle_shift = -ENODEV,
> +};
> +
> +static const struct sysc_capabilities sysc_dra7_mcan = {
> + .type = TI_SYSC_DRA7_MCAN,
> + .regbits = &sysc_regbits_dra7_mcan,
> +};
> +
> static int sysc_init_pdata(struct sysc *ddata)
> {
> struct ti_sysc_platform_data *pdata = dev_get_platdata(ddata->dev);
> @@ -1441,6 +1457,7 @@ static const struct of_device_id sysc_match[] = {
> { .compatible = "ti,sysc-mcasp", .data = &sysc_omap4_mcasp, },
> { .compatible = "ti,sysc-usb-host-fs",
> .data = &sysc_omap4_usb_host_fs, },
> + { .compatible = "ti,sysc-dra7-mcan", .data = &sysc_dra7_mcan, },
> { },
> };
Looks good to me. And presumably you checked that no other dra7 modules
use sysconfig register bit layout like this?
Regards,
Tony
* Faiz Abbas <[email protected]> [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
* Faiz Abbas <[email protected]> [180530 14:12]:
> From: Franklin S Cooper Jr <[email protected]>
>
> Add support for the MCAN peripheral which supports both classic
> CAN messages along with the new CAN-FD message.
...
> --- a/arch/arm/boot/dts/dra76x.dtsi
> +++ b/arch/arm/boot/dts/dra76x.dtsi
> @@ -27,6 +27,21 @@
> ti,syss-mask = <1>;
> clocks = <&wkupaon_clkctrl DRA7_ADC_CLKCTRL 0>;
> clock-names = "fck";
> +
> + m_can0: mcan@42c01a00 {
> + compatible = "bosch,m_can";
> + reg = <0x1a00 0x4000>, <0x0 0x18FC>;
> + reg-names = "m_can", "message_ram";
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "int0", "int1";
> + ti,hwmods = "mcan";
The "ti,hwmods" should be in the parent node now. But you may not even need
it, see the reset comment I made for the parent node patch.
> + clocks = <&mcan_clk>, <&l3_iclk_div>;
> + clock-names = "cclk", "hclk";
> + bosch,mram-cfg = <0x0 0 0 32 0 0 1 1>;
> + status = "disabled";
And then you should be able to also leave out status = "disabled" as the
hardware is there for sure and we can idle it. Then a board specific
dts file can set it to "disabled" if reallly needed. Setting everything
manually to "disabled" and then again to "enabled" is the wrong way
around, the default is "enabled".
Regards,
Tony
On 30/05/18 17:50, Tony Lindgren wrote:
> * Faiz Abbas <[email protected]> [180530 14:12]:
>> From: Lokesh Vutla <[email protected]>
>>
>> Add MCAN hwmod data and register it for dra762 silicons.
>>
>> Signed-off-by: Lokesh Vutla <[email protected]>
>> Signed-off-by: Faiz Abbas <[email protected]>
>> ---
>> arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 32 +++++++++++++++++++++++
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> index 62352d1e6361..a2cd7f865a60 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>> @@ -1355,6 +1355,29 @@ static struct omap_hwmod dra7xx_mailbox13_hwmod = {
>> },
>> };
>>
>> +/*
>> + * 'mcan' class
>> + *
>> + */
>> +static struct omap_hwmod_class dra76x_mcan_hwmod_class = {
>> + .name = "mcan",
>> +};
>> +
>> +/* mcan */
>> +static struct omap_hwmod dra76x_mcan_hwmod = {
>> + .name = "mcan",
>> + .class = &dra76x_mcan_hwmod_class,
>> + .clkdm_name = "wkupaon_clkdm",
>> + .main_clk = "mcan_clk",
>> + .prcm = {
>> + .omap4 = {
>> + .clkctrl_offs = DRA7XX_CM_WKUPAON_ADC_CLKCTRL_OFFSET,
>> + .context_offs = DRA7XX_RM_WKUPAON_ADC_CONTEXT_OFFSET,
>> + .modulemode = MODULEMODE_SWCTRL,
>> + },
>> + },
>> +};
>
> You should be now able to leave out at least the clkctrl_offs and modulemode
> here. Please also check if leaving out clkdm_name and main_clk now works :)
>
>> @@ -3818,6 +3841,14 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss2 = {
>> .user = OCP_USER_MPU,
>> };
>>
>> +/* l3_main_1 -> mcan */
>> +static struct omap_hwmod_ocp_if dra76x_l3_main_1__mcan = {
>> + .master = &dra7xx_l3_main_1_hwmod,
>> + .slave = &dra76x_mcan_hwmod,
>> + .clk = "l3_iclk_div",
>> + .user = OCP_USER_MPU | OCP_USER_SDMA,
>> +};
>
> I think this we still need though for the clk. Tero, do you have any comments
> on what all clocks can now be left out?
For the OCP if part, I think that is still needed until we switch over
to full sysc driver. clkctrl_offs you probably also need because that is
used for mapping the omap_hwmod against a specific clkctrl clock. Those
can be only removed once we are done with hwmod (or figure out some
other way to assign the clkctrl clock to a hwmod.)
-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
* Tero Kristo <[email protected]> [180530 15:18]:
> For the OCP if part, I think that is still needed until we switch over to
> full sysc driver. clkctrl_offs you probably also need because that is used
> for mapping the omap_hwmod against a specific clkctrl clock. Those can be
> only removed once we are done with hwmod (or figure out some other way to
> assign the clkctrl clock to a hwmod.)
Hmm might be worth testing. I thought your commit 70f05be32133
("ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available")
already parses the clkctrl from dts?
Regards,
Tony
On 30/05/18 18:28, Tony Lindgren wrote:
> * Tero Kristo <[email protected]> [180530 15:18]:
>> For the OCP if part, I think that is still needed until we switch over to
>> full sysc driver. clkctrl_offs you probably also need because that is used
>> for mapping the omap_hwmod against a specific clkctrl clock. Those can be
>> only removed once we are done with hwmod (or figure out some other way to
>> assign the clkctrl clock to a hwmod.)
>
> Hmm might be worth testing. I thought your commit 70f05be32133
> ("ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available")
> already parses the clkctrl from dts?
It maps the clkctrl clock to be used by hwmod, if those are available.
We didn't add any specific clock entries to DT for mapping the actual
clkctrl clock without the hwmod_data hints yet though, as that was
deemed temporary solution only due to transition to interconnect driver.
I.e., you would need something like this in DT for every device node:
&uart3 {
clocks = <l4per_clkctrl UART3_CLK 0>;
clock-names = "clkctrl";
};
... which is currently not present.
Alternatively you could rename the main_clk in the hwmod_data to point
towards the clkctrl clock, but again, that would be a temporary solution
only.
-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
* Tero Kristo <[email protected]> [180530 15:44]:
> On 30/05/18 18:28, Tony Lindgren wrote:
> > * Tero Kristo <[email protected]> [180530 15:18]:
> > > For the OCP if part, I think that is still needed until we switch over to
> > > full sysc driver. clkctrl_offs you probably also need because that is used
> > > for mapping the omap_hwmod against a specific clkctrl clock. Those can be
> > > only removed once we are done with hwmod (or figure out some other way to
> > > assign the clkctrl clock to a hwmod.)
> >
> > Hmm might be worth testing. I thought your commit 70f05be32133
> > ("ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available")
> > already parses the clkctrl from dts?
>
> It maps the clkctrl clock to be used by hwmod, if those are available. We
> didn't add any specific clock entries to DT for mapping the actual clkctrl
> clock without the hwmod_data hints yet though, as that was deemed temporary
> solution only due to transition to interconnect driver. I.e., you would need
> something like this in DT for every device node:
>
> &uart3 {
> clocks = <l4per_clkctrl UART3_CLK 0>;
> clock-names = "clkctrl";
> };
>
> ... which is currently not present.
Hmm is that not the "fck" clkctrl clock we have already in
the dts files for the interconnect target modules?
We can also use pdata callbacks to pass the clock node if
needed. But I guess I don't quite still understand what we
are missing :)
Regards,
Tony
On Wed, May 30, 2018 at 07:41:30PM +0530, Faiz Abbas wrote:
> Add clkctrl data for the m_can clocks and register it within the
> clkctrl driver
>
> CC: Tero Kristo <[email protected]>
> Signed-off-by: Faiz Abbas <[email protected]>
> ---
> drivers/clk/ti/clk-7xx.c | 1 +
> include/dt-bindings/clock/dra7.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
> index fb249a1637a5..71a122b2dc67 100644
> --- a/drivers/clk/ti/clk-7xx.c
> +++ b/drivers/clk/ti/clk-7xx.c
> @@ -708,6 +708,7 @@ static const struct omap_clkctrl_reg_data dra7_wkupaon_clkctrl_regs[] __initcons
> { DRA7_COUNTER_32K_CLKCTRL, NULL, 0, "wkupaon_iclk_mux" },
> { DRA7_UART10_CLKCTRL, dra7_uart10_bit_data, CLKF_SW_SUP, "wkupaon_cm:clk:0060:24" },
> { DRA7_DCAN1_CLKCTRL, dra7_dcan1_bit_data, CLKF_SW_SUP, "wkupaon_cm:clk:0068:24" },
> + { DRA7_ADC_CLKCTRL, NULL, CLKF_SW_SUP, "mcan_clk" },
> { 0 },
> };
>
> diff --git a/include/dt-bindings/clock/dra7.h b/include/dt-bindings/clock/dra7.h
> index 5e1061b15aed..d7549c57cac3 100644
> --- a/include/dt-bindings/clock/dra7.h
> +++ b/include/dt-bindings/clock/dra7.h
> @@ -168,5 +168,6 @@
> #define DRA7_COUNTER_32K_CLKCTRL DRA7_CLKCTRL_INDEX(0x50)
> #define DRA7_UART10_CLKCTRL DRA7_CLKCTRL_INDEX(0x80)
> #define DRA7_DCAN1_CLKCTRL DRA7_CLKCTRL_INDEX(0x88)
> +#define DRA7_ADC_CLKCTRL DRA7_CLKCTRL_INDEX(0xa0)
ADC and mcan are the same thing?
>
> #endif
> --
> 2.17.0
>
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 <[email protected]>
> Signed-off-by: Faiz Abbas <[email protected]>
> ---
> 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
>
On Wed, May 30, 2018 at 07:41:31PM +0530, Faiz Abbas wrote:
> The dra76x MCAN generic interconnect module has a its own
> format for the bits in the control registers.
>
> Therefore add a new module type, new regbits and new capabilities
> specific to the MCAN module.
>
> CC: Tony Lindgren <[email protected]>
> Signed-off-by: Faiz Abbas <[email protected]>
> ---
> .../devicetree/bindings/bus/ti-sysc.txt | 1 +
> drivers/bus/ti-sysc.c | 17 +++++++++++++++++
> include/dt-bindings/bus/ti-sysc.h | 2 ++
> include/linux/platform_data/ti-sysc.h | 1 +
> 4 files changed, 21 insertions(+)
For the DT bits:
Acked-by: Rob Herring <[email protected]>
On 30/05/18 18:54, Tony Lindgren wrote:
> * Tero Kristo <[email protected]> [180530 15:44]:
>> On 30/05/18 18:28, Tony Lindgren wrote:
>>> * Tero Kristo <[email protected]> [180530 15:18]:
>>>> For the OCP if part, I think that is still needed until we switch over to
>>>> full sysc driver. clkctrl_offs you probably also need because that is used
>>>> for mapping the omap_hwmod against a specific clkctrl clock. Those can be
>>>> only removed once we are done with hwmod (or figure out some other way to
>>>> assign the clkctrl clock to a hwmod.)
>>>
>>> Hmm might be worth testing. I thought your commit 70f05be32133
>>> ("ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available")
>>> already parses the clkctrl from dts?
>>
>> It maps the clkctrl clock to be used by hwmod, if those are available. We
>> didn't add any specific clock entries to DT for mapping the actual clkctrl
>> clock without the hwmod_data hints yet though, as that was deemed temporary
>> solution only due to transition to interconnect driver. I.e., you would need
>> something like this in DT for every device node:
>>
>> &uart3 {
>> clocks = <l4per_clkctrl UART3_CLK 0>;
>> clock-names = "clkctrl";
>> };
>>
>> ... which is currently not present.
>
> Hmm is that not the "fck" clkctrl clock we have already in
> the dts files for the interconnect target modules?
Oh okay, yeah, we could parse that one, but currently it is not done,
and is not present for everything either I believe.
> We can also use pdata callbacks to pass the clock node if
> needed. But I guess I don't quite still understand what we
> are missing :)
So, what is missing is the glue logic only from the hwmod codebase.
Right now this is not supported but should be relatively trivial thing
to add if we really want to do this.
-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Hi,
On Thursday 31 May 2018 09:33 AM, Rob Herring wrote:
> On Wed, May 30, 2018 at 07:41:30PM +0530, Faiz Abbas wrote:
>> Add clkctrl data for the m_can clocks and register it within the
...
>>
>> diff --git a/include/dt-bindings/clock/dra7.h b/include/dt-bindings/clock/dra7.h
>> index 5e1061b15aed..d7549c57cac3 100644
>> --- a/include/dt-bindings/clock/dra7.h
>> +++ b/include/dt-bindings/clock/dra7.h
>> @@ -168,5 +168,6 @@
>> #define DRA7_COUNTER_32K_CLKCTRL DRA7_CLKCTRL_INDEX(0x50)
>> #define DRA7_UART10_CLKCTRL DRA7_CLKCTRL_INDEX(0x80)
>> #define DRA7_DCAN1_CLKCTRL DRA7_CLKCTRL_INDEX(0x88)
>> +#define DRA7_ADC_CLKCTRL DRA7_CLKCTRL_INDEX(0xa0)
>
> ADC and mcan are the same thing?
>
The register to control MCAN clocks is called ADC_CLKCTRL, Yes.
Thanks,
Faiz
Hi,
On Wednesday 30 May 2018 08:27 PM, Tony Lindgren wrote:
> * Faiz Abbas <[email protected]> [180530 14:12]:
>> The dra76x MCAN generic interconnect module has a its own
>> format for the bits in the control registers.
...
>> static int sysc_init_pdata(struct sysc *ddata)
>> {
>> struct ti_sysc_platform_data *pdata = dev_get_platdata(ddata->dev);
>> @@ -1441,6 +1457,7 @@ static const struct of_device_id sysc_match[] = {
>> { .compatible = "ti,sysc-mcasp", .data = &sysc_omap4_mcasp, },
>> { .compatible = "ti,sysc-usb-host-fs",
>> .data = &sysc_omap4_usb_host_fs, },
>> + { .compatible = "ti,sysc-dra7-mcan", .data = &sysc_dra7_mcan, },
>> { },
>> };
>
> Looks good to me. And presumably you checked that no other dra7 modules
> use sysconfig register bit layout like this?
>
As far as I could see, Yes.
Thanks,
Faiz
Hi,
On Wednesday 30 May 2018 08:34 PM, Tony Lindgren wrote:
> * Faiz Abbas <[email protected]> [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 <[email protected]>
>> Signed-off-by: Faiz Abbas <[email protected]>
>> ---
>> 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
On 31/05/18 13:14, Faiz Abbas wrote:
> Hi,
>
> On Thursday 31 May 2018 09:33 AM, Rob Herring wrote:
>> On Wed, May 30, 2018 at 07:41:30PM +0530, Faiz Abbas wrote:
>>> Add clkctrl data for the m_can clocks and register it within the
> ...
>>>
>>> diff --git a/include/dt-bindings/clock/dra7.h b/include/dt-bindings/clock/dra7.h
>>> index 5e1061b15aed..d7549c57cac3 100644
>>> --- a/include/dt-bindings/clock/dra7.h
>>> +++ b/include/dt-bindings/clock/dra7.h
>>> @@ -168,5 +168,6 @@
>>> #define DRA7_COUNTER_32K_CLKCTRL DRA7_CLKCTRL_INDEX(0x50)
>>> #define DRA7_UART10_CLKCTRL DRA7_CLKCTRL_INDEX(0x80)
>>> #define DRA7_DCAN1_CLKCTRL DRA7_CLKCTRL_INDEX(0x88)
>>> +#define DRA7_ADC_CLKCTRL DRA7_CLKCTRL_INDEX(0xa0)
>>
>> ADC and mcan are the same thing?
>>
>
> The register to control MCAN clocks is called ADC_CLKCTRL, Yes.
Is there any reason for this or is that just a documentation bug?
-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
* Faiz Abbas <[email protected]> [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 <[email protected]> [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
* Faiz Abbas <[email protected]> [180531 10:17]:
> Hi,
>
> On Wednesday 30 May 2018 08:27 PM, Tony Lindgren wrote:
> > * Faiz Abbas <[email protected]> [180530 14:12]:
> >> The dra76x MCAN generic interconnect module has a its own
> >> format for the bits in the control registers.
> ...
> >> static int sysc_init_pdata(struct sysc *ddata)
> >> {
> >> struct ti_sysc_platform_data *pdata = dev_get_platdata(ddata->dev);
> >> @@ -1441,6 +1457,7 @@ static const struct of_device_id sysc_match[] = {
> >> { .compatible = "ti,sysc-mcasp", .data = &sysc_omap4_mcasp, },
> >> { .compatible = "ti,sysc-usb-host-fs",
> >> .data = &sysc_omap4_usb_host_fs, },
> >> + { .compatible = "ti,sysc-dra7-mcan", .data = &sysc_dra7_mcan, },
> >> { },
> >> };
> >
> > Looks good to me. And presumably you checked that no other dra7 modules
> > use sysconfig register bit layout like this?
> >
>
> As far as I could see, Yes.
OK thanks for checking it.
Regards,
Tony
* Tero Kristo <[email protected]> [180531 06:18]:
> On 30/05/18 18:54, Tony Lindgren wrote:
> > * Tero Kristo <[email protected]> [180530 15:44]:
> > > On 30/05/18 18:28, Tony Lindgren wrote:
> > > > * Tero Kristo <[email protected]> [180530 15:18]:
> > > > > For the OCP if part, I think that is still needed until we switch over to
> > > > > full sysc driver. clkctrl_offs you probably also need because that is used
> > > > > for mapping the omap_hwmod against a specific clkctrl clock. Those can be
> > > > > only removed once we are done with hwmod (or figure out some other way to
> > > > > assign the clkctrl clock to a hwmod.)
> > > >
> > > > Hmm might be worth testing. I thought your commit 70f05be32133
> > > > ("ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available")
> > > > already parses the clkctrl from dts?
> > >
> > > It maps the clkctrl clock to be used by hwmod, if those are available. We
> > > didn't add any specific clock entries to DT for mapping the actual clkctrl
> > > clock without the hwmod_data hints yet though, as that was deemed temporary
> > > solution only due to transition to interconnect driver. I.e., you would need
> > > something like this in DT for every device node:
> > >
> > > &uart3 {
> > > clocks = <l4per_clkctrl UART3_CLK 0>;
> > > clock-names = "clkctrl";
> > > };
> > >
> > > ... which is currently not present.
> >
> > Hmm is that not the "fck" clkctrl clock we have already in
> > the dts files for the interconnect target modules?
>
> Oh okay, yeah, we could parse that one, but currently it is not done, and is
> not present for everything either I believe.
>
> > We can also use pdata callbacks to pass the clock node if
> > needed. But I guess I don't quite still understand what we
> > are missing :)
>
> So, what is missing is the glue logic only from the hwmod codebase. Right
> now this is not supported but should be relatively trivial thing to add if
> we really want to do this.
OK let's think about this a bit for v4.19 then.
Regards,
Tony
Hi,
On Thursday 31 May 2018 08:56 PM, Tony Lindgren wrote:
> * Tero Kristo <[email protected]> [180531 06:18]:
>> On 30/05/18 18:54, Tony Lindgren wrote:
>>> * Tero Kristo <[email protected]> [180530 15:44]:
>>>> On 30/05/18 18:28, Tony Lindgren wrote:
>>>>> * Tero Kristo <[email protected]> [180530 15:18]:
>>>>>> For the OCP if part, I think that is still needed until we switch over to
>>>>>> full sysc driver. clkctrl_offs you probably also need because that is used
>>>>>> for mapping the omap_hwmod against a specific clkctrl clock. Those can be
>>>>>> only removed once we are done with hwmod (or figure out some other way to
>>>>>> assign the clkctrl clock to a hwmod.)
>>>>>
>>>>> Hmm might be worth testing. I thought your commit 70f05be32133
>>>>> ("ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available")
>>>>> already parses the clkctrl from dts?
>>>>
>>>> It maps the clkctrl clock to be used by hwmod, if those are available. We
>>>> didn't add any specific clock entries to DT for mapping the actual clkctrl
>>>> clock without the hwmod_data hints yet though, as that was deemed temporary
>>>> solution only due to transition to interconnect driver. I.e., you would need
>>>> something like this in DT for every device node:
>>>>
>>>> &uart3 {
>>>> clocks = <l4per_clkctrl UART3_CLK 0>;
>>>> clock-names = "clkctrl";
>>>> };
>>>>
>>>> ... which is currently not present.
>>>
>>> Hmm is that not the "fck" clkctrl clock we have already in
>>> the dts files for the interconnect target modules?
>>
>> Oh okay, yeah, we could parse that one, but currently it is not done, and is
>> not present for everything either I believe.
>>
>>> We can also use pdata callbacks to pass the clock node if
>>> needed. But I guess I don't quite still understand what we
>>> are missing :)
>>
>> So, what is missing is the glue logic only from the hwmod codebase. Right
>> now this is not supported but should be relatively trivial thing to add if
>> we really want to do this.
>
> OK let's think about this a bit for v4.19 then.
>
I am not sure what the conclusion is. Should I try removing the
clkctrl_offsets, clkdm_name, and main_clk?
Thanks,
Faiz
Hi,
On Thursday 31 May 2018 07:15 PM, Tony Lindgren wrote:
> * Faiz Abbas <[email protected]> [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
* Faiz Abbas <[email protected]> [180531 16:36]:
> Hi,
>
> On Thursday 31 May 2018 08:56 PM, Tony Lindgren wrote:
> > * Tero Kristo <[email protected]> [180531 06:18]:
> >> On 30/05/18 18:54, Tony Lindgren wrote:
> >>> * Tero Kristo <[email protected]> [180530 15:44]:
> >>>> On 30/05/18 18:28, Tony Lindgren wrote:
> >>>>> * Tero Kristo <[email protected]> [180530 15:18]:
> >>>>>> For the OCP if part, I think that is still needed until we switch over to
> >>>>>> full sysc driver. clkctrl_offs you probably also need because that is used
> >>>>>> for mapping the omap_hwmod against a specific clkctrl clock. Those can be
> >>>>>> only removed once we are done with hwmod (or figure out some other way to
> >>>>>> assign the clkctrl clock to a hwmod.)
> >>>>>
> >>>>> Hmm might be worth testing. I thought your commit 70f05be32133
> >>>>> ("ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available")
> >>>>> already parses the clkctrl from dts?
> >>>>
> >>>> It maps the clkctrl clock to be used by hwmod, if those are available. We
> >>>> didn't add any specific clock entries to DT for mapping the actual clkctrl
> >>>> clock without the hwmod_data hints yet though, as that was deemed temporary
> >>>> solution only due to transition to interconnect driver. I.e., you would need
> >>>> something like this in DT for every device node:
> >>>>
> >>>> &uart3 {
> >>>> clocks = <l4per_clkctrl UART3_CLK 0>;
> >>>> clock-names = "clkctrl";
> >>>> };
> >>>>
> >>>> ... which is currently not present.
> >>>
> >>> Hmm is that not the "fck" clkctrl clock we have already in
> >>> the dts files for the interconnect target modules?
> >>
> >> Oh okay, yeah, we could parse that one, but currently it is not done, and is
> >> not present for everything either I believe.
> >>
> >>> We can also use pdata callbacks to pass the clock node if
> >>> needed. But I guess I don't quite still understand what we
> >>> are missing :)
> >>
> >> So, what is missing is the glue logic only from the hwmod codebase. Right
> >> now this is not supported but should be relatively trivial thing to add if
> >> we really want to do this.
> >
> > OK let's think about this a bit for v4.19 then.
> >
>
> I am not sure what the conclusion is. Should I try removing the
> clkctrl_offsets, clkdm_name, and main_clk?
No need to, it's not going to work currently without them.
Regards,
Tony
Hi,
On Thursday 31 May 2018 06:59 PM, Tero Kristo wrote:
> On 31/05/18 13:14, Faiz Abbas wrote:
>> Hi,
>>
>> On Thursday 31 May 2018 09:33 AM, Rob Herring wrote:
>>> On Wed, May 30, 2018 at 07:41:30PM +0530, Faiz Abbas wrote:
>>>> Add clkctrl data for the m_can clocks and register it within the
>> ...
>>>> diff --git a/include/dt-bindings/clock/dra7.h
>>>> b/include/dt-bindings/clock/dra7.h
>>>> index 5e1061b15aed..d7549c57cac3 100644
>>>> --- a/include/dt-bindings/clock/dra7.h
>>>> +++ b/include/dt-bindings/clock/dra7.h
>>>> @@ -168,5 +168,6 @@
>>>> #define DRA7_COUNTER_32K_CLKCTRL DRA7_CLKCTRL_INDEX(0x50)
>>>> #define DRA7_UART10_CLKCTRL DRA7_CLKCTRL_INDEX(0x80)
>>>> #define DRA7_DCAN1_CLKCTRL DRA7_CLKCTRL_INDEX(0x88)
>>>> +#define DRA7_ADC_CLKCTRL DRA7_CLKCTRL_INDEX(0xa0)
>>>
>>> ADC and mcan are the same thing?
>>>
>>
>> The register to control MCAN clocks is called ADC_CLKCTRL, Yes.
>
> Is there any reason for this or is that just a documentation bug?
>
Looks like they meant to have an ADC in dra74 or dra72 but decided
against it and then many years later used the same registers for MCAN
instead. You can see ADC_CLKCTRL exists in dra72 TRM but is explicitly
disabled.
http://www.ti.com/lit/ug/spruic2b/spruic2b.pdf pg:1524
Thanks,
Faiz
* Faiz Abbas <[email protected]> [180601 06:49]:
> Hi,
>
> On Thursday 31 May 2018 06:59 PM, Tero Kristo wrote:
> > On 31/05/18 13:14, Faiz Abbas wrote:
> >> Hi,
> >>
> >> On Thursday 31 May 2018 09:33 AM, Rob Herring wrote:
> >>> On Wed, May 30, 2018 at 07:41:30PM +0530, Faiz Abbas wrote:
> >>>> Add clkctrl data for the m_can clocks and register it within the
> >> ...
> >>>> diff --git a/include/dt-bindings/clock/dra7.h
> >>>> b/include/dt-bindings/clock/dra7.h
> >>>> index 5e1061b15aed..d7549c57cac3 100644
> >>>> --- a/include/dt-bindings/clock/dra7.h
> >>>> +++ b/include/dt-bindings/clock/dra7.h
> >>>> @@ -168,5 +168,6 @@
> >>>> #define DRA7_COUNTER_32K_CLKCTRL DRA7_CLKCTRL_INDEX(0x50)
> >>>> #define DRA7_UART10_CLKCTRL DRA7_CLKCTRL_INDEX(0x80)
> >>>> #define DRA7_DCAN1_CLKCTRL DRA7_CLKCTRL_INDEX(0x88)
> >>>> +#define DRA7_ADC_CLKCTRL DRA7_CLKCTRL_INDEX(0xa0)
> >>>
> >>> ADC and mcan are the same thing?
> >>>
> >>
> >> The register to control MCAN clocks is called ADC_CLKCTRL, Yes.
> >
> > Is there any reason for this or is that just a documentation bug?
> >
>
> Looks like they meant to have an ADC in dra74 or dra72 but decided
> against it and then many years later used the same registers for MCAN
> instead. You can see ADC_CLKCTRL exists in dra72 TRM but is explicitly
> disabled.
>
> http://www.ti.com/lit/ug/spruic2b/spruic2b.pdf pg:1524
How about make add also something like to dra7.h:
#define DRA7_MCAN_CLKCTRL DRA7_ADC_CLKCTRL
And you can add a comment to the dts file to avoid people
getting confused with this constantly.
Regards,
Tony
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
Hi,
On Friday 01 June 2018 07:56 PM, Tony Lindgren wrote:
> * Faiz Abbas <[email protected]> [180601 06:49]:
>> Hi,
>>
>> On Thursday 31 May 2018 06:59 PM, Tero Kristo wrote:
>>> On 31/05/18 13:14, Faiz Abbas wrote:
>>>> Hi,
>>>>
>>>> On Thursday 31 May 2018 09:33 AM, Rob Herring wrote:
>>>>> On Wed, May 30, 2018 at 07:41:30PM +0530, Faiz Abbas wrote:
>>>>>> Add clkctrl data for the m_can clocks and register it within the
>>>> ...
>>>>>> diff --git a/include/dt-bindings/clock/dra7.h
>>>>>> b/include/dt-bindings/clock/dra7.h
>>>>>> index 5e1061b15aed..d7549c57cac3 100644
>>>>>> --- a/include/dt-bindings/clock/dra7.h
>>>>>> +++ b/include/dt-bindings/clock/dra7.h
>>>>>> @@ -168,5 +168,6 @@
>>>>>> #define DRA7_COUNTER_32K_CLKCTRL DRA7_CLKCTRL_INDEX(0x50)
>>>>>> #define DRA7_UART10_CLKCTRL DRA7_CLKCTRL_INDEX(0x80)
>>>>>> #define DRA7_DCAN1_CLKCTRL DRA7_CLKCTRL_INDEX(0x88)
>>>>>> +#define DRA7_ADC_CLKCTRL DRA7_CLKCTRL_INDEX(0xa0)
>>>>>
>>>>> ADC and mcan are the same thing?
>>>>>
>>>>
>>>> The register to control MCAN clocks is called ADC_CLKCTRL, Yes.
>>>
>>> Is there any reason for this or is that just a documentation bug?
>>>
>>
>> Looks like they meant to have an ADC in dra74 or dra72 but decided
>> against it and then many years later used the same registers for MCAN
>> instead. You can see ADC_CLKCTRL exists in dra72 TRM but is explicitly
>> disabled.
>>
>> http://www.ti.com/lit/ug/spruic2b/spruic2b.pdf pg:1524
>
> How about make add also something like to dra7.h:
>
> #define DRA7_MCAN_CLKCTRL DRA7_ADC_CLKCTRL
>
> And you can add a comment to the dts file to avoid people
> getting confused with this constantly.
>
I would prefer to follow the TRM so that people don't look for registers
that don't exist at all.
Thanks,
Faiz