pinctrl: aspeed: Fix GPIO requests on pass-through banks

Commit 6726fbff19bf ("pinctrl: aspeed: Fix GPI only function problem.")
fixes access to GPIO banks T and U on the AST2600. Both banks contain
input-only pins and the GPIO pin function is named GPITx and GPIUx
respectively. Unfortunately the fix had a negative impact on GPIO banks
D and E for the AST2400 and AST2500 where the GPIO pass-through
functions take similar "GPI"-style names. The net effect on the older
SoCs was that when the GPIO subsystem requested a pin in banks D or E be
muxed for GPIO, they were instead muxed for pass-through mode.
Mistakenly muxing pass-through mode e.g. breaks booting the host on
IBM's Witherspoon (AC922) platform where GPIOE0 is used for FSI.

Further exploit the names in the provided expression structure to
differentiate pass-through from pin-specific GPIO modes.

This follow-up fix gives the expected behaviour for the following tests:

Witherspoon BMC (AST2500):

1. Power-on the Witherspoon host
2. Request GPIOD1 be muxed via /sys/class/gpio/export
3. Request GPIOE1 be muxed via /sys/class/gpio/export
4. Request the balls for GPIOs E2 and E3 be muxed as GPIO pass-through
   ("GPIE2" mode) via a pinctrl hog in the devicetree

Rainier BMC (AST2600):

5. Request GPIT0 be muxed via /sys/class/gpio/export
6. Request GPIU0 be muxed via /sys/class/gpio/export

Together the tests demonstrate that all three pieces of functionality
(general GPIOs via 1, 2 and 3, input-only GPIOs via 5 and 6, pass-through
mode via 4) operate as desired across old and new SoCs.

Fixes: 9b92f5c51e ("pinctrl: aspeed: Fix GPI only function problem.")
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Tested-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Cc: Billy Tsai <billy_tsai@aspeedtech.com>
Cc: Joel Stanley <joel@jms.id.au>
Link: https://lore.kernel.org/r/20201126063337.489927-1-andrew@aj.id.au
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
This commit is contained in:
Andrew Jeffery 2020-11-26 17:03:37 +10:30 committed by Linus Walleij
parent 47a0001436
commit 7aeb353802
2 changed files with 72 additions and 9 deletions

View File

@ -286,14 +286,76 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
static bool aspeed_expr_is_gpio(const struct aspeed_sig_expr *expr)
{
/*
* The signal type is GPIO if the signal name has "GPI" as a prefix.
* strncmp (rather than strcmp) is used to implement the prefix
* requirement.
* We need to differentiate between GPIO and non-GPIO signals to
* implement the gpio_request_enable() interface. For better or worse
* the ASPEED pinctrl driver uses the expression names to determine
* whether an expression will mux a pin for GPIO.
*
* expr->signal might look like "GPIOB1" in the GPIO case.
* expr->signal might look like "GPIT0" in the GPI case.
* Generally we have the following - A GPIO such as B1 has:
*
* - expr->signal set to "GPIOB1"
* - expr->function set to "GPIOB1"
*
* Using this fact we can determine whether the provided expression is
* a GPIO expression by testing the signal name for the string prefix
* "GPIO".
*
* However, some GPIOs are input-only, and the ASPEED datasheets name
* them differently. An input-only GPIO such as T0 has:
*
* - expr->signal set to "GPIT0"
* - expr->function set to "GPIT0"
*
* It's tempting to generalise the prefix test from "GPIO" to "GPI" to
* account for both GPIOs and GPIs, but in doing so we run aground on
* another feature:
*
* Some pins in the ASPEED BMC SoCs have a "pass-through" GPIO
* function where the input state of one pin is replicated as the
* output state of another (as if they were shorted together - a mux
* configuration that is typically enabled by hardware strapping).
* This feature allows the BMC to pass e.g. power button state through
* to the host while the BMC is yet to boot, but take control of the
* button state once the BMC has booted by muxing each pin as a
* separate, pin-specific GPIO.
*
* Conceptually this pass-through mode is a form of GPIO and is named
* as such in the datasheets, e.g. "GPID0". This naming similarity
* trips us up with the simple GPI-prefixed-signal-name scheme
* discussed above, as the pass-through configuration is not what we
* want when muxing a pin as GPIO for the GPIO subsystem.
*
* On e.g. the AST2400, a pass-through function "GPID0" is grouped on
* balls A18 and D16, where we have:
*
* For ball A18:
* - expr->signal set to "GPID0IN"
* - expr->function set to "GPID0"
*
* For ball D16:
* - expr->signal set to "GPID0OUT"
* - expr->function set to "GPID0"
*
* By contrast, the pin-specific GPIO expressions for the same pins are
* as follows:
*
* For ball A18:
* - expr->signal looks like "GPIOD0"
* - expr->function looks like "GPIOD0"
*
* For ball D16:
* - expr->signal looks like "GPIOD1"
* - expr->function looks like "GPIOD1"
*
* Testing both the signal _and_ function names gives us the means
* differentiate the pass-through GPIO pinmux configuration from the
* pin-specific configuration that the GPIO subsystem is after: An
* expression is a pin-specific (non-pass-through) GPIO configuration
* if the signal prefix is "GPI" and the signal name matches the
* function name.
*/
return strncmp(expr->signal, "GPI", 3) == 0;
return !strncmp(expr->signal, "GPI", 3) &&
!strcmp(expr->signal, expr->function);
}
static bool aspeed_gpio_in_exprs(const struct aspeed_sig_expr **exprs)

View File

@ -452,10 +452,11 @@ struct aspeed_sig_desc {
* evaluation of the descriptors.
*
* @signal: The signal name for the priority level on the pin. If the signal
* type is GPIO, then the signal name must begin with the string
* "GPIO", e.g. GPIOA0, GPIOT4 etc.
* type is GPIO, then the signal name must begin with the
* prefix "GPI", e.g. GPIOA0, GPIT0 etc.
* @function: The name of the function the signal participates in for the
* associated expression
* associated expression. For pin-specific GPIO, the function
* name must match the signal name.
* @ndescs: The number of signal descriptors in the expression
* @descs: Pointer to an array of signal descriptors that comprise the
* function expression