Skip to contentSkip to conversation

i2c-octeon and i2c-thunderx driver

PATCH Superseded Jan Glauber 3 participants 18 messages Jun 15, 2016 – Aug 23, 2016

Subsystems (2)
I2C subsystem host drivers Cavium I2C driver

v10 of 11 Superseded Latest is v11 →

lore
Share & tools
lore

Jan Glauber

author
#0 lore raw

v10 cover i2c-octeon and i2c-thunderx driver

Hi Wolfram,

I'm reposting the rebased ThunderX i2c driver series. Patches are on-top of 4.7-rc3.

Changes to v9:
- rebased on top of upstreamed octeon i2c fixes
- reduced default sclk to 700Mhz

Changes to v8:
- Use device property for clock-frequency setting in thunderx, get rid of of_find_node_by_name
- Simplify adap.name by using device name
- SMBUS ACPI handling
- Re-phrase SMBUS error/not-specified message

Thanks,
Jan

-------------------------------------------------

Jan Glauber (8):
i2c: octeon: Rename driver to prepare for split
i2c: octeon: Split the driver into two parts
i2c: thunderx: Add i2c driver for ThunderX SOC
i2c: thunderx: Add SMBUS alert support
i2c: octeon,thunderx: Move register offsets to struct
i2c: octeon: Sort include files alphabetically
i2c: cavium: Use booleon values for booleon variables
i2c: octeon: thunderx: Add MAINTAINERS entry

 MAINTAINERS                                       |   25 +-
 drivers/i2c/busses/Kconfig                        |   10 +
 drivers/i2c/busses/Makefile                       |    3 +
 drivers/i2c/busses/{i2c-octeon.c => i2c-cavium.c} | 1039 ++++++---------------
 drivers/i2c/busses/i2c-cavium.h                   |  214 +++++
 drivers/i2c/busses/i2c-octeon-core.c              |  288 ++++++
 drivers/i2c/busses/i2c-thunderx-core.c            |  305 ++++++
 7 files changed, 1134 insertions(+), 750 deletions(-)
 rename drivers/i2c/busses/{i2c-octeon.c => i2c-cavium.c} (57%)
 create mode 100644 drivers/i2c/busses/i2c-cavium.h
 create mode 100644 drivers/i2c/busses/i2c-octeon-core.c
 create mode 100644 drivers/i2c/busses/i2c-thunderx-core.c
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH v10 1/8 i2c: octeon: Rename driver to prepare for split

This is an intermediate commit in preparation of the driver split. The module rename in this commit will be reverted in the next patch, this is just done to make the series bisectible.

