From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sascha Hauer Date: Fri, 20 Jun 2008 17:16:49 +0200 Subject: [U-Boot-Users] [Patch 5/9]U-boot-V2:cmd: add I2C commands In-Reply-To: <7A436F7769CA33409C6B44B358BFFF0CD32122AE@dlee02.ent.ti.com> References: <20080618155822.GF6603@pengutronix.de> <7A436F7769CA33409C6B44B358BFFF0CD3192E2E@dlee02.ent.ti.com> <20080618183806.GK1189@pengutronix.de> <7A436F7769CA33409C6B44B358BFFF0CD3192FBC@dlee02.ent.ti.com> <20080618191142.GN1189@pengutronix.de> <7A436F7769CA33409C6B44B358BFFF0CD3193101@dlee02.ent.ti.com> <20080618204407.GR1189@pengutronix.de> <7A436F7769CA33409C6B44B358BFFF0CD3211BBD@dlee02.ent.ti.com> <20080620102856.GI1479@pengutronix.de> <7A436F7769CA33409C6B44B358BFFF0CD32122AE@dlee02.ent.ti.com> Message-ID: <20080620151649.GA28397@pengutronix.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Fri, Jun 20, 2008 at 08:35:46AM -0500, Menon, Nishanth wrote: > 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: Just like md/mm are > 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. Lets see: +int read_i2c(int file, uchar daddr, u16 addr, u8 reg_width, char *buffer, + size_t count) +int write_i2c(int file, uchar daddr, u16 addr, u8 reg_width, char *buffer, + size_t count) These arguments map nicely to md/mm > 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. I do not understand why this is wasteful. It may be wasteful in terms of malloc space, but all boards I know have more than enough memory for the bootloader. > 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. I did not mean for every possible device but for each device that is actually present, or even from the present ones only the ones which are of use for u-boot. If you want to access a device which you know is present but U-Boot does not know about it, then add a command like i2c add /dev/i2cbus0 0x50 which would create the device node that you need to access device 0x50 on i2cbus0. > 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? The board setup code in this case as eeproms are not probable. > B) why should /dev/i2cbus0_50 be any different from /dev/eeprom? issuing md on /dev/i2cbus0_50 would let you read the registers of the chip, which of course happen to be the contents of the eeprom, so this might be a bad example. Consider an A/D converter; reading /dev/i2cbus0_xy would give you the register space while /dev/adxy would give you the digital values (provided a client driver is present). Thus /dev/adxy would require a driver whereas /dev/i2cbus0_xy does not. > 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? - An overview over the system for those who are not familiar with it. You may know the devices that are connected to the bus on your board, I don't. One of the benefits in U-Boot-V2 for me is that many different systems behave similarly although they have a very different hardware. - when md/mm are usable on a device it also means that cp/memcmp/cat or any other file command is usable. - reduction in code size since you can reuse what's already there (like file commands instead of new commands). That's one of the points that make U-Boot-V1 bigger than it has to be. > > 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 ;) ).. You are lucky, I already had to remove some lines as I hit the list limit ;) Regards, Sascha -- Pengutronix e.K. - Linux Solutions for Science and Industry ----------------------------------------------------------- Kontakt-Informationen finden Sie im Header dieser Mail oder auf der Webseite -> http://www.pengutronix.de/impressum/ <-