* [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes
@ 2008-02-01 15:01 Anton Vorontsov
2008-02-01 15:32 ` Kumar Gala
0 siblings, 1 reply; 8+ messages in thread
From: Anton Vorontsov @ 2008-02-01 15:01 UTC (permalink / raw)
To: linuxppc-dev
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 <avorontsov@ru.mvista.com>
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes
2008-02-01 15:01 [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes Anton Vorontsov
@ 2008-02-01 15:32 ` Kumar Gala
2008-02-01 16:23 ` Grant Likely
2008-02-01 17:33 ` [PATCH] [POWERPC][NET][SERIAL] UCCs: replace device-id with cell-index (was: Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes) Anton Vorontsov
0 siblings, 2 replies; 8+ messages in thread
From: Kumar Gala @ 2008-02-01 15:32 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linuxppc-dev
On Feb 1, 2008, at 9:01 AM, Anton Vorontsov wrote:
> 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 <avorontsov@ru.mvista.com>
> ---
>
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes
2008-02-01 15:32 ` Kumar Gala
@ 2008-02-01 16:23 ` Grant Likely
2008-02-05 13:20 ` David Gibson
2008-02-01 17:33 ` [PATCH] [POWERPC][NET][SERIAL] UCCs: replace device-id with cell-index (was: Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes) Anton Vorontsov
1 sibling, 1 reply; 8+ messages in thread
From: Grant Likely @ 2008-02-01 16:23 UTC (permalink / raw)
To: Kumar Gala; +Cc: Jon Loeliger, linuxppc-dev, Scott Wood
On 2/1/08, Kumar Gala <galak@kernel.crashing.org> wrote:
>
> > --- 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] [POWERPC][NET][SERIAL] UCCs: replace device-id with cell-index (was: Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes)
2008-02-01 15:32 ` Kumar Gala
2008-02-01 16:23 ` Grant Likely
@ 2008-02-01 17:33 ` Anton Vorontsov
2008-02-01 17:52 ` [PATCH] [POWERPC][NET][SERIAL] UCCs: replace device-id with cell-index Jeff Garzik
1 sibling, 1 reply; 8+ messages in thread
From: Anton Vorontsov @ 2008-02-01 17:33 UTC (permalink / raw)
To: Kumar Gala
Cc: netdev, linuxppc-dev, linux-serial, Li Yang, Jeff Garzik,
Timur Tabi
On Fri, Feb 01, 2008 at 09:32:38AM -0600, Kumar Gala wrote:
> On Feb 1, 2008, at 9:01 AM, Anton Vorontsov wrote:
>
> >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 <avorontsov@ru.mvista.com>
> >---
> >
> >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 <avorontsov@ru.mvista.com>
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 <avorontsov@ru.mvista.com>
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] [POWERPC][NET][SERIAL] UCCs: replace device-id with cell-index
2008-02-01 17:33 ` [PATCH] [POWERPC][NET][SERIAL] UCCs: replace device-id with cell-index (was: Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes) Anton Vorontsov
@ 2008-02-01 17:52 ` Jeff Garzik
0 siblings, 0 replies; 8+ messages in thread
From: Jeff Garzik @ 2008-02-01 17:52 UTC (permalink / raw)
To: avorontsov; +Cc: netdev, linuxppc-dev, linux-serial, Li Yang, Timur Tabi
Anton Vorontsov wrote:
> On Fri, Feb 01, 2008 at 09:32:38AM -0600, Kumar Gala wrote:
>> On Feb 1, 2008, at 9:01 AM, Anton Vorontsov wrote:
>>
>>> 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 <avorontsov@ru.mvista.com>
>>> ---
>>>
>>> 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 <avorontsov@ru.mvista.com>
> 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 <avorontsov@ru.mvista.com>
> ---
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes
2008-02-01 16:23 ` Grant Likely
@ 2008-02-05 13:20 ` David Gibson
2008-02-05 16:39 ` Grant Likely
0 siblings, 1 reply; 8+ messages in thread
From: David Gibson @ 2008-02-05 13:20 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, linuxppc-dev, Jon Loeliger
On Fri, Feb 01, 2008 at 09:23:47AM -0700, Grant Likely wrote:
> On 2/1/08, Kumar Gala <galak@kernel.crashing.org> wrote:
> >
> > > --- 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes
2008-02-05 13:20 ` David Gibson
@ 2008-02-05 16:39 ` Grant Likely
2008-02-08 6:14 ` David Gibson
0 siblings, 1 reply; 8+ messages in thread
From: Grant Likely @ 2008-02-05 16:39 UTC (permalink / raw)
To: Grant Likely, Kumar Gala, Jon Loeliger, linuxppc-dev, Scott Wood
On 2/5/08, David Gibson <david@gibson.dropbear.id.au> wrote:
> On Fri, Feb 01, 2008 at 09:23:47AM -0700, Grant Likely wrote:
> > 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)
> > 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)
> > 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes
2008-02-05 16:39 ` Grant Likely
@ 2008-02-08 6:14 ` David Gibson
0 siblings, 0 replies; 8+ messages in thread
From: David Gibson @ 2008-02-08 6:14 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, linuxppc-dev, Jon Loeliger
On Tue, Feb 05, 2008 at 09:39:02AM -0700, Grant Likely wrote:
> On 2/5/08, David Gibson <david@gibson.dropbear.id.au> wrote:
> > On Fri, Feb 01, 2008 at 09:23:47AM -0700, Grant Likely wrote:
> > > 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]
> >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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-02-08 6:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-01 15:01 [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes Anton Vorontsov
2008-02-01 15:32 ` Kumar Gala
2008-02-01 16:23 ` Grant Likely
2008-02-05 13:20 ` David Gibson
2008-02-05 16:39 ` Grant Likely
2008-02-08 6:14 ` David Gibson
2008-02-01 17:33 ` [PATCH] [POWERPC][NET][SERIAL] UCCs: replace device-id with cell-index (was: Re: [PATCH] [POWERPC] get rid of `model = "UCC"' in the ucc nodes) Anton Vorontsov
2008-02-01 17:52 ` [PATCH] [POWERPC][NET][SERIAL] UCCs: replace device-id with cell-index Jeff Garzik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).