[PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Geert Uytterhoeven]

	Hi all,

The Renesas Bus State Controller (BSC) provides an external bus for
connecting multiple external devices to an SoC, driving several chip
select lines, for e.g. NOR FLASH, Ethernet and USB.
On the kzm9g development board, an smsc9220 Ethernet controller is
connnected to the BSC of the SH-Mobile AG5 (sh73a0) SoC.

The BSC is a fairly simple memory-mapped bus, hence a "simple-bus"
compatibility seems suitable.  However, the BSC is special in two ways:
  1. It is part of a PM domain (A4S),
  2. It has a gateable functional clock (ZB).
Before a device connected to the BSC can be accessed, the PM domain
containing the BSC must be powered on, and the functional clock driving
the BSC must be enabled.

Both special properties can be described in DT in a standardized way
("power-domains = <&pd_a4s>" and "clocks = <&zb_clk>", cfr. patch 1).
Externally connected devices are described as children of the BSC node
(cfr. patch 2).

Unfortunately this doesn't mean everything will work out-of-the-box.
There are two problems:
  1. Without a device driver bound to the BSC device, this device is not
     attached to the PM domain. And although a child device is present
     and active, the PM domain may be powered down, as it's considered
     unused by the PM domain core.
  2. Without a device driver calling pm_runtime_enable(), its functional
     clock is not enabled. Once runtime PM is enabled, the R-Mobile PM
     domain platform driver manages the functional clock using runtime
     PM.

Hence patch 3 adds a very simple BSC driver, matching against
"renesas-bsc", and enabling runtime PM for the BSC device.

Due to the child-parent relationship of devices connected to the BSC, as
long as the device drivers for the child devices are runtime PM enabled
(see dependency 1), the BSC's PM domain will be powered, and the BSC's
clock will be enabled automatically when needed.
If any child device driver lacks runtime PM support, the BSC driver also
has to call pm_runtime_get_sync() in its .probe() function.
System suspend and resume (s2ram) also works fine.

As this minimal BSC driver isn't hardware-specific at all, I'm wondering
if there's a simpler way to do this?
  - Should the driver be renamed to "simple-bus", and match "simple-bus"?
  - Should this be moved to core code, without an explicit driver for
    "simple-bus"? I.e. should the driver core just enable runtime PM for
    all devices not bound to a driver, as they may represent buses with
    child devices that do rely on runtime PM?

Thanks for your comments and suggestions!

