As stated in the subject this series is a short prequel before submitting
the main patches adding the memory-mapped DW XPCS support to the DW XPCS
and DW *MAC (STMMAC) drivers. Originally it was a part of the bigger
patchset (see the changelog v2 link below) but was detached to a
preparation set to shrink down the main series thus simplifying it'
review.
The patchset' content is straightforward: drop the redundant sentinel
entry and the header files; return EINVAL errno from the soft-reset method
and make sure that the interface validation method return EINVAL straight
away if the requested interface isn't supported by the XPCS device
instance. All of these changes are required to simplify the changes being
introduced a bit later in the framework of the memory-mapped DW XPCS
support patches.
Link: https://lore.kernel.org/netdev/[email protected]
Changelog v2:
- Move the preparation patches to a separate series.
- Simplify the commit messages (@Russell, @Vladimir).
Signed-off-by: Serge Semin <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Mengyuan Lou <[email protected]>
Cc: Tomer Maimon <[email protected]>
Cc: Jiawen Wu <[email protected]>
Cc: Alexandre Torgue <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Serge Semin (4):
net: pcs: xpcs: Drop sentinel entry from 2500basex ifaces list
net: pcs: xpcs: Drop redundant workqueue.h include directive
net: pcs: xpcs: Return EINVAL in the internal methods
net: pcs: xpcs: Explicitly return error on caps validation
drivers/net/pcs/pcs-xpcs.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
--
2.43.0
There are currently only two methods (xpcs_find_compat() and
xpcs_get_interfaces()) defined in the driver which loop over the available
interfaces. All of them rely on the xpcs_compat::num_interfaces field
value to get the total number of supported interfaces. Thus the interface
arrays are supposed to be filled with actual interface IDs and there is no
need in the dummy terminating ID placed at the end of the arrays.
Based on the above drop the PHY_INTERFACE_MODE_MAX entry from the
xpcs_2500basex_interfaces array and the PHY_INTERFACE_MODE_MAX-based
conditional statement from the xpcs_get_interfaces() method as redundant.
Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Vladimir Oltean <[email protected]>
---
drivers/net/pcs/pcs-xpcs.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 31f0beba638a..dc7c374da495 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -130,7 +130,6 @@ static const phy_interface_t xpcs_1000basex_interfaces[] = {
static const phy_interface_t xpcs_2500basex_interfaces[] = {
PHY_INTERFACE_MODE_2500BASEX,
- PHY_INTERFACE_MODE_MAX,
};
enum {
@@ -636,8 +635,7 @@ void xpcs_get_interfaces(struct dw_xpcs *xpcs, unsigned long *interfaces)
const struct xpcs_compat *compat = &xpcs->id->compat[i];
for (j = 0; j < compat->num_interfaces; j++)
- if (compat->interface[j] < PHY_INTERFACE_MODE_MAX)
- __set_bit(compat->interface[j], interfaces);
+ __set_bit(compat->interface[j], interfaces);
}
}
EXPORT_SYMBOL_GPL(xpcs_get_interfaces);
--
2.43.0
There is nothing CM workqueue-related in the driver. So the respective
include directive can be dropped.
While at it add an empty line delimiter between the generic and local path
include directives to visually separate them.
Signed-off-by: Serge Semin <[email protected]>
Tested-by: Andrew Lunn <[email protected]>
---
drivers/net/pcs/pcs-xpcs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index dc7c374da495..7f8c63922a4b 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -10,7 +10,7 @@
#include <linux/pcs/pcs-xpcs.h>
#include <linux/mdio.h>
#include <linux/phylink.h>
-#include <linux/workqueue.h>
+
#include "pcs-xpcs.h"
#define phylink_pcs_to_xpcs(pl_pcs) \
--
2.43.0
In particular the xpcs_soft_reset() and xpcs_do_config() functions
currently return -1 if invalid auto-negotiation mode is specified. That
value might be then passed to the generic kernel subsystems which require
a standard kernel errno value. Even though the erroneous conditions are
very specific (memory corruption or buggy driver implementation) using a
hard-coded -1 literal doesn't seem correct anyway especially when it comes
to passing it higher to the network subsystem or printing to the system
log. Convert the hard-coded error values to -EINVAL then.
Signed-off-by: Serge Semin <[email protected]>
Tested-by: Andrew Lunn <[email protected]>
---
drivers/net/pcs/pcs-xpcs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 7f8c63922a4b..92c47da61db4 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -292,7 +292,7 @@ static int xpcs_soft_reset(struct dw_xpcs *xpcs,
dev = MDIO_MMD_VEND2;
break;
default:
- return -1;
+ return -EINVAL;
}
ret = xpcs_write(xpcs, dev, MDIO_CTRL1, MDIO_CTRL1_RESET);
@@ -889,7 +889,7 @@ int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
return ret;
break;
default:
- return -1;
+ return -EINVAL;
}
if (compat->pma_config) {
--
2.43.0
If an unsupported interface is passed to the PCS validation callback there
is no need in further link-modes calculations since the resultant array
will be initialized with zeros which will be perceived by the phylink
subsystem as error anyway (see phylink_validate_mac_and_pcs()). Instead
let's explicitly return the -EINVAL error to inform the caller about the
unsupported interface as it's done in the rest of the pcs_validate
callbacks.
Signed-off-by: Serge Semin <[email protected]>
---
drivers/net/pcs/pcs-xpcs.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 92c47da61db4..46afeb5510c0 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -613,14 +613,15 @@ static int xpcs_validate(struct phylink_pcs *pcs, unsigned long *supported,
xpcs = phylink_pcs_to_xpcs(pcs);
compat = xpcs_find_compat(xpcs->id, state->interface);
+ if (!compat)
+ return -EINVAL;
/* Populate the supported link modes for this PHY interface type.
* FIXME: what about the port modes and autoneg bit? This masks
* all those away.
*/
- if (compat)
- for (i = 0; compat->supported[i] != __ETHTOOL_LINK_MODE_MASK_NBITS; i++)
- set_bit(compat->supported[i], xpcs_supported);
+ for (i = 0; compat->supported[i] != __ETHTOOL_LINK_MODE_MASK_NBITS; i++)
+ set_bit(compat->supported[i], xpcs_supported);
linkmode_and(supported, supported, xpcs_supported);
--
2.43.0
Hello Serge,
On Thu, 22 Feb 2024 20:58:23 +0300
Serge Semin <[email protected]> wrote:
> If an unsupported interface is passed to the PCS validation callback there
> is no need in further link-modes calculations since the resultant array
> will be initialized with zeros which will be perceived by the phylink
> subsystem as error anyway (see phylink_validate_mac_and_pcs()). Instead
> let's explicitly return the -EINVAL error to inform the caller about the
> unsupported interface as it's done in the rest of the pcs_validate
> callbacks.
>
> Signed-off-by: Serge Semin <[email protected]>
This looks good to me,
Reviewed-by: Maxime Chevallier <[email protected]>
Hello:
This series was applied to netdev/net-next.git (main)
by David S. Miller <[email protected]>:
On Thu, 22 Feb 2024 20:58:19 +0300 you wrote:
> As stated in the subject this series is a short prequel before submitting
> the main patches adding the memory-mapped DW XPCS support to the DW XPCS
> and DW *MAC (STMMAC) drivers. Originally it was a part of the bigger
> patchset (see the changelog v2 link below) but was detached to a
> preparation set to shrink down the main series thus simplifying it'
> review.
>
> [...]
Here is the summary with links:
- [net-next,v2,1/4] net: pcs: xpcs: Drop sentinel entry from 2500basex ifaces list
https://git.kernel.org/netdev/net-next/c/0ffc3292c02b
- [net-next,v2,2/4] net: pcs: xpcs: Drop redundant workqueue.h include directive
https://git.kernel.org/netdev/net-next/c/e26802ebd295
- [net-next,v2,3/4] net: pcs: xpcs: Return EINVAL in the internal methods
https://git.kernel.org/netdev/net-next/c/f5151005d379
- [net-next,v2,4/4] net: pcs: xpcs: Explicitly return error on caps validation
https://git.kernel.org/netdev/net-next/c/361dd531a11b
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html