Quoting Doug Anderson (2018-11-05 08:52:39) > On Sat, Nov 3, 2018 at 7:40 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > > + clocks = <&gcc GCC_USB3_SEC_PHY_PIPE_CLK>; > > > + clock-names = "pipe0"; > > > + clock-output-names = "usb3_uni_phy_pipe_clk_src"; > > > > If this has clock-output-names then I would expect to see a #clock-cells > > property, but that isn't here in this node. Are we relying on the same > > property in the parent node? > > If I had to guess, I believe it's yet more confusing than that. I > believe you actually point to the parent phandle if you want to use > the clock. I notice that the parent has #clock-cells as 1 so > presumably this is how you point to one child or the other? ...but I > don't think it's documented how this works. There are 'clock-ranges', that almost nobody uses. It might be usable for this purpose. > The lane nodes don't have > any sort of ID as far as I can tell. ...and in any case having > #clock-cells of 1 makes no sense for USB 3 PHYs which are supposed to > only have one child? > > Let's look at the code, maybe? Hrm, phy_pipe_clk_register() takes no > ID or anything. Huh? OK, so as far as I can tell > of_clk_add_provider() is never called on this clock... > > So I think the answer is that #clock-cells should be <0> and should > move to the child node to match with clock-output-names. Then I guess > (if anyone references this clock from the device tree rather than > relying on the global clock-output-names) we should add the > of_clk_add_provider() into the code? > > Maybe we can add that as a patch to the end of this series? There are > so many crazy / random things wrong with these bindings that it makes > sense to make smaller / incremental changes? > Sure that sounds fine. It would be another case where a driver would want to call the proposed devm_of_clk_add_parent_provider() API.
