On 14/11/2022 17:18, Johan Hovold wrote:
On Mon, Nov 14, 2022 at 03:07:41PM +0100, Krzysztof Kozlowski wrote:
On 14/11/2022 14:27, Johan Hovold wrote:
On Fri, Nov 11, 2022 at 04:17:29PM +0100, Krzysztof Kozlowski wrote:
On 11/11/2022 10:24, Johan Hovold wrote:
The current QMP USB3-DP PHY bindings are based on the original MSM8996
binding which provided multiple PHYs per IP block and these in turn were
described by child nodes.
+ "#clock-cells":
+ const: 1
+
+ clock-output-names:
+ items:
+ - const: usb3_pipe
+ - const: dp_link
+ - const: dp_vco_div
Why defining here fixed names? The purpose of this field is to actually
allow customizing these - at least in most cases. If these have to be
fixed, then driver should just instantiate these clocks with such names,
right?
I'm only using these names as documentation of the indexes. The driver
What do you mean by documentation of indexes? You require these specific
entries and do not allow anything else.
I'm using this property as documentation of the valid indexes that can
be used when referring to clocks provided by this device.
There are currently three and the mapping is described by the
'clock-output-names' property.
doesn't use these names, but that's a Linux-specific implementation
detail.
I noticed that several bindings leave the clock indexes unspecified, or
have header files defining some or all of them. I first added a QMP
header but that seemed like overkill, especially if we'd end up with
one header per SoC (cf. the GCC headers) due to (known and potential)
platform differences.
Headers for the names? I do not recall such but that does not seem right.
Headers for the indexes.
On the other hand reproducing this list in each node is admittedly a bit
redundant.
Shall I add back a shared header for all PHYs handled by this driver
(another implementation detail) even if this could eventually lead to
describing clocks not supported by a particular SoC (so such constraints
would still need to be described by the binding somehow):
/* QMP clocks */
#define QMP_USB3_PIPE_CLK 0
#define QMP_DP_LINK_CLK 1
#define QMP_DP_VCO_DIV_CLK 2
Maybe QMP_COMBO_USB3_PIPE_CLK, QMP_COMBO_DP_LINK_CLK,
QMP_COMBO_DP_VCO_DIV_CLK?
I'll then extend this header with QMP_UFS_RX_SYMBOL_0_CLK
QMP_UFS_RX_SYMBOL_1_CLK and QMP_UFS_TX_SYMBOL_0_CLK.
What are these about? To remind - we talk about names of clocks this
device creates. The output names. Whatever IDs you have are not related
to the names.
As I mentioned above, this is not about the names that Linux gives to
its representation of these clocks. Its just about defining the valid
indexes in the binding.
If you think that that using 'clock-output-names' for this is a bit too
unconventional, I can add back the header with defines like the above
instead.
Note that the clock schema has:
clock-output-names:
description: |
Recommended to be a list of strings of clock output signal
names indexed by the first cell in the clock specifier.
However, the meaning of clock-output-names is domain
specific to the clock provider, ...
Johan
--
With best wishes
Dmitry