Signed-off-by Jan Glauber <[email protected]>
---
 drivers/i2c/busses/Makefile                            | 2 +-
 drivers/i2c/busses/{i2c-octeon.c => i2c-octeon-core.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/i2c/busses/{i2c-octeon.c => i2c-octeon-core.c} (100%)
drivers/i2c/busses/Makefile +1 -1
index 37f2819..3405286 100644
@@ -91,7 +91,7 @@ obj-$(CONFIG_I2C_UNIPHIER)	+= i2c-uniphier.o
 obj-$(CONFIG_I2C_UNIPHIER_F)	+= i2c-uniphier-f.o
 obj-$(CONFIG_I2C_VERSATILE)	+= i2c-versatile.o
 obj-$(CONFIG_I2C_WMT)		+= i2c-wmt.o
-obj-$(CONFIG_I2C_OCTEON)	+= i2c-octeon.o
+obj-$(CONFIG_I2C_OCTEON)	+= i2c-octeon-core.o
 obj-$(CONFIG_I2C_XILINX)	+= i2c-xiic.o
 obj-$(CONFIG_I2C_XLR)		+= i2c-xlr.o
 obj-$(CONFIG_I2C_XLP9XX)	+= i2c-xlp9xx.o
drivers/i2c/busses/i2c-octeon-core.c renamed from drivers/i2c/busses/i2c-octeon.c +0 -0
similarity index 100%
rename from drivers/i2c/busses/i2c-octeon.c
rename to drivers/i2c/busses/i2c-octeon-core.c
Signature · 1 line
1.9.1
author
#2 lore raw

PATCH v10 2/8 i2c: octeon: Split the driver into two parts

In reply to Jan Glauber

Move common functionality into a separate file in preparation of the re-use from the ThunderX i2c driver.

Functions are slightly re-ordered but no other changes are included.

Signed-off-by Jan Glauber <[email protected]>
---
 drivers/i2c/busses/Makefile          |   3 +-
 drivers/i2c/busses/i2c-cavium.c      | 810 +++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-cavium.h      | 197 +++++++
 drivers/i2c/busses/i2c-octeon-core.c | 973 +----------------------------------
 4 files changed, 1010 insertions(+), 973 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-cavium.c
 create mode 100644 drivers/i2c/busses/i2c-cavium.h
drivers/i2c/busses/Makefile +2 -1
index 3405286..282f781 100644
@@ -91,7 +91,8 @@ obj-$(CONFIG_I2C_UNIPHIER)	+= i2c-uniphier.o
 obj-$(CONFIG_I2C_UNIPHIER_F)	+= i2c-uniphier-f.o
 obj-$(CONFIG_I2C_VERSATILE)	+= i2c-versatile.o
 obj-$(CONFIG_I2C_WMT)		+= i2c-wmt.o
-obj-$(CONFIG_I2C_OCTEON)	+= i2c-octeon-core.o
+i2c-octeon-objs := i2c-cavium.o i2c-octeon-core.o
+obj-$(CONFIG_I2C_OCTEON)	+= i2c-octeon.o
 obj-$(CONFIG_I2C_XILINX)	+= i2c-xiic.o
 obj-$(CONFIG_I2C_XLR)		+= i2c-xlr.o
 obj-$(CONFIG_I2C_XLP9XX)	+= i2c-xlp9xx.o
drivers/i2c/busses/i2c-cavium.c new file +810 -0

810 lines not on this page · raw message

drivers/i2c/busses/i2c-cavium.h new file +197 -0

197 lines not on this page · raw message

drivers/i2c/busses/i2c-octeon-core.c +1 -972

999 lines not on this page · raw message

Signature · 1 line
1.9.1
author
#3 lore raw

PATCH v10 3/8 i2c: thunderx: Add i2c driver for ThunderX SOC

In reply to Jan Glauber

The ThunderX SOC uses the same i2c block as the Octeon SOC. The main difference is that on ThunderX the device is a PCI device so device probing is done via PCI, interrupts are MSI-X. The clock rates can be set via device tree or ACPI.

Signed-off-by Jan Glauber <[email protected]>
---
 drivers/i2c/busses/Kconfig             |  10 ++
 drivers/i2c/busses/Makefile            |   2 +
 drivers/i2c/busses/i2c-cavium.h        |  16 ++-
 drivers/i2c/busses/i2c-thunderx-core.c | 250 +++++++++++++++++++++++++++++++++
 4 files changed, 275 insertions(+), 3 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-thunderx-core.c
drivers/i2c/busses/Kconfig +10 -0
index f167021..0c17d7c 100644
@@ -953,6 +953,16 @@ config I2C_OCTEON
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-octeon.
 
+config I2C_THUNDERX
+	tristate "Cavium ThunderX I2C bus support"
+	depends on 64BIT && PCI && !CAVIUM_OCTEON_SOC
+	help
+	  Say yes if you want to support the I2C serial bus on Cavium
+	  ThunderX SOC.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-thunderx.
+
 config I2C_XILINX
 	tristate "Xilinx I2C Controller"
 	depends on HAS_IOMEM
drivers/i2c/busses/Makefile +2 -0
index 282f781..a32ff14 100644
@@ -93,6 +93,8 @@ obj-$(CONFIG_I2C_VERSATILE)	+= i2c-versatile.o
 obj-$(CONFIG_I2C_WMT)		+= i2c-wmt.o
 i2c-octeon-objs := i2c-cavium.o i2c-octeon-core.o
 obj-$(CONFIG_I2C_OCTEON)	+= i2c-octeon.o
+i2c-thunderx-objs := i2c-cavium.o i2c-thunderx-core.o
+obj-$(CONFIG_I2C_THUNDERX)	+= i2c-thunderx.o
 obj-$(CONFIG_I2C_XILINX)	+= i2c-xiic.o
 obj-$(CONFIG_I2C_XLR)		+= i2c-xlr.o
 obj-$(CONFIG_I2C_XLP9XX)	+= i2c-xlp9xx.o
drivers/i2c/busses/i2c-cavium.h +13 -3
index 81c6a81..33c7e1f 100644
@@ -8,9 +8,15 @@
 #include <linux/pci.h>
 
 /* Register offsets */
-#define SW_TWSI			0x00
-#define TWSI_INT		0x10
-#define SW_TWSI_EXT		0x18
+#if IS_ENABLED(CONFIG_I2C_THUNDERX)
+	#define SW_TWSI			0x1000
+	#define TWSI_INT		0x1010
+	#define SW_TWSI_EXT		0x1018
+#else
+	#define SW_TWSI			0x00
+	#define TWSI_INT		0x10
+	#define SW_TWSI_EXT		0x18
+#endif
 
 /* Controller command patterns */
 #define SW_TWSI_V		BIT_ULL(63)	/* Valid bit */

@@ -94,6 +100,7 @@
 struct octeon_i2c {
 	wait_queue_head_t queue;
 	struct i2c_adapter adap;
+	struct clk *clk;
 	int irq;
 	int hlc_irq;		/* For cn7890 only */
 	u32 twsi_freq;

@@ -109,6 +116,9 @@ struct octeon_i2c {
 	void (*hlc_int_disable)(struct octeon_i2c *);
 	atomic_t int_enable_cnt;
 	atomic_t hlc_int_enable_cnt;
+#if IS_ENABLED(CONFIG_I2C_THUNDERX)
+	struct msix_entry i2c_msix;
+#endif
 };
 
 static inline void octeon_i2c_writeq_flush(u64 val, void __iomem *addr)
drivers/i2c/busses/i2c-thunderx-core.c new file +250 -0

250 lines not on this page · raw message

Signature · 1 line
1.9.1
#4 lore raw

Re: [PATCH v10 3/8] i2c: thunderx: Add i2c driver for ThunderX SOC

drivers/i2c/busses/Makefile obj-$(CONFIG_I2C_VERSATILE) += i2c-versatile.o · line 97

Quoted 5 lines

i2c-octeon-objs := i2c-cavium.o i2c-octeon-core.o
obj-$(CONFIG_I2C_OCTEON) += i2c-octeon.o

drivers/i2c/busses/Makefile +2 -0
+i2c-thunderx-objs := i2c-cavium.o i2c-thunderx-core.o
+obj-$(CONFIG_I2C_THUNDERX)	+= i2c-thunderx.o

Shouldn't that rather be "i2c-cavium-core.o", "i2c-octeon-platdrv.o", and "i2c-thunderx-pcidrv.o" for the -objs? I mean, the cavium part is the core-part...

drivers/i2c/busses/i2c-thunderx-core.c line 98

Quoted 5 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +5 -0
+skip:
+	if (!i2c->sys_freq)
+		i2c->sys_freq = SYS_FREQ_DEFAULT;
+
+	dev_info(dev, "Set system clock to %u\n", i2c->sys_freq);

Too many dev_info IMO, see later.

drivers/i2c/busses/i2c-thunderx-core.c line 176

Quoted 1 line
drivers/i2c/busses/i2c-thunderx-core.c new file +1 -0
+	i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;

Do you really need that?

drivers/i2c/busses/i2c-thunderx-core.c line 188

Quoted 3 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +3 -0
+	ret = i2c_add_adapter(&i2c->adap);
+	if (ret < 0) {
+		dev_err(dev, "Failed to add i2c adapter\n");

I2C core does error reporting for you meanwhile.

drivers/i2c/busses/i2c-thunderx-core.c line 192

Quoted 4 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +4 -0
+		goto out_irq;
+	}
+
+	dev_info(i2c->dev, "probed\n");

I'd think all nice to know dev_info should go here in condensed form.

Review

Quoted 2 lines
(quoted diff) +2 -0
+out_irq:
+	devm_free_irq(dev, i2c->i2c_msix.vector, i2c);

If you need to free irq manually here anyhow, then you don't need the devm variant.

drivers/i2c/busses/i2c-thunderx-core.c line 207

Quoted 11 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +11 -0
+out_msix:
+	pci_disable_msix(pdev);
+out_unmap:
+	iounmap(i2c->twsi_base);
+	thunder_i2c_clock_disable(dev, i2c->clk);
+out_release_regions:
+	pci_release_regions(pdev);
+out_disable_device:
+	pci_disable_device(pdev);
+out_free_i2c:
+	pci_set_drvdata(pdev, NULL);

Similar to devm_*, there is also pcim_* for PCI devices. You might want to have a look for those.

Review

Quoted 1 line
(quoted diff) +1 -0
+	devm_kfree(dev, i2c);

Not needed.

drivers/i2c/busses/i2c-thunderx-core.c line 218

Quoted 10 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +10 -0
+	return ret;
+}
+
+static void thunder_i2c_remove_pci(struct pci_dev *pdev)
+{
+	struct octeon_i2c *i2c = pci_get_drvdata(pdev);
+	struct device *dev;
+
+	if (!i2c)
+		return;

How should that happen?

drivers/i2c/busses/i2c-thunderx-core.c line 248

Quoted 4 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +4 -0
+
+module_pci_driver(thunder_i2c_pci_driver);
+
+MODULE_LICENSE("GPL");

Please add a minimal GPL header at the top of the file, so it is clear if you mean "v2" or "v2 or later".

Quoted 5 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +2 -0
+MODULE_AUTHOR("Fred Martin <[email protected]>");
+MODULE_DESCRIPTION("I2C-Bus adapter for Cavium ThunderX SOC");
Signature · 1 line
1.9.1
#5 lore raw

Re: [PATCH v10 3/8] i2c: thunderx: Add i2c driver for ThunderX SOC

Quoted 7 lines · Wolfram Sang

drivers/i2c/busses/Makefile obj-$(CONFIG_I2C_VERSATILE) += i2c-versatile.o · line 97

Quoted 5 lines

i2c-octeon-objs := i2c-cavium.o i2c-octeon-core.o
obj-$(CONFIG_I2C_OCTEON) += i2c-octeon.o

drivers/i2c/busses/Makefile +2 -0
+i2c-thunderx-objs := i2c-cavium.o i2c-thunderx-core.o
+obj-$(CONFIG_I2C_THUNDERX)	+= i2c-thunderx.o

Shouldn't that rather be "i2c-cavium-core.o",

Thinking of it again, it should probably even be "i2c-octeon-core.o" to avoid confusion because all the functions start with octeon_*

Quoted 1 line

"i2c-octeon-platdrv.o", and "i2c-thunderx-pcidrv.o" for the -objs?

Those names still make sense :)

#6 lore raw

Re: [PATCH v10 3/8] i2c: thunderx: Add i2c driver for ThunderX SOC

Quoted 9 lines · Wolfram Sang
Quoted 7 lines · Wolfram Sang

drivers/i2c/busses/Makefile obj-$(CONFIG_I2C_VERSATILE) += i2c-versatile.o · line 97

Quoted 5 lines

i2c-octeon-objs := i2c-cavium.o i2c-octeon-core.o
obj-$(CONFIG_I2C_OCTEON) += i2c-octeon.o

drivers/i2c/busses/Makefile +2 -0
+i2c-thunderx-objs := i2c-cavium.o i2c-thunderx-core.o
+obj-$(CONFIG_I2C_THUNDERX)	+= i2c-thunderx.o

Shouldn't that rather be "i2c-cavium-core.o",

Quoted 7 lines

Thinking of it again, it should probably even be "i2c-octeon-core.o" to avoid confusion because all the functions start with octeon_*

Quoted 1 line

"i2c-octeon-platdrv.o", and "i2c-thunderx-pcidrv.o" for the -objs?

Those names still make sense :)

Agreed, the naming you propose looks much better.

--Jan

#7 lore raw

Re: [PATCH v10 3/8] i2c: thunderx: Add i2c driver for ThunderX SOC

In reply to Wolfram Sang
Quoted 10 lines · Wolfram Sang

drivers/i2c/busses/Makefile obj-$(CONFIG_I2C_VERSATILE) += i2c-versatile.o · line 97

Quoted 5 lines

i2c-octeon-objs := i2c-cavium.o i2c-octeon-core.o
obj-$(CONFIG_I2C_OCTEON) += i2c-octeon.o

drivers/i2c/busses/Makefile +2 -0
+i2c-thunderx-objs := i2c-cavium.o i2c-thunderx-core.o
+obj-$(CONFIG_I2C_THUNDERX)	+= i2c-thunderx.o

Shouldn't that rather be "i2c-cavium-core.o", "i2c-octeon-platdrv.o", and "i2c-thunderx-pcidrv.o" for the -objs? I mean, the cavium part is the core-part...

Quoted 7 lines

drivers/i2c/busses/i2c-thunderx-core.c line 98

Quoted 5 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +5 -0
+skip:
+	if (!i2c->sys_freq)
+		i2c->sys_freq = SYS_FREQ_DEFAULT;
+
+	dev_info(dev, "Set system clock to %u\n", i2c->sys_freq);

