netdev - Re: [PATCH net-next 1/6] net: stmmac: add platform library

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <u7sabfdqk7i6wlv2j4cxuyb6psjwqs2kukdkafhcpq2zc766m3@m6iqexqjrvkv>
Date: Tue, 12 Sep 2023 12:32:40 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: Jose Abreu <Jose.Abreu@...opsys.com>, 
	"Russell King (Oracle)" <rmk+kernel@...linux.org.uk>
Cc: Alexandre Torgue <alexandre.torgue@...s.st.com>, 
	Alexei Starovoitov <ast@...nel.org>, "bpf@...r.kernel.org" <bpf@...r.kernel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, "David S. Miller" <davem@...emloft.net>, 
	Emil Renner Berthing <kernel@...il.dk>, Eric Dumazet <edumazet@...gle.com>, 
	Fabio Estevam <festevam@...il.com>, Jakub Kicinski <kuba@...nel.org>, 
	Jesper Dangaard Brouer <hawk@...nel.org>, John Fastabend <john.fastabend@...il.com>, 
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, 
	"linux-stm32@...md-mailman.stormreply.com" <linux-stm32@...md-mailman.stormreply.com>, Maxime Coquelin <mcoquelin.stm32@...il.com>, 
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, NXP Linux Team <linux-imx@....com>, 
	Paolo Abeni <pabeni@...hat.com>, Pengutronix Kernel Team <kernel@...gutronix.de>, 
	Samin Guo <samin.guo@...rfivetech.com>, Sascha Hauer <s.hauer@...gutronix.de>, 
	Shawn Guo <shawnguo@...nel.org>
Subject: Re: [PATCH net-next 1/6] net: stmmac: add platform library

On Tue, Sep 12, 2023 at 07:59:49AM +0000, Jose Abreu wrote:
> From: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> Date: Mon, Sep 11, 2023 at 16:28:40
> 
> > Add a platform library of helper functions for common traits in the
> > platform drivers. Currently, this is setting the tx clock.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> > ---

> >  drivers/net/ethernet/stmicro/stmmac/Makefile  |  2 +-
> >  .../ethernet/stmicro/stmmac/stmmac_plat_lib.c | 29 +++++++++++++++++++
> >  .../ethernet/stmicro/stmmac/stmmac_plat_lib.h |  8 +++++
> 
> Wouldn't it be better to just call it "stmmac_lib{.c,.h}" in case we need to add
> more helpers on the future that are not only for platform-based drivers?

What is the difference between stmmac_platform.{c,h} and
stmmac_plat_lib.{c,h} files? It isn't clear really. In perspective it
may cause confusions like mixed definitions in both of these files.

Why not to use the stmmac_platform.{c,h} instead of adding one more
file? Especially seeing it already has some generic
platform/glue-drivers helpers like:
stmmac_get_platform_resources() <- especially this one.
(devm_)stmmac_probe_config_dt()
stmmac_remove_config_dt()
stmmac_pltfr_init()
stmmac_pltfr_exit()
(devm_)stmmac_pltfr_probe()
stmmac_pltfr_remove()
stmmac_pltfr_suspend()
stmmac_pltfr_resume()
stmmac_runtime_suspend()
stmmac_runtime_resume()
stmmac_pltfr_noirq_suspend()
stmmac_pltfr_noirq_resume()
All of them look like being decent to be defined in the generic
platform library methods too.

-Serge(y)

> 
> I believe it's also missing the SPDX identifiers?
> 
> Thanks,
> Jose