[lm-sensors] [RFC] adding back i2c_get_clients

[lm-sensors] [RFC] adding back i2c_get_clients

[Alessandro Zummo]

 Hello,

  I'd like to restart a topic that was discussed before, regarding the infrastructure
 required to send a command/ioctl directly to an i2c client.

  The topic was discussed in [1] and [2] more than one year ago. This missing
 functionality led to all sort of tricks in both i2c rtc drivers and media drivers.

  My previous proposal ([3]) addressed the separation between ioctls and
 commands. We now need a way to actually send those commands to the
 device we are interested in.

  I'd like to introduce one or more functions to be able
 to retrieve a client based on its driver id and/or i2c class.

  If there's general consensus, I'd be happy to take care of it.

[1] http://marc.theaimsgroup.com/?t\x108307854200001&r=1&w=2
[2] http://marc.theaimsgroup.com/?t\x108324800400004&r=1&w=2
[3] http://lists.lm-sensors.org/pipermail/lm-sensors/2005-November/014343.html

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Turin, Italy

  http://www.towertech.it

[lm-sensors] [RFC] adding back i2c_get_clients

[Greg KH]

On Thu, Nov 17, 2005 at 12:33:30AM +0100, Alessandro Zummo wrote:
> 
>  Hello,
> 
>   I'd like to restart a topic that was discussed before, regarding the infrastructure
>  required to send a command/ioctl directly to an i2c client.

No, we don't want to allow new ioctls to be created/used.  Please use
other methods for this.

>   The topic was discussed in [1] and [2] more than one year ago. This missing
>  functionality led to all sort of tricks in both i2c rtc drivers and media drivers.

What exactly is the problem you are trying to solve?

thanks,

greg k-h

[lm-sensors] [RFC] adding back i2c_get_clients

[Alessandro Zummo]

On Wed, 16 Nov 2005 23:40:54 -0800
Greg KH <greg@kroah.com> wrote:

> On Thu, Nov 17, 2005 at 12:33:30AM +0100, Alessandro Zummo wrote:
> > 
> >  Hello,
> > 
> >   I'd like to restart a topic that was discussed before, regarding the infrastructure
> >  required to send a command/ioctl directly to an i2c client.
> 
> No, we don't want to allow new ioctls to be created/used.  Please use
> other methods for this.

 You either send a command or an ioctl. To me, it doesn't really matter what
 I'm sending, as long as:

 a) I have a way to send a command directly to my device driver
 b) my device driver will not receive a command it is not supposed to receive, not
 like the i2c_clients_command is currently doing. ioctls would help here.

 Also, I don't want to create new ioctls, just to use existing ones. All
 the media drivers are currently using ioctls.

> >   The topic was discussed in [1] and [2] more than one year ago. This missing
> >  functionality led to all sort of tricks in both i2c rtc drivers and media drivers.
> 
> What exactly is the problem you are trying to solve?

 a) I would like to send specific commands to my driver without having
 to export a function to do that.

 b) I want to have drivers of the same class to respond to a common command
 subset, so that, for example, every i2c driver which is compatible
 with I2C_CLASS_RTC could be used in the same way from the higher layers.

 c) trying to understand and document the way i2c_driver->command should
 be used/abused.

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Turin, Italy

  http://www.towertech.it

[lm-sensors] [RFC] adding back i2c_get_clients

[Jean Delvare]

Hi Alessandro,

> > What exactly is the problem you are trying to solve?
>
> a) I would like to send specific commands to my driver without having
> to export a function to do that.

If the command is really specific, I see no reason why exporting a
function would be avoided.

> b) I want to have drivers of the same class to respond to a common command
> subset, so that, for example, every i2c driver which is compatible
> with I2C_CLASS_RTC could be used in the same way from the higher layers.

The fact that your RTC chip is I2C-based is an implementation detail. If
you want a command to work on all RTC drivers, you certainly do not want
to rely on i2c-core in any way. Better have some rtc-core driver (if it
doesn't already exist) and have your RTC driver register with it.

Also, please keep in mind that I2C_CLASS_* is meant to limit the extent
of automatic chip probing. Only i2c adapters and drivers which want to
take part in the automatic chip probing (which is the only way right
now, but this should change soon) should define .class. You should not
expect *all* RTC chip drivers based on I2C to define it. In fact, I even
expect .class to disappear in the long run, once alternative probing
methods have been implemented and deployed.

> c) trying to understand and document the way i2c_driver->command should
> be used/abused.

