Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1581086 > unrolled thread

[RESEND PATCH linux] aspeed: pinctrl: Allow disabling Port D and Port E loopback mode

Started byRick Altherr <raltherr@google.com>
First post2017-02-15 08:00 +0100
Last post2017-02-16 02:40 +0100
Articles 3 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [RESEND PATCH linux] aspeed: pinctrl: Allow disabling Port D and Port E loopback mode Rick Altherr <raltherr@google.com> - 2017-02-15 08:00 +0100
    Re: [RESEND PATCH linux] aspeed: pinctrl: Allow disabling Port D and  Port E loopback mode Joel Stanley <joel@jms.id.au> - 2017-02-16 02:30 +0100
    Re: [RESEND PATCH linux] aspeed: pinctrl: Allow disabling Port D  and Port E loopback mode Andrew Jeffery <andrew@aj.id.au> - 2017-02-16 02:40 +0100

#1581086 — [RESEND PATCH linux] aspeed: pinctrl: Allow disabling Port D and Port E loopback mode

FromRick Altherr <raltherr@google.com>
Date2017-02-15 08:00 +0100
Subject[RESEND PATCH linux] aspeed: pinctrl: Allow disabling Port D and Port E loopback mode
Message-ID<tb1Jf-4Ig-1@gated-at.bofh.it>
Port D and port E GPIO loopback modes are commonly enabled via hardware
straps for use with front-panel buttons.  When the BMC is powered
off or fails to boot, the front-panel buttons are directly connected to
the host chipset via the loopback to allow direct power-on and reset
control. Once the BMC has booted, the loopback mode must be disabled for
the BMC to take over control of host power-on and reset.

Disabling these loopback modes requires writing to the hardware strap
register which violates the current design of assuming the system
designer chose the strap settings for a specific reason and they should
be treated as read-only. Only the two bits of the strap register related
to these loopback modes are allowed to be written and comments have been
added to explain why.

Signed-off-by: Rick Altherr <raltherr@google.com>
---
 drivers/pinctrl/aspeed/pinctrl-aspeed.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
index 76f62bd45f02..5b49952e5fad 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
@@ -198,9 +198,19 @@ static int aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
 		 * them. This may mean that certain functions cannot be
 		 * deconfigured and is the reason we re-evaluate after writing
 		 * all descriptor bits.
+		 *
+		 * Port D and port E GPIO loopback modes are the only exception
+		 * as those are commonly used with front-panel buttons to allow
+		 * normal operation of the host when the BMC is powered off or
+		 * fails to boot. Once the BMC has booted, the loopback mode
+		 * must be disabled for the BMC to control host power-on and
+		 * reset.
 		 */
