| Message ID | E1qfiq8-007TOe-9F@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,1/6] net: stmmac: add platform library | expand |
From: Russell King (Oracle) <rmk+kernel@armlinux.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@armlinux.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? I believe it's also missing the SPDX identifiers? Thanks, Jose
On Tue, Sep 12, 2023 at 07:59:49AM +0000, Jose Abreu wrote: > From: Russell King (Oracle) <rmk+kernel@armlinux.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@armlinux.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
On Tue, Sep 12, 2023 at 12:32:40PM +0300, Serge Semin wrote: > On Tue, Sep 12, 2023 at 07:59:49AM +0000, Jose Abreu wrote: > > From: Russell King (Oracle) <rmk+kernel@armlinux.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@armlinux.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? Is stmmac_platform.{c,h} used by all the drivers that are making use of this? I'm not entirely sure. If it is, then yes, it can go in stmmac_platform.[ch]. If not, then I don't think we'd want the bloat of forcing all of stmmac_platform.[ch] onto drivers that only want to use this one function.
On Tue, Sep 12, 2023 at 06:08:23PM +0100, Russell King (Oracle) wrote: > On Tue, Sep 12, 2023 at 12:32:40PM +0300, Serge Semin wrote: > > On Tue, Sep 12, 2023 at 07:59:49AM +0000, Jose Abreu wrote: > > > From: Russell King (Oracle) <rmk+kernel@armlinux.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@armlinux.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? > > Is stmmac_platform.{c,h} used by all the drivers that are making use of > this? I'm not entirely sure. > > If it is, then yes, it can go in stmmac_platform.[ch]. If not, then I > don't think we'd want the bloat of forcing all of stmmac_platform.[ch] > onto drivers that only want to use this one function. With a few exceptions almost all the STMMAC/DW*MAC glue drivers use the methods from the stmmac_platform.c module including the bits touched by your patchset. AFAICS semantically both stmmac_platform.c and stmmac_plat_lib.c look the same. They don't do anything on its own but provide some common methods utilized by the glue drivers for some platform-specific setups. So basically stmmac_platform.[ch] is already a library of the common platform methods. There is no need in creating another one. -Serge(y) > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Wed, Sep 13, 2023 at 03:56:07AM +0300, Serge Semin wrote: > On Tue, Sep 12, 2023 at 06:08:23PM +0100, Russell King (Oracle) wrote: > > On Tue, Sep 12, 2023 at 12:32:40PM +0300, Serge Semin wrote: > > > On Tue, Sep 12, 2023 at 07:59:49AM +0000, Jose Abreu wrote: > > > > From: Russell King (Oracle) <rmk+kernel@armlinux.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@armlinux.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? > > > > > Is stmmac_platform.{c,h} used by all the drivers that are making use of > > this? I'm not entirely sure. > > > > If it is, then yes, it can go in stmmac_platform.[ch]. If not, then I > > don't think we'd want the bloat of forcing all of stmmac_platform.[ch] > > onto drivers that only want to use this one function. > > With a few exceptions almost all the STMMAC/DW*MAC glue drivers use > the methods from the stmmac_platform.c module including the bits > touched by your patchset. AFAICS semantically both stmmac_platform.c > and stmmac_plat_lib.c look the same. They don't do anything on its own > but provide some common methods utilized by the glue drivers for some > platform-specific setups. So basically stmmac_platform.[ch] is already > a library of the common platform methods. There is no need in creating > another one. I'm not questioning whether it should be merged, I'm questioning whether all drivers that I'm touching make use of stmmac_platform.c, so your long winded answer was entirely unnecessary. All you needed to do was answer the question I asked, rather than teach me how to suck eggs. Thanks.
On Wed, Sep 13, 2023 at 03:14:47PM +0100, Russell King (Oracle) wrote: > On Wed, Sep 13, 2023 at 03:56:07AM +0300, Serge Semin wrote: > > On Tue, Sep 12, 2023 at 06:08:23PM +0100, Russell King (Oracle) wrote: > > > On Tue, Sep 12, 2023 at 12:32:40PM +0300, Serge Semin wrote: > > > > On Tue, Sep 12, 2023 at 07:59:49AM +0000, Jose Abreu wrote: > > > > > From: Russell King (Oracle) <rmk+kernel@armlinux.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@armlinux.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? > > > > > > > > Is stmmac_platform.{c,h} used by all the drivers that are making use of > > > this? I'm not entirely sure. > > > > > > If it is, then yes, it can go in stmmac_platform.[ch]. If not, then I > > > don't think we'd want the bloat of forcing all of stmmac_platform.[ch] > > > onto drivers that only want to use this one function. > > > > With a few exceptions almost all the STMMAC/DW*MAC glue drivers use > > the methods from the stmmac_platform.c module including the bits > > touched by your patchset. AFAICS semantically both stmmac_platform.c > > and stmmac_plat_lib.c look the same. They don't do anything on its own > > but provide some common methods utilized by the glue drivers for some > > platform-specific setups. So basically stmmac_platform.[ch] is already > > a library of the common platform methods. There is no need in creating > > another one. > > I'm not questioning whether it should be merged, I'm questioning whether > all drivers that I'm touching make use of stmmac_platform.c, so your > long winded answer was entirely unnecessary. All you needed to do was > answer the question I asked, rather than teach me how to suck eggs. So what about the name of the function? Are you happy that it's called "dwmac_set_tx_clk_gmii" rather than "stmmac_set_tx_clk_gmii" ?
On Wed, Sep 13, 2023 at 03:21:10PM +0100, Russell King (Oracle) wrote: > On Wed, Sep 13, 2023 at 03:14:47PM +0100, Russell King (Oracle) wrote: > > On Wed, Sep 13, 2023 at 03:56:07AM +0300, Serge Semin wrote: > > > On Tue, Sep 12, 2023 at 06:08:23PM +0100, Russell King (Oracle) wrote: > > > > On Tue, Sep 12, 2023 at 12:32:40PM +0300, Serge Semin wrote: > > > > > On Tue, Sep 12, 2023 at 07:59:49AM +0000, Jose Abreu wrote: > > > > > > From: Russell King (Oracle) <rmk+kernel@armlinux.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@armlinux.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? > > > > > > > > > > > Is stmmac_platform.{c,h} used by all the drivers that are making use of > > > > this? I'm not entirely sure. > > > > > > > > If it is, then yes, it can go in stmmac_platform.[ch]. If not, then I > > > > don't think we'd want the bloat of forcing all of stmmac_platform.[ch] > > > > onto drivers that only want to use this one function. > > > > > > With a few exceptions almost all the STMMAC/DW*MAC glue drivers use > > > the methods from the stmmac_platform.c module including the bits > > > touched by your patchset. AFAICS semantically both stmmac_platform.c > > > and stmmac_plat_lib.c look the same. They don't do anything on its own > > > but provide some common methods utilized by the glue drivers for some > > > platform-specific setups. So basically stmmac_platform.[ch] is already > > > a library of the common platform methods. There is no need in creating > > > another one. > > > > I'm not questioning whether it should be merged, I'm questioning whether > > all drivers that I'm touching make use of stmmac_platform.c, so your > > long winded answer was entirely unnecessary. All you needed to do was > > answer the question I asked, rather than teach me how to suck eggs. > > So what about the name of the function? Are you happy that it's called > "dwmac_set_tx_clk_gmii" rather than "stmmac_set_tx_clk_gmii" ? Not really. I would suggest to preserve the local naming convention: 1. Generic names have stmmac_ prefix. 2. DW *MAC IP-core-specific names have dw(xg|xlg)?mac(100|1000|2|4|5)?_ prefixes. Alas it was violated in some places (like norm_desc and enh_desc.c files) but is still mainly preserved in the driver especially in the stmmac_platform.c which is concerned in this case. -Serge(y) > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Thu, Sep 14, 2023 at 01:42:19PM +0300, Serge Semin wrote: > On Wed, Sep 13, 2023 at 03:21:10PM +0100, Russell King (Oracle) wrote: > > On Wed, Sep 13, 2023 at 03:14:47PM +0100, Russell King (Oracle) wrote: > > > On Wed, Sep 13, 2023 at 03:56:07AM +0300, Serge Semin wrote: > > > > On Tue, Sep 12, 2023 at 06:08:23PM +0100, Russell King (Oracle) wrote: > > > > > On Tue, Sep 12, 2023 at 12:32:40PM +0300, Serge Semin wrote: > > > > > > On Tue, Sep 12, 2023 at 07:59:49AM +0000, Jose Abreu wrote: > > > > > > > From: Russell King (Oracle) <rmk+kernel@armlinux.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@armlinux.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? > > > > > > > > > > > > > > Is stmmac_platform.{c,h} used by all the drivers that are making use of > > > > > this? I'm not entirely sure. > > > > > > > > > > If it is, then yes, it can go in stmmac_platform.[ch]. If not, then I > > > > > don't think we'd want the bloat of forcing all of stmmac_platform.[ch] > > > > > onto drivers that only want to use this one function. > > > > > > > > With a few exceptions almost all the STMMAC/DW*MAC glue drivers use > > > > the methods from the stmmac_platform.c module including the bits > > > > touched by your patchset. AFAICS semantically both stmmac_platform.c > > > > and stmmac_plat_lib.c look the same. They don't do anything on its own > > > > but provide some common methods utilized by the glue drivers for some > > > > platform-specific setups. So basically stmmac_platform.[ch] is already > > > > a library of the common platform methods. There is no need in creating > > > > another one. > > > > > > I'm not questioning whether it should be merged, I'm questioning whether > > > all drivers that I'm touching make use of stmmac_platform.c, so your > > > long winded answer was entirely unnecessary. All you needed to do was > > > answer the question I asked, rather than teach me how to suck eggs. > > > > > So what about the name of the function? Are you happy that it's called > > "dwmac_set_tx_clk_gmii" rather than "stmmac_set_tx_clk_gmii" ? > > Not really. I would suggest to preserve the local naming convention: > 1. Generic names have stmmac_ prefix. > 2. DW *MAC IP-core-specific names have dw(xg|xlg)?mac(100|1000|2|4|5)?_ prefixes. > Alas it was violated in some places (like norm_desc and enh_desc.c > files) but is still mainly preserved in the driver especially in the > stmmac_platform.c which is concerned in this case. Thanks... so now I have you down as a single-issue reviewer - you spot something, you comment on it, and that's as far as you go. You don't seem to bother continuing the review and raising other points - which leads to lots of wasted time, and lots of patch set iterations, lots of email on mailing lists, etc. Do you think you could review the other patches before I go to the trouble of spinning a v2 please? Thanks.
On Thu, Sep 14, 2023 at 11:48:10AM +0100, Russell King (Oracle) wrote: > On Thu, Sep 14, 2023 at 01:42:19PM +0300, Serge Semin wrote: > > On Wed, Sep 13, 2023 at 03:21:10PM +0100, Russell King (Oracle) wrote: > > > On Wed, Sep 13, 2023 at 03:14:47PM +0100, Russell King (Oracle) wrote: > > > > On Wed, Sep 13, 2023 at 03:56:07AM +0300, Serge Semin wrote: > > > > > On Tue, Sep 12, 2023 at 06:08:23PM +0100, Russell King (Oracle) wrote: > > > > > > On Tue, Sep 12, 2023 at 12:32:40PM +0300, Serge Semin wrote: > > > > > > > On Tue, Sep 12, 2023 at 07:59:49AM +0000, Jose Abreu wrote: > > > > > > > > From: Russell King (Oracle) <rmk+kernel@armlinux.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@armlinux.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? > > > > > > > > > > > > > > > > > Is stmmac_platform.{c,h} used by all the drivers that are making use of > > > > > > this? I'm not entirely sure. > > > > > > > > > > > > If it is, then yes, it can go in stmmac_platform.[ch]. If not, then I > > > > > > don't think we'd want the bloat of forcing all of stmmac_platform.[ch] > > > > > > onto drivers that only want to use this one function. > > > > > > > > > > With a few exceptions almost all the STMMAC/DW*MAC glue drivers use > > > > > the methods from the stmmac_platform.c module including the bits > > > > > touched by your patchset. AFAICS semantically both stmmac_platform.c > > > > > and stmmac_plat_lib.c look the same. They don't do anything on its own > > > > > but provide some common methods utilized by the glue drivers for some > > > > > platform-specific setups. So basically stmmac_platform.[ch] is already > > > > > a library of the common platform methods. There is no need in creating > > > > > another one. > > > > > > > > I'm not questioning whether it should be merged, I'm questioning whether > > > > all drivers that I'm touching make use of stmmac_platform.c, so your > > > > long winded answer was entirely unnecessary. All you needed to do was > > > > answer the question I asked, rather than teach me how to suck eggs. > > > > > > > > So what about the name of the function? Are you happy that it's called > > > "dwmac_set_tx_clk_gmii" rather than "stmmac_set_tx_clk_gmii" ? > > > > Not really. I would suggest to preserve the local naming convention: > > 1. Generic names have stmmac_ prefix. > > 2. DW *MAC IP-core-specific names have dw(xg|xlg)?mac(100|1000|2|4|5)?_ prefixes. > > Alas it was violated in some places (like norm_desc and enh_desc.c > > files) but is still mainly preserved in the driver especially in the > > stmmac_platform.c which is concerned in this case. > > Thanks... so now I have you down as a single-issue reviewer - you spot > something, you comment on it, and that's as far as you go. You don't > seem to bother continuing the review and raising other points - which > leads to lots of wasted time, and lots of patch set iterations, lots > of email on mailing lists, etc. > > Do you think you could review the other patches before I go to the > trouble of spinning a v2 please? Ok. One more note about this patch: > --- /dev/null > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_plat_lib.c > @@ -0,0 +1,29 @@ > +#include <linux/stmmac.h> > +#include <linux/clk.h> > + > +#include "stmmac_plat_lib.h" > + > +int dwmac_set_tx_clk_gmii(struct clk *tx_clk, int speed) > +{ > + unsigned long rate; > + > + switch (speed) { > + case SPEED_1000: > + rate = 125000000; > + break; > + It's not described in the kernel coding style, but normally the switch-case operations are defined with no additional line separating the cases (I guess it gets to be redundant due to the indentations visually separating the parts anyway). I would have dropped the empty lines here too especially seeing the stmmac core driver mainly follow that implicit convention. > + case SPEED_100: > + rate = 25000000; > + break; > + ditto > + case SPEED_10: > + rate = 2500000; > + break; > + ditto -Serge(y) > + default: > + return -ENOTSUPP; > + } > + > + return clk_set_rate(tx_clk, rate); > +} > +EXPORT_SYMBOL_GPL(dwmac_set_tx_clk_gmii); ... > > Thanks. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile index 5b57aee19267..ba2cbfa0c9d1 100644 --- a/drivers/net/ethernet/stmicro/stmmac/Makefile +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile @@ -6,7 +6,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o \ mmc_core.o stmmac_hwtstamp.o stmmac_ptp.o dwmac4_descs.o \ dwmac4_dma.o dwmac4_lib.o dwmac4_core.o dwmac5.o hwif.o \ stmmac_tc.o dwxgmac2_core.o dwxgmac2_dma.o dwxgmac2_descs.o \ - stmmac_xdp.o \ + stmmac_xdp.o stmmac_plat_lib.o \ $(stmmac-y) stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_plat_lib.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_plat_lib.c new file mode 100644 index 000000000000..abb9f512bb0e --- /dev/null +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_plat_lib.c @@ -0,0 +1,29 @@ +#include <linux/stmmac.h> +#include <linux/clk.h> + +#include "stmmac_plat_lib.h" + +int dwmac_set_tx_clk_gmii(struct clk *tx_clk, int speed) +{ + unsigned long rate; + + switch (speed) { + case SPEED_1000: + rate = 125000000; + break; + + case SPEED_100: + rate = 25000000; + break; + + case SPEED_10: + rate = 2500000; + break; + + default: + return -ENOTSUPP; + } + + return clk_set_rate(tx_clk, rate); +} +EXPORT_SYMBOL_GPL(dwmac_set_tx_clk_gmii); diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_plat_lib.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_plat_lib.h new file mode 100644 index 000000000000..926fdce379b3 --- /dev/null +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_plat_lib.h @@ -0,0 +1,8 @@ +#ifndef STMMAC_PLAT_LIB_H +#define STMMAC_PLAT_LIB_H + +struct clk; + +int dwmac_set_tx_clk_gmii(struct clk *tx_clk, int speed); + +#endif
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@armlinux.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 +++++ 3 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_plat_lib.c create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_plat_lib.h