[PATCH 1/2] pinctrl:aspeed: add miss pin and function
Andrew Jeffery
andrew at aj.id.au
Mon Jan 23 12:23:35 AEDT 2023
Hi Ryan,
Thanks for the patch. I have a few comments:
On Tue, 3 Jan 2023, at 20:07, ryan_chen wrote:
> Add pin: A7,D7
> Add function: qspi, secure i2c, pcie rc
Generally if we're listing things the patch does in the commit message
it's a sign that the commit should be split up.
>
> Signed-off-by: ryan_chen <ryan_chen at aspeedtech.com>
Can you please fix up your git config so your name is capitalised and
to switch the underscore to a space? For example:
git config --global user.name 'Ryan Chen'
You might need to amend the commit and replace the S-o-B tag above.
> ---
> drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 341 +++++++++++++++------
> 1 file changed, 239 insertions(+), 102 deletions(-)
>
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> index 80838dc54b3a..cf9554f0911f 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> @@ -17,6 +17,8 @@
> #include "../pinctrl-utils.h"
> #include "pinctrl-aspeed.h"
>
> +#define SCU040 0x040 /* Reset Control Set 1 */
> +#define SCU0C8 0x0C8 /* Debug Control */
> #define SCU400 0x400 /* Multi-function Pin Control #1 */
> #define SCU404 0x404 /* Multi-function Pin Control #2 */
> #define SCU40C 0x40C /* Multi-function Pin Control #3 */
> @@ -31,6 +33,8 @@
> #define SCU450 0x450 /* Multi-function Pin Control #14 */
> #define SCU454 0x454 /* Multi-function Pin Control #15 */
> #define SCU458 0x458 /* Multi-function Pin Control #16 */
> +#define SCU470 0x470
Can we tack the description on as per the datasheet, like the others?
#define SCU470 0x470 /* Multi-function Pin Control */
It doesn't have a number assigned in my v13 copy of the datasheet, but
oh well.
> +#define SCU474 0x474
This one isn't documented at all in v13. What is its purpose?
> #define SCU4B0 0x4B0 /* Multi-function Pin Control #17 */
> #define SCU4B4 0x4B4 /* Multi-function Pin Control #18 */
> #define SCU4B8 0x4B8 /* Multi-function Pin Control #19 */
> @@ -48,19 +52,23 @@
> #define SCU638 0x638 /* Disable GPIO Internal Pull-Down #6 */
> #define SCU690 0x690 /* Multi-function Pin Control #24 */
> #define SCU694 0x694 /* Multi-function Pin Control #25 */
> +#define SCU698 0x698 /* Multi-function Pin Control #26 */
> #define SCU69C 0x69C /* Multi-function Pin Control #27 */
The ordering of these in the datasheet gets me every time :(
> #define SCU6D0 0x6D0 /* Multi-function Pin Control #29 */
> #define SCUC20 0xC20 /* PCIE configuration Setting Control */
> +#define SCUC24 0xC24 /* BMC MMIO Decode Setting */
I'm mildly concerned that this appears in the pinmux driver...
>
> -#define ASPEED_G6_NR_PINS 256
> +#define ASPEED_G6_NR_PINS 258
>
> #define M24 0
> -SIG_EXPR_LIST_DECL_SESG(M24, MDC3, MDIO3, SIG_DESC_SET(SCU410, 0));
> +SIG_EXPR_LIST_DECL_SESG(M24, MDC3, MDIO3, SIG_DESC_SET(SCU410, 0),
> + SIG_DESC_CLEAR(SCU470, 0));
Bits 15:0 are marked as 'Reserved' in the v13 datasheet. Can we please
get them documented?
Further, this is a new constraint on an existing function. Can we
please split this out into its own patch to limit the context for
review?
> SIG_EXPR_LIST_DECL_SESG(M24, SCL11, I2C11, SIG_DESC_SET(SCU4B0, 0));
> PIN_DECL_2(M24, GPIOA0, MDC3, SCL11);
>
> #define M25 1
> -SIG_EXPR_LIST_DECL_SESG(M25, MDIO3, MDIO3, SIG_DESC_SET(SCU410, 1));
> +SIG_EXPR_LIST_DECL_SESG(M25, MDIO3, MDIO3, SIG_DESC_SET(SCU410, 1),
> + SIG_DESC_CLEAR(SCU470, 1));
> SIG_EXPR_LIST_DECL_SESG(M25, SDA11, I2C11, SIG_DESC_SET(SCU4B0, 1));
> PIN_DECL_2(M25, GPIOA1, MDIO3, SDA11);
>
> @@ -68,12 +76,14 @@ FUNC_GROUP_DECL(MDIO3, M24, M25);
> FUNC_GROUP_DECL(I2C11, M24, M25);
>
> #define L26 2
> -SIG_EXPR_LIST_DECL_SESG(L26, MDC4, MDIO4, SIG_DESC_SET(SCU410, 2));
> +SIG_EXPR_LIST_DECL_SESG(L26, MDC4, MDIO4, SIG_DESC_SET(SCU410, 2),
> + SIG_DESC_CLEAR(SCU470, 2));
> SIG_EXPR_LIST_DECL_SESG(L26, SCL12, I2C12, SIG_DESC_SET(SCU4B0, 2));
> PIN_DECL_2(L26, GPIOA2, MDC4, SCL12);
>
> #define K24 3
> -SIG_EXPR_LIST_DECL_SESG(K24, MDIO4, MDIO4, SIG_DESC_SET(SCU410, 3));
> +SIG_EXPR_LIST_DECL_SESG(K24, MDIO4, MDIO4, SIG_DESC_SET(SCU410, 3),
> + SIG_DESC_CLEAR(SCU470, 3));
> SIG_EXPR_LIST_DECL_SESG(K24, SDA12, I2C12, SIG_DESC_SET(SCU4B0, 3));
> PIN_DECL_2(K24, GPIOA3, MDIO4, SDA12);
>
> @@ -81,7 +91,8 @@ FUNC_GROUP_DECL(MDIO4, L26, K24);
> FUNC_GROUP_DECL(I2C12, L26, K24);
>
> #define K26 4
> -SIG_EXPR_LIST_DECL_SESG(K26, MACLINK1, MACLINK1, SIG_DESC_SET(SCU410, 4));
> +SIG_EXPR_LIST_DECL_SESG(K26, MACLINK1, MACLINK1, SIG_DESC_SET(SCU410, 4),
> + SIG_DESC_CLEAR(SCU470, 4));
> SIG_EXPR_LIST_DECL_SESG(K26, SCL13, I2C13, SIG_DESC_SET(SCU4B0, 4));
> SIG_EXPR_LIST_DECL_SESG(K26, SGPS2CK, SGPS2, SIG_DESC_SET(SCU690, 4));
> SIG_EXPR_LIST_DECL_SESG(K26, SGPM2CLK, SGPM2, SIG_DESC_SET(SCU6D0, 4));
> @@ -89,7 +100,8 @@ PIN_DECL_4(K26, GPIOA4, MACLINK1, SCL13, SGPS2CK, SGPM2CLK);
> FUNC_GROUP_DECL(MACLINK1, K26);
>
> #define L24 5
> -SIG_EXPR_LIST_DECL_SESG(L24, MACLINK2, MACLINK2, SIG_DESC_SET(SCU410, 5));
> +SIG_EXPR_LIST_DECL_SESG(L24, MACLINK2, MACLINK2, SIG_DESC_SET(SCU410, 5),
> + SIG_DESC_CLEAR(SCU470, 5));
> SIG_EXPR_LIST_DECL_SESG(L24, SDA13, I2C13, SIG_DESC_SET(SCU4B0, 5));
> SIG_EXPR_LIST_DECL_SESG(L24, SGPS2LD, SGPS2, SIG_DESC_SET(SCU690, 5));
> SIG_EXPR_LIST_DECL_SESG(L24, SGPM2LD, SGPM2, SIG_DESC_SET(SCU6D0, 5));
> @@ -99,7 +111,8 @@ FUNC_GROUP_DECL(MACLINK2, L24);
> FUNC_GROUP_DECL(I2C13, K26, L24);
>
> #define L23 6
> -SIG_EXPR_LIST_DECL_SESG(L23, MACLINK3, MACLINK3, SIG_DESC_SET(SCU410, 6));
> +SIG_EXPR_LIST_DECL_SESG(L23, MACLINK3, MACLINK3, SIG_DESC_SET(SCU410, 6),
> + SIG_DESC_CLEAR(SCU470, 6));
> SIG_EXPR_LIST_DECL_SESG(L23, SCL14, I2C14, SIG_DESC_SET(SCU4B0, 6));
> SIG_EXPR_LIST_DECL_SESG(L23, SGPS2O, SGPS2, SIG_DESC_SET(SCU690, 6));
> SIG_EXPR_LIST_DECL_SESG(L23, SGPM2O, SGPM2, SIG_DESC_SET(SCU6D0, 6));
> @@ -107,7 +120,8 @@ PIN_DECL_4(L23, GPIOA6, MACLINK3, SCL14, SGPS2O, SGPM2O);
> FUNC_GROUP_DECL(MACLINK3, L23);
>
> #define K25 7
> -SIG_EXPR_LIST_DECL_SESG(K25, MACLINK4, MACLINK4, SIG_DESC_SET(SCU410, 7));
> +SIG_EXPR_LIST_DECL_SESG(K25, MACLINK4, MACLINK4, SIG_DESC_SET(SCU410, 7),
> + SIG_DESC_CLEAR(SCU470, 7));
> SIG_EXPR_LIST_DECL_SESG(K25, SDA14, I2C14, SIG_DESC_SET(SCU4B0, 7));
> SIG_EXPR_LIST_DECL_SESG(K25, SGPS2I, SGPS2, SIG_DESC_SET(SCU690, 7));
> SIG_EXPR_LIST_DECL_SESG(K25, SGPM2I, SGPM2, SIG_DESC_SET(SCU6D0, 7));
> @@ -143,12 +157,14 @@ PIN_DECL_2(J25, GPIOB3, SALT4, LHAD3);
> FUNC_GROUP_DECL(SALT4, J25);
>
> #define J23 12
> -SIG_EXPR_LIST_DECL_SESG(J23, MDC2, MDIO2, SIG_DESC_SET(SCU410, 12));
> +SIG_EXPR_LIST_DECL_SESG(J23, MDC2, MDIO2, SIG_DESC_SET(SCU410, 12),
> + SIG_DESC_CLEAR(SCU470, 12));
> SIG_EXPR_LIST_DECL_SESG(J23, LHCLK, LPCHC, SIG_DESC_SET(SCU4B0, 12));
> PIN_DECL_2(J23, GPIOB4, MDC2, LHCLK);
>
> #define G26 13
> -SIG_EXPR_LIST_DECL_SESG(G26, MDIO2, MDIO2, SIG_DESC_SET(SCU410, 13));
> +SIG_EXPR_LIST_DECL_SESG(G26, MDIO2, MDIO2, SIG_DESC_SET(SCU410, 13),
> + SIG_DESC_CLEAR(SCU470, 13));
> SIG_EXPR_LIST_DECL_SESG(G26, LHFRAME, LPCHC, SIG_DESC_SET(SCU4B0, 13));
> PIN_DECL_2(G26, GPIOB5, MDIO2, LHFRAME);
>
> @@ -254,102 +270,123 @@ FUNC_GROUP_DECL(RMII3, H24, J22, H22, H23, G23,
> F23, F26, F25, E26);
> #define F24 28
> SIG_EXPR_LIST_DECL_SESG(F24, NCTS3, NCTS3, SIG_DESC_SET(SCU410, 28));
> SIG_EXPR_LIST_DECL_SESG(F24, RGMII4TXCK, RGMII4, SIG_DESC_SET(SCU4B0,
> 28),
> - SIG_DESC_SET(SCU510, 1));
> + SIG_DESC_CLEAR(SCU410, 28), SIG_DESC_SET(SCU510, 1));
We don't need this as SCU410[28] is cleared as part of disabling NCTS3.
> SIG_EXPR_LIST_DECL_SESG(F24, RMII4RCLKO, RMII4, SIG_DESC_SET(SCU4B0,
> 28),
> - SIG_DESC_CLEAR(SCU510, 1));
> + SIG_DESC_CLEAR(SCU410, 28), SIG_DESC_CLEAR(SCU510, 1));
As above.
> PIN_DECL_3(F24, GPIOD4, NCTS3, RGMII4TXCK, RMII4RCLKO);
> FUNC_GROUP_DECL(NCTS3, F24);
>
> #define E23 29
> SIG_EXPR_LIST_DECL_SESG(E23, NDCD3, NDCD3, SIG_DESC_SET(SCU410, 29));
> SIG_EXPR_LIST_DECL_SESG(E23, RGMII4TXCTL, RGMII4, SIG_DESC_SET(SCU4B0, 29),
> - SIG_DESC_SET(SCU510, 1));
> + SIG_DESC_CLEAR(SCU410, 29), SIG_DESC_SET(SCU510, 1));
Here too.
> SIG_EXPR_LIST_DECL_SESG(E23, RMII4TXEN, RMII4, SIG_DESC_SET(SCU4B0, 29),
> - SIG_DESC_CLEAR(SCU510, 1));
> + SIG_DESC_CLEAR(SCU410, 29), SIG_DESC_CLEAR(SCU510, 1));
And here.
> PIN_DECL_3(E23, GPIOD5, NDCD3, RGMII4TXCTL, RMII4TXEN);
> FUNC_GROUP_DECL(NDCD3, E23);
>
> #define E24 30
> SIG_EXPR_LIST_DECL_SESG(E24, NDSR3, NDSR3, SIG_DESC_SET(SCU410, 30));
> SIG_EXPR_LIST_DECL_SESG(E24, RGMII4TXD0, RGMII4, SIG_DESC_SET(SCU4B0, 30),
> - SIG_DESC_SET(SCU510, 1));
> + SIG_DESC_CLEAR(SCU410, 30), SIG_DESC_SET(SCU510, 1));
> SIG_EXPR_LIST_DECL_SESG(E24, RMII4TXD0, RMII4, SIG_DESC_SET(SCU4B0, 30),
And here.
> - SIG_DESC_CLEAR(SCU510, 1));
> + SIG_DESC_CLEAR(SCU410, 30), SIG_DESC_CLEAR(SCU510, 1));
And here.
> PIN_DECL_3(E24, GPIOD6, NDSR3, RGMII4TXD0, RMII4TXD0);
> FUNC_GROUP_DECL(NDSR3, E24);
>
> #define E25 31
> SIG_EXPR_LIST_DECL_SESG(E25, NRI3, NRI3, SIG_DESC_SET(SCU410, 31));
> SIG_EXPR_LIST_DECL_SESG(E25, RGMII4TXD1, RGMII4, SIG_DESC_SET(SCU4B0, 31),
> - SIG_DESC_SET(SCU510, 1));
> + SIG_DESC_CLEAR(SCU410, 31), SIG_DESC_SET(SCU510, 1));
And here.
> SIG_EXPR_LIST_DECL_SESG(E25, RMII4TXD1, RMII4, SIG_DESC_SET(SCU4B0, 31),
> - SIG_DESC_CLEAR(SCU510, 1));
> + SIG_DESC_CLEAR(SCU410, 31), SIG_DESC_CLEAR(SCU510, 1));
And here.
> PIN_DECL_3(E25, GPIOD7, NRI3, RGMII4TXD1, RMII4TXD1);
> FUNC_GROUP_DECL(NRI3, E25);
>
> #define D26 32
> -SIG_EXPR_LIST_DECL_SESG(D26, NDTR3, NDTR3, SIG_DESC_SET(SCU414, 0));
> +SIG_EXPR_LIST_DECL_SESG(D26, NDTR3, NDTR3, SIG_DESC_SET(SCU414, 0),
> + SIG_DESC_CLEAR(SCU470, 16));
> SIG_EXPR_LIST_DECL_SESG(D26, RGMII4TXD2, RGMII4, SIG_DESC_SET(SCU4B4, 0),
> - SIG_DESC_SET(SCU510, 1));
> + SIG_DESC_CLEAR(SCU470, 16), SIG_DESC_CLEAR(SCU474, 0),
What is SCU470[16] actually doing? The documentation only defines it in
terms of what it disables and doesn't explain why. Under what
conditions would this bit need to be set? When we want to use the pin
as GPIO? Something else?
Until that's answered I'll hold off on commenting on how we should be
describing it here.
> + SIG_DESC_SET(SCU510, 1));
> PIN_DECL_2(D26, GPIOE0, NDTR3, RGMII4TXD2);
> FUNC_GROUP_DECL(NDTR3, D26);
>
> #define D24 33
> -SIG_EXPR_LIST_DECL_SESG(D24, NRTS3, NRTS3, SIG_DESC_SET(SCU414, 1));
> +SIG_EXPR_LIST_DECL_SESG(D24, NRTS3, NRTS3, SIG_DESC_SET(SCU414, 1),
> + SIG_DESC_CLEAR(SCU470, 17));
> SIG_EXPR_LIST_DECL_SESG(D24, RGMII4TXD3, RGMII4, SIG_DESC_SET(SCU4B4, 1),
> - SIG_DESC_SET(SCU510, 1));
> + SIG_DESC_CLEAR(SCU470, 17), SIG_DESC_CLEAR(SCU474, 1),
> + SIG_DESC_SET(SCU510, 1));
> PIN_DECL_2(D24, GPIOE1, NRTS3, RGMII4TXD3);
> FUNC_GROUP_DECL(NRTS3, D24);
>
> #define C25 34
> -SIG_EXPR_LIST_DECL_SESG(C25, NCTS4, NCTS4, SIG_DESC_SET(SCU414, 2));
> +SIG_EXPR_LIST_DECL_SESG(C25, NCTS4, NCTS4, SIG_DESC_SET(SCU414, 2),
> + SIG_DESC_CLEAR(SCU470, 18));
> SIG_EXPR_LIST_DECL_SESG(C25, RGMII4RXCK, RGMII4, SIG_DESC_SET(SCU4B4, 2),
> - SIG_DESC_SET(SCU510, 1));
> + SIG_DESC_CLEAR(SCU470, 18), SIG_DESC_CLEAR(SCU474, 2),
> + SIG_DESC_SET(SCU510, 1));
> SIG_EXPR_LIST_DECL_SESG(C25, RMII4RCLKI, RMII4, SIG_DESC_SET(SCU4B4, 2),
> - SIG_DESC_CLEAR(SCU510, 1));
> + SIG_DESC_CLEAR(SCU470, 18), SIG_DESC_CLEAR(SCU474, 2),
> + SIG_DESC_CLEAR(SCU510, 1));
> PIN_DECL_3(C25, GPIOE2, NCTS4, RGMII4RXCK, RMII4RCLKI);
> FUNC_GROUP_DECL(NCTS4, C25);
>
> #define C26 35
> -SIG_EXPR_LIST_DECL_SESG(C26, NDCD4, NDCD4, SIG_DESC_SET(SCU414, 3));
> +SIG_EXPR_LIST_DECL_SESG(C26, NDCD4, NDCD4, SIG_DESC_SET(SCU414, 3),
> + SIG_DESC_CLEAR(SCU470, 19));
> SIG_EXPR_LIST_DECL_SESG(C26, RGMII4RXCTL, RGMII4, SIG_DESC_SET(SCU4B4, 3),
> - SIG_DESC_SET(SCU510, 1));
> + SIG_DESC_CLEAR(SCU470, 19), SIG_DESC_CLEAR(SCU474, 3),
> + SIG_DESC_SET(SCU510, 1));
> PIN_DECL_2(C26, GPIOE3, NDCD4, RGMII4RXCTL);
> FUNC_GROUP_DECL(NDCD4, C26);
>
> #define C24 36
> -SIG_EXPR_LIST_DECL_SESG(C24, NDSR4, NDSR4, SIG_DESC_SET(SCU414, 4));
> +SIG_EXPR_LIST_DECL_SESG(C24, NDSR4, NDSR4, SIG_DESC_SET(SCU414, 4),
> + SIG_DESC_CLEAR(SCU470, 20));
> SIG_EXPR_LIST_DECL_SESG(C24, RGMII4RXD0, RGMII4, SIG_DESC_SET(SCU4B4, 4),
> - SIG_DESC_SET(SCU510, 1));
> + SIG_DESC_CLEAR(SCU470, 20), SIG_DESC_CLEAR(SCU474, 4),
> + SIG_DESC_SET(SCU510, 1));
> SIG_EXPR_LIST_DECL_SESG(C24, RMII4RXD0, RMII4, SIG_DESC_SET(SCU4B4, 4),
> - SIG_DESC_CLEAR(SCU510, 1));
> + SIG_DESC_CLEAR(SCU470, 20), SIG_DESC_CLEAR(SCU474, 4),
> + SIG_DESC_CLEAR(SCU510, 1));
> PIN_DECL_3(C24, GPIOE4, NDSR4, RGMII4RXD0, RMII4RXD0);
> FUNC_GROUP_DECL(NDSR4, C24);
>
> #define B26 37
> -SIG_EXPR_LIST_DECL_SESG(B26, NRI4, NRI4, SIG_DESC_SET(SCU414, 5));
> +SIG_EXPR_LIST_DECL_SESG(B26, NRI4, NRI4, SIG_DESC_SET(SCU414, 5),
> + SIG_DESC_CLEAR(SCU470, 21));
> SIG_EXPR_LIST_DECL_SESG(B26, RGMII4RXD1, RGMII4, SIG_DESC_SET(SCU4B4, 5),
> - SIG_DESC_SET(SCU510, 1));
> + SIG_DESC_CLEAR(SCU470, 21), SIG_DESC_CLEAR(SCU474, 5),
> + SIG_DESC_SET(SCU510, 1));
> SIG_EXPR_LIST_DECL_SESG(B26, RMII4RXD1, RMII4, SIG_DESC_SET(SCU4B4, 5),
> - SIG_DESC_CLEAR(SCU510, 1));
> + SIG_DESC_CLEAR(SCU470, 21), SIG_DESC_CLEAR(SCU474, 5),
> + SIG_DESC_CLEAR(SCU510, 1));
> PIN_DECL_3(B26, GPIOE5, NRI4, RGMII4RXD1, RMII4RXD1);
> FUNC_GROUP_DECL(NRI4, B26);
>
> #define B25 38
> -SIG_EXPR_LIST_DECL_SESG(B25, NDTR4, NDTR4, SIG_DESC_SET(SCU414, 6));
> +SIG_EXPR_LIST_DECL_SESG(B25, NDTR4, NDTR4, SIG_DESC_SET(SCU414, 6),
> + SIG_DESC_CLEAR(SCU470, 22));
> SIG_EXPR_LIST_DECL_SESG(B25, RGMII4RXD2, RGMII4, SIG_DESC_SET(SCU4B4, 6),
> - SIG_DESC_SET(SCU510, 1));
> + SIG_DESC_CLEAR(SCU470, 22), SIG_DESC_CLEAR(SCU474, 6),
> + SIG_DESC_SET(SCU510, 1));
> SIG_EXPR_LIST_DECL_SESG(B25, RMII4CRSDV, RMII4, SIG_DESC_SET(SCU4B4, 6),
> - SIG_DESC_CLEAR(SCU510, 1));
> + SIG_DESC_CLEAR(SCU470, 22), SIG_DESC_CLEAR(SCU474, 6),
> + SIG_DESC_CLEAR(SCU510, 1));
> PIN_DECL_3(B25, GPIOE6, NDTR4, RGMII4RXD2, RMII4CRSDV);
> FUNC_GROUP_DECL(NDTR4, B25);
>
> #define B24 39
> -SIG_EXPR_LIST_DECL_SESG(B24, NRTS4, NRTS4, SIG_DESC_SET(SCU414, 7));
> +SIG_EXPR_LIST_DECL_SESG(B24, NRTS4, NRTS4, SIG_DESC_SET(SCU414, 7),
> + SIG_DESC_CLEAR(SCU470, 23));
> SIG_EXPR_LIST_DECL_SESG(B24, RGMII4RXD3, RGMII4, SIG_DESC_SET(SCU4B4, 7),
> - SIG_DESC_SET(SCU510, 1));
> + SIG_DESC_CLEAR(SCU470, 23), SIG_DESC_CLEAR(SCU474, 7),
> + SIG_DESC_SET(SCU510, 1));
> SIG_EXPR_LIST_DECL_SESG(B24, RMII4RXER, RMII4, SIG_DESC_SET(SCU4B4, 7),
> - SIG_DESC_CLEAR(SCU510, 1));
> + SIG_DESC_CLEAR(SCU470, 23), SIG_DESC_CLEAR(SCU474, 7),
> + SIG_DESC_CLEAR(SCU510, 1));
> PIN_DECL_3(B24, GPIOE7, NRTS4, RGMII4RXD3, RMII4RXER);
> FUNC_GROUP_DECL(NRTS4, B24);
>
> @@ -545,12 +582,14 @@ GROUP_DECL(UART12G0, D17, A16);
>
> #define E17 66
> SIG_EXPR_LIST_DECL_SESG(E17, MTCK, JTAGM, SIG_DESC_SET(SCU418, 2));
> -SIG_EXPR_LIST_DECL_SEMG(E17, TXD13, UART13G0, UART13, SIG_DESC_SET(SCU4B8, 2));
> +SIG_EXPR_LIST_DECL_SEMG(E17, TXD13, UART13G0, UART13, SIG_DESC_SET(SCU4B8, 2),
> + SIG_DESC_CLEAR(SCU418, 2));
This is cleared by disabling the MTCK signal, so no need to specify
SIG_DESC_CLEAR(SCU418, 2) here.
> PIN_DECL_2(E17, GPIOI2, MTCK, TXD13);
>
> #define D16 67
> SIG_EXPR_LIST_DECL_SESG(D16, MTMS, JTAGM, SIG_DESC_SET(SCU418, 3));
> -SIG_EXPR_LIST_DECL_SEMG(D16, RXD13, UART13G0, UART13, SIG_DESC_SET(SCU4B8, 3));
> +SIG_EXPR_LIST_DECL_SEMG(D16, RXD13, UART13G0, UART13, SIG_DESC_SET(SCU4B8, 3),
> + SIG_DESC_CLEAR(SCU418, 3));
As above.
> PIN_DECL_2(D16, GPIOI3, MTMS, RXD13);
>
> GROUP_DECL(UART13G0, E17, D16);
> @@ -581,114 +620,144 @@ FUNC_GROUP_DECL(SIOSCI, A15);
> #define B20 72
> SIG_EXPR_LIST_DECL_SEMG(B20, I3C3SCL, HVI3C3, I3C3, SIG_DESC_SET(SCU418, 8));
> SIG_EXPR_LIST_DECL_SESG(B20, SCL1, I2C1, SIG_DESC_SET(SCU4B8, 8));
> -PIN_DECL_2(B20, GPIOJ0, I3C3SCL, SCL1);
> +SIG_EXPR_LIST_DECL_SESG(B20, SSCL1, SI2C1, SIG_DESC_SET(SCU698, 8));
> +PIN_DECL_3(B20, GPIOJ0, I3C3SCL, SCL1, SSCL1);
Can you please split the Secure I2C changes out into a separate patch?
>
> #define A20 73
> SIG_EXPR_LIST_DECL_SEMG(A20, I3C3SDA, HVI3C3, I3C3, SIG_DESC_SET(SCU418, 9));
> SIG_EXPR_LIST_DECL_SESG(A20, SDA1, I2C1, SIG_DESC_SET(SCU4B8, 9));
> -PIN_DECL_2(A20, GPIOJ1, I3C3SDA, SDA1);
> +SIG_EXPR_LIST_DECL_SESG(A20, SSDA1, SI2C1, SIG_DESC_SET(SCU698, 9));
> +PIN_DECL_3(A20, GPIOJ1, I3C3SDA, SDA1, SSDA1);
>
> GROUP_DECL(HVI3C3, B20, A20);
> FUNC_GROUP_DECL(I2C1, B20, A20);
> +FUNC_GROUP_DECL(SI2C1, B20, A20);
>
> #define E19 74
> SIG_EXPR_LIST_DECL_SEMG(E19, I3C4SCL, HVI3C4, I3C4, SIG_DESC_SET(SCU418, 10));
> SIG_EXPR_LIST_DECL_SESG(E19, SCL2, I2C2, SIG_DESC_SET(SCU4B8, 10));
> -PIN_DECL_2(E19, GPIOJ2, I3C4SCL, SCL2);
> +SIG_EXPR_LIST_DECL_SESG(E19, SSCL2, SI2C2, SIG_DESC_SET(SCU698, 10));
> +PIN_DECL_3(E19, GPIOJ2, I3C4SCL, SCL2, SSCL2);
>
> #define D20 75
> SIG_EXPR_LIST_DECL_SEMG(D20, I3C4SDA, HVI3C4, I3C4, SIG_DESC_SET(SCU418, 11));
> SIG_EXPR_LIST_DECL_SESG(D20, SDA2, I2C2, SIG_DESC_SET(SCU4B8, 11));
> -PIN_DECL_2(D20, GPIOJ3, I3C4SDA, SDA2);
> +SIG_EXPR_LIST_DECL_SESG(D20, SSDA2, SI2C2, SIG_DESC_SET(SCU698, 11));
> +PIN_DECL_3(D20, GPIOJ3, I3C4SDA, SDA2, SSDA2);
>
> GROUP_DECL(HVI3C4, E19, D20);
> FUNC_GROUP_DECL(I2C2, E19, D20);
> +FUNC_GROUP_DECL(SI2C2, E19, D20);
>
> #define C19 76
> SIG_EXPR_LIST_DECL_SESG(C19, I3C5SCL, I3C5, SIG_DESC_SET(SCU418, 12));
> SIG_EXPR_LIST_DECL_SESG(C19, SCL3, I2C3, SIG_DESC_SET(SCU4B8, 12));
> -PIN_DECL_2(C19, GPIOJ4, I3C5SCL, SCL3);
> +SIG_EXPR_LIST_DECL_SESG(C19, SSCL3, SI2C3, SIG_DESC_SET(SCU698, 12));
> +PIN_DECL_3(C19, GPIOJ4, I3C5SCL, SCL3, SSCL3);
>
> #define A19 77
> SIG_EXPR_LIST_DECL_SESG(A19, I3C5SDA, I3C5, SIG_DESC_SET(SCU418, 13));
> SIG_EXPR_LIST_DECL_SESG(A19, SDA3, I2C3, SIG_DESC_SET(SCU4B8, 13));
> -PIN_DECL_2(A19, GPIOJ5, I3C5SDA, SDA3);
> +SIG_EXPR_LIST_DECL_SESG(A19, SSDA3, SI2C3, SIG_DESC_SET(SCU698, 13));
> +PIN_DECL_3(A19, GPIOJ5, I3C5SDA, SDA3, SSDA3);
>
> FUNC_GROUP_DECL(I3C5, C19, A19);
> FUNC_GROUP_DECL(I2C3, C19, A19);
> +FUNC_GROUP_DECL(SI2C3, C19, A19);
>
> #define C20 78
> SIG_EXPR_LIST_DECL_SESG(C20, I3C6SCL, I3C6, SIG_DESC_SET(SCU418, 14));
> SIG_EXPR_LIST_DECL_SESG(C20, SCL4, I2C4, SIG_DESC_SET(SCU4B8, 14));
> -PIN_DECL_2(C20, GPIOJ6, I3C6SCL, SCL4);
> +SIG_EXPR_LIST_DECL_SESG(C20, SSCL4, SI2C4, SIG_DESC_SET(SCU698, 14));
> +PIN_DECL_3(C20, GPIOJ6, I3C6SCL, SCL4, SSCL4);
>
> #define D19 79
> SIG_EXPR_LIST_DECL_SESG(D19, I3C6SDA, I3C6, SIG_DESC_SET(SCU418, 15));
> SIG_EXPR_LIST_DECL_SESG(D19, SDA4, I2C4, SIG_DESC_SET(SCU4B8, 15));
> -PIN_DECL_2(D19, GPIOJ7, I3C6SDA, SDA4);
> +SIG_EXPR_LIST_DECL_SESG(D19, SSDA4, SI2C4, SIG_DESC_SET(SCU698, 15));
> +PIN_DECL_3(D19, GPIOJ7, I3C6SDA, SDA4, SSDA4);
>
> FUNC_GROUP_DECL(I3C6, C20, D19);
> FUNC_GROUP_DECL(I2C4, C20, D19);
> +FUNC_GROUP_DECL(SI2C4, C20, D19);
>
> #define A11 80
> SIG_EXPR_LIST_DECL_SESG(A11, SCL5, I2C5, SIG_DESC_SET(SCU418, 16));
> -PIN_DECL_1(A11, GPIOK0, SCL5);
> +SIG_EXPR_LIST_DECL_SESG(A11, SSCL5, SI2C5, SIG_DESC_SET(SCU4B8, 16));
> +PIN_DECL_2(A11, GPIOK0, SCL5, SSCL5);
>
> #define C11 81
> SIG_EXPR_LIST_DECL_SESG(C11, SDA5, I2C5, SIG_DESC_SET(SCU418, 17));
> -PIN_DECL_1(C11, GPIOK1, SDA5);
> +SIG_EXPR_LIST_DECL_SESG(C11, SSDA5, SI2C5, SIG_DESC_SET(SCU4B8, 17));
> +PIN_DECL_2(C11, GPIOK1, SDA5, SSDA5);
>
> FUNC_GROUP_DECL(I2C5, A11, C11);
> +FUNC_GROUP_DECL(SI2C5, A11, C11);
>
> #define D12 82
> SIG_EXPR_LIST_DECL_SESG(D12, SCL6, I2C6, SIG_DESC_SET(SCU418, 18));
> -PIN_DECL_1(D12, GPIOK2, SCL6);
> +SIG_EXPR_LIST_DECL_SESG(D12, SSCL6, SI2C6, SIG_DESC_SET(SCU4B8, 18));
> +PIN_DECL_2(D12, GPIOK2, SCL6, SSCL6);
>
> #define E13 83
> SIG_EXPR_LIST_DECL_SESG(E13, SDA6, I2C6, SIG_DESC_SET(SCU418, 19));
> -PIN_DECL_1(E13, GPIOK3, SDA6);
> +SIG_EXPR_LIST_DECL_SESG(E13, SSDA6, SI2C6, SIG_DESC_SET(SCU4B8, 19));
> +PIN_DECL_2(E13, GPIOK3, SDA6, SSDA6);
>
> FUNC_GROUP_DECL(I2C6, D12, E13);
> +FUNC_GROUP_DECL(SI2C6, D12, E13);
>
> #define D11 84
> SIG_EXPR_LIST_DECL_SESG(D11, SCL7, I2C7, SIG_DESC_SET(SCU418, 20));
> -PIN_DECL_1(D11, GPIOK4, SCL7);
> +SIG_EXPR_LIST_DECL_SESG(D11, SSCL7, SI2C7, SIG_DESC_SET(SCU4B8, 20));
> +PIN_DECL_2(D11, GPIOK4, SCL7, SSCL7);
>
> #define E11 85
> SIG_EXPR_LIST_DECL_SESG(E11, SDA7, I2C7, SIG_DESC_SET(SCU418, 21));
> -PIN_DECL_1(E11, GPIOK5, SDA7);
> +SIG_EXPR_LIST_DECL_SESG(E11, SSDA7, SI2C7, SIG_DESC_SET(SCU4B8, 21));
> +PIN_DECL_2(E11, GPIOK5, SDA7, SSDA7);
>
> FUNC_GROUP_DECL(I2C7, D11, E11);
> +FUNC_GROUP_DECL(SI2C7, D11, E11);
>
> #define F13 86
> SIG_EXPR_LIST_DECL_SESG(F13, SCL8, I2C8, SIG_DESC_SET(SCU418, 22));
> -PIN_DECL_1(F13, GPIOK6, SCL8);
> +SIG_EXPR_LIST_DECL_SESG(F13, SSCL8, SI2C8, SIG_DESC_SET(SCU4B8, 22));
> +PIN_DECL_2(F13, GPIOK6, SCL8, SSCL8);
>
> #define E12 87
> SIG_EXPR_LIST_DECL_SESG(E12, SDA8, I2C8, SIG_DESC_SET(SCU418, 23));
> -PIN_DECL_1(E12, GPIOK7, SDA8);
> +SIG_EXPR_LIST_DECL_SESG(E12, SSDA8, SI2C8, SIG_DESC_SET(SCU4B8, 23));
> +PIN_DECL_2(E12, GPIOK7, SDA8, SSDA8);
>
> FUNC_GROUP_DECL(I2C8, F13, E12);
> +FUNC_GROUP_DECL(SI2C8, F13, E12);
>
> #define D15 88
> SIG_EXPR_LIST_DECL_SESG(D15, SCL9, I2C9, SIG_DESC_SET(SCU418, 24));
> -PIN_DECL_1(D15, GPIOL0, SCL9);
> +SIG_EXPR_LIST_DECL_SESG(D15, SSCL9, SI2C9, SIG_DESC_SET(SCU4B8, 24));
> +PIN_DECL_2(D15, GPIOL0, SCL9, SSCL9);
>
> #define A14 89
> SIG_EXPR_LIST_DECL_SESG(A14, SDA9, I2C9, SIG_DESC_SET(SCU418, 25));
> -PIN_DECL_1(A14, GPIOL1, SDA9);
> +SIG_EXPR_LIST_DECL_SESG(A14, SSDA9, SI2C9, SIG_DESC_SET(SCU4B8, 25));
> +PIN_DECL_2(A14, GPIOL1, SDA9, SSDA9);
>
> FUNC_GROUP_DECL(I2C9, D15, A14);
> +FUNC_GROUP_DECL(SI2C9, D15, A14);
>
> #define E15 90
> SIG_EXPR_LIST_DECL_SESG(E15, SCL10, I2C10, SIG_DESC_SET(SCU418, 26));
> -PIN_DECL_1(E15, GPIOL2, SCL10);
> +SIG_EXPR_LIST_DECL_SESG(E15, SSCL10, SI2C10, SIG_DESC_SET(SCU4B8, 26));
> +PIN_DECL_2(E15, GPIOL2, SCL10, SSCL10);
>
> #define A13 91
> SIG_EXPR_LIST_DECL_SESG(A13, SDA10, I2C10, SIG_DESC_SET(SCU418, 27));
> -PIN_DECL_1(A13, GPIOL3, SDA10);
> +SIG_EXPR_LIST_DECL_SESG(A13, SSDA10, SI2C10, SIG_DESC_SET(SCU4B8, 27));
> +PIN_DECL_2(A13, GPIOL3, SDA10, SSDA10);
>
> FUNC_GROUP_DECL(I2C10, E15, A13);
> +FUNC_GROUP_DECL(SI2C10, E15, A13);
>
> #define C15 92
> SSSF_PIN_DECL(C15, GPIOL4, TXD3, SIG_DESC_SET(SCU418, 28));
> @@ -983,9 +1052,9 @@ FUNC_GROUP_DECL(ADC7, AE18);
>
> #define AB16 160
> SIG_EXPR_LIST_DECL_SEMG(AB16, SALT9, SALT9G1, SALT9, SIG_DESC_SET(SCU434, 0),
> - SIG_DESC_CLEAR(SCU694, 16));
> + SIG_DESC_CLEAR(SCU694, 16), SIG_DESC_SET(SCU4D4, 0));
> SIG_EXPR_LIST_DECL_SESG(AB16, GPIU0, GPIU0, SIG_DESC_SET(SCU434, 0),
> - SIG_DESC_SET(SCU694, 16));
> + SIG_DESC_CLEAR(SCU4D4, 0));
This one seems legitimate. I'm wondering if what we need is a better
approach to the short-circuit expression evaluation, something between
my original short-circuit at first opportunity and Billy's never
short-circuit approach. It almost feels like we need groups that are
always configured in certain ways.
That or we split the enable and disable paths up properly and not
assume the inverse of enable is enough.
> SIG_EXPR_LIST_DECL_SESG(AB16, ADC8, ADC8);
> PIN_DECL_(AB16, SIG_EXPR_LIST_PTR(AB16, SALT9), SIG_EXPR_LIST_PTR(AB16, GPIU0),
> SIG_EXPR_LIST_PTR(AB16, ADC8));
> @@ -996,9 +1065,9 @@ FUNC_GROUP_DECL(ADC8, AB16);
>
> #define AA17 161
> SIG_EXPR_LIST_DECL_SEMG(AA17, SALT10, SALT10G1, SALT10,
> SIG_DESC_SET(SCU434, 1),
> - SIG_DESC_CLEAR(SCU694, 17));
> + SIG_DESC_CLEAR(SCU694, 17), SIG_DESC_SET(SCU4D4, 1));
> SIG_EXPR_LIST_DECL_SESG(AA17, GPIU1, GPIU1, SIG_DESC_SET(SCU434, 1),
> - SIG_DESC_SET(SCU694, 17));
> + SIG_DESC_CLEAR(SCU4D4, 1));
> SIG_EXPR_LIST_DECL_SESG(AA17, ADC9, ADC9);
> PIN_DECL_(AA17, SIG_EXPR_LIST_PTR(AA17, SALT10),
> SIG_EXPR_LIST_PTR(AA17, GPIU1),
> SIG_EXPR_LIST_PTR(AA17, ADC9));
> @@ -1009,9 +1078,9 @@ FUNC_GROUP_DECL(ADC9, AA17);
>
> #define AB17 162
> SIG_EXPR_LIST_DECL_SEMG(AB17, SALT11, SALT11G1, SALT11,
> SIG_DESC_SET(SCU434, 2),
> - SIG_DESC_CLEAR(SCU694, 18));
> + SIG_DESC_CLEAR(SCU694, 18), SIG_DESC_SET(SCU4D4, 2));
> SIG_EXPR_LIST_DECL_SESG(AB17, GPIU2, GPIU2, SIG_DESC_SET(SCU434, 2),
> - SIG_DESC_SET(SCU694, 18));
> + SIG_DESC_CLEAR(SCU4D4, 2));
> SIG_EXPR_LIST_DECL_SESG(AB17, ADC10, ADC10);
> PIN_DECL_(AB17, SIG_EXPR_LIST_PTR(AB17, SALT11),
> SIG_EXPR_LIST_PTR(AB17, GPIU2),
> SIG_EXPR_LIST_PTR(AB17, ADC10));
> @@ -1022,9 +1091,9 @@ FUNC_GROUP_DECL(ADC10, AB17);
>
> #define AE16 163
> SIG_EXPR_LIST_DECL_SEMG(AE16, SALT12, SALT12G1, SALT12,
> SIG_DESC_SET(SCU434, 3),
> - SIG_DESC_CLEAR(SCU694, 19));
> + SIG_DESC_CLEAR(SCU694, 19), SIG_DESC_SET(SCU4D4, 3));
> SIG_EXPR_LIST_DECL_SESG(AE16, GPIU3, GPIU3, SIG_DESC_SET(SCU434, 3),
> - SIG_DESC_SET(SCU694, 19));
> + SIG_DESC_CLEAR(SCU4D4, 3));
> SIG_EXPR_LIST_DECL_SESG(AE16, ADC11, ADC11);
> PIN_DECL_(AE16, SIG_EXPR_LIST_PTR(AE16, SALT12),
> SIG_EXPR_LIST_PTR(AE16, GPIU3),
> SIG_EXPR_LIST_PTR(AE16, ADC11));
> @@ -1035,9 +1104,9 @@ FUNC_GROUP_DECL(ADC11, AE16);
>
> #define AC16 164
> SIG_EXPR_LIST_DECL_SEMG(AC16, SALT13, SALT13G1, SALT13,
> SIG_DESC_SET(SCU434, 4),
> - SIG_DESC_CLEAR(SCU694, 20));
> + SIG_DESC_CLEAR(SCU694, 20), SIG_DESC_SET(SCU4D4, 4));
> SIG_EXPR_LIST_DECL_SESG(AC16, GPIU4, GPIU4, SIG_DESC_SET(SCU434, 4),
> - SIG_DESC_SET(SCU694, 20));
> + SIG_DESC_CLEAR(SCU4D4, 4));
> SIG_EXPR_LIST_DECL_SESG(AC16, ADC12, ADC12);
> PIN_DECL_(AC16, SIG_EXPR_LIST_PTR(AC16, SALT13),
> SIG_EXPR_LIST_PTR(AC16, GPIU4),
> SIG_EXPR_LIST_PTR(AC16, ADC12));
> @@ -1048,9 +1117,9 @@ FUNC_GROUP_DECL(ADC12, AC16);
>
> #define AA16 165
> SIG_EXPR_LIST_DECL_SEMG(AA16, SALT14, SALT14G1, SALT14,
> SIG_DESC_SET(SCU434, 5),
> - SIG_DESC_CLEAR(SCU694, 21));
> + SIG_DESC_CLEAR(SCU694, 21), SIG_DESC_SET(SCU4D4, 5));
> SIG_EXPR_LIST_DECL_SESG(AA16, GPIU5, GPIU5, SIG_DESC_SET(SCU434, 5),
> - SIG_DESC_SET(SCU694, 21));
> + SIG_DESC_CLEAR(SCU4D4, 5));
> SIG_EXPR_LIST_DECL_SESG(AA16, ADC13, ADC13);
> PIN_DECL_(AA16, SIG_EXPR_LIST_PTR(AA16, SALT14),
> SIG_EXPR_LIST_PTR(AA16, GPIU5),
> SIG_EXPR_LIST_PTR(AA16, ADC13));
> @@ -1061,9 +1130,9 @@ FUNC_GROUP_DECL(ADC13, AA16);
>
> #define AD16 166
> SIG_EXPR_LIST_DECL_SEMG(AD16, SALT15, SALT15G1, SALT15,
> SIG_DESC_SET(SCU434, 6),
> - SIG_DESC_CLEAR(SCU694, 22));
> + SIG_DESC_CLEAR(SCU694, 22), SIG_DESC_SET(SCU4D4, 6));
> SIG_EXPR_LIST_DECL_SESG(AD16, GPIU6, GPIU6, SIG_DESC_SET(SCU434, 6),
> - SIG_DESC_SET(SCU694, 22));
> + SIG_DESC_CLEAR(SCU4D4, 6));
> SIG_EXPR_LIST_DECL_SESG(AD16, ADC14, ADC14);
> PIN_DECL_(AD16, SIG_EXPR_LIST_PTR(AD16, SALT15),
> SIG_EXPR_LIST_PTR(AD16, GPIU6),
> SIG_EXPR_LIST_PTR(AD16, ADC14));
> @@ -1074,9 +1143,9 @@ FUNC_GROUP_DECL(ADC14, AD16);
>
> #define AC17 167
> SIG_EXPR_LIST_DECL_SEMG(AC17, SALT16, SALT16G1, SALT16,
> SIG_DESC_SET(SCU434, 7),
> - SIG_DESC_CLEAR(SCU694, 23));
> + SIG_DESC_CLEAR(SCU694, 23), SIG_DESC_SET(SCU4D4, 7));
> SIG_EXPR_LIST_DECL_SESG(AC17, GPIU7, GPIU7, SIG_DESC_SET(SCU434, 7),
> - SIG_DESC_SET(SCU694, 23));
> + SIG_DESC_CLEAR(SCU4D4, 7));
> SIG_EXPR_LIST_DECL_SESG(AC17, ADC15, ADC15);
> PIN_DECL_(AC17, SIG_EXPR_LIST_PTR(AC17, SALT16),
> SIG_EXPR_LIST_PTR(AC17, GPIU7),
> SIG_EXPR_LIST_PTR(AC17, ADC15));
> @@ -1116,43 +1185,50 @@ 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_SET(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AB7, ESPID0, ESPI, SIG_DESC_SET(SCU434, 16));
> +SIG_EXPR_LIST_DECL_SESG(AB7, ESPID0, ESPI, SIG_DESC_SET(SCU434, 16),
> + SIG_DESC_CLEAR(SCU510, 6));
Another reason to dwell on my short-circuit/enable-disable path musing
above.
> 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_SET(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AB8, ESPID1, ESPI, SIG_DESC_SET(SCU434, 17));
> +SIG_EXPR_LIST_DECL_SESG(AB8, ESPID1, ESPI, SIG_DESC_SET(SCU434, 17),
> + SIG_DESC_CLEAR(SCU510, 6));
> 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_SET(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AC8, ESPID2, ESPI, SIG_DESC_SET(SCU434, 18));
> +SIG_EXPR_LIST_DECL_SESG(AC8, ESPID2, ESPI, SIG_DESC_SET(SCU434, 18),
> + SIG_DESC_CLEAR(SCU510, 6));
> 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_SET(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AC7, ESPID3, ESPI, SIG_DESC_SET(SCU434, 19));
> +SIG_EXPR_LIST_DECL_SESG(AC7, ESPID3, ESPI, SIG_DESC_SET(SCU434, 19),
> + SIG_DESC_CLEAR(SCU510, 6));
> 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_SET(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AE7, ESPICK, ESPI, SIG_DESC_SET(SCU434, 20));
> +SIG_EXPR_LIST_DECL_SESG(AE7, ESPICK, ESPI, SIG_DESC_SET(SCU434, 20),
> + SIG_DESC_CLEAR(SCU510, 6));
> 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_SET(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AF7, ESPICS, ESPI, SIG_DESC_SET(SCU434, 21));
> +SIG_EXPR_LIST_DECL_SESG(AF7, ESPICS, ESPI, SIG_DESC_SET(SCU434, 21),
> + SIG_DESC_CLEAR(SCU510, 6));
> 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_SET(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AD7, ESPIALT, ESPIALT, SIG_DESC_SET(SCU434, 22));
> +SIG_EXPR_LIST_DECL_SESG(AD7, ESPIALT, ESPIALT, SIG_DESC_SET(SCU434, 22),
> + SIG_DESC_CLEAR(SCU510, 6));
> PIN_DECL_2(AD7, GPIOW6, LSIRQ, ESPIALT);
> FUNC_GROUP_DECL(LSIRQ, AD7);
> FUNC_GROUP_DECL(ESPIALT, AD7);
> @@ -1160,7 +1236,8 @@ FUNC_GROUP_DECL(ESPIALT, AD7);
> #define AD8 183
> SIG_EXPR_LIST_DECL_SESG(AD8, LPCRST, LPC, SIG_DESC_SET(SCU434, 23),
> SIG_DESC_SET(SCU510, 6));
> -SIG_EXPR_LIST_DECL_SESG(AD8, ESPIRST, ESPI, SIG_DESC_SET(SCU434, 23));
> +SIG_EXPR_LIST_DECL_SESG(AD8, ESPIRST, ESPI, SIG_DESC_SET(SCU434, 23),
> + SIG_DESC_CLEAR(SCU510, 6));
> PIN_DECL_2(AD8, GPIOW7, LPCRST, ESPIRST);
>
> FUNC_GROUP_DECL(LPC, AB7, AB8, AC8, AC7, AE7, AF7, AD8);
> @@ -1201,7 +1278,7 @@ SIG_EXPR_LIST_DECL_SEMG(AB10, RXD12, UART12G1, UART12,
> SIG_DESC_SET(SCU4D4, 31));
> PIN_DECL_2(AB10, GPIOX7, SPI2DQ3, RXD12);
>
> -GROUP_DECL(QSPI2, AE8, AF8, AB9, AD9, AF9, AB10);
> +GROUP_DECL(QSPI2, AF9, AB10);
What's going on here? If we want QSPI then we want all the pins muxed, right?
> FUNC_DECL_2(SPI2, SPI2, QSPI2);
>
> GROUP_DECL(UART12G1, AF9, AB10);
> @@ -1236,15 +1313,21 @@ FUNC_GROUP_DECL(SALT8, AA12);
> FUNC_GROUP_DECL(WDTRST4, AA12);
>
> #define AE12 196
> +SIG_EXPR_LIST_DECL_SEMG(AE12, FWSPIDQ2, FWQSPID, FWSPID,
> + SIG_DESC_SET(SCU438, 4));
> SIG_EXPR_LIST_DECL_SESG(AE12, FWSPIQ2, FWQSPI, SIG_DESC_SET(SCU438, 4));
> SIG_EXPR_LIST_DECL_SESG(AE12, GPIOY4, GPIOY4);
> -PIN_DECL_(AE12, SIG_EXPR_LIST_PTR(AE12, FWSPIQ2),
> +PIN_DECL_(AE12, SIG_EXPR_LIST_PTR(AE12, FWSPIDQ2),
> + SIG_EXPR_LIST_PTR(AE12, FWSPIQ2),
> SIG_EXPR_LIST_PTR(AE12, GPIOY4));
>
> #define AF12 197
> +SIG_EXPR_LIST_DECL_SEMG(AF12, FWSPIDQ3, FWQSPID, FWSPID,
> + SIG_DESC_SET(SCU438, 5));
> SIG_EXPR_LIST_DECL_SESG(AF12, FWSPIQ3, FWQSPI, SIG_DESC_SET(SCU438, 5));
> SIG_EXPR_LIST_DECL_SESG(AF12, GPIOY5, GPIOY5);
> -PIN_DECL_(AF12, SIG_EXPR_LIST_PTR(AF12, FWSPIQ3),
> +PIN_DECL_(AF12, SIG_EXPR_LIST_PTR(AF12, FWSPIDQ3),
> + SIG_EXPR_LIST_PTR(AF12, FWSPIQ3),
> SIG_EXPR_LIST_PTR(AF12, GPIOY5));
> FUNC_GROUP_DECL(FWQSPI, AE12, AF12);
>
> @@ -1289,7 +1372,7 @@ SIG_EXPR_LIST_DECL_SEMG(AF10, RXD13, UART13G1, UART13,
> SIG_DESC_CLEAR(SCU4B8, 3), SIG_DESC_SET(SCU4D8, 15));
> PIN_DECL_2(AF10, GPIOZ7, SPI1DQ3, RXD13);
>
> -GROUP_DECL(QSPI1, AB11, AC11, AA11, AD11, AF10);
> +GROUP_DECL(QSPI1, AD11, AF10);
Same query. Changing this requires changing the mux state specification
in the platform devicetree, which means any existing consumers will
break, right?
> FUNC_DECL_2(SPI1, SPI1, QSPI1);
>
> GROUP_DECL(UART13G1, AD11, AF10);
> @@ -1519,8 +1602,9 @@ SIG_EXPR_LIST_DECL_SEMG(Y4, EMMCDAT7, EMMCG8,
> EMMC, SIG_DESC_SET(SCU404, 3));
> PIN_DECL_3(Y4, GPIO18E3, FWSPIDMISO, VBMISO, EMMCDAT7);
>
> GROUP_DECL(FWSPID, Y1, Y2, Y3, Y4);
> +GROUP_DECL(FWQSPID, Y1, Y2, Y3, Y4, AE12, AF12);
> GROUP_DECL(EMMCG8, AB4, AA4, AC4, AA5, Y5, AB5, AB6, AC5, Y1, Y2, Y3, Y4);
> -FUNC_DECL_1(FWSPID, FWSPID);
> +FUNC_DECL_2(FWSPID, FWSPID, FWQSPID);
> FUNC_GROUP_DECL(VB, Y1, Y2, Y3, Y4);
> FUNC_DECL_3(EMMC, EMMCG1, EMMCG4, EMMCG8);
> /*
> @@ -1528,14 +1612,18 @@ FUNC_DECL_3(EMMC, EMMCG1, EMMCG4, EMMCG8);
> * following 4 pins
> */
> #define AF25 244
> -SIG_EXPR_LIST_DECL_SEMG(AF25, I3C3SCL, I3C3, I3C3, SIG_DESC_SET(SCU438, 20));
> -SIG_EXPR_LIST_DECL_SESG(AF25, FSI1CLK, FSI1, SIG_DESC_SET(SCU4D8, 20));
> +SIG_EXPR_LIST_DECL_SEMG(AF25, I3C3SCL, I3C3, I3C3, SIG_DESC_SET(SCU438, 20),
> + SIG_DESC_SET(SCU4D8, 20));
> +SIG_EXPR_LIST_DECL_SESG(AF25, FSI1CLK, FSI1, SIG_DESC_CLEAR(SCU438, 20),
SCU438[20] is already cleared by disabling I3C3SCL.
> + SIG_DESC_SET(SCU4D8, 20));
> PIN_DECL_(AF25, SIG_EXPR_LIST_PTR(AF25, I3C3SCL),
> SIG_EXPR_LIST_PTR(AF25, FSI1CLK));
>
> #define AE26 245
> -SIG_EXPR_LIST_DECL_SEMG(AE26, I3C3SDA, I3C3, I3C3, SIG_DESC_SET(SCU438, 21));
> -SIG_EXPR_LIST_DECL_SESG(AE26, FSI1DATA, FSI1, SIG_DESC_SET(SCU4D8, 21));
> +SIG_EXPR_LIST_DECL_SEMG(AE26, I3C3SDA, I3C3, I3C3, SIG_DESC_SET(SCU438, 21),
> + SIG_DESC_SET(SCU4D8, 21));
> +SIG_EXPR_LIST_DECL_SESG(AE26, FSI1DATA, FSI1, SIG_DESC_CLEAR(SCU438, 21),
As above.
> + SIG_DESC_SET(SCU4D8, 21));
> PIN_DECL_(AE26, SIG_EXPR_LIST_PTR(AE26, I3C3SDA),
> SIG_EXPR_LIST_PTR(AE26, FSI1DATA));
>
> @@ -1544,14 +1632,18 @@ FUNC_DECL_2(I3C3, HVI3C3, I3C3);
> FUNC_GROUP_DECL(FSI1, AF25, AE26);
>
> #define AE25 246
> -SIG_EXPR_LIST_DECL_SEMG(AE25, I3C4SCL, I3C4, I3C4, SIG_DESC_SET(SCU438, 22));
> -SIG_EXPR_LIST_DECL_SESG(AE25, FSI2CLK, FSI2, SIG_DESC_SET(SCU4D8, 22));
> +SIG_EXPR_LIST_DECL_SEMG(AE25, I3C4SCL, I3C4, I3C4, SIG_DESC_SET(SCU438, 22),
As above.
> + SIG_DESC_SET(SCU4D8, 22));
> +SIG_EXPR_LIST_DECL_SESG(AE25, FSI2CLK, FSI2, SIG_DESC_CLEAR(SCU438, 22),
As above.
> + SIG_DESC_SET(SCU4D8, 22));
> PIN_DECL_(AE25, SIG_EXPR_LIST_PTR(AE25, I3C4SCL),
> SIG_EXPR_LIST_PTR(AE25, FSI2CLK));
>
> #define AF24 247
> -SIG_EXPR_LIST_DECL_SEMG(AF24, I3C4SDA, I3C4, I3C4, SIG_DESC_SET(SCU438, 23));
> -SIG_EXPR_LIST_DECL_SESG(AF24, FSI2DATA, FSI2, SIG_DESC_SET(SCU4D8, 23));
> +SIG_EXPR_LIST_DECL_SEMG(AF24, I3C4SDA, I3C4, I3C4, SIG_DESC_SET(SCU438, 23),
> + SIG_DESC_SET(SCU4D8, 23));
As above.
> +SIG_EXPR_LIST_DECL_SESG(AF24, FSI2DATA, FSI2, SIG_DESC_CLEAR(SCU438, 23),
As above.
> + SIG_DESC_SET(SCU4D8, 23));
> PIN_DECL_(AF24, SIG_EXPR_LIST_PTR(AF24, I3C4SDA),
> SIG_EXPR_LIST_PTR(AF24, FSI2DATA));
>
> @@ -1592,9 +1684,10 @@ SIG_EXPR_LIST_DECL_SEMG(A4, USB2ADPDP, USBA,
> USB2ADP, USB2ADP_DESC,
> SIG_DESC_SET(SCUC20, 16));
> SIG_EXPR_LIST_DECL_SEMG(A4, USB2ADDP, USBA, USB2AD, USB2AD_DESC);
> SIG_EXPR_LIST_DECL_SEMG(A4, USB2AHDP, USBA, USB2AH, USB2AH_DESC);
> -SIG_EXPR_LIST_DECL_SEMG(A4, USB2AHPDP, USBA, USB2AHP, USB2AHP_DESC);
> +SIG_EXPR_LIST_DECL_SEMG(A4, USB2AHPDP, USBA, USB2AHP, USB2AHP_DESC,
> + SIG_DESC_SET(SCUC20, 16));
Doesn't this only expose the device on PCIe? Why is this a problem for
pinmux?
Please split this out into a separate patch so we can debate it
separately.
> PIN_DECL_(A4, SIG_EXPR_LIST_PTR(A4, USB2ADPDP), SIG_EXPR_LIST_PTR(A4,
> USB2ADDP),
> - SIG_EXPR_LIST_PTR(A4, USB2AHDP));
> + SIG_EXPR_LIST_PTR(A4, USB2AHDP), SIG_EXPR_LIST_PTR(A4, USB2AHPDP));
>
> #define B4 253
> SIG_EXPR_LIST_DECL_SEMG(B4, USB2ADPDN, USBA, USB2ADP, USB2ADP_DESC);
> @@ -1602,7 +1695,7 @@ SIG_EXPR_LIST_DECL_SEMG(B4, USB2ADDN, USBA,
> USB2AD, USB2AD_DESC);
> SIG_EXPR_LIST_DECL_SEMG(B4, USB2AHDN, USBA, USB2AH, USB2AH_DESC);
> SIG_EXPR_LIST_DECL_SEMG(B4, USB2AHPDN, USBA, USB2AHP, USB2AHP_DESC);
> PIN_DECL_(B4, SIG_EXPR_LIST_PTR(B4, USB2ADPDN), SIG_EXPR_LIST_PTR(B4,
> USB2ADDN),
> - SIG_EXPR_LIST_PTR(B4, USB2AHDN));
> + SIG_EXPR_LIST_PTR(B4, USB2AHDN), SIG_EXPR_LIST_PTR(B4, USB2AHPDN));
>
> GROUP_DECL(USBA, A4, B4);
>
> @@ -1631,6 +1724,23 @@ FUNC_DECL_1(USB11BHID, USBB);
> FUNC_DECL_1(USB2BD, USBB);
> FUNC_DECL_1(USB2BH, USBB);
>
> +/* bit19: Enable RC-L DMA mode
> + * bit23: Enable RC-L DMA decode
> + */
> +#define PCIERC0_DESC { ASPEED_IP_SCU, SCUC24, GENMASK(23, 19), 0x1f, 0 }
DMA configuration doesn't sound like the responsibility of a pinctrl
driver.
> +
> +#define A7 256
> +SIG_EXPR_LIST_DECL_SESG(A7, PERST, PCIERC0, SIG_DESC_SET(SCU040, 21),
> + SIG_DESC_CLEAR(SCU0C8, 6), PCIERC0_DESC);
> +PIN_DECL_(A7, SIG_EXPR_LIST_PTR(A7, PERST));
> +FUNC_GROUP_DECL(PCIERC0, A7);
> +
> +#define D7 257
> +SIG_EXPR_LIST_DECL_SESG(D7, RCRST, PCIERC1, SIG_DESC_SET(SCU040, 19),
> + SIG_DESC_SET(SCU500, 24));
> +PIN_DECL_(D7, SIG_EXPR_LIST_PTR(D7, RCRST));
> +FUNC_GROUP_DECL(PCIERC1, D7);
> +
Please split this out into a separate patch.
> /* Pins, groups and functions are sort(1):ed alphabetically for sanity */
>
> static struct pinctrl_pin_desc aspeed_g6_pins[ASPEED_G6_NR_PINS] = {
> @@ -1653,6 +1763,7 @@ static struct pinctrl_pin_desc
> aspeed_g6_pins[ASPEED_G6_NR_PINS] = {
> ASPEED_PINCTRL_PIN(A3),
> ASPEED_PINCTRL_PIN(A4),
> ASPEED_PINCTRL_PIN(A6),
> + ASPEED_PINCTRL_PIN(A7),
> ASPEED_PINCTRL_PIN(AA11),
> ASPEED_PINCTRL_PIN(AA12),
> ASPEED_PINCTRL_PIN(AA16),
> @@ -1801,6 +1912,7 @@ static struct pinctrl_pin_desc
> aspeed_g6_pins[ASPEED_G6_NR_PINS] = {
> ASPEED_PINCTRL_PIN(D4),
> ASPEED_PINCTRL_PIN(D5),
> ASPEED_PINCTRL_PIN(D6),
> + ASPEED_PINCTRL_PIN(D7),
> ASPEED_PINCTRL_PIN(E1),
> ASPEED_PINCTRL_PIN(E11),
> ASPEED_PINCTRL_PIN(E12),
> @@ -1916,6 +2028,7 @@ static const struct aspeed_pin_group
> aspeed_g6_groups[] = {
> ASPEED_PINCTRL_GROUP(FSI2),
> ASPEED_PINCTRL_GROUP(FWSPIABR),
> ASPEED_PINCTRL_GROUP(FWSPID),
> + ASPEED_PINCTRL_GROUP(FWQSPID),
> ASPEED_PINCTRL_GROUP(FWQSPI),
> ASPEED_PINCTRL_GROUP(FWSPIWP),
> ASPEED_PINCTRL_GROUP(GPIT0),
> @@ -1953,6 +2066,16 @@ static const struct aspeed_pin_group
> aspeed_g6_groups[] = {
> ASPEED_PINCTRL_GROUP(I2C7),
> ASPEED_PINCTRL_GROUP(I2C8),
> ASPEED_PINCTRL_GROUP(I2C9),
> + ASPEED_PINCTRL_GROUP(SI2C1),
> + ASPEED_PINCTRL_GROUP(SI2C2),
> + ASPEED_PINCTRL_GROUP(SI2C3),
> + ASPEED_PINCTRL_GROUP(SI2C4),
> + ASPEED_PINCTRL_GROUP(SI2C5),
> + ASPEED_PINCTRL_GROUP(SI2C6),
> + ASPEED_PINCTRL_GROUP(SI2C7),
> + ASPEED_PINCTRL_GROUP(SI2C8),
> + ASPEED_PINCTRL_GROUP(SI2C9),
> + ASPEED_PINCTRL_GROUP(SI2C10),
This hunk should go in the separate patch introducing the Secure I2C bits.
> ASPEED_PINCTRL_GROUP(I3C1),
> ASPEED_PINCTRL_GROUP(I3C2),
> ASPEED_PINCTRL_GROUP(I3C3),
> @@ -2066,6 +2189,8 @@ static const struct aspeed_pin_group
> aspeed_g6_groups[] = {
> ASPEED_PINCTRL_GROUP(SALT9G1),
> ASPEED_PINCTRL_GROUP(SD1),
> ASPEED_PINCTRL_GROUP(SD2),
> + ASPEED_PINCTRL_GROUP(PCIERC0),
> + ASPEED_PINCTRL_GROUP(PCIERC1),
This hunk should go in the separate patch introducing the PCIe RC bits.
> ASPEED_PINCTRL_GROUP(EMMCG1),
> ASPEED_PINCTRL_GROUP(EMMCG4),
> ASPEED_PINCTRL_GROUP(EMMCG8),
> @@ -2193,6 +2318,16 @@ static const struct aspeed_pin_function
> aspeed_g6_functions[] = {
> ASPEED_PINCTRL_FUNC(I2C7),
> ASPEED_PINCTRL_FUNC(I2C8),
> ASPEED_PINCTRL_FUNC(I2C9),
> + ASPEED_PINCTRL_FUNC(SI2C1),
> + ASPEED_PINCTRL_FUNC(SI2C2),
> + ASPEED_PINCTRL_FUNC(SI2C3),
> + ASPEED_PINCTRL_FUNC(SI2C4),
> + ASPEED_PINCTRL_FUNC(SI2C5),
> + ASPEED_PINCTRL_FUNC(SI2C6),
> + ASPEED_PINCTRL_FUNC(SI2C7),
> + ASPEED_PINCTRL_FUNC(SI2C8),
> + ASPEED_PINCTRL_FUNC(SI2C9),
> + ASPEED_PINCTRL_FUNC(SI2C10),
Again in the Secure I2C patch.
> ASPEED_PINCTRL_FUNC(I3C1),
> ASPEED_PINCTRL_FUNC(I3C2),
> ASPEED_PINCTRL_FUNC(I3C3),
> @@ -2307,6 +2442,8 @@ static const struct aspeed_pin_function
> aspeed_g6_functions[] = {
> ASPEED_PINCTRL_FUNC(SPI2),
> ASPEED_PINCTRL_FUNC(SPI2CS1),
> ASPEED_PINCTRL_FUNC(SPI2CS2),
> + ASPEED_PINCTRL_FUNC(PCIERC0),
> + ASPEED_PINCTRL_FUNC(PCIERC1),
Again in the PCIe RC patch.
Cheers,
Andrew
More information about the Linux-aspeed
mailing list