Thread (16 messages) 16 messages, 5 authors, 2016-02-23

[PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

From: Alexandre Torgue <hidden>
Date: 2016-02-23 13:17:24
Also in: lkml, netdev

2016-02-23 12:21 GMT+01:00 Joachim  Eastwood [off-list ref]:
On 23 February 2016 at 10:59, Alexandre Torgue
[off-list ref] wrote:
quoted
2016-02-22 22:52 GMT+01:00 Joachim  Eastwood [off-list ref]:
quoted
On 22 February 2016 at 15:50, Alexandre Torgue
[off-list ref] wrote:
quoted
2016-02-13 14:48 GMT+01:00 Joachim  Eastwood [off-list ref]:
quoted
On 3 February 2016 at 15:54, Alexandre TORGUE
[off-list ref] wrote:
quoted
+       plat_dat->bsp_priv = dwmac;
+       plat_dat->init = stm32_dwmac_init;
+       plat_dat->exit = stm32_dwmac_exit;
Instead of using these callbacks could you rather implement the PM
callbacks directly in this driver?
I don't think it should add much code and it will make it look more
like standard driver. This will also give you some more control and
flexibility in your code.
I prefer to keep the code as it is. Glue layer is directly linked to
stmmac driver and I don't want to brake the link between the glue and
the stmmac driver.
What do you mean by break the link?
I thought that you wanted to split stmmac_pltfr_supend (glue part and
stmamc part), but I well understood it is not the case (sorry for
mistake).
quoted
There has been numerous of patch sets to make the stmmac "glue"
drivers into more standard platform drivers.
http://marc.info/?l=linux-netdev&m=143159850631093&w=2
http://marc.info/?l=linux-netdev&m=143708560009851&w=2
http://marc.info/?l=linux-netdev&m=143812136600541&w=2

Do you see any advantage by using the init and exit hooks in your
driver instead of using the standard driver PM callbacks and remove
function?
The only "cost" I see is slightly more boilerplate code. But since you
already have init/exit functions you could easily make them into PM
resume/suspend so I doubt there would be much increase in code size.
If I well understood you want to continue the stmmac glue driver
rework by moving stmmac_pltfr_suspend/resume/remove in each glue
driver (stm32, sun, sti ....).
At least I want to avoid the init/exit callbacks for new drivers like
stm32-dwmac.

quoted
Each glue driver will call directly stmmac_suspend/resume/remove and
their own init/exit function.
If it is what you meant, I can do it.
Yes, in your stm32 driver's suspend/resume/remove functions call
stmmac_suspend/stmmac_resume/stmmac_dvr_remove directly. Then you
shouldn't need to use the init/exit callbacks. Just put the need code
in the driver's suspend/resume/remove functions instead of init/exit
functions.

For example:
static int stm32_dwmac_resume(struct device *dev)
{
    struct net_device *ndev = dev_get_drvdata(dev);
    struct plat_stmmacenet_data *plat_dat = get_stmmac_plat_data(ndev)
    struct stm32_dwmac *dwmac =plat_dat->bsp_priv;

    /* enable clocks */
    /* set phy mode */

    return stmmac_resume(ndev);
}

If it makes sense to have the enable clk/phy mode stuff in it's own
function that is fine too.
Ok. I will send v2 with this approach. After this series I could
provide a new series to change all glues with this approach ? (but I
will not able to test on each platform).

Best regards

Alexandre
quoted
quoted
One other thing;
Do you need to have the PHY mode setup code in the init function which
is called each time on resume?
I can't guarantee that after a suspend the sysconfig register will
contain same data than before suspend.
I see.


regards,
Joachim Eastwood
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help