All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH linux dev-5.3 0/4] Pinmux fixes for AST2600 LPC
@ 2019-10-22  4:47 Andrew Jeffery
  2019-10-22  4:47 ` [PATCH linux dev-5.3 1/4] pinctrl: aspeed-g6: Make SIG_DESC_CLEAR() behave intuitively Andrew Jeffery
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Andrew Jeffery @ 2019-10-22  4:47 UTC (permalink / raw)
  To: joel; +Cc: openbmc

Hello,

This series fixes some issues with the AST2600 pinctrl driver.

Patch 1/4 backports an existing upstream fix. Patch 4/4 simply improves the
readability of the debug output of the driver. Patches 2 and 3 fix issues with
muxing pins for LPC, particularly for the Tacoma board.

Please review.

Andrew

Andrew Jeffery (4):
  pinctrl: aspeed-g6: Make SIG_DESC_CLEAR() behave intuitively
  pinctrl: aspeed-g6: Fix LPC/eSPI mux configuration
  ARM: dts: tacoma: Hog LPC pinmux
  pinctrl: aspeed: Improve debug output

 arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts |  7 +++
 drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c  | 56 +++++++++------------
 drivers/pinctrl/aspeed/pinctrl-aspeed.c     | 25 +++++++--
 drivers/pinctrl/aspeed/pinmux-aspeed.h      |  2 +-
 4 files changed, 54 insertions(+), 36 deletions(-)

-- 
2.20.1

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

* [PATCH linux dev-5.3 1/4] pinctrl: aspeed-g6: Make SIG_DESC_CLEAR() behave intuitively
  2019-10-22  4:47 [PATCH linux dev-5.3 0/4] Pinmux fixes for AST2600 LPC Andrew Jeffery
@ 2019-10-22  4:47 ` Andrew Jeffery
  2019-10-22  4:47 ` [PATCH linux dev-5.3 2/4] pinctrl: aspeed-g6: Fix LPC/eSPI mux configuration Andrew Jeffery
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2019-10-22  4:47 UTC (permalink / raw)
  To: joel; +Cc: openbmc

Signal descriptors can represent multi-bit bitfields and so have
explicit "enable" and "disable" states. However many descriptor
instances only describe a single bit, and so the SIG_DESC_SET() macro is
provides an abstraction for the single-bit cases: Its expansion
configures the "enable" state to set the bit and "disable" to clear.

SIG_DESC_CLEAR() was introduced to provide a similar single-bit
abstraction for for descriptors to clear the bit of interest. However
its behaviour was defined as the literal inverse of SIG_DESC_SET() - the
impact is the bit of interest is set in the disable path. This behaviour
isn't intuitive and doesn't align with how we want to use the macro in
practice, so make it clear the bit for both the enable and disable
paths.

(cherry-picked from commit c136d4c71f755a189fe13a0cd4f3e8f538dda567)
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 drivers/pinctrl/aspeed/pinmux-aspeed.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/aspeed/pinmux-aspeed.h b/drivers/pinctrl/aspeed/pinmux-aspeed.h
index 31d903953c68..f86739e800c3 100644
--- a/drivers/pinctrl/aspeed/pinmux-aspeed.h
+++ b/drivers/pinctrl/aspeed/pinmux-aspeed.h
@@ -508,7 +508,7 @@ struct aspeed_pin_desc {
  * @idx: The bit index in the register
  */
 #define SIG_DESC_SET(reg, idx) SIG_DESC_IP_BIT(ASPEED_IP_SCU, reg, idx, 1)
-#define SIG_DESC_CLEAR(reg, idx) SIG_DESC_IP_BIT(ASPEED_IP_SCU, reg, idx, 0)
+#define SIG_DESC_CLEAR(reg, idx) { ASPEED_IP_SCU, reg, BIT_MASK(idx), 0, 0 }
 
 #define SIG_DESC_LIST_SYM(sig, group) sig_descs_ ## sig ## _ ## group
 #define SIG_DESC_LIST_DECL(sig, group, ...) \
-- 
2.20.1

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

* [PATCH linux dev-5.3 2/4] pinctrl: aspeed-g6: Fix LPC/eSPI mux configuration
  2019-10-22  4:47 [PATCH linux dev-5.3 0/4] Pinmux fixes for AST2600 LPC Andrew Jeffery
  2019-10-22  4:47 ` [PATCH linux dev-5.3 1/4] pinctrl: aspeed-g6: Make SIG_DESC_CLEAR() behave intuitively Andrew Jeffery
