linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 5.0 009/262] i2c: sis630: correct format strings
       [not found] <20190327180158.10245-1-sashal@kernel.org>
@ 2019-03-27 17:57 ` Sasha Levin
  2019-03-28 10:07   ` Pavel Machek
  2019-03-27 17:59 ` [PATCH AUTOSEL 5.0 098/262] i2c: Allow recovery of the initial IRQ by an I2C client device Sasha Levin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 5+ messages in thread
From: Sasha Levin @ 2019-03-27 17:57 UTC (permalink / raw)
  To: linux-kernel, stable; +Cc: Louis Taylor, Wolfram Sang, Sasha Levin, linux-i2c

From: Louis Taylor <louis@kragniz.eu>

[ Upstream commit 60f7691c624b41a05bfc3493d9b0519e7951b7ef ]

When compiling with -Wformat, clang warns:

drivers/i2c/busses/i2c-sis630.c:482:4: warning: format specifies type
      'unsigned short' but the argument has type 'int' [-Wformat]
                        smbus_base + SMB_STS,
                        ^~~~~~~~~~~~~~~~~~~~

drivers/i2c/busses/i2c-sis630.c:483:4: warning: format specifies type
      'unsigned short' but the argument has type 'int' [-Wformat]
                        smbus_base + SMB_STS + SIS630_SMB_IOREGION - 1);
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

drivers/i2c/busses/i2c-sis630.c:531:37: warning: format specifies type
      'unsigned short' but the argument has type 'int' [-Wformat]
                 "SMBus SIS630 adapter at %04hx", smbus_base + SMB_STS);
                                          ~~~~~   ^~~~~~~~~~~~~~~~~~~~

This patch fixes the format strings to use the format type for int.

Link: https://github.com/ClangBuiltLinux/linux/issues/378
Signed-off-by: Louis Taylor <louis@kragniz.eu>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/i2c/busses/i2c-sis630.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sis630.c b/drivers/i2c/busses/i2c-sis630.c
index 1e6805b5cef2..a57aa4fe51a4 100644
--- a/drivers/i2c/busses/i2c-sis630.c
+++ b/drivers/i2c/busses/i2c-sis630.c
@@ -478,7 +478,7 @@ static int sis630_setup(struct pci_dev *sis630_dev)
 	if (!request_region(smbus_base + SMB_STS, SIS630_SMB_IOREGION,
 			    sis630_driver.name)) {
 		dev_err(&sis630_dev->dev,
-			"I/O Region 0x%04hx-0x%04hx for SMBus already in use.\n",
+			"I/O Region 0x%04x-0x%04x for SMBus already in use.\n",
 			smbus_base + SMB_STS,
 			smbus_base + SMB_STS + SIS630_SMB_IOREGION - 1);
 		retval = -EBUSY;
@@ -528,7 +528,7 @@ static int sis630_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	sis630_adapter.dev.parent = &dev->dev;
 
 	snprintf(sis630_adapter.name, sizeof(sis630_adapter.name),
-		 "SMBus SIS630 adapter at %04hx", smbus_base + SMB_STS);
+		 "SMBus SIS630 adapter at %04x", smbus_base + SMB_STS);
 
 	return i2c_add_adapter(&sis630_adapter);
 }
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH AUTOSEL 5.0 098/262] i2c: Allow recovery of the initial IRQ by an I2C client device.
       [not found] <20190327180158.10245-1-sashal@kernel.org>
  2019-03-27 17:57 ` [PATCH AUTOSEL 5.0 009/262] i2c: sis630: correct format strings Sasha Levin
