[PATCH v30 0/5] Add ASPEED AST2600 I2C controller driver

[PATCH v30 0/5] Add ASPEED AST2600 I2C controller driver

[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 and packet-based
transfer support 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 DT binding change to
reflect the reference to the global register set. 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
  (present 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 v30:
- 1/5: aspeed,ast2600-i2c.yaml: keep backward compatibility for
  existing in-tree AST2600 device trees (Sashiko AI review).
  - reg: add minItems: 1 so legacy single-reg DTs still validate.
  - retain bus-frequency as a deprecated property so DTs that still
    use it are not rejected by unevaluatedProperties: false.
- 2/5: new patch "i2c: aspeed: Read clock-frequency via
  i2c_parse_fw_timings()". The legacy i2c-aspeed driver now reads
  the standard clock-frequency property first and falls back to
  bus-frequency, avoiding a silent 100 kHz downgrade when a DT
  follows the updated binding but still binds to the legacy
  driver (Sashiko AI review).
- 4/5: address Sashiko AI code review feedback:
  - Use manual i2c_add_adapter() / i2c_del_adapter() instead of
    devm_i2c_add_adapter() so the adapter is torn down before the
    hardware is disabled in remove(); otherwise client .remove()
    callbacks can fail or hang after FUN_CTRL/IER have been cleared.
  - synchronize_irq() and clear pending IRQ status on the controller
    timeout path to avoid the ISR racing with the next transfer and
    touching freed msgs.
  - Use clamp_t() for AC TIMING divisor / scl_low / scl_high so
    extreme clock-frequency values cannot underflow into the unsigned
    domain and corrupt the AC TIMING register.
  - Derive the RX buffer offset from buf_size instead of hardcoding
    0x10, since the dual-pool split is configurable.
  - Clamp i2c-scl-clk-low-timeout-us to the TTIMEOUT field's 5-bit
    range (max 31 * 1024us) and emit a dev_warn() instead of letting
    AST2600_I2CC_TTIMEOUT()'s mask silently truncate larger values.
  - Return -EBUSY (not -ENOMEM) for every ast2600_i2c_do_start()
    failure path in the controller packet IRQ handler (NORMAL_STOP,
    TX_ACK, and RX_DONE branches).
  - Advertise I2C_AQ_NO_ZERO_LEN_READ via i2c_adapter_quirks so the
    i2c-core rejects zero-byte reads before they reach the driver.
    The AST2600 packet engine cannot encode a zero-length RX command
    and would otherwise stall waiting for an RX_DONE that never
    arrives.
- 5/5: address Sashiko AI code review feedback:
  - Force-stop path (target IRQ aborting an in-flight controller
    transfer): disable the controller IER and W1C-clear pending ISR
    before calling complete(), then restore the IER after the
    wake-up. Without the disable/clear sequence the controller IRQ
    handler can race with the target abort path and double-complete
    or touch freed msgs.
  - unreg_target() teardown ordering: disable the target IER first,
    then disable SLAVE_EN / clear ADDR_CTRL, synchronize_irq(), W1C
    pending ISR, and only then NULL i2c_bus->target and clear
    target_active. The old order left IER enabled while target was
    being cleared, allowing an in-flight handler to dereference a
    target pointer the caller had already freed.
  - reg_target() bring-up ordering: assign i2c_bus->target before
    enabling SLAVE_EN. Otherwise an IRQ that fires after SLAVE_EN
    is set but before the pointer is stored finds target == NULL,
    exits without clearing the ISR, and the unmasked event re-fires
    as an IRQ storm.
  - Use writel() instead of writeb() when staging a TX byte into
    the target buffer. The AST2600 buffer SRAM only supports 32-bit
    accesses; byte writes are silently dropped (or, on some
    revisions, raise a bus fault), so a SLAVE_READ_REQUESTED reply
    never reaches the master.
  - reg_target() rejects 10-bit client addresses with
    -EAFNOSUPPORT. AST2600_I2CS_ADDR1 is only a 7-bit field;
    without the check, the high bits of a 10-bit address overflow
    into the adjacent ADDR2 field and silently corrupt a second
    target slot.
  - Initialise the local `u8 value` to 0 in the target packet IRQ
    handler. Its address is passed to i2c_slave_event() for events
    such as I2C_SLAVE_STOP / I2C_SLAVE_READ_REQUESTED; a slave
    backend that reads the byte before writing would otherwise leak
    uninitialised kernel stack.
- Link to v29: https://lore.kernel.org/r/20260415-upstream_i2c-v29-0-317c1a905ae1@aspeedtech.com

Changes in v29:
- 2/4: remove aspeed,enable-dma properties.
- 3/4: update commit message remove transfer mode selection.
- 3/4: remove sysfs file.
- 3/4: remove define I2C_TARGET_MSG_BUF_SIZE and AST2600_I2C_DMA_SIZE.
- 3/4: remove buf_index in struct ast2600_i2c_bus.
- 3/4, 4/4: remove dma/byte mode, use buffer mode only.
- 4/4: fix race between unreg_target and IRQ handler.
- 4/4: move i2cs ier enable from ast2600_i2c_init to probe after master ier enable.
- Link to v28: https://lore.kernel.org/r/20260330-upstream_i2c-v28-0-17bdae39c5cb@aspeedtech.com

Changes in v28:
- 2/4: update commit message correspond with aspeed,enable-dma.
- 2/4: remove aspeed,transfer-mode and add aspeed,enable-dma property
  and description.
- 2/4: Fix aspeed,enable-dma description to reflect hardware capability
  rather than software behavior.
- 3/4: Separate xfer_mode_store into distinct parse and availability-check
  steps by introducing ast2600_i2c_xfer_mode_check().
- 3/4: fix tx dma memcpy source point address.
- 3/4: Use a temporary variable for devm_platform_get_and_ioremap_resource()
  to avoid storing an ERR_PTR in i2c_bus->buf_base; drop the redundant
  NULL assignment in the error path since i2c_bus is kzalloc()ed.
- 3/4: Add ABI documentation file
  Documentation/ABI/testing/sysfs-driver-ast2600-i2c.
- 4/4: fix typo condication -> condition.
- 4/4: fix compile error, when disable CONFIG_I2C_SLAVE.
- Link to v27: https://lore.kernel.org/r/20260324-upstream_i2c-v27-0-f19b511c8c28@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 (5):
      dt-bindings: i2c: Split AST2600 binding into a new YAML
      i2c: aspeed: Read clock-frequency via i2c_parse_fw_timings()
      dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs properties
      i2c: ast2600: Add controller driver for AST2600 new register set
      i2c: ast2600: Add target mode support

 .../bindings/i2c/aspeed,ast2600-i2c.yaml           |   80 ++
 .../devicetree/bindings/i2c/aspeed,i2c.yaml        |    3 +-
 drivers/i2c/busses/Makefile                        |    2 +-
 drivers/i2c/busses/i2c-aspeed.c                    |   24 +-
 drivers/i2c/busses/i2c-ast2600.c                   | 1186 ++++++++++++++++++++
 5 files changed, 1286 insertions(+), 9 deletions(-)
---
base-commit: a293ec25d59dd96309058c70df5a4dd0f889a1e4
change-id: 20260223-upstream_i2c-ebd07f89739c

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

[PATCH v30 1/5] dt-bindings: i2c: Split AST2600 binding into a new YAML

[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.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v30:
- Add minItems: 1 to reg so existing AST2600 DTs with a single reg
  region continue to validate (Sashiko AI review)
- Retain bus-frequency as a deprecated property to avoid breaking
  existing AST2600 DTs under unevaluatedProperties: false
  (Sashiko AI review)
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           | 73 ++++++++++++++++++++++
 .../devicetree/bindings/i2c/aspeed,i2c.yaml        |  3 +-
 2 files changed, 74 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..abc614315dff
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
@@ -0,0 +1,73 @@
+# 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:
+    minItems: 1
+    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
+
+  bus-frequency:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    deprecated: true
+    description:
+      Legacy name for clock-frequency. Existing AST2600 device trees
+      used this before the binding was split out. New device trees
+      should use the standard clock-frequency property instead.
+    minimum: 500
+    maximum: 4000000
+
+  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

[PATCH v30 2/5] i2c: aspeed: Read clock-frequency via i2c_parse_fw_timings()

[Ryan Chen]

Use i2c_parse_fw_timings() to read the standard "clock-frequency"
property, and fall back to "bus-frequency" only when the standard
property is absent.

This honors device trees written against the updated
aspeed,ast2600-i2c binding without silently falling back to 100 kHz,
while keeping existing in-tree device trees using "bus-frequency"
working.

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index a26b74c71206..137889e57ddc 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -1000,6 +1000,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	const struct of_device_id *match;
 	struct aspeed_i2c_bus *bus;
 	struct clk *parent_clk;
+	struct i2c_timings timings;
 	int irq, ret;
 
 	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
@@ -1025,12 +1026,18 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	}
 	reset_control_deassert(bus->rst);
 
-	ret = of_property_read_u32(pdev->dev.of_node,
-				   "bus-frequency", &bus->bus_frequency);
-	if (ret < 0) {
-		dev_err(&pdev->dev,
-			"Could not read bus-frequency property\n");
-		bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
+	i2c_parse_fw_timings(&pdev->dev, &timings, false);
+	if (timings.bus_freq_hz) {
+		bus->bus_frequency = timings.bus_freq_hz;
+	} else {
+		ret = of_property_read_u32(pdev->dev.of_node,
+					   "bus-frequency",
+					   &bus->bus_frequency);
+		if (ret < 0) {
+			dev_err(&pdev->dev,
+				"Could not read clock-frequency or bus-frequency property\n");
+			bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
+		}
 	}
 
 	match = of_match_node(aspeed_i2c_bus_of_table, pdev->dev.of_node);

-- 
2.34.1

[PATCH v30 3/5] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs properties

[Ryan Chen]

Add the aspeed,global-regs phandle to reference the AST2600 global
registers syscon node, containing the SoC-common I2C register set.

These properties apply only to the AST2600 binding. Legacy DTs remain
unchanged.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v29:
- remove aspeed,enable-dma properties.

Changes in v28:
- update commit message correspond with aspeed,enable-dma.
- remove aspeed,transfer-mode and add aspeed,enable-dma property and
  description.
- Fix aspeed,enable-dma description to reflect hardware capability rather
  than software behavior

Changes in v27:
- change aspeed,transfer-mode to aspeed,enable-dma.
---
 Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
index abc614315dff..21469715d845 100644
--- a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
@@ -48,6 +48,12 @@ properties:
   resets:
     maxItems: 1
 
+  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
@@ -70,4 +76,5 @@ examples:
         resets = <&syscon ASPEED_RESET_I2C>;
         clock-frequency = <100000>;
         interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+        aspeed,global-regs = <&i2c_global>;
     };

-- 
2.34.1

[PATCH v30 4/5] i2c: ast2600: Add controller driver for AST2600 new register set

[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)
- 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.

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.

Contact: Ryan Chen <ryan_chen@aspeedtech.com>
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v30:
- Address Sashiko AI code review feedback:
  - Use manual i2c_add_adapter() / i2c_del_adapter() instead of
    devm_i2c_add_adapter() so the adapter is torn down before the
    hardware is disabled in remove(); otherwise client .remove()
    callbacks can fail or hang after FUN_CTRL/IER have been cleared.
  - synchronize_irq() and clear pending IRQ status on the controller
    timeout path to avoid the ISR racing with the next transfer and
    touching freed msgs.
  - Use clamp_t() for AC TIMING divisor / scl_low / scl_high so
    extreme clock-frequency values cannot underflow into the unsigned
    domain and corrupt the AC TIMING register.
  - Derive the RX buffer offset from buf_size instead of hardcoding
    0x10, since the dual-pool split is configurable.
  - Clamp i2c-scl-clk-low-timeout-us to the TTIMEOUT field's 5-bit
    range (max 31 * 1024us) and emit a dev_warn() instead of letting
    AST2600_I2CC_TTIMEOUT()'s mask silently truncate larger values.
- Return -EBUSY (not -ENOMEM) for every ast2600_i2c_do_start() failure
  path in the controller packet IRQ handler (NORMAL_STOP, TX_ACK, and
  RX_DONE branches).
- Advertise I2C_AQ_NO_ZERO_LEN_READ via i2c_adapter_quirks so the
  i2c-core rejects zero-byte reads before they reach the driver. The
  AST2600 packet engine cannot encode a zero-length RX command and
  would otherwise stall waiting for an RX_DONE that never arrives.

Changes in v29:
- update commit message remove transfer mode selection.
- remove dma/byte transfer, use buffer mode only.
- remove sysfs file.
- remove define I2C_TARGET_MSG_BUF_SIZE and AST2600_I2C_DMA_SIZE.
- remove buf_index in struct ast2600_i2c_bus.

Changes in v28:
- Separate xfer_mode_store into distinct parse and availability-check
  steps by introducing ast2600_i2c_xfer_mode_check()
- fix tx dma memcpy source point address.
- Use a temporary variable for devm_platform_get_and_ioremap_resource()
  to avoid storing an ERR_PTR in i2c_bus->buf_base; drop the redundant
  NULL assignment in the error path since i2c_bus is kzalloc()ed
- Add ABI documentation file
  Documentation/ABI/testing/sysfs-driver-ast2600-i2c

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 | 829 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 835 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 137889e57ddc..fb4b2ac7b6ee 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>
 
@@ -1003,6 +1004,10 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	struct i2c_timings timings;
 	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..a06d0483865e
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ast2600.c
@@ -0,0 +1,829 @@
+// 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/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/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 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
+
+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;
+	u32			apb_clk;
+	u32			timeout;
+	int			irq;
+	int			cmd_err;
+	int			msgs_index;
+	int			msgs_count;
+	int			controller_xfer_cnt;
+	size_t			buf_size;
+	bool			multi_master;
+	void __iomem		*buf_base;
+};
+
+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 = clamp_t(int, baseclk_idx, 0, 15);
+	divisor = clamp_t(int, divisor, 2, 32);
+	scl_low = clamp_t(int, divisor * 9 / 16 - 1, 0, 15);
+	scl_high = clamp_t(int, divisor - scl_low - 2, 1, 15);
+	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_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_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_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);
+
+	i2c_bus->controller_xfer_cnt = 0;
+
+	if (msg->flags & I2C_M_RD)
+		return ast2600_i2c_setup_buff_rx(AST2600_I2CM_START_CMD, i2c_bus);
+
+	return ast2600_i2c_setup_buff_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 = -EBUSY;
+				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:
+		xfer_len = AST2600_I2CC_GET_TX_BUF_LEN(readl(i2c_bus->reg_base +
+						       AST2600_I2CC_BUFF_CTRL));
+		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 = -EBUSY;
+					complete(&i2c_bus->cmd_complete);
+				}
+			}
+		} else {
+			ast2600_i2c_setup_buff_tx(0, i2c_bus);
+		}
+		break;
+	case AST2600_I2CM_RX_DONE:
+	case AST2600_I2CM_RX_DONE | AST2600_I2CM_NORMAL_STOP:
+		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 + i2c_bus->buf_size + i);
+
+		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 = -EBUSY;
+					complete(&i2c_bus->cmd_complete);
+				}
+			}
+		} else {
+			ast2600_i2c_setup_buff_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(0, i2c_bus->reg_base + AST2600_I2CM_IER);
+		synchronize_irq(i2c_bus->irq);
+		writel(readl(i2c_bus->reg_base + AST2600_I2CM_ISR),
+		       i2c_bus->reg_base + AST2600_I2CM_ISR);
+
+		writel(ctrl & ~AST2600_I2CC_MASTER_EN, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+		writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+
+		writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
+		       i2c_bus->reg_base + AST2600_I2CM_IER);
+
+		/*
+		 * 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);
+
+	/* 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 const struct i2c_adapter_quirks ast2600_i2c_quirks = {
+	.flags = I2C_AQ_NO_ZERO_LEN_READ,
+};
+
+static int ast2600_i2c_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ast2600_i2c_bus *i2c_bus;
+	void __iomem *buf_base;
+	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");
+
+	buf_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
+	if (IS_ERR(buf_base))
+		return dev_err_probe(dev, PTR_ERR(buf_base), "Missing buffer resource\n");
+	i2c_bus->buf_base = buf_base;
+	i2c_bus->buf_size = resource_size(res) / 2;
+
+	/*
+	 * 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);
+		if (i2c_bus->timeout > GENMASK(4, 0)) {
+			dev_warn(dev,
+				 "i2c-scl-clk-low-timeout-us exceeds HW max (31 * 1024us), clamped\n");
+			i2c_bus->timeout = GENMASK(4, 0);
+		}
+	}
+
+	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.quirks = &ast2600_i2c_quirks;
+	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 initialize 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 = i2c_add_adapter(&i2c_bus->adap);
+	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);
+
+	i2c_del_adapter(&i2c_bus->adap);
+
+	/* 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

[PATCH v30 5/5] i2c: ast2600: Add target mode support

[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

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

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v30:
- Address Sashiko AI code review feedback:
  - Force-stop path (target IRQ aborting an in-flight controller
    transfer): disable the controller IER and W1C-clear pending ISR
    before calling complete(), then restore the IER after the wake-up.
    Without the disable/clear sequence the controller IRQ handler can
    race with the target abort path and double-complete or touch
    freed msgs.
  - unreg_target() teardown ordering: disable the target IER first,
    then disable SLAVE_EN / clear ADDR_CTRL, synchronize_irq(), W1C
    pending ISR, and only then NULL i2c_bus->target and clear
    target_active. The old order left IER enabled while target was
    being cleared, allowing an in-flight handler to dereference a
    target pointer the caller had already freed.
  - reg_target() bring-up ordering: assign i2c_bus->target before
    enabling SLAVE_EN. Otherwise an IRQ that fires after SLAVE_EN is
    set but before the pointer is stored finds target == NULL, exits
    without clearing the ISR, and the unmasked event re-fires as an
    IRQ storm.
  - Use writel() instead of writeb() when staging a TX byte into the
    target buffer. The AST2600 buffer SRAM only supports 32-bit
    accesses; byte writes are silently dropped (or, on some
    revisions, raise a bus fault), so a SLAVE_READ_REQUESTED reply
    never reaches the master.
  - reg_target() rejects 10-bit client addresses with -EAFNOSUPPORT.
    AST2600_I2CS_ADDR1 is only a 7-bit field; without the check, the
    high bits of a 10-bit address overflow into the adjacent ADDR2
    field and silently corrupt a second target slot.
  - Initialise the local `u8 value` to 0 in the target packet IRQ
    handler. Its address is passed to i2c_slave_event() for events
    such as I2C_SLAVE_STOP / I2C_SLAVE_READ_REQUESTED; a slave
    backend that reads the byte before writing would otherwise leak
    uninitialised kernel stack.

Changes in v29:
- fix race between unreg_target and IRQ handler.
- move i2cs ier enable from ast2600_i2c_init to probe after master ier enable.
- remove dma/byte transfer, use buffer mode only.

Changes in v28:
- fix typo condication -> condition
- fix compile error, when disable CONFIG_I2C_SLAVE

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 | 359 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 358 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-ast2600.c b/drivers/i2c/busses/i2c-ast2600.c
index a06d0483865e..7e609fdfa1a1 100644
--- a/drivers/i2c/busses/i2c-ast2600.c
+++ b/drivers/i2c/busses/i2c-ast2600.c
@@ -254,6 +254,11 @@ struct ast2600_i2c_bus {
 	size_t			buf_size;
 	bool			multi_master;
 	void __iomem		*buf_base;
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	/* target structure */
+	bool			target_active;
+	struct i2c_client	*target;
+#endif
 };
 
 static void ast2600_i2c_ac_timing_config(struct ast2600_i2c_bus *i2c_bus)
