[PATCH v2 0/3] i3c dw,ast2600: Add a driver for the AST2600 i3c controller

[PATCH v2 0/3] i3c dw,ast2600: Add a driver for the AST2600 i3c controller

[Jeremy Kerr]

This series adds a new i3c controller driver, for the ASPEED AST2600 13c
SoC peripheral. This device is very similar to the dw i3c controller, so
we implement this by adding a little platform abstraction to the dw
driver, and then a platform implementation for ast2600.

v2: This is a rework from an earlier series that implemented this as
part of the dw driver; I have adopted Ben Dooks' suggestion to split
into a new driver + exported hooks from the dw base.

As always: comments, queries etc. are most welcome.

Cheers,


Jeremy

Jeremy Kerr (3):
  i3c: dw: Add infrastructure for platform-specific implementations
  dt-bindings: i3c: Add AST2600 i3c controller
  i3c: ast2600: Add AST2600 platform-specific driver

 .../bindings/i3c/aspeed,ast2600-i3c.yaml      |  72 ++++++++
 MAINTAINERS                                   |   6 +
 drivers/i3c/master/Kconfig                    |  14 ++
 drivers/i3c/master/Makefile                   |   1 +
 drivers/i3c/master/ast2600-i3c-master.c       | 169 ++++++++++++++++++
 drivers/i3c/master/dw-i3c-master.c            |  76 ++++----
 drivers/i3c/master/dw-i3c-master.h            |  55 ++++++
 7 files changed, 360 insertions(+), 33 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml
 create mode 100644 drivers/i3c/master/ast2600-i3c-master.c
 create mode 100644 drivers/i3c/master/dw-i3c-master.h

-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v2 1/3] i3c: dw: Add infrastructure for platform-specific implementations

[Jeremy Kerr]

The dw i3c core can be integrated into various SoC devices. Platforms
that use this core may need a little configuration that is specific to
that platform.

Add some infrastructure to allow platform-specific behaviour: common
probe/remove functions, a set of platform hook operations, and a pointer
for platform-specific data in struct dw_i3c_master. Move the common api
into a new (i3c local) header file.

Platforms will provide their own struct platform_driver, which allocates
struct dw_i3c_master, does any platform-specific probe behaviour, and
calls into the common probe.

A future change will add new platform support that uses this
infrastructure.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

---
v2:
 - structure for platform implementations as separate platform_drivers,
   to be implemented in separate source files
 - provide default ops rather than null checks
---
 drivers/i3c/master/dw-i3c-master.c | 76 +++++++++++++++++-------------
 drivers/i3c/master/dw-i3c-master.h | 55 +++++++++++++++++++++
 2 files changed, 98 insertions(+), 33 deletions(-)
 create mode 100644 drivers/i3c/master/dw-i3c-master.h

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 1c146a39e1bd..9fc03108a5db 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -21,6 +21,8 @@
 #include <linux/reset.h>
 #include <linux/slab.h>
 
+#include "dw-i3c-master.h"
+
 #define DEVICE_CTRL			0x0
 #define DEV_CTRL_ENABLE			BIT(31)
 #define DEV_CTRL_RESUME			BIT(30)
@@ -189,8 +191,6 @@
 #define DEV_ADDR_TABLE_STATIC_ADDR(x)	((x) & GENMASK(6, 0))
 #define DEV_ADDR_TABLE_LOC(start, idx)	((start) + ((idx) << 2))
 
-#define MAX_DEVS 32
-
 #define I3C_BUS_SDR1_SCL_RATE		8000000
 #define I3C_BUS_SDR2_SCL_RATE		6000000
 #define I3C_BUS_SDR3_SCL_RATE		4000000
@@ -201,11 +201,6 @@
 
 #define XFER_TIMEOUT (msecs_to_jiffies(1000))
 