Dependencies:
  1. [PATCH] net/smsc911x: Add minimal runtime PM support
     (https://lkml.org/lkml/2014/11/24/610),
  2. [PATCH] clk: shmobile: div6: Avoid changing divisor in .disable()
     (http://marc.info/?l=linux-sh&m\x141684607012727&w=2),
  3. [PATCH v6 0/6] sh73a0 common clock framework implementation
     (http://marc.info/?l=linux-sh&m\x141650048308960&w=2),
  4. sh73a0 DT PM domain support (not yet posted),
  5. Alphabetical sorting of drivers/bus/{Kconfig,Makefile) (not yet
     posted).

References:
  1. [PATCH v3 7/7] ARM: shmobile: kzm9g-reference: require ZB clock for
     Ethernet controller (http://www.spinics.net/lists/linux-sh/msg36601.html)
     is an earlier workaround for getting the functional clock enabled, by
     letting the smc911x driver handle it. This predates sh73a0 PM domain
     support.

Patches:
  1. ARM: shmobile: sh73a0 dtsi: Add Bus State Controller node
  2. ARM: shmobile: kzm9g-reference dts: Move Ethernet node to BSC
  3. drivers: bus: Add Renesas Bus State Controller Driver

 arch/arm/boot/dts/sh73a0-kzm9g.dts | 28 +++++++++++----------
 arch/arm/boot/dts/sh73a0.dtsi      | 11 ++++++++
 drivers/bus/Kconfig                |  8 ++++++
 drivers/bus/Makefile               |  1 +
 drivers/bus/renesas-bsc.c          | 51 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 86 insertions(+), 13 deletions(-)
 create mode 100644 drivers/bus/renesas-bsc.c

-- 
1.9.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Geert Uytterhoeven]

	Hi all,

The Renesas Bus State Controller (BSC) provides an external bus for
connecting multiple external devices to an SoC, driving several chip
select lines, for e.g. NOR FLASH, Ethernet and USB.
On the kzm9g development board, an smsc9220 Ethernet controller is
connnected to the BSC of the SH-Mobile AG5 (sh73a0) SoC.

The BSC is a fairly simple memory-mapped bus, hence a "simple-bus"
compatibility seems suitable.  However, the BSC is special in two ways:
  1. It is part of a PM domain (A4S),
  2. It has a gateable functional clock (ZB).
Before a device connected to the BSC can be accessed, the PM domain
containing the BSC must be powered on, and the functional clock driving
the BSC must be enabled.

Both special properties can be described in DT in a standardized way
("power-domains = <&pd_a4s>" and "clocks = <&zb_clk>", cfr. patch 1).
Externally connected devices are described as children of the BSC node
(cfr. patch 2).

Unfortunately this doesn't mean everything will work out-of-the-box.
There are two problems:
  1. Without a device driver bound to the BSC device, this device is not
     attached to the PM domain. And although a child device is present
     and active, the PM domain may be powered down, as it's considered
     unused by the PM domain core.
  2. Without a device driver calling pm_runtime_enable(), its functional
     clock is not enabled. Once runtime PM is enabled, the R-Mobile PM
     domain platform driver manages the functional clock using runtime
     PM.

Hence patch 3 adds a very simple BSC driver, matching against
"renesas-bsc", and enabling runtime PM for the BSC device.

Due to the child-parent relationship of devices connected to the BSC, as
long as the device drivers for the child devices are runtime PM enabled
(see dependency 1), the BSC's PM domain will be powered, and the BSC's
clock will be enabled automatically when needed.
If any child device driver lacks runtime PM support, the BSC driver also
has to call pm_runtime_get_sync() in its .probe() function.
System suspend and resume (s2ram) also works fine.

As this minimal BSC driver isn't hardware-specific at all, I'm wondering
if there's a simpler way to do this?
  - Should the driver be renamed to "simple-bus", and match "simple-bus"?
  - Should this be moved to core code, without an explicit driver for
    "simple-bus"? I.e. should the driver core just enable runtime PM for
    all devices not bound to a driver, as they may represent buses with
    child devices that do rely on runtime PM?

Thanks for your comments and suggestions!

Dependencies:
  1. [PATCH] net/smsc911x: Add minimal runtime PM support
     (https://lkml.org/lkml/2014/11/24/610),
  2. [PATCH] clk: shmobile: div6: Avoid changing divisor in .disable()
     (http://marc.info/?l=linux-sh&m=141684607012727&w=2),
  3. [PATCH v6 0/6] sh73a0 common clock framework implementation
     (http://marc.info/?l=linux-sh&m=141650048308960&w=2),
  4. sh73a0 DT PM domain support (not yet posted),
  5. Alphabetical sorting of drivers/bus/{Kconfig,Makefile) (not yet
     posted).

References:
  1. [PATCH v3 7/7] ARM: shmobile: kzm9g-reference: require ZB clock for
     Ethernet controller (http://www.spinics.net/lists/linux-sh/msg36601.html)
     is an earlier workaround for getting the functional clock enabled, by
     letting the smc911x driver handle it. This predates sh73a0 PM domain
     support.

Patches:
  1. ARM: shmobile: sh73a0 dtsi: Add Bus State Controller node
  2. ARM: shmobile: kzm9g-reference dts: Move Ethernet node to BSC
  3. drivers: bus: Add Renesas Bus State Controller Driver

 arch/arm/boot/dts/sh73a0-kzm9g.dts | 28 +++++++++++----------
 arch/arm/boot/dts/sh73a0.dtsi      | 11 ++++++++
 drivers/bus/Kconfig                |  8 ++++++
 drivers/bus/Makefile               |  1 +
 drivers/bus/renesas-bsc.c          | 51 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 86 insertions(+), 13 deletions(-)
 create mode 100644 drivers/bus/renesas-bsc.c

-- 
1.9.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH/RFC 1/3] ARM: shmobile: sh73a0 dtsi: Add Bus State Controller node

[Geert Uytterhoeven]

Add a node for the Bus State Controller (BSC) on sh73a0, to which
multiple external devices can be connected.

The BSC is driven by the ZB clock, and located in PM domain A4S.

For now no device tree bindings have been defined yet. All of its
properties follow standard conventions.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/sh73a0.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/sh73a0.dtsi b/arch/arm/boot/dts/sh73a0.dtsi
index 87e7d5ef904bde89..b8d4c557c85eca0c 100644
--- a/arch/arm/boot/dts/sh73a0.dtsi
+++ b/arch/arm/boot/dts/sh73a0.dtsi
@@ -997,4 +997,15 @@
 				"intca0";
 		};
 	};
+
+	bsc: bus@fec10000 {
+		compatible = "renesas,bsc-sh73a0", "renesas,bsc", "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0xfec10000 0x400>;
+		ranges = <0 0 0x20000000>;
+		interrupts = <0 39 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&zb_clk>;
+		power-domains = <&pd_a4s>;
+	};
 };
-- 
1.9.1

[PATCH/RFC 1/3] ARM: shmobile: sh73a0 dtsi: Add Bus State Controller node

[Geert Uytterhoeven]

Add a node for the Bus State Controller (BSC) on sh73a0, to which
multiple external devices can be connected.

The BSC is driven by the ZB clock, and located in PM domain A4S.

For now no device tree bindings have been defined yet. All of its
properties follow standard conventions.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/sh73a0.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/sh73a0.dtsi b/arch/arm/boot/dts/sh73a0.dtsi
index 87e7d5ef904bde89..b8d4c557c85eca0c 100644
--- a/arch/arm/boot/dts/sh73a0.dtsi
+++ b/arch/arm/boot/dts/sh73a0.dtsi
@@ -997,4 +997,15 @@
 				"intca0";
 		};
 	};
+
+	bsc: bus@fec10000 {
+		compatible = "renesas,bsc-sh73a0", "renesas,bsc", "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0xfec10000 0x400>;
+		ranges = <0 0 0x20000000>;
+		interrupts = <0 39 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&zb_clk>;
+		power-domains = <&pd_a4s>;
+	};
 };
-- 
1.9.1

[PATCH/RFC 2/3] ARM: shmobile: kzm9g-reference dts: Move Ethernet node to BSC

[Geert Uytterhoeven]

Move the Ethernet node from the root of the device tree to the BSC node,
as it is connected to the Bus State Controller.  This allows the system
to know the right position of the Ethernet node in the clock and PM
domain hierarchy.

Also rename the node's name from "lan9220" to "ethernet", to confirm to
ePAPR generic name recomendations.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/sh73a0-kzm9g.dts | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/arch/arm/boot/dts/sh73a0-kzm9g.dts b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
index 45e2bc60ba9676ff..f80d411696da6b40 100644
--- a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
+++ b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
@@ -84,19 +84,6 @@
 		enable-active-high;
 	};
 
-	lan9220@10000000 {
-		compatible = "smsc,lan9220", "smsc,lan9115";
-		reg = <0x10000000 0x100>;
-		phy-mode = "mii";
-		interrupt-parent = <&irqpin0>;
-		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
-		reg-io-width = <4>;
-		smsc,irq-push-pull;
-		smsc,save-mac-address;
-		vddvario-supply = <&reg_1p8v>;
-		vdd33a-supply = <&reg_3p3v>;
-	};
-
 	leds {
 		compatible = "gpio-leds";
 		led1 {
@@ -179,6 +166,21 @@
 	};
 };
 
+&bsc {
+	ethernet@10000000 {
+		compatible = "smsc,lan9220", "smsc,lan9115";
+		reg = <0x10000000 0x100>;
+		phy-mode = "mii";
+		interrupt-parent = <&irqpin0>;
+		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+		reg-io-width = <4>;
+		smsc,irq-push-pull;
+		smsc,save-mac-address;
+		vddvario-supply = <&reg_1p8v>;
+		vdd33a-supply = <&reg_3p3v>;
+	};
+};
+
 &cmt1 {
 	status = "ok";
 };
-- 
1.9.1

[PATCH/RFC 2/3] ARM: shmobile: kzm9g-reference dts: Move Ethernet node to BSC

[Geert Uytterhoeven]