@@ -337,6 +342,249 @@ 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_buff_irq(struct ast2600_i2c_bus *i2c_bus, u32 sts)
+{
+	u8 value = 0;
+	int target_rx_len = 0;
+	u32 cmd = 0;
+	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);
+		writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
+		writel(readl(i2c_bus->reg_base + AST2600_I2CM_ISR),
+		       i2c_bus->reg_base + AST2600_I2CM_ISR);
+		i2c_bus->cmd_err = -EBUSY;
+		writel(0, i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+		complete(&i2c_bus->cmd_complete);
+		writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
+		       i2c_bus->reg_base + AST2600_I2CM_IER);
+	}
+
+	/* 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(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 + i2c_bus->buf_size + 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 + i2c_bus->buf_size + 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 + i2c_bus->buf_size + 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 + i2c_bus->buf_size + 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 + i2c_bus->buf_size + 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 + i2c_bus->buf_size + 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);
+		writel(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);
+		writel(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 + i2c_bus->buf_size + 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 + i2c_bus->buf_size + 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);
+		}
+		writel(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 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)
+		ast2600_i2c_target_packet_buff_irq(i2c_bus, isr);
+
+	return 1;
+}
+#endif
+
 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];
@@ -479,7 +727,7 @@ static void ast2600_i2c_controller_packet_irq(struct ast2600_i2c_bus *i2c_bus, u
 	case AST2600_I2CM_TX_ACK:
 	case AST2600_I2CM_TX_ACK | AST2600_I2CM_NORMAL_STOP:
 		xfer_len = AST2600_I2CC_GET_TX_BUF_LEN(readl(i2c_bus->reg_base +
-						       AST2600_I2CC_BUFF_CTRL));
+							       AST2600_I2CC_BUFF_CTRL));
 		i2c_bus->controller_xfer_cnt += xfer_len;
 
 		if (i2c_bus->controller_xfer_cnt == msg->len) {
@@ -498,6 +746,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:
 		xfer_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
 							     AST2600_I2CC_BUFF_CTRL));
@@ -583,6 +845,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));
 }
 
@@ -599,12 +867,31 @@ static int ast2600_i2c_controller_xfer(struct i2c_adapter *adap, struct i2c_msg
 			return ret;
 	}
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	if (i2c_bus->target_active)
+		return -EBUSY;
+	/*
+	 * 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 to avoid buffer corruption.
+	 */
+	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 condition target is wait and controller wait 1st target operate */
+	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);
@@ -632,6 +919,9 @@ static int ast2600_i2c_controller_xfer(struct i2c_adapter *adap, struct i2c_msg
 		 * if the bus is still busy.
 		 */
 		if (i2c_bus->multi_master &&
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+		    !i2c_bus->target_active &&
+#endif
 		    (readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF) &
 		    AST2600_I2CC_BUS_BUSY_STS))
 			ast2600_i2c_recover_bus(i2c_bus);