/deleted?

--
Jean Delvare

[lm-sensors] [RFC] adding back i2c_get_clients

[Alessandro Zummo]

On Thu, 17 Nov 2005 11:21:41 +0100 (CET)
"Jean Delvare" <khali@linux-fr.org> wrote:

> > a) I would like to send specific commands to my driver without having
> > to export a function to do that.
> 
> If the command is really specific, I see no reason why exporting a
> function would be avoided.

 abstraction. If we can abstract some rtc specific commands, less code will have
 to be written and maintaining it will be easier.


> The fact that your RTC chip is I2C-based is an implementation detail. If
> you want a command to work on all RTC drivers, you certainly do not want
> to rely on i2c-core in any way. Better have some rtc-core driver (if it
> doesn't already exist) and have your RTC driver register with it.

 That would bind the i2c driver definitely with its rtc counterpart.
 as I have said, rtc are not always rtcs. they may have memory, they may
 be used or not as a system rtc, thay may be timers. Should I have

 rtc-generic -> rtc-mychip -> i2c-myrtcchip *repeated* for each 
 i2c rtc chip?

 that makes no sense to me.

 I'd rather prefer having

 rtc-i2c -> i2c-anyrtcchip .

 This way, the i2c part could also be used to do other things on the rtc. If one
 needs to use it as the system rtc, you can simply modprobe the generic
 rtc i2c driver.


> > c) trying to understand and document the way i2c_driver->command should
> > be used/abused.

 I don't think the drivers/media people would be happy :) Most drivers implements
 a common subset of commands, which I think it's fair. Should each driver export
 a specific function? Should each caller have specific knowledge of the exact
 type of chip he is going to call? Certainly doable, but what a mess to maintain.

 I don't expect the i2c core to do anything specific, but I need to be sure
 all the i2c drivers conform to a standard, whatever that standard is.

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Turin, Italy

  http://www.towertech.it

[lm-sensors] [RFC] adding back i2c_get_clients

[Jean Delvare]

Hi Alessandro,

On 2005-11-17, Alessandro Zummo wrote:
> > If the command is really specific, I see no reason why exporting a
> > function would be avoided.
>
> abstraction. If we can abstract some rtc specific commands, less code
> will have to be written and maintaining it will be easier.

