[PATCH v1 04/12] i2c: isch: Switch to memory mapped IO accessors

Andy Shevchenko posted 12 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v1 04/12] i2c: isch: Switch to memory mapped IO accessors
Posted by Andy Shevchenko 2 months, 2 weeks ago
Convert driver to use memory mapped IO accessors.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-isch.c | 133 ++++++++++++++++++++--------------
 1 file changed, 78 insertions(+), 55 deletions(-)

diff --git a/drivers/i2c/busses/i2c-isch.c b/drivers/i2c/busses/i2c-isch.c
index f44c5fa276dc..8d34d4398f9b 100644
--- a/drivers/i2c/busses/i2c-isch.c
+++ b/drivers/i2c/busses/i2c-isch.c
@@ -24,16 +24,17 @@
 #include <linux/io.h>
 #include <linux/stddef.h>
 #include <linux/string_choices.h>
+#include <linux/types.h>
 
 /* SCH SMBus address offsets */
-#define SMBHSTCNT	(0 + sch_smba)
-#define SMBHSTSTS	(1 + sch_smba)
-#define SMBHSTCLK	(2 + sch_smba)
-#define SMBHSTADD	(4 + sch_smba) /* TSA */
-#define SMBHSTCMD	(5 + sch_smba)
-#define SMBHSTDAT0	(6 + sch_smba)
-#define SMBHSTDAT1	(7 + sch_smba)
-#define SMBBLKDAT	(0x20 + sch_smba)
+#define SMBHSTCNT	0x00
+#define SMBHSTSTS	0x01
+#define SMBHSTCLK	0x02
+#define SMBHSTADD	0x04	/* TSA */
+#define SMBHSTCMD	0x05
+#define SMBHSTDAT0	0x06
+#define SMBHSTDAT1	0x07
+#define SMBBLKDAT	0x20
 
 /* Other settings */
 #define MAX_RETRIES	5000
@@ -45,12 +46,33 @@
 #define SCH_WORD_DATA		0x03
 #define SCH_BLOCK_DATA		0x05
 
-static unsigned short sch_smba;
 static struct i2c_adapter sch_adapter;
+static void __iomem *sch_smba;
+
 static int backbone_speed = 33000; /* backbone speed in kHz */
 module_param(backbone_speed, int, S_IRUSR | S_IWUSR);
 MODULE_PARM_DESC(backbone_speed, "Backbone speed in kHz, (default = 33000)");
 
+static inline u8 sch_io_rd8(void __iomem *smba, unsigned int offset)
+{
+	return ioread8(smba + offset);
+}
+
+static inline void sch_io_wr8(void __iomem *smba, unsigned int offset, u8 value)
+{
+	iowrite8(value, smba + offset);
+}
+
+static inline u16 sch_io_rd16(void __iomem *smba, unsigned int offset)
+{
+	return ioread16(smba + offset);
+}
+
+static inline void sch_io_wr16(void __iomem *smba, unsigned int offset, u16 value)
+{
+	iowrite16(value, smba + offset);
+}
+
 /*
  * Start the i2c transaction -- the i2c_access will prepare the transaction
  * and this function will execute it.
@@ -64,20 +86,20 @@ static int sch_transaction(struct i2c_adapter *adap)
 
 	dev_dbg(&adap->dev,
 		"Transaction (pre): CNT=%02x, CMD=%02x, ADD=%02x, DAT0=%02x, DAT1=%02x\n",
-		inb(SMBHSTCNT),
-		inb(SMBHSTCMD), inb(SMBHSTADD), inb(SMBHSTDAT0),
-		inb(SMBHSTDAT1));
+		sch_io_rd8(sch_smba, SMBHSTCNT), sch_io_rd8(sch_smba, SMBHSTCMD),
+		sch_io_rd8(sch_smba, SMBHSTADD),
+		sch_io_rd8(sch_smba, SMBHSTDAT0), sch_io_rd8(sch_smba, SMBHSTDAT1));
 
 	/* Make sure the SMBus host is ready to start transmitting */
