| Message ID | 20231214142136.17564-1-ante.knezic@helmholz.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: dont use generic selftest strings for custom selftests | expand |
On Thu, Dec 14, 2023 at 03:21:36PM +0100, Ante Knezic wrote: > if dsa device supports custom selftests than we should use custom > selftest strings for ethtool. > > Signed-off-by: Ante Knezic <ante.knezic@helmholz.de> > --- I didn't notice when the selftest support was added that there is no implementation in DSA drivers of custom ds->ops->self_test(). Adding interfaces with no users is frowned upon, precisely because it doesn't show the big picture. You must have noticed this because you do have a driver implementation, so would you mind posting it together with this fix?
Indeed I do have a custom implementation for the mv88e6xxx chip, but its not come to state to be posted because of test/chip specifics. > I didn't notice when the selftest support was added that there is no > implementation in DSA drivers of custom ds->ops->self_test(). Adding > interfaces with no users is frowned upon, precisely because it doesn't > show the big picture. I was not aware of this, I apologize. If this is the case, perhaps this patch should wait for the first custom self test implementation and be reposted as a part of bigger series. Thanks, Ante
On Thu, Dec 14, 2023 at 03:47:51PM +0100, Ante Knezic wrote: > Indeed I do have a custom implementation for the mv88e6xxx chip, but its not > come to state to be posted because of test/chip specifics. > > > I didn't notice when the selftest support was added that there is no > > implementation in DSA drivers of custom ds->ops->self_test(). Adding > > interfaces with no users is frowned upon, precisely because it doesn't > > show the big picture. > > I was not aware of this, I apologize. If this is the case, perhaps this patch > should wait for the first custom self test implementation and be reposted as > a part of bigger series. > > Thanks, > Ante > I agree this should be resubmitted with a user of the API. Looking forward to seeing it. Thanks for the understanding. The only thing I would want to comment on the patch as it is is to avoid the pattern of: if (a) return x; else return y; and formulate it as: if (a) return x; return y; instead. The "else" is redundant for an "if" that ends with a return statement. --- pw-bot: changes-requested
diff --git a/net/dsa/user.c b/net/dsa/user.c index d438884a4eb0..d0e0d1a2bff7 100644 --- a/net/dsa/user.c +++ b/net/dsa/user.c @@ -1072,7 +1072,11 @@ static void dsa_user_get_strings(struct net_device *dev, ds->ops->get_strings(ds, dp->index, stringset, data + 4 * len); } else if (stringset == ETH_SS_TEST) { - net_selftest_get_strings(data); + if (ds->ops->self_test && ds->ops->get_strings) + ds->ops->get_strings(ds, dp->index, stringset, + data); + else + net_selftest_get_strings(data); } } @@ -1123,7 +1127,10 @@ static int dsa_user_get_sset_count(struct net_device *dev, int sset) return count + 4; } else if (sset == ETH_SS_TEST) { - return net_selftest_get_count(); + if (ds->ops->self_test && ds->ops->get_sset_count) + return ds->ops->get_sset_count(ds, dp->index, sset); + else + return net_selftest_get_count(); } return -EOPNOTSUPP;
if dsa device supports custom selftests than we should use custom selftest strings for ethtool. Signed-off-by: Ante Knezic <ante.knezic@helmholz.de> --- net/dsa/user.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)