Too many dev_info IMO, see later.

OK. I was working on an update where I dropped most of the messages.

Quoted 3 lines

drivers/i2c/busses/i2c-thunderx-core.c line 176

Quoted 1 line
drivers/i2c/busses/i2c-thunderx-core.c new file +1 -0
+	i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;

Do you really need that?

Not sure, just copy'n paste from what most of the other bus drivers do. Looking at i2c-core.c it might be a legacy thing, so I can probably remove it.

Quoted 5 lines

drivers/i2c/busses/i2c-thunderx-core.c line 188

Quoted 3 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +3 -0
+	ret = i2c_add_adapter(&i2c->adap);
+	if (ret < 0) {
+		dev_err(dev, "Failed to add i2c adapter\n");

I2C core does error reporting for you meanwhile.

OK.

Quoted 7 lines

drivers/i2c/busses/i2c-thunderx-core.c line 192

Quoted 4 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +4 -0
+		goto out_irq;
+	}
+
+	dev_info(i2c->dev, "probed\n");

I'd think all nice to know dev_info should go here in condensed form.

Quoted 5 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+out_irq:
+	devm_free_irq(dev, i2c->i2c_msix.vector, i2c);

If you need to free irq manually here anyhow, then you don't need the devm variant.

Yes. I paid attention to the devm documentation in the meantime and switched all probing to the managed fucntions and got rid of the goto's completely.

Quoted 6 lines
Quoted 6 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +6 -0
+out_msix:
+	pci_disable_msix(pdev);
+out_unmap:
+	iounmap(i2c->twsi_base);
+	thunder_i2c_clock_disable(dev, i2c->clk);
+out_release_regions:
Quoted 8 lines

drivers/i2c/busses/i2c-thunderx-core.c line 207

