[U-Boot-Users] [Patch 5/9]U-boot-V2:cmd: add I2C commands

[Menon, Nishanth]

Sascha,
> -----Original Message-----
> From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> Sent: Friday, June 20, 2008 5:29 AM
> To: Menon, Nishanth
> Cc: u-boot-users at lists.sourceforge.net
> Subject: Re: [Patch 5/9]U-boot-V2:cmd: add I2C commands
>
> Some overall comments first.
>
> You seem to register a device for each bus and do all access to slave
> devices through the bus. It would be better if we register a new device
> for each slave device.
No. that is not a good strategy. Many reasons why not to do it as the cmds
are a debug tool:
a) I am not interested in registering camera and all devices I may have in
code. Yet, the handy little tool is good to do some basic debug of the device.
b) Every client behaves in a different way. Camera for example may follow CCI
specification, in which case, they would have 8, 16 ,32 bit regs all in the same
device. In fact it may be near impossible to write a generic driver for them all.
We cannot predict what we want to do for every device out there.
c) Devices wanting to expose their own app may choose to do so. Yes, I get the
comment:
> Please resist to introduce a new command for each type of device.
d) It is wasteful to generate multiple device nodes when someone smart enough to
understand I2C specification will also know that the common denominator for all
devices is the I2C bus.
e) Creating device node for each client node requires extensive client mechanism
in I2C which has no "use case" for me.
f) 7 bit addressing allows 127 devices, and I have 3 busses. 127*3 device nodes as
a theoretical max is outrageous. When just 3 device nodes could have done the same job.
In short, the common access mechanism of exposing the bus and allowing users to
Do i2c accesses to the devices in the manner of choosing would be the most generic
Method to do it. In fact U-Boot v1  also does something similar.


> Slave devices could either be hardwired by a
> board setup or, if the board maintainer or user is brave, through an
> autodetect mechanism.
> Once we have a device for each slave most of this patch can be removed
> since you can use the generic md/mm commands for reading registers. In
> the device tree this could look like this:
>
> /dev/i2cbus0                    # bus 0
>         /dev/i2cbus0_50         # device 0x50 on bus 0...
>                 /dev/eeprom0    # which happens to be an eeprom
A) who would decide this would be a eeprom?
B) why should /dev/i2cbus0_50 be any different from /dev/eeprom?
C) you are looking at 3 different device nodes when a single one would suffice.

Note that complex development platforms like mine may have upto 30-40 devices on the bus.
I have power monitoring ADC chips on buses which I really don’t care about during boot, yet
May want to debug intermittently.

What do we achieve by introducing i2c device based nodes?

Will look at comments if we are aligned on this basic infrastructure. (overall I think the
Rest of the comments are valid, even though I'd wish you'd stop putting the entire patch
In the mail.. lots of scrolling down to reach your comment ;) )..

Regards,
Nishanth Menon