All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-19  8:04 ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-19  8:04 UTC (permalink / raw)
  To: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER
  Cc: ryan_chen, chiawei_wang, troy_lee, steven_lee, jamin_lin

Jamin Lin (3):
  i2c: aspeed: avoid new registers definition of AST2600
  ARM: dts: aspeed: Add node for AST2600 I2C
  dt-bindings: aspeed-i2c: Convert txt to yaml format

 .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
 .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
 arch/arm/boot/dts/aspeed-g6.dtsi              |  8 ++
 drivers/i2c/busses/i2c-aspeed.c               | 22 +++++
 4 files changed, 119 insertions(+), 49 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
 delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt

-- 
2.17.1


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

* [PATCH 0/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-19  8:04 ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-19  8:04 UTC (permalink / raw)
  To: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER
  Cc: steven_lee, chiawei_wang, troy_lee, ryan_chen, jamin_lin

Jamin Lin (3):
  i2c: aspeed: avoid new registers definition of AST2600
  ARM: dts: aspeed: Add node for AST2600 I2C
  dt-bindings: aspeed-i2c: Convert txt to yaml format

 .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
 .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
 arch/arm/boot/dts/aspeed-g6.dtsi              |  8 ++
 drivers/i2c/busses/i2c-aspeed.c               | 22 +++++
 4 files changed, 119 insertions(+), 49 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
 delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt

-- 
2.17.1


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

* [PATCH 0/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-19  8:04 ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-19  8:04 UTC (permalink / raw)
  To: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER
  Cc: ryan_chen, chiawei_wang, troy_lee, steven_lee, jamin_lin

Jamin Lin (3):
  i2c: aspeed: avoid new registers definition of AST2600
  ARM: dts: aspeed: Add node for AST2600 I2C
  dt-bindings: aspeed-i2c: Convert txt to yaml format

 .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
 .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
 arch/arm/boot/dts/aspeed-g6.dtsi              |  8 ++
 drivers/i2c/busses/i2c-aspeed.c               | 22 +++++
 4 files changed, 119 insertions(+), 49 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
 delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
  2021-05-19  8:04 ` Jamin Lin
  (?)
@ 2021-05-19  8:04   ` Jamin Lin
  -1 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-19  8:04 UTC (permalink / raw)
  To: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER
  Cc: ryan_chen, chiawei_wang, troy_lee, steven_lee, jamin_lin

The register definition between AST2600 A2 and A3 is different.
This patch avoid new registers definition of AST2600 to use
this driver. We will submit the path for the new registers
definition of AST2600.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 724bf30600d6..007309077d9f 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -19,14 +19,20 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
 
+/* I2C Global Registers */
+/* 0x0c : I2CG Global Control Register (AST2500)  */
+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
+
 /* I2C Register */
 #define ASPEED_I2C_FUN_CTRL_REG				0x00
 #define ASPEED_I2C_AC_TIMING_REG1			0x04
@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	struct resource *res;
 	int irq, ret;
 
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "aspeed,ast2600-i2c-bus")) {
+		u32 global_ctrl;
+		struct regmap *gr_regmap;
+
+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
+
+		if (IS_ERR(gr_regmap)) {
+			ret = PTR_ERR(gr_regmap);
+		} else {
+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
+			if (global_ctrl & BIT(2))
+				return -EIO;
+		}
+	}
+
 	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
 	if (!bus)
 		return -ENOMEM;
-- 
2.17.1


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

* [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-19  8:04   ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-19  8:04 UTC (permalink / raw)
  To: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER
  Cc: steven_lee, chiawei_wang, troy_lee, ryan_chen, jamin_lin

The register definition between AST2600 A2 and A3 is different.
This patch avoid new registers definition of AST2600 to use
this driver. We will submit the path for the new registers
definition of AST2600.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 724bf30600d6..007309077d9f 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -19,14 +19,20 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
 
+/* I2C Global Registers */
+/* 0x0c : I2CG Global Control Register (AST2500)  */
+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
+
 /* I2C Register */
 #define ASPEED_I2C_FUN_CTRL_REG				0x00
 #define ASPEED_I2C_AC_TIMING_REG1			0x04
@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	struct resource *res;
 	int irq, ret;
 
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "aspeed,ast2600-i2c-bus")) {
+		u32 global_ctrl;
+		struct regmap *gr_regmap;
+
+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
+
+		if (IS_ERR(gr_regmap)) {
+			ret = PTR_ERR(gr_regmap);
+		} else {
+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
+			if (global_ctrl & BIT(2))
+				return -EIO;
+		}
+	}
+
 	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
 	if (!bus)
 		return -ENOMEM;
-- 
2.17.1


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

* [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-19  8:04   ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-19  8:04 UTC (permalink / raw)
  To: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER
  Cc: ryan_chen, chiawei_wang, troy_lee, steven_lee, jamin_lin

The register definition between AST2600 A2 and A3 is different.
This patch avoid new registers definition of AST2600 to use
this driver. We will submit the path for the new registers
definition of AST2600.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 724bf30600d6..007309077d9f 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -19,14 +19,20 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
 
+/* I2C Global Registers */
+/* 0x0c : I2CG Global Control Register (AST2500)  */
+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
+
 /* I2C Register */
 #define ASPEED_I2C_FUN_CTRL_REG				0x00
 #define ASPEED_I2C_AC_TIMING_REG1			0x04
@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	struct resource *res;
 	int irq, ret;
 
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "aspeed,ast2600-i2c-bus")) {
+		u32 global_ctrl;
+		struct regmap *gr_regmap;
+
+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
+
+		if (IS_ERR(gr_regmap)) {
+			ret = PTR_ERR(gr_regmap);
+		} else {
+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
+			if (global_ctrl & BIT(2))
+				return -EIO;
+		}
+	}
+
 	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
 	if (!bus)
 		return -ENOMEM;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 2/3] ARM: dts: aspeed: Add node for AST2600 I2C
  2021-05-19  8:04 ` Jamin Lin
  (?)
@ 2021-05-19  8:04   ` Jamin Lin
  -1 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-19  8:04 UTC (permalink / raw)
  To: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER
  Cc: ryan_chen, chiawei_wang, troy_lee, steven_lee, jamin_lin

Add node to get the global register of i2c for AST2600.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-g6.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
index f96607b7b4e2..998d55a16c5c 100644
--- a/arch/arm/boot/dts/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6.dtsi
@@ -674,6 +674,14 @@
 #include "aspeed-g6-pinctrl.dtsi"
 
 &i2c {
+
+	i2c_gr: i2c-global-regs@0 {
+		compatible = "aspeed,ast2600-i2c-global", "syscon";
+		reg = <0x0 0x20>;
+		clocks = <&syscon ASPEED_CLK_APB2>;
+		resets = <&syscon ASPEED_RESET_I2C>;
+	};
+
 	i2c0: i2c-bus@80 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.17.1


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

* [PATCH 2/3] ARM: dts: aspeed: Add node for AST2600 I2C
@ 2021-05-19  8:04   ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-19  8:04 UTC (permalink / raw)
  To: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER
  Cc: steven_lee, chiawei_wang, troy_lee, ryan_chen, jamin_lin

Add node to get the global register of i2c for AST2600.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-g6.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
index f96607b7b4e2..998d55a16c5c 100644
--- a/arch/arm/boot/dts/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6.dtsi
@@ -674,6 +674,14 @@
 #include "aspeed-g6-pinctrl.dtsi"
 
 &i2c {
+
+	i2c_gr: i2c-global-regs@0 {
+		compatible = "aspeed,ast2600-i2c-global", "syscon";
+		reg = <0x0 0x20>;
+		clocks = <&syscon ASPEED_CLK_APB2>;
+		resets = <&syscon ASPEED_RESET_I2C>;
+	};
+
 	i2c0: i2c-bus@80 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.17.1


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

* [PATCH 2/3] ARM: dts: aspeed: Add node for AST2600 I2C
@ 2021-05-19  8:04   ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-19  8:04 UTC (permalink / raw)
  To: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER
  Cc: ryan_chen, chiawei_wang, troy_lee, steven_lee, jamin_lin

Add node to get the global register of i2c for AST2600.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-g6.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
index f96607b7b4e2..998d55a16c5c 100644
--- a/arch/arm/boot/dts/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6.dtsi
@@ -674,6 +674,14 @@
 #include "aspeed-g6-pinctrl.dtsi"
 
 &i2c {
+
+	i2c_gr: i2c-global-regs@0 {
+		compatible = "aspeed,ast2600-i2c-global", "syscon";
+		reg = <0x0 0x20>;
+		clocks = <&syscon ASPEED_CLK_APB2>;
+		resets = <&syscon ASPEED_RESET_I2C>;
+	};
+
 	i2c0: i2c-bus@80 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 3/3] dt-bindings: aspeed-i2c: Convert txt to yaml format
  2021-05-19  8:04 ` Jamin Lin
  (?)
@ 2021-05-19  8:04   ` Jamin Lin
  -1 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-19  8:04 UTC (permalink / raw)
  To: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER
  Cc: ryan_chen, chiawei_wang, troy_lee, steven_lee, jamin_lin

Add global-reg node for AST2600. Document the properties for
"aspeed,ast2600-i2c-global" compatible node.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
 .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
 2 files changed, 89 insertions(+), 49 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
 delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt

diff --git a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
new file mode 100644
index 000000000000..f469487935bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$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 Device Tree Bindings
+
+maintainers:
+  - Rayn Chen <rayn_chen@aspeedtech.com>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - aspeed,ast2400-i2c-bus
+      - aspeed,ast2500-i2c-bus
+      - aspeed,ast2600-i2c-bus
+      - aspeed,ast2600-i2c-global syscon
+
+  "#size-cells":
+    const: 0
+
+  "#address-cells":
+    const: 1
+
+  reg:
+    minItems: 1
+    maxItems: 2
+    items:
+      - description: address offset and range of bus
+      - description: address offset and range of bus buffer
+
+  interrupts:
+    description: interrupt number
+
+  clocks:
+    description:
+      root clock of bus, should reference the APB
+      clock in the second cell
+
+  reset:
+    description: phandle to reset controller with the reset number in
+      the second cell
+
+  bus-frequency:
+    minimum: 10000
+    maximum: 3400000
+    default: 100000
+    description: frequency of the bus clock in Hz defaults to 100 kHz when not
+      specified
+
+  multi-master:
+    maxItems: 1
+    description:
+      states that there is another master active on this bus
+
+required:
+  - reg
+  - compatible
+  - clocks
+  - resets
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/ast2600-clock.h>
+    i2c_gr: i2c-global-regs@0 {
+      compatible = "aspeed,ast2600-i2c-global", "syscon";
+      reg = <0x0 0x20>;
+      clocks = <&syscon ASPEED_CLK_APB2>;
+      resets = <&syscon ASPEED_RESET_I2C>;
+    };
+
+    i2c0: i2c-bus@80 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      #interrupt-cells = <1>;
+      reg = <0x80 0x80>;
+      compatible = "aspeed,ast2600-i2c-bus";
+      clocks = <&syscon ASPEED_CLK_APB2>;
+      resets = <&syscon ASPEED_RESET_I2C>;
+      interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+      bus-frequency = <100000>;
+    };
diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
deleted file mode 100644
index b47f6ccb196a..000000000000
--- a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
+++ /dev/null
@@ -1,49 +0,0 @@
-Device tree configuration for the I2C busses on the AST24XX, AST25XX, and AST26XX SoCs.
-
-Required Properties:
-- #address-cells	: should be 1
-- #size-cells		: should be 0
-- reg			: address offset and range of bus
-- compatible		: should be "aspeed,ast2400-i2c-bus"
-			  or "aspeed,ast2500-i2c-bus"
-			  or "aspeed,ast2600-i2c-bus"
-- clocks		: root clock of bus, should reference the APB
-			  clock in the second cell
-- resets		: phandle to reset controller with the reset number in
-			  the second cell
-- interrupts		: interrupt number
-
-Optional Properties:
-- bus-frequency	: frequency of the bus clock in Hz defaults to 100 kHz when not
-		  specified
-- multi-master	: states that there is another master active on this bus.
-
-Example:
-
-i2c {
-	compatible = "simple-bus";
-	#address-cells = <1>;
-	#size-cells = <1>;
-	ranges = <0 0x1e78a000 0x1000>;
-
-	i2c_ic: interrupt-controller@0 {
-		#interrupt-cells = <1>;
-		compatible = "aspeed,ast2400-i2c-ic";
-		reg = <0x0 0x40>;
-		interrupts = <12>;
-		interrupt-controller;
-	};
-
-	i2c0: i2c-bus@40 {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		#interrupt-cells = <1>;
-		reg = <0x40 0x40>;
-		compatible = "aspeed,ast2400-i2c-bus";
-		clocks = <&syscon ASPEED_CLK_APB>;
-		resets = <&syscon ASPEED_RESET_I2C>;
-		bus-frequency = <100000>;
-		interrupts = <0>;
-		interrupt-parent = <&i2c_ic>;
-	};
-};
-- 
2.17.1


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

* [PATCH 3/3] dt-bindings: aspeed-i2c: Convert txt to yaml format
@ 2021-05-19  8:04   ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-19  8:04 UTC (permalink / raw)
  To: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER
  Cc: steven_lee, chiawei_wang, troy_lee, ryan_chen, jamin_lin

Add global-reg node for AST2600. Document the properties for
"aspeed,ast2600-i2c-global" compatible node.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
 .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
 2 files changed, 89 insertions(+), 49 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
 delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt

diff --git a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
new file mode 100644
index 000000000000..f469487935bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$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 Device Tree Bindings
+
+maintainers:
+  - Rayn Chen <rayn_chen@aspeedtech.com>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - aspeed,ast2400-i2c-bus
+      - aspeed,ast2500-i2c-bus
+      - aspeed,ast2600-i2c-bus
+      - aspeed,ast2600-i2c-global syscon
+
+  "#size-cells":
+    const: 0
+
+  "#address-cells":
+    const: 1
+
+  reg:
+    minItems: 1
+    maxItems: 2
+    items:
+      - description: address offset and range of bus
+      - description: address offset and range of bus buffer
+
+  interrupts:
+    description: interrupt number
+
+  clocks:
+    description:
+      root clock of bus, should reference the APB
+      clock in the second cell
+
+  reset:
+    description: phandle to reset controller with the reset number in
+      the second cell
+
+  bus-frequency:
+    minimum: 10000
+    maximum: 3400000
+    default: 100000
+    description: frequency of the bus clock in Hz defaults to 100 kHz when not
+      specified
+
+  multi-master:
+    maxItems: 1
+    description:
+      states that there is another master active on this bus
+
+required:
+  - reg
+  - compatible
+  - clocks
+  - resets
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/ast2600-clock.h>
+    i2c_gr: i2c-global-regs@0 {
+      compatible = "aspeed,ast2600-i2c-global", "syscon";
+      reg = <0x0 0x20>;
+      clocks = <&syscon ASPEED_CLK_APB2>;
+      resets = <&syscon ASPEED_RESET_I2C>;
+    };
+
+    i2c0: i2c-bus@80 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      #interrupt-cells = <1>;
+      reg = <0x80 0x80>;
+      compatible = "aspeed,ast2600-i2c-bus";
+      clocks = <&syscon ASPEED_CLK_APB2>;
+      resets = <&syscon ASPEED_RESET_I2C>;
+      interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+      bus-frequency = <100000>;
+    };
diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
deleted file mode 100644
index b47f6ccb196a..000000000000
--- a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
+++ /dev/null
@@ -1,49 +0,0 @@
-Device tree configuration for the I2C busses on the AST24XX, AST25XX, and AST26XX SoCs.
-
-Required Properties:
-- #address-cells	: should be 1
-- #size-cells		: should be 0
-- reg			: address offset and range of bus
-- compatible		: should be "aspeed,ast2400-i2c-bus"
-			  or "aspeed,ast2500-i2c-bus"
-			  or "aspeed,ast2600-i2c-bus"
-- clocks		: root clock of bus, should reference the APB
-			  clock in the second cell
-- resets		: phandle to reset controller with the reset number in
-			  the second cell
-- interrupts		: interrupt number
-
-Optional Properties:
-- bus-frequency	: frequency of the bus clock in Hz defaults to 100 kHz when not
-		  specified
-- multi-master	: states that there is another master active on this bus.
-
-Example:
-
-i2c {
-	compatible = "simple-bus";
-	#address-cells = <1>;
-	#size-cells = <1>;
-	ranges = <0 0x1e78a000 0x1000>;
-
-	i2c_ic: interrupt-controller@0 {
-		#interrupt-cells = <1>;
-		compatible = "aspeed,ast2400-i2c-ic";
-		reg = <0x0 0x40>;
-		interrupts = <12>;
-		interrupt-controller;
-	};
-
-	i2c0: i2c-bus@40 {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		#interrupt-cells = <1>;
-		reg = <0x40 0x40>;
-		compatible = "aspeed,ast2400-i2c-bus";
-		clocks = <&syscon ASPEED_CLK_APB>;
-		resets = <&syscon ASPEED_RESET_I2C>;
-		bus-frequency = <100000>;
-		interrupts = <0>;
-		interrupt-parent = <&i2c_ic>;
-	};
-};
-- 
2.17.1


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

