[PATCH net-next] net: mv88e6xxx: use ethtool_puts

[PATCH net-next] net: mv88e6xxx: use ethtool_puts

[Rosen Penev]

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(-)

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);
 }
-- 
2.47.0

Re: [PATCH net-next] net: mv88e6xxx: use ethtool_puts

[Andrew Lunn]

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

Re: [PATCH net-next] net: mv88e6xxx: use ethtool_puts

[Kalesh Anakkur Purayil]

[-- Attachment #1: Type: text/plain, Size: 303 bytes --]

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>


-- 
Regards,
Kalesh A P

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]

Re: [PATCH net-next] net: mv88e6xxx: use ethtool_puts

[Rosen Penev]

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

Re: [PATCH net-next] net: mv88e6xxx: use ethtool_puts

[Andrew Lunn]

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