Re: [U-Boot] Many questions about U-Boot's Driver Model

2014-09-22 Thread Simon Glass
Hi Masahiro,

On 20 September 2014 00:56, Masahiro YAMADA yamad...@jp.panasonic.com wrote:
 Hi Simon,



 2014-09-20 7:25 GMT+09:00 Simon Glass s...@chromium.org:

 - the bind/unbind allows you to have devices which are known to the
 system, but not yet probed. This is important in a boot loader (e.g.
 we don't want to probe devices until they are actually needed, and
 some will never be probed during a boot), although not really
 necessary in an OS

 Uh, this view was missing from my mind.
 It is nice for faster boot.



Yes and it does make quite a difference in some cases.



 Platform data is bound to the driver to create an new device (instance
 of the driver, if you prefer). Linux has the same concept. See for
 example struct platform_device_info.

 Private data is used by that device to hold its information while it
 is active. Linux devices have this, normally in the -p-driver_data
 member,. See dev_get_drvdata().

 Now it is clearer.

 In Linux, platform looks like a bus,
 but in U-Boot it is implemented in the basic infrastructure
 (and it seems reasonable enough)



Yes.




 [4] What does struct driver_info stand for?

 It is the information used to bind platform data to a driver to
 produce a device. See Linux's struct platform_device which is similar
 (although in Linux the device is included in the structure).


 I cannot understand the following comment block at all.

 /**
  * struct driver_info - Information required to instantiate a device
  *
  * @name:   Device name
  * @platdata:   Driver-specific platform data
  */
 struct driver_info {
 const char *name;
 const void *platdata;
 };



 Does this structure give information of a driver or a device?

 driver + platform data = device


 Sorry, I am still not convinced with this part.


 In my mind, things happen in the following step:

 [1] Every driver is instantiated by U_BOOT_DRIVER()
 [2] Every device is instantiated by U_BOOT_DEVICE() (= struct driver_info)