-struct dw_i3c_master_caps {
-	u8 cmdfifodepth;
-	u8 datafifodepth;
-};
-
 struct dw_i3c_cmd {
 	u32 cmd_lo;
 	u32 cmd_hi;
@@ -224,25 +219,6 @@ struct dw_i3c_xfer {
 	struct dw_i3c_cmd cmds[];
 };
 
-struct dw_i3c_master {
-	struct i3c_master_controller base;
-	u16 maxdevs;
-	u16 datstartaddr;
-	u32 free_pos;
-	struct {
-		struct list_head list;
-		struct dw_i3c_xfer *cur;
-		spinlock_t lock;
-	} xferqueue;
-	struct dw_i3c_master_caps caps;
-	void __iomem *regs;
-	struct reset_control *core_rst;
-	struct clk *core_clk;
-	char version[5];
-	char type[5];
-	u8 addrs[MAX_DEVS];
-};
-
 struct dw_i3c_i2c_dev_data {
 	u8 index;
 };
@@ -602,6 +578,10 @@ static int dw_i3c_master_bus_init(struct i3c_master_controller *m)
 	u32 thld_ctrl;
 	int ret;
 
+	ret = master->platform_ops->init(master);
+	if (ret)
+		return ret;
+
 	switch (bus->mode) {
 	case I3C_BUS_MODE_MIXED_FAST:
 	case I3C_BUS_MODE_MIXED_LIMITED:
@@ -1124,14 +1104,23 @@ static const struct i3c_master_controller_ops dw_mipi_i3c_ops = {
 	.i2c_xfers = dw_i3c_master_i2c_xfers,
 };
 
-static int dw_i3c_probe(struct platform_device *pdev)
+/* default platform ops implementations */
+static int dw_i3c_platform_init_nop(struct dw_i3c_master *i3c)
+{
+	return 0;
+}
+
+static const struct dw_i3c_platform_ops dw_i3c_platform_ops_default = {
+	.init = dw_i3c_platform_init_nop,
+};
+
+int dw_i3c_common_probe(struct dw_i3c_master *master,
+			struct platform_device *pdev)
 {
-	struct dw_i3c_master *master;
 	int ret, irq;
 
-	master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
-	if (!master)
-		return -ENOMEM;
+	if (!master->platform_ops)
+		master->platform_ops = &dw_i3c_platform_ops_default;
 
 	master->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(master->regs))
@@ -1192,10 +1181,10 @@ static int dw_i3c_probe(struct platform_device *pdev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(dw_i3c_common_probe);
 
-static int dw_i3c_remove(struct platform_device *pdev)
+int dw_i3c_common_remove(struct dw_i3c_master *master)
 {
-	struct dw_i3c_master *master = platform_get_drvdata(pdev);
 	int ret;
 
 	ret = i3c_master_unregister(&master->base);
@@ -1208,6 +1197,27 @@ static int dw_i3c_remove(struct platform_device *pdev)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dw_i3c_common_remove);
+
+/* base platform implementation */
+
+static int dw_i3c_probe(struct platform_device *pdev)
+{
+	struct dw_i3c_master *master;
+
+	master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
+	if (!master)
+		return -ENOMEM;
+
+	return dw_i3c_common_probe(master, pdev);
+}
+
+static int dw_i3c_remove(struct platform_device *pdev)
+{
+	struct dw_i3c_master *master = platform_get_drvdata(pdev);
+
+	return dw_i3c_common_remove(master);
+}
 
 static const struct of_device_id dw_i3c_master_of_match[] = {
 	{ .compatible = "snps,dw-i3c-master-1.00a", },
diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h
new file mode 100644
index 000000000000..9f1e48211aa4
--- /dev/null
+++ b/drivers/i3c/master/dw-i3c-master.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2023 Code Construct
+ *
+ * Author: Jeremy Kerr <jk@codeconstruct.com.au>
+ */
+
+#include <linux/clk.h>
+#include <linux/i3c/master.h>
+#include <linux/reset.h>
+#include <linux/types.h>
+
+#define DW_I3C_MAX_DEVS 32
+
+struct dw_i3c_master_caps {
+	u8 cmdfifodepth;
+	u8 datafifodepth;
+};
+
+struct dw_i3c_master {
+	struct i3c_master_controller base;
+	u16 maxdevs;
+	u16 datstartaddr;
+	u32 free_pos;
+	struct {
+		struct list_head list;
+		struct dw_i3c_xfer *cur;
+		spinlock_t lock;
+	} xferqueue;
+	struct dw_i3c_master_caps caps;
+	void __iomem *regs;
+	struct reset_control *core_rst;
+	struct clk *core_clk;
+	char version[5];
+	char type[5];
+	u8 addrs[DW_I3C_MAX_DEVS];
+
+	/* platform-specific data */
+	const struct dw_i3c_platform_ops *platform_ops;
+	void *platform_data;
+};
+
+struct dw_i3c_platform_ops {
+	/*
+	 * Called on early bus init: the i3c has been set up, but before any
+	 * transactions have taken place. Platform implementations may use to
+	 * perform actual device enabling with the i3c core ready.
+	 */
+	int (*init)(struct dw_i3c_master *i3c);
+};
+
+extern int dw_i3c_common_probe(struct dw_i3c_master *master,
+			       struct platform_device *pdev);
+extern int dw_i3c_common_remove(struct dw_i3c_master *master);
+
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v2 2/3] dt-bindings: i3c: Add AST2600 i3c controller

[Jeremy Kerr]

Add a devicetree binding for the ast2600 i3c controller hardware. This
is heavily based on the designware i3c core, plus a reset facility
and two platform-specific properties:

 - sda-pullup-ohms: to specify the value of the configurable pullup
   resistors on the SDA line

 - aspeed,global-regs: to reference the (ast2600-specific) i3c global
   register block, and the device index to use within it.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> (on v1)
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

---
v2:
 - example: replace aspeed clock defines with constants, so we're not
   reliant on recent clk patches.
---
 .../bindings/i3c/aspeed,ast2600-i3c.yaml      | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml

diff --git a/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml b/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml
new file mode 100644
index 000000000000..fcc3dbff9c9a
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i3c/aspeed,ast2600-i3c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ASPEED AST2600 i3c controller
+
+maintainers:
+  - Jeremy Kerr <jk@codeconstruct.com.au>
+
+allOf:
+  - $ref: i3c.yaml#
+
+properties:
+  compatible:
+    const: aspeed,ast2600-i3c
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  sda-pullup-ohms:
+    enum: [545, 750, 2000]
+    default: 2000
+    description: |
+      Value to configure SDA pullup resistor, in Ohms.
+
+  aspeed,global-regs:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to i3c global register syscon node
+          - description: index of this i3c controller in the global register set
+    description: |
+      A (phandle, controller index) reference to the i3c global register set
+      used for this device.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+  - aspeed,global-regs
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    i3c-master@2000 {
+        compatible = "aspeed,ast2600-i3c";
+        reg = <0x2000 0x1000>;
+        #address-cells = <3>;
+        #size-cells = <0>;
+        clocks = <&syscon 0>;
+        resets = <&syscon 0>;
+        aspeed,global-regs = <&i3c_global 0>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_i3c1_default>;
+        interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>;
+    };
+...
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v2 3/3] i3c: ast2600: Add AST2600 platform-specific driver

[Jeremy Kerr]

Now that we have platform-specific infrastructure for the dw i3c driver,
add platform support for the ASPEED AST2600 SoC.

The AST2600 has a small set of "i3c global" registers, providing
platform-level i3c configuration outside of the i3c core.

For the ast2600, we need a couple of extra setup operations:

 - on probe: find the i3c global register set and parse the SDA pullup
   resistor values

 - on init: set the pullups accordingly, and set the i3c instance IDs

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

---
v2:
 - use new dw platform infrastructure
---
 MAINTAINERS                             |   6 +
 drivers/i3c/master/Kconfig              |  14 ++
 drivers/i3c/master/Makefile             |   1 +
 drivers/i3c/master/ast2600-i3c-master.c | 169 ++++++++++++++++++++++++
 4 files changed, 190 insertions(+)
 create mode 100644 drivers/i3c/master/ast2600-i3c-master.c

diff --git a/MAINTAINERS b/MAINTAINERS
index bd8ebc25afcf..ecadd5ccf771 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9714,6 +9714,12 @@ S:	Orphan
 F:	Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
 F:	drivers/i3c/master/dw*
 
+I3C DRIVER FOR ASPEED AST2600
+M:	Jeremy Kerr <jk@codeconstruct.com.au>
+S:	Maintained
+F:	Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml
+F:	drivers/i3c/master/ast2600-i3c-master.c
+
 I3C SUBSYSTEM
 M:	Alexandre Belloni <alexandre.belloni@bootlin.com>
 L:	linux-i3c@lists.infradead.org (moderated for non-subscribers)
diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
index 3b8f95916f46..90dee3ec5520 100644
--- a/drivers/i3c/master/Kconfig
+++ b/drivers/i3c/master/Kconfig
@@ -22,6 +22,20 @@ config DW_I3C_MASTER
 	  This driver can also be built as a module.  If so, the module
 	  will be called dw-i3c-master.
 
+config AST2600_I3C_MASTER
+	tristate "ASPEED AST2600 I3C master driver"
+	depends on DW_I3C_MASTER
+	depends on ARCH_ASPEED || COMPILE_TEST
+	select MFD_SYSCON
+	help
+	  Support for ASPEED AST2600 I3C Controller.
+
+	  This hardware is an instance of the DW I3C controller; this
+	  driver adds platform- specific support for AST2600 hardware.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ast2600-i3c-master.
+
 config SVC_I3C_MASTER
 	tristate "Silvaco I3C Dual-Role Master driver"
 	depends on I3C
diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
index b3fee0f690b2..3e97960160bc 100644
--- a/drivers/i3c/master/Makefile
+++ b/drivers/i3c/master/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_CDNS_I3C_MASTER)		+= i3c-master-cdns.o
 obj-$(CONFIG_DW_I3C_MASTER)		+= dw-i3c-master.o
+obj-$(CONFIG_AST2600_I3C_MASTER)	+= ast2600-i3c-master.o
 obj-$(CONFIG_SVC_I3C_MASTER)		+= svc-i3c-master.o
 obj-$(CONFIG_MIPI_I3C_HCI)		+= mipi-i3c-hci/
diff --git a/drivers/i3c/master/ast2600-i3c-master.c b/drivers/i3c/master/ast2600-i3c-master.c
new file mode 100644
index 000000000000..d3d7b7d19f22
--- /dev/null
+++ b/drivers/i3c/master/ast2600-i3c-master.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 Code Construct
+ *
+ * Author: Jeremy Kerr <jk@codeconstruct.com.au>
+ */
+
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include "dw-i3c-master.h"
+
+/* AST2600-specific global register set */
+#define AST2600_I3CG_REG0(idx)	(((idx) * 4 * 4) + 0x10)
+#define AST2600_I3CG_REG1(idx)	(((idx) * 4 * 4) + 0x14)
+
+#define AST2600_I3CG_REG0_SDA_PULLUP_EN_MASK	GENMASK(29, 28)
+#define AST2600_I3CG_REG0_SDA_PULLUP_EN_2K	(0x0 << 28)
+#define AST2600_I3CG_REG0_SDA_PULLUP_EN_750	(0x2 << 28)
+
+#define AST2600_I3CG_REG1_I2C_MODE		BIT(0)
+#define AST2600_I3CG_REG1_TEST_MODE		BIT(1)
+#define AST2600_I3CG_REG1_ACT_MODE_MASK		GENMASK(3, 2)
+#define AST2600_I3CG_REG1_ACT_MODE(x)		(((x) << 2) & AST2600_I3CG_REG1_ACT_MODE_MASK)
+#define AST2600_I3CG_REG1_PENDING_INT_MASK	GENMASK(7, 4)
+#define AST2600_I3CG_REG1_PENDING_INT(x)	(((x) << 4) & AST2600_I3CG_REG1_PENDING_INT_MASK)
+#define AST2600_I3CG_REG1_SA_MASK		GENMASK(14, 8)
+#define AST2600_I3CG_REG1_SA(x)			(((x) << 8) & AST2600_I3CG_REG1_SA_MASK)
+#define AST2600_I3CG_REG1_SA_EN			BIT(15)
+#define AST2600_I3CG_REG1_INST_ID_MASK		GENMASK(19, 16)
+#define AST2600_I3CG_REG1_INST_ID(x)		(((x) << 16) & AST2600_I3CG_REG1_INST_ID_MASK)
+
+#define AST2600_DEFAULT_SDA_PULLUP_OHMS		2000
+
+struct ast2600_i3c_platform_data {
+	struct regmap *global_regs;
+	unsigned int global_idx;
+	unsigned int sda_pullup;
+};
+
+static int ast2600_i3c_pullup_to_reg(unsigned int ohms, u32 *regp)
+{
+	u32 reg;
+
+	switch (ohms) {
+	case 2000:
+		reg = AST2600_I3CG_REG0_SDA_PULLUP_EN_2K;
+		break;
+	case 750:
+		reg = AST2600_I3CG_REG0_SDA_PULLUP_EN_750;
+		break;
+	case 545:
+		reg = AST2600_I3CG_REG0_SDA_PULLUP_EN_2K |
+			AST2600_I3CG_REG0_SDA_PULLUP_EN_750;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (regp)
+		*regp = reg;
+
+	return 0;
+}
+
+static int ast2600_i3c_init(struct dw_i3c_master *master)
+{
+	struct ast2600_i3c_platform_data *pdata = master->platform_data;
+	u32 reg = 0;
+	int rc;
+
+	/* reg0: set SDA pullup values */
+	rc = ast2600_i3c_pullup_to_reg(pdata->sda_pullup, &reg);
+	if (rc)
+		return rc;
+
+	rc = regmap_write(pdata->global_regs,
+			  AST2600_I3CG_REG0(pdata->global_idx), reg);
+	if (rc)
+		return rc;
+
+	/* reg1: set up the instance id, but leave everything else disabled,
+	 * as it's all for client mode
+	 */
+	reg = AST2600_I3CG_REG1_INST_ID(pdata->global_idx);
+	rc = regmap_write(pdata->global_regs,
+			  AST2600_I3CG_REG1(pdata->global_idx), reg);
+
+	return rc;
+}
+
+const struct dw_i3c_platform_ops ast2600_i3c_ops = {
+	.init = ast2600_i3c_init,
+};
+
+static int ast2600_i3c_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct ast2600_i3c_platform_data *pdata;
+	struct of_phandle_args gspec;
+	struct dw_i3c_master *dw_i3c;
+	int rc;
+
+	dw_i3c = devm_kzalloc(&pdev->dev, sizeof(*dw_i3c) + sizeof(*pdata),
+			      GFP_KERNEL);
+	if (!dw_i3c)
+		return -ENOMEM;
+
+	pdata = (void *)(dw_i3c + 1);
+
+	rc = of_parse_phandle_with_fixed_args(np, "aspeed,global-regs", 1, 0,
+					      &gspec);
+	if (rc)
+		return -ENODEV;
+
+	pdata->global_regs = syscon_node_to_regmap(gspec.np);
+	of_node_put(gspec.np);
+
+	if (IS_ERR(pdata->global_regs))
+		return PTR_ERR(pdata->global_regs);
+
+	pdata->global_idx = gspec.args[0];
+
+	rc = of_property_read_u32(np, "sda-pullup-ohms", &pdata->sda_pullup);
+	if (rc)
+		pdata->sda_pullup = AST2600_DEFAULT_SDA_PULLUP_OHMS;
+
+	rc = ast2600_i3c_pullup_to_reg(pdata->sda_pullup, NULL);
+	if (rc)
+		dev_err(&dw_i3c->base.dev, "invalid sda-pullup value %d\n",
+			pdata->sda_pullup);
+
+	dw_i3c->platform_ops = &ast2600_i3c_ops;
+	dw_i3c->platform_data = pdata;
+	rc = dw_i3c_common_probe(dw_i3c, pdev);
+
+	return rc;
+}
+
+static int ast2600_i3c_remove(struct platform_device *pdev)
+{
+	struct dw_i3c_master *dw_i3c = platform_get_drvdata(pdev);
+
+	return dw_i3c_common_remove(dw_i3c);
+}
+
+static const struct of_device_id ast2600_i3c_master_of_match[] = {
+	{ .compatible = "aspeed,ast2600-i3c", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ast2600_i3c_master_of_match);
+
+static struct platform_driver ast2600_i3c_driver = {
+	.probe = ast2600_i3c_probe,
+	.remove = ast2600_i3c_remove,
+	.driver = {
+		.name = "ast2600-i3c-master",
+		.of_match_table = of_match_ptr(ast2600_i3c_master_of_match),
+	},
+};
+module_platform_driver(ast2600_i3c_driver);
+
+MODULE_AUTHOR("Jeremy Kerr <jk@codeconstruct.com.au>");
+MODULE_DESCRIPTION("ASPEED AST2600 I3C driver");
+MODULE_LICENSE("GPL");
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v2 2/3] dt-bindings: i3c: Add AST2600 i3c controller

[Krzysztof Kozlowski]

On 30/03/2023 09:22, Jeremy Kerr wrote:
> Add a devicetree binding for the ast2600 i3c controller hardware. This
> is heavily based on the designware i3c core, plus a reset facility
> and two platform-specific properties:
> 
>  - sda-pullup-ohms: to specify the value of the configurable pullup
>    resistors on the SDA line
> 
>  - aspeed,global-regs: to reference the (ast2600-specific) i3c global
>    register block, and the device index to use within it.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> (on v1)
> Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC.  It might happen, that command when run on an older
kernel, gives you outdated entries.  Therefore please be sure you base
your patches on recent Linux kernel.


Best regards,
Krzysztof


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v2 0/3] i3c dw,ast2600: Add a driver for the AST2600 i3c controller

[Jack Chen]

Hi Jeremy,
Thanks for the change, especially IBI features in other threads.
From my understanding, ASPEED AST2600 is a SoC which uses Synopsys'
I3C IP core, and several different registers, especially the pull-up
resistor.
If so, I am wondering if this is the right place to add
ast2600-i3c-master.c, given that all current three xxx-i3c-master.c
drivers in this directory are from IP providers directly.
What about moving it under ~/driver/soc/?
Thanks
Zenghu Chen

On Thu, Mar 30, 2023 at 3:22 AM Jeremy Kerr <jk@codeconstruct.com.au> wrote:
>
> This series adds a new i3c controller driver, for the ASPEED AST2600 13c
> SoC peripheral. This device is very similar to the dw i3c controller, so
> we implement this by adding a little platform abstraction to the dw
> driver, and then a platform implementation for ast2600.
>
> v2: This is a rework from an earlier series that implemented this as
> part of the dw driver; I have adopted Ben Dooks' suggestion to split
> into a new driver + exported hooks from the dw base.
>
> As always: comments, queries etc. are most welcome.
>
> Cheers,
>
>
> Jeremy
>
> Jeremy Kerr (3):
>   i3c: dw: Add infrastructure for platform-specific implementations
>   dt-bindings: i3c: Add AST2600 i3c controller
>   i3c: ast2600: Add AST2600 platform-specific driver
>
>  .../bindings/i3c/aspeed,ast2600-i3c.yaml      |  72 ++++++++
>  MAINTAINERS                                   |   6 +
>  drivers/i3c/master/Kconfig                    |  14 ++
>  drivers/i3c/master/Makefile                   |   1 +
>  drivers/i3c/master/ast2600-i3c-master.c       | 169 ++++++++++++++++++
>  drivers/i3c/master/dw-i3c-master.c            |  76 ++++----
>  drivers/i3c/master/dw-i3c-master.h            |  55 ++++++
>  7 files changed, 360 insertions(+), 33 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml
>  create mode 100644 drivers/i3c/master/ast2600-i3c-master.c
>  create mode 100644 drivers/i3c/master/dw-i3c-master.h
>
> --
> 2.39.2
>

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v2 3/3] i3c: ast2600: Add AST2600 platform-specific driver

