[PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes

Subsystems: documentation, linux for powerpc (32-bit and 64-bit), the rest

STALE6757d

8 messages, 5 authors, 2008-02-08 · open the first message on its own page

[PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes

From: Anton Vorontsov <hidden>
Date: 2008-02-01 15:01:24

It isn't used anywhere, so remove it. If we'll ever need something
like this, we'll use compatible property instead.

Signed-off-by: Anton Vorontsov <redacted>
---

Rebased on top of recent tree.

 Documentation/powerpc/booting-without-of.txt |    1 -
 arch/powerpc/boot/dts/mpc832x_mds.dts        |    3 ---
 arch/powerpc/boot/dts/mpc832x_rdb.dts        |    2 --
 arch/powerpc/boot/dts/mpc836x_mds.dts        |    2 --
 arch/powerpc/boot/dts/mpc8568mds.dts         |    2 --
 5 files changed, 0 insertions(+), 10 deletions(-)
diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index 410c847..dcf9758 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -1675,7 +1675,6 @@ platforms are moved over to use the flattened-device-tree model.
 	ucc@2000 {
 		device_type = "network";
 		compatible = "ucc_geth";
-		model = "UCC";
 		device-id = <1>;
 		reg = <2000 200>;
 		interrupts = <a0 0>;
diff --git a/arch/powerpc/boot/dts/mpc832x_mds.dts b/arch/powerpc/boot/dts/mpc832x_mds.dts
index 9bb4083..f8b4a37 100644
--- a/arch/powerpc/boot/dts/mpc832x_mds.dts
+++ b/arch/powerpc/boot/dts/mpc832x_mds.dts
@@ -255,7 +255,6 @@
 		enet0: ucc@2200 {
 			device_type = "network";
 			compatible = "ucc_geth";
-			model = "UCC";
 			cell-index = <3>;
 			device-id = <3>;
 			reg = <0x2200 0x200>;
@@ -271,7 +270,6 @@
 		enet1: ucc@3200 {
 			device_type = "network";
 			compatible = "ucc_geth";
-			model = "UCC";
 			cell-index = <4>;
 			device-id = <4>;
 			reg = <0x3200 0x200>;
@@ -287,7 +285,6 @@
 		ucc@2400 {
 			device_type = "serial";
 			compatible = "ucc_uart";
-			model = "UCC";
 			device-id = <5>;	/* The UCC number, 1-7*/
 			port-number = <0>;	/* Which ttyQEx device */
 			soft-uart;		/* We need Soft-UART */
diff --git a/arch/powerpc/boot/dts/mpc832x_rdb.dts b/arch/powerpc/boot/dts/mpc832x_rdb.dts
index 94f93d2..ea7fcbf 100644
--- a/arch/powerpc/boot/dts/mpc832x_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc832x_rdb.dts
@@ -208,7 +208,6 @@
 		enet0: ucc@3000 {
 			device_type = "network";
 			compatible = "ucc_geth";
-			model = "UCC";
 			cell-index = <2>;
 			device-id = <2>;
 			reg = <0x3000 0x200>;
@@ -224,7 +223,6 @@
 		enet1: ucc@2200 {
 			device_type = "network";
 			compatible = "ucc_geth";
-			model = "UCC";
 			cell-index = <3>;
 			device-id = <3>;
 			reg = <0x2200 0x200>;
diff --git a/arch/powerpc/boot/dts/mpc836x_mds.dts b/arch/powerpc/boot/dts/mpc836x_mds.dts
index 55f03e8..525423c 100644
--- a/arch/powerpc/boot/dts/mpc836x_mds.dts
+++ b/arch/powerpc/boot/dts/mpc836x_mds.dts
@@ -257,7 +257,6 @@
 		enet0: ucc@2000 {
 			device_type = "network";
 			compatible = "ucc_geth";
-			model = "UCC";
 			cell-index = <1>;
 			device-id = <1>;
 			reg = <0x2000 0x200>;
@@ -274,7 +273,6 @@
 		enet1: ucc@3000 {
 			device_type = "network";
 			compatible = "ucc_geth";
-			model = "UCC";
 			cell-index = <2>;
 			device-id = <2>;
 			reg = <0x3000 0x200>;
diff --git a/arch/powerpc/boot/dts/mpc8568mds.dts b/arch/powerpc/boot/dts/mpc8568mds.dts
index 97bc048..66087d5 100644
--- a/arch/powerpc/boot/dts/mpc8568mds.dts
+++ b/arch/powerpc/boot/dts/mpc8568mds.dts
@@ -324,7 +324,6 @@
 		enet2: ucc@2000 {
 			device_type = "network";
 			compatible = "ucc_geth";
-			model = "UCC";
 			cell-index = <1>;
 			device-id = <1>;
 			reg = <2000 200>;
@@ -341,7 +340,6 @@
 		enet3: ucc@3000 {
 			device_type = "network";
 			compatible = "ucc_geth";
-			model = "UCC";
 			cell-index = <2>;
 			device-id = <2>;
 			reg = <3000 200>;
-- 
1.5.2.2

Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes

From: Kumar Gala <hidden>
Date: 2008-02-01 15:32:41

On Feb 1, 2008, at 9:01 AM, Anton Vorontsov wrote:
quoted hunk
It isn't used anywhere, so remove it. If we'll ever need something
like this, we'll use compatible property instead.

Signed-off-by: Anton Vorontsov <redacted>
---

Rebased on top of recent tree.

Documentation/powerpc/booting-without-of.txt |    1 -
arch/powerpc/boot/dts/mpc832x_mds.dts        |    3 ---
arch/powerpc/boot/dts/mpc832x_rdb.dts        |    2 --
arch/powerpc/boot/dts/mpc836x_mds.dts        |    2 --
arch/powerpc/boot/dts/mpc8568mds.dts         |    2 --
5 files changed, 0 insertions(+), 10 deletions(-)
diff --git a/Documentation/powerpc/booting-without-of.txt b/ 
Documentation/powerpc/booting-without-of.txt
index 410c847..dcf9758 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -1675,7 +1675,6 @@ platforms are moved over to use the flattened- 
device-tree model.
	ucc@2000 {
		device_type = "network";
		compatible = "ucc_geth";
-		model = "UCC";
		device-id = <1>;
can we change device-id to cell-index?
- k

Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes

From: Grant Likely <hidden>
Date: 2008-02-01 16:33:49

On 2/1/08, Kumar Gala [off-list ref] wrote:
quoted
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -1675,7 +1675,6 @@ platforms are moved over to use the flattened-
device-tree model.
      ucc@2000 {
              device_type = "network";
              compatible = "ucc_geth";
-             model = "UCC";
              device-id = <1>;
can we change device-id to cell-index?
<aside>
Here's a thought; do we really need a cell-index at all?  (and I'm
talking in general; not just this specific case).  I'm starting to
think we should migrate away from using it.

cell-index has been useful for things like clock controllers to know
what offset into a shared clock control register or something like
that and a driver would pass the cell-index value to the shared reg
driver when requesting service.  However, I think the information
encoded in cell-index is already encoded in the device tree in a
different manor.

Typically, shared registers and the like are all chip specific and the
behaviour of the shared register drivers usually needs to be tweaked
for different SoCs.  Each ip core on an SoC is already uniquely
indexed via the reg property.  True, 'reg' is sparse (0x2000, 0x2200,
0x2300, ...) whereas cell-index is tight (0,1,2,3,...), but I don't
think that introduces any additional difficulty.

So, instead of a driver passing it's cell-index value to the shared
reg driver, it would pass it's reg base instead.  The shared register
driver could then choose an internal representation that makes sense
for it instead of whatever layout was chosen by the device tree.

Dropping cell-index would mean one less property to keep in sync when
tailoring device trees. (== less complexity for board porters).
Besides, the purpose of cell-index is often misunderstood already by
people trying to use it to describe port numbers (ttyS0, ttyS1, etc).

Thoughts?
</aside>

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

[PATCH] [POWERPC][NET][SERIAL] UCCs: replace device-id with cell-index (was: Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes)

From: Anton Vorontsov <hidden>
Date: 2008-02-01 17:33:31

On Fri, Feb 01, 2008 at 09:32:38AM -0600, Kumar Gala wrote:
On Feb 1, 2008, at 9:01 AM, Anton Vorontsov wrote:
quoted
It isn't used anywhere, so remove it. If we'll ever need something
like this, we'll use compatible property instead.

Signed-off-by: Anton Vorontsov <redacted>
---

Rebased on top of recent tree.

Documentation/powerpc/booting-without-of.txt |    1 -
arch/powerpc/boot/dts/mpc832x_mds.dts        |    3 ---
arch/powerpc/boot/dts/mpc832x_rdb.dts        |    2 --
arch/powerpc/boot/dts/mpc836x_mds.dts        |    2 --
arch/powerpc/boot/dts/mpc8568mds.dts         |    2 --
5 files changed, 0 insertions(+), 10 deletions(-)
diff --git a/Documentation/powerpc/booting-without-of.txt b/ 
Documentation/powerpc/booting-without-of.txt
index 410c847..dcf9758 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -1675,7 +1675,6 @@ platforms are moved over to use the flattened- 
device-tree model.
ucc@2000 {
	device_type = "network";
	compatible = "ucc_geth";
-		model = "UCC";
	device-id = <1>;
can we change device-id to cell-index?
Sure. But let's do this in the separate patch? Because this change
actually touches the code in the two subsystems: net and serial.

I hope everybody will agree to pass it through powerpc tree..?

- - - -
From: Anton Vorontsov <redacted>
Subject: [POWERPC][NET][SERIAL] UCCs: replace device-id with cell-index

device-id is worse than cell-index. Probably cell-index isn't
good either, but device-id is worse anyway.

Drivers are modified for backward compatibility's sake.

Signed-off-by: Anton Vorontsov <redacted>
---
 Documentation/powerpc/booting-without-of.txt |    4 ++--
 arch/powerpc/boot/dts/mpc832x_mds.dts        |    4 +---
 arch/powerpc/boot/dts/mpc832x_rdb.dts        |    2 --
 arch/powerpc/boot/dts/mpc836x_mds.dts        |    2 --
 arch/powerpc/boot/dts/mpc836x_rdk.dts        |   12 ++++++------
 arch/powerpc/boot/dts/mpc8568mds.dts         |    2 --
 drivers/net/ucc_geth.c                       |    8 +++++++-
 drivers/net/ucc_geth_mii.c                   |   11 ++++++++---
 drivers/serial/ucc_uart.c                    |   16 ++++++++++++----
 9 files changed, 36 insertions(+), 25 deletions(-)
diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index dcf9758..7b30798 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -1618,7 +1618,7 @@ platforms are moved over to use the flattened-device-tree model.
      "bisync", "atm", or "serial".
    - compatible : could be "ucc_geth" or "fsl_atm" and so on.
    - model : should be "UCC".
-   - device-id : the ucc number(1-8), corresponding to UCCx in UM.
+   - cell-index : the ucc number(1-8), corresponding to UCCx in UM.
    - reg : Offset and length of the register set for the device
    - interrupts : <a b> where a is the interrupt number and b is a
      field that represents an encoding of the sense and level
@@ -1675,7 +1675,7 @@ platforms are moved over to use the flattened-device-tree model.
 	ucc@2000 {
 		device_type = "network";
 		compatible = "ucc_geth";
-		device-id = <1>;
+		cell-index = <1>;
 		reg = <2000 200>;
 		interrupts = <a0 0>;
 		interrupt-parent = <700>;
diff --git a/arch/powerpc/boot/dts/mpc832x_mds.dts b/arch/powerpc/boot/dts/mpc832x_mds.dts
index f8b4a37..539e02f 100644
--- a/arch/powerpc/boot/dts/mpc832x_mds.dts
+++ b/arch/powerpc/boot/dts/mpc832x_mds.dts
@@ -256,7 +256,6 @@
 			device_type = "network";
 			compatible = "ucc_geth";
 			cell-index = <3>;
-			device-id = <3>;
 			reg = <0x2200 0x200>;
 			interrupts = <34>;
 			interrupt-parent = <&qeic>;
@@ -271,7 +270,6 @@
 			device_type = "network";
 			compatible = "ucc_geth";
 			cell-index = <4>;
-			device-id = <4>;
 			reg = <0x3200 0x200>;
 			interrupts = <35>;
 			interrupt-parent = <&qeic>;
@@ -285,7 +283,7 @@
 		ucc@2400 {
 			device_type = "serial";
 			compatible = "ucc_uart";
-			device-id = <5>;	/* The UCC number, 1-7*/
+			cell-index = <5>;	/* The UCC number, 1-7*/
 			port-number = <0>;	/* Which ttyQEx device */
 			soft-uart;		/* We need Soft-UART */
 			reg = <0x2400 0x200>;
diff --git a/arch/powerpc/boot/dts/mpc832x_rdb.dts b/arch/powerpc/boot/dts/mpc832x_rdb.dts
index ea7fcbf..179c81c 100644
--- a/arch/powerpc/boot/dts/mpc832x_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc832x_rdb.dts
@@ -209,7 +209,6 @@
 			device_type = "network";
 			compatible = "ucc_geth";
 			cell-index = <2>;
-			device-id = <2>;
 			reg = <0x3000 0x200>;
 			interrupts = <33>;
 			interrupt-parent = <&qeic>;
@@ -224,7 +223,6 @@
 			device_type = "network";
 			compatible = "ucc_geth";
 			cell-index = <3>;
-			device-id = <3>;
 			reg = <0x2200 0x200>;
 			interrupts = <34>;
 			interrupt-parent = <&qeic>;
diff --git a/arch/powerpc/boot/dts/mpc836x_mds.dts b/arch/powerpc/boot/dts/mpc836x_mds.dts
index 525423c..8160ff2 100644
--- a/arch/powerpc/boot/dts/mpc836x_mds.dts
+++ b/arch/powerpc/boot/dts/mpc836x_mds.dts
@@ -258,7 +258,6 @@
 			device_type = "network";
 			compatible = "ucc_geth";
 			cell-index = <1>;
-			device-id = <1>;
 			reg = <0x2000 0x200>;
 			interrupts = <32>;
 			interrupt-parent = <&qeic>;
@@ -274,7 +273,6 @@
 			device_type = "network";
 			compatible = "ucc_geth";
 			cell-index = <2>;
-			device-id = <2>;
 			reg = <0x3000 0x200>;
 			interrupts = <33>;
 			interrupt-parent = <&qeic>;
diff --git a/arch/powerpc/boot/dts/mpc836x_rdk.dts b/arch/powerpc/boot/dts/mpc836x_rdk.dts
index e35200a..cc56338 100644
--- a/arch/powerpc/boot/dts/mpc836x_rdk.dts
+++ b/arch/powerpc/boot/dts/mpc836x_rdk.dts
@@ -179,7 +179,7 @@
 			enet0: ucc@2000 {
 				device_type = "network";
 				compatible = "ucc_geth";
-				device-id = <1>;
+				cell-index = <1>;
 				reg = <0x2000 0x200>;
 				interrupts = <32>;
 				interrupt-parent = <&qeic>;
@@ -194,7 +194,7 @@
 			enet1: ucc@3000 {
 				device_type = "network";
 				compatible = "ucc_geth";
-				device-id = <2>;
+				cell-index = <2>;
 				reg = <0x3000 0x200>;
 				interrupts = <33>;
 				interrupt-parent = <&qeic>;
@@ -209,7 +209,7 @@
 			enet2: ucc@2600 {
 				device_type = "network";
 				compatible = "ucc_geth";
-				device-id = <7>;
+				cell-index = <7>;
 				reg = <0x2600 0x200>;
 				interrupts = <42>;
 				interrupt-parent = <&qeic>;
@@ -224,7 +224,7 @@
 			enet3: ucc@3200 {
 				device_type = "network";
 				compatible = "ucc_geth";
-				device-id = <4>;
+				cell-index = <4>;
 				reg = <0x3200 0x200>;
 				interrupts = <35>;
 				interrupt-parent = <&qeic>;
@@ -271,7 +271,7 @@
 				device_type = "serial";
 				compatible = "ucc_uart";
 				reg = <0x2400 0x200>;
-				device-id = <5>;
+				cell-index = <5>;
 				port-number = <0>;
 				rx-clock-name = "brg7";
 				tx-clock-name = "brg8";
@@ -284,7 +284,7 @@
 				device_type = "serial";
 				compatible = "ucc_uart";
 				reg = <0x3400 0x200>;
-				device-id = <6>;
+				cell-index = <6>;
 				port-number = <1>;
 				rx-clock-name = "brg13";
 				tx-clock-name = "brg14";
diff --git a/arch/powerpc/boot/dts/mpc8568mds.dts b/arch/powerpc/boot/dts/mpc8568mds.dts
index 66087d5..df4b5e8 100644
--- a/arch/powerpc/boot/dts/mpc8568mds.dts
+++ b/arch/powerpc/boot/dts/mpc8568mds.dts
@@ -325,7 +325,6 @@
 			device_type = "network";
 			compatible = "ucc_geth";
 			cell-index = <1>;
-			device-id = <1>;
 			reg = <2000 200>;
 			interrupts = <20>;
 			interrupt-parent = <&qeic>;
@@ -341,7 +340,6 @@
 			device_type = "network";
 			compatible = "ucc_geth";
 			cell-index = <2>;
-			device-id = <2>;
 			reg = <3000 200>;
 			interrupts = <21>;
 			interrupt-parent = <&qeic>;
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index fba0811..3a68b94 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3852,7 +3852,13 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
 
 	ugeth_vdbg("%s: IN", __FUNCTION__);
 
-	prop = of_get_property(np, "device-id", NULL);
+	prop = of_get_property(np, "cell-index", NULL);
+	if (!prop) {
+		prop = of_get_property(np, "device-id", NULL);
+		if (!prop)
+			return -ENODEV;
+	}
+
 	ucc_num = *prop - 1;
 	if ((ucc_num < 0) || (ucc_num > 7))
 		return -ENODEV;
diff --git a/drivers/net/ucc_geth_mii.c b/drivers/net/ucc_geth_mii.c
index e3ba14a..e5b17d7 100644
--- a/drivers/net/ucc_geth_mii.c
+++ b/drivers/net/ucc_geth_mii.c
@@ -203,9 +203,14 @@ static int uec_mdio_probe(struct of_device *ofdev, const struct of_device_id *ma
 		if ((res.start >= tempres.start) &&
 		    (res.end <= tempres.end)) {
 			/* set this UCC to be the MII master */
-			const u32 *id = of_get_property(tempnp, "device-id", NULL);
-			if (id == NULL)
-				goto bus_register_fail;
+			const u32 *id;
+
+			id = of_get_property(tempnp, "cell-index", NULL);
+			if (!id) {
+				id = of_get_property(tempnp, "device-id", NULL);
+				if (!id)
+					goto bus_register_fail;
+			}
 
 			ucc_set_qe_mux_mii_mng(*id - 1);
 
diff --git a/drivers/serial/ucc_uart.c b/drivers/serial/ucc_uart.c
index e0994f0..5e4310c 100644
--- a/drivers/serial/ucc_uart.c
+++ b/drivers/serial/ucc_uart.c
@@ -1270,10 +1270,18 @@ static int ucc_uart_probe(struct of_device *ofdev,
 
 	/* Get the UCC number (device ID) */
 	/* UCCs are numbered 1-7 */
-	iprop = of_get_property(np, "device-id", NULL);
-	if (!iprop || (*iprop < 1) || (*iprop > UCC_MAX_NUM)) {
-		dev_err(&ofdev->dev,
-			"missing or invalid UCC specified in device tree\n");
+	iprop = of_get_property(np, "cell-index", NULL);
+	if (!iprop) {
+		iprop = of_get_property(np, "device-id", NULL);
+		if (!iprop) {
+			dev_err(&ofdev->dev, "UCC is unspecified in "
+				"device tree\n");
+			return -EINVAL;
+		}
+	}
+
+	if ((*iprop < 1) || (*iprop > UCC_MAX_NUM)) {
+		dev_err(&ofdev->dev, "no support for UCC%u\n", *iprop);
 		kfree(qe_port);
 		return -ENODEV;
 	}
-- 
1.5.2.2

Re: [PATCH] [POWERPC][NET][SERIAL] UCCs: replace device-id with cell-index

From: Jeff Garzik <hidden>
Date: 2008-02-01 17:53:01

Anton Vorontsov wrote:
On Fri, Feb 01, 2008 at 09:32:38AM -0600, Kumar Gala wrote:
quoted
On Feb 1, 2008, at 9:01 AM, Anton Vorontsov wrote:
quoted
It isn't used anywhere, so remove it. If we'll ever need something
like this, we'll use compatible property instead.

Signed-off-by: Anton Vorontsov <redacted>
---

Rebased on top of recent tree.

Documentation/powerpc/booting-without-of.txt |    1 -
arch/powerpc/boot/dts/mpc832x_mds.dts        |    3 ---
arch/powerpc/boot/dts/mpc832x_rdb.dts        |    2 --
arch/powerpc/boot/dts/mpc836x_mds.dts        |    2 --
arch/powerpc/boot/dts/mpc8568mds.dts         |    2 --
5 files changed, 0 insertions(+), 10 deletions(-)
diff --git a/Documentation/powerpc/booting-without-of.txt b/ 
Documentation/powerpc/booting-without-of.txt
index 410c847..dcf9758 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -1675,7 +1675,6 @@ platforms are moved over to use the flattened- 
device-tree model.
	ucc@2000 {
		device_type = "network";
		compatible = "ucc_geth";
-		model = "UCC";
		device-id = <1>;
can we change device-id to cell-index?
Sure. But let's do this in the separate patch? Because this change
actually touches the code in the two subsystems: net and serial.

I hope everybody will agree to pass it through powerpc tree..?

- - - -
From: Anton Vorontsov <redacted>
Subject: [POWERPC][NET][SERIAL] UCCs: replace device-id with cell-index

device-id is worse than cell-index. Probably cell-index isn't
good either, but device-id is worse anyway.

Drivers are modified for backward compatibility's sake.

Signed-off-by: Anton Vorontsov <redacted>
---
 Documentation/powerpc/booting-without-of.txt |    4 ++--
 arch/powerpc/boot/dts/mpc832x_mds.dts        |    4 +---
 arch/powerpc/boot/dts/mpc832x_rdb.dts        |    2 --
 arch/powerpc/boot/dts/mpc836x_mds.dts        |    2 --
 arch/powerpc/boot/dts/mpc836x_rdk.dts        |   12 ++++++------
 arch/powerpc/boot/dts/mpc8568mds.dts         |    2 --
 drivers/net/ucc_geth.c                       |    8 +++++++-
 drivers/net/ucc_geth_mii.c                   |   11 ++++++++---
 drivers/serial/ucc_uart.c                    |   16 ++++++++++++----
 9 files changed, 36 insertions(+), 25 deletions(-)
ACK drivers/net

Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes

From: David Gibson <hidden>
Date: 2008-02-05 13:20:26

On Fri, Feb 01, 2008 at 09:23:47AM -0700, Grant Likely wrote:
On 2/1/08, Kumar Gala [off-list ref] wrote:
quoted
quoted
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -1675,7 +1675,6 @@ platforms are moved over to use the flattened-
device-tree model.
      ucc@2000 {
              device_type = "network";
              compatible = "ucc_geth";
-             model = "UCC";
              device-id = <1>;
can we change device-id to cell-index?
<aside>
Here's a thought; do we really need a cell-index at all?  (and I'm
talking in general; not just this specific case).  I'm starting to
think we should migrate away from using it.
_Need_ perhaps not, but in some cases I think the alternatives are
overly complex.
cell-index has been useful for things like clock controllers to know
what offset into a shared clock control register or something like
that and a driver would pass the cell-index value to the shared reg
driver when requesting service.
Right.  Except that if the shared resource is just a single register,
calling the routines to access it a "shared reg driver" gives a
misleading impression.  Depending on how the shared reg is used, even
a lock may not be necessary, so potentially the drivers for the
individual device instances using the shared resource can (safely)
directly access it.
 However, I think the information
encoded in cell-index is already encoded in the device tree in a
different manor.

Typically, shared registers and the like are all chip specific and the
behaviour of the shared register drivers usually needs to be tweaked
for different SoCs.  Each ip core on an SoC is already uniquely
indexed via the reg property.  True, 'reg' is sparse (0x2000, 0x2200,
0x2300, ...) whereas cell-index is tight (0,1,2,3,...), but I don't
think that introduces any additional difficulty.

So, instead of a driver passing it's cell-index value to the shared
reg driver, it would pass it's reg base instead.  The shared register
driver could then choose an internal representation that makes sense
for it instead of whatever layout was chosen by the device tree.
Except that the "shared reg driver" then needs a way to map from reg
property to index.  So either it has to have that hardcoded, or the
shared resource will need its own device tree representation for the
mapping.

In some cases the shared resource is complex enough that that makes
sense (e.g. an mdio bus shared between ethernet MACs to take an
extreme example).  But when the "shared reg driver" consists entirely
of:
	(readl(shared_reg_addr) & (1UL << cell_index))
and/or:
	writel(shared_reg_addr, 1UL << cell_index)
your approach would seem to be overkill and then some.  (The latter
example can be safe without locking in the case of a read/clear
register).

Exactly this sort of thing is fairly common on 4xx, which is just the
context in which BenH invented "cell-index" as a really simple way of
representing the index into shared registers like this.
Dropping cell-index would mean one less property to keep in sync when
tailoring device trees. (== less complexity for board porters).
Besides, the purpose of cell-index is often misunderstood already by
people trying to use it to describe port numbers (ttyS0, ttyS1, etc).
This is indeed a problem.  But I don't think ditching cell-index
entirely is a sensible solution, sorry.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes

From: Grant Likely <hidden>
Date: 2008-02-05 16:39:05

On 2/5/08, David Gibson [off-list ref] wrote:
On Fri, Feb 01, 2008 at 09:23:47AM -0700, Grant Likely wrote:
quoted
cell-index has been useful for things like clock controllers to know
what offset into a shared clock control register or something like
that and a driver would pass the cell-index value to the shared reg
driver when requesting service.
Right.  Except that if the shared resource is just a single register,
calling the routines to access it a "shared reg driver" gives a
misleading impression.  Depending on how the shared reg is used, even
a lock may not be necessary, so potentially the drivers for the
individual device instances using the shared resource can (safely)
directly access it.
Fair enough.  In the case of a single shared, or a homogeneous set of
shared registers (all of them use the same index) I can see the
argument for cell index.

However, there are places where cell-index is being used where the
value of cell-index has no relation to the offset into a register.
But what about the case where the device uses multiple shared
registers, each one using a *different* offset.  cell-index doesn't
describe this situation well (or at least no better than just using
the value of reg instead, a translation is still required)
quoted
Typically, shared registers and the like are all chip specific and the
behaviour of the shared register drivers usually needs to be tweaked
for different SoCs.  Each ip core on an SoC is already uniquely
indexed via the reg property.  True, 'reg' is sparse (0x2000, 0x2200,
0x2300, ...) whereas cell-index is tight (0,1,2,3,...), but I don't
think that introduces any additional difficulty.

So, instead of a driver passing it's cell-index value to the shared
reg driver, it would pass it's reg base instead.  The shared register
driver could then choose an internal representation that makes sense
for it instead of whatever layout was chosen by the device tree.
Except that the "shared reg driver" then needs a way to map from reg
property to index.  So either it has to have that hardcoded, or the
shared resource will need its own device tree representation for the
mapping.
SoCs are fickle things; Writing code to handle differences between the
same devices on different SoCs is going to happen anyway.  Isn't that
what a device driver is for?  :-)
In some cases the shared resource is complex enough that that makes
sense (e.g. an mdio bus shared between ethernet MACs to take an
extreme example).  But when the "shared reg driver" consists entirely
of:
        (readl(shared_reg_addr) & (1UL << cell_index))
and/or:
        writel(shared_reg_addr, 1UL << cell_index)
your approach would seem to be overkill and then some.  (The latter
example can be safe without locking in the case of a read/clear
register).

Exactly this sort of thing is fairly common on 4xx, which is just the
context in which BenH invented "cell-index" as a really simple way of
representing the index into shared registers like this.
From booting-without-of (in the EMAC description):
- cell-index        : 1 cell, hardware index of the EMAC cell on a
given ASIC (typically 0x0 and 0x1 for EMAC0 and EMAC1 on each Axon
chip.

So, even if the intent was for cell-index to specify offsets into
shared regs, the description does not reflect that purpose.  And
reading thorough the rest of the document, cell-index is described
purely in terms of enumerating ip blocks, so that is clearly the
assumption that people are making when using it.

In other words, my point is this:  *If* cell-index is just a way to
encode the manufacturing assigned ip-block number (EMAC0, EMAC1, etc)
then there is probably little or no value in it.  The two arguments I
see for using cell-index in that mode are:

1) to offset into shared registers (but this doesn't hold because ip
block numbers often don't match register offsets and the reg property
would be just as suitable)

2) to logically identify ip blocks to the user (but cell-index was
never intended for this and /aliases is a better solution anyway)
quoted
Dropping cell-index would mean one less property to keep in sync when
tailoring device trees. (== less complexity for board porters).
Besides, the purpose of cell-index is often misunderstood already by
people trying to use it to describe port numbers (ttyS0, ttyS1, etc).
This is indeed a problem.  But I don't think ditching cell-index
entirely is a sensible solution, sorry.
Gah!  Don't apologize!  :-)  My goal was to spur on debate to better
firm up our device tree conventions.  Mission accomplished (or at
least in progress.)  :-)

Also, I can probably be convinced on the continued usage of
cell-index, but as it stands right now I think we're over using it.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes

From: David Gibson <hidden>
Date: 2008-02-08 06:14:51

On Tue, Feb 05, 2008 at 09:39:02AM -0700, Grant Likely wrote:
On 2/5/08, David Gibson [off-list ref] wrote:
quoted
On Fri, Feb 01, 2008 at 09:23:47AM -0700, Grant Likely wrote:
quoted
cell-index has been useful for things like clock controllers to know
what offset into a shared clock control register or something like
that and a driver would pass the cell-index value to the shared reg
driver when requesting service.
Right.  Except that if the shared resource is just a single register,
calling the routines to access it a "shared reg driver" gives a
misleading impression.  Depending on how the shared reg is used, even
a lock may not be necessary, so potentially the drivers for the
individual device instances using the shared resource can (safely)
directly access it.
Fair enough.  In the case of a single shared, or a homogeneous set of
shared registers (all of them use the same index) I can see the
argument for cell index.
Yes, that's the situation it was created for.
However, there are places where cell-index is being used where the
value of cell-index has no relation to the offset into a register.
But what about the case where the device uses multiple shared
registers, each one using a *different* offset.  cell-index doesn't
describe this situation well (or at least no better than just using
the value of reg instead, a translation is still required)
Absolutely.  This is not a suitable situation in which to use
cell-index.

[snip]
quoted
From booting-without-of (in the EMAC description):
- cell-index        : 1 cell, hardware index of the EMAC cell on a
given ASIC (typically 0x0 and 0x1 for EMAC0 and EMAC1 on each Axon
chip.

So, even if the intent was for cell-index to specify offsets into
shared regs, the description does not reflect that purpose.  And
reading thorough the rest of the document, cell-index is described
purely in terms of enumerating ip blocks, so that is clearly the
assumption that people are making when using it.

In other words, my point is this:  *If* cell-index is just a way to
encode the manufacturing assigned ip-block number (EMAC0, EMAC1, etc)
then there is probably little or no value in it.  The two arguments I
see for using cell-index in that mode are:

1) to offset into shared registers (but this doesn't hold because ip
block numbers often don't match register offsets and the reg property
would be just as suitable)

2) to logically identify ip blocks to the user (but cell-index was
never intended for this and /aliases is a better solution anyway)
Right.  The confusion arises because cell-index was invented on 4xx,
where it's common practice to index global registers by the ip block
number.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help