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 v4] dsa: lan9303: Add 3 ethtool stats
Thread-Topic: [PATCH net-next v4] dsa: lan9303: Add 3 ethtool stats
Thread-Index: AQHZBPd7JcFTQQToA0qvxVT5rs7YJ65X8puAgAFEzcA=
Date:   Thu, 1 Dec 2022 16:42:10 +0000
Message-ID: <MWHPR11MB1693DA619CAC5AA135B47424EF149@MWHPR11MB1693.namprd11.prod.outlook.com>
References: <20221130200804.21778-1-jerry.ray@microchip.com>
 <20221130205651.4kgh7dpqp72ywbuq@skbuf>
In-Reply-To: <20221130205651.4kgh7dpqp72ywbuq@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?lE/QZ2MxgdEgmRJw41t8nngK+jnYmRaz9bssCKhZ6ockxBvUhUNnVuS8nQFN?=
 =?us-ascii?Q?TtVDFEZCUD/mo9e9ubV6b3ZheZi3PY+p/Qq7IKp9/bWazIVyz1dKpF3I4JCL?=
 =?us-ascii?Q?YvYtyTplz16kucWtbwrXdiaeZV0eYpZdkA4/oLweBVCFhoy5CaMRA3/UHcEs?=
 =?us-ascii?Q?pG+m9vD+3rVwmeN3dESpvjn0Lq56f2UxB8kaYGU/qauT3jKEsFSNzY41vw8+?=
 =?us-ascii?Q?Ra5TYCHHyhCQ3zUt3O1Ght9zajDykPnDXCmyFy3cyUz7Rgp1tCArWSD1BzVI?=
 =?us-ascii?Q?SSDm9Rky/eTvYWDOXowpSKWzgpR8Nc8Z292e1GkUVk5Fq9ODQ6TRY7uP/kMK?=
 =?us-ascii?Q?tg0YT/v3Cd6ntXyWj1GHYW0f1745ObgeARC57M4wtdp8JU2pQSVafEcqStfA?=
 =?us-ascii?Q?/BfM4zHXms7IYlJejOyB+CS52+FFryqEV9GFIGWNT2vFfwVspbBdJJ16pF0t?=
 =?us-ascii?Q?aDcD9d3uO40S+yhlduQz7MDm1LigWJyGDeQFYYPCD+sSUs58rE2pt0nb5hnA?=
 =?us-ascii?Q?bTnYjsI4KpHZHmTR/Ugf4a4iI9K0fNrOoQKsGBvYI3xhMZmfx0j4Z4gplG4O?=
 =?us-ascii?Q?AByHpNdSv9SqnBG+2zruBLAsnuYrt8mlxev+a3/m5adC/zfgl+coX12tCEEE?=
 =?us-ascii?Q?VvqLacGDhiunL0dEYFE8wjiZbleRvqXsMy8Oxb9FNM+aDGMwS3HV37w8XJKR?=
 =?us-ascii?Q?aTz054iGQ6c4hgauY0RfD+P5PXjEVS3AnF8ySAZlhGdioqtNahVm7AH3uKEo?=
 =?us-ascii?Q?bSBkqRXuKEZfU+POrnjElXx4SAOWXFXjJ9qRst8duqeBVjbGPG3vVWAeoba0?=
 =?us-ascii?Q?mT3EI1waEXaX9JMEkB5snfozF1+rpzeTSCKMPs+3mEpCOvxwaeRX8kEyemV9?=
 =?us-ascii?Q?w639p+C3joll2evvx9l8tg8kHouUk/J0dFsOalyj+c8A9nniiKZJARUMq14u?=
 =?us-ascii?Q?Wdx3gFb+nKeRyjaSdPyb9FON8Hn4Us8BkhyPIDbCABe5Ns43GlR02qVl4IRq?=
 =?us-ascii?Q?owI+BsmdMPtVFdLrp7awQLFCFvChniI38OdX73/Qtb2kMFd3/ZEGFPLBsLsR?=
 =?us-ascii?Q?QUQ22FSXjk4aHqv0CNbqOjF84RDMUwVUcsisQUJ08hdsX0eM8g/qDmzNNCIG?=
 =?us-ascii?Q?Uaygf1Jmm6CxpNkfcHD5Wbnh3fjAvmhkyUuLlpHYIkzQscgvpx4PgA/Fx5di?=
 =?us-ascii?Q?jf/lmPIScOtDY8l4X4iQHoiIYyp4q+gbarOfERUm1x3lwHaD0S4khWkpYwXz?=
 =?us-ascii?Q?79yHZk3hgsO+pKmwSLGQzXfLHtwn7c/1QhDJ9H8wVTESHdqTiPJSLqEVqzUC?=
 =?us-ascii?Q?aGwy8WId/f50S2NvXYSoKv9b5teSoBWSAhZWurWl4k/Ywg4iG1Qvo0IOj39M?=
 =?us-ascii?Q?CE2suiOEp6PSOnT6SDXvMlQXvyRKu2AuuV8yYjtbhXfCLlYtsDaRP6ejCnuw?=
 =?us-ascii?Q?+bsMCAHzZ47ArS15CeswN2CFJC3SwKoI5i+gaZsWNpReeJeRRaHTlPW/uJ2s?=
 =?us-ascii?Q?XRaWftOOCpJ5Dn+BqdlueRUXWvDcGWt6LrQ87p1icbxbTqPBlrXU3ndQ7flc?=
 =?us-ascii?Q?CXJDe215WuU800Vfsa3npur/wvHBLgRKyfTlqx+0?=
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: dd8a2b4c-5065-410c-7fd4-08dad3bafe0a
X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Dec 2022 16:42:10.5200
 (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: 35OYP+CKuR+q9BJB38B+Y+h4VNn7Rb2o2KWiHLn327cHSdk8654wAYg4BfwHzhH/AHSkl+mBJcWq8Z89uySt3w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB6290
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,
        RCVD_IN_MSPIKE_H2,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;
>>
>>       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);
>>
>> -             if (ret)
>> +             if (ret) {
>>                       dev_warn(chip->dev, "Reading status port %d reg %u=
 failed\n",
>>                                port, lan9303_mib[u].offset);
>> +                     reg =3D 0;
>> +             }
>
>This part of the change still is unrelated and affects existing code.
>Bug fixes to existing code are submitted as separate patches. In some
>kernel trees, they are at the very least tagged with a Fixes: tag and
>put before other development work. In netdev, they are sent to a different
>git tree (net.git) which eventually lands in a different set of branches
>than net-next.git. You need to not mix bug fixes with development code.
>Andrew also suggested that you separate each logical change into a
>separate patch.
>
>This, plus the fact that Jakub asked you to also provide standardized
>counters, not just free-form ones, which you found it ok to disregard.
>
>I hope that only a misunderstanding is involved, because if it isn't,
>then Jakub will know you, alright, but as the person who disregards
>review feedback and expects that it'll just disappear. I think Jakub
>has pretty solid grounds to not expect that you'll come back with what
>has been requested.
>

I was hoping to get at least this change upstreamed this cycle and address
the standardized counters down the road.  get_stats64 will require PHYLINK.
I replied as such and was hoping to get the benefit of the doubt.
I have a patch set under internal review for migrating to phylink and addin=
g
support for the port_max_mtu api.

I have been asked to, and have plans for, adding VLAN support and then addi=
ng
Hardware Timestamping support for the LAN9354 variant.  I was hoping to sho=
w
some progress with this patchset, but if I need to pull it and pick it up
later then I'll shuffle things around. I wasn't disregarding Jakub's reques=
t
and apologize if was viewed that way. I'm simply trying to generate a corre=
ct
patchset on something relatively simple before moving on to more complicate=
d
patchsets.

>Sorry, this patch has a NACK from me at least until you come back with
>some clarifications, and split the change.
>
>>               data[u] =3D reg;
>>       }

My plan is to correct the issues pointed out in this patchset, but not exte=
nd
it to cover standardized counters. get_stats64 is a separate beast requirin=
g
a background thread to keep the stats stored in the driver accumulated and
up to date.  The current patch simply reads a few more registers from the
hardware when asked.

Regards,
Jerry.