| Message ID | 20170215065514.16673-1-raltherr@google.com |
|---|---|
| State | New |
| Headers | show |
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,
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,
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(-)