linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon
@ 2022-08-20 19:45 Arminder Singh
  2022-08-20 21:39 ` kernel test robot
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Arminder Singh @ 2022-08-20 19:45 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sven Peter, Alyssa Rosenzweig, Hector Martin, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, linux-arm-kernel,
	linuxppc-dev, linux-i2c, Arminder Singh

This is the first time I'm interacting with the Linux mailing lists, so 
please don't eviscerate me *too much* if I get the formatting wrong.
Of course I'm always willing to take criticism and improve my formatting 
in the future.

This patch adds support for IRQs to the PASemi I2C controller driver.
This will allow for faster performing I2C transactions on Apple Silicon
hardware, as previously, the driver was forced to poll the SMSTA register
for a set amount of time.

With this patchset the driver on Apple silicon hardware will instead wait
for an interrupt which will signal the completion of the I2C transaction.
The timeout value for this completion will be the same as the current
amount of time the I2C driver polls for.

This will result in some performance improvement since the driver will be
waiting for less time than it does right now on Apple Silicon hardware.

The patch right now will only enable IRQs for Apple Silicon I2C chips,
and only if it's able to successfully request the IRQ from the kernel.

=== Testing ===

This patch has been tested on both the mainline Linux kernel tree and
the Asahi branch (https://github.com/AsahiLinux/linux.git) on both an
M1 and M2 MacBook Air, and it compiles successfully as both a module and
built-in to the kernel itself. The patch in both trees successfully boots
to userspace without any hitch.

I do not have PASemi hardware on hand unfortunately, so I'm unable to test
the impact of this patch on old PASemi hardware. This is also why I've
elected to do the IRQ request and enablement on the Apple platform driver
and not in the common file, as I'm not sure if PASemi hardware supports
IRQs.

I also fixed a quick checkpatch warning on line 303. "i ++" is now "i++".

Any and all critiques of the patch would be well appreciated.




Signed-off-by: Arminder Singh <arminders208@outlook.com>
---
 drivers/i2c/busses/i2c-pasemi-core.c     | 29 ++++++++++++++++++++----
 drivers/i2c/busses/i2c-pasemi-core.h     |  5 ++++
 drivers/i2c/busses/i2c-pasemi-platform.c |  8 +++++++
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c b/drivers/i2c/busses/i2c-pasemi-core.c
index 9028ffb58cc0..375aa9528233 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -21,6 +21,7 @@
 #define REG_MTXFIFO	0x00
 #define REG_MRXFIFO	0x04
 #define REG_SMSTA	0x14
+#define REG_IMASK   0x18
 #define REG_CTL		0x1c
 #define REG_REV		0x28
 
@@ -80,14 +81,21 @@ static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
 {
 	int timeout = 10;
 	unsigned int status;
+	unsigned int bitmask = SMSTA_XEN | SMSTA_MTN;
 
-	status = reg_read(smbus, REG_SMSTA);
-
-	while (!(status & SMSTA_XEN) && timeout--) {
-		msleep(1);
+	if (smbus->use_irq) {
+		reinit_completion(&smbus->irq_completion);
+		reg_write(smbus, REG_IMASK, bitmask);
+		wait_for_completion_timeout(&smbus->irq_completion, msecs_to_jiffies(10));
 		status = reg_read(smbus, REG_SMSTA);
+	} else {
+		while (!(status & SMSTA_XEN) && timeout--) {
+			msleep(1);
+			status = reg_read(smbus, REG_SMSTA);
+		}
 	}
 
+
 	/* Got NACK? */
 	if (status & SMSTA_MTN)
 		return -ENXIO;
@@ -300,7 +308,7 @@ static int pasemi_smb_xfer(struct i2c_adapter *adapter,
 	case I2C_SMBUS_BLOCK_DATA:
 	case I2C_SMBUS_BLOCK_PROC_CALL:
 		data->block[0] = len;
-		for (i = 1; i <= len; i ++) {
+		for (i = 1; i <= len; i++) {
 			rd = RXFIFO_RD(smbus);
 			if (rd & MRXFIFO_EMPTY) {
 				err = -ENODATA;
@@ -348,6 +356,8 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
 	if (smbus->hw_rev != PASEMI_HW_REV_PCI)
 		smbus->hw_rev = reg_read(smbus, REG_REV);
 
+	reg_write(smbus, REG_IMASK, 0);
+
 	pasemi_reset(smbus);
 
 	error = devm_i2c_add_adapter(smbus->dev, &smbus->adapter);
@@ -356,3 +366,12 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
 
 	return 0;
 }
+
+irqreturn_t pasemi_irq_handler(int irq, void *dev_id)
+{
+	struct pasemi_smbus *smbus = (struct pasemi_smbus *)dev_id;
+
+	reg_write(smbus, REG_IMASK, 0);
+	complete(&smbus->irq_completion);
+	return IRQ_HANDLED;
+}
diff --git a/drivers/i2c/busses/i2c-pasemi-core.h b/drivers/i2c/busses/i2c-pasemi-core.h
index 4655124a37f3..045e4a9a3d13 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.h
+++ b/drivers/i2c/busses/i2c-pasemi-core.h
@@ -7,6 +7,7 @@
 #include <linux/i2c-smbus.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/completion.h>
 
 #define PASEMI_HW_REV_PCI -1
 
@@ -16,6 +17,10 @@ struct pasemi_smbus {
 	void __iomem		*ioaddr;
 	unsigned int		 clk_div;
 	int			 hw_rev;
+	int          use_irq;
+	struct completion irq_completion;
 };
 
 int pasemi_i2c_common_probe(struct pasemi_smbus *smbus);
+
+irqreturn_t pasemi_irq_handler(int irq, void *dev_id);
diff --git a/drivers/i2c/busses/i2c-pasemi-platform.c b/drivers/i2c/busses/i2c-pasemi-platform.c
index 88a54aaf7e3c..ee1c84e7734b 100644
--- a/drivers/i2c/busses/i2c-pasemi-platform.c
+++ b/drivers/i2c/busses/i2c-pasemi-platform.c
@@ -49,6 +49,7 @@ static int pasemi_platform_i2c_probe(struct platform_device *pdev)
 	struct pasemi_smbus *smbus;
 	u32 frequency;
 	int error;
+	int irq_num;
 
 	data = devm_kzalloc(dev, sizeof(struct pasemi_platform_i2c_data),
 			    GFP_KERNEL);
@@ -82,6 +83,13 @@ static int pasemi_platform_i2c_probe(struct platform_device *pdev)
 	if (error)
 		goto out_clk_disable;
 
+	smbus->use_irq = 0;
+	init_completion(&smbus->irq_completion);
+	irq_num = platform_get_irq(pdev, 0);
+	error = request_irq(irq_num, pasemi_irq_handler, 0, "pasemi_apple_i2c", (void *)smbus);
+
+	if (!error)
+		smbus->use_irq = 1;
 	platform_set_drvdata(pdev, data);
 
 	return 0;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon
  2022-08-20 19:45 [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon Arminder Singh
@ 2022-08-20 21:39 ` kernel test robot
  2022-08-21  7:24 ` Christophe Leroy
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2022-08-20 21:39 UTC (permalink / raw)
  To: Arminder Singh, linux-kernel
  Cc: llvm, kbuild-all, Sven Peter, Alyssa Rosenzweig, Hector Martin,
	Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	linux-arm-kernel, linuxppc-dev, linux-i2c, Arminder Singh

Hi Arminder,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on linus/master v6.0-rc1 next-20220819]
[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/Arminder-Singh/i2c-pasemi-Add-IRQ-support-for-Apple-Silicon/20220821-034703
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: riscv-randconfig-r042-20220821 (https://download.01.org/0day-ci/archive/20220821/202208210548.MCoIwyQl-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project c9a41fe60ab62f7a40049c100adcc8087a47669b)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/98584b2b51d808ab582798c7a50511e8c1889ced
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Arminder-Singh/i2c-pasemi-Add-IRQ-support-for-Apple-Silicon/20220821-034703
        git checkout 98584b2b51d808ab582798c7a50511e8c1889ced
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/i2c/busses/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/i2c/busses/i2c-pasemi-core.c:9:
   In file included from include/linux/pci.h:38:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from drivers/i2c/busses/i2c-pasemi-core.c:9:
   In file included from include/linux/pci.h:38:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from drivers/i2c/busses/i2c-pasemi-core.c:9:
   In file included from include/linux/pci.h:38:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:1134:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
                                                     ~~~~~~~~~~ ^
>> drivers/i2c/busses/i2c-pasemi-core.c:86:6: warning: variable 'status' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
           if (smbus->use_irq) {
               ^~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-pasemi-core.c:92:12: note: uninitialized use occurs here
                   while (!(status & SMSTA_XEN) && timeout--) {
                            ^~~~~~
   drivers/i2c/busses/i2c-pasemi-core.c:86:2: note: remove the 'if' if its condition is always true
           if (smbus->use_irq) {
           ^~~~~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-pasemi-core.c:83:21: note: initialize the variable 'status' to silence this warning
           unsigned int status;
                              ^
                               = 0
   8 warnings generated.


vim +86 drivers/i2c/busses/i2c-pasemi-core.c

    79	
    80	static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
    81	{
    82		int timeout = 10;
    83		unsigned int status;
    84		unsigned int bitmask = SMSTA_XEN | SMSTA_MTN;
    85	
  > 86		if (smbus->use_irq) {
    87			reinit_completion(&smbus->irq_completion);
    88			reg_write(smbus, REG_IMASK, bitmask);
    89			wait_for_completion_timeout(&smbus->irq_completion, msecs_to_jiffies(10));
    90			status = reg_read(smbus, REG_SMSTA);
    91		} else {
    92			while (!(status & SMSTA_XEN) && timeout--) {
    93				msleep(1);
    94				status = reg_read(smbus, REG_SMSTA);
    95			}
    96		}
    97	
    98	
    99		/* Got NACK? */
   100		if (status & SMSTA_MTN)
   101			return -ENXIO;
   102	
   103		if (timeout < 0) {
   104			dev_warn(smbus->dev, "Timeout, status 0x%08x\n", status);
   105			reg_write(smbus, REG_SMSTA, status);
   106			return -ETIME;
   107		}
   108	
   109		/* Clear XEN */
   110		reg_write(smbus, REG_SMSTA, SMSTA_XEN);
   111	
   112		return 0;
   113	}
   114	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon
  2022-08-20 19:45 [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon Arminder Singh
  2022-08-20 21:39 ` kernel test robot