* [PATCH 3/3] dt-bindings: aspeed-i2c: Convert txt to yaml format
@ 2021-05-19  8:04   ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-19  8:04 UTC (permalink / raw)
  To: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER
  Cc: ryan_chen, chiawei_wang, troy_lee, steven_lee, jamin_lin

Add global-reg node for AST2600. Document the properties for
"aspeed,ast2600-i2c-global" compatible node.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
 .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
 2 files changed, 89 insertions(+), 49 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
 delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt

diff --git a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
new file mode 100644
index 000000000000..f469487935bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$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 Device Tree Bindings
+
+maintainers:
+  - Rayn Chen <rayn_chen@aspeedtech.com>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - aspeed,ast2400-i2c-bus
+      - aspeed,ast2500-i2c-bus
+      - aspeed,ast2600-i2c-bus
+      - aspeed,ast2600-i2c-global syscon
+
+  "#size-cells":
+    const: 0
+
+  "#address-cells":
+    const: 1
+
+  reg:
+    minItems: 1
+    maxItems: 2
+    items:
+      - description: address offset and range of bus
+      - description: address offset and range of bus buffer
+
+  interrupts:
+    description: interrupt number
+
+  clocks:
+    description:
+      root clock of bus, should reference the APB
+      clock in the second cell
+
+  reset:
+    description: phandle to reset controller with the reset number in
+      the second cell
+
+  bus-frequency:
+    minimum: 10000
+    maximum: 3400000
+    default: 100000
+    description: frequency of the bus clock in Hz defaults to 100 kHz when not
+      specified
+
+  multi-master:
+    maxItems: 1
+    description:
+      states that there is another master active on this bus
+
+required:
+  - reg
+  - compatible
+  - clocks
+  - resets
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/ast2600-clock.h>
+    i2c_gr: i2c-global-regs@0 {
+      compatible = "aspeed,ast2600-i2c-global", "syscon";
+      reg = <0x0 0x20>;
+      clocks = <&syscon ASPEED_CLK_APB2>;
+      resets = <&syscon ASPEED_RESET_I2C>;
+    };
+
+    i2c0: i2c-bus@80 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      #interrupt-cells = <1>;
+      reg = <0x80 0x80>;
+      compatible = "aspeed,ast2600-i2c-bus";
+      clocks = <&syscon ASPEED_CLK_APB2>;
+      resets = <&syscon ASPEED_RESET_I2C>;
+      interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+      bus-frequency = <100000>;
+    };
diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
deleted file mode 100644
index b47f6ccb196a..000000000000
--- a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
+++ /dev/null
@@ -1,49 +0,0 @@
-Device tree configuration for the I2C busses on the AST24XX, AST25XX, and AST26XX SoCs.
-
-Required Properties:
-- #address-cells	: should be 1
-- #size-cells		: should be 0
-- reg			: address offset and range of bus
-- compatible		: should be "aspeed,ast2400-i2c-bus"
-			  or "aspeed,ast2500-i2c-bus"
-			  or "aspeed,ast2600-i2c-bus"
-- clocks		: root clock of bus, should reference the APB
-			  clock in the second cell
-- resets		: phandle to reset controller with the reset number in
-			  the second cell
-- interrupts		: interrupt number
-
-Optional Properties:
-- bus-frequency	: frequency of the bus clock in Hz defaults to 100 kHz when not
-		  specified
-- multi-master	: states that there is another master active on this bus.
-
-Example:
-
-i2c {
-	compatible = "simple-bus";
-	#address-cells = <1>;
-	#size-cells = <1>;
-	ranges = <0 0x1e78a000 0x1000>;
-
-	i2c_ic: interrupt-controller@0 {
-		#interrupt-cells = <1>;
-		compatible = "aspeed,ast2400-i2c-ic";
-		reg = <0x0 0x40>;
-		interrupts = <12>;
-		interrupt-controller;
-	};
-
-	i2c0: i2c-bus@40 {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		#interrupt-cells = <1>;
-		reg = <0x40 0x40>;
-		compatible = "aspeed,ast2400-i2c-bus";
-		clocks = <&syscon ASPEED_CLK_APB>;
-		resets = <&syscon ASPEED_RESET_I2C>;
-		bus-frequency = <100000>;
-		interrupts = <0>;
-		interrupt-parent = <&i2c_ic>;
-	};
-};
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 3/3] dt-bindings: aspeed-i2c: Convert txt to yaml format
  2021-05-19  8:04   ` Jamin Lin
  (?)
@ 2021-05-19 15:29     ` Rob Herring
  -1 siblings, 0 replies; 51+ messages in thread
From: Rob Herring @ 2021-05-19 15:29 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Brendan Higgins, steven_lee, Rayn Chen, Benjamin Herrenschmidt,
	Andrew Jeffery, moderated list:ARM/ASPEED MACHINE SUPPORT,
	open list:I2C SUBSYSTEM HOST DRIVERS, chiawei_wang, Joel Stanley,
	ryan_chen, Rob Herring, moderated list:ARM/ASPEED I2C DRIVER,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list, troy_lee,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Wed, 19 May 2021 16:04:29 +0800, Jamin Lin wrote:
> Add global-reg node for AST2600. Document the properties for
> "aspeed,ast2600-i2c-global" compatible node.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>  .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
>  .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
>  2 files changed, 89 insertions(+), 49 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
>  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/i2c/aspeed,i2c.example.dt.yaml:0:0: /example-0/i2c-global-regs@0: failed to match any schema with compatible: ['aspeed,ast2600-i2c-global', 'syscon']

See https://patchwork.ozlabs.org/patch/1480769

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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

pip3 install dtschema --upgrade

Please check and re-submit.


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

* Re: [PATCH 3/3] dt-bindings: aspeed-i2c: Convert txt to yaml format
@ 2021-05-19 15:29     ` Rob Herring
  0 siblings, 0 replies; 51+ messages in thread
From: Rob Herring @ 2021-05-19 15:29 UTC (permalink / raw)
  To: Jamin Lin
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	ryan_chen, moderated list:ARM/ASPEED MACHINE SUPPORT,
	Andrew Jeffery, Brendan Higgins, chiawei_wang, troy_lee,
	steven_lee, open list, Rob Herring, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	moderated list:ARM/ASPEED I2C DRIVER,
	moderated list:ARM/ASPEED MACHINE SUPPORT

On Wed, 19 May 2021 16:04:29 +0800, Jamin Lin wrote:
> Add global-reg node for AST2600. Document the properties for
> "aspeed,ast2600-i2c-global" compatible node.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>  .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
>  .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
>  2 files changed, 89 insertions(+), 49 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
>  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/i2c/aspeed,i2c.example.dt.yaml:0:0: /example-0/i2c-global-regs@0: failed to match any schema with compatible: ['aspeed,ast2600-i2c-global', 'syscon']

See https://patchwork.ozlabs.org/patch/1480769

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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

pip3 install dtschema --upgrade

Please check and re-submit.


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

* Re: [PATCH 3/3] dt-bindings: aspeed-i2c: Convert txt to yaml format
@ 2021-05-19 15:29     ` Rob Herring
  0 siblings, 0 replies; 51+ messages in thread
From: Rob Herring @ 2021-05-19 15:29 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Brendan Higgins, steven_lee, Rayn Chen, Benjamin Herrenschmidt,
	Andrew Jeffery, moderated list:ARM/ASPEED MACHINE SUPPORT,
	open list:I2C SUBSYSTEM HOST DRIVERS, chiawei_wang, Joel Stanley,
	ryan_chen, Rob Herring, moderated list:ARM/ASPEED I2C DRIVER,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list, troy_lee,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Wed, 19 May 2021 16:04:29 +0800, Jamin Lin wrote:
> Add global-reg node for AST2600. Document the properties for
> "aspeed,ast2600-i2c-global" compatible node.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>  .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
>  .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
>  2 files changed, 89 insertions(+), 49 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
>  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/i2c/aspeed,i2c.example.dt.yaml:0:0: /example-0/i2c-global-regs@0: failed to match any schema with compatible: ['aspeed,ast2600-i2c-global', 'syscon']

See https://patchwork.ozlabs.org/patch/1480769

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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

pip3 install dtschema --upgrade

Please check and re-submit.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 3/3] dt-bindings: aspeed-i2c: Convert txt to yaml format
  2021-05-19  8:04   ` Jamin Lin
  (?)
@ 2021-05-19 18:28     ` Rob Herring
  -1 siblings, 0 replies; 51+ messages in thread
From: Rob Herring @ 2021-05-19 18:28 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, ryan_chen, chiawei_wang,
	troy_lee, steven_lee

On Wed, May 19, 2021 at 04:04:29PM +0800, Jamin Lin wrote:
> Add global-reg node for AST2600. Document the properties for
> "aspeed,ast2600-i2c-global" compatible node.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>  .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
>  .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
>  2 files changed, 89 insertions(+), 49 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
>  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> new file mode 100644
> index 000000000000..f469487935bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$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 Device Tree Bindings
> +
> +maintainers:
> +  - Rayn Chen <rayn_chen@aspeedtech.com>
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - aspeed,ast2400-i2c-bus
> +      - aspeed,ast2500-i2c-bus
> +      - aspeed,ast2600-i2c-bus
> +      - aspeed,ast2600-i2c-global syscon

"aspeed,ast2600-i2c-global syscon" is not a valid compatible value.

What makes this a syscon?

> +
> +  "#size-cells":
> +    const: 0
> +
> +  "#address-cells":
> +    const: 1
> +
> +  reg:
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      - description: address offset and range of bus
> +      - description: address offset and range of bus buffer
> +
> +  interrupts:
> +    description: interrupt number
> +
> +  clocks:
> +    description:
> +      root clock of bus, should reference the APB
> +      clock in the second cell

How many clocks? Needs maxItems or items.

> +
> +  reset:
> +    description: phandle to reset controller with the reset number in
> +      the second cell

How many?

> +
> +  bus-frequency:
> +    minimum: 10000
> +    maximum: 3400000
> +    default: 100000
> +    description: frequency of the bus clock in Hz defaults to 100 kHz when not
> +      specified
> +
> +  multi-master:
> +    maxItems: 1

multi-master is an array?

> +    description:
> +      states that there is another master active on this bus
> +
> +required:
> +  - reg
> +  - compatible
> +  - clocks
> +  - resets
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/ast2600-clock.h>
> +    i2c_gr: i2c-global-regs@0 {
> +      compatible = "aspeed,ast2600-i2c-global", "syscon";
> +      reg = <0x0 0x20>;
> +      clocks = <&syscon ASPEED_CLK_APB2>;
> +      resets = <&syscon ASPEED_RESET_I2C>;
> +    };
> +
> +    i2c0: i2c-bus@80 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      #interrupt-cells = <1>;
> +      reg = <0x80 0x80>;
> +      compatible = "aspeed,ast2600-i2c-bus";
> +      clocks = <&syscon ASPEED_CLK_APB2>;
> +      resets = <&syscon ASPEED_RESET_I2C>;
> +      interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
> +      bus-frequency = <100000>;
> +    };
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> deleted file mode 100644
> index b47f6ccb196a..000000000000
> --- a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -Device tree configuration for the I2C busses on the AST24XX, AST25XX, and AST26XX SoCs.
> -
> -Required Properties:
> -- #address-cells	: should be 1
> -- #size-cells		: should be 0
> -- reg			: address offset and range of bus
> -- compatible		: should be "aspeed,ast2400-i2c-bus"
> -			  or "aspeed,ast2500-i2c-bus"
> -			  or "aspeed,ast2600-i2c-bus"
> -- clocks		: root clock of bus, should reference the APB
> -			  clock in the second cell
> -- resets		: phandle to reset controller with the reset number in
> -			  the second cell
> -- interrupts		: interrupt number
> -
> -Optional Properties:
> -- bus-frequency	: frequency of the bus clock in Hz defaults to 100 kHz when not
> -		  specified
> -- multi-master	: states that there is another master active on this bus.
> -
> -Example:
> -
> -i2c {
> -	compatible = "simple-bus";
> -	#address-cells = <1>;
> -	#size-cells = <1>;
> -	ranges = <0 0x1e78a000 0x1000>;
> -
> -	i2c_ic: interrupt-controller@0 {
> -		#interrupt-cells = <1>;
> -		compatible = "aspeed,ast2400-i2c-ic";
> -		reg = <0x0 0x40>;
> -		interrupts = <12>;
> -		interrupt-controller;
> -	};
> -
> -	i2c0: i2c-bus@40 {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		#interrupt-cells = <1>;
> -		reg = <0x40 0x40>;
> -		compatible = "aspeed,ast2400-i2c-bus";
> -		clocks = <&syscon ASPEED_CLK_APB>;
> -		resets = <&syscon ASPEED_RESET_I2C>;
> -		bus-frequency = <100000>;
> -		interrupts = <0>;
> -		interrupt-parent = <&i2c_ic>;
> -	};
> -};
> -- 
> 2.17.1
> 

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

