All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@kernel.org>
To: ChunyanZhang <zhanglyra@163.com>
Cc: Chunyan Zhang <zhang.lyra@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
	Baolin Wang <baolin.wang7@gmail.com>,
	Orson Zhai <orsonzhai@gmail.com>,
	Chunyan Zhang <chunyan.zhang@unisoc.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re:Re: [PATCH v4 2/4] dt-bindings: mfd: sprd: Add bindings for ums512 global registers
Date: Thu, 14 Oct 2021 16:09:12 -0700	[thread overview]
Message-ID: <163425295208.1688384.11023187625793114662@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <71369ca3.307a.17c7de16354.Coremail.zhanglyra@163.com>

Quoting ChunyanZhang (2021-10-14 01:18:15)
> 
> Resend this since I forgot to switch to plain text mode :(
> 
> At 2021-10-14 14:38:42, "Stephen Boyd" <sboyd@kernel.org> wrote:
> >Quoting ChunyanZhang (2021-10-13 19:02:44)
> >> At 2021-10-14 09:51:25, "Stephen Boyd" <sboyd@kernel.org> wrote:
> >> >Quoting ChunyanZhang (2021-10-13 16:49:40)
> >> >> At 2021-10-14 06:04:32, "Stephen Boyd" <sboyd@kernel.org> wrote:
> >> >> >Quoting Chunyan Zhang (2021-09-22 23:41:35)
> >> >> >> diff --git a/Documentation/devicetree/bindings/mfd/sprd,ums512-glbreg.yaml b/Documentation/devicetree/bindings/mfd/sprd,ums512-glbreg.yaml
> >> >> >> +
> >> >> >> +examples:
> >> >> >> +  - |
> >> >> >> +    ap_apb_regs: syscon@71000000 {
> >> >> >> +      compatible = "sprd,ums512-glbregs", "syscon", "simple-mfd";
> >> >> >> +      reg = <0x71000000 0x3000>;
> >> >> >> +      #address-cells = <1>;
> >> >> >> +      #size-cells = <1>;
> >> >> >> +      ranges = <0 0x71000000 0x3000>;
> >> >> >> +
> >> >> >> +      clock-controller@0 {
> >> >> >> +        compatible = "sprd,ums512-apahb-gate";
> >> >> >
> >> >> >Why is this a subnode of a syscon and simple-mfd? Why not put the>clock-controller@71000000 directly onto the bus? Does making it a child
> >> >> >node help somehow?
> >> >> 
> >> >> These clocks are at the same register range with global registers. I originally put them directly onto the bus indeed when submitting the patches for SC9863A clocks last year, and it had a private property named 'sprd,syscon' which could provide regmap for these clocks.
> >> >> 
> >> >> Rob suggested [1] us to make them a child of the syscon, and would not need the private property 'sprd, syscon' then.
> >> >
> >> >Why do you need to use a syscon? Are the registers shared with some
> >> >other driver?
> >> 
> >> Yes, shared with more than one devices which basically are multimedia devices. You may noticed that these are all gate clocks which are in the global registers ranges and are used to controll the enable status of some devices or some part of devices.
> >> 
> >
> 
> >Where does the multimedia device address space start? I see 0x71000000
> 
> It doesn't mean that multimedia device address is in this space, multidedia devices have their own address space actually.

Ok got it.

> All the registers in this space (started from 0x71000000) are more like controll registers for device power, and they were designed as "global registers".
> Some gate clocks are also in global register space, so we make this kind of clocks a subnode of syscon, and clock driver can use regmap which mapped by syscon to avoid remapping the same address space to more than one virtual addresses.

So it looks like we're making the "power controller" device into a
syscon and then adding nodes in DT to describe the various pieces that
some registers inside that correspond to, like clks, power domains, etc.
I wonder why the ap_apb_regs device can't be a single node covering the
entire register space and then use the auxiliary bus in the kernel to
carve that device up into various kernel subsytems so that the
appropriate maintainers can review it.

> 
> Hope I've made that clear, I'd like to give you an example, but I'm on a long leave and cannot look up the specification right now :(
> This is not a new issue, we discussed this when the first time I submitted the patches for sprd clocks IIRC.

Yes, I know it's not a new issue. Take care! We can pick this discussion
back up when you get back from leave.

  reply	other threads:[~2021-10-14 23:09 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-23  6:41 [PATCH v4 0/4] Add Unisoc's UMS512 clock support Chunyan Zhang
2021-09-23  6:41 ` [PATCH v4 1/4] dt-bindings: clk: sprd: Add bindings for ums512 clock controller Chunyan Zhang
2021-09-27 19:48   ` Rob Herring
2021-10-13 14:22   ` Geert Uytterhoeven
2021-10-16  8:41     ` Chunyan Zhang
2021-10-17 12:29       ` Geert Uytterhoeven
2021-09-23  6:41 ` [PATCH v4 2/4] dt-bindings: mfd: sprd: Add bindings for ums512 global registers Chunyan Zhang
2021-10-06  8:00   ` Lee Jones
2021-10-11 14:11     ` Rob Herring
2021-10-12  8:14       ` Lee Jones
2021-10-13 14:23         ` Geert Uytterhoeven
2021-10-14  6:48           ` Lee Jones
2021-10-14 16:18             ` Rob Herring
2021-10-14 23:02               ` Stephen Boyd
2021-10-15 13:58                 ` Rob Herring
2021-10-15 17:09                   ` Geert Uytterhoeven
2021-10-15 22:03                     ` Stephen Boyd
2021-10-18 11:37                       ` Geert Uytterhoeven
2021-10-15 12:45               ` Lee Jones
2021-10-15 17:11                 ` Rob Herring
2021-10-18 11:53                   ` Lee Jones
2021-10-20 15:09       ` Rob Herring
2021-10-20 15:58         ` Lee Jones
2021-10-13 22:04   ` Stephen Boyd
2021-10-13 23:49     ` ChunyanZhang
2021-10-14  1:51       ` Stephen Boyd
2021-10-14  2:02         ` ChunyanZhang
2021-10-14  6:38           ` Stephen Boyd
2021-10-14  8:18             ` ChunyanZhang
2021-10-14 23:09               ` Stephen Boyd [this message]
2021-09-23  6:41 ` [PATCH v4 3/4] clk: sprd: Add dt-bindings include file for UMS512 Chunyan Zhang
2021-09-27 19:49   ` Rob Herring
2021-09-23  6:41 ` [PATCH v4 4/4] clk: sprd: Add Unisoc's UMS512 clock driver Chunyan Zhang
2021-10-12 12:09 ` Re:[PATCH v4 0/4] Add Unisoc's UMS512 clock support ChunyanZhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=163425295208.1688384.11023187625793114662@swboyd.mtv.corp.google.com \
    --to=sboyd@kernel.org \
    --cc=baolin.wang7@gmail.com \
    --cc=chunyan.zhang@unisoc.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=orsonzhai@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=zhang.lyra@gmail.com \
    --cc=zhanglyra@163.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.