Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   <Jerry.Ray@microchip.com>
To:     <olteanv@gmail.com>
CC:     <andrew@lunn.ch>, <f.fainelli@gmail.com>, <davem@davemloft.net>,
        <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH net-next v3] dsa: lan9303: Add 3 ethtool stats
Thread-Topic: [PATCH net-next v3] dsa: lan9303: Add 3 ethtool stats
Thread-Index: AQHZA2vPxK4eDIuP5kOkg3W32g1dkK5U02GAgALOauA=
Date:   Wed, 30 Nov 2022 15:57:35 +0000
Message-ID: <MWHPR11MB1693C29AFEAE5386D0EE3AEFEF159@MWHPR11MB1693.namprd11.prod.outlook.com>
References: <20221128205521.32116-1-jerry.ray@microchip.com>
 <20221128210515.kqvdshlh3phmdpxx@skbuf>
In-Reply-To: <20221128210515.kqvdshlh3phmdpxx@skbuf>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?yxarYRalWUSs4eGmCqO43KBx/O22i/BEjZFt9QgTs1Wy4/rMizPlS6vAg4kG?=
 =?us-ascii?Q?0YEKU6jaTK3YIzM4o4VFT8zTDYP6cx37BRdsPy3Rh+/piJ8yFI+NJu9VdL4I?=
 =?us-ascii?Q?s5sF+HWfVmKndYl9ZAvwZGY81nghfNOGgc1wizqXT4vcn1m+DxF04GOTht82?=
 =?us-ascii?Q?VUKxrIHlh50wYdDbu+0RmBqbD0I3YhdTRk2nXgd7qHPRbnYo08R4fzchZ7jO?=
 =?us-ascii?Q?RanVU0H+dnN6++98F3IouxzCxNKmNuodxva+WonrtmYV+jQL5r1bjy4628c1?=
 =?us-ascii?Q?ph5SHG9HQrZO9BHQeF5mJOneqZGUBGIAIYwr7iopUyArV0YBFElcomaGwfdX?=
 =?us-ascii?Q?KIb8vlPhwyytClc5UWNtnJJ5lCFRUmrjhYhTq8OUQC2tFfGpjLtzHKMAX53p?=
 =?us-ascii?Q?BrzTqvHR5/UJVKQHPxXp92aBjZrkodUzAXh8132eOZU/HnCNWT46sCt3v9XB?=
 =?us-ascii?Q?QRvyWjlYnQ0zQytVRXiUQ9lWuZUTIvC3Ipfu9RpBzXT1VqeNp1VIVhBUab90?=
 =?us-ascii?Q?pzO9LZs61NtJTV/J/29kvl0sdMwBGeaAIc1sYw/bmhl7v1U/Rtusskc2nrBM?=
 =?us-ascii?Q?lQtuyru59XP30SQD4y3InnLZY8KydEta65NbEegyJoCH0FdSwu1t64VfWLYG?=
 =?us-ascii?Q?ilKiAiiOsnp5YAVrflysean6bBpJg/a78FSohYaUWAee1hQyacgzkAPX5SWR?=
 =?us-ascii?Q?RGlPl9BL4Lgk26/DjQ+kNH7+SQdvDaUOZ+xD/KhB3BAOO9s9L2nQfe4v2nct?=
 =?us-ascii?Q?TAIWMqyiFFjUrTNAGTR1LcJfKXS3yVkPUkOyrJ+l9QoxdneIP6rlgUSwpoXH?=
 =?us-ascii?Q?1+0XfrbPdY+nfNAel+1qGivudLP5SnKovuwdt9xYZb4QOVYOWRbd1Oxsf8Sy?=
 =?us-ascii?Q?Hm5bB2ABTcWDUlRgpzLaIZdm6Cs5RiaT383+3aqtatkdLU/y6pG8U72IEoVV?=
 =?us-ascii?Q?43FYw8gSwXrk08lOWSUuSCGpEYhpl/G/zhBZxCON5reu0MGbTj7DK8k21/un?=
 =?us-ascii?Q?31unMjyJD9ijg+Eu+PCErqzowd0m7hBkiInDW+SLGmCq7miiFhJmN8TizjkI?=
 =?us-ascii?Q?YipK78uY3E6wTRIje1Y5iO7e8kxhFVKN9rrwrG+BvgouON9SnRYIc0oHPlq0?=
 =?us-ascii?Q?Btgju73fkmyNr7PR3GDZtSfVDDkJ/F1Czkripgwlov5zHwOSLpexa70Q15AN?=
 =?us-ascii?Q?is2ehTIw4xJfknMCZ/zTxFiwpXxkV5lI/EM1eyXgq7iSbBMd/j9ZAK5yMjma?=
 =?us-ascii?Q?4Mndj+JLNyaAiKQXed//WLxNNj17lgUEw1GXLTGXhZ5iEJrGPzVbIGlAGmFr?=
 =?us-ascii?Q?7n6wFPqvTn0VJL3afTG7WoExZzCvGNT1Smt8cRB+bjXhSMUTaCEf2z+8uaKn?=
 =?us-ascii?Q?YS2whxwuSJ0ZK+uIeFmmklBpsGcMQCsbGXQwj/m1EGCnN0MBvosczc37VKYC?=
 =?us-ascii?Q?gVLsGx0QQWO8DGeUBT/b9tBOCOWumijLM3qQlmWMloyCO9cGagISbsEPCY8U?=
 =?us-ascii?Q?BACv9AtEFSbDL9g7xzONdZ1fCgRKGTSvP4a+fKCnKdyqh83qJNsICM/+wLZZ?=
 =?us-ascii?Q?Zrs5T4LraWW6eQT9kcgnmKtiXP1DoZJLLfamqhR7?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1693.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 13bb3574-ebbc-434e-e275-08dad2eb990c
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Nov 2022 15:57:35.3096
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ktpedLb3eH5NJiRFRmqY9ZMTgAGspGsOUYX2sHHGUjfI7okdakO9lnUXa2VOCqcHT0huW/NubKSEFsX79WZELA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR11MB4558
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,
        SPF_PASS autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

