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: <20221221210917.458537-1-fabrizio.castro.jz@renesas.com> <20221221210917.458537-3-fabrizio.castro.jz@renesas.com>
In-Reply-To: <20221221210917.458537-3-fabrizio.castro.jz@renesas.com>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Tue, 3 Jan 2023 09:36:49 +0100
Message-ID: <CAMuHMdXDwEUzBpG+w_G6=CzKo=n92cdVw6v8JwOwf9o86HnOZQ@mail.gmail.com>
Subject: Re: [PATCH v2 2/4] mfd: Add RZ/V2M PWC core driver
To:     fabrizio.castro.jz@renesas.com
Cc:     Linus Walleij <linus.walleij@linaro.org>,
        Bartosz Golaszewski <brgl@bgdev.pl>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Sebastian Reichel <sre@kernel.org>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        Lee Jones <lee@kernel.org>, linux-gpio@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-pm@vger.kernel.org,
        Chris Paterson <Chris.Paterson2@renesas.com>,
        Biju Das <biju.das@bp.renesas.com>,
        linux-renesas-soc@vger.kernel.org,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Jacopo Mondi <jacopo@jmondi.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Fabrizio,

On Wed, Dec 21, 2022 at 10:09 PM Fabrizio Castro
<fabrizio.castro.jz@renesas.com> wrote:
> The External Power Sequence Controller (PWC) IP (found in the
> RZ/V2M SoC) is a controller for external power supplies (regulators
> and power switches), and it supports the following features: it
> generates a power on/off sequence for external power supplies,
> it generates an on/off sequence for the LPDDR4 core power supply
> (LPVDD), it comes with General-Purpose Outputs, and it processes
> key input signals.

Thanks for your patch!

> The PWC is basically a Multi-Function Device (MFD), its software
> support comes with a core driver, and specialized drivers for
> its specific features.

I have to admit I'm not such a big fan of MFD.  In this driver,
you are not even sharing resources in the MFD cells, just the mapped
register base.  So I think you can easily save +100 LoC and reduce
maintenance synchronization overhead across subsystems by just having
a single non-MFD driver instead.

Did you pick MFD because the PWC poweroff feature depends on board
wiring, and thus is optional?

Are there any other MFD cells planned (e.g. regulators) to be added
later? The public datasheet does not list the actual registers of the
block, only a high-level overview with (rather detailed) behavioral
information.

Thanks!

> --- /dev/null
> +++ b/drivers/mfd/rzv2m-pwc.h

> +struct rzv2m_pwc_priv {
> +       void __iomem *base;
> +};
> +
> +#endif /* __LINUX_RZV2M_PWC_H__ */

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds