[PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for RGMII2

[PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for RGMII2

[Zev Weiss]

The bits configured by this pinctrl group are set to the same values
as their reset defaults, so RGMII2 functionality would likely work
anyway, but it seems good to make it explicit instead of relying on
that.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 drivers/pinctrl/aspeed/pinctrl_ast2400.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pinctrl/aspeed/pinctrl_ast2400.c b/drivers/pinctrl/aspeed/pinctrl_ast2400.c
index a8a5ff517108..debe6b83d553 100644
--- a/drivers/pinctrl/aspeed/pinctrl_ast2400.c
+++ b/drivers/pinctrl/aspeed/pinctrl_ast2400.c
@@ -133,6 +133,10 @@ static struct aspeed_sig_desc rxd3_link[] = {
 	{ 0x80, BIT(23), 0},
 };
 
+static struct aspeed_sig_desc rgmii2_link[] = {
+	{ 0xa0, GENMASK(23, 18) | GENMASK(11, 6), 1 },
+};
+
 static const struct aspeed_group_config ast2400_groups[] = {
 	{ "MAC1LINK", 1, mac1_link },
 	{ "MAC2LINK", 1, mac2_link },
@@ -156,6 +160,7 @@ static const struct aspeed_group_config ast2400_groups[] = {
 	{ "SPI1CS1", 1, spi1cs1_link},
 	{ "TXD3", 1, txd3_link },
 	{ "RXD3", 1, rxd3_link },
+	{ "RGMII2", 1, rgmii2_link },
 };
 
 static int ast2400_pinctrl_get_groups_count(struct udevice *dev)
-- 
2.35.1

Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for RGMII2

[Joel Stanley]

On Thu, 14 Apr 2022 at 21:00, Zev Weiss <zev@bewilderbeest.net> wrote:
>
> The bits configured by this pinctrl group are set to the same values
> as their reset defaults, so RGMII2 functionality would likely work
> anyway, but it seems good to make it explicit instead of relying on
> that.
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>

This looks good. When reviewing I checked it against the datasheet,
and I think it makes it clearer if we also add rgmii1:

--- a/drivers/pinctrl/aspeed/pinctrl_ast2400.c
+++ b/drivers/pinctrl/aspeed/pinctrl_ast2400.c
@@ -133,6 +133,10 @@ static struct aspeed_sig_desc rxd3_link[] = {
        { 0x80, BIT(23), 0},
 };

+static struct aspeed_sig_desc rgmii1_link[] = {
+       { 0xa0, GENMASK(17, 12) | GENMASK(5, 0), 1 },
+};
+
 static struct aspeed_sig_desc rgmii2_link[] = {
        { 0xa0, GENMASK(23, 18) | GENMASK(11, 6), 1 },
 };
@@ -160,6 +164,7 @@ static const struct aspeed_group_config ast2400_groups[] = {
        { "SPI1CS1", 1, spi1cs1_link},
        { "TXD3", 1, txd3_link },
        { "RXD3", 1, rxd3_link },
+       { "RGMII1", 1, rgmii1_link },
        { "RGMII2", 1, rgmii2_link },
 };

Can you resend with that squashed in, if it looks correct to you?


> ---
>  drivers/pinctrl/aspeed/pinctrl_ast2400.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/pinctrl/aspeed/pinctrl_ast2400.c b/drivers/pinctrl/aspeed/pinctrl_ast2400.c
> index a8a5ff517108..debe6b83d553 100644
> --- a/drivers/pinctrl/aspeed/pinctrl_ast2400.c
> +++ b/drivers/pinctrl/aspeed/pinctrl_ast2400.c
> @@ -133,6 +133,10 @@ static struct aspeed_sig_desc rxd3_link[] = {
>         { 0x80, BIT(23), 0},
>  };
>
> +static struct aspeed_sig_desc rgmii2_link[] = {
> +       { 0xa0, GENMASK(23, 18) | GENMASK(11, 6), 1 },
> +};
> +
>  static const struct aspeed_group_config ast2400_groups[] = {
>         { "MAC1LINK", 1, mac1_link },
>         { "MAC2LINK", 1, mac2_link },
> @@ -156,6 +160,7 @@ static const struct aspeed_group_config ast2400_groups[] = {
>         { "SPI1CS1", 1, spi1cs1_link},
>         { "TXD3", 1, txd3_link },
>         { "RXD3", 1, rxd3_link },
> +       { "RGMII2", 1, rgmii2_link },
>  };
>
>  static int ast2400_pinctrl_get_groups_count(struct udevice *dev)
> --
> 2.35.1
>

Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for RGMII2

[Zev Weiss]

On Tue, May 10, 2022 at 10:55:32PM PDT, Joel Stanley wrote:
> On Thu, 14 Apr 2022 at 21:00, Zev Weiss <zev@bewilderbeest.net> wrote:
> >
> > The bits configured by this pinctrl group are set to the same values
> > as their reset defaults, so RGMII2 functionality would likely work
> > anyway, but it seems good to make it explicit instead of relying on
> > that.
> >
> > Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> 
> This looks good. When reviewing I checked it against the datasheet,
> and I think it makes it clearer if we also add rgmii1:
> 
> --- a/drivers/pinctrl/aspeed/pinctrl_ast2400.c
> +++ b/drivers/pinctrl/aspeed/pinctrl_ast2400.c
> @@ -133,6 +133,10 @@ static struct aspeed_sig_desc rxd3_link[] = {
>         { 0x80, BIT(23), 0},
>  };
> 
> +static struct aspeed_sig_desc rgmii1_link[] = {
> +       { 0xa0, GENMASK(17, 12) | GENMASK(5, 0), 1 },
> +};
> +
>  static struct aspeed_sig_desc rgmii2_link[] = {
>         { 0xa0, GENMASK(23, 18) | GENMASK(11, 6), 1 },
>  };
> @@ -160,6 +164,7 @@ static const struct aspeed_group_config ast2400_groups[] = {
>         { "SPI1CS1", 1, spi1cs1_link},
>         { "TXD3", 1, txd3_link },
>         { "RXD3", 1, rxd3_link },
> +       { "RGMII1", 1, rgmii1_link },
>         { "RGMII2", 1, rgmii2_link },
>  };
> 
> Can you resend with that squashed in, if it looks correct to you?
> 

Sure, will do -- and yes, that aligns with my reading of the datasheet 
as well.


Thanks,
Zev