@@ -667,8 +957,65 @@ 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)
+	writel(GENMASK(27, 0), i2c_bus->reg_base + AST2600_I2CS_ISR);
+#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;
+
+	if (client->flags & I2C_CLIENT_TEN)
+		return -EAFNOSUPPORT;
+
+	dev_dbg(i2c_bus->dev, "target addr %x\n", client->addr);
+
+	writel(0, i2c_bus->reg_base + AST2600_I2CS_ADDR_CTRL);
+
+	i2c_bus->target = client;
+
+	writel(AST2600_I2CC_SLAVE_EN | readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL),
+	       i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+
+	writel(cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+	/* 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;
+
+	writel(0, i2c_bus->reg_base + AST2600_I2CS_IER);
+
+	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);
+
+	synchronize_irq(i2c_bus->irq);
+
+	writel(readl(i2c_bus->reg_base + AST2600_I2CS_ISR),
+	       i2c_bus->reg_base + AST2600_I2CS_ISR);
+
+	i2c_bus->target = NULL;
+	WRITE_ONCE(i2c_bus->target_active, false);
+
 	return 0;
 }
+#endif
 
 static u32 ast2600_i2c_functionality(struct i2c_adapter *adap)
 {
@@ -678,6 +1025,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 const struct i2c_adapter_quirks ast2600_i2c_quirks = {
@@ -720,6 +1071,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");
 
@@ -784,6 +1138,9 @@ static int ast2600_i2c_probe(struct platform_device *pdev)
 
 	writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
 	       i2c_bus->reg_base + AST2600_I2CM_IER);
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_IER);
+#endif
 
 	ret = i2c_add_adapter(&i2c_bus->adap);
 	if (ret)

-- 
2.34.1

Re: [PATCH v30 1/5] dt-bindings: i2c: Split AST2600 binding into a new YAML

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message claims the binding requires two `reg` regions, but the schema specifies `minItems: 1`, making the second region optional.
--

commit 1bb6224b110a8990b3c95cd44d683d1ba249d994
Author: Ryan Chen <ryan_chen@aspeedtech.com>

dt-bindings: i2c: Split AST2600 binding into a new YAML

This commit separates the AST2600 I2C controller bindings from the older
AST2400/AST2500 schemas into a dedicated file. It also updates the schema
to enforce AST2600-specific constraints like interrupts, clock frequencies,
and register regions.

> 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 0000000000000..abc614315dffb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - aspeed,ast2600-i2c-bus
> +
> +  reg:
> +    minItems: 1

[Severity: Medium]
The commit message states that this binding adjusts AST2600-specific
requirements to "require two reg regions (controller register block + buffer
block)".

Should minItems be set to 2 here to enforce that requirement? Setting it
to 1 makes the second region optional, which allows device tree nodes with
only a single reg region to silently pass schema validation.

> +    items:
> +      - description: controller registers
> +      - description: controller buffer space

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-upstream_i2c-v30-0-5d4f9adc3530@aspeedtech.com?part=1

Re: [PATCH v30 2/5] i2c: aspeed: Read clock-frequency via i2c_parse_fw_timings()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The `struct i2c_timings timings` variable is declared on the stack without initialization. If the `clock-frequency` device tree property is missing, `timings.bus_freq_hz` remains uninitialized, leading to undefined behavior and incorrectly bypassing the fallback logic.

Pre-existing issues:
- [High] The driver disables the I2C controller and asserts hardware reset before unregistering the I2C adapter, leading to hanging I2C transfers during teardown.
--

commit 295034b0d28e53ac8b8ea47c906ff258ad2c83f6
Author: Ryan Chen <ryan_chen@aspeedtech.com>
Subject: i2c: aspeed: Read clock-frequency via i2c_parse_fw_timings()

This commit updates the Aspeed I2C driver to read the standard
clock-frequency property using i2c_parse_fw_timings(). It falls back to
the legacy bus-frequency property only if the standard property is absent.

> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index a26b74c712061..137889e57ddc6 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -1000,6 +1000,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>  	const struct of_device_id *match;
>  	struct aspeed_i2c_bus *bus;
>  	struct clk *parent_clk;
> +	struct i2c_timings timings;

[Severity: High]
Does the timings structure need to be zero-initialized here?

>  	int irq, ret;
>  
>  	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);

[ ... ]

> @@ -1025,12 +1026,18 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>  	}
>  	reset_control_deassert(bus->rst);
>  
> -	ret = of_property_read_u32(pdev->dev.of_node,
> -				   "bus-frequency", &bus->bus_frequency);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev,
> -			"Could not read bus-frequency property\n");
> -		bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
> +	i2c_parse_fw_timings(&pdev->dev, &timings, false);
> +	if (timings.bus_freq_hz) {

[Severity: High]
Since use_defaults is passed as false to i2c_parse_fw_timings(), if the
clock-frequency device tree property is missing, i2c_parse_timing() will
not modify timings.bus_freq_hz.

Could this leave timings.bus_freq_hz with uninitialized stack memory,
which might evaluate to true and bypass the fallback to the bus-frequency
property?

> +		bus->bus_frequency = timings.bus_freq_hz;
> +	} else {
> +		ret = of_property_read_u32(pdev->dev.of_node,
> +					   "bus-frequency",
> +					   &bus->bus_frequency);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev,
> +				"Could not read clock-frequency or bus-frequency property\n");
> +			bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
> +		}
>  	}

[ ... ]

>  static void aspeed_i2c_remove_bus(struct platform_device *pdev)
>  {
>  	struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&bus->lock, flags);
>  
>  	/* Disable everything. */
>  	writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
>  	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
>  
>  	spin_unlock_irqrestore(&bus->lock, flags);
>  
>  	reset_control_assert(bus->rst);
>  
>  	i2c_del_adapter(&bus->adap);

[Severity: High]
This is a pre-existing issue, but does the I2C controller get disabled
before child devices are unregistered?

In aspeed_i2c_remove_bus(), the driver disables the I2C controller and
asserts the hardware reset before calling i2c_del_adapter().

Since i2c_del_adapter() coordinates the removal of registered I2C child
devices, their remove() callbacks might issue final I2C transfers to
gracefully shut down. Because the controller is already disabled and held
in reset, could these transfers hang and expire the timeout, leading to
teardown delays and inconsistent hardware state?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-upstream_i2c-v30-0-5d4f9adc3530@aspeedtech.com?part=2

Re: [PATCH v30 4/5] i2c: ast2600: Add controller driver for AST2600 new register set

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] Missing lifecycle validation of `i2c_bus->msgs` on transfer timeout leads to a Use-After-Free (UAF) and memory corruption.
- [Critical] The RX interrupt handler blindly trusts the hardware's RX buffer length, leading to a potential out-of-bounds memory write (buffer overflow).
- [High] Non-atomic read-modify-write of a shared global syscon register clobbers existing bits and introduces a TOCTOU race.
- [High] SMBus Block Read with a block length of 0 corrupts the length byte in the receive buffer.
- [High] Unaborted bus recovery operations on timeout lead to cross-transfer completion races.
--

