On 2022-03-02 16:36, Chris Morgan wrote:
On Fri, Feb 25, 2022 at 10:44:55AM +0000, Robin Murphy wrote:
On 2022-02-24 19:30, Rob Herring wrote:
[...]
+required:
+ - compatible
+ - reg
+ - interrupts
+ - "#clock-cells"
Is this actually required (ditto elsewhere)? Technically it's only necessary
if there are any clock consumers targeting this node, so arguably it should
be the clock binding's responsibility to validate that.
It wouldn't make much sense for a dedicated clock controller to omit
#clock-cells such that it couldn't have any consumers, but given that these
things are primarily PMICs I think it's reasonable to allow a board not to
care about the clocks at all if it doesn't use them. I know that the
original binding claimed it was required, but if we're already relaxing that
for RK805 here then we may as well relax it entirely.
Fair enough. However, if the consumer could be in an overlay, then I
think we want it to be required and not make the overlay add the
property. Properties just appearing within nodes at runtime is likely
not well supported in OSs.
Ah yes, that's an angle I hadn't considered, and I reckon it clearly answers
my original question in the affirmative :)
Indeed these clock outputs are often hooked up to SDIO WiFi modules, and I'm
sure I *have* seen boards which put such modules on pluggable daughterboards
in a manner which could reasonably use overlays, so in principle it does
seem like a realistic concern. I'm happy with setting a general principle
that if a clock output is exposed on a physical pin, then at the DTS level
we can't know for sure that it *won't* be consumed (even if the original
board design didn't intend it), therefore the device is always a potential
clock controller and "#clock-cells" should be required. In that case, the
consistency argument would fall the other way, to enforcing it for RK805 as
well.
Okay. So the existing point of contentions are:
1) "#clock-cells" should always be required. This causes a few boards
to fail to check properly, but I assume that can be easily remedied by
adding the "#clock-cells" to the devicetree.
2) The rk805, rk809, and rk817 only have a single clock-out. To
workaround a quirk in the driver some boards have 2 clock-output-names.
To fix the devicetree to accurately describe the hardware, the driver
will have to be updated along with many boards with these PMICs.
3) The rk808 has no vcc13 or vcc14 input, but at least 4 boards preport
to use such a voltage input anyway.
Not a point of contention, but I need to add examples for the rk805,
rk809, and rk818 which I will just pull from a popular devicetree.
I can solve the clock-cells issue by simply adding that to the correct
devicetrees (though I have no devices to test those on I assume they
should be benign changes?). Is that acceptable to fix that?
For the single clock out, I can't really fix it without updating the
driver and modifying a large number of devicetrees. Should I just make
it 1 min/2 max across all these YAML files and note for the rk805,
rk809, and rk817 that there is only really one clock output?
What should I do for #3? I've checked the schematic for the Pinebook
Pro (which is one of the 4 boards affected) and can confirm that
VCC13 and VCC14 on these boards is literally just VCC1 and VCC2,
respectively. I can't seem to find the schematics for the other 3
boards affected though, but I assume it's something similar.
Let me know, I'd like to get this finalized so I can get the battery
code for the rk817 charger pushed too.
In my opinion:
1) Yup, just fix the DTs. We're only adding a property where it would
have to be anyway were there any consumers, so there's no risk of
breakage, and the overlay support argument is a good one.
2) Make "clock-output-names" 2 items when "#clock-cells" = 1, or 1 item
when "#clock-cells" = 0; allow the latter case for at least RK809 where
it's in use already. Fixing the driver and DTs to converge all the
single-clock-output chips consistently on the latter form can go on a
to-do list somewhere.
3) Just fix the DTs. The properties aren't used, so they won't be
missed. (FWIW it looks like those spurious names on RK808 pins 23 and 44
probably originated from the original RK3399 reference design schematic)
How does that sound?
Thank you very much for all your help.
And thank you for stepping up and persevering with such an annoyingly
awkward binding conversion!
Robin.
