devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Extend peci feature
@ 2020-10-16  6:25 Billy Tsai
  2020-10-16  6:26 ` [PATCH 1/3] peci: aspeed: make the driver support 64byte mode Billy Tsai
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Billy Tsai @ 2020-10-16  6:25 UTC (permalink / raw)
  To: robh+dt, joel, andrew, jae.hyun.yoo, billy_tsai, haiyue.wang,
	james.feist, vernon.mauery, devicetree, linux-arm-kernel,
	linux-aspeed
  Cc: BMC-SW

This patch series extend the peci to support 64byte mode and simplify
the setting of peci operation rate. 

Billy Tsai (3):
  peci: aspeed: make the driver support 64byte mode
  peci: aspeed: Auto calculate the adapter divisor
  dt-bindings: Change the meaning of clock-frequency

 .../devicetree/bindings/peci/peci-aspeed.yaml |  56 +++----
 arch/arm/boot/dts/aspeed-g6.dtsi              |   4 +-
 drivers/peci/busses/peci-aspeed.c             | 158 ++++++++++++------
 3 files changed, 136 insertions(+), 82 deletions(-)

-- 
2.17.1


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

* [PATCH 1/3] peci: aspeed: make the driver support 64byte mode
  2020-10-16  6:25 [PATCH 0/3] Extend peci feature Billy Tsai
@ 2020-10-16  6:26 ` Billy Tsai
  2020-10-16  6:26 ` [PATCH 2/3] peci: aspeed: Auto calculate the adapter divisor Billy Tsai
  2020-10-16  6:26 ` [PATCH 3/3] dt-bindings: Change the meaning of clock-frequency Billy Tsai
  2 siblings, 0 replies; 6+ messages in thread
From: Billy Tsai @ 2020-10-16  6:26 UTC (permalink / raw)
  To: robh+dt, joel, andrew, jae.hyun.yoo, billy_tsai, haiyue.wang,
	james.feist, vernon.mauery, devicetree, linux-arm-kernel,
	linux-aspeed
  Cc: BMC-SW

At ast2600 peci add the new feature can support the transfer size up to
64 bytes.
We can use the dts property "64byte-mode" to enable it.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/peci/busses/peci-aspeed.c | 67 +++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 12 deletions(-)

diff --git a/drivers/peci/busses/peci-aspeed.c b/drivers/peci/busses/peci-aspeed.c
index 2673d4c4dcf9..d6039b1c4494 100644
--- a/drivers/peci/busses/peci-aspeed.c
+++ b/drivers/peci/busses/peci-aspeed.c
@@ -28,6 +28,9 @@
 #define   ASPEED_PECI_CTRL_INVERT_IN		BIT(6)
 #define   ASPEED_PECI_CTRL_BUS_CONTENT_EN	BIT(5)
 #define   ASPEED_PECI_CTRL_PECI_EN		BIT(4)
+#define   ASPEED_PECI_CTRL_64BYTE_MODE_EN	BIT(1)
+#define     ASPEED_PECI_32BYTE_MODE		0
+#define     ASPEED_PECI_64BYTE_MODE		1
 #define   ASPEED_PECI_CTRL_PECI_CLK_EN		BIT(0)
 
 /* Timing Negotiation Register */
@@ -79,7 +82,7 @@
 #define   ASPEED_PECI_INT_TIMING_RESULT_MASK	GENMASK(29, 16)
 	  /* bits[4..0]: Same bit fields in the 'Interrupt Register' */
 
-/* Rx/Tx Data Buffer Registers */
+/* 32Bytes mode: Rx/Tx Data Buffer Registers */
 #define ASPEED_PECI_W_DATA0			0x20
 #define ASPEED_PECI_W_DATA1			0x24
 #define ASPEED_PECI_W_DATA2			0x28
@@ -96,7 +99,39 @@
 #define ASPEED_PECI_R_DATA5			0x54
 #define ASPEED_PECI_R_DATA6			0x58
 #define ASPEED_PECI_R_DATA7			0x5c