commit 00d3297d874ae70d7ea1c3a5e963a09c214da736
Author: Ryan Chen <ryan_chen@aspeedtech.com>
Subject: i2c: ast2600: Add controller driver for AST2600 new register set

The AST2600 introduces a new I2C controller register layout, selectable at
runtime via global control registers. This patch adds an AST2600-specific I2C
controller driver implementing the new register layout, including support for
packet-based transfers.

> diff --git a/drivers/i2c/busses/i2c-ast2600.c b/drivers/i2c/busses/i2c-ast2600.c
> new file mode 100644
> index 0000000000000..a06d0483865ed
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-ast2600.c

[ ... ]

> +static int ast2600_i2c_recover_bus(struct ast2600_i2c_bus *i2c_bus)
> +{

[ ... ]

> +	/* 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;

[Severity: High]
Does returning immediately on timeout leave the hardware in a state that could
corrupt future transfers?

If the delayed hardware recovery operation from the previous attempt finishes
later, it will trigger the BUS_RECOVER interrupt. This could execute
complete(&i2c_bus->cmd_complete), prematurely completing the wait for a new,
unrelated transfer and tricking the state machine into treating an incomplete
transfer as successful.

> +		} else if (i2c_bus->cmd_err) {
> +			dev_dbg(i2c_bus->dev, "recovery error\n");
> +			ret = -EPROTO;
> +		}
> +	}

[ ... ]

> +static void ast2600_i2c_controller_packet_irq(struct ast2600_i2c_bus *i2c_bus, u32 sts)
> +{

[ ... ]

> +	case AST2600_I2CM_RX_DONE:
> +	case AST2600_I2CM_RX_DONE | AST2600_I2CM_NORMAL_STOP:
> +		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 + i2c_bus->buf_size + i);

[Severity: Critical]
Is there a missing bounds check on xfer_len here?

If the hardware malfunctions, misinterprets the bus state, or returns a larger
length than expected, this loop could write data past the end of the
dynamically allocated msg->buf. Could this cause a kernel buffer overflow?

> +
> +		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;

[Severity: High]
Could setting controller_xfer_cnt to 0 corrupt the block length byte?

If recv_len is 0, msg->len is updated to 1 to perform a 1-byte read.
When the hardware receives this next byte and triggers RX_DONE again, the loop
above copies the new byte into msg->buf[i2c_bus->controller_xfer_cnt + i].

Since controller_xfer_cnt was reset to 0, does this unconditionally overwrite
and corrupt the block length byte in msg->buf[0]?

> +			else
> +				i2c_bus->controller_xfer_cnt = 1;
> +		} else {

[ ... ]

> +static int ast2600_i2c_controller_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> +{

[ ... ]

> +	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) {

[Severity: Critical]
Does this timeout path need to clear i2c_bus->msgs?

If wait_for_completion_timeout() times out, the function returns -ETIMEDOUT
and the I2C subsystem will free the msgs array. However, the driver never
clears the i2c_bus->msgs pointer.

If a delayed hardware response (such as a spurious PKT_DONE or RX_DONE)
later triggers an interrupt, the ast2600_i2c_controller_packet_irq()
handler will unconditionally dereference the dangling i2c_bus->msgs
pointer. Could this lead to a use-after-free or out-of-bounds memory
write causing kernel memory corruption?

> +		u32 ctrl = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);

[ ... ]

> +static int ast2600_i2c_probe(struct platform_device *pdev)
> +{

[ ... ]

> +	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);

[Severity: High]
Does this non-atomic write clobber existing bits in the shared control register?

Because AST2600_I2CG_CTRL is shared across multiple I2C controllers,
using regmap_write() to unconditionally overwrite the register with
AST2600_GLOBAL_INIT destroys any other configuration bits that may have
been set by the bootloader or other subsystems.

Also, concurrent probing of multiple buses could introduce a Time-Of-Check-
To-Time-Of-Use race. Should this use regmap_update_bits() instead to
atomically modify only the relevant bits?

> +		regmap_write(i2c_bus->global_regs, AST2600_I2CG_CLK_DIV_CTRL, I2CCG_DIV_CTRL);
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-upstream_i2c-v30-0-5d4f9adc3530@aspeedtech.com?part=4

Re: [PATCH v30 5/5] i2c: ast2600: Add target mode support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:

New issues:
- [Critical] Aborting an in-flight controller transfer from the target IRQ handler races with controller IRQs, leading to a Use-After-Free.
- [Critical] A Time-Of-Check to Time-Of-Use (TOCTOU) race in `ast2600_i2c_controller_xfer` allows target hardware to corrupt the shared I2C buffer.
- [High] Invalid I2C Slave event ordering: STOP events are emitted before WRITE_REQUESTED and WRITE_RECEIVED data events.
- [High] The `target_active` flag is permanently leaked as true under specific hardware interrupt combinations, deadlocking controller transfers.
- [High] Target interrupts are permanently disabled if a target is unregistered and later re-registered.
- [High] A specific target ISR STOP case stalls the I2C target state machine by failing to trigger the hardware command.
- [High] Unhandled target interrupt states fail to re-arm the hardware, permanently hanging the I2C bus.
- [High] Checking the raw ISR register to block controller transfers permanently bricks the controller due to uncleared W1C bits.
- [Medium] Target interrupts are unconditionally enabled even when no target is registered.

Pre-existing issues:
- [High] In controller mode, the received `xfer_len` is not validated against the remaining bounds of the `msg->buf` array.
--

commit fdd43bfe0f52b1390eb013eb7a0a2e7b1131f43e
Author: Ryan Chen <ryan_chen@aspeedtech.com>

i2c: ast2600: Add target mode support

Add target mode support to the AST2600 I2C driver. This complements the 
existing controller-mode support, enabling dual-role capability.

> diff --git a/drivers/i2c/busses/i2c-ast2600.c b/drivers/i2c/busses/i2c-ast2600.c
> index a06d0483865ed..7e609fdfa1a19 100644
> --- a/drivers/i2c/busses/i2c-ast2600.c
> +++ b/drivers/i2c/busses/i2c-ast2600.c
[ ... ]
> @@ -337,6 +342,249 @@ 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_buff_irq(struct ast2600_i2c_bus *i2c_bus, u32 sts)
> +{
> +	u8 value = 0;
> +	int target_rx_len = 0;
> +	u32 cmd = 0;
> +	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);
> +		writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
> +		writel(readl(i2c_bus->reg_base + AST2600_I2CM_ISR),
> +		       i2c_bus->reg_base + AST2600_I2CM_ISR);
> +		i2c_bus->cmd_err = -EBUSY;
> +		writel(0, i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
> +		complete(&i2c_bus->cmd_complete);
> +		writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
> +		       i2c_bus->reg_base + AST2600_I2CM_IER);
> +	}

[Severity: Critical]
Does calling complete() here race with re-enabling the controller interrupts?

If the controller thread wakes up immediately and frees the locally allocated
msgs array, could a subsequent delayed hardware interrupt cause
ast2600_i2c_controller_packet_irq() to dereference the freed msgs pointer?

> +
> +	/* Handle i2c target timeout condition */
> +	if (sts & AST2600_I2CS_INACTIVE_TO) {
[ ... ]
> +	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:

[Severity: High]
Does falling through here emit events in the wrong order?

The I2C slave API expects I2C_SLAVE_STOP to be the final event indicating
the end of a transaction. If we fall through to the block that issues
I2C_SLAVE_WRITE_REQUESTED and I2C_SLAVE_WRITE_RECEIVED, could this corrupt
the state machine of backend devices?

[ ... ]
> +	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 + i2c_bus->buf_size + 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);
> +		writel(value, i2c_bus->buf_base);
> +		break;

[Severity: High]
Is this case missing an assignment to cmd?

Since cmd defaults to 0 and is not updated here, the driver will not write
a command to AST2600_I2CS_CMD_STS. Could this leave the hardware state machine
hanging or stretching the clock indefinitely?

[ ... ]
> +	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);

[Severity: High]
Does an unhandled interrupt state hang the I2C bus?

If we hit the default case, cmd remains 0. Without writing a valid command
to acknowledge and proceed, will the hardware remain paused and stretch
the clock?

> +
> +	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);
> +}

[Severity: High]
Can the target_active flag become permanently stuck as true?

The switch statement above explicitly handles combined interrupt cases where
both AST2600_I2CS_STOP and AST2600_I2CS_SLAVE_PENDING are set. In those
cases, target_active will remain true.

Since a STOP occurred, no further interrupts will arrive for this transaction.
Will this cause all future controller transfers to permanently return -EBUSY?

> +
> +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);
[ ... ]
> +	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;

[Severity: High]
Are these Write-1-to-Clear (W1C) bits being left set in the hardware?

The bits are masked out of the local isr variable but are never written back
to AST2600_I2CS_ISR to clear them in hardware. Because they remain set,
will later checks against AST2600_I2CS_ISR always evaluate to true, blocking
all future controller transfers?

[ ... ]
>  	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:
>  		xfer_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
>  							     AST2600_I2CC_BUFF_CTRL));

[Severity: High]
This is a pre-existing issue, but does xfer_len need bounds checking here?

The xfer_len value is read directly from hardware and used unconditionally in
the copy loop below. If the hardware reports a length longer than the
remaining message size, could it overflow the msg->buf array?