OK, so what you call "specific", I call "generic" ;) Sure, exporting
functions is not suitable for a generic interface. You want to either
define an ioctl-like interface (f(target, command, &parameter), but Greg
doesn't seem to be happy with that solution?), or a structure with
attributes and function pointers to specific implementations of the
interface. Something like:

struct rtc_operations
{
        int(* set_time)(struct rtc_time),
        int(* get_time)(strcut rtc_time *),
}

> > The fact that your RTC chip is I2C-based is an implementation detail.
> > If you want a command to work on all RTC drivers, you certainly do
> > not want to rely on i2c-core in any way. Better have some rtc-core
> > driver (if it doesn't already exist) and have your RTC driver register
> > with it.
>
> That would bind the i2c driver definitely with its rtc counterpart.

Exactly the contrary. There would no more be a specific rtc counterpart,
or at least it would no more bound directly to the i2c part. This is the
point of having a core and well-defined interfaces.

> as I have said, rtc are not always rtcs. they may have memory, they may
> be used or not as a system rtc, thay may be timers. Should I have
>
> rtc-generic -> rtc-mychip -> i2c-myrtcchip *repeated* for each
> i2c rtc chip?
>
> that makes no sense to me.
>
> I'd rather prefer having
>
> rtc-i2c -> i2c-anyrtcchip .

And I'd rather have:

platform-rtc <-> rtc-core <-> rtc-i2c

What you call "rtc counterpart of the rtc-i2c driver" is actually
platform-specific. The same RTC chip could be used as the system RTC on
one system, and used differently on other systems. We don't want to
write as many drivers as platforms for the same chip.

There is an additional difficulty due to the lack of possible
indentification of I2C-based RTC chips. The "rtc-i2c" driver above may
need platform-specific data. This is being discussed in a different
thread on this list.

All an rtc-i2c driver needs to do is offer a standard interface to get,
and set, the date and/or time. And maybe other things (alarms?). It does
not need to know how these functions will be used.

> This way, the i2c part could also be used to do other things on the rtc.

This is true regardless of how it interfaces with the hypothetical
rtc-core or "rtc-counterpart" or anything else.

> If one needs to use it as the system rtc, you can simply modprobe the
> generic rtc i2c driver.

There is no such thing as a "generic rtc i2c driver", and won't ever
be. I2C is really only the transport layer here. Whatever you want to
implement here doesn't belong to the i2c-core in any way.

> I don't expect the i2c core to do anything specific, but I need to be
> sure all the i2c drivers conform to a standard, whatever that standard
> is.

Again, no. You want to be sure that all *RTC* drivers conform to a
standard. I am very surprised that it ain't the case already, given the
large number of rtc drivers which already exist.

Whatever, we should really implement the alternative driver attaching
method which was discussed earlier before doing anything else, because
once we have that, it will probably help us think of better solutions to
all the other i2c-related problems which are currently being discussed
here and there. I will try to work on this again as soon as possible. Oh
my, why am I so short of time all the time? :(

Thanks,
--
Jean Delvare

[lm-sensors] [RFC] adding back i2c_get_clients

[Alessandro Zummo]

On Thu, 17 Nov 2005 12:49:14 +0100 (CET)
"Jean Delvare" <khali@linux-fr.org> wrote:

> OK, so what you call "specific", I call "generic" ;) Sure, exporting
> functions is not suitable for a generic interface. You want to either
> define an ioctl-like interface (f(target, command, &parameter), but Greg

 exactly. or, at least, document what i2c_driver->command should be used
 for. If it can be used for ioctls, and only for them, then we have no problems.

 If we can use command for whatever we want, the i2c_clients_commands
 must be removed, otherwise we are going to have oopses.

 It's just a matter of writing things down.



> > That would bind the i2c driver definitely with its rtc counterpart.
> 
> Exactly the contrary. There would no more be a specific rtc counterpart,
> or at least it would no more bound directly to the i2c part. This is the
> point of having a core and well-defined interfaces.

 I agree, but we still have to define the interface between the i2c_driver
 and the external world, as i2c_driver->command is not adeguately documented.



> platform-rtc <-> rtc-core <-> rtc-i2c
> 
> What you call "rtc counterpart of the rtc-i2c driver" is actually
> platform-specific. The same RTC chip could be used as the system RTC on
> one system, and used differently on other systems. We don't want to
> write as many drivers as platforms for the same chip.

 I agree. I think we are saying similar things with different
 terms.. Let's do an example with the X1205: 

 if you want to use it you must

 modprobe x1205

 if you want to _also_ use it as the system RTC, you should
 _also_

 modprobe rtc-i2c-interfacing-driver

 which will take care of interfacing x1205 with the kernel rtc 
 subsytem.

 Do we agree here?


-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Turin, Italy

  http://www.towertech.it

[lm-sensors] [RFC] adding back i2c_get_clients

[Jean Delvare]

Hi Alessandro,

On 2005-11-17, Alessandro Zummo wrote:
> I agree, but we still have to define the interface between the i2c_driver
> and the external world, as i2c_driver->command is not adeguately
> documented.

I think you should just forget about ->command. It's there for
historical reasons, but all in all doesn't make much sense that I can
see. It's kind of a bold statement at this point, but I think ->command
is likely to be removed in the future. I would need to spend more time
investigating the exact implementation and uses of it before going on
about this though, and such extra time I unfortunately don't have right
now.

> I agree. I think we are saying similar things with different
> terms.. Let's do an example with the X1205:
>
> if you want to use it you must
>
> modprobe x1205
>
> if you want to _also_ use it as the system RTC, you should
> _also_
>
> modprobe rtc-i2c-interfacing-driver
>
> which will take care of interfacing x1205 with the kernel rtc
> subsytem.
>
> Do we agree here?

Probably not. My views are more like:

* The platform code needs x1205, so it loads it if needed.
Platform-specific data can be used to hint x1205 on what adapter,
address pair it should attach to.

* When loaded, x1205 registers with i2c-core (as it already does) and
rtc-core or whatever this will be called.

* The platform code then accesses x1205 through rtc-core, and only this
way.

But I probably better stop here, as I am not the one who will write that
code, at least not within the next few months. Most of the problem
really doesn't depend on i2c, so I will happily leave the discussion
and decision to others.

Thanks,
--
Jean Delvare

[lm-sensors] [RFC] adding back i2c_get_clients

[Alessandro Zummo]

On Thu, 17 Nov 2005 14:20:04 +0100 (CET)
"Jean Delvare" <khali@linux-fr.org> wrote:

> I think you should just forget about ->command. It's there for
> historical reasons, but all in all doesn't make much sense that I can
> see. It's kind of a bold statement at this point, but I think ->command
> is likely to be removed in the future. I would need to spend more time
> investigating the exact implementation and uses of it before going on
> about this though, and such extra time I unfortunately don't have right
> now.

 I can tell you about the two kind of uses I've seen until now:

 a) media drivers use it to send ioctls.
 b) other drivers (rtcs) use it to send non-ioctl messages.

 If an "other driver" receives an ioctl via i2c_clients_commands
 an oops is likely to happen.

 If you don't give the media drivers a way to send ioctl-like commands
 to theri chips, they will star exporting tons of function.. I agree
 that you can't stop the caos in the universe, but that is like
 throwing an atomic bomb in the middle of the caos...


> > Do we agree here?
> 
> Probably not. My views are more like:
> 
> * The platform code needs x1205, so it loads it if needed.
> Platform-specific data can be used to hint x1205 on what adapter,
> address pair it should attach to.

 I agree.

 
> * When loaded, x1205 registers with i2c-core (as it already does) and
> rtc-core or whatever this will be called.

 If it registers with "rtc-core", you will be forced to use
 it as rtc, right? Otherwise I should have an option
 array which specifies which instance of the client
 should register and which not, because I may have two
 rtcs (ipotetically) connected.

 That's doable, but I would rather preferred to have
 only the portion of the driver that is strictly relevant
 to the I2C in i2c/chips.

 That would also mean tha more code will be repeated across
 different rtc drivers, code that will be the 95% the same, maybe
 except for function names.


> But I probably better stop here, as I am not the one who will write that
> code, at least not within the next few months. Most of the problem
> really doesn't depend on i2c, so I will happily leave the discussion
> and decision to others.

 Just let me know who will have to decide :)

 I need to address this situation in the near future. If someone decides,
 I will be happy to write code _according_ to the decision. If nobody
 decides, nothing will change.

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Turin, Italy

  http://www.towertech.it

