Re: [U-Boot] [PATCH v2 17/29] dm: Introduce device sequence numbering

2014-07-18 Thread Simon Glass
Hi Jon,

On 9 July 2014 07:53, Jon Loeliger  wrote:
> HI Simon,
>
> On Tue, Jul 8, 2014 at 10:38 PM, Simon Glass  wrote:
>> In U-Boot it is pretty common to number devices from 0 and access them
>> on the command line using this numbering. While it may come to pass that
>> we will move away from this numbering, the possibility seems remote at
>> present.
>>
>> Given that devices within a uclass will have an implied numbering, it
>> makes sense to build this into driver model as a core feature. The cost
>> is fairly small in terms of code and data space.
>
> Hmmm.  I'm not entirely in agreement here.  I think this is the wrong
> long-term approach, and this just reinforces the status quo rather than
> allowing a migration to a better approach.

If we don't add numbering to driver model, then it will be done badly
(and ad-hoc) in support code. I can't even quite imagine how we might
do it. Also while U-Boot's numbering may not be the right long-term
approach, that would involve a large discussion and long change-over
to an as-yet unknown method. So I think we need to have realistic
expectations of driver mode's place in the world. It can enable new
methods, but not mandate them.

>
>> With each uclass having numbered devices we can ask for SPI port 0 or
>> serial port 1 and receive a single device.
>
> That's nice, but we should allow them to be named by their actual
> names as found in the device tree too.

There's nothing in this patch that precludes that. It would require
changes to U-Boot's command processing, probably best done as a
separate series sometime in the future.

>
>> Devices typically request a sequence number using aliases in the device
>> tree. These are resolved when the device is probed, to deal with conflicts.
>> Sequence numbers need not be sequential and holes are permitted.
>
> So they are unreliably unpredictable, unless you also happen
> to have the DTS decoder ring in hand too?

If you compare with how things are now, we only support a single
driver for each class. For example, if you are using SPI, you can only
have one SPI driver - perhaps with multiple ports. If you are not
using DT, then the U_BOOT_DEVICE() macros will all appear in that
driver or the board file (TBD at this stage), and will be parsed in
order. I believe we can honour the numbering in that case, albeit I
don't want to implement something before it is needed.

>
>
>> +This indicates that in the uclass called "serial", the named node
>> +("/serial@2223") will be given sequence number 2. Any command or driver
>> +which requests serial device 2 will obtain this device.
>> +
>> +Some devices represent buses where the devices on the bus are numbered or
>> +addressed. For example, SPI typically numbers its slaves from 0, and I2C
>> +uses a 7-bit address. In these cases the 'reg' property of the subnode is
>> +used, for example:
>> +
>> +{
>> +   aliases {
>> +   spi2 = "/spi@2230";
>> +   };
>> +
>> +   spi@2230 {
>> +   #address-cells = <1>;
>> +   #size-cells = <1>;
>> +   spi-flash@0 {
>> +   reg = <0>;
>> +   ...
>> +   }
>> +   eeprom@1 {
>> +   reg = <1>;
>> +   };
>> +   };
>
> And not everyone agrees that this is the best approach, even in Linux land.
> Specifically, we should be in agreement with Linux, and we should not
> have our DTS stray from the definitions that Linux will accept for the same
> devices.  And this approach won't be bought by the Linux crowd.  (Yes,
> there are some that use  a "reg = <0>;" approach here, but there are many
> that do not too.  It's not a universally accepted approach.)
>
> This concept is crucial.
>
> I've said it before, and I will say it again if needed.
>
> So:  Sure, put this approach in, but make it be the backward compatible
> approach.  Also please put in a correct naming approach so that we can
> move forward with a better longer term solution too.

I think you are saying that something like:

sf probe spi1:flash

should be possible instead of

sf probe 1:0

It feels right to me too, but it doesn't belong in this patch, needs
further discussion and the command parsing aspect needs more general
thought. It would be great to figure this out and get it agreed, and
driver model can certainly support it.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 17/29] dm: Introduce device sequence numbering

2014-07-09 Thread Jon Loeliger
HI Simon,

On Tue, Jul 8, 2014 at 10:38 PM, Simon Glass  wrote:
> In U-Boot it is pretty common to number devices from 0 and access them
> on the command line using this numbering. While it may come to pass that
> we will move away from this numbering, the possibility seems remote at
> present.
>
> Given that devices within a uclass will have an implied numbering, it
> makes sense to build this into driver model as a core feature. The cost
> is fairly small in terms of code and data space.

Hmmm.  I'm not entirely in agreement here.  I think this is the wrong
long-term approach, and this just reinforces the status quo rather than
allowing a migration to a better approach.

> With each uclass having numbered devices we can ask for SPI port 0 or
> serial port 1 and receive a single device.

That's nice, but we should allow them to be named by their actual
names as found in the device tree too.

> Devices typically request a sequence number using aliases in the device
> tree. These are resolved when the device is probed, to deal with conflicts.
> Sequence numbers need not be sequential and holes are permitted.

So they are unreliably unpredictable, unless you also happen
to have the DTS decoder ring in hand too?


