Skip to contentSkip to conversation

Add ASPEED AST2600 I2Cv2 controller driver

PATCH Superseded Ryan Chen 3 participants 6 messages Mar 27, 2023 – Mar 29, 2023

Subsystems (4)

v7 of 12 Superseded Latest is v12 →

lore
Share & tools
lore

Ryan Chen

author maintainer
#0 lore raw

v7 cover Add ASPEED AST2600 I2Cv2 controller driver

This series add AST2600 i2cv2 new register set driver. The i2cv2 driver is new register set that have new clock divider option for more flexiable generation. And also have separate i2c master and slave register set for control.

The legacy register layout is mix master/slave register control together. The following is add more detail description about new register layout. And new feature set add for register.

-Add new clock divider option for more flexible and accurate clock rate generation -Add tCKHighMin timing to guarantee SCL high pulse width. -Add support dual pool buffer mode, split 32 bytes pool buffer of each device into 2 x 16 bytes for Tx and Rx individually. -Increase DMA buffer size to 4096 bytes and support byte alignment. -Re-define the base address of BUS1 ~ BUS16 and Pool buffer. -Re-define registers for separating master and slave mode control. -Support 4 individual DMA buffers for master Tx and Rx, slave Tx and Rx.

And following is new register set for package transfer sequence. -New Master operation mode:
S -> Aw -> P
S -> Aw -> TxD -> P
S -> Ar -> RxD -> P
S -> Aw -> RxD -> Sr -> Ar -> TxD -> P
-Bus SDA lock auto-release capability for new master DMA command mode. -Bus auto timeout for new master/slave DMA mode.

The following is two versus register layout.
Old:
{I2CD00}: Function Control Register
{I2CD04}: Clock and AC Timing Control Register {I2CD08}: Clock and AC Timing Control Register {I2CD0C}: Interrupt Control Register
{I2CD10}: Interrupt Status Register
{I2CD14}: Command/Status Register
{I2CD18}: Slave Device Address Register
{I2CD1C}: Pool Buffer Control Register
{I2CD20}: Transmit/Receive Byte Buffer Register {I2CD24}: DMA Mode Buffer Address Register
{I2CD28}: DMA Transfer Length Register
{I2CD2C}: Original DMA Mode Buffer Address Setting {I2CD30}: Original DMA Transfer Length Setting and Final Status

New Register mode
{I2CC00}: Master/Slave Function Control Register {I2CC04}: Master/Slave Clock and AC Timing Control Register {I2CC08}: Master/Slave Transmit/Receive Byte Buffer Register {I2CC0C}: Master/Slave Pool Buffer Control Register {I2CM10}: Master Interrupt Control Register {I2CM14}: Master Interrupt Status Register {I2CM18}: Master Command/Status Register
{I2CM1C}: Master DMA Buffer Length Register {I2CS20}: Slave~ Interrupt Control Register {I2CS24}: Slave~ Interrupt Status Register {I2CS28}: Slave~ Command/Status Register
{I2CS2C}: Slave~ DMA Buffer Length Register {I2CM30}: Master DMA Mode Tx Buffer Base Address {I2CM34}: Master DMA Mode Rx Buffer Base Address {I2CS38}: Slave~ DMA Mode Tx Buffer Base Address {I2CS3C}: Slave~ DMA Mode Rx Buffer Base Address {I2CS40}: Slave Device Address Register
{I2CM48}: Master DMA Length Status Register {I2CS4C}: Slave DMA Length Status Register {I2CC50}: Current DMA Operating Address Status {I2CC54}: Current DMA Operating Length Status

aspeed,global-regs:
This global register is needed, global register is setting for new clock divide control, and new register set control.

ASPEED SOC chip is server product, i2c bus may have fingerprint connect to another board. And also support hotplug. The following is board-specific design example.
Board A Board B
------------------------- ------------------------

|i2c bus#1(master/slave)  <===fingerprint ===> i2c bus#x (master/slave)|
|i2c bus#2(master)-> tmp i2c device |          |                       |
|i2c bus#3(master)-> adc i2c device |          |                       |
-------------------------                       ------------------------

i2c-scl-clk-low-timeout-us:
For example I2C controller as slave mode, and suddenly disconnected. Slave state machine will keep waiting for master clock in for rx/tx transmit. So it need timeout setting to enable timeout unlock controller state. And in another side. In Master side also need avoid suddenly slave miss(un-plug), Master will timeout and release the SDA/SCL.

aspeed,enable-dma:
For example The bus#1 have trunk data needed for transfer, it can enable bus dma mode transfer, it can reduce cpu utilized. Others bus bus#2/3 use defautl buffer mode.

v7:
-aspeed,i2c.yaml
-Update ASPEED I2C maintainers email.
-use aspeed,enable-dma property instead of aspeed,xfer-mode selection. -fix allOf and else false properties for aspeed,ast2600-i2cv2.
-i2c-ast2600.c
-remove aspeed,xfer-mode instead of aspeed,enable-dma mode. buffer mode
is default.
-remove aspeed,timeout instead of i2c-scl-clk-low-timeout-us for
timeout setting.

v6:
-remove aspeed,i2cv2.yaml, merge to aspeed,i2c.yaml -add support for
i2cv2 properites.
-i2c-ast2600.c
-fix ast2600_i2c_remove ordering.
-remove ast2600_i2c_probe goto labels, and add dev_err_probe -remove
redundant deb_dbg debug message.
-rename gr_regmap -> global_regs

v5:
-remove ast2600-i2c-global.yaml, i2c-ast2600-global.c. -i2c-ast2600.c
-remove legacy clock divide, all go for new clock divide. -remove duplicated read isr.
-remove no used driver match
-fix probe return for each labels return.
-global use mfd driver, driver use phandle to regmap read/write.
-rename aspeed,i2c-ast2600.yaml to aspeed,i2cv2.yaml -remove bus-frequency. -add required aspeed,gr
-add timeout, byte-mode, buff-mode properites.

v4:
-fix i2c-ast2600.c driver buffer mode use single buffer conflit in
master slave mode both enable.
-fix kmemleak issue when use dma mode.
-fix typo aspeed,i2c-ast2600.yaml compatible is "aspeed,ast2600-i2c" -fix typo aspeed,i2c-ast2600.ymal to aspeed,i2c-ast2600.yaml

v3:
-fix i2c global clock divide default value.
-remove i2c slave no used dev_dbg info.

v2:
-add i2c global ymal file commit.
-rename file name from new to ast2600.
aspeed-i2c-new-global.c -> i2c-ast2600-global.c
aspeed-i2c-new-global.h -> i2c-ast2600-global.h
i2c-new-aspeed.c -> i2c-ast2600.c
-rename all driver function name to ast2600.

Ryan Chen (2):
dt-bindings: i2c: aspeed: support for AST2600-i2cv2
i2c: aspeed: support ast2600 i2c new register mode driver

 .../devicetree/bindings/i2c/aspeed,i2c.yaml   |   56 +-
 MAINTAINERS                                   |    9 +
 drivers/i2c/busses/Kconfig                    |   11 +
 drivers/i2c/busses/Makefile                   |    1 +
 drivers/i2c/busses/i2c-ast2600.c              | 1605 +++++++++++++++++
 5 files changed, 1678 insertions(+), 4 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-ast2600.c
Signature · 1 line
2.34.1
author maintainer
#1 lore raw

PATCH v7 1/2 dt-bindings: i2c: aspeed: support for AST2600-i2cv2

Update ASPEED I2C maintainers email.
Add ast2600-i2cv2 compatible and aspeed,global-regs, aspeed,enable-dma and description for ast2600-i2cv2.

---
 .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 56 +++++++++++++++++--
 1 file changed, 52 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml +52 -4
index f597f73ccd87..00b92c97f432 100644
@@ -7,10 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 title: ASPEED I2C on the AST24XX, AST25XX, and AST26XX SoCs Device Tree Bindings
 
 maintainers:
-  - Rayn Chen <[email protected]>
-
-allOf:
-  - $ref: /schemas/i2c/i2c-controller.yaml#
+  - Ryan Chen <[email protected]>
 
 properties:
   compatible:
@@ -49,6 +46,25 @@ properties:
     description:
       states that there is another master active on this bus
 
+  aspeed,enable-dma:
+    type: boolean
+    description: |
+      I2C bus enable dma mode transfer.
+
+      ASPEED ast2600 platform equipped with 16 I2C controllers that share a
+      single DMA engine. DTS files can specify the data transfer mode to/from
+      the device, either DMA or programmed I/O. However, hardware limitations
+      may require a DTS to manually allocate which controller can use DMA mode.
+      The "aspeed,enable-dma" property allows control of this.
+
+      In cases where one the hardware design results in a specific
+      controller handling a larger amount of data, a DTS would likely
+      enable DMA mode for that one controller.
+
+  aspeed,global-regs:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: The phandle of i2c global register node.
+
 required:
   - reg
   - compatible
@@ -57,6 +73,26 @@ required:
 
 unevaluatedProperties: false
 
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: aspeed,ast2600-i2cv2
+
+    then:
+      properties:
+        reg:
+          minItems: 2
+      required:
+        - aspeed,global-regs
+    else:
+      properties:
+        aspeed,global-regs: false
+        aspeed,enable-dma: false
+
+
 examples:
   - |
     #include <dt-bindings/clock/aspeed-clock.h>
@@ -71,3 +107,15 @@ examples:
       interrupts = <0>;
       interrupt-parent = <&i2c_ic>;
     };
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    i2c1: i2c at 80 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      compatible = "aspeed,ast2600-i2cv2";
+      reg = <0x80 0x80>, <0xc00 0x20>;
+      interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+      aspeed,global-regs = <&i2c_global>;
+      clocks = <&syscon ASPEED_CLK_APB>;
+      resets = <&syscon ASPEED_RESET_I2C>;
+    };
Signature · 1 line
2.34.1
#2 lore raw

Re: [PATCH v7 1/2] dt-bindings: i2c: aspeed: support for AST2600-i2cv2

Quoted 1 line · Ryan Chen

Update ASPEED I2C maintainers email.

Everything is an update. Describe what is wrong in original maintainer email (e.g. Fix typo in maintainer's name and email).

Quoted 78 lines

Add ast2600-i2cv2 compatible and aspeed,global-regs, aspeed,enable-dma and description for ast2600-i2cv2.

---
 .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 56 +++++++++++++++++--
 1 file changed, 52 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml +40 -4
index f597f73ccd87..00b92c97f432 100644
@@ -7,10 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 title: ASPEED I2C on the AST24XX, AST25XX, and AST26XX SoCs Device Tree Bindings
 
 maintainers:
-  - Rayn Chen <[email protected]>
-
-allOf:
-  - $ref: /schemas/i2c/i2c-controller.yaml#
+  - Ryan Chen <[email protected]>
 
 properties:
   compatible:
@@ -49,6 +46,25 @@ properties:
     description:
       states that there is another master active on this bus
 
+  aspeed,enable-dma:
+    type: boolean
+    description: |
+      I2C bus enable dma mode transfer.
+
+      ASPEED ast2600 platform equipped with 16 I2C controllers that share a
+      single DMA engine. DTS files can specify the data transfer mode to/from
+      the device, either DMA or programmed I/O. However, hardware limitations
+      may require a DTS to manually allocate which controller can use DMA mode.
+      The "aspeed,enable-dma" property allows control of this.
+
+      In cases where one the hardware design results in a specific
+      controller handling a larger amount of data, a DTS would likely
+      enable DMA mode for that one controller.
+
+  aspeed,global-regs:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: The phandle of i2c global register node.
+
 required:
   - reg
   - compatible
@@ -57,6 +73,26 @@ required:
 
 unevaluatedProperties: false
 
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: aspeed,ast2600-i2cv2
+
+    then:
+      properties:
+        reg:
+          minItems: 2
+      required:
+        - aspeed,global-regs
+    else:
+      properties:
+        aspeed,global-regs: false
+        aspeed,enable-dma: false
+
+
 examples:
   - |
     #include <dt-bindings/clock/aspeed-clock.h>

Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml examples: · line 116

Quoted 10 lines
Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml +7 -0
@@ -71,3 +107,15 @@ examples:
       interrupts = <0>;
       interrupt-parent = <&i2c_ic>;
     };
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    i2c1: i2c at 80 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      compatible = "aspeed,ast2600-i2cv2";
+      reg = <0x80 0x80>, <0xc00 0x20>;

Compatible and reg are always first properties in DTS.

Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml examples: · line 118

Quoted 2 lines
Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml +2 -0
+      interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+      aspeed,global-regs = <&i2c_global>;

Best regards,
Krzysztof

author maintainer
#3 lore raw

PATCH v7 2/2 i2c: aspeed: support ast2600 i2c new register mode driver

In reply to Ryan Chen

Add i2c new register mode driver to support AST2600 i2c new register mode. AST2600 i2c controller have legacy and new register mode. The new register mode have global register support 4 base clock for scl clock selection, and new clock divider mode. The i2c new register mode have separate register set to control i2c master and slave.

---
 MAINTAINERS                      |    9 +
 drivers/i2c/busses/Kconfig       |   11 +
 drivers/i2c/busses/Makefile      |    1 +
 drivers/i2c/busses/i2c-ast2600.c | 1605 ++++++++++++++++++++++++++++++
 4 files changed, 1626 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-ast2600.c
MAINTAINERS +9 -0
index 749710e22b4d..433ec2309441 100644
@@ -1924,6 +1924,15 @@ F:	Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.
 F:	drivers/i2c/busses/i2c-aspeed.c
 F:	drivers/irqchip/irq-aspeed-i2c-ic.c
 
+ARM/ASPEED I2CV2 DRIVER
+M:	Ryan Chen <[email protected]>
+R:	Andrew Jeffery <[email protected]>
+L:	linux-i2c at vger.kernel.org
+L:	openbmc at lists.ozlabs.org (moderated for non-subscribers)
+S:	Maintained
+F:	Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
+F:	drivers/i2c/busses/i2c-ast2600.c
+
 ARM/ASPEED MACHINE SUPPORT
 M:	Joel Stanley <[email protected]>
 R:	Andrew Jeffery <[email protected]>
drivers/i2c/busses/Kconfig +11 -0
index 7284206b278b..4f6935866257 100644
@@ -389,6 +389,17 @@ config I2C_ALTERA
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-altera.
 
+config I2C_AST2600
+	tristate "Aspeed I2C v2 Controller"
+	depends on ARCH_ASPEED || COMPILE_TEST
+	select I2C_SMBUS
+	help
+	  If you say yes to this option, support will be included for the
+	  Aspeed I2C controller with new register set.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-ast2600.
+
 config I2C_ASPEED
 	tristate "Aspeed I2C Controller"
 	depends on ARCH_ASPEED || COMPILE_TEST
drivers/i2c/busses/Makefile +1 -0
index c5cac15f075c..d0ab4d45781c 100644
@@ -38,6 +38,7 @@ obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o
 obj-$(CONFIG_I2C_ALTERA)	+= i2c-altera.o
 obj-$(CONFIG_I2C_AMD_MP2)	+= i2c-amd-mp2-pci.o i2c-amd-mp2-plat.o
 obj-$(CONFIG_I2C_ASPEED)	+= i2c-aspeed.o
+obj-$(CONFIG_I2C_AST2600)	+= i2c-ast2600.o
 obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
 i2c-at91-objs			:= i2c-at91-core.o i2c-at91-master.o
 ifeq ($(CONFIG_I2C_AT91_SLAVE_EXPERIMENTAL),y)
drivers/i2c/busses/i2c-ast2600.c new file +1605 -0

1,605 lines not on this page · raw message

Signature · 1 line
2.34.1
maintainer
#4 lore raw

Re: [PATCH v7 2/2] i2c: aspeed: support ast2600 i2c new register mode driver

Hi Ryan,

[...]