-	temp = inb(SMBHSTSTS) & 0x0f;
+	temp = sch_io_rd8(sch_smba, SMBHSTSTS) & 0x0f;
 	if (temp) {
 		/* Can not be busy since we checked it in sch_access */
 		if (temp & 0x01)
 			dev_dbg(&adap->dev, "Completion (%02x). Clear...\n", temp);
 		if (temp & 0x06)
 			dev_dbg(&adap->dev, "SMBus error (%02x). Resetting...\n", temp);
-		outb(temp, SMBHSTSTS);
-		temp = inb(SMBHSTSTS) & 0x0f;
+		sch_io_wr8(sch_smba, SMBHSTSTS, temp);
+		temp = sch_io_rd8(sch_smba, SMBHSTSTS) & 0x0f;
 		if (temp) {
 			dev_err(&adap->dev, "SMBus is not ready: (%02x)\n", temp);
 			return -EAGAIN;
@@ -85,11 +107,13 @@ static int sch_transaction(struct i2c_adapter *adap)
 	}
 
 	/* start the transaction by setting bit 4 */
-	outb(inb(SMBHSTCNT) | 0x10, SMBHSTCNT);
+	temp = sch_io_rd8(sch_smba, SMBHSTCNT);
+	temp |= 0x10;
+	sch_io_wr8(sch_smba, SMBHSTCNT, temp);
 
 	do {
 		usleep_range(100, 200);
-		temp = inb(SMBHSTSTS) & 0x0f;
+		temp = sch_io_rd8(sch_smba, SMBHSTSTS) & 0x0f;
 	} while ((temp & 0x08) && (retries++ < MAX_RETRIES));
 
 	/* If the SMBus is still busy, we give up */
@@ -105,8 +129,8 @@ static int sch_transaction(struct i2c_adapter *adap)
 		dev_err(&adap->dev, "Error: no response!\n");
 	} else if (temp & 0x01) {
 		dev_dbg(&adap->dev, "Post complete!\n");
-		outb(temp, SMBHSTSTS);
-		temp = inb(SMBHSTSTS) & 0x07;
+		sch_io_wr8(sch_smba, SMBHSTSTS, temp);
+		temp = sch_io_rd8(sch_smba, SMBHSTSTS) & 0x07;
 		if (temp & 0x06) {
 			/* Completion clear failed */
 			dev_dbg(&adap->dev,
@@ -117,9 +141,9 @@ static int sch_transaction(struct i2c_adapter *adap)
 		dev_dbg(&adap->dev, "No such address.\n");
 	}
 	dev_dbg(&adap->dev, "Transaction (post): CNT=%02x, CMD=%02x, ADD=%02x, DAT0=%02x, DAT1=%02x\n",
-		inb(SMBHSTCNT),
-		inb(SMBHSTCMD), inb(SMBHSTADD), inb(SMBHSTDAT0),
-		inb(SMBHSTDAT1));
+		sch_io_rd8(sch_smba, SMBHSTCNT), sch_io_rd8(sch_smba, SMBHSTCMD),
+		sch_io_rd8(sch_smba, SMBHSTADD),
+		sch_io_rd8(sch_smba, SMBHSTDAT0), sch_io_rd8(sch_smba, SMBHSTDAT1));
 	return result;
 }
 
