diff mbox

[v3,7/7] ARM: shmobile: kzm9g-reference: require ZB clock for Ethernet controller

Message ID 1414515736-24905-8-git-send-email-ulrich.hecht+renesas@gmail.com (mailing list archive)
State Changes Requested
Commit dcb8a0ad79ab14dae5aebd17ceebbde10c50e3dd
Headers show

Commit Message

Ulrich Hecht Oct. 28, 2014, 5:02 p.m. UTC
Unlike the legacy stuff, CCF disables the ethernet controller's clock
unless it is used somewhere.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 arch/arm/boot/dts/sh73a0-kzm9g-reference.dts | 1 +
 1 file changed, 1 insertion(+)

Comments

Simon Horman Oct. 29, 2014, 12:21 a.m. UTC | #1
On Tue, Oct 28, 2014 at 06:02:16PM +0100, Ulrich Hecht wrote:
> Unlike the legacy stuff, CCF disables the ethernet controller's clock
> unless it is used somewhere.

This feels like it should come earlier in the series to avoid
breaking then fixing ethernet support for kzm9g-reference.

> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  arch/arm/boot/dts/sh73a0-kzm9g-reference.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
> index bad6492..15782f5 100644
> --- a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
> +++ b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
> @@ -91,6 +91,7 @@
>  		interrupt-parent = <&irqpin0>;
>  		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
>  		reg-io-width = <4>;
> +		clocks = <&zb_clk>;
>  		smsc,irq-push-pull;
>  		smsc,save-mac-address;
>  		vddvario-supply = <&reg_1p8v>;
> -- 
> 1.8.4.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulrich Hecht Oct. 30, 2014, 5:41 p.m. UTC | #2
On Wed, Oct 29, 2014 at 1:21 AM, Simon Horman <horms@verge.net.au> wrote:
> On Tue, Oct 28, 2014 at 06:02:16PM +0100, Ulrich Hecht wrote:
>> Unlike the legacy stuff, CCF disables the ethernet controller's clock
>> unless it is used somewhere.
>
> This feels like it should come earlier in the series to avoid
> breaking then fixing ethernet support for kzm9g-reference.

The CCF support, not being used yet, doesn't actually break anything.
The reason this patch is separated from the rest is that it works, but
it's not completely clear (to me) how.

As Laurent has pointed out before, the LAN controller is not
(directly) hooked up to this clock, but to an external oscillator.
Nonetheless Ethernet won't work unless ZB is on, which I have
determined by trial and error. Maybe that should have been pointed out
in the description...

CU
Uli
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven Oct. 30, 2014, 6:03 p.m. UTC | #3
On Thu, Oct 30, 2014 at 6:41 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
> On Wed, Oct 29, 2014 at 1:21 AM, Simon Horman <horms@verge.net.au> wrote:
>> On Tue, Oct 28, 2014 at 06:02:16PM +0100, Ulrich Hecht wrote:
>>> Unlike the legacy stuff, CCF disables the ethernet controller's clock
>>> unless it is used somewhere.
>>
>> This feels like it should come earlier in the series to avoid
>> breaking then fixing ethernet support for kzm9g-reference.
>
> The CCF support, not being used yet, doesn't actually break anything.
> The reason this patch is separated from the rest is that it works, but
> it's not completely clear (to me) how.
>
> As Laurent has pointed out before, the LAN controller is not
> (directly) hooked up to this clock, but to an external oscillator.
> Nonetheless Ethernet won't work unless ZB is on, which I have
> determined by trial and error. Maybe that should have been pointed out
> in the description...

I don't have the schematics for kzm9g, but I guess the LAN controller
is hooked up to the BSC (Bus State Controller). According to the sh73a0
datasheet, ZBø is the clock for the BSC.

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
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulrich Hecht Oct. 31, 2014, 9:56 a.m. UTC | #4
On Thu, Oct 30, 2014 at 7:03 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> I don't have the schematics for kzm9g, but I guess the LAN controller
> is hooked up to the BSC (Bus State Controller). According to the sh73a0
> datasheet, ZBø is the clock for the BSC.

Yes, I remember something like that. (Please ignore my previous
ramblings, I was looking at the APE6EVM schematics...)

