[PATCH] arch/sh: fix Ethernet clock for SH7786

[PATCH] arch/sh: fix Ethernet clock for SH7786

[Thomas Petazzoni]

The sh_eth driver (used for the Ethernet controller on SH7786) uses
devm_clk_get(&pdev->dev, NULL) to get its clock. It means that con_id
= NULL, and therefore a clock lookup made on the con_id doesn't work.

In order to make sure that the sh_eth driver is able to find its clock
on SH7786, we switch from using a con_id to a dev_id in the clock
lookup table for this SoC.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/sh/kernel/cpu/sh4a/clock-sh7786.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7786.c b/arch/sh/kernel/cpu/sh4a/clock-sh7786.c
index ac3dcfe5d303..fa183280a62c 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-sh7786.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-sh7786.c
@@ -170,7 +170,7 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_CON_ID("dmac_11_6_fck", &mstp_clks[MSTP105]),
 	CLKDEV_CON_ID("dmac_5_0_fck", &mstp_clks[MSTP104]),
 	CLKDEV_CON_ID("du_fck", &mstp_clks[MSTP103]),
-	CLKDEV_CON_ID("ether_fck", &mstp_clks[MSTP102]),
+	CLKDEV_DEV_ID("sh7786-ether.0", &mstp_clks[MSTP102]),
 };
 
 int __init arch_clk_init(void)
-- 
2.13.6

Re: [PATCH] arch/sh: fix Ethernet clock for SH7786

[Geert Uytterhoeven]

Hi Thomas,

On Mon, Dec 4, 2017 at 3:30 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> The sh_eth driver (used for the Ethernet controller on SH7786) uses
> devm_clk_get(&pdev->dev, NULL) to get its clock. It means that con_id
> = NULL, and therefore a clock lookup made on the con_id doesn't work.
>
> In order to make sure that the sh_eth driver is able to find its clock
> on SH7786, we switch from using a con_id to a dev_id in the clock
> lookup table for this SoC.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

The clock is optional, right? Do you need it? It's "needed" for WoL only,
but that dependency is planned to be removed, cfr. "sh_eth: Remove obsolete
explicit clock handling for WoL".

Regardless:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] arch/sh: fix Ethernet clock for SH7786

[Thomas Petazzoni]

Hello,

On Mon, 4 Dec 2017 15:58:42 +0100, Geert Uytterhoeven wrote:

> The clock is optional, right? Do you need it? It's "needed" for WoL only,
> but that dependency is planned to be removed, cfr. "sh_eth: Remove obsolete
> explicit clock handling for WoL".

Hm, that's a good point what you raise here. Indeed looking at the
code, the clock isn't enabled unless you go in suspend() and WoL
support is enabled. I don't go into suspend, and I don't have WoL
enabled, but if I revert my patch, the kernel hangs at:

Generic PHY sh7786-ether-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=sh7786-ether-0:00, irq=POLL)
Waiting up to 110 more seconds for network.

(I booting over NFS, so the kernel tries to get an IP address over DHCP
at boot time)

With my patch applied:

Generic PHY sh7786-ether-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=sh7786-ether-0:00, irq=POLL)
sh-eth sh7786-ether.0 eth0: Link is Up - 100Mbps/Full - flow control rx/tx
Sending DHCP requests ., OK
IP-Config: Got DHCP answer from 192.168.0.254, my address is 192.168.0.202
IP-Config: Complete:
     device=eth0, hwaddrÔ:81:d7:45:df:5a, ipaddr\x192.168.0.202, mask%5.255.255.0, gw\x192.168.0.254
     host\x192.168.0.202, domain=lan, nis-domain=(none)
     bootserver\x192.168.0.254, rootserver\x192.168.0.11, rootpath=     nameserver0\x192.168.0.254
VFS: Mounted root (nfs filesystem) on device 0:13.
devtmpfs: mounted
Freeing unused kernel memory: 132K

So it seems like getting the reference to the clock has some
side-effect. I guess this needs more debugging :-)

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH] arch/sh: fix Ethernet clock for SH7786

[Geert Uytterhoeven]

Hi Thomas,

On Mon, Dec 4, 2017 at 4:23 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> On Mon, 4 Dec 2017 15:58:42 +0100, Geert Uytterhoeven wrote:
>> The clock is optional, right? Do you need it? It's "needed" for WoL only,
>> but that dependency is planned to be removed, cfr. "sh_eth: Remove obsolete
>> explicit clock handling for WoL".
>
> Hm, that's a good point what you raise here. Indeed looking at the
> code, the clock isn't enabled unless you go in suspend() and WoL
> support is enabled. I don't go into suspend, and I don't have WoL
> enabled, but if I revert my patch, the kernel hangs at:
>
> Generic PHY sh7786-ether-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=sh7786-ether-0:00, irq=POLL)
> Waiting up to 110 more seconds for network.
>
> (I booting over NFS, so the kernel tries to get an IP address over DHCP
> at boot time)
>
> With my patch applied:
>
> Generic PHY sh7786-ether-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=sh7786-ether-0:00, irq=POLL)
> sh-eth sh7786-ether.0 eth0: Link is Up - 100Mbps/Full - flow control rx/tx
> Sending DHCP requests ., OK
> IP-Config: Got DHCP answer from 192.168.0.254, my address is 192.168.0.202
> IP-Config: Complete:
>      device=eth0, hwaddrÔ:81:d7:45:df:5a, ipaddr\x192.168.0.202, mask%5.255.255.0, gw\x192.168.0.254
>      host\x192.168.0.202, domain=lan, nis-domain=(none)
>      bootserver\x192.168.0.254, rootserver\x192.168.0.11, rootpath=     nameserver0\x192.168.0.254
> VFS: Mounted root (nfs filesystem) on device 0:13.
> devtmpfs: mounted
> Freeing unused kernel memory: 132K
>
> So it seems like getting the reference to the clock has some
> side-effect. I guess this needs more debugging :-)

Ah, it makes sure the legacy clock domain code in drivers/sh/pm_runtime.c
finds the clock, and enables it when pm_runtime_get_sync() is called.

So yes, you do need the patch ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] arch/sh: fix Ethernet clock for SH7786

[Thomas Petazzoni]

Hello,

On Mon,  4 Dec 2017 15:30:49 +0100, Thomas Petazzoni wrote:
> The sh_eth driver (used for the Ethernet controller on SH7786) uses
> devm_clk_get(&pdev->dev, NULL) to get its clock. It means that con_id
> = NULL, and therefore a clock lookup made on the con_id doesn't work.
> 
> In order to make sure that the sh_eth driver is able to find its clock
> on SH7786, we switch from using a con_id to a dev_id in the clock
> lookup table for this SoC.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Is it possible to get this patch merged? It has a Reviewed-by from
Geert.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH] arch/sh: fix Ethernet clock for SH7786

[Thomas Petazzoni]

Hello,

On Mon,  4 Dec 2017 15:30:49 +0100, Thomas Petazzoni wrote:
> The sh_eth driver (used for the Ethernet controller on SH7786) uses
> devm_clk_get(&pdev->dev, NULL) to get its clock. It means that con_id
> = NULL, and therefore a clock lookup made on the con_id doesn't work.
> 
> In order to make sure that the sh_eth driver is able to find its clock
> on SH7786, we switch from using a con_id to a dev_id in the clock
> lookup table for this SoC.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Here as well: patch sent almost 3 months ago, it has a Reviewed-by from
Geert.

Is it possible to apply this patch ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com