@@ -137,12 +161,12 @@ static s32 sch_access(struct i2c_adapter *adap, u16 addr,
 	int i, len, temp, rc;
 
 	/* Make sure the SMBus host is not busy */
-	temp = inb(SMBHSTSTS) & 0x0f;
+	temp = sch_io_rd8(sch_smba, SMBHSTSTS) & 0x0f;
 	if (temp & 0x08) {
 		dev_dbg(&adap->dev, "SMBus busy (%02x)\n", temp);
 		return -EAGAIN;
 	}
-	temp = inw(SMBHSTCLK);
+	temp = sch_io_rd16(sch_smba, SMBHSTCLK);
 	if (!temp) {
 		/*
 		 * We can't determine if we have 33 or 25 MHz clock for
@@ -151,47 +175,47 @@ static s32 sch_access(struct i2c_adapter *adap, u16 addr,
 		 * run ~75 kHz instead which should do no harm.
 		 */
 		dev_notice(&adap->dev, "Clock divider uninitialized. Setting defaults\n");
-		outw(backbone_speed / (4 * 100), SMBHSTCLK);
+		sch_io_wr16(sch_smba, SMBHSTCLK, backbone_speed / (4 * 100));
 	}
 
 	dev_dbg(&adap->dev, "access size: %d %s\n", size, str_read_write(read_write));
 	switch (size) {
 	case I2C_SMBUS_QUICK:
-		outb((addr << 1) | read_write, SMBHSTADD);
+		sch_io_wr8(sch_smba, SMBHSTADD, (addr << 1) | read_write);
 		size = SCH_QUICK;
 		break;
 	case I2C_SMBUS_BYTE:
-		outb((addr << 1) | read_write, SMBHSTADD);
+		sch_io_wr8(sch_smba, SMBHSTADD, (addr << 1) | read_write);
 		if (read_write == I2C_SMBUS_WRITE)
-			outb(command, SMBHSTCMD);
+			sch_io_wr8(sch_smba, SMBHSTCMD, command);
 		size = SCH_BYTE;
 		break;
 	case I2C_SMBUS_BYTE_DATA:
-		outb((addr << 1) | read_write, SMBHSTADD);
-		outb(command, SMBHSTCMD);
+		sch_io_wr8(sch_smba, SMBHSTADD, (addr << 1) | read_write);
+		sch_io_wr8(sch_smba, SMBHSTCMD, command);
 		if (read_write == I2C_SMBUS_WRITE)
-			outb(data->byte, SMBHSTDAT0);
+			sch_io_wr8(sch_smba, SMBHSTDAT0, data->byte);
 		size = SCH_BYTE_DATA;
 		break;
 	case I2C_SMBUS_WORD_DATA:
-		outb((addr << 1) | read_write, SMBHSTADD);
-		outb(command, SMBHSTCMD);
+		sch_io_wr8(sch_smba, SMBHSTADD, (addr << 1) | read_write);
+		sch_io_wr8(sch_smba, SMBHSTCMD, command);
 		if (read_write == I2C_SMBUS_WRITE) {
-			outb(data->word & 0xff, SMBHSTDAT0);
-			outb((data->word & 0xff00) >> 8, SMBHSTDAT1);
+			sch_io_wr8(sch_smba, SMBHSTDAT0, data->word >> 0);
+			sch_io_wr8(sch_smba, SMBHSTDAT1, data->word >> 8);
 		}
 		size = SCH_WORD_DATA;
 		break;
 	case I2C_SMBUS_BLOCK_DATA:
-		outb((addr << 1) | read_write, SMBHSTADD);
-		outb(command, SMBHSTCMD);
+		sch_io_wr8(sch_smba, SMBHSTADD, (addr << 1) | read_write);
+		sch_io_wr8(sch_smba, SMBHSTCMD, command);
 		if (read_write == I2C_SMBUS_WRITE) {
 			len = data->block[0];
 			if (len == 0 || len > I2C_SMBUS_BLOCK_MAX)
 				return -EINVAL;
-			outb(len, SMBHSTDAT0);
+			sch_io_wr8(sch_smba, SMBHSTDAT0, len);
 			for (i = 1; i <= len; i++)
-				outb(data->block[i], SMBBLKDAT+i-1);
+				sch_io_wr8(sch_smba, SMBBLKDAT + i - 1, data->block[i]);
 		}
 		size = SCH_BLOCK_DATA;
 		break;
@@ -200,7 +224,10 @@ static s32 sch_access(struct i2c_adapter *adap, u16 addr,
 		return -EOPNOTSUPP;
 	}
 	dev_dbg(&adap->dev, "write size %d to 0x%04x\n", size, SMBHSTCNT);
-	outb((inb(SMBHSTCNT) & 0xb0) | (size & 0x7), SMBHSTCNT);
+
+	temp = sch_io_rd8(sch_smba, SMBHSTCNT);
+	temp = (temp & 0xb0) | (size & 0x7);
+	sch_io_wr8(sch_smba, SMBHSTCNT, temp);
 
 	rc = sch_transaction(adap);
 	if (rc)	/* Error in transaction */
@@ -212,17 +239,18 @@ static s32 sch_access(struct i2c_adapter *adap, u16 addr,
 	switch (size) {
 	case SCH_BYTE:
 	case SCH_BYTE_DATA:
-		data->byte = inb(SMBHSTDAT0);
+		data->byte = sch_io_rd8(sch_smba, SMBHSTDAT0);
 		break;
 	case SCH_WORD_DATA:
-		data->word = inb(SMBHSTDAT0) + (inb(SMBHSTDAT1) << 8);
+		data->word = (sch_io_rd8(sch_smba, SMBHSTDAT0) << 0) +
+			     (sch_io_rd8(sch_smba, SMBHSTDAT1) << 8);
 		break;
 	case SCH_BLOCK_DATA:
-		data->block[0] = inb(SMBHSTDAT0);
+		data->block[0] = sch_io_rd8(sch_smba, SMBHSTDAT0);
 		if (data->block[0] == 0 || data->block[0] > I2C_SMBUS_BLOCK_MAX)
 			return -EPROTO;
 		for (i = 1; i <= data->block[0]; i++)
-			data->block[i] = inb(SMBBLKDAT+i-1);
+			data->block[i] = sch_io_rd8(sch_smba, SMBBLKDAT + i - 1);
 		break;
 	}
 	return 0;
@@ -255,26 +283,21 @@ static int smbus_sch_probe(struct platform_device *dev)
 	if (!res)
 		return -EBUSY;
 
-	if (!devm_request_region(&dev->dev, res->start, resource_size(res),
-				 dev->name)) {
-		dev_err(&dev->dev, "SMBus region 0x%x already in use!\n",
-			sch_smba);
+	sch_smba = devm_ioport_map(&dev->dev, res->start, resource_size(res));
+	if (!sch_smba) {
+		dev_err(&dev->dev, "SMBus region %pR already in use!\n", res);
 		return -EBUSY;
 	}
 
-	sch_smba = res->start;
-
-	dev_dbg(&dev->dev, "SMBA = 0x%X\n", sch_smba);
-
 	/* set up the sysfs linkage to our parent device */
 	sch_adapter.dev.parent = &dev->dev;
 
 	snprintf(sch_adapter.name, sizeof(sch_adapter.name),
-		"SMBus SCH adapter at %04x", sch_smba);
+		"SMBus SCH adapter at %04x", res->start);
 
 	retval = i2c_add_adapter(&sch_adapter);
 	if (retval)
-		sch_smba = 0;
+		sch_smba = NULL;
 
 	return retval;
 }
@@ -283,7 +306,7 @@ static void smbus_sch_remove(struct platform_device *pdev)
 {
 	if (sch_smba) {
 		i2c_del_adapter(&sch_adapter);
-		sch_smba = 0;
+		sch_smba = NULL;
 	}
 }
 
-- 
2.43.0.rc1.1336.g36b5255a03ac
Re: [PATCH v1 04/12] i2c: isch: Switch to memory mapped IO accessors
Posted by kernel test robot 2 months, 2 weeks ago
Hi Andy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on andi-shyti/i2c/i2c-host]
[also build test WARNING on linus/master v6.11-rc7]
[cannot apply to next-20240913]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/i2c-isch-Add-missed-else/20240912-002224
base:   https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host
patch link:    https://lore.kernel.org/r/20240911154820.2846187-5-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v1 04/12] i2c: isch: Switch to memory mapped IO accessors
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240914/202409141436.QFCDQrRF-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/202409141436.QFCDQrRF-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409141436.QFCDQrRF-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/i2c/busses/i2c-isch.c: In function 'smbus_sch_probe':
>> drivers/i2c/busses/i2c-isch.c:296:42: warning: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'resource_size_t' {aka 'long long unsigned int'} [-Wformat=]
     296 |                 "SMBus SCH adapter at %04x", res->start);
         |                                       ~~~^   ~~~~~~~~~~
         |                                          |      |
         |                                          |      resource_size_t {aka long long unsigned int}
         |                                          unsigned int
         |                                       %04llx


