Re: [PATCH v2 5/5] dt-bindings: net: ipq4019-mdio: Document ipq5332 platform

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/15/2023 9:41 PM, Conor Dooley wrote:
On Fri, Dec 15, 2023 at 08:40:20PM +0800, Jie Luo wrote:


On 12/15/2023 8:19 PM, Krzysztof Kozlowski wrote:
On 15/12/2023 12:42, Jie Luo wrote:
Which clocks are these mentioned in the property? From where do they come?

Anyway, property is in existing form is not correct - this is not a
generic property.


This property cmn-reference-clock is just the hardware register
configuration, since the different IPQ platform needs to select
the different reference clock source for the CMN PLL block that
provides the various clock outputs to the all kinds of Ethernet
devices, which is not from GCC provider.

AGAIN: where do the clocks come from? Which device generates them?

Oh, OK, the reference clock is from wifi that provides 48MHZ to
Ethernet block.

Then WiFi should be providing you the clock and this device should be
clock consumer, right?

Yes, wifi provides 48MHz clock to CMM PLL block, there is no GCC
for this 48MHZ clock output, it is the hardware PIN connection.

All clocks are some hardware pin connections.

Best regards,
Krzysztof


Yes, all reference clocks here are from hardware pin connection.

You keep answering with short sentences without touching the root of the
problem. I don't know exactly why, but I feel this discussion leads
nowhere. After long discussion you finally admitted that clocks came
from another device - Wifi. It took us like 6 emails?

So last statement: if you have clock provider and clock consumer, you
must represent it in the bindings or provide rationale why it should not
or must not be represented in the bindings. So far I do not see any of
such arguments.

If you use arguments like:
"My driver....": sorry, bindings are not about drivers
"I don't have clock driver for WiFi": sorry, it does not matter if you
can write one, right?

Please reach internally your colleagues to solve these problems and make
review process smoother.

These reference clocks source do not need the hardware configuration,
that is the reason why the clock provider is not needed, some reference
clock source are even from external crystal.

I fail to understand how that makes this clock different to the clocks
on any other platform. Clocks from external crystals are present in many
many systems. See for example fixed-clock.yaml.

Hi Conor,
The reference clock rate has no meaning to the CMN PLL block, since the
software can't control the behavior of CMN PLL, and various output
clocks of CMN PLL block are fixed, adding this custom property is just
for selecting the different reference clock source, since different
IPQ platform needs to be configured the different reference clock source
for the CMN PLL block.

let's say if we register 48MHZ reference clock as the fix clock, we
can't distinguish it is internal 48MHZ or external 48MHZ, for these
two reference clock sources, there are different hardware configuration
of CMN PLL block, and this reference clock selection is not applicable
for the IPQ4019 platform.


There is also no enable control for the reference clocks since it is
inputted by the hardware PIN connection, i will update these description
in the DT to make it more clear.

Again, this does not justify having custom properties for this clock,
as it is no different to other platforms. As far as I can tell, the only
thing that a standard "clocks" property cannot convey here is the
internal reference. I would suggest that since there is only one
internal clock frequency, the absence of this particular clock in the
"clocks" property can be used to determine that the reference is the
internal one.

Thanks,
Conor.

Yes, we can get the clock rate of the clocks property if we register
these as the fix clock to distinguish the different clock source.

Since the reference clock rate value has no matter with the CMN clock
configuration, it is just the reference clock source selection, so
i did not use the fix clock for this.

Thanks for this suggestion, i will verify the fix clock register solution.






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux