public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v27 0/4] Add ASPEED AST2600 I2C controller driver
@ 2026-03-24  3:06 Ryan Chen
  2026-03-24  3:06 ` [PATCH v27 1/4] dt-bindings: i2c: Split AST2600 binding into a new YAML Ryan Chen
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Ryan Chen @ 2026-03-24  3:06 UTC (permalink / raw)
  To: jk, andriy.shevchenko, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
	Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
  Cc: linux-i2c, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, openbmc, Ryan Chen

This series adds support for the AST2600 I2C controller “new register
set” implementation.

The AST2600 I2C controller introduces a revised register layout which
separates controller and target functionality into distinct register
blocks, and extends clock divider configuration, packet-based transfer
support, and DMA capabilities compared to the legacy mixed register
layout used on earlier ASPEED SoCs.

The current driver implementation for the AST2600 I2C peripheral is
through the hardware's "compatibility mode", which exposes a register
set that matches the previous generation hardware (AST2500 and earlier).

Instead, add a driver that works in new-register-set mode, to allow the
new features, and will provide support for future hardware that will
not implement compatibility mode.

In order to support the new mode, we need a couple of DT binding changes
to reflect the expanded hardware interfaces: references to a global
register set, and buffer mode selection. Since the binding still
represents the same (AST2600 SoC) physical hardware, we continue to use
the existing compatible string of "aspeed,ast2600-i2c-bus".

However: since we're changing semantics for an existing binding, we
allow backwards compatibility by selecting on presence/absence of the
newly-added properties, and fall back to the old driver (ie., in
compatibility mode) when we detect a DT using the old binding spec.

Specifically:

- ast2600-i2c-bus nodes that provide the `aspeed,global-regs` property
  (which is mandatory in the new binding and absent in the legacy
  binding) will be successfully probed by the new driver

- ast2600-i2c-bus nodes without `aspeed,global-regs` continue to use the
  existing driver (in legacy register mode), ensuring that platforms
  with the current DTBs remain functional

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v27:
- 1/4 use aspeed,enable-dma instead aspeed,transfer-mode.
- 2/4 remove aspeed,transfer-mode selection instad aspeed,transfer-mode
- 2/4 add sysfs for xfer mode.
- Link to v26: https://lore.kernel.org/r/20260309-upstream_i2c-v26-0-5fedcff8ffe8@aspeedtech.com

Changes in v26:
- 1/4: binding reworks based on review feedback
- Link to v25: https://lore.kernel.org/r/20260225-upstream_i2c-v25-0-9f4bdd954f3f@aspeedtech.com

Changes in v25:
- Use b4 to send series.
- Rebase on v7.0-rc1.
- Clarify cover letter and commit logs based on review feedback.
- Remove the i2c-aspeed-core multiplexer infrastructure and
  implement driver selection via conditional -ENODEV handling
  in individual probe() functions.
- 3/4: incorporate review feedback and refactor new driver
- Link to v24: https://lore.kernel.org/r/20251118014034.820988-1-ryan_chen@aspeedtech.com

Changes in v24:
- aspeed,ast2600-i2c.yaml
 - fix make dt_binding_check blank warning.
- Link to v23: https://lore.kernel.org/all/20251117025040.3622984-1-ryan_chen@aspeedtech.com/

Changes in v23:
- update typo patch (1/4) commit message.
- aspeed,ast2600-i2c.yaml
 - update reg and description.
- i2c-ast2600.c controller
 - replace ast2600_select_i2c_clock to ast2600_i2c_ac_timing_config.
- i2c-ast2600.c target
 - I2C_TARGET_MSG_BUF_SIZE 256 to 4096
 - remove blank line.
 - refine Master comment description to controller
- Link to v22: https://lore.kernel.org/all/20251112085649.1903631-1-ryan_chen@aspeedtech.com/

Changes in v22:
- update patch (1/4) commit message add dts example reason.
- aspeed,ast2600-i2c.yaml @patch (1/4)
 - rename ast2600-i2c.yaml to aspeed,ast2600-i2c.yaml.
 - update reg, clock-frequency description.
- aspeed,ast2600-i2c.yaml @patch (2/4)
 - aspeed,transfer-mode, aspeed,transfer-mode add for ast2600.
- i2c-aspeed-core.c,h @patch (3/4)
 - add i2c-aspeed-core allow both old and new device trees using the
   same compatible string "aspeed,ast2600-i2c-bus".
- Link to v21: https://lore.kernel.org/all/20251027061240.3427875-1-ryan_chen@aspeedtech.com/

Changes in v21:
- update patch (1/4) commit message
- i2c-ast2600.c
 - move rst to local variable in ast2600_i2c_probe().
- Link to v20: https://lore.kernel.org/all/20251021013548.2375190-1-ryan_chen@aspeedtech.com/

Changes in v20:
- ast2600-i2c.yaml
 - fix warning at make dt_binding_check.
- Link to v19: https://lore.kernel.org/all/20251020013200.1858325-1-ryan_chen@aspeedtech.com/

Changes in v19:
- Split AST2600 binding into its own YAML file
 - Removed `aspeed,ast2600-i2c-bus` from `aspeed,i2c.yaml`
 - Added `aspeed,global-regs` and `aspeed,transfer-mode` to AST2600 binding
- Link to v18: https://lore.kernel.org/all/20250820051832.3605405-1-ryan_chen@aspeedtech.com/

Changes in v18:
- refine patch (1/3) commit message (reason for commit not list.)
- i2c-ast2600.c
 - remove redundant reset_control_deassert in driver probe.
 - remove reset_control_assert(i2c_bus->rst) in driver remove.
- Link to v17: https://lore.kernel.org/all/20250814084156.1650432-1-ryan_chen@aspeedtech.com/

Changes in v17:
- move i2c new mode register and feature into driver commit message.
- aspeed,i2c.yaml
 - remove multi-master properties.
 - use aspeed,transfer-mode properties for aspeed,enable-byte/enable-dma.
-i2c-ast2600.c
 - rename dma_safe_buf to controller_dma_safe_buf.
 - fix ast2600_i2c_recover_bus return overflow warnings.
 - add ast2600_i2c_target_packet_buff_irq unhandle case.
 - add parameter "cmd" in ast2600_i2c_setup_dma_rx,
   ast2600_i2c_setup_buff_rx, ast2600_i2c_setup_byte_rx
 - use reset_control_deassert replace
   devm_reset_control_get_shared_deasserted.
 - useaspeed,transfer-mode properties for transfer mode setting.
 - change compatible = "aspeed,ast2600-i2cv2" to "aspeed,ast2600-i2c-bus".
- Link to v16: https://lore.kernel.org/all/20250224055936.1804279-1-ryan_chen@aspeedtech.com/

Changes in v16:
- aspeed,i2c.yaml: add aspeed,enable-byte properties for force byte mode.
- i2c-ast2600.c
 - change include asm/unaligned.h to linux/unaligned.h.
 - add reset timeout councter when slave active timeout.
 - modify issue i2c_recovery_bus before slave re-enable.
 - add aspeed,enable-byte properties.
- Link to v15: https://lore.kernel.org/all/20241007035235.2254138-1-ryan_chen@aspeedtech.com/

Changes in v15:
- i2c-ast2600.c
 - add include unaligned.h
 - rename all master -> controller, slave -> target.
 - keep multi-master to align property.
 - remove no used element in ast2600_i2c_bus.
- Link to v14: https://lore.kernel.org/all/20241002070213.1165263-1-ryan_chen@aspeedtech.com/

Changes in v14:
- aspeed,i2c.yaml
 - v13 change people reviewed-by tag, v14 fixed to original people tag,
   modify to Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
 - struct ast2600_i2c_bus layout optimal.
 - ast2600_select_i2c_clock refine.
 - ast2600_i2c_recover_bus overridden fix.
 - dma_mapping_error() returned error code shadowed modify.
 - buffer register in a 4-byte aligned simplified
 - remove smbus alert
- Link to v13: https://lore.kernel.org/all/20240819092850.1590758-1-ryan_chen@aspeedtech.com/

Changes in v13:
 - separate i2c master and slave driver to be two patchs.
 - modify include header list, add bits.h include. remove of*.h
 - modify (((x) >> 24) & GENMASK(5, 0)) to (((x) & GENMASK(29, 24)) >> 24)
 - modify ast2600_select_i2c_clock function implement.
 - modify ast2600_i2c_recover_bus function u32 claim to
   u32 state = readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF);
- Link to v12: https://lore.kernel.org/all/20230714074522.23827-1-ryan_chen@aspeedtech.com/

Changes in v12:
- aspeed,i2c.yaml
 - add Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
- i2c-ast2600.c
 - update include by alphabetical order
 - make just a one TAB and put the last two lines on the single one
 - remove no used timing_table structre
 - remove enum explicit assinment
 - rewritten to avoid this and using loop in ast2600_select_i2c_clock
 - use GENMASK for most 0xffff
 - remove too many parentheses
 - use str_read_write replace read write string
 - remove redundant blank line after ast2600_i2c_bus_of_table
 - fix wrong multi-line style of the comment
 - use macro for i2c standard speeds
 - remove useless noise dev_info
- Link to v11: https://lore.kernel.org/all/20230430041712.3247998-1-ryan_chen@aspeedtech.com/

Changes in v11:
- aspeed,i2c.yaml
 - no change, the same with v10.
- i2c-ast2600.c
 - modify alert_enable from int -> boolean.
 - modify dbg string recovery -> recover.
 - remove no need to init 0.
 - remove new line after break.
 - remove unneeded empty line.
 - modify dma_alloc_coherent to dmam_alloc_coherent
 - modify probe nomem return dev_err_probe
 - modify i2c_add_adapter to devm_i2c_adapter
 - modify checkpatch: Alignment should match open parenthesis
 - modify checkpatch: braces {} should be used on all arms of this statement
 - modify checkpatch: Unbalanced braces around else statement
- Link to v10: https://lore.kernel.org/all/20230415012848.1777768-1-ryan_chen@aspeedtech.com/

Changes in v10:
- aspeed,i2c.yaml
 - move unevaluatedProperties after allOf.
 - remove extra one blank line.
- i2c-ast2600.c
 - no change, the same with v8.
- Link to v9: https://lore.kernel.org/all/20230405022825.333246-1-ryan_chen@aspeedtech.com/

Changes in v9:
- aspeed,i2c.yaml
 - backoff to v7.
  - no fix typo in maintainer's name and email. this would be another patch.
  - no remove address-cells, size-cells, this would be another patch.
 - use aspeed,enable-dma property instead of aspeed,xfer-mode selection.
 - fix allOf and else false properties for aspeed,ast2600-i2cv2.
- i2c-ast2600.c
 - no change, the same with v8
- Link to v8: https://lore.kernel.org/all/20230330073259.485606-1-ryan_chen@aspeedtech.com/

Changes in v8:
- aspeed,i2c.yaml
 - modify commit message.
 - Fix typo in maintainer's name and email.
 - remove address-cells, size-cells.
- i2c-ast2600.c
 - move "i2c timeout counter" comment description before property_read.
 - remove redundant code "return ret" in probe end.
- Link to v7: https://lore.kernel.org/all/20230327092524.3916389-1-ryan_chen@aspeedtech.com/

Changes in 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.
- Link to v6: https://lore.kernel.org/all/20230226031321.3126756-1-ryan_chen@aspeedtech.com/

Changes in 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
- Link to v5: https://lore.kernel.org/all/20230220061745.1973981-1-ryan_chen@aspeedtech.com/

Changes in 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.
- Link to v4: https://lore.kernel.org/all/20230201103359.1742140-1-ryan_chen@aspeedtech.com/

Changes in 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
- Link to v3: https://lore.kernel.org/all/20220516064900.30517-1-ryan_chen@aspeedtech.com/

Changes in v3:
- fix i2c global clock divide default value.
- remove i2c slave no used dev_dbg info.
- Link to v2: https://lore.kernel.org/all/20220413101735.27678-1-ryan_chen@aspeedtech.com/

Changes in 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.
- Link to v1: https://lore.kernel.org/all/20220323004009.943298-1-ryan_chen@aspeedtech.com/

---
Ryan Chen (4):
      dt-bindings: i2c: Split AST2600 binding into a new YAML
      dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties
      i2c: ast2600: Add controller driver for AST2600 new register set
      i2c: ast2600: Add target mode support

 .../bindings/i2c/aspeed,ast2600-i2c.yaml           |   79 +
 .../devicetree/bindings/i2c/aspeed,i2c.yaml        |    3 +-
 drivers/i2c/busses/Makefile                        |    2 +-
 drivers/i2c/busses/i2c-aspeed.c                    |    5 +
 drivers/i2c/busses/i2c-ast2600.c                   | 1627 ++++++++++++++++++++
 5 files changed, 1713 insertions(+), 3 deletions(-)
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20260223-upstream_i2c-ebd07f89739c

Best regards,
-- 
Ryan Chen <ryan_chen@aspeedtech.com>



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

* [PATCH v27 1/4] dt-bindings: i2c: Split AST2600 binding into a new YAML
  2026-03-24  3:06 [PATCH v27 0/4] Add ASPEED AST2600 I2C controller driver Ryan Chen
@ 2026-03-24  3:06 ` Ryan Chen
  2026-03-24  3:06 ` [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties Ryan Chen
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: Ryan Chen @ 2026-03-24  3:06 UTC (permalink / raw)
  To: jk, andriy.shevchenko, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
	Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
  Cc: linux-i2c, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, openbmc, Ryan Chen

The AST2600 I2C controller introduces a completely new register layout
with separate controller and target register blocks, unlike the mixed
register layout used by AST2400/AST2500.

Move AST2600 I2C binding from aspeed,i2c.yaml to a dedicated
aspeed,ast2600-i2c.yaml schema.

Besides the split, this also adjusts for AST2600-specific requirements.
- require two reg regions (controller register block + buffer block)
- use clock-frequency for bus speed description
- interrupts are required on AST2600
- use correct DTS coding style in example

No compatible strings are changed.

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v26:
- commit message: include details of changes from original binding
- fix example property ordering to follow DTS coding style
- use consistent "AST2600" naming
---
 .../bindings/i2c/aspeed,ast2600-i2c.yaml           | 62 ++++++++++++++++++++++
 .../devicetree/bindings/i2c/aspeed,i2c.yaml        |  3 +-
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
new file mode 100644
index 000000000000..de2c359037da
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/aspeed,ast2600-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ASPEED I2C on the AST2600 SoCs
+
+maintainers:
+  - Ryan Chen <ryan_chen@aspeedtech.com>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - aspeed,ast2600-i2c-bus
+
+  reg:
+    items:
+      - description: controller registers
+      - description: controller buffer space
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-frequency:
+    description: Desired operating frequency of the I2C bus in Hz.
+    minimum: 500
+    maximum: 4000000
+    default: 100000
+
+  resets:
+    maxItems: 1
+
+required:
+  - reg
+  - compatible
+  - clocks
+  - resets
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/aspeed-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    i2c@80 {
+        compatible = "aspeed,ast2600-i2c-bus";
+        reg = <0x80 0x80>, <0xc00 0x20>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        clocks = <&syscon ASPEED_CLK_APB>;
+        resets = <&syscon ASPEED_RESET_I2C>;
+        clock-frequency = <100000>;
+        interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+    };
diff --git a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
index 5b9bd2feda3b..d4e4f412feba 100644
--- a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/i2c/aspeed,i2c.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: ASPEED I2C on the AST24XX, AST25XX, and AST26XX SoCs
+title: ASPEED I2C on the AST24XX, AST25XX SoCs
 
 maintainers:
   - Rayn Chen <rayn_chen@aspeedtech.com>
@@ -17,7 +17,6 @@ properties:
     enum:
       - aspeed,ast2400-i2c-bus
       - aspeed,ast2500-i2c-bus
-      - aspeed,ast2600-i2c-bus
 
   reg:
     minItems: 1

-- 
2.34.1



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

* [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties
  2026-03-24  3:06 [PATCH v27 0/4] Add ASPEED AST2600 I2C controller driver Ryan Chen
  2026-03-24  3:06 ` [PATCH v27 1/4] dt-bindings: i2c: Split AST2600 binding into a new YAML Ryan Chen