@ 2019-10-22  4:47 ` Andrew Jeffery
  2019-10-22 18:34   ` Jae Hyun Yoo
  2019-10-22  4:47 ` [PATCH linux dev-5.3 3/4] ARM: dts: tacoma: Hog LPC pinmux Andrew Jeffery
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Andrew Jeffery @ 2019-10-22  4:47 UTC (permalink / raw)
  To: joel; +Cc: openbmc

Early revisions of the AST2600 datasheet are conflicted about the state
of the LPC/eSPI strapping bit (SCU510[6]). Conversations with ASPEED
determined that the reference pinmux configuration tables were in error
and the SCU documentation contained the correct configuration. Update
the driver to reflect the state described in the SCU documentation.

Fixes: 2eda1cdec49f ("pinctrl: aspeed: Add AST2600 pinmux support")
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 56 ++++++++++------------
 1 file changed, 24 insertions(+), 32 deletions(-)

diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
index eb0c11a9fbf2..fb96e8d2e6c8 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
@@ -1098,61 +1098,53 @@ SSSF_PIN_DECL(AD15, GPIOV6, LPCPME, SIG_DESC_SET(SCU434, 14));
 SSSF_PIN_DECL(AF15, GPIOV7, LPCSMI, SIG_DESC_SET(SCU434, 15));
 
 #define AB7 176
-SIG_EXPR_LIST_DECL_SESG(AB7, LAD0, LPC, SIG_DESC_SET(SCU434, 16),
-			  SIG_DESC_CLEAR(SCU510, 6));
-SIG_EXPR_LIST_DECL_SESG(AB7, ESPID0, ESPI, SIG_DESC_SET(SCU434, 16),
-			  SIG_DESC_SET(SCU510, 6));
+SIG_EXPR_LIST_DECL_SESG(AB7, LAD0, LPC, SIG_DESC_SET(SCU510, 6),
+			SIG_DESC_SET(SCU434, 16));
+SIG_EXPR_LIST_DECL_SESG(AB7, ESPID0, ESPI, SIG_DESC_SET(SCU434, 16));
 PIN_DECL_2(AB7, GPIOW0, LAD0, ESPID0);
 
 #define AB8 177
-SIG_EXPR_LIST_DECL_SESG(AB8, LAD1, LPC, SIG_DESC_SET(SCU434, 17),
-			  SIG_DESC_CLEAR(SCU510, 6));
-SIG_EXPR_LIST_DECL_SESG(AB8, ESPID1, ESPI, SIG_DESC_SET(SCU434, 17),
-			  SIG_DESC_SET(SCU510, 6));
+SIG_EXPR_LIST_DECL_SESG(AB8, LAD1, LPC, SIG_DESC_SET(SCU510, 6),
+			SIG_DESC_SET(SCU434, 17));
+SIG_EXPR_LIST_DECL_SESG(AB8, ESPID1, ESPI, SIG_DESC_SET(SCU434, 17));
 PIN_DECL_2(AB8, GPIOW1, LAD1, ESPID1);
 
 #define AC8 178
-SIG_EXPR_LIST_DECL_SESG(AC8, LAD2, LPC, SIG_DESC_SET(SCU434, 18),
-			  SIG_DESC_CLEAR(SCU510, 6));
-SIG_EXPR_LIST_DECL_SESG(AC8, ESPID2, ESPI, SIG_DESC_SET(SCU434, 18),
-			  SIG_DESC_SET(SCU510, 6));
+SIG_EXPR_LIST_DECL_SESG(AC8, LAD2, LPC, SIG_DESC_SET(SCU510, 6),
+			SIG_DESC_SET(SCU434, 18));
+SIG_EXPR_LIST_DECL_SESG(AC8, ESPID2, ESPI, SIG_DESC_SET(SCU434, 18));
 PIN_DECL_2(AC8, GPIOW2, LAD2, ESPID2);
 
 #define AC7 179
-SIG_EXPR_LIST_DECL_SESG(AC7, LAD3, LPC, SIG_DESC_SET(SCU434, 19),
-			  SIG_DESC_CLEAR(SCU510, 6));
-SIG_EXPR_LIST_DECL_SESG(AC7, ESPID3, ESPI, SIG_DESC_SET(SCU434, 19),
-			  SIG_DESC_SET(SCU510, 6));
+SIG_EXPR_LIST_DECL_SESG(AC7, LAD3, LPC, SIG_DESC_SET(SCU510, 6),
+			SIG_DESC_SET(SCU434, 19));
+SIG_EXPR_LIST_DECL_SESG(AC7, ESPID3, ESPI, SIG_DESC_SET(SCU434, 19));
 PIN_DECL_2(AC7, GPIOW3, LAD3, ESPID3);
 
 #define AE7 180
-SIG_EXPR_LIST_DECL_SESG(AE7, LCLK, LPC, SIG_DESC_SET(SCU434, 20),
-			  SIG_DESC_CLEAR(SCU510, 6));
-SIG_EXPR_LIST_DECL_SESG(AE7, ESPICK, ESPI, SIG_DESC_SET(SCU434, 20),
-			  SIG_DESC_SET(SCU510, 6));
+SIG_EXPR_LIST_DECL_SESG(AE7, LCLK, LPC, SIG_DESC_SET(SCU510, 6),
+			SIG_DESC_SET(SCU434, 20));
+SIG_EXPR_LIST_DECL_SESG(AE7, ESPICK, ESPI, SIG_DESC_SET(SCU434, 20));
 PIN_DECL_2(AE7, GPIOW4, LCLK, ESPICK);
 
 #define AF7 181
-SIG_EXPR_LIST_DECL_SESG(AF7, LFRAME, LPC, SIG_DESC_SET(SCU434, 21),
-			  SIG_DESC_CLEAR(SCU510, 6));
-SIG_EXPR_LIST_DECL_SESG(AF7, ESPICS, ESPI, SIG_DESC_SET(SCU434, 21),
-			  SIG_DESC_SET(SCU510, 6));
+SIG_EXPR_LIST_DECL_SESG(AF7, LFRAME, LPC, SIG_DESC_SET(SCU510, 6),
+			SIG_DESC_SET(SCU434, 21));
+SIG_EXPR_LIST_DECL_SESG(AF7, ESPICS, ESPI, SIG_DESC_SET(SCU434, 21));
 PIN_DECL_2(AF7, GPIOW5, LFRAME, ESPICS);
 
 #define AD7 182
-SIG_EXPR_LIST_DECL_SESG(AD7, LSIRQ, LSIRQ, SIG_DESC_SET(SCU434, 22),
-			  SIG_DESC_CLEAR(SCU510, 6));
-SIG_EXPR_LIST_DECL_SESG(AD7, ESPIALT, ESPIALT, SIG_DESC_SET(SCU434, 22),
-			  SIG_DESC_SET(SCU510, 6));
+SIG_EXPR_LIST_DECL_SESG(AD7, LSIRQ, LSIRQ, SIG_DESC_SET(SCU510, 6),
+			SIG_DESC_SET(SCU434, 22));
+SIG_EXPR_LIST_DECL_SESG(AD7, ESPIALT, ESPIALT, SIG_DESC_SET(SCU434, 22));
 PIN_DECL_2(AD7, GPIOW6, LSIRQ, ESPIALT);
 FUNC_GROUP_DECL(LSIRQ, AD7);
 FUNC_GROUP_DECL(ESPIALT, AD7);
 
 #define AD8 183
-SIG_EXPR_LIST_DECL_SESG(AD8, LPCRST, LPC, SIG_DESC_SET(SCU434, 23),
-			  SIG_DESC_CLEAR(SCU510, 6));
-SIG_EXPR_LIST_DECL_SESG(AD8, ESPIRST, ESPI, SIG_DESC_SET(SCU434, 23),
-			  SIG_DESC_SET(SCU510, 6));
+SIG_EXPR_LIST_DECL_SESG(AD8, LPCRST, LPC, SIG_DESC_SET(SCU510, 6),
+			SIG_DESC_SET(SCU434, 23));
+SIG_EXPR_LIST_DECL_SESG(AD8, ESPIRST, ESPI, SIG_DESC_SET(SCU434, 23));
 PIN_DECL_2(AD8, GPIOW7, LPCRST, ESPIRST);
 
 FUNC_GROUP_DECL(LPC, AB7, AB8, AC8, AC7, AE7, AF7, AD8);
-- 
2.20.1

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

* [PATCH linux dev-5.3 3/4] ARM: dts: tacoma: Hog LPC pinmux
  2019-10-22  4:47 [PATCH linux dev-5.3 0/4] Pinmux fixes for AST2600 LPC Andrew Jeffery
  2019-10-22  4:47 ` [PATCH linux dev-5.3 1/4] pinctrl: aspeed-g6: Make SIG_DESC_CLEAR() behave intuitively Andrew Jeffery
  2019-10-22  4:47 ` [PATCH linux dev-5.3 2/4] pinctrl: aspeed-g6: Fix LPC/eSPI mux configuration Andrew Jeffery
@ 2019-10-22  4:47 ` Andrew Jeffery
  2019-10-22  4:47 ` [PATCH linux dev-5.3 4/4] pinctrl: aspeed: Improve debug output Andrew Jeffery
  2019-10-23  3:23 ` [PATCH linux dev-5.3 0/4] Pinmux fixes for AST2600 LPC Joel Stanley
  4 siblings, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2019-10-22  4:47 UTC (permalink / raw)
  To: joel; +Cc: openbmc