-#define   ASPEED_PECI_DATA_BUF_SIZE_MAX		32
+/* 64Bytes mode: Rx/Tx Data Buffer Registers */
+#define ASPEED_PECI_64B_W_DATA0  0x80
+#define ASPEED_PECI_64B_W_DATA1  0x84
+#define ASPEED_PECI_64B_W_DATA2  0x88
+#define ASPEED_PECI_64B_W_DATA3  0x8C
+#define ASPEED_PECI_64B_W_DATA4  0x90
+#define ASPEED_PECI_64B_W_DATA5  0x94
+#define ASPEED_PECI_64B_W_DATA6  0x98
+#define ASPEED_PECI_64B_W_DATA7  0x9C
+#define ASPEED_PECI_64B_W_DATA8  0xA0
+#define ASPEED_PECI_64B_W_DATA9  0xA4
+#define ASPEED_PECI_64B_W_DATAA  0xA8
+#define ASPEED_PECI_64B_W_DATAB  0xAC
+#define ASPEED_PECI_64B_W_DATAC  0xB0
+#define ASPEED_PECI_64B_W_DATAD  0xB4
+#define ASPEED_PECI_64B_W_DATAE  0xB8
+#define ASPEED_PECI_64B_W_DATAF  0xBC
+#define ASPEED_PECI_64B_R_DATA0  0xC0
+#define ASPEED_PECI_64B_R_DATA1  0xC4
+#define ASPEED_PECI_64B_R_DATA2  0xC8
+#define ASPEED_PECI_64B_R_DATA3  0xCC
+#define ASPEED_PECI_64B_R_DATA4  0xD0
+#define ASPEED_PECI_64B_R_DATA5  0xD4
+#define ASPEED_PECI_64B_R_DATA6  0xD8
+#define ASPEED_PECI_64B_R_DATA7  0xDC
+#define ASPEED_PECI_64B_R_DATA8  0xE0
+#define ASPEED_PECI_64B_R_DATA9  0xE4
+#define ASPEED_PECI_64B_R_DATAA  0xE8
+#define ASPEED_PECI_64B_R_DATAB  0xEC
+#define ASPEED_PECI_64B_R_DATAC  0xF0
+#define ASPEED_PECI_64B_R_DATAD  0xF4
+#define ASPEED_PECI_64B_R_DATAE  0xF8
+#define ASPEED_PECI_64B_R_DATAF  0xFC
 
 /* Timing Negotiation */
 #define ASPEED_PECI_RD_SAMPLING_POINT_DEFAULT	8
@@ -125,6 +160,8 @@ struct aspeed_peci {
 	struct completion	xfer_complete;
 	u32			status;
 	u32			cmd_timeout_ms;
+	/* 0: older 32 bytes, 1 : 64bytes mode */
+	int			xfer_mode;
 };
 
 static inline int aspeed_peci_check_idle(struct aspeed_peci *priv)
