Hi Geert, Shimoda-san,
First, thank you very much for your comments. I will refactor the
patch and send a v2 patchset as soon as possible.
To my understanding, although the RCAR Gen3 HW manual references the
clk frequencies (50MHz for XTAL and 100MHz for external clock) the
actual register description only allows choosing between between an
on-chip clock and an external reference signal (no clock rate /
divisor is specified in the PHY registers ).
In my opinion, the "use-on-chip-clk" parameter was meant to
differentiate between the internal/external references, and not to by
a specific clock rate. My only concern here is that the device tree
description of the hardware might be a little difficult to understand
when compared to the actual description from SOC's HW manual.
Many thanks for your support,
Petre
On Thu, May 18, 2017 at 3:42 PM, Geert Uytterhoeven
<geert@xxxxxxxxxxxxxx> wrote:
> Hi Shimoda-san,
>
> On Thu, May 18, 2017 at 2:19 PM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
>>> From: Geert Uytterhoeven
>>> Sent: Thursday, May 18, 2017 8:41 PM
>>> On Thu, May 18, 2017 at 1:13 PM, Yoshihiro Shimoda
>>> <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
>>> >> >> +- reg: offset and length of the partial USB 3.0 Host PHY register block.
>>> >> >> +- #phy-cells: see phy-bindings.txt in the same directory, must be <0>.
>>> >> >
>>> >> > I think we should add "clocks" property as required.
>>> >> >
>>> >> >> +Optional properties:
>>> >> >
>>> >> > You should add "renesas,use-on-chip-clk" here.
>>> >> > And, the name of "use-on-chip-clk" is not good to me.
>>> >> > FYI, my developing patch names "renesas,usb-extal".
>>
>> I'm sorry for this. I missed description "on-chip clock source is supplied though
>> USB_EXTAL/USB_XTAL" in the manual. So, this "renesas,use-on-chip-clk" is reasonable.
>>
>>> >> Can this be decided at runtime, e.g. by looking at the rates of the clocks
>>> >> to see which one is available/best suited?
>>> >
>>> > According to the HW manual, this module cannot see which one is available/best suited.
>>> > So, I don't think this can be decided at runtime.
>>>
>>> I mean, can't Linux look at the rates of the two clocks, and if one is zero,
>>> use the other?
>>
>> I still misunderstand your question though, but software cannot look at the rates
>> of USB3S0_CLK_P/USB3S0_CLK_M and USB_EXTAL/XTAL because the HW doesn't have registers
>> for indicating the rates.
>
> If we have "clocks" properties, as you suggested, both clocks will have to
> be described in DT. Hence Linux will create clock objects for them, and the
> USB PHY driver can call clk_get_rate() on those objects.
>
> If no clock is connected to USB3S0_CLK_P/USB3S0_CLK_M, a dummy clock
> with zero rate should be described in DT.
>
> Cfr. the existing clocks with "clock-frequency = <0>" in r8a7795.dtsi.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
>
> 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