[Ben Dooks]

On 30/03/2023 08:22, Jeremy Kerr wrote:
> Now that we have platform-specific infrastructure for the dw i3c driver,
> add platform support for the ASPEED AST2600 SoC.
> 
> The AST2600 has a small set of "i3c global" registers, providing
> platform-level i3c configuration outside of the i3c core.
> 
> For the ast2600, we need a couple of extra setup operations:
> 
>   - on probe: find the i3c global register set and parse the SDA pullup
>     resistor values
> 
>   - on init: set the pullups accordingly, and set the i3c instance IDs
> 
> Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
> 
> ---
> v2:
>   - use new dw platform infrastructure
> ---
>   MAINTAINERS                             |   6 +
>   drivers/i3c/master/Kconfig              |  14 ++
>   drivers/i3c/master/Makefile             |   1 +
>   drivers/i3c/master/ast2600-i3c-master.c | 169 ++++++++++++++++++++++++
>   4 files changed, 190 insertions(+)
>   create mode 100644 drivers/i3c/master/ast2600-i3c-master.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bd8ebc25afcf..ecadd5ccf771 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9714,6 +9714,12 @@ S:	Orphan
>   F:	Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
>   F:	drivers/i3c/master/dw*
>   
> +I3C DRIVER FOR ASPEED AST2600
> +M:	Jeremy Kerr <jk@codeconstruct.com.au>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml
> +F:	drivers/i3c/master/ast2600-i3c-master.c
> +
>   I3C SUBSYSTEM
>   M:	Alexandre Belloni <alexandre.belloni@bootlin.com>
>   L:	linux-i3c@lists.infradead.org (moderated for non-subscribers)
> diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
> index 3b8f95916f46..90dee3ec5520 100644
> --- a/drivers/i3c/master/Kconfig
> +++ b/drivers/i3c/master/Kconfig
> @@ -22,6 +22,20 @@ config DW_I3C_MASTER
>   	  This driver can also be built as a module.  If so, the module
>   	  will be called dw-i3c-master.
>   
> +config AST2600_I3C_MASTER
> +	tristate "ASPEED AST2600 I3C master driver"
> +	depends on DW_I3C_MASTER
> +	depends on ARCH_ASPEED || COMPILE_TEST
> +	select MFD_SYSCON
> +	help
> +	  Support for ASPEED AST2600 I3C Controller.
> +
> +	  This hardware is an instance of the DW I3C controller; this
> +	  driver adds platform- specific support for AST2600 hardware.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called ast2600-i3c-master.
> +
>   config SVC_I3C_MASTER
>   	tristate "Silvaco I3C Dual-Role Master driver"
>   	depends on I3C
> diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
> index b3fee0f690b2..3e97960160bc 100644
> --- a/drivers/i3c/master/Makefile
> +++ b/drivers/i3c/master/Makefile
> @@ -1,5 +1,6 @@
>   # SPDX-License-Identifier: GPL-2.0-only
>   obj-$(CONFIG_CDNS_I3C_MASTER)		+= i3c-master-cdns.o
>   obj-$(CONFIG_DW_I3C_MASTER)		+= dw-i3c-master.o
> +obj-$(CONFIG_AST2600_I3C_MASTER)	+= ast2600-i3c-master.o
>   obj-$(CONFIG_SVC_I3C_MASTER)		+= svc-i3c-master.o
>   obj-$(CONFIG_MIPI_I3C_HCI)		+= mipi-i3c-hci/
> diff --git a/drivers/i3c/master/ast2600-i3c-master.c b/drivers/i3c/master/ast2600-i3c-master.c
> new file mode 100644
> index 000000000000..d3d7b7d19f22
> --- /dev/null
> +++ b/drivers/i3c/master/ast2600-i3c-master.c
> @@ -0,0 +1,169 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2023 Code Construct
> + *
> + * Author: Jeremy Kerr <jk@codeconstruct.com.au>
> + */
> +
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include "dw-i3c-master.h"
> +
> +/* AST2600-specific global register set */
> +#define AST2600_I3CG_REG0(idx)	(((idx) * 4 * 4) + 0x10)
> +#define AST2600_I3CG_REG1(idx)	(((idx) * 4 * 4) + 0x14)
> +
> +#define AST2600_I3CG_REG0_SDA_PULLUP_EN_MASK	GENMASK(29, 28)
> +#define AST2600_I3CG_REG0_SDA_PULLUP_EN_2K	(0x0 << 28)
> +#define AST2600_I3CG_REG0_SDA_PULLUP_EN_750	(0x2 << 28)
> +
> +#define AST2600_I3CG_REG1_I2C_MODE		BIT(0)
> +#define AST2600_I3CG_REG1_TEST_MODE		BIT(1)
> +#define AST2600_I3CG_REG1_ACT_MODE_MASK		GENMASK(3, 2)
> +#define AST2600_I3CG_REG1_ACT_MODE(x)		(((x) << 2) & AST2600_I3CG_REG1_ACT_MODE_MASK)
> +#define AST2600_I3CG_REG1_PENDING_INT_MASK	GENMASK(7, 4)
> +#define AST2600_I3CG_REG1_PENDING_INT(x)	(((x) << 4) & AST2600_I3CG_REG1_PENDING_INT_MASK)
> +#define AST2600_I3CG_REG1_SA_MASK		GENMASK(14, 8)
> +#define AST2600_I3CG_REG1_SA(x)			(((x) << 8) & AST2600_I3CG_REG1_SA_MASK)
> +#define AST2600_I3CG_REG1_SA_EN			BIT(15)
> +#define AST2600_I3CG_REG1_INST_ID_MASK		GENMASK(19, 16)
> +#define AST2600_I3CG_REG1_INST_ID(x)		(((x) << 16) & AST2600_I3CG_REG1_INST_ID_MASK)
> +
> +#define AST2600_DEFAULT_SDA_PULLUP_OHMS		2000
> +
> +struct ast2600_i3c_platform_data {
> +	struct regmap *global_regs;
> +	unsigned int global_idx;
> +	unsigned int sda_pullup;
> +};
> +
> +static int ast2600_i3c_pullup_to_reg(unsigned int ohms, u32 *regp)
> +{
> +	u32 reg;
> +
> +	switch (ohms) {
> +	case 2000:
> +		reg = AST2600_I3CG_REG0_SDA_PULLUP_EN_2K;
> +		break;
> +	case 750:
> +		reg = AST2600_I3CG_REG0_SDA_PULLUP_EN_750;
> +		break;
> +	case 545:
> +		reg = AST2600_I3CG_REG0_SDA_PULLUP_EN_2K |
> +			AST2600_I3CG_REG0_SDA_PULLUP_EN_750;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (regp)
> +		*regp = reg;
> +
> +	return 0;
> +}
> +
> +static int ast2600_i3c_init(struct dw_i3c_master *master)
> +{
> +	struct ast2600_i3c_platform_data *pdata = master->platform_data;
> +	u32 reg = 0;
> +	int rc;
> +
> +	/* reg0: set SDA pullup values */
> +	rc = ast2600_i3c_pullup_to_reg(pdata->sda_pullup, &reg);
> +	if (rc)
> +		return rc;
> +
> +	rc = regmap_write(pdata->global_regs,
> +			  AST2600_I3CG_REG0(pdata->global_idx), reg);
> +	if (rc)
> +		return rc;
> +
> +	/* reg1: set up the instance id, but leave everything else disabled,
> +	 * as it's all for client mode
> +	 */
> +	reg = AST2600_I3CG_REG1_INST_ID(pdata->global_idx);
> +	rc = regmap_write(pdata->global_regs,
> +			  AST2600_I3CG_REG1(pdata->global_idx), reg);
> +
> +	return rc;
> +}
> +
> +const struct dw_i3c_platform_ops ast2600_i3c_ops = {
> +	.init = ast2600_i3c_init,
> +};
> +
> +static int ast2600_i3c_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct ast2600_i3c_platform_data *pdata;
> +	struct of_phandle_args gspec;
> +	struct dw_i3c_master *dw_i3c;
> +	int rc;
> +
> +	dw_i3c = devm_kzalloc(&pdev->dev, sizeof(*dw_i3c) + sizeof(*pdata),
> +			      GFP_KERNEL);
> +	if (!dw_i3c)
> +		return -ENOMEM;


How about wrapping the dw_i3c_master struct in an as2600_master struct
and then you can use container of, and simply do sizeof(struct 
ast2600_master) ?

Also removes the need below to set pdata at-all

> +	pdata = (void *)(dw_i3c + 1);
> +
> +	rc = of_parse_phandle_with_fixed_args(np, "aspeed,global-regs", 1, 0,
> +					      &gspec);
> +	if (rc)
> +		return -ENODEV;
> +
> +	pdata->global_regs = syscon_node_to_regmap(gspec.np);
> +	of_node_put(gspec.np);
> +
> +	if (IS_ERR(pdata->global_regs))
> +		return PTR_ERR(pdata->global_regs);
> +
> +	pdata->global_idx = gspec.args[0];
> +
> +	rc = of_property_read_u32(np, "sda-pullup-ohms", &pdata->sda_pullup);
> +	if (rc)
> +		pdata->sda_pullup = AST2600_DEFAULT_SDA_PULLUP_OHMS;
> +
> +	rc = ast2600_i3c_pullup_to_reg(pdata->sda_pullup, NULL);
> +	if (rc)
> +		dev_err(&dw_i3c->base.dev, "invalid sda-pullup value %d\n",
> +			pdata->sda_pullup);
> +
> +	dw_i3c->platform_ops = &ast2600_i3c_ops;
> +	dw_i3c->platform_data = pdata;
> +	rc = dw_i3c_common_probe(dw_i3c, pdev);
> +
> +	return rc;
> +}
> +
> +static int ast2600_i3c_remove(struct platform_device *pdev)
> +{
> +	struct dw_i3c_master *dw_i3c = platform_get_drvdata(pdev);
> +
> +	return dw_i3c_common_remove(dw_i3c);
> +}
> +
> +static const struct of_device_id ast2600_i3c_master_of_match[] = {
> +	{ .compatible = "aspeed,ast2600-i3c", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ast2600_i3c_master_of_match);
> +
> +static struct platform_driver ast2600_i3c_driver = {
> +	.probe = ast2600_i3c_probe,
> +	.remove = ast2600_i3c_remove,
> +	.driver = {
> +		.name = "ast2600-i3c-master",
> +		.of_match_table = of_match_ptr(ast2600_i3c_master_of_match),

given this is probably an of-device only, of_match_ptr not needed here?

> +	},
> +};
> +module_platform_driver(ast2600_i3c_driver);
> +
> +MODULE_AUTHOR("Jeremy Kerr <jk@codeconstruct.com.au>");
> +MODULE_DESCRIPTION("ASPEED AST2600 I3C driver");
> +MODULE_LICENSE("GPL");


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v2 3/3] i3c: ast2600: Add AST2600 platform-specific driver

