Skip to contentSkip to conversation

Add class definition to the elektor bus driver

PATCH New Jean Delvare 3 participants 7 messages May 19, 2005 I2C subsystem host drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Add class definition to the elektor bus driver

Hi Frank, all,

Quoted 13 lines
Quoted 2 lines

Which bus driver are you using? It obviously lacks class declaration, so the correct fix is to add the class there.

The modules that are loading are (in reverse order):

     adm1031
     ad5321
     mic184
     pca9540
     i2c_sensor
     i2c_elektor
     i2c_algo_pcf
     i2c_core
Quoted 2 lines

So I believe what you are asking for is the i2c_elektor driver for the PCF8584 ISA to I2C chip.

Correct, I just checked and this one actually lacks its class. Patch follows.

Greg, can you please apply this one to your tree?
Thanks.

This patch adds a class definition to the elektor i2c bus driver. Without this definition, hardware monitoring chips located on such busses cannot possibly be driven.

drivers/i2c/busses/i2c-elektor.c renamed from drivers/i2c/busses/i2c-elektor.c.orig +1 -0
@@ -183,6 +183,7 @@
 
 static struct i2c_adapter pcf_isa_ops = {
 	.owner		= THIS_MODULE,
+	.class		= I2C_CLASS_HWMON,
 	.id		= I2C_HW_P_ELEK,
 	.algo_data	= &pcf_isa_data,
 	.name		= "PCF8584 ISA adapter",
Signature · 1 line
Jean Delvare
Applied
#1 lore raw

PATCH 1/1 Add class definition to the elektor bus driver

Quoted 21 lines · Jean Delvare

Hi Frank, all,

Quoted 13 lines
Quoted 2 lines

Which bus driver are you using? It obviously lacks class declaration, so the correct fix is to add the class there.

The modules that are loading are (in reverse order):

     adm1031
     ad5321
     mic184
     pca9540
     i2c_sensor
     i2c_elektor
     i2c_algo_pcf
     i2c_core
Quoted 2 lines

So I believe what you are asking for is the i2c_elektor driver for the PCF8584 ISA to I2C chip.

Correct, I just checked and this one actually lacks its class. Patch follows.

Quoted 8 lines

Greg, can you please apply this one to your tree?
Thanks.

This patch adds a class definition to the elektor i2c bus driver. Without this definition, hardware monitoring chips located on such busses cannot possibly be driven.

Applied, thanks.

greg k-h

#2 lore raw

PATCH 1/1 Add class definition to the elektor bus driver

In reply to Jean Delvare

Hi Frank,

Quoted 6 lines

The patch works BUT...
The PCF8584 device is a bridge, not a hardware monitor. One of the other I2C device types could very well be behind it such as a sound chip. Maybe we could add a BRIDGE type or have it as a I2C_CLASS_ALL device. This is just a suggestion - like I said, the patch works for me.

The PCF8584 is an I2C master, as I understand it. It's as much of a bridge as every other I2C or SMBus master is. The idea of a bridge class doesn't make any sense to me. A class is meant to isolate chip drivers from bus drivers. As such, class definitions are carried by bus drivers and checked by chip drivers. This doesn't leave any room for a bridge class unless I'm missing something obvious.

Setting class to I2C_CLASS_ALL would somehow defeat our attempt to isolate different types of I2C busses. It's better to add classes when it is found that we actually need them.

That being said, I don't know exactly where the PCF8584 is found and what the Elektor is. Care to tell me what kind of hardware your are playing with?

Thanks,

Signature · 1 line
Jean Delvare
#3 lore raw

PATCH 1/1 Add class definition to the elektor bus driver

In reply to Jean Delvare
Quoted 5 lines · Jean Delvare
Quoted 4 lines

So I believe what you are asking for is the i2c_elektor driver for the PCF8584 ISA to I2C chip.

Quoted 8 lines

Correct, I just checked and this one actually lacks its class. Patch follows.

Greg, can you please apply this one to your tree?
Thanks.

The patch works BUT...
The PCF8584 device is a bridge, not a hardware monitor. One of the other I2C device types could very well be behind it such as a sound chip. Maybe we could add a BRIDGE type or have it as a I2C_CLASS_ALL device. This is just a suggestion - like I said, the patch works for me.

Frank

Quoted 20 lines

This patch adds a class definition to the elektor i2c bus driver. Without this definition, hardware monitoring chips located on such busses cannot possibly be driven.

drivers/i2c/busses/i2c-elektor.c renamed from drivers/i2c/busses/i2c-elektor.c.orig +1 -0
@@ -183,6 +183,7 @@
 
 static struct i2c_adapter pcf_isa_ops = {
 	.owner		= THIS_MODULE,
+	.class		= I2C_CLASS_HWMON,
 	.id		= I2C_HW_P_ELEK,
 	.algo_data	= &pcf_isa_data,
 	.name		= "PCF8584 ISA adapter",
#4 lore raw

PATCH 1/1 Add class definition to the elektor bus driver

In reply to Jean Delvare
Quoted 24 lines · Jean Delvare

Hi Frank,

Quoted 8 lines

The patch works BUT...
The PCF8584 device is a bridge, not a hardware monitor. One of the other I2C device types could very well be behind it such as a sound chip. Maybe we could add a BRIDGE type or have it as a I2C_CLASS_ALL device. This is just a suggestion - like I said, the patch works for me.

The PCF8584 is an I2C master, as I understand it. It's as much of a bridge as every other I2C or SMBus master is. The idea of a bridge class doesn't make any sense to me. A class is meant to isolate chip drivers from bus drivers. As such, class definitions are carried by bus drivers and checked by chip drivers. This doesn't leave any room for a bridge class unless I'm missing something obvious.

Setting class to I2C_CLASS_ALL would somehow defeat our attempt to isolate different types of I2C busses. It's better to add classes when it is found that we actually need them.

Quoted 7 lines

That being said, I don't know exactly where the PCF8584 is found and what the Elektor is. Care to tell me what kind of hardware your are playing with?

Thanks,

In my application the PCF8584 is located on the ISA bus from the cpu's south bridge. I have a Cypress CPLD doing the ISA address decoding and chip selects. From the list I sent earlier, you can see that I chain several sensors on this I2C bus. The one part I didn't include in my list earlier is a X24165 EEPROM device (going obsolete and being replaced by X4165) of which I have four installed. I have a driver for this part but it is not completely tested so I have not submitted it for consideration. FYI - I don't know where the elektor name came from.

I understand and agree with your comments about isolating chip drivers from bus drivers. My point is since the Elektor is just a bus master, it does not know what class of chips might be placed on its bus just that they are I2C. The only analogy I can think of is saying a PCI Bridge/Master chip is of class "video" because there is a video device on its bus. There may also be an ethernet device, sound device, etc. This is where my comment comes from that the use of the Elektor driver/module does not define that only hardware monitor chips/devices are located on the bus. Yes is capable of communicating with hardware monitor chips but it can also communicate with other classes such as eeprom and sound.

Thanks

#5 lore raw

PATCH 1/1 Add class definition to the elektor bus driver

In reply to Jean Delvare

Hi Frank,

Sorry for the delay.

Quoted 4 lines

I understand and agree with your comments about isolating chip drivers from bus drivers. My point is since the Elektor is just a bus master, it does not know what class of chips might be placed on its bus just that they are I2C.

Same is true of all I2C and SMBus masters.

Quoted 3 lines

The only analogy I can think of is saying a PCI Bridge/Master chip is of class "video" because there is a video device on its bus. There may also be an ethernet device, sound device, etc.

The analogy doesn't fly IMHO. The PCI specification handles the classes natively, and additionally supports identification of devices through unique IDs. The I2C bus (or SMBus which builts on top of it) doesn't have anything like that. The only way we can find out what a chip on such a bus is, is by probing it manually, and recognize a register map we already know of. There is no way to find out the class of a given chip.

This is the reason why classes were introduced. I still agree with you that in theory, any type of chip could be connected to every bus. But it happens not to be true in the real world. So classes let us skip a number of probes we know cannot succeed, and at the same time probes that could have confused some chips. Sure this isn't a perfect solution, but there's hardly more we can do.

Quoted 5 lines

This is where my comment comes from that the use of the Elektor driver/module does not define that only hardware monitor chips/devices are located on the bus. Yes is capable of communicating with hardware monitor chips but it can also communicate with other classes such as eeprom and sound.

Agreed. If this ever really happens, we'll update the class definition accordingly.

Note that the way things are done, it is possible to alter the class of a bus driver depending on the running system. If a given bus driver never hosts a given class of chips except on one given system, the class can by dynamically altered for this one system, so that the behavior for the other systems will be left untouched. I don't think this has ever been done, but this should be remembered.

Thanks,

Signature · 1 line
Jean Delvare
#6 lore raw

PATCH 1/1 Add class definition to the elektor bus driver

In reply to Jean Delvare

Works for me.
Thanks

Quoted 60 lines · Jean Delvare

Hi Frank,

Sorry for the delay.

Quoted 6 lines

I understand and agree with your comments about isolating chip drivers from bus drivers. My point is since the Elektor is just a bus master, it does not know what class of chips might be placed on its bus just that they are I2C.

Same is true of all I2C and SMBus masters.

Quoted 5 lines

The only analogy I can think of is saying a PCI Bridge/Master chip is of class "video" because there is a video device on its bus. There may also be an ethernet device, sound device, etc.

The analogy doesn't fly IMHO. The PCI specification handles the classes natively, and additionally supports identification of devices through unique IDs. The I2C bus (or SMBus which builts on top of it) doesn't have anything like that. The only way we can find out what a chip on such a bus is, is by probing it manually, and recognize a register map we already know of. There is no way to find out the class of a given chip.

This is the reason why classes were introduced. I still agree with you that in theory, any type of chip could be connected to every bus. But it happens not to be true in the real world. So classes let us skip a number of probes we know cannot succeed, and at the same time probes that could have confused some chips. Sure this isn't a perfect solution, but there's hardly more we can do.

Quoted 7 lines

This is where my comment comes from that the use of the Elektor driver/module does not define that only hardware monitor chips/devices are located on the bus. Yes is capable of communicating with hardware monitor chips but it can also communicate with other classes such as eeprom and sound.

Agreed. If this ever really happens, we'll update the class definition accordingly.

Note that the way things are done, it is possible to alter the class of a bus driver depending on the running system. If a given bus driver never hosts a given class of chips except on one given system, the class can by dynamically altered for this one system, so that the behavior for the other systems will be left untouched. I don't think this has ever been done, but this should be remembered.

Thanks,