CU
Uli
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Horman Nov. 5, 2014, 12:36 a.m. UTC | #5
On Fri, Oct 31, 2014 at 10:56:39AM +0100, Ulrich Hecht wrote:
> On Thu, Oct 30, 2014 at 7:03 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > I don't have the schematics for kzm9g, but I guess the LAN controller
> > is hooked up to the BSC (Bus State Controller). According to the sh73a0
> > datasheet, ZBø is the clock for the BSC.
> 
> Yes, I remember something like that. (Please ignore my previous
> ramblings, I was looking at the APE6EVM schematics...)

Thanks, I have queued up this series.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Pinchart Nov. 5, 2014, 3:43 p.m. UTC | #6
Hi Geert,

On Thursday 30 October 2014 19:03:37 Geert Uytterhoeven wrote:
> On Thu, Oct 30, 2014 at 6:41 PM, Ulrich Hecht wrote:
> > On Wed, Oct 29, 2014 at 1:21 AM, Simon Horman <horms@verge.net.au> wrote:
> >> On Tue, Oct 28, 2014 at 06:02:16PM +0100, Ulrich Hecht wrote:
> >>> Unlike the legacy stuff, CCF disables the ethernet controller's clock
> >>> unless it is used somewhere.
> >> 
> >> This feels like it should come earlier in the series to avoid
> >> breaking then fixing ethernet support for kzm9g-reference.
> > 
> > The CCF support, not being used yet, doesn't actually break anything.
> > The reason this patch is separated from the rest is that it works, but
> > it's not completely clear (to me) how.
> > 
> > As Laurent has pointed out before, the LAN controller is not
> > (directly) hooked up to this clock, but to an external oscillator.
> > Nonetheless Ethernet won't work unless ZB is on, which I have
> > determined by trial and error. Maybe that should have been pointed out
> > in the description...
> 
> I don't have the schematics for kzm9g, but I guess the LAN controller
> is hooked up to the BSC (Bus State Controller).

Yes it is. We should probably model the BSC in DT, and add the LAN controller 
as a child of the BSC node. With automatic clock management in runtime PM, 
could we do without a BSC driver entirely ?

> According to the sh73a0 datasheet, ZBø is the clock for the BSC.
Geert Uytterhoeven Nov. 5, 2014, 9:06 p.m. UTC | #7
Hi Laurent,

On Wed, Nov 5, 2014 at 4:43 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> > As Laurent has pointed out before, the LAN controller is not
>> > (directly) hooked up to this clock, but to an external oscillator.
>> > Nonetheless Ethernet won't work unless ZB is on, which I have
>> > determined by trial and error. Maybe that should have been pointed out
>> > in the description...
>>
>> I don't have the schematics for kzm9g, but I guess the LAN controller
>> is hooked up to the BSC (Bus State Controller).
>
> Yes it is. We should probably model the BSC in DT, and add the LAN controller
> as a child of the BSC node. With automatic clock management in runtime PM,
> could we do without a BSC driver entirely ?

I'm afraid we're gonna need a bus driver to handle that correctly.
Especially when considering the BSC is in a PM domain (A4S).

Fortunately A4S is always powered when the CPU is talking to Ethernet,
as A4S contains the CPU cores, too.

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
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Pinchart Nov. 6, 2014, 6:20 p.m. UTC | #8
Hi Geert,

On Wednesday 05 November 2014 22:06:38 Geert Uytterhoeven wrote:
> On Wed, Nov 5, 2014 at 4:43 PM, Laurent Pinchart wrote:
> >> > As Laurent has pointed out before, the LAN controller is not
> >> > (directly) hooked up to this clock, but to an external oscillator.
> >> > Nonetheless Ethernet won't work unless ZB is on, which I have
> >> > determined by trial and error. Maybe that should have been pointed out
> >> > in the description...
> >> 
> >> I don't have the schematics for kzm9g, but I guess the LAN controller
> >> is hooked up to the BSC (Bus State Controller).
> > 
> > Yes it is. We should probably model the BSC in DT, and add the LAN
> > controller as a child of the BSC node. With automatic clock management in
> > runtime PM, could we do without a BSC driver entirely ?
> 
> I'm afraid we're gonna need a bus driver to handle that correctly.
> Especially when considering the BSC is in a PM domain (A4S).
> 
> Fortunately A4S is always powered when the CPU is talking to Ethernet,
> as A4S contains the CPU cores, too.

If the BSC DT node references the MSTP clock and the power domain, do you 
think it would be possible to let core code handle them without requiring a 
custom driver (obviously with extensions to core code) ?
Geert Uytterhoeven Nov. 10, 2014, 9:14 a.m. UTC | #9
Hi Laurent,

