Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
From:   "Gan, Yi Fang" <yi.fang.gan@intel.com>
To:     Russell King <linux@armlinux.org.uk>
CC:     Alexandre Torgue <alexandre.torgue@foss.st.com>,
        Jose Abreu <joabreu@synopsys.com>,
        "David S . Miller" <davem@davemloft.net>,
        "Eric Dumazet" <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-stm32@st-md-mailman.stormreply.com" 
        <linux-stm32@st-md-mailman.stormreply.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Looi, Hong Aun" <hong.aun.looi@intel.com>,
        "Voon, Weifeng" <weifeng.voon@intel.com>,
        "Song, Yoong Siang" <yoong.siang.song@intel.com>,
        "Ahmad Tarmizi, Noor Azura" <noor.azura.ahmad.tarmizi@intel.com>
Subject: RE: [PATCH net-next 1/1] net: stmmac: add check for advertising
 linkmode request for set-eee
Thread-Topic: [PATCH net-next 1/1] net: stmmac: add check for advertising
 linkmode request for set-eee
Thread-Index: AQHaCKJwVwif6aPTx0uyiZuY8b+qrrBdP/QAgAZSMzA=
Date:   Tue, 31 Oct 2023 08:44:23 +0000
Message-ID: <DM6PR11MB3306A3162F6A6086A4CBA049B9A0A@DM6PR11MB3306.namprd11.prod.outlook.com>
References: <20231027065054.3808352-1-yi.fang.gan@intel.com>
 <ZTtpBCZuB+bjVt9D@shell.armlinux.org.uk>
In-Reply-To: <ZTtpBCZuB+bjVt9D@shell.armlinux.org.uk>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Lr11f5KUEU3bGBG2HCtrQfN4c471UX7X/uq/yRyMNXa7H1/dU8e3ELdP8mas?=
 =?us-ascii?Q?CAk63kgvl3FvfzDHq0KHR3XbKCZmX9c/7xXgoxx8CEeLDRBJtnaoVgttfTUH?=
 =?us-ascii?Q?MbUXxZ5tec4/2Xd2KTsMsOSBilyyFSePwmdSivEtgw5Oz9jTjsH1BpFfVOnW?=
 =?us-ascii?Q?EkXtbVdNrsVGGdaGME8xRD/2D1HIdlyrhPAk19MNkJnF+LLXdAv7EqavZa13?=
 =?us-ascii?Q?NrNQcP9wGcpTQPmHmALutRcur1G6cMvYoKw/j7HSR7dfGn89Rg2oJE6ehdjy?=
 =?us-ascii?Q?1TBw19+L3e+GIdz3QJuyQv+6heZzGv+gYZYEEwQVF5sOENvxPQ18ivSZsivL?=
 =?us-ascii?Q?45XSWuB3HBC2gS4K1igz2xGa1jECcLDS4AXnRRMTW9Pk8wkMMrkJy/1FUC2j?=
 =?us-ascii?Q?AvSyu5qxjYup9SfRPzmcfBgKvoZQL8WIuz9kHLb8zAzuDjsTJJkZwlNVOkol?=
 =?us-ascii?Q?zzByVFjfUfV45LYHXz99Sz6NHRajaCzL0EEA5CjScaAlEXYJgRvaaPiAMNiv?=
 =?us-ascii?Q?yjdrYusmAvruiFQUt2gt16jU2JCDIrBYLhNvbur+ZuPBuXhING+S79BOnlHM?=
 =?us-ascii?Q?IwBga2fjjBd1NBMdDQ3nWLC3IQgTdBweJ8bZLoLJ02UedAEtHsKI9Mnb3dJm?=
 =?us-ascii?Q?NqYEisrHARbgyIkrMj2xvcmGXeEgbuCSKb2rD+Yir0n5NLU7N0D4YR2SMldQ?=
 =?us-ascii?Q?2mfj75H5vhcN7uH+VjpWJ+dsF0Gl16M3JLoyYY1HgcCkFiL4pabEOur6euok?=
 =?us-ascii?Q?Rc/za1JadaRqFKO4WJGQQdcCaQqI7dHkWmxwYiPXaQvNaeiygFGzPoMCCLPd?=
 =?us-ascii?Q?sRCtLs2JjUTQU4wOqPueXyTzASj+s8l2oKi7w37h3Zui+fR0ZSYokwgJC6i0?=
 =?us-ascii?Q?t0bD3fu/NAjjYJPDWA1dODVrqYe/MsVo6tsQZBJ1Lo8roMSbqnZgsQW9rZNx?=
 =?us-ascii?Q?k3bzuRKhFe7suonYxoVll6O3JfOhaJg+rGchQHTDkqEH9lCc9CJ19WBpgvSx?=
 =?us-ascii?Q?pCNCQMabRg42zIA7W3nwhTwMhlt/04zKQFfi+cz8k2i03DfvRPJfi7UrEoc/?=
 =?us-ascii?Q?pNS2OIgwXms7xlES7HOmGvglW1C6qjTO7j6yrazf1lWARGilGEAuPAZJfeC/?=
 =?us-ascii?Q?KVWJ5pMVbLMgDQdJwD8He37qOChXcvdzSK+BIVlNyfDSFDST62y5zVfQTofw?=
 =?us-ascii?Q?LXsBmQAOOBlMJF5r3km1xSL6FoQnsz3x36q0LpAj+GE77/YiySFlrpxyFmif?=
 =?us-ascii?Q?X/rDBWxilre3bE/mIWykRELpUI5IAzUsO6h/VewogXRhE9HXmm+LdehTgYS8?=
 =?us-ascii?Q?UbJpWX14g0GnafiLmoOGz0fVlalB62KHQQS1KwVLVdFckYEQKS8wtsDIu7jQ?=
 =?us-ascii?Q?dqLrr1yZERCfL/HSIZesx+l9EY/cW6c8w9fnv4Jdz3U01TsXLTJu9SKLaaXk?=
 =?us-ascii?Q?vQk6YR+L0DRAqzQE7kHFWGkc3+9/7QkgMielGk65yTumOPe4tsF42CUSIS70?=
 =?us-ascii?Q?qyvIQhGXiKrtK10eQWYk7cyx8zViOqczOrqBPCbjsZCmqTzv7yc+V+LA4Vkt?=
 =?us-ascii?Q?4k+lOH1o9tcr4khCMfr4UqLmXxmrRYa3rA9wqT3i?=
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: DM6PR11MB3306.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4937b629-5519-4ec8-2ee3-08dbd9ed9539
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Oct 2023 08:44:23.7067
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 3dp81aIfZosGG5sCSwk2P6XK9mmI0Dx+v4lK63x0RDiFbkYjJb/PZLaH+MxDGnMeDQ8Pz4An6lkcshYRpD+fDg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB5057
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,
        RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        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
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Tue, 31 Oct 2023 01:44:39 -0700 (PDT)