[Jeremy Kerr]

Hi Ben,

Thanks for the review. Comments inline:

> How about wrapping the dw_i3c_master struct in an as2600_master struct
> and then you can use container of, and simply do sizeof(struct
> ast2600_master) ?
> 
> Also removes the need below to set pdata at-all

Yep, sounds good, and avoids my quirky pointer arithmetic below that.

> > +static struct platform_driver ast2600_i3c_driver = {
> > +       .probe = ast2600_i3c_probe,
> > +       .remove = ast2600_i3c_remove,
> > +       .driver = {
> > +               .name = "ast2600-i3c-master",
> > +               .of_match_table =
> > of_match_ptr(ast2600_i3c_master_of_match),
> 
> given this is probably an of-device only, of_match_ptr not needed
> here?

Ack, will remove.

Thanks,


Jeremy

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v2 0/3] i3c dw,ast2600: Add a driver for the AST2600 i3c controller

[Jeremy Kerr]

Hi Zenghu,

> Thanks for the change, especially IBI features in other threads.
> From my understanding, ASPEED AST2600 is a SoC which uses Synopsys'
> I3C IP core, and several different registers, especially the pull-up
> resistor.
> If so, I am wondering if this is the right place to add
> ast2600-i3c-master.c, given that all current three xxx-i3c-master.c
> drivers in this directory are from IP providers directly.
> What about moving it under ~/driver/soc/?

It's my understanding that drivers/soc/ is for smaller parts of
SoC-specific functions, rather than implementing SoC driver instances
for existing subsystems.

I'd prefer to keep it with the i3c controller drivers; this means we can
keep the dw platform API as contained as possible (ie., within
drivers/i3c/master/). I expect that we will need some coordination of
changes until we have the platform-specific behaviour mostly described
(see the IBI series for another hook), and so coordinating changes
between drivers/soc/ and drivers/i3c/ may make things more complicated
than necessary.

There's certainly precedence for this pattern:

 * the aspeed ethernet mac is provided by the ftgmac100 driver plus some
   SoC-specific behaviour; that's entirely within drivers/net/

 * the aspeed VUART device is essentially a 16550 plus extra registers;
   that's entirely within drivers/tty/

Cheers,


Jeremy

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v2 0/3] i3c dw,ast2600: Add a driver for the AST2600 i3c controller

