-----Original Message----- From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@xxxxxxxxxxxxx] Sent: Thursday, March 17, 2022 3:37 AM To: Hawkins, Nick <nick.hawkins@xxxxxxx>; Verdun, Jean-Marie <verdun@xxxxxxx> Cc: Arnd Bergmann <arnd@xxxxxxxx>; Olof Johansson <olof@xxxxxxxxx>; soc@xxxxxxxxxx; Rob Herring <robh+dt@xxxxxxxxxx>; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx Subject: Re: [PATCH v3 09/10] arch: arm: boot: dts: Introduce HPE GXP Device tree On 16/03/2022 21:10, Hawkins, Nick wrote: (...) >>>>>> I think one of previous comments was that you cannot have "generic-ehci" >>>>>> only, right? >>> >>>> Yes there was, I removed the usb0: ehci@cefe0000. I see now that this is in reference to the compatible. This is our ehci controller. What would be a more appropriate compatible? Do we need hpe,gxp-ehci perhaps? >> >>> Yes,, see other cases in generic-ehci.yaml bindings. Your current choice would be pointed out by dtbs_check, that it's invalid according to current bindings. >> >> For some reason when I compile I am not seeing a warning for that file. I have been using "make dtbs_check" and "make dtbs W=1". Perhaps I am missing an important flag? > My bad, I misread the generic-ehci binding, so your compatible is actually correct from bindings point of view. Still common practice is to add own compatible which allows later customization. >> In the case of creating a hpe,gxp-ehci binding would I need to add that to the generic-ehci.yaml? > Yes, add your compatible to that big enum with list of many implementations. Done. > (...) >>>>>>>> + >>>>>>>> + memclk: memclk { >>>>>>>> + compatible = "fixed-clock"; >>>>>>>> + #clock-cells = <0>>>>; >>>>>>>> + clock-output-names = "memclk"; >>>>>>>> + clock-frequency = <800000000>>>>; >>>>>>>> + }; >>> >>>>>> What are these clocks? If external to the SoC, then where are they? On the board? >>> >>>> This was the internal iopclk and memclk they were both internal to the chip. >>>> For now I am removing osc and memclk and will just have an iopclk that Gxp-timer will refer to. >> >>> You should rather have a clock controller driver which defines this (and many others). They can stay as a temporary work-around, if you really need them for some other nodes. >> >> I am trying to picture what you are saying but I am unsure, I know that on a separate review you mentioned that the gxp-timer needed to have clocks, and clock-names inside the node. Would it be improper for the gxp-timer to reference iopclk? > It all depends on the architecture of your SoC. I could imagine such one: > 1. Few external (from SoC point of view) oscillators, usually provided on the board. It could be 24 MHz, could be 32767 Hz for Bluetooth etc. > 2. One or several clock controllers inside the SoC which take as input these external clocks. The clock controller provides clocks for several other components/blocks. Allows also gating clocks, reparenting and changing dividers (rate). > 3. Other blocks within your SoC, e.g. gxp-timer, use these clocks. > The true question is where is that memclk or iopclk generated? Is it controllable (gate/mux/rate)? Even some fixed-frequency clocks, coming out of that clock controller (point 2.), are defined in the clock controller because that's the logical place for >them.
