Hi Masahiro,

On 19 September 2014 07:22, Masahiro Yamada yamada.m@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 "register"ing 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 understand well the difference between "platdata" and "priv" (private data).

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

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().

[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

The driver cannot operate without some idea of what it is attached to. For example you might have a serial driver which 6 lots of platform data, each with a different register address in the memory map. As a result you end up with 6 serial devices.

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".

That should be driver name of course :-(

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.

Yes, each one of these creates a new device instance, by providing some platform data and a driver name.

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.

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?

Yes

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!

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.

Indeed. Bear in mind that it is still fairly early days for driver model in U-Boot. My intention is to get SPI in soon, and then widen out the support to a few more ARM SoCs. I am still finding problems and needed tweaks with each new area.

Thanks very much for looking at this. If you are able to suggest doc changes it would be very helpful. Unfortunately once you understand something it is too late to come up with these sorts of perspectives.

Regards, Simon