| Message ID | 20241021010652.4944-1-rosenp@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: mv88e6xxx: use ethtool_puts | expand |
On Sun, Oct 20, 2024 at 06:06:52PM -0700, Rosen Penev wrote: > Allows simplifying get_strings and avoids manual pointer manipulation. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Mon, Oct 21, 2024 at 6:37 AM Rosen Penev <rosenp@gmail.com> wrote: > > Allows simplifying get_strings and avoids manual pointer manipulation. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> LGTM Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
On Mon, Oct 21, 2024 at 9:27 AM Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com> wrote: > > On Mon, Oct 21, 2024 at 6:37 AM Rosen Penev <rosenp@gmail.com> wrote: > > > > Allows simplifying get_strings and avoids manual pointer manipulation. Looking more at these files, I see further pointer manipulation later on. Specifically I have this change locally: static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, u32 stringset, uint8_t *data) { struct mv88e6xxx_chip *chip = ds->priv; - int count = 0; if (stringset != ETH_SS_STATS) return; mv88e6xxx_reg_lock(chip); - if (chip->info->ops->stats_get_strings) - count = chip->info->ops->stats_get_strings(chip, data); - - if (chip->info->ops->serdes_get_strings) { - data += count * ETH_GSTRING_LEN; - count = chip->info->ops->serdes_get_strings(chip, port, data); - } - - data += count * ETH_GSTRING_LEN; mv88e6xxx_atu_vtu_get_strings(data); mv88e6xxx_reg_unlock(chip); Do you guys think a v2 is in order? > > > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > > LGTM > Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> > > > -- > Regards, > Kalesh A P
On Mon, Oct 21, 2024 at 11:56:47AM -0700, Rosen Penev wrote: > On Mon, Oct 21, 2024 at 9:27 AM Kalesh Anakkur Purayil > <kalesh-anakkur.purayil@broadcom.com> wrote: > > > > On Mon, Oct 21, 2024 at 6:37 AM Rosen Penev <rosenp@gmail.com> wrote: > > > > > > Allows simplifying get_strings and avoids manual pointer manipulation. > Looking more at these files, I see further pointer manipulation later > on. Specifically I have this change locally: So lets mark this as change-requested. Andrew --- pw-bot: cr
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index fe123abf7b5b..8416b03e131d 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -1162,8 +1162,7 @@ static int mv88e6xxx_stats_get_strings(struct mv88e6xxx_chip *chip, for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) { stat = &mv88e6xxx_hw_stats[i]; if (stat->type & types) { - memcpy(data + j * ETH_GSTRING_LEN, stat->string, - ETH_GSTRING_LEN); + ethtool_puts(&data, stat->string); j++; } } @@ -1204,9 +1203,7 @@ static void mv88e6xxx_atu_vtu_get_strings(uint8_t *data) unsigned int i; for (i = 0; i < ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); i++) - strscpy(data + i * ETH_GSTRING_LEN, - mv88e6xxx_atu_vtu_stats_strings[i], - ETH_GSTRING_LEN); + ethtool_puts(&data, mv88e6xxx_atu_vtu_stats_strings[i]); } static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c index 01ea53940786..327831d2b547 100644 --- a/drivers/net/dsa/mv88e6xxx/serdes.c +++ b/drivers/net/dsa/mv88e6xxx/serdes.c @@ -144,8 +144,7 @@ int mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip, for (i = 0; i < ARRAY_SIZE(mv88e6352_serdes_hw_stats); i++) { stat = &mv88e6352_serdes_hw_stats[i]; - memcpy(data + i * ETH_GSTRING_LEN, stat->string, - ETH_GSTRING_LEN); + ethtool_puts(&data, stat->string); } return ARRAY_SIZE(mv88e6352_serdes_hw_stats); } @@ -405,8 +404,7 @@ int mv88e6390_serdes_get_strings(struct mv88e6xxx_chip *chip, for (i = 0; i < ARRAY_SIZE(mv88e6390_serdes_hw_stats); i++) { stat = &mv88e6390_serdes_hw_stats[i]; - memcpy(data + i * ETH_GSTRING_LEN, stat->string, - ETH_GSTRING_LEN); + ethtool_puts(&data, stat->string); } return ARRAY_SIZE(mv88e6390_serdes_hw_stats); }
Allows simplifying get_strings and avoids manual pointer manipulation. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- drivers/net/dsa/mv88e6xxx/chip.c | 7 ++----- drivers/net/dsa/mv88e6xxx/serdes.c | 6 ++---- 2 files changed, 4 insertions(+), 9 deletions(-)