-		if ((desc->reg == HW_STRAP1 || desc->reg == HW_STRAP2) &&
-				desc->ip == ASPEED_IP_SCU)
+		if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1 &&
+		    !(desc->mask & (BIT(21) | BIT(22))))
+			continue;
+
+		if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
 			continue;
 
 		ret = regmap_update_bits(maps[desc->ip], desc->reg,
-- 
2.11.0.483.g087da7b7c-goog

[toc] | [next] | [standalone]


#1582215 — Re: [RESEND PATCH linux] aspeed: pinctrl: Allow disabling Port D and Port E loopback mode

FromJoel Stanley <joel@jms.id.au>
Date2017-02-16 02:30 +0100
SubjectRe: [RESEND PATCH linux] aspeed: pinctrl: Allow disabling Port D and Port E loopback mode
Message-ID<tbj3s-7Db-9@gated-at.bofh.it>
In reply to#1581086
On Wed, Feb 15, 2017 at 5:25 PM, Rick Altherr <raltherr@google.com> wrote:
> Port D and port E GPIO loopback modes are commonly enabled via hardware
> straps for use with front-panel buttons.  When the BMC is powered
> off or fails to boot, the front-panel buttons are directly connected to
> the host chipset via the loopback to allow direct power-on and reset
> control. Once the BMC has booted, the loopback mode must be disabled for
> the BMC to take over control of host power-on and reset.
>
> Disabling these loopback modes requires writing to the hardware strap
> register which violates the current design of assuming the system
> designer chose the strap settings for a specific reason and they should
> be treated as read-only. Only the two bits of the strap register related
> to these loopback modes are allowed to be written and comments have been
> added to explain why.

I had to clarify this on IRC. The Aspeed soc is flexible if you want
no-passthrough when powered off and want to enable it at  runtime with
normal SCU registers.
However if you want passthrough when powered off, and then disable it
at runtime, you need to change the strap. Hence the patch.

> Signed-off-by: Rick Altherr <raltherr@google.com>

Acked-by: Joel Stanley <joel@jms.id.au>

Cheers,

Joel

> ---
>  drivers/pinctrl/aspeed/pinctrl-aspeed.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> index 76f62bd45f02..5b49952e5fad 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> @@ -198,9 +198,19 @@ static int aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
>                  * them. This may mean that certain functions cannot be
>                  * deconfigured and is the reason we re-evaluate after writing
>                  * all descriptor bits.
> +                *
> +                * Port D and port E GPIO loopback modes are the only exception
> +                * as those are commonly used with front-panel buttons to allow
> +                * normal operation of the host when the BMC is powered off or
> +                * fails to boot. Once the BMC has booted, the loopback mode
> +                * must be disabled for the BMC to control host power-on and
> +                * reset.
>                  */
> -               if ((desc->reg == HW_STRAP1 || desc->reg == HW_STRAP2) &&
> -                               desc->ip == ASPEED_IP_SCU)
> +               if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1 &&
> +                   !(desc->mask & (BIT(21) | BIT(22))))
> +                       continue;
> +
> +               if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
>                         continue;
>
>                 ret = regmap_update_bits(maps[desc->ip], desc->reg,
> --
> 2.11.0.483.g087da7b7c-goog
>

[toc] | [prev] | [next] | [standalone]


#1582219 — Re: [RESEND PATCH linux] aspeed: pinctrl: Allow disabling Port D and Port E loopback mode

FromAndrew Jeffery <andrew@aj.id.au>
Date2017-02-16 02:40 +0100
SubjectRe: [RESEND PATCH linux] aspeed: pinctrl: Allow disabling Port D and Port E loopback mode
Message-ID<tbjd8-7Gj-9@gated-at.bofh.it>
In reply to#1581086

[Multipart message — attachments visible in raw view] — view raw

On Tue, 2017-02-14 at 22:55 -0800, Rick Altherr wrote:
> Port D and port E GPIO loopback modes are commonly enabled via hardware
> straps for use with front-panel buttons.  When the BMC is powered
> off or fails to boot, the front-panel buttons are directly connected to
> the host chipset via the loopback to allow direct power-on and reset
> control. Once the BMC has booted, the loopback mode must be disabled for
> the BMC to take over control of host power-on and reset.
> 
> Disabling these loopback modes requires writing to the hardware strap
> register which violates the current design of assuming the system
> designer chose the strap settings for a specific reason and they should
> be treated as read-only. Only the two bits of the strap register related
> to these loopback modes are allowed to be written and comments have been
> added to explain why.
> 
> Signed-off-by: Rick Altherr <raltherr@google.com>

For the record this is one of at least two awkward cases. The other is
runtime switching between the using the SoC as an SPI master, and
simply performing SPI pass-through for an external controller (e.g. to
allow either the BMC or the host to reprogram the host BIOS image).

If the list of exceptions continues to grow we should pull the tests
out to a separate function so we can keep the condition readable.

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  drivers/pinctrl/aspeed/pinctrl-aspeed.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> index 76f62bd45f02..5b49952e5fad 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> @@ -198,9 +198,19 @@ static int aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
> >  		 * them. This may mean that certain functions cannot be
> >  		 * deconfigured and is the reason we re-evaluate after writing
> >  		 * all descriptor bits.
> > +		 *
> > +		 * Port D and port E GPIO loopback modes are the only exception
> > +		 * as those are commonly used with front-panel buttons to allow
> > +		 * normal operation of the host when the BMC is powered off or
> > +		 * fails to boot. Once the BMC has booted, the loopback mode
> > +		 * must be disabled for the BMC to control host power-on and
> > +		 * reset.
> >  		 */
> > -		if ((desc->reg == HW_STRAP1 || desc->reg == HW_STRAP2) &&
> > -				desc->ip == ASPEED_IP_SCU)
> > +		if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1 &&
> > +		    !(desc->mask & (BIT(21) | BIT(22))))
> > +			continue;
> +
> > +		if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
> >  			continue;
>  
> >  		ret = regmap_update_bits(maps[desc->ip], desc->reg,

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web