Requesting pinmux configuration is done at driver probe time. The LPC IP
is composed of many sub-devices, each with their own driver, and no
driver exists for the entire IP block. Avoid having each sub-device
request the LPC pinmux by just hogging it in the pinctrl node.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts b/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
index bf363a735232..e51e8839bd56 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
@@ -792,3 +792,10 @@
 &wdt2 {
 	status = "okay";
 };
+
+&pinctrl {
+	/* Hog these as no driver is probed for the entire LPC block */
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_lpc_default>,
+		    <&pinctrl_lsirq_default>;
+};
-- 
2.20.1

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

* [PATCH linux dev-5.3 4/4] pinctrl: aspeed: Improve debug output
  2019-10-22  4:47 [PATCH linux dev-5.3 0/4] Pinmux fixes for AST2600 LPC Andrew Jeffery
                   ` (2 preceding siblings ...)
  2019-10-22  4:47 ` [PATCH linux dev-5.3 3/4] ARM: dts: tacoma: Hog LPC pinmux Andrew Jeffery
@ 2019-10-22  4:47 ` Andrew Jeffery
  2019-10-23  3:23 ` [PATCH linux dev-5.3 0/4] Pinmux fixes for AST2600 LPC Joel Stanley
  4 siblings, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2019-10-22  4:47 UTC (permalink / raw)
  To: joel; +Cc: openbmc

We need to iterate over each pin in a group for a function and
disable higher priority mux configurations on the pin before finally
muxing the relevant function's signal. With the current debug output it
is hard to track what register output is relevant to which operation, so
break up the actions in the debug output by providing some more context.

Before:

    [    5.446656] aspeed-g6-pinctrl 1e6e2000.syscon:pinctrl: request pin 37 (B26) for 1e780000.gpio:341
    [    5.447377] Want SCU414[0x00000020]=0x1, got 0x0 from 0x00000000
    [    5.447854] Want SCU4B4[0x00000020]=0x1, got 0x0 from 0x00000000
    [    5.448340] Want SCU4B4[0x00000020]=0x1, got 0x0 from 0x00000000