@ 2019-03-27 17:59 ` Sasha Levin
  2019-03-27 17:59 ` [PATCH AUTOSEL 5.0 102/262] i2c: designware: Do not allow i2c_dw_xfer() calls while suspended Sasha Levin
  2019-03-27 18:00 ` [PATCH AUTOSEL 5.0 180/262] i2c: of: Try to find an I2C adapter matching the parent Sasha Levin
  3 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2019-03-27 17:59 UTC (permalink / raw)
  To: linux-kernel, stable; +Cc: Jim Broadus, Wolfram Sang, Sasha Levin, linux-i2c

From: Jim Broadus <jbroadus@gmail.com>

[ Upstream commit 93b6604c5a669d84e45fe5129294875bf82eb1ff ]

A previous change allowed I2C client devices to discover new IRQs upon
reprobe by clearing the IRQ in i2c_device_remove. However, if an IRQ was
assigned in i2c_new_device, that information is lost.

For example, the touchscreen and trackpad devices on a Dell Inspiron laptop
are I2C devices whose IRQs are defined by ACPI extended IRQ types. The
client device structures are initialized during an ACPI walk. After
removing the i2c_hid device, modprobe fails.

This change caches the initial IRQ value in i2c_new_device and then resets
the client device IRQ to the initial value in i2c_device_remove.

Fixes: 6f108dd70d30 ("i2c: Clear client->irq in i2c_device_remove")
Signed-off-by: Jim Broadus <jbroadus@gmail.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
[wsa: this is an easy to backport fix for the regression. We will
refactor the code to handle irq assignments better in general.]
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/i2c/i2c-core-base.c | 9 +++++----
 include/linux/i2c.h         | 1 +
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 28460f6a60cc..af87a16ac3a5 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -430,7 +430,7 @@ static int i2c_device_remove(struct device *dev)
 	dev_pm_clear_wake_irq(&client->dev);
 	device_init_wakeup(&client->dev, false);
 
-	client->irq = 0;
+	client->irq = client->init_irq;
 
 	return status;
 }
@@ -741,10 +741,11 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 	client->flags = info->flags;
 	client->addr = info->addr;
 
-	client->irq = info->irq;
-	if (!client->irq)
-		client->irq = i2c_dev_irq_from_resources(info->resources,
+	client->init_irq = info->irq;
+	if (!client->init_irq)
+		client->init_irq = i2c_dev_irq_from_resources(info->resources,
 							 info->num_resources);
+	client->irq = client->init_irq;
 
 	strlcpy(client->name, info->type, sizeof(client->name));
 
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 65b4eaed1d96..7e748648c7d3 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -333,6 +333,7 @@ struct i2c_client {
 	char name[I2C_NAME_SIZE];
 	struct i2c_adapter *adapter;	/* the adapter we sit on	*/
 	struct device dev;		/* the device structure		*/
+	int init_irq;			/* irq set at initialization	*/
 	int irq;			/* irq issued by device		*/
 	struct list_head detected;
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH AUTOSEL 5.0 102/262] i2c: designware: Do not allow i2c_dw_xfer() calls while suspended
       [not found] <20190327180158.10245-1-sashal@kernel.org>
  2019-03-27 17:57 ` [PATCH AUTOSEL 5.0 009/262] i2c: sis630: correct format strings Sasha Levin
  2019-03-27 17:59 ` [PATCH AUTOSEL 5.0 098/262] i2c: Allow recovery of the initial IRQ by an I2C client device Sasha Levin
@ 2019-03-27 17:59 ` Sasha Levin
  2019-03-27 18:00 ` [PATCH AUTOSEL 5.0 180/262] i2c: of: Try to find an I2C adapter matching the parent Sasha Levin
  3 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2019-03-27 17:59 UTC (permalink / raw)
  To: linux-kernel, stable; +Cc: Hans de Goede, Wolfram Sang, Sasha Levin, linux-i2c

From: Hans de Goede <hdegoede@redhat.com>

[ Upstream commit 2751541555382dfa7661bcfaac3ee0fac49f505d ]

On most Intel Bay- and Cherry-Trail systems the PMIC is connected over I2C
and the PMIC is accessed through various means by the _PS0 and _PS3 ACPI
methods (power on / off methods) of various devices.

This leads to suspend/resume ordering problems where a device may be
resumed and get its _PS0 method executed before the I2C controller is
resumed. On Cherry Trail this leads to errors like these:

     i2c_designware 808622C1:06: controller timed out
     ACPI Error: AE_ERROR, Returned by Handler for [UserDefinedRegion]
     ACPI Error: Method parse/execution failed \_SB.P18W._ON, AE_ERROR
     video LNXVIDEO:00: Failed to change power state to D0

But on Bay Trail this caused I2C reads to seem to succeed, but they end
up returning wrong data, which ends up getting written back by the typical
read-modify-write cycle done to turn on various power-resources.

Debugging the problems caused by this silent data corruption is quite
nasty. This commit adds a check which disallows i2c_dw_xfer() calls to
happen until the controller's resume method has completed.

Which turns the silent data corruption into getting these errors in
dmesg instead:

    i2c_designware 80860F41:04: Error i2c_dw_xfer call while suspended
    ACPI Error: AE_ERROR, Returned by Handler for [UserDefinedRegion]
    ACPI Error: Method parse/execution failed \_SB.PCI0.GFX0._PS0, AE_ERROR

Which is much better.

Note the above errors are an example of issues which this patch will
help to debug, the actual fix requires fixing the suspend order and
this has been fixed by a different commit.

Note the setting / clearing of the suspended flag in the suspend / resume
methods is NOT protected by i2c_lock_bus(). This is intentional as these
methods get called from i2c_dw_xfer() (through pm_runtime_get/put) a nd
i2c_dw_xfer() is called with the i2c_bus_lock held, so otherwise we would
deadlock. This means that there is a theoretical race between a non runtime
suspend and the suspended check in i2c_dw_xfer(), this is not a problem
since normally we should not hit the race and this check is primarily a
debugging tool so hitting the check if there are suspend/resume ordering
problems does not need to be 100% reliable.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/i2c/busses/i2c-designware-core.h    | 2 ++
 drivers/i2c/busses/i2c-designware-master.c  | 6 ++++++
 drivers/i2c/busses/i2c-designware-pcidrv.c  | 7 ++++++-
 drivers/i2c/busses/i2c-designware-platdrv.c | 3 +++
 4 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index b4a0b2b99a78..6b4ef1d38fb2 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -215,6 +215,7 @@
  * @disable_int: function to disable all interrupts
  * @init: function to initialize the I2C hardware
  * @mode: operation mode - DW_IC_MASTER or DW_IC_SLAVE
+ * @suspended: set to true if the controller is suspended
  *
  * HCNT and LCNT parameters can be used if the platform knows more accurate
  * values than the one computed based only on the input clock frequency.
@@ -270,6 +271,7 @@ struct dw_i2c_dev {
 	int			(*set_sda_hold_time)(struct dw_i2c_dev *dev);
 	int			mode;
 	struct i2c_bus_recovery_info rinfo;
+	bool			suspended;
 };
 
 #define ACCESS_SWAP		0x00000001
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 8d1bc44d2530..bb8e3f149979 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -426,6 +426,12 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 
 	pm_runtime_get_sync(dev->dev);
 
+	if (dev->suspended) {
+		dev_err(dev->dev, "Error %s call while suspended\n", __func__);
+		ret = -ESHUTDOWN;
+		goto done_nolock;
+	}
+
 	reinit_completion(&dev->cmd_complete);
 	dev->msgs = msgs;
 	dev->msgs_num = num;
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index d50f80487214..76810deb2de6 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -176,6 +176,7 @@ static int i2c_dw_pci_suspend(struct device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct dw_i2c_dev *i_dev = pci_get_drvdata(pdev);
 
+	i_dev->suspended = true;
 	i_dev->disable(i_dev);
 
 	return 0;
@@ -185,8 +186,12 @@ static int i2c_dw_pci_resume(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct dw_i2c_dev *i_dev = pci_get_drvdata(pdev);
+	int ret;
 
-	return i_dev->init(i_dev);
+	ret = i_dev->init(i_dev);
+	i_dev->suspended = false;
+
+	return ret;
 }
 #endif
 
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 9eaac3be1f63..ead5e7de3e4d 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -454,6 +454,8 @@ static int dw_i2c_plat_suspend(struct device *dev)
 {
 	struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
 
+	i_dev->suspended = true;
+
 	if (i_dev->shared_with_punit)
 		return 0;
 
@@ -471,6 +473,7 @@ static int dw_i2c_plat_resume(struct device *dev)
 		i2c_dw_prepare_clk(i_dev, true);
 
 	i_dev->init(i_dev);
+	i_dev->suspended = false;
 
 	return 0;
 }
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH AUTOSEL 5.0 180/262] i2c: of: Try to find an I2C adapter matching the parent
       [not found] <20190327180158.10245-1-sashal@kernel.org>
                   ` (2 preceding siblings ...)
  2019-03-27 17:59 ` [PATCH AUTOSEL 5.0 102/262] i2c: designware: Do not allow i2c_dw_xfer() calls while suspended Sasha Levin
@ 2019-03-27 18:00 ` Sasha Levin
  3 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2019-03-27 18:00 UTC (permalink / raw)
  To: linux-kernel, stable; +Cc: Thierry Reding, Wolfram Sang, Sasha Levin, linux-i2c

From: Thierry Reding <treding@nvidia.com>

[ Upstream commit e814e688413aabd7b0d75e2a8ed1caa472951dec ]

If an I2C adapter doesn't match the provided device tree node, also try
matching the parent's device tree node. This allows finding an adapter
based on the device node of the parent device that was used to register
it.

This fixes a regression on Tegra124-based Chromebooks (Nyan) where the
eDP controller registers an I2C adapter that is used to read to EDID.
After commit 993a815dcbb2 ("dt-bindings: panel: Add missing .txt
suffix") this stopped working because the I2C adapter could no longer
be found. The approach in this patch fixes the regression without
introducing the issues that the above commit solved.

Fixes: 17ab7806de0c ("drm: don't link DP aux i2c adapter to the hardware device node")
Signed-off-by: Thierry Reding <treding@nvidia.com>
Tested-by: Tristan Bastian <tristan-c.bastian@gmx.de>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/i2c/i2c-core-of.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index 6cb7ad608bcd..0f01cdba9d2c 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -121,6 +121,17 @@ static int of_dev_node_match(struct device *dev, void *data)
 	return dev->of_node == data;
 }
 
+static int of_dev_or_parent_node_match(struct device *dev, void *data)
+{
+	if (dev->of_node == data)
+		return 1;
+
+	if (dev->parent)
+		return dev->parent->of_node == data;
+
+	return 0;
+}
+
 /* must call put_device() when done with returned i2c_client device */
 struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
 {
@@ -145,7 +156,8 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
 	struct device *dev;
 	struct i2c_adapter *adapter;
 
-	dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match);
+	dev = bus_find_device(&i2c_bus_type, NULL, node,
+			      of_dev_or_parent_node_match);
 	if (!dev)
 		return NULL;
 
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH AUTOSEL 5.0 009/262] i2c: sis630: correct format strings
  2019-03-27 17:57 ` [PATCH AUTOSEL 5.0 009/262] i2c: sis630: correct format strings Sasha Levin
@ 2019-03-28 10:07   ` Pavel Machek
  0 siblings, 0 replies; 5+ messages in thread
