[PATCH] assign a default class to MPC adapter

[PATCH] assign a default class to MPC adapter

[Michael Lawnick]

For MPC adapter there is no class assigned as it is done in other
adapters. This way no new-style client will ever be instantiated. There
should be at least a default class assignment.

Signed-off-by: Michael Lawnick <ml.lawnick-Mmb7MZpHnFY@public.gmane.org>
Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
---
 drivers/i2c/busses/i2c-mpc.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -310,6 +310,7 @@ static const struct i2c_algorithm mpc_algo = {
 static struct i2c_adapter mpc_ops = {
 	.owner = THIS_MODULE,
 	.name = "MPC adapter",
+	.class = I2C_CLASS_HWMON,
 	.algo = &mpc_algo,
 	.timeout = 1,
 };

-- 
Michael Lawnick

Re: [PATCH] assign a default class to MPC adapter

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1418 bytes --]

On Thu, Apr 16, 2009 at 11:06:01AM +0200, Michael Lawnick wrote:
> For MPC adapter there is no class assigned as it is done in other
> adapters. This way no new-style client will ever be instantiated. There
> should be at least a default class assignment.

Please have a loot at 618b26d52843c0f85b8eb143cf2695d7f6fd072d
Therefore, NACK.

> 
> Signed-off-by: Michael Lawnick <ml.lawnick-Mmb7MZpHnFY@public.gmane.org>
> Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-mpc.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> --- a/drivers/i2c/busses/i2c-mpc.c
> +++ b/drivers/i2c/busses/i2c-mpc.c
> @@ -310,6 +310,7 @@ static const struct i2c_algorithm mpc_algo = {
>  static struct i2c_adapter mpc_ops = {
>  	.owner = THIS_MODULE,
>  	.name = "MPC adapter",
> +	.class = I2C_CLASS_HWMON,
>  	.algo = &mpc_algo,
>  	.timeout = 1,
>  };
> 
> -- 
> Michael Lawnick
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] assign a default class to MPC adapter

[Michael Lawnick]

Wolfram Sang said the following:
> On Thu, Apr 16, 2009 at 11:06:01AM +0200, Michael Lawnick wrote:
>> For MPC adapter there is no class assigned as it is done in other
>> adapters. This way no new-style client will ever be instantiated. There
>> should be at least a default class assignment.
> 
> Please have a loot at 618b26d52843c0f85b8eb143cf2695d7f6fd072d
> Therefore, NACK.

AFAICS, this patch related to 2.6.26 and i2c code has changed since.
In i2c-core.c@i2c_detect there is now a check whether the driver is
legacy, if it is the function is left and driver initialization handled
in another way. This was not the case when the patch of Wolfgang
Grandegger was applied.

I still don't think it makes much sense to include code (i2c-mpc.c) that
has no function by default - what in fact is the case since last change
of i2c-core.c

-- 
Kind Regards,
Michael

Re: [PATCH] assign a default class to MPC adapter

[Michael Lawnick]

Wolfram Sang said the following:
> On Thu, Apr 16, 2009 at 11:06:01AM +0200, Michael Lawnick wrote:
>> For MPC adapter there is no class assigned as it is done in other
>> adapters. This way no new-style client will ever be instantiated. There
>> should be at least a default class assignment.
> 
> Please have a loot at 618b26d52843c0f85b8eb143cf2695d7f6fd072d
> Therefore, NACK.

Beside my first answer, in your commit
0a346dacee18ff69f6162d9860d723a058f47321 you say:
> Might need updates to dts files using it.

Could you please elaborate?

-- 
Kind Regards,
Michael

Re: [PATCH] assign a default class to MPC adapter

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 750 bytes --]

> Could you please elaborate?

Instead of probing you specify the devices on the i2c-bus in the dts. Example
for the pcm032 looks like this (from the kernel-tree):

i2c@3d40 {
	#address-cells = <1>;
	#size-cells = <0>;
	compatible = "fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c";
	reg = <0x3d40 0x40>;
	interrupts = <2 16 0>;
	fsl5200-clocking;

	rtc@51 {
		compatible = "nxp,pcf8563";
		reg = <0x51>;
	};
	eeprom@52 {
		compatible = "at24,24c32";
		reg = <0x52>;
	};
};

Boards which relied on probing need to have such child nodes added.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] assign a default class to MPC adapter

[Michael Lawnick]

