Re: [lm-sensors] [PATCH] Support for DS75 thermal sensor

Re: [lm-sensors] [PATCH] Support for DS75 thermal sensor

[Wolfgang Grandegger]

Hi Jean,

Jean Delvare wrote:
> Hi Wolfgang,
> 
> On Fri, 11 Jul 2008 14:40:58 +0200, Wolfgang Grandegger wrote:
>> Jean Delvare wrote:
>>> There's no "detection" involved for the new-style i2c devices.
>>> Presumably you are still using the old binding model, not taking
>>> benefit of the patch at all. Use i2c_register_board_info() to
>>> instantiate your "ds75" i2c device from you board's platform code, and
>>> it should work.
>> I have some general question on I2C device probing. Our board does have 
>> two I2C buses. On the first on, there is an RTC at addr 0x32 and a DS75 
>> at 0x4c. Both are defined in the Flattened Device Tree and therefore 
>> i2c_register_board_info() will be called for them. On the second bus, 
>> there are other I2C devices. Nevertheless, probing for RTC and DS75 at 
>> addr 0x32 and 0x4c will be performed also on that bus. Is this the 
>> normal/intended behavior? Can such probing be suppressed?
> 
> Probing isn't supposed to happen at all for RTC chips. There are a few
> drivers which remain to be converted though (rtc-ds1672, rtc-max6900,
> rtc-pcf8583.) If you use one of these and would like to help converting
> it, please tell me (and Alessandro Zummo.)

OK. We ported an old RTC driver for the Epson RX8025 for our board. 
Unfortunately, it did not use the new bindings but I fixed that in the 
meantime. It will be posted for kernel inclusion soon.

> For hwmon chips, probing only occurs if the i2c adapter accepts to be
> probed, by setting the I2C_CLASS_HWMON flag in i2c_adapter.class. If
> you do not want a given i2c bus to be probed, then do not set the flag
> for that bus.

I see. My problem is that I2C_CLASS_HWMON is set in the I2C bus driver 
for the MPC:

   http://lxr.linux.no/linux+v2.6.26/drivers/i2c/busses/i2c-mpc.c#L314

If I disable it, the LM75 driver only probes the addresses of the I2C 
nodes defined in the FDT DTS file. I wonder if this should not be the 
default behaviour for kernels using OF platform description. For this 
reason, I have put Linuxppc-dev ML on CC. I also realized, that there 
are some patches for OF I2C pending. I will check.

Wolfgang.

Re: [PATCH] Support for DS75 thermal sensor

[Jean Delvare]

Hi Wolfgang,

On Wed, 16 Jul 2008 11:12:50 +0200, Wolfgang Grandegger wrote:
> Jean Delvare wrote:
> > For hwmon chips, probing only occurs if the i2c adapter accepts to be
> > probed, by setting the I2C_CLASS_HWMON flag in i2c_adapter.class. If
> > you do not want a given i2c bus to be probed, then do not set the flag
> > for that bus.
> 
> I see. My problem is that I2C_CLASS_HWMON is set in the I2C bus driver 
> for the MPC:
> 
>    http://lxr.linux.no/linux+v2.6.26/drivers/i2c/busses/i2c-mpc.c#L314
> 
> If I disable it, the LM75 driver only probes the addresses of the I2C 
> nodes defined in the FDT DTS file. I wonder if this should not be the 
> default behaviour for kernels using OF platform description. For this 
> reason, I have put Linuxppc-dev ML on CC. I also realized, that there 
> are some patches for OF I2C pending. I will check.

The problem is that at this point in time, only a couple hwmon drivers
have been converted to new-style i2c. So, dropping the I2C_CLASS_HWMON
would break most systems.

I have a set of patches converting most hwmon drivers to new-style i2c.
I plan to send it to Linus later today. Once all drivers are converted,
everyone can start adding device definitions to platform code. And only
once this is done for all platforms, you may remove I2C_CLASS_HWMON
from the i2c-mpc driver.

But even then, you can't exclude the possibility that some people want
to keep relying on the auto-detection mode. In that case, the setting
of the I2C_CLASS_HWMON flag should become an attribute of eacg i2c-mpc
device.

Anyway, this is something for every platform community to decide and
work on. As the i2c subsystem maintainer, I made my best so that
everything (sensible) is possible. But in the end it's up to the
"users" (i.e. platform developers and maintainers) to make the decision
of how things should work in their area. I can give advice on migration
paths, but I can't take decisions for them.

-- 
Jean Delvare

Re: [PATCH] Support for DS75 thermal sensor

[Wolfgang Grandegger]

Hi Jean;

Jean Delvare wrote:
> Hi Wolfgang,
> 
> On Wed, 16 Jul 2008 11:12:50 +0200, Wolfgang Grandegger wrote:
>> Jean Delvare wrote:
>>> For hwmon chips, probing only occurs if the i2c adapter accepts to be
>>> probed, by setting the I2C_CLASS_HWMON flag in i2c_adapter.class. If
>>> you do not want a given i2c bus to be probed, then do not set the flag
>>> for that bus.
>> I see. My problem is that I2C_CLASS_HWMON is set in the I2C bus driver 
>> for the MPC:
>>
>>    http://lxr.linux.no/linux+v2.6.26/drivers/i2c/busses/i2c-mpc.c#L314
>>
>> If I disable it, the LM75 driver only probes the addresses of the I2C 
>> nodes defined in the FDT DTS file. I wonder if this should not be the 
>> default behaviour for kernels using OF platform description. For this 
>> reason, I have put Linuxppc-dev ML on CC. I also realized, that there 
>> are some patches for OF I2C pending. I will check.
> 
> The problem is that at this point in time, only a couple hwmon drivers
> have been converted to new-style i2c. So, dropping the I2C_CLASS_HWMON
> would break most systems.
> 
> I have a set of patches converting most hwmon drivers to new-style i2c.
> I plan to send it to Linus later today. Once all drivers are converted,
> everyone can start adding device definitions to platform code. And only
> once this is done for all platforms, you may remove I2C_CLASS_HWMON
> from the i2c-mpc driver.

Of course.

> But even then, you can't exclude the possibility that some people want
> to keep relying on the auto-detection mode. In that case, the setting

I understood that this is only true for the HWMON devices. Why the 
special treatment?

> of the I2C_CLASS_HWMON flag should become an attribute of eacg i2c-mpc
> device.

Yep, as probing might not be acceptable in some cases, I makes sense to 
add a property to suppress probing:

		i2c@3000 {
			...
			compatible = "fsl-i2c";
			dfsrr;
			no-probing;
			rtc@32 {
				compatible = "epson,rx8025";
				reg = <0x32>;
			};
			dtt@4c {
				compatible = "dallas,ds75";
				reg = <0x4c>;
			};
		};


> Anyway, this is something for every platform community to decide and
> work on. As the i2c subsystem maintainer, I made my best so that
> everything (sensible) is possible. But in the end it's up to the
> "users" (i.e. platform developers and maintainers) to make the decision
> of how things should work in their area. I can give advice on migration
> paths, but I can't take decisions for them.

I'm going to prepare a patch for that purpose.

Wolfgang.

Re: [PATCH] Support for DS75 thermal sensor

[Jean Delvare]

On Wed, 16 Jul 2008 11:50:15 +0200, Wolfgang Grandegger wrote:
> Jean Delvare wrote:
> > The problem is that at this point in time, only a couple hwmon drivers
> > have been converted to new-style i2c. So, dropping the I2C_CLASS_HWMON
> > would break most systems.
> > 
> > I have a set of patches converting most hwmon drivers to new-style i2c.
> > I plan to send it to Linus later today. Once all drivers are converted,
> > everyone can start adding device definitions to platform code. And only
> > once this is done for all platforms, you may remove I2C_CLASS_HWMON
> > from the i2c-mpc driver.
> 
> Of course.
> 
> > But even then, you can't exclude the possibility that some people want
> > to keep relying on the auto-detection mode. In that case, the setting
> 
> I understood that this is only true for the HWMON devices. Why the 
> special treatment?

Not really. There are other I2C_CLASS_* flags, just check <linux/i2c.h>.

What makes hwmon a bit different is the historical context. Originally,
the hwmon drivers were written for PCs, which have no per-system
platform code, so declaring the devices was simply not an option.
Additionally, i2c was maintained as part of the lm-sensors project
itself. This determined the probe-everything approach that has ruled
the i2c subsystem until recently. I've spent (with a few other
developers) the past few years drawing a clear separation between i2c
and hwmon, and now making it possible to declare i2c devices where
possible. This is a lot of work if you want to do this without breaking
any system out there (which is my case.)

> > of the I2C_CLASS_HWMON flag should become an attribute of each i2c-mpc
> > device.
> 
> Yep, as probing might not be acceptable in some cases, I makes sense to 
> add a property to suppress probing:

It'd rather make no-probing the default if possible. My understanding
is that all systems using i2c-mpc should have proper platform data.

But then again, that's not really my business. The decision is left to
the actual users of this i2c bus driver.

-- 
Jean Delvare

Re: [PATCH] Support for DS75 thermal sensor

[Wolfgang Grandegger]

