On Tue, Sep 12, 2023 at 07:59:49AM +0000, Jose Abreu wrote: > From: Russell King (Oracle) <rmk+kernel@xxxxxxxxxxxxxxx> > 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@xxxxxxxxxxxxxxx> > > --- > > 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