[ ... ]
> @@ -599,12 +867,31 @@ static int ast2600_i2c_controller_xfer(struct i2c_adapter *adap, struct i2c_msg
>  			return ret;
>  	}
>  
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +	if (i2c_bus->target_active)
> +		return -EBUSY;
> +	/*
> +	 * 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 to avoid buffer corruption.
> +	 */
> +	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);

[Severity: Critical]
Is there a race condition between software checks and the hardware state machine?

Disabling CPU interrupts via AST2600_I2CS_IER does not pause the autonomous
hardware state machine since AST2600_I2CC_SLAVE_EN is still active. Can an
external I2C master address the target right after the check, causing the
hardware to overwrite the shared buffer while ast2600_i2c_do_start() is
preparing the master transfer?

[ ... ]
> +#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;
[ ... ]
> +	writel(cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
> +	/* 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;
> +
> +	writel(0, i2c_bus->reg_base + AST2600_I2CS_IER);

[Severity: High]
Will target interrupts remain permanently disabled if a target is unbound and
then rebound?

The ast2600_i2c_unreg_target() function disables target interrupts, but
ast2600_i2c_reg_target() does not re-enable them. Does this break target
functionality on subsequent registrations?

[ ... ]
> @@ -784,6 +1138,9 @@ static int ast2600_i2c_probe(struct platform_device *pdev)
>  
>  	writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
>  	       i2c_bus->reg_base + AST2600_I2CM_IER);
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +	writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_IER);
> +#endif
>  
>  	ret = i2c_add_adapter(&i2c_bus->adap);

[Severity: Medium]
Is it safe to unconditionally enable target interrupts here?

Since i2c_bus->target is NULL until a target is registered, ast2600_i2c_bus_irq
will ignore target interrupts. If the hardware asserts an interrupt before
a target is bound, could this lead to an unhandled interrupt storm?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-upstream_i2c-v30-0-5d4f9adc3530@aspeedtech.com?part=5

Re: [PATCH v30 4/5] i2c: ast2600: Add controller driver for AST2600 new register set

[William A. Kennington III]

On 5/28/26 00:41, Ryan Chen wrote:
> 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)
> - 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.
> 
> 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.
> 
> Contact: Ryan Chen <ryan_chen@aspeedtech.com>
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> ---
> Changes in v30:
> - Address Sashiko AI code review feedback:
>    - Use manual i2c_add_adapter() / i2c_del_adapter() instead of
>      devm_i2c_add_adapter() so the adapter is torn down before the
>      hardware is disabled in remove(); otherwise client .remove()
>      callbacks can fail or hang after FUN_CTRL/IER have been cleared.
>    - synchronize_irq() and clear pending IRQ status on the controller
>      timeout path to avoid the ISR racing with the next transfer and
>      touching freed msgs.
>    - Use clamp_t() for AC TIMING divisor / scl_low / scl_high so
>      extreme clock-frequency values cannot underflow into the unsigned
>      domain and corrupt the AC TIMING register.
>    - Derive the RX buffer offset from buf_size instead of hardcoding
>      0x10, since the dual-pool split is configurable.
>    - Clamp i2c-scl-clk-low-timeout-us to the TTIMEOUT field's 5-bit
>      range (max 31 * 1024us) and emit a dev_warn() instead of letting
>      AST2600_I2CC_TTIMEOUT()'s mask silently truncate larger values.
> - Return -EBUSY (not -ENOMEM) for every ast2600_i2c_do_start() failure
>    path in the controller packet IRQ handler (NORMAL_STOP, TX_ACK, and
>    RX_DONE branches).
> - Advertise I2C_AQ_NO_ZERO_LEN_READ via i2c_adapter_quirks so the
>    i2c-core rejects zero-byte reads before they reach the driver. The
>    AST2600 packet engine cannot encode a zero-length RX command and
>    would otherwise stall waiting for an RX_DONE that never arrives.
> 
> Changes in v29:
> - update commit message remove transfer mode selection.
> - remove dma/byte transfer, use buffer mode only.
> - remove sysfs file.
> - remove define I2C_TARGET_MSG_BUF_SIZE and AST2600_I2C_DMA_SIZE.
> - remove buf_index in struct ast2600_i2c_bus.
> 
> Changes in v28:
> - Separate xfer_mode_store into distinct parse and availability-check
>    steps by introducing ast2600_i2c_xfer_mode_check()
> - fix tx dma memcpy source point address.
> - Use a temporary variable for devm_platform_get_and_ioremap_resource()
>    to avoid storing an ERR_PTR in i2c_bus->buf_base; drop the redundant
>    NULL assignment in the error path since i2c_bus is kzalloc()ed
> - Add ABI documentation file
>    Documentation/ABI/testing/sysfs-driver-ast2600-i2c
> 
> 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 | 829 +++++++++++++++++++++++++++++++++++++++
>   3 files changed, 835 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 137889e57ddc..fb4b2ac7b6ee 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>
>   
> @@ -1003,6 +1004,10 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>   	struct i2c_timings timings;
>   	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..a06d0483865e
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-ast2600.c
> @@ -0,0 +1,829 @@
> +// 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/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/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 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
> +
> +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;
> +	u32			apb_clk;
> +	u32			timeout;
> +	int			irq;
> +	int			cmd_err;
> +	int			msgs_index;
> +	int			msgs_count;
> +	int			controller_xfer_cnt;
> +	size_t			buf_size;
> +	bool			multi_master;
> +	void __iomem		*buf_base;
> +};
> +
> +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 = clamp_t(int, baseclk_idx, 0, 15);
> +	divisor = clamp_t(int, divisor, 2, 32);
> +	scl_low = clamp_t(int, divisor * 9 / 16 - 1, 0, 15);
> +	scl_high = clamp_t(int, divisor - scl_low - 2, 1, 15);
> +	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_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_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;
> +	}

You need to check `xfer_len > 0` somewhere before 
`AST2600_I2CC_SET_RX_BUF_LEN(xfer_len)`. Otherwise, a value of 0 will 
become -1 inside `AST2600_I2CC_SET_RX_BUF_LEN(x)` and cause a kernel 
crash when the device reads back extra memory.

Consider userspace doing the following
```
union i2c_smbus_data data = {};
// data.block[0] (length) is 0 here
struct i2c_smbus_ioctl_data args = {};
args.read_write = I2C_SMBUS_READ;
args.command = mycmd;
args.size = I2C_SMBUS_I2C_BLOCK_DATA;
args.data = &data;
ioctl(fd, I2C_SMBUS, &args);
```

> +	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_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);
> +
> +	i2c_bus->controller_xfer_cnt = 0;
> +
> +	if (msg->flags & I2C_M_RD)
> +		return ast2600_i2c_setup_buff_rx(AST2600_I2CM_START_CMD, i2c_bus);
> +
> +	return ast2600_i2c_setup_buff_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 = -EBUSY;
> +				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:
> +		xfer_len = AST2600_I2CC_GET_TX_BUF_LEN(readl(i2c_bus->reg_base +
> +						       AST2600_I2CC_BUFF_CTRL));
> +		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 = -EBUSY;
> +					complete(&i2c_bus->cmd_complete);
> +				}
> +			}
> +		} else {
> +			ast2600_i2c_setup_buff_tx(0, i2c_bus);
> +		}
> +		break;
> +	case AST2600_I2CM_RX_DONE:
> +	case AST2600_I2CM_RX_DONE | AST2600_I2CM_NORMAL_STOP:
> +		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 + i2c_bus->buf_size + i);
> +
> +		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 = -EBUSY;
> +					complete(&i2c_bus->cmd_complete);
> +				}
> +			}
> +		} else {
> +			ast2600_i2c_setup_buff_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(0, i2c_bus->reg_base + AST2600_I2CM_IER);
> +		synchronize_irq(i2c_bus->irq);
> +		writel(readl(i2c_bus->reg_base + AST2600_I2CM_ISR),
> +		       i2c_bus->reg_base + AST2600_I2CM_ISR);
> +
> +		writel(ctrl & ~AST2600_I2CC_MASTER_EN, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> +		writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> +
> +		writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
> +		       i2c_bus->reg_base + AST2600_I2CM_IER);
> +
> +		/*
> +		 * 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);
> +
> +	/* 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 const struct i2c_adapter_quirks ast2600_i2c_quirks = {
> +	.flags = I2C_AQ_NO_ZERO_LEN_READ,
> +};
> +
> +static int ast2600_i2c_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct ast2600_i2c_bus *i2c_bus;
> +	void __iomem *buf_base;
> +	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");
> +
> +	buf_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
> +	if (IS_ERR(buf_base))
> +		return dev_err_probe(dev, PTR_ERR(buf_base), "Missing buffer resource\n");
> +	i2c_bus->buf_base = buf_base;
> +	i2c_bus->buf_size = resource_size(res) / 2;
> +
> +	/*
> +	 * 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);
> +		if (i2c_bus->timeout > GENMASK(4, 0)) {
> +			dev_warn(dev,
> +				 "i2c-scl-clk-low-timeout-us exceeds HW max (31 * 1024us), clamped\n");
> +			i2c_bus->timeout = GENMASK(4, 0);
> +		}
> +	}
> +
> +	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.quirks = &ast2600_i2c_quirks;
> +	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 initialize 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 = i2c_add_adapter(&i2c_bus->adap);
> +	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);
> +
> +	i2c_del_adapter(&i2c_bus->adap);
> +
> +	/* 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");
> 

RE: [PATCH v30 4/5] i2c: ast2600: Add controller driver for AST2600 new register set

[Ryan Chen]

Hello William,

> Subject: Re: [PATCH v30 4/5] i2c: ast2600: Add controller driver for AST2600
> new register set
> 
.......
> > +
> > +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;
> > +	}
> 
> You need to check `xfer_len > 0` somewhere before
> `AST2600_I2CC_SET_RX_BUF_LEN(xfer_len)`. Otherwise, a value of 0 will
> become -1 inside `AST2600_I2CC_SET_RX_BUF_LEN(x)` and cause a kernel
> crash when the device reads back extra memory.
> 
> Consider userspace doing the following
> ```
> union i2c_smbus_data data = {};
> // data.block[0] (length) is 0 here
> struct i2c_smbus_ioctl_data args = {};
> args.read_write = I2C_SMBUS_READ;
> args.command = mycmd;
> args.size = I2C_SMBUS_I2C_BLOCK_DATA;
> args.data = &data;
> ioctl(fd, I2C_SMBUS, &args);
> ```
OH, thanks your example.
I will update with following.
	if (xfer_len <= 0)
		return -EINVAL;

> > +	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_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);
> > +
> > +	i2c_bus->controller_xfer_cnt = 0;
> > +
> > +	if (msg->flags & I2C_M_RD)
> > +		return ast2600_i2c_setup_buff_rx(AST2600_I2CM_START_CMD,
> i2c_bus);
> > +
> > +	return ast2600_i2c_setup_buff_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 = -EBUSY;
> > +				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:
> > +		xfer_len =
> AST2600_I2CC_GET_TX_BUF_LEN(readl(i2c_bus->reg_base +
> > +						       AST2600_I2CC_BUFF_CTRL));
> > +		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 = -EBUSY;
> > +					complete(&i2c_bus->cmd_complete);
> > +				}
> > +			}
> > +		} else {
> > +			ast2600_i2c_setup_buff_tx(0, i2c_bus);
> > +		}
> > +		break;
> > +	case AST2600_I2CM_RX_DONE:
> > +	case AST2600_I2CM_RX_DONE | AST2600_I2CM_NORMAL_STOP:
> > +		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 + i2c_bus->buf_size + i);
> > +
> > +		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 = -EBUSY;
> > +					complete(&i2c_bus->cmd_complete);
> > +				}
> > +			}
> > +		} else {
> > +			ast2600_i2c_setup_buff_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(0, i2c_bus->reg_base + AST2600_I2CM_IER);
> > +		synchronize_irq(i2c_bus->irq);
> > +		writel(readl(i2c_bus->reg_base + AST2600_I2CM_ISR),
> > +		       i2c_bus->reg_base + AST2600_I2CM_ISR);
> > +
> > +		writel(ctrl & ~AST2600_I2CC_MASTER_EN, i2c_bus->reg_base +
> AST2600_I2CC_FUN_CTRL);
> > +		writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > +
> > +		writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
> > +		       i2c_bus->reg_base + AST2600_I2CM_IER);
> > +
> > +		/*
> > +		 * 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);
> > +
> > +	/* 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 const struct i2c_adapter_quirks ast2600_i2c_quirks = {
> > +	.flags = I2C_AQ_NO_ZERO_LEN_READ,
> > +};
> > +
> > +static int ast2600_i2c_probe(struct platform_device *pdev) {
> > +	struct device *dev = &pdev->dev;
> > +	struct ast2600_i2c_bus *i2c_bus;
> > +	void __iomem *buf_base;
> > +	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");
> > +
> > +	buf_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
> > +	if (IS_ERR(buf_base))
> > +		return dev_err_probe(dev, PTR_ERR(buf_base), "Missing buffer
> resource\n");
> > +	i2c_bus->buf_base = buf_base;
> > +	i2c_bus->buf_size = resource_size(res) / 2;
> > +
> > +	/*
> > +	 * 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);
> > +		if (i2c_bus->timeout > GENMASK(4, 0)) {
> > +			dev_warn(dev,
> > +				 "i2c-scl-clk-low-timeout-us exceeds HW max (31 *
> 1024us), clamped\n");
> > +			i2c_bus->timeout = GENMASK(4, 0);
> > +		}
> > +	}
> > +
> > +	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.quirks = &ast2600_i2c_quirks;
> > +	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 initialize 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 = i2c_add_adapter(&i2c_bus->adap);
> > +	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);
> > +
> > +	i2c_del_adapter(&i2c_bus->adap);
> > +
> > +	/* 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");
> >

RE: [PATCH v30 1/5] dt-bindings: i2c: Split AST2600 binding into a new YAML

[Ryan Chen]

> Subject: Re: [PATCH v30 1/5] dt-bindings: i2c: Split AST2600 binding into a new
> YAML
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to
> consider:
> - [Medium] The commit message claims the binding requires two `reg` regions,
> but the schema specifies `minItems: 1`, making the second region optional.
> --
> 
> commit 1bb6224b110a8990b3c95cd44d683d1ba249d994
> Author: Ryan Chen <ryan_chen@aspeedtech.com>
> 
> dt-bindings: i2c: Split AST2600 binding into a new YAML
> 
> This commit separates the AST2600 I2C controller bindings from the older
> AST2400/AST2500 schemas into a dedicated file. It also updates the schema to
> enforce AST2600-specific constraints like interrupts, clock frequencies, and
> register regions.
> 
> > 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 0000000000000..abc614315dffb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> 
> [ ... ]
> 
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - aspeed,ast2600-i2c-bus
> > +
> > +  reg:
> > +    minItems: 1
> 
> [Severity: Medium]
> The commit message states that this binding adjusts AST2600-specific
> requirements to "require two reg regions (controller register block + buffer
> block)".
> 
> Should minItems be set to 2 here to enforce that requirement? Setting it to 1
> makes the second region optional, which allows device tree nodes with only a
> single reg region to silently pass schema validation.

I will update in commit message 
    - describe two reg regions (controller register block + buffer block);
      the second region is optional (minItems: 1) so existing AST2600 DTs
      that only declare the controller register block continue to validate

> 
> > +    items:
> > +      - description: controller registers
> > +      - description: controller buffer space
> 
> [ ... ]
> 
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/20260528-upstream_i2c-v30-0-5d4f9adc3530@
> aspeedtech.com?part=1

RE: [PATCH v30 2/5] i2c: aspeed: Read clock-frequency via i2c_parse_fw_timings()

[Ryan Chen]

> Subject: Re: [PATCH v30 2/5] i2c: aspeed: Read clock-frequency via
> i2c_parse_fw_timings()
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to
> consider:
> 
> New issues:
> - [High] The `struct i2c_timings timings` variable is declared on the stack
> without initialization. If the `clock-frequency` device tree property is missing,
> `timings.bus_freq_hz` remains uninitialized, leading to undefined behavior and
> incorrectly bypassing the fallback logic.
> 
> Pre-existing issues:
> - [High] The driver disables the I2C controller and asserts hardware reset
> before unregistering the I2C adapter, leading to hanging I2C transfers during
> teardown.
> --
> 
> commit 295034b0d28e53ac8b8ea47c906ff258ad2c83f6
> Author: Ryan Chen <ryan_chen@aspeedtech.com>
> Subject: i2c: aspeed: Read clock-frequency via i2c_parse_fw_timings()
> 
> This commit updates the Aspeed I2C driver to read the standard
> clock-frequency property using i2c_parse_fw_timings(). It falls back to the
> legacy bus-frequency property only if the standard property is absent.
> 
> > diff --git a/drivers/i2c/busses/i2c-aspeed.c
> > b/drivers/i2c/busses/i2c-aspeed.c index a26b74c712061..137889e57ddc6
> > 100644
> > --- a/drivers/i2c/busses/i2c-aspeed.c
> > +++ b/drivers/i2c/busses/i2c-aspeed.c
> > @@ -1000,6 +1000,7 @@ static int aspeed_i2c_probe_bus(struct
> platform_device *pdev)
> >  	const struct of_device_id *match;
> >  	struct aspeed_i2c_bus *bus;
> >  	struct clk *parent_clk;
> > +	struct i2c_timings timings;
> 
> [Severity: High]
> Does the timings structure need to be zero-initialized here?
I will update with following
	struct i2c_timings timings = {};
> 
> >  	int irq, ret;
> >
> >  	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> 
> [ ... ]
> 
> > @@ -1025,12 +1026,18 @@ static int aspeed_i2c_probe_bus(struct
> platform_device *pdev)
> >  	}
> >  	reset_control_deassert(bus->rst);
> >
> > -	ret = of_property_read_u32(pdev->dev.of_node,
> > -				   "bus-frequency", &bus->bus_frequency);
> > -	if (ret < 0) {
> > -		dev_err(&pdev->dev,
> > -			"Could not read bus-frequency property\n");
> > -		bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
> > +	i2c_parse_fw_timings(&pdev->dev, &timings, false);
> > +	if (timings.bus_freq_hz) {
> 
> [Severity: High]
> Since use_defaults is passed as false to i2c_parse_fw_timings(), if the
> clock-frequency device tree property is missing, i2c_parse_timing() will not
> modify timings.bus_freq_hz.
> 
> Could this leave timings.bus_freq_hz with uninitialized stack memory, which
> might evaluate to true and bypass the fallback to the bus-frequency property?
> 
> > +		bus->bus_frequency = timings.bus_freq_hz;
> > +	} else {
> > +		ret = of_property_read_u32(pdev->dev.of_node,
> > +					   "bus-frequency",
> > +					   &bus->bus_frequency);
> > +		if (ret < 0) {
> > +			dev_err(&pdev->dev,
> > +				"Could not read clock-frequency or bus-frequency
> property\n");
> > +			bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
> > +		}
> >  	}
> 
> [ ... ]
> 
> >  static void aspeed_i2c_remove_bus(struct platform_device *pdev)  {
> >  	struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
> >  	unsigned long flags;
> >
> >  	spin_lock_irqsave(&bus->lock, flags);
> >
> >  	/* Disable everything. */
> >  	writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
> >  	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
> >
> >  	spin_unlock_irqrestore(&bus->lock, flags);
> >
> >  	reset_control_assert(bus->rst);
> >
> >  	i2c_del_adapter(&bus->adap);
> 
> [Severity: High]
> This is a pre-existing issue, but does the I2C controller get disabled before child
> devices are unregistered?
> 
> In aspeed_i2c_remove_bus(), the driver disables the I2C controller and asserts
> the hardware reset before calling i2c_del_adapter().
> 
> Since i2c_del_adapter() coordinates the removal of registered I2C child devices,
> their remove() callbacks might issue final I2C transfers to gracefully shut down.
> Because the controller is already disabled and held in reset, could these
> transfers hang and expire the timeout, leading to teardown delays and
> inconsistent hardware state?
> 
> >  }
> 
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/20260528-upstream_i2c-v30-0-5d4f9adc3530@
> aspeedtech.com?part=2