[PATCH 0/2] Two ti-sysc driver fixes for v5.3 merge window

[PATCH 0/2] Two ti-sysc driver fixes for v5.3 merge window

[Tony Lindgren]

Hi all,

Here are few fixes for the am335x d_can boot issue Sebastian reported for
Beaglebone.

Regards,

Tony


Tony Lindgren (2):
  ARM: dts: Configure osc clock for d_can on am335x
  bus: ti-sysc: Handle devices with no control registers

 arch/arm/boot/dts/am33xx-l4.dtsi | 14 ++++++--------
 arch/arm/boot/dts/am437x-l4.dtsi |  4 ----
 drivers/bus/ti-sysc.c            | 23 +++++++++++------------
 3 files changed, 17 insertions(+), 24 deletions(-)

-- 
2.21.0

[PATCH 1/2] ARM: dts: Configure osc clock for d_can on am335x

[Tony Lindgren]

Reading the module revision register can cause an external abort on
non-linefetch depending of osc clock is not already enabled. This
started happening with commit 1a5cd7c23cc5 ("bus: ti-sysc: Enable all
clocks directly during init to read revision") as reported by
Sebastian Andrzej Siewior <bigeasy@linutronix.de>.

The reason why the issue happens is because we now attempt to read the
interconnect target module revision register by first manually enabling
all the device clocks in sysc_probe(). And looks like d_can also needs
the osc clock in addition to the module clock, and it may or may not be
enabled depending on the bootloader version and if other devices have
already requested osc clock.

Let's fix the issue by adding osc clock as an optional clock for the
module for am335x. Note that am437x does not seem to list the osc clock
at all, so presumably it is not needed for am437x.

I also noticed that we're incorrectly assuming the revision register for
d_can exists. But the module does not seem to have any revision, sysconfig
or sysstatus registers. But that's mostly a cosmetic issues, so I'll send
a patch separately for that.

Fixes: 1a5cd7c23cc5 ("bus: ti-sysc: Enable all clocks directly during init to read revision")
Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/boot/dts/am33xx-l4.dtsi | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/am33xx-l4.dtsi b/arch/arm/boot/dts/am33xx-l4.dtsi
--- a/arch/arm/boot/dts/am33xx-l4.dtsi
+++ b/arch/arm/boot/dts/am33xx-l4.dtsi
@@ -1762,8 +1762,9 @@
 			reg = <0xcc000 0x4>;
 			reg-names = "rev";
 			/* Domains (P, C): per_pwrdm, l4ls_clkdm */
-			clocks = <&l4ls_clkctrl AM3_L4LS_D_CAN0_CLKCTRL 0>;
-			clock-names = "fck";
+			clocks = <&l4ls_clkctrl AM3_L4LS_D_CAN0_CLKCTRL 0>,
+				 <&dcan0_fck>;
+			clock-names = "fck", "osc";
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges = <0x0 0xcc000 0x2000>;
@@ -1785,8 +1786,9 @@
 			reg = <0xd0000 0x4>;
 			reg-names = "rev";
 			/* Domains (P, C): per_pwrdm, l4ls_clkdm */
-			clocks = <&l4ls_clkctrl AM3_L4LS_D_CAN1_CLKCTRL 0>;
-			clock-names = "fck";
+			clocks = <&l4ls_clkctrl AM3_L4LS_D_CAN1_CLKCTRL 0>,
+				 <&dcan1_fck>;
+			clock-names = "fck", "osc";
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges = <0x0 0xd0000 0x2000>;
-- 
2.21.0

[PATCH 2/2] bus: ti-sysc: Handle devices with no control registers

[Tony Lindgren]

Some interconnect target modules have no module control registers at
all, such as d_can on am335x and am437x.

The d_can register offset at 0 is CTL register with 0x401 as the default
value. I guess I mistook the 0x401 value for a revision register as the
value happens to look similar to what the revision registers typically
have for other modules.

To handle modules with no control registers, we need to improve the
ti-sysc driver a bit to bail out with errors on no control registers,
and then we can remove the bogus revision registers for d_can.

Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/boot/dts/am33xx-l4.dtsi |  4 ----
 arch/arm/boot/dts/am437x-l4.dtsi |  4 ----
 drivers/bus/ti-sysc.c            | 23 +++++++++++------------
 3 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/arch/arm/boot/dts/am33xx-l4.dtsi b/arch/arm/boot/dts/am33xx-l4.dtsi
--- a/arch/arm/boot/dts/am33xx-l4.dtsi
+++ b/arch/arm/boot/dts/am33xx-l4.dtsi
@@ -1759,8 +1759,6 @@
 		target-module@cc000 {			/* 0x481cc000, ap 60 46.0 */
 			compatible = "ti,sysc-omap4", "ti,sysc";
 			ti,hwmods = "d_can0";
-			reg = <0xcc000 0x4>;
-			reg-names = "rev";
 			/* Domains (P, C): per_pwrdm, l4ls_clkdm */
 			clocks = <&l4ls_clkctrl AM3_L4LS_D_CAN0_CLKCTRL 0>,
 				 <&dcan0_fck>;
@@ -1783,8 +1781,6 @@
 		target-module@d0000 {			/* 0x481d0000, ap 62 42.0 */
 			compatible = "ti,sysc-omap4", "ti,sysc";
 			ti,hwmods = "d_can1";
-			reg = <0xd0000 0x4>;
-			reg-names = "rev";
 			/* Domains (P, C): per_pwrdm, l4ls_clkdm */
 			clocks = <&l4ls_clkctrl AM3_L4LS_D_CAN1_CLKCTRL 0>,
 				 <&dcan1_fck>;
diff --git a/arch/arm/boot/dts/am437x-l4.dtsi b/arch/arm/boot/dts/am437x-l4.dtsi
--- a/arch/arm/boot/dts/am437x-l4.dtsi
+++ b/arch/arm/boot/dts/am437x-l4.dtsi
@@ -1575,8 +1575,6 @@
 		target-module@cc000 {			/* 0x481cc000, ap 50 46.0 */
 			compatible = "ti,sysc-omap4", "ti,sysc";
 			ti,hwmods = "d_can0";
-			reg = <0xcc000 0x4>;
-			reg-names = "rev";
 			/* Domains (P, C): per_pwrdm, l4ls_clkdm */
 			clocks = <&l4ls_clkctrl AM4_L4LS_D_CAN0_CLKCTRL 0>;
 			clock-names = "fck";
@@ -1596,8 +1594,6 @@
 		target-module@d0000 {			/* 0x481d0000, ap 52 3a.0 */
 			compatible = "ti,sysc-omap4", "ti,sysc";
 			ti,hwmods = "d_can1";
-			reg = <0xd0000 0x4>;
-			reg-names = "rev";
 			/* Domains (P, C): per_pwrdm, l4ls_clkdm */
 			clocks = <&l4ls_clkctrl AM4_L4LS_D_CAN1_CLKCTRL 0>;
 			clock-names = "fck";
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -660,12 +660,6 @@ static int sysc_check_registers(struct sysc *ddata)
 		nr_regs++;
 	}
 
-	if (nr_regs < 1) {
-		dev_err(ddata->dev, "missing registers\n");
-
-		return -EINVAL;
-	}
-
 	if (nr_matches > nr_regs) {
 		dev_err(ddata->dev, "overlapping registers: (%i/%i)",
 			nr_regs, nr_matches);
@@ -691,12 +685,18 @@ static int sysc_ioremap(struct sysc *ddata)
 {
 	int size;
 
-	size = max3(ddata->offsets[SYSC_REVISION],
-		    ddata->offsets[SYSC_SYSCONFIG],
-		    ddata->offsets[SYSC_SYSSTATUS]);
+	if (ddata->offsets[SYSC_REVISION] < 0 &&
+	    ddata->offsets[SYSC_SYSCONFIG] < 0 &&
+	    ddata->offsets[SYSC_SYSSTATUS] < 0) {
+		size = ddata->module_size;
+	} else {
+		size = max3(ddata->offsets[SYSC_REVISION],
+			    ddata->offsets[SYSC_SYSCONFIG],
+			    ddata->offsets[SYSC_SYSSTATUS]);
 
-	if (size < 0 || (size + sizeof(u32)) > ddata->module_size)
-		return -EINVAL;
+		if ((size + sizeof(u32)) > ddata->module_size)
+			return -EINVAL;
+	}
 
 	ddata->module_va = devm_ioremap(ddata->dev,
 					ddata->module_pa,
@@ -1128,7 +1128,6 @@ static const struct sysc_revision_quirk sysc_revision_quirks[] = {
 	SYSC_QUIRK("cpgmac", 0, 0x1200, 0x1208, 0x1204, 0x4edb1902,
 		   0xffff00f0, 0),
 	SYSC_QUIRK("dcan", 0, 0, -1, -1, 0xffffffff, 0xffffffff, 0),
-	SYSC_QUIRK("dcan", 0, 0, -1, -1, 0x00001401, 0xffffffff, 0),
 	SYSC_QUIRK("dmic", 0, 0, 0x10, -1, 0x50010000, 0xffffffff, 0),
 	SYSC_QUIRK("dwc3", 0, 0, 0x10, -1, 0x500a0200, 0xffffffff, 0),
 	SYSC_QUIRK("epwmss", 0, 0, 0x4, -1, 0x47400001, 0xffffffff, 0),
-- 
2.21.0

Re: [PATCH 0/2] Two ti-sysc driver fixes for v5.3 merge window

[Keerthy]

On 02/05/19 3:11 AM, Tony Lindgren wrote:
> Hi all,
> 
> Here are few fixes for the am335x d_can boot issue Sebastian reported for
> Beaglebone.

Tested for AM437x-gp-evm RTC+DDR mode and DS0.
Also tried DS0 on Am335x beaglebone black.

For the above:

Tested-by: Keerthy <j-keerthy@ti.com>

> 
> Regards,
> 
> Tony
> 
> 
> Tony Lindgren (2):
>    ARM: dts: Configure osc clock for d_can on am335x
>    bus: ti-sysc: Handle devices with no control registers
> 
>   arch/arm/boot/dts/am33xx-l4.dtsi | 14 ++++++--------
>   arch/arm/boot/dts/am437x-l4.dtsi |  4 ----
>   drivers/bus/ti-sysc.c            | 23 +++++++++++------------
>   3 files changed, 17 insertions(+), 24 deletions(-)
>