* Re: [PATCH 3/3] dt-bindings: aspeed-i2c: Convert txt to yaml format
@ 2021-05-19 18:28     ` Rob Herring
  0 siblings, 0 replies; 51+ messages in thread
From: Rob Herring @ 2021-05-19 18:28 UTC (permalink / raw)
  To: Jamin Lin
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	ryan_chen, moderated list:ARM/ASPEED MACHINE SUPPORT,
	Andrew Jeffery, moderated list:ARM/ASPEED I2C DRIVER, troy_lee,
	Brendan Higgins, open list, Rayn Chen, steven_lee, chiawei_wang,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	open list:I2C SUBSYSTEM HOST DRIVERS

On Wed, May 19, 2021 at 04:04:29PM +0800, Jamin Lin wrote:
> Add global-reg node for AST2600. Document the properties for
> "aspeed,ast2600-i2c-global" compatible node.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>  .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
>  .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
>  2 files changed, 89 insertions(+), 49 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
>  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> new file mode 100644
> index 000000000000..f469487935bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$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 Device Tree Bindings
> +
> +maintainers:
> +  - Rayn Chen <rayn_chen@aspeedtech.com>
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - aspeed,ast2400-i2c-bus
> +      - aspeed,ast2500-i2c-bus
> +      - aspeed,ast2600-i2c-bus
> +      - aspeed,ast2600-i2c-global syscon

"aspeed,ast2600-i2c-global syscon" is not a valid compatible value.

What makes this a syscon?

> +
> +  "#size-cells":
> +    const: 0
> +
> +  "#address-cells":
> +    const: 1
> +
> +  reg:
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      - description: address offset and range of bus
> +      - description: address offset and range of bus buffer
> +
> +  interrupts:
> +    description: interrupt number
> +
> +  clocks:
> +    description:
> +      root clock of bus, should reference the APB
> +      clock in the second cell

How many clocks? Needs maxItems or items.

> +
> +  reset:
> +    description: phandle to reset controller with the reset number in
> +      the second cell

How many?

> +
> +  bus-frequency:
> +    minimum: 10000
> +    maximum: 3400000
> +    default: 100000
> +    description: frequency of the bus clock in Hz defaults to 100 kHz when not
> +      specified
> +
> +  multi-master:
> +    maxItems: 1

multi-master is an array?

> +    description:
> +      states that there is another master active on this bus
> +
> +required:
> +  - reg
> +  - compatible
> +  - clocks
> +  - resets
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/ast2600-clock.h>
> +    i2c_gr: i2c-global-regs@0 {
> +      compatible = "aspeed,ast2600-i2c-global", "syscon";
> +      reg = <0x0 0x20>;
> +      clocks = <&syscon ASPEED_CLK_APB2>;
> +      resets = <&syscon ASPEED_RESET_I2C>;
> +    };
> +
> +    i2c0: i2c-bus@80 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      #interrupt-cells = <1>;
> +      reg = <0x80 0x80>;
> +      compatible = "aspeed,ast2600-i2c-bus";
> +      clocks = <&syscon ASPEED_CLK_APB2>;
> +      resets = <&syscon ASPEED_RESET_I2C>;
> +      interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
> +      bus-frequency = <100000>;
> +    };
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> deleted file mode 100644
> index b47f6ccb196a..000000000000
> --- a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -Device tree configuration for the I2C busses on the AST24XX, AST25XX, and AST26XX SoCs.
> -
> -Required Properties:
> -- #address-cells	: should be 1
> -- #size-cells		: should be 0
> -- reg			: address offset and range of bus
> -- compatible		: should be "aspeed,ast2400-i2c-bus"
> -			  or "aspeed,ast2500-i2c-bus"
> -			  or "aspeed,ast2600-i2c-bus"
> -- clocks		: root clock of bus, should reference the APB
> -			  clock in the second cell
> -- resets		: phandle to reset controller with the reset number in
> -			  the second cell
> -- interrupts		: interrupt number
> -
> -Optional Properties:
> -- bus-frequency	: frequency of the bus clock in Hz defaults to 100 kHz when not
> -		  specified
> -- multi-master	: states that there is another master active on this bus.
> -
> -Example:
> -
> -i2c {
> -	compatible = "simple-bus";
> -	#address-cells = <1>;
> -	#size-cells = <1>;
> -	ranges = <0 0x1e78a000 0x1000>;
> -
> -	i2c_ic: interrupt-controller@0 {
> -		#interrupt-cells = <1>;
> -		compatible = "aspeed,ast2400-i2c-ic";
> -		reg = <0x0 0x40>;
> -		interrupts = <12>;
> -		interrupt-controller;
> -	};
> -
> -	i2c0: i2c-bus@40 {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		#interrupt-cells = <1>;
> -		reg = <0x40 0x40>;
> -		compatible = "aspeed,ast2400-i2c-bus";
> -		clocks = <&syscon ASPEED_CLK_APB>;
> -		resets = <&syscon ASPEED_RESET_I2C>;
> -		bus-frequency = <100000>;
> -		interrupts = <0>;
> -		interrupt-parent = <&i2c_ic>;
> -	};
> -};
> -- 
> 2.17.1
> 

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

* Re: [PATCH 3/3] dt-bindings: aspeed-i2c: Convert txt to yaml format
@ 2021-05-19 18:28     ` Rob Herring
  0 siblings, 0 replies; 51+ messages in thread
From: Rob Herring @ 2021-05-19 18:28 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, ryan_chen, chiawei_wang,
	troy_lee, steven_lee

On Wed, May 19, 2021 at 04:04:29PM +0800, Jamin Lin wrote:
> Add global-reg node for AST2600. Document the properties for
> "aspeed,ast2600-i2c-global" compatible node.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>  .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
>  .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
>  2 files changed, 89 insertions(+), 49 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
>  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> new file mode 100644
> index 000000000000..f469487935bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$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 Device Tree Bindings
> +
> +maintainers:
> +  - Rayn Chen <rayn_chen@aspeedtech.com>
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - aspeed,ast2400-i2c-bus
> +      - aspeed,ast2500-i2c-bus
> +      - aspeed,ast2600-i2c-bus
> +      - aspeed,ast2600-i2c-global syscon

"aspeed,ast2600-i2c-global syscon" is not a valid compatible value.

What makes this a syscon?

> +
> +  "#size-cells":
> +    const: 0
> +
> +  "#address-cells":
> +    const: 1
> +
> +  reg:
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      - description: address offset and range of bus
> +      - description: address offset and range of bus buffer
> +
> +  interrupts:
> +    description: interrupt number
> +
> +  clocks:
> +    description:
> +      root clock of bus, should reference the APB
> +      clock in the second cell

How many clocks? Needs maxItems or items.

> +
> +  reset:
> +    description: phandle to reset controller with the reset number in
> +      the second cell

How many?

> +
> +  bus-frequency:
> +    minimum: 10000
> +    maximum: 3400000
> +    default: 100000
> +    description: frequency of the bus clock in Hz defaults to 100 kHz when not
> +      specified
> +
> +  multi-master:
> +    maxItems: 1

multi-master is an array?

> +    description:
> +      states that there is another master active on this bus
> +
> +required:
> +  - reg
> +  - compatible
> +  - clocks
> +  - resets
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/ast2600-clock.h>
> +    i2c_gr: i2c-global-regs@0 {
> +      compatible = "aspeed,ast2600-i2c-global", "syscon";
> +      reg = <0x0 0x20>;
> +      clocks = <&syscon ASPEED_CLK_APB2>;
> +      resets = <&syscon ASPEED_RESET_I2C>;
> +    };
> +
> +    i2c0: i2c-bus@80 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      #interrupt-cells = <1>;
> +      reg = <0x80 0x80>;
> +      compatible = "aspeed,ast2600-i2c-bus";
> +      clocks = <&syscon ASPEED_CLK_APB2>;
> +      resets = <&syscon ASPEED_RESET_I2C>;
> +      interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
> +      bus-frequency = <100000>;
> +    };
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> deleted file mode 100644
> index b47f6ccb196a..000000000000
> --- a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -Device tree configuration for the I2C busses on the AST24XX, AST25XX, and AST26XX SoCs.
> -
> -Required Properties:
> -- #address-cells	: should be 1
> -- #size-cells		: should be 0
> -- reg			: address offset and range of bus
> -- compatible		: should be "aspeed,ast2400-i2c-bus"
> -			  or "aspeed,ast2500-i2c-bus"
> -			  or "aspeed,ast2600-i2c-bus"
> -- clocks		: root clock of bus, should reference the APB
> -			  clock in the second cell
> -- resets		: phandle to reset controller with the reset number in
> -			  the second cell
> -- interrupts		: interrupt number
> -
> -Optional Properties:
> -- bus-frequency	: frequency of the bus clock in Hz defaults to 100 kHz when not
> -		  specified
> -- multi-master	: states that there is another master active on this bus.
> -
> -Example:
> -
> -i2c {
> -	compatible = "simple-bus";
> -	#address-cells = <1>;
> -	#size-cells = <1>;
> -	ranges = <0 0x1e78a000 0x1000>;
> -
> -	i2c_ic: interrupt-controller@0 {
> -		#interrupt-cells = <1>;
> -		compatible = "aspeed,ast2400-i2c-ic";
> -		reg = <0x0 0x40>;
> -		interrupts = <12>;
> -		interrupt-controller;
> -	};
> -
> -	i2c0: i2c-bus@40 {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		#interrupt-cells = <1>;
> -		reg = <0x40 0x40>;
> -		compatible = "aspeed,ast2400-i2c-bus";
> -		clocks = <&syscon ASPEED_CLK_APB>;
> -		resets = <&syscon ASPEED_RESET_I2C>;
> -		bus-frequency = <100000>;
> -		interrupts = <0>;
> -		interrupt-parent = <&i2c_ic>;
> -	};
> -};
> -- 
> 2.17.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
  2021-05-19  8:04   ` Jamin Lin
  (?)
@ 2021-05-19 19:02     ` Zev Weiss
  -1 siblings, 0 replies; 51+ messages in thread
From: Zev Weiss @ 2021-05-19 19:02 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, steven_lee, chiawei_wang,
	troy_lee, ryan_chen

On Wed, May 19, 2021 at 03:04:27AM CDT, Jamin Lin wrote:
>The register definition between AST2600 A2 and A3 is different.
>This patch avoid new registers definition of AST2600 to use
>this driver. We will submit the path for the new registers
>definition of AST2600.
>
>Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>---
> drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
>diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
>index 724bf30600d6..007309077d9f 100644
>--- a/drivers/i2c/busses/i2c-aspeed.c
>+++ b/drivers/i2c/busses/i2c-aspeed.c
>@@ -19,14 +19,20 @@
> #include <linux/irqchip/chained_irq.h>
> #include <linux/irqdomain.h>
> #include <linux/kernel.h>
>+#include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of_address.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
>+#include <linux/regmap.h>
> #include <linux/reset.h>
> #include <linux/slab.h>
>
>+/* I2C Global Registers */
>+/* 0x0c : I2CG Global Control Register (AST2500)  */
>+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
>+
> /* I2C Register */
> #define ASPEED_I2C_FUN_CTRL_REG				0x00
> #define ASPEED_I2C_AC_TIMING_REG1			0x04
>@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> 	struct resource *res;
> 	int irq, ret;
>
>+	if (of_device_is_compatible(pdev->dev.of_node,
>+				    "aspeed,ast2600-i2c-bus")) {
>+		u32 global_ctrl;
>+		struct regmap *gr_regmap;
>+
>+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
>+
>+		if (IS_ERR(gr_regmap)) {
>+			ret = PTR_ERR(gr_regmap);
>+		} else {
>+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
>+			if (global_ctrl & BIT(2))
>+				return -EIO;

A macro definition might be a bit nicer than a raw BIT(2) here I'd
think.

Also, it seems a bit unfortunate to just bail on the device entirely if
we find this bit set (seems like a good way for a bootloader to
inadvertently DoS the kernel), though I guess poking global syscon bits
in the bus probe function might not be ideal.  Could/should we consider
some module-level init code to ensure that bit is cleared?


Zev

>+		}
>+	}
>+
> 	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> 	if (!bus)
> 		return -ENOMEM;
>-- 
>2.17.1
>

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-19 19:02     ` Zev Weiss
  0 siblings, 0 replies; 51+ messages in thread
From: Zev Weiss @ 2021-05-19 19:02 UTC (permalink / raw)
  To: Jamin Lin
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	ryan_chen, moderated list:ARM/ASPEED MACHINE SUPPORT,
	Andrew Jeffery, moderated list:ARM/ASPEED I2C DRIVER, troy_lee,
	Brendan Higgins, open list, Rob Herring, steven_lee,
	chiawei_wang, moderated list:ARM/ASPEED MACHINE SUPPORT,
	open list:I2C SUBSYSTEM HOST DRIVERS

On Wed, May 19, 2021 at 03:04:27AM CDT, Jamin Lin wrote:
>The register definition between AST2600 A2 and A3 is different.
>This patch avoid new registers definition of AST2600 to use
>this driver. We will submit the path for the new registers
>definition of AST2600.
>
>Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>---
> drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
>diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
>index 724bf30600d6..007309077d9f 100644
>--- a/drivers/i2c/busses/i2c-aspeed.c
>+++ b/drivers/i2c/busses/i2c-aspeed.c
>@@ -19,14 +19,20 @@
> #include <linux/irqchip/chained_irq.h>
> #include <linux/irqdomain.h>
> #include <linux/kernel.h>
>+#include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of_address.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
>+#include <linux/regmap.h>
> #include <linux/reset.h>
> #include <linux/slab.h>
>
>+/* I2C Global Registers */
>+/* 0x0c : I2CG Global Control Register (AST2500)  */
>+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
>+
> /* I2C Register */
> #define ASPEED_I2C_FUN_CTRL_REG				0x00
> #define ASPEED_I2C_AC_TIMING_REG1			0x04
>@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> 	struct resource *res;
> 	int irq, ret;
>
>+	if (of_device_is_compatible(pdev->dev.of_node,
>+				    "aspeed,ast2600-i2c-bus")) {
>+		u32 global_ctrl;
>+		struct regmap *gr_regmap;
>+
>+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
>+
>+		if (IS_ERR(gr_regmap)) {
>+			ret = PTR_ERR(gr_regmap);
>+		} else {
>+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
>+			if (global_ctrl & BIT(2))
>+				return -EIO;

A macro definition might be a bit nicer than a raw BIT(2) here I'd
think.

Also, it seems a bit unfortunate to just bail on the device entirely if
we find this bit set (seems like a good way for a bootloader to
inadvertently DoS the kernel), though I guess poking global syscon bits
in the bus probe function might not be ideal.  Could/should we consider
some module-level init code to ensure that bit is cleared?


Zev

>+		}
>+	}
>+
> 	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> 	if (!bus)
> 		return -ENOMEM;
>-- 
>2.17.1
>

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-19 19:02     ` Zev Weiss
  0 siblings, 0 replies; 51+ messages in thread
From: Zev Weiss @ 2021-05-19 19:02 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, steven_lee, chiawei_wang,
	troy_lee, ryan_chen

On Wed, May 19, 2021 at 03:04:27AM CDT, Jamin Lin wrote:
>The register definition between AST2600 A2 and A3 is different.
>This patch avoid new registers definition of AST2600 to use
>this driver. We will submit the path for the new registers
>definition of AST2600.
>
>Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>---
> drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
>diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
>index 724bf30600d6..007309077d9f 100644
>--- a/drivers/i2c/busses/i2c-aspeed.c
>+++ b/drivers/i2c/busses/i2c-aspeed.c
>@@ -19,14 +19,20 @@
> #include <linux/irqchip/chained_irq.h>
> #include <linux/irqdomain.h>
> #include <linux/kernel.h>
>+#include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of_address.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
>+#include <linux/regmap.h>
> #include <linux/reset.h>
> #include <linux/slab.h>
>
>+/* I2C Global Registers */
>+/* 0x0c : I2CG Global Control Register (AST2500)  */
>+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
>+
> /* I2C Register */
> #define ASPEED_I2C_FUN_CTRL_REG				0x00
> #define ASPEED_I2C_AC_TIMING_REG1			0x04
>@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> 	struct resource *res;
> 	int irq, ret;
>
>+	if (of_device_is_compatible(pdev->dev.of_node,
>+				    "aspeed,ast2600-i2c-bus")) {
>+		u32 global_ctrl;
>+		struct regmap *gr_regmap;
>+
>+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
>+
>+		if (IS_ERR(gr_regmap)) {
>+			ret = PTR_ERR(gr_regmap);
>+		} else {
>+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
>+			if (global_ctrl & BIT(2))
>+				return -EIO;

A macro definition might be a bit nicer than a raw BIT(2) here I'd
think.

Also, it seems a bit unfortunate to just bail on the device entirely if
we find this bit set (seems like a good way for a bootloader to
inadvertently DoS the kernel), though I guess poking global syscon bits
in the bus probe function might not be ideal.  Could/should we consider
some module-level init code to ensure that bit is cleared?


Zev

>+		}
>+	}
>+
> 	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> 	if (!bus)
> 		return -ENOMEM;
>-- 
>2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
  2021-05-19  8:04   ` Jamin Lin
  (?)
@ 2021-05-19 22:59     ` Joel Stanley
  -1 siblings, 0 replies; 51+ messages in thread