vim +296 drivers/i2c/busses/i2c-isch.c

   276	
   277	static int smbus_sch_probe(struct platform_device *dev)
   278	{
   279		struct resource *res;
   280		int retval;
   281	
   282		res = platform_get_resource(dev, IORESOURCE_IO, 0);
   283		if (!res)
   284			return -EBUSY;
   285	
   286		sch_smba = devm_ioport_map(&dev->dev, res->start, resource_size(res));
   287		if (!sch_smba) {
   288			dev_err(&dev->dev, "SMBus region %pR already in use!\n", res);
   289			return -EBUSY;
   290		}
   291	
   292		/* set up the sysfs linkage to our parent device */
   293		sch_adapter.dev.parent = &dev->dev;
   294	
   295		snprintf(sch_adapter.name, sizeof(sch_adapter.name),
 > 296			"SMBus SCH adapter at %04x", res->start);
   297	
   298		retval = i2c_add_adapter(&sch_adapter);
   299		if (retval)
   300			sch_smba = NULL;
   301	
   302		return retval;
   303	}
   304	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v1 04/12] i2c: isch: Switch to memory mapped IO accessors
Posted by Andy Shevchenko 2 months, 2 weeks ago
On Sat, Sep 14, 2024 at 02:56:19PM +0800, kernel test robot wrote:
> Hi Andy,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on andi-shyti/i2c/i2c-host]
> [also build test WARNING on linus/master v6.11-rc7]
> [cannot apply to next-20240913]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/i2c-isch-Add-missed-else/20240912-002224
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host
> patch link:    https://lore.kernel.org/r/20240911154820.2846187-5-andriy.shevchenko%40linux.intel.com
> patch subject: [PATCH v1 04/12] i2c: isch: Switch to memory mapped IO accessors
> config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240914/202409141436.QFCDQrRF-lkp@intel.com/config)
> compiler: alpha-linux-gcc (GCC) 13.3.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/202409141436.QFCDQrRF-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202409141436.QFCDQrRF-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    drivers/i2c/busses/i2c-isch.c: In function 'smbus_sch_probe':
> >> drivers/i2c/busses/i2c-isch.c:296:42: warning: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'resource_size_t' {aka 'long long unsigned int'} [-Wformat=]
>      296 |                 "SMBus SCH adapter at %04x", res->start);
>          |                                       ~~~^   ~~~~~~~~~~
>          |                                          |      |
>          |                                          |      resource_size_t {aka long long unsigned int}
>          |                                          unsigned int
>          |                                       %04llx

Yeah, this should be something like %pa, but the problem with that that it
always uses the same, fixed-width format with a prefix. We don't want this. But
to make sure we have proper specifier we need to introduce a temporary variable
and assign the resource start address to it and then use that variable in here.
I'll update this in v2 and send it after we have v6.12-rc1 is out.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 04/12] i2c: isch: Switch to memory mapped IO accessors
Posted by Andi Shyti 2 months, 2 weeks ago
Hi Andy,

On Mon, Sep 16, 2024 at 12:07:28PM GMT, Andy Shevchenko wrote:
> On Sat, Sep 14, 2024 at 02:56:19PM +0800, kernel test robot wrote:
> > Hi Andy,
> > 
> > kernel test robot noticed the following build warnings:
> > 
> > [auto build test WARNING on andi-shyti/i2c/i2c-host]
> > [also build test WARNING on linus/master v6.11-rc7]
> > [cannot apply to next-20240913]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > 
> > url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/i2c-isch-Add-missed-else/20240912-002224
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host
> > patch link:    https://lore.kernel.org/r/20240911154820.2846187-5-andriy.shevchenko%40linux.intel.com
> > patch subject: [PATCH v1 04/12] i2c: isch: Switch to memory mapped IO accessors
> > config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240914/202409141436.QFCDQrRF-lkp@intel.com/config)
> > compiler: alpha-linux-gcc (GCC) 13.3.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/202409141436.QFCDQrRF-lkp@intel.com/reproduce)
> > 
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202409141436.QFCDQrRF-lkp@intel.com/
> > 
> > All warnings (new ones prefixed by >>):
> > 
> >    drivers/i2c/busses/i2c-isch.c: In function 'smbus_sch_probe':
> > >> drivers/i2c/busses/i2c-isch.c:296:42: warning: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'resource_size_t' {aka 'long long unsigned int'} [-Wformat=]
> >      296 |                 "SMBus SCH adapter at %04x", res->start);
> >          |                                       ~~~^   ~~~~~~~~~~
> >          |                                          |      |
> >          |                                          |      resource_size_t {aka long long unsigned int}
> >          |                                          unsigned int
> >          |                                       %04llx
> 
> Yeah, this should be something like %pa, but the problem with that that it
> always uses the same, fixed-width format with a prefix. We don't want this. But
> to make sure we have proper specifier we need to introduce a temporary variable
> and assign the resource start address to it and then use that variable in here.
> I'll update this in v2 and send it after we have v6.12-rc1 is out.