[Alexandre Belloni]

On 31/03/2023 10:33:35+0800, Jeremy Kerr wrote:
> Hi Zenghu,
> 
> > Thanks for the change, especially IBI features in other threads.
> > From my understanding, ASPEED AST2600 is a SoC which uses Synopsys'
> > I3C IP core, and several different registers, especially the pull-up
> > resistor.
> > If so, I am wondering if this is the right place to add
> > ast2600-i3c-master.c, given that all current three xxx-i3c-master.c
> > drivers in this directory are from IP providers directly.
> > What about moving it under ~/driver/soc/?
> 
> It's my understanding that drivers/soc/ is for smaller parts of
> SoC-specific functions, rather than implementing SoC driver instances
> for existing subsystems.
> 
> I'd prefer to keep it with the i3c controller drivers; this means we can
> keep the dw platform API as contained as possible (ie., within
> drivers/i3c/master/). I expect that we will need some coordination of
> changes until we have the platform-specific behaviour mostly described
> (see the IBI series for another hook), and so coordinating changes
> between drivers/soc/ and drivers/i3c/ may make things more complicated
> than necessary.
> 
> There's certainly precedence for this pattern:
> 
>  * the aspeed ethernet mac is provided by the ftgmac100 driver plus some
>    SoC-specific behaviour; that's entirely within drivers/net/
> 
>  * the aspeed VUART device is essentially a 16550 plus extra registers;
>    that's entirely within drivers/tty/
> 

I confirm this has to be in drivers/i3c


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c