linux-kernel - Re: [PATCH v1 2/2] soc: nuvoton: add NPCM LPC BPC driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <577e273d-ff9b-4d8d-b797-d7275ab8374f@app.fastmail.com>
Date:   Wed, 23 Nov 2022 11:57:57 +0100
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Tomer Maimon" <tmaimon77@...il.com>, avifishman70@...il.com,
        tali.perry1@...il.com, "Joel Stanley" <joel@....id.au>,
        venture@...gle.com, yuenn@...gle.com, benjaminfair@...gle.com,
        "Hitomi Hasegawa" <hasegawa-hitomi@...itsu.com>,
        "Hector Martin" <marcan@...can.st>,
        "Nicolas Ferre" <nicolas.ferre@...rochip.com>,
        "Conor.Dooley" <conor.dooley@...rochip.com>,
        Heiko Stübner <heiko@...ech.de>,
        "Sven Peter" <sven@...npeter.dev>,
        "Brian Norris" <briannorris@...omium.org>,
        "Rob Herring" <robh+dt@...nel.org>,
        krzysztof.kozlowski+dt@...aro.org
Cc:     openbmc@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v1 2/2] soc: nuvoton: add NPCM LPC BPC driver

On Tue, Nov 22, 2022, at 21:12, Tomer Maimon wrote:
> Add Nuvoton BMC NPCM LPC BIOS post code (BPC) driver.
>
> The NPCM BPC monitoring two configurable I/O address written by the host
> on the Low Pin Count (LPC) bus.
>
> Signed-off-by: Tomer Maimon <tmaimon77@...il.com>
> ---
>  drivers/soc/Kconfig                |   1 +
>  drivers/soc/Makefile               |   1 +
>  drivers/soc/nuvoton/Kconfig        |  24 ++
>  drivers/soc/nuvoton/Makefile       |   3 +
>  drivers/soc/nuvoton/npcm-lpc-bpc.c | 396 +++++++++++++++++++++++++++++

In general, I try to keep drivers/soc/ for drivers that are
used purely inside of the kernel and don't provide their
own user space ABI, those should normally be part of
some subsystem grouped by functionality.

It appears that we have similar drivers for aspeed already,
so there is some precedent, but I would still like to ask
you and Joel to try to make sure the two are compatible,
or ideally share the code for the user-facing part of the
LPC driver.

> +config NPCM_PCI_MBOX
> +	tristate "NPCM PCI Mailbox Controller"
> +	depends on (ARCH_NPCM || COMPILE_TEST) && REGMAP && MFD_SYSCON
> +	help
> +	  Expose the NPCM BMC PCI MBOX registers found on Nuvoton SOCs
> +	  to userspace.

This looks unrelated to the LPC driver, so this should
probably be a separate patch. The same comment about user
space presumably applies here, but I have not seen the driver.

The implementation of npcm-lpc-bpc looks fine otherwise, I only
noticed one minor detail that I would change:

> +	np = pdev->dev.parent->of_node;
> +	if (!of_device_is_compatible(np, "nuvoton,npcm750-lpc") &&
> +	    !of_device_is_compatible(np, "nuvoton,npcm845-lpc")) {
> +		dev_err(dev, "unsupported LPC device binding\n");
> +		return -ENODEV;
> +	}

This check doesn't seem to make sense here, since those are
the only two types you support.

      Arnd