>>  static void lan9303_get_ethtool_stats(struct dsa_switch *ds, int port,
>>                                     uint64_t *data)
>>  {
>>       struct lan9303 *chip =3D ds->priv;
>> -     unsigned int u;
>> +     unsigned int i, u;
>>
>>       for (u =3D 0; u < ARRAY_SIZE(lan9303_mib); u++) {
>>               u32 reg;
>>               int ret;
>>
>> -             ret =3D lan9303_read_switch_port(
>> -                     chip, port, lan9303_mib[u].offset, &reg);
>> -
>> +             /* Read Port-based MIB stats. */
>> +             ret =3D lan9303_read_switch_port(chip, port,
>> +                                            lan9303_mib[u].offset,
>> +                                            &reg);
>
>Speaking of unrelated changes...
>

Cleaning up a warning generated from checkpatch

>>               if (ret)
>>                       dev_warn(chip->dev, "Reading status port %d reg %u=
 failed\n",
>>                                port, lan9303_mib[u].offset);
>
>...If lan9303_read_switch_port() fails, should we copy kernel stack
>uninitialized memory (reg) to user space?
>

Good catch.  I'll zero out the returned stat.

>>               data[u] =3D reg;
>>       }
>> +     for (i =3D 0; i < ARRAY_SIZE(lan9303_switch_mib); i++) {
>> +             u32 reg;
>> +             int ret;
>> +
>> +             /* Read Switch stats indexed by port. */
>> +             ret =3D lan9303_read_switch_reg(chip,
>> +                                           (lan9303_switch_mib[i].offse=
t +
>> +                                            port), &reg);
>> +             if (ret)
>> +                     dev_warn(chip->dev, "Reading status port %d reg %u=
 failed\n",
>> +                              port, lan9303_switch_mib[i].offset);
>
>Because the same, existing pattern is also used for new code here.
>

Ditto

>> +             data[i + u] =3D reg;
>> +     }
>>  }
>>
>>  static int lan9303_get_sset_count(struct dsa_switch *ds, int port, int =
sset)
>> @@ -1017,7 +1061,7 @@ static int lan9303_get_sset_count(struct dsa_switc=
h *ds, int port, int sset)
>>       if (sset !=3D ETH_SS_STATS)
>>               return 0;
>>
>> -     return ARRAY_SIZE(lan9303_mib);
>> +     return ARRAY_SIZE(lan9303_mib) + ARRAY_SIZE(lan9303_switch_mib);
>>  }
>>
>>  static int lan9303_phy_read(struct dsa_switch *ds, int phy, int regnum)
>> --
>> 2.17.1
>>
>