@@ -144,12 +181,13 @@ static int aspeed_peci_xfer(struct peci_adapter *adapter,
 	struct aspeed_peci *priv = peci_get_adapdata(adapter);
 	long err, timeout = msecs_to_jiffies(priv->cmd_timeout_ms);
 	u32 peci_head, peci_state, rx_data = 0;
+	u32 max_buffer_size = (priv->xfer_mode) ? 64 : 32;
 	ulong flags;
 	int i, ret;
 	uint reg;
 
-	if (msg->tx_len > ASPEED_PECI_DATA_BUF_SIZE_MAX ||
-	    msg->rx_len > ASPEED_PECI_DATA_BUF_SIZE_MAX)
+	if (msg->tx_len > max_buffer_size ||
+	    msg->rx_len > max_buffer_size)
 		return -EINVAL;
 
 	/* Check command sts and bus idle state */
@@ -167,8 +205,11 @@ static int aspeed_peci_xfer(struct peci_adapter *adapter,
 	writel(peci_head, priv->base + ASPEED_PECI_RW_LENGTH);
 
 	for (i = 0; i < msg->tx_len; i += 4) {
-		reg = i < 16 ? ASPEED_PECI_W_DATA0 + i % 16 :
-			       ASPEED_PECI_W_DATA4 + i % 16;
+		if (priv->xfer_mode)
+			reg = ASPEED_PECI_64B_W_DATA0 + i % 16;
+		else
+			reg = i < 16 ? ASPEED_PECI_W_DATA0 + i % 16 :
+				       ASPEED_PECI_W_DATA4 + i % 16;
 		writel(le32_to_cpup((__le32 *)&msg->tx_buf[i]),
 		       priv->base + reg);
 	}
@@ -215,8 +256,11 @@ static int aspeed_peci_xfer(struct peci_adapter *adapter,
 		u8 byte_offset = i % 4;
 
 		if (byte_offset == 0) {
-			reg = i < 16 ? ASPEED_PECI_R_DATA0 + i % 16 :
-				       ASPEED_PECI_R_DATA4 + i % 16;
+			if (priv->xfer_mode)
+				reg = ASPEED_PECI_64B_R_DATA0 + i % 16;
+			else
+				reg = i < 16 ? ASPEED_PECI_R_DATA0 + i % 16 :
+					ASPEED_PECI_R_DATA4 + i % 16;
 			rx_data = readl(priv->base + reg);
 		}
 
@@ -349,10 +393,8 @@ static int aspeed_peci_init_ctrl(struct aspeed_peci *priv)
 		priv->cmd_timeout_ms = ASPEED_PECI_CMD_TIMEOUT_MS_DEFAULT;
 	}
 
-	writel(FIELD_PREP(ASPEED_PECI_CTRL_CLK_DIV_MASK,
-			  ASPEED_PECI_CLK_DIV_DEFAULT) |
-	       ASPEED_PECI_CTRL_PECI_CLK_EN, priv->base + ASPEED_PECI_CTRL);
-
+	if (of_property_read_bool(priv->dev->of_node, "64byte-mode"))
+		priv->xfer_mode = 1;
 	/*
 	 * Timing negotiation period setting.
 	 * The unit of the programmed value is 4 times of PECI clock period.
@@ -373,6 +415,7 @@ static int aspeed_peci_init_ctrl(struct aspeed_peci *priv)
 	/* Read sampling point and clock speed setting */
 	writel(FIELD_PREP(ASPEED_PECI_CTRL_SAMPLING_MASK, rd_sampling_point) |
 	       FIELD_PREP(ASPEED_PECI_CTRL_CLK_DIV_MASK, clk_div_val) |
+	       (priv->xfer_mode ? ASPEED_PECI_CTRL_64BYTE_MODE_EN : 0) |
 	       ASPEED_PECI_CTRL_PECI_EN | ASPEED_PECI_CTRL_PECI_CLK_EN,
 	       priv->base + ASPEED_PECI_CTRL);
 
-- 
2.17.1


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

* [PATCH 2/3] peci: aspeed: Auto calculate the adapter divisor
  2020-10-16  6:25 [PATCH 0/3] Extend peci feature Billy Tsai
  2020-10-16  6:26 ` [PATCH 1/3] peci: aspeed: make the driver support 64byte mode Billy Tsai
@ 2020-10-16  6:26 ` Billy Tsai
  2020-10-28  4:40   ` Joel Stanley
  2020-10-16  6:26 ` [PATCH 3/3] dt-bindings: Change the meaning of clock-frequency Billy Tsai
  2 siblings, 1 reply; 6+ messages in thread
From: Billy Tsai @ 2020-10-16  6:26 UTC (permalink / raw)
  To: robh+dt, joel, andrew, jae.hyun.yoo, billy_tsai, haiyue.wang,
	james.feist, vernon.mauery, devicetree, linux-arm-kernel,
	linux-aspeed
  Cc: BMC-SW

This pach change the meaning of clk-frequency property from original
controller clock to bit frequency of peci negotiation stage and auto
calculate the adapter divisor setting to close aim.
The expected frequency and real frequency may have errors because of the
granularities of the divisor.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-g6.dtsi  |  4 +-
 drivers/peci/busses/peci-aspeed.c | 91 ++++++++++++++++++-------------
 2 files changed, 53 insertions(+), 42 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
index cb053a996e87..6e1e5b5733e6 100644
--- a/arch/arm/boot/dts/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6.dtsi
@@ -750,9 +750,7 @@
 		interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&syscon ASPEED_CLK_GATE_REF0CLK>;
 		resets = <&syscon ASPEED_RESET_PECI>;
-		clock-frequency = <24000000>;
-		msg-timing = <1>;
-		addr-timing = <1>;
+		clock-frequency = <1000000>;
 		rd-sampling-point = <8>;
 		cmd-timeout-ms = <1000>;
 		status = "disabled";
diff --git a/drivers/peci/busses/peci-aspeed.c b/drivers/peci/busses/peci-aspeed.c
index d6039b1c4494..9e7c7582e4bb 100644
--- a/drivers/peci/busses/peci-aspeed.c
+++ b/drivers/peci/busses/peci-aspeed.c
@@ -133,6 +133,11 @@
 #define ASPEED_PECI_64B_R_DATAE  0xF8
 #define ASPEED_PECI_64B_R_DATAF  0xFC
 
+/* Bus Frequency */
+#define ASPEED_PECI_BUS_FREQ_MAX	2000000
+#define ASPEED_PECI_BUS_FREQ_MIN	2000
+#define ASPEED_PECI_BUS_FREQ_DEFAULT	1000000
+
 /* Timing Negotiation */
 #define ASPEED_PECI_RD_SAMPLING_POINT_DEFAULT	8
 #define ASPEED_PECI_RD_SAMPLING_POINT_MAX	15
@@ -324,51 +329,47 @@ static irqreturn_t aspeed_peci_irq_handler(int irq, void *arg)
 static int aspeed_peci_init_ctrl(struct aspeed_peci *priv)
 {
 	u32 msg_timing, addr_timing, rd_sampling_point;
-	u32 clk_freq, clk_divisor, clk_div_val = 0;
+	u32 clk_freq, clk_div_val = 0;
+	u32 msg_timing_idx, clk_div_val_idx;
+	int delta_value, delta_tmp, clk_divisor, clk_divisor_tmp;
 	int ret;
 
-	priv->clk = devm_clk_get(priv->dev, NULL);
-	if (IS_ERR(priv->clk)) {
-		dev_err(priv->dev, "Failed to get clk source.\n");
-		return PTR_ERR(priv->clk);
-	}
-
-	ret = clk_prepare_enable(priv->clk);
-	if (ret) {
-		dev_err(priv->dev, "Failed to enable clock.\n");
-		return ret;
-	}
-
 	ret = device_property_read_u32(priv->dev, "clock-frequency", &clk_freq);
-	if (ret) {
-		dev_err(priv->dev,
-			"Could not read clock-frequency property.\n");
-		clk_disable_unprepare(priv->clk);
-		return ret;
-	}
-
-	clk_divisor = clk_get_rate(priv->clk) / clk_freq;
-
-	while ((clk_divisor >> 1) && (clk_div_val < ASPEED_PECI_CLK_DIV_MAX))
-		clk_div_val++;
-
-	ret = device_property_read_u32(priv->dev, "msg-timing", &msg_timing);
-	if (ret || msg_timing > ASPEED_PECI_MSG_TIMING_MAX) {
+	if (ret ||
+	clk_freq > ASPEED_PECI_BUS_FREQ_MAX ||
+	clk_freq < ASPEED_PECI_BUS_FREQ_MIN) {
 		if (!ret)
 			dev_warn(priv->dev,
-				 "Invalid msg-timing : %u, Use default : %u\n",
-				 msg_timing, ASPEED_PECI_MSG_TIMING_DEFAULT);
-		msg_timing = ASPEED_PECI_MSG_TIMING_DEFAULT;
-	}
-
-	ret = device_property_read_u32(priv->dev, "addr-timing", &addr_timing);
-	if (ret || addr_timing > ASPEED_PECI_ADDR_TIMING_MAX) {
-		if (!ret)
-			dev_warn(priv->dev,
-				 "Invalid addr-timing : %u, Use default : %u\n",
-				 addr_timing, ASPEED_PECI_ADDR_TIMING_DEFAULT);
-		addr_timing = ASPEED_PECI_ADDR_TIMING_DEFAULT;
+				 "Invalid clock-frequency : %u, Use default : %u\n",
+				 clk_freq, ASPEED_PECI_BUS_FREQ_DEFAULT);
+		clk_freq = ASPEED_PECI_BUS_FREQ_DEFAULT;
 	}
+	/*
+	 * PECI bus clock = (Ref. clk) / (1 << PECI00[10:8])
+	 * PECI operation clock = (PECI bus clock)/ 4*(PECI04[15:8]*4+1)
+	 * (1 << PECI00[10:8]) * (PECI04[15:8]*4+1) =
+	 * (Ref. clk) / (4 * PECI operation clock)
+	 */
+	clk_divisor = clk_get_rate(priv->clk) / (4*clk_freq);
+	delta_value = clk_divisor;
+	/* Find the closest divisor for clock-frequency */
+	for (msg_timing_idx = 1; msg_timing_idx <= 255; msg_timing_idx++)
+		for (clk_div_val_idx = 0; clk_div_val_idx < 7;
+			clk_div_val_idx++) {
+			clk_divisor_tmp = (1 << clk_div_val_idx) *
+					(msg_timing_idx * 4 + 1);
+			delta_tmp = abs(clk_divisor - clk_divisor_tmp);
+			if (delta_tmp < delta_value) {
+				delta_value = delta_tmp;
+				msg_timing = msg_timing_idx;
+				clk_div_val = clk_div_val_idx;
+			}
+		}
+	addr_timing = msg_timing;
+	dev_info(priv->dev, "Expect frequency: %d Real frequency is about: %lu",
+		clk_freq,
+		clk_get_rate(priv->clk) /
+		(4 * (1 << clk_div_val) * (msg_timing * 4 + 1)));
 
 	ret = device_property_read_u32(priv->dev, "rd-sampling-point",
 				       &rd_sampling_point);
@@ -463,6 +464,18 @@ static int aspeed_peci_probe(struct platform_device *pdev)
 	priv->adapter->xfer = aspeed_peci_xfer;
 	priv->adapter->use_dma = false;
 
+	priv->clk = devm_clk_get(priv->dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		dev_err(priv->dev, "Failed to get clk source.\n");
+		return PTR_ERR(priv->clk);
+	}
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(priv->dev, "Failed to enable clock.\n");
+		return ret;
+	}
+
 	priv->rst = devm_reset_control_get(&pdev->dev, NULL);
 	if (IS_ERR(priv->rst)) {
 		dev_err(&pdev->dev,
-- 
2.17.1


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

* [PATCH 3/3] dt-bindings: Change the meaning of clock-frequency
  2020-10-16  6:25 [PATCH 0/3] Extend peci feature Billy Tsai
  2020-10-16  6:26 ` [PATCH 1/3] peci: aspeed: make the driver support 64byte mode Billy Tsai
  2020-10-16  6:26 ` [PATCH 2/3] peci: aspeed: Auto calculate the adapter divisor Billy Tsai
@ 2020-10-16  6:26 ` Billy Tsai
  2020-10-19 21:22   ` Rob Herring
  2 siblings, 1 reply; 6+ messages in thread
From: Billy Tsai @ 2020-10-16  6:26 UTC (permalink / raw)
  To: robh+dt, joel, andrew, jae.hyun.yoo, billy_tsai, haiyue.wang,
	james.feist, vernon.mauery, devicetree, linux-arm-kernel,
	linux-aspeed
  Cc: BMC-SW

Integration of the usage of msg-timing and addr-timing to clock-frequency.
User can just set it to adjust the peci work efficient.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 .../devicetree/bindings/peci/peci-aspeed.yaml | 56 +++++++++----------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/Documentation/devicetree/bindings/peci/peci-aspeed.yaml b/Documentation/devicetree/bindings/peci/peci-aspeed.yaml
index 0f5c2993fe9b..7c6c895784af 100644
--- a/Documentation/devicetree/bindings/peci/peci-aspeed.yaml
+++ b/Documentation/devicetree/bindings/peci/peci-aspeed.yaml
@@ -40,31 +40,33 @@ properties:
     maxItems: 1
 
   clock-frequency:
-    # Operation frequency of PECI controller in units of Hz.
-    minimum: 187500
-    maximum: 24000000
-
-  msg-timing:
-    description: |
-      Message timing negotiation period. This value will determine the period
-      of message timing negotiation to be issued by PECI controller. The unit
-      of the programmed value is four times of PECI clock period.
-    allOf:
-      - $ref: /schemas/types.yaml#/definitions/uint32
-      - minimum: 0
-        maximum: 255
-        default: 1
-
-  addr-timing:
-    description: |
-      Address timing negotiation period. This value will determine the period
-      of address timing negotiation to be issued by PECI controller. The unit
-      of the programmed value is four times of PECI clock period.
-    allOf:
-      - $ref: /schemas/types.yaml#/definitions/uint32
-      - minimum: 0
-        maximum: 255
-        default: 1
+    # The bit frequency of PECI negotiation stage in units of Hz.
+    # Driver will calculate the best divisor setting of msg-timing and
+    # addr-timing to meet the value.
+    minimum: 2000
+    maximum: 2000000
+
+  # msg-timing:
+  #   description: |
+  #     Message timing negotiation period. This value will determine the period
+  #     of message timing negotiation to be issued by PECI controller. The unit
+  #     of the programmed value is four times of PECI clock period.
+  #   allOf:
+  #     - $ref: /schemas/types.yaml#/definitions/uint32
+  #     - minimum: 0
+  #       maximum: 255
+  #       default: 1
+
+  # addr-timing:
+  #   description: |
+  #     Address timing negotiation period. This value will determine the period
+  #     of address timing negotiation to be issued by PECI controller. The unit
+  #     of the programmed value is four times of PECI clock period.
+  #   allOf:
+  #     - $ref: /schemas/types.yaml#/definitions/uint32
+  #     - minimum: 0
+  #       maximum: 255
+  #       default: 1
 
   rd-sampling-point:
     description: |
@@ -114,9 +116,7 @@ examples:
             interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
             clocks = <&syscon ASPEED_CLK_GATE_REF0CLK>;
             resets = <&syscon ASPEED_RESET_PECI>;
-            clock-frequency = <24000000>;
-            msg-timing = <1>;
-            addr-timing = <1>;
+            clock-frequency = <2000000>;
             rd-sampling-point = <8>;
             cmd-timeout-ms = <1000>;
         };
-- 
2.17.1


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

* Re: [PATCH 3/3] dt-bindings: Change the meaning of clock-frequency
  2020-10-16  6:26 ` [PATCH 3/3] dt-bindings: Change the meaning of clock-frequency Billy Tsai
@ 2020-10-19 21:22   ` Rob Herring
  0 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2020-10-19 21:22 UTC (permalink / raw)
  To: Billy Tsai
  Cc: joel, andrew, jae.hyun.yoo, haiyue.wang, james.feist,
	vernon.mauery, devicetree, linux-arm-kernel, linux-aspeed,
	BMC-SW

On Fri, Oct 16, 2020 at 02:26:02PM +0800, Billy Tsai wrote:
> Integration of the usage of msg-timing and addr-timing to clock-frequency.
> User can just set it to adjust the peci work efficient.

You're breaking the ABI changing this.

> 
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
>  .../devicetree/bindings/peci/peci-aspeed.yaml | 56 +++++++++----------
>  1 file changed, 28 insertions(+), 28 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/peci/peci-aspeed.yaml b/Documentation/devicetree/bindings/peci/peci-aspeed.yaml
> index 0f5c2993fe9b..7c6c895784af 100644
> --- a/Documentation/devicetree/bindings/peci/peci-aspeed.yaml
> +++ b/Documentation/devicetree/bindings/peci/peci-aspeed.yaml
> @@ -40,31 +40,33 @@ properties:
>      maxItems: 1
>  
>    clock-frequency:
> -    # Operation frequency of PECI controller in units of Hz.
> -    minimum: 187500
> -    maximum: 24000000
> -
> -  msg-timing:
> -    description: |
> -      Message timing negotiation period. This value will determine the period
> -      of message timing negotiation to be issued by PECI controller. The unit
> -      of the programmed value is four times of PECI clock period.
> -    allOf:
> -      - $ref: /schemas/types.yaml#/definitions/uint32
> -      - minimum: 0
> -        maximum: 255
> -        default: 1
> -
> -  addr-timing:
> -    description: |
> -      Address timing negotiation period. This value will determine the period
> -      of address timing negotiation to be issued by PECI controller. The unit
> -      of the programmed value is four times of PECI clock period.
> -    allOf:
> -      - $ref: /schemas/types.yaml#/definitions/uint32
> -      - minimum: 0
> -        maximum: 255
> -        default: 1
> +    # The bit frequency of PECI negotiation stage in units of Hz.
> +    # Driver will calculate the best divisor setting of msg-timing and
> +    # addr-timing to meet the value.

Use 'description' rather than comments.

> +    minimum: 2000
> +    maximum: 2000000
> +
> +  # msg-timing:
> +  #   description: |
> +  #     Message timing negotiation period. This value will determine the period
> +  #     of message timing negotiation to be issued by PECI controller. The unit
> +  #     of the programmed value is four times of PECI clock period.
> +  #   allOf:
> +  #     - $ref: /schemas/types.yaml#/definitions/uint32
> +  #     - minimum: 0
> +  #       maximum: 255
> +  #       default: 1

Why is this commented out?

> +
> +  # addr-timing:
> +  #   description: |
> +  #     Address timing negotiation period. This value will determine the period
> +  #     of address timing negotiation to be issued by PECI controller. The unit
> +  #     of the programmed value is four times of PECI clock period.
> +  #   allOf:
> +  #     - $ref: /schemas/types.yaml#/definitions/uint32
> +  #     - minimum: 0
> +  #       maximum: 255
> +  #       default: 1
>  
>    rd-sampling-point:
>      description: |
> @@ -114,9 +116,7 @@ examples:
>              interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
>              clocks = <&syscon ASPEED_CLK_GATE_REF0CLK>;
>              resets = <&syscon ASPEED_RESET_PECI>;
> -            clock-frequency = <24000000>;
> -            msg-timing = <1>;
> -            addr-timing = <1>;
> +            clock-frequency = <2000000>;
>              rd-sampling-point = <8>;
>              cmd-timeout-ms = <1000>;
>          };
> -- 
> 2.17.1
> 

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

* Re: [PATCH 2/3] peci: aspeed: Auto calculate the adapter divisor
  2020-10-16  6:26 ` [PATCH 2/3] peci: aspeed: Auto calculate the adapter divisor Billy Tsai
@ 2020-10-28  4:40   ` Joel Stanley
  0 siblings, 0 replies; 6+ messages in thread
From: Joel Stanley @ 2020-10-28  4:40 UTC (permalink / raw)
  To: Billy Tsai
  Cc: Rob Herring, Andrew Jeffery, Jae Hyun Yoo, Haiyue Wang,
	James Feist, Vernon Mauery, devicetree, Linux ARM, linux-aspeed,
	BMC-SW

Hi Billy,

On Fri, 16 Oct 2020 at 06:26, Billy Tsai <billy_tsai@aspeedtech.com> wrote:
>
> This pach change the meaning of clk-frequency property from original
> controller clock to bit frequency of peci negotiation stage and auto
> calculate the adapter divisor setting to close aim.
> The expected frequency and real frequency may have errors because of the
> granularities of the divisor.

This patch can't go to the mainline lists as the authors (Intel) have
not yet had it merged.

Cheers,

Joel

>
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
>  arch/arm/boot/dts/aspeed-g6.dtsi  |  4 +-
>  drivers/peci/busses/peci-aspeed.c | 91 ++++++++++++++++++-------------
>  2 files changed, 53 insertions(+), 42 deletions(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
> index cb053a996e87..6e1e5b5733e6 100644
> --- a/arch/arm/boot/dts/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g6.dtsi
> @@ -750,9 +750,7 @@
>                 interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
>                 clocks = <&syscon ASPEED_CLK_GATE_REF0CLK>;
>                 resets = <&syscon ASPEED_RESET_PECI>;
> -               clock-frequency = <24000000>;
> -               msg-timing = <1>;
> -               addr-timing = <1>;
> +               clock-frequency = <1000000>;
>                 rd-sampling-point = <8>;
>                 cmd-timeout-ms = <1000>;
>                 status = "disabled";
> diff --git a/drivers/peci/busses/peci-aspeed.c b/drivers/peci/busses/peci-aspeed.c
> index d6039b1c4494..9e7c7582e4bb 100644
> --- a/drivers/peci/busses/peci-aspeed.c
> +++ b/drivers/peci/busses/peci-aspeed.c
> @@ -133,6 +133,11 @@
>  #define ASPEED_PECI_64B_R_DATAE  0xF8
>  #define ASPEED_PECI_64B_R_DATAF  0xFC
>
> +/* Bus Frequency */
> +#define ASPEED_PECI_BUS_FREQ_MAX       2000000
> +#define ASPEED_PECI_BUS_FREQ_MIN       2000
> +#define ASPEED_PECI_BUS_FREQ_DEFAULT   1000000
> +
>  /* Timing Negotiation */
>  #define ASPEED_PECI_RD_SAMPLING_POINT_DEFAULT  8
>  #define ASPEED_PECI_RD_SAMPLING_POINT_MAX      15
> @@ -324,51 +329,47 @@ static irqreturn_t aspeed_peci_irq_handler(int irq, void *arg)
>  static int aspeed_peci_init_ctrl(struct aspeed_peci *priv)
>  {
>         u32 msg_timing, addr_timing, rd_sampling_point;
> -       u32 clk_freq, clk_divisor, clk_div_val = 0;
> +       u32 clk_freq, clk_div_val = 0;
> +       u32 msg_timing_idx, clk_div_val_idx;
> +       int delta_value, delta_tmp, clk_divisor, clk_divisor_tmp;
>         int ret;
>
> -       priv->clk = devm_clk_get(priv->dev, NULL);
> -       if (IS_ERR(priv->clk)) {
> -               dev_err(priv->dev, "Failed to get clk source.\n");
> -               return PTR_ERR(priv->clk);
> -       }
> -
> -       ret = clk_prepare_enable(priv->clk);
> -       if (ret) {
> -               dev_err(priv->dev, "Failed to enable clock.\n");
> -               return ret;
> -       }
> -
>         ret = device_property_read_u32(priv->dev, "clock-frequency", &clk_freq);
> -       if (ret) {
> -               dev_err(priv->dev,
> -                       "Could not read clock-frequency property.\n");
> -               clk_disable_unprepare(priv->clk);
> -               return ret;
> -       }
> -
> -       clk_divisor = clk_get_rate(priv->clk) / clk_freq;
> -
> -       while ((clk_divisor >> 1) && (clk_div_val < ASPEED_PECI_CLK_DIV_MAX))
> -               clk_div_val++;
> -
> -       ret = device_property_read_u32(priv->dev, "msg-timing", &msg_timing);
> -       if (ret || msg_timing > ASPEED_PECI_MSG_TIMING_MAX) {
> +       if (ret ||
> +       clk_freq > ASPEED_PECI_BUS_FREQ_MAX ||
> +       clk_freq < ASPEED_PECI_BUS_FREQ_MIN) {
>                 if (!ret)
>                         dev_warn(priv->dev,
> -                                "Invalid msg-timing : %u, Use default : %u\n",
> -                                msg_timing, ASPEED_PECI_MSG_TIMING_DEFAULT);
> -               msg_timing = ASPEED_PECI_MSG_TIMING_DEFAULT;
> -       }
> -
> -       ret = device_property_read_u32(priv->dev, "addr-timing", &addr_timing);
> -       if (ret || addr_timing > ASPEED_PECI_ADDR_TIMING_MAX) {
> -               if (!ret)
> -                       dev_warn(priv->dev,
> -                                "Invalid addr-timing : %u, Use default : %u\n",
> -                                addr_timing, ASPEED_PECI_ADDR_TIMING_DEFAULT);
> -               addr_timing = ASPEED_PECI_ADDR_TIMING_DEFAULT;
> +                                "Invalid clock-frequency : %u, Use default : %u\n",
> +                                clk_freq, ASPEED_PECI_BUS_FREQ_DEFAULT);
> +               clk_freq = ASPEED_PECI_BUS_FREQ_DEFAULT;
>         }
> +       /*
> +        * PECI bus clock = (Ref. clk) / (1 << PECI00[10:8])
> +        * PECI operation clock = (PECI bus clock)/ 4*(PECI04[15:8]*4+1)
> +        * (1 << PECI00[10:8]) * (PECI04[15:8]*4+1) =
> +        * (Ref. clk) / (4 * PECI operation clock)
> +        */
> +       clk_divisor = clk_get_rate(priv->clk) / (4*clk_freq);
> +       delta_value = clk_divisor;
> +       /* Find the closest divisor for clock-frequency */
> +       for (msg_timing_idx = 1; msg_timing_idx <= 255; msg_timing_idx++)
> +               for (clk_div_val_idx = 0; clk_div_val_idx < 7;
> +                       clk_div_val_idx++) {
> +                       clk_divisor_tmp = (1 << clk_div_val_idx) *
> +                                       (msg_timing_idx * 4 + 1);
> +                       delta_tmp = abs(clk_divisor - clk_divisor_tmp);
> +                       if (delta_tmp < delta_value) {
> +                               delta_value = delta_tmp;
> +                               msg_timing = msg_timing_idx;
> +                               clk_div_val = clk_div_val_idx;
> +                       }
> +               }
> +       addr_timing = msg_timing;
> +       dev_info(priv->dev, "Expect frequency: %d Real frequency is about: %lu",
> +               clk_freq,
> +               clk_get_rate(priv->clk) /
> +               (4 * (1 << clk_div_val) * (msg_timing * 4 + 1)));
>
>         ret = device_property_read_u32(priv->dev, "rd-sampling-point",
>                                        &rd_sampling_point);
> @@ -463,6 +464,18 @@ static int aspeed_peci_probe(struct platform_device *pdev)
>         priv->adapter->xfer = aspeed_peci_xfer;
>         priv->adapter->use_dma = false;
>
> +       priv->clk = devm_clk_get(priv->dev, NULL);
> +       if (IS_ERR(priv->clk)) {
> +               dev_err(priv->dev, "Failed to get clk source.\n");
> +               return PTR_ERR(priv->clk);
> +       }
> +
> +       ret = clk_prepare_enable(priv->clk);
> +       if (ret) {
> +               dev_err(priv->dev, "Failed to enable clock.\n");
> +               return ret;
> +       }
> +
>         priv->rst = devm_reset_control_get(&pdev->dev, NULL);
>         if (IS_ERR(priv->rst)) {
>                 dev_err(&pdev->dev,
> --
> 2.17.1
>

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

end of thread, other threads:[~2020-10-28 23:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-16  6:25 [PATCH 0/3] Extend peci feature Billy Tsai
2020-10-16  6:26 ` [PATCH 1/3] peci: aspeed: make the driver support 64byte mode Billy Tsai
2020-10-16  6:26 ` [PATCH 2/3] peci: aspeed: Auto calculate the adapter divisor Billy Tsai
2020-10-28  4:40   ` Joel Stanley
2020-10-16  6:26 ` [PATCH 3/3] dt-bindings: Change the meaning of clock-frequency Billy Tsai
2020-10-19 21:22   ` Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).