Move the Ethernet node from the root of the device tree to the BSC node,
as it is connected to the Bus State Controller.  This allows the system
to know the right position of the Ethernet node in the clock and PM
domain hierarchy.

Also rename the node's name from "lan9220" to "ethernet", to confirm to
ePAPR generic name recomendations.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/sh73a0-kzm9g.dts | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/arch/arm/boot/dts/sh73a0-kzm9g.dts b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
index 45e2bc60ba9676ff..f80d411696da6b40 100644
--- a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
+++ b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
@@ -84,19 +84,6 @@
 		enable-active-high;
 	};
 
-	lan9220@10000000 {
-		compatible = "smsc,lan9220", "smsc,lan9115";
-		reg = <0x10000000 0x100>;
-		phy-mode = "mii";
-		interrupt-parent = <&irqpin0>;
-		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
-		reg-io-width = <4>;
-		smsc,irq-push-pull;
-		smsc,save-mac-address;
-		vddvario-supply = <&reg_1p8v>;
-		vdd33a-supply = <&reg_3p3v>;
-	};
-
 	leds {
 		compatible = "gpio-leds";
 		led1 {
@@ -179,6 +166,21 @@
 	};
 };
 
+&bsc {
+	ethernet@10000000 {
+		compatible = "smsc,lan9220", "smsc,lan9115";
+		reg = <0x10000000 0x100>;
+		phy-mode = "mii";
+		interrupt-parent = <&irqpin0>;
+		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+		reg-io-width = <4>;
+		smsc,irq-push-pull;
+		smsc,save-mac-address;
+		vddvario-supply = <&reg_1p8v>;
+		vdd33a-supply = <&reg_3p3v>;
+	};
+};
+
 &cmt1 {
 	status = "ok";
 };
-- 
1.9.1

Re: [PATCH/RFC 2/3] ARM: shmobile: kzm9g-reference dts: Move Ethernet node to BSC

[Sergei Shtylyov]

Hello.

On 11/24/2014 11:10 PM, Geert Uytterhoeven wrote:

> Move the Ethernet node from the root of the device tree to the BSC node,
> as it is connected to the Bus State Controller.  This allows the system
> to know the right position of the Ethernet node in the clock and PM
> domain hierarchy.

> Also rename the node's name from "lan9220" to "ethernet", to confirm to

    s/confirm/comply/, you mean?

> ePAPR generic name recomendations.

    Thank you, I've never found the time to rename those device nodes.

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

WBR, Sergei

Re: [PATCH/RFC 2/3] ARM: shmobile: kzm9g-reference dts: Move Ethernet node to BSC

[Sergei Shtylyov]

Hello.

On 11/24/2014 11:10 PM, Geert Uytterhoeven wrote:

> Move the Ethernet node from the root of the device tree to the BSC node,
> as it is connected to the Bus State Controller.  This allows the system
> to know the right position of the Ethernet node in the clock and PM
> domain hierarchy.

> Also rename the node's name from "lan9220" to "ethernet", to confirm to

    s/confirm/comply/, you mean?

> ePAPR generic name recomendations.

    Thank you, I've never found the time to rename those device nodes.

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

WBR, Sergei

Re: [PATCH/RFC 2/3] ARM: shmobile: kzm9g-reference dts: Move Ethernet node to BSC

[Geert Uytterhoeven]

On Mon, Nov 24, 2014 at 10:11 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
>> Also rename the node's name from "lan9220" to "ethernet", to confirm to
>
>    s/confirm/comply/, you mean?

Sorry, conform.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH/RFC 2/3] ARM: shmobile: kzm9g-reference dts: Move Ethernet node to BSC

[Geert Uytterhoeven]

On Mon, Nov 24, 2014 at 10:11 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
>> Also rename the node's name from "lan9220" to "ethernet", to confirm to
>
>    s/confirm/comply/, you mean?

Sorry, conform.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH/RFC 3/3] drivers: bus: Add Renesas Bus State Controller Driver

[Geert Uytterhoeven]

Add a driver for the Renesas Bus State Controller (BSC, sometimes called
"LBSC within Bus Bridge", or "External Bus Interface") found on several
Renesas ARM SoCs.

The sole purpose of this driver is to enable its clock and PM domain (if
exist(s)), which are specified in the DT and managed from platform and
PM domain code.  The BSC clock and PM domain must be enabled for
external devices connected to the BSC (e.g. NOR FLASH or Ethernet) to
work.

Due to the child-parent relationship with devices connected to the BSC,
PM domain and clock state transitions are handled in the correct order.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
  - Is there a simpler way to do this?
  - As none of this code is really hardware-specific, should the driver
    be renamed to simple-bus, and match against "simple-bus"?
  - Should this be moved to core code, without an explicit driver for
    "simple-bus"?
Please reply to the cover letter!

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/bus/Kconfig       |  8 ++++++++
 drivers/bus/Makefile      |  1 +
 drivers/bus/renesas-bsc.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+)
 create mode 100644 drivers/bus/renesas-bsc.c

diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 7697fc8903f62d07..7e901480ba67dcad 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -58,6 +58,14 @@ config OMAP_OCP2SCP
 	  OCP2SCP and in OMAP5, both USB PHY and SATA PHY is connected via
 	  OCP2SCP.
 
+config RENESAS_BSC
+	bool "Renesas Bus State Controller"
+	depends on ARCH_SHMOBILE && OF
+	help
+	  Driver for the Renesas Bus State Controller (BSC, sometimes called
+	  "LBSC within Bus Bridge", or "External Bus Interface") found on
+	  several Renesas ARM SoCs.
+
 config VEXPRESS_CONFIG
 	bool "Versatile Express configuration bus"
 	default y if ARCH_VEXPRESS
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index 2eb1f668c7fbc00d..b1ac67cfa9d5663c 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -14,4 +14,5 @@ obj-$(CONFIG_MVEBU_MBUS) += mvebu-mbus.o
 obj-$(CONFIG_OMAP_INTERCONNECT)	+= omap_l3_smx.o omap_l3_noc.o
 
 obj-$(CONFIG_OMAP_OCP2SCP)	+= omap-ocp2scp.o
+obj-$(CONFIG_RENESAS_BSC)	+= renesas-bsc.o
 obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o