From: Joel Stanley @ 2021-05-19 22:59 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Rob Herring, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, Ryan Chen, Chia-Wei, Wang,
	Troy Lee, steven_lee

On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
>
> The register definition between AST2600 A2 and A3 is different.
> This patch avoid new registers definition of AST2600 to use
> this driver. We will submit the path for the new registers
> definition of AST2600.

The AST2600 v9 datasheet says that bit 2 selects between old and new
register sets, and that the old register set is the default.

Has the default changed for the A3?, and the datasheet is incorrect?

Does the A3 still support the old register set?

>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>  drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index 724bf30600d6..007309077d9f 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -19,14 +19,20 @@
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/irqdomain.h>
>  #include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/reset.h>
>  #include <linux/slab.h>
>
> +/* I2C Global Registers */
> +/* 0x0c : I2CG Global Control Register (AST2500)  */
> +#define ASPEED_I2CG_GLOBAL_CTRL_REG                    0x0c
> +
>  /* I2C Register */
>  #define ASPEED_I2C_FUN_CTRL_REG                                0x00
>  #define ASPEED_I2C_AC_TIMING_REG1                      0x04
> @@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>         struct resource *res;
>         int irq, ret;
>
> +       if (of_device_is_compatible(pdev->dev.of_node,
> +                                   "aspeed,ast2600-i2c-bus")) {
> +               u32 global_ctrl;
> +               struct regmap *gr_regmap;
> +
> +               gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
> +
> +               if (IS_ERR(gr_regmap)) {
> +                       ret = PTR_ERR(gr_regmap);
> +               } else {
> +                       regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
> +                       if (global_ctrl & BIT(2))
> +                               return -EIO;
> +               }
> +       }
> +
>         bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
>         if (!bus)
>                 return -ENOMEM;
> --
> 2.17.1
>

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-19 22:59     ` Joel Stanley
  0 siblings, 0 replies; 51+ messages in thread
From: Joel Stanley @ 2021-05-19 22:59 UTC (permalink / raw)
  To: Jamin Lin
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Ryan Chen, moderated list:ARM/ASPEED MACHINE SUPPORT,
	Andrew Jeffery, moderated list:ARM/ASPEED I2C DRIVER, Troy Lee,
	Brendan Higgins, open list, Rob Herring, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS, steven_lee, Chia-Wei, Wang,
	moderated list:ARM/ASPEED MACHINE SUPPORT

On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
>
> The register definition between AST2600 A2 and A3 is different.
> This patch avoid new registers definition of AST2600 to use
> this driver. We will submit the path for the new registers
> definition of AST2600.

The AST2600 v9 datasheet says that bit 2 selects between old and new
register sets, and that the old register set is the default.

Has the default changed for the A3?, and the datasheet is incorrect?

Does the A3 still support the old register set?

>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>  drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index 724bf30600d6..007309077d9f 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -19,14 +19,20 @@
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/irqdomain.h>
>  #include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/reset.h>
>  #include <linux/slab.h>
>
> +/* I2C Global Registers */
> +/* 0x0c : I2CG Global Control Register (AST2500)  */
> +#define ASPEED_I2CG_GLOBAL_CTRL_REG                    0x0c
> +
>  /* I2C Register */
>  #define ASPEED_I2C_FUN_CTRL_REG                                0x00
>  #define ASPEED_I2C_AC_TIMING_REG1                      0x04
> @@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>         struct resource *res;
>         int irq, ret;
>
> +       if (of_device_is_compatible(pdev->dev.of_node,
> +                                   "aspeed,ast2600-i2c-bus")) {
> +               u32 global_ctrl;
> +               struct regmap *gr_regmap;
> +
> +               gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
> +
> +               if (IS_ERR(gr_regmap)) {
> +                       ret = PTR_ERR(gr_regmap);
> +               } else {
> +                       regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
> +                       if (global_ctrl & BIT(2))
> +                               return -EIO;
> +               }
> +       }
> +
>         bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
>         if (!bus)
>                 return -ENOMEM;
> --
> 2.17.1
>

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-19 22:59     ` Joel Stanley
  0 siblings, 0 replies; 51+ messages in thread
From: Joel Stanley @ 2021-05-19 22:59 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Rob Herring, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, Ryan Chen, Chia-Wei, Wang,
	Troy Lee, steven_lee

On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
>
> The register definition between AST2600 A2 and A3 is different.
> This patch avoid new registers definition of AST2600 to use
> this driver. We will submit the path for the new registers
> definition of AST2600.

The AST2600 v9 datasheet says that bit 2 selects between old and new
register sets, and that the old register set is the default.

Has the default changed for the A3?, and the datasheet is incorrect?

Does the A3 still support the old register set?

>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>  drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index 724bf30600d6..007309077d9f 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -19,14 +19,20 @@
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/irqdomain.h>
>  #include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/reset.h>
>  #include <linux/slab.h>
>
> +/* I2C Global Registers */
> +/* 0x0c : I2CG Global Control Register (AST2500)  */
> +#define ASPEED_I2CG_GLOBAL_CTRL_REG                    0x0c
> +
>  /* I2C Register */
>  #define ASPEED_I2C_FUN_CTRL_REG                                0x00
>  #define ASPEED_I2C_AC_TIMING_REG1                      0x04
> @@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>         struct resource *res;
>         int irq, ret;
>
> +       if (of_device_is_compatible(pdev->dev.of_node,
> +                                   "aspeed,ast2600-i2c-bus")) {
> +               u32 global_ctrl;
> +               struct regmap *gr_regmap;
> +
> +               gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
> +
> +               if (IS_ERR(gr_regmap)) {
> +                       ret = PTR_ERR(gr_regmap);
> +               } else {
> +                       regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
> +                       if (global_ctrl & BIT(2))
> +                               return -EIO;
> +               }
> +       }
> +
>         bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
>         if (!bus)
>                 return -ENOMEM;
> --
> 2.17.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 3/3] dt-bindings: aspeed-i2c: Convert txt to yaml format
  2021-05-19 15:29     ` Rob Herring
  (?)
@ 2021-05-20  3:16       ` Jamin Lin
  -1 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-20  3:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: Brendan Higgins, Steven Lee, Rayn Chen, Benjamin Herrenschmidt,
	Andrew Jeffery, moderated list:ARM/ASPEED MACHINE SUPPORT,
	open list:I2C SUBSYSTEM HOST DRIVERS, ChiaWei Wang, Joel Stanley,
	Ryan Chen, Rob Herring, moderated list:ARM/ASPEED I2C DRIVER,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list, Troy Lee,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

The 05/19/2021 15:29, Rob Herring wrote:
> On Wed, 19 May 2021 16:04:29 +0800, Jamin Lin wrote:
> > Add global-reg node for AST2600. Document the properties for
> > "aspeed,ast2600-i2c-global" compatible node.
> > 
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >  .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
> >  .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
> >  2 files changed, 89 insertions(+), 49 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/i2c/aspeed,i2c.example.dt.yaml:0:0: /example-0/i2c-global-regs@0: failed to match any schema with compatible: ['aspeed,ast2600-i2c-global', 'syscon']
> 
> See https://patchwork.ozlabs.org/patch/1480769
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
>
Thanks for your review.
yes, I did not add "DT_CHECKER_FLAGS" to check my patch.
I will re-sent this patch.
Thanks

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

* Re: [PATCH 3/3] dt-bindings: aspeed-i2c: Convert txt to yaml format
@ 2021-05-20  3:16       ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-20  3:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Ryan Chen, moderated list:ARM/ASPEED MACHINE SUPPORT,
	Andrew Jeffery, Brendan Higgins, ChiaWei Wang, Troy Lee,
	Steven Lee, open list, Rob Herring, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	moderated list:ARM/ASPEED I2C DRIVER,
	moderated list:ARM/ASPEED MACHINE SUPPORT

The 05/19/2021 15:29, Rob Herring wrote:
> On Wed, 19 May 2021 16:04:29 +0800, Jamin Lin wrote:
> > Add global-reg node for AST2600. Document the properties for
> > "aspeed,ast2600-i2c-global" compatible node.
> > 
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >  .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
> >  .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
> >  2 files changed, 89 insertions(+), 49 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/i2c/aspeed,i2c.example.dt.yaml:0:0: /example-0/i2c-global-regs@0: failed to match any schema with compatible: ['aspeed,ast2600-i2c-global', 'syscon']
> 
> See https://patchwork.ozlabs.org/patch/1480769
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
>
Thanks for your review.
yes, I did not add "DT_CHECKER_FLAGS" to check my patch.
I will re-sent this patch.
Thanks

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

* Re: [PATCH 3/3] dt-bindings: aspeed-i2c: Convert txt to yaml format
@ 2021-05-20  3:16       ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-20  3:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: Brendan Higgins, Steven Lee, Rayn Chen, Benjamin Herrenschmidt,
	Andrew Jeffery, moderated list:ARM/ASPEED MACHINE SUPPORT,
	open list:I2C SUBSYSTEM HOST DRIVERS, ChiaWei Wang, Joel Stanley,
	Ryan Chen, Rob Herring, moderated list:ARM/ASPEED I2C DRIVER,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list, Troy Lee,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

The 05/19/2021 15:29, Rob Herring wrote:
> On Wed, 19 May 2021 16:04:29 +0800, Jamin Lin wrote:
> > Add global-reg node for AST2600. Document the properties for
> > "aspeed,ast2600-i2c-global" compatible node.
> > 
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >  .../devicetree/bindings/i2c/aspeed,i2c.yaml   | 89 +++++++++++++++++++
> >  .../devicetree/bindings/i2c/i2c-aspeed.txt    | 49 ----------
> >  2 files changed, 89 insertions(+), 49 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/i2c/aspeed,i2c.example.dt.yaml:0:0: /example-0/i2c-global-regs@0: failed to match any schema with compatible: ['aspeed,ast2600-i2c-global', 'syscon']
> 
> See https://patchwork.ozlabs.org/patch/1480769
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
>
Thanks for your review.
yes, I did not add "DT_CHECKER_FLAGS" to check my patch.
I will re-sent this patch.
Thanks

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
  2021-05-19 22:59     ` Joel Stanley
  (?)
@ 2021-05-20  3:31       ` Jamin Lin
  -1 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-20  3:31 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Rob Herring, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, Ryan Chen, ChiaWei Wang,
	Troy Lee, Steven Lee

The 05/19/2021 22:59, Joel Stanley wrote:
> On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> >
> > The register definition between AST2600 A2 and A3 is different.
> > This patch avoid new registers definition of AST2600 to use
> > this driver. We will submit the path for the new registers
> > definition of AST2600.
> 
> The AST2600 v9 datasheet says that bit 2 selects between old and new
> register sets, and that the old register set is the default.
> 
> Has the default changed for the A3?, and the datasheet is incorrect?
> 
> Does the A3 still support the old register set?
> 
We suggest user to use the new i2c driver for AST2600 and we will sumbit
it. This driver is used to AST2500 and AST2400 SOCs. Change this
driver to check global register of i2c to avoid user build the wrong driver. 

> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >  drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> > index 724bf30600d6..007309077d9f 100644
> > --- a/drivers/i2c/busses/i2c-aspeed.c
> > +++ b/drivers/i2c/busses/i2c-aspeed.c
> > @@ -19,14 +19,20 @@
> >  #include <linux/irqchip/chained_irq.h>
> >  #include <linux/irqdomain.h>
> >  #include <linux/kernel.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/module.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> >  #include <linux/reset.h>
> >  #include <linux/slab.h>
> >
> > +/* I2C Global Registers */
> > +/* 0x0c : I2CG Global Control Register (AST2500)  */
> > +#define ASPEED_I2CG_GLOBAL_CTRL_REG                    0x0c
> > +
> >  /* I2C Register */
> >  #define ASPEED_I2C_FUN_CTRL_REG                                0x00
> >  #define ASPEED_I2C_AC_TIMING_REG1                      0x04
> > @@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> >         struct resource *res;
> >         int irq, ret;
> >
> > +       if (of_device_is_compatible(pdev->dev.of_node,
> > +                                   "aspeed,ast2600-i2c-bus")) {
> > +               u32 global_ctrl;
> > +               struct regmap *gr_regmap;
> > +
> > +               gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
> > +
> > +               if (IS_ERR(gr_regmap)) {
> > +                       ret = PTR_ERR(gr_regmap);
> > +               } else {
> > +                       regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
> > +                       if (global_ctrl & BIT(2))
> > +                               return -EIO;
> > +               }
> > +       }
> > +
> >         bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> >         if (!bus)
> >                 return -ENOMEM;
> > --
> > 2.17.1
> >

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-20  3:31       ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-20  3:31 UTC (permalink / raw)
  To: Joel Stanley
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Ryan Chen, moderated list:ARM/ASPEED MACHINE SUPPORT,
	Andrew Jeffery, moderated list:ARM/ASPEED I2C DRIVER, Troy Lee,
	Brendan Higgins, open list, Rob Herring, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS, Steven Lee, ChiaWei Wang,
	moderated list:ARM/ASPEED MACHINE SUPPORT

The 05/19/2021 22:59, Joel Stanley wrote:
> On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> >
> > The register definition between AST2600 A2 and A3 is different.
> > This patch avoid new registers definition of AST2600 to use
> > this driver. We will submit the path for the new registers
> > definition of AST2600.
> 
> The AST2600 v9 datasheet says that bit 2 selects between old and new
> register sets, and that the old register set is the default.
> 
> Has the default changed for the A3?, and the datasheet is incorrect?
> 
> Does the A3 still support the old register set?
> 
We suggest user to use the new i2c driver for AST2600 and we will sumbit
it. This driver is used to AST2500 and AST2400 SOCs. Change this
driver to check global register of i2c to avoid user build the wrong driver. 

> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >  drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> > index 724bf30600d6..007309077d9f 100644
> > --- a/drivers/i2c/busses/i2c-aspeed.c
> > +++ b/drivers/i2c/busses/i2c-aspeed.c
> > @@ -19,14 +19,20 @@
> >  #include <linux/irqchip/chained_irq.h>
> >  #include <linux/irqdomain.h>
> >  #include <linux/kernel.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/module.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> >  #include <linux/reset.h>
> >  #include <linux/slab.h>
> >
> > +/* I2C Global Registers */
> > +/* 0x0c : I2CG Global Control Register (AST2500)  */
> > +#define ASPEED_I2CG_GLOBAL_CTRL_REG                    0x0c
> > +
> >  /* I2C Register */
> >  #define ASPEED_I2C_FUN_CTRL_REG                                0x00
> >  #define ASPEED_I2C_AC_TIMING_REG1                      0x04
> > @@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> >         struct resource *res;
> >         int irq, ret;
> >
> > +       if (of_device_is_compatible(pdev->dev.of_node,
> > +                                   "aspeed,ast2600-i2c-bus")) {
> > +               u32 global_ctrl;
> > +               struct regmap *gr_regmap;
> > +
> > +               gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
> > +
> > +               if (IS_ERR(gr_regmap)) {
> > +                       ret = PTR_ERR(gr_regmap);
> > +               } else {
> > +                       regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
> > +                       if (global_ctrl & BIT(2))
> > +                               return -EIO;
> > +               }
> > +       }
> > +
> >         bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> >         if (!bus)
> >                 return -ENOMEM;
> > --
> > 2.17.1
> >

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-20  3:31       ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-20  3:31 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Rob Herring, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, Rayn Chen,
	open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, Ryan Chen, ChiaWei Wang,
	Troy Lee, Steven Lee