Feel free to send it, I will apply it in i2c/i2c-host-for-6.12,
that's where I'm collecting the next patches.

Andi
Re: [PATCH v1 04/12] i2c: isch: Switch to memory mapped IO accessors
Posted by Andy Shevchenko 2 months, 2 weeks ago
On Mon, Sep 16, 2024 at 12:10:32PM +0200, Andi Shyti wrote:
> On Mon, Sep 16, 2024 at 12:07:28PM GMT, Andy Shevchenko wrote:
> > On Sat, Sep 14, 2024 at 02:56:19PM +0800, kernel test robot wrote:

...

> > >    drivers/i2c/busses/i2c-isch.c: In function 'smbus_sch_probe':
> > > >> drivers/i2c/busses/i2c-isch.c:296:42: warning: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'resource_size_t' {aka 'long long unsigned int'} [-Wformat=]
> > >      296 |                 "SMBus SCH adapter at %04x", res->start);
> > >          |                                       ~~~^   ~~~~~~~~~~
> > >          |                                          |      |
> > >          |                                          |      resource_size_t {aka long long unsigned int}
> > >          |                                          unsigned int
> > >          |                                       %04llx
> > 
> > Yeah, this should be something like %pa, but the problem with that that it
> > always uses the same, fixed-width format with a prefix. We don't want this. But
> > to make sure we have proper specifier we need to introduce a temporary variable
> > and assign the resource start address to it and then use that variable in here.
> > I'll update this in v2 and send it after we have v6.12-rc1 is out.
> 
> Feel free to send it, I will apply it in i2c/i2c-host-for-6.12,
> that's where I'm collecting the next patches.

But I believe it's a material for v6.13, no?
From the whole series the first patch is only a fix, the rest is pure
refactoring and cleanup.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 04/12] i2c: isch: Switch to memory mapped IO accessors
Posted by Andi Shyti 2 months, 2 weeks ago
On Mon, Sep 16, 2024 at 01:30:48PM GMT, Andy Shevchenko wrote:
> On Mon, Sep 16, 2024 at 12:10:32PM +0200, Andi Shyti wrote:
> > On Mon, Sep 16, 2024 at 12:07:28PM GMT, Andy Shevchenko wrote:
> > > On Sat, Sep 14, 2024 at 02:56:19PM +0800, kernel test robot wrote:
> 
> ...
> 
> > > >    drivers/i2c/busses/i2c-isch.c: In function 'smbus_sch_probe':
> > > > >> drivers/i2c/busses/i2c-isch.c:296:42: warning: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'resource_size_t' {aka 'long long unsigned int'} [-Wformat=]
> > > >      296 |                 "SMBus SCH adapter at %04x", res->start);
> > > >          |                                       ~~~^   ~~~~~~~~~~
> > > >          |                                          |      |
> > > >          |                                          |      resource_size_t {aka long long unsigned int}
> > > >          |                                          unsigned int
> > > >          |                                       %04llx
> > > 
> > > Yeah, this should be something like %pa, but the problem with that that it
> > > always uses the same, fixed-width format with a prefix. We don't want this. But
> > > to make sure we have proper specifier we need to introduce a temporary variable
> > > and assign the resource start address to it and then use that variable in here.
> > > I'll update this in v2 and send it after we have v6.12-rc1 is out.
> > 
> > Feel free to send it, I will apply it in i2c/i2c-host-for-6.12,
> > that's where I'm collecting the next patches.
> 
> But I believe it's a material for v6.13, no?

