* [PATCH net-next v2 1/6] net: dsa: add support for phylink mac_select_pcs()
2022-02-17 18:29 [PATCH net-next v2 0/6] net: dsa: qca8k: convert to phylink_pcs and mark as non-legacy Russell King (Oracle)
@ 2022-02-17 18:30 ` Russell King (Oracle)
2022-02-19 21:12 ` Vladimir Oltean
2022-02-17 18:30 ` [PATCH net-next v2 2/6] net: dsa: qca8k: move qca8k_setup() Russell King (Oracle)
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Russell King (Oracle) @ 2022-02-17 18:30 UTC (permalink / raw)
To: Ansuel Smith
Cc: Andrew Lunn, David S. Miller, Florian Fainelli, Jakub Kicinski,
netdev, Vivien Didelot, Vladimir Oltean
Add DSA support for the phylink mac_select_pcs() method so DSA drivers
can return provide phylink with the appropriate PCS for the PHY
interface mode.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
include/net/dsa.h | 3 +++
net/dsa/port.c | 15 +++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 85cb9aed4c51..87aef3ed88a7 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -788,6 +788,9 @@ struct dsa_switch_ops {
void (*phylink_validate)(struct dsa_switch *ds, int port,
unsigned long *supported,
struct phylink_link_state *state);
+ struct phylink_pcs *(*phylink_mac_select_pcs)(struct dsa_switch *ds,
+ int port,
+ phy_interface_t iface);
int (*phylink_mac_link_state)(struct dsa_switch *ds, int port,
struct phylink_link_state *state);
void (*phylink_mac_config)(struct dsa_switch *ds, int port,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index cca5cf686f74..d8534fd9fab9 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -1053,6 +1053,20 @@ static void dsa_port_phylink_mac_pcs_get_state(struct phylink_config *config,
}
}
+static struct phylink_pcs *
+dsa_port_phylink_mac_select_pcs(struct phylink_config *config,
+ phy_interface_t interface)
+{
+ struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
+ struct dsa_switch *ds = dp->ds;
+ struct phylink_pcs *pcs = NULL;
+
+ if (ds->ops->phylink_mac_select_pcs)
+ pcs = ds->ops->phylink_mac_select_pcs(ds, dp->index, interface);
+
+ return pcs;
+}
+
static void dsa_port_phylink_mac_config(struct phylink_config *config,
unsigned int mode,
const struct phylink_link_state *state)
@@ -1119,6 +1133,7 @@ static void dsa_port_phylink_mac_link_up(struct phylink_config *config,
static const struct phylink_mac_ops dsa_port_phylink_mac_ops = {
.validate = dsa_port_phylink_validate,
+ .mac_select_pcs = dsa_port_phylink_mac_select_pcs,
.mac_pcs_get_state = dsa_port_phylink_mac_pcs_get_state,
.mac_config = dsa_port_phylink_mac_config,
.mac_an_restart = dsa_port_phylink_mac_an_restart,
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH net-next v2 1/6] net: dsa: add support for phylink mac_select_pcs()
2022-02-17 18:30 ` [PATCH net-next v2 1/6] net: dsa: add support for phylink mac_select_pcs() Russell King (Oracle)
@ 2022-02-19 21:12 ` Vladimir Oltean
2022-02-19 21:22 ` Vladimir Oltean
0 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2022-02-19 21:12 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Ansuel Smith, Andrew Lunn, David S. Miller, Florian Fainelli,
Jakub Kicinski, netdev, Vivien Didelot
Hello,
On Thu, Feb 17, 2022 at 06:30:35PM +0000, Russell King (Oracle) wrote:
> Add DSA support for the phylink mac_select_pcs() method so DSA drivers
> can return provide phylink with the appropriate PCS for the PHY
> interface mode.
>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
> include/net/dsa.h | 3 +++
> net/dsa/port.c | 15 +++++++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 85cb9aed4c51..87aef3ed88a7 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -788,6 +788,9 @@ struct dsa_switch_ops {
> void (*phylink_validate)(struct dsa_switch *ds, int port,
> unsigned long *supported,
> struct phylink_link_state *state);
> + struct phylink_pcs *(*phylink_mac_select_pcs)(struct dsa_switch *ds,
> + int port,
> + phy_interface_t iface);
> int (*phylink_mac_link_state)(struct dsa_switch *ds, int port,
> struct phylink_link_state *state);
> void (*phylink_mac_config)(struct dsa_switch *ds, int port,
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index cca5cf686f74..d8534fd9fab9 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -1053,6 +1053,20 @@ static void dsa_port_phylink_mac_pcs_get_state(struct phylink_config *config,
> }
> }
>
> +static struct phylink_pcs *
> +dsa_port_phylink_mac_select_pcs(struct phylink_config *config,
> + phy_interface_t interface)
> +{
> + struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
> + struct dsa_switch *ds = dp->ds;
> + struct phylink_pcs *pcs = NULL;
> +
> + if (ds->ops->phylink_mac_select_pcs)
> + pcs = ds->ops->phylink_mac_select_pcs(ds, dp->index, interface);
> +
> + return pcs;
> +}
> +
> static void dsa_port_phylink_mac_config(struct phylink_config *config,
> unsigned int mode,
> const struct phylink_link_state *state)
> @@ -1119,6 +1133,7 @@ static void dsa_port_phylink_mac_link_up(struct phylink_config *config,
>
> static const struct phylink_mac_ops dsa_port_phylink_mac_ops = {
> .validate = dsa_port_phylink_validate,
> + .mac_select_pcs = dsa_port_phylink_mac_select_pcs,
This patch breaks probing on DSA switch drivers that weren't converted
to supported_interfaces, due to this check in phylink_create():
/* Validate the supplied configuration */
if (mac_ops->mac_select_pcs &&
phy_interface_empty(config->supported_interfaces)) {
dev_err(config->dev,
"phylink: error: empty supported_interfaces but mac_select_pcs() method present\n");
return ERR_PTR(-EINVAL);
}
> .mac_pcs_get_state = dsa_port_phylink_mac_pcs_get_state,
> .mac_config = dsa_port_phylink_mac_config,
> .mac_an_restart = dsa_port_phylink_mac_an_restart,
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net-next v2 1/6] net: dsa: add support for phylink mac_select_pcs()
2022-02-19 21:12 ` Vladimir Oltean
@ 2022-02-19 21:22 ` Vladimir Oltean
2022-02-21 13:30 ` Russell King (Oracle)
0 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2022-02-19 21:22 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Ansuel Smith, Andrew Lunn, David S. Miller, Florian Fainelli,
Jakub Kicinski, netdev, Vivien Didelot
On Sat, Feb 19, 2022 at 11:12:41PM +0200, Vladimir Oltean wrote:
> > static const struct phylink_mac_ops dsa_port_phylink_mac_ops = {
> > .validate = dsa_port_phylink_validate,
> > + .mac_select_pcs = dsa_port_phylink_mac_select_pcs,
>
> This patch breaks probing on DSA switch drivers that weren't converted
> to supported_interfaces, due to this check in phylink_create():
And this is only the most superficial layer of breakage. Everywhere in
phylink.c where pl->mac_ops->mac_select_pcs() is used, its presence is
checked and non-zero return codes from it are treated as hard errors,
even -EOPNOTSUPP, even if this particular error code is probably
intended to behave identically as the absence of the function pointer,
for compatibility.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net-next v2 1/6] net: dsa: add support for phylink mac_select_pcs()
2022-02-19 21:22 ` Vladimir Oltean
@ 2022-02-21 13:30 ` Russell King (Oracle)
2022-02-21 14:15 ` Russell King (Oracle)
2022-02-21 14:32 ` Vladimir Oltean
0 siblings, 2 replies; 17+ messages in thread
From: Russell King (Oracle) @ 2022-02-21 13:30 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Ansuel Smith, Andrew Lunn, David S. Miller, Florian Fainelli,
Jakub Kicinski, netdev, Vivien Didelot
On Sat, Feb 19, 2022 at 11:22:24PM +0200, Vladimir Oltean wrote:
> On Sat, Feb 19, 2022 at 11:12:41PM +0200, Vladimir Oltean wrote:
> > > static const struct phylink_mac_ops dsa_port_phylink_mac_ops = {
> > > .validate = dsa_port_phylink_validate,
> > > + .mac_select_pcs = dsa_port_phylink_mac_select_pcs,
> >
> > This patch breaks probing on DSA switch drivers that weren't converted
> > to supported_interfaces, due to this check in phylink_create():
>
> And this is only the most superficial layer of breakage. Everywhere in
> phylink.c where pl->mac_ops->mac_select_pcs() is used, its presence is
> checked and non-zero return codes from it are treated as hard errors,
> even -EOPNOTSUPP, even if this particular error code is probably
> intended to behave identically as the absence of the function pointer,
> for compatibility.
I don't understand what problem you're getting at here - and I don't
think there is a problem.
While I know it's conventional in DSA to use EOPNOTSUPP to indicate
that a called method is not implemented, this is not something that
is common across the board - and is not necessary here.
The implementation of dsa_port_phylink_mac_select_pcs() returns a
NULL PCS when the DSA operation for it is not implemented. This
means that:
1) phylink_validate_mac_and_pcs() won't fail due to mac_select_pcs()
being present but DSA drivers not implementing it.
2) phylink_major_config() will not attempt to call phylink_set_pcs()
to change the PCS.
So, that much is perfectly safe.
As for your previous email reporting the problem with phylink_create(),
thanks for the report and sorry for the breakage - the breakage was
obviously not intended, and came about because of all the patch
shuffling I've done over the last six months trying to get these
changes in, and having forgotten about this dependency.
I imagine the reason you've raised EOPNOTSUPP is because you wanted to
change dsa_port_phylink_mac_select_pcs() to return an error-pointer
encoded with that error code rather than NULL, but you then (no
surprises to me) caused phylink to fail.
Considering the idea of using EOPNOTSUPP, at the two places we call
mac_select_pcs(), we would need to treat this the same way we currently
treat NULL. We would also need phylink_create() to call
mac_select_pcs() if the method is non-NULL to discover if the DSA
sub-driver implements the method - but we would need to choose an
interface at this point.
I think at this point, I'd rather:
1) add a bool in struct phylink to indicate whether we should be calling
mac_select_pcs, and replace the
if (pl->mac_ops->mac_select_pcs)
with
if (pl->using_mac_select_pcs)
2) have phylink_create() do:
bool using_mac_select_pcs = false;
if (mac_ops->mac_select_pcs &&
mac_ops->mac_select_pcs(config, PHY_INTERFACE_MODE_NA) !=
ERR_PTR(-EOPNOTSUPP))
using_mac_select_pcs = true;
if (using_mac_select_pcs &&
phy_interface_empty(config->supported_interfaces)) {
...
...
pl->using_mac_select_pcs = using_mac_select_pcs;
which should give what was intended until DSA drivers are all updated
to fill in config->supported_interfaces.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net-next v2 1/6] net: dsa: add support for phylink mac_select_pcs()
2022-02-21 13:30 ` Russell King (Oracle)
@ 2022-02-21 14:15 ` Russell King (Oracle)
2022-02-21 14:41 ` Vladimir Oltean
2022-02-21 14:32 ` Vladimir Oltean
1 sibling, 1 reply; 17+ messages in thread
From: Russell King (Oracle) @ 2022-02-21 14:15 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Ansuel Smith, Andrew Lunn, David S. Miller, Florian Fainelli,
Jakub Kicinski, netdev, Vivien Didelot
On Mon, Feb 21, 2022 at 01:30:09PM +0000, Russell King (Oracle) wrote:
> On Sat, Feb 19, 2022 at 11:22:24PM +0200, Vladimir Oltean wrote:
> > On Sat, Feb 19, 2022 at 11:12:41PM +0200, Vladimir Oltean wrote:
> > > > static const struct phylink_mac_ops dsa_port_phylink_mac_ops = {
> > > > .validate = dsa_port_phylink_validate,
> > > > + .mac_select_pcs = dsa_port_phylink_mac_select_pcs,
> > >
> > > This patch breaks probing on DSA switch drivers that weren't converted
> > > to supported_interfaces, due to this check in phylink_create():
> >
> > And this is only the most superficial layer of breakage. Everywhere in
> > phylink.c where pl->mac_ops->mac_select_pcs() is used, its presence is
> > checked and non-zero return codes from it are treated as hard errors,
> > even -EOPNOTSUPP, even if this particular error code is probably
> > intended to behave identically as the absence of the function pointer,
> > for compatibility.
>
> I don't understand what problem you're getting at here - and I don't
> think there is a problem.
>
> While I know it's conventional in DSA to use EOPNOTSUPP to indicate
> that a called method is not implemented, this is not something that
> is common across the board - and is not necessary here.
>
> The implementation of dsa_port_phylink_mac_select_pcs() returns a
> NULL PCS when the DSA operation for it is not implemented. This
> means that:
>
> 1) phylink_validate_mac_and_pcs() won't fail due to mac_select_pcs()
> being present but DSA drivers not implementing it.
>
> 2) phylink_major_config() will not attempt to call phylink_set_pcs()
> to change the PCS.
>
> So, that much is perfectly safe.
>
> As for your previous email reporting the problem with phylink_create(),
> thanks for the report and sorry for the breakage - the breakage was
> obviously not intended, and came about because of all the patch
> shuffling I've done over the last six months trying to get these
> changes in, and having forgotten about this dependency.
>
> I imagine the reason you've raised EOPNOTSUPP is because you wanted to
> change dsa_port_phylink_mac_select_pcs() to return an error-pointer
> encoded with that error code rather than NULL, but you then (no
> surprises to me) caused phylink to fail.
>
> Considering the idea of using EOPNOTSUPP, at the two places we call
> mac_select_pcs(), we would need to treat this the same way we currently
> treat NULL. We would also need phylink_create() to call
> mac_select_pcs() if the method is non-NULL to discover if the DSA
> sub-driver implements the method - but we would need to choose an
> interface at this point.
>
> I think at this point, I'd rather:
>
> 1) add a bool in struct phylink to indicate whether we should be calling
> mac_select_pcs, and replace the
>
> if (pl->mac_ops->mac_select_pcs)
>
> with
>
> if (pl->using_mac_select_pcs)
>
> 2) have phylink_create() do:
>
> bool using_mac_select_pcs = false;
>
> if (mac_ops->mac_select_pcs &&
> mac_ops->mac_select_pcs(config, PHY_INTERFACE_MODE_NA) !=
> ERR_PTR(-EOPNOTSUPP))
> using_mac_select_pcs = true;
>
> if (using_mac_select_pcs &&
> phy_interface_empty(config->supported_interfaces)) {
> ...
>
> ...
>
> pl->using_mac_select_pcs = using_mac_select_pcs;
>
> which should give what was intended until DSA drivers are all updated
> to fill in config->supported_interfaces.
Please try this patch, thanks:
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 6c7ab4a7a3be..de0557bbd4a7 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -74,6 +74,7 @@ struct phylink {
struct work_struct resolve;
bool mac_link_dropped;
+ bool using_mac_select_pcs;
struct sfp_bus *sfp_bus;
bool sfp_may_have_phy;
@@ -416,7 +417,7 @@ static int phylink_validate_mac_and_pcs(struct phylink *pl,
int ret;
/* Get the PCS for this interface mode */
- if (pl->mac_ops->mac_select_pcs) {
+ if (pl->using_mac_select_pcs) {
pcs = pl->mac_ops->mac_select_pcs(pl->config, state->interface);
if (IS_ERR(pcs))
return PTR_ERR(pcs);
@@ -791,7 +792,7 @@ static void phylink_major_config(struct phylink *pl, bool restart,
phylink_dbg(pl, "major config %s\n", phy_modes(state->interface));
- if (pl->mac_ops->mac_select_pcs) {
+ if (pl->using_mac_select_pcs) {
pcs = pl->mac_ops->mac_select_pcs(pl->config, state->interface);
if (IS_ERR(pcs)) {
phylink_err(pl,
@@ -1204,11 +1205,17 @@ struct phylink *phylink_create(struct phylink_config *config,
phy_interface_t iface,
const struct phylink_mac_ops *mac_ops)
{
+ bool using_mac_select_pcs = false;
struct phylink *pl;
int ret;
- /* Validate the supplied configuration */
if (mac_ops->mac_select_pcs &&
+ mac_ops->mac_select_pcs(config, PHY_INTERFACE_MODE_NA) !=
+ ERR_PTR(-EOPNOTSUPP))
+ using_mac_select_pcs = true;
+
+ /* Validate the supplied configuration */
+ if (using_mac_select_pcs &&
phy_interface_empty(config->supported_interfaces)) {
dev_err(config->dev,
"phylink: error: empty supported_interfaces but mac_select_pcs() method present\n");
@@ -1232,6 +1239,7 @@ struct phylink *phylink_create(struct phylink_config *config,
return ERR_PTR(-EINVAL);
}
+ pl->using_mac_select_pcs = using_mac_select_pcs;
pl->phy_state.interface = iface;
pl->link_interface = iface;
if (iface == PHY_INTERFACE_MODE_MOCA)
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 258782bf4271..367d141c6971 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -1058,8 +1058,8 @@ dsa_port_phylink_mac_select_pcs(struct phylink_config *config,
phy_interface_t interface)
{
struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
+ struct phylink_pcs *pcs = ERR_PTR(-EOPNOTSUPP);
struct dsa_switch *ds = dp->ds;
- struct phylink_pcs *pcs = NULL;
if (ds->ops->phylink_mac_select_pcs)
pcs = ds->ops->phylink_mac_select_pcs(ds, dp->index, interface);
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH net-next v2 1/6] net: dsa: add support for phylink mac_select_pcs()
2022-02-21 14:15 ` Russell King (Oracle)
@ 2022-02-21 14:41 ` Vladimir Oltean
0 siblings, 0 replies; 17+ messages in thread
From: Vladimir Oltean @ 2022-02-21 14:41 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Ansuel Smith, Andrew Lunn, David S. Miller, Florian Fainelli,
Jakub Kicinski, netdev, Vivien Didelot
On Mon, Feb 21, 2022 at 02:15:26PM +0000, Russell King (Oracle) wrote:
> Please try this patch, thanks:
Probing, bring the interface up, down, and testing with traffic over a
QSGMII port with a pre-March 2020 phylink_pcs works as before, thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 1/6] net: dsa: add support for phylink mac_select_pcs()
2022-02-21 13:30 ` Russell King (Oracle)
2022-02-21 14:15 ` Russell King (Oracle)
@ 2022-02-21 14:32 ` Vladimir Oltean
2022-02-21 14:44 ` Russell King (Oracle)
1 sibling, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2022-02-21 14:32 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Ansuel Smith, Andrew Lunn, David S. Miller, Florian Fainelli,
Jakub Kicinski, netdev, Vivien Didelot
Hello,
On Mon, Feb 21, 2022 at 01:30:09PM +0000, Russell King (Oracle) wrote:
> On Sat, Feb 19, 2022 at 11:22:24PM +0200, Vladimir Oltean wrote:
> > On Sat, Feb 19, 2022 at 11:12:41PM +0200, Vladimir Oltean wrote:
> > > > static const struct phylink_mac_ops dsa_port_phylink_mac_ops = {
> > > > .validate = dsa_port_phylink_validate,
> > > > + .mac_select_pcs = dsa_port_phylink_mac_select_pcs,
> > >
> > > This patch breaks probing on DSA switch drivers that weren't converted
> > > to supported_interfaces, due to this check in phylink_create():
> >
> > And this is only the most superficial layer of breakage. Everywhere in
> > phylink.c where pl->mac_ops->mac_select_pcs() is used, its presence is
> > checked and non-zero return codes from it are treated as hard errors,
> > even -EOPNOTSUPP, even if this particular error code is probably
> > intended to behave identically as the absence of the function pointer,
> > for compatibility.
>
> I don't understand what problem you're getting at here - and I don't
> think there is a problem.
>
> While I know it's conventional in DSA to use EOPNOTSUPP to indicate
> that a called method is not implemented, this is not something that
> is common across the board - and is not necessary here.
>
> The implementation of dsa_port_phylink_mac_select_pcs() returns a
> NULL PCS when the DSA operation for it is not implemented. This
> means that:
>
> 1) phylink_validate_mac_and_pcs() won't fail due to mac_select_pcs()
> being present but DSA drivers not implementing it.
>
> 2) phylink_major_config() will not attempt to call phylink_set_pcs()
> to change the PCS.
>
> So, that much is perfectly safe.
>
> As for your previous email reporting the problem with phylink_create(),
> thanks for the report and sorry for the breakage - the breakage was
> obviously not intended, and came about because of all the patch
> shuffling I've done over the last six months trying to get these
> changes in, and having forgotten about this dependency.
>
> I imagine the reason you've raised EOPNOTSUPP is because you wanted to
> change dsa_port_phylink_mac_select_pcs() to return an error-pointer
> encoded with that error code rather than NULL, but you then (no
> surprises to me) caused phylink to fail.
>
> Considering the idea of using EOPNOTSUPP, at the two places we call
> mac_select_pcs(), we would need to treat this the same way we currently
> treat NULL. We would also need phylink_create() to call
> mac_select_pcs() if the method is non-NULL to discover if the DSA
> sub-driver implements the method - but we would need to choose an
> interface at this point.
>
> I think at this point, I'd rather:
>
> 1) add a bool in struct phylink to indicate whether we should be calling
> mac_select_pcs, and replace the
>
> if (pl->mac_ops->mac_select_pcs)
>
> with
>
> if (pl->using_mac_select_pcs)
>
> 2) have phylink_create() do:
>
> bool using_mac_select_pcs = false;
>
> if (mac_ops->mac_select_pcs &&
> mac_ops->mac_select_pcs(config, PHY_INTERFACE_MODE_NA) !=
> ERR_PTR(-EOPNOTSUPP))
> using_mac_select_pcs = true;
>
> if (using_mac_select_pcs &&
> phy_interface_empty(config->supported_interfaces)) {
> ...
>
> ...
>
> pl->using_mac_select_pcs = using_mac_select_pcs;
>
> which should give what was intended until DSA drivers are all updated
> to fill in config->supported_interfaces.
I didn't study the problem enough to be in the position to suggest the
best solution.
As you've explained*, phylink works properly when mac_select_pcs()
returns NULL but not special error codes, so extra handling would be
required for those - and you've shown an approach that seems reasonable
if we use -EOPNOTSUPP.
Alternatively, phylink_create() gets the initial PHY interface mode
passed to it, I wonder, couldn't we call mac_select_pcs() with that in
order to determine whether the function is a stub or not? I see that
only axienet_mac_select_pcs returns NULL based on the interface mode,
and I assume it will never get passed an invalid-to-it PHY interface
mode. For this reason we can't pass PHY_INTERFACE_MODE_NA as long as we
check for NULL instead of -EOPNOTSUPP - because drivers may not expect
this PHY interface type. So this second approach may also work and may
require less phylink rework, although it may be slightly more prone to
subtle breakage, if for whatever reason I don't see right now, the
phylink instance gets created with an undetermined PHY mode.
Either of these solutions is fine by me, with -EOPNOTSUPP probably being
preferable for the extra safety at the expense of extra rework.
*and as I haven't considered, to be honest. When phylink_major_config()
gets called after a SGMII to 10GBaseR switchover, and mac_select_pcs is
called and returns NULL, the current behavior is to keep working with
the PCS for SGMII. Is that intended?
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net-next v2 1/6] net: dsa: add support for phylink mac_select_pcs()
2022-02-21 14:32 ` Vladimir Oltean
@ 2022-02-21 14:44 ` Russell King (Oracle)
2022-02-21 14:55 ` Vladimir Oltean
0 siblings, 1 reply; 17+ messages in thread
From: Russell King (Oracle) @ 2022-02-21 14:44 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Ansuel Smith, Andrew Lunn, David S. Miller, Florian Fainelli,
Jakub Kicinski, netdev, Vivien Didelot
On Mon, Feb 21, 2022 at 04:32:54PM +0200, Vladimir Oltean wrote:
> Alternatively, phylink_create() gets the initial PHY interface mode
> passed to it, I wonder, couldn't we call mac_select_pcs() with that in
> order to determine whether the function is a stub or not?
That would be rather prone to odd behaviour depending on how
phylink_create() is called, depending on the initial interface mode.
If the initial interface mode causes mac_select_pcs() to return NULL
but it actually needed to return a PCS for a different interface mode,
then we fail.
> *and as I haven't considered, to be honest. When phylink_major_config()
> gets called after a SGMII to 10GBaseR switchover, and mac_select_pcs is
> called and returns NULL, the current behavior is to keep working with
> the PCS for SGMII. Is that intended?
It was not originally intended, but as a result of the discussion
around this patch which didn't go anywhere useful, I dropped it as
a means to a path of least resistance.
https://patchwork.kernel.org/project/linux-arm-kernel/patch/E1mpSba-00BXp6-9e@rmk-PC.armlinux.org.uk/
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 1/6] net: dsa: add support for phylink mac_select_pcs()
2022-02-21 14:44 ` Russell King (Oracle)
@ 2022-02-21 14:55 ` Vladimir Oltean
2022-02-21 16:38 ` Russell King (Oracle)
0 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2022-02-21 14:55 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Ansuel Smith, Andrew Lunn, David S. Miller, Florian Fainelli,
Jakub Kicinski, netdev, Vivien Didelot
On Mon, Feb 21, 2022 at 02:44:34PM +0000, Russell King (Oracle) wrote:
> On Mon, Feb 21, 2022 at 04:32:54PM +0200, Vladimir Oltean wrote:
> > Alternatively, phylink_create() gets the initial PHY interface mode
> > passed to it, I wonder, couldn't we call mac_select_pcs() with that in
> > order to determine whether the function is a stub or not?
>
> That would be rather prone to odd behaviour depending on how
> phylink_create() is called, depending on the initial interface mode.
> If the initial interface mode causes mac_select_pcs() to return NULL
> but it actually needed to return a PCS for a different interface mode,
> then we fail.
I agree. I just wanted to make it clear that if you have a better idea
than a pointer-encoded -EOPNOTSUPP, I'm not bent on going with -EOPNOTSUPP.
> > *and as I haven't considered, to be honest. When phylink_major_config()
> > gets called after a SGMII to 10GBaseR switchover, and mac_select_pcs is
> > called and returns NULL, the current behavior is to keep working with
> > the PCS for SGMII. Is that intended?
>
> It was not originally intended, but as a result of the discussion
> around this patch which didn't go anywhere useful, I dropped it as
> a means to a path of least resistance.
>
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/E1mpSba-00BXp6-9e@rmk-PC.armlinux.org.uk/
Oh, but that patch didn't close exactly this condition that we're
talking about here, did it? It allows phylink_set_pcs() to be called
with NULL, but phylink_major_config() still has the non-NULL check,
which prevents it from having any effect in this scenario:
/* If we have a new PCS, switch to the new PCS after preparing the MAC
* for the change.
*/
if (pcs)
phylink_set_pcs(pl, pcs);
I re-read the conversation and I still don't see this argument being
given, otherwise I wouldn't have opposed...
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 1/6] net: dsa: add support for phylink mac_select_pcs()
2022-02-21 14:55 ` Vladimir Oltean
@ 2022-02-21 16:38 ` Russell King (Oracle)
0 siblings, 0 replies; 17+ messages in thread
From: Russell King (Oracle) @ 2022-02-21 16:38 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Ansuel Smith, Andrew Lunn, David S. Miller, Florian Fainelli,
Jakub Kicinski, netdev, Vivien Didelot
On Mon, Feb 21, 2022 at 04:55:40PM +0200, Vladimir Oltean wrote:
> On Mon, Feb 21, 2022 at 02:44:34PM +0000, Russell King (Oracle) wrote:
> > > *and as I haven't considered, to be honest. When phylink_major_config()
> > > gets called after a SGMII to 10GBaseR switchover, and mac_select_pcs is
> > > called and returns NULL, the current behavior is to keep working with
> > > the PCS for SGMII. Is that intended?
> >
> > It was not originally intended, but as a result of the discussion
> > around this patch which didn't go anywhere useful, I dropped it as
> > a means to a path of least resistance.
> >
> > https://patchwork.kernel.org/project/linux-arm-kernel/patch/E1mpSba-00BXp6-9e@rmk-PC.armlinux.org.uk/
>
> Oh, but that patch didn't close exactly this condition that we're
> talking about here, did it? It allows phylink_set_pcs() to be called
> with NULL, but phylink_major_config() still has the non-NULL check,
> which prevents it from having any effect in this scenario:
>
> /* If we have a new PCS, switch to the new PCS after preparing the MAC
> * for the change.
> */
> if (pcs)
> phylink_set_pcs(pl, pcs);
>
> I re-read the conversation and I still don't see this argument being
> given, otherwise I wouldn't have opposed...
The reason for that condition above is to avoid disrupting the case
where drivers which do not (yet) provide a mac_select_pcs()
implementation (therefore pcs will be NULL here) but instead register
their PCS by calling phylink_set_pcs() immediately after a call to
phylink_create(). If the above call to phylink_set_pcs() was
unconditional, then:
1) it would break these existing drivers,
2) we would definitely need to make phylink_set_pcs() safe to call
with a NULL pcs argument to avoid crashing when in e.g. pcs-less
modes.
If that usage was eliminated, then the problem goes away... but that
means changing drivers, and changing drivers is always a long hard
slog that takes months and several kernel cycles to do.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next v2 2/6] net: dsa: qca8k: move qca8k_setup()
2022-02-17 18:29 [PATCH net-next v2 0/6] net: dsa: qca8k: convert to phylink_pcs and mark as non-legacy Russell King (Oracle)
2022-02-17 18:30 ` [PATCH net-next v2 1/6] net: dsa: add support for phylink mac_select_pcs() Russell King (Oracle)
@ 2022-02-17 18:30 ` Russell King (Oracle)
2022-02-17 18:30 ` [PATCH net-next v2 3/6] net: dsa: qca8k: move qca8k_phylink_mac_link_state() Russell King (Oracle)
` (4 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Russell King (Oracle) @ 2022-02-17 18:30 UTC (permalink / raw)
To: Ansuel Smith
Cc: Andrew Lunn, David S. Miller, Florian Fainelli, Jakub Kicinski,
netdev, Vivien Didelot, Vladimir Oltean
Move qca8k_setup() to be later in the file to avoid needing prototypes
for called functions.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/dsa/qca8k.c | 428 ++++++++++++++++++++--------------------
1 file changed, 214 insertions(+), 214 deletions(-)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index c09d1569e66b..760fbc6e3c4d 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1632,220 +1632,6 @@ qca8k_parse_port_config(struct qca8k_priv *priv)
return 0;
}
-static int
-qca8k_setup(struct dsa_switch *ds)
-{
- struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
- int cpu_port, ret, i;
- u32 mask;
-
- cpu_port = qca8k_find_cpu_port(ds);
- if (cpu_port < 0) {
- dev_err(priv->dev, "No cpu port configured in both cpu port0 and port6");
- return cpu_port;
- }
-
- /* Parse CPU port config to be later used in phy_link mac_config */
- ret = qca8k_parse_port_config(priv);
- if (ret)
- return ret;
-
- ret = qca8k_setup_mdio_bus(priv);
- if (ret)
- return ret;
-
- ret = qca8k_setup_of_pws_reg(priv);
- if (ret)
- return ret;
-
- ret = qca8k_setup_mac_pwr_sel(priv);
- if (ret)
- return ret;
-
- /* Make sure MAC06 is disabled */
- ret = regmap_clear_bits(priv->regmap, QCA8K_REG_PORT0_PAD_CTRL,
- QCA8K_PORT0_PAD_MAC06_EXCHANGE_EN);
- if (ret) {
- dev_err(priv->dev, "failed disabling MAC06 exchange");
- return ret;
- }
-
- /* Enable CPU Port */
- ret = regmap_set_bits(priv->regmap, QCA8K_REG_GLOBAL_FW_CTRL0,
- QCA8K_GLOBAL_FW_CTRL0_CPU_PORT_EN);
- if (ret) {
- dev_err(priv->dev, "failed enabling CPU port");
- return ret;
- }
-
- /* Enable MIB counters */
- ret = qca8k_mib_init(priv);
- if (ret)
- dev_warn(priv->dev, "mib init failed");
-
- /* Initial setup of all ports */
- for (i = 0; i < QCA8K_NUM_PORTS; i++) {
- /* Disable forwarding by default on all ports */
- ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
- QCA8K_PORT_LOOKUP_MEMBER, 0);
- if (ret)
- return ret;
-
- /* Enable QCA header mode on all cpu ports */
- if (dsa_is_cpu_port(ds, i)) {
- ret = qca8k_write(priv, QCA8K_REG_PORT_HDR_CTRL(i),
- FIELD_PREP(QCA8K_PORT_HDR_CTRL_TX_MASK, QCA8K_PORT_HDR_CTRL_ALL) |
- FIELD_PREP(QCA8K_PORT_HDR_CTRL_RX_MASK, QCA8K_PORT_HDR_CTRL_ALL));
- if (ret) {
- dev_err(priv->dev, "failed enabling QCA header mode");
- return ret;
- }
- }
-
- /* Disable MAC by default on all user ports */
- if (dsa_is_user_port(ds, i))
- qca8k_port_set_status(priv, i, 0);
- }
-
- /* Forward all unknown frames to CPU port for Linux processing
- * Notice that in multi-cpu config only one port should be set
- * for igmp, unknown, multicast and broadcast packet
- */
- ret = qca8k_write(priv, QCA8K_REG_GLOBAL_FW_CTRL1,
- FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_IGMP_DP_MASK, BIT(cpu_port)) |
- FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_BC_DP_MASK, BIT(cpu_port)) |
- FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_MC_DP_MASK, BIT(cpu_port)) |
- FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_UC_DP_MASK, BIT(cpu_port)));
- if (ret)
- return ret;
-
- /* Setup connection between CPU port & user ports
- * Configure specific switch configuration for ports
- */
- for (i = 0; i < QCA8K_NUM_PORTS; i++) {
- /* CPU port gets connected to all user ports of the switch */
- if (dsa_is_cpu_port(ds, i)) {
- ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
- QCA8K_PORT_LOOKUP_MEMBER, dsa_user_ports(ds));
- if (ret)
- return ret;
- }
-
- /* Individual user ports get connected to CPU port only */
- if (dsa_is_user_port(ds, i)) {
- ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
- QCA8K_PORT_LOOKUP_MEMBER,
- BIT(cpu_port));
- if (ret)
- return ret;
-
- /* Enable ARP Auto-learning by default */
- ret = regmap_set_bits(priv->regmap, QCA8K_PORT_LOOKUP_CTRL(i),
- QCA8K_PORT_LOOKUP_LEARN);
- if (ret)
- return ret;
-
- /* For port based vlans to work we need to set the
- * default egress vid
- */
- ret = qca8k_rmw(priv, QCA8K_EGRESS_VLAN(i),
- QCA8K_EGREES_VLAN_PORT_MASK(i),
- QCA8K_EGREES_VLAN_PORT(i, QCA8K_PORT_VID_DEF));
- if (ret)
- return ret;
-
- ret = qca8k_write(priv, QCA8K_REG_PORT_VLAN_CTRL0(i),
- QCA8K_PORT_VLAN_CVID(QCA8K_PORT_VID_DEF) |
- QCA8K_PORT_VLAN_SVID(QCA8K_PORT_VID_DEF));
- if (ret)
- return ret;
- }
-
- /* The port 5 of the qca8337 have some problem in flood condition. The
- * original legacy driver had some specific buffer and priority settings
- * for the different port suggested by the QCA switch team. Add this
- * missing settings to improve switch stability under load condition.
- * This problem is limited to qca8337 and other qca8k switch are not affected.
- */
- if (priv->switch_id == QCA8K_ID_QCA8337) {
- switch (i) {
- /* The 2 CPU port and port 5 requires some different
- * priority than any other ports.
- */
- case 0:
- case 5:
- case 6:
- mask = QCA8K_PORT_HOL_CTRL0_EG_PRI0(0x3) |
- QCA8K_PORT_HOL_CTRL0_EG_PRI1(0x4) |
- QCA8K_PORT_HOL_CTRL0_EG_PRI2(0x4) |
- QCA8K_PORT_HOL_CTRL0_EG_PRI3(0x4) |
- QCA8K_PORT_HOL_CTRL0_EG_PRI4(0x6) |
- QCA8K_PORT_HOL_CTRL0_EG_PRI5(0x8) |
- QCA8K_PORT_HOL_CTRL0_EG_PORT(0x1e);
- break;
- default:
- mask = QCA8K_PORT_HOL_CTRL0_EG_PRI0(0x3) |
- QCA8K_PORT_HOL_CTRL0_EG_PRI1(0x4) |
- QCA8K_PORT_HOL_CTRL0_EG_PRI2(0x6) |
- QCA8K_PORT_HOL_CTRL0_EG_PRI3(0x8) |
- QCA8K_PORT_HOL_CTRL0_EG_PORT(0x19);
- }
- qca8k_write(priv, QCA8K_REG_PORT_HOL_CTRL0(i), mask);
-
- mask = QCA8K_PORT_HOL_CTRL1_ING(0x6) |
- QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN |
- QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN |
- QCA8K_PORT_HOL_CTRL1_WRED_EN;
- qca8k_rmw(priv, QCA8K_REG_PORT_HOL_CTRL1(i),
- QCA8K_PORT_HOL_CTRL1_ING_BUF_MASK |
- QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN |
- QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN |
- QCA8K_PORT_HOL_CTRL1_WRED_EN,
- mask);
- }
-
- /* Set initial MTU for every port.
- * We have only have a general MTU setting. So track
- * every port and set the max across all port.
- * Set per port MTU to 1500 as the MTU change function
- * will add the overhead and if its set to 1518 then it
- * will apply the overhead again and we will end up with
- * MTU of 1536 instead of 1518
- */
- priv->port_mtu[i] = ETH_DATA_LEN;
- }
-
- /* Special GLOBAL_FC_THRESH value are needed for ar8327 switch */
- if (priv->switch_id == QCA8K_ID_QCA8327) {
- mask = QCA8K_GLOBAL_FC_GOL_XON_THRES(288) |
- QCA8K_GLOBAL_FC_GOL_XOFF_THRES(496);
- qca8k_rmw(priv, QCA8K_REG_GLOBAL_FC_THRESH,
- QCA8K_GLOBAL_FC_GOL_XON_THRES_MASK |
- QCA8K_GLOBAL_FC_GOL_XOFF_THRES_MASK,
- mask);
- }
-
- /* Setup our port MTUs to match power on defaults */
- ret = qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, ETH_FRAME_LEN + ETH_FCS_LEN);
- if (ret)
- dev_warn(priv->dev, "failed setting MTU settings");
-
- /* Flush the FDB table */
- qca8k_fdb_flush(priv);
-
- /* We don't have interrupts for link changes, so we need to poll */
- ds->pcs_poll = true;
-
- /* Set min a max ageing value supported */
- ds->ageing_time_min = 7000;
- ds->ageing_time_max = 458745000;
-
- /* Set max number of LAGs supported */
- ds->num_lag_ids = QCA8K_NUM_LAGS;
-
- return 0;
-}
-
static void
qca8k_mac_config_setup_internal_delay(struct qca8k_priv *priv, int cpu_port_index,
u32 reg)
@@ -2990,6 +2776,220 @@ static int qca8k_connect_tag_protocol(struct dsa_switch *ds,
return 0;
}
+static int
+qca8k_setup(struct dsa_switch *ds)
+{
+ struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
+ int cpu_port, ret, i;
+ u32 mask;
+
+ cpu_port = qca8k_find_cpu_port(ds);
+ if (cpu_port < 0) {
+ dev_err(priv->dev, "No cpu port configured in both cpu port0 and port6");
+ return cpu_port;
+ }
+
+ /* Parse CPU port config to be later used in phy_link mac_config */
+ ret = qca8k_parse_port_config(priv);
+ if (ret)
+ return ret;
+
+ ret = qca8k_setup_mdio_bus(priv);
+ if (ret)
+ return ret;
+
+ ret = qca8k_setup_of_pws_reg(priv);
+ if (ret)
+ return ret;
+
+ ret = qca8k_setup_mac_pwr_sel(priv);
+ if (ret)
+ return ret;
+
+ /* Make sure MAC06 is disabled */
+ ret = regmap_clear_bits(priv->regmap, QCA8K_REG_PORT0_PAD_CTRL,
+ QCA8K_PORT0_PAD_MAC06_EXCHANGE_EN);
+ if (ret) {
+ dev_err(priv->dev, "failed disabling MAC06 exchange");
+ return ret;
+ }
+
+ /* Enable CPU Port */
+ ret = regmap_set_bits(priv->regmap, QCA8K_REG_GLOBAL_FW_CTRL0,
+ QCA8K_GLOBAL_FW_CTRL0_CPU_PORT_EN);
+ if (ret) {
+ dev_err(priv->dev, "failed enabling CPU port");
+ return ret;
+ }
+
+ /* Enable MIB counters */
+ ret = qca8k_mib_init(priv);
+ if (ret)
+ dev_warn(priv->dev, "mib init failed");
+
+ /* Initial setup of all ports */
+ for (i = 0; i < QCA8K_NUM_PORTS; i++) {
+ /* Disable forwarding by default on all ports */
+ ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
+ QCA8K_PORT_LOOKUP_MEMBER, 0);
+ if (ret)
+ return ret;
+
+ /* Enable QCA header mode on all cpu ports */
+ if (dsa_is_cpu_port(ds, i)) {
+ ret = qca8k_write(priv, QCA8K_REG_PORT_HDR_CTRL(i),
+ FIELD_PREP(QCA8K_PORT_HDR_CTRL_TX_MASK, QCA8K_PORT_HDR_CTRL_ALL) |
+ FIELD_PREP(QCA8K_PORT_HDR_CTRL_RX_MASK, QCA8K_PORT_HDR_CTRL_ALL));
+ if (ret) {
+ dev_err(priv->dev, "failed enabling QCA header mode");
+ return ret;
+ }
+ }
+
+ /* Disable MAC by default on all user ports */
+ if (dsa_is_user_port(ds, i))
+ qca8k_port_set_status(priv, i, 0);
+ }
+
+ /* Forward all unknown frames to CPU port for Linux processing
+ * Notice that in multi-cpu config only one port should be set
+ * for igmp, unknown, multicast and broadcast packet
+ */
+ ret = qca8k_write(priv, QCA8K_REG_GLOBAL_FW_CTRL1,
+ FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_IGMP_DP_MASK, BIT(cpu_port)) |
+ FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_BC_DP_MASK, BIT(cpu_port)) |
+ FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_MC_DP_MASK, BIT(cpu_port)) |
+ FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_UC_DP_MASK, BIT(cpu_port)));
+ if (ret)
+ return ret;
+
+ /* Setup connection between CPU port & user ports
+ * Configure specific switch configuration for ports
+ */
+ for (i = 0; i < QCA8K_NUM_PORTS; i++) {
+ /* CPU port gets connected to all user ports of the switch */
+ if (dsa_is_cpu_port(ds, i)) {
+ ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
+ QCA8K_PORT_LOOKUP_MEMBER, dsa_user_ports(ds));
+ if (ret)
+ return ret;
+ }
+
+ /* Individual user ports get connected to CPU port only */
+ if (dsa_is_user_port(ds, i)) {
+ ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
+ QCA8K_PORT_LOOKUP_MEMBER,
+ BIT(cpu_port));
+ if (ret)
+ return ret;
+
+ /* Enable ARP Auto-learning by default */
+ ret = regmap_set_bits(priv->regmap, QCA8K_PORT_LOOKUP_CTRL(i),
+ QCA8K_PORT_LOOKUP_LEARN);
+ if (ret)
+ return ret;
+
+ /* For port based vlans to work we need to set the
+ * default egress vid
+ */
+ ret = qca8k_rmw(priv, QCA8K_EGRESS_VLAN(i),
+ QCA8K_EGREES_VLAN_PORT_MASK(i),
+ QCA8K_EGREES_VLAN_PORT(i, QCA8K_PORT_VID_DEF));
+ if (ret)
+ return ret;
+
+ ret = qca8k_write(priv, QCA8K_REG_PORT_VLAN_CTRL0(i),
+ QCA8K_PORT_VLAN_CVID(QCA8K_PORT_VID_DEF) |
+ QCA8K_PORT_VLAN_SVID(QCA8K_PORT_VID_DEF));
+ if (ret)
+ return ret;
+ }
+
+ /* The port 5 of the qca8337 have some problem in flood condition. The
+ * original legacy driver had some specific buffer and priority settings
+ * for the different port suggested by the QCA switch team. Add this
+ * missing settings to improve switch stability under load condition.
+ * This problem is limited to qca8337 and other qca8k switch are not affected.
+ */
+ if (priv->switch_id == QCA8K_ID_QCA8337) {
+ switch (i) {
+ /* The 2 CPU port and port 5 requires some different
+ * priority than any other ports.
+ */
+ case 0:
+ case 5:
+ case 6:
+ mask = QCA8K_PORT_HOL_CTRL0_EG_PRI0(0x3) |
+ QCA8K_PORT_HOL_CTRL0_EG_PRI1(0x4) |
+ QCA8K_PORT_HOL_CTRL0_EG_PRI2(0x4) |
+ QCA8K_PORT_HOL_CTRL0_EG_PRI3(0x4) |
+ QCA8K_PORT_HOL_CTRL0_EG_PRI4(0x6) |
+ QCA8K_PORT_HOL_CTRL0_EG_PRI5(0x8) |
+ QCA8K_PORT_HOL_CTRL0_EG_PORT(0x1e);
+ break;
+ default:
+ mask = QCA8K_PORT_HOL_CTRL0_EG_PRI0(0x3) |
+ QCA8K_PORT_HOL_CTRL0_EG_PRI1(0x4) |
+ QCA8K_PORT_HOL_CTRL0_EG_PRI2(0x6) |
+ QCA8K_PORT_HOL_CTRL0_EG_PRI3(0x8) |
+ QCA8K_PORT_HOL_CTRL0_EG_PORT(0x19);
+ }
+ qca8k_write(priv, QCA8K_REG_PORT_HOL_CTRL0(i), mask);
+
+ mask = QCA8K_PORT_HOL_CTRL1_ING(0x6) |
+ QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN |
+ QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN |
+ QCA8K_PORT_HOL_CTRL1_WRED_EN;
+ qca8k_rmw(priv, QCA8K_REG_PORT_HOL_CTRL1(i),
+ QCA8K_PORT_HOL_CTRL1_ING_BUF_MASK |
+ QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN |
+ QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN |
+ QCA8K_PORT_HOL_CTRL1_WRED_EN,
+ mask);
+ }
+
+ /* Set initial MTU for every port.
+ * We have only have a general MTU setting. So track
+ * every port and set the max across all port.
+ * Set per port MTU to 1500 as the MTU change function
+ * will add the overhead and if its set to 1518 then it
+ * will apply the overhead again and we will end up with
+ * MTU of 1536 instead of 1518
+ */
+ priv->port_mtu[i] = ETH_DATA_LEN;
+ }
+
+ /* Special GLOBAL_FC_THRESH value are needed for ar8327 switch */
+ if (priv->switch_id == QCA8K_ID_QCA8327) {
+ mask = QCA8K_GLOBAL_FC_GOL_XON_THRES(288) |
+ QCA8K_GLOBAL_FC_GOL_XOFF_THRES(496);
+ qca8k_rmw(priv, QCA8K_REG_GLOBAL_FC_THRESH,
+ QCA8K_GLOBAL_FC_GOL_XON_THRES_MASK |
+ QCA8K_GLOBAL_FC_GOL_XOFF_THRES_MASK,
+ mask);
+ }
+
+ /* Setup our port MTUs to match power on defaults */
+ ret = qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, ETH_FRAME_LEN + ETH_FCS_LEN);
+ if (ret)
+ dev_warn(priv->dev, "failed setting MTU settings");
+
+ /* Flush the FDB table */
+ qca8k_fdb_flush(priv);
+
+ /* We don't have interrupts for link changes, so we need to poll */
+ ds->pcs_poll = true;
+
+ /* Set min a max ageing value supported */
+ ds->ageing_time_min = 7000;
+ ds->ageing_time_max = 458745000;
+
+ /* Set max number of LAGs supported */
+ ds->num_lag_ids = QCA8K_NUM_LAGS;
+
+ return 0;
+}
+
static const struct dsa_switch_ops qca8k_switch_ops = {
.get_tag_protocol = qca8k_get_tag_protocol,
.setup = qca8k_setup,
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH net-next v2 3/6] net: dsa: qca8k: move qca8k_phylink_mac_link_state()
2022-02-17 18:29 [PATCH net-next v2 0/6] net: dsa: qca8k: convert to phylink_pcs and mark as non-legacy Russell King (Oracle)
2022-02-17 18:30 ` [PATCH net-next v2 1/6] net: dsa: add support for phylink mac_select_pcs() Russell King (Oracle)
2022-02-17 18:30 ` [PATCH net-next v2 2/6] net: dsa: qca8k: move qca8k_setup() Russell King (Oracle)
@ 2022-02-17 18:30 ` Russell King (Oracle)
2022-02-17 18:30 ` [PATCH net-next v2 4/6] net: dsa: qca8k: convert to use phylink_pcs Russell King (Oracle)
` (3 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Russell King (Oracle) @ 2022-02-17 18:30 UTC (permalink / raw)
To: Ansuel Smith
Cc: Andrew Lunn, David S. Miller, Florian Fainelli, Jakub Kicinski,
netdev, Vivien Didelot, Vladimir Oltean
Move qca8k_phylink_mac_link_state() to separate the code movement from
code changes.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/dsa/qca8k.c | 84 ++++++++++++++++++++---------------------
1 file changed, 42 insertions(+), 42 deletions(-)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 760fbc6e3c4d..ff572aba1430 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1852,48 +1852,6 @@ static void qca8k_phylink_get_caps(struct dsa_switch *ds, int port,
MAC_10 | MAC_100 | MAC_1000FD;
}
-static int
-qca8k_phylink_mac_link_state(struct dsa_switch *ds, int port,
- struct phylink_link_state *state)
-{
- struct qca8k_priv *priv = ds->priv;
- u32 reg;
- int ret;
-
- ret = qca8k_read(priv, QCA8K_REG_PORT_STATUS(port), ®);
- if (ret < 0)
- return ret;
-
- state->link = !!(reg & QCA8K_PORT_STATUS_LINK_UP);
- state->an_complete = state->link;
- state->an_enabled = !!(reg & QCA8K_PORT_STATUS_LINK_AUTO);
- state->duplex = (reg & QCA8K_PORT_STATUS_DUPLEX) ? DUPLEX_FULL :
- DUPLEX_HALF;
-
- switch (reg & QCA8K_PORT_STATUS_SPEED) {
- case QCA8K_PORT_STATUS_SPEED_10:
- state->speed = SPEED_10;
- break;
- case QCA8K_PORT_STATUS_SPEED_100:
- state->speed = SPEED_100;
- break;
- case QCA8K_PORT_STATUS_SPEED_1000:
- state->speed = SPEED_1000;
- break;
- default:
- state->speed = SPEED_UNKNOWN;
- break;
- }
-
- state->pause = MLO_PAUSE_NONE;
- if (reg & QCA8K_PORT_STATUS_RXFLOW)
- state->pause |= MLO_PAUSE_RX;
- if (reg & QCA8K_PORT_STATUS_TXFLOW)
- state->pause |= MLO_PAUSE_TX;
-
- return 1;
-}
-
static void
qca8k_phylink_mac_link_down(struct dsa_switch *ds, int port, unsigned int mode,
phy_interface_t interface)
@@ -1944,6 +1902,48 @@ qca8k_phylink_mac_link_up(struct dsa_switch *ds, int port, unsigned int mode,
qca8k_write(priv, QCA8K_REG_PORT_STATUS(port), reg);
}
+static int
+qca8k_phylink_mac_link_state(struct dsa_switch *ds, int port,
+ struct phylink_link_state *state)
+{
+ struct qca8k_priv *priv = ds->priv;
+ u32 reg;
+ int ret;
+
+ ret = qca8k_read(priv, QCA8K_REG_PORT_STATUS(port), ®);
+ if (ret < 0)
+ return ret;
+
+ state->link = !!(reg & QCA8K_PORT_STATUS_LINK_UP);
+ state->an_complete = state->link;
+ state->an_enabled = !!(reg & QCA8K_PORT_STATUS_LINK_AUTO);
+ state->duplex = (reg & QCA8K_PORT_STATUS_DUPLEX) ? DUPLEX_FULL :
+ DUPLEX_HALF;
+
+ switch (reg & QCA8K_PORT_STATUS_SPEED) {
+ case QCA8K_PORT_STATUS_SPEED_10:
+ state->speed = SPEED_10;
+ break;
+ case QCA8K_PORT_STATUS_SPEED_100:
+ state->speed = SPEED_100;
+ break;
+ case QCA8K_PORT_STATUS_SPEED_1000:
+ state->speed = SPEED_1000;
+ break;
+ default:
+ state->speed = SPEED_UNKNOWN;
+ break;
+ }
+
+ state->pause = MLO_PAUSE_NONE;
+ if (reg & QCA8K_PORT_STATUS_RXFLOW)
+ state->pause |= MLO_PAUSE_RX;
+ if (reg & QCA8K_PORT_STATUS_TXFLOW)
+ state->pause |= MLO_PAUSE_TX;
+
+ return 1;
+}
+
static void
qca8k_get_strings(struct dsa_switch *ds, int port, u32 stringset, uint8_t *data)
{
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH net-next v2 4/6] net: dsa: qca8k: convert to use phylink_pcs
2022-02-17 18:29 [PATCH net-next v2 0/6] net: dsa: qca8k: convert to phylink_pcs and mark as non-legacy Russell King (Oracle)
` (2 preceding siblings ...)
2022-02-17 18:30 ` [PATCH net-next v2 3/6] net: dsa: qca8k: move qca8k_phylink_mac_link_state() Russell King (Oracle)
@ 2022-02-17 18:30 ` Russell King (Oracle)
2022-02-17 18:30 ` [PATCH net-next v2 5/6] net: dsa: qca8k: move pcs configuration Russell King (Oracle)
` (2 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Russell King (Oracle) @ 2022-02-17 18:30 UTC (permalink / raw)
To: Ansuel Smith
Cc: Andrew Lunn, David S. Miller, Florian Fainelli, Jakub Kicinski,
netdev, Vivien Didelot, Vladimir Oltean
Convert the qca8k driver to use the phylink_pcs support to talk to the
SGMII PCS.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/dsa/qca8k.c | 85 +++++++++++++++++++++++++++++++++++------
drivers/net/dsa/qca8k.h | 8 ++++
2 files changed, 81 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index ff572aba1430..4df7b55a181a 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1673,6 +1673,34 @@ qca8k_mac_config_setup_internal_delay(struct qca8k_priv *priv, int cpu_port_inde
cpu_port_index == QCA8K_CPU_PORT0 ? 0 : 6);
}
+static struct phylink_pcs *
+qca8k_phylink_mac_select_pcs(struct dsa_switch *ds, int port,
+ phy_interface_t interface)
+{
+ struct qca8k_priv *priv = ds->priv;
+ struct phylink_pcs *pcs = NULL;
+
+ switch (interface) {
+ case PHY_INTERFACE_MODE_SGMII:
+ case PHY_INTERFACE_MODE_1000BASEX:
+ switch (port) {
+ case 0:
+ pcs = &priv->pcs_port_0.pcs;
+ break;
+
+ case 6:
+ pcs = &priv->pcs_port_6.pcs;
+ break;
+ }
+ break;
+
+ default:
+ break;
+ }
+
+ return pcs;
+}
+
static void
qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
const struct phylink_link_state *state)
@@ -1902,17 +1930,24 @@ qca8k_phylink_mac_link_up(struct dsa_switch *ds, int port, unsigned int mode,
qca8k_write(priv, QCA8K_REG_PORT_STATUS(port), reg);
}
-static int
-qca8k_phylink_mac_link_state(struct dsa_switch *ds, int port,
- struct phylink_link_state *state)
+static struct qca8k_pcs *pcs_to_qca8k_pcs(struct phylink_pcs *pcs)
{
- struct qca8k_priv *priv = ds->priv;
+ return container_of(pcs, struct qca8k_pcs, pcs);
+}
+
+static void qca8k_pcs_get_state(struct phylink_pcs *pcs,
+ struct phylink_link_state *state)
+{
+ struct qca8k_priv *priv = pcs_to_qca8k_pcs(pcs)->priv;
+ int port = pcs_to_qca8k_pcs(pcs)->port;
u32 reg;
int ret;
ret = qca8k_read(priv, QCA8K_REG_PORT_STATUS(port), ®);
- if (ret < 0)
- return ret;
+ if (ret < 0) {
+ state->link = false;
+ return;
+ }
state->link = !!(reg & QCA8K_PORT_STATUS_LINK_UP);
state->an_complete = state->link;
@@ -1935,13 +1970,39 @@ qca8k_phylink_mac_link_state(struct dsa_switch *ds, int port,
break;
}
- state->pause = MLO_PAUSE_NONE;
if (reg & QCA8K_PORT_STATUS_RXFLOW)
state->pause |= MLO_PAUSE_RX;
if (reg & QCA8K_PORT_STATUS_TXFLOW)
state->pause |= MLO_PAUSE_TX;
+}
- return 1;
+static int qca8k_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
+ phy_interface_t interface,
+ const unsigned long *advertising,
+ bool permit_pause_to_mac)
+{
+ return 0;
+}
+
+static void qca8k_pcs_an_restart(struct phylink_pcs *pcs)
+{
+}
+
+static const struct phylink_pcs_ops qca8k_pcs_ops = {
+ .pcs_get_state = qca8k_pcs_get_state,
+ .pcs_config = qca8k_pcs_config,
+ .pcs_an_restart = qca8k_pcs_an_restart,
+};
+
+static void qca8k_setup_pcs(struct qca8k_priv *priv, struct qca8k_pcs *qpcs,
+ int port)
+{
+ qpcs->pcs.ops = &qca8k_pcs_ops;
+
+ /* We don't have interrupts for link changes, so we need to poll */
+ qpcs->pcs.poll = true;
+ qpcs->priv = priv;
+ qpcs->port = port;
}
static void
@@ -2806,6 +2867,9 @@ qca8k_setup(struct dsa_switch *ds)
if (ret)
return ret;
+ qca8k_setup_pcs(priv, &priv->pcs_port_0, 0);
+ qca8k_setup_pcs(priv, &priv->pcs_port_6, 6);
+
/* Make sure MAC06 is disabled */
ret = regmap_clear_bits(priv->regmap, QCA8K_REG_PORT0_PAD_CTRL,
QCA8K_PORT0_PAD_MAC06_EXCHANGE_EN);
@@ -2977,9 +3041,6 @@ qca8k_setup(struct dsa_switch *ds)
/* Flush the FDB table */
qca8k_fdb_flush(priv);
- /* We don't have interrupts for link changes, so we need to poll */
- ds->pcs_poll = true;
-
/* Set min a max ageing value supported */
ds->ageing_time_min = 7000;
ds->ageing_time_max = 458745000;
@@ -3018,7 +3079,7 @@ static const struct dsa_switch_ops qca8k_switch_ops = {
.port_vlan_add = qca8k_port_vlan_add,
.port_vlan_del = qca8k_port_vlan_del,
.phylink_get_caps = qca8k_phylink_get_caps,
- .phylink_mac_link_state = qca8k_phylink_mac_link_state,
+ .phylink_mac_select_pcs = qca8k_phylink_mac_select_pcs,
.phylink_mac_config = qca8k_phylink_mac_config,
.phylink_mac_link_down = qca8k_phylink_mac_link_down,
.phylink_mac_link_up = qca8k_phylink_mac_link_up,
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index c3d3c2269b1d..f375627174c8 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -376,6 +376,12 @@ struct qca8k_mdio_cache {
u16 hi;
};
+struct qca8k_pcs {
+ struct phylink_pcs pcs;
+ struct qca8k_priv *priv;
+ int port;
+};
+
struct qca8k_priv {
u8 switch_id;
u8 switch_revision;
@@ -397,6 +403,8 @@ struct qca8k_priv {
struct qca8k_mgmt_eth_data mgmt_eth_data;
struct qca8k_mib_eth_data mib_eth_data;
struct qca8k_mdio_cache mdio_cache;
+ struct qca8k_pcs pcs_port_0;
+ struct qca8k_pcs pcs_port_6;
};
struct qca8k_mib_desc {
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH net-next v2 5/6] net: dsa: qca8k: move pcs configuration
2022-02-17 18:29 [PATCH net-next v2 0/6] net: dsa: qca8k: convert to phylink_pcs and mark as non-legacy Russell King (Oracle)
` (3 preceding siblings ...)
2022-02-17 18:30 ` [PATCH net-next v2 4/6] net: dsa: qca8k: convert to use phylink_pcs Russell King (Oracle)
@ 2022-02-17 18:30 ` Russell King (Oracle)
2022-02-17 18:31 ` [PATCH net-next v2 6/6] net: dsa: qca8k: mark as non-legacy Russell King (Oracle)
2022-02-18 11:50 ` [PATCH net-next v2 0/6] net: dsa: qca8k: convert to phylink_pcs and " patchwork-bot+netdevbpf
6 siblings, 0 replies; 17+ messages in thread
From: Russell King (Oracle) @ 2022-02-17 18:30 UTC (permalink / raw)
To: Ansuel Smith
Cc: Andrew Lunn, David S. Miller, Florian Fainelli, Jakub Kicinski,
netdev, Vivien Didelot, Vladimir Oltean
Move the PCS configuration to qca8k_pcs_config().
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/dsa/qca8k.c | 150 ++++++++++++++++++++++------------------
1 file changed, 84 insertions(+), 66 deletions(-)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 4df7b55a181a..2f8b03ff748c 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1706,8 +1706,8 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
const struct phylink_link_state *state)
{
struct qca8k_priv *priv = ds->priv;
- int cpu_port_index, ret;
- u32 reg, val;
+ int cpu_port_index;
+ u32 reg;
switch (port) {
case 0: /* 1st CPU port */
@@ -1773,70 +1773,6 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
case PHY_INTERFACE_MODE_1000BASEX:
/* Enable SGMII on the port */
qca8k_write(priv, reg, QCA8K_PORT_PAD_SGMII_EN);
-
- /* Enable/disable SerDes auto-negotiation as necessary */
- ret = qca8k_read(priv, QCA8K_REG_PWS, &val);
- if (ret)
- return;
- if (phylink_autoneg_inband(mode))
- val &= ~QCA8K_PWS_SERDES_AEN_DIS;
- else
- val |= QCA8K_PWS_SERDES_AEN_DIS;
- qca8k_write(priv, QCA8K_REG_PWS, val);
-
- /* Configure the SGMII parameters */
- ret = qca8k_read(priv, QCA8K_REG_SGMII_CTRL, &val);
- if (ret)
- return;
-
- val |= QCA8K_SGMII_EN_SD;
-
- if (priv->ports_config.sgmii_enable_pll)
- val |= QCA8K_SGMII_EN_PLL | QCA8K_SGMII_EN_RX |
- QCA8K_SGMII_EN_TX;
-
- if (dsa_is_cpu_port(ds, port)) {
- /* CPU port, we're talking to the CPU MAC, be a PHY */
- val &= ~QCA8K_SGMII_MODE_CTRL_MASK;
- val |= QCA8K_SGMII_MODE_CTRL_PHY;
- } else if (state->interface == PHY_INTERFACE_MODE_SGMII) {
- val &= ~QCA8K_SGMII_MODE_CTRL_MASK;
- val |= QCA8K_SGMII_MODE_CTRL_MAC;
- } else if (state->interface == PHY_INTERFACE_MODE_1000BASEX) {
- val &= ~QCA8K_SGMII_MODE_CTRL_MASK;
- val |= QCA8K_SGMII_MODE_CTRL_BASEX;
- }
-
- qca8k_write(priv, QCA8K_REG_SGMII_CTRL, val);
-
- /* From original code is reported port instability as SGMII also
- * require delay set. Apply advised values here or take them from DT.
- */
- if (state->interface == PHY_INTERFACE_MODE_SGMII)
- qca8k_mac_config_setup_internal_delay(priv, cpu_port_index, reg);
-
- /* For qca8327/qca8328/qca8334/qca8338 sgmii is unique and
- * falling edge is set writing in the PORT0 PAD reg
- */
- if (priv->switch_id == QCA8K_ID_QCA8327 ||
- priv->switch_id == QCA8K_ID_QCA8337)
- reg = QCA8K_REG_PORT0_PAD_CTRL;
-
- val = 0;
-
- /* SGMII Clock phase configuration */
- if (priv->ports_config.sgmii_rx_clk_falling_edge)
- val |= QCA8K_PORT0_PAD_SGMII_RXCLK_FALLING_EDGE;
-
- if (priv->ports_config.sgmii_tx_clk_falling_edge)
- val |= QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE;
-
- if (val)
- ret = qca8k_rmw(priv, reg,
- QCA8K_PORT0_PAD_SGMII_RXCLK_FALLING_EDGE |
- QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE,
- val);
-
break;
default:
dev_err(ds->dev, "xMII mode %s not supported for port %d\n",
@@ -1981,6 +1917,88 @@ static int qca8k_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
const unsigned long *advertising,
bool permit_pause_to_mac)
{
+ struct qca8k_priv *priv = pcs_to_qca8k_pcs(pcs)->priv;
+ int cpu_port_index, ret, port;
+ u32 reg, val;
+
+ port = pcs_to_qca8k_pcs(pcs)->port;
+ switch (port) {
+ case 0:
+ reg = QCA8K_REG_PORT0_PAD_CTRL;
+ cpu_port_index = QCA8K_CPU_PORT0;
+ break;
+
+ case 6:
+ reg = QCA8K_REG_PORT6_PAD_CTRL;
+ cpu_port_index = QCA8K_CPU_PORT6;
+ break;
+
+ default:
+ WARN_ON(1);
+ }
+
+ /* Enable/disable SerDes auto-negotiation as necessary */
+ ret = qca8k_read(priv, QCA8K_REG_PWS, &val);
+ if (ret)
+ return ret;
+ if (phylink_autoneg_inband(mode))
+ val &= ~QCA8K_PWS_SERDES_AEN_DIS;
+ else
+ val |= QCA8K_PWS_SERDES_AEN_DIS;
+ qca8k_write(priv, QCA8K_REG_PWS, val);
+
+ /* Configure the SGMII parameters */
+ ret = qca8k_read(priv, QCA8K_REG_SGMII_CTRL, &val);
+ if (ret)
+ return ret;
+
+ val |= QCA8K_SGMII_EN_SD;
+
+ if (priv->ports_config.sgmii_enable_pll)
+ val |= QCA8K_SGMII_EN_PLL | QCA8K_SGMII_EN_RX |
+ QCA8K_SGMII_EN_TX;
+
+ if (dsa_is_cpu_port(priv->ds, port)) {
+ /* CPU port, we're talking to the CPU MAC, be a PHY */
+ val &= ~QCA8K_SGMII_MODE_CTRL_MASK;
+ val |= QCA8K_SGMII_MODE_CTRL_PHY;
+ } else if (interface == PHY_INTERFACE_MODE_SGMII) {
+ val &= ~QCA8K_SGMII_MODE_CTRL_MASK;
+ val |= QCA8K_SGMII_MODE_CTRL_MAC;
+ } else if (interface == PHY_INTERFACE_MODE_1000BASEX) {
+ val &= ~QCA8K_SGMII_MODE_CTRL_MASK;
+ val |= QCA8K_SGMII_MODE_CTRL_BASEX;
+ }
+
+ qca8k_write(priv, QCA8K_REG_SGMII_CTRL, val);
+
+ /* From original code is reported port instability as SGMII also
+ * require delay set. Apply advised values here or take them from DT.
+ */
+ if (interface == PHY_INTERFACE_MODE_SGMII)
+ qca8k_mac_config_setup_internal_delay(priv, cpu_port_index, reg);
+ /* For qca8327/qca8328/qca8334/qca8338 sgmii is unique and
+ * falling edge is set writing in the PORT0 PAD reg
+ */
+ if (priv->switch_id == QCA8K_ID_QCA8327 ||
+ priv->switch_id == QCA8K_ID_QCA8337)
+ reg = QCA8K_REG_PORT0_PAD_CTRL;
+
+ val = 0;
+
+ /* SGMII Clock phase configuration */
+ if (priv->ports_config.sgmii_rx_clk_falling_edge)
+ val |= QCA8K_PORT0_PAD_SGMII_RXCLK_FALLING_EDGE;
+
+ if (priv->ports_config.sgmii_tx_clk_falling_edge)
+ val |= QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE;
+
+ if (val)
+ ret = qca8k_rmw(priv, reg,
+ QCA8K_PORT0_PAD_SGMII_RXCLK_FALLING_EDGE |
+ QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE,
+ val);
+
return 0;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH net-next v2 6/6] net: dsa: qca8k: mark as non-legacy
2022-02-17 18:29 [PATCH net-next v2 0/6] net: dsa: qca8k: convert to phylink_pcs and mark as non-legacy Russell King (Oracle)
` (4 preceding siblings ...)
2022-02-17 18:30 ` [PATCH net-next v2 5/6] net: dsa: qca8k: move pcs configuration Russell King (Oracle)
@ 2022-02-17 18:31 ` Russell King (Oracle)
2022-02-18 11:50 ` [PATCH net-next v2 0/6] net: dsa: qca8k: convert to phylink_pcs and " patchwork-bot+netdevbpf
6 siblings, 0 replies; 17+ messages in thread
From: Russell King (Oracle) @ 2022-02-17 18:31 UTC (permalink / raw)
To: Ansuel Smith
Cc: Andrew Lunn, David S. Miller, Florian Fainelli, Jakub Kicinski,
netdev, Vivien Didelot, Vladimir Oltean
The qca8k driver does not make use of the speed, duplex, pause or
advertisement in its phylink_mac_config() implementation, so it can be
marked as a non-legacy driver.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/dsa/qca8k.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 2f8b03ff748c..04fa21e37dfa 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1814,6 +1814,8 @@ static void qca8k_phylink_get_caps(struct dsa_switch *ds, int port,
config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
MAC_10 | MAC_100 | MAC_1000FD;
+
+ config->legacy_pre_march2020 = false;
}
static void
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH net-next v2 0/6] net: dsa: qca8k: convert to phylink_pcs and mark as non-legacy
2022-02-17 18:29 [PATCH net-next v2 0/6] net: dsa: qca8k: convert to phylink_pcs and mark as non-legacy Russell King (Oracle)
` (5 preceding siblings ...)
2022-02-17 18:31 ` [PATCH net-next v2 6/6] net: dsa: qca8k: mark as non-legacy Russell King (Oracle)
@ 2022-02-18 11:50 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 17+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-02-18 11:50 UTC (permalink / raw)
To: Russell King
Cc: ansuelsmth, andrew, davem, f.fainelli, kuba, netdev,
vivien.didelot, olteanv
Hello:
This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:
On Thu, 17 Feb 2022 18:29:50 +0000 you wrote:
> This series adds support into DSA for the mac_select_pcs method, and
> converts qca8k to make use of this, eventually marking qca8k as non-
> legacy.
>
> Patch 1 adds DSA support for mac_select_pcs.
> Patch 2 and patch 3 moves code around in qca8k to make patch 4 more
> readable.
> Patch 4 does a simple conversion to phylink_pcs.
> Patch 5 moves the serdes configuration to phylink_pcs.
> Patch 6 marks qca8k as non-legacy.
>
> [...]
Here is the summary with links:
- [net-next,v2,1/6] net: dsa: add support for phylink mac_select_pcs()
https://git.kernel.org/netdev/net-next/c/bde018222c6b
- [net-next,v2,2/6] net: dsa: qca8k: move qca8k_setup()
https://git.kernel.org/netdev/net-next/c/3ce855f0408a
- [net-next,v2,3/6] net: dsa: qca8k: move qca8k_phylink_mac_link_state()
https://git.kernel.org/netdev/net-next/c/10728cd7967a
- [net-next,v2,4/6] net: dsa: qca8k: convert to use phylink_pcs
https://git.kernel.org/netdev/net-next/c/9612a8f9154f
- [net-next,v2,5/6] net: dsa: qca8k: move pcs configuration
https://git.kernel.org/netdev/net-next/c/7544b3ff745b
- [net-next,v2,6/6] net: dsa: qca8k: mark as non-legacy
https://git.kernel.org/netdev/net-next/c/d9cbacf0574a
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 17+ messages in thread