[PATCH 4/6]: i2c-pcf: Add a way for bus driver to ask for no smbus quick mode support.

[PATCH 4/6]: i2c-pcf: Add a way for bus driver to ask for no smbus quick mode support.

[David Miller]

i2c-pcf: Add a way for bus driver to ask for no smbus quick mode support.

Some PCF bus implementations have precise knowledge of the i2c
devices sitting on the bus, using things such as firmware device
trees, and do not want any of the auto I2C device probing to
occur.

Signed-off-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
index c171ee6..b8e34b0 100644
--- a/drivers/i2c/algos/i2c-algo-pcf.c
+++ b/drivers/i2c/algos/i2c-algo-pcf.c
@@ -416,8 +416,9 @@ out:
 
 static u32 pcf_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | 
-	       I2C_FUNC_PROTOCOL_MANGLING;
+	struct i2c_algo_pcf_data *pcf_adap = adap->algo_data;
+
+	return pcf_adap->func_flags;
 }
 
 /* -----exported algorithm data: -------------------------------------	*/
@@ -430,13 +431,18 @@ static const struct i2c_algorithm pcf_algo = {
 /* 
  * registering functions to load algorithms at runtime 
  */
-int i2c_pcf_add_bus(struct i2c_adapter *adap)
+int i2c_pcf_add_bus(struct i2c_adapter *adap, int support_smbus_quick)
 {
 	struct i2c_algo_pcf_data *pcf_adap = adap->algo_data;
 	int rval;
 
 	DEB2(dev_dbg(&adap->dev, "hw routines registered.\n"));
 
+	pcf_adap->func_flags = (I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | 
+				I2C_FUNC_PROTOCOL_MANGLING);
+	if (!support_smbus_quick)
+		pcf_adap->func_flags &= ~I2C_FUNC_SMBUS_QUICK;
+
 	/* register new adapter to i2c module... */
 	adap->algo = &pcf_algo;
 	adap->timeout = 100;
diff --git a/drivers/i2c/busses/i2c-elektor.c b/drivers/i2c/busses/i2c-elektor.c
index 37bb528..2be6b5b 100644
--- a/drivers/i2c/busses/i2c-elektor.c
+++ b/drivers/i2c/busses/i2c-elektor.c
@@ -281,7 +281,7 @@ static int __devinit elektor_probe(struct device *dev, unsigned int id)
 	if (pcf_isa_init())
 		return -ENODEV;
 	pcf_isa_ops.dev.parent = dev;
-	if (i2c_pcf_add_bus(&pcf_isa_ops) < 0)
+	if (i2c_pcf_add_bus(&pcf_isa_ops, 1) < 0)
 		goto fail;
 
 	dev_info(dev, "found device at %#x\n", base);
diff --git a/include/linux/i2c-algo-pcf.h b/include/linux/i2c-algo-pcf.h
index 78c62ed..388776a 100644
--- a/include/linux/i2c-algo-pcf.h
+++ b/include/linux/i2c-algo-pcf.h
@@ -36,6 +36,8 @@ struct i2c_algo_pcf_data {
 	void (*xfer_begin)(void *data);
 	void (*xfer_end)(void *data);
 
+	u32 func_flags;
+
 	/* Multi-master lost arbitration back-off delay (msecs)
 	 * This should be set by the bus adapter or knowledgable client
 	 * if bus is multi-mastered, else zero
@@ -43,6 +45,6 @@ struct i2c_algo_pcf_data {
 	unsigned long lab_mdelay;
 };
 
-int i2c_pcf_add_bus(struct i2c_adapter *);
+int i2c_pcf_add_bus(struct i2c_adapter *adap, int support_smbus_quick);
 
 #endif /* _LINUX_I2C_ALGO_PCF_H */
-- 
1.5.6.5.GIT


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 4/6]: i2c-pcf: Add a way for bus driver to ask for no smbus quick mode support.

[Jean Delvare]

Hi David,

On Thu, 21 Aug 2008 02:43:27 -0700 (PDT), David Miller wrote:
> 
> i2c-pcf: Add a way for bus driver to ask for no smbus quick mode support.
> 
> Some PCF bus implementations have precise knowledge of the i2c
> devices sitting on the bus, using things such as firmware device
> trees, and do not want any of the auto I2C device probing to
> occur.

Nack. For one thing, the SMBus quick command is not an equivalent for
device probing. Probing can be done with virtually any I2C transaction,
and SMBus quick command can be used as a regular transaction (even
though I am not aware of any Linux device driver using this at the
moment.)

For another, with the introduction of I2C classes several years ago,
I2C device drivers should no longer probe random I2C adapters. They
should only probe adapters which share a class bit with them, meaning
that an adapter with no class bits set should never be probed. With the
advent of so-called new-style i2c drivers (which follow the standard
device driver binding model) this is even being enforced. So, if you
find that a given I2C device driver is probing your adapter when it
isn't supposed to, get that driver fixed, or even better, turn it into
a new-style I2C driver, and your problem is solved.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 4/6]: i2c-pcf: Add a way for bus driver to ask for no smbus quick mode support.

[David Miller]

From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Date: Wed, 15 Oct 2008 14:33:21 +0200

> For another, with the introduction of I2C classes several years ago,
> I2C device drivers should no longer probe random I2C adapters.  

Jean, they do, that's why I wrote this change.

> They should only probe adapters which share a class bit with them,

The pcf driver sets the class bit used by FAN and temperature sensors,
so the I2C layer goes poking around all of the standard address
locations each of those drivers list.

I don't want that to happen, ever.  I know exactly what address each
and every I2C device sits at, so I don't want auto probing at all even
even if the class bits match.

Again where the heck were you two months ago when I submitted these
changes originally?  I could have been working on changes meanwhile,
but instead you're reviewing this stuff _NOW_ in the middle of the
merge window when I have to be on top of things for networking and
sparc stuff going into the tree.

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 4/6]: i2c-pcf: Add a way for bus driver to ask for no smbus quick mode support.

[David Miller]

From: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Date: Wed, 15 Oct 2008 14:34:52 -0700 (PDT)

> From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Date: Wed, 15 Oct 2008 14:33:21 +0200
> 
> > For another, with the introduction of I2C classes several years ago,
> > I2C device drivers should no longer probe random I2C adapters.  
> 
> Jean, they do, that's why I wrote this change.
> 
> > They should only probe adapters which share a class bit with them,
> 
> The pcf driver sets the class bit used by FAN and temperature sensors,
> so the I2C layer goes poking around all of the standard address
> locations each of those drivers list.
> 
> I don't want that to happen, ever.  I know exactly what address each
> and every I2C device sits at, so I don't want auto probing at all even
> even if the class bits match.

BTW, the powerpc folks want things the same exact way.

Look at the following change below which went into Linus's tree today.

Arguing against this is pointless.  With openfirmware device trees
describing exactly where every I2C device is, we want none of the
class based auto-probing stuff, we never ever want it.

Openfirmware platforms automatically trigger the device discovery
based upon the openfirmware device tree nodes.  None of the I2C
layer class based probing infrastructure is necessary nor wanted.

commit 618b26d52843c0f85b8eb143cf2695d7f6fd072d
Author: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
Date:   Wed Oct 8 11:36:42 2008 -0600

    i2c-mpc: suppress I2C device probing
    
    This patch suppresses I2C device probing by clearing the class field
    of the "struct i2c_adapter" for the MPC I2C bus adapters. Some board
    configurations which rely on probing must be fixed up by adding a
    proper I2C device node to the DTS file, like the TQM85xx modules.
    
    Signed-off-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
    Signed-off-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 27443f0..a9a45fc 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -312,7 +312,6 @@ static struct i2c_adapter mpc_ops = {
 	.name = "MPC adapter",
 	.id = I2C_HW_MPC107,
 	.algo = &mpc_algo,
-	.class = I2C_CLASS_HWMON | I2C_CLASS_SPD,
 	.timeout = 1,
 };
 

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 4/6]: i2c-pcf: Add a way for bus driver to ask for no smbus quick mode support.

[Jean Delvare]

Hi David,

On Wed, 15 Oct 2008 14:34:52 -0700 (PDT), David Miller wrote:
> From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Date: Wed, 15 Oct 2008 14:33:21 +0200
> 
> > For another, with the introduction of I2C classes several years ago,
> > I2C device drivers should no longer probe random I2C adapters.  
> 
> Jean, they do, that's why I wrote this change.
> 
> > They should only probe adapters which share a class bit with them,
> 
> The pcf driver sets the class bit used by FAN and temperature sensors,
> so the I2C layer goes poking around all of the standard address
> locations each of those drivers list.

By setting this class flag in your driver, you ask for these probes.

> I don't want that to happen, ever.  I know exactly what address each
> and every I2C device sits at, so I don't want auto probing at all even
> even if the class bits match.

Then just don't set any class flag in your i2c bus driver and you're
done. Easy as that, just as was done by the powerpc folks for the
i2c-mpc driver (as you just found out yourself.)

> Again where the heck were you two months ago when I submitted these
> changes originally?  I could have been working on changes meanwhile,
> but instead you're reviewing this stuff _NOW_ in the middle of the
> merge window when I have to be on top of things for networking and
> sparc stuff going into the tree.

Two months ago, I was processing all the patches and other requests
that had been sent to me while I was on vacation. I was also taking
care of my family. The fact that I didn't review your patches when you
sent them is unfortunate, but it happened, and similar events will
happen again in the future because there's only so many hours in a day
and only so much work I can do for free on my spare time. Whatever my
wife and baby daughter need from me has higher priority than kernel
patches. You can understand that, can't you?

You're complaining because I'm reviewing your patches during the merge
window. But you did you not bother reminding me about these patches
earlier, while it was pretty obvious that I had missed them? My i2c
patch set is public [1] and if a patch isn't there then it won't go to
Linus. You're blaming it on me, but you could as well blame it on
yourself.

I'm not quite sure why you are complaining anyway. I finally reviewed
your patches. And as far as this one is concerned, I just gave you a
much simpler way to achieve what you wanted to achieve.

[1] http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 4/6]: i2c-pcf: Add a way for bus driver to ask for no smbus quick mode support.

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 1266 bytes --]

Hi Jean,

On Thu, Oct 16, 2008 at 12:13:15PM +0200, Jean Delvare wrote:

> > The pcf driver sets the class bit used by FAN and temperature sensors,
> > so the I2C layer goes poking around all of the standard address
> > locations each of those drivers list.
> 
> By setting this class flag in your driver, you ask for these probes.

What about making this more obvious? Something like this?

All the best,

   Wolfram

===