The 05/19/2021 22:59, Joel Stanley wrote:
> On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> >
> > The register definition between AST2600 A2 and A3 is different.
> > This patch avoid new registers definition of AST2600 to use
> > this driver. We will submit the path for the new registers
> > definition of AST2600.
> 
> The AST2600 v9 datasheet says that bit 2 selects between old and new
> register sets, and that the old register set is the default.
> 
> Has the default changed for the A3?, and the datasheet is incorrect?
> 
> Does the A3 still support the old register set?
> 
We suggest user to use the new i2c driver for AST2600 and we will sumbit
it. This driver is used to AST2500 and AST2400 SOCs. Change this
driver to check global register of i2c to avoid user build the wrong driver. 

> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >  drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> > index 724bf30600d6..007309077d9f 100644
> > --- a/drivers/i2c/busses/i2c-aspeed.c
> > +++ b/drivers/i2c/busses/i2c-aspeed.c
> > @@ -19,14 +19,20 @@
> >  #include <linux/irqchip/chained_irq.h>
> >  #include <linux/irqdomain.h>
> >  #include <linux/kernel.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/module.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> >  #include <linux/reset.h>
> >  #include <linux/slab.h>
> >
> > +/* I2C Global Registers */
> > +/* 0x0c : I2CG Global Control Register (AST2500)  */
> > +#define ASPEED_I2CG_GLOBAL_CTRL_REG                    0x0c
> > +
> >  /* I2C Register */
> >  #define ASPEED_I2C_FUN_CTRL_REG                                0x00
> >  #define ASPEED_I2C_AC_TIMING_REG1                      0x04
> > @@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> >         struct resource *res;
> >         int irq, ret;
> >
> > +       if (of_device_is_compatible(pdev->dev.of_node,
> > +                                   "aspeed,ast2600-i2c-bus")) {
> > +               u32 global_ctrl;
> > +               struct regmap *gr_regmap;
> > +
> > +               gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
> > +
> > +               if (IS_ERR(gr_regmap)) {
> > +                       ret = PTR_ERR(gr_regmap);
> > +               } else {
> > +                       regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
> > +                       if (global_ctrl & BIT(2))
> > +                               return -EIO;
> > +               }
> > +       }
> > +
> >         bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> >         if (!bus)
> >                 return -ENOMEM;
> > --
> > 2.17.1
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
  2021-05-20  3:31       ` Jamin Lin
  (?)
@ 2021-05-21  2:00         ` Tao Ren
  -1 siblings, 0 replies; 51+ messages in thread
From: Tao Ren @ 2021-05-21  2:00 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Joel Stanley,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED I2C DRIVER, Brendan Higgins, open list,
	Rob Herring, Rayn Chen, open list:I2C SUBSYSTEM HOST DRIVERS,
	Steven Lee, moderated list:ARM/ASPEED MACHINE SUPPORT

Hi Jamin,

On Thu, May 20, 2021 at 11:31:41AM +0800, Jamin Lin wrote:
> The 05/19/2021 22:59, Joel Stanley wrote:
> > On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> > >
> > > The register definition between AST2600 A2 and A3 is different.
> > > This patch avoid new registers definition of AST2600 to use
> > > this driver. We will submit the path for the new registers
> > > definition of AST2600.
> > 
> > The AST2600 v9 datasheet says that bit 2 selects between old and new
> > register sets, and that the old register set is the default.
> > 
> > Has the default changed for the A3?, and the datasheet is incorrect?
> > 
> > Does the A3 still support the old register set?
> > 
> We suggest user to use the new i2c driver for AST2600 and we will sumbit
> it. This driver is used to AST2500 and AST2400 SOCs. Change this
> driver to check global register of i2c to avoid user build the wrong driver. 

If I understand correctly, the answer implies old register set is still
supported in A3 although aspeed suggest people using the new driver/mode?

Can you please share more context behind the suggestion? Such as new
register mode has better performance? Or some known issues that were
deteted in old mode are fixed in new register mode?


Cheers,

Tao

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-21  2:00         ` Tao Ren
  0 siblings, 0 replies; 51+ messages in thread
From: Tao Ren @ 2021-05-21  2:00 UTC (permalink / raw)
  To: Jamin Lin
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT, Steven Lee,
	moderated list:ARM/ASPEED I2C DRIVER, Brendan Higgins, open list,
	Rob Herring, Rayn Chen,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	open list:I2C SUBSYSTEM HOST DRIVERS

Hi Jamin,

On Thu, May 20, 2021 at 11:31:41AM +0800, Jamin Lin wrote:
> The 05/19/2021 22:59, Joel Stanley wrote:
> > On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> > >
> > > The register definition between AST2600 A2 and A3 is different.
> > > This patch avoid new registers definition of AST2600 to use
> > > this driver. We will submit the path for the new registers
> > > definition of AST2600.
> > 
> > The AST2600 v9 datasheet says that bit 2 selects between old and new
> > register sets, and that the old register set is the default.
> > 
> > Has the default changed for the A3?, and the datasheet is incorrect?
> > 
> > Does the A3 still support the old register set?
> > 
> We suggest user to use the new i2c driver for AST2600 and we will sumbit
> it. This driver is used to AST2500 and AST2400 SOCs. Change this
> driver to check global register of i2c to avoid user build the wrong driver. 

If I understand correctly, the answer implies old register set is still
supported in A3 although aspeed suggest people using the new driver/mode?

Can you please share more context behind the suggestion? Such as new
register mode has better performance? Or some known issues that were
deteted in old mode are fixed in new register mode?


Cheers,

Tao

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-21  2:00         ` Tao Ren
  0 siblings, 0 replies; 51+ messages in thread
From: Tao Ren @ 2021-05-21  2:00 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Joel Stanley,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED I2C DRIVER, Brendan Higgins, open list,
	Rob Herring, Rayn Chen, open list:I2C SUBSYSTEM HOST DRIVERS,
	Steven Lee, moderated list:ARM/ASPEED MACHINE SUPPORT

Hi Jamin,

On Thu, May 20, 2021 at 11:31:41AM +0800, Jamin Lin wrote:
> The 05/19/2021 22:59, Joel Stanley wrote:
> > On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> > >
> > > The register definition between AST2600 A2 and A3 is different.
> > > This patch avoid new registers definition of AST2600 to use
> > > this driver. We will submit the path for the new registers
> > > definition of AST2600.
> > 
> > The AST2600 v9 datasheet says that bit 2 selects between old and new
> > register sets, and that the old register set is the default.
> > 
> > Has the default changed for the A3?, and the datasheet is incorrect?
> > 
> > Does the A3 still support the old register set?
> > 
> We suggest user to use the new i2c driver for AST2600 and we will sumbit
> it. This driver is used to AST2500 and AST2400 SOCs. Change this
> driver to check global register of i2c to avoid user build the wrong driver. 

If I understand correctly, the answer implies old register set is still
supported in A3 although aspeed suggest people using the new driver/mode?

Can you please share more context behind the suggestion? Such as new
register mode has better performance? Or some known issues that were
deteted in old mode are fixed in new register mode?


Cheers,

Tao

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
  2021-05-21  2:00         ` Tao Ren
  (?)
@ 2021-05-24  1:53           ` Jamin Lin
  -1 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-24  1:53 UTC (permalink / raw)
  To: Tao Ren
  Cc: Joel Stanley,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED I2C DRIVER, Brendan Higgins, open list,
	Rob Herring, Rayn Chen, open list:I2C SUBSYSTEM HOST DRIVERS,
	Steven Lee, moderated list:ARM/ASPEED MACHINE SUPPORT

The 05/21/2021 02:00, Tao Ren wrote:
> Hi Jamin,
> 
> On Thu, May 20, 2021 at 11:31:41AM +0800, Jamin Lin wrote:
> > The 05/19/2021 22:59, Joel Stanley wrote:
> > > On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> > > >
> > > > The register definition between AST2600 A2 and A3 is different.
> > > > This patch avoid new registers definition of AST2600 to use
> > > > this driver. We will submit the path for the new registers
> > > > definition of AST2600.
> > > 
> > > The AST2600 v9 datasheet says that bit 2 selects between old and new
> > > register sets, and that the old register set is the default.
> > > 
> > > Has the default changed for the A3?, and the datasheet is incorrect?
> > > 
> > > Does the A3 still support the old register set?
> > > 
> > We suggest user to use the new i2c driver for AST2600 and we will sumbit
> > it. This driver is used to AST2500 and AST2400 SOCs. Change this
> > driver to check global register of i2c to avoid user build the wrong driver. 
> 
> If I understand correctly, the answer implies old register set is still
> supported in A3 although aspeed suggest people using the new driver/mode?
> 
> Can you please share more context behind the suggestion? Such as new
> register mode has better performance? Or some known issues that were
> deteted in old mode are fixed in new register mode?
>
Yes, AST2600 A1, A2 and A3 support both old and new register set. The difference
between old and new register set are the register address and supported registers.
User can choose to use both old and new register set. However, ASPEED would like to 
change new register set by default for AST2600.
Thanks-Jamin
> 
> Cheers,
> 
> Tao

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-24  1:53           ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-24  1:53 UTC (permalink / raw)
  To: Tao Ren
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT, Steven Lee,
	moderated list:ARM/ASPEED I2C DRIVER, Brendan Higgins, open list,
	Rob Herring, Rayn Chen,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	open list:I2C SUBSYSTEM HOST DRIVERS

The 05/21/2021 02:00, Tao Ren wrote:
> Hi Jamin,
> 
> On Thu, May 20, 2021 at 11:31:41AM +0800, Jamin Lin wrote:
> > The 05/19/2021 22:59, Joel Stanley wrote:
> > > On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> > > >
> > > > The register definition between AST2600 A2 and A3 is different.
> > > > This patch avoid new registers definition of AST2600 to use
> > > > this driver. We will submit the path for the new registers
> > > > definition of AST2600.
> > > 
> > > The AST2600 v9 datasheet says that bit 2 selects between old and new
> > > register sets, and that the old register set is the default.
> > > 
> > > Has the default changed for the A3?, and the datasheet is incorrect?
> > > 
> > > Does the A3 still support the old register set?
> > > 
> > We suggest user to use the new i2c driver for AST2600 and we will sumbit
> > it. This driver is used to AST2500 and AST2400 SOCs. Change this
> > driver to check global register of i2c to avoid user build the wrong driver. 
> 
> If I understand correctly, the answer implies old register set is still
> supported in A3 although aspeed suggest people using the new driver/mode?
> 
> Can you please share more context behind the suggestion? Such as new
> register mode has better performance? Or some known issues that were
> deteted in old mode are fixed in new register mode?
>
Yes, AST2600 A1, A2 and A3 support both old and new register set. The difference
between old and new register set are the register address and supported registers.
User can choose to use both old and new register set. However, ASPEED would like to 
change new register set by default for AST2600.
Thanks-Jamin
> 
> Cheers,
> 
> Tao

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-24  1:53           ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-24  1:53 UTC (permalink / raw)
  To: Tao Ren
  Cc: Joel Stanley,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED I2C DRIVER, Brendan Higgins, open list,
	Rob Herring, Rayn Chen, open list:I2C SUBSYSTEM HOST DRIVERS,
	Steven Lee, moderated list:ARM/ASPEED MACHINE SUPPORT

The 05/21/2021 02:00, Tao Ren wrote:
> Hi Jamin,
> 
> On Thu, May 20, 2021 at 11:31:41AM +0800, Jamin Lin wrote:
> > The 05/19/2021 22:59, Joel Stanley wrote:
> > > On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> > > >
> > > > The register definition between AST2600 A2 and A3 is different.
> > > > This patch avoid new registers definition of AST2600 to use
> > > > this driver. We will submit the path for the new registers
> > > > definition of AST2600.
> > > 
> > > The AST2600 v9 datasheet says that bit 2 selects between old and new
> > > register sets, and that the old register set is the default.
> > > 
> > > Has the default changed for the A3?, and the datasheet is incorrect?
> > > 
> > > Does the A3 still support the old register set?
> > > 
> > We suggest user to use the new i2c driver for AST2600 and we will sumbit
> > it. This driver is used to AST2500 and AST2400 SOCs. Change this
> > driver to check global register of i2c to avoid user build the wrong driver. 
> 
> If I understand correctly, the answer implies old register set is still
> supported in A3 although aspeed suggest people using the new driver/mode?
> 
> Can you please share more context behind the suggestion? Such as new
> register mode has better performance? Or some known issues that were
> deteted in old mode are fixed in new register mode?
>
Yes, AST2600 A1, A2 and A3 support both old and new register set. The difference
between old and new register set are the register address and supported registers.
User can choose to use both old and new register set. However, ASPEED would like to 
change new register set by default for AST2600.
Thanks-Jamin
> 
> Cheers,
> 
> Tao

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
  2021-05-19 19:02     ` Zev Weiss
  (?)
@ 2021-05-24  2:08       ` Jamin Lin
  -1 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-24  2:08 UTC (permalink / raw)
  To: Zev Weiss
  Cc: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, Steven Lee, ChiaWei Wang,
	Troy Lee, Ryan Chen

The 05/19/2021 19:02, Zev Weiss wrote:
> On Wed, May 19, 2021 at 03:04:27AM CDT, Jamin Lin wrote:
> >The register definition between AST2600 A2 and A3 is different.
> >This patch avoid new registers definition of AST2600 to use
> >this driver. We will submit the path for the new registers
> >definition of AST2600.
> >
> >Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> >---
> > drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> >
> >diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> >index 724bf30600d6..007309077d9f 100644
> >--- a/drivers/i2c/busses/i2c-aspeed.c
> >+++ b/drivers/i2c/busses/i2c-aspeed.c
> >@@ -19,14 +19,20 @@
> > #include <linux/irqchip/chained_irq.h>
> > #include <linux/irqdomain.h>
> > #include <linux/kernel.h>
> >+#include <linux/mfd/syscon.h>
> > #include <linux/module.h>
> > #include <linux/of_address.h>
> > #include <linux/of_irq.h>
> > #include <linux/of_platform.h>
> > #include <linux/platform_device.h>
> >+#include <linux/regmap.h>
> > #include <linux/reset.h>
> > #include <linux/slab.h>
> >
> >+/* I2C Global Registers */
> >+/* 0x0c : I2CG Global Control Register (AST2500)  */
> >+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
> >+
> > /* I2C Register */
> > #define ASPEED_I2C_FUN_CTRL_REG				0x00
> > #define ASPEED_I2C_AC_TIMING_REG1			0x04
> >@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> > 	struct resource *res;
> > 	int irq, ret;
> >
> >+	if (of_device_is_compatible(pdev->dev.of_node,
> >+				    "aspeed,ast2600-i2c-bus")) {
> >+		u32 global_ctrl;
> >+		struct regmap *gr_regmap;
> >+
> >+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
> >+
> >+		if (IS_ERR(gr_regmap)) {
> >+			ret = PTR_ERR(gr_regmap);
> >+		} else {
> >+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
> >+			if (global_ctrl & BIT(2))
> >+				return -EIO;
> 
> A macro definition might be a bit nicer than a raw BIT(2) here I'd
> think.
Will modify 
> 
> Also, it seems a bit unfortunate to just bail on the device entirely if
> we find this bit set (seems like a good way for a bootloader to
> inadvertently DoS the kernel), though I guess poking global syscon bits
> in the bus probe function might not be ideal.  Could/should we consider
> some module-level init code to ensure that bit is cleared?
> 
> 
We use syscon API to get the global register of i2c not the specific i2c
bus.
Can you describe it more detail?
Thanks-Jamin
> Zev
> 
> >+		}
> >+	}
> >+
> > 	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> > 	if (!bus)
> > 		return -ENOMEM;
> >-- 
> >2.17.1
> >

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-24  2:08       ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-24  2:08 UTC (permalink / raw)
  To: Zev Weiss
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Ryan Chen, moderated list:ARM/ASPEED MACHINE SUPPORT,
	Andrew Jeffery, moderated list:ARM/ASPEED I2C DRIVER, Troy Lee,
	Brendan Higgins, open list, Rob Herring, Steven Lee,
	ChiaWei Wang, moderated list:ARM/ASPEED MACHINE SUPPORT,
	open list:I2C SUBSYSTEM HOST DRIVERS

