On Sat, Feb 13, 2021 at 10:18:14AM +0900, Daniel Palmer wrote:

Hi Stephen,

On Sat, 13 Feb 2021 at 09:11, Stephen Boyd [off-list ref] wrote:

quoted

quoted

+examples:
+  - |
+    clkgen_mux_mspi0: clkgen_mux_mspi0 {
+      compatible = "mstar,msc313-clkgen-mux";
+      regmap = <&clkgen>;
+      offset = <0xcc>;
+      #clock-cells = <1>;
+      mstar,gate = <0>;
+      mstar,mux-shift = <2>;
+      mstar,mux-width = <2>;

It looks like a node-per clk sort of binding which has been rejected
multiple times in the past. If the clks are spread across various
devices then it sounds like the mediatek design where they have many
syscon nodes that also register clks inside those register spaces. In
this case, I would expect the clkgen node to be registering clks. Given
that there isn't a reg property and there's these mstar specific
properties like shift/width it looks really wrong. Please don't do this.

Ok. I will rethink this. One of the problems I face here is that there
isn't any documentation for what the clkgen looks like.

All the more reason to not do a node per clock.

I have a list of offsets and bit positions for these muxes but very little else.
Looking at the mediatek clock drivers it seems like they have a driver
that consumes some register areas and then creates all of the muxes
etc within those areas within the driver instead. If that's an
acceptable solution I will go for that.
There would probably be 2 compatible strings right now (one for the pm
area and one for the normal area) and that would take a phandle to the
syscon that holds the registers. Then there would be a big table of
the offsets, masks etc in the driver.

Ideally, the 'syscon' is just the clock provider or a child node is.

Rob