i2c: Make clear what the class field of i2c_adapter is good for

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 include/linux/i2c.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/include/linux/i2c.h
===================================================================
--- linux-2.6.orig/include/linux/i2c.h
+++ linux-2.6/include/linux/i2c.h
@@ -345,7 +345,7 @@ struct i2c_algorithm {
 struct i2c_adapter {
 	struct module *owner;
 	unsigned int id;
-	unsigned int class;
+	unsigned int class;		  /* classes to allow probing for */
 	const struct i2c_algorithm *algo; /* the algorithm to access the bus */
 	void *algo_data;

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

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

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 4/6]: i2c-pcf: Add a way for bus driver to ask for no smbus quick mode support.

[Jean Delvare]

Hi Wolfram,

On Thu, 16 Oct 2008 12:46:23 +0200, Wolfram Sang wrote:
> Hi Jean,
> 
> On Thu, Oct 16, 2008 at 12:13:15PM +0200, Jean Delvare wrote:
> 
> > > The pcf driver sets the class bit used by FAN and temperature sensors,
> > > so the I2C layer goes poking around all of the standard address
> > > locations each of those drivers list.
> > 
> > By setting this class flag in your driver, you ask for these probes.
> 
> What about making this more obvious? Something like this?
> 
> All the best,
> 
>    Wolfram
> 
> ===
> 
> i2c: Make clear what the class field of i2c_adapter is good for
> 
> Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  include/linux/i2c.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-2.6/include/linux/i2c.h
> ===================================================================
> --- linux-2.6.orig/include/linux/i2c.h
> +++ linux-2.6/include/linux/i2c.h
> @@ -345,7 +345,7 @@ struct i2c_algorithm {
>  struct i2c_adapter {
>  	struct module *owner;
>  	unsigned int id;
> -	unsigned int class;
> +	unsigned int class;		  /* classes to allow probing for */
>  	const struct i2c_algorithm *algo; /* the algorithm to access the bus */
>  	void *algo_data;
> 

Sure, why not. Thanks for the patch, applied.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 4/6]: i2c-pcf: Add a way for bus driver to ask for no smbus quick mode support.

[David Miller]

From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Date: Thu, 16 Oct 2008 12:13:15 +0200

> Hi David,
> 
> On Wed, 15 Oct 2008 14:34:52 -0700 (PDT), David Miller wrote:
> > From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > Date: Wed, 15 Oct 2008 14:33:21 +0200
> > 
> > > For another, with the introduction of I2C classes several years ago,
> > > I2C device drivers should no longer probe random I2C adapters.  
> > 
> > Jean, they do, that's why I wrote this change.
> > 
> > > They should only probe adapters which share a class bit with them,
> > 
> > The pcf driver sets the class bit used by FAN and temperature sensors,
> > so the I2C layer goes poking around all of the standard address
> > locations each of those drivers list.
> 
> By setting this class flag in your driver, you ask for these probes.

The PCF driver sets this flag by default, and my patch which you're
reviewing here allows my sparc specific backend driver (the last
patch in the series) to turn the bit off.

So you are arguing for exactly what I'm adding support for.

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 4/6]: i2c-pcf: Add a way for bus driver to ask for no smbus quick mode support.

[Jean Delvare]

On Thu, 16 Oct 2008 12:25:42 -0700 (PDT), David Miller wrote:
> From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Date: Thu, 16 Oct 2008 12:13:15 +0200
> 
> > Hi David,
> > 
> > On Wed, 15 Oct 2008 14:34:52 -0700 (PDT), David Miller wrote:
> > > From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > > Date: Wed, 15 Oct 2008 14:33:21 +0200
> > > 
> > > > For another, with the introduction of I2C classes several years ago,
> > > > I2C device drivers should no longer probe random I2C adapters.  
> > > 
> > > Jean, they do, that's why I wrote this change.
> > > 
> > > > They should only probe adapters which share a class bit with them,
> > > 
> > > The pcf driver sets the class bit used by FAN and temperature sensors,
> > > so the I2C layer goes poking around all of the standard address
> > > locations each of those drivers list.
> > 
> > By setting this class flag in your driver, you ask for these probes.
> 
> The PCF driver sets this flag by default, and my patch which you're
> reviewing here allows my sparc specific backend driver (the last
> patch in the series) to turn the bit off.
> 
> So you are arguing for exactly what I'm adding support for.

Please define "PCF driver" and "flag" in your previous sentence.
Apparently we are not speaking of the same driver nor the same flag, so
this discussion isn't going anywhere.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 4/6]: i2c-pcf: Add a way for bus driver to ask for no smbus quick mode support.

[David Miller]

From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Date: Thu, 16 Oct 2008 21:34:07 +0200

> Apparently we are not speaking of the same driver nor the same flag, so
> this discussion isn't going anywhere.

I'm not engaging in this discussion any longer.

I'll merge my Sparc I2C changes directly during the 2.6.29 merge
window, which is what I should have done in the first place.


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c