Quoted 5 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +5 -0
+	pci_release_regions(pdev);
+out_disable_device:
+	pci_disable_device(pdev);
+out_free_i2c:
+	pci_set_drvdata(pdev, NULL);

Similar to devm_*, there is also pcim_* for PCI devices. You might want to have a look for those.

See above.

Quoted 3 lines

Review

Quoted 1 line
(quoted diff) +1 -0
+	devm_kfree(dev, i2c);

Not needed.

Yes.

Quoted 4 lines
Quoted 4 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +4 -0
+	return ret;
+}
+
+static void thunder_i2c_remove_pci(struct pci_dev *pdev)
Quoted 8 lines

drivers/i2c/busses/i2c-thunderx-core.c line 218

Quoted 6 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +6 -0
+{
+	struct octeon_i2c *i2c = pci_get_drvdata(pdev);
+	struct device *dev;
+
+	if (!i2c)
+		return;

How should that happen?

Just a safety net, can be removed.

Quoted 7 lines

drivers/i2c/busses/i2c-thunderx-core.c line 248

Quoted 4 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +4 -0
+
+module_pci_driver(thunder_i2c_pci_driver);
+
+MODULE_LICENSE("GPL");

Please add a minimal GPL header at the top of the file, so it is clear if you mean "v2" or "v2 or later".

OK.

Quoted 5 lines
Quoted 5 lines
drivers/i2c/busses/i2c-thunderx-core.c new file +2 -0
+MODULE_AUTHOR("Fred Martin <[email protected]>");
+MODULE_DESCRIPTION("I2C-Bus adapter for Cavium ThunderX SOC");
Signature · 1 line
1.9.1
author
#8 lore raw

PATCH v10 4/8 i2c: thunderx: Add SMBUS alert support

In reply to Jan Glauber

Add SMBUS alert interrupt support. For now only device tree is supported for specifying the alert. In case of ACPI an error is returned.

Signed-off-by Jan Glauber <[email protected]>
---
 drivers/i2c/busses/i2c-cavium.h        |  6 ++++
 drivers/i2c/busses/i2c-thunderx-core.c | 51 ++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)
drivers/i2c/busses/i2c-cavium.h +6 -0
index 33c7e1f..9c128c1 100644
@@ -3,6 +3,7 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/i2c.h>
+#include <linux/i2c-smbus.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/pci.h>

@@ -119,6 +120,11 @@ struct octeon_i2c {
 #if IS_ENABLED(CONFIG_I2C_THUNDERX)
 	struct msix_entry i2c_msix;
 #endif
+
+#if IS_ENABLED(CONFIG_I2C_SMBUS)
+	struct i2c_smbus_alert_setup alert_data;
+	struct i2c_client *ara;
+#endif
 };
 
 static inline void octeon_i2c_writeq_flush(u64 val, void __iomem *addr)
drivers/i2c/busses/i2c-thunderx-core.c +51 -0

78 lines not on this page · raw message

Signature · 1 line
1.9.1
#9 lore raw

Re: [PATCH v10 4/8] i2c: thunderx: Add SMBUS alert support

Quoted 5 lines · Jan Glauber

Add SMBUS alert interrupt support. For now only device tree is supported for specifying the alert. In case of ACPI an error is returned.

Signed-off-by Jan Glauber <[email protected]>

What about 'select I2C_SMBUS' in Kconfig and skip all the ifdeffery?

#10 lore raw

Re: [PATCH v10 4/8] i2c: thunderx: Add SMBUS alert support

Quoted 9 lines · Wolfram Sang
Quoted 5 lines · Jan Glauber

Add SMBUS alert interrupt support. For now only device tree is supported for specifying the alert. In case of ACPI an error is returned.

Signed-off-by Jan Glauber <[email protected]>

What about 'select I2C_SMBUS' in Kconfig and skip all the ifdeffery?

Wouldn't that prevent a distribution that has I2C_HELPER_AUTO set from enabling the ThunderX i2c driver at all?

--Jan

#11 lore raw

Re: [PATCH v10 4/8] i2c: thunderx: Add SMBUS alert support

Quoted 6 lines · Jan Glauber
Quoted 5 lines · Wolfram Sang
Quoted 4 lines · Jan Glauber

Add SMBUS alert interrupt support. For now only device tree is supported for specifying the alert. In case of ACPI an error is returned.

Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Jan Glauber <[email protected]>

What about 'select I2C_SMBUS' in Kconfig and skip all the ifdeffery?

Wouldn't that prevent a distribution that has I2C_HELPER_AUTO set from enabling the ThunderX i2c driver at all?

From commit e2ca307439fb9df922c3e8891289a2ac05812fb7:

...
Bus drivers which implement SMBus alert should select this option, so in most cases this option is hidden and the user doesn't have to care about it.

#12 lore raw

Re: [PATCH v10 4/8] i2c: thunderx: Add SMBUS alert support

