| Message ID | 20220414205950.26406-1-zev@bewilderbeest.net |
|---|---|
| State | New |
| Headers | show |
| Series | [u-boot,v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for RGMII2 | expand |
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 >
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
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)
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(+)