On Tue, Dec 14, 2021 at 2:08 PM Martin Povišer <povik@xxxxxxxxxxxxxx> wrote:
>
> Hi Rob,
>
> > On 14. 12. 2021, at 16:40, Rob Herring <robh@xxxxxxxxxx> wrote:
> >
> > On Tue, Dec 14, 2021 at 12:02:48PM +0000, Martin Povišer wrote:
> >> The NCO block found on Apple SoCs is a programmable clock generator
> >> performing fractional division of a high frequency input clock.
> >>
> >> Signed-off-by: Martin Povišer <povik@xxxxxxxxxxxxxx>
> >> ---
> >> .../devicetree/bindings/clock/apple,nco.yaml | 70 +++++++++++++++++++
> >> 1 file changed, 70 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/clock/apple,nco.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/apple,nco.yaml b/Documentation/devicetree/bindings/clock/apple,nco.yaml
> >> new file mode 100644
> >> index 000000000000..5029824ab179
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/clock/apple,nco.yaml
> >> @@ -0,0 +1,70 @@
>
> >> +
> >> + apple,nchannels:
> >> + $ref: /schemas/types.yaml#/definitions/uint32
> >> + description:
> >> + The number of output channels the NCO block has been
> >> + synthesized for.
> >
> > I'd assume there is some max number?
>
> There might be some limit to the underlying IP but we wouldn’t know.
> What we know about the hardware comes from blackbox reversing, and that
> doesn't suggest a particular limit to the number of channels we might
> see on the SoC block in future.
All the more reason to not put the size in the DT, but imply from the
compatible. Unless it varies by instance...
Though I guess you would need DT updates anyways to use the new clock.
> > Do you really need to know this? If this is just to validate the clock
> > cell values are less than this, then just drop that and the property.
> > It's not the kernel's job to validate the DT.
>
> Well strictly speaking the driver could do clock registration on-demand
> at the cost of additional book-keeping, in which case we could drop
> the property, but I would prefer we don’t do that. Rather than providing
> validation the property simplifies drivers.
>
> Another option is calculating the no. of channels from size of the reg
> range, but I assume that’s worse than having the nchannels property.
>
> >> +
> >> + nco: clock-generator@23b044000 {
> >
> > clock-controller@...
>
> Okay, will change.
>
> >
> >> + compatible = "apple,t8103-nco", "apple,nco";
> >> + reg = <0x3b044000 0x14000>;
> >
> > You really have 0x14000 worth of registers here because all of that
> > will be mapped into virtual memory? Doesn't matter so much on 64-bit,
> > but it did for 32-bit.
>
> There is about 5 registers per channel with 0x4000 stride between them,
> blame Apple (or Samsung? I don’t know...).
I would think you could walk the 0x4000 until you hit registers that
behave differently.
The register size / 0x4000 gives you the number of channels, too.
Another question, how do you know this is 1 block with N channels vs.
N blocks just happening to be next to each other in the memory map?
Rob