Jean Delvare wrote:
> On Wed, 16 Jul 2008 11:50:15 +0200, Wolfgang Grandegger wrote:
>> Jean Delvare wrote:
>>> The problem is that at this point in time, only a couple hwmon drivers
>>> have been converted to new-style i2c. So, dropping the I2C_CLASS_HWMON
>>> would break most systems.
>>>
>>> I have a set of patches converting most hwmon drivers to new-style i2c.
>>> I plan to send it to Linus later today. Once all drivers are converted,
>>> everyone can start adding device definitions to platform code. And only
>>> once this is done for all platforms, you may remove I2C_CLASS_HWMON
>>> from the i2c-mpc driver.
>> Of course.
>>
>>> But even then, you can't exclude the possibility that some people want
>>> to keep relying on the auto-detection mode. In that case, the setting
>> I understood that this is only true for the HWMON devices. Why the 
>> special treatment?
> 
> Not really. There are other I2C_CLASS_* flags, just check <linux/i2c.h>.
> 
> What makes hwmon a bit different is the historical context. Originally,
> the hwmon drivers were written for PCs, which have no per-system
> platform code, so declaring the devices was simply not an option.
> Additionally, i2c was maintained as part of the lm-sensors project
> itself. This determined the probe-everything approach that has ruled
> the i2c subsystem until recently. I've spent (with a few other
> developers) the past few years drawing a clear separation between i2c
> and hwmon, and now making it possible to declare i2c devices where
> possible. This is a lot of work if you want to do this without breaking
> any system out there (which is my case.)

I can image. Thanks for the explanation.
> 
>>> of the I2C_CLASS_HWMON flag should become an attribute of each i2c-mpc
>>> device.
>> Yep, as probing might not be acceptable in some cases, I makes sense to 
>> add a property to suppress probing:
> 
> It'd rather make no-probing the default if possible. My understanding
> is that all systems using i2c-mpc should have proper platform data.

Yep, unfortunately, HWMON devices are not defined in most cases.

> But then again, that's not really my business. The decision is left to
> the actual users of this i2c bus driver.

A RFC patch is in preparation.

Wolfgang.

Re: [PATCH] Support for DS75 thermal sensor

[Grant Likely]

On Wed, Jul 16, 2008 at 12:10:59PM +0200, Jean Delvare wrote:
> On Wed, 16 Jul 2008 11:50:15 +0200, Wolfgang Grandegger wrote:
> > Jean Delvare wrote:
> > 
> > Yep, as probing might not be acceptable in some cases, I makes sense to 
> > add a property to suppress probing:
> 
> It'd rather make no-probing the default if possible. My understanding
> is that all systems using i2c-mpc should have proper platform data.

Total ACK.  From my perspective, probing should be off by default because the
typical use case in powerpc land is to trust data in the device tree.  Add the
property to turn on probing, not to turn it off.  Also, you'll need to
document the semantics of such a property.  ie. what exactly does it
mean when the probing property is present and the spi bus node has child
nodes?

g.

Re: [PATCH] Support for DS75 thermal sensor

[Jon Smirl]

On 7/16/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Wed, Jul 16, 2008 at 12:10:59PM +0200, Jean Delvare wrote:
>  > On Wed, 16 Jul 2008 11:50:15 +0200, Wolfgang Grandegger wrote:
>  > > Jean Delvare wrote:
>  > >
>
> > > Yep, as probing might not be acceptable in some cases, I makes sense to
>  > > add a property to suppress probing:
>  >
>  > It'd rather make no-probing the default if possible. My understanding
>  > is that all systems using i2c-mpc should have proper platform data.
>
>
> Total ACK.  From my perspective, probing should be off by default because the
>  typical use case in powerpc land is to trust data in the device tree.  Add the
>  property to turn on probing, not to turn it off.  Also, you'll need to
>  document the semantics of such a property.  ie. what exactly does it
>  mean when the probing property is present and the spi bus node has child
>  nodes?

I've found this thread now. Why can't we totally remove probing from
i2c-mpc? These are embedded systems, not open boxes like a PC. If a
i2c client hasn't been converted to the new model yet, convert it
before deploying with the new i2c-mpc driver.  It's not very hard to
convert the client drivers.

-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH] Support for DS75 thermal sensor

[Jean Delvare]

On Wed, 16 Jul 2008 10:18:26 -0400, Jon Smirl wrote:
> On 7/16/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> > On Wed, Jul 16, 2008 at 12:10:59PM +0200, Jean Delvare wrote:
> >  > On Wed, 16 Jul 2008 11:50:15 +0200, Wolfgang Grandegger wrote:
> >  > > Jean Delvare wrote:
> >  > >
> >
> > > > Yep, as probing might not be acceptable in some cases, I makes sense to
> >  > > add a property to suppress probing:
> >  >
> >  > It'd rather make no-probing the default if possible. My understanding
> >  > is that all systems using i2c-mpc should have proper platform data.
> >
> >
> > Total ACK.  From my perspective, probing should be off by default because the
> >  typical use case in powerpc land is to trust data in the device tree.  Add the
> >  property to turn on probing, not to turn it off.  Also, you'll need to
> >  document the semantics of such a property.  ie. what exactly does it
> >  mean when the probing property is present and the spi bus node has child
> >  nodes?
> 
> I've found this thread now. Why can't we totally remove probing from
> i2c-mpc? These are embedded systems, not open boxes like a PC. If a
> i2c client hasn't been converted to the new model yet, convert it
> before deploying with the new i2c-mpc driver.  It's not very hard to
> convert the client drivers.