s/instantiated/described/ - they are just tables in memory at this stage.

 [3] If the system finds a driver and a device with the same name, they are 
 bound


 [1] and [2] occur at the build time
 [3] occurs at run time  (more exactly, when dm_init_and_scan()
 function is called)


 At the build time (i.e.  [3] hasn't happened yet), struct driver_info
 is information belonging to a *device*.
 (This is why I thought struct driver_info should be more likely
 struct device_info.)


 After [3] happens, you are right, the device's information is attached
 to the driver
 and driver + platform data = device.
 That's what binding is for.

 Eventually, your explanation in pointing to the same one in my mind.
 But there is a slight difference of a nuance.

 In my understanding, binding is an action that couples a driver and
 a device together
 by finding the same name.   (In the device tree world, by finding
 compatible ones)

 Mixing up a driver and a device results in confusion.



Well I'm not going to disagree with you about the confusion. But
'device' as in 'struct udevice' is the term which means a single
instance of a driver. This same term is used in U-Boot and Linux.

So I would not say that binding couples a driver and a device, rather
that binding couples a driver and its information, to produce a
device.




 I am wordering if struct driver_info should have been named struct 
 device_info?


 Possibly, although that might be confusing also, since it is not
 really information about a device. The idea is that it is the info
 needed for a driver to instantiate a new device.

 Indeed, it is information about a device.
 because .platdata is an attribute of each *device*, right?


 I am for statements:
   - It is information of a device
   - It is information for a driver to bind this device

I like this one ^^


 I am against a statement:
   - It is information of a driver


Agreed.



 I think comments and namings are confusing.


 Well it looks like you have found some errors in the comments - we
 should get those fixed. If you have time, please send a patch!

 I do not mind if it is a matter of accidental errors in comments. Send
 a patch and fix them, that's it.
 But I don't think so.

 I suspect the struct driver_info is confusing enough and these errors
 happened because of that.


OK. I'm resisting because I'm not sure that device_info is better,
particularly when we already have udevice. Looking at it another way,
driver_info is the info you need to find the driver so you can bind a
device to it. I'm happy to change it if we have something obviously
better.

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


Re: [U-Boot] Many questions about U-Boot's Driver Model

2014-09-20 Thread Masahiro YAMADA
Hi Simon,



2014-09-20 7:25 GMT+09:00 Simon Glass s...@chromium.org:

 - the bind/unbind allows you to have devices which are known to the
 system, but not yet probed. This is important in a boot loader (e.g.
 we don't want to probe devices until they are actually needed, and
 some will never be probed during a boot), although not really
 necessary in an OS

Uh, this view was missing from my mind.
It is nice for faster boot.




 Platform data is bound to the driver to create an new device (instance
 of the driver, if you prefer). Linux has the same concept. See for
 example struct platform_device_info.

 Private data is used by that device to hold its information while it
 is active. Linux devices have this, normally in the -p-driver_data
 member,. See dev_get_drvdata().

Now it is clearer.

In Linux, platform looks like a bus,
but in U-Boot it is implemented in the basic infrastructure
(and it seems reasonable enough)





 [4] What does struct driver_info stand for?

 It is the information used to bind platform data to a driver to
 produce a device. See Linux's struct platform_device which is similar
 (although in Linux the device is included in the structure).


 I cannot understand the following comment block at all.

 /**
  * struct driver_info - Information required to instantiate a device
  *
  * @name:   Device name
  * @platdata:   Driver-specific platform data
  */
 struct driver_info {
 const char *name;
 const void *platdata;
 };



 Does this structure give information of a driver or a device?

 driver + platform data = device


Sorry, I am still not convinced with this part.


In my mind, things happen in the following step:

[1] Every driver is instantiated by U_BOOT_DRIVER()
[2] Every device is instantiated by U_BOOT_DEVICE() (= struct driver_info)
[3] If the system finds a driver and a device with the same name, they are bound


[1] and [2] occur at the build time
[3] occurs at run time  (more exactly, when dm_init_and_scan()
function is called)


At the build time (i.e.  [3] hasn't happened yet), struct driver_info
is information belonging to a *device*.
(This is why I thought struct driver_info should be more likely
struct device_info.)


After [3] happens, you are right, the device's information is attached
to the driver
and driver + platform data = device.
That's what binding is for.

Eventually, your explanation in pointing to the same one in my mind.
But there is a slight difference of a nuance.

In my understanding, binding is an action that couples a driver and
a device together
by finding the same name.   (In the device tree world, by finding
compatible ones)

Mixing up a driver and a device results in confusion.





 I am wordering if struct driver_info should have been named struct 
 device_info?


 Possibly, although that might be confusing also, since it is not
 really information about a device. The idea is that it is the info
 needed for a driver to instantiate a new device.

Indeed, it is information about a device.
because .platdata is an attribute of each *device*, right?


I am for statements:
  - It is information of a device
  - It is information for a driver to bind this device

I am against a statement:
  - It is information of a driver



 I think comments and namings are confusing.


 Well it looks like you have found some errors in the comments - we
 should get those fixed. If you have time, please send a patch!

I do not mind if it is a matter of accidental errors in comments. Send
a patch and fix them, that's it.
But I don't think so.

I suspect the struct driver_info is confusing enough and these errors
happened because of that.



-- 
Best Regards
Masahiro Yamada
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] Many questions about U-Boot's Driver Model

2014-09-19 Thread Masahiro Yamada
Hi.

I am digging into the driver model.


I fear this triggers a controversy, but, first,
it looks like the driver model of U-Boot is pretty different from
the Linux's one, in spite of the fact that U-Boot has ported lots of
Linux sources and ideas (looks like U-Boot is getting a mirror of Linux)
and most of U-Boot developers are also working on Linux development.

This seems kind of unfortunate because
we have to study a new driver model in addition to Linux's one.
(Perhaps isn't it an obstacle in the future?)


[1] Why is usage (needed to be) different?

I expect the usage is something like this:

static int foo_init(void)
{
 return foo_driver_register(vendorA_foo_driver);
}
module_init(foo_init)


I mean, something is triggered by registering something
in each init function.




[2] Why uclass?

In Linux, struct device_driver is something like a superclass
of each driver such platform_driver, uart_driver, usb_driver.

Usually, struct foo_device includes(inherits) struct device
and struct foo_driver includes struct device_driver.



   ||  |--|
   ||  |  |
   | struct device  |  | struct device_driver |
   ||  |  |
   ||  |--|
  /|\   /|\
   | |   include (inherit)
   | |
   ||  |---|
   | struct |  | struct|
   |   platform_device  |  |   platform_driver |
   ||  |   |
   ||  |---|


struct uclass is a totally different approach.

For ex.

  int demo_hello(struct udevice *dev, int ch)

takes a pointer to struct udevice, but it must be the one of
DEMO class.

We can still pass a udevice belonging to GPIO class,
which the compiler never complains about, but it will crash at run time.

I'd like to know the philosophy in the background of this approach.



[3] Is platform driver(device) a special case?


I am not sure if I understood correctly,
but it looks like platform data in U-Boot is a special case.

I do not understand well the difference between
platdata  and priv (private data).

The platform driver(platform device) is derived from
device_driver(device), isn't it?



[4] What does struct driver_info stand for?

I cannot understand the following comment block at all.

/**
 * struct driver_info - Information required to instantiate a device
 *
 * @name:   Device name
 * @platdata:   Driver-specific platform data
 */
struct driver_info {
const char *name;
const void *platdata;
};



Does this structure give information of a driver or a device?

My first guess was the former because of the name driver_info.

But this comment says, the name member of struct drive_info
means Device name.

Moreover, U_BOOT_DEVICE macro is a shorthand of struct driver_info.


U_BOOT_DEVICE(demo0) = {
.name = demo_shape_drv,
.platdata = red_square,
};

U_BOOT_DEVICE(demo1) = {
.name = demo_simple_drv,
.platdata = red_square,
};

U_BOOT_DEVICE(demo2) = {
.name = demo_shape_drv,
.platdata = green_triangle,
};


looks like instances of devices, not drivers.

I am wordering if struct driver_info should have been named struct 
device_info?




I found another confusing part.


/**
 * lists_bind_drivers() - search for and bind all drivers to parent
 *
 * This searches the U_BOOT_DEVICE() structures and creates new devices for
 * each one. The devices will have @parent as their parent.
 *
 * @parent: parent driver (root)
 * @early_only: If true, bind only drivers with the DM_INIT_F flag. If false
 * bind all drivers.
 */
int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only)


This comment says, the parent argument is parent driver.
^^^
So, do you mean, struct udevice is a driver?

It is absolutely a device, right?

I think comments and namings are confusing.




Sorry if I am wrong.
Perhaps I am writing this mail without understaing the code correctly.
But I do not feel comfortable to move forward base on something I cannot 
understand.
Also, I want to be sure if this is the right direction we should go to
before lots of conversions occur.



Best Regards
Masahiro Yamada

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


Re: [U-Boot] Many questions about U-Boot's Driver Model

2014-09-19 Thread Simon Glass
Hi Masahiro,

On 19 September 2014 07:22, Masahiro Yamada yamad...@jp.panasonic.com wrote:
 Hi.

 I am digging into the driver model.


Great! It would be good to add patches to the driver model README.txt
file with some of the results of this thread.


 I fear this triggers a controversy, but, first,
 it looks like the driver model of U-Boot is pretty different from
 the Linux's one, in spite of the fact that U-Boot has ported lots of
 Linux sources and ideas (looks like U-Boot is getting a mirror of Linux)
 and most of U-Boot developers are also working on Linux development.

 This seems kind of unfortunate because
 we have to study a new driver model in addition to Linux's one.
 (Perhaps isn't it an obstacle in the future?)

I believe what you are seeing in general is more that some of the
things which are driver-specific in Linux are catered for in the
framework in U-Boot. For example:

- nearly every device has private state, in Linux you have to allocate
memory for it, although the devm_... functions try to make the patern
more common. In U-Boot it is a built-in feature, partly to reduce code
size. You can of course ignore this and do your own memory allocation
if you prefer
- many devices want to translate ofdata to platform data. In Linux you
call that as part of probe, but in U-Boot it is a separate method.
- the rename of struct device to struct udevice is actually supposed
to make it easier to port Linux code
- the bind/unbind allows you to have devices which are known to the
system, but not yet probed. This is important in a boot loader (e.g.
we don't want to probe devices until they are actually needed, and
some will never be probed during a boot), although not really
necessary in an OS



 [1] Why is usage (needed to be) different?

 I expect the usage is something like this:

 static int foo_init(void)
 {
  return foo_driver_register(vendorA_foo_driver);
 }
 module_init(foo_init)


 I mean, something is triggered by registering something
 in each init function.

This is done with platform data or device tree nodes. U-Boot does not
have modules, but if it does get them one day, the device would be
registered automatically when added. You can call device_bind() to
bind a new device, but in most cases you don't need to.





 [2] Why uclass?

 In Linux, struct device_driver is something like a superclass
 of each driver such platform_driver, uart_driver, usb_driver.

 Usually, struct foo_device includes(inherits) struct device
 and struct foo_driver includes struct device_driver.



||  |--|
||  |  |
| struct device  |  | struct device_driver |
||  |  |
||  |--|
   /|\   /|\
| |   include (inherit)
| |
||  |---|
| struct |  | struct|
|   platform_device  |  |   platform_driver |
||  |   |
||  |---|


 struct uclass is a totally different approach.

 For ex.

   int demo_hello(struct udevice *dev, int ch)

 takes a pointer to struct udevice, but it must be the one of
 DEMO class.

 We can still pass a udevice belonging to GPIO class,
 which the compiler never complains about, but it will crash at run time.

 I'd like to know the philosophy in the background of this approach.


It seems we need to expand the README on this point.

Linux effectively has the concept of a uclass, but it is not built
into the infrastructure. For example, if you have a RTC driver you
call rtc_device_register() and rtc_device_unregister(). But from
outside the RTC subsystem how do you find the RTC? How would you
implement a command to list the RTCs. Given a device tree node, how do
you find the device(s) associated with it? How do you implement a
command that wants to operate on device #2 out of the list (for any
given class of devices).

In fact Linux does have the concept of classes (see struct class), and
it almost feels like it is edging towards having some sort of
registration framework for them. But if so it is not there yet.

Re the struct udevice, you can of course create these sorts of
included structs, but I'm not sure what it buys you. Where did you get
the udevice from? Normally it would come from
uclass_first_device(UCLASS_DEMO, dev) or similar. Are you worried
about getting a device from one uclass and passing it to another? I
suppose we could add checks for that. I have certainly thought about
having magic numbers/datatype IDs in the structures for debugging
purposes, but have not implemented it.



 [3] Is platform driver(device) a special case?


 I am not sure if I understood correctly,
 but it looks like platform data in U-Boot is a special case.

 I do not