@ 2022-08-21  7:24 ` Christophe Leroy
  2022-08-21  9:55 ` Sven Peter
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2022-08-21  7:24 UTC (permalink / raw)
  To: Arminder Singh
  Cc: Sven Peter, Hector Martin, Paul Mackerras, linux-arm-kernel,
	linuxppc-dev, Alyssa Rosenzweig, linux-i2c, linux-kernel



Le 20/08/2022 à 21:45, Arminder Singh a écrit :
> This is the first time I'm interacting with the Linux mailing lists, so
> please don't eviscerate me *too much* if I get the formatting wrong.
> Of course I'm always willing to take criticism and improve my formatting
> in the future.

The above text should go after you signed-off-by, after the --- , so 
that it get's ignored by 'git am' when applying.

> 
> This patch adds support for IRQs to the PASemi I2C controller driver.
> This will allow for faster performing I2C transactions on Apple Silicon
> hardware, as previously, the driver was forced to poll the SMSTA register
> for a set amount of time.
> 
> With this patchset the driver on Apple silicon hardware will instead wait
> for an interrupt which will signal the completion of the I2C transaction.
> The timeout value for this completion will be the same as the current
> amount of time the I2C driver polls for.
> 
> This will result in some performance improvement since the driver will be
> waiting for less time than it does right now on Apple Silicon hardware.
> 
> The patch right now will only enable IRQs for Apple Silicon I2C chips,
> and only if it's able to successfully request the IRQ from the kernel.
> 
> === Testing ===
> 
> This patch has been tested on both the mainline Linux kernel tree and
> the Asahi branch (https://github.com/AsahiLinux/linux.git) on both an
> M1 and M2 MacBook Air, and it compiles successfully as both a module and
> built-in to the kernel itself. The patch in both trees successfully boots
> to userspace without any hitch.
> 
> I do not have PASemi hardware on hand unfortunately, so I'm unable to test
> the impact of this patch on old PASemi hardware. This is also why I've
> elected to do the IRQ request and enablement on the Apple platform driver
> and not in the common file, as I'm not sure if PASemi hardware supports
> IRQs.
> 
> I also fixed a quick checkpatch warning on line 303. "i ++" is now "i++".
> 
> Any and all critiques of the patch would be well appreciated.

That as well shouldn't be part of the commit message.

> 
> 
> 
> 
> Signed-off-by: Arminder Singh <arminders208@outlook.com>
> ---
>   drivers/i2c/busses/i2c-pasemi-core.c     | 29 ++++++++++++++++++++----
>   drivers/i2c/busses/i2c-pasemi-core.h     |  5 ++++
>   drivers/i2c/busses/i2c-pasemi-platform.c |  8 +++++++
>   3 files changed, 37 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-pasemi-core.c b/drivers/i2c/busses/i2c-pasemi-core.c
> index 9028ffb58cc0..375aa9528233 100644
> --- a/drivers/i2c/busses/i2c-pasemi-core.c
> +++ b/drivers/i2c/busses/i2c-pasemi-core.c
> @@ -21,6 +21,7 @@
>   #define REG_MTXFIFO    0x00
>   #define REG_MRXFIFO    0x04
>   #define REG_SMSTA      0x14
> +#define REG_IMASK   0x18
>   #define REG_CTL                0x1c
>   #define REG_REV                0x28
> 
> @@ -80,14 +81,21 @@ static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
>   {
>          int timeout = 10;
>          unsigned int status;
> +       unsigned int bitmask = SMSTA_XEN | SMSTA_MTN;

Used only inside the if (smbus->use_irq), bitmask should be defined only 
in that scope.

> 
> -       status = reg_read(smbus, REG_SMSTA);

This should go in the else block.

> -
> -       while (!(status & SMSTA_XEN) && timeout--) {
> -               msleep(1);
> +       if (smbus->use_irq) {
> +               reinit_completion(&smbus->irq_completion);
> +               reg_write(smbus, REG_IMASK, bitmask);
> +               wait_for_completion_timeout(&smbus->irq_completion, msecs_to_jiffies(10));
>                  status = reg_read(smbus, REG_SMSTA);
> +       } else {

status is uninitialised when entering the while loop.

> +               while (!(status & SMSTA_XEN) && timeout--) {
> +                       msleep(1);
> +                       status = reg_read(smbus, REG_SMSTA);
> +               }
>          }
> 
> +

Why a second blank line here ?

>          /* Got NACK? */
>          if (status & SMSTA_MTN)
>                  return -ENXIO;
> @@ -300,7 +308,7 @@ static int pasemi_smb_xfer(struct i2c_adapter *adapter,
>          case I2C_SMBUS_BLOCK_DATA:
>          case I2C_SMBUS_BLOCK_PROC_CALL:
>                  data->block[0] = len;
> -               for (i = 1; i <= len; i ++) {
> +               for (i = 1; i <= len; i++) {

This shouldn't be part of this patch, it is unrelated.

>                          rd = RXFIFO_RD(smbus);
>                          if (rd & MRXFIFO_EMPTY) {
>                                  err = -ENODATA;
> @@ -348,6 +356,8 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
>          if (smbus->hw_rev != PASEMI_HW_REV_PCI)
>                  smbus->hw_rev = reg_read(smbus, REG_REV);
> 
> +       reg_write(smbus, REG_IMASK, 0);
> +
>          pasemi_reset(smbus);
> 
>          error = devm_i2c_add_adapter(smbus->dev, &smbus->adapter);
> @@ -356,3 +366,12 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
> 
>          return 0;
>   }
> +
> +irqreturn_t pasemi_irq_handler(int irq, void *dev_id)
> +{
> +       struct pasemi_smbus *smbus = (struct pasemi_smbus *)dev_id;

dev_id is void*, the cast is not needed.

> +
> +       reg_write(smbus, REG_IMASK, 0);
> +       complete(&smbus->irq_completion);
> +       return IRQ_HANDLED;
> +}
> diff --git a/drivers/i2c/busses/i2c-pasemi-core.h b/drivers/i2c/busses/i2c-pasemi-core.h
> index 4655124a37f3..045e4a9a3d13 100644
> --- a/drivers/i2c/busses/i2c-pasemi-core.h
> +++ b/drivers/i2c/busses/i2c-pasemi-core.h
> @@ -7,6 +7,7 @@
>   #include <linux/i2c-smbus.h>
>   #include <linux/io.h>
>   #include <linux/kernel.h>
> +#include <linux/completion.h>
> 
>   #define PASEMI_HW_REV_PCI -1
> 
> @@ -16,6 +17,10 @@ struct pasemi_smbus {
>          void __iomem            *ioaddr;
>          unsigned int             clk_div;
>          int                      hw_rev;
> +       int          use_irq;
> +       struct completion irq_completion;

Keep same alignment for member name.

>   };
> 
>   int pasemi_i2c_common_probe(struct pasemi_smbus *smbus);
> +
> +irqreturn_t pasemi_irq_handler(int irq, void *dev_id);
> diff --git a/drivers/i2c/busses/i2c-pasemi-platform.c b/drivers/i2c/busses/i2c-pasemi-platform.c
> index 88a54aaf7e3c..ee1c84e7734b 100644
> --- a/drivers/i2c/busses/i2c-pasemi-platform.c
> +++ b/drivers/i2c/busses/i2c-pasemi-platform.c
> @@ -49,6 +49,7 @@ static int pasemi_platform_i2c_probe(struct platform_device *pdev)
>          struct pasemi_smbus *smbus;
>          u32 frequency;
>          int error;
> +       int irq_num;
> 
>          data = devm_kzalloc(dev, sizeof(struct pasemi_platform_i2c_data),
>                              GFP_KERNEL);
> @@ -82,6 +83,13 @@ static int pasemi_platform_i2c_probe(struct platform_device *pdev)
>          if (error)
>                  goto out_clk_disable;
> 
> +       smbus->use_irq = 0;
> +       init_completion(&smbus->irq_completion);
> +       irq_num = platform_get_irq(pdev, 0);
> +       error = request_irq(irq_num, pasemi_irq_handler, 0, "pasemi_apple_i2c", (void *)smbus);
> +
> +       if (!error)
> +               smbus->use_irq = 1;
>          platform_set_drvdata(pdev, data);
> 
>          return 0;
> --
> 2.34.1
> 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon
  2022-08-20 19:45 [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon Arminder Singh
  2022-08-20 21:39 ` kernel test robot
  2022-08-21  7:24 ` Christophe Leroy
@ 2022-08-21  9:55 ` Sven Peter
  2022-08-22  8:44 ` Dan Carpenter
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Sven Peter @ 2022-08-21  9:55 UTC (permalink / raw)
  To: Arminder Singh, linux-kernel
  Cc: Alyssa Rosenzweig, Hector Martin, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, linux-arm-kernel,
	linuxppc-dev, linux-i2c

Hi,

Thanks for the patch! Some additional comments:

On Sat, Aug 20, 2022, at 21:45, Arminder Singh wrote:
> This is the first time I'm interacting with the Linux mailing lists, so 
> please don't eviscerate me *too much* if I get the formatting wrong.
> Of course I'm always willing to take criticism and improve my formatting 
> in the future.
>
> This patch adds support for IRQs to the PASemi I2C controller driver.
> This will allow for faster performing I2C transactions on Apple Silicon
> hardware, as previously, the driver was forced to poll the SMSTA register
> for a set amount of time.
>
> With this patchset the driver on Apple silicon hardware will instead wait
> for an interrupt which will signal the completion of the I2C transaction.
> The timeout value for this completion will be the same as the current
> amount of time the I2C driver polls for.
>
> This will result in some performance improvement since the driver will be
> waiting for less time than it does right now on Apple Silicon hardware.
>
> The patch right now will only enable IRQs for Apple Silicon I2C chips,
> and only if it's able to successfully request the IRQ from the kernel.
>
> === Testing ===
>
> This patch has been tested on both the mainline Linux kernel tree and
> the Asahi branch (https://github.com/AsahiLinux/linux.git) on both an
> M1 and M2 MacBook Air, and it compiles successfully as both a module and
> built-in to the kernel itself. The patch in both trees successfully boots
> to userspace without any hitch.
>
> I do not have PASemi hardware on hand unfortunately, so I'm unable to test
> the impact of this patch on old PASemi hardware. This is also why I've
> elected to do the IRQ request and enablement on the Apple platform driver
> and not in the common file, as I'm not sure if PASemi hardware supports
> IRQs.
>
> I also fixed a quick checkpatch warning on line 303. "i ++" is now "i++".
>
> Any and all critiques of the patch would be well appreciated.
>
>
>
>
> Signed-off-by: Arminder Singh <arminders208@outlook.com>
> ---
>  drivers/i2c/busses/i2c-pasemi-core.c     | 29 ++++++++++++++++++++----
>  drivers/i2c/busses/i2c-pasemi-core.h     |  5 ++++
>  drivers/i2c/busses/i2c-pasemi-platform.c |  8 +++++++
>  3 files changed, 37 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
> b/drivers/i2c/busses/i2c-pasemi-core.c
> index 9028ffb58cc0..375aa9528233 100644
> --- a/drivers/i2c/busses/i2c-pasemi-core.c
> +++ b/drivers/i2c/busses/i2c-pasemi-core.c
> @@ -21,6 +21,7 @@
>  #define REG_MTXFIFO	0x00
>  #define REG_MRXFIFO	0x04
>  #define REG_SMSTA	0x14
> +#define REG_IMASK   0x18
>  #define REG_CTL		0x1c
>  #define REG_REV		0x28
> 
> @@ -80,14 +81,21 @@ static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
>  {
>  	int timeout = 10;
>  	unsigned int status;
> +	unsigned int bitmask = SMSTA_XEN | SMSTA_MTN;
> 
> -	status = reg_read(smbus, REG_SMSTA);
> -
> -	while (!(status & SMSTA_XEN) && timeout--) {
> -		msleep(1);
> +	if (smbus->use_irq) {
> +		reinit_completion(&smbus->irq_completion);
> +		reg_write(smbus, REG_IMASK, bitmask);

s/bitmask/SMSTA_XEN | SMSTA_MTN/ and then you can just drop the bitmask
variable which isn't used anywhere else.

> +		wait_for_completion_timeout(&smbus->irq_completion, msecs_to_jiffies(10));
>  		status = reg_read(smbus, REG_SMSTA);

If the irq hasn't fired and wait_for_completion_timeout timed out the irq
is still enabled here. I'd put a reg_write(smbus, REG_IMASK, 0); here
to be safe.

> +	} else {

You also need status = reg_read(smbus, REG_SMSTA); here.

> +		while (!(status & SMSTA_XEN) && timeout--) {
> +			msleep(1);
> +			status = reg_read(smbus, REG_SMSTA);
> +		}
>  	}
> 
> +
>  	/* Got NACK? */
>  	if (status & SMSTA_MTN)
>  		return -ENXIO;
> @@ -300,7 +308,7 @@ static int pasemi_smb_xfer(struct i2c_adapter *adapter,
>  	case I2C_SMBUS_BLOCK_DATA:
>  	case I2C_SMBUS_BLOCK_PROC_CALL:
>  		data->block[0] = len;
> -		for (i = 1; i <= len; i ++) {
> +		for (i = 1; i <= len; i++) {
>  			rd = RXFIFO_RD(smbus);
>  			if (rd & MRXFIFO_EMPTY) {
>  				err = -ENODATA;
> @@ -348,6 +356,8 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
>  	if (smbus->hw_rev != PASEMI_HW_REV_PCI)
>  		smbus->hw_rev = reg_read(smbus, REG_REV);
> 
> +	reg_write(smbus, REG_IMASK, 0);
> +
>  	pasemi_reset(smbus);
> 
>  	error = devm_i2c_add_adapter(smbus->dev, &smbus->adapter);
> @@ -356,3 +366,12 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
> 
>  	return 0;
>  }
> +
> +irqreturn_t pasemi_irq_handler(int irq, void *dev_id)
> +{
> +	struct pasemi_smbus *smbus = (struct pasemi_smbus *)dev_id;
> +
> +	reg_write(smbus, REG_IMASK, 0);
> +	complete(&smbus->irq_completion);
> +	return IRQ_HANDLED;
> +}
> diff --git a/drivers/i2c/busses/i2c-pasemi-core.h 
> b/drivers/i2c/busses/i2c-pasemi-core.h
> index 4655124a37f3..045e4a9a3d13 100644
> --- a/drivers/i2c/busses/i2c-pasemi-core.h
> +++ b/drivers/i2c/busses/i2c-pasemi-core.h
> @@ -7,6 +7,7 @@
>  #include <linux/i2c-smbus.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
> +#include <linux/completion.h>
> 
>  #define PASEMI_HW_REV_PCI -1
> 
> @@ -16,6 +17,10 @@ struct pasemi_smbus {
>  	void __iomem		*ioaddr;
>  	unsigned int		 clk_div;
>  	int			 hw_rev;
> +	int          use_irq;
> +	struct completion irq_completion;
>  };
> 
>  int pasemi_i2c_common_probe(struct pasemi_smbus *smbus);
> +
> +irqreturn_t pasemi_irq_handler(int irq, void *dev_id);
> diff --git a/drivers/i2c/busses/i2c-pasemi-platform.c 
> b/drivers/i2c/busses/i2c-pasemi-platform.c
> index 88a54aaf7e3c..ee1c84e7734b 100644
> --- a/drivers/i2c/busses/i2c-pasemi-platform.c
> +++ b/drivers/i2c/busses/i2c-pasemi-platform.c
> @@ -49,6 +49,7 @@ static int pasemi_platform_i2c_probe(struct 
> platform_device *pdev)
>  	struct pasemi_smbus *smbus;
>  	u32 frequency;
>  	int error;
> +	int irq_num;
> 
>  	data = devm_kzalloc(dev, sizeof(struct pasemi_platform_i2c_data),
>  			    GFP_KERNEL);
> @@ -82,6 +83,13 @@ static int pasemi_platform_i2c_probe(struct 
> platform_device *pdev)
>  	if (error)
>  		goto out_clk_disable;
> 
> +	smbus->use_irq = 0;
> +	init_completion(&smbus->irq_completion);

I'd move this into the common probe function. If someone eventually wants
to add irq support to the PASemi boards it'll be required there as well.

> +	irq_num = platform_get_irq(pdev, 0);
> +	error = request_irq(irq_num, pasemi_irq_handler, 0, 
> "pasemi_apple_i2c", (void *)smbus);

If you use request_irq here you'll have to add a remove function and
call free_irq there. I'd just use devm_request_irq which takes care of
that automatically.

> +
> +	if (!error)
> +		smbus->use_irq = 1;
>  	platform_set_drvdata(pdev, data);
> 
>  	return 0;
> -- 
> 2.34.1

Best,


Sven

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon
  2022-08-20 19:45 [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon Arminder Singh
                   ` (2 preceding siblings ...)
  2022-08-21  9:55 ` Sven Peter
@ 2022-08-22  8:44 ` Dan Carpenter
  2022-08-23  1:50 ` Michael Ellerman
  2022-08-25 12:29 ` Arnd Bergmann
  5 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2022-08-22  8:44 UTC (permalink / raw)
  To: kbuild, Arminder Singh, linux-kernel
  Cc: lkp, kbuild-all, Sven Peter, Alyssa Rosenzweig, Hector Martin,
	Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	linux-arm-kernel, linuxppc-dev, linux-i2c, Arminder Singh

Hi Arminder,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Arminder-Singh/i2c-pasemi-Add-IRQ-support-for-Apple-Silicon/20220821-034703
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: parisc-randconfig-m031-20220821 (https://download.01.org/0day-ci/archive/20220822/202208220231.f88sizqa-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/i2c/busses/i2c-pasemi-core.c:92 pasemi_smb_waitready() error: uninitialized symbol 'status'.

vim +/status +92 drivers/i2c/busses/i2c-pasemi-core.c

8032214346c0c8 drivers/i2c/busses/i2c-pasemi.c      Julia Lawall   2010-09-05   80  static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13   81  {
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13   82  	int timeout = 10;
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13   83  	unsigned int status;
98584b2b51d808 drivers/i2c/busses/i2c-pasemi-core.c Arminder Singh 2022-08-20   84  	unsigned int bitmask = SMSTA_XEN | SMSTA_MTN;
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13   85  
98584b2b51d808 drivers/i2c/busses/i2c-pasemi-core.c Arminder Singh 2022-08-20   86  	if (smbus->use_irq) {
98584b2b51d808 drivers/i2c/busses/i2c-pasemi-core.c Arminder Singh 2022-08-20   87  		reinit_completion(&smbus->irq_completion);
98584b2b51d808 drivers/i2c/busses/i2c-pasemi-core.c Arminder Singh 2022-08-20   88  		reg_write(smbus, REG_IMASK, bitmask);
98584b2b51d808 drivers/i2c/busses/i2c-pasemi-core.c Arminder Singh 2022-08-20   89  		wait_for_completion_timeout(&smbus->irq_completion, msecs_to_jiffies(10));
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13   90  		status = reg_read(smbus, REG_SMSTA);
98584b2b51d808 drivers/i2c/busses/i2c-pasemi-core.c Arminder Singh 2022-08-20   91  	} else {
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13  @92  		while (!(status & SMSTA_XEN) && timeout--) {

"status" not intialized.

beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13   93  			msleep(1);
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13   94  			status = reg_read(smbus, REG_SMSTA);
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13   95  		}
98584b2b51d808 drivers/i2c/busses/i2c-pasemi-core.c Arminder Singh 2022-08-20   96  	}
98584b2b51d808 drivers/i2c/busses/i2c-pasemi-core.c Arminder Singh 2022-08-20   97  
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13   98  
be8a1f7cd4501c drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-11-15   99  	/* Got NACK? */
be8a1f7cd4501c drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-11-15  100  	if (status & SMSTA_MTN)
be8a1f7cd4501c drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-11-15  101  		return -ENXIO;
be8a1f7cd4501c drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-11-15  102  
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13  103  	if (timeout < 0) {
c06f50ed36cc0a drivers/i2c/busses/i2c-pasemi.c      Sven Peter     2021-10-08  104  		dev_warn(smbus->dev, "Timeout, status 0x%08x\n", status);
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13  105  		reg_write(smbus, REG_SMSTA, status);
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13  106  		return -ETIME;
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13  107  	}
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13  108  
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13  109  	/* Clear XEN */
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13  110  	reg_write(smbus, REG_SMSTA, SMSTA_XEN);
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13  111  
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13  112  	return 0;
beb58aa39e6e5a drivers/i2c/busses/i2c-pasemi.c      Olof Johansson 2007-02-13  113  }

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon
  2022-08-20 19:45 [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon Arminder Singh
                   ` (3 preceding siblings ...)
  2022-08-22  8:44 ` Dan Carpenter
@ 2022-08-23  1:50 ` Michael Ellerman
  2022-08-25 14:45   ` Christian Zigotzky
  2022-08-25 12:29 ` Arnd Bergmann
  5 siblings, 1 reply; 8+ messages in thread
From: Michael Ellerman @ 2022-08-23  1:50 UTC (permalink / raw)
  To: Arminder Singh, linux-kernel
  Cc: Sven Peter, Alyssa Rosenzweig, Hector Martin,
	Benjamin Herrenschmidt, Paul Mackerras, linux-arm-kernel,
	linuxppc-dev, linux-i2c, Arminder Singh, Darren Stevens,
	Christian Zigotzky

Arminder Singh <arminders208@outlook.com> writes:
> This is the first time I'm interacting with the Linux mailing lists, so 
> please don't eviscerate me *too much* if I get the formatting wrong.
> Of course I'm always willing to take criticism and improve my formatting 
> in the future.
>
> This patch adds support for IRQs to the PASemi I2C controller driver.
> This will allow for faster performing I2C transactions on Apple Silicon
> hardware, as previously, the driver was forced to poll the SMSTA register
> for a set amount of time.
>
> With this patchset the driver on Apple silicon hardware will instead wait
> for an interrupt which will signal the completion of the I2C transaction.
> The timeout value for this completion will be the same as the current
> amount of time the I2C driver polls for.
>
> This will result in some performance improvement since the driver will be
> waiting for less time than it does right now on Apple Silicon hardware.
>
> The patch right now will only enable IRQs for Apple Silicon I2C chips,
> and only if it's able to successfully request the IRQ from the kernel.
>
> === Testing ===
>
> This patch has been tested on both the mainline Linux kernel tree and
> the Asahi branch (https://github.com/AsahiLinux/linux.git) on both an
> M1 and M2 MacBook Air, and it compiles successfully as both a module and
> built-in to the kernel itself. The patch in both trees successfully boots
> to userspace without any hitch.
>
> I do not have PASemi hardware on hand unfortunately, so I'm unable to test
> the impact of this patch on old PASemi hardware. This is also why I've
> elected to do the IRQ request and enablement on the Apple platform driver
> and not in the common file, as I'm not sure if PASemi hardware supports
> IRQs.

I've added Darren and Christian to Cc, they have helped with PASemi
development and testing in the past, and may be able to help test this
series on PASemi hardware.

cheers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon
  2022-08-20 19:45 [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon Arminder Singh
                   ` (4 preceding siblings ...)
  2022-08-23  1:50 ` Michael Ellerman
@ 2022-08-25 12:29 ` Arnd Bergmann
  5 siblings, 0 replies; 8+ messages in thread
From: Arnd Bergmann @ 2022-08-25 12:29 UTC (permalink / raw)
  To: Arminder Singh
  Cc: linux-kernel, Sven Peter, Alyssa Rosenzweig, Hector Martin,
	Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	linux-arm-kernel, linuxppc-dev, linux-i2c

On Sat, Aug 20, 2022 at 9:45 PM Arminder Singh <arminders208@outlook.com> wrote:
>
> I also fixed a quick checkpatch warning on line 303. "i ++" is now "i++".

In general, anything that is mentioned in the changelog as "also done this"
is worth splitting into a separate patch, or dropping from the patch, to
make reviewing easier.

In this case, I would just not do the trivial change. Alternatively you
can consider doing a larger patch for coding style cleanup on the
patch, as there are possibly other issues as well. Usually it's not worth
changing things unless they hurt readability.

> @@ -80,14 +81,21 @@ static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
>  {
>         int timeout = 10;
>         unsigned int status;
> +       unsigned int bitmask = SMSTA_XEN | SMSTA_MTN;
>
> +       if (smbus->use_irq) {
> +               reinit_completion(&smbus->irq_completion);
> +               reg_write(smbus, REG_IMASK, bitmask);
> +               wait_for_completion_timeout(&smbus->irq_completion, msecs_to_jiffies(10));
>                 status = reg_read(smbus, REG_SMSTA);
>         }
>
> +
>         /* Got NACK? */
>         if (status & SMSTA_MTN)
>                 return -ENXIO;
...
> @@ -356,3 +366,12 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
>
>         return 0;
>  }
> +
> +irqreturn_t pasemi_irq_handler(int irq, void *dev_id)
> +{
> +       struct pasemi_smbus *smbus = (struct pasemi_smbus *)dev_id;
> +
> +       reg_write(smbus, REG_IMASK, 0);
> +       complete(&smbus->irq_completion);
> +       return IRQ_HANDLED;
> +}

I think the completion structure gets out of sync if you run into a
timeout here,
so a subsequent wait_for_completion will never complete after we missed one
interrupt.

Since this already causes a bus reset, I think you can just do
reinit_completion() at the end of pasemi_reset() to avoid this.

        Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon
  2022-08-23  1:50 ` Michael Ellerman
@ 2022-08-25 14:45   ` Christian Zigotzky
  0 siblings, 0 replies; 8+ messages in thread
From: Christian Zigotzky @ 2022-08-25 14:45 UTC (permalink / raw)
  To: Michael Ellerman, Arminder Singh, linux-kernel
  Cc: Sven Peter, Alyssa Rosenzweig, Hector Martin,
	Benjamin Herrenschmidt, Paul Mackerras, linux-arm-kernel,
	linuxppc-dev, linux-i2c, Darren Stevens, R.T.Dickinson,
	Christian Zigotzky

On 23 August 2022 at 03:50 am, Michael Ellerman wrote:
> Arminder Singh <arminders208@outlook.com> writes:
>> This is the first time I'm interacting with the Linux mailing lists, so
>> please don't eviscerate me *too much* if I get the formatting wrong.
>> Of course I'm always willing to take criticism and improve my formatting
>> in the future.
>>
>> This patch adds support for IRQs to the PASemi I2C controller driver.
>> This will allow for faster performing I2C transactions on Apple Silicon
>> hardware, as previously, the driver was forced to poll the SMSTA register
>> for a set amount of time.
>>
>> With this patchset the driver on Apple silicon hardware will instead wait
>> for an interrupt which will signal the completion of the I2C transaction.
>> The timeout value for this completion will be the same as the current
>> amount of time the I2C driver polls for.
>>
>> This will result in some performance improvement since the driver will be
>> waiting for less time than it does right now on Apple Silicon hardware.
>>
>> The patch right now will only enable IRQs for Apple Silicon I2C chips,
>> and only if it's able to successfully request the IRQ from the kernel.
>>
>> === Testing ===
>>
>> This patch has been tested on both the mainline Linux kernel tree and
>> the Asahi branch (https://github.com/AsahiLinux/linux.git) on both an
>> M1 and M2 MacBook Air, and it compiles successfully as both a module and
>> built-in to the kernel itself. The patch in both trees successfully boots
>> to userspace without any hitch.
>>
>> I do not have PASemi hardware on hand unfortunately, so I'm unable to test
>> the impact of this patch on old PASemi hardware. This is also why I've
>> elected to do the IRQ request and enablement on the Apple platform driver
>> and not in the common file, as I'm not sure if PASemi hardware supports
>> IRQs.
> I've added Darren and Christian to Cc, they have helped with PASemi
> development and testing in the past, and may be able to help test this
> series on PASemi hardware.
>
> cheers

Tested-by: Christian Zigotzky <chzigotzky at xenosoft.de>

on an A-EON AmigaOne X1000 with a PASemi PWRficient PA6T-1682 processor.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2022-08-25 14:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-20 19:45 [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon Arminder Singh
2022-08-20 21:39 ` kernel test robot
2022-08-21  7:24 ` Christophe Leroy
2022-08-21  9:55 ` Sven Peter
2022-08-22  8:44 ` Dan Carpenter
2022-08-23  1:50 ` Michael Ellerman
2022-08-25 14:45   ` Christian Zigotzky
2022-08-25 12:29 ` Arnd Bergmann

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).