Re: [PATCH v4 2/4] dt-bindings: soc: spacemit: Add spacemit,k1-syscon

[Alex Elder <elder@riscstar.com>]

I have a general proposal on how to represent this, but I'd
like to know whether it makes sense.  It might be what Krzysztof
is suggesting, but in any case, I hope this representation would
work, because it could simplify the code, and compartmentalizes
things.

Part of what motivates this is that I've been looking at the
downstream reset code this week.  It contains a large number of
register offset definitions identical to what's used for the
clock driver.  The reset driver uses exactly the same registers
as the clock driver does.  Downstream they are separate drivers,
but the clock driver exports a shared spinlock for both drivers
to use.

These really need to be incorporated into the same driver for
upstream.

The clock code defines four distinct "units" (a term I'll use
from here on; there might be a better name):
   MPMU  Main Power Management Unit
   APMU  Application Power Management Unit
   APBC  APB Clock
   APBS  APB Spare

The reset code defines some of those, but doesn't use APBS.
It also defines three more:
   APBC2 Another APB Clock
   RCPU  Real-time CPU?
   RCPU2 Another Real-time CPU

Each of these "units" has a distinct I/O memory region that
contains registers that manage the clocks and reset signals.

I suggest a single "k1-clocks" device be created, which has
access to all of the I/O address ranges.  And then within
the DT node for that device there is a sub-node for the
clocks, and another sub-node for the resets.  Each of these
uses 2 cells for addressing.  The first indicates which
"unit" contains the clock or reset registers, and the
second indicates the clock or reset line implemented by
that unit.

I might not have this exactly right, but below I show some
DTS code that I hope demonstrates what I mean.

Could this approach, or something close, work?

					-Alex

/* SpacemiT clock/reset unit numbers */
#define SPACEMIT_CRST_RCPU      0
#define SPACEMIT_CRST_RCPU2     1
#define SPACEMIT_CRST_APBC      2
         /* . . . */
#define SPACEMIT_CRST_APBC2     6

/* SpacemiT RCPU unit reset IDs */
#define SPACEMIT_RCPU_HDMIAUDIO 0
#define SPACEMIT_RCPU_CAN       1
         /* . . . */
#define SPACEMIT_RCPU_UART1     6

/* SpacemiT RCPU2 unit reset IDs */
#define SPACEMIT_RCPU2_PWM0     0
#define SPACEMIT_RCPU2_PWM1     1
         /* . . . */
#define SPACEMIT_RCPU2_PWM9     9

/* SpacemiT APBC unit reset/clock IDs */
#define SPACEMIT_APBC_UART1     0
#define SPACEMIT_APBC_UART2     1
#define SPACEMIT_APBC_GPIO      2
         /* . . . */
#define SPACEMIT_APBC_TWSI0     27

/* APBC reset/clock IDs */
#define SPACEMIT_APBC_UART1     0
#define SPACEMIT_APBC_UART2     1
#define SPACEMIT_APBC_GPIO      2
/* . . . */

     clocks {
         compatible = "spacemit,k1-clocks";

         reg = <0x0 0xc0880000 0x0 0x2050>,
               <0x0 0xc0888000 0x0 0x30>,
               <0x0 0xd4015000 0x0 0x1000>,
               <0x0 0xd4050000 0x0 0x209c>,
               <0x0 0xd4090000 0x0 0x1000>,
               <0x0 0xd4282800 0x0 0x400>,
               <0x0 0xf0610000 0x0 0x20>;
         reg-names = "rcpu",
                     "rcpu2",
                     "apbc",
                     "mpmu",
                     "apbs",
                     "apmu",
                     "apbc2";

         /*
          * The two reset cells indicate:
          *     unit number (e.g. SPACEMIT_CRST_RCPU)
          *     reset within that unit (e.g. SPACEMIT_RCPU_CAN)
          */
         k1_reset: spacemit-k1-reset {
             #reset-cells = <2>;
         };

         /*
          * The two clock cells indicate:
          *     unit number (e.g. SPACEMIT_CRST_APBC)
          *     clock within that unit (e.g. SPACEMIT_APBC_UART1)
          */
         k1_clock: spacemit-k1-clock {
             #clock-cells = <2>;
         };
         /* . . . */
     };

     /* . . . */

     uart8: serial@d4017700 {
         compatible = "spacemit,k1-uart",
                      "intel,xscale-uart";
         reg = <0x0 0xd4017700 0x0 0x100>;
         interrupts = <50>;
         clocks = <&k1_clock SPACEMIT_CRST_APBC SPACEMIT_APBC_UART8>;
                  <&k1_clock SPACEMIT_CRST_APBC SPACEMIT_APBC_UART8_BUS>;
         clock-names = "core",
                       "bus";
         reg-shift = <2>;
         reg-io-width = <4>;

         resets = <&k1_reset SPACEMIT_CRST_APBC SPACEMIT_APBC_UART8>;

         status = "disabled";
     };