Hi Russell King,

Why should this functionality be specific to stmmac?
This functionality is not specific to stmmac but other drivers can have the=
ir
 own implementation.=20
(e.g. https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/q=
logic/qede/qede_ethtool.c#L1855)

Why do we need this?
Current implementation will not take any effect if user enters unsupported =
value but user might
not aware. With this, an error will be prompted if unsupported value is giv=
en.=20

What is wrong with the checking and masking that phylib is doing?
Nothing wrong with the phylib but there is no error return back to ethtool =
commands=20
if unsupported value is given.

Why should we trust the value in edata->supported provided by the user?
The edata->supported is getting from the current setting and the value is s=
et upon bootup.
Users are not allowed to change it.

Sorry, but no. I see no reason that this should be done, especially not in =
the stmmac driver.
I understand your reasoning. From your point of view, is this kind of error=
 message/ error handling=20
not needed?


Best Regards,
Fang

> -----Original Message-----
> From: Russell King <linux@armlinux.org.uk>
> Sent: Friday, October 27, 2023 3:39 PM
> To: Gan, Yi Fang <yi.fang.gan@intel.com>
> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>; Jose Abreu
> <joabreu@synopsys.com>; David S . Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; Maxime Coquelin
> <mcoquelin.stm32@gmail.com>; netdev@vger.kernel.org; linux-stm32@st-
> md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Looi, Hong Aun <hong.aun.looi@intel.com>; Voon,
> Weifeng <weifeng.voon@intel.com>; Song, Yoong Siang
> <yoong.siang.song@intel.com>; Ahmad Tarmizi, Noor Azura
> <noor.azura.ahmad.tarmizi@intel.com>
> Subject: Re: [PATCH net-next 1/1] net: stmmac: add check for advertising
> linkmode request for set-eee
>=20
> On Fri, Oct 27, 2023 at 02:50:54PM +0800, Gan Yi Fang wrote:
> > From: Noor Azura Ahmad Tarmizi <noor.azura.ahmad.tarmizi@intel.com>
> >
> > Add check for advertising linkmode set request with what is currently
> > being supported by PHY before configuring the EEE. Unsupported setting
> > will be rejected and a message will be prompted. No checking is
> > required while setting the EEE to off.
>=20
> Why should this functionality be specific to stmmac?
>=20
> Why do we need this?
>=20
> What is wrong with the checking and masking that phylib is doing?
>=20
> Why should we trust the value in edata->supported provided by the user?
>=20
> Sorry, but no. I see no reason that this should be done, especially not i=
n the
> stmmac driver.
>=20
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!