I tend to agree. And the number of unconverted drivers is getting very
low these days. Only 2 RTC drivers are left, and by the end of the day,
almost all hwmon drivers will be converted as well.

-- 
Jean Delvare

Re: [PATCH] Support for DS75 thermal sensor

[Wolfgang Grandegger]

Jean Delvare wrote:
> On Wed, 16 Jul 2008 10:18:26 -0400, Jon Smirl wrote:
>> On 7/16/08, Grant Likely <grant.likely@secretlab.ca> wrote:
>>> On Wed, Jul 16, 2008 at 12:10:59PM +0200, Jean Delvare wrote:
>>>  > On Wed, 16 Jul 2008 11:50:15 +0200, Wolfgang Grandegger wrote:
>>>  > > Jean Delvare wrote:
>>>  > >
>>>
>>>>> Yep, as probing might not be acceptable in some cases, I makes sense to
>>>  > > add a property to suppress probing:
>>>  >
>>>  > It'd rather make no-probing the default if possible. My understanding
>>>  > is that all systems using i2c-mpc should have proper platform data.
>>>
>>>
>>> Total ACK.  From my perspective, probing should be off by default because the
>>>  typical use case in powerpc land is to trust data in the device tree.  Add the
>>>  property to turn on probing, not to turn it off.  Also, you'll need to
>>>  document the semantics of such a property.  ie. what exactly does it
>>>  mean when the probing property is present and the spi bus node has child
>>>  nodes?
>> I've found this thread now. Why can't we totally remove probing from
>> i2c-mpc? These are embedded systems, not open boxes like a PC. If a
>> i2c client hasn't been converted to the new model yet, convert it
>> before deploying with the new i2c-mpc driver.  It's not very hard to
>> convert the client drivers.
> 
> I tend to agree. And the number of unconverted drivers is getting very
> low these days. Only 2 RTC drivers are left, and by the end of the day,
> almost all hwmon drivers will be converted as well.

Thinking more about it I also prefer removing the I2C_CLASS_HWMON flag 
completely. It just affects HWMON devices anyhow and if there is still 
an old style driver around, it should be converted.

Wolfgang.

Re: [PATCH] Support for DS75 thermal sensor

[Grant Likely]

On Thu, Jul 17, 2008 at 09:31:24AM +0200, Wolfgang Grandegger wrote:
> Jean Delvare wrote:
>> On Wed, 16 Jul 2008 10:18:26 -0400, Jon Smirl wrote:
>>> I've found this thread now. Why can't we totally remove probing from
>>> i2c-mpc? These are embedded systems, not open boxes like a PC. If a
>>> i2c client hasn't been converted to the new model yet, convert it
>>> before deploying with the new i2c-mpc driver.  It's not very hard to
>>> convert the client drivers.
>>
>> I tend to agree. And the number of unconverted drivers is getting very
>> low these days. Only 2 RTC drivers are left, and by the end of the day,
>> almost all hwmon drivers will be converted as well.
>
> Thinking more about it I also prefer removing the I2C_CLASS_HWMON flag  
> completely. It just affects HWMON devices anyhow and if there is still  
> an old style driver around, it should be converted.

I'm cool with that.

g.

Re: [PATCH] Support for DS75 thermal sensor

[Wolfgang Grandegger]

Grant Likely wrote:
> On Thu, Jul 17, 2008 at 09:31:24AM +0200, Wolfgang Grandegger wrote:
>> Jean Delvare wrote:
>>> On Wed, 16 Jul 2008 10:18:26 -0400, Jon Smirl wrote:
>>>> I've found this thread now. Why can't we totally remove probing from
>>>> i2c-mpc? These are embedded systems, not open boxes like a PC. If a
>>>> i2c client hasn't been converted to the new model yet, convert it
>>>> before deploying with the new i2c-mpc driver.  It's not very hard to
>>>> convert the client drivers.
>>> I tend to agree. And the number of unconverted drivers is getting very
>>> low these days. Only 2 RTC drivers are left, and by the end of the day,
>>> almost all hwmon drivers will be converted as well.
>> Thinking more about it I also prefer removing the I2C_CLASS_HWMON flag  
>> completely. It just affects HWMON devices anyhow and if there is still  
>> an old style driver around, it should be converted.
> 
> I'm cool with that.

Good, just sent a new patch.

Wolfgang.