From: "Arnd Bergmann" <arnd@arndb.de> To: "Tomer Maimon" <tmaimon77@gmail.com> Cc: avifishman70@gmail.com, tali.perry1@gmail.com, "Joel Stanley" <joel@jms.id.au>, venture@google.com, yuenn@google.com, benjaminfair@google.com, "Hitomi Hasegawa" <hasegawa-hitomi@fujitsu.com>, "Hector Martin" <marcan@marcan.st>, "Nicolas Ferre" <nicolas.ferre@microchip.com>, "Conor.Dooley" <conor.dooley@microchip.com>, "Heiko Stübner" <heiko@sntech.de>, "Sven Peter" <sven@svenpeter.dev>, "Brian Norris" <briannorris@chromium.org>, "Rob Herring" <robh+dt@kernel.org>, krzysztof.kozlowski+dt@linaro.org, openbmc@lists.ozlabs.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH v1 2/2] soc: nuvoton: add NPCM LPC BPC driver Date: Fri, 25 Nov 2022 11:25:00 +0100 [thread overview] Message-ID: <7f43febb-0a89-4313-9c85-a7a44c231b45@app.fastmail.com> (raw) In-Reply-To: <CAP6Zq1ikqtKOGUZX-VAdyhs+nsvy7ah4gqRrbXVA8Gp9L46hXQ@mail.gmail.com> On Wed, Nov 23, 2022, at 19:01, Tomer Maimon wrote: > On Wed, 23 Nov 2022 at 12:58, Arnd Bergmann <arnd@arndb.de> wrote: >> >> 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@gmail.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. > Nuvoton and Aspeed use the same user-facing code to manage the host snooping. > https://github.com/openbmc/phosphor-host-postd Ok, great! >> 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. > About the LPC, I like to double check with our architectures on it > because the BPC should working on eSPI as well. > Maybe I should remove the LPC part. The version you posted only has LPC support, not eSPI, so that wouldn't work. I'm not sure how eSPI is normally represented in device drivers, does that show up the same way as an LPC device, or do you need to register a separate spi_driver? If it's part of the same platform driver with different OF compatible strings, the normal way to handle this would be to use the .data field in the of_device_id to pass model specific information to other parts of the driver. Arnd
WARNING: multiple messages have this Message-ID (diff)
From: "Arnd Bergmann" <arnd@arndb.de> To: "Tomer Maimon" <tmaimon77@gmail.com> Cc: "Hitomi Hasegawa" <hasegawa-hitomi@fujitsu.com>, devicetree@vger.kernel.org, "Brian Norris" <briannorris@chromium.org>, benjaminfair@google.com, "Sven Peter" <sven@svenpeter.dev>, avifishman70@gmail.com, venture@google.com, openbmc@lists.ozlabs.org, "Hector Martin" <marcan@marcan.st>, "Nicolas Ferre" <nicolas.ferre@microchip.com>, tali.perry1@gmail.com, "Conor.Dooley" <conor.dooley@microchip.com>, "Rob Herring" <robh+dt@kernel.org>, "Joel Stanley" <joel@jms.id.au>, krzysztof.kozlowski+dt@linaro.org, linux-kernel@vger.kernel.org, "Heiko Stübner" <heiko@sntech.de> Subject: Re: [PATCH v1 2/2] soc: nuvoton: add NPCM LPC BPC driver Date: Fri, 25 Nov 2022 11:25:00 +0100 [thread overview] Message-ID: <7f43febb-0a89-4313-9c85-a7a44c231b45@app.fastmail.com> (raw) In-Reply-To: <CAP6Zq1ikqtKOGUZX-VAdyhs+nsvy7ah4gqRrbXVA8Gp9L46hXQ@mail.gmail.com> On Wed, Nov 23, 2022, at 19:01, Tomer Maimon wrote: > On Wed, 23 Nov 2022 at 12:58, Arnd Bergmann <arnd@arndb.de> wrote: >> >> 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@gmail.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. > Nuvoton and Aspeed use the same user-facing code to manage the host snooping. > https://github.com/openbmc/phosphor-host-postd Ok, great! >> 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. > About the LPC, I like to double check with our architectures on it > because the BPC should working on eSPI as well. > Maybe I should remove the LPC part. The version you posted only has LPC support, not eSPI, so that wouldn't work. I'm not sure how eSPI is normally represented in device drivers, does that show up the same way as an LPC device, or do you need to register a separate spi_driver? If it's part of the same platform driver with different OF compatible strings, the normal way to handle this would be to use the .data field in the of_device_id to pass model specific information to other parts of the driver. Arnd
next prev parent reply other threads:[~2022-11-25 10:25 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-11-22 20:12 [PATCH v1 0/2] soc: add NPCM LPC BPC driver support Tomer Maimon 2022-11-22 20:12 ` Tomer Maimon 2022-11-22 20:12 ` [PATCH v1 1/2] dt-binding: soc: nuvoton: Add NPCM BPC LPC documentation Tomer Maimon 2022-11-22 20:12 ` Tomer Maimon 2022-11-23 10:03 ` Krzysztof Kozlowski 2022-11-24 15:38 ` Tomer Maimon 2022-11-24 15:38 ` Tomer Maimon 2022-11-24 16:18 ` Krzysztof Kozlowski 2022-11-29 16:44 ` Tomer Maimon 2022-11-29 16:44 ` Tomer Maimon 2022-11-30 8:27 ` Krzysztof Kozlowski 2022-11-30 10:31 ` Tomer Maimon 2022-11-30 10:31 ` Tomer Maimon 2022-11-30 19:30 ` Rob Herring 2022-11-30 19:30 ` Rob Herring 2022-11-22 20:12 ` [PATCH v1 2/2] soc: nuvoton: add NPCM LPC BPC driver Tomer Maimon 2022-11-22 20:12 ` Tomer Maimon 2022-11-23 10:57 ` Arnd Bergmann 2022-11-23 10:57 ` Arnd Bergmann 2022-11-23 18:01 ` Tomer Maimon 2022-11-23 18:01 ` Tomer Maimon 2022-11-25 10:25 ` Arnd Bergmann [this message] 2022-11-25 10:25 ` Arnd Bergmann 2022-11-29 18:20 ` Tomer Maimon 2022-11-29 18:20 ` Tomer Maimon 2022-11-25 2:27 ` kernel test robot 2022-11-25 2:27 ` kernel test robot -- strict thread matches above, loose matches on Subject: below -- 2019-06-10 13:32 [PATCH v1 0/2] soc: add NPCM LPC BPC driver support Tomer Maimon 2019-06-10 13:32 ` [PATCH v1 2/2] soc: nuvoton: add NPCM LPC BPC driver Tomer Maimon 2019-06-11 13:11 ` Arnd Bergmann 2019-06-11 13:11 ` Arnd Bergmann 2019-06-13 9:26 ` Tomer Maimon 2019-06-13 9:26 ` Tomer Maimon 2019-06-13 11:49 ` Arnd Bergmann 2019-06-13 11:49 ` Arnd Bergmann
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=7f43febb-0a89-4313-9c85-a7a44c231b45@app.fastmail.com \ --to=arnd@arndb.de \ --cc=avifishman70@gmail.com \ --cc=benjaminfair@google.com \ --cc=briannorris@chromium.org \ --cc=conor.dooley@microchip.com \ --cc=devicetree@vger.kernel.org \ --cc=hasegawa-hitomi@fujitsu.com \ --cc=heiko@sntech.de \ --cc=joel@jms.id.au \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=marcan@marcan.st \ --cc=nicolas.ferre@microchip.com \ --cc=openbmc@lists.ozlabs.org \ --cc=robh+dt@kernel.org \ --cc=sven@svenpeter.dev \ --cc=tali.perry1@gmail.com \ --cc=tmaimon77@gmail.com \ --cc=venture@google.com \ --cc=yuenn@google.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.