> +This indicates that in the uclass called "serial", the named node
> +("/serial@2223") will be given sequence number 2. Any command or driver
> +which requests serial device 2 will obtain this device.
> +
> +Some devices represent buses where the devices on the bus are numbered or
> +addressed. For example, SPI typically numbers its slaves from 0, and I2C
> +uses a 7-bit address. In these cases the 'reg' property of the subnode is
> +used, for example:
> +
> +{
> +   aliases {
> +   spi2 = "/spi@2230";
> +   };
> +
> +   spi@2230 {
> +   #address-cells = <1>;
> +   #size-cells = <1>;
> +   spi-flash@0 {
> +   reg = <0>;
> +   ...
> +   }
> +   eeprom@1 {
> +   reg = <1>;
> +   };
> +   };

And not everyone agrees that this is the best approach, even in Linux land.
Specifically, we should be in agreement with Linux, and we should not
have our DTS stray from the definitions that Linux will accept for the same
devices.  And this approach won't be bought by the Linux crowd.  (Yes,
there are some that use  a "reg = <0>;" approach here, but there are many
that do not too.  It's not a universally accepted approach.)

This concept is crucial.

I've said it before, and I will say it again if needed.

So:  Sure, put this approach in, but make it be the backward compatible
approach.  Also please put in a correct naming approach so that we can
move forward with a better longer term solution too.

Thanks,
jdl
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 17/29] dm: Introduce device sequence numbering

2014-07-08 Thread Simon Glass
In U-Boot it is pretty common to number devices from 0 and access them
on the command line using this numbering. While it may come to pass that
we will move away from this numbering, the possibility seems remote at
present.

Given that devices within a uclass will have an implied numbering, it
makes sense to build this into driver model as a core feature. The cost
is fairly small in terms of code and data space.

With each uclass having numbered devices we can ask for SPI port 0 or
serial port 1 and receive a single device.

Devices typically request a sequence number using aliases in the device
tree. These are resolved when the device is probed, to deal with conflicts.
Sequence numbers need not be sequential and holes are permitted.

At present there is no support for sequence numbers using static platform
data. It could easily be added to 'struct driver_info' if needed, but it
seems better to add features as we find a use for them, and the use of -1
to mean 'no sequence' makes the default value somewhat painful.

Signed-off-by: Simon Glass 
---

Changes in v2: None

 doc/driver-model/README.txt  | 101 ---
 drivers/core/device.c|  28 
 drivers/core/uclass.c|  78 +
 include/dm/device.h  |  29 +
 include/dm/uclass-internal.h |  23 ++
 include/dm/uclass.h  |  31 +
 test/dm/test-fdt.c   |  54 ++-
 test/dm/test.dts |  11 -
 8 files changed, 347 insertions(+), 8 deletions(-)

diff --git a/doc/driver-model/README.txt b/doc/driver-model/README.txt
index 4858281..4ac6287 100644
--- a/doc/driver-model/README.txt
+++ b/doc/driver-model/README.txt
@@ -95,12 +95,16 @@ are provided in test/dm. To run them, try:
 You should see something like this:
 
 <...U-Boot banner...>
-Running 14 driver model tests
+Running 15 driver model tests
 Test: dm_test_autobind
 Test: dm_test_autoprobe
 Test: dm_test_children
 Test: dm_test_fdt
+Device 'd-test': seq 3 is in use by 'b-test'
 Test: dm_test_fdt_pre_reloc
+Test: dm_test_fdt_uclass_seq
+Device 'd-test': seq 3 is in use by 'b-test'
+Device 'a-test': seq 0 is in use by 'd-test'
 Test: dm_test_gpio
 sandbox_gpio: sb_gpio_get_value: error: offset 4 not reserved
 Test: dm_test_leak
@@ -339,6 +343,80 @@ numbering comes from include/dm/uclass.h. To add a new 
uclass, add to the
 end of the enum there, then declare your uclass as above.
 
 
+Device Sequence Numbers
+---
+
+U-Boot numbers devices from 0 in many situations, such as in the command
+line for I2C and SPI buses, and the device names for serial ports (serial0,
+serial1, ...). Driver model supports this numbering and permits devices
+to be locating by their 'sequence'.
+
+Sequence numbers start from 0 but gaps are permitted. For example, a board
+may have I2C buses 0, 1, 4, 5 but no 2 or 3. The choice of how devices are
+numbered is up to a particular board, and may be set by the SoC in some
+cases. While it might be tempting to automatically renumber the devices
+where there are gaps in the sequence, this can lead to confusion and is
+not the way that U-Boot works.
+
+Each device can request a sequence number. If none is required then the
+device will be automatically allocated the next available sequence number.
+
+To specify the sequence number in the device tree an alias is typically
+used.
+
+aliases {
+   serial2 = "/serial@2223";
+};
+
+This indicates that in the uclass called "serial", the named node
+("/serial@2223") will be given sequence number 2. Any command or driver
+which requests serial device 2 will obtain this device.
+
+Some devices represent buses where the devices on the bus are numbered or
+addressed. For example, SPI typically numbers its slaves from 0, and I2C
+uses a 7-bit address. In these cases the 'reg' property of the subnode is
+used, for example:
+
+{
+   aliases {
+   spi2 = "/spi@2230";
+   };
+
+   spi@2230 {
+   #address-cells = <1>;
+   #size-cells = <1>;
+   spi-flash@0 {
+   reg = <0>;
+   ...
+   }
+   eeprom@1 {
+   reg = <1>;
+   };
+   };
+
+In this case we have a SPI bus with two slaves at 0 and 1. The SPI bus
+itself is numbered 2. So we might access the SPI flash with:
+
+   sf probe 2:0
+
+and the eeprom with
+
+   sspi 2:1 32 ef
+
+These commands simply need to look up the 2nd device in the SPI uclass to
+find the right SPI bus. Then, they look at the children of that bus for the
+right sequence number (0 or 1 in this case).
+
+Typically the alias method is used for top-level nodes and the 'reg' method
+is used only for buses.
+
+Device sequence numbers are resolved when a device is probed. Before then
+the sequence number is only