diff --git a/drivers/bus/renesas-bsc.c b/drivers/bus/renesas-bsc.c
new file mode 100644
index 0000000000000000..ca35aae266d3365a
--- /dev/null
+++ b/drivers/bus/renesas-bsc.c
@@ -0,0 +1,51 @@
+/*
+ * Renesas Bus State Controller Driver
+ *
+ * Copyright (C) 2014 Glider bvba
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+
+static int renesas_bsc_probe(struct platform_device *pdev)
+{
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+
+	pm_runtime_enable(&pdev->dev);
+	return 0;
+}
+
+static int renesas_bsc_remove(struct platform_device *pdev)
+{
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+
+	pm_runtime_disable(&pdev->dev);
+	return 0;
+}
+
+static const struct of_device_id renesas_bsc_of_match[] = {
+	{ .compatible = "renesas,bsc", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, renesas_bsc_of_match);
+
+static struct platform_driver renesas_bsc_driver = {
+	.probe = renesas_bsc_probe,
+	.remove = renesas_bsc_remove,
+	.driver = {
+		.name = "renesas-bsc",
+		.of_match_table = renesas_bsc_of_match,
+	},
+};
+
+module_platform_driver(renesas_bsc_driver);
+
+MODULE_DESCRIPTION("Renesas Bus State Controller Driver");
+MODULE_AUTHOR("Geert Uytterhoeven <geert@linux-m68k.org>");
+MODULE_LICENSE("GPL");
-- 
1.9.1

[PATCH/RFC 3/3] drivers: bus: Add Renesas Bus State Controller Driver

[Geert Uytterhoeven]

Add a driver for the Renesas Bus State Controller (BSC, sometimes called
"LBSC within Bus Bridge", or "External Bus Interface") found on several
Renesas ARM SoCs.

The sole purpose of this driver is to enable its clock and PM domain (if
exist(s)), which are specified in the DT and managed from platform and
PM domain code.  The BSC clock and PM domain must be enabled for
external devices connected to the BSC (e.g. NOR FLASH or Ethernet) to
work.

Due to the child-parent relationship with devices connected to the BSC,
PM domain and clock state transitions are handled in the correct order.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
  - Is there a simpler way to do this?
  - As none of this code is really hardware-specific, should the driver
    be renamed to simple-bus, and match against "simple-bus"?
  - Should this be moved to core code, without an explicit driver for
    "simple-bus"?
Please reply to the cover letter!

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/bus/Kconfig       |  8 ++++++++
 drivers/bus/Makefile      |  1 +
 drivers/bus/renesas-bsc.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+)
 create mode 100644 drivers/bus/renesas-bsc.c

diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 7697fc8903f62d07..7e901480ba67dcad 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -58,6 +58,14 @@ config OMAP_OCP2SCP
 	  OCP2SCP and in OMAP5, both USB PHY and SATA PHY is connected via
 	  OCP2SCP.
 
+config RENESAS_BSC
+	bool "Renesas Bus State Controller"
+	depends on ARCH_SHMOBILE && OF
+	help
+	  Driver for the Renesas Bus State Controller (BSC, sometimes called
+	  "LBSC within Bus Bridge", or "External Bus Interface") found on
+	  several Renesas ARM SoCs.
+
 config VEXPRESS_CONFIG
 	bool "Versatile Express configuration bus"
 	default y if ARCH_VEXPRESS
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index 2eb1f668c7fbc00d..b1ac67cfa9d5663c 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -14,4 +14,5 @@ obj-$(CONFIG_MVEBU_MBUS) += mvebu-mbus.o
 obj-$(CONFIG_OMAP_INTERCONNECT)	+= omap_l3_smx.o omap_l3_noc.o
 
 obj-$(CONFIG_OMAP_OCP2SCP)	+= omap-ocp2scp.o
+obj-$(CONFIG_RENESAS_BSC)	+= renesas-bsc.o
 obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o
diff --git a/drivers/bus/renesas-bsc.c b/drivers/bus/renesas-bsc.c
new file mode 100644
index 0000000000000000..ca35aae266d3365a
--- /dev/null
+++ b/drivers/bus/renesas-bsc.c
@@ -0,0 +1,51 @@
+/*
+ * Renesas Bus State Controller Driver
+ *
+ * Copyright (C) 2014 Glider bvba
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+
+static int renesas_bsc_probe(struct platform_device *pdev)
+{
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+
+	pm_runtime_enable(&pdev->dev);
+	return 0;
+}
+
+static int renesas_bsc_remove(struct platform_device *pdev)
+{
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+
+	pm_runtime_disable(&pdev->dev);
+	return 0;
+}
+
+static const struct of_device_id renesas_bsc_of_match[] = {
+	{ .compatible = "renesas,bsc", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, renesas_bsc_of_match);
+
+static struct platform_driver renesas_bsc_driver = {
+	.probe = renesas_bsc_probe,
+	.remove = renesas_bsc_remove,
+	.driver = {
+		.name = "renesas-bsc",
+		.of_match_table = renesas_bsc_of_match,
+	},
+};
+
+module_platform_driver(renesas_bsc_driver);
+
+MODULE_DESCRIPTION("Renesas Bus State Controller Driver");
+MODULE_AUTHOR("Geert Uytterhoeven <geert@linux-m68k.org>");
+MODULE_LICENSE("GPL");
-- 
1.9.1

Re: [PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Arnd Bergmann]

On Monday 24 November 2014 21:10:05 Geert Uytterhoeven wrote:
> As this minimal BSC driver isn't hardware-specific at all, I'm wondering
> if there's a simpler way to do this?
>   - Should the driver be renamed to "simple-bus", and match "simple-bus"?
>   - Should this be moved to core code, without an explicit driver for
>     "simple-bus"? I.e. should the driver core just enable runtime PM for
>     all devices not bound to a driver, as they may represent buses with
>     child devices that do rely on runtime PM?
> 
> Thanks for your comments and suggestions!

My understanding of simple-bus is that it's something that does
not have any power-management capabilities, and I'd rather not
add clocks or interrupts to it.

What I think makes more sense is to have a bus driver for it
in drivers/bus, remove the "simple-bus" compatibility value
and have the driver take care of registering the power domain
and probing the child devices using of_platform_populate on
itself.

	Arnd

Re: [PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Arnd Bergmann]

On Monday 24 November 2014 21:10:05 Geert Uytterhoeven wrote:
> As this minimal BSC driver isn't hardware-specific at all, I'm wondering
> if there's a simpler way to do this?
>   - Should the driver be renamed to "simple-bus", and match "simple-bus"?
>   - Should this be moved to core code, without an explicit driver for
>     "simple-bus"? I.e. should the driver core just enable runtime PM for
>     all devices not bound to a driver, as they may represent buses with
>     child devices that do rely on runtime PM?
> 
> Thanks for your comments and suggestions!

My understanding of simple-bus is that it's something that does
not have any power-management capabilities, and I'd rather not
add clocks or interrupts to it.

What I think makes more sense is to have a bus driver for it
in drivers/bus, remove the "simple-bus" compatibility value
and have the driver take care of registering the power domain
and probing the child devices using of_platform_populate on
itself.

	Arnd

Re: [PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Geert Uytterhoeven]

Hi Arnd,

On Mon, Nov 24, 2014 at 9:15 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 24 November 2014 21:10:05 Geert Uytterhoeven wrote:
>> As this minimal BSC driver isn't hardware-specific at all, I'm wondering
>> if there's a simpler way to do this?
>>   - Should the driver be renamed to "simple-bus", and match "simple-bus"?
>>   - Should this be moved to core code, without an explicit driver for
>>     "simple-bus"? I.e. should the driver core just enable runtime PM for
>>     all devices not bound to a driver, as they may represent buses with
>>     child devices that do rely on runtime PM?
>>
>> Thanks for your comments and suggestions!
>
> My understanding of simple-bus is that it's something that does
> not have any power-management capabilities, and I'd rather not
> add clocks or interrupts to it.

OK.

> What I think makes more sense is to have a bus driver for it
> in drivers/bus, remove the "simple-bus" compatibility value
> and have the driver take care of registering the power domain
> and probing the child devices using of_platform_populate on
> itself.

Registering power domains is already handled by the core code.

So compared to my RFC code, I only have to
  1. Drop "simple-bus" from the compatible property in the .dtsi,
  2. Call of_platform_populate() from renesas_bsc_probe(), to
     register the child devices, now the core code no longer does that.

I find it a bit strange having to add _more_ code, as the core code handles
registering child devices fine. Doing it from my bus driver only protects
against people trying to run a kernel without my bus driver included
(which currently works fine, as long as no PM domain or clock is involved,
 e.g. if the clock is forgotten in the SoC's .dtsi :-).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Geert Uytterhoeven]

Hi Arnd,

On Mon, Nov 24, 2014 at 9:15 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 24 November 2014 21:10:05 Geert Uytterhoeven wrote:
>> As this minimal BSC driver isn't hardware-specific at all, I'm wondering
>> if there's a simpler way to do this?
>>   - Should the driver be renamed to "simple-bus", and match "simple-bus"?
>>   - Should this be moved to core code, without an explicit driver for
>>     "simple-bus"? I.e. should the driver core just enable runtime PM for
>>     all devices not bound to a driver, as they may represent buses with
>>     child devices that do rely on runtime PM?
>>
>> Thanks for your comments and suggestions!
>
> My understanding of simple-bus is that it's something that does
> not have any power-management capabilities, and I'd rather not
> add clocks or interrupts to it.

OK.

> What I think makes more sense is to have a bus driver for it
> in drivers/bus, remove the "simple-bus" compatibility value
> and have the driver take care of registering the power domain
> and probing the child devices using of_platform_populate on
> itself.

Registering power domains is already handled by the core code.

So compared to my RFC code, I only have to
  1. Drop "simple-bus" from the compatible property in the .dtsi,
  2. Call of_platform_populate() from renesas_bsc_probe(), to
     register the child devices, now the core code no longer does that.

I find it a bit strange having to add _more_ code, as the core code handles
registering child devices fine. Doing it from my bus driver only protects
against people trying to run a kernel without my bus driver included
(which currently works fine, as long as no PM domain or clock is involved,
 e.g. if the clock is forgotten in the SoC's .dtsi :-).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Kevin Hilman]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Arnd,
>
> On Mon, Nov 24, 2014 at 9:15 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Monday 24 November 2014 21:10:05 Geert Uytterhoeven wrote:
>>> As this minimal BSC driver isn't hardware-specific at all, I'm wondering
>>> if there's a simpler way to do this?
>>>   - Should the driver be renamed to "simple-bus", and match "simple-bus"?
>>>   - Should this be moved to core code, without an explicit driver for
>>>     "simple-bus"? I.e. should the driver core just enable runtime PM for
>>>     all devices not bound to a driver, as they may represent buses with
>>>     child devices that do rely on runtime PM?
>>>
>>> Thanks for your comments and suggestions!
>>
>> My understanding of simple-bus is that it's something that does
>> not have any power-management capabilities, and I'd rather not
>> add clocks or interrupts to it.
>
> OK.
>
>> What I think makes more sense is to have a bus driver for it
>> in drivers/bus, remove the "simple-bus" compatibility value
>> and have the driver take care of registering the power domain
>> and probing the child devices using of_platform_populate on
>> itself.
>
> Registering power domains is already handled by the core code.
>
> So compared to my RFC code, I only have to
>   1. Drop "simple-bus" from the compatible property in the .dtsi,
>   2. Call of_platform_populate() from renesas_bsc_probe(), to
>      register the child devices, now the core code no longer does that.
>
> I find it a bit strange having to add _more_ code, as the core code handles
> registering child devices fine. Doing it from my bus driver only protects
> against people trying to run a kernel without my bus driver included
> (which currently works fine, as long as no PM domain or clock is involved,
>  e.g. if the clock is forgotten in the SoC's .dtsi :-).

I actually rather like the simple bus driver in it's current form, where
the device and pm-domain registration is taken care of by the
core. Since it's really not HW specific at all, maybe just rename it
simple-pm-bus or something like that?

Kevin

Re: [PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Kevin Hilman]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Arnd,
>
> On Mon, Nov 24, 2014 at 9:15 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Monday 24 November 2014 21:10:05 Geert Uytterhoeven wrote:
>>> As this minimal BSC driver isn't hardware-specific at all, I'm wondering
>>> if there's a simpler way to do this?
>>>   - Should the driver be renamed to "simple-bus", and match "simple-bus"?
>>>   - Should this be moved to core code, without an explicit driver for
>>>     "simple-bus"? I.e. should the driver core just enable runtime PM for
>>>     all devices not bound to a driver, as they may represent buses with
>>>     child devices that do rely on runtime PM?
>>>
>>> Thanks for your comments and suggestions!
>>
>> My understanding of simple-bus is that it's something that does
>> not have any power-management capabilities, and I'd rather not
>> add clocks or interrupts to it.
>
> OK.
>
>> What I think makes more sense is to have a bus driver for it
>> in drivers/bus, remove the "simple-bus" compatibility value
>> and have the driver take care of registering the power domain
>> and probing the child devices using of_platform_populate on
>> itself.
>
> Registering power domains is already handled by the core code.
>
> So compared to my RFC code, I only have to
>   1. Drop "simple-bus" from the compatible property in the .dtsi,
>   2. Call of_platform_populate() from renesas_bsc_probe(), to
>      register the child devices, now the core code no longer does that.
>
> I find it a bit strange having to add _more_ code, as the core code handles
> registering child devices fine. Doing it from my bus driver only protects
> against people trying to run a kernel without my bus driver included
> (which currently works fine, as long as no PM domain or clock is involved,
>  e.g. if the clock is forgotten in the SoC's .dtsi :-).

I actually rather like the simple bus driver in it's current form, where
the device and pm-domain registration is taken care of by the
core. Since it's really not HW specific at all, maybe just rename it
simple-pm-bus or something like that?

Kevin

Re: [PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Kevin Hilman]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Arnd,
>
> On Mon, Nov 24, 2014 at 9:15 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Monday 24 November 2014 21:10:05 Geert Uytterhoeven wrote:
>>> As this minimal BSC driver isn't hardware-specific at all, I'm wondering
>>> if there's a simpler way to do this?
>>>   - Should the driver be renamed to "simple-bus", and match "simple-bus"?
>>>   - Should this be moved to core code, without an explicit driver for
>>>     "simple-bus"? I.e. should the driver core just enable runtime PM for
>>>     all devices not bound to a driver, as they may represent buses with
>>>     child devices that do rely on runtime PM?
>>>
>>> Thanks for your comments and suggestions!
>>
>> My understanding of simple-bus is that it's something that does
>> not have any power-management capabilities, and I'd rather not
>> add clocks or interrupts to it.
>
> OK.
>
>> What I think makes more sense is to have a bus driver for it
>> in drivers/bus, remove the "simple-bus" compatibility value
>> and have the driver take care of registering the power domain
>> and probing the child devices using of_platform_populate on
>> itself.
>
> Registering power domains is already handled by the core code.
>
> So compared to my RFC code, I only have to
>   1. Drop "simple-bus" from the compatible property in the .dtsi,
>   2. Call of_platform_populate() from renesas_bsc_probe(), to
>      register the child devices, now the core code no longer does that.
>
> I find it a bit strange having to add _more_ code, as the core code handles
> registering child devices fine. Doing it from my bus driver only protects
> against people trying to run a kernel without my bus driver included
> (which currently works fine, as long as no PM domain or clock is involved,
>  e.g. if the clock is forgotten in the SoC's .dtsi :-).

I actually rather like the simple bus driver in it's current form, where
the device and pm-domain registration is taken care of by the
core. Since it's really not HW specific at all, maybe just rename it
simple-pm-bus or something like that?

Kevin

Re: [PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Geert Uytterhoeven]

Hi Kevin, Arnd,

I've been giving this some more thoughts...

On Wed, Nov 26, 2014 at 9:11 PM, Kevin Hilman <khilman@kernel.org> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>> On Mon, Nov 24, 2014 at 9:15 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Monday 24 November 2014 21:10:05 Geert Uytterhoeven wrote:
>>>> As this minimal BSC driver isn't hardware-specific at all, I'm wondering
>>>> if there's a simpler way to do this?
>>>>   - Should the driver be renamed to "simple-bus", and match "simple-bus"?
>>>>   - Should this be moved to core code, without an explicit driver for
>>>>     "simple-bus"? I.e. should the driver core just enable runtime PM for
>>>>     all devices not bound to a driver, as they may represent buses with
>>>>     child devices that do rely on runtime PM?
>>>>
>>>> Thanks for your comments and suggestions!
>>>
>>> My understanding of simple-bus is that it's something that does
>>> not have any power-management capabilities, and I'd rather not
>>> add clocks or interrupts to it.

Should I not add the interrupt to the device node, even though I do
know the hardware block has an interrupt?
Of course, to make use of the interrupt, you do need a hardware-specific
driver that binds to the specific compatible name. But for now such a
driver is overkill, as I do not need to use the interrupt.

Power domains (and clocks used for power management) are different,
as they are platform features: they may appear in any device node, even if
the bindings for the device node don't mention them.
Else I cannot add them to a node that's compatible with e.g. "arm,cortex-a15"
or "arm,pl310-cache", without adding (or completely replacing them with)
compatible names like "renesas,r8a73a4-cortex-a15" resp.
"renesas,sh73a0-pl310-cache"?
Hence I think they should be allowed in "simple-bus" nodes, too.

>>> What I think makes more sense is to have a bus driver for it
>>> in drivers/bus, remove the "simple-bus" compatibility value
>>> and have the driver take care of registering the power domain
>>> and probing the child devices using of_platform_populate on
>>> itself.
>>
>> Registering power domains is already handled by the core code.
>>
>> So compared to my RFC code, I only have to
>>   1. Drop "simple-bus" from the compatible property in the .dtsi,
>>   2. Call of_platform_populate() from renesas_bsc_probe(), to
>>      register the child devices, now the core code no longer does that.
>>
>> I find it a bit strange having to add _more_ code, as the core code handles
>> registering child devices fine. Doing it from my bus driver only protects
>> against people trying to run a kernel without my bus driver included
>> (which currently works fine, as long as no PM domain or clock is involved,
>>  e.g. if the clock is forgotten in the SoC's .dtsi :-).
>
> I actually rather like the simple bus driver in it's current form, where
> the device and pm-domain registration is taken care of by the
> core. Since it's really not HW specific at all, maybe just rename it
> simple-pm-bus or something like that?

That means writing bindings for a new bus type "simple-pm-bus"?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Geert Uytterhoeven]

Hi Kevin, Arnd,

I've been giving this some more thoughts...

On Wed, Nov 26, 2014 at 9:11 PM, Kevin Hilman <khilman@kernel.org> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>> On Mon, Nov 24, 2014 at 9:15 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Monday 24 November 2014 21:10:05 Geert Uytterhoeven wrote:
>>>> As this minimal BSC driver isn't hardware-specific at all, I'm wondering
>>>> if there's a simpler way to do this?
>>>>   - Should the driver be renamed to "simple-bus", and match "simple-bus"?
>>>>   - Should this be moved to core code, without an explicit driver for
>>>>     "simple-bus"? I.e. should the driver core just enable runtime PM for
>>>>     all devices not bound to a driver, as they may represent buses with
>>>>     child devices that do rely on runtime PM?
>>>>
>>>> Thanks for your comments and suggestions!
>>>
>>> My understanding of simple-bus is that it's something that does
>>> not have any power-management capabilities, and I'd rather not
>>> add clocks or interrupts to it.

Should I not add the interrupt to the device node, even though I do
know the hardware block has an interrupt?
Of course, to make use of the interrupt, you do need a hardware-specific
driver that binds to the specific compatible name. But for now such a
driver is overkill, as I do not need to use the interrupt.

Power domains (and clocks used for power management) are different,
as they are platform features: they may appear in any device node, even if
the bindings for the device node don't mention them.
Else I cannot add them to a node that's compatible with e.g. "arm,cortex-a15"
or "arm,pl310-cache", without adding (or completely replacing them with)
compatible names like "renesas,r8a73a4-cortex-a15" resp.
"renesas,sh73a0-pl310-cache"?
Hence I think they should be allowed in "simple-bus" nodes, too.

>>> What I think makes more sense is to have a bus driver for it
>>> in drivers/bus, remove the "simple-bus" compatibility value
>>> and have the driver take care of registering the power domain
>>> and probing the child devices using of_platform_populate on
>>> itself.
>>
>> Registering power domains is already handled by the core code.
>>
>> So compared to my RFC code, I only have to
>>   1. Drop "simple-bus" from the compatible property in the .dtsi,
>>   2. Call of_platform_populate() from renesas_bsc_probe(), to
>>      register the child devices, now the core code no longer does that.
>>
>> I find it a bit strange having to add _more_ code, as the core code handles
>> registering child devices fine. Doing it from my bus driver only protects
>> against people trying to run a kernel without my bus driver included
>> (which currently works fine, as long as no PM domain or clock is involved,
>>  e.g. if the clock is forgotten in the SoC's .dtsi :-).
>
> I actually rather like the simple bus driver in it's current form, where
> the device and pm-domain registration is taken care of by the
> core. Since it's really not HW specific at all, maybe just rename it
> simple-pm-bus or something like that?

That means writing bindings for a new bus type "simple-pm-bus"?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Kevin Hilman]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Kevin, Arnd,
>
> I've been giving this some more thoughts...
>
> On Wed, Nov 26, 2014 at 9:11 PM, Kevin Hilman <khilman@kernel.org> wrote:
>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>> On Mon, Nov 24, 2014 at 9:15 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>> On Monday 24 November 2014 21:10:05 Geert Uytterhoeven wrote:
>>>>> As this minimal BSC driver isn't hardware-specific at all, I'm wondering
>>>>> if there's a simpler way to do this?
>>>>>   - Should the driver be renamed to "simple-bus", and match "simple-bus"?
>>>>>   - Should this be moved to core code, without an explicit driver for
>>>>>     "simple-bus"? I.e. should the driver core just enable runtime PM for
>>>>>     all devices not bound to a driver, as they may represent buses with
>>>>>     child devices that do rely on runtime PM?
>>>>>
>>>>> Thanks for your comments and suggestions!
>>>>
>>>> My understanding of simple-bus is that it's something that does
>>>> not have any power-management capabilities, and I'd rather not
>>>> add clocks or interrupts to it.
>
> Should I not add the interrupt to the device node, even though I do
> know the hardware block has an interrupt?
> Of course, to make use of the interrupt, you do need a hardware-specific
> driver that binds to the specific compatible name. But for now such a
> driver is overkill, as I do not need to use the interrupt.
>
> Power domains (and clocks used for power management) are different,
> as they are platform features: they may appear in any device node, even if
> the bindings for the device node don't mention them.
> Else I cannot add them to a node that's compatible with e.g. "arm,cortex-a15"
> or "arm,pl310-cache", without adding (or completely replacing them with)
> compatible names like "renesas,r8a73a4-cortex-a15" resp.
> "renesas,sh73a0-pl310-cache"?
> Hence I think they should be allowed in "simple-bus" nodes, too.
>
>>>> What I think makes more sense is to have a bus driver for it
>>>> in drivers/bus, remove the "simple-bus" compatibility value
>>>> and have the driver take care of registering the power domain
>>>> and probing the child devices using of_platform_populate on
>>>> itself.
>>>
>>> Registering power domains is already handled by the core code.
>>>
>>> So compared to my RFC code, I only have to
>>>   1. Drop "simple-bus" from the compatible property in the .dtsi,
>>>   2. Call of_platform_populate() from renesas_bsc_probe(), to
>>>      register the child devices, now the core code no longer does that.
>>>
>>> I find it a bit strange having to add _more_ code, as the core code handles
>>> registering child devices fine. Doing it from my bus driver only protects
>>> against people trying to run a kernel without my bus driver included
>>> (which currently works fine, as long as no PM domain or clock is involved,
>>>  e.g. if the clock is forgotten in the SoC's .dtsi :-).
>>
>> I actually rather like the simple bus driver in it's current form, where
>> the device and pm-domain registration is taken care of by the
>> core. Since it's really not HW specific at all, maybe just rename it
>> simple-pm-bus or something like that?
>
> That means writing bindings for a new bus type "simple-pm-bus"?

Well, I'm OK with adding common properties to simple-bus also, but
presuably you'll want to document those as well in a binding doc for
simple-bus.

Kevin

Re: [PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Kevin Hilman]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Kevin, Arnd,
>
> I've been giving this some more thoughts...
>
> On Wed, Nov 26, 2014 at 9:11 PM, Kevin Hilman <khilman@kernel.org> wrote:
>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>> On Mon, Nov 24, 2014 at 9:15 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>> On Monday 24 November 2014 21:10:05 Geert Uytterhoeven wrote:
>>>>> As this minimal BSC driver isn't hardware-specific at all, I'm wondering
>>>>> if there's a simpler way to do this?
>>>>>   - Should the driver be renamed to "simple-bus", and match "simple-bus"?
>>>>>   - Should this be moved to core code, without an explicit driver for
>>>>>     "simple-bus"? I.e. should the driver core just enable runtime PM for
>>>>>     all devices not bound to a driver, as they may represent buses with
>>>>>     child devices that do rely on runtime PM?
>>>>>
>>>>> Thanks for your comments and suggestions!
>>>>
>>>> My understanding of simple-bus is that it's something that does
>>>> not have any power-management capabilities, and I'd rather not
>>>> add clocks or interrupts to it.
>
> Should I not add the interrupt to the device node, even though I do
> know the hardware block has an interrupt?
> Of course, to make use of the interrupt, you do need a hardware-specific
> driver that binds to the specific compatible name. But for now such a
> driver is overkill, as I do not need to use the interrupt.
>
> Power domains (and clocks used for power management) are different,
> as they are platform features: they may appear in any device node, even if
> the bindings for the device node don't mention them.
> Else I cannot add them to a node that's compatible with e.g. "arm,cortex-a15"
> or "arm,pl310-cache", without adding (or completely replacing them with)
> compatible names like "renesas,r8a73a4-cortex-a15" resp.
> "renesas,sh73a0-pl310-cache"?
> Hence I think they should be allowed in "simple-bus" nodes, too.
>
>>>> What I think makes more sense is to have a bus driver for it
>>>> in drivers/bus, remove the "simple-bus" compatibility value
>>>> and have the driver take care of registering the power domain
>>>> and probing the child devices using of_platform_populate on
>>>> itself.
>>>
>>> Registering power domains is already handled by the core code.
>>>
>>> So compared to my RFC code, I only have to
>>>   1. Drop "simple-bus" from the compatible property in the .dtsi,
>>>   2. Call of_platform_populate() from renesas_bsc_probe(), to
>>>      register the child devices, now the core code no longer does that.
>>>
>>> I find it a bit strange having to add _more_ code, as the core code handles
>>> registering child devices fine. Doing it from my bus driver only protects
>>> against people trying to run a kernel without my bus driver included
>>> (which currently works fine, as long as no PM domain or clock is involved,
>>>  e.g. if the clock is forgotten in the SoC's .dtsi :-).
>>
>> I actually rather like the simple bus driver in it's current form, where
>> the device and pm-domain registration is taken care of by the
>> core. Since it's really not HW specific at all, maybe just rename it
>> simple-pm-bus or something like that?
>
> That means writing bindings for a new bus type "simple-pm-bus"?

Well, I'm OK with adding common properties to simple-bus also, but
presuably you'll want to document those as well in a binding doc for
simple-bus.

Kevin

Re: [PATCH RFC 0/3] Add "Simple" / Renesas Bus State Controller Driver

[Kevin Hilman]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Kevin, Arnd,
>
> I've been giving this some more thoughts...
>
> On Wed, Nov 26, 2014 at 9:11 PM, Kevin Hilman <khilman@kernel.org> wrote:
>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>> On Mon, Nov 24, 2014 at 9:15 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>> On Monday 24 November 2014 21:10:05 Geert Uytterhoeven wrote:
>>>>> As this minimal BSC driver isn't hardware-specific at all, I'm wondering
>>>>> if there's a simpler way to do this?
>>>>>   - Should the driver be renamed to "simple-bus", and match "simple-bus"?
>>>>>   - Should this be moved to core code, without an explicit driver for
>>>>>     "simple-bus"? I.e. should the driver core just enable runtime PM for
>>>>>     all devices not bound to a driver, as they may represent buses with
>>>>>     child devices that do rely on runtime PM?
>>>>>
>>>>> Thanks for your comments and suggestions!
>>>>
>>>> My understanding of simple-bus is that it's something that does
>>>> not have any power-management capabilities, and I'd rather not
>>>> add clocks or interrupts to it.
>
> Should I not add the interrupt to the device node, even though I do
> know the hardware block has an interrupt?
> Of course, to make use of the interrupt, you do need a hardware-specific
> driver that binds to the specific compatible name. But for now such a
> driver is overkill, as I do not need to use the interrupt.
>
> Power domains (and clocks used for power management) are different,
> as they are platform features: they may appear in any device node, even if
> the bindings for the device node don't mention them.
> Else I cannot add them to a node that's compatible with e.g. "arm,cortex-a15"
> or "arm,pl310-cache", without adding (or completely replacing them with)
> compatible names like "renesas,r8a73a4-cortex-a15" resp.
> "renesas,sh73a0-pl310-cache"?
> Hence I think they should be allowed in "simple-bus" nodes, too.
>
>>>> What I think makes more sense is to have a bus driver for it
>>>> in drivers/bus, remove the "simple-bus" compatibility value
>>>> and have the driver take care of registering the power domain
>>>> and probing the child devices using of_platform_populate on
>>>> itself.
>>>
>>> Registering power domains is already handled by the core code.
>>>
>>> So compared to my RFC code, I only have to
>>>   1. Drop "simple-bus" from the compatible property in the .dtsi,
>>>   2. Call of_platform_populate() from renesas_bsc_probe(), to
>>>      register the child devices, now the core code no longer does that.
>>>
>>> I find it a bit strange having to add _more_ code, as the core code handles
>>> registering child devices fine. Doing it from my bus driver only protects
>>> against people trying to run a kernel without my bus driver included
>>> (which currently works fine, as long as no PM domain or clock is involved,
>>>  e.g. if the clock is forgotten in the SoC's .dtsi :-).
>>
>> I actually rather like the simple bus driver in it's current form, where
>> the device and pm-domain registration is taken care of by the
>> core. Since it's really not HW specific at all, maybe just rename it
>> simple-pm-bus or something like that?
>
> That means writing bindings for a new bus type "simple-pm-bus"?

Well, I'm OK with adding common properties to simple-bus also, but
presuably you'll want to document those as well in a binding doc for
simple-bus.

Kevin