[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bf5a4aa0fc1a324a17c25e8ed5acbfd94d240251.camel@pengutronix.de>
Date: Thu, 14 Aug 2025 12:42:33 +0200
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Ryan Chen <ryan_chen@...eedtech.com>, benh@...nel.crashing.org,
joel@....id.au, andi.shyti@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, andrew@...econstruct.com.au,
andriy.shevchenko@...ux.intel.com, naresh.solanki@...ements.com,
linux-i2c@...r.kernel.org, openbmc@...ts.ozlabs.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-aspeed@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v17 2/3] i2c: ast2600: Add controller driver for new
register layout
On Do, 2025-08-14 at 16:41 +0800, Ryan Chen wrote:
> Add i2c-ast2600 new register mode driver to support AST2600
> i2c new register mode. This i2c-ast2600 new driver and the
> legacy i2c-aspeed driver both match the same compatible string
> "aspeed,ast2600-i2c-bus" because they target the same I2C
> controller IP on AST2600. However, AST2600 SoCs may configure
> the controller instances to operate either in the legacy
> register layout or the new layout (via global register).
> The new register mode support following.
>
> - Add new clock divider option for more flexible and accurate
> clock rate generation
> - Add tCKHighMin timing to guarantee SCL high pulse width.
> - Add support dual pool buffer mode, split 32 bytes pool buffer
> of each device into 2 x 16 bytes for Tx and Rx individually.
> - Increase DMA buffer size to 4096 bytes and support byte alignment.
> - Re-define the base address of BUS1 ~ BUS16 and Pool buffer.
> - Re-define registers for separating controller and target
> mode control.
> - Support 4 individual DMA buffers for controller Tx and Rx,
> target Tx and Rx.
>
> And following is new register set for package transfer sequence.
> - New Master operation mode:
> S -> Aw -> P
> S -> Aw -> TxD -> P
> S -> Ar -> RxD -> P
> S -> Aw -> TxD -> Sr -> Ar -> RxD -> P
> - Bus SDA lock auto-release capability for new controller DMA
> command mode.
> - Bus auto timeout for new controller/target DMA mode.
>
> Since the register layout is selected via a global register at
> runtime and both drivers bind to the same compatible string,
> this patch defines the driver selection at build-time using
> Kconfig, ensuring that only one driver is compiled into the
> kernel. This approach avoids ambiguity and ensures consistent
> behavior for each platform configuration.
>
> The following is two versus register layout.
> Old register mode:
> {I2CD00}: Function Control Register
> {I2CD04}: Clock and AC Timing Control Register
> {I2CD08}: Clock and AC Timing Control Register
> {I2CD0C}: Interrupt Control Register
> {I2CD10}: Interrupt Status Register
> {I2CD14}: Command/Status Register
> {I2CD18}: Slave Device Address Register
> {I2CD1C}: Pool Buffer Control Register
> {I2CD20}: Transmit/Receive Byte Buffer Register
> {I2CD24}: DMA Mode Buffer Address Register
> {I2CD28}: DMA Transfer Length Register
> {I2CD2C}: Original DMA Mode Buffer Address Setting
> {I2CD30}: Original DMA Transfer Length Setting and Final Status
>
> New Register mode
> {I2CC00}: Master/Slave Function Control Register
> {I2CC04}: Master/Slave Clock and AC Timing Control Register
> {I2CC08}: Master/Slave Transmit/Receive Byte Buffer Register
> {I2CC0C}: Master/Slave Pool Buffer Control Register
> {I2CM10}: Master Interrupt Control Register
> {I2CM14}: Master Interrupt Status Register
> {I2CM18}: Master Command/Status Register
> {I2CM1C}: Master DMA Buffer Length Register
> {I2CS20}: Slave~ Interrupt Control Register
> {I2CS24}: Slave~ Interrupt Status Register
> {I2CS28}: Slave~ Command/Status Register
> {I2CS2C}: Slave~ DMA Buffer Length Register
> {I2CM30}: Master DMA Mode Tx Buffer Base Address
> {I2CM34}: Master DMA Mode Rx Buffer Base Address
> {I2CS38}: Slave~ DMA Mode Tx Buffer Base Address
> {I2CS3C}: Slave~ DMA Mode Rx Buffer Base Address
> {I2CS40}: Slave Device Address Register
> {I2CM48}: Master DMA Length Status Register
> {I2CS4C}: Slave DMA Length Status Register
> {I2CC50}: Current DMA Operating Address Status
> {I2CC54}: Current DMA Operating Length Status
>
> Signed-off-by: Ryan Chen <ryan_chen@...eedtech.com>
> ---
> drivers/i2c/busses/Kconfig | 23 +-
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-ast2600.c | 1038 ++++++++++++++++++++++++++++++
> 3 files changed, 1054 insertions(+), 8 deletions(-)
> create mode 100644 drivers/i2c/busses/i2c-ast2600.c
>
[...]
> diff --git a/drivers/i2c/busses/i2c-ast2600.c b/drivers/i2c/busses/i2c-ast2600.c
> new file mode 100644
> index 000000000000..15e600ff50ec
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-ast2600.c
> @@ -0,0 +1,1038 @@
[...]
> +static int ast2600_i2c_probe(struct platform_device *pdev)
> +{
>
[...]
> + i2c_bus->rst = devm_reset_control_get_shared(dev, NULL);
> + if (IS_ERR(i2c_bus->rst))
> + return dev_err_probe(dev, PTR_ERR(i2c_bus->rst), "Missing reset ctrl\n");
What ...
> + i2c_bus->rst = devm_reset_control_get_shared_deasserted(dev, NULL);
> + if (IS_ERR(i2c_bus->rst))
> + return dev_err_probe(dev, PTR_ERR(i2c_bus->rst), "Missing reset ctrl\n");
... is this?
Choose one. If you use 1), call reset_control_deassert() somewhere. If
you use 2), remove reset_control_assert() below.
[...]
> +static void ast2600_i2c_remove(struct platform_device *pdev)
> +{
> + struct ast2600_i2c_bus *i2c_bus = platform_get_drvdata(pdev);
> +
> + /* Disable everything. */
> + writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> + writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
> + reset_control_assert(i2c_bus->rst);
Drop if using devm_reset_control_get_shared_deasserted()
regards
Philipp
Powered by blists - more mailing lists