[lm-sensors] [RFC] adding back i2c_get_clients

[Greg KH]

On Thu, Nov 17, 2005 at 03:04:15PM +0100, Alessandro Zummo wrote:
>  If you don't give the media drivers a way to send ioctl-like commands
>  to theri chips, they will star exporting tons of function.. I agree
>  that you can't stop the caos in the universe, but that is like
>  throwing an atomic bomb in the middle of the caos...

Good, exporting functions is the proper way to do things like this.
Like Jean showed, a simple set of function pointers is the correct way.
We want type-safe checking and explicit interfaces like this.  Not
wide-open, unchecked, and possibly unsafe interfaces like ioctls.

That's why I, and the other kernel developers are trying to prevent any
future ioctls being added.

thanks,

greg k-h

[lm-sensors] [RFC] adding back i2c_get_clients

[Alessandro Zummo]

On Thu, 17 Nov 2005 08:34:46 -0800
Greg KH <greg@kroah.com> wrote:

> Good, exporting functions is the proper way to do things like this.
> Like Jean showed, a simple set of function pointers is the correct way.
> We want type-safe checking and explicit interfaces like this.  Not
> wide-open, unchecked, and possibly unsafe interfaces like ioctls.
> 
> That's why I, and the other kernel developers are trying to prevent any
> future ioctls being added.

 Ok, I'll work in that direction. Should I thus consider
 i2c_driver->command to be deprecated?

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Turin, Italy

  http://www.towertech.it

[lm-sensors] [RFC] adding back i2c_get_clients

[Greg KH]

On Thu, Nov 17, 2005 at 07:23:12PM +0100, Alessandro Zummo wrote:
> On Thu, 17 Nov 2005 08:34:46 -0800
> Greg KH <greg@kroah.com> wrote:
> 
> > Good, exporting functions is the proper way to do things like this.
> > Like Jean showed, a simple set of function pointers is the correct way.
> > We want type-safe checking and explicit interfaces like this.  Not
> > wide-open, unchecked, and possibly unsafe interfaces like ioctls.
> > 
> > That's why I, and the other kernel developers are trying to prevent any
> > future ioctls being added.
> 
>  Ok, I'll work in that direction. Should I thus consider
>  i2c_driver->command to be deprecated?

Yes.

thanks,

greg k-h