On Thu, Nov 6, 2014 at 7:20 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Wednesday 05 November 2014 22:06:38 Geert Uytterhoeven wrote:
>> On Wed, Nov 5, 2014 at 4:43 PM, Laurent Pinchart wrote:
>> >> > As Laurent has pointed out before, the LAN controller is not
>> >> > (directly) hooked up to this clock, but to an external oscillator.
>> >> > Nonetheless Ethernet won't work unless ZB is on, which I have
>> >> > determined by trial and error. Maybe that should have been pointed out
>> >> > in the description...
>> >>
>> >> I don't have the schematics for kzm9g, but I guess the LAN controller
>> >> is hooked up to the BSC (Bus State Controller).
>> >
>> > Yes it is. We should probably model the BSC in DT, and add the LAN
>> > controller as a child of the BSC node. With automatic clock management in
>> > runtime PM, could we do without a BSC driver entirely ?
>>
>> I'm afraid we're gonna need a bus driver to handle that correctly.
>> Especially when considering the BSC is in a PM domain (A4S).
>>
>> Fortunately A4S is always powered when the CPU is talking to Ethernet,
>> as A4S contains the CPU cores, too.
>
> If the BSC DT node references the MSTP clock and the power domain, do you
> think it would be possible to let core code handle them without requiring a
> custom driver (obviously with extensions to core code) ?

I'll have to dive a bit deeper to answer that question...

For now, adding the clocks to the Ethernet node works, as the
drivers/sh/pm_runtime.c hack works for all platform devices.
Once we'll have DT PM domains on sh73a0, it'll no longer work, as SoC clocks
will be managed for devices belonging to an SoC PM domain only.
You can still cheat by referencing the PM domain from the Ethernet node,
though ;-)

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
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven Nov. 21, 2014, 9:23 p.m. UTC | #10
Hi Laurent,

On Mon, Nov 10, 2014 at 10:14 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, Nov 6, 2014 at 7:20 PM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>> On Wednesday 05 November 2014 22:06:38 Geert Uytterhoeven wrote:
>>> On Wed, Nov 5, 2014 at 4:43 PM, Laurent Pinchart wrote:
>>> >> > As Laurent has pointed out before, the LAN controller is not
>>> >> > (directly) hooked up to this clock, but to an external oscillator.
>>> >> > Nonetheless Ethernet won't work unless ZB is on, which I have
>>> >> > determined by trial and error. Maybe that should have been pointed out
>>> >> > in the description...
>>> >>
>>> >> I don't have the schematics for kzm9g, but I guess the LAN controller
>>> >> is hooked up to the BSC (Bus State Controller).
>>> >
>>> > Yes it is. We should probably model the BSC in DT, and add the LAN
>>> > controller as a child of the BSC node. With automatic clock management in
>>> > runtime PM, could we do without a BSC driver entirely ?
>>>
>>> I'm afraid we're gonna need a bus driver to handle that correctly.
>>> Especially when considering the BSC is in a PM domain (A4S).
>>>
>>> Fortunately A4S is always powered when the CPU is talking to Ethernet,
>>> as A4S contains the CPU cores, too.
>>
>> If the BSC DT node references the MSTP clock and the power domain, do you
>> think it would be possible to let core code handle them without requiring a
>> custom driver (obviously with extensions to core code) ?
>
> I'll have to dive a bit deeper to answer that question...

I now believe it can work fine with a really minimal BSC driver, provided the
drivers for the devices hooked up on the bus also have runtime PM support.
I'm sorting out some rough edges, so please stay tuned...

As this minimal BSC driver isn't hardware-specific at all, we may be able to
shove this into core code...

> For now, adding the clocks to the Ethernet node works, as the
> drivers/sh/pm_runtime.c hack works for all platform devices.
> Once we'll have DT PM domains on sh73a0, it'll no longer work, as SoC clocks
> will be managed for devices belonging to an SoC PM domain only.

Please ignore my last comment. As the smsc911x driver manages the clock
explicitly (if it exists), it does not rely on clock management from
drivers/sh/pm_runtime.c or pm-rmobile.c.

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
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
index bad6492..15782f5 100644
--- a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
+++ b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
@@ -91,6 +91,7 @@ 
 		interrupt-parent = <&irqpin0>;
 		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
 		reg-io-width = <4>;
+		clocks = <&zb_clk>;
 		smsc,irq-push-pull;
 		smsc,save-mac-address;
 		vddvario-supply = <&reg_1p8v>;