Re: Fundamental Design Flaw of the Device Driver Model?

Previous thread: Compiling the kernel with VMware by Alex Zajac on Thursday, August 21, 2008 - 9:02 pm. (2 messages)

Next thread: [Fwd: [Patch] sata_via.c: add support for VT8251, fix the internal chips issue and] by Jeff Garzik on Thursday, August 21, 2008 - 11:39 pm. (1 message)
From: Eric Miao
Date: Thursday, August 21, 2008 - 11:25 pm

Fundamental Design Flaw of the Device Driver Model?
===================================================

Sorry for the misleading subject, its purpose is to draw your attention :-)
The ideas below are preliminary and I hope I'm not making serious mistakes
here.

This question has actually been around in my mind for several months, when
I started to work on some devices with multiple functions. Specifically, a
Power Management IC (PMIC in short in the following text) usually includes
LEDs support (charging, indication...) audio, touch screen, power monitoring,
LDOs, DC-DC bucks, and possibly some others.

The initial two ideas came into my mind were:

   1. separate the functions into multiple devices, write a driver for each
      of these devices

   2. a big all-in-one driver

Pros and Cons

   Solution (1) is obviously more attractive: better modularization and
   structure, work can be incrementally done, individual patches can go
   through each subsystem's git tree, reviewed by dedicated people, and
   so on.

   One thing I'm not so happy with solution (1) is: you have to create
   intermediate devices in order that the dedicated sub-device drivers
   to match up. E.g.

   ==== pmic.c =============================================================

   ...

   pmic_add_subdevs(void)
   {
   	for all sub-devices on this pmic:
		pdev = platform_device_alloc(sub-device-name, sub-device-id)
		platform_device_add(pdev);
		...
   }

   pmic_probe(struct i2c_client *client)
   {
   	...

	pmic_add_subdevs();

	...
   }

   struct i2c_driver pmic_driver = {
	.probe	= pmic_probe,

	...
   }

   ==== pmic.c =============================================================

   ==== pmic-subdev.c ======================================================

   static int sub_device_driver_probe(struct platform_device *sub_device)
   {
   	... ...
   }

   static struct platform_driver pmic_subdev_driver = {
   	.name	= sub-device-name,
	.probe	= ...
From: Pavel Machek
Date: Friday, August 22, 2008 - 2:03 am

Memory is not _that_ expensive, and struct device is not that
big. Adding infrastructure to driver model for supporting this would
also cost you memory, this time in .text segment.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--

From: Eric Miao
Date: Friday, August 22, 2008 - 2:32 am

Actually I don't mind wasting additional memory for a better structure,
but option (1) isn't. And I just assume you have read to the end of thi
 mail, then you will see it's more like a concept change,no fundamental
change to the code itself.

Actually the reason to have two different kind of devices are obvious:

1. physical device - handles the bus related operations (read, write,
locking, I/O)
2. virtual device - handles the functionality (interface with the
upper framework)

With this separation, the same IP on different semiconductor can be
shared naturally. There are lot of such products esp in embedded market,
with multiple shared IPs within a single die.

Attached is a patch series to get you guys a feeling how the option
(1) will look like, and think again about the device-driver relationship,
the correctness of introducing an intermediate platform_device.
From: pHilipp Zabel
Date: Friday, August 22, 2008 - 4:29 am

Instead of allocating/adding the platform subdevices manually in
da903x_add_subdevs, couldn't you use the MFD (multi function device)
infrastructure that seems to be intended for this case? Also, I think
that the da9030 base driver would then better fit in drivers/mfd, not
drivers/i2c/chips (didn't Jean Delvare himself state that ideally
drivers/i2c/chips should be empty?).

regards
Philipp
--

From: Eric Miao
Date: Friday, August 22, 2008 - 5:56 pm

Actually, I don't mind keep it anywhere. The intermediate
platform_device is what I'd like to avoid mostly. Using my
own simple logic allows me to do that later.
--

From: Liam Girdwood
Date: Friday, August 22, 2008 - 4:17 am

They are not useless as most of my client devices _need_ some sort of
platform data to be passed on for their probe() e.g. my LED driver needs
to know it's brightness values, trigger etc. They also all need a PMIC
reference (passed in the platform_data) which they will need to call any

Fwiw, I've made the WM8350 I2C driver the parent. The I2C driver also

Ok, I basically have :-

                                 Platform devices
                                     |        |
                                     V        V
Platform Bus --> I2C controller --> PMIC +-> LED1
                                         +-> LED2
                                         +-> Audio
                                         +-> DCDC1
                                         +-> Backlight

I originally had a PMIC bus (like above) but decided it was easier just
to use the existing kernel infrastructure. My I2C PMIC driver just
registers the client platform_devices when it probes.

Cheers

Liam

--

From: Stefan Richter
Date: Friday, August 22, 2008 - 7:00 am

I didn't read and understand all of your post.  Nevertheless I dare to
comment on these last 3 points here, because they don't fully match what
we already have been dealing with for a long time now.  For example,
SCSI (including newer transport types over serial buses or networks)
deals with target devices which have children called logical units.
Logical units of the same target can have very different functionality
(= implement different command sets) while there is still a dependency
of the logical units on the target which provides them.  Other example:
 USB presumably deals with multi-function devices with some inner
structure and dependency too.  (I don't know details.)

Ditto FireWire:  There are "nodes" on a bus, and each node can have 0..n
"units".  The units can implement various different protocols.  During
the presence of a node on the bus, units on it can come and go at any
time.  (Our drivers handle this in a simplified manner though.)  The
ieee1394 driver stack and its future replacement, the firewire stack,
instantiate a struct device for each node, and a struct device for each
unit.  There are appropriate parent-child relationships between them.
(Furthermore, node devices are children of the respective bus controller
device.)

All these devices have the same bus_type.  The older ieee1394 core
distinguishes between node devices and unit devices by means of
different class devices.  The newer, leaner firewire core distinguishes
between them by means of different device_type.

There are high-level drivers which are bound to unit devices, but not to
node devices.  These drivers have the necessary knowledge of the
hierarchy though because their business with a unit device typically
also involves some dealings with the parent and grandparent.

(Types of the new stack:  struct fw_device and struct fw_unit in
drivers/firewire/fw-device.[hc],
types of the old stack:  struct node_entry and struct unit_directory in
drivers/ieee1394/nodemgr.[hc])

Anyway; there are ...
From: Jon Smirl
Date: Friday, August 22, 2008 - 7:33 am

On 8/22/08, Eric Miao <eric.y.miao@gmail.com> wrote:

Another option is making your own bus. If I understand your hardware
it effectively has an internal bus.

-- 
Jon Smirl
jonsmirl@gmail.com
--

From: Eric Miao
Date: Friday, August 22, 2008 - 6:02 pm

That's another option around, but it didn't solve my fundamental question
of, (e.g. an PCI card with multiple network interfaces and other functionality):

Why should I have to create an intermediate device provided that a
"struct net_device" already contains a "struct device"? And that
device-driver binding, parameter passing (platform_data), bus and
other functionalities of this "struct net_device" is not used while
that's used solely by that intermediate device (platform_device maybe)?

They should have perfectly been combined into a single virtual device.
--

From: Jon Smirl
Date: Friday, August 22, 2008 - 8:54 pm

First device represent the hardware with the PCI interface for the
card. Instantiating it creates a bus for the card. You then auto add
devices to this bus for each of the sub-devices on the card.  Auto add
is fine in this case since the sub-devices aren't optional.
A multifunction card really is a local private bus with multiple
devices on it. First device is a real device - it's the bus controller
for the multifunction card.

A stranger problem is encountered with audio hardware. The SOC CPU
code loads an i2s/ac97 driver. A generic driver for the audio codec is
also loaded. Now you have to create a strange "fabric" device which
represent the specific PCB the generic codec and SOC have been
soldered into. The fabric driver describes how the codec and CPU are
wired together and if all of the codec functions are brought out to
jacks. For example, you don't want to display an ALSA capture device
if the codec supports it but no mic-in jack has been soldered to the
PCB. Is the fabric device a real device? You can't program it but you
can't get rid of it either.

-- 
Jon Smirl
jonsmirl@gmail.com
--

Previous thread: Compiling the kernel with VMware by Alex Zajac on Thursday, August 21, 2008 - 9:02 pm. (2 messages)

Next thread: [Fwd: [Patch] sata_via.c: add support for VT8251, fix the internal chips issue and] by Jeff Garzik on Thursday, August 21, 2008 - 11:39 pm. (1 message)