linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Masahiro Yamada <yamada.masahiro@socionext.com>
To: Lee Jones <lee.jones@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org
Cc: linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Felipe Balbi <felipe.balbi@linux.intel.com>,
	linux-usb@vger.kernel.org,
	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>,
	Masami Hiramatsu <masami.hiramatsu@linaro.org>,
	Jassi Brar <jaswinder.singh@linaro.org>
Subject: [Question] MFD driver that handles clocks/resets and populates child nodes
Date: Mon, 2 Apr 2018 16:17:58 +0900	[thread overview]
Message-ID: <CAK7LNARFWDMgmWsCjUmVrm4ZeYOcKCbizijxmDXob618ABjNWg@mail.gmail.com> (raw)

Hi.


Some MFD drivers populate child nodes
after some basic hardware setups.


My question is, is it acceptable to upstream
a MFD driver that handles only clocks/resets,
then calls of_platform_populate.


The probe function will look like follows:


static int uniphier_usb3_glue_probe(struct platform_device *pdev)
{
        [get some clocks];

        [get some resets];

        [enable some clocks];

        [deassert some resets];

        return devm_of_platform_populate(dev);
}


With this driver, the child devices do not need
to handle clocks/resets.
But, I am not sure if it is the right thing to do.



Background of this question
---------------------------

Socionext is trying to upstream
the glue layer for DWC3 USB IP.

The original patch is this.
https://patchwork.kernel.org/patch/10180167/

This driver enables clocks, deassert resets,
and sets up some more registers,
then populates the DWC3 core node.

The maintainer of DWC3, Felipe Balbi, requested to
split the glue layer driver into small parts such as
reset, regulator, phy, etc.

So, the node will look like follows:

   usb-glue {
           reset {
                   ...
           };

           regulator {
                   ...
           };

           hs-phy {
                   ...
           };

           ss-phy {
                   ...
           };
   }

Here, one question popped up.
Where should the hardware resources be described?
The register, clocks, resets are shared among the sub-nodes.
This is the obvious result since it is a single hardware block
in the first place, and we are splitting it into small chunks.


If the MFD driver approach is acceptable,
clocks, resets will be handles in the parent node.
(Such a driver looks stupid, though)


   usb-glue {
           compatible = "socionext,uniphier-ld20-usb3-glue",
                        "syscon";
           reg = <0x65b00000 0x1000>;
           clocks = <sys_clk 14>;
           resets = <sys_clk 14>;

           reset {
                   ...
           };

           regulator {
                   ...
           };

           hs-phy {
                   ...
           };

           ss-phy {
                   ...
           };
   }


Of course, it is possible to use "simple-mfd"
and each driver can repeat the same clock/reset like follows.
(this also looks a bit clumsy...)

   usb-glue {
           compatible = "socionext,uniphier-ld20-usb3-glue",
                        "simple-mfd", "syscon";
           reg = <0x65b00000 0x1000>;

           reset {
                   clocks = <sys_clk 14>;
                   resets = <sys_clk 14>;
                   ...
           };

           regulator {
                   clocks = <sys_clk 14>;
                   resets = <sys_clk 14>;
                   ...
           };

           hs-phy {
                   clocks = <sys_clk 14>;
                   resets = <sys_clk 14>;
                   ...
           };

           ss-phy {
                   clocks = <sys_clk 14>;
                   resets = <sys_clk 14>;
                   ...
           };
   }


Which is better?
Or any other good idea?

Thanks.


-- 
Best Regards
Masahiro Yamada

             reply	other threads:[~2018-04-02  7:19 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-02  7:17 Masahiro Yamada [this message]
2018-04-02 12:04 ` [Question] MFD driver that handles clocks/resets and populates child nodes Andrew Lunn
2018-04-02 13:21   ` Masahiro Yamada
2018-04-02 13:32     ` Andrew Lunn
2018-04-03  8:03       ` Lee Jones
2018-04-03 12:14         ` Masahiro Yamada
2018-04-03 12:59           ` Lee Jones

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=CAK7LNARFWDMgmWsCjUmVrm4ZeYOcKCbizijxmDXob618ABjNWg@mail.gmail.com \
    --to=yamada.masahiro@socionext.com \
    --cc=arnd@arndb.de \
    --cc=devicetree@vger.kernel.org \
    --cc=felipe.balbi@linux.intel.com \
    --cc=hayashi.kunihiko@socionext.com \
    --cc=jaswinder.singh@linaro.org \
    --cc=lee.jones@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=masami.hiramatsu@linaro.org \
    --cc=robh+dt@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).