Quoted 15 lines · Wolfram Sang
Quoted 6 lines · Jan Glauber
Quoted 5 lines · Wolfram Sang
Quoted 4 lines · Jan Glauber

Add SMBUS alert interrupt support. For now only device tree is supported for specifying the alert. In case of ACPI an error is returned.

Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Jan Glauber <[email protected]>

What about 'select I2C_SMBUS' in Kconfig and skip all the ifdeffery?

Wouldn't that prevent a distribution that has I2C_HELPER_AUTO set from enabling the ThunderX i2c driver at all?

Quoted 7 lines

From commit e2ca307439fb9df922c3e8891289a2ac05812fb7:

...
Bus drivers which implement SMBus alert should select this option, so in most cases this option is hidden and the user doesn't have to care about it.

OK, makes sense. I'll remove the ifdef's.

author
#13 lore raw

PATCH v10 5/8 i2c: octeon,thunderx: Move register offsets to struct

In reply to Jan Glauber

The register offsets are different between Octeon and ThunderX so move them into the algorithm struct and get rid of the define.

Signed-off-by Jan Glauber <[email protected]>
---
 drivers/i2c/busses/i2c-cavium.c        | 28 +++++++++++++--------------
 drivers/i2c/busses/i2c-cavium.h        | 35 +++++++++++++++++-----------------
 drivers/i2c/busses/i2c-octeon-core.c   |  4 ++++
 drivers/i2c/busses/i2c-thunderx-core.c |  4 ++++
 4 files changed, 40 insertions(+), 31 deletions(-)
drivers/i2c/busses/i2c-cavium.c +14 -14
index 3e4dd64..56b23c2 100644
@@ -99,7 +99,7 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c)
 
 static bool octeon_i2c_hlc_test_valid(struct octeon_i2c *i2c)
 {
-	return (__raw_readq(i2c->twsi_base + SW_TWSI) & SW_TWSI_V) == 0;
+	return (__raw_readq(i2c->twsi_base + SW_TWSI(i2c)) & SW_TWSI_V) == 0;
 }
 
 static bool octeon_i2c_hlc_test_ready(struct octeon_i2c *i2c, bool *first)

@@ -446,12 +446,12 @@ static int octeon_i2c_hlc_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)
 	else
 		cmd |= SW_TWSI_OP_7;
 
-	octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI);
+	octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
 	ret = octeon_i2c_hlc_wait(i2c);
 	if (ret)
 		goto err;
 
-	cmd = __raw_readq(i2c->twsi_base + SW_TWSI);
+	cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
 	if ((cmd & SW_TWSI_R) == 0)
 		return -EAGAIN;
 

@@ -459,7 +459,7 @@ static int octeon_i2c_hlc_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)
 		msgs[0].buf[j] = (cmd >> (8 * i)) & 0xff;
 
 	if (msgs[0].len > 4) {
-		cmd = __raw_readq(i2c->twsi_base + SW_TWSI_EXT);
+		cmd = __raw_readq(i2c->twsi_base + SW_TWSI_EXT(i2c));
 		for (i = 0; i  < msgs[0].len - 4 && i < 4; i++, j--)
 			msgs[0].buf[j] = (cmd >> (8 * i)) & 0xff;
 	}

@@ -496,15 +496,15 @@ static int octeon_i2c_hlc_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
 
 		for (i = 0; i < msgs[0].len - 4 && i < 4; i++, j--)
 			ext |= (u64)msgs[0].buf[j] << (8 * i);
-		octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT);
+		octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
 	}
 
-	octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI);
+	octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
 	ret = octeon_i2c_hlc_wait(i2c);
 	if (ret)
 		goto err;
 
-	cmd = __raw_readq(i2c->twsi_base + SW_TWSI);
+	cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
 	if ((cmd & SW_TWSI_R) == 0)
 		return -EAGAIN;
 

@@ -539,19 +539,19 @@ static int octeon_i2c_hlc_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs
 		cmd |= SW_TWSI_EIA;
 		ext = (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT;
 		cmd |= (u64)msgs[0].buf[1] << SW_TWSI_IA_SHIFT;
-		octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT);
+		octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
 	} else {
 		cmd |= (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT;
 	}
 
 	octeon_i2c_hlc_int_clear(i2c);
-	octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI);
+	octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
 
 	ret = octeon_i2c_hlc_wait(i2c);
 	if (ret)
 		goto err;
 
-	cmd = __raw_readq(i2c->twsi_base + SW_TWSI);
+	cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
 	if ((cmd & SW_TWSI_R) == 0)
 		return -EAGAIN;
 

@@ -559,7 +559,7 @@ static int octeon_i2c_hlc_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs
 		msgs[1].buf[j] = (cmd >> (8 * i)) & 0xff;
 
 	if (msgs[1].len > 4) {
-		cmd = __raw_readq(i2c->twsi_base + SW_TWSI_EXT);
+		cmd = __raw_readq(i2c->twsi_base + SW_TWSI_EXT(i2c));
 		for (i = 0; i  < msgs[1].len - 4 && i < 4; i++, j--)
 			msgs[1].buf[j] = (cmd >> (8 * i)) & 0xff;
 	}