After:

    [    5.298053] Muxing pin 37 for GPIO
    [    5.298294] Disabling signal NRI4 for NRI4
    [    5.298593] Want SCU414[0x00000020]=0x1, got 0x0 from 0x00000000
    [    5.298983] Disabling signal RGMII4RXD1 for RGMII4
    [    5.299309] Want SCU4B4[0x00000020]=0x1, got 0x0 from 0x00000000
    [    5.299694] Disabling signal RMII4RXD1 for RMII4
    [    5.300014] Want SCU4B4[0x00000020]=0x1, got 0x0 from 0x00000000
    [    5.300396] Enabling signal GPIOE5 for GPIOE5
    [    5.300687] Muxed pin 37 as GPIOE5

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 drivers/pinctrl/aspeed/pinctrl-aspeed.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
index b625a657171e..53f3f8aec695 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
@@ -76,6 +76,9 @@ static int aspeed_sig_expr_enable(struct aspeed_pinmux_data *ctx,
 {
 	int ret;
 
+	pr_debug("Enabling signal %s for %s\n", expr->signal,
+		 expr->function);
+
 	ret = aspeed_sig_expr_eval(ctx, expr, true);
 	if (ret < 0)
 		return ret;
@@ -91,6 +94,9 @@ static int aspeed_sig_expr_disable(struct aspeed_pinmux_data *ctx,
 {
 	int ret;
 
+	pr_debug("Disabling signal %s for %s\n", expr->signal,
+		 expr->function);
+
 	ret = aspeed_sig_expr_eval(ctx, expr, true);
 	if (ret < 0)
 		return ret;
@@ -229,7 +235,7 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
 		const struct aspeed_sig_expr **funcs;
 		const struct aspeed_sig_expr ***prios;
 
-		pr_debug("Muxing pin %d for %s\n", pin, pfunc->name);
+		pr_debug("Muxing pin %s for %s\n", pdesc->name, pfunc->name);
 
 		if (!pdesc)
 			return -EINVAL;
@@ -269,6 +275,9 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
 		ret = aspeed_sig_expr_enable(&pdata->pinmux, expr);
 		if (ret)
 			return ret;
+
+		pr_debug("Muxed pin %s as %s for %s\n", pdesc->name, expr->signal,
+			 expr->function);
 	}
 
 	return 0;
@@ -317,6 +326,8 @@ int aspeed_gpio_request_enable(struct pinctrl_dev *pctldev,
 	if (!prios)
 		return -ENXIO;
 
+	pr_debug("Muxing pin %s for GPIO\n", pdesc->name);
+
 	/* Disable any functions of higher priority than GPIO */
 	while ((funcs = *prios)) {
 		if (aspeed_gpio_in_exprs(funcs))
@@ -346,14 +357,22 @@ int aspeed_gpio_request_enable(struct pinctrl_dev *pctldev,
 	 * lowest-priority signal type. As such it has no associated
 	 * expression.
 	 */
-	if (!expr)
+	if (!expr) {
+		pr_debug("Muxed pin %s as GPIO\n", pdesc->name);
 		return 0;
+	}
 
 	/*
 	 * If GPIO is not the lowest priority signal type, assume there is only
 	 * one expression defined to enable the GPIO function
 	 */
-	return aspeed_sig_expr_enable(&pdata->pinmux, expr);
+	ret = aspeed_sig_expr_enable(&pdata->pinmux, expr);
+	if (ret)
+		return ret;
+
+	pr_debug("Muxed pin %s as %s\n", pdesc->name, expr->signal);
+
+	return 0;
 }
 
 int aspeed_pinctrl_probe(struct platform_device *pdev,
-- 
2.20.1

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

* Re: [PATCH linux dev-5.3 2/4] pinctrl: aspeed-g6: Fix LPC/eSPI mux configuration
  2019-10-22  4:47 ` [PATCH linux dev-5.3 2/4] pinctrl: aspeed-g6: Fix LPC/eSPI mux configuration Andrew Jeffery
@ 2019-10-22 18:34   ` Jae Hyun Yoo
  2019-10-22 18:42     ` Jae Hyun Yoo
  0 siblings, 1 reply; 9+ messages in thread
From: Jae Hyun Yoo @ 2019-10-22 18:34 UTC (permalink / raw)
  To: Andrew Jeffery, joel; +Cc: openbmc

Hi Andrew,

On 10/21/2019 9:47 PM, Andrew Jeffery wrote:
> Early revisions of the AST2600 datasheet are conflicted about the state
> of the LPC/eSPI strapping bit (SCU510[6]). Conversations with ASPEED
> determined that the reference pinmux configuration tables were in error
> and the SCU documentation contained the correct configuration. Update
> the driver to reflect the state described in the SCU documentation.
> 
> Fixes: 2eda1cdec49f ("pinctrl: aspeed: Add AST2600 pinmux support")
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>   drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 56 ++++++++++------------
>   1 file changed, 24 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> index eb0c11a9fbf2..fb96e8d2e6c8 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> @@ -1098,61 +1098,53 @@ SSSF_PIN_DECL(AD15, GPIOV6, LPCPME, SIG_DESC_SET(SCU434, 14));
>   SSSF_PIN_DECL(AF15, GPIOV7, LPCSMI, SIG_DESC_SET(SCU434, 15));
>   
>   #define AB7 176
> -SIG_EXPR_LIST_DECL_SESG(AB7, LAD0, LPC, SIG_DESC_SET(SCU434, 16),
> -			  SIG_DESC_CLEAR(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AB7, ESPID0, ESPI, SIG_DESC_SET(SCU434, 16),
> -			  SIG_DESC_SET(SCU510, 6));
> +SIG_EXPR_LIST_DECL_SESG(AB7, LAD0, LPC, SIG_DESC_SET(SCU510, 6),
> +			SIG_DESC_SET(SCU434, 16));
> +SIG_EXPR_LIST_DECL_SESG(AB7, ESPID0, ESPI, SIG_DESC_SET(SCU434, 16));
>   PIN_DECL_2(AB7, GPIOW0, LAD0, ESPID0);
>   
>   #define AB8 177
> -SIG_EXPR_LIST_DECL_SESG(AB8, LAD1, LPC, SIG_DESC_SET(SCU434, 17),
> -			  SIG_DESC_CLEAR(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AB8, ESPID1, ESPI, SIG_DESC_SET(SCU434, 17),
> -			  SIG_DESC_SET(SCU510, 6));
> +SIG_EXPR_LIST_DECL_SESG(AB8, LAD1, LPC, SIG_DESC_SET(SCU510, 6),
> +			SIG_DESC_SET(SCU434, 17));
> +SIG_EXPR_LIST_DECL_SESG(AB8, ESPID1, ESPI, SIG_DESC_SET(SCU434, 17));
>   PIN_DECL_2(AB8, GPIOW1, LAD1, ESPID1);
>   
>   #define AC8 178
> -SIG_EXPR_LIST_DECL_SESG(AC8, LAD2, LPC, SIG_DESC_SET(SCU434, 18),
> -			  SIG_DESC_CLEAR(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AC8, ESPID2, ESPI, SIG_DESC_SET(SCU434, 18),
> -			  SIG_DESC_SET(SCU510, 6));
> +SIG_EXPR_LIST_DECL_SESG(AC8, LAD2, LPC, SIG_DESC_SET(SCU510, 6),
> +			SIG_DESC_SET(SCU434, 18));
> +SIG_EXPR_LIST_DECL_SESG(AC8, ESPID2, ESPI, SIG_DESC_SET(SCU434, 18));
>   PIN_DECL_2(AC8, GPIOW2, LAD2, ESPID2);
>   
>   #define AC7 179
> -SIG_EXPR_LIST_DECL_SESG(AC7, LAD3, LPC, SIG_DESC_SET(SCU434, 19),
> -			  SIG_DESC_CLEAR(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AC7, ESPID3, ESPI, SIG_DESC_SET(SCU434, 19),
> -			  SIG_DESC_SET(SCU510, 6));
> +SIG_EXPR_LIST_DECL_SESG(AC7, LAD3, LPC, SIG_DESC_SET(SCU510, 6),
> +			SIG_DESC_SET(SCU434, 19));
> +SIG_EXPR_LIST_DECL_SESG(AC7, ESPID3, ESPI, SIG_DESC_SET(SCU434, 19));
>   PIN_DECL_2(AC7, GPIOW3, LAD3, ESPID3);
>   
>   #define AE7 180
> -SIG_EXPR_LIST_DECL_SESG(AE7, LCLK, LPC, SIG_DESC_SET(SCU434, 20),
> -			  SIG_DESC_CLEAR(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AE7, ESPICK, ESPI, SIG_DESC_SET(SCU434, 20),
> -			  SIG_DESC_SET(SCU510, 6));
> +SIG_EXPR_LIST_DECL_SESG(AE7, LCLK, LPC, SIG_DESC_SET(SCU510, 6),
> +			SIG_DESC_SET(SCU434, 20));
> +SIG_EXPR_LIST_DECL_SESG(AE7, ESPICK, ESPI, SIG_DESC_SET(SCU434, 20));
>   PIN_DECL_2(AE7, GPIOW4, LCLK, ESPICK);
>   
>   #define AF7 181
> -SIG_EXPR_LIST_DECL_SESG(AF7, LFRAME, LPC, SIG_DESC_SET(SCU434, 21),
> -			  SIG_DESC_CLEAR(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AF7, ESPICS, ESPI, SIG_DESC_SET(SCU434, 21),
> -			  SIG_DESC_SET(SCU510, 6));
> +SIG_EXPR_LIST_DECL_SESG(AF7, LFRAME, LPC, SIG_DESC_SET(SCU510, 6),
> +			SIG_DESC_SET(SCU434, 21));
> +SIG_EXPR_LIST_DECL_SESG(AF7, ESPICS, ESPI, SIG_DESC_SET(SCU434, 21));
>   PIN_DECL_2(AF7, GPIOW5, LFRAME, ESPICS);
>   
>   #define AD7 182
> -SIG_EXPR_LIST_DECL_SESG(AD7, LSIRQ, LSIRQ, SIG_DESC_SET(SCU434, 22),
> -			  SIG_DESC_CLEAR(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AD7, ESPIALT, ESPIALT, SIG_DESC_SET(SCU434, 22),
> -			  SIG_DESC_SET(SCU510, 6));
> +SIG_EXPR_LIST_DECL_SESG(AD7, LSIRQ, LSIRQ, SIG_DESC_SET(SCU510, 6),
> +			SIG_DESC_SET(SCU434, 22));
> +SIG_EXPR_LIST_DECL_SESG(AD7, ESPIALT, ESPIALT, SIG_DESC_SET(SCU434, 22));
>   PIN_DECL_2(AD7, GPIOW6, LSIRQ, ESPIALT);
>   FUNC_GROUP_DECL(LSIRQ, AD7);
>   FUNC_GROUP_DECL(ESPIALT, AD7);
>   
>   #define AD8 183
> -SIG_EXPR_LIST_DECL_SESG(AD8, LPCRST, LPC, SIG_DESC_SET(SCU434, 23),
> -			  SIG_DESC_CLEAR(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AD8, ESPIRST, ESPI, SIG_DESC_SET(SCU434, 23),
> -			  SIG_DESC_SET(SCU510, 6));
> +SIG_EXPR_LIST_DECL_SESG(AD8, LPCRST, LPC, SIG_DESC_SET(SCU510, 6),
> +			SIG_DESC_SET(SCU434, 23));
> +SIG_EXPR_LIST_DECL_SESG(AD8, ESPIRST, ESPI, SIG_DESC_SET(SCU434, 23));
>   PIN_DECL_2(AD8, GPIOW7, LPCRST, ESPIRST);
>   
>   FUNC_GROUP_DECL(LPC, AB7, AB8, AC8, AC7, AE7, AF7, AD8);

Does it need AD7 too in this group?

I think it should be:
FUNC_GROUP_DECL(LPC, AB7, AB8, AC8, AC7, AE7, AF7, AD7, AD8);
FUNC_GROUP_DECL(ESPI, AB7, AB8, AC8, AC7, AE7, AF7, AD7, AD8);

Cheers,

Jae

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

* Re: [PATCH linux dev-5.3 2/4] pinctrl: aspeed-g6: Fix LPC/eSPI mux configuration
  2019-10-22 18:34   ` Jae Hyun Yoo
@ 2019-10-22 18:42     ` Jae Hyun Yoo
  2019-10-23  0:00       ` Andrew Jeffery
  0 siblings, 1 reply; 9+ messages in thread
From: Jae Hyun Yoo @ 2019-10-22 18:42 UTC (permalink / raw)
  To: Andrew Jeffery, joel; +Cc: openbmc

On 10/22/2019 11:34 AM, Jae Hyun Yoo wrote:
> Hi Andrew,
> 
> On 10/21/2019 9:47 PM, Andrew Jeffery wrote:
>> Early revisions of the AST2600 datasheet are conflicted about the state
>> of the LPC/eSPI strapping bit (SCU510[6]). Conversations with ASPEED
>> determined that the reference pinmux configuration tables were in error
>> and the SCU documentation contained the correct configuration. Update
>> the driver to reflect the state described in the SCU documentation.
>>
>> Fixes: 2eda1cdec49f ("pinctrl: aspeed: Add AST2600 pinmux support")
>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>> ---
>>   drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 56 ++++++++++------------
>>   1 file changed, 24 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c 
>> b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
>> index eb0c11a9fbf2..fb96e8d2e6c8 100644
>> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
>> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
>> @@ -1098,61 +1098,53 @@ SSSF_PIN_DECL(AD15, GPIOV6, LPCPME, 
>> SIG_DESC_SET(SCU434, 14));
>>   SSSF_PIN_DECL(AF15, GPIOV7, LPCSMI, SIG_DESC_SET(SCU434, 15));
>>   #define AB7 176
>> -SIG_EXPR_LIST_DECL_SESG(AB7, LAD0, LPC, SIG_DESC_SET(SCU434, 16),
>> -              SIG_DESC_CLEAR(SCU510, 6));
>> -SIG_EXPR_LIST_DECL_SESG(AB7, ESPID0, ESPI, SIG_DESC_SET(SCU434, 16),
>> -              SIG_DESC_SET(SCU510, 6));
>> +SIG_EXPR_LIST_DECL_SESG(AB7, LAD0, LPC, SIG_DESC_SET(SCU510, 6),
>> +            SIG_DESC_SET(SCU434, 16));
>> +SIG_EXPR_LIST_DECL_SESG(AB7, ESPID0, ESPI, SIG_DESC_SET(SCU434, 16));
>>   PIN_DECL_2(AB7, GPIOW0, LAD0, ESPID0);
>>   #define AB8 177
>> -SIG_EXPR_LIST_DECL_SESG(AB8, LAD1, LPC, SIG_DESC_SET(SCU434, 17),
>> -              SIG_DESC_CLEAR(SCU510, 6));
>> -SIG_EXPR_LIST_DECL_SESG(AB8, ESPID1, ESPI, SIG_DESC_SET(SCU434, 17),
>> -              SIG_DESC_SET(SCU510, 6));
>> +SIG_EXPR_LIST_DECL_SESG(AB8, LAD1, LPC, SIG_DESC_SET(SCU510, 6),
>> +            SIG_DESC_SET(SCU434, 17));
>> +SIG_EXPR_LIST_DECL_SESG(AB8, ESPID1, ESPI, SIG_DESC_SET(SCU434, 17));
>>   PIN_DECL_2(AB8, GPIOW1, LAD1, ESPID1);
>>   #define AC8 178
>> -SIG_EXPR_LIST_DECL_SESG(AC8, LAD2, LPC, SIG_DESC_SET(SCU434, 18),
>> -              SIG_DESC_CLEAR(SCU510, 6));
>> -SIG_EXPR_LIST_DECL_SESG(AC8, ESPID2, ESPI, SIG_DESC_SET(SCU434, 18),
>> -              SIG_DESC_SET(SCU510, 6));
>> +SIG_EXPR_LIST_DECL_SESG(AC8, LAD2, LPC, SIG_DESC_SET(SCU510, 6),
>> +            SIG_DESC_SET(SCU434, 18));
>> +SIG_EXPR_LIST_DECL_SESG(AC8, ESPID2, ESPI, SIG_DESC_SET(SCU434, 18));
>>   PIN_DECL_2(AC8, GPIOW2, LAD2, ESPID2);
>>   #define AC7 179
>> -SIG_EXPR_LIST_DECL_SESG(AC7, LAD3, LPC, SIG_DESC_SET(SCU434, 19),
>> -              SIG_DESC_CLEAR(SCU510, 6));
>> -SIG_EXPR_LIST_DECL_SESG(AC7, ESPID3, ESPI, SIG_DESC_SET(SCU434, 19),
>> -              SIG_DESC_SET(SCU510, 6));
>> +SIG_EXPR_LIST_DECL_SESG(AC7, LAD3, LPC, SIG_DESC_SET(SCU510, 6),
>> +            SIG_DESC_SET(SCU434, 19));
>> +SIG_EXPR_LIST_DECL_SESG(AC7, ESPID3, ESPI, SIG_DESC_SET(SCU434, 19));
>>   PIN_DECL_2(AC7, GPIOW3, LAD3, ESPID3);
>>   #define AE7 180
>> -SIG_EXPR_LIST_DECL_SESG(AE7, LCLK, LPC, SIG_DESC_SET(SCU434, 20),
>> -              SIG_DESC_CLEAR(SCU510, 6));
>> -SIG_EXPR_LIST_DECL_SESG(AE7, ESPICK, ESPI, SIG_DESC_SET(SCU434, 20),
>> -              SIG_DESC_SET(SCU510, 6));
>> +SIG_EXPR_LIST_DECL_SESG(AE7, LCLK, LPC, SIG_DESC_SET(SCU510, 6),
>> +            SIG_DESC_SET(SCU434, 20));
>> +SIG_EXPR_LIST_DECL_SESG(AE7, ESPICK, ESPI, SIG_DESC_SET(SCU434, 20));
>>   PIN_DECL_2(AE7, GPIOW4, LCLK, ESPICK);
>>   #define AF7 181
>> -SIG_EXPR_LIST_DECL_SESG(AF7, LFRAME, LPC, SIG_DESC_SET(SCU434, 21),
>> -              SIG_DESC_CLEAR(SCU510, 6));
>> -SIG_EXPR_LIST_DECL_SESG(AF7, ESPICS, ESPI, SIG_DESC_SET(SCU434, 21),
>> -              SIG_DESC_SET(SCU510, 6));
>> +SIG_EXPR_LIST_DECL_SESG(AF7, LFRAME, LPC, SIG_DESC_SET(SCU510, 6),
>> +            SIG_DESC_SET(SCU434, 21));
>> +SIG_EXPR_LIST_DECL_SESG(AF7, ESPICS, ESPI, SIG_DESC_SET(SCU434, 21));
>>   PIN_DECL_2(AF7, GPIOW5, LFRAME, ESPICS);
>>   #define AD7 182
>> -SIG_EXPR_LIST_DECL_SESG(AD7, LSIRQ, LSIRQ, SIG_DESC_SET(SCU434, 22),
>> -              SIG_DESC_CLEAR(SCU510, 6));
>> -SIG_EXPR_LIST_DECL_SESG(AD7, ESPIALT, ESPIALT, SIG_DESC_SET(SCU434, 22),
>> -              SIG_DESC_SET(SCU510, 6));
>> +SIG_EXPR_LIST_DECL_SESG(AD7, LSIRQ, LSIRQ, SIG_DESC_SET(SCU510, 6),
>> +            SIG_DESC_SET(SCU434, 22));
>> +SIG_EXPR_LIST_DECL_SESG(AD7, ESPIALT, ESPIALT, SIG_DESC_SET(SCU434, 
>> 22));
>>   PIN_DECL_2(AD7, GPIOW6, LSIRQ, ESPIALT);
>>   FUNC_GROUP_DECL(LSIRQ, AD7);
>>   FUNC_GROUP_DECL(ESPIALT, AD7);
>>   #define AD8 183
>> -SIG_EXPR_LIST_DECL_SESG(AD8, LPCRST, LPC, SIG_DESC_SET(SCU434, 23),
>> -              SIG_DESC_CLEAR(SCU510, 6));
>> -SIG_EXPR_LIST_DECL_SESG(AD8, ESPIRST, ESPI, SIG_DESC_SET(SCU434, 23),
>> -              SIG_DESC_SET(SCU510, 6));
>> +SIG_EXPR_LIST_DECL_SESG(AD8, LPCRST, LPC, SIG_DESC_SET(SCU510, 6),
>> +            SIG_DESC_SET(SCU434, 23));
>> +SIG_EXPR_LIST_DECL_SESG(AD8, ESPIRST, ESPI, SIG_DESC_SET(SCU434, 23));
>>   PIN_DECL_2(AD8, GPIOW7, LPCRST, ESPIRST);
>>   FUNC_GROUP_DECL(LPC, AB7, AB8, AC8, AC7, AE7, AF7, AD8);
> 
> Does it need AD7 too in this group?
> 
> I think it should be:
> FUNC_GROUP_DECL(LPC, AB7, AB8, AC8, AC7, AE7, AF7, AD7, AD8);
> FUNC_GROUP_DECL(ESPI, AB7, AB8, AC8, AC7, AE7, AF7, AD7, AD8);

Ah, I realized that there are seperate group definitions for LSIRQ and
ESPIALT. Ignore my comment.

Cheers,

Jae

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

* Re: [PATCH linux dev-5.3 2/4] pinctrl: aspeed-g6: Fix LPC/eSPI mux configuration
  2019-10-22 18:42     ` Jae Hyun Yoo
@ 2019-10-23  0:00       ` Andrew Jeffery
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2019-10-23  0:00 UTC (permalink / raw)
  To: Jae Hyun Yoo, Joel Stanley; +Cc: openbmc



On Wed, 23 Oct 2019, at 05:42, Jae Hyun Yoo wrote:
> On 10/22/2019 11:34 AM, Jae Hyun Yoo wrote:
> > Hi Andrew,
> > 
> > On 10/21/2019 9:47 PM, Andrew Jeffery wrote:
> >> Early revisions of the AST2600 datasheet are conflicted about the state
> >> of the LPC/eSPI strapping bit (SCU510[6]). Conversations with ASPEED
> >> determined that the reference pinmux configuration tables were in error
> >> and the SCU documentation contained the correct configuration. Update
> >> the driver to reflect the state described in the SCU documentation.
> >>
> >> Fixes: 2eda1cdec49f ("pinctrl: aspeed: Add AST2600 pinmux support")
> >> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> >> ---
> >>   drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 56 ++++++++++------------
> >>   1 file changed, 24 insertions(+), 32 deletions(-)
> >>
> >> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c 
> >> b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> >> index eb0c11a9fbf2..fb96e8d2e6c8 100644
> >> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> >> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> >> @@ -1098,61 +1098,53 @@ SSSF_PIN_DECL(AD15, GPIOV6, LPCPME, 
> >> SIG_DESC_SET(SCU434, 14));
> >>   SSSF_PIN_DECL(AF15, GPIOV7, LPCSMI, SIG_DESC_SET(SCU434, 15));
> >>   #define AB7 176
> >> -SIG_EXPR_LIST_DECL_SESG(AB7, LAD0, LPC, SIG_DESC_SET(SCU434, 16),
> >> -              SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AB7, ESPID0, ESPI, SIG_DESC_SET(SCU434, 16),
> >> -              SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AB7, LAD0, LPC, SIG_DESC_SET(SCU510, 6),
> >> +            SIG_DESC_SET(SCU434, 16));
> >> +SIG_EXPR_LIST_DECL_SESG(AB7, ESPID0, ESPI, SIG_DESC_SET(SCU434, 16));
> >>   PIN_DECL_2(AB7, GPIOW0, LAD0, ESPID0);
> >>   #define AB8 177
> >> -SIG_EXPR_LIST_DECL_SESG(AB8, LAD1, LPC, SIG_DESC_SET(SCU434, 17),
> >> -              SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AB8, ESPID1, ESPI, SIG_DESC_SET(SCU434, 17),
> >> -              SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AB8, LAD1, LPC, SIG_DESC_SET(SCU510, 6),
> >> +            SIG_DESC_SET(SCU434, 17));
> >> +SIG_EXPR_LIST_DECL_SESG(AB8, ESPID1, ESPI, SIG_DESC_SET(SCU434, 17));
> >>   PIN_DECL_2(AB8, GPIOW1, LAD1, ESPID1);
> >>   #define AC8 178
> >> -SIG_EXPR_LIST_DECL_SESG(AC8, LAD2, LPC, SIG_DESC_SET(SCU434, 18),
> >> -              SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AC8, ESPID2, ESPI, SIG_DESC_SET(SCU434, 18),
> >> -              SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AC8, LAD2, LPC, SIG_DESC_SET(SCU510, 6),
> >> +            SIG_DESC_SET(SCU434, 18));
> >> +SIG_EXPR_LIST_DECL_SESG(AC8, ESPID2, ESPI, SIG_DESC_SET(SCU434, 18));
> >>   PIN_DECL_2(AC8, GPIOW2, LAD2, ESPID2);
> >>   #define AC7 179
> >> -SIG_EXPR_LIST_DECL_SESG(AC7, LAD3, LPC, SIG_DESC_SET(SCU434, 19),
> >> -              SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AC7, ESPID3, ESPI, SIG_DESC_SET(SCU434, 19),
> >> -              SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AC7, LAD3, LPC, SIG_DESC_SET(SCU510, 6),
> >> +            SIG_DESC_SET(SCU434, 19));
> >> +SIG_EXPR_LIST_DECL_SESG(AC7, ESPID3, ESPI, SIG_DESC_SET(SCU434, 19));
> >>   PIN_DECL_2(AC7, GPIOW3, LAD3, ESPID3);
> >>   #define AE7 180
> >> -SIG_EXPR_LIST_DECL_SESG(AE7, LCLK, LPC, SIG_DESC_SET(SCU434, 20),
> >> -              SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AE7, ESPICK, ESPI, SIG_DESC_SET(SCU434, 20),
> >> -              SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AE7, LCLK, LPC, SIG_DESC_SET(SCU510, 6),
> >> +            SIG_DESC_SET(SCU434, 20));
> >> +SIG_EXPR_LIST_DECL_SESG(AE7, ESPICK, ESPI, SIG_DESC_SET(SCU434, 20));
> >>   PIN_DECL_2(AE7, GPIOW4, LCLK, ESPICK);
> >>   #define AF7 181
> >> -SIG_EXPR_LIST_DECL_SESG(AF7, LFRAME, LPC, SIG_DESC_SET(SCU434, 21),
> >> -              SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AF7, ESPICS, ESPI, SIG_DESC_SET(SCU434, 21),
> >> -              SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AF7, LFRAME, LPC, SIG_DESC_SET(SCU510, 6),
> >> +            SIG_DESC_SET(SCU434, 21));
> >> +SIG_EXPR_LIST_DECL_SESG(AF7, ESPICS, ESPI, SIG_DESC_SET(SCU434, 21));
> >>   PIN_DECL_2(AF7, GPIOW5, LFRAME, ESPICS);
> >>   #define AD7 182
> >> -SIG_EXPR_LIST_DECL_SESG(AD7, LSIRQ, LSIRQ, SIG_DESC_SET(SCU434, 22),
> >> -              SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AD7, ESPIALT, ESPIALT, SIG_DESC_SET(SCU434, 22),
> >> -              SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AD7, LSIRQ, LSIRQ, SIG_DESC_SET(SCU510, 6),
> >> +            SIG_DESC_SET(SCU434, 22));
> >> +SIG_EXPR_LIST_DECL_SESG(AD7, ESPIALT, ESPIALT, SIG_DESC_SET(SCU434, 
> >> 22));
> >>   PIN_DECL_2(AD7, GPIOW6, LSIRQ, ESPIALT);
> >>   FUNC_GROUP_DECL(LSIRQ, AD7);
> >>   FUNC_GROUP_DECL(ESPIALT, AD7);
> >>   #define AD8 183
> >> -SIG_EXPR_LIST_DECL_SESG(AD8, LPCRST, LPC, SIG_DESC_SET(SCU434, 23),
> >> -              SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AD8, ESPIRST, ESPI, SIG_DESC_SET(SCU434, 23),
> >> -              SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AD8, LPCRST, LPC, SIG_DESC_SET(SCU510, 6),
> >> +            SIG_DESC_SET(SCU434, 23));
> >> +SIG_EXPR_LIST_DECL_SESG(AD8, ESPIRST, ESPI, SIG_DESC_SET(SCU434, 23));
> >>   PIN_DECL_2(AD8, GPIOW7, LPCRST, ESPIRST);
> >>   FUNC_GROUP_DECL(LPC, AB7, AB8, AC8, AC7, AE7, AF7, AD8);
> > 
> > Does it need AD7 too in this group?
> > 
> > I think it should be:
> > FUNC_GROUP_DECL(LPC, AB7, AB8, AC8, AC7, AE7, AF7, AD7, AD8);
> > FUNC_GROUP_DECL(ESPI, AB7, AB8, AC8, AC7, AE7, AF7, AD7, AD8);
> 
> Ah, I realized that there are seperate group definitions for LSIRQ and
> ESPIALT. Ignore my comment.

Yeah, I grouped them based on required/optional signals in the LPC/eSPI specs.

I suspect we'll always want to mux e.g. LSIRQ if we're using LPC, but who knows.
I'm trying not to constrain what lines can be used for GPIO where possible.

Thanks for taking a look :)

Andrew

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

* Re: [PATCH linux dev-5.3 0/4] Pinmux fixes for AST2600 LPC
  2019-10-22  4:47 [PATCH linux dev-5.3 0/4] Pinmux fixes for AST2600 LPC Andrew Jeffery
                   ` (3 preceding siblings ...)
  2019-10-22  4:47 ` [PATCH linux dev-5.3 4/4] pinctrl: aspeed: Improve debug output Andrew Jeffery
@ 2019-10-23  3:23 ` Joel Stanley
  4 siblings, 0 replies; 9+ messages in thread
From: Joel Stanley @ 2019-10-23  3:23 UTC (permalink / raw)
  To: Andrew Jeffery; +Cc: OpenBMC Maillist

On Tue, 22 Oct 2019 at 04:46, Andrew Jeffery <andrew@aj.id.au> wrote:
>
> Hello,
>
> This series fixes some issues with the AST2600 pinctrl driver.
>
> Patch 1/4 backports an existing upstream fix. Patch 4/4 simply improves the
> readability of the debug output of the driver. Patches 2 and 3 fix issues with
> muxing pins for LPC, particularly for the Tacoma board.

They look good to me. I've applied them to dev-5.3

Cheers,

Joel

>
> Please review.
>
> Andrew
>
> Andrew Jeffery (4):
>   pinctrl: aspeed-g6: Make SIG_DESC_CLEAR() behave intuitively
>   pinctrl: aspeed-g6: Fix LPC/eSPI mux configuration
>   ARM: dts: tacoma: Hog LPC pinmux
>   pinctrl: aspeed: Improve debug output
>
>  arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts |  7 +++
>  drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c  | 56 +++++++++------------
>  drivers/pinctrl/aspeed/pinctrl-aspeed.c     | 25 +++++++--
>  drivers/pinctrl/aspeed/pinmux-aspeed.h      |  2 +-
>  4 files changed, 54 insertions(+), 36 deletions(-)
>
> --
> 2.20.1
>

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

end of thread, other threads:[~2019-10-23  3:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-22  4:47 [PATCH linux dev-5.3 0/4] Pinmux fixes for AST2600 LPC Andrew Jeffery
2019-10-22  4:47 ` [PATCH linux dev-5.3 1/4] pinctrl: aspeed-g6: Make SIG_DESC_CLEAR() behave intuitively Andrew Jeffery
2019-10-22  4:47 ` [PATCH linux dev-5.3 2/4] pinctrl: aspeed-g6: Fix LPC/eSPI mux configuration Andrew Jeffery
2019-10-22 18:34   ` Jae Hyun Yoo
2019-10-22 18:42     ` Jae Hyun Yoo
2019-10-23  0:00       ` Andrew Jeffery
2019-10-22  4:47 ` [PATCH linux dev-5.3 3/4] ARM: dts: tacoma: Hog LPC pinmux Andrew Jeffery
2019-10-22  4:47 ` [PATCH linux dev-5.3 4/4] pinctrl: aspeed: Improve debug output Andrew Jeffery
2019-10-23  3:23 ` [PATCH linux dev-5.3 0/4] Pinmux fixes for AST2600 LPC Joel Stanley

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.