The 05/19/2021 19:02, Zev Weiss wrote:
> On Wed, May 19, 2021 at 03:04:27AM CDT, Jamin Lin wrote:
> >The register definition between AST2600 A2 and A3 is different.
> >This patch avoid new registers definition of AST2600 to use
> >this driver. We will submit the path for the new registers
> >definition of AST2600.
> >
> >Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> >---
> > drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> >
> >diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> >index 724bf30600d6..007309077d9f 100644
> >--- a/drivers/i2c/busses/i2c-aspeed.c
> >+++ b/drivers/i2c/busses/i2c-aspeed.c
> >@@ -19,14 +19,20 @@
> > #include <linux/irqchip/chained_irq.h>
> > #include <linux/irqdomain.h>
> > #include <linux/kernel.h>
> >+#include <linux/mfd/syscon.h>
> > #include <linux/module.h>
> > #include <linux/of_address.h>
> > #include <linux/of_irq.h>
> > #include <linux/of_platform.h>
> > #include <linux/platform_device.h>
> >+#include <linux/regmap.h>
> > #include <linux/reset.h>
> > #include <linux/slab.h>
> >
> >+/* I2C Global Registers */
> >+/* 0x0c : I2CG Global Control Register (AST2500)  */
> >+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
> >+
> > /* I2C Register */
> > #define ASPEED_I2C_FUN_CTRL_REG				0x00
> > #define ASPEED_I2C_AC_TIMING_REG1			0x04
> >@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> > 	struct resource *res;
> > 	int irq, ret;
> >
> >+	if (of_device_is_compatible(pdev->dev.of_node,
> >+				    "aspeed,ast2600-i2c-bus")) {
> >+		u32 global_ctrl;
> >+		struct regmap *gr_regmap;
> >+
> >+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
> >+
> >+		if (IS_ERR(gr_regmap)) {
> >+			ret = PTR_ERR(gr_regmap);
> >+		} else {
> >+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
> >+			if (global_ctrl & BIT(2))
> >+				return -EIO;
> 
> A macro definition might be a bit nicer than a raw BIT(2) here I'd
> think.
Will modify 
> 
> Also, it seems a bit unfortunate to just bail on the device entirely if
> we find this bit set (seems like a good way for a bootloader to
> inadvertently DoS the kernel), though I guess poking global syscon bits
> in the bus probe function might not be ideal.  Could/should we consider
> some module-level init code to ensure that bit is cleared?
> 
> 
We use syscon API to get the global register of i2c not the specific i2c
bus.
Can you describe it more detail?
Thanks-Jamin
> Zev
> 
> >+		}
> >+	}
> >+
> > 	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> > 	if (!bus)
> > 		return -ENOMEM;
> >-- 
> >2.17.1
> >

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-24  2:08       ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-24  2:08 UTC (permalink / raw)
  To: Zev Weiss
  Cc: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, Steven Lee, ChiaWei Wang,
	Troy Lee, Ryan Chen

The 05/19/2021 19:02, Zev Weiss wrote:
> On Wed, May 19, 2021 at 03:04:27AM CDT, Jamin Lin wrote:
> >The register definition between AST2600 A2 and A3 is different.
> >This patch avoid new registers definition of AST2600 to use
> >this driver. We will submit the path for the new registers
> >definition of AST2600.
> >
> >Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> >---
> > drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> >
> >diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> >index 724bf30600d6..007309077d9f 100644
> >--- a/drivers/i2c/busses/i2c-aspeed.c
> >+++ b/drivers/i2c/busses/i2c-aspeed.c
> >@@ -19,14 +19,20 @@
> > #include <linux/irqchip/chained_irq.h>
> > #include <linux/irqdomain.h>
> > #include <linux/kernel.h>
> >+#include <linux/mfd/syscon.h>
> > #include <linux/module.h>
> > #include <linux/of_address.h>
> > #include <linux/of_irq.h>
> > #include <linux/of_platform.h>
> > #include <linux/platform_device.h>
> >+#include <linux/regmap.h>
> > #include <linux/reset.h>
> > #include <linux/slab.h>
> >
> >+/* I2C Global Registers */
> >+/* 0x0c : I2CG Global Control Register (AST2500)  */
> >+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
> >+
> > /* I2C Register */
> > #define ASPEED_I2C_FUN_CTRL_REG				0x00
> > #define ASPEED_I2C_AC_TIMING_REG1			0x04
> >@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> > 	struct resource *res;
> > 	int irq, ret;
> >
> >+	if (of_device_is_compatible(pdev->dev.of_node,
> >+				    "aspeed,ast2600-i2c-bus")) {
> >+		u32 global_ctrl;
> >+		struct regmap *gr_regmap;
> >+
> >+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
> >+
> >+		if (IS_ERR(gr_regmap)) {
> >+			ret = PTR_ERR(gr_regmap);
> >+		} else {
> >+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
> >+			if (global_ctrl & BIT(2))
> >+				return -EIO;
> 
> A macro definition might be a bit nicer than a raw BIT(2) here I'd
> think.
Will modify 
> 
> Also, it seems a bit unfortunate to just bail on the device entirely if
> we find this bit set (seems like a good way for a bootloader to
> inadvertently DoS the kernel), though I guess poking global syscon bits
> in the bus probe function might not be ideal.  Could/should we consider
> some module-level init code to ensure that bit is cleared?
> 
> 
We use syscon API to get the global register of i2c not the specific i2c
bus.
Can you describe it more detail?
Thanks-Jamin
> Zev
> 
> >+		}
> >+	}
> >+
> > 	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> > 	if (!bus)
> > 		return -ENOMEM;
> >-- 
> >2.17.1
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
  2021-05-24  1:53           ` Jamin Lin
  (?)
@ 2021-05-24  2:34             ` Joel Stanley
  -1 siblings, 0 replies; 51+ messages in thread
From: Joel Stanley @ 2021-05-24  2:34 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Tao Ren,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED I2C DRIVER, Brendan Higgins, open list,
	Rob Herring, Rayn Chen, open list:I2C SUBSYSTEM HOST DRIVERS,
	Steven Lee, moderated list:ARM/ASPEED MACHINE SUPPORT

On Mon, 24 May 2021 at 01:53, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
>
> The 05/21/2021 02:00, Tao Ren wrote:
> > Hi Jamin,
> >
> > On Thu, May 20, 2021 at 11:31:41AM +0800, Jamin Lin wrote:
> > > The 05/19/2021 22:59, Joel Stanley wrote:
> > > > On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> > > > >
> > > > > The register definition between AST2600 A2 and A3 is different.
> > > > > This patch avoid new registers definition of AST2600 to use
> > > > > this driver. We will submit the path for the new registers
> > > > > definition of AST2600.
> > > >
> > > > The AST2600 v9 datasheet says that bit 2 selects between old and new
> > > > register sets, and that the old register set is the default.
> > > >
> > > > Has the default changed for the A3?, and the datasheet is incorrect?
> > > >
> > > > Does the A3 still support the old register set?
> > > >
> > > We suggest user to use the new i2c driver for AST2600 and we will sumbit
> > > it. This driver is used to AST2500 and AST2400 SOCs. Change this
> > > driver to check global register of i2c to avoid user build the wrong driver.
> >
> > If I understand correctly, the answer implies old register set is still
> > supported in A3 although aspeed suggest people using the new driver/mode?
> >
> > Can you please share more context behind the suggestion? Such as new
> > register mode has better performance? Or some known issues that were
> > deteted in old mode are fixed in new register mode?
> >
> Yes, AST2600 A1, A2 and A3 support both old and new register set. The difference
> between old and new register set are the register address and supported registers.
> User can choose to use both old and new register set. However, ASPEED would like to
> change new register set by default for AST2600.

We can certainly make the driver for the new register set the default
for AST2600 when the new driver is merged.

I disagree that we should introduce a run time check to fail to probe
the old driver. Please do not merge this patch.

Please focus your effort on getting the new driver merged instead.

Cheers,

Joel

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-24  2:34             ` Joel Stanley
  0 siblings, 0 replies; 51+ messages in thread
From: Joel Stanley @ 2021-05-24  2:34 UTC (permalink / raw)
  To: Jamin Lin
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT, Steven Lee,
	moderated list:ARM/ASPEED I2C DRIVER, Brendan Higgins, open list,
	Rob Herring, Rayn Chen, open list:I2C SUBSYSTEM HOST DRIVERS,
	Tao Ren, moderated list:ARM/ASPEED MACHINE SUPPORT

On Mon, 24 May 2021 at 01:53, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
>
> The 05/21/2021 02:00, Tao Ren wrote:
> > Hi Jamin,
> >
> > On Thu, May 20, 2021 at 11:31:41AM +0800, Jamin Lin wrote:
> > > The 05/19/2021 22:59, Joel Stanley wrote:
> > > > On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> > > > >
> > > > > The register definition between AST2600 A2 and A3 is different.
> > > > > This patch avoid new registers definition of AST2600 to use
> > > > > this driver. We will submit the path for the new registers
> > > > > definition of AST2600.
> > > >
> > > > The AST2600 v9 datasheet says that bit 2 selects between old and new
> > > > register sets, and that the old register set is the default.
> > > >
> > > > Has the default changed for the A3?, and the datasheet is incorrect?
> > > >
> > > > Does the A3 still support the old register set?
> > > >
> > > We suggest user to use the new i2c driver for AST2600 and we will sumbit
> > > it. This driver is used to AST2500 and AST2400 SOCs. Change this
> > > driver to check global register of i2c to avoid user build the wrong driver.
> >
> > If I understand correctly, the answer implies old register set is still
> > supported in A3 although aspeed suggest people using the new driver/mode?
> >
> > Can you please share more context behind the suggestion? Such as new
> > register mode has better performance? Or some known issues that were
> > deteted in old mode are fixed in new register mode?
> >
> Yes, AST2600 A1, A2 and A3 support both old and new register set. The difference
> between old and new register set are the register address and supported registers.
> User can choose to use both old and new register set. However, ASPEED would like to
> change new register set by default for AST2600.

We can certainly make the driver for the new register set the default
for AST2600 when the new driver is merged.

I disagree that we should introduce a run time check to fail to probe
the old driver. Please do not merge this patch.

Please focus your effort on getting the new driver merged instead.

Cheers,

Joel

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-24  2:34             ` Joel Stanley
  0 siblings, 0 replies; 51+ messages in thread
From: Joel Stanley @ 2021-05-24  2:34 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Tao Ren,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED I2C DRIVER, Brendan Higgins, open list,
	Rob Herring, Rayn Chen, open list:I2C SUBSYSTEM HOST DRIVERS,
	Steven Lee, moderated list:ARM/ASPEED MACHINE SUPPORT

On Mon, 24 May 2021 at 01:53, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
>
> The 05/21/2021 02:00, Tao Ren wrote:
> > Hi Jamin,
> >
> > On Thu, May 20, 2021 at 11:31:41AM +0800, Jamin Lin wrote:
> > > The 05/19/2021 22:59, Joel Stanley wrote:
> > > > On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> > > > >
> > > > > The register definition between AST2600 A2 and A3 is different.
> > > > > This patch avoid new registers definition of AST2600 to use
> > > > > this driver. We will submit the path for the new registers
> > > > > definition of AST2600.
> > > >
> > > > The AST2600 v9 datasheet says that bit 2 selects between old and new
> > > > register sets, and that the old register set is the default.
> > > >
> > > > Has the default changed for the A3?, and the datasheet is incorrect?
> > > >
> > > > Does the A3 still support the old register set?
> > > >
> > > We suggest user to use the new i2c driver for AST2600 and we will sumbit
> > > it. This driver is used to AST2500 and AST2400 SOCs. Change this
> > > driver to check global register of i2c to avoid user build the wrong driver.
> >
> > If I understand correctly, the answer implies old register set is still
> > supported in A3 although aspeed suggest people using the new driver/mode?
> >
> > Can you please share more context behind the suggestion? Such as new
> > register mode has better performance? Or some known issues that were
> > deteted in old mode are fixed in new register mode?
> >
> Yes, AST2600 A1, A2 and A3 support both old and new register set. The difference
> between old and new register set are the register address and supported registers.
> User can choose to use both old and new register set. However, ASPEED would like to
> change new register set by default for AST2600.

We can certainly make the driver for the new register set the default
for AST2600 when the new driver is merged.

I disagree that we should introduce a run time check to fail to probe
the old driver. Please do not merge this patch.

Please focus your effort on getting the new driver merged instead.

Cheers,

Joel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
  2021-05-24  2:08       ` Jamin Lin
  (?)
@ 2021-05-24 21:16         ` Zev Weiss
  -1 siblings, 0 replies; 51+ messages in thread
From: Zev Weiss @ 2021-05-24 21:16 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, Steven Lee, ChiaWei Wang,
	Troy Lee, Ryan Chen