@@ -606,16 +606,16 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg
 		set_ext = true;
 	}
 	if (set_ext)
-		octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT);
+		octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
 
 	octeon_i2c_hlc_int_clear(i2c);
-	octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI);
+	octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
 
 	ret = octeon_i2c_hlc_wait(i2c);
 	if (ret)
 		goto err;
 
-	cmd = __raw_readq(i2c->twsi_base + SW_TWSI);
+	cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
 	if ((cmd & SW_TWSI_R) == 0)
 		return -EAGAIN;
drivers/i2c/busses/i2c-cavium.h +18 -17
index 9c128c1..749dffc 100644
@@ -8,17 +8,6 @@
 #include <linux/kernel.h>
 #include <linux/pci.h>
 
-/* Register offsets */
-#if IS_ENABLED(CONFIG_I2C_THUNDERX)
-	#define SW_TWSI			0x1000
-	#define TWSI_INT		0x1010
-	#define SW_TWSI_EXT		0x1018
-#else
-	#define SW_TWSI			0x00
-	#define TWSI_INT		0x10
-	#define SW_TWSI_EXT		0x18
-#endif
-
 /* Controller command patterns */
 #define SW_TWSI_V		BIT_ULL(63)	/* Valid bit */
 #define SW_TWSI_EIA		BIT_ULL(61)	/* Extended internal address */

@@ -98,9 +87,21 @@
 
 #define I2C_OCTEON_EVENT_WAIT 80 /* microseconds */
 
+/* Register offsets */
+struct octeon_i2c_reg_offset {
+	unsigned int sw_twsi;
+	unsigned int twsi_int;
+	unsigned int sw_twsi_ext;
+};
+
+#define SW_TWSI(x)	(x->roff.sw_twsi)
+#define TWSI_INT(x)	(x->roff.twsi_int)
+#define SW_TWSI_EXT(x)	(x->roff.sw_twsi_ext)
+
 struct octeon_i2c {
 	wait_queue_head_t queue;
 	struct i2c_adapter adap;
+	struct octeon_i2c_reg_offset roff;
 	struct clk *clk;
 	int irq;
 	int hlc_irq;		/* For cn7890 only */

@@ -145,9 +146,9 @@ static inline void octeon_i2c_reg_write(struct octeon_i2c *i2c, u64 eop_reg, u8
 {
 	u64 tmp;
 
-	__raw_writeq(SW_TWSI_V | eop_reg | data, i2c->twsi_base + SW_TWSI);
+	__raw_writeq(SW_TWSI_V | eop_reg | data, i2c->twsi_base + SW_TWSI(i2c));
 	do {
-		tmp = __raw_readq(i2c->twsi_base + SW_TWSI);
+		tmp = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
 	} while ((tmp & SW_TWSI_V) != 0);
 }
 

@@ -169,9 +170,9 @@ static inline u8 octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg)
 {
 	u64 tmp;
 
-	__raw_writeq(SW_TWSI_V | eop_reg | SW_TWSI_R, i2c->twsi_base + SW_TWSI);
+	__raw_writeq(SW_TWSI_V | eop_reg | SW_TWSI_R, i2c->twsi_base + SW_TWSI(i2c));
 	do {
-		tmp = __raw_readq(i2c->twsi_base + SW_TWSI);
+		tmp = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
 	} while ((tmp & SW_TWSI_V) != 0);
 
 	return tmp & 0xFF;

@@ -192,7 +193,7 @@ static inline u8 octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg)
  */
 static inline u64 octeon_i2c_read_int(struct octeon_i2c *i2c)
 {
-	return __raw_readq(i2c->twsi_base + TWSI_INT);
+	return __raw_readq(i2c->twsi_base + TWSI_INT(i2c));
 }
 
 /**

@@ -202,7 +203,7 @@ static inline u64 octeon_i2c_read_int(struct octeon_i2c *i2c)
  */
 static inline void octeon_i2c_write_int(struct octeon_i2c *i2c, u64 data)
 {
-	octeon_i2c_writeq_flush(data, i2c->twsi_base + TWSI_INT);
+	octeon_i2c_writeq_flush(data, i2c->twsi_base + TWSI_INT(i2c));
 }
 
 /* Prototypes */
drivers/i2c/busses/i2c-octeon-core.c +4 -0
index fec4306..038007d 100644
@@ -163,6 +163,10 @@ static int octeon_i2c_probe(struct platform_device *pdev)
 	}
 	i2c->dev = &pdev->dev;
 
