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

[Sascha Hauer <s.hauer@pengutronix.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/ <-