i2c multimaster and the device driver detect function

i2c multimaster and the device driver detect function

[Jean-François Dagenais]

Hi all,

I've read the discussion on multimaster and I have to agree with Uwe about masters doing the arbitration (and retry). However, there's another issue which one quickly discovers when adding a second master on a physical i2c bus.

Here's the scenario: Using driver jc42 which adds a "detect" function, and using two masters which end-up creating "i2c-0" and "i2c-1", the temp sensors get instanciated twice, i.e. once on each bus master. This is obviously very problematic as there are two driver instances controlling the exact same chip.

Before I dive a bit too much into this, is there something I am missing?

Should there be a "struct i2c_bus" along side "struct i2c_adapter" where the adapter would point to an i2c_bus or something? Sounds like this would have many downstream implications, and until today, I've only written device drivers not core. I could try something, but due diligence dictates i2c maintainers should provide input before I get lost into un-mergeable work.

Thanks for your input!

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Guenter Roeck]

On Wed, May 08, 2013 at 11:57:59AM -0400, Jean-François Dagenais wrote:
> Hi all,
> 
> I've read the discussion on multimaster and I have to agree with Uwe about masters doing the arbitration (and retry). However, there's another issue which one quickly discovers when adding a second master on a physical i2c bus.
> 
> Here's the scenario: Using driver jc42 which adds a "detect" function, and using two masters which end-up creating "i2c-0" and "i2c-1", the temp sensors get instanciated twice, i.e. once on each bus master. This is obviously very problematic as there are two driver instances controlling the exact same chip.
> 
> Before I dive a bit too much into this, is there something I am missing?
> 
Isn't it the point of having multiple masters on the same bus, that each of
them can manage the same devices ?

Question for me is why you would want two masters in the same system context
point to the same I2C bus. Usually the second master would be something like an
IPMI controller or a second CPU/controller board in a redundant system.

Even then the multi-master scenario is problematic, as you still end up with
multiple masters controlling the same device. That is a problem inherent to I2C,
and especially problematic with multi-page devices (typical problem: master 1
sets page, master 2 sets page, master 1 accesses wrong data). I don't think there
is a clean solution to solve that, other than to block i2c access for one of the
masters entirely.

Guenter

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean-François Dagenais]

On 2013-05-08, at 13:54, Guenter Roeck wrote:

> [...]
> Isn't it the point of having multiple masters on the same bus, that each of
> them can manage the same devices ?

I wouldn't think so. You mention why in your last paragraph and I totally see
and agree with that. So that is not my intention at all.

> 
> Question for me is why you would want two masters in the same system context
> point to the same I2C bus. Usually the second master would be something like an
> IPMI controller or a second CPU/controller board in a redundant system.

I guess that's one scenario. In our scenario, we have an assambled board stack.
The main board has a sub-par i2c master onto which heavy slaves such as the
ad7147 capacitive input device consumes too much cpu power (cpu has to poll the
Kontron PLD which houses the i2c master core). Our hoped solution was to
allocate an i2c IP core in our FPGA. But board inter-connect budget didn't allow
for separate i2c segments, especially in the light that both master are said to
support multimaster operation. I got this working experimentally right now, I
just stumbled on jc42's detect, and am trying to project this kind of design to
the future. Right now, my only solution is to patch my version of jc42 not to
auto-detect, and explicitely enumerate my temperature sensors like my other
slaves in arch setup... yikes.

> 
> Even then the multi-master scenario is problematic, as you still end up with
> multiple masters controlling the same device. That is a problem inherent to I2C,
> and especially problematic with multi-page devices (typical problem: master 1
> sets page, master 2 sets page, master 1 accesses wrong data). I don't think there
> is a clean solution to solve that, other than to block i2c access for one of the
> masters entirely.

Other than the redundant/resiliency scenario, why would you setup the same slave
on two different masters? Right now I have a working setup where some slaves are
declared on bus 0 (PLD i2c master) and others on bus 1 (FPGA i2c master). I have
yet to stress test the setup within the next day or so, but so far, it seems to
work ok.

Thoughts?--
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

Re: i2c multimaster and the device driver detect function

[Guenter Roeck]