From: Pavel Machek @ 2019-03-28 10:07 UTC (permalink / raw)
  To: Sasha Levin; +Cc: linux-kernel, stable, Louis Taylor, Wolfram Sang, linux-i2c

> From: Louis Taylor <louis@kragniz.eu>
> 
> [ Upstream commit 60f7691c624b41a05bfc3493d9b0519e7951b7ef ]
> 
> When compiling with -Wformat, clang warns:
> 
> drivers/i2c/busses/i2c-sis630.c:482:4: warning: format specifies type
>       'unsigned short' but the argument has type 'int' [-Wformat]
>                         smbus_base + SMB_STS,
>                         ^~~~~~~~~~~~~~~~~~~~
> 
> drivers/i2c/busses/i2c-sis630.c:483:4: warning: format specifies type
>       'unsigned short' but the argument has type 'int' [-Wformat]
>                         smbus_base + SMB_STS + SIS630_SMB_IOREGION - 1);
>                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> drivers/i2c/busses/i2c-sis630.c:531:37: warning: format specifies type
>       'unsigned short' but the argument has type 'int' [-Wformat]
>                  "SMBus SIS630 adapter at %04hx", smbus_base + SMB_STS);
>                                           ~~~~~   ^~~~~~~~~~~~~~~~~~~~

Warning with unsupported compiler; not a "serious bug".
	
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-03-28 10:07 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20190327180158.10245-1-sashal@kernel.org>
2019-03-27 17:57 ` [PATCH AUTOSEL 5.0 009/262] i2c: sis630: correct format strings Sasha Levin
2019-03-28 10:07   ` Pavel Machek
2019-03-27 17:59 ` [PATCH AUTOSEL 5.0 098/262] i2c: Allow recovery of the initial IRQ by an I2C client device Sasha Levin
2019-03-27 17:59 ` [PATCH AUTOSEL 5.0 102/262] i2c: designware: Do not allow i2c_dw_xfer() calls while suspended Sasha Levin
2019-03-27 18:00 ` [PATCH AUTOSEL 5.0 180/262] i2c: of: Try to find an I2C adapter matching the parent Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).