drivers/i2c/busses/i2c-ast2600.c line 1497

Quoted 3 lines
drivers/i2c/busses/i2c-ast2600.c new file +3 -0
+	ret = of_property_read_u32(dev->of_node,
+							"i2c-scl-clk-low-timeout-us",
+							&i2c_bus->timeout);

in your v6 patch this was a boolean value. If you need to keep it boolean you have the "i2c-scl-has-clk-low-timeout".

drivers/i2c/busses/i2c-ast2600.c line 1505

Quoted 8 lines
drivers/i2c/busses/i2c-ast2600.c new file +8 -0
+	if (ret < 0) {
+		i2c_bus->timeout = 0;
+	} else {
+		/* i2c timeout counter: use base clk4 1Mhz
+		 * per unit: 1/(1000/4096) = 4096us
+		 */
+		i2c_bus->timeout /= 4096;
+	}

Can you please run checkpatch.pl before sending the patch?

[...]

drivers/i2c/busses/i2c-ast2600.c line 1569

Quoted 7 lines
drivers/i2c/busses/i2c-ast2600.c new file +7 -0
+	dev_info(dev, "%s [%d]: adapter [%d khz] mode [%d]\n",
+		 dev->of_node->name, i2c_bus->adap.nr, i2c_bus->bus_frequency / 1000,
+		 i2c_bus->mode);
+
+	return 0;
+
+	return ret;

can you also please do some cleanups before sending the patch?

Thanks,
Andi

author maintainer
#5 lore raw

RE: [PATCH v7 2/2] i2c: aspeed: support ast2600 i2c new register mode driver

Hello Andi,

Quoted 15 lines

-----Original Message-----
From: Andi Shyti <[email protected]>
Sent: Tuesday, March 28, 2023 9:12 PM
To: Ryan Chen <[email protected]>
Cc: jk at codeconstruct.com.au; openbmc at lists.ozlabs.org; linux-arm-kernel at lists.infradead.org; linux-i2c at vger.kernel.org; Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Joel Stanley <[email protected]>; Andrew Jeffery <[email protected]>; Linus Walleij <[email protected]>; linux-aspeed at lists.ozlabs.org
Subject: Re: [PATCH v7 2/2] i2c: aspeed: support ast2600 i2c new register mode driver

Hi Ryan,

Quoted 8 lines

[...]

drivers/i2c/busses/i2c-ast2600.c line 1497

Quoted 3 lines
drivers/i2c/busses/i2c-ast2600.c new file +3 -0
+	ret = of_property_read_u32(dev->of_node,
+							"i2c-scl-clk-low-timeout-us",
+							&i2c_bus->timeout);

in your v6 patch this was a boolean value. If you need to keep it boolean you have the "i2c-scl-has-clk-low-timeout".

Thank your remind, I will keep " i2c-scl-clk-low-timeout-us", if property is empty, will disable it. If not, will enable the timeout. It is ok for me.

Quoted 17 lines

drivers/i2c/busses/i2c-ast2600.c line 1505

Quoted 9 lines
drivers/i2c/busses/i2c-ast2600.c new file +8 -0
+	if (ret < 0) {
+		i2c_bus->timeout = 0;
+	} else {
+		/* i2c timeout counter: use base clk4 1Mhz
+		 * per unit: 1/(1000/4096) = 4096us
+		 */
+		i2c_bus->timeout /= 4096;
+	}

Can you please run checkpatch.pl before sending the patch?

[...]

Review

Quoted 2 lines
(quoted diff) +2 -0
+	dev_info(dev, "%s [%d]: adapter [%d khz] mode [%d]\n",
+		 dev->of_node->name, i2c_bus->adap.nr, i2c_bus->bus_frequency /

1000,

Quoted 7 lines

drivers/i2c/busses/i2c-ast2600.c line 1569

Quoted 5 lines
drivers/i2c/busses/i2c-ast2600.c new file +5 -0
+		 i2c_bus->mode);
+
+	return 0;
+
+	return ret;

can you also please do some cleanups before sending the patch?

Sorry, will update these two. And send next patch.

Best Regards,
Ryan.