On Wed, May 08, 2013 at 09:50:44PM -0400, Jean-François Dagenais wrote:
> 
> On 2013-05-08, at 13:54, Guenter Roeck wrote:
> 
> > [...]
> > Isn't it the point of having multiple masters on the same bus, that each of
> > them can manage the same devices ?
> 
> I wouldn't think so. You mention why in your last paragraph and I totally see
> and agree with that. So that is not my intention at all.
> 
Guess the real conclusion is that one should avoid two active masters
in the first place if possible.

> > 
> > Question for me is why you would want two masters in the same system context
> > point to the same I2C bus. Usually the second master would be something like an
> > IPMI controller or a second CPU/controller board in a redundant system.
> 
> I guess that's one scenario. In our scenario, we have an assambled board stack.
> The main board has a sub-par i2c master onto which heavy slaves such as the
> ad7147 capacitive input device consumes too much cpu power (cpu has to poll the
> Kontron PLD which houses the i2c master core). Our hoped solution was to
> allocate an i2c IP core in our FPGA. But board inter-connect budget didn't allow
> for separate i2c segments, especially in the light that both master are said to
> support multimaster operation. I got this working experimentally right now, I
> just stumbled on jc42's detect, and am trying to project this kind of design to
> the future. Right now, my only solution is to patch my version of jc42 not to
> auto-detect, and explicitely enumerate my temperature sensors like my other
> slaves in arch setup... yikes.
> 
Is one of the I2C adapter drivers your own ? If so, you can disable auto-detection
in the adapter code by setting the adapter class to 0 (specifically, don't set it
to I2C_CLASS_HWMON). You can do the same in the Kontron driver if you have the
source (it is GPL so you should be able to find it). While not perfect, that should
be better than disabling auto-detection in the affected chip drivers.
Note that the Kontron driver also sets I2C_CLASS_SPD, which means EEPROMs are
auto-detected on address 0x50.

> > 
> > Even then the multi-master scenario is problematic, as you still end up with
> > multiple masters controlling the same device. That is a problem inherent to I2C,
> > and especially problematic with multi-page devices (typical problem: master 1
> > sets page, master 2 sets page, master 1 accesses wrong data). I don't think there
> > is a clean solution to solve that, other than to block i2c access for one of the
> > masters entirely.
> 
> Other than the redundant/resiliency scenario, why would you setup the same slave
> on two different masters? Right now I have a working setup where some slaves are
> declared on bus 0 (PLD i2c master) and others on bus 1 (FPGA i2c master). I have
> yet to stress test the setup within the next day or so, but so far, it seems to
> work ok.
> 
Sure, it does work, I am just not sure what the benefits are of having two
masters in this scenario. Guess you are saying that the I2C master in the
Kontron PLD can not drive the AD7147 - is that correct ? If so, and if that
means you have to use your own I2C core anyway, why bother using the Kontron
PLD's I2C bus to start with ? You could just ignore it (ie not instantiate
or build it at all) and use your own.

Did you tell Kontron about the problems with their PLD ? Maybe they have a
solution.

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean-François Dagenais]

On 2013-05-08, at 11:53 PM, Guenter Roeck wrote:

> Guess the real conclusion is that one should avoid two active masters
> in the first place if possible.

I agree, I can't think of any benefits worth the trouble.

> Is one of the I2C adapter drivers your own ? If so, you can disable auto-detection
> in the adapter code by setting the adapter class to 0 (specifically, don't set it
> to I2C_CLASS_HWMON). You can do the same in the Kontron driver if you have the
> source (it is GPL so you should be able to find it). While not perfect, that should
> be better than disabling auto-detection in the affected chip drivers.

That's great advice!! I will look into this, thanks!

> Note that the Kontron driver also sets I2C_CLASS_SPD, which means EEPROMs are
> auto-detected on address 0x50.

Funny, I had to explicitly inject "I2C_BOARD_INFO("24c32", 0x50)" to see
Kontron's JIDA chip.

>> [...] Right now I have a working setup where some slaves are
>> declared on bus 0 (PLD i2c master) and others on bus 1 (FPGA i2c master). I have
>> yet to stress test the setup within the next day or so, but so far, it seems to
>> work ok.
>> 
> Sure, it does work, I am just not sure what the benefits are of having two
> masters in this scenario.

