Re: [PATCH v2 0/9] iommu: Convert dart & iommufd to the new domain_alloc_paging()
> > 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
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
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
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
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
> > 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
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
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
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
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
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
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
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_*
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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_*
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
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
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
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
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
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
> > 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
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()
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
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.
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.
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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