Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220310195229.109477-1-nick.hawkins@hpe.com> <20220310195229.109477-9-nick.hawkins@hpe.com>
 <eb66cc83-2da9-8e19-db69-633e34fef946@canonical.com> <PH0PR84MB1718C31DB71AA2A67FEC5F6E88119@PH0PR84MB1718.NAMPRD84.PROD.OUTLOOK.COM>
 <b1cc2566-cd78-7cb4-f8a5-d6fc8065fe6e@canonical.com> <PH0PR84MB1718292B1C11F4FE83326A5388119@PH0PR84MB1718.NAMPRD84.PROD.OUTLOOK.COM>
 <fb4def96-4b20-604f-d15d-fef87eb2232d@canonical.com> <PH0PR84MB1718A2CFBFC90C9C0CAA5515881E9@PH0PR84MB1718.NAMPRD84.PROD.OUTLOOK.COM>
 <CAK8P3a0fCE_NM=z68d1m9BTfuKixh1pKLw3gn+Sr7SxKb6UJAg@mail.gmail.com>
 <PH0PR84MB17181C316E55073EBC28C386881E9@PH0PR84MB1718.NAMPRD84.PROD.OUTLOOK.COM>
 <PH0PR84MB17185ADBA824F9CC9EB3E022881F9@PH0PR84MB1718.NAMPRD84.PROD.OUTLOOK.COM>
 <CAK8P3a1080yz9jggOrwz2iQ1sAB2Xe2Emh22uCuqRok60BQSiA@mail.gmail.com> <PH0PR84MB17186189F5025374AA39584D88E19@PH0PR84MB1718.NAMPRD84.PROD.OUTLOOK.COM>
In-Reply-To: <PH0PR84MB17186189F5025374AA39584D88E19@PH0PR84MB1718.NAMPRD84.PROD.OUTLOOK.COM>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Thu, 31 Mar 2022 23:52:57 +0200
Message-ID: <CAK8P3a2b+ESAAkc=_2hqx0kOxwYVS6mNjMGBKhhipaSj_5jV9g@mail.gmail.com>
Subject: Re: [PATCH v3 09/10] arch: arm: boot: dts: Introduce HPE GXP Device tree
To:     "Hawkins, Nick" <nick.hawkins@hpe.com>
Cc:     Arnd Bergmann <arnd@arndb.de>,
        "Verdun, Jean-Marie" <verdun@hpe.com>,
        Olof Johansson <olof@lixom.net>,
        "soc@kernel.org" <soc@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Thu, Mar 31, 2022 at 11:09 PM Hawkins, Nick <nick.hawkins@hpe.com> wrote:
> On Thu, Mar 31, 2022 at 12:27 AM Hawkins, Nick <nick.hawkins@hpe.com> wrote:
> > On Tue, Mar 29, 2022 at 9:38 PM Hawkins, Nick <nick.hawkins@hpe.com>> wrote:
> > I'd have to study the other examples myself to see what is most common.
>
> > My feeling would be that it's better to either have a "hpe,gxp-timer" parent device with a watchdog child but no syscon, or to have a syscon/simple-mfd parent with both the timer and the watchdog as children.
>
> Arnd, thanks for the feedback. I am trying to use the approach you recommend where you have a syscon/simple-mfd parent with watchdog and timer as children.
>
> st: chip-controller@80 {
>                                 compatible = "hpe,gxp-ctrl-st","syscon","simple-mfd";
>                                 reg = <0x80 0x16>;
>
>                                 timer0: timer {
>                                         compatible = "hpe,gxp-timer";
>                                         interrupts = <0>;
>                                         interrupt-parent = <&vic0>;
>                                         clocks = <&ppuclk>;
>                                         clock-names = "ppuclk";
>                                 };
>
>                                 watchdog {
>                                         compatible = "hpe,gxp-wdt";
>                                 };
> };
>
> This compiles without any errors but I do have some questions about accessing the regmap in both drivers, specifically the timer code. How do you use a regmap with clocksource_mmio_init? I tried searching through the codebase and could not find the answer. I assume I am missing some vital step.

I don't think you can do this, if you are using the syscon regmap, you
go through the
regmap indirection rather than accessing the mmio register by virtual address,
and this may result in some extra code in your driver, and a little
runtime overhead.

If you prefer to avoid that, you can go back to having the timer node as the
parent, but without being a syscon. In this case, the watchdog would be handled
in one of these ways:

a) a child device gets created from the clocksource driver and bound to the
    watchdog driver, which then uses a private interface between the clocksource
    and the watchdog to access the registers

b) the clocksource driver itself registers as a watchdog driver, without
    having a separate driver module

One thing to consider is whether the register range here contains any
registers that may be used in another driver, e.g. a second timer,
a PWM, or a clk controller. If not, you are fairly free to pick any of these
approaches.

        Arnd