My thoughts exactly. I would have avoided it. Right now I am trying to improve
and existing design.

>  Guess you are saying that the I2C master in the
> Kontron PLD can not drive the AD7147 - is that correct ?

Well no, it does drive it, we have it in stable production. It's just that when
you have your finger on the wheel, CPU usage goes up to 5-15%. More intense
polling of the sensors also takes a toll on the CPU. For different reasons other
than the ad714x, the FPGA i2c core option suddenly became attractive.

> If so, and if that
> means you have to use your own I2C core anyway, why bother using the Kontron
> PLD's I2C bus to start with ? You could just ignore it (ie not instantiate

> or build it at all) and use your own.

Yeah, we already considered that... our FPGA's reset and flash select are
controlled by an i2c I/O expander! Hehe, so this I/O expander needs to be
mastered from the kempld. Reality is a bummer ;)

> 
> Did you tell Kontron about the problems with their PLD ? Maybe they have a
> solution.

Yes we did. If I remember correctly, the problem is the absence of an interrupt
line from the PLD to the CPU, which explains the high CPU since the driver
sleeps-polls for async PLD completions and statuses. ... I know...

Thanks for the reply!
Cheers,
/jfd--
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

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Guenter Roeck]

On Thu, May 09, 2013 at 08:38:28AM -0400, Jean-François Dagenais wrote:
> 
> On 2013-05-08, at 11:53 PM, Guenter Roeck wrote:
> 
> > Guess the real conclusion is that one should avoid two active masters
> > in the first place if possible.
> 
> I agree, I can't think of any benefits worth the trouble.
> 
> > Is one of the I2C adapter drivers your own ? If so, you can disable auto-detection
> > in the adapter code by setting the adapter class to 0 (specifically, don't set it
> > to I2C_CLASS_HWMON). You can do the same in the Kontron driver if you have the
> > source (it is GPL so you should be able to find it). While not perfect, that should
> > be better than disabling auto-detection in the affected chip drivers.
> 
> That's great advice!! I will look into this, thanks!
> 
> > Note that the Kontron driver also sets I2C_CLASS_SPD, which means EEPROMs are
> > auto-detected on address 0x50.
> 
> Funny, I had to explicitly inject "I2C_BOARD_INFO("24c32", 0x50)" to see
> Kontron's JIDA chip.
> 
Interesting.

> >> [...] Right now I have a working setup where some slaves are
> >> declared on bus 0 (PLD i2c master) and others on bus 1 (FPGA i2c master). I have
> >> yet to stress test the setup within the next day or so, but so far, it seems to
> >> work ok.
> >> 
> > Sure, it does work, I am just not sure what the benefits are of having two
> > masters in this scenario.
> 
> My thoughts exactly. I would have avoided it. Right now I am trying to improve
> and existing design.
> 
> >  Guess you are saying that the I2C master in the
> > Kontron PLD can not drive the AD7147 - is that correct ?
> 
> Well no, it does drive it, we have it in stable production. It's just that when
> you have your finger on the wheel, CPU usage goes up to 5-15%. More intense
> polling of the sensors also takes a toll on the CPU. For different reasons other
> than the ad714x, the FPGA i2c core option suddenly became attractive.
> 
> > If so, and if that
> > means you have to use your own I2C core anyway, why bother using the Kontron
> > PLD's I2C bus to start with ? You could just ignore it (ie not instantiate
> 
> > or build it at all) and use your own.
> 
> Yeah, we already considered that... our FPGA's reset and flash select are
> controlled by an i2c I/O expander! Hehe, so this I/O expander needs to be
> mastered from the kempld. Reality is a bummer ;)
> 
> > 
> > Did you tell Kontron about the problems with their PLD ? Maybe they have a
> > solution.
> 
> Yes we did. If I remember correctly, the problem is the absence of an interrupt
> line from the PLD to the CPU, which explains the high CPU since the driver
> sleeps-polls for async PLD completions and statuses. ... I know...
> 
Ah, that one. Yes, the higher CPU utilization is an indication that this is due
to polling, not due to power draw on the I2C lines. You might try to increase
the usleep_range interval in the Kontron driver to reduce the load a bit.

Anyway (kind of) good news - newer Kontron boards do support interrupts and no
longer have to revert to polling.