On Sun, May 23, 2021 at 09:08:47PM CDT, Jamin Lin wrote:
>The 05/19/2021 19:02, Zev Weiss wrote:
>> On Wed, May 19, 2021 at 03:04:27AM CDT, Jamin Lin wrote:
>> >The register definition between AST2600 A2 and A3 is different.
>> >This patch avoid new registers definition of AST2600 to use
>> >this driver. We will submit the path for the new registers
>> >definition of AST2600.
>> >
>> >Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>> >---
>> > drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
>> > 1 file changed, 22 insertions(+)
>> >
>> >diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
>> >index 724bf30600d6..007309077d9f 100644
>> >--- a/drivers/i2c/busses/i2c-aspeed.c
>> >+++ b/drivers/i2c/busses/i2c-aspeed.c
>> >@@ -19,14 +19,20 @@
>> > #include <linux/irqchip/chained_irq.h>
>> > #include <linux/irqdomain.h>
>> > #include <linux/kernel.h>
>> >+#include <linux/mfd/syscon.h>
>> > #include <linux/module.h>
>> > #include <linux/of_address.h>
>> > #include <linux/of_irq.h>
>> > #include <linux/of_platform.h>
>> > #include <linux/platform_device.h>
>> >+#include <linux/regmap.h>
>> > #include <linux/reset.h>
>> > #include <linux/slab.h>
>> >
>> >+/* I2C Global Registers */
>> >+/* 0x0c : I2CG Global Control Register (AST2500)  */
>> >+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
>> >+
>> > /* I2C Register */
>> > #define ASPEED_I2C_FUN_CTRL_REG				0x00
>> > #define ASPEED_I2C_AC_TIMING_REG1			0x04
>> >@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>> > 	struct resource *res;
>> > 	int irq, ret;
>> >
>> >+	if (of_device_is_compatible(pdev->dev.of_node,
>> >+				    "aspeed,ast2600-i2c-bus")) {
>> >+		u32 global_ctrl;
>> >+		struct regmap *gr_regmap;
>> >+
>> >+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
>> >+
>> >+		if (IS_ERR(gr_regmap)) {
>> >+			ret = PTR_ERR(gr_regmap);
>> >+		} else {
>> >+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
>> >+			if (global_ctrl & BIT(2))
>> >+				return -EIO;
>>
>> A macro definition might be a bit nicer than a raw BIT(2) here I'd
>> think.
>Will modify
>>
>> Also, it seems a bit unfortunate to just bail on the device entirely if
>> we find this bit set (seems like a good way for a bootloader to
>> inadvertently DoS the kernel), though I guess poking global syscon bits
>> in the bus probe function might not be ideal.  Could/should we consider
>> some module-level init code to ensure that bit is cleared?
>>
>>
>We use syscon API to get the global register of i2c not the specific i2c
>bus.
>Can you describe it more detail?

Sure -- I just meant that if for whatever reason the kernel is booting
on a system that's had that syscon bit set to enable the new register
access mode (e.g. by a newer bootloader or something), it seems like
we'd just give up entirely on enabling any i2c busses, when as far as I
know there shouldn't be anything stopping us from resetting the bit back
to be in the state this driver needs it to be in (old register mode) and
then continuing along normally.


Zev

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-24 21:16         ` Zev Weiss
  0 siblings, 0 replies; 51+ messages in thread
From: Zev Weiss @ 2021-05-24 21:16 UTC (permalink / raw)
  To: Jamin Lin
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Ryan Chen, moderated list:ARM/ASPEED MACHINE SUPPORT,
	Andrew Jeffery, moderated list:ARM/ASPEED I2C DRIVER, Troy Lee,
	Brendan Higgins, open list, Rob Herring, Steven Lee,
	ChiaWei Wang, moderated list:ARM/ASPEED MACHINE SUPPORT,
	open list:I2C SUBSYSTEM HOST DRIVERS

On Sun, May 23, 2021 at 09:08:47PM CDT, Jamin Lin wrote:
>The 05/19/2021 19:02, Zev Weiss wrote:
>> On Wed, May 19, 2021 at 03:04:27AM CDT, Jamin Lin wrote:
>> >The register definition between AST2600 A2 and A3 is different.
>> >This patch avoid new registers definition of AST2600 to use
>> >this driver. We will submit the path for the new registers
>> >definition of AST2600.
>> >
>> >Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>> >---
>> > drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
>> > 1 file changed, 22 insertions(+)
>> >
>> >diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
>> >index 724bf30600d6..007309077d9f 100644
>> >--- a/drivers/i2c/busses/i2c-aspeed.c
>> >+++ b/drivers/i2c/busses/i2c-aspeed.c
>> >@@ -19,14 +19,20 @@
>> > #include <linux/irqchip/chained_irq.h>
>> > #include <linux/irqdomain.h>
>> > #include <linux/kernel.h>
>> >+#include <linux/mfd/syscon.h>
>> > #include <linux/module.h>
>> > #include <linux/of_address.h>
>> > #include <linux/of_irq.h>
>> > #include <linux/of_platform.h>
>> > #include <linux/platform_device.h>
>> >+#include <linux/regmap.h>
>> > #include <linux/reset.h>
>> > #include <linux/slab.h>
>> >
>> >+/* I2C Global Registers */
>> >+/* 0x0c : I2CG Global Control Register (AST2500)  */
>> >+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
>> >+
>> > /* I2C Register */
>> > #define ASPEED_I2C_FUN_CTRL_REG				0x00
>> > #define ASPEED_I2C_AC_TIMING_REG1			0x04
>> >@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>> > 	struct resource *res;
>> > 	int irq, ret;
>> >
>> >+	if (of_device_is_compatible(pdev->dev.of_node,
>> >+				    "aspeed,ast2600-i2c-bus")) {
>> >+		u32 global_ctrl;
>> >+		struct regmap *gr_regmap;
>> >+
>> >+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
>> >+
>> >+		if (IS_ERR(gr_regmap)) {
>> >+			ret = PTR_ERR(gr_regmap);
>> >+		} else {
>> >+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
>> >+			if (global_ctrl & BIT(2))
>> >+				return -EIO;
>>
>> A macro definition might be a bit nicer than a raw BIT(2) here I'd
>> think.
>Will modify
>>
>> Also, it seems a bit unfortunate to just bail on the device entirely if
>> we find this bit set (seems like a good way for a bootloader to
>> inadvertently DoS the kernel), though I guess poking global syscon bits
>> in the bus probe function might not be ideal.  Could/should we consider
>> some module-level init code to ensure that bit is cleared?
>>
>>
>We use syscon API to get the global register of i2c not the specific i2c
>bus.
>Can you describe it more detail?

Sure -- I just meant that if for whatever reason the kernel is booting
on a system that's had that syscon bit set to enable the new register
access mode (e.g. by a newer bootloader or something), it seems like
we'd just give up entirely on enabling any i2c busses, when as far as I
know there shouldn't be anything stopping us from resetting the bit back
to be in the state this driver needs it to be in (old register mode) and
then continuing along normally.


Zev

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-24 21:16         ` Zev Weiss
  0 siblings, 0 replies; 51+ messages in thread
From: Zev Weiss @ 2021-05-24 21:16 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, Steven Lee, ChiaWei Wang,
	Troy Lee, Ryan Chen

On Sun, May 23, 2021 at 09:08:47PM CDT, Jamin Lin wrote:
>The 05/19/2021 19:02, Zev Weiss wrote:
>> On Wed, May 19, 2021 at 03:04:27AM CDT, Jamin Lin wrote:
>> >The register definition between AST2600 A2 and A3 is different.
>> >This patch avoid new registers definition of AST2600 to use
>> >this driver. We will submit the path for the new registers
>> >definition of AST2600.
>> >
>> >Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>> >---
>> > drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
>> > 1 file changed, 22 insertions(+)
>> >
>> >diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
>> >index 724bf30600d6..007309077d9f 100644
>> >--- a/drivers/i2c/busses/i2c-aspeed.c
>> >+++ b/drivers/i2c/busses/i2c-aspeed.c
>> >@@ -19,14 +19,20 @@
>> > #include <linux/irqchip/chained_irq.h>
>> > #include <linux/irqdomain.h>
>> > #include <linux/kernel.h>
>> >+#include <linux/mfd/syscon.h>
>> > #include <linux/module.h>
>> > #include <linux/of_address.h>
>> > #include <linux/of_irq.h>
>> > #include <linux/of_platform.h>
>> > #include <linux/platform_device.h>
>> >+#include <linux/regmap.h>
>> > #include <linux/reset.h>
>> > #include <linux/slab.h>
>> >
>> >+/* I2C Global Registers */
>> >+/* 0x0c : I2CG Global Control Register (AST2500)  */
>> >+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
>> >+
>> > /* I2C Register */
>> > #define ASPEED_I2C_FUN_CTRL_REG				0x00
>> > #define ASPEED_I2C_AC_TIMING_REG1			0x04
>> >@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>> > 	struct resource *res;
>> > 	int irq, ret;
>> >
>> >+	if (of_device_is_compatible(pdev->dev.of_node,
>> >+				    "aspeed,ast2600-i2c-bus")) {
>> >+		u32 global_ctrl;
>> >+		struct regmap *gr_regmap;
>> >+
>> >+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
>> >+
>> >+		if (IS_ERR(gr_regmap)) {
>> >+			ret = PTR_ERR(gr_regmap);
>> >+		} else {
>> >+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
>> >+			if (global_ctrl & BIT(2))
>> >+				return -EIO;
>>
>> A macro definition might be a bit nicer than a raw BIT(2) here I'd
>> think.
>Will modify
>>
>> Also, it seems a bit unfortunate to just bail on the device entirely if
>> we find this bit set (seems like a good way for a bootloader to
>> inadvertently DoS the kernel), though I guess poking global syscon bits
>> in the bus probe function might not be ideal.  Could/should we consider
>> some module-level init code to ensure that bit is cleared?
>>
>>
>We use syscon API to get the global register of i2c not the specific i2c
>bus.
>Can you describe it more detail?

Sure -- I just meant that if for whatever reason the kernel is booting
on a system that's had that syscon bit set to enable the new register
access mode (e.g. by a newer bootloader or something), it seems like
we'd just give up entirely on enabling any i2c busses, when as far as I
know there shouldn't be anything stopping us from resetting the bit back
to be in the state this driver needs it to be in (old register mode) and
then continuing along normally.


Zev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
  2021-05-24  2:34             ` Joel Stanley
  (?)
@ 2021-05-25  2:04               ` Jamin Lin
  -1 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-25  2:04 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Tao Ren,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED I2C DRIVER, Brendan Higgins, open list,
	Rob Herring, Rayn Chen, open list:I2C SUBSYSTEM HOST DRIVERS,
	Steven Lee, moderated list:ARM/ASPEED MACHINE SUPPORT

The 05/24/2021 02:34, Joel Stanley wrote:
> On Mon, 24 May 2021 at 01:53, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> >
> > The 05/21/2021 02:00, Tao Ren wrote:
> > > Hi Jamin,
> > >
> > > On Thu, May 20, 2021 at 11:31:41AM +0800, Jamin Lin wrote:
> > > > The 05/19/2021 22:59, Joel Stanley wrote:
> > > > > On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> > > > > >
> > > > > > The register definition between AST2600 A2 and A3 is different.
> > > > > > This patch avoid new registers definition of AST2600 to use
> > > > > > this driver. We will submit the path for the new registers
> > > > > > definition of AST2600.
> > > > >
> > > > > The AST2600 v9 datasheet says that bit 2 selects between old and new
> > > > > register sets, and that the old register set is the default.
> > > > >
> > > > > Has the default changed for the A3?, and the datasheet is incorrect?
> > > > >
> > > > > Does the A3 still support the old register set?
> > > > >
> > > > We suggest user to use the new i2c driver for AST2600 and we will sumbit
> > > > it. This driver is used to AST2500 and AST2400 SOCs. Change this
> > > > driver to check global register of i2c to avoid user build the wrong driver.
> > >
> > > If I understand correctly, the answer implies old register set is still
> > > supported in A3 although aspeed suggest people using the new driver/mode?
> > >
> > > Can you please share more context behind the suggestion? Such as new
> > > register mode has better performance? Or some known issues that were
> > > deteted in old mode are fixed in new register mode?
> > >
> > Yes, AST2600 A1, A2 and A3 support both old and new register set. The difference
> > between old and new register set are the register address and supported registers.
> > User can choose to use both old and new register set. However, ASPEED would like to
> > change new register set by default for AST2600.
> 
> We can certainly make the driver for the new register set the default
> for AST2600 when the new driver is merged.
> 
> I disagree that we should introduce a run time check to fail to probe
> the old driver. Please do not merge this patch.
> 
> Please focus your effort on getting the new driver merged instead.
> 
> Cheers,
> 
> Joel

Thanks for your suggestion. I will submit the new i2c driver for AST2600
soon.
Jamin

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-25  2:04               ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-25  2:04 UTC (permalink / raw)
  To: Joel Stanley
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT, Steven Lee,
	moderated list:ARM/ASPEED I2C DRIVER, Brendan Higgins, open list,
	Rob Herring, Rayn Chen, open list:I2C SUBSYSTEM HOST DRIVERS,
	Tao Ren, moderated list:ARM/ASPEED MACHINE SUPPORT

The 05/24/2021 02:34, Joel Stanley wrote:
> On Mon, 24 May 2021 at 01:53, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> >
> > The 05/21/2021 02:00, Tao Ren wrote:
> > > Hi Jamin,
> > >
> > > On Thu, May 20, 2021 at 11:31:41AM +0800, Jamin Lin wrote:
> > > > The 05/19/2021 22:59, Joel Stanley wrote:
> > > > > On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> > > > > >
> > > > > > The register definition between AST2600 A2 and A3 is different.
> > > > > > This patch avoid new registers definition of AST2600 to use
> > > > > > this driver. We will submit the path for the new registers
> > > > > > definition of AST2600.
> > > > >
> > > > > The AST2600 v9 datasheet says that bit 2 selects between old and new
> > > > > register sets, and that the old register set is the default.
> > > > >
> > > > > Has the default changed for the A3?, and the datasheet is incorrect?
> > > > >
> > > > > Does the A3 still support the old register set?
> > > > >
> > > > We suggest user to use the new i2c driver for AST2600 and we will sumbit
> > > > it. This driver is used to AST2500 and AST2400 SOCs. Change this
> > > > driver to check global register of i2c to avoid user build the wrong driver.
> > >
> > > If I understand correctly, the answer implies old register set is still
> > > supported in A3 although aspeed suggest people using the new driver/mode?
> > >
> > > Can you please share more context behind the suggestion? Such as new
> > > register mode has better performance? Or some known issues that were
> > > deteted in old mode are fixed in new register mode?
> > >
> > Yes, AST2600 A1, A2 and A3 support both old and new register set. The difference
> > between old and new register set are the register address and supported registers.
> > User can choose to use both old and new register set. However, ASPEED would like to
> > change new register set by default for AST2600.
> 
> We can certainly make the driver for the new register set the default
> for AST2600 when the new driver is merged.
> 
> I disagree that we should introduce a run time check to fail to probe
> the old driver. Please do not merge this patch.
> 
> Please focus your effort on getting the new driver merged instead.
> 
> Cheers,
> 
> Joel

Thanks for your suggestion. I will submit the new i2c driver for AST2600
soon.
Jamin

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-25  2:04               ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-25  2:04 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Tao Ren,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED I2C DRIVER, Brendan Higgins, open list,
	Rob Herring, Rayn Chen, open list:I2C SUBSYSTEM HOST DRIVERS,
	Steven Lee, moderated list:ARM/ASPEED MACHINE SUPPORT

The 05/24/2021 02:34, Joel Stanley wrote:
> On Mon, 24 May 2021 at 01:53, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> >
> > The 05/21/2021 02:00, Tao Ren wrote:
> > > Hi Jamin,
> > >
> > > On Thu, May 20, 2021 at 11:31:41AM +0800, Jamin Lin wrote:
> > > > The 05/19/2021 22:59, Joel Stanley wrote:
> > > > > On Wed, 19 May 2021 at 08:05, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> > > > > >
> > > > > > The register definition between AST2600 A2 and A3 is different.
> > > > > > This patch avoid new registers definition of AST2600 to use
> > > > > > this driver. We will submit the path for the new registers
> > > > > > definition of AST2600.
> > > > >
> > > > > The AST2600 v9 datasheet says that bit 2 selects between old and new
> > > > > register sets, and that the old register set is the default.
> > > > >
> > > > > Has the default changed for the A3?, and the datasheet is incorrect?
> > > > >
> > > > > Does the A3 still support the old register set?
> > > > >
> > > > We suggest user to use the new i2c driver for AST2600 and we will sumbit
> > > > it. This driver is used to AST2500 and AST2400 SOCs. Change this
> > > > driver to check global register of i2c to avoid user build the wrong driver.
> > >
> > > If I understand correctly, the answer implies old register set is still
> > > supported in A3 although aspeed suggest people using the new driver/mode?
> > >
> > > Can you please share more context behind the suggestion? Such as new
> > > register mode has better performance? Or some known issues that were
> > > deteted in old mode are fixed in new register mode?
> > >
> > Yes, AST2600 A1, A2 and A3 support both old and new register set. The difference
> > between old and new register set are the register address and supported registers.
> > User can choose to use both old and new register set. However, ASPEED would like to
> > change new register set by default for AST2600.
> 
> We can certainly make the driver for the new register set the default
> for AST2600 when the new driver is merged.
> 
> I disagree that we should introduce a run time check to fail to probe
> the old driver. Please do not merge this patch.
> 
> Please focus your effort on getting the new driver merged instead.
> 
> Cheers,
> 
> Joel

Thanks for your suggestion. I will submit the new i2c driver for AST2600
soon.
Jamin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
  2021-05-24 21:16         ` Zev Weiss
  (?)
@ 2021-05-25  2:08           ` Jamin Lin
  -1 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-25  2:08 UTC (permalink / raw)
  To: Zev Weiss
  Cc: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, Steven Lee, ChiaWei Wang,
	Troy Lee, Ryan Chen

The 05/24/2021 21:16, Zev Weiss wrote:
> On Sun, May 23, 2021 at 09:08:47PM CDT, Jamin Lin wrote:
> >The 05/19/2021 19:02, Zev Weiss wrote:
> >> On Wed, May 19, 2021 at 03:04:27AM CDT, Jamin Lin wrote:
> >> >The register definition between AST2600 A2 and A3 is different.
> >> >This patch avoid new registers definition of AST2600 to use
> >> >this driver. We will submit the path for the new registers
> >> >definition of AST2600.
> >> >
> >> >Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> >> >---
> >> > drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
> >> > 1 file changed, 22 insertions(+)
> >> >
> >> >diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> >> >index 724bf30600d6..007309077d9f 100644
> >> >--- a/drivers/i2c/busses/i2c-aspeed.c
> >> >+++ b/drivers/i2c/busses/i2c-aspeed.c
> >> >@@ -19,14 +19,20 @@
> >> > #include <linux/irqchip/chained_irq.h>
> >> > #include <linux/irqdomain.h>
> >> > #include <linux/kernel.h>
> >> >+#include <linux/mfd/syscon.h>
> >> > #include <linux/module.h>
> >> > #include <linux/of_address.h>
> >> > #include <linux/of_irq.h>
> >> > #include <linux/of_platform.h>
> >> > #include <linux/platform_device.h>
> >> >+#include <linux/regmap.h>
> >> > #include <linux/reset.h>
> >> > #include <linux/slab.h>
> >> >
> >> >+/* I2C Global Registers */
> >> >+/* 0x0c : I2CG Global Control Register (AST2500)  */
> >> >+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
> >> >+
> >> > /* I2C Register */
> >> > #define ASPEED_I2C_FUN_CTRL_REG				0x00
> >> > #define ASPEED_I2C_AC_TIMING_REG1			0x04
> >> >@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> >> > 	struct resource *res;
> >> > 	int irq, ret;
> >> >
> >> >+	if (of_device_is_compatible(pdev->dev.of_node,
> >> >+				    "aspeed,ast2600-i2c-bus")) {
> >> >+		u32 global_ctrl;
> >> >+		struct regmap *gr_regmap;
> >> >+
> >> >+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
> >> >+
> >> >+		if (IS_ERR(gr_regmap)) {
> >> >+			ret = PTR_ERR(gr_regmap);
> >> >+		} else {
> >> >+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
> >> >+			if (global_ctrl & BIT(2))
> >> >+				return -EIO;
> >>
> >> A macro definition might be a bit nicer than a raw BIT(2) here I'd
> >> think.
> >Will modify
> >>
> >> Also, it seems a bit unfortunate to just bail on the device entirely if
> >> we find this bit set (seems like a good way for a bootloader to
> >> inadvertently DoS the kernel), though I guess poking global syscon bits
> >> in the bus probe function might not be ideal.  Could/should we consider
> >> some module-level init code to ensure that bit is cleared?
> >>
> >>
> >We use syscon API to get the global register of i2c not the specific i2c
> >bus.
> >Can you describe it more detail?
> 
> Sure -- I just meant that if for whatever reason the kernel is booting
> on a system that's had that syscon bit set to enable the new register
> access mode (e.g. by a newer bootloader or something), it seems like
> we'd just give up entirely on enabling any i2c busses, when as far as I
> know there shouldn't be anything stopping us from resetting the bit back
> to be in the state this driver needs it to be in (old register mode) and
> then continuing along normally.
> 
> 
> Zev

Thanks for your suggestion. I will submit the new i2c driver for
AST2600.
Jamin

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-25  2:08           ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-25  2:08 UTC (permalink / raw)
  To: Zev Weiss
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Ryan Chen, moderated list:ARM/ASPEED MACHINE SUPPORT,
	Andrew Jeffery, moderated list:ARM/ASPEED I2C DRIVER, Troy Lee,
	Brendan Higgins, open list, Rob Herring, Steven Lee,
	ChiaWei Wang, moderated list:ARM/ASPEED MACHINE SUPPORT,
	open list:I2C SUBSYSTEM HOST DRIVERS

The 05/24/2021 21:16, Zev Weiss wrote:
> On Sun, May 23, 2021 at 09:08:47PM CDT, Jamin Lin wrote:
> >The 05/19/2021 19:02, Zev Weiss wrote:
> >> On Wed, May 19, 2021 at 03:04:27AM CDT, Jamin Lin wrote:
> >> >The register definition between AST2600 A2 and A3 is different.
> >> >This patch avoid new registers definition of AST2600 to use
> >> >this driver. We will submit the path for the new registers
> >> >definition of AST2600.
> >> >
> >> >Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> >> >---
> >> > drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
> >> > 1 file changed, 22 insertions(+)
> >> >
> >> >diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> >> >index 724bf30600d6..007309077d9f 100644
> >> >--- a/drivers/i2c/busses/i2c-aspeed.c
> >> >+++ b/drivers/i2c/busses/i2c-aspeed.c
> >> >@@ -19,14 +19,20 @@
> >> > #include <linux/irqchip/chained_irq.h>
> >> > #include <linux/irqdomain.h>
> >> > #include <linux/kernel.h>
> >> >+#include <linux/mfd/syscon.h>
> >> > #include <linux/module.h>
> >> > #include <linux/of_address.h>
> >> > #include <linux/of_irq.h>
> >> > #include <linux/of_platform.h>
> >> > #include <linux/platform_device.h>
> >> >+#include <linux/regmap.h>
> >> > #include <linux/reset.h>
> >> > #include <linux/slab.h>
> >> >
> >> >+/* I2C Global Registers */
> >> >+/* 0x0c : I2CG Global Control Register (AST2500)  */
> >> >+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
> >> >+
> >> > /* I2C Register */
> >> > #define ASPEED_I2C_FUN_CTRL_REG				0x00
> >> > #define ASPEED_I2C_AC_TIMING_REG1			0x04
> >> >@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> >> > 	struct resource *res;
> >> > 	int irq, ret;
> >> >
> >> >+	if (of_device_is_compatible(pdev->dev.of_node,
> >> >+				    "aspeed,ast2600-i2c-bus")) {
> >> >+		u32 global_ctrl;
> >> >+		struct regmap *gr_regmap;
> >> >+
> >> >+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
> >> >+
> >> >+		if (IS_ERR(gr_regmap)) {
> >> >+			ret = PTR_ERR(gr_regmap);
> >> >+		} else {
> >> >+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
> >> >+			if (global_ctrl & BIT(2))
> >> >+				return -EIO;
> >>
> >> A macro definition might be a bit nicer than a raw BIT(2) here I'd
> >> think.
> >Will modify
> >>
> >> Also, it seems a bit unfortunate to just bail on the device entirely if
> >> we find this bit set (seems like a good way for a bootloader to
> >> inadvertently DoS the kernel), though I guess poking global syscon bits
> >> in the bus probe function might not be ideal.  Could/should we consider
> >> some module-level init code to ensure that bit is cleared?
> >>
> >>
> >We use syscon API to get the global register of i2c not the specific i2c
> >bus.
> >Can you describe it more detail?
> 
> Sure -- I just meant that if for whatever reason the kernel is booting
> on a system that's had that syscon bit set to enable the new register
> access mode (e.g. by a newer bootloader or something), it seems like
> we'd just give up entirely on enabling any i2c busses, when as far as I
> know there shouldn't be anything stopping us from resetting the bit back
> to be in the state this driver needs it to be in (old register mode) and
> then continuing along normally.
> 
> 
> Zev

Thanks for your suggestion. I will submit the new i2c driver for
AST2600.
Jamin

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

* Re: [PATCH 1/3] i2c: aspeed: avoid new registers definition of AST2600
@ 2021-05-25  2:08           ` Jamin Lin
  0 siblings, 0 replies; 51+ messages in thread
From: Jamin Lin @ 2021-05-25  2:08 UTC (permalink / raw)
  To: Zev Weiss
  Cc: Rob Herring, Joel Stanley, Andrew Jeffery, Brendan Higgins,
	Benjamin Herrenschmidt, open list:I2C SUBSYSTEM HOST DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/ASPEED MACHINE SUPPORT,
	moderated list:ARM/ASPEED MACHINE SUPPORT, open list,
	moderated list:ARM/ASPEED I2C DRIVER, Steven Lee, ChiaWei Wang,
	Troy Lee, Ryan Chen

The 05/24/2021 21:16, Zev Weiss wrote:
> On Sun, May 23, 2021 at 09:08:47PM CDT, Jamin Lin wrote:
> >The 05/19/2021 19:02, Zev Weiss wrote:
> >> On Wed, May 19, 2021 at 03:04:27AM CDT, Jamin Lin wrote:
> >> >The register definition between AST2600 A2 and A3 is different.
> >> >This patch avoid new registers definition of AST2600 to use
> >> >this driver. We will submit the path for the new registers
> >> >definition of AST2600.
> >> >
> >> >Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> >> >---
> >> > drivers/i2c/busses/i2c-aspeed.c | 22 ++++++++++++++++++++++
> >> > 1 file changed, 22 insertions(+)
> >> >
> >> >diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> >> >index 724bf30600d6..007309077d9f 100644
> >> >--- a/drivers/i2c/busses/i2c-aspeed.c
> >> >+++ b/drivers/i2c/busses/i2c-aspeed.c
> >> >@@ -19,14 +19,20 @@
> >> > #include <linux/irqchip/chained_irq.h>
> >> > #include <linux/irqdomain.h>
> >> > #include <linux/kernel.h>
> >> >+#include <linux/mfd/syscon.h>
> >> > #include <linux/module.h>
> >> > #include <linux/of_address.h>
> >> > #include <linux/of_irq.h>
> >> > #include <linux/of_platform.h>
> >> > #include <linux/platform_device.h>
> >> >+#include <linux/regmap.h>
> >> > #include <linux/reset.h>
> >> > #include <linux/slab.h>
> >> >
> >> >+/* I2C Global Registers */
> >> >+/* 0x0c : I2CG Global Control Register (AST2500)  */
> >> >+#define ASPEED_I2CG_GLOBAL_CTRL_REG			0x0c
> >> >+
> >> > /* I2C Register */
> >> > #define ASPEED_I2C_FUN_CTRL_REG				0x00
> >> > #define ASPEED_I2C_AC_TIMING_REG1			0x04
> >> >@@ -973,6 +979,22 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> >> > 	struct resource *res;
> >> > 	int irq, ret;
> >> >
> >> >+	if (of_device_is_compatible(pdev->dev.of_node,
> >> >+				    "aspeed,ast2600-i2c-bus")) {
> >> >+		u32 global_ctrl;
> >> >+		struct regmap *gr_regmap;
> >> >+
> >> >+		gr_regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2600-i2c-global");
> >> >+
> >> >+		if (IS_ERR(gr_regmap)) {
> >> >+			ret = PTR_ERR(gr_regmap);
> >> >+		} else {
> >> >+			regmap_read(gr_regmap, ASPEED_I2CG_GLOBAL_CTRL_REG, &global_ctrl);
> >> >+			if (global_ctrl & BIT(2))
> >> >+				return -EIO;
> >>
> >> A macro definition might be a bit nicer than a raw BIT(2) here I'd
> >> think.
> >Will modify
> >>
> >> Also, it seems a bit unfortunate to just bail on the device entirely if
> >> we find this bit set (seems like a good way for a bootloader to
> >> inadvertently DoS the kernel), though I guess poking global syscon bits
> >> in the bus probe function might not be ideal.  Could/should we consider
> >> some module-level init code to ensure that bit is cleared?
> >>
> >>
> >We use syscon API to get the global register of i2c not the specific i2c
> >bus.
> >Can you describe it more detail?
> 
> Sure -- I just meant that if for whatever reason the kernel is booting
> on a system that's had that syscon bit set to enable the new register
> access mode (e.g. by a newer bootloader or something), it seems like
> we'd just give up entirely on enabling any i2c busses, when as far as I
> know there shouldn't be anything stopping us from resetting the bit back
> to be in the state this driver needs it to be in (old register mode) and
> then continuing along normally.
> 
> 
> Zev

Thanks for your suggestion. I will submit the new i2c driver for
AST2600.
Jamin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2021-05-25  3:38 UTC | newest]

Thread overview: 51+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-19  8:04 [PATCH 0/3] i2c: aspeed: avoid new registers definition of AST2600 Jamin Lin
2021-05-19  8:04 ` Jamin Lin
2021-05-19  8:04 ` Jamin Lin
2021-05-19  8:04 ` [PATCH 1/3] " Jamin Lin
2021-05-19  8:04   ` Jamin Lin
2021-05-19  8:04   ` Jamin Lin
2021-05-19 19:02   ` Zev Weiss
2021-05-19 19:02     ` Zev Weiss
2021-05-19 19:02     ` Zev Weiss
2021-05-24  2:08     ` Jamin Lin
2021-05-24  2:08       ` Jamin Lin
2021-05-24  2:08       ` Jamin Lin
2021-05-24 21:16       ` Zev Weiss
2021-05-24 21:16         ` Zev Weiss
2021-05-24 21:16         ` Zev Weiss
2021-05-25  2:08         ` Jamin Lin
2021-05-25  2:08           ` Jamin Lin
2021-05-25  2:08           ` Jamin Lin
2021-05-19 22:59   ` Joel Stanley
2021-05-19 22:59     ` Joel Stanley
2021-05-19 22:59     ` Joel Stanley
2021-05-20  3:31     ` Jamin Lin
2021-05-20  3:31       ` Jamin Lin
2021-05-20  3:31       ` Jamin Lin
2021-05-21  2:00       ` Tao Ren
2021-05-21  2:00         ` Tao Ren
2021-05-21  2:00         ` Tao Ren
2021-05-24  1:53         ` Jamin Lin
2021-05-24  1:53           ` Jamin Lin
2021-05-24  1:53           ` Jamin Lin
2021-05-24  2:34           ` Joel Stanley
2021-05-24  2:34             ` Joel Stanley
2021-05-24  2:34             ` Joel Stanley
2021-05-25  2:04             ` Jamin Lin
2021-05-25  2:04               ` Jamin Lin
2021-05-25  2:04               ` Jamin Lin
2021-05-19  8:04 ` [PATCH 2/3] ARM: dts: aspeed: Add node for AST2600 I2C Jamin Lin
2021-05-19  8:04   ` Jamin Lin
2021-05-19  8:04   ` Jamin Lin
2021-05-19  8:04 ` [PATCH 3/3] dt-bindings: aspeed-i2c: Convert txt to yaml format Jamin Lin
2021-05-19  8:04   ` Jamin Lin
2021-05-19  8:04   ` Jamin Lin
2021-05-19 15:29   ` Rob Herring
2021-05-19 15:29     ` Rob Herring
2021-05-19 15:29     ` Rob Herring
2021-05-20  3:16     ` Jamin Lin
2021-05-20  3:16       ` Jamin Lin
2021-05-20  3:16       ` Jamin Lin
2021-05-19 18:28   ` Rob Herring
2021-05-19 18:28     ` Rob Herring
2021-05-19 18:28     ` Rob Herring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.