Wolfram Sang said the following:
>> Could you please elaborate?
> 
> Instead of probing you specify the devices on the i2c-bus in the dts. Example
> for the pcm032 looks like this (from the kernel-tree):
> 
> i2c@3d40 {
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 	compatible = "fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c";
> 	reg = <0x3d40 0x40>;
> 	interrupts = <2 16 0>;
> 	fsl5200-clocking;
> 
> 	rtc@51 {
> 		compatible = "nxp,pcf8563";
> 		reg = <0x51>;
> 	};
> 	eeprom@52 {
> 		compatible = "at24,24c32";
> 		reg = <0x52>;
> 	};
> };
> 
> Boards which relied on probing need to have such child nodes added.

Ok, that doesn't help :-(
I hoped you show me a way how to set the class of adapter. My clients
are loaded with insmod, so dts has no effect (or is this wrong?)

The current situation is:
I can't see a method beside force to instantiate a client at runtime.
But force bypasses clients detect method and is deprecated anyway if I
got Jean right.

Currently I'm lost. Jean NACKs all attempts to do detected client
instantiations at runtime, you do a NACK on making adapter working
properly by indicating to patches that have no more relevance.

Can't anybody help me?

-- 
:-(

Re: [PATCH] assign a default class to MPC adapter

[Jean Delvare]

On Thu, 16 Apr 2009 15:36:12 +0200, Michael Lawnick wrote:
> Wolfram Sang said the following:
> >> Could you please elaborate?
> > 
> > Instead of probing you specify the devices on the i2c-bus in the dts. Example
> > for the pcm032 looks like this (from the kernel-tree):
> > 
> > i2c@3d40 {
> > 	#address-cells = <1>;
> > 	#size-cells = <0>;
> > 	compatible = "fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c";
> > 	reg = <0x3d40 0x40>;
> > 	interrupts = <2 16 0>;
> > 	fsl5200-clocking;
> > 
> > 	rtc@51 {
> > 		compatible = "nxp,pcf8563";
> > 		reg = <0x51>;
> > 	};
> > 	eeprom@52 {
> > 		compatible = "at24,24c32";
> > 		reg = <0x52>;
> > 	};
> > };
> > 
> > Boards which relied on probing need to have such child nodes added.
> 
> Ok, that doesn't help :-(
> I hoped you show me a way how to set the class of adapter. My clients
> are loaded with insmod, so dts has no effect (or is this wrong?)

No, you don't load "clients" with insmod. You load i2c device drivers
with insmod. This is in no way incompatible with what Wolfram is
suggesting. "Clients" (or i2c devices) can (and in general should)
exist independent of their driver being loaded or not.

> The current situation is:
> I can't see a method beside force to instantiate a client at runtime.

That's right, but why do you insist on instantiating them at runtime?

> But force bypasses clients detect method and is deprecated anyway if I
> got Jean right.

I confirm that the force module parameters are planed for removal
(although I can really call them deprecated in the absence of a
replacement at this time - I know there's a patch from you pending my
review.)

> Currently I'm lost. Jean NACKs all attempts to do detected client
> instantiations at runtime, you do a NACK on making adapter working
> properly by indicating to patches that have no more relevance.
> 
> Can't anybody help me?

The question is: why every other user of i2c-mpc is apparently able to
declare i2c devices through dts and you aren't?

-- 
Jean Delvare

Re: [PATCH] assign a default class to MPC adapter

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1648 bytes --]

On Thu, Apr 16, 2009 at 03:36:12PM +0200, Michael Lawnick wrote:
> Wolfram Sang said the following:
> >> Could you please elaborate?
> > 
> > Instead of probing you specify the devices on the i2c-bus in the dts. Example
> > for the pcm032 looks like this (from the kernel-tree):
> > 
> > i2c@3d40 {
> > 	#address-cells = <1>;
> > 	#size-cells = <0>;
> > 	compatible = "fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c";
> > 	reg = <0x3d40 0x40>;
> > 	interrupts = <2 16 0>;
> > 	fsl5200-clocking;
> > 
> > 	rtc@51 {
> > 		compatible = "nxp,pcf8563";
> > 		reg = <0x51>;
> > 	};
> > 	eeprom@52 {
> > 		compatible = "at24,24c32";
> > 		reg = <0x52>;
> > 	};
> > };
> > 
> > Boards which relied on probing need to have such child nodes added.
> 
> Ok, that doesn't help :-(
> I hoped you show me a way how to set the class of adapter. My clients
> are loaded with insmod, so dts has no effect (or is this wrong?)

Are you sure you put the correct names into the compatible field?

With my dts-files, I can see the i2c-devices in sysfs. After modprobing
the necessary modules, I can access the devices. Example on my current
board:

root@A:~ hwclock -r
hwclock: can't open '/dev/misc/rtc': No such file or directory
root@A:~ modprobe rtc-ds1307
rtc-ds1307 0-0068: rtc core: registered ds3231 as rtc0
root@A:~ hwclock -r
Sat Jan  1 01:23:06 2000  0.000000 seconds
root@A:~ hwclock -r
Sat Jan  1 01:23:12 2000  0.000000 seconds

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] assign a default class to MPC adapter

[Jean Delvare]

Hi Michael,

please keep the list CC'd.

On Fri, 17 Apr 2009 08:32:16 +0200, Michael Lawnick wrote:
> Jean Delvare said the following:
> > That's right, but why do you insist on instantiating them at runtime?
> > 
> Because we have 2 different boards with same main processor but
> different on-board peripherals. They should run with same binary kernel
> to avoid duplicate production tests. On boot the system checks on which
> of the boards it is running and loads the necessary drivers with
> matching parameters.

Why would the system check need to be done in user-space? I can easily
imagine a single kernel supporting more than one system, all I2C
devices being described in the kernel and the right subset of I2C
devices being instantiated at boot based on which system the kernel is
running on.

> > The question is: why every other user of i2c-mpc is apparently able 
> > to declare i2c devices through dts and you aren't?
>
> The question was: does dts work for drivers that are loaded on runtime.

Answer is yes.

> I did not check before I asked, because it is a bit complicated here to
> do. In difference to you Wolfram was able to give a verbose 'yes' which
> I will check now.

In difference to Wolfram I have no specific knowledge of embedded
boards. I try to help with i2c in general but I am limited when it
comes to embedded.

-- 
Jean Delvare

Re: [PATCH] assign a default class to MPC adapter

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 609 bytes --]


> > Because we have 2 different boards with same main processor but
> > different on-board peripherals. They should run with same binary kernel
> > to avoid duplicate production tests. On boot the system checks on which

This is what the device-tree is mainly made for.

> > The question was: does dts work for drivers that are loaded on runtime.

Its use would be quite limited if it couldn't do that, right? ;)

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] assign a default class to MPC adapter

[Michael Lawnick]

Wolfram Sang said the following:
>> Could you please elaborate?
> 
> Instead of probing you specify the devices on the i2c-bus in the dts. Example
> for the pcm032 looks like this (from the kernel-tree):
> 
> i2c@3d40 {
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 	compatible = "fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c";
> 	reg = <0x3d40 0x40>;
> 	interrupts = <2 16 0>;
> 	fsl5200-clocking;
> 
> 	rtc@51 {
> 		compatible = "nxp,pcf8563";
> 		reg = <0x51>;
> 	};
> 	eeprom@52 {
> 		compatible = "at24,24c32";
> 		reg = <0x52>;
> 	};
> };
> 
> Boards which relied on probing need to have such child nodes added.

This entry forces the sysFs basic entries, but instantiation of
_new-style_ drivers still fails, at least on my system.

I now proposed a patch for loading the class parameter of the adapter
from device tree.(--><49ED6AD3.2060808-Mmb7MZpHnFY@public.gmane.org>) and hope to get an ACK.

-- 
Kind regards,
Michael

Re: [PATCH] assign a default class to MPC adapter

[Wolfgang Grandegger]

Michael Lawnick wrote:
> Wolfram Sang said the following:
>>> Could you please elaborate?
>> Instead of probing you specify the devices on the i2c-bus in the dts. Example
>> for the pcm032 looks like this (from the kernel-tree):
>>
>> i2c@3d40 {
>> 	#address-cells = <1>;
>> 	#size-cells = <0>;
>> 	compatible = "fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c";
>> 	reg = <0x3d40 0x40>;
>> 	interrupts = <2 16 0>;
>> 	fsl5200-clocking;
>>
>> 	rtc@51 {
>> 		compatible = "nxp,pcf8563";
>> 		reg = <0x51>;
>> 	};
>> 	eeprom@52 {
>> 		compatible = "at24,24c32";
>> 		reg = <0x52>;
>> 	};
>> };
>>
>> Boards which relied on probing need to have such child nodes added.
> 
> This entry forces the sysFs basic entries, but instantiation of
> _new-style_ drivers still fails, at least on my system.

Could you switch I2C debugging on and check the transfers to the device?
Is the address correct? What DTS node do you use? You could compare it
with a kernel with your patch applied and using probing.

> I now proposed a patch for loading the class parameter of the adapter
> from device tree.(--><49ED6AD3.2060808-Mmb7MZpHnFY@public.gmane.org>) and hope to get an ACK.

What kernel version do you use?

Wolfgang.