@ 2026-03-24  3:06 ` Ryan Chen
  2026-03-24  3:11   ` Jeremy Kerr
  2026-03-25  1:46   ` Rob Herring (Arm)
  2026-03-24  3:06 ` [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set Ryan Chen
  2026-03-24  3:06 ` [PATCH v27 4/4] i2c: ast2600: Add target mode support Ryan Chen
  3 siblings, 2 replies; 17+ messages in thread
From: Ryan Chen @ 2026-03-24  3:06 UTC (permalink / raw)
  To: jk, andriy.shevchenko, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
	Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
  Cc: linux-i2c, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, openbmc, Ryan Chen

The AST2600 I2C controller supports three transfer modes (byte, buffer,
DMA). Add "aspeed,transfer-mode" so DT can select the preferred transfer
method per controller instance. Also add the "aspeed,global-regs"
phandle to reference the AST2600 global registers syscon/regmap used by
the controller.

These properties apply only to the AST2600 binding and are not part of
the legacy binding, which uses a mixed controller/target register layout
and does not have the split register blocks or these new configuration
registers. Legacy DTs remain unchanged.

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v27:
- change aspeed,transfer-mode to aspeed,enable-dma.
---
 .../devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml     | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
index de2c359037da..38da6fc6424f 100644
--- a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
@@ -37,6 +37,21 @@ properties:
   resets:
     maxItems: 1
 
+  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.
+
+  aspeed,global-regs:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle reference to the i2c global syscon node, containing the
+      SoC-common i2c register set.
+
 required:
   - reg
   - compatible
@@ -59,4 +74,6 @@ examples:
         resets = <&syscon ASPEED_RESET_I2C>;
         clock-frequency = <100000>;
         interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+        aspeed,global-regs = <&i2c_global>;
+        aspeed,transfer-mode = "buffer";
     };

-- 
2.34.1



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

* [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set
  2026-03-24  3:06 [PATCH v27 0/4] Add ASPEED AST2600 I2C controller driver Ryan Chen
  2026-03-24  3:06 ` [PATCH v27 1/4] dt-bindings: i2c: Split AST2600 binding into a new YAML Ryan Chen
  2026-03-24  3:06 ` [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties Ryan Chen
@ 2026-03-24  3:06 ` Ryan Chen
  2026-03-24  3:37   ` Jeremy Kerr
                     ` (2 more replies)
  2026-03-24  3:06 ` [PATCH v27 4/4] i2c: ast2600: Add target mode support Ryan Chen
  3 siblings, 3 replies; 17+ messages in thread
From: Ryan Chen @ 2026-03-24  3:06 UTC (permalink / raw)
  To: jk, andriy.shevchenko, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
	Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
  Cc: linux-i2c, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, openbmc, Ryan Chen

The AST2600 introduces a new I2C controller register layout, selectable
at runtime via global control registers. Compared to the legacy layout
used on AST2400/AST2500, the new layout separates controller (master)
and target (slave) registers and adds support for packet-based transfers

The new register set extends the hardware capabilities with:

- Enhanced clock divider configuration for improved timing precision
- tCKHighMin timing control for SCL high pulse width
- Dual pool buffer mode (separate Tx/Rx buffers)
- Extended DMA support with larger buffer size and alignment handling
- Dedicated DMA buffers for controller and target directions
- Hardware-assisted bus recovery and timeout mechanisms

This patch adds an AST2600-specific I2C controller driver implementing
the new register layout, including support for packet-based transfers
and byte, buffer and DMA transfer modes.

The legacy and new register layouts represent the same AST2600 I2C
controller IP and therefore share the existing compatible string:

  "aspeed,ast2600-i2c-bus"

To preserve DT ABI compatibility, driver selection is performed at probe
time based on DT contents. In particular, the new binding requires the
`aspeed,global-regs` phandle, which is absent from legacy DTBs:

- The new driver only probes successfully when `aspeed,global-regs` is
  present.

- The existing i2c-aspeed driver returns -ENODEV for AST2600 nodes that
  provide `aspeed,global-regs`, allowing the new driver to bind.

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>

---
Changes in v27:
- remove aspeed,transfer-mode selection instead aspeed,dma-mode.
- add sysfs for xfer mode.
Changes in v25:
- Rename AST2600_I2CM_SMBUS_ALT to AST2600_I2CM_SMBUS_ALERT.
- Refactor transfer mode handling using setup_tx/setup_rx helpers.
- Rework DMA handling to use pre-allocated buffers and reduce
  mapping overhead in interrupt context.
- Fix IRQ status checks to use consistent (sts & value) style.
- Move device_property_read_bool() to probe().
- Improve probe error handling.
- Handle timeout condition in target_byte_irq().
- Rename "package" to "packet".
- Remove target reset when master wait_for_completion_timeout().
---
 drivers/i2c/busses/Makefile      |    2 +-
 drivers/i2c/busses/i2c-aspeed.c  |    5 +
 drivers/i2c/busses/i2c-ast2600.c | 1064 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 1070 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 547123ab351f..ece201a67d41 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -37,7 +37,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_AMD_ASF)	+= i2c-amd-asf-plat.o
-obj-$(CONFIG_I2C_ASPEED)	+= i2c-aspeed.o
+obj-$(CONFIG_I2C_ASPEED)	+= i2c-aspeed.o i2c-ast2600.o
 obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
 i2c-at91-y			:= i2c-at91-core.o i2c-at91-master.o
 i2c-at91-$(CONFIG_I2C_AT91_SLAVE_EXPERIMENTAL)	+= i2c-at91-slave.o
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index a26b74c71206..8286fd2cd130 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -22,6 +22,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
 
@@ -1002,6 +1003,10 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	struct clk *parent_clk;
 	int irq, ret;
 
+	if (device_is_compatible(&pdev->dev, "aspeed,ast2600-i2c-bus") &&
+	    device_property_present(&pdev->dev, "aspeed,global-regs"))
+		return -ENODEV;
+
 	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
 	if (!bus)
 		return -ENOMEM;
diff --git a/drivers/i2c/busses/i2c-ast2600.c b/drivers/i2c/busses/i2c-ast2600.c
new file mode 100644
index 000000000000..cf3006eaded9
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ast2600.c
@@ -0,0 +1,1064 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ASPEED AST2600 new register set I2C controller driver
+ *
+ * Copyright (C) 2026 ASPEED Technology Inc.
+ */
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/i2c-smbus.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/minmax.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/string_helpers.h>
+#include <linux/unaligned.h>
+
+#define AST2600_I2CG_ISR			0x00
+#define AST2600_I2CG_SLAVE_ISR		0x04
+#define AST2600_I2CG_OWNER		0x08
+#define AST2600_I2CG_CTRL		0x0C
+#define AST2600_I2CG_CLK_DIV_CTRL	0x10
+
+#define AST2600_I2CG_SLAVE_PKT_NAK	BIT(4)
+#define AST2600_I2CG_M_S_SEPARATE_INTR	BIT(3)
+#define AST2600_I2CG_CTRL_NEW_REG	BIT(2)
+#define AST2600_I2CG_CTRL_NEW_CLK_DIV	BIT(1)
+#define AST2600_GLOBAL_INIT	\
+	(AST2600_I2CG_CTRL_NEW_REG | AST2600_I2CG_CTRL_NEW_CLK_DIV)
+/*
+ * APB clk : 100Mhz
+ * div	: scl		: baseclk [APB/((div/2) + 1)] : tBuf [1/bclk * 16]
+ * I2CG10[31:24] base clk4 for i2c auto recovery timeout counter (0xC6)
+ * I2CG10[23:16] base clk3 for Standard-mode (100Khz) min tBuf 4.7us
+ * 0x3c : 100.8Khz	: 3.225Mhz					  : 4.96us
+ * 0x3d : 99.2Khz	: 3.174Mhz					  : 5.04us
+ * 0x3e : 97.65Khz	: 3.125Mhz					  : 5.12us
+ * 0x40 : 97.75Khz	: 3.03Mhz					  : 5.28us
+ * 0x41 : 99.5Khz	: 2.98Mhz					  : 5.36us (default)
+ * I2CG10[15:8] base clk2 for Fast-mode (400Khz) min tBuf 1.3us
+ * 0x12 : 400Khz	: 10Mhz						  : 1.6us
+ * I2CG10[7:0] base clk1 for Fast-mode Plus (1Mhz) min tBuf 0.5us
+ * 0x08 : 1Mhz		: 20Mhz						  : 0.8us
+ */
+#define I2CCG_DIV_CTRL 0xC6411208
+
+/* 0x00 : I2CC Controller/Target Function Control Register  */
+#define AST2600_I2CC_FUN_CTRL		0x00
+#define AST2600_I2CC_SLAVE_ADDR_RX_EN		BIT(20)
+#define AST2600_I2CC_MASTER_RETRY_MASK		GENMASK(19, 18)
+#define AST2600_I2CC_MASTER_RETRY(x)		(((x) & GENMASK(1, 0)) << 18)
+#define AST2600_I2CC_BUS_AUTO_RELEASE		BIT(17)
+#define AST2600_I2CC_M_SDA_LOCK_EN			BIT(16)
+#define AST2600_I2CC_MULTI_MASTER_DIS		BIT(15)
+#define AST2600_I2CC_M_SCL_DRIVE_EN			BIT(14)
+#define AST2600_I2CC_MSB_STS				BIT(9)
+#define AST2600_I2CC_SDA_DRIVE_1T_EN		BIT(8)
+#define AST2600_I2CC_M_SDA_DRIVE_1T_EN		BIT(7)
+#define AST2600_I2CC_M_HIGH_SPEED_EN		BIT(6)
+/* reserver 5 : 2 */
+#define AST2600_I2CC_SLAVE_EN			BIT(1)
+#define AST2600_I2CC_MASTER_EN			BIT(0)
+
+/* 0x04 : I2CC Controller/Target Clock and AC Timing Control Register #1 */
+#define AST2600_I2CC_AC_TIMING		0x04
+#define AST2600_I2CC_TTIMEOUT(x)			(((x) & GENMASK(4, 0)) << 24)
+#define AST2600_I2CC_TCKHIGHMIN(x)			(((x) & GENMASK(3, 0)) << 20)
+#define AST2600_I2CC_TCKHIGH(x)			(((x) & GENMASK(3, 0)) << 16)
+#define AST2600_I2CC_TCKLOW(x)			(((x) & GENMASK(3, 0)) << 12)
+#define AST2600_I2CC_THDDAT(x)			(((x) & GENMASK(1, 0)) << 10)
+#define AST2600_I2CC_TOUTBASECLK(x)			(((x) & GENMASK(1, 0)) << 8)
+#define AST2600_I2CC_TBASECLK(x)			((x) & GENMASK(3, 0))
+#define AST2600_I2CC_AC_TIMING_MASK		GENMASK(23, 0)
+
+/* 0x08 : I2CC Controller/Target Transmit/Receive Byte Buffer Register */
+#define AST2600_I2CC_STS_AND_BUFF		0x08
+#define AST2600_I2CC_TX_DIR_MASK			GENMASK(31, 29)
+#define AST2600_I2CC_SDA_OE				BIT(28)
+#define AST2600_I2CC_SDA_O				BIT(27)
+#define AST2600_I2CC_SCL_OE				BIT(26)
+#define AST2600_I2CC_SCL_O				BIT(25)
+
+#define AST2600_I2CC_SCL_LINE_STS			BIT(18)
+#define AST2600_I2CC_SDA_LINE_STS			BIT(17)
+#define AST2600_I2CC_BUS_BUSY_STS			BIT(16)
+
+#define AST2600_I2CC_GET_RX_BUFF(x)			(((x) >> 8) & GENMASK(7, 0))
+
+/* 0x0C : I2CC Controller/Target Pool Buffer Control Register  */
+#define AST2600_I2CC_BUFF_CTRL		0x0C
+#define AST2600_I2CC_GET_RX_BUF_LEN(x)      (((x) & GENMASK(29, 24)) >> 24)
+#define AST2600_I2CC_SET_RX_BUF_LEN(x)		(((((x) - 1) & GENMASK(4, 0)) << 16) | BIT(0))
+#define AST2600_I2CC_SET_TX_BUF_LEN(x)		(((((x) - 1) & GENMASK(4, 0)) << 8) | BIT(0))
+#define AST2600_I2CC_GET_TX_BUF_LEN(x)      ((((x) & GENMASK(12, 8)) >> 8) + 1)
+
+/* 0x10 : I2CM Controller Interrupt Control Register */
+#define AST2600_I2CM_IER			0x10
+/* 0x14 : I2CM Controller Interrupt Status Register   : WC */
+#define AST2600_I2CM_ISR			0x14
+
+#define AST2600_I2CM_PKT_TIMEOUT			BIT(18)
+#define AST2600_I2CM_PKT_ERROR			BIT(17)
+#define AST2600_I2CM_PKT_DONE			BIT(16)
+
+#define AST2600_I2CM_BUS_RECOVER_FAIL		BIT(15)
+#define AST2600_I2CM_SDA_DL_TO			BIT(14)
+#define AST2600_I2CM_BUS_RECOVER			BIT(13)
+#define AST2600_I2CM_SMBUS_ALERT			BIT(12)
+
+#define AST2600_I2CM_SCL_LOW_TO			BIT(6)
+#define AST2600_I2CM_ABNORMAL			BIT(5)
+#define AST2600_I2CM_NORMAL_STOP			BIT(4)
+#define AST2600_I2CM_ARBIT_LOSS			BIT(3)
+#define AST2600_I2CM_RX_DONE			BIT(2)
+#define AST2600_I2CM_TX_NAK				BIT(1)
+#define AST2600_I2CM_TX_ACK				BIT(0)
+
+/* 0x18 : I2CM Controller Command/Status Register   */
+#define AST2600_I2CM_CMD_STS		0x18
+#define AST2600_I2CM_PKT_ADDR(x)			(((x) & GENMASK(6, 0)) << 24)
+#define AST2600_I2CM_PKT_EN				BIT(16)
+#define AST2600_I2CM_SDA_OE_OUT_DIR			BIT(15)
+#define AST2600_I2CM_SDA_O_OUT_DIR			BIT(14)
+#define AST2600_I2CM_SCL_OE_OUT_DIR			BIT(13)
+#define AST2600_I2CM_SCL_O_OUT_DIR			BIT(12)
+#define AST2600_I2CM_RECOVER_CMD_EN			BIT(11)
+
+#define AST2600_I2CM_RX_DMA_EN			BIT(9)
+#define AST2600_I2CM_TX_DMA_EN			BIT(8)
+/* Command Bit */
+#define AST2600_I2CM_RX_BUFF_EN			BIT(7)
+#define AST2600_I2CM_TX_BUFF_EN			BIT(6)
+#define AST2600_I2CM_STOP_CMD			BIT(5)
+#define AST2600_I2CM_RX_CMD_LAST			BIT(4)
+#define AST2600_I2CM_RX_CMD				BIT(3)
+
+#define AST2600_I2CM_TX_CMD				BIT(1)
+#define AST2600_I2CM_START_CMD			BIT(0)
+
+/* 0x1C : I2CM Controller DMA Transfer Length Register	 */
+#define AST2600_I2CM_DMA_LEN		0x1C
+/* Tx Rx support length 1 ~ 4096 */
+#define AST2600_I2CM_SET_RX_DMA_LEN(x)	((((x) & GENMASK(11, 0)) << 16) | BIT(31))
+#define AST2600_I2CM_SET_TX_DMA_LEN(x)	(((x) & GENMASK(11, 0)) | BIT(15))
+
+/* 0x20 : I2CS Target Interrupt Control Register   */
+#define AST2600_I2CS_IER			0x20
+/* 0x24 : I2CS Target Interrupt Status Register	 */
+#define AST2600_I2CS_ISR			0x24
+
+#define AST2600_I2CS_ADDR_INDICATE_MASK	GENMASK(31, 30)
+#define AST2600_I2CS_SLAVE_PENDING			BIT(29)
+
+#define AST2600_I2CS_WAIT_TX_DMA			BIT(25)
+#define AST2600_I2CS_WAIT_RX_DMA			BIT(24)
+
+#define AST2600_I2CS_ADDR3_NAK			BIT(22)
+#define AST2600_I2CS_ADDR2_NAK			BIT(21)
+#define AST2600_I2CS_ADDR1_NAK			BIT(20)
+
+#define AST2600_I2CS_ADDR_MASK			GENMASK(19, 18)
+#define AST2600_I2CS_PKT_ERROR			BIT(17)
+#define AST2600_I2CS_PKT_DONE			BIT(16)
+#define AST2600_I2CS_INACTIVE_TO			BIT(15)
+
+#define AST2600_I2CS_SLAVE_MATCH			BIT(7)
+#define AST2600_I2CS_ABNOR_STOP			BIT(5)
+#define AST2600_I2CS_STOP				BIT(4)
+#define AST2600_I2CS_RX_DONE_NAK			BIT(3)
+#define AST2600_I2CS_RX_DONE			BIT(2)
+#define AST2600_I2CS_TX_NAK				BIT(1)
+#define AST2600_I2CS_TX_ACK				BIT(0)
+
+/* 0x28 : I2CS Target CMD/Status Register   */
+#define AST2600_I2CS_CMD_STS		0x28
+#define AST2600_I2CS_ACTIVE_ALL			GENMASK(18, 17)
+#define AST2600_I2CS_PKT_MODE_EN			BIT(16)
+#define AST2600_I2CS_AUTO_NAK_NOADDR		BIT(15)
+#define AST2600_I2CS_AUTO_NAK_EN			BIT(14)
+
+#define AST2600_I2CS_ALT_EN				BIT(10)
+#define AST2600_I2CS_RX_DMA_EN			BIT(9)
+#define AST2600_I2CS_TX_DMA_EN			BIT(8)
+#define AST2600_I2CS_RX_BUFF_EN			BIT(7)
+#define AST2600_I2CS_TX_BUFF_EN			BIT(6)
+#define AST2600_I2CS_RX_CMD_LAST			BIT(4)
+
+#define AST2600_I2CS_TX_CMD				BIT(2)
+
+#define AST2600_I2CS_DMA_LEN		0x2C
+#define AST2600_I2CS_SET_RX_DMA_LEN(x)	(((((x) - 1) & GENMASK(11, 0)) << 16) | BIT(31))
+#define AST2600_I2CS_SET_TX_DMA_LEN(x)	((((x) - 1) & GENMASK(11, 0)) | BIT(15))
+
+/* I2CM Controller DMA Tx Buffer Register   */
+#define AST2600_I2CM_TX_DMA			0x30
+/* I2CM Controller DMA Rx Buffer Register	*/
+#define AST2600_I2CM_RX_DMA			0x34
+/* I2CS Target DMA Tx Buffer Register   */
+#define AST2600_I2CS_TX_DMA			0x38
+/* I2CS Target DMA Rx Buffer Register   */
+#define AST2600_I2CS_RX_DMA			0x3C
+
+#define AST2600_I2CS_ADDR_CTRL		0x40
+
+#define	AST2600_I2CS_ADDR3_MASK		GENMASK(22, 16)
+#define	AST2600_I2CS_ADDR2_MASK		GENMASK(14, 8)
+#define	AST2600_I2CS_ADDR1_MASK		GENMASK(6, 0)
+
+#define AST2600_I2CM_DMA_LEN_STS		0x48
+#define AST2600_I2CS_DMA_LEN_STS		0x4C
+
+#define AST2600_I2C_GET_TX_DMA_LEN(x)		((x) & GENMASK(12, 0))
+#define AST2600_I2C_GET_RX_DMA_LEN(x)        (((x) & GENMASK(28, 16)) >> 16)
+
+/* 0x40 : Target Device Address Register */
+#define AST2600_I2CS_ADDR3_ENABLE			BIT(23)
+#define AST2600_I2CS_ADDR3(x)			((x) << 16)
+#define AST2600_I2CS_ADDR2_ENABLE			BIT(15)
+#define AST2600_I2CS_ADDR2(x)			((x) << 8)
+#define AST2600_I2CS_ADDR1_ENABLE			BIT(7)
+#define AST2600_I2CS_ADDR1(x)			(x)
+
+#define I2C_TARGET_MSG_BUF_SIZE		4096
+
+#define AST2600_I2C_DMA_SIZE		4096
+
+#define CONTROLLER_TRIGGER_LAST_STOP	(AST2600_I2CM_RX_CMD_LAST | AST2600_I2CM_STOP_CMD)
+#define TARGET_TRIGGER_CMD	(AST2600_I2CS_ACTIVE_ALL | AST2600_I2CS_PKT_MODE_EN)
+
+#define AST_I2C_TIMEOUT_CLK		0x1
+
+enum xfer_mode {
+	BYTE_MODE,
+	BUFF_MODE,
+	DMA_MODE,
+};
+
+struct ast2600_i2c_bus {
+	struct i2c_adapter	adap;
+	struct device		*dev;
+	void __iomem		*reg_base;
+	struct regmap		*global_regs;
+	struct clk		*clk;
+	struct i2c_timings	timing_info;
+	struct completion	cmd_complete;
+	struct i2c_msg		*msgs;
+	u8			*controller_dma_buf;
+	dma_addr_t		controller_dma_addr;
+	u32			apb_clk;
+	u32			timeout;
+	int			irq;
+	int			cmd_err;
+	int			msgs_index;
+	int			msgs_count;
+	int			controller_xfer_cnt;
+	size_t			buf_index;
+	size_t			buf_size;
+	enum xfer_mode		mode;
+	bool			dma_available;
+	bool			multi_master;
+	/* Buffer mode */
+	void __iomem		*buf_base;
+	int (*setup_tx)(u32 cmd, struct ast2600_i2c_bus *i2c_bus);
+	int (*setup_rx)(u32 cmd, struct ast2600_i2c_bus *i2c_bus);
+};
+
+static void ast2600_i2c_ac_timing_config(struct ast2600_i2c_bus *i2c_bus)
+{
+	unsigned long base_clk[16];
+	int baseclk_idx = 0;
+	int divisor = 0;
+	u32 clk_div_reg;
+	u32 scl_low;
+	u32 scl_high;
+	u32 data;
+
+	regmap_read(i2c_bus->global_regs, AST2600_I2CG_CLK_DIV_CTRL, &clk_div_reg);
+
+	for (int i = 0; i < ARRAY_SIZE(base_clk); i++) {
+		if (i == 0)
+			base_clk[i] = i2c_bus->apb_clk;
+		else if (i < 5)
+			base_clk[i] = (i2c_bus->apb_clk * 2) /
+			   (((clk_div_reg >> ((i - 1) * 8)) & GENMASK(7, 0)) + 2);
+		else
+			base_clk[i] = base_clk[4] >> (i - 4);
+
+		if ((base_clk[i] / i2c_bus->timing_info.bus_freq_hz) <= 32) {
+			baseclk_idx = i;
+			divisor = DIV_ROUND_UP(base_clk[i], i2c_bus->timing_info.bus_freq_hz);
+			break;
+		}
+	}
+	baseclk_idx = min(baseclk_idx, 15);
+	divisor = min(divisor, 32);
+	scl_low = min(divisor * 9 / 16 - 1, 15);
+	scl_high = (divisor - scl_low - 2) & GENMASK(3, 0);
+	data = (scl_high - 1) << 20 | scl_high << 16 | scl_low << 12 | baseclk_idx;
+	if (i2c_bus->timeout) {
+		data |= AST2600_I2CC_TOUTBASECLK(AST_I2C_TIMEOUT_CLK);
+		data |= AST2600_I2CC_TTIMEOUT(i2c_bus->timeout);
+	}
+
+	writel(data, i2c_bus->reg_base + AST2600_I2CC_AC_TIMING);
+}
+
+static int ast2600_i2c_recover_bus(struct ast2600_i2c_bus *i2c_bus)
+{
+	u32 state = readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF);
+	int ret = 0;
+	u32 ctrl;
+	int r;
+
+	dev_dbg(i2c_bus->dev, "%d-bus recovery bus [%x]\n", i2c_bus->adap.nr, state);
+
+	/* reset controller */
+	ctrl = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+	writel(ctrl & ~AST2600_I2CC_MASTER_EN, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+	writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+
+	reinit_completion(&i2c_bus->cmd_complete);
+	i2c_bus->cmd_err = 0;
+
+	/* Check SDA/SCL status in the status register. */
+	state = readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF);
+	if (!(state & AST2600_I2CC_SDA_LINE_STS) && (state & AST2600_I2CC_SCL_LINE_STS)) {
+		writel(AST2600_I2CM_RECOVER_CMD_EN, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
+		r = wait_for_completion_timeout(&i2c_bus->cmd_complete, i2c_bus->adap.timeout);
+		if (r == 0) {
+			dev_dbg(i2c_bus->dev, "recovery timed out\n");
+			return -ETIMEDOUT;
+		} else if (i2c_bus->cmd_err) {
+			dev_dbg(i2c_bus->dev, "recovery error\n");
+			ret = -EPROTO;
+		}
+	}
+
+	/* Recovery done */
+	state = readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF);
+	if (state & AST2600_I2CC_BUS_BUSY_STS) {
+		dev_dbg(i2c_bus->dev, "Can't recover bus [%x]\n", state);
+		ret = -EPROTO;
+	}
+
+	return ret;
+}
+
+static int ast2600_i2c_setup_dma_tx(u32 cmd, struct ast2600_i2c_bus *i2c_bus)
+{
+	struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
+	int xfer_len = msg->len - i2c_bus->controller_xfer_cnt;
+
+	cmd |= AST2600_I2CM_PKT_EN;
+
+	if (xfer_len > AST2600_I2C_DMA_SIZE)
+		xfer_len = AST2600_I2C_DMA_SIZE;
+	else if (i2c_bus->msgs_index + 1 == i2c_bus->msgs_count)
+		cmd |= AST2600_I2CM_STOP_CMD;
+
+	if (cmd & AST2600_I2CM_START_CMD)
+		cmd |= AST2600_I2CM_PKT_ADDR(msg->addr);
+
+	if (xfer_len) {
+		memcpy(i2c_bus->controller_dma_buf, msg->buf, xfer_len);
+		cmd |= AST2600_I2CM_TX_DMA_EN | AST2600_I2CM_TX_CMD;
+		writel(AST2600_I2CM_SET_TX_DMA_LEN(xfer_len - 1),
+		       i2c_bus->reg_base + AST2600_I2CM_DMA_LEN);
+	}
+
+	writel(cmd, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
+
+	return 0;
+}
+
+static int ast2600_i2c_setup_buff_tx(u32 cmd, struct ast2600_i2c_bus *i2c_bus)
+{
+	struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
+	int xfer_len = msg->len - i2c_bus->controller_xfer_cnt;
+	u32 wbuf_dword;
+	int i;
+
+	cmd |= AST2600_I2CM_PKT_EN;
+
+	if (xfer_len > i2c_bus->buf_size)
+		xfer_len = i2c_bus->buf_size;
+	else if (i2c_bus->msgs_index + 1 == i2c_bus->msgs_count)
+		cmd |= AST2600_I2CM_STOP_CMD;
+
+	if (cmd & AST2600_I2CM_START_CMD)
+		cmd |= AST2600_I2CM_PKT_ADDR(msg->addr);
+
+	if (xfer_len) {
+		cmd |= AST2600_I2CM_TX_BUFF_EN | AST2600_I2CM_TX_CMD;
+		/*
+		 * The controller's buffer register supports dword writes only.
+		 * Therefore, write dwords to the buffer register in a 4-byte aligned,
+		 * and write the remaining unaligned data at the end.
+		 */
+		for (i = 0; i < xfer_len; i += 4) {
+			int xfer_cnt = i2c_bus->controller_xfer_cnt + i;
+
+			switch (min(xfer_len - i, 4) % 4) {
+			case 1:
+				wbuf_dword = msg->buf[xfer_cnt];
+				break;
+			case 2:
+				wbuf_dword = get_unaligned_le16(&msg->buf[xfer_cnt]);
+				break;
+			case 3:
+				wbuf_dword = get_unaligned_le24(&msg->buf[xfer_cnt]);
+				break;
+			default:
+				wbuf_dword = get_unaligned_le32(&msg->buf[xfer_cnt]);
+				break;
+			}
+			writel(wbuf_dword, i2c_bus->buf_base + i);
+		}
+		writel(AST2600_I2CC_SET_TX_BUF_LEN(xfer_len),
+		       i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+	}
+
+	writel(cmd, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
+
+	return 0;
+}
+
+static int ast2600_i2c_setup_byte_tx(u32 cmd, struct ast2600_i2c_bus *i2c_bus)
+{
+	struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
+	int xfer_len;
+
+	xfer_len = msg->len - i2c_bus->controller_xfer_cnt;
+
+	cmd |= AST2600_I2CM_PKT_EN;
+
+	if (cmd & AST2600_I2CM_START_CMD)
+		cmd |= AST2600_I2CM_PKT_ADDR(msg->addr);
+
+	if ((i2c_bus->msgs_index + 1 == i2c_bus->msgs_count) &&
+	    ((i2c_bus->controller_xfer_cnt + 1) == msg->len))
+		cmd |= AST2600_I2CM_STOP_CMD;
+
+	if (xfer_len) {
+		cmd |= AST2600_I2CM_TX_CMD;
+		writel(msg->buf[i2c_bus->controller_xfer_cnt],
+		       i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF);
+	}
+
+	writel(cmd, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
+
+	return 0;
+}
+
+static int ast2600_i2c_setup_dma_rx(u32 cmd, struct ast2600_i2c_bus *i2c_bus)
+{
+	struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
+	int xfer_len = msg->len - i2c_bus->controller_xfer_cnt;
+
+	cmd |= AST2600_I2CM_PKT_EN | AST2600_I2CM_RX_DMA_EN | AST2600_I2CM_RX_CMD;
+
+	if (msg->flags & I2C_M_RECV_LEN)
+		xfer_len = 1;
+	else if (xfer_len > AST2600_I2C_DMA_SIZE)
+		xfer_len = AST2600_I2C_DMA_SIZE;
+	else if (i2c_bus->msgs_index + 1 == i2c_bus->msgs_count)
+		cmd |= CONTROLLER_TRIGGER_LAST_STOP;
+
+	writel(AST2600_I2CM_SET_RX_DMA_LEN(xfer_len - 1), i2c_bus->reg_base + AST2600_I2CM_DMA_LEN);
+
+	if (cmd & AST2600_I2CM_START_CMD)
+		cmd |= AST2600_I2CM_PKT_ADDR(msg->addr);
+
+	writel(cmd, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
+
+	return 0;
+}
+
+static int ast2600_i2c_setup_buff_rx(u32 cmd, struct ast2600_i2c_bus *i2c_bus)
+{
+	struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
+	int xfer_len = msg->len - i2c_bus->controller_xfer_cnt;
+
+	cmd |= AST2600_I2CM_PKT_EN | AST2600_I2CM_RX_BUFF_EN | AST2600_I2CM_RX_CMD;
+
+	if (cmd & AST2600_I2CM_START_CMD)
+		cmd |= AST2600_I2CM_PKT_ADDR(msg->addr);
+
+	if (msg->flags & I2C_M_RECV_LEN) {
+		dev_dbg(i2c_bus->dev, "smbus read\n");
+		xfer_len = 1;
+	} else if (xfer_len > i2c_bus->buf_size) {
+		xfer_len = i2c_bus->buf_size;
+	} else if (i2c_bus->msgs_index + 1 == i2c_bus->msgs_count) {
+		cmd |= CONTROLLER_TRIGGER_LAST_STOP;
+	}
+	writel(AST2600_I2CC_SET_RX_BUF_LEN(xfer_len), i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+
+	writel(cmd, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
+
+	return 0;
+}
+
+static int ast2600_i2c_setup_byte_rx(u32 cmd, struct ast2600_i2c_bus *i2c_bus)
+{
+	struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
+
+	cmd |= AST2600_I2CM_PKT_EN | AST2600_I2CM_RX_CMD;
+
+	if (cmd & AST2600_I2CM_START_CMD)
+		cmd |= AST2600_I2CM_PKT_ADDR(msg->addr);
+
+	if (msg->flags & I2C_M_RECV_LEN) {
+		dev_dbg(i2c_bus->dev, "smbus read\n");
+	} else if ((i2c_bus->msgs_index + 1 == i2c_bus->msgs_count) &&
+		   ((i2c_bus->controller_xfer_cnt + 1) == msg->len)) {
+		cmd |= CONTROLLER_TRIGGER_LAST_STOP;
+	}
+
+	writel(cmd, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
+
+	return 0;
+}
+
+static int ast2600_i2c_do_start(struct ast2600_i2c_bus *i2c_bus)
+{
+	struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
+
+	/* send start */
+	dev_dbg(i2c_bus->dev, "[%d] %s %d byte%s %s 0x%02x\n",
+		i2c_bus->msgs_index, str_read_write(msg->flags & I2C_M_RD),
+		msg->len, str_plural(msg->len),
+		msg->flags & I2C_M_RD ? "from" : "to", msg->addr);
+
+	if (!i2c_bus->setup_rx || !i2c_bus->setup_tx)
+		return -EINVAL;
+
+	i2c_bus->controller_xfer_cnt = 0;
+	i2c_bus->buf_index = 0;
+
+	if (msg->flags & I2C_M_RD)
+		return i2c_bus->setup_rx(AST2600_I2CM_START_CMD, i2c_bus);
+
+	return i2c_bus->setup_tx(AST2600_I2CM_START_CMD, i2c_bus);
+}
+
+static int ast2600_i2c_irq_err_to_errno(u32 irq_status)
+{
+	if (irq_status & AST2600_I2CM_ARBIT_LOSS)
+		return -EAGAIN;
+	if (irq_status & (AST2600_I2CM_SDA_DL_TO | AST2600_I2CM_SCL_LOW_TO))
+		return -ETIMEDOUT;
+	if (irq_status & (AST2600_I2CM_ABNORMAL))
+		return -EPROTO;
+
+	return 0;
+}
+
+static void ast2600_i2c_controller_packet_irq(struct ast2600_i2c_bus *i2c_bus, u32 sts)
+{
+	struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
+	int xfer_len;
+	int i;
+
+	sts &= ~AST2600_I2CM_PKT_DONE;
+	writel(AST2600_I2CM_PKT_DONE, i2c_bus->reg_base + AST2600_I2CM_ISR);
+	switch (sts) {
+	case AST2600_I2CM_PKT_ERROR:
+		i2c_bus->cmd_err = -EAGAIN;
+		complete(&i2c_bus->cmd_complete);
+		break;
+	case AST2600_I2CM_PKT_ERROR | AST2600_I2CM_TX_NAK: /* a0 fix for issue */
+		fallthrough;
+	case AST2600_I2CM_PKT_ERROR | AST2600_I2CM_TX_NAK | AST2600_I2CM_NORMAL_STOP:
+		i2c_bus->cmd_err = -ENXIO;
+		complete(&i2c_bus->cmd_complete);
+		break;
+	case AST2600_I2CM_NORMAL_STOP:
+		/* write 0 byte only have stop isr */
+		i2c_bus->msgs_index++;
+		if (i2c_bus->msgs_index < i2c_bus->msgs_count) {
+			if (ast2600_i2c_do_start(i2c_bus)) {
+				i2c_bus->cmd_err = -ENOMEM;
+				complete(&i2c_bus->cmd_complete);
+			}
+		} else {
+			i2c_bus->cmd_err = i2c_bus->msgs_index;
+			complete(&i2c_bus->cmd_complete);
+		}
+		break;
+	case AST2600_I2CM_TX_ACK:
+	case AST2600_I2CM_TX_ACK | AST2600_I2CM_NORMAL_STOP:
+		if (i2c_bus->mode == DMA_MODE)
+			xfer_len = AST2600_I2C_GET_TX_DMA_LEN(readl(i2c_bus->reg_base +
+							  AST2600_I2CM_DMA_LEN_STS));
+		else if (i2c_bus->mode == BUFF_MODE)
+			xfer_len = AST2600_I2CC_GET_TX_BUF_LEN(readl(i2c_bus->reg_base +
+							   AST2600_I2CC_BUFF_CTRL));
+		else
+			xfer_len = 1;
+
+		i2c_bus->controller_xfer_cnt += xfer_len;
+
+		if (i2c_bus->controller_xfer_cnt == msg->len) {
+			i2c_bus->msgs_index++;
+			if (i2c_bus->msgs_index == i2c_bus->msgs_count) {
+				i2c_bus->cmd_err = i2c_bus->msgs_index;
+				complete(&i2c_bus->cmd_complete);
+			} else {
+				if (ast2600_i2c_do_start(i2c_bus)) {
+					i2c_bus->cmd_err = -ENOMEM;
+					complete(&i2c_bus->cmd_complete);
+				}
+			}
+		} else {
+			i2c_bus->setup_tx(0, i2c_bus);
+		}
+		break;
+	case AST2600_I2CM_RX_DONE:
+	case AST2600_I2CM_RX_DONE | AST2600_I2CM_NORMAL_STOP:
+		/* do next rx */
+		if (i2c_bus->mode == DMA_MODE) {
+			xfer_len = AST2600_I2C_GET_RX_DMA_LEN(readl(i2c_bus->reg_base +
+								    AST2600_I2CM_DMA_LEN_STS));
+			memcpy(&msg->buf[i2c_bus->controller_xfer_cnt],
+			       i2c_bus->controller_dma_buf, xfer_len);
+		} else if (i2c_bus->mode == BUFF_MODE) {
+			xfer_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+								     AST2600_I2CC_BUFF_CTRL));
+			for (i = 0; i < xfer_len; i++)
+				msg->buf[i2c_bus->controller_xfer_cnt + i] =
+					readb(i2c_bus->buf_base + 0x10 + i);
+		} else {
+			xfer_len = 1;
+			msg->buf[i2c_bus->controller_xfer_cnt] =
+				AST2600_I2CC_GET_RX_BUFF(readl(i2c_bus->reg_base +
+						     AST2600_I2CC_STS_AND_BUFF));
+		}
+
+		if (msg->flags & I2C_M_RECV_LEN) {
+			u8 recv_len = AST2600_I2CC_GET_RX_BUFF(readl(i2c_bus->reg_base
+						       + AST2600_I2CC_STS_AND_BUFF));
+			msg->len = min_t(unsigned int, recv_len, I2C_SMBUS_BLOCK_MAX);
+			msg->len += ((msg->flags & I2C_CLIENT_PEC) ? 2 : 1);
+			msg->flags &= ~I2C_M_RECV_LEN;
+			if (!recv_len)
+				i2c_bus->controller_xfer_cnt = 0;
+			else
+				i2c_bus->controller_xfer_cnt = 1;
+		} else {
+			i2c_bus->controller_xfer_cnt += xfer_len;
+		}
+
+		if (i2c_bus->controller_xfer_cnt == msg->len) {
+			i2c_bus->msgs_index++;
+			if (i2c_bus->msgs_index == i2c_bus->msgs_count) {
+				i2c_bus->cmd_err = i2c_bus->msgs_index;
+				complete(&i2c_bus->cmd_complete);
+			} else {
+				if (ast2600_i2c_do_start(i2c_bus)) {
+					i2c_bus->cmd_err = -ENOMEM;
+					complete(&i2c_bus->cmd_complete);
+				}
+			}
+		} else {
+			i2c_bus->setup_rx(0, i2c_bus);
+		}
+		break;
+	default:
+		dev_dbg(i2c_bus->dev, "unhandled sts %x\n", sts);
+		break;
+	}
+}
+
+static int ast2600_i2c_controller_irq(struct ast2600_i2c_bus *i2c_bus)
+{
+	u32 sts = readl(i2c_bus->reg_base + AST2600_I2CM_ISR);
+	u32 ctrl;
+
+	sts &= ~AST2600_I2CM_SMBUS_ALERT;
+
+	if (sts & AST2600_I2CM_BUS_RECOVER_FAIL) {
+		writel(AST2600_I2CM_BUS_RECOVER_FAIL, i2c_bus->reg_base + AST2600_I2CM_ISR);
+		ctrl = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+		writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+		writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+		i2c_bus->cmd_err = -EPROTO;
+		complete(&i2c_bus->cmd_complete);
+		return 1;
+	}
+
+	if (sts & AST2600_I2CM_BUS_RECOVER) {
+		writel(AST2600_I2CM_BUS_RECOVER, i2c_bus->reg_base + AST2600_I2CM_ISR);
+		i2c_bus->cmd_err = 0;
+		complete(&i2c_bus->cmd_complete);
+		return 1;
+	}
+
+	i2c_bus->cmd_err = ast2600_i2c_irq_err_to_errno(sts);
+	if (i2c_bus->cmd_err) {
+		writel(AST2600_I2CM_PKT_DONE, i2c_bus->reg_base + AST2600_I2CM_ISR);
+		complete(&i2c_bus->cmd_complete);
+		return 1;
+	}
+
+	if (sts & AST2600_I2CM_PKT_DONE) {
+		ast2600_i2c_controller_packet_irq(i2c_bus, sts);
+		return 1;
+	}
+
+	return 0;
+}
+
+static irqreturn_t ast2600_i2c_bus_irq(int irq, void *dev_id)
+{
+	struct ast2600_i2c_bus *i2c_bus = dev_id;
+
+	return IRQ_RETVAL(ast2600_i2c_controller_irq(i2c_bus));
+}
+
+static int ast2600_i2c_controller_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+	struct ast2600_i2c_bus *i2c_bus = i2c_get_adapdata(adap);
+	unsigned long timeout;
+	int ret;
+
+	if (!i2c_bus->multi_master &&
+	    (readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF) & AST2600_I2CC_BUS_BUSY_STS)) {
+		ret = ast2600_i2c_recover_bus(i2c_bus);
+		if (ret)
+			return ret;
+	}
+
+	i2c_bus->cmd_err = 0;
+	i2c_bus->msgs = msgs;
+	i2c_bus->msgs_index = 0;
+	i2c_bus->msgs_count = num;
+	reinit_completion(&i2c_bus->cmd_complete);
+	ret = ast2600_i2c_do_start(i2c_bus);
+	if (ret)
+		goto controller_out;
+	timeout = wait_for_completion_timeout(&i2c_bus->cmd_complete, i2c_bus->adap.timeout);
+	if (timeout == 0) {
+		u32 ctrl = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+
+		dev_dbg(i2c_bus->dev, "timeout isr[%x], sts[%x]\n",
+			readl(i2c_bus->reg_base + AST2600_I2CM_ISR),
+			readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF));
+		writel(ctrl & ~AST2600_I2CC_MASTER_EN, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+		writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+
+		/*
+		 * A slave holding SCL low can stall the transfer and trigger
+		 * a master timeout. In multi-master mode, attempt bus recovery
+		 * if the bus is still busy.
+		 */
+		if (i2c_bus->multi_master &&
+		    (readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF) &
+		    AST2600_I2CC_BUS_BUSY_STS))
+			ast2600_i2c_recover_bus(i2c_bus);
+		ret = -ETIMEDOUT;
+	} else {
+		ret = i2c_bus->cmd_err;
+	}
+
+	dev_dbg(i2c_bus->dev, "bus%d-m: %d end\n", i2c_bus->adap.nr, i2c_bus->cmd_err);
+
+controller_out:
+	return ret;
+}
+
+static int ast2600_i2c_init(struct ast2600_i2c_bus *i2c_bus)
+{
+	u32 fun_ctrl = AST2600_I2CC_BUS_AUTO_RELEASE | AST2600_I2CC_MASTER_EN;
+
+	/* I2C Reset */
+	writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+
+	if (!i2c_bus->multi_master)
+		fun_ctrl |= AST2600_I2CC_MULTI_MASTER_DIS;
+
+	/* Enable Controller Mode */
+	writel(fun_ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+	/* disable target address */
+	writel(0, i2c_bus->reg_base + AST2600_I2CS_ADDR_CTRL);
+
+	/* Set AC Timing */
+	ast2600_i2c_ac_timing_config(i2c_bus);
+
+	if (i2c_bus->dma_available) {
+		i2c_bus->controller_dma_buf =
+			dmam_alloc_coherent(i2c_bus->dev, AST2600_I2C_DMA_SIZE,
+					    &i2c_bus->controller_dma_addr, GFP_KERNEL);
+		if (!i2c_bus->controller_dma_buf)
+			return -ENOMEM;
+		writel(i2c_bus->controller_dma_addr,
+		       i2c_bus->reg_base + AST2600_I2CM_TX_DMA);
+		writel(i2c_bus->controller_dma_addr,
+		       i2c_bus->reg_base + AST2600_I2CM_RX_DMA);
+	}
+
+	/* Clear Interrupt */
+	writel(GENMASK(27, 0), i2c_bus->reg_base + AST2600_I2CM_ISR);
+
+	return 0;
+}
+
+static u32 ast2600_i2c_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+static const struct i2c_algorithm i2c_ast2600_algorithm = {
+	.xfer = ast2600_i2c_controller_xfer,
+	.functionality = ast2600_i2c_functionality,
+};
+
+static void ast2600_i2c_set_xfer_mode(struct ast2600_i2c_bus *i2c_bus,
+				      enum xfer_mode mode)
+{
+	i2c_bus->mode = mode;
+
+	switch (mode) {
+	case DMA_MODE:
+		i2c_bus->setup_tx = ast2600_i2c_setup_dma_tx;
+		i2c_bus->setup_rx = ast2600_i2c_setup_dma_rx;
+		break;
+	case BYTE_MODE:
+		i2c_bus->setup_tx = ast2600_i2c_setup_byte_tx;
+		i2c_bus->setup_rx = ast2600_i2c_setup_byte_rx;
+		break;
+	case BUFF_MODE:
+	default:
+		i2c_bus->setup_tx = ast2600_i2c_setup_buff_tx;
+		i2c_bus->setup_rx = ast2600_i2c_setup_buff_rx;
+		break;
+	}
+}
+
+static int ast2600_i2c_xfer_mode_parse(struct ast2600_i2c_bus *i2c_bus,
+				       const char *buf, enum xfer_mode *mode)
+{
+	if (sysfs_streq(buf, "byte")) {
+		*mode = BYTE_MODE;
+		return 0;
+	}
+
+	if (sysfs_streq(buf, "buffer")) {
+		if (!i2c_bus->buf_base)
+			return -EINVAL;
+		*mode = BUFF_MODE;
+		return 0;
+	}
+
+	if (sysfs_streq(buf, "dma")) {
+		if (!i2c_bus->dma_available)
+			return -EINVAL;
+		*mode = DMA_MODE;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static const char *ast2600_i2c_xfer_mode_name(enum xfer_mode mode)
+{
+	switch (mode) {
+	case BYTE_MODE:
+		return "byte";
+	case DMA_MODE:
+		return "dma";
+	case BUFF_MODE:
+	default:
+		return "buffer";
+	}
+}
+
+static ssize_t xfer_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct ast2600_i2c_bus *i2c_bus = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%s\n", ast2600_i2c_xfer_mode_name(i2c_bus->mode));
+}
+
+static ssize_t xfer_mode_store(struct device *dev,
+			       struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	struct ast2600_i2c_bus *i2c_bus = dev_get_drvdata(dev);
+	enum xfer_mode mode;
+	int ret;
+
+	ret = ast2600_i2c_xfer_mode_parse(i2c_bus, buf, &mode);
+	if (ret)
+		return ret;
+
+	i2c_lock_bus(&i2c_bus->adap, I2C_LOCK_ROOT_ADAPTER);
+	ast2600_i2c_set_xfer_mode(i2c_bus, mode);
+	i2c_unlock_bus(&i2c_bus->adap, I2C_LOCK_ROOT_ADAPTER);
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(xfer_mode);
+
+static int ast2600_i2c_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ast2600_i2c_bus *i2c_bus;
+	struct reset_control *rst;
+	struct resource *res;
+	u32 global_ctrl;
+	int ret;
+
+	if (!device_property_present(dev, "aspeed,global-regs"))
+		return -ENODEV;
+
+	i2c_bus = devm_kzalloc(dev, sizeof(*i2c_bus), GFP_KERNEL);
+	if (!i2c_bus)
+		return -ENOMEM;
+
+	i2c_bus->reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(i2c_bus->reg_base))
+		return PTR_ERR(i2c_bus->reg_base);
+
+	rst = devm_reset_control_get_shared_deasserted(dev, NULL);
+	if (IS_ERR(rst))
+		return dev_err_probe(dev, PTR_ERR(rst), "Missing reset ctrl\n");
+
+	i2c_bus->global_regs =
+		syscon_regmap_lookup_by_phandle(dev_of_node(dev), "aspeed,global-regs");
+	if (IS_ERR(i2c_bus->global_regs))
+		return PTR_ERR(i2c_bus->global_regs);
+
+	regmap_read(i2c_bus->global_regs, AST2600_I2CG_CTRL, &global_ctrl);
+	if ((global_ctrl & AST2600_GLOBAL_INIT) != AST2600_GLOBAL_INIT) {
+		regmap_write(i2c_bus->global_regs, AST2600_I2CG_CTRL, AST2600_GLOBAL_INIT);
+		regmap_write(i2c_bus->global_regs, AST2600_I2CG_CLK_DIV_CTRL, I2CCG_DIV_CTRL);
+	}
+
+	i2c_bus->dev = dev;
+	i2c_bus->multi_master = device_property_read_bool(dev, "multi-master");
+	i2c_bus->dma_available = device_property_read_bool(dev, "aspeed,enable-dma");
+	if (i2c_bus->dma_abailable)
+		i2c_bus->mode = DMA_MODE;
+	else
+		i2c_bus->mode = BUFF_MODE;
+
+	if (i2c_bus->mode == BUFF_MODE) {
+		i2c_bus->buf_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
+		if (IS_ERR(i2c_bus->buf_base))
+			i2c_bus->mode = BYTE_MODE;
+		else
+			i2c_bus->buf_size = resource_size(res) / 2;
+	}
+
+	ast2600_i2c_set_xfer_mode(i2c_bus, i2c_bus->mode);
+
+	/*
+	 * i2c timeout counter: use base clk4 1Mhz,
+	 * per unit: 1/(1000/1024) = 1024us
+	 */
+	ret = device_property_read_u32(dev, "i2c-scl-clk-low-timeout-us", &i2c_bus->timeout);
+	if (!ret)
+		i2c_bus->timeout = DIV_ROUND_UP(i2c_bus->timeout, 1024);
+
+	init_completion(&i2c_bus->cmd_complete);
+
+	i2c_bus->irq = platform_get_irq(pdev, 0);
+	if (i2c_bus->irq < 0)
+		return i2c_bus->irq;
+
+	platform_set_drvdata(pdev, i2c_bus);
+
+	i2c_bus->clk = devm_clk_get(i2c_bus->dev, NULL);
+	if (IS_ERR(i2c_bus->clk))
+		return dev_err_probe(i2c_bus->dev, PTR_ERR(i2c_bus->clk), "Can't get clock\n");
+
+	i2c_bus->apb_clk = clk_get_rate(i2c_bus->clk);
+
+	i2c_parse_fw_timings(i2c_bus->dev, &i2c_bus->timing_info, true);
+
+	/* Initialize the I2C adapter */
+	i2c_bus->adap.owner = THIS_MODULE;
+	i2c_bus->adap.algo = &i2c_ast2600_algorithm;
+	i2c_bus->adap.retries = 0;
+	i2c_bus->adap.dev.parent = i2c_bus->dev;
+	device_set_node(&i2c_bus->adap.dev, dev_fwnode(dev));
+	i2c_bus->adap.algo_data = i2c_bus;
+	strscpy(i2c_bus->adap.name, pdev->name);
+	i2c_set_adapdata(&i2c_bus->adap, i2c_bus);
+
+	ret = ast2600_i2c_init(i2c_bus);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Unable to initial i2c %d\n", ret);
+
+	ret = devm_request_irq(dev, i2c_bus->irq, ast2600_i2c_bus_irq, 0,
+			       dev_name(dev), i2c_bus);
+	if (ret < 0) {
+		ret = dev_err_probe(dev, ret, "Unable to request irq %d\n",
+				    i2c_bus->irq);
+		goto err;
+	}
+
+	writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
+	       i2c_bus->reg_base + AST2600_I2CM_IER);
+
+	ret = devm_i2c_add_adapter(dev, &i2c_bus->adap);
+	if (ret)
+		goto err;
+
+	ret = sysfs_create_file(&dev->kobj, &dev_attr_xfer_mode.attr);
+	if (ret)
+		goto err;
+
+	return 0;
+
+err:
+	writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+	writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
+	return ret;
+}
+
+static void ast2600_i2c_remove(struct platform_device *pdev)
+{
+	struct ast2600_i2c_bus *i2c_bus = platform_get_drvdata(pdev);
+
+	sysfs_remove_file(&pdev->dev.kobj, &dev_attr_xfer_mode.attr);
+
+	/* Disable everything. */
+	writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+	writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
+}
+
+static const struct of_device_id ast2600_i2c_of_match[] = {
+	{ .compatible = "aspeed,ast2600-i2c-bus" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ast2600_i2c_of_match);
+
+static struct platform_driver ast2600_i2c_driver = {
+	.probe		= ast2600_i2c_probe,
+	.remove		= ast2600_i2c_remove,
+	.driver		= {
+		.name		= "ast2600-i2c",
+		.of_match_table	= ast2600_i2c_of_match,
+	},
+};
+module_platform_driver(ast2600_i2c_driver);
+
+MODULE_AUTHOR("Ryan Chen <ryan_chen@aspeedtech.com>");
+MODULE_DESCRIPTION("ASPEED AST2600 I2C Controller Driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1



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

* [PATCH v27 4/4] i2c: ast2600: Add target mode support
  2026-03-24  3:06 [PATCH v27 0/4] Add ASPEED AST2600 I2C controller driver Ryan Chen
                   ` (2 preceding siblings ...)
  2026-03-24  3:06 ` [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set Ryan Chen
@ 2026-03-24  3:06 ` Ryan Chen
  3 siblings, 0 replies; 17+ messages in thread
From: Ryan Chen @ 2026-03-24  3:06 UTC (permalink / raw)
  To: jk, andriy.shevchenko, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
	Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
  Cc: linux-i2c, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, openbmc, Ryan Chen

Add target mode support to the AST2600 I2C driver.

Target mode features implemented include:
- Add target interrupt handling
- Address match and response logic
- Separate Tx/Rx DMA address and length configuration

This complements the existing controller-mode support, enabling
dual-role capability.

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v26:
- change int to bool target_operate
- rename target_operate to target_active
- use i2c_bus->target replace require IO
- use WRITE_ONCE replace target_operate write.
---
 drivers/i2c/busses/i2c-ast2600.c | 565 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 564 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-ast2600.c b/drivers/i2c/busses/i2c-ast2600.c
index cf3006eaded9..0ae0cd71a78d 100644
--- a/drivers/i2c/busses/i2c-ast2600.c
+++ b/drivers/i2c/busses/i2c-ast2600.c
@@ -274,6 +274,13 @@ struct ast2600_i2c_bus {
 	void __iomem		*buf_base;
 	int (*setup_tx)(u32 cmd, struct ast2600_i2c_bus *i2c_bus);
 	int (*setup_rx)(u32 cmd, struct ast2600_i2c_bus *i2c_bus);
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	/* target structure */
+	bool			target_active;
+	unsigned char	*target_dma_buf;
+	dma_addr_t		target_dma_addr;
+	struct i2c_client	*target;
+#endif
 };
 
 static void ast2600_i2c_ac_timing_config(struct ast2600_i2c_bus *i2c_bus)
@@ -357,6 +364,440 @@ static int ast2600_i2c_recover_bus(struct ast2600_i2c_bus *i2c_bus)
 	return ret;
 }
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static void ast2600_i2c_target_packet_dma_irq(struct ast2600_i2c_bus *i2c_bus, u32 sts)
+{
+	int target_rx_len = 0;
+	u32 cmd = 0;
+	u8 value;
+	int i;
+
+	sts &= ~(AST2600_I2CS_SLAVE_PENDING);
+	/* Handle i2c target timeout condition */
+	if (sts & AST2600_I2CS_INACTIVE_TO) {
+		/* Reset timeout counter */
+		u32 ac_timing = readl(i2c_bus->reg_base + AST2600_I2CC_AC_TIMING) &
+				AST2600_I2CC_AC_TIMING_MASK;
+
+		writel(ac_timing, i2c_bus->reg_base + AST2600_I2CC_AC_TIMING);
+		ac_timing |= AST2600_I2CC_TTIMEOUT(i2c_bus->timeout);
+		writel(ac_timing, i2c_bus->reg_base + AST2600_I2CC_AC_TIMING);
+		cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_RX_DMA_EN;
+		writel(AST2600_I2CS_SET_RX_DMA_LEN(I2C_TARGET_MSG_BUF_SIZE),
+		       i2c_bus->reg_base + AST2600_I2CS_DMA_LEN);
+		writel(cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+		writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_ISR);
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		return;
+	}
+
+	sts &= ~(AST2600_I2CS_PKT_DONE | AST2600_I2CS_PKT_ERROR);
+
+	switch (sts) {
+	case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE | AST2600_I2CS_WAIT_RX_DMA:
+	case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_WAIT_RX_DMA:
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+		target_rx_len = AST2600_I2C_GET_RX_DMA_LEN(readl(i2c_bus->reg_base +
+						      AST2600_I2CS_DMA_LEN_STS));
+		for (i = 0; i < target_rx_len; i++) {
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED,
+					&i2c_bus->target_dma_buf[i]);
+		}
+		writel(AST2600_I2CS_SET_RX_DMA_LEN(I2C_TARGET_MSG_BUF_SIZE),
+		       i2c_bus->reg_base + AST2600_I2CS_DMA_LEN);
+		cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_RX_DMA_EN;
+		break;
+	case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_STOP:
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		writel(AST2600_I2CS_SET_RX_DMA_LEN(I2C_TARGET_MSG_BUF_SIZE),
+		       i2c_bus->reg_base + AST2600_I2CS_DMA_LEN);
+		cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_RX_DMA_EN;
+		break;
+	case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE_NAK |
+			AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+	case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_WAIT_RX_DMA |
+			AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+	case AST2600_I2CS_RX_DONE_NAK | AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+	case AST2600_I2CS_RX_DONE | AST2600_I2CS_WAIT_RX_DMA | AST2600_I2CS_STOP:
+	case AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+	case AST2600_I2CS_RX_DONE | AST2600_I2CS_WAIT_RX_DMA:
+	case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+		if (sts & AST2600_I2CS_SLAVE_MATCH)
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+
+		target_rx_len = AST2600_I2C_GET_RX_DMA_LEN(readl(i2c_bus->reg_base +
+						      AST2600_I2CS_DMA_LEN_STS));
+		for (i = 0; i < target_rx_len; i++) {
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED,
+					&i2c_bus->target_dma_buf[i]);
+		}
+		writel(AST2600_I2CS_SET_RX_DMA_LEN(I2C_TARGET_MSG_BUF_SIZE),
+		       i2c_bus->reg_base + AST2600_I2CS_DMA_LEN);
+		if (sts & AST2600_I2CS_STOP)
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_RX_DMA_EN;
+		break;
+
+	/* it is Mw data Mr coming -> it need send tx */
+	case AST2600_I2CS_RX_DONE | AST2600_I2CS_WAIT_TX_DMA:
+	case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE | AST2600_I2CS_WAIT_TX_DMA:
+		/* it should be repeat start read */
+		if (sts & AST2600_I2CS_SLAVE_MATCH)
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+
+		target_rx_len = AST2600_I2C_GET_RX_DMA_LEN(readl(i2c_bus->reg_base +
+						      AST2600_I2CS_DMA_LEN_STS));
+		for (i = 0; i < target_rx_len; i++) {
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED,
+					&i2c_bus->target_dma_buf[i]);
+		}
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_READ_REQUESTED,
+				&i2c_bus->target_dma_buf[0]);
+		writel(AST2600_I2CS_SET_TX_DMA_LEN(1),
+		       i2c_bus->reg_base + AST2600_I2CS_DMA_LEN);
+		cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_TX_DMA_EN;
+		break;
+	case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_WAIT_TX_DMA:
+		/* First Start read */
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_READ_REQUESTED,
+				&i2c_bus->target_dma_buf[0]);
+		writel(AST2600_I2CS_SET_TX_DMA_LEN(1),
+		       i2c_bus->reg_base + AST2600_I2CS_DMA_LEN);
+		cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_TX_DMA_EN;
+		break;
+	case AST2600_I2CS_WAIT_TX_DMA:
+		/* it should be next start read */
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_READ_PROCESSED,
+				&i2c_bus->target_dma_buf[0]);
+		writel(AST2600_I2CS_SET_TX_DMA_LEN(1),
+		       i2c_bus->reg_base + AST2600_I2CS_DMA_LEN);
+		cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_TX_DMA_EN;
+		break;
+	case AST2600_I2CS_TX_NAK | AST2600_I2CS_STOP:
+		/* it just tx complete */
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		writel(AST2600_I2CS_SET_RX_DMA_LEN(I2C_TARGET_MSG_BUF_SIZE),
+		       i2c_bus->reg_base + AST2600_I2CS_DMA_LEN);
+		cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_RX_DMA_EN;
+		break;
+	case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE:
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+		break;
+	case AST2600_I2CS_STOP:
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		break;
+	default:
+		dev_dbg(i2c_bus->dev, "unhandled target isr case %x, sts %x\n", sts,
+			readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF));
+		break;
+	}
+
+	if (cmd)
+		writel(cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+	writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_ISR);
+	readl(i2c_bus->reg_base + AST2600_I2CS_ISR);
+}
+
+static void ast2600_i2c_target_packet_buff_irq(struct ast2600_i2c_bus *i2c_bus, u32 sts)
+{
+	int target_rx_len = 0;
+	u32 cmd = 0;
+	u8 value;
+	int i;
+
+	/* due to controller target is common buffer, need force the master stop not issue */
+	if (readl(i2c_bus->reg_base + AST2600_I2CM_CMD_STS) & GENMASK(15, 0)) {
+		writel(0, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
+		i2c_bus->cmd_err = -EBUSY;
+		writel(0, i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+		complete(&i2c_bus->cmd_complete);
+	}
+
+	/* Handle i2c target timeout condition */
+	if (AST2600_I2CS_INACTIVE_TO & sts) {
+		/* Reset timeout counter */
+		u32 ac_timing = readl(i2c_bus->reg_base + AST2600_I2CC_AC_TIMING) &
+				AST2600_I2CC_AC_TIMING_MASK;
+
+		writel(ac_timing, i2c_bus->reg_base + AST2600_I2CC_AC_TIMING);
+		ac_timing |= AST2600_I2CC_TTIMEOUT(i2c_bus->timeout);
+		writel(ac_timing, i2c_bus->reg_base + AST2600_I2CC_AC_TIMING);
+		writel(TARGET_TRIGGER_CMD, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+		writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_ISR);
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		WRITE_ONCE(i2c_bus->target_active, false);
+		return;
+	}
+
+	sts &= ~(AST2600_I2CS_PKT_DONE | AST2600_I2CS_PKT_ERROR);
+
+	if (sts & AST2600_I2CS_SLAVE_MATCH)
+		WRITE_ONCE(i2c_bus->target_active, true);
+
+	switch (sts) {
+	case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_WAIT_RX_DMA |
+		 AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+	case AST2600_I2CS_SLAVE_PENDING |
+		 AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+	case AST2600_I2CS_SLAVE_PENDING |
+		 AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_STOP:
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		fallthrough;
+	case AST2600_I2CS_SLAVE_PENDING |
+		 AST2600_I2CS_WAIT_RX_DMA | AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE:
+	case AST2600_I2CS_WAIT_RX_DMA | AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE:
+	case AST2600_I2CS_WAIT_RX_DMA | AST2600_I2CS_SLAVE_MATCH:
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+		cmd = TARGET_TRIGGER_CMD;
+		if (sts & AST2600_I2CS_RX_DONE) {
+			target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+							       AST2600_I2CC_BUFF_CTRL));
+			for (i = 0; i < target_rx_len; i++) {
+				value = readb(i2c_bus->buf_base + 0x10 + i);
+				i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+			}
+		}
+		if (readl(i2c_bus->reg_base + AST2600_I2CS_CMD_STS) & AST2600_I2CS_RX_BUFF_EN)
+			cmd = 0;
+		else
+			cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_RX_BUFF_EN;
+
+		writel(AST2600_I2CC_SET_RX_BUF_LEN(i2c_bus->buf_size),
+		       i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+		break;
+	case AST2600_I2CS_WAIT_RX_DMA | AST2600_I2CS_RX_DONE:
+		cmd = TARGET_TRIGGER_CMD;
+		target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+						       AST2600_I2CC_BUFF_CTRL));
+		for (i = 0; i < target_rx_len; i++) {
+			value = readb(i2c_bus->buf_base + 0x10 + i);
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+		}
+		cmd |= AST2600_I2CS_RX_BUFF_EN;
+		writel(AST2600_I2CC_SET_RX_BUF_LEN(i2c_bus->buf_size),
+		       i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+		break;
+	case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_WAIT_RX_DMA |
+				AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+		cmd = TARGET_TRIGGER_CMD;
+		target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+								 AST2600_I2CC_BUFF_CTRL));
+		for (i = 0; i < target_rx_len; i++) {
+			value = readb(i2c_bus->buf_base + 0x10 + i);
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+		}
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		cmd |= AST2600_I2CS_RX_BUFF_EN;
+		writel(AST2600_I2CC_SET_RX_BUF_LEN(i2c_bus->buf_size),
+		       i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+		break;
+	case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+		cmd = TARGET_TRIGGER_CMD;
+		target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+								 AST2600_I2CC_BUFF_CTRL));
+		for (i = 0; i < target_rx_len; i++) {
+			value = readb(i2c_bus->buf_base + 0x10 + i);
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+		}
+		/* workaround for avoid next start with len != 0 */
+		writel(BIT(0), i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		break;
+	case AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+		cmd = TARGET_TRIGGER_CMD;
+		target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+								 AST2600_I2CC_BUFF_CTRL));
+		for (i = 0; i < target_rx_len; i++) {
+			value = readb(i2c_bus->buf_base + 0x10 + i);
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+		}
+		/* workaround for avoid next start with len != 0 */
+		writel(BIT(0), i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		break;
+	case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_RX_DONE |
+	     AST2600_I2CS_WAIT_TX_DMA | AST2600_I2CS_STOP:
+		target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+								 AST2600_I2CC_BUFF_CTRL));
+		for (i = 0; i < target_rx_len; i++) {
+			value = readb(i2c_bus->buf_base + 0x10 + i);
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+		}
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_READ_REQUESTED, &value);
+		writeb(value, i2c_bus->buf_base);
+		break;
+	case AST2600_I2CS_WAIT_TX_DMA | AST2600_I2CS_SLAVE_MATCH:
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_READ_REQUESTED, &value);
+		writeb(value, i2c_bus->buf_base);
+		writel(AST2600_I2CC_SET_TX_BUF_LEN(1),
+		       i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+		cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_TX_BUFF_EN;
+		break;
+	case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_STOP |
+	     AST2600_I2CS_TX_NAK | AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE:
+	case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_WAIT_RX_DMA | AST2600_I2CS_STOP |
+	     AST2600_I2CS_TX_NAK | AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE:
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+		target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+						AST2600_I2CC_BUFF_CTRL));
+		for (i = 0; i < target_rx_len; i++) {
+			value = readb(i2c_bus->buf_base + 0x10 + i);
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+		}
+		writel(AST2600_I2CC_SET_RX_BUF_LEN(i2c_bus->buf_size),
+		       i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+		cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_RX_BUFF_EN;
+		break;
+	case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_WAIT_TX_DMA | AST2600_I2CS_RX_DONE:
+	case AST2600_I2CS_WAIT_TX_DMA | AST2600_I2CS_RX_DONE:
+	case AST2600_I2CS_WAIT_TX_DMA:
+		if (sts & AST2600_I2CS_SLAVE_MATCH)
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+
+		if (sts & AST2600_I2CS_RX_DONE) {
+			target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+							AST2600_I2CC_BUFF_CTRL));
+			for (i = 0; i < target_rx_len; i++) {
+				value = readb(i2c_bus->buf_base + 0x10 + i);
+				i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+			}
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_READ_REQUESTED, &value);
+		} else {
+			i2c_slave_event(i2c_bus->target, I2C_SLAVE_READ_PROCESSED, &value);
+		}
+		writeb(value, i2c_bus->buf_base);
+		writel(AST2600_I2CC_SET_TX_BUF_LEN(1),
+		       i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+		cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_TX_BUFF_EN;
+		break;
+	/* workaround : trigger the cmd twice to fix next state keep 1000000 */
+	case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE:
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+		cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_RX_BUFF_EN;
+		writel(cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+		break;
+	case AST2600_I2CS_TX_NAK | AST2600_I2CS_STOP:
+	case AST2600_I2CS_STOP:
+		cmd = TARGET_TRIGGER_CMD;
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		break;
+	default:
+		dev_dbg(i2c_bus->dev, "unhandled target isr case %x, sts %x\n", sts,
+			readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF));
+		break;
+	}
+
+	if (cmd)
+		writel(cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+	writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_ISR);
+	readl(i2c_bus->reg_base + AST2600_I2CS_ISR);
+
+	if ((sts & AST2600_I2CS_STOP) && !(sts & AST2600_I2CS_SLAVE_PENDING))
+		WRITE_ONCE(i2c_bus->target_active, false);
+}
+
+static void ast2600_i2c_target_byte_irq(struct ast2600_i2c_bus *i2c_bus, u32 sts)
+{
+	u32 i2c_buff = readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF);
+	u32 cmd = AST2600_I2CS_ACTIVE_ALL;
+	u8 byte_data;
+	u8 value;
+
+	/* Handle i2c target timeout condition */
+	if (sts & AST2600_I2CS_INACTIVE_TO) {
+		/* Reset timeout counter */
+		u32 ac_timing = readl(i2c_bus->reg_base + AST2600_I2CC_AC_TIMING) &
+				AST2600_I2CC_AC_TIMING_MASK;
+
+		writel(ac_timing, i2c_bus->reg_base + AST2600_I2CC_AC_TIMING);
+		ac_timing |= AST2600_I2CC_TTIMEOUT(i2c_bus->timeout);
+		writel(ac_timing, i2c_bus->reg_base + AST2600_I2CC_AC_TIMING);
+		writel(AST2600_I2CS_ACTIVE_ALL, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+		writel(sts, i2c_bus->reg_base + AST2600_I2CS_ISR);
+		readl(i2c_bus->reg_base + AST2600_I2CS_ISR);
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		WRITE_ONCE(i2c_bus->target_active, false);
+		return;
+	}
+
+	switch (sts) {
+	case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE | AST2600_I2CS_WAIT_RX_DMA:
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+		/* first address match is address */
+		byte_data = AST2600_I2CC_GET_RX_BUFF(i2c_buff);
+		break;
+	case AST2600_I2CS_RX_DONE | AST2600_I2CS_WAIT_RX_DMA:
+		byte_data = AST2600_I2CC_GET_RX_BUFF(i2c_buff);
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &byte_data);
+		break;
+	case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE | AST2600_I2CS_WAIT_TX_DMA:
+		cmd |= AST2600_I2CS_TX_CMD;
+		byte_data = AST2600_I2CC_GET_RX_BUFF(i2c_buff);
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_READ_REQUESTED, &byte_data);
+		writel(byte_data, i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF);
+		break;
+	case AST2600_I2CS_TX_ACK | AST2600_I2CS_WAIT_TX_DMA:
+		cmd |= AST2600_I2CS_TX_CMD;
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_READ_PROCESSED, &byte_data);
+		writel(byte_data, i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF);
+		break;
+	case AST2600_I2CS_STOP:
+	case AST2600_I2CS_STOP | AST2600_I2CS_TX_NAK:
+		i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+		break;
+	default:
+		dev_dbg(i2c_bus->dev, "unhandled pkt isr %x\n", sts);
+		break;
+	}
+	writel(cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+	writel(sts, i2c_bus->reg_base + AST2600_I2CS_ISR);
+	readl(i2c_bus->reg_base + AST2600_I2CS_ISR);
+}
+
+static int ast2600_i2c_target_irq(struct ast2600_i2c_bus *i2c_bus)
+{
+	u32 ier = readl(i2c_bus->reg_base + AST2600_I2CS_IER);
+	u32 isr = readl(i2c_bus->reg_base + AST2600_I2CS_ISR);
+
+	if (!(isr & ier))
+		return 0;
+
+	/*
+	 * Target interrupt coming after controller packet done
+	 * So need handle controller first.
+	 */
+	if (readl(i2c_bus->reg_base + AST2600_I2CM_ISR) & AST2600_I2CM_PKT_DONE)
+		return 0;
+
+	isr &= ~(AST2600_I2CS_ADDR_INDICATE_MASK);
+
+	if (AST2600_I2CS_ADDR1_NAK & isr)
+		isr &= ~AST2600_I2CS_ADDR1_NAK;
+
+	if (AST2600_I2CS_ADDR2_NAK & isr)
+		isr &= ~AST2600_I2CS_ADDR2_NAK;
+
+	if (AST2600_I2CS_ADDR3_NAK & isr)
+		isr &= ~AST2600_I2CS_ADDR3_NAK;
+
+	if (AST2600_I2CS_ADDR_MASK & isr)
+		isr &= ~AST2600_I2CS_ADDR_MASK;
+
+	if (AST2600_I2CS_PKT_DONE & isr) {
+		if (i2c_bus->mode == DMA_MODE)
+			ast2600_i2c_target_packet_dma_irq(i2c_bus, isr);
+		else
+			ast2600_i2c_target_packet_buff_irq(i2c_bus, isr);
+	} else {
+		ast2600_i2c_target_byte_irq(i2c_bus, isr);
+	}
+
+	return 1;
+}
+#endif
+
 static int ast2600_i2c_setup_dma_tx(u32 cmd, struct ast2600_i2c_bus *i2c_bus)
 {
 	struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
@@ -628,6 +1069,20 @@ static void ast2600_i2c_controller_packet_irq(struct ast2600_i2c_bus *i2c_bus, u
 		}
 		break;
 	case AST2600_I2CM_RX_DONE:
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+		/*
+		 * Workaround for controller/target packet mode enable rx done stuck issue
+		 * When controller go for first read (RX_DONE), target mode will also effect
+		 * Then controller will send nack, not operate anymore.
+		 */
+		if (readl(i2c_bus->reg_base + AST2600_I2CS_CMD_STS) & AST2600_I2CS_PKT_MODE_EN) {
+			u32 target_cmd = readl(i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+
+			writel(0, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+			writel(target_cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+		}
+		fallthrough;
+#endif
 	case AST2600_I2CM_RX_DONE | AST2600_I2CM_NORMAL_STOP:
 		/* do next rx */
 		if (i2c_bus->mode == DMA_MODE) {
@@ -726,6 +1181,12 @@ static irqreturn_t ast2600_i2c_bus_irq(int irq, void *dev_id)
 {
 	struct ast2600_i2c_bus *i2c_bus = dev_id;
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	if (i2c_bus->target) {
+		if (ast2600_i2c_target_irq(i2c_bus))
+			return IRQ_HANDLED;
+	}
+#endif
 	return IRQ_RETVAL(ast2600_i2c_controller_irq(i2c_bus));
 }
 
@@ -742,12 +1203,35 @@ static int ast2600_i2c_controller_xfer(struct i2c_adapter *adap, struct i2c_msg
 			return ret;
 	}
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	if (i2c_bus->mode == BUFF_MODE) {
+		if (i2c_bus->target_active)
+			return -EBUSY;
+		/**
+		 * In BUFF_MODE, controller and target share the same buffer register,
+		 * A target transaction can update buffer state asynchronously via IRQ,
+		 * so block controller transfers while target is active, otherwise we can
+		 * corrupt buffer.
+		 */
+		writel(0, i2c_bus->reg_base + AST2600_I2CS_IER);
+		if (readl(i2c_bus->reg_base + AST2600_I2CS_ISR) || i2c_bus->target_active) {
+			writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_IER);
+			return -EBUSY;
+		}
+	}
+#endif
+
 	i2c_bus->cmd_err = 0;
 	i2c_bus->msgs = msgs;
 	i2c_bus->msgs_index = 0;
 	i2c_bus->msgs_count = num;
 	reinit_completion(&i2c_bus->cmd_complete);
 	ret = ast2600_i2c_do_start(i2c_bus);
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	/* avoid race condication target is wait and controller wait 1st target operate */
+	if (i2c_bus->mode == BUFF_MODE)
+		writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_IER);
+#endif
 	if (ret)
 		goto controller_out;
 	timeout = wait_for_completion_timeout(&i2c_bus->cmd_complete, i2c_bus->adap.timeout);
@@ -765,7 +1249,7 @@ static int ast2600_i2c_controller_xfer(struct i2c_adapter *adap, struct i2c_msg
 		 * a master timeout. In multi-master mode, attempt bus recovery
 		 * if the bus is still busy.
 		 */
-		if (i2c_bus->multi_master &&
+		if (i2c_bus->multi_master && !i2c_bus->target_active &&
 		    (readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF) &
 		    AST2600_I2CC_BUS_BUSY_STS))
 			ast2600_i2c_recover_bus(i2c_bus);
@@ -813,8 +1297,80 @@ static int ast2600_i2c_init(struct ast2600_i2c_bus *i2c_bus)
 	/* Clear Interrupt */
 	writel(GENMASK(27, 0), i2c_bus->reg_base + AST2600_I2CM_ISR);
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	/* for memory buffer initial */
+	if (i2c_bus->dma_available) {
+		i2c_bus->target_dma_buf =
+			dmam_alloc_coherent(i2c_bus->dev, I2C_TARGET_MSG_BUF_SIZE,
+					    &i2c_bus->target_dma_addr, GFP_KERNEL);
+		if (!i2c_bus->target_dma_buf)
+			return -ENOMEM;
+	}
+
+	writel(GENMASK(27, 0), i2c_bus->reg_base + AST2600_I2CS_ISR);
+
+	if (i2c_bus->mode == BYTE_MODE)
+		writel(GENMASK(15, 0), i2c_bus->reg_base + AST2600_I2CS_IER);
+	else
+		writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_IER);
+#endif
+
+	return 0;
+}
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static int ast2600_i2c_reg_target(struct i2c_client *client)
+{
+	struct ast2600_i2c_bus *i2c_bus = i2c_get_adapdata(client->adapter);
+	u32 cmd = TARGET_TRIGGER_CMD;
+
+	if (i2c_bus->target)
+		return -EINVAL;
+
+	dev_dbg(i2c_bus->dev, "target addr %x\n", client->addr);
+
+	writel(0, i2c_bus->reg_base + AST2600_I2CS_ADDR_CTRL);
+	writel(AST2600_I2CC_SLAVE_EN | readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL),
+	       i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+
+	/* trigger rx buffer */
+	if (i2c_bus->mode == DMA_MODE) {
+		cmd |= AST2600_I2CS_RX_DMA_EN;
+		writel(i2c_bus->target_dma_addr, i2c_bus->reg_base + AST2600_I2CS_RX_DMA);
+		writel(i2c_bus->target_dma_addr, i2c_bus->reg_base + AST2600_I2CS_TX_DMA);
+		writel(AST2600_I2CS_SET_RX_DMA_LEN(I2C_TARGET_MSG_BUF_SIZE),
+		       i2c_bus->reg_base + AST2600_I2CS_DMA_LEN);
+	} else if (i2c_bus->mode == BUFF_MODE) {
+		cmd = TARGET_TRIGGER_CMD;
+	} else {
+		cmd &= ~AST2600_I2CS_PKT_MODE_EN;
+	}
+
+	writel(cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+	i2c_bus->target = client;
+	/* Set target addr. */
+	writel(client->addr | AST2600_I2CS_ADDR1_ENABLE,
+	       i2c_bus->reg_base + AST2600_I2CS_ADDR_CTRL);
+
+	return 0;
+}
+
+static int ast2600_i2c_unreg_target(struct i2c_client *client)
+{
+	struct ast2600_i2c_bus *i2c_bus = i2c_get_adapdata(client->adapter);
+	u32 val;
+
+	/* Turn off target mode. */
+	val = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+	writel(val & ~AST2600_I2CC_SLAVE_EN, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+	val = readl(i2c_bus->reg_base + AST2600_I2CS_ADDR_CTRL);
+	writel(val & ~AST2600_I2CS_ADDR1_MASK, i2c_bus->reg_base + AST2600_I2CS_ADDR_CTRL);
+
+	i2c_bus->target = NULL;
+
 	return 0;
 }
+#endif
 
 static u32 ast2600_i2c_functionality(struct i2c_adapter *adap)
 {
@@ -824,6 +1380,10 @@ static u32 ast2600_i2c_functionality(struct i2c_adapter *adap)
 static const struct i2c_algorithm i2c_ast2600_algorithm = {
 	.xfer = ast2600_i2c_controller_xfer,
 	.functionality = ast2600_i2c_functionality,
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	.reg_target = ast2600_i2c_reg_target,
+	.unreg_target = ast2600_i2c_unreg_target,
+#endif
 };
 
 static void ast2600_i2c_set_xfer_mode(struct ast2600_i2c_bus *i2c_bus,
@@ -949,6 +1509,9 @@ static int ast2600_i2c_probe(struct platform_device *pdev)
 		regmap_write(i2c_bus->global_regs, AST2600_I2CG_CLK_DIV_CTRL, I2CCG_DIV_CTRL);
 	}
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	WRITE_ONCE(i2c_bus->target_active, false);
+#endif
 	i2c_bus->dev = dev;
 	i2c_bus->multi_master = device_property_read_bool(dev, "multi-master");
 	i2c_bus->dma_available = device_property_read_bool(dev, "aspeed,enable-dma");

-- 
2.34.1



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

* Re: [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties
  2026-03-24  3:06 ` [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties Ryan Chen
@ 2026-03-24  3:11   ` Jeremy Kerr
  2026-03-25  8:11     ` Ryan Chen
  2026-03-25  1:46   ` Rob Herring (Arm)
  1 sibling, 1 reply; 17+ messages in thread
From: Jeremy Kerr @ 2026-03-24  3:11 UTC (permalink / raw)
  To: Ryan Chen, andriy.shevchenko, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
	Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
  Cc: linux-i2c, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, openbmc

Hi Ryan,

> The AST2600 I2C controller supports three transfer modes (byte, buffer,
> DMA). Add "aspeed,transfer-mode" so DT can select the preferred transfer
> method per controller instance.

This patch does not add an aspeed,transfer-mode property.

> Also add the "aspeed,global-regs"
> phandle to reference the AST2600 global registers syscon/regmap used by
> the controller.
> 
> These properties apply only to the AST2600 binding and are not part of
> the legacy binding, which uses a mixed controller/target register layout
> and does not have the split register blocks or these new configuration
> registers. Legacy DTs remain unchanged.
> 
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> ---
> Changes in v27:
> - change aspeed,transfer-mode to aspeed,enable-dma.

What about all the previous changes?

> ---
>  .../devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml     | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> index de2c359037da..38da6fc6424f 100644
> --- a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> @@ -37,6 +37,21 @@ properties:
>    resets:
>      maxItems: 1
>  
> +  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.

As we had discussed: this does not define the transfer mode, only
whether DMA is available to the peripheral.

Why mention the 16 i2c controllers here?

Please keep this description simple and relevant to the specific purpose
of the property.

> +
> +  aspeed,global-regs:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle reference to the i2c global syscon node, containing the
> +      SoC-common i2c register set.
> +
>  required:
>    - reg
>    - compatible
> @@ -59,4 +74,6 @@ examples:
>          resets = <&syscon ASPEED_RESET_I2C>;
>          clock-frequency = <100000>;
>          interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
> +        aspeed,global-regs = <&i2c_global>;
> +        aspeed,transfer-mode = "buffer";

This example does not match the binding.

Cheers,


Jeremy


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

* Re: [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set
  2026-03-24  3:06 ` [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set Ryan Chen
@ 2026-03-24  3:37   ` Jeremy Kerr
  2026-03-25  8:46     ` Ryan Chen
  2026-03-25 10:48   ` kernel test robot
  2026-03-25 11:20   ` kernel test robot
  2 siblings, 1 reply; 17+ messages in thread
From: Jeremy Kerr @ 2026-03-24  3:37 UTC (permalink / raw)
  To: Ryan Chen, andriy.shevchenko, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
	Benjamin Herrenschmidt, Philipp Zabel
  Cc: linux-i2c, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, openbmc

Hi Ryan,

> +static void ast2600_i2c_set_xfer_mode(struct ast2600_i2c_bus *i2c_bus,
> +                                     enum xfer_mode mode)
> +{
> +       i2c_bus->mode = mode;
> +
> +       switch (mode) {
> +       case DMA_MODE:
> +               i2c_bus->setup_tx = ast2600_i2c_setup_dma_tx;
> +               i2c_bus->setup_rx = ast2600_i2c_setup_dma_rx;
> +               break;
> +       case BYTE_MODE:
> +               i2c_bus->setup_tx = ast2600_i2c_setup_byte_tx;
> +               i2c_bus->setup_rx = ast2600_i2c_setup_byte_rx;
> +               break;
> +       case BUFF_MODE:
> +       default:
> +               i2c_bus->setup_tx = ast2600_i2c_setup_buff_tx;
> +               i2c_bus->setup_rx = ast2600_i2c_setup_buff_rx;
> +               break;
> +       }
> +}
> +
> +static int ast2600_i2c_xfer_mode_parse(struct ast2600_i2c_bus *i2c_bus,
> +                                      const char *buf, enum xfer_mode *mode)
> +{
> +       if (sysfs_streq(buf, "byte")) {
> +               *mode = BYTE_MODE;
> +               return 0;
> +       }
> +
> +       if (sysfs_streq(buf, "buffer")) {
> +               if (!i2c_bus->buf_base)
> +                       return -EINVAL;
> +               *mode = BUFF_MODE;
> +               return 0;
> +       }
> +
> +       if (sysfs_streq(buf, "dma")) {
> +               if (!i2c_bus->dma_available)
> +                       return -EINVAL;
> +               *mode = DMA_MODE;
> +               return 0;
> +       }
> +
> +       return -EINVAL;
> +}

I would suggest separating the string parsing from the "is the mode
available" logic, more on that below.

> +
> +static const char *ast2600_i2c_xfer_mode_name(enum xfer_mode mode)
> +{
> +       switch (mode) {
> +       case BYTE_MODE:
> +               return "byte";
> +       case DMA_MODE:
> +               return "dma";
> +       case BUFF_MODE:
> +       default:
> +               return "buffer";
> +       }
> +}
> +
> +static ssize_t xfer_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +       struct ast2600_i2c_bus *i2c_bus = dev_get_drvdata(dev);
> +
> +       return sysfs_emit(buf, "%s\n", ast2600_i2c_xfer_mode_name(i2c_bus->mode));
> +}
> +
> +static ssize_t xfer_mode_store(struct device *dev,
> +                              struct device_attribute *attr,
> +                              const char *buf, size_t count)
> +{
> +       struct ast2600_i2c_bus *i2c_bus = dev_get_drvdata(dev);
> +       enum xfer_mode mode;
> +       int ret;
> +
> +       ret = ast2600_i2c_xfer_mode_parse(i2c_bus, buf, &mode);
> +       if (ret)
> +               return ret;
> +
> +       i2c_lock_bus(&i2c_bus->adap, I2C_LOCK_ROOT_ADAPTER);
> +       ast2600_i2c_set_xfer_mode(i2c_bus, mode);
> +       i2c_unlock_bus(&i2c_bus->adap, I2C_LOCK_ROOT_ADAPTER);
> +
> +       return count;
> +}
> +
> +static DEVICE_ATTR_RW(xfer_mode);

This will need sysfs ABI documentation.

> +
> +static int ast2600_i2c_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct ast2600_i2c_bus *i2c_bus;
> +       struct reset_control *rst;
> +       struct resource *res;
> +       u32 global_ctrl;
> +       int ret;
> +
> +       if (!device_property_present(dev, "aspeed,global-regs"))
> +               return -ENODEV;
> +
> +       i2c_bus = devm_kzalloc(dev, sizeof(*i2c_bus), GFP_KERNEL);
> +       if (!i2c_bus)
> +               return -ENOMEM;
> +
> +       i2c_bus->reg_base = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(i2c_bus->reg_base))
> +               return PTR_ERR(i2c_bus->reg_base);
> +
> +       rst = devm_reset_control_get_shared_deasserted(dev, NULL);
> +       if (IS_ERR(rst))
> +               return dev_err_probe(dev, PTR_ERR(rst), "Missing reset ctrl\n");
> +
> +       i2c_bus->global_regs =
> +               syscon_regmap_lookup_by_phandle(dev_of_node(dev), "aspeed,global-regs");
> +       if (IS_ERR(i2c_bus->global_regs))
> +               return PTR_ERR(i2c_bus->global_regs);
> +
> +       regmap_read(i2c_bus->global_regs, AST2600_I2CG_CTRL, &global_ctrl);
> +       if ((global_ctrl & AST2600_GLOBAL_INIT) != AST2600_GLOBAL_INIT) {
> +               regmap_write(i2c_bus->global_regs, AST2600_I2CG_CTRL, AST2600_GLOBAL_INIT);
> +               regmap_write(i2c_bus->global_regs, AST2600_I2CG_CLK_DIV_CTRL, I2CCG_DIV_CTRL);
> +       }
> +
> +       i2c_bus->dev = dev;
> +       i2c_bus->multi_master = device_property_read_bool(dev, "multi-master");
> +       i2c_bus->dma_available = device_property_read_bool(dev, "aspeed,enable-dma");
> +       if (i2c_bus->dma_abailable)

dma_abailable? you didn't even build this? :(

> +               i2c_bus->mode = DMA_MODE;
> +       else
> +               i2c_bus->mode = BUFF_MODE;
> +
> +       if (i2c_bus->mode == BUFF_MODE) {
> +               i2c_bus->buf_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);

So you only set ->buf_base if we are in buffer mode during probe.

However, the ->buf_base check in xfer_mode_parse() will fail when trying
to change from any other mode to buffer mode. This means you can never
select buffer mode after probe.


> +               if (IS_ERR(i2c_bus->buf_base))
> +                       i2c_bus->mode = BYTE_MODE;
> +               else
> +                       i2c_bus->buf_size = resource_size(res) / 2;
> +       }
> +
> +       ast2600_i2c_set_xfer_mode(i2c_bus, i2c_bus->mode);

Minor: set_xfer_mode() sets i2c_bus->mode itself. Maybe you want a
temporary instead, or change the semantics of that?

> +
> +       /*
> +        * i2c timeout counter: use base clk4 1Mhz,
> +        * per unit: 1/(1000/1024) = 1024us
> +        */
> +       ret = device_property_read_u32(dev, "i2c-scl-clk-low-timeout-us", &i2c_bus->timeout);
> +       if (!ret)
> +               i2c_bus->timeout = DIV_ROUND_UP(i2c_bus->timeout, 1024);
> +
> +       init_completion(&i2c_bus->cmd_complete);
> +
> +       i2c_bus->irq = platform_get_irq(pdev, 0);
> +       if (i2c_bus->irq < 0)
> +               return i2c_bus->irq;
> +
> +       platform_set_drvdata(pdev, i2c_bus);
> +
> +       i2c_bus->clk = devm_clk_get(i2c_bus->dev, NULL);
> +       if (IS_ERR(i2c_bus->clk))
> +               return dev_err_probe(i2c_bus->dev, PTR_ERR(i2c_bus->clk), "Can't get clock\n");
> +
> +       i2c_bus->apb_clk = clk_get_rate(i2c_bus->clk);
> +
> +       i2c_parse_fw_timings(i2c_bus->dev, &i2c_bus->timing_info, true);
> +
> +       /* Initialize the I2C adapter */
> +       i2c_bus->adap.owner = THIS_MODULE;
> +       i2c_bus->adap.algo = &i2c_ast2600_algorithm;
> +       i2c_bus->adap.retries = 0;
> +       i2c_bus->adap.dev.parent = i2c_bus->dev;
> +       device_set_node(&i2c_bus->adap.dev, dev_fwnode(dev));
> +       i2c_bus->adap.algo_data = i2c_bus;
> +       strscpy(i2c_bus->adap.name, pdev->name);
> +       i2c_set_adapdata(&i2c_bus->adap, i2c_bus);
> +
> +       ret = ast2600_i2c_init(i2c_bus);
> +       if (ret < 0)
> +               return dev_err_probe(dev, ret, "Unable to initial i2c %d\n", ret);

Super minor: `initial` is not a verb in this context, you want
`initialise` (Australian) or `initialize` (US) or `init` (keeping
everyone happy)

> +
> +       ret = devm_request_irq(dev, i2c_bus->irq, ast2600_i2c_bus_irq, 0,
> +                              dev_name(dev), i2c_bus);
> +       if (ret < 0) {
> +               ret = dev_err_probe(dev, ret, "Unable to request irq %d\n",
> +                                   i2c_bus->irq);
> +               goto err;
> +       }
> +
> +       writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
> +              i2c_bus->reg_base + AST2600_I2CM_IER);
> +
> +       ret = devm_i2c_add_adapter(dev, &i2c_bus->adap);
> +       if (ret)
> +               goto err;
> +
> +       ret = sysfs_create_file(&dev->kobj, &dev_attr_xfer_mode.attr);
> +       if (ret)
> +               goto err;

This error path will fail the probe but not unregister the i2c adapter.
You probably want to only register the adapter last (and remove the
sysfs file if that fails).

> +
> +       return 0;
> +
> +err:
> +       writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> +       writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
> +       return ret;
> +}
> +
> +static void ast2600_i2c_remove(struct platform_device *pdev)
> +{
> +       struct ast2600_i2c_bus *i2c_bus = platform_get_drvdata(pdev);
> +
> +       sysfs_remove_file(&pdev->dev.kobj, &dev_attr_xfer_mode.attr);
> +
> +       /* Disable everything. */
> +       writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> +       writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
> +}
> +
> +static const struct of_device_id ast2600_i2c_of_match[] = {
> +       { .compatible = "aspeed,ast2600-i2c-bus" },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, ast2600_i2c_of_match);
> +
> +static struct platform_driver ast2600_i2c_driver = {
> +       .probe          = ast2600_i2c_probe,
> +       .remove         = ast2600_i2c_remove,
> +       .driver         = {
> +               .name           = "ast2600-i2c",
> +               .of_match_table = ast2600_i2c_of_match,
> +       },
> +};
> +module_platform_driver(ast2600_i2c_driver);
> +
> +MODULE_AUTHOR("Ryan Chen <ryan_chen@aspeedtech.com>");
> +MODULE_DESCRIPTION("ASPEED AST2600 I2C Controller Driver");
> +MODULE_LICENSE("GPL");

Cheers,


Jeremy

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

* Re: [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties
  2026-03-24  3:06 ` [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties Ryan Chen
  2026-03-24  3:11   ` Jeremy Kerr
@ 2026-03-25  1:46   ` Rob Herring (Arm)
  1 sibling, 0 replies; 17+ messages in thread
From: Rob Herring (Arm) @ 2026-03-25  1:46 UTC (permalink / raw)
  To: Ryan Chen
  Cc: jk, devicetree, Conor Dooley, Andrew Jeffery, Krzysztof Kozlowski,
	Joel Stanley, Rayn Chen, linux-arm-kernel, linux-i2c, Andi Shyti,
	andriy.shevchenko, Philipp Zabel, linux-kernel, openbmc,
	linux-aspeed, Benjamin Herrenschmidt


On Tue, 24 Mar 2026 11:06:27 +0800, Ryan Chen wrote:
> The AST2600 I2C controller supports three transfer modes (byte, buffer,
> DMA). Add "aspeed,transfer-mode" so DT can select the preferred transfer
> method per controller instance. Also add the "aspeed,global-regs"
> phandle to reference the AST2600 global registers syscon/regmap used by
> the controller.
> 
> These properties apply only to the AST2600 binding and are not part of
> the legacy binding, which uses a mixed controller/target register layout
> and does not have the split register blocks or these new configuration
> registers. Legacy DTs remain unchanged.
> 
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> ---
> Changes in v27:
> - change aspeed,transfer-mode to aspeed,enable-dma.
> ---
>  .../devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml     | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.example.dtb: i2c@80 (aspeed,ast2600-i2c-bus): Unevaluated properties are not allowed ('aspeed,transfer-mode' was unexpected)
	from schema $id: http://devicetree.org/schemas/i2c/aspeed,ast2600-i2c.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260324-upstream_i2c-v27-2-f19b511c8c28@aspeedtech.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.



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

* RE: [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties
  2026-03-24  3:11   ` Jeremy Kerr
@ 2026-03-25  8:11     ` Ryan Chen
  2026-03-25 16:52       ` Rob Herring
  0 siblings, 1 reply; 17+ messages in thread
From: Ryan Chen @ 2026-03-25  8:11 UTC (permalink / raw)
  To: Jeremy Kerr, andriy.shevchenko@linux.intel.com, Andi Shyti,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley,
	Andrew Jeffery, Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
  Cc: linux-i2c@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	openbmc@lists.ozlabs.org

Hello Jeremy,
	Thanks the review.

> Subject: Re: [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs
> and transfer-mode properties
> 
> Hi Ryan,
> 
> > The AST2600 I2C controller supports three transfer modes (byte,
> > buffer, DMA). Add "aspeed,transfer-mode" so DT can select the
> > preferred transfer method per controller instance.
> 
> This patch does not add an aspeed,transfer-mode property.
Will update use aspeed,enable-dma
> 
> > Also add the "aspeed,global-regs"
> > phandle to reference the AST2600 global registers syscon/regmap used
> > by the controller.
> >
> > These properties apply only to the AST2600 binding and are not part of
> > the legacy binding, which uses a mixed controller/target register
> > layout and does not have the split register blocks or these new
> > configuration registers. Legacy DTs remain unchanged.
> >
> > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > ---
> > Changes in v27:
> > - change aspeed,transfer-mode to aspeed,enable-dma.
> 
> What about all the previous changes?
Will update
Remove aspeed,transfer-mode instead aspeed,enable-dma
> 
> > ---
> >  .../devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml     | 17
> > +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> > b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> > index de2c359037da..38da6fc6424f 100644
> > --- a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> > +++ b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> > @@ -37,6 +37,21 @@ properties:
> >    resets:
> >      maxItems: 1
> >
> > +  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.
> 
> As we had discussed: this does not define the transfer mode, only whether
> DMA is available to the peripheral.
> 
> Why mention the 16 i2c controllers here?
> 
> Please keep this description simple and relevant to the specific purpose of the
> property.

Will update with following.
description: Enable DMA for transfers on this I2C bus.
> 
> > +
> > +  aspeed,global-regs:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      Phandle reference to the i2c global syscon node, containing the
> > +      SoC-common i2c register set.
> > +
> >  required:
> >    - reg
> >    - compatible
> > @@ -59,4 +74,6 @@ examples:
> >          resets = <&syscon ASPEED_RESET_I2C>;
> >          clock-frequency = <100000>;
> >          interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
> > +        aspeed,global-regs = <&i2c_global>;
> > +        aspeed,transfer-mode = "buffer";
> 
> This example does not match the binding.
Will remove aspeed,transfer-mode = "buffer";
Add with aspeed,enable-dma

> 
> Cheers,
> 
> 
> Jeremy

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

* RE: [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set
  2026-03-24  3:37   ` Jeremy Kerr
@ 2026-03-25  8:46     ` Ryan Chen
  2026-03-25  9:15       ` Jeremy Kerr
  0 siblings, 1 reply; 17+ messages in thread
From: Ryan Chen @ 2026-03-25  8:46 UTC (permalink / raw)
  To: Jeremy Kerr, andriy.shevchenko@linux.intel.com, Andi Shyti,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley,
	Andrew Jeffery, Benjamin Herrenschmidt, Philipp Zabel
  Cc: linux-i2c@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	openbmc@lists.ozlabs.org

> Subject: Re: [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600
> new register set
> 
> Hi Ryan,
> 
> > +static void ast2600_i2c_set_xfer_mode(struct ast2600_i2c_bus
> > +*i2c_bus,
> > +                                     enum xfer_mode
> mode) {
> > +       i2c_bus->mode = mode;
> > +
> > +       switch (mode) {
> > +       case DMA_MODE:
> > +               i2c_bus->setup_tx = ast2600_i2c_setup_dma_tx;
> > +               i2c_bus->setup_rx = ast2600_i2c_setup_dma_rx;
> > +               break;
> > +       case BYTE_MODE:
> > +               i2c_bus->setup_tx = ast2600_i2c_setup_byte_tx;
> > +               i2c_bus->setup_rx = ast2600_i2c_setup_byte_rx;
> > +               break;
> > +       case BUFF_MODE:
> > +       default:
> > +               i2c_bus->setup_tx = ast2600_i2c_setup_buff_tx;
> > +               i2c_bus->setup_rx = ast2600_i2c_setup_buff_rx;
> > +               break;
> > +       }
> > +}
> > +
> > +static int ast2600_i2c_xfer_mode_parse(struct ast2600_i2c_bus
> > +*i2c_bus,
> > +                                      const char *buf,
> enum xfer_mode
> > +*mode) {
> > +       if (sysfs_streq(buf, "byte")) {
> > +               *mode = BYTE_MODE;
> > +               return 0;
> > +       }
> > +
> > +       if (sysfs_streq(buf, "buffer")) {
> > +               if (!i2c_bus->buf_base)
> > +                       return -EINVAL;
> > +               *mode = BUFF_MODE;
> > +               return 0;
> > +       }
> > +
> > +       if (sysfs_streq(buf, "dma")) {
> > +               if (!i2c_bus->dma_available)
> > +                       return -EINVAL;
> > +               *mode = DMA_MODE;
> > +               return 0;
> > +       }
> > +
> > +       return -EINVAL;
> > +}
> 
> I would suggest separating the string parsing from the "is the mode available"
> logic, more on that below.
>
I will separate with following.
 
static int ast2600_i2c_xfer_mode_parse(const char *buf, enum xfer_mode *mode)
{
    if (sysfs_streq(buf, "byte"))   { *mode = BYTE_MODE; return 0; }
    if (sysfs_streq(buf, "buffer")) { *mode = BUFF_MODE; return 0; }
    if (sysfs_streq(buf, "dma"))    { *mode = DMA_MODE;  return 0; }
    return -EINVAL;
}

static int ast2600_i2c_xfer_mode_check(struct ast2600_i2c_bus *i2c_bus,
                                       enum xfer_mode mode)
{
    if (mode == BUFF_MODE && !i2c_bus->buf_base)
        return -EINVAL;
    if (mode == DMA_MODE && !i2c_bus->dma_available)
        return -EINVAL;
    return 0;
}

> > +
> > +static const char *ast2600_i2c_xfer_mode_name(enum xfer_mode mode) {
> > +       switch (mode) {
> > +       case BYTE_MODE:
> > +               return "byte";
> > +       case DMA_MODE:
> > +               return "dma";
> > +       case BUFF_MODE:
> > +       default:
> > +               return "buffer";
> > +       }
> > +}
> > +
> > +static ssize_t xfer_mode_show(struct device *dev, struct
> > +device_attribute *attr, char *buf) {
> > +       struct ast2600_i2c_bus *i2c_bus = dev_get_drvdata(dev);
> > +
> > +       return sysfs_emit(buf, "%s\n",
> > +ast2600_i2c_xfer_mode_name(i2c_bus->mode));
> > +}
> > +
> > +static ssize_t xfer_mode_store(struct device *dev,
> > +                              struct device_attribute *attr,
> > +                              const char *buf, size_t count)
> {
> > +       struct ast2600_i2c_bus *i2c_bus = dev_get_drvdata(dev);
> > +       enum xfer_mode mode;
> > +       int ret;
> > +
> > +       ret = ast2600_i2c_xfer_mode_parse(i2c_bus, buf, &mode);
> > +       if (ret)
> > +               return ret;
> > +
> > +       i2c_lock_bus(&i2c_bus->adap, I2C_LOCK_ROOT_ADAPTER);
> > +       ast2600_i2c_set_xfer_mode(i2c_bus, mode);
> > +       i2c_unlock_bus(&i2c_bus->adap, I2C_LOCK_ROOT_ADAPTER);
> > +
> > +       return count;
> > +}
> > +
> > +static DEVICE_ATTR_RW(xfer_mode);
> 
> This will need sysfs ABI documentation.

Since it is in sysfs /sys/bus/platform/drivers/i2c_ast2600
So I add Documentation/ABI/testing/sysfs-bus-platform-drivers-i2c-ast2600 
am I right? 

What:		/sys/bus/platform/drivers/i2c-ast2600/.../xfer_mode
Date:		March 2026
KernelVersion:	6.x
Contact:	Ryan Chen <ryan_chen@aspeedtech.com>
Description:
		Shows or sets the transfer mode for the ASPEED AST2600 I2C
		controller. Valid values are:

		- "byte":   Programmed I/O, one byte at a time.
		- "buffer": Programmed I/O using the hardware FIFO buffer.
		- "dma":    DMA transfer (only available if aspeed,enable-dma
		            is set in the device tree).

		Writing an unsupported mode returns -EINVAL.

> 
> > +
> > +static int ast2600_i2c_probe(struct platform_device *pdev) {
> > +       struct device *dev = &pdev->dev;
> > +       struct ast2600_i2c_bus *i2c_bus;
> > +       struct reset_control *rst;
> > +       struct resource *res;
> > +       u32 global_ctrl;
> > +       int ret;
> > +
> > +       if (!device_property_present(dev, "aspeed,global-regs"))
> > +               return -ENODEV;
> > +
> > +       i2c_bus = devm_kzalloc(dev, sizeof(*i2c_bus), GFP_KERNEL);
> > +       if (!i2c_bus)
> > +               return -ENOMEM;
> > +
> > +       i2c_bus->reg_base = devm_platform_ioremap_resource(pdev, 0);
> > +       if (IS_ERR(i2c_bus->reg_base))
> > +               return PTR_ERR(i2c_bus->reg_base);
> > +
> > +       rst = devm_reset_control_get_shared_deasserted(dev, NULL);
> > +       if (IS_ERR(rst))
> > +               return dev_err_probe(dev, PTR_ERR(rst), "Missing
> reset
> > +ctrl\n");
> > +
> > +       i2c_bus->global_regs =
> >
> +               syscon_regmap_lookup_by_phandle(dev_of_node(dev
> ),
> > +"aspeed,global-regs");
> > +       if (IS_ERR(i2c_bus->global_regs))
> > +               return PTR_ERR(i2c_bus->global_regs);
> > +
> > +       regmap_read(i2c_bus->global_regs, AST2600_I2CG_CTRL,
> > +&global_ctrl);
> > +       if ((global_ctrl & AST2600_GLOBAL_INIT) !=
> > +AST2600_GLOBAL_INIT) {
> > +               regmap_write(i2c_bus->global_regs,
> AST2600_I2CG_CTRL,
> > +AST2600_GLOBAL_INIT);
> > +               regmap_write(i2c_bus->global_regs,
> > +AST2600_I2CG_CLK_DIV_CTRL, I2CCG_DIV_CTRL);
> > +       }
> > +
> > +       i2c_bus->dev = dev;
> > +       i2c_bus->multi_master = device_property_read_bool(dev,
> > +"multi-master");
> > +       i2c_bus->dma_available = device_property_read_bool(dev,
> > +"aspeed,enable-dma");
> > +       if (i2c_bus->dma_abailable)
> 
> dma_abailable? you didn't even build this? :(
OH, Sorry it is my bad. 
> 
> > +               i2c_bus->mode = DMA_MODE;
> > +       else
> > +               i2c_bus->mode = BUFF_MODE;
> > +
> > +       if (i2c_bus->mode == BUFF_MODE) {
> > +               i2c_bus->buf_base =
> > +devm_platform_get_and_ioremap_resource(pdev, 1, &res);
> 
> So you only set ->buf_base if we are in buffer mode during probe.
> 
> However, the ->buf_base check in xfer_mode_parse() will fail when trying to
> change from any other mode to buffer mode. This means you can never select
> buffer mode after probe.


Will modify with following.

i2c_bus->buf_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
if (!IS_ERR(i2c_bus->buf_base))
    i2c_bus->buf_size = resource_size(res) / 2;
else
    i2c_bus->buf_base = NULL;


> 
> 
> > +               if (IS_ERR(i2c_bus->buf_base))
> > +                       i2c_bus->mode = BYTE_MODE;
> > +               else
> > +                       i2c_bus->buf_size = resource_size(res) /
> 2;
> > +       }
> > +
> > +       ast2600_i2c_set_xfer_mode(i2c_bus, i2c_bus->mode);
> 
> Minor: set_xfer_mode() sets i2c_bus->mode itself. Maybe you want a
> temporary instead, or change the semantics of that?
Will update 

if (i2c_bus->dma_available)
    mode = DMA_MODE;
else if (i2c_bus->buf_base)
    mode = BUFF_MODE;
else
    mode = BYTE_MODE;

ast2600_i2c_set_xfer_mode(i2c_bus, mode);
> 
> > +
> > +       /*
> > +        * i2c timeout counter: use base clk4 1Mhz,
> > +        * per unit: 1/(1000/1024) = 1024us
> > +        */
> > +       ret = device_property_read_u32(dev,
> > +"i2c-scl-clk-low-timeout-us", &i2c_bus->timeout);
> > +       if (!ret)
> > +               i2c_bus->timeout =
> DIV_ROUND_UP(i2c_bus->timeout,
> > +1024);
> > +
> > +       init_completion(&i2c_bus->cmd_complete);
> > +
> > +       i2c_bus->irq = platform_get_irq(pdev, 0);
> > +       if (i2c_bus->irq < 0)
> > +               return i2c_bus->irq;
> > +
> > +       platform_set_drvdata(pdev, i2c_bus);
> > +
> > +       i2c_bus->clk = devm_clk_get(i2c_bus->dev, NULL);
> > +       if (IS_ERR(i2c_bus->clk))
> > +               return dev_err_probe(i2c_bus->dev,
> > +PTR_ERR(i2c_bus->clk), "Can't get clock\n");
> > +
> > +       i2c_bus->apb_clk = clk_get_rate(i2c_bus->clk);
> > +
> > +       i2c_parse_fw_timings(i2c_bus->dev, &i2c_bus->timing_info,
> > +true);
> > +
> > +       /* Initialize the I2C adapter */
> > +       i2c_bus->adap.owner = THIS_MODULE;
> > +       i2c_bus->adap.algo = &i2c_ast2600_algorithm;
> > +       i2c_bus->adap.retries = 0;
> > +       i2c_bus->adap.dev.parent = i2c_bus->dev;
> > +       device_set_node(&i2c_bus->adap.dev, dev_fwnode(dev));
> > +       i2c_bus->adap.algo_data = i2c_bus;
> > +       strscpy(i2c_bus->adap.name, pdev->name);
> > +       i2c_set_adapdata(&i2c_bus->adap, i2c_bus);
> > +
> > +       ret = ast2600_i2c_init(i2c_bus);
> > +       if (ret < 0)
> > +               return dev_err_probe(dev, ret, "Unable to initial i2c
> > +%d\n", ret);
> 
> Super minor: `initial` is not a verb in this context, you want `initialise`
> (Australian) or `initialize` (US) or `init` (keeping everyone happy)

Will update 
> 
> > +
> > +       ret = devm_request_irq(dev, i2c_bus->irq, ast2600_i2c_bus_irq,
> > +0,
> > +                              dev_name(dev), i2c_bus);
> > +       if (ret < 0) {
> > +               ret = dev_err_probe(dev, ret, "Unable to request irq
> > +%d\n",
> > +                                   i2c_bus->irq);
> > +               goto err;
> > +       }
> > +
> > +       writel(AST2600_I2CM_PKT_DONE |
> AST2600_I2CM_BUS_RECOVER,
> > +              i2c_bus->reg_base + AST2600_I2CM_IER);
> > +
> > +       ret = devm_i2c_add_adapter(dev, &i2c_bus->adap);
> > +       if (ret)
> > +               goto err;
> > +
> > +       ret = sysfs_create_file(&dev->kobj, &dev_attr_xfer_mode.attr);
> > +       if (ret)
> > +               goto err;
> 
> This error path will fail the probe but not unregister the i2c adapter.
> You probably want to only register the adapter last (and remove the sysfs file if
> that fails).

Will update
sysfs_create_file();
if (ret)
    goto err;    

devm_i2c_add_adapter();
if (ret) {
    sysfs_remove_file(); 
    goto err;
}
> 
> > +
> > +       return 0;
> > +
> > +err:
> > +       writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > +       writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
> > +       return ret;
> > +}
> > +
> > +static void ast2600_i2c_remove(struct platform_device *pdev) {
> > +       struct ast2600_i2c_bus *i2c_bus = platform_get_drvdata(pdev);
> > +
> > +       sysfs_remove_file(&pdev->dev.kobj, &dev_attr_xfer_mode.attr);
> > +
> > +       /* Disable everything. */
> > +       writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > +       writel(0, i2c_bus->reg_base + AST2600_I2CM_IER); }
> > +
> > +static const struct of_device_id ast2600_i2c_of_match[] = {
> > +       { .compatible = "aspeed,ast2600-i2c-bus" },
> > +       { }
> > +};
> > +MODULE_DEVICE_TABLE(of, ast2600_i2c_of_match);
> > +
> > +static struct platform_driver ast2600_i2c_driver = {
> > +       .probe          = ast2600_i2c_probe,
> > +       .remove         = ast2600_i2c_remove,
> > +       .driver         = {
> > +               .name           = "ast2600-i2c",
> > +               .of_match_table = ast2600_i2c_of_match,
> > +       },
> > +};
> > +module_platform_driver(ast2600_i2c_driver);
> > +
> > +MODULE_AUTHOR("Ryan Chen <ryan_chen@aspeedtech.com>");
> > +MODULE_DESCRIPTION("ASPEED AST2600 I2C Controller Driver");
> > +MODULE_LICENSE("GPL");
> 
> Cheers,
> 
> 
> Jeremy

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

* Re: [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set
  2026-03-25  8:46     ` Ryan Chen
@ 2026-03-25  9:15       ` Jeremy Kerr
  2026-03-26  2:04         ` Ryan Chen
  0 siblings, 1 reply; 17+ messages in thread
From: Jeremy Kerr @ 2026-03-25  9:15 UTC (permalink / raw)
  To: Ryan Chen, andriy.shevchenko@linux.intel.com, Andi Shyti,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley,
	Andrew Jeffery, Benjamin Herrenschmidt, Philipp Zabel
  Cc: linux-i2c@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	openbmc@lists.ozlabs.org

Hi Ryan,

> > I would suggest separating the string parsing from the "is the mode available"
> > logic, more on that below.
> > 
> I will separate with following.
>  
> static int ast2600_i2c_xfer_mode_parse(const char *buf, enum xfer_mode *mode)
> {
>     if (sysfs_streq(buf, "byte"))   { *mode = BYTE_MODE; return 0; }
>     if (sysfs_streq(buf, "buffer")) { *mode = BUFF_MODE; return 0; }
>     if (sysfs_streq(buf, "dma"))    { *mode = DMA_MODE;  return 0; }
>     return -EINVAL;
> }

OK, but with kernel-style formatting.

> 
> static int ast2600_i2c_xfer_mode_check(struct ast2600_i2c_bus *i2c_bus,
>                                        enum xfer_mode mode)
> {
>     if (mode == BUFF_MODE && !i2c_bus->buf_base)
>         return -EINVAL;
>     if (mode == DMA_MODE && !i2c_bus->dma_available)
>         return -EINVAL;
>     return 0;
> }
> 
> > > +
> > > +static const char *ast2600_i2c_xfer_mode_name(enum xfer_mode mode) {
> > > +       switch (mode) {
> > > +       case BYTE_MODE:
> > > +               return "byte";
> > > +       case DMA_MODE:
> > > +               return "dma";
> > > +       case BUFF_MODE:
> > > +       default:
> > > +               return "buffer";
> > > +       }
> > > +}
> > > +
> > > +static ssize_t xfer_mode_show(struct device *dev, struct
> > > +device_attribute *attr, char *buf) {
> > > +       struct ast2600_i2c_bus *i2c_bus = dev_get_drvdata(dev);
> > > +
> > > +       return sysfs_emit(buf, "%s\n",
> > > +ast2600_i2c_xfer_mode_name(i2c_bus->mode));
> > > +}
> > > +
> > > +static ssize_t xfer_mode_store(struct device *dev,
> > > +                              struct device_attribute *attr,
> > > +                              const char *buf, size_t count)
> > {
> > > +       struct ast2600_i2c_bus *i2c_bus = dev_get_drvdata(dev);
> > > +       enum xfer_mode mode;
> > > +       int ret;
> > > +
> > > +       ret = ast2600_i2c_xfer_mode_parse(i2c_bus, buf, &mode);
> > > +       if (ret)
> > > +               return ret;
> > > +
> > > +       i2c_lock_bus(&i2c_bus->adap, I2C_LOCK_ROOT_ADAPTER);
> > > +       ast2600_i2c_set_xfer_mode(i2c_bus, mode);
> > > +       i2c_unlock_bus(&i2c_bus->adap, I2C_LOCK_ROOT_ADAPTER);
> > > +
> > > +       return count;
> > > +}
> > > +
> > > +static DEVICE_ATTR_RW(xfer_mode);
> > 
> > This will need sysfs ABI documentation.
> 
> Since it is in sysfs /sys/bus/platform/drivers/i2c_ast2600
> So I add Documentation/ABI/testing/sysfs-bus-platform-drivers-i2c-ast2600 
> am I right?

I would suggest Documentation/ABI/testing/sysfs-driver-ast2600-i2c

> 
> What:           /sys/bus/platform/drivers/i2c-ast2600/.../xfer_mode
> Date:           March 2026
> KernelVersion:  6.x

KernelVersion is optional, but if you include it, it would be 7.x.

> Contact:        Ryan Chen <ryan_chen@aspeedtech.com>
> Description:

Keep the first line of the description on the same line.

>                 Shows or sets the transfer mode for the ASPEED AST2600 I2C
>                 controller. Valid values are:
> 
>                 - "byte":   Programmed I/O, one byte at a time.
>                 - "buffer": Programmed I/O using the hardware FIFO buffer.
>                 - "dma":    DMA transfer (only available if aspeed,enable-dma
>                             is set in the device tree).

Decouple this from the device tree configuration mechanism:

                 - "dma":    DMA transfer (if DMA is available for this
                             controller)

> i2c_bus->buf_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
> if (!IS_ERR(i2c_bus->buf_base))
>     i2c_bus->buf_size = resource_size(res) / 2;
> else
>     i2c_bus->buf_base = NULL;

I would suggest a temporary, so there's no chance that future changes
could see an ERR_PTR value in i2c_bus->buf_base:

    buf_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
    if (!IS_ERR(buf_base)) {
        i2c_bus->buf_base = buf_base
        i2c_bus->buf_size = resource_size(res) / 2;
    }

and you have kzalloc()ed, so no need for the NULL init in the error path.

Cheers,


Jeremy

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

* Re: [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set
  2026-03-24  3:06 ` [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set Ryan Chen
  2026-03-24  3:37   ` Jeremy Kerr
@ 2026-03-25 10:48   ` kernel test robot
  2026-03-25 11:20   ` kernel test robot
  2 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2026-03-25 10:48 UTC (permalink / raw)
  To: Ryan Chen, jk, andriy.shevchenko, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
	Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
  Cc: oe-kbuild-all, linux-i2c, devicetree, linux-arm-kernel,
	linux-aspeed, linux-kernel, openbmc, Ryan Chen

Hi Ryan,

kernel test robot noticed the following build errors:

[auto build test ERROR on 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f]

url:    https://github.com/intel-lab-lkp/linux/commits/Ryan-Chen/dt-bindings-i2c-Split-AST2600-binding-into-a-new-YAML/20260325-112805
base:   6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
patch link:    https://lore.kernel.org/r/20260324-upstream_i2c-v27-3-f19b511c8c28%40aspeedtech.com
patch subject: [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set
config: sparc-randconfig-r073-20260325 (https://download.01.org/0day-ci/archive/20260325/202603251835.KJc3nKCn-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 8.5.0
smatch: v0.5.0-9004-gb810ac53
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260325/202603251835.KJc3nKCn-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603251835.KJc3nKCn-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/i2c/busses/i2c-ast2600.c: In function 'ast2600_i2c_probe':
>> drivers/i2c/busses/i2c-ast2600.c:955:15: error: 'struct ast2600_i2c_bus' has no member named 'dma_abailable'; did you mean 'dma_available'?
     if (i2c_bus->dma_abailable)
                  ^~~~~~~~~~~~~
                  dma_available


vim +955 drivers/i2c/busses/i2c-ast2600.c

   916	
   917	static int ast2600_i2c_probe(struct platform_device *pdev)
   918	{
   919		struct device *dev = &pdev->dev;
   920		struct ast2600_i2c_bus *i2c_bus;
   921		struct reset_control *rst;
   922		struct resource *res;
   923		u32 global_ctrl;
   924		int ret;
   925	
   926		if (!device_property_present(dev, "aspeed,global-regs"))
   927			return -ENODEV;
   928	
   929		i2c_bus = devm_kzalloc(dev, sizeof(*i2c_bus), GFP_KERNEL);
   930		if (!i2c_bus)
   931			return -ENOMEM;
   932	
   933		i2c_bus->reg_base = devm_platform_ioremap_resource(pdev, 0);
   934		if (IS_ERR(i2c_bus->reg_base))
   935			return PTR_ERR(i2c_bus->reg_base);
   936	
   937		rst = devm_reset_control_get_shared_deasserted(dev, NULL);
   938		if (IS_ERR(rst))
   939			return dev_err_probe(dev, PTR_ERR(rst), "Missing reset ctrl\n");
   940	
   941		i2c_bus->global_regs =
   942			syscon_regmap_lookup_by_phandle(dev_of_node(dev), "aspeed,global-regs");
   943		if (IS_ERR(i2c_bus->global_regs))
   944			return PTR_ERR(i2c_bus->global_regs);
   945	
   946		regmap_read(i2c_bus->global_regs, AST2600_I2CG_CTRL, &global_ctrl);
   947		if ((global_ctrl & AST2600_GLOBAL_INIT) != AST2600_GLOBAL_INIT) {
   948			regmap_write(i2c_bus->global_regs, AST2600_I2CG_CTRL, AST2600_GLOBAL_INIT);
   949			regmap_write(i2c_bus->global_regs, AST2600_I2CG_CLK_DIV_CTRL, I2CCG_DIV_CTRL);
   950		}
   951	
   952		i2c_bus->dev = dev;
   953		i2c_bus->multi_master = device_property_read_bool(dev, "multi-master");
   954		i2c_bus->dma_available = device_property_read_bool(dev, "aspeed,enable-dma");
 > 955		if (i2c_bus->dma_abailable)
   956			i2c_bus->mode = DMA_MODE;
   957		else
   958			i2c_bus->mode = BUFF_MODE;
   959	
   960		if (i2c_bus->mode == BUFF_MODE) {
   961			i2c_bus->buf_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
   962			if (IS_ERR(i2c_bus->buf_base))
   963				i2c_bus->mode = BYTE_MODE;
   964			else
   965				i2c_bus->buf_size = resource_size(res) / 2;
   966		}
   967	
   968		ast2600_i2c_set_xfer_mode(i2c_bus, i2c_bus->mode);
   969	
   970		/*
   971		 * i2c timeout counter: use base clk4 1Mhz,
   972		 * per unit: 1/(1000/1024) = 1024us
   973		 */
   974		ret = device_property_read_u32(dev, "i2c-scl-clk-low-timeout-us", &i2c_bus->timeout);
   975		if (!ret)
   976			i2c_bus->timeout = DIV_ROUND_UP(i2c_bus->timeout, 1024);
   977	
   978		init_completion(&i2c_bus->cmd_complete);
   979	
   980		i2c_bus->irq = platform_get_irq(pdev, 0);
   981		if (i2c_bus->irq < 0)
   982			return i2c_bus->irq;
   983	
   984		platform_set_drvdata(pdev, i2c_bus);
   985	
   986		i2c_bus->clk = devm_clk_get(i2c_bus->dev, NULL);
   987		if (IS_ERR(i2c_bus->clk))
   988			return dev_err_probe(i2c_bus->dev, PTR_ERR(i2c_bus->clk), "Can't get clock\n");
   989	
   990		i2c_bus->apb_clk = clk_get_rate(i2c_bus->clk);
   991	
   992		i2c_parse_fw_timings(i2c_bus->dev, &i2c_bus->timing_info, true);
   993	
   994		/* Initialize the I2C adapter */
   995		i2c_bus->adap.owner = THIS_MODULE;
   996		i2c_bus->adap.algo = &i2c_ast2600_algorithm;
   997		i2c_bus->adap.retries = 0;
   998		i2c_bus->adap.dev.parent = i2c_bus->dev;
   999		device_set_node(&i2c_bus->adap.dev, dev_fwnode(dev));
  1000		i2c_bus->adap.algo_data = i2c_bus;
  1001		strscpy(i2c_bus->adap.name, pdev->name);
  1002		i2c_set_adapdata(&i2c_bus->adap, i2c_bus);
  1003	
  1004		ret = ast2600_i2c_init(i2c_bus);
  1005		if (ret < 0)
  1006			return dev_err_probe(dev, ret, "Unable to initial i2c %d\n", ret);
  1007	
  1008		ret = devm_request_irq(dev, i2c_bus->irq, ast2600_i2c_bus_irq, 0,
  1009				       dev_name(dev), i2c_bus);
  1010		if (ret < 0) {
  1011			ret = dev_err_probe(dev, ret, "Unable to request irq %d\n",
  1012					    i2c_bus->irq);
  1013			goto err;
  1014		}
  1015	
  1016		writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
  1017		       i2c_bus->reg_base + AST2600_I2CM_IER);
  1018	
  1019		ret = devm_i2c_add_adapter(dev, &i2c_bus->adap);
  1020		if (ret)
  1021			goto err;
  1022	
  1023		ret = sysfs_create_file(&dev->kobj, &dev_attr_xfer_mode.attr);
  1024		if (ret)
  1025			goto err;
  1026	
  1027		return 0;
  1028	
  1029	err:
  1030		writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
  1031		writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
  1032		return ret;
  1033	}
  1034	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


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

* Re: [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set
  2026-03-24  3:06 ` [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set Ryan Chen
  2026-03-24  3:37   ` Jeremy Kerr
  2026-03-25 10:48   ` kernel test robot
@ 2026-03-25 11:20   ` kernel test robot
  2026-03-25 11:26     ` Krzysztof Kozlowski
  2 siblings, 1 reply; 17+ messages in thread
From: kernel test robot @ 2026-03-25 11:20 UTC (permalink / raw)
  To: Ryan Chen, jk, andriy.shevchenko, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
	Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
  Cc: llvm, oe-kbuild-all, linux-i2c, devicetree, linux-arm-kernel,
	linux-aspeed, linux-kernel, openbmc, Ryan Chen

Hi Ryan,

kernel test robot noticed the following build errors:

[auto build test ERROR on 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f]

url:    https://github.com/intel-lab-lkp/linux/commits/Ryan-Chen/dt-bindings-i2c-Split-AST2600-binding-into-a-new-YAML/20260325-112805
base:   6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
patch link:    https://lore.kernel.org/r/20260324-upstream_i2c-v27-3-f19b511c8c28%40aspeedtech.com
patch subject: [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set
config: i386-buildonly-randconfig-003-20260325 (https://download.01.org/0day-ci/archive/20260325/202603251922.TFJuUipj-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260325/202603251922.TFJuUipj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603251922.TFJuUipj-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/i2c/busses/i2c-ast2600.c:955:15: error: no member named 'dma_abailable' in 'struct ast2600_i2c_bus'; did you mean 'dma_available'?
     955 |         if (i2c_bus->dma_abailable)
         |                      ^~~~~~~~~~~~~
         |                      dma_available
   drivers/i2c/busses/i2c-ast2600.c:271:9: note: 'dma_available' declared here
     271 |         bool                    dma_available;
         |                                 ^
   1 error generated.


vim +955 drivers/i2c/busses/i2c-ast2600.c

   916	
   917	static int ast2600_i2c_probe(struct platform_device *pdev)
   918	{
   919		struct device *dev = &pdev->dev;
   920		struct ast2600_i2c_bus *i2c_bus;
   921		struct reset_control *rst;
   922		struct resource *res;
   923		u32 global_ctrl;
   924		int ret;
   925	
   926		if (!device_property_present(dev, "aspeed,global-regs"))
   927			return -ENODEV;
   928	
   929		i2c_bus = devm_kzalloc(dev, sizeof(*i2c_bus), GFP_KERNEL);
   930		if (!i2c_bus)
   931			return -ENOMEM;
   932	
   933		i2c_bus->reg_base = devm_platform_ioremap_resource(pdev, 0);
   934		if (IS_ERR(i2c_bus->reg_base))
   935			return PTR_ERR(i2c_bus->reg_base);
   936	
   937		rst = devm_reset_control_get_shared_deasserted(dev, NULL);
   938		if (IS_ERR(rst))
   939			return dev_err_probe(dev, PTR_ERR(rst), "Missing reset ctrl\n");
   940	
   941		i2c_bus->global_regs =
   942			syscon_regmap_lookup_by_phandle(dev_of_node(dev), "aspeed,global-regs");
   943		if (IS_ERR(i2c_bus->global_regs))
   944			return PTR_ERR(i2c_bus->global_regs);
   945	
   946		regmap_read(i2c_bus->global_regs, AST2600_I2CG_CTRL, &global_ctrl);
   947		if ((global_ctrl & AST2600_GLOBAL_INIT) != AST2600_GLOBAL_INIT) {
   948			regmap_write(i2c_bus->global_regs, AST2600_I2CG_CTRL, AST2600_GLOBAL_INIT);
   949			regmap_write(i2c_bus->global_regs, AST2600_I2CG_CLK_DIV_CTRL, I2CCG_DIV_CTRL);
   950		}
   951	
   952		i2c_bus->dev = dev;
   953		i2c_bus->multi_master = device_property_read_bool(dev, "multi-master");
   954		i2c_bus->dma_available = device_property_read_bool(dev, "aspeed,enable-dma");
 > 955		if (i2c_bus->dma_abailable)
   956			i2c_bus->mode = DMA_MODE;
   957		else
   958			i2c_bus->mode = BUFF_MODE;
   959	
   960		if (i2c_bus->mode == BUFF_MODE) {
   961			i2c_bus->buf_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
   962			if (IS_ERR(i2c_bus->buf_base))
   963				i2c_bus->mode = BYTE_MODE;
   964			else
   965				i2c_bus->buf_size = resource_size(res) / 2;
   966		}
   967	
   968		ast2600_i2c_set_xfer_mode(i2c_bus, i2c_bus->mode);
   969	
   970		/*
   971		 * i2c timeout counter: use base clk4 1Mhz,
   972		 * per unit: 1/(1000/1024) = 1024us
   973		 */
   974		ret = device_property_read_u32(dev, "i2c-scl-clk-low-timeout-us", &i2c_bus->timeout);
   975		if (!ret)
   976			i2c_bus->timeout = DIV_ROUND_UP(i2c_bus->timeout, 1024);
   977	
   978		init_completion(&i2c_bus->cmd_complete);
   979	
   980		i2c_bus->irq = platform_get_irq(pdev, 0);
   981		if (i2c_bus->irq < 0)
   982			return i2c_bus->irq;
   983	
   984		platform_set_drvdata(pdev, i2c_bus);
   985	
   986		i2c_bus->clk = devm_clk_get(i2c_bus->dev, NULL);
   987		if (IS_ERR(i2c_bus->clk))
   988			return dev_err_probe(i2c_bus->dev, PTR_ERR(i2c_bus->clk), "Can't get clock\n");
   989	
   990		i2c_bus->apb_clk = clk_get_rate(i2c_bus->clk);
   991	
   992		i2c_parse_fw_timings(i2c_bus->dev, &i2c_bus->timing_info, true);
   993	
   994		/* Initialize the I2C adapter */
   995		i2c_bus->adap.owner = THIS_MODULE;
   996		i2c_bus->adap.algo = &i2c_ast2600_algorithm;
   997		i2c_bus->adap.retries = 0;
   998		i2c_bus->adap.dev.parent = i2c_bus->dev;
   999		device_set_node(&i2c_bus->adap.dev, dev_fwnode(dev));
  1000		i2c_bus->adap.algo_data = i2c_bus;
  1001		strscpy(i2c_bus->adap.name, pdev->name);
  1002		i2c_set_adapdata(&i2c_bus->adap, i2c_bus);
  1003	
  1004		ret = ast2600_i2c_init(i2c_bus);
  1005		if (ret < 0)
  1006			return dev_err_probe(dev, ret, "Unable to initial i2c %d\n", ret);
  1007	
  1008		ret = devm_request_irq(dev, i2c_bus->irq, ast2600_i2c_bus_irq, 0,
  1009				       dev_name(dev), i2c_bus);
  1010		if (ret < 0) {
  1011			ret = dev_err_probe(dev, ret, "Unable to request irq %d\n",
  1012					    i2c_bus->irq);
  1013			goto err;
  1014		}
  1015	
  1016		writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
  1017		       i2c_bus->reg_base + AST2600_I2CM_IER);
  1018	
  1019		ret = devm_i2c_add_adapter(dev, &i2c_bus->adap);
  1020		if (ret)
  1021			goto err;
  1022	
  1023		ret = sysfs_create_file(&dev->kobj, &dev_attr_xfer_mode.attr);
  1024		if (ret)
  1025			goto err;
  1026	
  1027		return 0;
  1028	
  1029	err:
  1030		writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
  1031		writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
  1032		return ret;
  1033	}
  1034	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


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

* Re: [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set
  2026-03-25 11:20   ` kernel test robot
@ 2026-03-25 11:26     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-25 11:26 UTC (permalink / raw)
  To: kernel test robot, Ryan Chen, jk, andriy.shevchenko, Andi Shyti,
	Rob Herring, Conor Dooley, Joel Stanley, Andrew Jeffery,
	Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
  Cc: llvm, oe-kbuild-all, linux-i2c, devicetree, linux-arm-kernel,
	linux-aspeed, linux-kernel, openbmc

On 25/03/2026 12:20, kernel test robot wrote:
> Hi Ryan,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Ryan-Chen/dt-bindings-i2c-Split-AST2600-binding-into-a-new-YAML/20260325-112805
> base:   6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
> patch link:    https://lore.kernel.org/r/20260324-upstream_i2c-v27-3-f19b511c8c28%40aspeedtech.com
> patch subject: [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set
> config: i386-buildonly-randconfig-003-20260325 (https://download.01.org/0day-ci/archive/20260325/202603251922.TFJuUipj-lkp@intel.com/config)
> compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260325/202603251922.TFJuUipj-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202603251922.TFJuUipj-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>>> drivers/i2c/busses/i2c-ast2600.c:955:15: error: no member named 'dma_abailable' in 'struct ast2600_i2c_bus'; did you mean 'dma_available'?
>      955 |         if (i2c_bus->dma_abailable)


v27 which is still not building.

This is madness.

Best regards,
Krzysztof


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

* Re: [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties
  2026-03-25  8:11     ` Ryan Chen
@ 2026-03-25 16:52       ` Rob Herring
  2026-03-26  2:19         ` Ryan Chen
  0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2026-03-25 16:52 UTC (permalink / raw)
  To: Ryan Chen
  Cc: Jeremy Kerr, andriy.shevchenko@linux.intel.com, Andi Shyti,
	Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
	Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel,
	linux-i2c@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	openbmc@lists.ozlabs.org

On Wed, Mar 25, 2026 at 08:11:54AM +0000, Ryan Chen wrote:
> Hello Jeremy,
> 	Thanks the review.
> 
> > Subject: Re: [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs
> > and transfer-mode properties
> > 
> > Hi Ryan,
> > 
> > > The AST2600 I2C controller supports three transfer modes (byte,
> > > buffer, DMA). Add "aspeed,transfer-mode" so DT can select the
> > > preferred transfer method per controller instance.
> > 
> > This patch does not add an aspeed,transfer-mode property.
> Will update use aspeed,enable-dma

[...]

> > > +  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.
> > 
> > As we had discussed: this does not define the transfer mode, only whether
> > DMA is available to the peripheral.
> > 
> > Why mention the 16 i2c controllers here?
> > 
> > Please keep this description simple and relevant to the specific purpose of the
> > property.
> 
> Will update with following.
> description: Enable DMA for transfers on this I2C bus.

You still don't understand the distinction. It's not enable, but that 
the h/w instance *has* DMA capability or not. It is still up to the OS 
what to do with that information.

Rob


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

* RE: [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set
  2026-03-25  9:15       ` Jeremy Kerr
@ 2026-03-26  2:04         ` Ryan Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Ryan Chen @ 2026-03-26  2:04 UTC (permalink / raw)
  To: Jeremy Kerr, andriy.shevchenko@linux.intel.com, Andi Shyti,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley,
	Andrew Jeffery, Benjamin Herrenschmidt, Philipp Zabel
  Cc: linux-i2c@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	openbmc@lists.ozlabs.org

> Subject: Re: [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600
> new register set
> 
> Hi Ryan,
> 
> > > I would suggest separating the string parsing from the "is the mode
> available"
> > > logic, more on that below.
> > >
> > I will separate with following.
> >
> > static int ast2600_i2c_xfer_mode_parse(const char *buf, enum xfer_mode
> > *mode) {
> >     if (sysfs_streq(buf, "byte"))   { *mode = BYTE_MODE; return 0; }
> >     if (sysfs_streq(buf, "buffer")) { *mode = BUFF_MODE; return 0; }
> >     if (sysfs_streq(buf, "dma"))    { *mode = DMA_MODE;  return 0; }
> >     return -EINVAL;
> > }
> 
> OK, but with kernel-style formatting.
Yes, will update
static int ast2600_i2c_xfer_mode_parse(const char *buf, enum xfer_mode *mode)
{
	if (sysfs_streq(buf, "byte")) {
		*mode = BYTE_MODE;
		return 0;
	}

	if (sysfs_streq(buf, "buffer")) {
		*mode = BUFF_MODE;
		return 0;
	}

	if (sysfs_streq(buf, "dma")) {
		*mode = DMA_MODE;
		return 0;
	}

	return -EINVAL;
}
> 
> >
> > static int ast2600_i2c_xfer_mode_check(struct ast2600_i2c_bus
> > *i2c_bus,
> >                                        enum
> xfer_mode mode) {
> >     if (mode == BUFF_MODE && !i2c_bus->buf_base)
> >         return -EINVAL;
> >     if (mode == DMA_MODE && !i2c_bus->dma_available)
> >         return -EINVAL;
> >     return 0;
> > }
> >
> > > > +
> > > > +static const char *ast2600_i2c_xfer_mode_name(enum xfer_mode
> > > > +mode) {
> > > > +       switch (mode) {
> > > > +       case BYTE_MODE:
> > > > +               return "byte";
> > > > +       case DMA_MODE:
> > > > +               return "dma";
> > > > +       case BUFF_MODE:
> > > > +       default:
> > > > +               return "buffer";
> > > > +       }
> > > > +}
> > > > +
> > > > +static ssize_t xfer_mode_show(struct device *dev, struct
> > > > +device_attribute *attr, char *buf) {
> > > > +       struct ast2600_i2c_bus *i2c_bus = dev_get_drvdata(dev);
> > > > +
> > > > +       return sysfs_emit(buf, "%s\n",
> > > > +ast2600_i2c_xfer_mode_name(i2c_bus->mode));
> > > > +}
> > > > +
> > > > +static ssize_t xfer_mode_store(struct device *dev,
> > > > +                              struct device_attribute
> *attr,
> > > > +                              const char *buf, size_t
> count)
> > > {
> > > > +       struct ast2600_i2c_bus *i2c_bus = dev_get_drvdata(dev);
> > > > +       enum xfer_mode mode;
> > > > +       int ret;
> > > > +
> > > > +       ret = ast2600_i2c_xfer_mode_parse(i2c_bus, buf, &mode);
> > > > +       if (ret)
> > > > +               return ret;
> > > > +
> > > > +       i2c_lock_bus(&i2c_bus->adap, I2C_LOCK_ROOT_ADAPTER);
> > > > +       ast2600_i2c_set_xfer_mode(i2c_bus, mode);
> > > > +       i2c_unlock_bus(&i2c_bus->adap,
> I2C_LOCK_ROOT_ADAPTER);
> > > > +
> > > > +       return count;
> > > > +}
> > > > +
> > > > +static DEVICE_ATTR_RW(xfer_mode);
> > >
> > > This will need sysfs ABI documentation.
> >
> > Since it is in sysfs /sys/bus/platform/drivers/i2c_ast2600
> > So I add
> > Documentation/ABI/testing/sysfs-bus-platform-drivers-i2c-ast2600
> > am I right?
> 
> I would suggest Documentation/ABI/testing/sysfs-driver-ast2600-i2c
Thanks  ~ will update.
> 
> >
> > What:           /sys/bus/platform/drivers/i2c-ast2600/.../xfer_mode
> > Date:           March 2026
> > KernelVersion:  6.x
> 
> KernelVersion is optional, but if you include it, it would be 7.x.

Will update 
> 
> > Contact:        Ryan Chen <ryan_chen@aspeedtech.com>
> > Description:
> 
> Keep the first line of the description on the same line.

Will update
> 
> >                 Shows or sets the transfer mode for the ASPEED
> AST2600
> > I2C
> >                 controller. Valid values are:
> >
> >                 - "byte":   Programmed I/O, one byte at a time.
> >                 - "buffer": Programmed I/O using the hardware
> FIFO buffer.
> >                 - "dma":    DMA transfer (only available if
> > aspeed,enable-dma
> >                             is set in the device tree).
> 
> Decouple this from the device tree configuration mechanism:
Will remove.
> 
>                  - "dma":    DMA transfer (if DMA is available for
> this
>                              controller)
> 
> > i2c_bus->buf_base = devm_platform_get_and_ioremap_resource(pdev, 1,
> > &res); if (!IS_ERR(i2c_bus->buf_base))
> >     i2c_bus->buf_size = resource_size(res) / 2; else
> >     i2c_bus->buf_base = NULL;
> 
> I would suggest a temporary, so there's no chance that future changes could
> see an ERR_PTR value in i2c_bus->buf_base:
> 
>     buf_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
>     if (!IS_ERR(buf_base)) {
>         i2c_bus->buf_base = buf_base
>         i2c_bus->buf_size = resource_size(res) / 2;
>     }
> 
> and you have kzalloc()ed, so no need for the NULL init in the error path.
Thanks will update.
> 
> Cheers,
> 
> 
> Jeremy

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

* RE: [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties
  2026-03-25 16:52       ` Rob Herring
@ 2026-03-26  2:19         ` Ryan Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Ryan Chen @ 2026-03-26  2:19 UTC (permalink / raw)
  To: Rob Herring
  Cc: Jeremy Kerr, andriy.shevchenko@linux.intel.com, Andi Shyti,
	Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
	Benjamin Herrenschmidt, Philipp Zabel, linux-i2c@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	openbmc@lists.ozlabs.org


> Subject: Re: [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs
> and transfer-mode properties
> 
> On Wed, Mar 25, 2026 at 08:11:54AM +0000, Ryan Chen wrote:
> > Hello Jeremy,
> > 	Thanks the review.
> >
> > > Subject: Re: [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add
> > > global-regs and transfer-mode properties
> > >
> > > Hi Ryan,
> > >
> > > > The AST2600 I2C controller supports three transfer modes (byte,
> > > > buffer, DMA). Add "aspeed,transfer-mode" so DT can select the
> > > > preferred transfer method per controller instance.
> > >
> > > This patch does not add an aspeed,transfer-mode property.
> > Will update use aspeed,enable-dma
> 
> [...]
> 
> > > > +  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.
> > >
> > > As we had discussed: this does not define the transfer mode, only
> > > whether DMA is available to the peripheral.
> > >
> > > Why mention the 16 i2c controllers here?
> > >
> > > Please keep this description simple and relevant to the specific
> > > purpose of the property.
> >
> > Will update with following.
> > description: Enable DMA for transfers on this I2C bus.
> 
> You still don't understand the distinction. It's not enable, but that the h/w
> instance *has* DMA capability or not. It is still up to the OS what to do with
> that information.
> 
Thanks 
Will update
description: Indicates this I2C controller instance has DMA capability.


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

end of thread, other threads:[~2026-03-26  2:19 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-24  3:06 [PATCH v27 0/4] Add ASPEED AST2600 I2C controller driver Ryan Chen
2026-03-24  3:06 ` [PATCH v27 1/4] dt-bindings: i2c: Split AST2600 binding into a new YAML Ryan Chen
2026-03-24  3:06 ` [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties Ryan Chen
2026-03-24  3:11   ` Jeremy Kerr
2026-03-25  8:11     ` Ryan Chen
2026-03-25 16:52       ` Rob Herring
2026-03-26  2:19         ` Ryan Chen
2026-03-25  1:46   ` Rob Herring (Arm)
2026-03-24  3:06 ` [PATCH v27 3/4] i2c: ast2600: Add controller driver for AST2600 new register set Ryan Chen
2026-03-24  3:37   ` Jeremy Kerr
2026-03-25  8:46     ` Ryan Chen
2026-03-25  9:15       ` Jeremy Kerr
2026-03-26  2:04         ` Ryan Chen
2026-03-25 10:48   ` kernel test robot
2026-03-25 11:20   ` kernel test robot
2026-03-25 11:26     ` Krzysztof Kozlowski
2026-03-24  3:06 ` [PATCH v27 4/4] i2c: ast2600: Add target mode support Ryan Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox