Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
MIME-Version: 1.0
References: <20231213190528.3751583-1-tmaimon77@gmail.com> <20231213190528.3751583-4-tmaimon77@gmail.com>
 <cf3ce945-2f1c-4dae-86b8-349dae3d962b@app.fastmail.com>
In-Reply-To: <cf3ce945-2f1c-4dae-86b8-349dae3d962b@app.fastmail.com>
From:   Tomer Maimon <tmaimon77@gmail.com>
Date:   Thu, 14 Dec 2023 16:09:29 +0200
Message-ID: <CAP6Zq1inLOMHORqO8=RbP6NfwJ63kLaH0G3+TKBhfn0p2CE53w@mail.gmail.com>
Subject: Re: [PATCH v3 3/3] soc: nuvoton: add NPCM BPC driver
To:     Arnd Bergmann <arnd@arndb.de>
Cc:     pmenzel@molgen.mpg.de, Rob Herring <robh+dt@kernel.org>,
        krzysztof.kozlowski+dt@linaro.org,
        Conor Dooley <conor+dt@kernel.org>, avifishman70@gmail.com,
        tali.perry1@gmail.com, Joel Stanley <joel@jms.id.au>,
        venture@google.com, yuenn@google.com, benjaminfair@google.com,
        =?UTF-8?Q?Jonathan_Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>,
        openbmc@lists.ozlabs.org,
        "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Arnd,

Thanks for your comments.

On Thu, 14 Dec 2023 at 14:44, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Wed, Dec 13, 2023, at 20:05, Tomer Maimon wrote:
> > Add Nuvoton BMC NPCM BIOS post code (BPC) driver.
> >
> > The NPCM BPC monitoring two configurable I/O address written by the host
> > on the bus.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > ---
> >  drivers/soc/nuvoton/Kconfig    |   9 +
> >  drivers/soc/nuvoton/Makefile   |   1 +
> >  drivers/soc/nuvoton/npcm-bpc.c | 387 +++++++++++++++++++++++++++++++++
> >  3 files changed, 397 insertions(+)
> >  create mode 100644 drivers/soc/nuvoton/npcm-bpc.c
>
> I try hard to avoid having user interfaces in drivers/soc/, that
> subsystem should primarily be used for things that don't have an
> existing subsystem in the kernel and are used by other in-kernel
> drivers but don't export hteir own misc device.
>
> > diff --git a/drivers/soc/nuvoton/Kconfig b/drivers/soc/nuvoton/Kconfig
> > index d5102f5f0c28..ebd162633942 100644
> > --- a/drivers/soc/nuvoton/Kconfig
> > +++ b/drivers/soc/nuvoton/Kconfig
> > @@ -2,6 +2,15 @@
> >
> >  menu "NUVOTON SoC drivers"
> >
> > +config NPCM_BPC
> > +     tristate "NPCM BIOS Post Code support"
> > +     depends on (ARCH_NPCM || COMPILE_TEST)
> > +     help
> > +       Provides NPCM driver to control the BIOS Post Code
> > +       interface which allows the BMC to monitor and save
> > +       the data written by the host to an arbitrary I/O port,
> > +       the BPC is connected to the host thourgh LPC or eSPI bus.
> > +
>
> This one in particular looks like this might be implemented
> by more than one BMC type, it's a fairly generic functionality.
>
> Have you talked to the other maintainers of SoCs used in
> OpenBMC about coming up with a common interface?
Yes, Both Nuvoton and Aspeed use the same user-facing code to manage
the host snooping.
https://github.com/openbmc/phosphor-host-postd
>
> > +#define DEVICE_NAME  "npcm-bpc"
Will do.
>
> [nitpicking] No need for macros like this one, open-coding the
> string is usually more readable.
>
>     Arnd

Thanks,

Tomer