+	i2c->roff.sw_twsi = 0x00;
+	i2c->roff.twsi_int = 0x10;
+	i2c->roff.sw_twsi_ext = 0x18;
+
 	res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	i2c->twsi_base = devm_ioremap_resource(&pdev->dev, res_mem);
 	if (IS_ERR(i2c->twsi_base)) {
drivers/i2c/busses/i2c-thunderx-core.c +4 -0
index d68ce08..084f940 100644
@@ -164,6 +164,10 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
 	if (!i2c)
 		return -ENOMEM;
 
+	i2c->roff.sw_twsi = 0x1000;
+	i2c->roff.twsi_int = 0x1010;
+	i2c->roff.sw_twsi_ext = 0x1018;
+
 	i2c->dev = dev;
 	pci_set_drvdata(pdev, i2c);
 	ret = pci_enable_device(pdev);
Signature · 1 line
1.9.1
author
#14 lore raw

PATCH v10 6/8 i2c: octeon: Sort include files alphabetically

In reply to Jan Glauber

Sort include files alphabetically to reduce probability of merge conflicts.

Signed-off-by Jan Glauber <[email protected]>
---
 drivers/i2c/busses/i2c-octeon-core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/i2c/busses/i2c-octeon-core.c +5 -5
index 038007d..ce535fa 100644
@@ -12,16 +12,16 @@
  */
 
 #include <linux/atomic.h>
-#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
-#include <linux/i2c.h>
-#include <linux/io.h>
-#include <linux/of.h>
 
 #include <asm/octeon/octeon.h>
 #include "i2c-cavium.h"
Signature · 1 line
1.9.1
author
#15 lore raw

PATCH v10 7/8 i2c: cavium: Use booleon values for booleon variables

In reply to Jan Glauber

Initialize booleon values with true instead of 1.

Signed-off-by Jan Glauber <[email protected]>
---
 drivers/i2c/busses/i2c-cavium.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-cavium.c +2 -2
index 56b23c2..3a581c9 100644
@@ -63,7 +63,7 @@ static bool octeon_i2c_test_ready(struct octeon_i2c *i2c, bool *first)
 static int octeon_i2c_wait(struct octeon_i2c *i2c)
 {
 	long time_left;
-	bool first = 1;
+	bool first = true;
 
 	/*
 	 * Some chip revisions don't assert the irq in the interrupt

@@ -176,7 +176,7 @@ static void octeon_i2c_hlc_disable(struct octeon_i2c *i2c)
  */
 static int octeon_i2c_hlc_wait(struct octeon_i2c *i2c)
 {
-	bool first = 1;
+	bool first = true;
 	int time_left;
 
 	/*
Signature · 1 line
1.9.1
author
#16 lore raw

PATCH v10 8/8 i2c: octeon: thunderx: Add MAINTAINERS entry

In reply to Jan Glauber

The i2c Octeon and ThunderX drivers are maintained by Cavium. While at it fix the whitespace errors of the next entry.

Signed-off-by Jan Glauber <[email protected]>
Acked-by David Daney <[email protected]>
---
 MAINTAINERS | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)
MAINTAINERS +17 -8
index 16700e4..ff8075a 100644
@@ -2827,15 +2827,24 @@ S:	Maintained
 F:	drivers/iio/light/cm*
 F:	Documentation/devicetree/bindings/i2c/trivial-devices.txt
 
+CAVIUM I2C DRIVER
+M:	Jan Glauber <[email protected]>
+M:	David Daney <[email protected]>
+W:	http://www.cavium.com
+S:	Supported
+F:	drivers/i2c/busses/i2c-cavium*
+F:	drivers/i2c/busses/i2c-octeon*
+F:	drivers/i2c/busses/i2c-thunderx*
+
 CAVIUM LIQUIDIO NETWORK DRIVER
-M:     Derek Chickles <[email protected]>
-M:     Satanand Burla <[email protected]>
-M:     Felix Manlunas <[email protected]>
-M:     Raghu Vatsavayi <[email protected]>
-L:     [email protected]
-W:     http://www.cavium.com
-S:     Supported
-F:     drivers/net/ethernet/cavium/liquidio/
+M:	Derek Chickles <[email protected]>
+M:	Satanand Burla <[email protected]>
+M:	Felix Manlunas <[email protected]>
+M:	Raghu Vatsavayi <[email protected]>
+L:	[email protected]
+W:	http://www.cavium.com
+S:	Supported
+F:	drivers/net/ethernet/cavium/liquidio/
 
 CC2520 IEEE-802.15.4 RADIO DRIVER
 M:	Varka Bhadram <[email protected]>
Signature · 1 line
1.9.1
#17 lore raw

Re: [PATCH v10 8/8] i2c: octeon: thunderx: Add MAINTAINERS entry

Quoted 2 lines · Jan Glauber

The i2c Octeon and ThunderX drivers are maintained by Cavium. While at it fix the whitespace errors of the next entry.

I'd rather have the network driver changes go in with a seperate patch (probably via netdev).