Thanks,
Guenter

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean Delvare]

Salut Jean-François, hi Guenter,

Sorry for jumping in a little late, I am just back from vacation.

On Thu, 9 May 2013 08:38:28 -0400, Jean-François Dagenais wrote:
> On 2013-05-08, at 11:53 PM, Guenter Roeck wrote:
> > Is one of the I2C adapter drivers your own ? If so, you can disable auto-detection
> > in the adapter code by setting the adapter class to 0 (specifically, don't set it
> > to I2C_CLASS_HWMON). You can do the same in the Kontron driver if you have the
> > source (it is GPL so you should be able to find it). While not perfect, that should
> > be better than disabling auto-detection in the affected chip drivers.
> 
> That's great advice!! I will look into this, thanks!

Guenter is right. You never have to disable auto-detection in the slave
drivers (jc42 etc.) All these slave drivers do is claim "I _can_ do
auto-detection", not "I _will_ do auto-detection." It's always up to the
I2C adapter driver, whether auto-detection will happen or not. And it
is disabled by default. So if you don't want it, just do not enable it
in the bus driver. You can even set it per adapter, when the driver
controls more than one adapter, and per bus segment, when multiplexing
is taking place.

Please also note: the jc42 driver now uses I2C_CLASS_SPD not
I2C_CLASS_HWMON, because memory modules typically use a single chip for
SPD EEPROM and JEDEC JC42.2 temperature sensor. Think of I2C_CLASS_SPD
as "I2C class for memory modules."

> > Note that the Kontron driver also sets I2C_CLASS_SPD, which means EEPROMs are
> > auto-detected on address 0x50.
> 
> Funny, I had to explicitly inject "I2C_BOARD_INFO("24c32", 0x50)" to see
> Kontron's JIDA chip.

The only EEPROMs which are auto-detected are SPD and EDID EEPROMs. The
legacy eeprom driver is used for these. The 24C32 is a larger EEPROM,
you must use the at24 driver for it and it doesn't support
auto-detection (this is simply not possible.)

In the long run, the legacy eeprom driver could be killed, but not
before someone verifies that the at24 driver can take over completely,
including the auto-detection feature, performance optimizations and
corner case coverage.

> > (...)
> > Sure, it does work, I am just not sure what the benefits are of having two
> > masters in this scenario.
> 
> My thoughts exactly. I would have avoided it. Right now I am trying to improve
> and existing design.

It might be a little late now, but you may want to look into the
PCA9541, for which Guenter has written a driver.

-- 
Jean Delvare

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean-François Dagenais]

Salut Jean, merci de participer!

On 2013-05-13, at 4:11 AM, Jean Delvare wrote:

> 
> Guenter is right. You never have to disable auto-detection in the slave
> drivers (jc42 etc.) All these slave drivers do is claim "I _can_ do
> auto-detection", not "I _will_ do auto-detection." It's always up to the
> I2C adapter driver, whether auto-detection will happen or not. And it
> is disabled by default. So if you don't want it, just do not enable it
> in the bus driver. You can even set it per adapter, when the driver
> controls more than one adapter, and per bus segment, when multiplexing
> is taking place.

I am just wondering where the clean hook is for doing this. From what I can
gather, the master driver(s I've seen) declare ".class = I2C_CLASS_HWMON |
I2C_CLASS_SPD," pretty statically. Is it just that they are missing this
flexibility? Something along the lines of patching the pdata of such a master
driver to provide a 'class' variable in pdata? If so, one would have to take
into account the existing users of the master which expect the previous class
setting which may not be '0', thus requiring patching the existing upstreamed
users...  Suggestions?

> [...]
> It might be a little late now, but you may want to look into the
> PCA9541, for which Guenter has written a driver.

Thanks for that. Unfortunately, I am bound to existing hardware on this
platform. I will keep it in mind for future designs though.

Since my first post, I have stumbled on more problems with multimaster. Namely,
the master drivers are not consistent with the way they deal with arbitration
loss. I've all but re-written i2c-xiic, and dealt with arblost by catching
BusNotBusy interrupt and retrying the transfer. But the same cannot be said
about i2c-kempld (Kontron's in-PLD i2c master). Even if I manage to get the
kempld driver to handle this correctly, xiic still has some issues. The IP core
seems to enter a state when scanning unpopulated addresses (i2cdetect) for which
only a soft reset is required to recover, but this means it looses track of the
peer start-stop sequences and it's detection of the bus being busy or not.

I have decided to abandon the multimaster option for now as the hurdles are too
great. The only time I need another i2c master is when I need to hot reset the
fpga which houses the xiic core. (the reset pin is driven from gpo on an i2c i/o
expander). I will therefore handle this special case in userspace by unloading
i2c-xiic, then loading i2c-kempld. The slaves are arch declared on hard-coded
bus 0, so this should just work.

Thanks for all your help!
/jfd

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean Delvare]

On Mon, 13 May 2013 09:54:47 -0400, Jean-François Dagenais wrote:
> Salut Jean, merci de participer!
> 
> On 2013-05-13, at 4:11 AM, Jean Delvare wrote:
> 
> > 
> > Guenter is right. You never have to disable auto-detection in the slave
> > drivers (jc42 etc.) All these slave drivers do is claim "I _can_ do
> > auto-detection", not "I _will_ do auto-detection." It's always up to the
> > I2C adapter driver, whether auto-detection will happen or not. And it
> > is disabled by default. So if you don't want it, just do not enable it
> > in the bus driver. You can even set it per adapter, when the driver
> > controls more than one adapter, and per bus segment, when multiplexing
> > is taking place.
> 
> I am just wondering where the clean hook is for doing this. From what I can
> gather, the master driver(s I've seen) declare ".class = I2C_CLASS_HWMON |
> I2C_CLASS_SPD," pretty statically. Is it just that they are missing this
> flexibility? Something along the lines of patching the pdata of such a master
> driver to provide a 'class' variable in pdata?

Yes, exactly.

> If so, one would have to take
> into account the existing users of the master which expect the previous class
> setting which may not be '0', thus requiring patching the existing upstreamed
> users...  Suggestions?

Yes, you have to do something like that. The static class declarations
come from the PC world drivers where they (almost) never change.
Embedded systems definitely want a per-bus decision and should avoid
static declarations as much as possible. Especially when in most cases
they know exactly what slaves they are so they don't need
auto-detection. There's a reason why auto-detection is an optional
feature...

-- 
Jean Delvare

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Guenter Roeck]

On Mon, May 13, 2013 at 04:14:13PM +0200, Jean Delvare wrote:
> On Mon, 13 May 2013 09:54:47 -0400, Jean-François Dagenais wrote:
> > Salut Jean, merci de participer!
> > 
> > On 2013-05-13, at 4:11 AM, Jean Delvare wrote:
> > 
> > > 
> > > Guenter is right. You never have to disable auto-detection in the slave
> > > drivers (jc42 etc.) All these slave drivers do is claim "I _can_ do
> > > auto-detection", not "I _will_ do auto-detection." It's always up to the
> > > I2C adapter driver, whether auto-detection will happen or not. And it
> > > is disabled by default. So if you don't want it, just do not enable it
> > > in the bus driver. You can even set it per adapter, when the driver
> > > controls more than one adapter, and per bus segment, when multiplexing
> > > is taking place.
> > 
> > I am just wondering where the clean hook is for doing this. From what I can
> > gather, the master driver(s I've seen) declare ".class = I2C_CLASS_HWMON |
> > I2C_CLASS_SPD," pretty statically. Is it just that they are missing this
> > flexibility? Something along the lines of patching the pdata of such a master
> > driver to provide a 'class' variable in pdata?
> 
> Yes, exactly.
> 
Or use devicetree, which is quite prevalent in embedded systems nowadays.

Guenter

> > If so, one would have to take
> > into account the existing users of the master which expect the previous class
> > setting which may not be '0', thus requiring patching the existing upstreamed
> > users...  Suggestions?
> 
> Yes, you have to do something like that. The static class declarations
> come from the PC world drivers where they (almost) never change.
> Embedded systems definitely want a per-bus decision and should avoid
> static declarations as much as possible. Especially when in most cases
> they know exactly what slaves they are so they don't need
> auto-detection. There's a reason why auto-detection is an optional
> feature...
> 
> -- 
> Jean Delvare
>