Sascha, > -----Original Message----- > From: Sascha Hauer [mailto:[EMAIL PROTECTED] > Sent: Friday, June 20, 2008 5:29 AM > To: Menon, Nishanth > Cc: [email protected] > 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 ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://sourceforge.net/services/buy/index.php _______________________________________________ U-Boot-Users mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/u-boot-users
