Re: [PATCH v2 0/9] iommu: Convert dart & iommufd to the new domain_alloc_paging()

2023-10-26 Thread Sven Peter


> 
> On 26. Oct 2023, at 09:49, Joerg Roedel  wrote:
> 
> On Wed, Sep 27, 2023 at 08:47:30PM -0300, Jason Gunthorpe wrote:
>> Jason Gunthorpe (9):
>>  iommu: Move IOMMU_DOMAIN_BLOCKED global statics to ops->blocked_domain
>>  iommu/vt-d: Update the definition of the blocking domain
>>  iommu/vt-d: Use ops->blocked_domain
>>  iommufd: Convert to alloc_domain_paging()
> 
> Applied these 4, the dart patches need reviewed-by/tested-by/acked-by
> from one of the Dart maintainers.

Sorry, must’ve missed the series. I took a brief look and this all looks good 
to me. Given that Janne already reviewed it in detail:

Acked-by: Sven Peter  


Thanks,


Sven


> Regards,
> 
>Joerg



Re: [PATCH] i2c: pasemi: Add IRQ support for Apple Silicon

2022-08-21 Thread Sven Peter
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 
> ---
>  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_SMSTA0x14
> +#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);
> + comp

Re: [PATCH v2] i2c/pasemi: PASemi I2C controller IRQ enablement

2022-10-02 Thread Sven Peter
Hi,

Looks almost good to me, just a few minor things:

On Sun, Oct 2, 2022, at 00:25, Arminder Singh wrote:
> Hello,
>
> This is v2 of the PASemi I2C controller IRQ enablement patch.

This shouldn't be inside the commit description.

>
> This patch adds IRQ support to the PASemi I2C controller driver to 
> increase the performace of I2C transactions on platforms with PASemi I2C 
> controllers. While the patch is primarily intended for Apple silicon 
> platforms, this patch should also help in enabling IRQ support for 
> older PASemi hardware as well should the need arise.

This is probably the only paragraph that should be the entire commit 
description.

>
> This version of the patch has been tested on an M1 Ultra Mac Studio,
> as well as an M1 MacBook Pro, and userspace launches successfully
> while using the IRQ path for I2C transactions.
>
> Tested-by: Arminder Singh 

I think it's usually implied that you tested your own patches ;)

> Signed-off-by: Arminder Singh 
> ---
> Changes from v1:
>  - moved completion setup from pasemi_platform_i2c_probe to
>pasemi_i2c_common_probe to allow PASemi and Apple platforms to share
>common completion setup code in case PASemi hardware gets IRQ support
>added
>  - initialized the status variable in pasemi_smb_waitready when going down
>the non-IRQ path
>  - removed an unnecessary cast of dev_id in the IRQ handler
>  - fixed alignment of struct member names in i2c-pasemi-core.h
>(addresses Christophe's feedback in the original submission)
>  - IRQs are now disabled after the wait_for_completion_timeout call
>instead of inside the IRQ handler
>(prevents the IRQ from going off after the completion times out)
>  - changed the request_irq call to a devm_request_irq call to obviate
>the need for a remove function and a free_irq call
>(thanks to Sven for pointing this out in the original submission)
>  - added a reinit_completion call to pasemi_reset 
>as a failsafe to prevent missed interrupts from causing the completion
>to never complete (thanks to Arnd Bergmann for pointing this out)
>  - removed the bitmask variable in favor of just using the value
>directly (it wasn't used anywhere else)
>
> v1 linked here: 
> https://lore.kernel.org/linux-i2c/mn2pr01mb535838492432c910f2381f929f...@mn2pr01mb5358.prod.exchangelabs.com/T/#m11b3504c2667517aad7521514c99ca0e07a9381f
>
> Thanks for all the feedback on the previous submission, I'm sorry
> I wasn't able to answer everyone's emails, was just pretty busy, I'll
> make sure to be more responsive this time around! Also wasn't sure whether
> the v1 changelog belonged before or after the '---' so I put it after
> to keep the commit changelog short and concise.
> (This is just one patch, didn't think it needed a cover letter)
>
>  drivers/i2c/busses/i2c-pasemi-core.c | 29 
>  drivers/i2c/busses/i2c-pasemi-core.h |  7 +-
>  drivers/i2c/busses/i2c-pasemi-platform.c |  6 +
>  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..05af8f3575bc 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_SMSTA0x14
> +#define REG_IMASK   0x18

This doesn't seem to be aligned correctly, this file seems to use a tab
to separate the register name and the offset and you used spaces here.

>  #define REG_CTL  0x1c
>  #define REG_REV  0x28
> 
> @@ -66,6 +67,7 @@ static void pasemi_reset(struct pasemi_smbus *smbus)
>   val |= CTL_EN;
> 
>   reg_write(smbus, REG_CTL, val);
> + reinit_completion(&smbus->irq_completion);
>  }
> 
>  static void pasemi_smb_clear(struct pasemi_smbus *smbus)
> @@ -81,11 +83,18 @@ static int pasemi_smb_waitready(struct pasemi_smbus 
> *smbus)
>   int timeout = 10;
>   unsigned int status;
> 
> - 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, SMSTA_XEN | SMSTA_MTN);
> + wait_for_completion_timeout(&smbus->irq_completion, 
> msecs_to_jiffies(10));
> + reg_write(smbus, REG_IMASK, 0);
>   status = reg_read(smbus, REG_SMSTA);
> + } else {
> + status = reg_read(smbus, REG_SMSTA);
> + while (!(status & SMSTA_XEN) && timeout--) {
> + msleep(1);
> + status = reg_read(smbus, REG_SMSTA);
> + }
>   }
> 
>   /* Got NACK? */
> @@ -344,10 +353,14 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
> 
>   /* set up the sysfs linkage to our parent device */
>   smbus->adapter.dev.parent = smbus->dev;
> + smbu

Re: [PATCH v2] i2c-pasemi: PASemi I2C controller IRQ enablement

2022-10-02 Thread Sven Peter
Hi,


On Sun, Oct 2, 2022, at 16:07, Arminder Singh wrote:
> Hi,
>
>>  #define REG_MTXFIFO 0x00
>>  #define REG_MRXFIFO 0x04
>>  #define REG_SMSTA   0x14
>> +#define REG_IMASK   0x18
>
>> This doesn't seem to be aligned correctly, this file seems to use a tab
>> to separate the register name and the offset and you used spaces here.
>
>> @@ -15,7 +16,11 @@ struct pasemi_smbus {
>>  struct i2c_adapter   adapter;
>>  void __iomem*ioaddr;
>>  unsigned int clk_div;
>> -int  hw_rev;
>> +int  hw_rev;
>> +int  use_irq;
>> +struct completionirq_completion;
>
>> This doesn't seem to be aligned correctly and the hw_rev line
>> doesn't have to be changed.
>
> I'm sorry for the alignment issues in the patch, I genuinely didn't notice
> them as from the perspective of my primary editor (Visual Studio Code)
> the entries were aligned. I just saw them when opening the files in
> nano.

No worries, it's just a small nit and quickly fixed after all! :)

>
> Does fixing the alignment issues and the commit description justify a v3
> of the patch or should the minor fixes go out as a "resend"? Just not sure
> in this particular case as the fixes seem to be very minor ones.

I'd send a v3. I've only used resend when e.g. my previous mail provider
messed up and silently converted all my outgoing mails to HTML.


Best,

Sven


Re: [PATCH v3] i2c/pasemi: PASemi I2C controller IRQ enablement

2022-10-08 Thread Sven Peter
Hi,

> On 7. Oct 2022, at 02:43, Arminder Singh  wrote:
> 
> This patch adds IRQ support to the PASemi I2C controller driver to 
> increase the performace of I2C transactions on platforms with PASemi I2C 
> controllers. While primarily intended for Apple silicon platforms, this 
> patch should also help in enabling IRQ support for older PASemi hardware 
> as well should the need arise.
> 
> Signed-off-by: Arminder Singh 
> ---
> This version of the patch has been tested on an M1 Ultra Mac Studio,
> as well as an M1 MacBook Pro, and userspace launches successfully
> while using the IRQ path for I2C transactions.

I think Wolfram suggested to keep this in the commit message. If in doubt 
listen to him and not me because he’s much more experienced with the kernel 
than I am ;)

> 
> This version of the patch only contains fixes to the whitespace and
> alignment issues found in v2 of the patch, and as such the testing that
> Christian Zigotsky did on PASemi hardware for v2 of the patch also applies
> to this version of the patch as well.
> (See v2 patch email thread for the "Tested-by" tag)

You can just collect and keep those tags above your signed off by if you only 
change things like whitespaces.

> 
> v2 to v3 changes:
> - Fixed some whitespace and alignment issues found in v2 of the patch
> 
> v1 to v2 changes:
> - moved completion setup from pasemi_platform_i2c_probe to
>   pasemi_i2c_common_probe to allow PASemi and Apple platforms to share
>   common completion setup code in case PASemi hardware gets IRQ support
>   added
> - initialized the status variable in pasemi_smb_waitready when going down
>   the non-IRQ path
> - removed an unnecessary cast of dev_id in the IRQ handler
> - fixed alignment of struct member names in i2c-pasemi-core.h
>   (addresses Christophe's feedback in the original submission)
> - IRQs are now disabled after the wait_for_completion_timeout call
>   instead of inside the IRQ handler
>   (prevents the IRQ from going off after the completion times out)
> - changed the request_irq call to a devm_request_irq call to obviate
>   the need for a remove function and a free_irq call
>   (thanks to Sven for pointing this out in the original submission)
> - added a reinit_completion call to pasemi_reset 
>   as a failsafe to prevent missed interrupts from causing the completion
>   to never complete (thanks to Arnd Bergmann for pointing this out)
> - removed the bitmask variable in favor of just using the value
>   directly (it wasn't used anywhere else)
> 
> v2 linked here: 
> https://lore.kernel.org/linux-i2c/mn2pr01mb535821c8058c7814b2f8eedf9f...@mn2pr01mb5358.prod.exchangelabs.com/
> v1 linked here: 
> https://lore.kernel.org/linux-i2c/mn2pr01mb535838492432c910f2381f929f...@mn2pr01mb5358.prod.exchangelabs.com/T/#m11b3504c2667517aad7521514c99ca0e07a9381f
> 
> Hopefully the patch is good to go this time around!
> 
> drivers/i2c/busses/i2c-pasemi-core.c | 29 
> drivers/i2c/busses/i2c-pasemi-core.h |  5 
> drivers/i2c/busses/i2c-pasemi-platform.c |  6 +
> 3 files changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
> b/drivers/i2c/busses/i2c-pasemi-core.c
> index 9028ffb58cc0..4855144b370e 100644
> --- a/drivers/i2c/busses/i2c-pasemi-core.c
> +++ b/drivers/i2c/busses/i2c-pasemi-core.c
> @@ -21,6 +21,7 @@
> #define REG_MTXFIFO0x00
> #define REG_MRXFIFO0x04
> #define REG_SMSTA0x14
> +#define REG_IMASK0x18
> #define REG_CTL0x1c
> #define REG_REV0x28
> 
> @@ -66,6 +67,7 @@ static void pasemi_reset(struct pasemi_smbus *smbus)
>val |= CTL_EN;
> 
>reg_write(smbus, REG_CTL, val);
> +reinit_completion(&smbus->irq_completion);
> }
> 
> static void pasemi_smb_clear(struct pasemi_smbus *smbus)
> @@ -81,11 +83,18 @@ static int pasemi_smb_waitready(struct pasemi_smbus 
> *smbus)
>int timeout = 10;
>unsigned int status;
> 
> -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, SMSTA_XEN | SMSTA_MTN);
> +wait_for_completion_timeout(&smbus->irq_completion, 
> msecs_to_jiffies(10));
> +reg_write(smbus, REG_IMASK, 0);
>status = reg_read(smbus, REG_SMSTA);
> +} else {
> +status = reg_read(smbus, REG_SMSTA);
> +while (!(status & SMSTA_XEN) && timeout--) {
> +msleep(1);
> +status = reg_read(smbus, REG_SMSTA);
> +}
>}
> 
>/* Got NACK? */
> @@ -344,10 +353,14 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
> 
>/* set up the sysfs linkage to our parent device */
>smbus->adapter.dev.parent = smbus->dev;
> +smbus->use_irq = 0;
> +init_completion(&smbus->irq_completion);
> 
>if (smbus->hw_rev != PASEMI_HW_REV_PCI)
>smbus->hw_rev = reg_read(smbus, REG_REV);
> 
> +re

Re: [PATCH] i2c: pasemi: split driver into two separate modules

2024-02-13 Thread Sven Peter


> 
> On 12. Feb 2024, at 12:19, Arnd Bergmann  wrote:
> 
> From: Arnd Bergmann 
> 
> On powerpc, it is possible to compile test both the new apple (arm) and
> old pasemi (powerpc) drivers for the i2c hardware at the same time,
> which leads to a warning about linking the same object file twice:
> 
> scripts/Makefile.build:244: drivers/i2c/busses/Makefile: i2c-pasemi-core.o is 
> added to multiple modules: i2c-apple i2c-pasemi
> 
> Rework the driver to have an explicit helper module, letting Kbuild
> take care of whether this should be built-in or a loadable driver.
> 
> Fixes: 9bc5f4f660ff ("i2c: pasemi: Split pci driver to its own file")
> Signed-off-by: Arnd Bergmann 
> ---

Reviewed-by: Sven Peter 

thanks, totally forgot about this!


Sven


> drivers/i2c/busses/Makefile  | 6 ++
> drivers/i2c/busses/i2c-pasemi-core.c | 6 ++
> 2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 3757b9391e60..aa0ee8ecd6f2 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -90,10 +90,8 @@ obj-$(CONFIG_I2C_NPCM)+= i2c-npcm7xx.o
> obj-$(CONFIG_I2C_OCORES)+= i2c-ocores.o
> obj-$(CONFIG_I2C_OMAP)+= i2c-omap.o
> obj-$(CONFIG_I2C_OWL)+= i2c-owl.o
> -i2c-pasemi-objs := i2c-pasemi-core.o i2c-pasemi-pci.o
> -obj-$(CONFIG_I2C_PASEMI)+= i2c-pasemi.o
> -i2c-apple-objs := i2c-pasemi-core.o i2c-pasemi-platform.o
> -obj-$(CONFIG_I2C_APPLE)+= i2c-apple.o
> +obj-$(CONFIG_I2C_PASEMI)+= i2c-pasemi-core.o i2c-pasemi-pci.o
> +obj-$(CONFIG_I2C_APPLE)+= i2c-pasemi-core.o i2c-pasemi-platform.o
> obj-$(CONFIG_I2C_PCA_PLATFORM)+= i2c-pca-platform.o
> obj-$(CONFIG_I2C_PNX)+= i2c-pnx.o
> obj-$(CONFIG_I2C_PXA)+= i2c-pxa.o
> diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
> b/drivers/i2c/busses/i2c-pasemi-core.c
> index 7d54a9f34c74..bd8becbdeeb2 100644
> --- a/drivers/i2c/busses/i2c-pasemi-core.c
> +++ b/drivers/i2c/busses/i2c-pasemi-core.c
> @@ -369,6 +369,7 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
> 
>   return 0;
> }
> +EXPORT_SYMBOL_GPL(pasemi_i2c_common_probe);
> 
> irqreturn_t pasemi_irq_handler(int irq, void *dev_id)
> {
> @@ -378,3 +379,8 @@ irqreturn_t pasemi_irq_handler(int irq, void *dev_id)
>   complete(&smbus->irq_completion);
>   return IRQ_HANDLED;
> }
> +EXPORT_SYMBOL_GPL(pasemi_irq_handler);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Olof Johansson ");
> +MODULE_DESCRIPTION("PA Semi PWRficient SMBus driver");
> --
> 2.39.2



Re: [PATCH v4] i2c/pasemi: PASemi I2C controller IRQ enablement

2022-11-06 Thread Sven Peter
Hi,

On Sat, Nov 5, 2022, at 12:56, Arminder Singh wrote:
> This patch adds IRQ support to the PASemi I2C controller driver to
> increase the performace of I2C transactions on platforms with PASemi I2C
> controllers. While primarily intended for Apple silicon platforms, this
> patch should also help in enabling IRQ support for older PASemi hardware
> as well should the need arise.
>
> This version of the patch has been tested on an M1 Ultra Mac Studio,
> as well as an M1 MacBook Pro, and userspace launches successfully
> while using the IRQ path for I2C transactions.
>
> Signed-off-by: Arminder Singh 

Thanks for following up on this! This looks good to me now.

Reviewed-by: Sven Peter 



Best,


Sven


[PATCH 00/10] Add Apple M1 support to PASemi i2c driver

2021-09-26 Thread Sven Peter
Hi,

This series adds support for the I2C controller found on Apple Silicon Macs
which has quite a bit of history:

Apple bought P.A. Semi in 2008 and it looks like a part of its legacy continues
to live on in the M1. This controller has actually been used since at least the
iPhone 4S and hasn't changed much since then.
Essentially, there are only a few differences that matter:

- The controller no longer is a PCI device
- Starting at some iPhone an additional bit in one register
  must be set in order to start transmissions.
- The reference clock and hence the clock dividers are different

In order to add support for a platform device I first replaced PCI-specific
bits and split out the PCI driver to its own file. Then I added support
to make the clock divider configurable and converted the driver to use
managed device resources to make it a bit simpler.

The Apple and PASemi driver will never be compiled in the same kernel
since the Apple one will run on arm64 while the original PASemi driver
will only be useful on powerpc.
I've thus followed the octeon (mips)/thunderx(arm64) approach to do the
split: I created a -core.c file which contains the shared logic and just
compile that one for both the PASemi and the new Apple driver.

Now unfortunately I don't have access to any old PASemi hardware and
cannot confirm that my changes haven't broken anything for those.
I believe Hector was in contact with Olof a few months ago who
said that he might still have an old machine on which he could
test this.
I'd very much appreciate if he (or anyone else for that matter :-)) 
could give this series a quick test on the old PASemi machines.


Best,

Sven

Sven Peter (10):
  dt-bindings: i2c: Add Apple I2C controller bindings
  i2c: pasemi: Use io{read,write}32
  i2c: pasemi: Remove usage of pci_dev
  i2c: pasemi: Split off common probing code
  i2c: pasemi: Split pci driver to its own file
  i2c: pasemi: Move common reset code to own function
  i2c: pasemi: Allow to configure bus frequency
  i2c: pasemi: Refactor _probe to use devm_*
  i2c: pasemi: Add Apple platform driver
  i2c: pasemi: Set enable bit for Apple variant

 .../devicetree/bindings/i2c/apple,i2c.yaml|  61 +
 MAINTAINERS   |   2 +
 drivers/i2c/busses/Kconfig|  11 ++
 drivers/i2c/busses/Makefile   |   3 +
 drivers/i2c/busses/i2c-pasemi-apple.c | 122 ++
 .../{i2c-pasemi.c => i2c-pasemi-core.c}   | 114 +---
 drivers/i2c/busses/i2c-pasemi-core.h  |  21 +++
 drivers/i2c/busses/i2c-pasemi-pci.c   |  85 
 8 files changed, 334 insertions(+), 85 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/apple,i2c.yaml
 create mode 100644 drivers/i2c/busses/i2c-pasemi-apple.c
 rename drivers/i2c/busses/{i2c-pasemi.c => i2c-pasemi-core.c} (77%)
 create mode 100644 drivers/i2c/busses/i2c-pasemi-core.h
 create mode 100644 drivers/i2c/busses/i2c-pasemi-pci.c

-- 
2.25.1



[PATCH 10/10] i2c: pasemi: Set enable bit for Apple variant

2021-09-26 Thread Sven Peter
Some later revisions after the original PASemi I2C controller introduce
what likely is an enable bit to the CTL register. Without setting it the
actual i2c transmission is never started.

Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 8 
 drivers/i2c/busses/i2c-pasemi-core.h | 3 +++
 drivers/i2c/busses/i2c-pasemi-pci.c  | 6 ++
 3 files changed, 17 insertions(+)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 0ec65263fd08..b52a65beda99 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -22,6 +22,7 @@
 #define REG_MRXFIFO0x04
 #define REG_SMSTA  0x14
 #define REG_CTL0x1c
+#define REG_REV0x28
 
 /* Register defs */
 #define MTXFIFO_READ   0x0400
@@ -37,6 +38,7 @@
 
 #define CTL_MRR0x0400
 #define CTL_MTR0x0200
+#define CTL_EN 0x0800
 #define CTL_CLK_M  0x00ff
 
 static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
@@ -60,6 +62,9 @@ static void pasemi_reset(struct pasemi_smbus *smbus)
 {
u32 val = (CTL_MTR | CTL_MRR | (smbus->clk_div & CTL_CLK_M));
 
+   if (smbus->hw_rev >= 6)
+   val |= CTL_EN;
+
reg_write(smbus, REG_CTL, val);
 }
 
@@ -335,6 +340,9 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
/* set up the sysfs linkage to our parent device */
smbus->adapter.dev.parent = smbus->dev;
 
+   if (smbus->hw_rev != PASEMI_HW_REV_PCI)
+   smbus->hw_rev = reg_read(smbus, REG_REV);
+
pasemi_reset(smbus);
 
error = devm_i2c_add_adapter(smbus->dev, &smbus->adapter);
diff --git a/drivers/i2c/busses/i2c-pasemi-core.h 
b/drivers/i2c/busses/i2c-pasemi-core.h
index aca4e2da9089..4655124a37f3 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.h
+++ b/drivers/i2c/busses/i2c-pasemi-core.h
@@ -8,11 +8,14 @@
 #include 
 #include 
 
+#define PASEMI_HW_REV_PCI -1
+
 struct pasemi_smbus {
struct device   *dev;
struct i2c_adapter   adapter;
void __iomem*ioaddr;
unsigned int clk_div;
+   int  hw_rev;
 };
 
 int pasemi_i2c_common_probe(struct pasemi_smbus *smbus);
diff --git a/drivers/i2c/busses/i2c-pasemi-pci.c 
b/drivers/i2c/busses/i2c-pasemi-pci.c
index c1b8901110c0..2b7be35421bc 100644
--- a/drivers/i2c/busses/i2c-pasemi-pci.c
+++ b/drivers/i2c/busses/i2c-pasemi-pci.c
@@ -42,6 +42,12 @@ static int pasemi_smb_pci_probe(struct pci_dev *dev,
size = pci_resource_len(dev, 0);
smbus->clk_div = CLK_100K_DIV;
 
+   /*
+* The original PASemi PCI controllers don't have a register for
+* their HW revision.
+*/
+   smbus->hw_rev = PASEMI_HW_REV_PCI;
+
if (!devm_request_region(&dev->dev, base, size,
pasemi_smb_pci_driver.name))
return -EBUSY;
-- 
2.25.1



[PATCH 06/10] i2c: pasemi: Move common reset code to own function

2021-09-26 Thread Sven Peter
Split out common reset call to its own function so that we
can later add support for selecting the clock frequency
and an additional enable bit found in newer revisions.

Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 3eda5e375fa2..7c6715f5dbb8 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -61,6 +61,12 @@ static inline int reg_read(struct pasemi_smbus *smbus, int 
reg)
 #define TXFIFO_WR(smbus, reg)  reg_write((smbus), REG_MTXFIFO, (reg))
 #define RXFIFO_RD(smbus)   reg_read((smbus), REG_MRXFIFO)
 
+static void pasemi_reset(struct pasemi_smbus *smbus)
+{
+   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
+ (CLK_100K_DIV & CTL_CLK_M)));
+}
+
 static void pasemi_smb_clear(struct pasemi_smbus *smbus)
 {
unsigned int status;
@@ -135,8 +141,7 @@ static int pasemi_i2c_xfer_msg(struct i2c_adapter *adapter,
return 0;
 
  reset_out:
-   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
- (CLK_100K_DIV & CTL_CLK_M)));
+   pasemi_reset(smbus);
return err;
 }
 
@@ -302,8 +307,7 @@ static int pasemi_smb_xfer(struct i2c_adapter *adapter,
return 0;
 
  reset_out:
-   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
- (CLK_100K_DIV & CTL_CLK_M)));
+   pasemi_reset(smbus);
return err;
 }
 
@@ -335,8 +339,7 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
/* set up the sysfs linkage to our parent device */
smbus->adapter.dev.parent = smbus->dev;
 
-   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
- (CLK_100K_DIV & CTL_CLK_M)));
+   pasemi_reset(smbus);
 
error = i2c_add_adapter(&smbus->adapter);
if (error)
-- 
2.25.1



[PATCH 05/10] i2c: pasemi: Split pci driver to its own file

2021-09-26 Thread Sven Peter
Split off the PCI driver so that we can reuse common code for the
platform driver.

Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/Makefile   |  1 +
 .../{i2c-pasemi.c => i2c-pasemi-core.c}   | 88 +
 drivers/i2c/busses/i2c-pasemi-core.h  | 19 
 drivers/i2c/busses/i2c-pasemi-pci.c   | 96 +++
 4 files changed, 118 insertions(+), 86 deletions(-)
 rename drivers/i2c/busses/{i2c-pasemi.c => i2c-pasemi-core.c} (81%)
 create mode 100644 drivers/i2c/busses/i2c-pasemi-core.h
 create mode 100644 drivers/i2c/busses/i2c-pasemi-pci.c

diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 1336b04f40e2..0ab1b4cb2228 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -84,6 +84,7 @@ obj-$(CONFIG_I2C_NPCM7XX) += i2c-npcm7xx.o
 obj-$(CONFIG_I2C_OCORES)   += i2c-ocores.o
 obj-$(CONFIG_I2C_OMAP) += i2c-omap.o
 obj-$(CONFIG_I2C_OWL)  += i2c-owl.o
+i2c-pasemi-objs := i2c-pasemi-core.o i2c-pasemi-pci.o
 obj-$(CONFIG_I2C_PASEMI)   += i2c-pasemi.o
 obj-$(CONFIG_I2C_PCA_PLATFORM) += i2c-pca-platform.o
 obj-$(CONFIG_I2C_PNX)  += i2c-pnx.o
diff --git a/drivers/i2c/busses/i2c-pasemi.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
similarity index 81%
rename from drivers/i2c/busses/i2c-pasemi.c
rename to drivers/i2c/busses/i2c-pasemi-core.c
index 9d69ff63f674..3eda5e375fa2 100644
--- a/drivers/i2c/busses/i2c-pasemi.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -15,15 +15,7 @@
 #include 
 #include 
 
-static struct pci_driver pasemi_smb_driver;
-
-struct pasemi_smbus {
-   struct device   *dev;
-   struct i2c_adapter   adapter;
-   void __iomem*ioaddr;
-   unsigned longbase;
-   int  size;
-};
+#include "i2c-pasemi-core.h"
 
 /* Register offsets */
 #define REG_MTXFIFO0x00
@@ -329,7 +321,7 @@ static const struct i2c_algorithm smbus_algorithm = {
.functionality  = pasemi_smb_func,
 };
 
-static int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
+int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
 {
int error;
 
@@ -352,79 +344,3 @@ static int pasemi_i2c_common_probe(struct pasemi_smbus 
*smbus)
 
return 0;
 }
-
-static int pasemi_smb_probe(struct pci_dev *dev,
- const struct pci_device_id *id)
-{
-   struct pasemi_smbus *smbus;
-   int error;
-
-   if (!(pci_resource_flags(dev, 0) & IORESOURCE_IO))
-   return -ENODEV;
-
-   smbus = kzalloc(sizeof(struct pasemi_smbus), GFP_KERNEL);
-   if (!smbus)
-   return -ENOMEM;
-
-   smbus->dev = &dev->dev;
-   smbus->base = pci_resource_start(dev, 0);
-   smbus->size = pci_resource_len(dev, 0);
-
-   if (!request_region(smbus->base, smbus->size,
-   pasemi_smb_driver.name)) {
-   error = -EBUSY;
-   goto out_kfree;
-   }
-
-   smbus->ioaddr = ioport_map(smbus->base, smbus->size);
-   if (!smbus->ioaddr) {
-   error = -EBUSY;
-   goto out_release_region;
-   }
-
-   int error = pasemi_i2c_common_probe(smbus);
-   if (error)
-   goto out_ioport_unmap;
-
-   pci_set_drvdata(dev, smbus);
-
-   return 0;
-
- out_ioport_unmap:
-   ioport_unmap(smbus->ioaddr);
- out_release_region:
-   release_region(smbus->base, smbus->size);
- out_kfree:
-   kfree(smbus);
-   return error;
-}
-
-static void pasemi_smb_remove(struct pci_dev *dev)
-{
-   struct pasemi_smbus *smbus = pci_get_drvdata(dev);
-
-   i2c_del_adapter(&smbus->adapter);
-   ioport_unmap(smbus->ioaddr);
-   release_region(smbus->base, smbus->size);
-   kfree(smbus);
-}
-
-static const struct pci_device_id pasemi_smb_ids[] = {
-   { PCI_DEVICE(0x1959, 0xa003) },
-   { 0, }
-};
-
-MODULE_DEVICE_TABLE(pci, pasemi_smb_ids);
-
-static struct pci_driver pasemi_smb_driver = {
-   .name   = "i2c-pasemi",
-   .id_table   = pasemi_smb_ids,
-   .probe  = pasemi_smb_probe,
-   .remove = pasemi_smb_remove,
-};
-
-module_pci_driver(pasemi_smb_driver);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR ("Olof Johansson ");
-MODULE_DESCRIPTION("PA Semi PWRficient SMBus driver");
diff --git a/drivers/i2c/busses/i2c-pasemi-core.h 
b/drivers/i2c/busses/i2c-pasemi-core.h
new file mode 100644
index ..7acc33de6ce1
--- /dev/null
+++ b/drivers/i2c/busses/i2c-pasemi-core.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pasemi_smbus {
+   struct device   *dev;
+   struct i2c_adapter   adapter;
+   void __iomem*ioaddr;
+   unsigned longba

[PATCH 09/10] i2c: pasemi: Add Apple platform driver

2021-09-26 Thread Sven Peter
With all the previous preparations we can now finally add
the platform driver to support the PASemi-based controllers
in Apple SoCs. This does not work on the M1 yet but should
work on the early iPhones already.

Signed-off-by: Sven Peter 
---
 MAINTAINERS   |   1 +
 drivers/i2c/busses/Kconfig|  11 +++
 drivers/i2c/busses/Makefile   |   2 +
 drivers/i2c/busses/i2c-pasemi-apple.c | 122 ++
 4 files changed, 136 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-pasemi-apple.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 380a680db92f..6e952158b6e1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1738,6 +1738,7 @@ F:
Documentation/devicetree/bindings/i2c/apple,i2c.yaml
 F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
 F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
 F: arch/arm64/boot/dts/apple/
+F: drivers/i2c/busses/i2c-pasemi-apple.c
 F: drivers/irqchip/irq-apple-aic.c
 F: include/dt-bindings/interrupt-controller/apple-aic.h
 F: include/dt-bindings/pinctrl/apple.h
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index e17790fe35a7..cf4dae07e319 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -856,6 +856,17 @@ config I2C_PASEMI
help
  Supports the PA Semi PWRficient on-chip SMBus interfaces.
 
+config I2C_APPLE
+   tristate "Apple SMBus platform driver"
+   depends on ARCH_APPLE || COMPILE_TEST
+   default ARCH_APPLE
+   help
+ Say Y here if you want to use the I2C controller present on Apple
+ Silicon chips such as the M1.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-apple.
+
 config I2C_PCA_PLATFORM
tristate "PCA9564/PCA9665 as platform device"
select I2C_ALGOPCA
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 0ab1b4cb2228..474fe2c520d0 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -86,6 +86,8 @@ obj-$(CONFIG_I2C_OMAP)+= i2c-omap.o
 obj-$(CONFIG_I2C_OWL)  += i2c-owl.o
 i2c-pasemi-objs := i2c-pasemi-core.o i2c-pasemi-pci.o
 obj-$(CONFIG_I2C_PASEMI)   += i2c-pasemi.o
+i2c-apple-objs := i2c-pasemi-core.o i2c-pasemi-apple.o
+obj-$(CONFIG_I2C_APPLE)+= i2c-apple.o
 obj-$(CONFIG_I2C_PCA_PLATFORM) += i2c-pca-platform.o
 obj-$(CONFIG_I2C_PNX)  += i2c-pnx.o
 obj-$(CONFIG_I2C_PXA)  += i2c-pxa.o
diff --git a/drivers/i2c/busses/i2c-pasemi-apple.c 
b/drivers/i2c/busses/i2c-pasemi-apple.c
new file mode 100644
index ..c87f8e516eff
--- /dev/null
+++ b/drivers/i2c/busses/i2c-pasemi-apple.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021 The Asahi Linux Contributors
+ *
+ * PA Semi PWRficient SMBus host driver for Apple SoCs
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "i2c-pasemi-core.h"
+
+struct pasemi_apple_i2c_data {
+   struct pasemi_smbus smbus;
+   struct clk *clk_ref;
+   struct pinctrl *pctrl;
+};
+
+static int pasemi_apple_i2c_calc_clk_div(struct pasemi_apple_i2c_data *data,
+u32 frequency)
+{
+   unsigned long clk_rate = clk_get_rate(data->clk_ref);
+
+   if (!clk_rate)
+   return -EINVAL;
+
+   data->smbus.clk_div = DIV_ROUND_UP(clk_rate, 16 * frequency);
+   if (data->smbus.clk_div < 4)
+   return dev_err_probe(data->smbus.dev, -EINVAL,
+"Bus frequency %d is too fast.\n",
+frequency);
+   if (data->smbus.clk_div > 0xff)
+   return dev_err_probe(data->smbus.dev, -EINVAL,
+"Bus frequency %d is too slow.\n",
+frequency);
+
+   return 0;
+}
+
+static int pasemi_apple_i2c_probe(struct platform_device *pdev)
+{
+   struct device *dev = &pdev->dev;
+   struct pasemi_apple_i2c_data *data;
+   struct pasemi_smbus *smbus;
+   u32 frequency;
+   int error;
+
+   data = devm_kzalloc(dev, sizeof(struct pasemi_apple_i2c_data),
+   GFP_KERNEL);
+   if (!data)
+   return -ENOMEM;
+
+   smbus = &data->smbus;
+   smbus->dev = dev;
+
+   smbus->ioaddr = devm_platform_ioremap_resource(pdev, 0);
+   if (IS_ERR(smbus->ioaddr))
+   return PTR_ERR(smbus->ioaddr);
+
+   if (of_property_read_u32(dev->of_node, "clock-frequency", &frequency))
+   frequency = I2C_MAX_STANDARD_MODE_FREQ;
+
+   data->clk_ref = devm_clk_get(dev, NULL);
+   if (IS_ERR(data->clk_ref))
+   return PTR_ERR(data->c

[PATCH 04/10] i2c: pasemi: Split off common probing code

2021-09-26 Thread Sven Peter
Split off common probing code that will be used by both the PCI and the
platform device.

Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi.c | 39 +
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi.c b/drivers/i2c/busses/i2c-pasemi.c
index 5a25c2e54b9e..9d69ff63f674 100644
--- a/drivers/i2c/busses/i2c-pasemi.c
+++ b/drivers/i2c/busses/i2c-pasemi.c
@@ -329,6 +329,30 @@ static const struct i2c_algorithm smbus_algorithm = {
.functionality  = pasemi_smb_func,
 };
 
+static int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
+{
+   int error;
+
+   smbus->adapter.owner = THIS_MODULE;
+   snprintf(smbus->adapter.name, sizeof(smbus->adapter.name),
+"PA Semi SMBus adapter at 0x%lx", smbus->base);
+   smbus->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
+   smbus->adapter.algo = &smbus_algorithm;
+   smbus->adapter.algo_data = smbus;
+
+   /* set up the sysfs linkage to our parent device */
+   smbus->adapter.dev.parent = smbus->dev;
+
+   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
+ (CLK_100K_DIV & CTL_CLK_M)));
+
+   error = i2c_add_adapter(&smbus->adapter);
+   if (error)
+   return error;
+
+   return 0;
+}
+
 static int pasemi_smb_probe(struct pci_dev *dev,
  const struct pci_device_id *id)
 {
@@ -358,20 +382,7 @@ static int pasemi_smb_probe(struct pci_dev *dev,
goto out_release_region;
}
 
-   smbus->adapter.owner = THIS_MODULE;
-   snprintf(smbus->adapter.name, sizeof(smbus->adapter.name),
-"PA Semi SMBus adapter at 0x%lx", smbus->base);
-   smbus->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
-   smbus->adapter.algo = &smbus_algorithm;
-   smbus->adapter.algo_data = smbus;
-
-   /* set up the sysfs linkage to our parent device */
-   smbus->adapter.dev.parent = smbus->dev;
-
-   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
- (CLK_100K_DIV & CTL_CLK_M)));
-
-   error = i2c_add_adapter(&smbus->adapter);
+   int error = pasemi_i2c_common_probe(smbus);
if (error)
goto out_ioport_unmap;
 
-- 
2.25.1



[PATCH 08/10] i2c: pasemi: Refactor _probe to use devm_*

2021-09-26 Thread Sven Peter
Using managed device resources means there's nothing left to be done in
pasemi_smb_pci_remove and also allows to remove base and size from
struct pasemi_smbus.

Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 10 +++
 drivers/i2c/busses/i2c-pasemi-core.h |  2 --
 drivers/i2c/busses/i2c-pasemi-pci.c  | 45 
 3 files changed, 16 insertions(+), 41 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index a39e3258b162..0ec65263fd08 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -41,8 +41,7 @@
 
 static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
 {
-   dev_dbg(smbus->dev, "smbus write reg %lx val %08x\n",
-   smbus->base + reg, val);
+   dev_dbg(smbus->dev, "smbus write reg %x val %08x\n", reg, val);
iowrite32(val, smbus->ioaddr + reg);
 }
 
@@ -50,8 +49,7 @@ static inline int reg_read(struct pasemi_smbus *smbus, int 
reg)
 {
int ret;
ret = ioread32(smbus->ioaddr + reg);
-   dev_dbg(smbus->dev, "smbus read reg %lx val %08x\n",
-   smbus->base + reg, ret);
+   dev_dbg(smbus->dev, "smbus read reg %x val %08x\n", reg, ret);
return ret;
 }
 
@@ -329,7 +327,7 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
 
smbus->adapter.owner = THIS_MODULE;
snprintf(smbus->adapter.name, sizeof(smbus->adapter.name),
-"PA Semi SMBus adapter at 0x%lx", smbus->base);
+"PA Semi SMBus adapter at 0x%p", smbus->ioaddr);
smbus->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
smbus->adapter.algo = &smbus_algorithm;
smbus->adapter.algo_data = smbus;
@@ -339,7 +337,7 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
 
pasemi_reset(smbus);
 
-   error = i2c_add_adapter(&smbus->adapter);
+   error = devm_i2c_add_adapter(smbus->dev, &smbus->adapter);
if (error)
return error;
 
diff --git a/drivers/i2c/busses/i2c-pasemi-core.h 
b/drivers/i2c/busses/i2c-pasemi-core.h
index 30a7990825ef..aca4e2da9089 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.h
+++ b/drivers/i2c/busses/i2c-pasemi-core.h
@@ -12,8 +12,6 @@ struct pasemi_smbus {
struct device   *dev;
struct i2c_adapter   adapter;
void __iomem*ioaddr;
-   unsigned longbase;
-   int  size;
unsigned int clk_div;
 };
 
diff --git a/drivers/i2c/busses/i2c-pasemi-pci.c 
b/drivers/i2c/busses/i2c-pasemi-pci.c
index 7405e0b48514..c1b8901110c0 100644
--- a/drivers/i2c/busses/i2c-pasemi-pci.c
+++ b/drivers/i2c/busses/i2c-pasemi-pci.c
@@ -26,57 +26,37 @@ static int pasemi_smb_pci_probe(struct pci_dev *dev,
  const struct pci_device_id *id)
 {
struct pasemi_smbus *smbus;
+   unsigned long base;
+   int size;
int error;
 
if (!(pci_resource_flags(dev, 0) & IORESOURCE_IO))
return -ENODEV;
 
-   smbus = kzalloc(sizeof(struct pasemi_smbus), GFP_KERNEL);
+   smbus = devm_kzalloc(&dev->dev, sizeof(*smbus), GFP_KERNEL);
if (!smbus)
return -ENOMEM;
 
smbus->dev = &dev->dev;
-   smbus->base = pci_resource_start(dev, 0);
-   smbus->size = pci_resource_len(dev, 0);
+   base = pci_resource_start(dev, 0);
+   size = pci_resource_len(dev, 0);
smbus->clk_div = CLK_100K_DIV;
 
-   if (!request_region(smbus->base, smbus->size,
-   pasemi_smb_pci_driver.name)) {
-   error = -EBUSY;
-   goto out_kfree;
-   }
+   if (!devm_request_region(&dev->dev, base, size,
+   pasemi_smb_pci_driver.name))
+   return -EBUSY;
 
-   smbus->ioaddr = ioport_map(smbus->base, smbus->size);
-   if (!smbus->ioaddr) {
-   error = -EBUSY;
-   goto out_release_region;
-   }
+   smbus->ioaddr = devm_ioport_map(&dev->dev, base, size);
+   if (!smbus->ioaddr)
+   return -EBUSY;
 
error = pasemi_i2c_common_probe(smbus);
if (error)
-   goto out_ioport_unmap;
+   return error;
 
pci_set_drvdata(dev, smbus);
 
return 0;
-
- out_ioport_unmap:
-   ioport_unmap(smbus->ioaddr);
- out_release_region:
-   release_region(smbus->base, smbus->size);
- out_kfree:
-   kfree(smbus);
-   return error;
-}
-
-static void pasemi_smb_pci_remove(struct pci_dev *dev)
-{
-   struct pasemi_smbus *smbus = pci_get_drvdata(dev);
-
-   i2c_del_adapter(&smbus->adapter);
-   ioport_unmap(smbus->ioaddr)

[PATCH 02/10] i2c: pasemi: Use io{read,write}32

2021-09-26 Thread Sven Peter
In preparation for splitting this driver up into a platform_driver
and a pci_driver, replace outl/inl usage with ioport_map and
ioread32/iowrite32.

Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi.c b/drivers/i2c/busses/i2c-pasemi.c
index 20f2772c0e79..dd31d902a621 100644
--- a/drivers/i2c/busses/i2c-pasemi.c
+++ b/drivers/i2c/busses/i2c-pasemi.c
@@ -20,6 +20,7 @@ static struct pci_driver pasemi_smb_driver;
 struct pasemi_smbus {
struct pci_dev  *dev;
struct i2c_adapter   adapter;
+   void __iomem*ioaddr;
unsigned longbase;
int  size;
 };
@@ -53,13 +54,13 @@ static inline void reg_write(struct pasemi_smbus *smbus, 
int reg, int val)
 {
dev_dbg(&smbus->dev->dev, "smbus write reg %lx val %08x\n",
smbus->base + reg, val);
-   outl(val, smbus->base + reg);
+   iowrite32(val, smbus->ioaddr + reg);
 }
 
 static inline int reg_read(struct pasemi_smbus *smbus, int reg)
 {
int ret;
-   ret = inl(smbus->base + reg);
+   ret = ioread32(smbus->ioaddr + reg);
dev_dbg(&smbus->dev->dev, "smbus read reg %lx val %08x\n",
smbus->base + reg, ret);
return ret;
@@ -351,6 +352,12 @@ static int pasemi_smb_probe(struct pci_dev *dev,
goto out_kfree;
}
 
+   smbus->ioaddr = ioport_map(smbus->base, smbus->size);
+   if (!smbus->ioaddr) {
+   error = -EBUSY;
+   goto out_release_region;
+   }
+
smbus->adapter.owner = THIS_MODULE;
snprintf(smbus->adapter.name, sizeof(smbus->adapter.name),
 "PA Semi SMBus adapter at 0x%lx", smbus->base);
@@ -366,12 +373,14 @@ static int pasemi_smb_probe(struct pci_dev *dev,
 
error = i2c_add_adapter(&smbus->adapter);
if (error)
-   goto out_release_region;
+   goto out_ioport_unmap;
 
pci_set_drvdata(dev, smbus);
 
return 0;
 
+ out_ioport_unmap:
+   ioport_unmap(smbus->ioaddr);
  out_release_region:
release_region(smbus->base, smbus->size);
  out_kfree:
@@ -384,6 +393,7 @@ static void pasemi_smb_remove(struct pci_dev *dev)
struct pasemi_smbus *smbus = pci_get_drvdata(dev);
 
i2c_del_adapter(&smbus->adapter);
+   ioport_unmap(smbus->ioaddr);
release_region(smbus->base, smbus->size);
kfree(smbus);
 }
-- 
2.25.1



[PATCH 03/10] i2c: pasemi: Remove usage of pci_dev

2021-09-26 Thread Sven Peter
Prepare to create a platform driver by removing all usages of pci_dev we
can.

Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi.c b/drivers/i2c/busses/i2c-pasemi.c
index dd31d902a621..5a25c2e54b9e 100644
--- a/drivers/i2c/busses/i2c-pasemi.c
+++ b/drivers/i2c/busses/i2c-pasemi.c
@@ -18,7 +18,7 @@
 static struct pci_driver pasemi_smb_driver;
 
 struct pasemi_smbus {
-   struct pci_dev  *dev;
+   struct device   *dev;
struct i2c_adapter   adapter;
void __iomem*ioaddr;
unsigned longbase;
@@ -52,7 +52,7 @@ struct pasemi_smbus {
 
 static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
 {
-   dev_dbg(&smbus->dev->dev, "smbus write reg %lx val %08x\n",
+   dev_dbg(smbus->dev, "smbus write reg %lx val %08x\n",
smbus->base + reg, val);
iowrite32(val, smbus->ioaddr + reg);
 }
@@ -61,7 +61,7 @@ static inline int reg_read(struct pasemi_smbus *smbus, int 
reg)
 {
int ret;
ret = ioread32(smbus->ioaddr + reg);
-   dev_dbg(&smbus->dev->dev, "smbus read reg %lx val %08x\n",
+   dev_dbg(smbus->dev, "smbus read reg %lx val %08x\n",
smbus->base + reg, ret);
return ret;
 }
@@ -94,7 +94,7 @@ static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
return -ENXIO;
 
if (timeout < 0) {
-   dev_warn(&smbus->dev->dev, "Timeout, status 0x%08x\n", status);
+   dev_warn(smbus->dev, "Timeout, status 0x%08x\n", status);
reg_write(smbus, REG_SMSTA, status);
return -ETIME;
}
@@ -342,7 +342,7 @@ static int pasemi_smb_probe(struct pci_dev *dev,
if (!smbus)
return -ENOMEM;
 
-   smbus->dev = dev;
+   smbus->dev = &dev->dev;
smbus->base = pci_resource_start(dev, 0);
smbus->size = pci_resource_len(dev, 0);
 
@@ -366,7 +366,7 @@ static int pasemi_smb_probe(struct pci_dev *dev,
smbus->adapter.algo_data = smbus;
 
/* set up the sysfs linkage to our parent device */
-   smbus->adapter.dev.parent = &dev->dev;
+   smbus->adapter.dev.parent = smbus->dev;
 
reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
  (CLK_100K_DIV & CTL_CLK_M)));
-- 
2.25.1



[PATCH 01/10] dt-bindings: i2c: Add Apple I2C controller bindings

2021-09-26 Thread Sven Peter
The Apple I2C controller is based on the PASemi I2C controller.
It is present on Apple SoCs such as the M1.

Signed-off-by: Sven Peter 
---
 .../devicetree/bindings/i2c/apple,i2c.yaml| 61 +++
 MAINTAINERS   |  1 +
 2 files changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/apple,i2c.yaml

diff --git a/Documentation/devicetree/bindings/i2c/apple,i2c.yaml 
b/Documentation/devicetree/bindings/i2c/apple,i2c.yaml
new file mode 100644
index ..22fc8483256f
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/apple,i2c.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/i2c/apple,i2c.yaml#";
+$schema: "http://devicetree.org/meta-schemas/core.yaml#";
+
+title: Apple/PASemi I2C controller
+
+maintainers:
+  - Sven Peter 
+
+description: |
+  Apple SoCs such as the M1 come with a I2C controller based on the one found
+  in machines with P. A. Semi's PWRficient processors.
+  The bus is used to communicate with e.g. USB PD chips or the speaker
+  amp.
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+enum:
+  - apple,t8103-i2c
+  - apple,i2c
+
+  reg:
+maxItems: 1
+
+  clocks:
+items:
+  - description: I2C bus reference clock
+
+  interrupts:
+maxItems: 1
+
+  clock-frequency:
+description: |
+  Desired I2C bus clock frequency in Hz. If not specified, 100 kHz will be
+  used. This frequency is generated by dividing the reference clock.
+  Allowed values are between ref_clk/(16*4) and ref_clk/(16*255).
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+i2c@3501 {
+  compatible = "apple,t8103-i2c";
+  reg = <0x3501 0x4000>;
+  interrupt-parent = <&aic>;
+  interrupts = <0 627 4>;
+  clocks = <&ref_clk>;
+  #address-cells = <1>;
+  #size-cells = <0>;
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 329d3a0a9fdb..380a680db92f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1734,6 +1734,7 @@ B:https://github.com/AsahiLinux/linux/issues
 C: irc://irc.oftc.net/asahi-dev
 T: git https://github.com/AsahiLinux/linux.git
 F: Documentation/devicetree/bindings/arm/apple.yaml
+F: Documentation/devicetree/bindings/i2c/apple,i2c.yaml
 F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
 F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
 F: arch/arm64/boot/dts/apple/
-- 
2.25.1



[PATCH 07/10] i2c: pasemi: Allow to configure bus frequency

2021-09-26 Thread Sven Peter
Right now the bus frequency has always been hardcoded as
100 KHz with the specific reference clock used in the PASemi
PCI controllers. Make this configurable to prepare for the
platform driver.

Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 8 +++-
 drivers/i2c/busses/i2c-pasemi-core.h | 1 +
 drivers/i2c/busses/i2c-pasemi-pci.c  | 4 
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 7c6715f5dbb8..a39e3258b162 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -39,9 +39,6 @@
 #define CTL_MTR0x0200
 #define CTL_CLK_M  0x00ff
 
-#define CLK_100K_DIV   84
-#define CLK_400K_DIV   21
-
 static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
 {
dev_dbg(smbus->dev, "smbus write reg %lx val %08x\n",
@@ -63,8 +60,9 @@ static inline int reg_read(struct pasemi_smbus *smbus, int 
reg)
 
 static void pasemi_reset(struct pasemi_smbus *smbus)
 {
-   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
- (CLK_100K_DIV & CTL_CLK_M)));
+   u32 val = (CTL_MTR | CTL_MRR | (smbus->clk_div & CTL_CLK_M));
+
+   reg_write(smbus, REG_CTL, val);
 }
 
 static void pasemi_smb_clear(struct pasemi_smbus *smbus)
diff --git a/drivers/i2c/busses/i2c-pasemi-core.h 
b/drivers/i2c/busses/i2c-pasemi-core.h
index 7acc33de6ce1..30a7990825ef 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.h
+++ b/drivers/i2c/busses/i2c-pasemi-core.h
@@ -14,6 +14,7 @@ struct pasemi_smbus {
void __iomem*ioaddr;
unsigned longbase;
int  size;
+   unsigned int clk_div;
 };
 
 int pasemi_i2c_common_probe(struct pasemi_smbus *smbus);
diff --git a/drivers/i2c/busses/i2c-pasemi-pci.c 
b/drivers/i2c/busses/i2c-pasemi-pci.c
index 9a19df31866b..7405e0b48514 100644
--- a/drivers/i2c/busses/i2c-pasemi-pci.c
+++ b/drivers/i2c/busses/i2c-pasemi-pci.c
@@ -17,6 +17,9 @@
 
 #include "i2c-pasemi-core.h"
 
+#define CLK_100K_DIV   84
+#define CLK_400K_DIV   21
+
 static struct pci_driver pasemi_smb_pci_driver;
 
 static int pasemi_smb_pci_probe(struct pci_dev *dev,
@@ -35,6 +38,7 @@ static int pasemi_smb_pci_probe(struct pci_dev *dev,
smbus->dev = &dev->dev;
smbus->base = pci_resource_start(dev, 0);
smbus->size = pci_resource_len(dev, 0);
+   smbus->clk_div = CLK_100K_DIV;
 
if (!request_region(smbus->base, smbus->size,
pasemi_smb_pci_driver.name)) {
-- 
2.25.1



Re: Add Apple M1 support to PASemi i2c driver

2021-09-27 Thread Sven Peter
Hi Christian,

Thanks already for volunteering to test this!

On Sun, Sep 26, 2021, at 22:27, Christian Zigotzky wrote:
> Hi Sven,
>
> I can't apply your patch 5 (i2c: pasemi: Split pci driver to its own 
> file). [1]

That's strange because it should apply cleanly. I'll double check
after to work today to see if I messed up while sending this.

>
> Error message:
>
> patching file b/drivers/i2c/busses/i2c-pasemi-core.c (renamed from 
> a/drivers/i2c/busses/i2c-pasemi.c)
> Hunk #3 FAILED at 344.
> 1 out of 3 hunks FAILED -- saving rejects to file 
> b/drivers/i2c/busses/i2c-pasemi-core.c.rej
> patching file b/drivers/i2c/busses/i2c-pasemi-core.h
> patching file b/drivers/i2c/busses/i2c-pasemi-pci.c
>
> Please post one patch with all your modifications.
>

Sure, will do that later as well!


Best,


Sven


Re: [PATCH 02/10] i2c: pasemi: Use io{read,write}32

2021-09-28 Thread Sven Peter
On Mon, Sep 27, 2021, at 09:39, Arnd Bergmann wrote:
> On Sun, Sep 26, 2021 at 12:00 PM Sven Peter  wrote:
>>
>> In preparation for splitting this driver up into a platform_driver
>> and a pci_driver, replace outl/inl usage with ioport_map and
>> ioread32/iowrite32.
>>
>> Signed-off-by: Sven Peter 
>>
>> +   smbus->ioaddr = ioport_map(smbus->base, smbus->size);
>> +   if (!smbus->ioaddr) {
>> +   error = -EBUSY;
>> +   goto out_release_region;
>> +   }
>
> While this works, I would suggest using the more regular pci_iomap()
> or pcim_iomap() helper to turn the port number into an __iomem token.

Thanks a lot for the review!

I'll replace it with pci_iomap here and then later in this series with
pcim_iomap when also switching the rest to devres.


Thanks,

Sven



Re: Add Apple M1 support to PASemi i2c driver

2021-10-03 Thread Sven Peter
Hi,


On Fri, Oct 1, 2021, at 06:47, Christian Zigotzky wrote:
> On 27 September 2021 at 07:39 am, Sven Peter wrote:
>  > Hi Christian,
>  >
>  > Thanks already for volunteering to test this!
>  >
> Hello Sven,
>
> Damian (Hypex) has successfully tested the RC3 of kernel 5.15 with your 
> modified i2c driver on his Nemo board yesterday. [1]

Thanks a lot, that's great to hear!
If he wants to I can credit him with a Tested-by tag in the commit message,
see e.g. 
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes.


Best,


Sven


Re: [PATCH 09/10] i2c: pasemi: Add Apple platform driver

2021-10-03 Thread Sven Peter
On Wed, Sep 29, 2021, at 22:33, Wolfram Sang wrote:
>>  drivers/i2c/busses/i2c-pasemi-apple.c | 122 ++
>
> Can't we name it 'i2c-pasemi-platform.c' instead? Makes more sense to me
> because the other instance is named -pci.

Sure, that's more consistent. I'll change the filename for v2.


Thanks,


Sven


Re: Add Apple M1 support to PASemi i2c driver

2021-10-04 Thread Sven Peter



On Mon, Oct 4, 2021, at 13:20, Arnd Bergmann wrote:
> On Mon, Oct 4, 2021 at 11:55 AM Wolfram Sang  wrote:
>>
>>
>> > i2c-8 i2c PA Semi SMBus adapter at 0x(ptrval) I2C 
>> > adapter
>> > i2c-9 i2c PA Semi SMBus adapter at 0x(ptrval) I2C 
>> > adapter
>> > i2c-10i2c PA Semi SMBus adapter at 0x(ptrval)  
>> >I2C adapter
>>
>> As Sven correctly switched from %lx to %p, this is intended behaviour.
>> Run 'i2cdetect' as root to see the values again.
>
> I think the address could just get removed here, as this is clearly not 
> helpful.
> port number, which is somewhat useful for identifying the device, now
> it's either the pointless string, or the virtual address that the
> device is mapped
> to, which is not helpful either and potentially leaks information about kernel
> internal structures.

Yeah, now that I'm looking at it again it doesn't make much sense to
include it there. Maybe just dev_name(smbus->dev) instead of the address?


Sven


[PATCH v2 00/11] Add Apple M1 support to PASemi i2c driver

2021-10-08 Thread Sven Peter
Hi,

v1: https://lore.kernel.org/linux-i2c/20210926095847.38261-1-s...@svenpeter.dev/

Changes for v2:
 - Added reviewed-by/acks
 - Switched from ioport_map to pci_iomap as suggested by Arnd Bergmann
 - Renamed i2c-pasemi-apple.c to i2c-pasemi-platform.c as suggested by
   Wolfram Sang
 - Replaced the ioport number in the adapter name with dev_name to be
   able to identify separate busses in e.g. i2cdetect.

I still don't have access to any old PASemi hardware but the changes from
v1 are pretty small and I expect them to still work. Would still be nice
if someone with access to such hardware could give this a quick test.


And for those who didn't see v1 the (almost) unchanged original cover letter:

This series adds support for the I2C controller found on Apple Silicon Macs
which has quite a bit of history:

Apple bought P.A. Semi in 2008 and it looks like a part of its legacy continues
to live on in the M1. This controller has actually been used since at least the
iPhone 4S and hasn't changed much since then.
Essentially, there are only a few differences that matter:

- The controller no longer is a PCI device
- Starting at some iPhone an additional bit in one register
  must be set in order to start transmissions.
- The reference clock and hence the clock dividers are different

In order to add support for a platform device I first replaced PCI-specific
bits and split out the PCI driver to its own file. Then I added support
to make the clock divider configurable and converted the driver to use
managed device resources to make it a bit simpler.

The Apple and PASemi driver will never be compiled in the same kernel
since the Apple one will run on arm64 while the original PASemi driver
will only be useful on powerpc.
I've thus followed the octeon (mips)/thunderx(arm64) approach to do the
split: I created a -core.c file which contains the shared logic and just
compile that one for both the PASemi and the new Apple driver.


Best,

Sven

Sven Peter (11):
  dt-bindings: i2c: Add Apple I2C controller bindings
  i2c: pasemi: Use io{read,write}32
  i2c: pasemi: Use dev_name instead of port number
  i2c: pasemi: Remove usage of pci_dev
  i2c: pasemi: Split off common probing code
  i2c: pasemi: Split pci driver to its own file
  i2c: pasemi: Move common reset code to own function
  i2c: pasemi: Allow to configure bus frequency
  i2c: pasemi: Refactor _probe to use devm_*
  i2c: pasemi: Add Apple platform driver
  i2c: pasemi: Set enable bit for Apple variant

 .../devicetree/bindings/i2c/apple,i2c.yaml|  61 +
 MAINTAINERS   |   2 +
 drivers/i2c/busses/Kconfig|  11 ++
 drivers/i2c/busses/Makefile   |   3 +
 .../{i2c-pasemi.c => i2c-pasemi-core.c}   | 114 +---
 drivers/i2c/busses/i2c-pasemi-core.h  |  21 +++
 drivers/i2c/busses/i2c-pasemi-pci.c   |  85 
 drivers/i2c/busses/i2c-pasemi-platform.c  | 122 ++
 8 files changed, 334 insertions(+), 85 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/apple,i2c.yaml
 rename drivers/i2c/busses/{i2c-pasemi.c => i2c-pasemi-core.c} (77%)
 create mode 100644 drivers/i2c/busses/i2c-pasemi-core.h
 create mode 100644 drivers/i2c/busses/i2c-pasemi-pci.c
 create mode 100644 drivers/i2c/busses/i2c-pasemi-platform.c

-- 
2.25.1



[PATCH v2 01/11] dt-bindings: i2c: Add Apple I2C controller bindings

2021-10-08 Thread Sven Peter
The Apple I2C controller is based on the PASemi I2C controller.
It is present on Apple SoCs such as the M1.

Reviewed-by: Arnd Bergmann 
Reviewed-by: Rob Herring 
Signed-off-by: Sven Peter 
---
v1 -> v2: no changes

 .../devicetree/bindings/i2c/apple,i2c.yaml| 61 +++
 MAINTAINERS   |  1 +
 2 files changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/apple,i2c.yaml

diff --git a/Documentation/devicetree/bindings/i2c/apple,i2c.yaml 
b/Documentation/devicetree/bindings/i2c/apple,i2c.yaml
new file mode 100644
index ..22fc8483256f
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/apple,i2c.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/i2c/apple,i2c.yaml#";
+$schema: "http://devicetree.org/meta-schemas/core.yaml#";
+
+title: Apple/PASemi I2C controller
+
+maintainers:
+  - Sven Peter 
+
+description: |
+  Apple SoCs such as the M1 come with a I2C controller based on the one found
+  in machines with P. A. Semi's PWRficient processors.
+  The bus is used to communicate with e.g. USB PD chips or the speaker
+  amp.
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+enum:
+  - apple,t8103-i2c
+  - apple,i2c
+
+  reg:
+maxItems: 1
+
+  clocks:
+items:
+  - description: I2C bus reference clock
+
+  interrupts:
+maxItems: 1
+
+  clock-frequency:
+description: |
+  Desired I2C bus clock frequency in Hz. If not specified, 100 kHz will be
+  used. This frequency is generated by dividing the reference clock.
+  Allowed values are between ref_clk/(16*4) and ref_clk/(16*255).
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+i2c@3501 {
+  compatible = "apple,t8103-i2c";
+  reg = <0x3501 0x4000>;
+  interrupt-parent = <&aic>;
+  interrupts = <0 627 4>;
+  clocks = <&ref_clk>;
+  #address-cells = <1>;
+  #size-cells = <0>;
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 7cfd63ce7122..74aa85967ca3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1735,6 +1735,7 @@ B:https://github.com/AsahiLinux/linux/issues
 C: irc://irc.oftc.net/asahi-dev
 T: git https://github.com/AsahiLinux/linux.git
 F: Documentation/devicetree/bindings/arm/apple.yaml
+F: Documentation/devicetree/bindings/i2c/apple,i2c.yaml
 F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
 F: Documentation/devicetree/bindings/pci/apple,pcie.yaml
 F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
-- 
2.25.1



[PATCH v2 02/11] i2c: pasemi: Use io{read,write}32

2021-10-08 Thread Sven Peter
In preparation for splitting this driver up into a platform_driver
and a pci_driver, replace outl/inl usage with pci_iomap and
ioread32/iowrite32.

Reviewed-by: Arnd Bergmann 
Signed-off-by: Sven Peter 
---
v1 -> v2: replaced ioport_map with pci_iomap 

 drivers/i2c/busses/i2c-pasemi.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi.c b/drivers/i2c/busses/i2c-pasemi.c
index 20f2772c0e79..39fcc584264a 100644
--- a/drivers/i2c/busses/i2c-pasemi.c
+++ b/drivers/i2c/busses/i2c-pasemi.c
@@ -20,6 +20,7 @@ static struct pci_driver pasemi_smb_driver;
 struct pasemi_smbus {
struct pci_dev  *dev;
struct i2c_adapter   adapter;
+   void __iomem*ioaddr;
unsigned longbase;
int  size;
 };
@@ -53,13 +54,13 @@ static inline void reg_write(struct pasemi_smbus *smbus, 
int reg, int val)
 {
dev_dbg(&smbus->dev->dev, "smbus write reg %lx val %08x\n",
smbus->base + reg, val);
-   outl(val, smbus->base + reg);
+   iowrite32(val, smbus->ioaddr + reg);
 }
 
 static inline int reg_read(struct pasemi_smbus *smbus, int reg)
 {
int ret;
-   ret = inl(smbus->base + reg);
+   ret = ioread32(smbus->ioaddr + reg);
dev_dbg(&smbus->dev->dev, "smbus read reg %lx val %08x\n",
smbus->base + reg, ret);
return ret;
@@ -351,6 +352,12 @@ static int pasemi_smb_probe(struct pci_dev *dev,
goto out_kfree;
}
 
+   smbus->ioaddr = pci_iomap(dev, 0, 0);
+   if (!smbus->ioaddr) {
+   error = -EBUSY;
+   goto out_release_region;
+   }
+
smbus->adapter.owner = THIS_MODULE;
snprintf(smbus->adapter.name, sizeof(smbus->adapter.name),
 "PA Semi SMBus adapter at 0x%lx", smbus->base);
@@ -366,12 +373,14 @@ static int pasemi_smb_probe(struct pci_dev *dev,
 
error = i2c_add_adapter(&smbus->adapter);
if (error)
-   goto out_release_region;
+   goto out_ioport_unmap;
 
pci_set_drvdata(dev, smbus);
 
return 0;
 
+ out_ioport_unmap:
+   pci_iounmap(dev, smbus->ioaddr);
  out_release_region:
release_region(smbus->base, smbus->size);
  out_kfree:
@@ -384,6 +393,7 @@ static void pasemi_smb_remove(struct pci_dev *dev)
struct pasemi_smbus *smbus = pci_get_drvdata(dev);
 
i2c_del_adapter(&smbus->adapter);
+   pci_iounmap(dev, smbus->ioaddr);
release_region(smbus->base, smbus->size);
kfree(smbus);
 }
-- 
2.25.1



[PATCH v2 03/11] i2c: pasemi: Use dev_name instead of port number

2021-10-08 Thread Sven Peter
Right now the i2c adapter name includes the port number which can
indirectly be used to identify the device. Replace that with dev_name
to directly identify the device and to also allow this to work correctly
once we add platform support.

Signed-off-by: Sven Peter 
---
v1 -> v2: new commit

 drivers/i2c/busses/i2c-pasemi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-pasemi.c b/drivers/i2c/busses/i2c-pasemi.c
index 39fcc584264a..ca5a86cf53f1 100644
--- a/drivers/i2c/busses/i2c-pasemi.c
+++ b/drivers/i2c/busses/i2c-pasemi.c
@@ -360,7 +360,7 @@ static int pasemi_smb_probe(struct pci_dev *dev,
 
smbus->adapter.owner = THIS_MODULE;
snprintf(smbus->adapter.name, sizeof(smbus->adapter.name),
-"PA Semi SMBus adapter at 0x%lx", smbus->base);
+"PA Semi SMBus adapter (%s)", dev_name(smbus->dev));
smbus->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
smbus->adapter.algo = &smbus_algorithm;
smbus->adapter.algo_data = smbus;
-- 
2.25.1



[PATCH v2 04/11] i2c: pasemi: Remove usage of pci_dev

2021-10-08 Thread Sven Peter
Prepare to create a platform driver by removing all usages of pci_dev we
can.

Reviewed-by: Arnd Bergmann 
Signed-off-by: Sven Peter 
---
v1 -> v2: no changes

 drivers/i2c/busses/i2c-pasemi.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi.c b/drivers/i2c/busses/i2c-pasemi.c
index ca5a86cf53f1..a018e9376023 100644
--- a/drivers/i2c/busses/i2c-pasemi.c
+++ b/drivers/i2c/busses/i2c-pasemi.c
@@ -18,7 +18,7 @@
 static struct pci_driver pasemi_smb_driver;
 
 struct pasemi_smbus {
-   struct pci_dev  *dev;
+   struct device   *dev;
struct i2c_adapter   adapter;
void __iomem*ioaddr;
unsigned longbase;
@@ -52,7 +52,7 @@ struct pasemi_smbus {
 
 static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
 {
-   dev_dbg(&smbus->dev->dev, "smbus write reg %lx val %08x\n",
+   dev_dbg(smbus->dev, "smbus write reg %lx val %08x\n",
smbus->base + reg, val);
iowrite32(val, smbus->ioaddr + reg);
 }
@@ -61,7 +61,7 @@ static inline int reg_read(struct pasemi_smbus *smbus, int 
reg)
 {
int ret;
ret = ioread32(smbus->ioaddr + reg);
-   dev_dbg(&smbus->dev->dev, "smbus read reg %lx val %08x\n",
+   dev_dbg(smbus->dev, "smbus read reg %lx val %08x\n",
smbus->base + reg, ret);
return ret;
 }
@@ -94,7 +94,7 @@ static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
return -ENXIO;
 
if (timeout < 0) {
-   dev_warn(&smbus->dev->dev, "Timeout, status 0x%08x\n", status);
+   dev_warn(smbus->dev, "Timeout, status 0x%08x\n", status);
reg_write(smbus, REG_SMSTA, status);
return -ETIME;
}
@@ -342,7 +342,7 @@ static int pasemi_smb_probe(struct pci_dev *dev,
if (!smbus)
return -ENOMEM;
 
-   smbus->dev = dev;
+   smbus->dev = &dev->dev;
smbus->base = pci_resource_start(dev, 0);
smbus->size = pci_resource_len(dev, 0);
 
@@ -366,7 +366,7 @@ static int pasemi_smb_probe(struct pci_dev *dev,
smbus->adapter.algo_data = smbus;
 
/* set up the sysfs linkage to our parent device */
-   smbus->adapter.dev.parent = &dev->dev;
+   smbus->adapter.dev.parent = smbus->dev;
 
reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
  (CLK_100K_DIV & CTL_CLK_M)));
-- 
2.25.1



[PATCH v2 05/11] i2c: pasemi: Split off common probing code

2021-10-08 Thread Sven Peter
Split off common probing code that will be used by both the PCI and the
platform device.

Reviewed-by: Arnd Bergmann 
Signed-off-by: Sven Peter 
---
v1 -> v2: no changes

 drivers/i2c/busses/i2c-pasemi.c | 39 +
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi.c b/drivers/i2c/busses/i2c-pasemi.c
index a018e9376023..baf338149673 100644
--- a/drivers/i2c/busses/i2c-pasemi.c
+++ b/drivers/i2c/busses/i2c-pasemi.c
@@ -329,6 +329,30 @@ static const struct i2c_algorithm smbus_algorithm = {
.functionality  = pasemi_smb_func,
 };
 
+static int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
+{
+   int error;
+
+   smbus->adapter.owner = THIS_MODULE;
+   snprintf(smbus->adapter.name, sizeof(smbus->adapter.name),
+"PA Semi SMBus adapter (%s)", dev_name(smbus->dev));
+   smbus->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
+   smbus->adapter.algo = &smbus_algorithm;
+   smbus->adapter.algo_data = smbus;
+
+   /* set up the sysfs linkage to our parent device */
+   smbus->adapter.dev.parent = smbus->dev;
+
+   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
+ (CLK_100K_DIV & CTL_CLK_M)));
+
+   error = i2c_add_adapter(&smbus->adapter);
+   if (error)
+   return error;
+
+   return 0;
+}
+
 static int pasemi_smb_probe(struct pci_dev *dev,
  const struct pci_device_id *id)
 {
@@ -358,20 +382,7 @@ static int pasemi_smb_probe(struct pci_dev *dev,
goto out_release_region;
}
 
-   smbus->adapter.owner = THIS_MODULE;
-   snprintf(smbus->adapter.name, sizeof(smbus->adapter.name),
-"PA Semi SMBus adapter (%s)", dev_name(smbus->dev));
-   smbus->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
-   smbus->adapter.algo = &smbus_algorithm;
-   smbus->adapter.algo_data = smbus;
-
-   /* set up the sysfs linkage to our parent device */
-   smbus->adapter.dev.parent = smbus->dev;
-
-   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
- (CLK_100K_DIV & CTL_CLK_M)));
-
-   error = i2c_add_adapter(&smbus->adapter);
+   int error = pasemi_i2c_common_probe(smbus);
if (error)
goto out_ioport_unmap;
 
-- 
2.25.1



[PATCH v2 06/11] i2c: pasemi: Split pci driver to its own file

2021-10-08 Thread Sven Peter
Split off the PCI driver so that we can reuse common code for the
platform driver.

Reviewed-by: Arnd Bergmann 
Signed-off-by: Sven Peter 
---
v1 -> v2: no changes

 drivers/i2c/busses/Makefile   |  1 +
 .../{i2c-pasemi.c => i2c-pasemi-core.c}   | 88 +
 drivers/i2c/busses/i2c-pasemi-core.h  | 19 
 drivers/i2c/busses/i2c-pasemi-pci.c   | 96 +++
 4 files changed, 118 insertions(+), 86 deletions(-)
 rename drivers/i2c/busses/{i2c-pasemi.c => i2c-pasemi-core.c} (81%)
 create mode 100644 drivers/i2c/busses/i2c-pasemi-core.h
 create mode 100644 drivers/i2c/busses/i2c-pasemi-pci.c

diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 1336b04f40e2..0ab1b4cb2228 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -84,6 +84,7 @@ obj-$(CONFIG_I2C_NPCM7XX) += i2c-npcm7xx.o
 obj-$(CONFIG_I2C_OCORES)   += i2c-ocores.o
 obj-$(CONFIG_I2C_OMAP) += i2c-omap.o
 obj-$(CONFIG_I2C_OWL)  += i2c-owl.o
+i2c-pasemi-objs := i2c-pasemi-core.o i2c-pasemi-pci.o
 obj-$(CONFIG_I2C_PASEMI)   += i2c-pasemi.o
 obj-$(CONFIG_I2C_PCA_PLATFORM) += i2c-pca-platform.o
 obj-$(CONFIG_I2C_PNX)  += i2c-pnx.o
diff --git a/drivers/i2c/busses/i2c-pasemi.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
similarity index 81%
rename from drivers/i2c/busses/i2c-pasemi.c
rename to drivers/i2c/busses/i2c-pasemi-core.c
index baf338149673..d1cab11a4d50 100644
--- a/drivers/i2c/busses/i2c-pasemi.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -15,15 +15,7 @@
 #include 
 #include 
 
-static struct pci_driver pasemi_smb_driver;
-
-struct pasemi_smbus {
-   struct device   *dev;
-   struct i2c_adapter   adapter;
-   void __iomem*ioaddr;
-   unsigned longbase;
-   int  size;
-};
+#include "i2c-pasemi-core.h"
 
 /* Register offsets */
 #define REG_MTXFIFO0x00
@@ -329,7 +321,7 @@ static const struct i2c_algorithm smbus_algorithm = {
.functionality  = pasemi_smb_func,
 };
 
-static int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
+int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
 {
int error;
 
@@ -352,79 +344,3 @@ static int pasemi_i2c_common_probe(struct pasemi_smbus 
*smbus)
 
return 0;
 }
-
-static int pasemi_smb_probe(struct pci_dev *dev,
- const struct pci_device_id *id)
-{
-   struct pasemi_smbus *smbus;
-   int error;
-
-   if (!(pci_resource_flags(dev, 0) & IORESOURCE_IO))
-   return -ENODEV;
-
-   smbus = kzalloc(sizeof(struct pasemi_smbus), GFP_KERNEL);
-   if (!smbus)
-   return -ENOMEM;
-
-   smbus->dev = &dev->dev;
-   smbus->base = pci_resource_start(dev, 0);
-   smbus->size = pci_resource_len(dev, 0);
-
-   if (!request_region(smbus->base, smbus->size,
-   pasemi_smb_driver.name)) {
-   error = -EBUSY;
-   goto out_kfree;
-   }
-
-   smbus->ioaddr = pci_iomap(dev, 0, 0);
-   if (!smbus->ioaddr) {
-   error = -EBUSY;
-   goto out_release_region;
-   }
-
-   int error = pasemi_i2c_common_probe(smbus);
-   if (error)
-   goto out_ioport_unmap;
-
-   pci_set_drvdata(dev, smbus);
-
-   return 0;
-
- out_ioport_unmap:
-   pci_iounmap(dev, smbus->ioaddr);
- out_release_region:
-   release_region(smbus->base, smbus->size);
- out_kfree:
-   kfree(smbus);
-   return error;
-}
-
-static void pasemi_smb_remove(struct pci_dev *dev)
-{
-   struct pasemi_smbus *smbus = pci_get_drvdata(dev);
-
-   i2c_del_adapter(&smbus->adapter);
-   pci_iounmap(dev, smbus->ioaddr);
-   release_region(smbus->base, smbus->size);
-   kfree(smbus);
-}
-
-static const struct pci_device_id pasemi_smb_ids[] = {
-   { PCI_DEVICE(0x1959, 0xa003) },
-   { 0, }
-};
-
-MODULE_DEVICE_TABLE(pci, pasemi_smb_ids);
-
-static struct pci_driver pasemi_smb_driver = {
-   .name   = "i2c-pasemi",
-   .id_table   = pasemi_smb_ids,
-   .probe  = pasemi_smb_probe,
-   .remove = pasemi_smb_remove,
-};
-
-module_pci_driver(pasemi_smb_driver);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR ("Olof Johansson ");
-MODULE_DESCRIPTION("PA Semi PWRficient SMBus driver");
diff --git a/drivers/i2c/busses/i2c-pasemi-core.h 
b/drivers/i2c/busses/i2c-pasemi-core.h
new file mode 100644
index ..7acc33de6ce1
--- /dev/null
+++ b/drivers/i2c/busses/i2c-pasemi-core.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pasemi_smbus {
+   struct device   *dev;
+   struct i2c_adapter   adapter;
+   void __iomem*ioad

[PATCH v2 07/11] i2c: pasemi: Move common reset code to own function

2021-10-08 Thread Sven Peter
Split out common reset call to its own function so that we
can later add support for selecting the clock frequency
and an additional enable bit found in newer revisions.

Reviewed-by: Arnd Bergmann 
Signed-off-by: Sven Peter 
---
v1 -> v2: no changes

 drivers/i2c/busses/i2c-pasemi-core.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index d1cab11a4d50..232587c70a38 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -61,6 +61,12 @@ static inline int reg_read(struct pasemi_smbus *smbus, int 
reg)
 #define TXFIFO_WR(smbus, reg)  reg_write((smbus), REG_MTXFIFO, (reg))
 #define RXFIFO_RD(smbus)   reg_read((smbus), REG_MRXFIFO)
 
+static void pasemi_reset(struct pasemi_smbus *smbus)
+{
+   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
+ (CLK_100K_DIV & CTL_CLK_M)));
+}
+
 static void pasemi_smb_clear(struct pasemi_smbus *smbus)
 {
unsigned int status;
@@ -135,8 +141,7 @@ static int pasemi_i2c_xfer_msg(struct i2c_adapter *adapter,
return 0;
 
  reset_out:
-   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
- (CLK_100K_DIV & CTL_CLK_M)));
+   pasemi_reset(smbus);
return err;
 }
 
@@ -302,8 +307,7 @@ static int pasemi_smb_xfer(struct i2c_adapter *adapter,
return 0;
 
  reset_out:
-   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
- (CLK_100K_DIV & CTL_CLK_M)));
+   pasemi_reset(smbus);
return err;
 }
 
@@ -335,8 +339,7 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
/* set up the sysfs linkage to our parent device */
smbus->adapter.dev.parent = smbus->dev;
 
-   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
- (CLK_100K_DIV & CTL_CLK_M)));
+   pasemi_reset(smbus);
 
error = i2c_add_adapter(&smbus->adapter);
if (error)
-- 
2.25.1



[PATCH v2 08/11] i2c: pasemi: Allow to configure bus frequency

2021-10-08 Thread Sven Peter
Right now the bus frequency has always been hardcoded as
100 KHz with the specific reference clock used in the PASemi
PCI controllers. Make this configurable to prepare for the
platform driver.

Reviewed-by: Arnd Bergmann 
Signed-off-by: Sven Peter 
---
v1 -> v2: no changes

 drivers/i2c/busses/i2c-pasemi-core.c | 8 +++-
 drivers/i2c/busses/i2c-pasemi-core.h | 1 +
 drivers/i2c/busses/i2c-pasemi-pci.c  | 4 
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 232587c70a38..9fb8fac53f2b 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -39,9 +39,6 @@
 #define CTL_MTR0x0200
 #define CTL_CLK_M  0x00ff
 
-#define CLK_100K_DIV   84
-#define CLK_400K_DIV   21
-
 static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
 {
dev_dbg(smbus->dev, "smbus write reg %lx val %08x\n",
@@ -63,8 +60,9 @@ static inline int reg_read(struct pasemi_smbus *smbus, int 
reg)
 
 static void pasemi_reset(struct pasemi_smbus *smbus)
 {
-   reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
- (CLK_100K_DIV & CTL_CLK_M)));
+   u32 val = (CTL_MTR | CTL_MRR | (smbus->clk_div & CTL_CLK_M));
+
+   reg_write(smbus, REG_CTL, val);
 }
 
 static void pasemi_smb_clear(struct pasemi_smbus *smbus)
diff --git a/drivers/i2c/busses/i2c-pasemi-core.h 
b/drivers/i2c/busses/i2c-pasemi-core.h
index 7acc33de6ce1..30a7990825ef 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.h
+++ b/drivers/i2c/busses/i2c-pasemi-core.h
@@ -14,6 +14,7 @@ struct pasemi_smbus {
void __iomem*ioaddr;
unsigned longbase;
int  size;
+   unsigned int clk_div;
 };
 
 int pasemi_i2c_common_probe(struct pasemi_smbus *smbus);
diff --git a/drivers/i2c/busses/i2c-pasemi-pci.c 
b/drivers/i2c/busses/i2c-pasemi-pci.c
index 644656e28012..96585bbf8c24 100644
--- a/drivers/i2c/busses/i2c-pasemi-pci.c
+++ b/drivers/i2c/busses/i2c-pasemi-pci.c
@@ -17,6 +17,9 @@
 
 #include "i2c-pasemi-core.h"
 
+#define CLK_100K_DIV   84
+#define CLK_400K_DIV   21
+
 static struct pci_driver pasemi_smb_pci_driver;
 
 static int pasemi_smb_pci_probe(struct pci_dev *dev,
@@ -35,6 +38,7 @@ static int pasemi_smb_pci_probe(struct pci_dev *dev,
smbus->dev = &dev->dev;
smbus->base = pci_resource_start(dev, 0);
smbus->size = pci_resource_len(dev, 0);
+   smbus->clk_div = CLK_100K_DIV;
 
if (!request_region(smbus->base, smbus->size,
pasemi_smb_pci_driver.name)) {
-- 
2.25.1



[PATCH v2 09/11] i2c: pasemi: Refactor _probe to use devm_*

2021-10-08 Thread Sven Peter
Using managed device resources means there's nothing left to be done in
pasemi_smb_pci_remove and also allows to remove base and size from
struct pasemi_smbus.

Reviewed-by: Arnd Bergmann 
Signed-off-by: Sven Peter 
---
v1 -> v2: no changes

 drivers/i2c/busses/i2c-pasemi-core.c |  8 ++---
 drivers/i2c/busses/i2c-pasemi-core.h |  2 --
 drivers/i2c/busses/i2c-pasemi-pci.c  | 45 
 3 files changed, 15 insertions(+), 40 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 9fb8fac53f2b..3d87b64dd9f7 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -41,8 +41,7 @@
 
 static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
 {
-   dev_dbg(smbus->dev, "smbus write reg %lx val %08x\n",
-   smbus->base + reg, val);
+   dev_dbg(smbus->dev, "smbus write reg %x val %08x\n", reg, val);
iowrite32(val, smbus->ioaddr + reg);
 }
 
@@ -50,8 +49,7 @@ static inline int reg_read(struct pasemi_smbus *smbus, int 
reg)
 {
int ret;
ret = ioread32(smbus->ioaddr + reg);
-   dev_dbg(smbus->dev, "smbus read reg %lx val %08x\n",
-   smbus->base + reg, ret);
+   dev_dbg(smbus->dev, "smbus read reg %x val %08x\n", reg, ret);
return ret;
 }
 
@@ -339,7 +337,7 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
 
pasemi_reset(smbus);
 
-   error = i2c_add_adapter(&smbus->adapter);
+   error = devm_i2c_add_adapter(smbus->dev, &smbus->adapter);
if (error)
return error;
 
diff --git a/drivers/i2c/busses/i2c-pasemi-core.h 
b/drivers/i2c/busses/i2c-pasemi-core.h
index 30a7990825ef..aca4e2da9089 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.h
+++ b/drivers/i2c/busses/i2c-pasemi-core.h
@@ -12,8 +12,6 @@ struct pasemi_smbus {
struct device   *dev;
struct i2c_adapter   adapter;
void __iomem*ioaddr;
-   unsigned longbase;
-   int  size;
unsigned int clk_div;
 };
 
diff --git a/drivers/i2c/busses/i2c-pasemi-pci.c 
b/drivers/i2c/busses/i2c-pasemi-pci.c
index 96585bbf8c24..4251e7b9f177 100644
--- a/drivers/i2c/busses/i2c-pasemi-pci.c
+++ b/drivers/i2c/busses/i2c-pasemi-pci.c
@@ -26,57 +26,37 @@ static int pasemi_smb_pci_probe(struct pci_dev *dev,
  const struct pci_device_id *id)
 {
struct pasemi_smbus *smbus;
+   unsigned long base;
+   int size;
int error;
 
if (!(pci_resource_flags(dev, 0) & IORESOURCE_IO))
return -ENODEV;
 
-   smbus = kzalloc(sizeof(struct pasemi_smbus), GFP_KERNEL);
+   smbus = devm_kzalloc(&dev->dev, sizeof(*smbus), GFP_KERNEL);
if (!smbus)
return -ENOMEM;
 
smbus->dev = &dev->dev;
-   smbus->base = pci_resource_start(dev, 0);
-   smbus->size = pci_resource_len(dev, 0);
+   base = pci_resource_start(dev, 0);
+   size = pci_resource_len(dev, 0);
smbus->clk_div = CLK_100K_DIV;
 
-   if (!request_region(smbus->base, smbus->size,
-   pasemi_smb_pci_driver.name)) {
-   error = -EBUSY;
-   goto out_kfree;
-   }
+   if (!devm_request_region(&dev->dev, base, size,
+   pasemi_smb_pci_driver.name))
+   return -EBUSY;
 
-   smbus->ioaddr = pci_iomap(dev, 0, 0);
-   if (!smbus->ioaddr) {
-   error = -EBUSY;
-   goto out_release_region;
-   }
+   smbus->ioaddr = pcim_iomap(dev, 0, 0);
+   if (!smbus->ioaddr)
+   return -EBUSY;
 
error = pasemi_i2c_common_probe(smbus);
if (error)
-   goto out_ioport_unmap;
+   return error;
 
pci_set_drvdata(dev, smbus);
 
return 0;
-
- out_ioport_unmap:
-   pci_iounmap(dev, smbus->ioaddr);
- out_release_region:
-   release_region(smbus->base, smbus->size);
- out_kfree:
-   kfree(smbus);
-   return error;
-}
-
-static void pasemi_smb_pci_remove(struct pci_dev *dev)
-{
-   struct pasemi_smbus *smbus = pci_get_drvdata(dev);
-
-   i2c_del_adapter(&smbus->adapter);
-   pci_iounmap(dev, smbus->ioaddr);
-   release_region(smbus->base, smbus->size);
-   kfree(smbus);
 }
 
 static const struct pci_device_id pasemi_smb_pci_ids[] = {
@@ -90,7 +70,6 @@ static struct pci_driver pasemi_smb_pci_driver = {
.name   = "i2c-pasemi",
.id_table   = pasemi_smb_pci_ids,
.probe  = pasemi_smb_pci_probe,
-   .remove = pasemi_smb_pci_remove,
 };
 
 module_pci_driver(pasemi_smb_pci_driver);
-- 
2.25.1



[PATCH v2 11/11] i2c: pasemi: Set enable bit for Apple variant

2021-10-08 Thread Sven Peter
Some later revisions after the original PASemi I2C controller introduce
what likely is an enable bit to the CTL register. Without setting it the
actual i2c transmission is never started.

Reviewed-by: Arnd Bergmann 
Signed-off-by: Sven Peter 
---
v1 -> v2: no changes

 drivers/i2c/busses/i2c-pasemi-core.c | 8 
 drivers/i2c/busses/i2c-pasemi-core.h | 3 +++
 drivers/i2c/busses/i2c-pasemi-pci.c  | 6 ++
 3 files changed, 17 insertions(+)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 3d87b64dd9f7..4e161a4089d8 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -22,6 +22,7 @@
 #define REG_MRXFIFO0x04
 #define REG_SMSTA  0x14
 #define REG_CTL0x1c
+#define REG_REV0x28
 
 /* Register defs */
 #define MTXFIFO_READ   0x0400
@@ -37,6 +38,7 @@
 
 #define CTL_MRR0x0400
 #define CTL_MTR0x0200
+#define CTL_EN 0x0800
 #define CTL_CLK_M  0x00ff
 
 static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
@@ -60,6 +62,9 @@ static void pasemi_reset(struct pasemi_smbus *smbus)
 {
u32 val = (CTL_MTR | CTL_MRR | (smbus->clk_div & CTL_CLK_M));
 
+   if (smbus->hw_rev >= 6)
+   val |= CTL_EN;
+
reg_write(smbus, REG_CTL, val);
 }
 
@@ -335,6 +340,9 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
/* set up the sysfs linkage to our parent device */
smbus->adapter.dev.parent = smbus->dev;
 
+   if (smbus->hw_rev != PASEMI_HW_REV_PCI)
+   smbus->hw_rev = reg_read(smbus, REG_REV);
+
pasemi_reset(smbus);
 
error = devm_i2c_add_adapter(smbus->dev, &smbus->adapter);
diff --git a/drivers/i2c/busses/i2c-pasemi-core.h 
b/drivers/i2c/busses/i2c-pasemi-core.h
index aca4e2da9089..4655124a37f3 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.h
+++ b/drivers/i2c/busses/i2c-pasemi-core.h
@@ -8,11 +8,14 @@
 #include 
 #include 
 
+#define PASEMI_HW_REV_PCI -1
+
 struct pasemi_smbus {
struct device   *dev;
struct i2c_adapter   adapter;
void __iomem*ioaddr;
unsigned int clk_div;
+   int  hw_rev;
 };
 
 int pasemi_i2c_common_probe(struct pasemi_smbus *smbus);
diff --git a/drivers/i2c/busses/i2c-pasemi-pci.c 
b/drivers/i2c/busses/i2c-pasemi-pci.c
index 4251e7b9f177..1ab1f28744fb 100644
--- a/drivers/i2c/busses/i2c-pasemi-pci.c
+++ b/drivers/i2c/busses/i2c-pasemi-pci.c
@@ -42,6 +42,12 @@ static int pasemi_smb_pci_probe(struct pci_dev *dev,
size = pci_resource_len(dev, 0);
smbus->clk_div = CLK_100K_DIV;
 
+   /*
+* The original PASemi PCI controllers don't have a register for
+* their HW revision.
+*/
+   smbus->hw_rev = PASEMI_HW_REV_PCI;
+
if (!devm_request_region(&dev->dev, base, size,
pasemi_smb_pci_driver.name))
return -EBUSY;
-- 
2.25.1



[PATCH v2 10/11] i2c: pasemi: Add Apple platform driver

2021-10-08 Thread Sven Peter
With all the previous preparations we can now finally add
the platform driver to support the PASemi-based controllers
in Apple SoCs. This does not work on the M1 yet but should
work on the early iPhones already.

Reviewed-by: Arnd Bergmann 
Signed-off-by: Sven Peter 
---
v1 -> v2:
 - renamed i2c-pasemi-apple.c to i2c-pasemi-platform.c and adjusted
   function names as well
 - removed unused struct pinctrl *pctrl which snuck into v1

 MAINTAINERS  |   1 +
 drivers/i2c/busses/Kconfig   |  11 ++
 drivers/i2c/busses/Makefile  |   2 +
 drivers/i2c/busses/i2c-pasemi-platform.c | 122 +++
 4 files changed, 136 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-pasemi-platform.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 74aa85967ca3..8e0f1dc94b5b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1740,6 +1740,7 @@ F:
Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
 F: Documentation/devicetree/bindings/pci/apple,pcie.yaml
 F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
 F: arch/arm64/boot/dts/apple/
+F: drivers/i2c/busses/i2c-pasemi-platform.c
 F: drivers/irqchip/irq-apple-aic.c
 F: include/dt-bindings/interrupt-controller/apple-aic.h
 F: include/dt-bindings/pinctrl/apple.h
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 1df19ccc310b..dce392839017 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -859,6 +859,17 @@ config I2C_PASEMI
help
  Supports the PA Semi PWRficient on-chip SMBus interfaces.
 
+config I2C_APPLE
+   tristate "Apple SMBus platform driver"
+   depends on ARCH_APPLE || COMPILE_TEST
+   default ARCH_APPLE
+   help
+ Say Y here if you want to use the I2C controller present on Apple
+ Silicon chips such as the M1.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-apple.
+
 config I2C_PCA_PLATFORM
tristate "PCA9564/PCA9665 as platform device"
select I2C_ALGOPCA
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 0ab1b4cb2228..d85899fef8c7 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -86,6 +86,8 @@ obj-$(CONFIG_I2C_OMAP)+= i2c-omap.o
 obj-$(CONFIG_I2C_OWL)  += i2c-owl.o
 i2c-pasemi-objs := i2c-pasemi-core.o i2c-pasemi-pci.o
 obj-$(CONFIG_I2C_PASEMI)   += i2c-pasemi.o
+i2c-apple-objs := i2c-pasemi-core.o i2c-pasemi-platform.o
+obj-$(CONFIG_I2C_APPLE)+= i2c-apple.o
 obj-$(CONFIG_I2C_PCA_PLATFORM) += i2c-pca-platform.o
 obj-$(CONFIG_I2C_PNX)  += i2c-pnx.o
 obj-$(CONFIG_I2C_PXA)  += i2c-pxa.o
diff --git a/drivers/i2c/busses/i2c-pasemi-platform.c 
b/drivers/i2c/busses/i2c-pasemi-platform.c
new file mode 100644
index ..88a54aaf7e3c
--- /dev/null
+++ b/drivers/i2c/busses/i2c-pasemi-platform.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021 The Asahi Linux Contributors
+ *
+ * PA Semi PWRficient SMBus host driver for Apple SoCs
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "i2c-pasemi-core.h"
+
+struct pasemi_platform_i2c_data {
+   struct pasemi_smbus smbus;
+   struct clk *clk_ref;
+};
+
+static int
+pasemi_platform_i2c_calc_clk_div(struct pasemi_platform_i2c_data *data,
+u32 frequency)
+{
+   unsigned long clk_rate = clk_get_rate(data->clk_ref);
+
+   if (!clk_rate)
+   return -EINVAL;
+
+   data->smbus.clk_div = DIV_ROUND_UP(clk_rate, 16 * frequency);
+   if (data->smbus.clk_div < 4)
+   return dev_err_probe(data->smbus.dev, -EINVAL,
+"Bus frequency %d is too fast.\n",
+frequency);
+   if (data->smbus.clk_div > 0xff)
+   return dev_err_probe(data->smbus.dev, -EINVAL,
+"Bus frequency %d is too slow.\n",
+frequency);
+
+   return 0;
+}
+
+static int pasemi_platform_i2c_probe(struct platform_device *pdev)
+{
+   struct device *dev = &pdev->dev;
+   struct pasemi_platform_i2c_data *data;
+   struct pasemi_smbus *smbus;
+   u32 frequency;
+   int error;
+
+   data = devm_kzalloc(dev, sizeof(struct pasemi_platform_i2c_data),
+   GFP_KERNEL);
+   if (!data)
+   return -ENOMEM;
+
+   smbus = &data->smbus;
+   smbus->dev = dev;
+
+   smbus->ioaddr = devm_platform_ioremap_resource(pdev, 0);
+   if (IS_ERR(smbus->ioaddr))
+   return PTR_ERR(smbus->ioaddr);
+
+   if (of_property_read_u32(dev->of_node, "clock-frequency", &frequency))
+ 

Re: [PATCH v2 10/11] i2c: pasemi: Add Apple platform driver

2021-10-09 Thread Sven Peter



On Sat, Oct 9, 2021, at 12:09, Wolfram Sang wrote:
>>  F:  arch/arm64/boot/dts/apple/
>> +F:  drivers/i2c/busses/i2c-pasemi-platform.c
>
> We have no dedicated maintainer for PASEMI. Are maybe you or your
> project interested in maintaining the pasemi-core, too? I guess not many
> patches will show up and they will likely be for M1 anyhow.
>
> If so, then no need to resend, I could add the extra line while
> applying.

Sure, feel free to add the core to the entry as well.


Best,

Sven


Re: [PATCH v2 00/11] Add Apple M1 support to PASemi i2c driver

2021-10-09 Thread Sven Peter
On Sat, Oct 9, 2021, at 12:10, Wolfram Sang wrote:
>> I still don't have access to any old PASemi hardware but the changes from
>> v1 are pretty small and I expect them to still work. Would still be nice
>> if someone with access to such hardware could give this a quick test.
>
> Looks good to me. I will wait a few more days so that people can report
> their tests. But it will be in the next merge window.

Sounds great, thanks!


Sven


Re: [PATCH v2 00/11] Add Apple M1 support to PASemi i2c driver

2021-10-10 Thread Sven Peter
On Sat, Oct 9, 2021, at 15:57, Christian Zigotzky wrote:
> On 09 October 2021 at 12:10 pm, Wolfram Sang wrote:
>>> I still don't have access to any old PASemi hardware but the changes from
>>> v1 are pretty small and I expect them to still work. Would still be nice
>>> if someone with access to such hardware could give this a quick test.
>> Looks good to me. I will wait a few more days so that people can report
>> their tests. But it will be in the next merge window.
>>
> Series v2:
>
> Tested-by: Christian Zigotzky  [1]

thanks a lot, glad to hear everything works on P.A Semi CPUs as well!

And regarding that git am issue you wrote about: I think I based this series on
torvald's tree instead of 5.15-rc4 and there have been some changes to at least
MAINTAINERS. It'll probably apply cleanly to 5.15-rc5 but if that happens again
in the future you can try

  git am -3 mbox

instead. It'll try to do a three way merge if the patch doesn't apply cleanly.


Sven



Re: [PATCH v2 37/60] i2c: pasemi: reword according to newest specification

2024-07-07 Thread Sven Peter


> 
> On 6. Jul 2024, at 13:22, Wolfram Sang  
> wrote:
> 
> Change the wording of this driver wrt. the newest I2C v7 and SMBus 3.2
> specifications and replace "master/slave" with more appropriate terms.
> 
> Signed-off-by: Wolfram Sang 
> ---

Acked-by: Sven Peter 


> drivers/i2c/busses/i2c-pasemi-core.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
> b/drivers/i2c/busses/i2c-pasemi-core.c
> index bd8becbdeeb2..dac694a9d781 100644
> --- a/drivers/i2c/busses/i2c-pasemi-core.c
> +++ b/drivers/i2c/busses/i2c-pasemi-core.c
> @@ -336,9 +336,9 @@ static u32 pasemi_smb_func(struct i2c_adapter *adapter)
> }
> 
> static const struct i2c_algorithm smbus_algorithm = {
> -.master_xfer= pasemi_i2c_xfer,
> -.smbus_xfer= pasemi_smb_xfer,
> -.functionality= pasemi_smb_func,
> +.xfer = pasemi_i2c_xfer,
> +.smbus_xfer = pasemi_smb_xfer,
> +.functionality = pasemi_smb_func,
> };
> 
> int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
> --
> 2.43.0
> 



Re: [PATCH 2/4] i2c: pasemi: Improve error recovery

2025-03-22 Thread Sven Peter
Hi Andi,

Thanks for the review! Will send a v2 after -rc1 is out.

On Thu, Mar 20, 2025, at 01:17, Andi Shyti wrote:
> Hi Sven,
>
> On Sat, Feb 22, 2025 at 01:38:34PM +0000, Sven Peter via B4 Relay wrote:
>> The hardware (supposedly) has a 25ms timeout for clock stretching
>> and the driver uses 100ms which should be plenty.
>
> Can we add this lines as a comment to the define you are adding?

Sure.

>
>> The error
>> reocvery itself is however lacking.
>
> ...
>
>> -static void pasemi_smb_clear(struct pasemi_smbus *smbus)
>> +static int pasemi_smb_clear(struct pasemi_smbus *smbus)
>>  {
>>  unsigned int status;
>> +int timeout = TRANSFER_TIMEOUT_MS;
>>  
>>  status = reg_read(smbus, REG_SMSTA);
>> +
>> +/* First wait for the bus to go idle */
>> +while ((status & (SMSTA_XIP | SMSTA_JAM)) && timeout--) {
>> +msleep(1);
>
> Please, use usleep_range for 1 millisecond timeout.

Ack.

>
>> +status = reg_read(smbus, REG_SMSTA);
>> +}
>
> You could use here readx_poll_timeout() here.

Yup, that should work.

>
>> +
>> +if (timeout < 0) {
>> +dev_warn(smbus->dev, "Bus is still stuck (status 0x%08x)\n", 
>> status);
>
> if it's an error, please use an error.

Ack.

>
>> +return -EIO;
>> +}
>> +
>> +/* If any badness happened or there is data in the FIFOs, reset the 
>> FIFOs */
>> +if ((status & (SMSTA_MRNE | SMSTA_JMD | SMSTA_MTO | SMSTA_TOM | 
>> SMSTA_MTN | SMSTA_MTA)) ||
>> +!(status & SMSTA_MTE))
>
> Please, fixe the alignment here.

Ok.

>
>> +pasemi_reset(smbus);
>> +
>> +/* Clear the flags */
>>  reg_write(smbus, REG_SMSTA, status);
>> +
>> +return 0;
>>  }
>>  
>>  static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
>>  {
>> -int timeout = 100;
>> +int timeout = TRANSFER_TIMEOUT_MS;
>>  unsigned int status;
>>  
>>  if (smbus->use_irq) {
>>  reinit_completion(&smbus->irq_completion);
>> -reg_write(smbus, REG_IMASK, SMSTA_XEN | SMSTA_MTN);
>> -wait_for_completion_timeout(&smbus->irq_completion, 
>> msecs_to_jiffies(100));
>> +/* XEN should be set when a transaction terminates, whether due 
>> to error or not */
>> +reg_write(smbus, REG_IMASK, SMSTA_XEN);
>> +wait_for_completion_timeout(&smbus->irq_completion, 
>> msecs_to_jiffies(timeout));
>
> what happens if the timeout expires?

I think that can only happen if the hardware is seriously broken because
it's always supposed to set XEN. I'll make sure to catch that case in v2
though and print a separate error message similar to how the polling case
below is taken care of right now.

>
>>  reg_write(smbus, REG_IMASK, 0);
>>  status = reg_read(smbus, REG_SMSTA);
>>  } else {
>
> ...
>
>>  struct pasemi_smbus *smbus = adapter->algo_data;
>>  int ret, i;
>>  
>> -pasemi_smb_clear(smbus);
>> +if (pasemi_smb_clear(smbus))
>> +return -EIO;
>
> Can we use
>
>   ret = ...
>   if (ret)
>   return ret;
>
> This way we return whatever comes from pasemi_smb_clear().
>
>>  
>>  ret = 0;
>
> This way we can remove this line, as well.

Sure, will do both for v2.



Thanks,


Sven




Re: [PATCH 1/4] i2c: pasemi: Add registers bits and switch to BIT()

2025-03-30 Thread Sven Peter
On Sat, Mar 22, 2025, at 14:23, Andy Shevchenko wrote:
> Thu, Mar 20, 2025 at 01:23:25AM +0100, Andi Shyti kirjoitti:
>> Hi Sven,
>> 
>> On Sat, Feb 22, 2025 at 01:38:33PM +, Sven Peter via B4 Relay wrote:
>> > From: Sven Peter 
>> > 
>> > Add the missing register bits to the defines and also switch
>> > those to use the BIT macro which is much more readable than
>> > using hardcoded masks
>> > 
>> > Co-developed-by: Hector Martin 
>> > Signed-off-by: Hector Martin 
>> > Signed-off-by: Sven Peter 
>> 
>> Just this patch merged to i2c/i2c-host.
>
> This needs an update as well. The proper header for BIT() et al. is bits.h.
> bitfield.h is for FIELD_*() et al.

Ugh, good catch. Since this commit is already on the way upstream I'll send a 
fix
(and another one to sort the headers alphabetically while I'm at it anyway).


Thanks,


Sven



Re: [PATCH v2 4/6] i2c: pasemi: Improve error recovery

2025-04-27 Thread Sven Peter
Hi,


On Thu, Apr 17, 2025, at 15:07, Andi Shyti wrote:
> Hi Sven, Hector,
>
> ...
>
>> +/*
>> + * The hardware (supposedly) has a 25ms timeout for clock stretching, thus
>> + * use 100ms here which should be plenty.
>> + */
>> +#define TRANSFER_TIMEOUT_MS 100
>
> Please use the PASEMI prefix here. TRANSFER_TIMEOUT_MS it's not a
> naming belonging to this driver.
>
> 100ms looks a bit too much to me, but if you say it works, then
> it works.
>

The problem here is that we only have very outdated documentation for this
hardware and no real idea what changed since Apple bought PASemi and continued
using their i2c controller.
We know that 10ms (which used to be the original timeout iirc) is not nearly
enough and we also know that we need at least 25ms for clock strechting
(assuming nothing changed in the past 10+ years).
We just bumped it to 100ms to be safe after we very rarely got error
reports which we tracked down to timeouts and haven't gotten any reports
since.


I've addressed all your other comments for v3 which I'll send out in a few 
minutes.


Best,


Sven



Re: [BUG] rmmod i2c-pasemi-platform causing kernel crash on Apple M1.

2025-06-05 Thread Sven Peter

Hi,

On 05.06.25 13:55, chenglingfei wrote:




> -原始邮件-
> 发件人: "Sven Peter" 
> 发送时间: 2025-06-05 18:25:09 (星期四)
> 收件人: 程凌飞 , j...@jannau.net, 
aly...@rosenzweig.io, n...@gompa.dev
> 抄送: zhangzhenwei...@ict.ac.cn, wangzh...@ict.ac.cn, ma...@linux.ibm.com, 
m...@ellerman.id.au, npig...@gmail.com, christophe.le...@csgroup.eu, 
nav...@kernel.org, andi.sh...@kernel.org, as...@lists.linux.dev, 
linux-arm-ker...@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, 
linux-...@vger.kernel.org, linux-ker...@vger.kernel.org
> 主题: Re: [BUG] rmmod i2c-pasemi-platform causing kernel crash on Apple M1.
>
> Hi,
>
> On 05.06.25 05:02, 程凌飞 wrote:
> > Hi, all!
> >
> > We’ve encountered a kernel crash when running rmmod 
i2c-pasemi-platform on a Mac Mini M1 (T8103) running Asahi Arch Linux.
> >
> > The bug was first found on the Linux v6.6, which is built manually 
with the Asahi given config to run our services.
> > At that time, the i2c-pasemi-platform was i2c-apple.
> >
> > We noticed in the Linux v6.7, the pasemi is splitted into two 
separate modules, one of which is i2c-pasemi-platform.
> > Therefore, we built Linux v6.14.6 and tried to rmmod 
i2c-pasemi-platform again, the crash still exists. Moreover, we fetched
> > the latest i2c-pasemi-platform on 
linux-next(911483b25612c8bc32a706ba940738cc43299496) and asahi, built them and
> > tested again with Linux v6.14.6, but the crash remains.
> >
> > Because kexec is not supported and will never be fully supported on 
Apple Silicon platforms due to hardware and firmware
> > design constraints, we can not record the panic logs through kdump.
>
> Do you have UART connected to a device under test which you could use to
> grab the panic log from the kernel? Alternatively you can also run the
> kernel under m1n1's hypervisor and grab the log that way. It'll emulate
> the serial port and redirect its output via USB.
>

I don't have UART, but I have tried to run the kernel under m1n1's hypervisor. 
However, it does not trigger the release of cs42l83.
Given that m1n1 provides full peripheral device emulation capability, the most 
plausible explanation would be an incorrect
firmware loading sequence. But the documentation of Asahi provides little 
details about how to generate an initramfs with
firmware (I think), can you give more guidance about it?


I'm not sure why you are even trying to create a special initramfs. Just
load your usual kernel using the usual boot flow as a guest. There's 
also no firmware involved in i2c and I'm not sure what you mean with 
"full peripheral device emulation" either or how that's related to firmware.
You also mention that the crash happens when you run rmmod so I again 
don't understand what "it does not trigger the release of cs42l83" means 
here.




> >
> > Thus we tried to find the root cause of the issue manually. When we 
perform rmmod, the kernel performs device releasing on
> > the i2c bus, then calls the remove function in snd-soc-cs42l83-i2c, 
which calls the cs42l42_common_remove in cs42l42,
> > because cs42l42->init_done is true, it performs regmap_write, and 
finally calls into pasemi_smb_waitready in i2c-pasemi
> > -core.c. We noticed that smbus->use_irq is true, and after it calls 
into wait_for_completion_timeout, the system crashs!>
> > We found that wait_for_completion_timeout is one of the core 
scheduler APIs used by tens of thousands of other drivers,
> > it is unlikely causing the crash. So we tried to remove the call to 
wait_for_completion_timeout, then the system seems to
> > run well.
> >
> > However, because we have little knowledge about i2c devices and 
specifications, we are not sure whether this change will
> > cause other potential harms for the system and device. Is this call 
to wait necesary here? Or can you give a more
> > sophisticated fix?
>
> Yes, that call is necessary. It waits for the "transfer completed"
> interrupt from the hardware. Without it the driver will try to read data
> before it's available and you'll see corruption. I'm surprised hardware
> attached to i2c (usb pd controller and audio I think) works at all with
> that change.
>
>
> Sven

Are there any methods or tools to systematically verify its functionality? I am 
not sure whether the devices attached to i2c
should work well even after the i2c-pasemi-platform has been removed.


I don't understand. You say you saw a crash inside pasemi_smb_waitready 
when calling wait_for_completion_timeout and decided to remove that 
method. When you remove the call you break the entire driver because it 
will now try to read data long before the i2c transaction has been 
completed.
Obviously, no i2c device will work when the driver isn't loaded but 
without waiting for the completion they also won't work when the driver 
is loaded.



Sven




Re: [BUG] rmmod i2c-pasemi-platform causing kernel crash on Apple M1.

2025-06-05 Thread Sven Peter

Hi,

On 05.06.25 17:13, chenglingfei wrote:




> -原始邮件-
> 发件人: "Sven Peter" 
> 发送时间: 2025-06-05 22:02:35 (星期四)
> 收件人: chenglingfei 
> 抄送: j...@jannau.net, aly...@rosenzweig.io, n...@gompa.dev, 
zhangzhenwei...@ict.ac.cn, wangzh...@ict.ac.cn, ma...@linux.ibm.com, 
m...@ellerman.id.au, npig...@gmail.com, christophe.le...@csgroup.eu, 
nav...@kernel.org, andi.sh...@kernel.org, as...@lists.linux.dev, 
linux-arm-ker...@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, 
linux-...@vger.kernel.org, linux-ker...@vger.kernel.org
> 主题: Re: [BUG] rmmod i2c-pasemi-platform causing kernel crash on Apple M1.
>
> Hi,
>
> On 05.06.25 13:55, chenglingfei wrote:
> >
> >
> >
> > > -原始邮件-
> > > 发件人: "Sven Peter" 
> > > 发送时间: 2025-06-05 18:25:09 (星期四)
> > > 收件人: 程凌飞 , j...@jannau.net, 
aly...@rosenzweig.io, n...@gompa.dev
> > > 抄送: zhangzhenwei...@ict.ac.cn, wangzh...@ict.ac.cn, 
ma...@linux.ibm.com, m...@ellerman.id.au, npig...@gmail.com, christophe.le...@csgroup.eu, 
nav...@kernel.org, andi.sh...@kernel.org, as...@lists.linux.dev, 
linux-arm-ker...@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, 
linux-...@vger.kernel.org, linux-ker...@vger.kernel.org
> > > 主题: Re: [BUG] rmmod i2c-pasemi-platform causing kernel crash on 
Apple M1.
> > >
> > > Hi,
> > >
> > > On 05.06.25 05:02, 程凌飞 wrote:
> > > > Hi, all!
> > > >
> > > > We’ve encountered a kernel crash when running rmmod 
i2c-pasemi-platform on a Mac Mini M1 (T8103) running Asahi Arch Linux.
> > > >
> > > > The bug was first found on the Linux v6.6, which is built 
manually with the Asahi given config to run our services.
> > > > At that time, the i2c-pasemi-platform was i2c-apple.
> > > >
> > > > We noticed in the Linux v6.7, the pasemi is splitted into 
two separate modules, one of which is i2c-pasemi-platform.
> > > > Therefore, we built Linux v6.14.6 and tried to rmmod 
i2c-pasemi-platform again, the crash still exists. Moreover, we fetched
> > > > the latest i2c-pasemi-platform on 
linux-next(911483b25612c8bc32a706ba940738cc43299496) and asahi, built them and
> > > > tested again with Linux v6.14.6, but the crash remains.
> > > >
> > > > Because kexec is not supported and will never be fully 
supported on Apple Silicon platforms due to hardware and firmware
> > > > design constraints, we can not record the panic logs 
through kdump.
> > >
> > > Do you have UART connected to a device under test which you 
could use to
> > > grab the panic log from the kernel? Alternatively you can also 
run the
> > > kernel under m1n1's hypervisor and grab the log that way. It'll 
emulate
> > > the serial port and redirect its output via USB.
> > >
> >
> > I don't have UART, but I have tried to run the kernel under m1n1's 
hypervisor. However, it does not trigger the release of cs42l83.
> > Given that m1n1 provides full peripheral device emulation capability, 
the most plausible explanation would be an incorrect
> > firmware loading sequence. But the documentation of Asahi provides 
little details about how to generate an initramfs with
> > firmware (I think), can you give more guidance about it?
>
> I'm not sure why you are even trying to create a special initramfs. Just
> load your usual kernel using the usual boot flow as a guest. There's
> also no firmware involved in i2c and I'm not sure what you mean with
> "full peripheral device emulation" either or how that's related to 
firmware.
> You also mention that the crash happens when you run rmmod so I again
> don't understand what "it does not trigger the release of cs42l83" means
> here.
>

Well, simply running rmmod i2c-pasemi-platform doesn't directly cause a crash.
The crash occurs when the module removal triggers device_remove for cs42l83,
which ultimately calls pasemi_smb_waitready in i2c-pasemi-platform. You may 
refer
to the brief analysis provided in my first email for more details.

When booting the kernel without m1n1, cs42l83 is automatically probed after
i2c-pasemi-platform loads and subsequently removed when executing rmmod
i2c-pasemi-platform, resulting in a kernel crash. However, when booting under 
m1n1,
cs42l83 isn't probed or removed -- the device appears to be non-existent. This
observation led me to mention "full peripheral device emulation."


I'm still not sure what "full peripheral device emulation" means in that 
context. If cs42l83 isn't probed that's most likely an issue with your 
kernel or your device tree or your boot device. Hence my suggestion to 
just the regular kernel and boot device.



Unrelated, there's something wrong with your email setup, see e.g. 
https://lore.kernel.org/asahi/6064d018.2b279.19740a7eb1c.coremail.chenglingfei...@ict.ac.cn/ 
how your mail arrive.


Sven





Re: [BUG] rmmod i2c-pasemi-platform causing kernel crash on Apple M1.

2025-06-05 Thread Sven Peter

Hi,

On 05.06.25 05:02, 程凌飞 wrote:

Hi, all!

We’ve encountered a kernel crash when running rmmod i2c-pasemi-platform on a 
Mac Mini M1 (T8103) running Asahi Arch Linux.

The bug was first found on the Linux v6.6, which is built manually with the 
Asahi given config to run our services.
At that time, the i2c-pasemi-platform was i2c-apple.

We noticed in the Linux v6.7, the pasemi is splitted into two separate modules, 
one of which is i2c-pasemi-platform.
Therefore, we built Linux v6.14.6 and tried to rmmod i2c-pasemi-platform again, 
the crash still exists. Moreover, we fetched
the latest i2c-pasemi-platform on 
linux-next(911483b25612c8bc32a706ba940738cc43299496) and asahi, built them and
tested again with Linux v6.14.6, but the crash remains.

Because kexec is not supported and will never be fully supported on Apple 
Silicon platforms due to hardware and firmware
design constraints, we can not record the panic logs through kdump.


Do you have UART connected to a device under test which you could use to 
grab the panic log from the kernel? Alternatively you can also run the 
kernel under m1n1's hypervisor and grab the log that way. It'll emulate 
the serial port and redirect its output via USB.




Thus we tried to find the root cause of the issue manually. When we perform 
rmmod, the kernel performs device releasing on
the i2c bus, then calls the remove function in snd-soc-cs42l83-i2c, which calls 
the cs42l42_common_remove in cs42l42,
because cs42l42->init_done is true, it performs regmap_write, and finally 
calls into pasemi_smb_waitready in i2c-pasemi
-core.c. We noticed that smbus->use_irq is true, and after it calls into 
wait_for_completion_timeout, the system crashs!>
We found that wait_for_completion_timeout is one of the core scheduler APIs 
used by tens of thousands of other drivers,
it is unlikely causing the crash. So we tried to remove the call to 
wait_for_completion_timeout, then the system seems to
run well.

However, because we have little knowledge about i2c devices and specifications, 
we are not sure whether this change will
cause other potential harms for the system and device. Is this call to wait 
necesary here? Or can you give a more
sophisticated fix?


Yes, that call is necessary. It waits for the "transfer completed" 
interrupt from the hardware. Without it the driver will try to read data 
before it's available and you'll see corruption. I'm surprised hardware 
attached to i2c (usb pd controller and audio I think) works at all with 
that change.



Sven




[PATCH v3 0/4] Apple/PASemi i2c error recovery fixes

2025-04-27 Thread Sven Peter via B4 Relay
Hi,

This series adds a few fixes/improvements to the error recovery for
Apple/PASemi i2c controllers.
The patches have been in our downstream tree and were originally used
to debug a rare glitch caused by clock strechting but are useful in
general. We haven't seen the controller misbehave since adding these.

Best,

Sven

Signed-off-by: Sven Peter 
---
Changes in v3:
- dev_err instead of dev_warn for errors
- Added PASEMI_ prefix to the timeout define
- Declared new variables in the innermost scope they're used
- Re-added a dev_err that was dropped by mistake
- Removed already applied commits
- Removed open-coded readx_poll_timeout in the non-irq path
- Reorder commits
- Link to v2: 
https://lore.kernel.org/r/20250415-pasemi-fixes-v2-0-c543bf531...@svenpeter.dev

Changes in v2:
- Added commit to use the correct include (bits.h instead of bitfield.h)
- Added commit to sort includes
- Moved timeout explanations to code instead of just the commit log
- Made timeout recovery also work correctly in the interrupt case when
  waiting for the condition failed
- Used readx_poll_timeout instead of open-coded alternative
- Link to v1: 
https://lore.kernel.org/r/20250222-pasemi-fixes-v1-0-d7ea33d50...@svenpeter.dev

---
Hector Martin (3):
  i2c: pasemi: Enable the unjam machine
  i2c: pasemi: Improve error recovery
  i2c: pasemi: Log bus reset causes

Sven Peter (1):
  i2c: pasemi: Improve timeout handling

 drivers/i2c/busses/i2c-pasemi-core.c | 107 ---
 1 file changed, 88 insertions(+), 19 deletions(-)
---
base-commit: 7cfa6946c58989507a52f38a1267faa74a65ab0e
change-id: 20250220-pasemi-fixes-916cb77404ba

Best regards,
-- 
Sven Peter 





[PATCH v3 3/4] i2c: pasemi: Improve error recovery

2025-04-27 Thread Sven Peter via B4 Relay
From: Hector Martin 

Add handling for all the missing error condition, and better recovery in
pasemi_smb_clear().

Reviewed-by: Alyssa Rosenzweig 
Reviewed-by: Neal Gompa 
Signed-off-by: Hector Martin 
Co-developed-by: Sven Peter 
Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 61 +++-
 1 file changed, 54 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 
ee38e8a1e1f5d062384e85a2fd49d7da9257aacc..dad20ee0c6689eda309cb0374aa75b42669cbcdc
 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -87,12 +87,31 @@ static void pasemi_reset(struct pasemi_smbus *smbus)
reinit_completion(&smbus->irq_completion);
 }
 
-static void pasemi_smb_clear(struct pasemi_smbus *smbus)
+static int pasemi_smb_clear(struct pasemi_smbus *smbus)
 {
unsigned int status;
+   int ret;
 
-   status = reg_read(smbus, REG_SMSTA);
+   /* First wait for the bus to go idle */
+   ret = readx_poll_timeout(ioread32, smbus->ioaddr + REG_SMSTA,
+status, !(status & (SMSTA_XIP | SMSTA_JAM)),
+USEC_PER_MSEC,
+USEC_PER_MSEC * PASEMI_TRANSFER_TIMEOUT_MS);
+
+   if (ret < 0) {
+   dev_err(smbus->dev, "Bus is still stuck (status 0x%08x)\n", 
status);
+   return -EIO;
+   }
+
+   /* If any badness happened or there is data in the FIFOs, reset the 
FIFOs */
+   if ((status & (SMSTA_MRNE | SMSTA_JMD | SMSTA_MTO | SMSTA_TOM | 
SMSTA_MTN | SMSTA_MTA)) ||
+   !(status & SMSTA_MTE))
+   pasemi_reset(smbus);
+
+   /* Clear the flags */
reg_write(smbus, REG_SMSTA, status);
+
+   return 0;
 }
 
 static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
@@ -130,9 +149,35 @@ static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
}
}
 
+   /* Controller timeout? */
+   if (status & SMSTA_TOM) {
+   dev_err(smbus->dev, "Controller timeout, status 0x%08x\n", 
status);
+   return -EIO;
+   }
+
+   /* Peripheral timeout? */
+   if (status & SMSTA_MTO) {
+   dev_err(smbus->dev, "Peripheral timeout, status 0x%08x\n", 
status);
+   return -ETIME;
+   }
+
+   /* Still stuck in a transaction? */
+   if (status & SMSTA_XIP) {
+   dev_err(smbus->dev, "Bus stuck, status 0x%08x\n", status);
+   return -EIO;
+   }
+
+   /* Arbitration loss? */
+   if (status & SMSTA_MTA) {
+   dev_err(smbus->dev, "Arbitration loss, status 0x%08x\n", 
status);
+   return -EBUSY;
+   }
+
/* Got NACK? */
-   if (status & SMSTA_MTN)
+   if (status & SMSTA_MTN) {
+   dev_err(smbus->dev, "NACK, status 0x%08x\n", status);
return -ENXIO;
+   }
 
/* Clear XEN */
reg_write(smbus, REG_SMSTA, SMSTA_XEN);
@@ -194,9 +239,9 @@ static int pasemi_i2c_xfer(struct i2c_adapter *adapter,
struct pasemi_smbus *smbus = adapter->algo_data;
int ret, i;
 
-   pasemi_smb_clear(smbus);
-
-   ret = 0;
+   ret = pasemi_smb_clear(smbus);
+   if (ret)
+   return ret;
 
for (i = 0; i < num && !ret; i++)
ret = pasemi_i2c_xfer_msg(adapter, &msgs[i], (i == (num - 1)));
@@ -217,7 +262,9 @@ static int pasemi_smb_xfer(struct i2c_adapter *adapter,
addr <<= 1;
read_flag = read_write == I2C_SMBUS_READ;
 
-   pasemi_smb_clear(smbus);
+   err = pasemi_smb_clear(smbus);
+   if (err)
+   return err;
 
switch (size) {
case I2C_SMBUS_QUICK:

-- 
2.34.1





[PATCH v3 1/4] i2c: pasemi: Enable the unjam machine

2025-04-27 Thread Sven Peter via B4 Relay
From: Hector Martin 

The I2C bus can get stuck under some conditions (desync between
controller and device). The pasemi controllers include an unjam feature
that is enabled on reset, but was being disabled by the driver. Keep it
enabled by explicitly setting the UJM bit in the CTL register. This
should help recover the bus from certain conditions, which would
otherwise remain stuck forever.

Signed-off-by: Hector Martin 
Reviewed-by: Neal Gompa 
Reviewed-by: Alyssa Rosenzweig 
Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 
df1b0087dcacb0a3b94196368137d5e20b0e6d7e..3f5571a90c1d268ea2a9d95285f1a5e47f0481ff
 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -71,7 +71,7 @@ static inline int reg_read(struct pasemi_smbus *smbus, int 
reg)
 
 static void pasemi_reset(struct pasemi_smbus *smbus)
 {
-   u32 val = (CTL_MTR | CTL_MRR | (smbus->clk_div & CTL_CLK_M));
+   u32 val = (CTL_MTR | CTL_MRR | CTL_UJM | (smbus->clk_div & CTL_CLK_M));
 
if (smbus->hw_rev >= 6)
val |= CTL_EN;

-- 
2.34.1





[PATCH v3 4/4] i2c: pasemi: Log bus reset causes

2025-04-27 Thread Sven Peter via B4 Relay
From: Hector Martin 

This ensures we get all information we need to debug issues when users
forward us their logs.

Signed-off-by: Hector Martin 
Reviewed-by: Neal Gompa 
Reviewed-by: Alyssa Rosenzweig 
Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 
dad20ee0c6689eda309cb0374aa75b42669cbcdc..f4eca44ed18395331a366537bd06f2eb3ba08e21
 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -22,6 +22,7 @@
 /* Register offsets */
 #define REG_MTXFIFO0x00
 #define REG_MRXFIFO0x04
+#define REG_XFSTA  0x0c
 #define REG_SMSTA  0x14
 #define REG_IMASK  0x18
 #define REG_CTL0x1c
@@ -99,14 +100,18 @@ static int pasemi_smb_clear(struct pasemi_smbus *smbus)
 USEC_PER_MSEC * PASEMI_TRANSFER_TIMEOUT_MS);
 
if (ret < 0) {
-   dev_err(smbus->dev, "Bus is still stuck (status 0x%08x)\n", 
status);
+   dev_err(smbus->dev, "Bus is still stuck (status 0x%08x xfstatus 
0x%08x)\n",
+status, reg_read(smbus, REG_XFSTA));
return -EIO;
}
 
/* If any badness happened or there is data in the FIFOs, reset the 
FIFOs */
if ((status & (SMSTA_MRNE | SMSTA_JMD | SMSTA_MTO | SMSTA_TOM | 
SMSTA_MTN | SMSTA_MTA)) ||
-   !(status & SMSTA_MTE))
+   !(status & SMSTA_MTE)) {
+   dev_warn(smbus->dev, "Issuing reset due to status 0x%08x 
(xfstatus 0x%08x)\n",
+status, reg_read(smbus, REG_XFSTA));
pasemi_reset(smbus);
+   }
 
/* Clear the flags */
reg_write(smbus, REG_SMSTA, status);

-- 
2.34.1





[PATCH v3 2/4] i2c: pasemi: Improve timeout handling

2025-04-27 Thread Sven Peter via B4 Relay
From: Sven Peter 

The hardware (supposedly) has a 25ms timeout for clock stretching
and the driver uses 100ms which should be plenty.
The interrupt path however misses handling for errors while waiting for
the completion and the polling path uses an open-coded readx_poll_timeout.
Note that we drop reg_write(smbus, REG_SMSTA, status) while fixing those
issues here which will be done anyway whenever the next transaction starts
via pasemi_smb_clear.

Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 41 +---
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 
3f5571a90c1d268ea2a9d95285f1a5e47f0481ff..ee38e8a1e1f5d062384e85a2fd49d7da9257aacc
 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -52,6 +53,12 @@
 #define CTL_UJMBIT(8)
 #define CTL_CLK_M  GENMASK(7, 0)
 
+/*
+ * The hardware (supposedly) has a 25ms timeout for clock stretching, thus
+ * use 100ms here which should be plenty.
+ */
+#define PASEMI_TRANSFER_TIMEOUT_MS 100
+
 static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
 {
dev_dbg(smbus->dev, "smbus write reg %x val %08x\n", reg, val);
@@ -90,20 +97,36 @@ static void pasemi_smb_clear(struct pasemi_smbus *smbus)
 
 static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
 {
-   int timeout = 100;
unsigned int status;
 
if (smbus->use_irq) {
reinit_completion(&smbus->irq_completion);
reg_write(smbus, REG_IMASK, SMSTA_XEN | SMSTA_MTN);
-   wait_for_completion_timeout(&smbus->irq_completion, 
msecs_to_jiffies(100));
+   int ret = wait_for_completion_timeout(
+   &smbus->irq_completion,
+   msecs_to_jiffies(PASEMI_TRANSFER_TIMEOUT_MS));
reg_write(smbus, REG_IMASK, 0);
status = reg_read(smbus, REG_SMSTA);
+
+   if (ret < 0) {
+   dev_err(smbus->dev,
+   "Completion wait failed with %d, status 
0x%08x\n",
+   ret, status);
+   return ret;
+   } else if (ret == 0) {
+   dev_err(smbus->dev, "Timeout, status 0x%08x\n", status);
+   return -ETIME;
+   }
} else {
-   status = reg_read(smbus, REG_SMSTA);
-   while (!(status & SMSTA_XEN) && timeout--) {
-   msleep(1);
-   status = reg_read(smbus, REG_SMSTA);
+   int ret = readx_poll_timeout(
+   ioread32, smbus->ioaddr + REG_SMSTA,
+   status, status & SMSTA_XEN,
+   USEC_PER_MSEC,
+   USEC_PER_MSEC * PASEMI_TRANSFER_TIMEOUT_MS);
+
+   if (ret < 0) {
+   dev_err(smbus->dev, "Timeout, status 0x%08x\n", status);
+   return -ETIME;
}
}
 
@@ -111,12 +134,6 @@ static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
if (status & SMSTA_MTN)
return -ENXIO;
 
-   if (timeout < 0) {
-   dev_warn(smbus->dev, "Timeout, status 0x%08x\n", status);
-   reg_write(smbus, REG_SMSTA, status);
-   return -ETIME;
-   }
-
/* Clear XEN */
reg_write(smbus, REG_SMSTA, SMSTA_XEN);
 

-- 
2.34.1





[PATCH v2 3/6] i2c: pasemi: Improve timeout handling

2025-04-15 Thread Sven Peter via B4 Relay
From: Sven Peter 

Add proper timeout handling for the interrupt path.
Previously, this was only correctly done for the polling path.
Note that we drop reg_write(smbus, REG_SMSTA, status) here which
will be done anyway whenever the next transaction starts via
pasemi_smb_clear.

Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 24 +---
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 
df1b0087dcacb0a3b94196368137d5e20b0e6d7e..9b611dbdfef23e78a4ea75ac0311938d52b6ba96
 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -91,32 +91,42 @@ static void pasemi_smb_clear(struct pasemi_smbus *smbus)
 static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
 {
int timeout = 100;
+   int ret;
unsigned int status;
 
if (smbus->use_irq) {
reinit_completion(&smbus->irq_completion);
reg_write(smbus, REG_IMASK, SMSTA_XEN | SMSTA_MTN);
-   wait_for_completion_timeout(&smbus->irq_completion, 
msecs_to_jiffies(100));
+   ret = wait_for_completion_timeout(&smbus->irq_completion, 
msecs_to_jiffies(100));
reg_write(smbus, REG_IMASK, 0);
status = reg_read(smbus, REG_SMSTA);
+
+   if (ret < 0) {
+   dev_err(smbus->dev,
+   "Completion wait failed with %d, status 
0x%08x\n",
+   ret, status);
+   return ret;
+   } else if (ret == 0) {
+   dev_warn(smbus->dev, "Timeout, status 0x%08x\n", 
status);
+   return -ETIME;
+   }
} else {
status = reg_read(smbus, REG_SMSTA);
while (!(status & SMSTA_XEN) && timeout--) {
msleep(1);
status = reg_read(smbus, REG_SMSTA);
}
+
+   if (timeout < 0) {
+   dev_warn(smbus->dev, "Timeout, status 0x%08x\n", 
status);
+   return -ETIME;
+   }
}
 
/* Got NACK? */
if (status & SMSTA_MTN)
return -ENXIO;
 
-   if (timeout < 0) {
-   dev_warn(smbus->dev, "Timeout, status 0x%08x\n", status);
-   reg_write(smbus, REG_SMSTA, status);
-   return -ETIME;
-   }
-
/* Clear XEN */
reg_write(smbus, REG_SMSTA, SMSTA_XEN);
 

-- 
2.34.1





[PATCH v2 0/6] Apple/PASemi i2c error recovery fixes

2025-04-15 Thread Sven Peter via B4 Relay
Hi,

This series adds a few fixes/improvements to the error recovery for
Apple/PASemi i2c controllers.
The patches have been in our downstream tree and were originally used
to debug a rare glitch caused by clock strechting but are useful in
general. We haven't seen the controller misbehave since adding these.

Best,

Sven

Signed-off-by: Sven Peter 
---
Changes in v2:
- Added commit to use the correct include (bits.h instead of bitfield.h)
- Added commit to sort includes
- Moved timeout explanations to code instead of just the commit log
- Made timeout recovery also work correctly in the interrupt case when
  waiting for the condition failed
- Used readx_poll_timeout instead of open-coded alternative
- Link to v1: 
https://lore.kernel.org/r/20250222-pasemi-fixes-v1-0-d7ea33d50...@svenpeter.dev

---
Hector Martin (3):
  i2c: pasemi: Improve error recovery
  i2c: pasemi: Enable the unjam machine
  i2c: pasemi: Log bus reset causes

Sven Peter (3):
  i2c: pasemi: Use correct bits.h include
  i2c: pasemi: Sort includes alphabetically
  i2c: pasemi: Improve timeout handling

 drivers/i2c/busses/i2c-pasemi-core.c | 114 ---
 drivers/i2c/busses/i2c-pasemi-pci.c  |  10 +--
 2 files changed, 96 insertions(+), 28 deletions(-)
---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250220-pasemi-fixes-916cb77404ba

Best regards,
-- 
Sven Peter 





[PATCH v2 2/6] i2c: pasemi: Sort includes alphabetically

2025-04-15 Thread Sven Peter via B4 Relay
From: Sven Peter 

No functional changes.

Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 10 +-
 drivers/i2c/busses/i2c-pasemi-pci.c  | 10 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 
71cc8cfc7c5cbf3924269f6217712d42008c03ff..df1b0087dcacb0a3b94196368137d5e20b0e6d7e
 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -6,15 +6,15 @@
  */
 
 #include 
+#include 
+#include 
+#include 
+#include 
 #include 
 #include 
-#include 
-#include 
 #include 
-#include 
-#include 
 #include 
-#include 
+#include 
 
 #include "i2c-pasemi-core.h"
 
diff --git a/drivers/i2c/busses/i2c-pasemi-pci.c 
b/drivers/i2c/busses/i2c-pasemi-pci.c
index 
77f90c7436eda2df16afd7f1cac79355fb005bfd..b9ccb54ec77e22bb1b77848c858e2e0cc51db7e3
 100644
--- a/drivers/i2c/busses/i2c-pasemi-pci.c
+++ b/drivers/i2c/busses/i2c-pasemi-pci.c
@@ -5,15 +5,15 @@
  * SMBus host driver for PA Semi PWRficient
  */
 
+#include 
+#include 
+#include 
+#include 
 #include 
 #include 
-#include 
-#include 
 #include 
-#include 
-#include 
 #include 
-#include 
+#include 
 
 #include "i2c-pasemi-core.h"
 

-- 
2.34.1





[PATCH v2 5/6] i2c: pasemi: Enable the unjam machine

2025-04-15 Thread Sven Peter via B4 Relay
From: Hector Martin 

The I2C bus can get stuck under some conditions (desync between
controller and device). The pasemi controllers include an unjam feature
that is enabled on reset, but was being disabled by the driver. Keep it
enabled by explicitly setting the UJM bit in the CTL register. This
should help recover the bus from certain conditions, which would
otherwise remain stuck forever.

Signed-off-by: Hector Martin 
Reviewed-by: Neal Gompa 
Reviewed-by: Alyssa Rosenzweig 
Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 
2f31f039bedfd7f78d6572fe925125b1a6d0724b..41db38d82fe62c73614b8fafe4cb73c7d1a24762
 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -78,7 +78,7 @@ static inline int reg_read(struct pasemi_smbus *smbus, int 
reg)
 
 static void pasemi_reset(struct pasemi_smbus *smbus)
 {
-   u32 val = (CTL_MTR | CTL_MRR | (smbus->clk_div & CTL_CLK_M));
+   u32 val = (CTL_MTR | CTL_MRR | CTL_UJM | (smbus->clk_div & CTL_CLK_M));
 
if (smbus->hw_rev >= 6)
val |= CTL_EN;

-- 
2.34.1





[PATCH v2 4/6] i2c: pasemi: Improve error recovery

2025-04-15 Thread Sven Peter via B4 Relay
From: Hector Martin 

The hardware (supposedly) has a 25ms timeout for clock stretching
and the driver uses 100ms which should be plenty. The error
reocvery itself is however lacking.

Add handling for all the missing error condition, and better recovery in
pasemi_smb_clear(). Also move the timeout to a #define since it's used
in multiple places now.

Signed-off-by: Hector Martin 
[sven: adjusted commit message after splitting the commit into two]
Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 75 +++-
 1 file changed, 65 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 
9b611dbdfef23e78a4ea75ac0311938d52b6ba96..2f31f039bedfd7f78d6572fe925125b1a6d0724b
 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -52,6 +53,12 @@
 #define CTL_UJMBIT(8)
 #define CTL_CLK_M  GENMASK(7, 0)
 
+/*
+ * The hardware (supposedly) has a 25ms timeout for clock stretching, thus
+ * use 100ms here which should be plenty.
+ */
+#define TRANSFER_TIMEOUT_MS100
+
 static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
 {
dev_dbg(smbus->dev, "smbus write reg %x val %08x\n", reg, val);
@@ -80,24 +87,44 @@ static void pasemi_reset(struct pasemi_smbus *smbus)
reinit_completion(&smbus->irq_completion);
 }
 
-static void pasemi_smb_clear(struct pasemi_smbus *smbus)
+static int pasemi_smb_clear(struct pasemi_smbus *smbus)
 {
unsigned int status;
+   int ret;
 
-   status = reg_read(smbus, REG_SMSTA);
+   /* First wait for the bus to go idle */
+   ret = readx_poll_timeout(ioread32, smbus->ioaddr + REG_SMSTA,
+status, !(status & (SMSTA_XIP | SMSTA_JAM)),
+USEC_PER_MSEC, USEC_PER_MSEC * 
TRANSFER_TIMEOUT_MS);
+
+   if (ret < 0) {
+   dev_err(smbus->dev, "Bus is still stuck (status 0x%08x)\n", 
status);
+   return -EIO;
+   }
+
+   /* If any badness happened or there is data in the FIFOs, reset the 
FIFOs */
+   if ((status & (SMSTA_MRNE | SMSTA_JMD | SMSTA_MTO | SMSTA_TOM | 
SMSTA_MTN | SMSTA_MTA)) ||
+   !(status & SMSTA_MTE))
+   pasemi_reset(smbus);
+
+   /* Clear the flags */
reg_write(smbus, REG_SMSTA, status);
+
+   return 0;
 }
 
 static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
 {
-   int timeout = 100;
+   int timeout = TRANSFER_TIMEOUT_MS;
int ret;
unsigned int status;
 
if (smbus->use_irq) {
reinit_completion(&smbus->irq_completion);
-   reg_write(smbus, REG_IMASK, SMSTA_XEN | SMSTA_MTN);
-   ret = wait_for_completion_timeout(&smbus->irq_completion, 
msecs_to_jiffies(100));
+   /* XEN should be set when a transaction terminates, whether due 
to error or not */
+   reg_write(smbus, REG_IMASK, SMSTA_XEN);
+   ret = wait_for_completion_timeout(&smbus->irq_completion,
+ msecs_to_jiffies(timeout));
reg_write(smbus, REG_IMASK, 0);
status = reg_read(smbus, REG_SMSTA);
 
@@ -123,9 +150,35 @@ static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
}
}
 
+   /* Controller timeout? */
+   if (status & SMSTA_TOM) {
+   dev_warn(smbus->dev, "Controller timeout, status 0x%08x\n", 
status);
+   return -EIO;
+   }
+
+   /* Peripheral timeout? */
+   if (status & SMSTA_MTO) {
+   dev_warn(smbus->dev, "Peripheral timeout, status 0x%08x\n", 
status);
+   return -ETIME;
+   }
+
+   /* Still stuck in a transaction? */
+   if (status & SMSTA_XIP) {
+   dev_warn(smbus->dev, "Bus stuck, status 0x%08x\n", status);
+   return -EIO;
+   }
+
+   /* Arbitration loss? */
+   if (status & SMSTA_MTA) {
+   dev_warn(smbus->dev, "Arbitration loss, status 0x%08x\n", 
status);
+   return -EBUSY;
+   }
+
/* Got NACK? */
-   if (status & SMSTA_MTN)
+   if (status & SMSTA_MTN) {
+   dev_warn(smbus->dev, "NACK, status 0x%08x\n", status);
return -ENXIO;
+   }
 
/* Clear XEN */
reg_write(smbus, REG_SMSTA, SMSTA_XEN);
@@ -187,9 +240,9 @@ static int pasemi_i2c_xfer(struct i2c_adapter *adapter,
struct pasemi_smbus *smbus = adapter->algo_data;
int ret, i;
 
-   pasemi_smb_clear(smbus);
-
-   ret = 0;
+   ret = pasemi_smb_clear(smbus);
+   if (ret)
+ 

[PATCH v2 1/6] i2c: pasemi: Use correct bits.h include

2025-04-15 Thread Sven Peter via B4 Relay
From: Sven Peter 

When changing the #defines to use BIT and GENMASK the bitfield.h include
was added instead of the correct bits.h include.

Reported-by: Andy Shevchenko 
Closes: https://lore.kernel.org/asahi/Z965tVhC5jxy1kqZ@surfacebook.localdomain/
Fixes: 8b4da3ef9206 ("i2c: pasemi: Add registers bits and switch to BIT()")
Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 
bd128ab2e2ebb64929f2f6a3525835a880c3114d..71cc8cfc7c5cbf3924269f6217712d42008c03ff
 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -5,7 +5,7 @@
  * SMBus host driver for PA Semi PWRficient
  */
 
-#include 
+#include 
 #include 
 #include 
 #include 

-- 
2.34.1





[PATCH v2 6/6] i2c: pasemi: Log bus reset causes

2025-04-15 Thread Sven Peter via B4 Relay
From: Hector Martin 

This ensures we get all information we need to debug issues when users
forward us their logs.

Signed-off-by: Hector Martin 
Reviewed-by: Neal Gompa 
Reviewed-by: Alyssa Rosenzweig 
Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 
41db38d82fe62c73614b8fafe4cb73c7d1a24762..e5e8a748638f02e48d6ffa65e49aff5b12f70e06
 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -22,6 +22,7 @@
 /* Register offsets */
 #define REG_MTXFIFO0x00
 #define REG_MRXFIFO0x04
+#define REG_XFSTA  0x0c
 #define REG_SMSTA  0x14
 #define REG_IMASK  0x18
 #define REG_CTL0x1c
@@ -89,7 +90,7 @@ static void pasemi_reset(struct pasemi_smbus *smbus)
 
 static int pasemi_smb_clear(struct pasemi_smbus *smbus)
 {
-   unsigned int status;
+   unsigned int status, xfstatus;
int ret;
 
/* First wait for the bus to go idle */
@@ -98,7 +99,9 @@ static int pasemi_smb_clear(struct pasemi_smbus *smbus)
 USEC_PER_MSEC, USEC_PER_MSEC * 
TRANSFER_TIMEOUT_MS);
 
if (ret < 0) {
-   dev_err(smbus->dev, "Bus is still stuck (status 0x%08x)\n", 
status);
+   xfstatus = reg_read(smbus, REG_XFSTA);
+   dev_err(smbus->dev, "Bus is still stuck (status 0x%08x xfstatus 
0x%08x)\n",
+status, xfstatus);
return -EIO;
}
 

-- 
2.34.1





[PATCH 2/4] i2c: pasemi: Improve error recovery

2025-02-22 Thread Sven Peter via B4 Relay
From: Hector Martin 

The hardware (supposedly) has a 25ms timeout for clock stretching
and the driver uses 100ms which should be plenty. The error
reocvery itself is however lacking.

Add handling for all the missing error condition, and better recovery in
pasemi_smb_clear(). Also move the timeout to a #define since it's used
in multiple places now.

Signed-off-by: Hector Martin 
[sven: adjusted commit message after splitting the commit into two]
Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 70 +---
 1 file changed, 58 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index bd128ab2e2eb..770b86b92a10 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -52,6 +52,8 @@
 #define CTL_UJMBIT(8)
 #define CTL_CLK_M  GENMASK(7, 0)
 
+#define TRANSFER_TIMEOUT_MS100
+
 static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
 {
dev_dbg(smbus->dev, "smbus write reg %x val %08x\n", reg, val);
@@ -80,23 +82,45 @@ static void pasemi_reset(struct pasemi_smbus *smbus)
reinit_completion(&smbus->irq_completion);
 }
 
-static void pasemi_smb_clear(struct pasemi_smbus *smbus)
+static int pasemi_smb_clear(struct pasemi_smbus *smbus)
 {
unsigned int status;
+   int timeout = TRANSFER_TIMEOUT_MS;
 
status = reg_read(smbus, REG_SMSTA);
+
+   /* First wait for the bus to go idle */
+   while ((status & (SMSTA_XIP | SMSTA_JAM)) && timeout--) {
+   msleep(1);
+   status = reg_read(smbus, REG_SMSTA);
+   }
+
+   if (timeout < 0) {
+   dev_warn(smbus->dev, "Bus is still stuck (status 0x%08x)\n", 
status);
+   return -EIO;
+   }
+
+   /* If any badness happened or there is data in the FIFOs, reset the 
FIFOs */
+   if ((status & (SMSTA_MRNE | SMSTA_JMD | SMSTA_MTO | SMSTA_TOM | 
SMSTA_MTN | SMSTA_MTA)) ||
+   !(status & SMSTA_MTE))
+   pasemi_reset(smbus);
+
+   /* Clear the flags */
reg_write(smbus, REG_SMSTA, status);
+
+   return 0;
 }
 
 static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
 {
-   int timeout = 100;
+   int timeout = TRANSFER_TIMEOUT_MS;
unsigned int status;
 
if (smbus->use_irq) {
reinit_completion(&smbus->irq_completion);
-   reg_write(smbus, REG_IMASK, SMSTA_XEN | SMSTA_MTN);
-   wait_for_completion_timeout(&smbus->irq_completion, 
msecs_to_jiffies(100));
+   /* XEN should be set when a transaction terminates, whether due 
to error or not */
+   reg_write(smbus, REG_IMASK, SMSTA_XEN);
+   wait_for_completion_timeout(&smbus->irq_completion, 
msecs_to_jiffies(timeout));
reg_write(smbus, REG_IMASK, 0);
status = reg_read(smbus, REG_SMSTA);
} else {
@@ -107,16 +131,36 @@ static int pasemi_smb_waitready(struct pasemi_smbus 
*smbus)
}
}
 
-   /* Got NACK? */
-   if (status & SMSTA_MTN)
-   return -ENXIO;
+   /* Controller timeout? */
+   if (status & SMSTA_TOM) {
+   dev_warn(smbus->dev, "Controller timeout, status 0x%08x\n", 
status);
+   return -EIO;
+   }
 
-   if (timeout < 0) {
-   dev_warn(smbus->dev, "Timeout, status 0x%08x\n", status);
-   reg_write(smbus, REG_SMSTA, status);
+   /* Peripheral timeout? */
+   if (status & SMSTA_MTO) {
+   dev_warn(smbus->dev, "Peripheral timeout, status 0x%08x\n", 
status);
return -ETIME;
}
 
+   /* Still stuck in a transaction? */
+   if (status & SMSTA_XIP) {
+   dev_warn(smbus->dev, "Bus stuck, status 0x%08x\n", status);
+   return -EIO;
+   }
+
+   /* Arbitration loss? */
+   if (status & SMSTA_MTA) {
+   dev_warn(smbus->dev, "Arbitration loss, status 0x%08x\n", 
status);
+   return -EBUSY;
+   }
+
+   /* Got NACK? */
+   if (status & SMSTA_MTN) {
+   dev_warn(smbus->dev, "NACK, status 0x%08x\n", status);
+   return -ENXIO;
+   }
+
/* Clear XEN */
reg_write(smbus, REG_SMSTA, SMSTA_XEN);
 
@@ -177,7 +221,8 @@ static int pasemi_i2c_xfer(struct i2c_adapter *adapter,
struct pasemi_smbus *smbus = adapter->algo_data;
int ret, i;
 
-   pasemi_smb_clear(smbus);
+   if (pasemi_smb_clear(smbus))
+   return -EIO;
 
ret = 0;
 
@@ -200,7 +245,8 @@ static int pasemi_smb_xfer(struct i2c_adapter *adapter,
addr <<= 1;
read_flag = read_write == I2C_SMBUS_READ;
 
-   pasemi_smb_clear(smbus);
+   if (pasemi_smb_clear(smbus))
+   return -EIO;
 
switch (size) {
case I2C_SMBUS_QUICK:

-- 
2.34.1





[PATCH 0/4] Apple/PASemi i2c error recovery fixes

2025-02-22 Thread Sven Peter via B4 Relay
Hi,

This series adds a few fixes/improvements to the error recovery for
Apple/PASemi i2c controllers.
The patches have been in our downstream tree and were originally used
to debug a rare glitch caused by clock strechting but are useful in
general. We haven't seen the controller misbehave since adding these.

Best,

Sven

Signed-off-by: Sven Peter 
---
Hector Martin (3):
  i2c: pasemi: Improve error recovery
  i2c: pasemi: Enable the unjam machine
  i2c: pasemi: Log bus reset causes

Sven Peter (1):
  i2c: pasemi: Add registers bits and switch to BIT()

 drivers/i2c/busses/i2c-pasemi-core.c | 121 ++-
 1 file changed, 92 insertions(+), 29 deletions(-)
---
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
change-id: 20250220-pasemi-fixes-916cb77404ba

Best regards,
-- 
Sven Peter 





[PATCH 4/4] i2c: pasemi: Log bus reset causes

2025-02-22 Thread Sven Peter via B4 Relay
From: Hector Martin 

This ensures we get all information we need to debug issues when users
forward us their logs.

Signed-off-by: Hector Martin 
Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 8f0ba975172f..ae0181a76470 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -21,6 +21,7 @@
 /* Register offsets */
 #define REG_MTXFIFO0x00
 #define REG_MRXFIFO0x04
+#define REG_XFSTA  0x0c
 #define REG_SMSTA  0x14
 #define REG_IMASK  0x18
 #define REG_CTL0x1c
@@ -84,7 +85,7 @@ static void pasemi_reset(struct pasemi_smbus *smbus)
 
 static int pasemi_smb_clear(struct pasemi_smbus *smbus)
 {
-   unsigned int status;
+   unsigned int status, xfstatus;
int timeout = TRANSFER_TIMEOUT_MS;
 
status = reg_read(smbus, REG_SMSTA);
@@ -95,15 +96,21 @@ static int pasemi_smb_clear(struct pasemi_smbus *smbus)
status = reg_read(smbus, REG_SMSTA);
}
 
+   xfstatus = reg_read(smbus, REG_XFSTA);
+
if (timeout < 0) {
-   dev_warn(smbus->dev, "Bus is still stuck (status 0x%08x)\n", 
status);
+   dev_warn(smbus->dev, "Bus is still stuck (status 0x%08x 
xfstatus 0x%08x)\n",
+status, xfstatus);
return -EIO;
}
 
/* If any badness happened or there is data in the FIFOs, reset the 
FIFOs */
if ((status & (SMSTA_MRNE | SMSTA_JMD | SMSTA_MTO | SMSTA_TOM | 
SMSTA_MTN | SMSTA_MTA)) ||
-   !(status & SMSTA_MTE))
+   !(status & SMSTA_MTE)) {
+   dev_warn(smbus->dev, "Issuing reset due to status 0x%08x 
(xfstatus 0x%08x)\n",
+status, xfstatus);
pasemi_reset(smbus);
+   }
 
/* Clear the flags */
reg_write(smbus, REG_SMSTA, status);

-- 
2.34.1





[PATCH 1/4] i2c: pasemi: Add registers bits and switch to BIT()

2025-02-22 Thread Sven Peter via B4 Relay
From: Sven Peter 

Add the missing register bits to the defines and also switch
those to use the BIT macro which is much more readable than
using hardcoded masks

Co-developed-by: Hector Martin 
Signed-off-by: Hector Martin 
Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 40 ++--
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index dac694a9d781..bd128ab2e2eb 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -5,6 +5,7 @@
  * SMBus host driver for PA Semi PWRficient
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -26,21 +27,30 @@
 #define REG_REV0x28
 
 /* Register defs */
-#define MTXFIFO_READ   0x0400
-#define MTXFIFO_STOP   0x0200
-#define MTXFIFO_START  0x0100
-#define MTXFIFO_DATA_M 0x00ff
-
-#define MRXFIFO_EMPTY  0x0100
-#define MRXFIFO_DATA_M 0x00ff
-
-#define SMSTA_XEN  0x0800
-#define SMSTA_MTN  0x0020
-
-#define CTL_MRR0x0400
-#define CTL_MTR0x0200
-#define CTL_EN 0x0800
-#define CTL_CLK_M  0x00ff
+#define MTXFIFO_READ   BIT(10)
+#define MTXFIFO_STOP   BIT(9)
+#define MTXFIFO_START  BIT(8)
+#define MTXFIFO_DATA_M GENMASK(7, 0)
+
+#define MRXFIFO_EMPTY  BIT(8)
+#define MRXFIFO_DATA_M GENMASK(7, 0)
+
+#define SMSTA_XIP  BIT(28)
+#define SMSTA_XEN  BIT(27)
+#define SMSTA_JMD  BIT(25)
+#define SMSTA_JAM  BIT(24)
+#define SMSTA_MTO  BIT(23)
+#define SMSTA_MTA  BIT(22)
+#define SMSTA_MTN  BIT(21)
+#define SMSTA_MRNE BIT(19)
+#define SMSTA_MTE  BIT(16)
+#define SMSTA_TOM  BIT(6)
+
+#define CTL_EN BIT(11)
+#define CTL_MRRBIT(10)
+#define CTL_MTRBIT(9)
+#define CTL_UJMBIT(8)
+#define CTL_CLK_M  GENMASK(7, 0)
 
 static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
 {

-- 
2.34.1





[PATCH 3/4] i2c: pasemi: Enable the unjam machine

2025-02-22 Thread Sven Peter via B4 Relay
From: Hector Martin 

The I2C bus can get stuck under some conditions (desync between
controller and device). The pasemi controllers include an unjam feature
that is enabled on reset, but was being disabled by the driver. Keep it
enabled by explicitly setting the UJM bit in the CTL register. This
should help recover the bus from certain conditions, which would
otherwise remain stuck forever.

Signed-off-by: Hector Martin 
Signed-off-by: Sven Peter 
---
 drivers/i2c/busses/i2c-pasemi-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c 
b/drivers/i2c/busses/i2c-pasemi-core.c
index 770b86b92a10..8f0ba975172f 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -73,7 +73,7 @@ static inline int reg_read(struct pasemi_smbus *smbus, int 
reg)
 
 static void pasemi_reset(struct pasemi_smbus *smbus)
 {
-   u32 val = (CTL_MTR | CTL_MRR | (smbus->clk_div & CTL_CLK_M));
+   u32 val = (CTL_MTR | CTL_MRR | CTL_UJM | (smbus->clk_div & CTL_CLK_M));
 
if (smbus->hw_rev >= 6)
val |= CTL_EN;

-- 
2.34.1