yes, I gave it the wrong name :-)
I renamed it now to i2c/i2c-host-for-6.13(*).

But it doesn't matter. It will become the next i2c/i2c-host after
Linus has taken the actual for-6.12 patches into mainline.

> From the whole series the first patch is only a fix, the rest is pure
> refactoring and cleanup.

Yes, you can omit the first patch. It has already been sent in
the fixes pull request.

Andi

(*) https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git/log/?h=i2c/i2c-host-for-6.13
Re: [PATCH v1 04/12] i2c: isch: Switch to memory mapped IO accessors
Posted by Andy Shevchenko 2 months, 2 weeks ago
On Mon, Sep 16, 2024 at 01:58:06PM +0200, Andi Shyti wrote:
> On Mon, Sep 16, 2024 at 01:30:48PM GMT, Andy Shevchenko wrote:
> > On Mon, Sep 16, 2024 at 12:10:32PM +0200, Andi Shyti wrote:
> > > On Mon, Sep 16, 2024 at 12:07:28PM GMT, Andy Shevchenko wrote:

...

> > > Feel free to send it, I will apply it in i2c/i2c-host-for-6.12,
> > > that's where I'm collecting the next patches.
> > 
> > But I believe it's a material for v6.13, no?
> 
> yes, I gave it the wrong name :-)
> I renamed it now to i2c/i2c-host-for-6.13(*).
> 
> But it doesn't matter. It will become the next i2c/i2c-host after
> Linus has taken the actual for-6.12 patches into mainline.

v2 has just been sent out.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 04/12] i2c: isch: Switch to memory mapped IO accessors
Posted by kernel test robot 2 months, 2 weeks ago
Hi Andy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on andi-shyti/i2c/i2c-host]
[also build test WARNING on linus/master v6.11-rc7]
[cannot apply to next-20240913]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/i2c-isch-Add-missed-else/20240912-002224
base:   https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host
patch link:    https://lore.kernel.org/r/20240911154820.2846187-5-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v1 04/12] i2c: isch: Switch to memory mapped IO accessors
config: i386-buildonly-randconfig-001-20240913 (https://download.01.org/0day-ci/archive/20240914/202409140743.kKVc8T3C-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/202409140743.kKVc8T3C-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409140743.kKVc8T3C-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/i2c/busses/i2c-isch.c:296:32: warning: format specifies type 'unsigned int' but the argument has type 'resource_size_t' (aka 'unsigned long long') [-Wformat]
     296 |                 "SMBus SCH adapter at %04x", res->start);
         |                                       ~~~~   ^~~~~~~~~~
         |                                       %04llx
   1 warning generated.


vim +296 drivers/i2c/busses/i2c-isch.c

   276	
   277	static int smbus_sch_probe(struct platform_device *dev)
   278	{
   279		struct resource *res;
   280		int retval;
   281	
   282		res = platform_get_resource(dev, IORESOURCE_IO, 0);
   283		if (!res)
   284			return -EBUSY;
   285	
   286		sch_smba = devm_ioport_map(&dev->dev, res->start, resource_size(res));
   287		if (!sch_smba) {
   288			dev_err(&dev->dev, "SMBus region %pR already in use!\n", res);
   289			return -EBUSY;
   290		}
   291	
   292		/* set up the sysfs linkage to our parent device */
   293		sch_adapter.dev.parent = &dev->dev;
   294	
   295		snprintf(sch_adapter.name, sizeof(sch_adapter.name),
 > 296			"SMBus SCH adapter at %04x", res->start);
   297	
   298		retval = i2c_add_adapter(&sch_adapter);
   299		if (retval)
   300			sch_smba = NULL;
   301	
   302		return retval;
   303	}
   304	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki