[PATCH] i2c-piix4 - Add support for secondary SMBus on AMD SB800 and AMD FCH chipsets
Hello all,
Attached patch adds support for secondary SMBus of AMD SB800 and new AMD FCH
chipsets. The base address of secondary SMBus is different from SB700 and it is
stored on similar place as SB800 primary SMBus.
More verbose info:
Probing function was just modified to read the SMBus base from address 0x28 or
from original 0x2c. The secondary bus does not provide IRQ information.
I think the SB700 has same secondary controller, so revision/IRQ information
should not be printed too. This can be fixed in some other patch.
Chipset datasheet can be found here:
http://support.amd.com/us/Embedded_TechDocs/45482.pdf
Tested on SB800 and FCH boards.
Tested-by: Paul Menzel
ASRock E350M1 with SB800
Signed-off-by: Rudolf Marek
Thanks
Rudolf
diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 39ab78c..7145d3c 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -231,11 +231,11 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
}
static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
- const struct pci_device_id *id)
+ const struct pci_device_id *id, u8 aux)
{
unsigned short piix4_smba;
unsigned short smba_idx = 0xcd6;
- u8 smba_en_lo, smba_en_hi, i2ccfg, i2ccfg_offset = 0x10, smb_en = 0x2c;
+ u8 smba_en_lo, smba_en_hi, i2ccfg, i2ccfg_offset = 0x10, smb_en;
/* SB800 and later SMBus does not support forcing address */
if (force || force_addr) {
@@ -245,6 +245,8 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
}
/* Determine the address of the SMBus areas */
+ smb_en = (aux) ? 0x28 : 0x2c;
+
if (!request_region(smba_idx, 2, "smba_idx")) {
dev_err(&PIIX4_dev->dev, "SMBus base address index region "
"0x%x already in use!\n", smba_idx);
@@ -272,6 +274,13 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
return -EBUSY;
}
+ /* Aux SMBus does not support IRQ information */
+ if (aux) {
+ dev_info(&PIIX4_dev->dev,
+ "SMBus Host Controller at 0x%x\n", piix4_smba);
+ return piix4_smba;
+ }
+
/* Request the SMBus I2C bus config region */
if (!request_region(piix4_smba + i2ccfg_offset, 1, "i2ccfg")) {
dev_err(&PIIX4_dev->dev, "SMBus I2C bus config region "
@@ -596,7 +605,7 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id)
dev->revision >= 0x40) ||
dev->vendor == PCI_VENDOR_ID_AMD)
/* base address location etc changed in SB800 */
- retval = piix4_setup_sb800(dev, id);
+ retval = piix4_setup_sb800(dev, id, 0);
else
retval = piix4_setup(dev, id);
@@ -610,17 +619,29 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id)
return retval;
/* Check for auxiliary SMBus on some AMD chipsets */
+ retval = -ENODEV;
+
if (dev->vendor == PCI_VENDOR_ID_ATI &&
- dev->device == PCI_DEVICE_ID_ATI_SBX00_SMBUS &&
- dev->revision < 0x40) {
- retval = piix4_setup_aux(dev, id, 0x58);
- if (retval > 0) {
- /* Try to add the aux adapter if it exists,
- * piix4_add_adapter will clean up if this fails */
- piix4_add_adapter(dev, retval, &piix4_aux_adapter);
+ dev->device == PCI_DEVICE_ID_ATI_SBX00_SMBUS) {
+ if (dev->revision < 0x40) {
+ retval = piix4_setup_aux(dev, id, 0x58);
+ } else {
+ /* SB800 added aux bus too */
+ retval = piix4_setup_sb800(dev, id, 1);
}
}
+ if (dev->vendor == PCI_VENDOR_ID_AMD &&
+ dev->device == PCI_DEVICE_ID_AMD_HUDSON2_SMBUS) {
+ retval = piix4_setup_sb800(dev, id, 1);
+ }
+
+ if (retval > 0) {
+ /* Try to add the aux adapter if it exists,
+ * piix4_add_adapter will clean up if this fails */
+ piix4_add_adapter(dev, retval, &piix4_aux_adapter);
+ }
+
return 0;
}
Re: [PATCH] i2c: suppress lockdep warning on delete_device
On Fri, May 17, 2013 at 02:56:35PM +0200, Alexander Sverdlin wrote: > i2c: suppress lockdep warning on delete_device > > Since commit 846f99749ab68bbc7f75c74fec305de675b1a1bf the following lockdep > warning is thrown in case i2c device is removed (via delete_device sysfs > attribute) which contains subdevices (e.g. i2c multiplexer): Applied to for-current with Alan's ack, thanks! I couldn't apply the patch due to whitespace problems. I fixed them here because I wanted to have the patch in my very soon pull request. But please have a look. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: suppress lockdep warning on delete_device
On Fri, 17 May 2013, Alexander Sverdlin wrote: > Hi! > > On 05/17/2013 05:31 PM, ext Alan Stern wrote: > i2c: suppress lockdep warning on delete_device > > Since commit 846f99749ab68bbc7f75c74fec305de675b1a1bf the following > lockdep > warning is thrown in case i2c device is removed (via delete_device sysfs > attribute) which contains subdevices (e.g. i2c multiplexer): > >>> > >>> Can you explain this in a little more detail? Exactly what does the > >>> delete_device attribute do, when called for device D? > >>> > >>> That is, what does a write to /sys/bus/i2c/devices/D/delete_device do? > >> > >> Like USB with its hubs, I2C structure could be tree-like, with I2C > >> multiplexers. > >> delete_device attribute removes I2C device from the subsystem, which is > >> usually not > >> a problem, except the case when device is a multiplexer and sub-devices > >> should be > >> removed first. Here we have the problem: holding a "s_active" lock on > >> "delete_device" attribute of a parent (multiplexer) we need to delete > >> children, but > >> they were created with the same static attribute "delete_device". > >> > >> The safety of this operation is exactly the same as in USB case... Any > >> more clever > >> lockdep annotation is exactly not so easy... > > > > If I understand you correctly, if D is an I2C multiplexer then writing > > to /sys/bus/i2c/devices/D/delete_device will get rid of all of D's > > children (and their descendants) and will also get rid of D itself -- > > right? > > > > That's _not_ what the USB attributes do. For example, if D is a USB > > hub then writing 0 to /sys/bus/usb/devices/D/bConfigurationValue will > > get rid of all D's descendants but will not get rid of D. > > Well, seems that what I've described above wasn't precise enough... That's why I asked what "exactly" the attribute does. :-) > Actually only "bus controllers" have "delete_device" attribute. Simple devices > on the bus do not have it. User write an address of the slave device to this > attribute to delete it. This works fine for simple devices. In case of > multiplexer > (which is in turn also a "bus controller") there are nested "delete_device" > attributes. So it's only possible to delete child nodes and not the > controller itself, > writing to its "delete_device". So from my POV, it fits to USB concept... Okay. If writing to the attribute doesn't delete the object which the attribute is attached to, then your patch is the correct approach. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] I2C: mv64xxx: work around signals causing I2C transactions to be aborted
On Thu, May 16, 2013 at 09:30:59PM +0100, Russell King wrote: > Do not use interruptible waits in an I2C driver; if a process uses > signals (eg, Xorg uses SIGALRM and SIGPIPE) then these signals can > cause the I2C driver to abort a transaction in progress by another > driver, which can cause that driver to fail. I2C drivers are not > expected to abort transactions on signals. > > Signed-off-by: Russell King > --- I don't have hardware to test but I have no issues with these patches so, FWIW: Acked-by: Mark A. Greer for the entire series. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: suppress lockdep warning on delete_device
Hi! On 05/17/2013 05:31 PM, ext Alan Stern wrote: i2c: suppress lockdep warning on delete_device Since commit 846f99749ab68bbc7f75c74fec305de675b1a1bf the following lockdep warning is thrown in case i2c device is removed (via delete_device sysfs attribute) which contains subdevices (e.g. i2c multiplexer): Can you explain this in a little more detail? Exactly what does the delete_device attribute do, when called for device D? That is, what does a write to /sys/bus/i2c/devices/D/delete_device do? Like USB with its hubs, I2C structure could be tree-like, with I2C multiplexers. delete_device attribute removes I2C device from the subsystem, which is usually not a problem, except the case when device is a multiplexer and sub-devices should be removed first. Here we have the problem: holding a "s_active" lock on "delete_device" attribute of a parent (multiplexer) we need to delete children, but they were created with the same static attribute "delete_device". The safety of this operation is exactly the same as in USB case... Any more clever lockdep annotation is exactly not so easy... If I understand you correctly, if D is an I2C multiplexer then writing to /sys/bus/i2c/devices/D/delete_device will get rid of all of D's children (and their descendants) and will also get rid of D itself -- right? That's _not_ what the USB attributes do. For example, if D is a USB hub then writing 0 to /sys/bus/usb/devices/D/bConfigurationValue will get rid of all D's descendants but will not get rid of D. Well, seems that what I've described above wasn't precise enough... Actually only "bus controllers" have "delete_device" attribute. Simple devices on the bus do not have it. User write an address of the slave device to this attribute to delete it. This works fine for simple devices. In case of multiplexer (which is in turn also a "bus controller") there are nested "delete_device" attributes. So it's only possible to delete child nodes and not the controller itself, writing to its "delete_device". So from my POV, it fits to USB concept... Instead of doing it this way, the attribute method should call device_schedule_callback(). See commit d9a9cdfb078d. -- Best regards, Alexander Sverdlin. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: suppress lockdep warning on delete_device
On Fri, 17 May 2013, Alexander Sverdlin wrote: > Hi! > > On 05/17/2013 04:42 PM, ext Alan Stern wrote: > >> i2c: suppress lockdep warning on delete_device > >> > >> Since commit 846f99749ab68bbc7f75c74fec305de675b1a1bf the following lockdep > >> warning is thrown in case i2c device is removed (via delete_device sysfs > >> attribute) which contains subdevices (e.g. i2c multiplexer): > > > > Can you explain this in a little more detail? Exactly what does the > > delete_device attribute do, when called for device D? > > > > That is, what does a write to /sys/bus/i2c/devices/D/delete_device do? > > Like USB with its hubs, I2C structure could be tree-like, with I2C > multiplexers. > delete_device attribute removes I2C device from the subsystem, which is > usually not > a problem, except the case when device is a multiplexer and sub-devices > should be > removed first. Here we have the problem: holding a "s_active" lock on > "delete_device" attribute of a parent (multiplexer) we need to delete > children, but > they were created with the same static attribute "delete_device". > > The safety of this operation is exactly the same as in USB case... Any more > clever > lockdep annotation is exactly not so easy... If I understand you correctly, if D is an I2C multiplexer then writing to /sys/bus/i2c/devices/D/delete_device will get rid of all of D's children (and their descendants) and will also get rid of D itself -- right? That's _not_ what the USB attributes do. For example, if D is a USB hub then writing 0 to /sys/bus/usb/devices/D/bConfigurationValue will get rid of all D's descendants but will not get rid of D. Instead of doing it this way, the attribute method should call device_schedule_callback(). See commit d9a9cdfb078d. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: suppress lockdep warning on delete_device
Hi! On 05/17/2013 04:42 PM, ext Alan Stern wrote: i2c: suppress lockdep warning on delete_device Since commit 846f99749ab68bbc7f75c74fec305de675b1a1bf the following lockdep warning is thrown in case i2c device is removed (via delete_device sysfs attribute) which contains subdevices (e.g. i2c multiplexer): Can you explain this in a little more detail? Exactly what does the delete_device attribute do, when called for device D? That is, what does a write to /sys/bus/i2c/devices/D/delete_device do? Like USB with its hubs, I2C structure could be tree-like, with I2C multiplexers. delete_device attribute removes I2C device from the subsystem, which is usually not a problem, except the case when device is a multiplexer and sub-devices should be removed first. Here we have the problem: holding a "s_active" lock on "delete_device" attribute of a parent (multiplexer) we need to delete children, but they were created with the same static attribute "delete_device". The safety of this operation is exactly the same as in USB case... Any more clever lockdep annotation is exactly not so easy... -- Best regards, Alexander Sverdlin. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: suppress lockdep warning on delete_device
On Fri, 17 May 2013, Alexander Sverdlin wrote: > i2c: suppress lockdep warning on delete_device > > Since commit 846f99749ab68bbc7f75c74fec305de675b1a1bf the following lockdep > warning is thrown in case i2c device is removed (via delete_device sysfs > attribute) which contains subdevices (e.g. i2c multiplexer): Can you explain this in a little more detail? Exactly what does the delete_device attribute do, when called for device D? That is, what does a write to /sys/bus/i2c/devices/D/delete_device do? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: busses: remove superfluous comment
On Fri, May 17, 2013 at 11:53:23AM +0200, Wolfram Sang wrote: > The comment already got copy&pasted around and is not really useful. > Remove it. > > Signed-off-by: Wolfram Sang FWIW on i2c-omap.c: Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
[PATCH] i2c: suppress lockdep warning on delete_device
i2c: suppress lockdep warning on delete_device
Since commit 846f99749ab68bbc7f75c74fec305de675b1a1bf the following lockdep
warning is thrown in case i2c device is removed (via delete_device sysfs
attribute) which contains subdevices (e.g. i2c multiplexer):
=
[ INFO: possible recursive locking detected ]
3.8.7-0-sampleversion-fct #8 Tainted: G O
-
bash/3743 is trying to acquire lock:
(s_active#110){.+}, at: []
sysfs_hash_and_remove+0x58/0xc8
but task is already holding lock:
(s_active#110){.+}, at: [] sysfs_write_file+0xc8/0x208
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
lock(s_active#110);
lock(s_active#110);
*** DEADLOCK ***
May be due to missing lock nesting notation
4 locks held by bash/3743:
#0: (&buffer->mutex){+.+.+.}, at: []
sysfs_write_file+0x4c/0x208
#1: (s_active#110){.+}, at: []
sysfs_write_file+0xc8/0x208
#2: (&adap->userspace_clients_lock/1){+.+.+.}, at: []
i2c_sysfs_delete_device+0x90/0x238
#3: (&__lockdep_no_validate__){..}, at: []
device_release_driver+0x24/0x48
stack backtrace:
Call Trace:
[] dump_stack+0x8/0x34
[] __lock_acquire+0x161c/0x2110
[] lock_acquire+0x4c/0x70
[] sysfs_addrm_finish+0x19c/0x1e0
[] sysfs_hash_and_remove+0x58/0xc8
[] sysfs_remove_group+0x64/0x148
[] device_remove_attrs+0x9c/0x1a8
[] device_del+0x104/0x1d8
[] device_unregister+0x28/0x70
[] i2c_del_adapter+0x1cc/0x328
[] i2c_del_mux_adapter+0x14/0x38
[] pca954x_remove+0x90/0xe0 [pca954x]
[] i2c_device_remove+0x80/0xe8
[] __device_release_driver+0x74/0xf8
[] device_release_driver+0x2c/0x48
[] bus_remove_device+0x13c/0x1d8
[] device_del+0x10c/0x1d8
[] device_unregister+0x28/0x70
[] i2c_sysfs_delete_device+0x180/0x238
[] sysfs_write_file+0xe4/0x208
[] vfs_write+0xbc/0x160
[] SyS_write+0x54/0xd8
[] handle_sys64+0x44/0x64
The problem is already known for USB and PCI subsystems. The reason is that
delete_device attribute is defined statically in i2c-core.c and used for all
devices in i2c subsystem.
Discussion of original USB problem:
http://lkml.indiana.edu/hypermail/linux/kernel/1204.3/01160.html
Commit 356c05d58af05d582e634b54b40050c73609617b introduced new macro to suppress
lockdep warnings for this special case and included workaround for USB code.
LKML discussion of the workaround:
http://lkml.indiana.edu/hypermail/linux/kernel/1205.1/03634.html
As i2c case is in principle the same, the same workaround could be used here.
Signed-off-by: Alexander Sverdlin
Cc: Eric W. Biederman
Cc: Tejun Heo
Cc: Alan Stern
Cc: Peter Zijlstra
Cc: Greg Kroah-Hartman
---
--- linux-next.orig/drivers/i2c/i2c-core.c
+++ linux-next/drivers/i2c/i2c-core.c
@@ -892,7 +892,8 @@ i2c_sysfs_delete_device(struct device *d
}
static DEVICE_ATTR(new_device, S_IWUSR, NULL, i2c_sysfs_new_device);
-static DEVICE_ATTR(delete_device, S_IWUSR, NULL, i2c_sysfs_delete_device);
+static DEVICE_ATTR_IGNORE_LOCKDEP(delete_device, S_IWUSR, NULL,
+ i2c_sysfs_delete_device);
static struct attribute *i2c_adapter_attrs[] = {
&dev_attr_name.attr,
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: modprobe hang at startup (3.8.x, 3.9.x, IBM x3550)
On Fri, May 17, 2013 at 10:36:22AM +0200, Jean Delvare wrote: > IPMI is still likely to access the SMBus controller. If there's a BMC > in the machine, it can also access the SMBus slave with its own > controller. It would be good to rule this out by disabling IPMI > completely, removing the BMC from the machine if it has one, and > checking if it makes the issue go away or not. This ended up being easier than I thought. The BMC can't be physically removed, but there is a jumper on the board to disable it. We flipped it and got a message during POST about it not being present. Additionally all IPMI functions did nothing (hung, but interruptable) which is what you'd expect. I think it really is disabled. In this state, I re-ran the previous tests, with identical results. That is: - "modprobe i2c_i801" succeeds - "modprobe i2c_i801 disable_features=0x10" succeeds - With interrupts disabled, "modprobe ics932s401" - With interrupts enabled, "modprobe ics932s401" hangs - With interrupts enabled, "i2cget 4 0x50 0x00" hangs I'll leave the BMC disabled for now in case that's important for further testing. If you need other tests run with the BMC enabled, I'll use a differen machine. Cheers, Rob. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] I2C: mv64xxx: work around signals causing I2C transactions to be aborted
On Thu, May 16, 2013 at 09:30:59PM +0100, Russell King wrote: > Do not use interruptible waits in an I2C driver; if a process uses > signals (eg, Xorg uses SIGALRM and SIGPIPE) then these signals can > cause the I2C driver to abort a transaction in progress by another > driver, which can cause that driver to fail. I2C drivers are not > expected to abort transactions on signals. > > Signed-off-by: Russell King Applied to for-current, thanks! Rest will be reviewed later... -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 9/9] I2C: mv64xxx: fix race between FSM/interrupt and process context
On Fri, May 17, 2013 at 11:00:16AM +0100, Russell King - ARM Linux wrote:
> On Fri, May 17, 2013 at 11:51:51AM +0200, Wolfram Sang wrote:
> > > {
> > > switch(drv_data->action) {
> > > case MV64XXX_I2C_ACTION_SEND_RESTART:
> > > + /* We should only get here if we have further messages */
> > > + BUG_ON(drv_data->num_msgs == 0);
> > > +
> >
> > ...
> >
> > > @@ -453,16 +463,20 @@ static int
> > > mv64xxx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int
> > > num)
> > > {
> > > struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap);
> > > - int i, rc;
> > > + int rc, ret = num;
> > >
> > > - for (i = 0; i < num; i++) {
> > > - rc = mv64xxx_i2c_execute_msg(drv_data, &msgs[i],
> > > - i == 0, i + 1 == num);
> > > - if (rc < 0)
> > > - return rc;
> > > - }
> > > + BUG_ON(drv_data->msgs != NULL);
> >
> > Can't we handle this more gracefully than to halt the kernel? Like
> > WARN_ON and resetting the controller or disabling the bus or...
>
> Well, the latter really is something which should never ever happen,
> and if it does happen it can only really be because something's
> screwed up in the locking in the I2C layer.
I'd think we should trust the layer here.
> The former is more probable, and I thought about that, but I don't
> have any good alternative solution. Given the problems I've seen,
> I don't think resetting the controller is really an option, because
> that'll likely cause the bus to lock (that's the original problem
> which I'm trying to solve in this patch.) The thing really does
> have to work according to the I2C protocol otherwise stuff goes
> irrecoverably wrong to the point of needing an entire system reset.
Fine with me for now. If somebody later has a setup where I2C slaves can
be reset (e.g. via GPIO), so a complete bus reset is possible, we might
need another solution, then.
Thanks,
Wolfram
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-designware: fix RX FIFO overrun
>> Applied to for-current, thanks! > > Added stable, too. > Thanks Wolfram. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: modprobe hang at startup (3.8.x, 3.9.x, IBM x3550)
On Fri, 17 May 2013 20:27:06 +1000, Robert Norris wrote: > On Fri, May 17, 2013 at 10:49:28AM +0200, Jean Delvare wrote: > > Hmm, can you please dump the PCI configuration space of the SMBus > > controller? > > > > # /sbin/lspci -s 00:1f.3 -xxx > > 00:1f.3 SMBus: Intel Corporation 631xESB/632xESB/3100 Chipset SMBus > Controller (rev 09) > 00: 86 80 9b 26 41 05 80 02 09 00 05 0c 00 00 00 00 > 10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 20: 41 04 00 00 00 00 00 00 00 00 00 00 14 10 dd 02 > 30: 00 00 00 00 00 00 00 00 00 00 00 00 00 02 00 00 > 40: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ^^ Hmm, no, SMI# isn't enabled. Wrong theory. -- Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: modprobe hang at startup (3.8.x, 3.9.x, IBM x3550)
On Fri, May 17, 2013 at 10:49:28AM +0200, Jean Delvare wrote: > Hmm, can you please dump the PCI configuration space of the SMBus > controller? > > # /sbin/lspci -s 00:1f.3 -xxx 00:1f.3 SMBus: Intel Corporation 631xESB/632xESB/3100 Chipset SMBus Controller (rev 09) 00: 86 80 9b 26 41 05 80 02 09 00 05 0c 00 00 00 00 10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 20: 41 04 00 00 00 00 00 00 00 00 00 00 14 10 dd 02 30: 00 00 00 00 00 00 00 00 00 00 00 00 00 02 00 00 40: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f0: 00 00 00 00 00 00 00 00 80 0f 01 00 00 00 00 00 Rob. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: modprobe hang at startup (3.8.x, 3.9.x, IBM x3550)
On Fri, May 17, 2013 at 05:54:33PM +0800, Daniel Kurtz wrote: > Was Robert able to get the system working without hangs by disabling > the IRQ feature of i2c-i801 module when it was builtin? Yes. There are no hangs when interrupts are explicitly disabled with disable_features=0x10 or when 6676a847 is reverted and the module rebuilt. Cheers, Rob. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: modprobe hang at startup (3.8.x, 3.9.x, IBM x3550)
On Fri, May 17, 2013 at 10:36:22AM +0200, Jean Delvare wrote: > IPMI is still likely to access the SMBus controller. If there's a BMC > in the machine, it can also access the SMBus slave with its own > controller. It would be good to rule this out by disabling IPMI > completely, removing the BMC from the machine if it has one, and > checking if it makes the issue go away or not. I think they do have a BMC (dmidecode would suggest so). I'll need to confirm this, and then get the datacentre support guys to pull it (yeah, messy - inherited machines on the other side of the world). It might take a couple of days, I'll let you know how it goes. Cheers, Rob. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9] i2c: exynos5: add High Speed I2C controller driver
Adds support for High Speed I2C driver found in Exynos5 and
later SoCs from Samsung.
Driver only supports Device Tree method.
Changes since v1:
1. Added FIFO functionality
2. Added High speed mode functionality
3. Remove SMBUS_QUICK
4. Remove the debugfs functionality
5. Use devm_* functions where ever possible
6. Driver is free from GPIO configs (only supports pinctrl method)
7. Use OF data string "clock-frequency" to get the bus operating frequencies
8. Split the clock divisor calculation function
9. Add resets for the failed transacton cases
10. few other bug fixes and cosmetic changes
Signed-off-by: Taekgyun Ko
Signed-off-by: Naveen Krishna Chatradhi
Reviewed-by: Simon Glass
Tested-by: Andrew Bresticker
Signed-off-by: Yuvaraj Kumar C D
Signed-off-by: Andrew Bresticker
---
Changes since v8
1. improved the device tree bindings description page for i2c-exynos5
2. fixed the return value check for devm_ioremap_resource
.../devicetree/bindings/i2c/i2c-exynos5.txt| 45 +
drivers/i2c/busses/Kconfig |7 +
drivers/i2c/busses/Makefile|1 +
drivers/i2c/busses/i2c-exynos5.c | 888
4 files changed, 941 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
create mode 100644 drivers/i2c/busses/i2c-exynos5.c
diff --git a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
new file mode 100644
index 000..29c01c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
@@ -0,0 +1,45 @@
+* Samsung's High Speed I2C controller
+
+The Samsung's High Speed I2C controller is used to interface with I2C devices
+at various speeds ranging from 100khz to 3.4Mhz.
+
+Required properties:
+ - compatible: value should be.
+ -> "samsung,exynos5-hsi2c", for i2c compatible with exynos5 hsi2c.
+ - reg: physical base address of the controller and length of memory mapped
+region.
+ - interrupts: interrupt number to the cpu.
+ - #address-cells: always 1 (for i2c addresses)
+ - #size-cells: always 0
+
+ - Pinctrl:
+- pinctrl-0: Pin control group to be used for this controller.
+- pinctrl-names: Should contain only one value - "default".
+
+Optional properties:
+ - samsung,hs-mode: Mode of operation, High speed or Fast speed mode. If not
+specified, default value is 0.
+ - clock-frequency: Desired operating frequency in Hz of the bus.
+If not specified, the default value in Hz is 10.
+
+Example:
+
+hsi2c@12ca {
+ compatible = "samsung,exynos5-hsi2c";
+ reg = <0x12ca 0x100>;
+ interrupts = <56>;
+ clock-frequency = <10>;
+
+ /* Pinctrl variant begins here */
+ pinctrl-0 = <&i2c4_bus>;
+ pinctrl-names = "default";
+ /* Pinctrl variant ends here */
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ s2mps11_pmic@66 {
+ compatible = "samsung,s2mps11-pmic";
+ reg = <0x66>;
+ };
+};
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index adfee98..49a665f 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -434,6 +434,13 @@ config I2C_EG20T
ML7213/ML7223/ML7831 is companion chip for Intel Atom E6xx series.
ML7213/ML7223/ML7831 is completely compatible for Intel EG20T PCH.
+config I2C_EXYNOS5
+ tristate "Exynos5 high-speed I2C driver"
+ depends on ARCH_EXYNOS5 && OF
+ help
+ Say Y here to include support for high-speed I2C controller in the
+ Exynos5 based Samsung SoCs.
+
config I2C_GPIO
tristate "GPIO-based bitbanging I2C"
depends on GENERIC_GPIO
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 8f4fc23..b19366c 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -42,6 +42,7 @@ i2c-designware-platform-objs := i2c-designware-platdrv.o
obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c-designware-pci.o
i2c-designware-pci-objs := i2c-designware-pcidrv.o
obj-$(CONFIG_I2C_EG20T)+= i2c-eg20t.o
+obj-$(CONFIG_I2C_EXYNOS5) += i2c-exynos5.o
obj-$(CONFIG_I2C_GPIO) += i2c-gpio.o
obj-$(CONFIG_I2C_HIGHLANDER) += i2c-highlander.o
obj-$(CONFIG_I2C_IBM_IIC) += i2c-ibm_iic.o
diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
new file mode 100644
index 000..33c481d
--- /dev/null
+++ b/drivers/i2c/busses/i2c-exynos5.c
@@ -0,0 +1,888 @@
+/**
+ * i2c-exynos5.c - Samsung Exynos5 I2C Controller Driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
Re: [PATCH 9/9] I2C: mv64xxx: fix race between FSM/interrupt and process context
On Fri, May 17, 2013 at 11:51:51AM +0200, Wolfram Sang wrote:
> > {
> > switch(drv_data->action) {
> > case MV64XXX_I2C_ACTION_SEND_RESTART:
> > + /* We should only get here if we have further messages */
> > + BUG_ON(drv_data->num_msgs == 0);
> > +
>
> ...
>
> > @@ -453,16 +463,20 @@ static int
> > mv64xxx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> > {
> > struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap);
> > - int i, rc;
> > + int rc, ret = num;
> >
> > - for (i = 0; i < num; i++) {
> > - rc = mv64xxx_i2c_execute_msg(drv_data, &msgs[i],
> > - i == 0, i + 1 == num);
> > - if (rc < 0)
> > - return rc;
> > - }
> > + BUG_ON(drv_data->msgs != NULL);
>
> Can't we handle this more gracefully than to halt the kernel? Like
> WARN_ON and resetting the controller or disabling the bus or...
Well, the latter really is something which should never ever happen,
and if it does happen it can only really be because something's
screwed up in the locking in the I2C layer.
The former is more probable, and I thought about that, but I don't
have any good alternative solution. Given the problems I've seen,
I don't think resetting the controller is really an option, because
that'll likely cause the bus to lock (that's the original problem
which I'm trying to solve in this patch.) The thing really does
have to work according to the I2C protocol otherwise stuff goes
irrecoverably wrong to the point of needing an entire system reset.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: modprobe hang at startup (3.8.x, 3.9.x, IBM x3550)
On Fri, May 17, 2013 at 4:36 PM, Jean Delvare wrote: > Hi Robert, > > On Thu, 16 May 2013 13:44:55 +1000, Robert Norris wrote: >> On Wed, May 15, 2013 at 09:49:23PM +0200, Jean Delvare wrote: >> > > Interrupt: pin B routed to IRQ 0 >> > >> > Hmm, this "IRQ 0" is quite odd. I'm wondering if this could be the >> > reason for this hang. Was it with the i2c-i801 driver loaded, or >> > blacklisted? Please check if it makes a difference. >> >> That was without the driver loaded (blacklisted). After loading (with >> interrupts enabled) we get: >> >> Interrupt: pin B routed to IRQ 20 > > For the record, I also see the IRQ value change after loading the > i2c-i801 driver on my system (with an ICH10 south bridge.) From 14 to > 22 in my case. So it's a bit different (no IRQ 0) but not still > somewhat similar, so I'm still not sure if this has anything to do with > your issue. > >> >> > Do you see the same (and more generally, this issue) on one, some or >> > all of your x3550 servers? >> >> The issue has occured on at least three x3550s (we have 11). I haven't >> tested more, because knowingly crashing production machines sucks. > > Yes of course, I understand, I did not expect you to do that ;) > >> This appears to be the case on other machines. With the module >> blacklisted (never loaded), lspci shows IRQ 0. After load, IRQ 20. >> (tested on 3.4 and 3.9). > > OK. > >> > Are you using IPMI on these machines? >> >> Yes, but only for monitoring/sensors, if that makes a difference. > > IPMI is still likely to access the SMBus controller. If there's a BMC > in the machine, it can also access the SMBus slave with its own > controller. It would be good to rule this out by disabling IPMI > completely, removing the BMC from the machine if it has one, and > checking if it makes the issue go away or not. > >> > I would appreciate if you could test the following: >> > * Blacklist i2c-i801 and ics932s401 so that none of them get >> > auto-loaded. >> >> Done. >> >> > * Manually load i2c-i801 with interrupts enabled, and see what >> > happens. >> >> Returned immediately: >> >> [ 60.527140] i801_smbus :00:1f.3: SMBus using PCI Interrupt > > This confirms that the i2c-i801 driver loading itself isn't the problem. > >> > * If no hang happens, load i2c-dev, find the i801 bus number with >> > i2cdetect -l (from the i2c-tools package - it should be 4 according >> > to what you reported so far but there is no guarantee that it won't >> > change across reboots.) >> >> $ i2cdetect -l >> i2c-0 i2c Radeon i2c bit bus DVI_DDC I2C adapter >> i2c-1 i2c Radeon i2c bit bus VGA_DDC I2C adapter >> i2c-2 i2c Radeon i2c bit bus MONIDI2C adapter >> i2c-3 i2c Radeon i2c bit bus CRT2_DDC I2C adapter >> i2c-4 smbus SMBus I801 adapter at 0440 SMBus adapter >> >> > Then do a simple read from a random address >> > with: >> > # i2cget 4 0x50 0x00 >> > (Adjust the bus number as needed.) >> > I am curious if this will hang as well or only when accessing the >> > clock chip at address 0x69. >> >> Yep, that one hangs. The hung task handler picked it up after a few >> minutes. > > OK, this means that any transaction request to the SMBus controller > causes the hang. > > The i2c-i801 driver is optimistically using wait_event() when waiting > for an interrupt to arrive. I suppose that the interrupt is never > delivered in your case (all 0 in /proc/interrupts.) > > Daniel, shouldn't we use wait_event_timeout() instead to catch issues > like this and fail cleanly? Maybe even fallback to polling > automatically? We could try to do something like that, I guess. The only question is how long to wait, b/c SMBus can pretty slow. But that kind of hack sounds more like something you'd do if irqs were getting sporadically lost on an otherwise correctly configured system. In this case, it sounds like there are never interrupts, but we are expecting some due to an incorrectly assuming that irqs are supported. What is different about his configuration where there would be no IRQs? Was Robert able to get the system working without hangs by disabling the IRQ feature of i2c-i801 module when it was builtin? > > -- > Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] i2c: busses: remove superfluous comment
The comment already got copy&pasted around and is not really useful.
Remove it.
Signed-off-by: Wolfram Sang
---
drivers/i2c/busses/i2c-davinci.c|1 -
drivers/i2c/busses/i2c-designware-platdrv.c |1 -
drivers/i2c/busses/i2c-omap.c |1 -
3 files changed, 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index cf20e06..2daa253 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -646,7 +646,6 @@ static int davinci_i2c_probe(struct platform_device *pdev)
struct resource *mem, *irq;
int r;
- /* NOTE: driver uses the static register mapping */
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!mem) {
dev_err(&pdev->dev, "no mem resource?\n");
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 35b70a1..da53d64 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -87,7 +87,6 @@ static int dw_i2c_probe(struct platform_device *pdev)
struct resource *mem;
int irq, r;
- /* NOTE: driver uses the static register mapping */
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!mem) {
dev_err(&pdev->dev, "no mem resource?\n");
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index e02f9e3..b241ae6 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1084,7 +1084,6 @@ omap_i2c_probe(struct platform_device *pdev)
u32 rev;
u16 minor, major, scheme;
- /* NOTE: driver uses the static register mapping */
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!mem) {
dev_err(&pdev->dev, "no mem resource?\n");
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 9/9] I2C: mv64xxx: fix race between FSM/interrupt and process context
> @@ -271,12 +273,25 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
> {
> switch(drv_data->action) {
> case MV64XXX_I2C_ACTION_SEND_RESTART:
> + /* We should only get here if we have further messages */
> + BUG_ON(drv_data->num_msgs == 0);
> +
...
> @@ -453,16 +463,20 @@ static int
> mv64xxx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> {
> struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap);
> - int i, rc;
> + int rc, ret = num;
>
> - for (i = 0; i < num; i++) {
> - rc = mv64xxx_i2c_execute_msg(drv_data, &msgs[i],
> - i == 0, i + 1 == num);
> - if (rc < 0)
> - return rc;
> - }
> + BUG_ON(drv_data->msgs != NULL);
Can't we handle this more gracefully than to halt the kernel? Like
WARN_ON and resetting the controller or disabling the bus or...
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: modprobe hang at startup (3.8.x, 3.9.x, IBM x3550)
On Fri, 17 May 2013 11:22:17 +0200, Martin Mokrejs wrote: > Hi, > while you are chasing some problem with i2c_801 I would like to mention > that I never got an answer on the thread https://lkml.org/lkml/2013/1/23/405 > about a kmemleak reported by kernel . Maybe this could give you a hint? > If these do not overlap I would be anyways glad to receive an answer via > the original thread I have started. > Thank you, > Martin I have no clue what the problem is nor how to investigate it, and in fact I strongly suspect that this is either a false positive or a problem in lower layers - driver core, sysfs etc. so nothing I can help with. So until someone comes with an evidence that there is an actual memory leak in the i2c-i801 driver itself I'm not going to pay any attention to your report, sorry. -- Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/2] i2c: fix two wrong mem release
On Thu, May 09, 2013 at 04:27:22PM +0800, Libo Chen wrote:
> fix two wrong mem release
>
> * Changelog from v1:
> * exchange out_error_no_irq and out_error_pin_mux suggested by Sonic
>
> * add some new tag , make code cleanly
Thanks for the submission. I do think the proper solution would be to
convert the drivers to use the devm_* mechanisms, though. You can check
commit 857968434bb6dbda0911f38ec46b0c3d0c963771 ("i2c: davinci: update
to devm_* API") as a reference.
Thanks,
Wolfram
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/9] I2C: mv64xxx: use devm_ioremap_resource()
On Fri, May 17, 2013 at 11:23:16AM +0200, Jean-Francois Moine wrote:
> On Thu, 16 May 2013 21:33:09 +0100
> Russell King wrote:
>
> > Eliminate reg_base_p and reg_size, mv64xxx_i2c_unmap_regs() and an
> > unchecked ioremap() return from this driver by using the devm_*
> > API for requesting and ioremapping resources.
> >
> > Signed-off-by: Russell King
> > ---
> > drivers/i2c/busses/i2c-mv64xxx.c | 46
> > +-
> > 1 files changed, 6 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-mv64xxx.c
> > b/drivers/i2c/busses/i2c-mv64xxx.c
> > index 0339cd8..19cc9bf 100644
> > --- a/drivers/i2c/busses/i2c-mv64xxx.c
> > +++ b/drivers/i2c/busses/i2c-mv64xxx.c
> [snip]
> > @@ -704,7 +671,6 @@ mv64xxx_i2c_remove(struct platform_device *dev)
> >
> > rc = i2c_del_adapter(&drv_data->adapter);
> > free_irq(drv_data->irq, drv_data);
> > - mv64xxx_i2c_unmap_regs(drv_data);
> > #if defined(CONFIG_HAVE_CLK)
> > /* Not all platforms have a clk */
> > if (!IS_ERR(drv_data->clk)) {
>
> The patch does not apply: it seems it lacks:
I should've mentioned that the patches are against v3.9, not v3.10-rc1.
> + int rc;
>
> - i2c_del_adapter(&drv_data->adapter);
> + rc = i2c_del_adapter(&drv_data->adapter);
>
> - return 0;
> + return rc;
>
> BTW, is this return code useful?
No it isn't. Removals *can't* fail. Once the remove function is called,
the device _will_ be unbound no matter what the return value of that
function is. The module will also be unloaded (if that's why it's being
unbound) whether or not you return an error.
So, removals must always succeed. Why their return type hasn't been void
from the start I've no idea - but that's a question for Patrick Mochel
who implemented the original driver model design.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/9] I2C: mv64xxx: use devm_ioremap_resource()
On Thu, 16 May 2013 21:33:09 +0100
Russell King wrote:
> Eliminate reg_base_p and reg_size, mv64xxx_i2c_unmap_regs() and an
> unchecked ioremap() return from this driver by using the devm_*
> API for requesting and ioremapping resources.
>
> Signed-off-by: Russell King
> ---
> drivers/i2c/busses/i2c-mv64xxx.c | 46
> +-
> 1 files changed, 6 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-mv64xxx.c
> b/drivers/i2c/busses/i2c-mv64xxx.c
> index 0339cd8..19cc9bf 100644
> --- a/drivers/i2c/busses/i2c-mv64xxx.c
> +++ b/drivers/i2c/busses/i2c-mv64xxx.c
[snip]
> @@ -704,7 +671,6 @@ mv64xxx_i2c_remove(struct platform_device *dev)
>
> rc = i2c_del_adapter(&drv_data->adapter);
> free_irq(drv_data->irq, drv_data);
> - mv64xxx_i2c_unmap_regs(drv_data);
> #if defined(CONFIG_HAVE_CLK)
> /* Not all platforms have a clk */
> if (!IS_ERR(drv_data->clk)) {
The patch does not apply: it seems it lacks:
+ int rc;
- i2c_del_adapter(&drv_data->adapter);
+ rc = i2c_del_adapter(&drv_data->adapter);
- return 0;
+ return rc;
BTW, is this return code useful?
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: modprobe hang at startup (3.8.x, 3.9.x, IBM x3550)
Hi, while you are chasing some problem with i2c_801 I would like to mention that I never got an answer on the thread https://lkml.org/lkml/2013/1/23/405 about a kmemleak reported by kernel . Maybe this could give you a hint? If these do not overlap I would be anyways glad to receive an answer via the original thread I have started. Thank you, Martin Jean Delvare wrote: > Hi Robert, > > On Thu, 16 May 2013 13:44:55 +1000, Robert Norris wrote: >> On Wed, May 15, 2013 at 09:49:23PM +0200, Jean Delvare wrote: Interrupt: pin B routed to IRQ 0 >>> >>> Hmm, this "IRQ 0" is quite odd. I'm wondering if this could be the >>> reason for this hang. Was it with the i2c-i801 driver loaded, or >>> blacklisted? Please check if it makes a difference. >> >> That was without the driver loaded (blacklisted). After loading (with >> interrupts enabled) we get: >> >> Interrupt: pin B routed to IRQ 20 > > For the record, I also see the IRQ value change after loading the > i2c-i801 driver on my system (with an ICH10 south bridge.) From 14 to > 22 in my case. So it's a bit different (no IRQ 0) but not still > somewhat similar, so I'm still not sure if this has anything to do with > your issue. > >> >>> Do you see the same (and more generally, this issue) on one, some or >>> all of your x3550 servers? >> >> The issue has occured on at least three x3550s (we have 11). I haven't >> tested more, because knowingly crashing production machines sucks. > > Yes of course, I understand, I did not expect you to do that ;) > >> This appears to be the case on other machines. With the module >> blacklisted (never loaded), lspci shows IRQ 0. After load, IRQ 20. >> (tested on 3.4 and 3.9). > > OK. > >>> Are you using IPMI on these machines? >> >> Yes, but only for monitoring/sensors, if that makes a difference. > > IPMI is still likely to access the SMBus controller. If there's a BMC > in the machine, it can also access the SMBus slave with its own > controller. It would be good to rule this out by disabling IPMI > completely, removing the BMC from the machine if it has one, and > checking if it makes the issue go away or not. > >>> I would appreciate if you could test the following: >>> * Blacklist i2c-i801 and ics932s401 so that none of them get >>> auto-loaded. >> >> Done. >> >>> * Manually load i2c-i801 with interrupts enabled, and see what >>> happens. >> >> Returned immediately: >> >> [ 60.527140] i801_smbus :00:1f.3: SMBus using PCI Interrupt > > This confirms that the i2c-i801 driver loading itself isn't the problem. > >>> * If no hang happens, load i2c-dev, find the i801 bus number with >>> i2cdetect -l (from the i2c-tools package - it should be 4 according >>> to what you reported so far but there is no guarantee that it won't >>> change across reboots.) >> >> $ i2cdetect -l >> i2c-0 i2c Radeon i2c bit bus DVI_DDC I2C adapter >> i2c-1 i2c Radeon i2c bit bus VGA_DDC I2C adapter >> i2c-2 i2c Radeon i2c bit bus MONIDI2C adapter >> i2c-3 i2c Radeon i2c bit bus CRT2_DDC I2C adapter >> i2c-4 smbus SMBus I801 adapter at 0440 SMBus adapter >> >>> Then do a simple read from a random address >>> with: >>> # i2cget 4 0x50 0x00 >>> (Adjust the bus number as needed.) >>> I am curious if this will hang as well or only when accessing the >>> clock chip at address 0x69. >> >> Yep, that one hangs. The hung task handler picked it up after a few >> minutes. > > OK, this means that any transaction request to the SMBus controller > causes the hang. > > The i2c-i801 driver is optimistically using wait_event() when waiting > for an interrupt to arrive. I suppose that the interrupt is never > delivered in your case (all 0 in /proc/interrupts.) > > Daniel, shouldn't we use wait_event_timeout() instead to catch issues > like this and fail cleanly? Maybe even fallback to polling > automatically? > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] i2c-designware: make SDA hold time configurable
> > New bindings are complicated. I need to check if there is an > > existing/similar one, check if it is generic enough for drivers still to > > come, etc. I didn't have the time to do this for 3.10, so it is > > 3.11 material. > > OK, thanks. I just wanted to be sure this patch wasn't forgotten as it is > useful for Haswell/Lynxpoint as well. i2c list is monitored by patchwork which is rigorously not forgetting ;) -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: modprobe hang at startup (3.8.x, 3.9.x, IBM x3550)
On Thu, 16 May 2013 13:44:55 +1000, Robert Norris wrote: > > Then do a simple read from a random address > > with: > > # i2cget 4 0x50 0x00 > > (Adjust the bus number as needed.) > > I am curious if this will hang as well or only when accessing the > > clock chip at address 0x69. > > Yep, that one hangs. The hung task handler picked it up after a few > minutes. Hmm, can you please dump the PCI configuration space of the SMBus controller? # /sbin/lspci -s 00:1f.3 -xxx It might be that you have interrupts routed to SMI# instead of the regular IRQ line. The driver is supposed to disable interrupt support in that case, but I have never tested it. Thanks, -- Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-i801: Document feature bits in modinfo
On Wed, May 15, 2013 at 02:44:10PM +0200, Jean Delvare wrote: > Duplicate the feature bits documentation in modinfo, as not every user > will read the driver's source code or documentation file. > > Signed-off-by: Jean Delvare Applied to for-current, thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] i2c-designware: make SDA hold time configurable
On Fri, May 17, 2013 at 10:29:28AM +0200, Wolfram Sang wrote: > On Tue, May 14, 2013 at 02:07:45PM +0300, Mika Westerberg wrote: > > On Tue, Apr 09, 2013 at 12:59:54PM +0200, Christian Ruppert wrote: > > > This patch makes the SDA hold time configurable through device tree. > > > > > > Signed-off-by: Christian Ruppert > > > Signed-off-by: Pierrick Hascoet > > > > Hi Wolfram, > > > > What happened to this patch? I don't see it merged for 3.10. > > > > The reason I'm asking is that I would like to add ACPI support for the SDA > > hold time parameter analogous to the DT version. > > New bindings are complicated. I need to check if there is an > existing/similar one, check if it is generic enough for drivers still to > come, etc. I didn't have the time to do this for 3.10, so it is > 3.11 material. OK, thanks. I just wanted to be sure this patch wasn't forgotten as it is useful for Haswell/Lynxpoint as well. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: modprobe hang at startup (3.8.x, 3.9.x, IBM x3550)
Hi Robert, On Thu, 16 May 2013 13:44:55 +1000, Robert Norris wrote: > On Wed, May 15, 2013 at 09:49:23PM +0200, Jean Delvare wrote: > > > Interrupt: pin B routed to IRQ 0 > > > > Hmm, this "IRQ 0" is quite odd. I'm wondering if this could be the > > reason for this hang. Was it with the i2c-i801 driver loaded, or > > blacklisted? Please check if it makes a difference. > > That was without the driver loaded (blacklisted). After loading (with > interrupts enabled) we get: > > Interrupt: pin B routed to IRQ 20 For the record, I also see the IRQ value change after loading the i2c-i801 driver on my system (with an ICH10 south bridge.) From 14 to 22 in my case. So it's a bit different (no IRQ 0) but not still somewhat similar, so I'm still not sure if this has anything to do with your issue. > > > Do you see the same (and more generally, this issue) on one, some or > > all of your x3550 servers? > > The issue has occured on at least three x3550s (we have 11). I haven't > tested more, because knowingly crashing production machines sucks. Yes of course, I understand, I did not expect you to do that ;) > This appears to be the case on other machines. With the module > blacklisted (never loaded), lspci shows IRQ 0. After load, IRQ 20. > (tested on 3.4 and 3.9). OK. > > Are you using IPMI on these machines? > > Yes, but only for monitoring/sensors, if that makes a difference. IPMI is still likely to access the SMBus controller. If there's a BMC in the machine, it can also access the SMBus slave with its own controller. It would be good to rule this out by disabling IPMI completely, removing the BMC from the machine if it has one, and checking if it makes the issue go away or not. > > I would appreciate if you could test the following: > > * Blacklist i2c-i801 and ics932s401 so that none of them get > > auto-loaded. > > Done. > > > * Manually load i2c-i801 with interrupts enabled, and see what > > happens. > > Returned immediately: > > [ 60.527140] i801_smbus :00:1f.3: SMBus using PCI Interrupt This confirms that the i2c-i801 driver loading itself isn't the problem. > > * If no hang happens, load i2c-dev, find the i801 bus number with > > i2cdetect -l (from the i2c-tools package - it should be 4 according > > to what you reported so far but there is no guarantee that it won't > > change across reboots.) > > $ i2cdetect -l > i2c-0 i2c Radeon i2c bit bus DVI_DDC I2C adapter > i2c-1 i2c Radeon i2c bit bus VGA_DDC I2C adapter > i2c-2 i2c Radeon i2c bit bus MONIDI2C adapter > i2c-3 i2c Radeon i2c bit bus CRT2_DDC I2C adapter > i2c-4 smbus SMBus I801 adapter at 0440 SMBus adapter > > > Then do a simple read from a random address > > with: > > # i2cget 4 0x50 0x00 > > (Adjust the bus number as needed.) > > I am curious if this will hang as well or only when accessing the > > clock chip at address 0x69. > > Yep, that one hangs. The hung task handler picked it up after a few > minutes. OK, this means that any transaction request to the SMBus controller causes the hang. The i2c-i801 driver is optimistically using wait_event() when waiting for an interrupt to arrive. I suppose that the interrupt is never delivered in your case (all 0 in /proc/interrupts.) Daniel, shouldn't we use wait_event_timeout() instead to catch issues like this and fail cleanly? Maybe even fallback to polling automatically? -- Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] I2C: mv64xxx: work around signals causing I2C transactions to be aborted
On Fri, May 17, 2013 at 08:37:43AM +0200, Jean-Francois Moine wrote: > On Thu, 16 May 2013 21:30:59 +0100 > Russell King wrote: > > > Do not use interruptible waits in an I2C driver; if a process uses > > signals (eg, Xorg uses SIGALRM and SIGPIPE) then these signals can > > cause the I2C driver to abort a transaction in progress by another > > driver, which can cause that driver to fail. I2C drivers are not > > expected to abort transactions on signals. > > Hi Russell, > > I had the same problem with my dove drm driver, but I don't fully agree > with your solution. > > Using wait_event_timeout() stops the system, and reading the EDID from > an external screen may take some time. It doesn't stop the system. It stops the process until that has completed. Using the DRM TDA19988 driver, things are nice and quick while reading the EDID, because it does reads an entire EDID block using one message. Blocking the signals like you do has exactly the same effect - you prevent the I2C driver being interrupted by signals. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-designware: fix RX FIFO overrun
> Applied to for-current, thanks! Added stable, too. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] i2c-designware: add Intel BayTrail ACPI ID
On Mon, May 13, 2013 at 01:54:31PM +0300, Mika Westerberg wrote: > This is the same controller as on Intel Lynxpoint but the ACPI ID is > different (8086F41). Add support for this. > > Signed-off-by: Mika Westerberg Applied to for-current, thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c-designware: always clear interrupts before enabling them
On Mon, May 13, 2013 at 01:54:30PM +0300, Mika Westerberg wrote: > If the I2C bus is put to a low power state by an ACPI method it might pull > the SDA line low (as its power is removed). Once the bus is put to full > power state again, the SDA line is pulled back to high. This transition > looks like a STOP condition from the controller point-of-view which sets > STOP detected bit in its status register causing the driver to fail > subsequent transfers. > > Fix this by always clearing all interrupts before we start a transfer. > > Signed-off-by: Mika Westerberg Applied to for-current, thanks! Added stable, too. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] i2c-designware: make SDA hold time configurable
On Tue, May 14, 2013 at 02:07:45PM +0300, Mika Westerberg wrote: > On Tue, Apr 09, 2013 at 12:59:54PM +0200, Christian Ruppert wrote: > > This patch makes the SDA hold time configurable through device tree. > > > > Signed-off-by: Christian Ruppert > > Signed-off-by: Pierrick Hascoet > > Hi Wolfram, > > What happened to this patch? I don't see it merged for 3.10. > > The reason I'm asking is that I would like to add ACPI support for the SDA > hold time parameter analogous to the DT version. New bindings are complicated. I need to check if there is an existing/similar one, check if it is generic enough for drivers still to come, etc. I didn't have the time to do this for 3.10, so it is 3.11 material. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-designware: fix RX FIFO overrun
On Wed, May 08, 2013 at 02:34:36PM +0100, Josef Ahmad wrote: > From 8a4773d0c0df6fe2e816ad37fde30a2d90a1ad31 Mon Sep 17 00:00:00 2001 > From: Josef Ahmad > Date: Fri, 19 Apr 2013 17:28:10 +0100 > Subject: [PATCH] i2c-designware: fix RX FIFO overrun > > i2c_dw_xfer_msg() pushes a number of bytes to transmit/receive > to/from the bus into the TX FIFO. > For master-rx transactions, the maximum amount of data that can be > received is calculated depending solely on TX and RX FIFO load. > > This is racy - TX FIFO may contain master-rx data yet to be > processed, which will eventually land into the RX FIFO. This > data is not taken into account and the function may request more > data than the controller is actually capable of storing. > > This patch ensures the driver takes into account the outstanding > master-rx data in TX FIFO to prevent RX FIFO overrun. > > Signed-off-by: Josef Ahmad > Acked-by: Mika Westerberg Applied to for-current, thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
