Hello William, On Tue, Oct 24, 2023 at 05:16:49PM +0800, William Qiu wrote: > On 2023/10/20 19:25, Uwe Kleine-König wrote: > > Hello, > > > > On Fri, Oct 20, 2023 at 06:37:39PM +0800, William Qiu wrote: > >> Add Pulse Width Modulation driver support for OpenCores. > >> > >> Co-developed-by: Hal Feng <hal.feng@xxxxxxxxxxxxxxxx> > >> Signed-off-by: Hal Feng <hal.feng@xxxxxxxxxxxxxxxx> > >> Signed-off-by: William Qiu <william.qiu@xxxxxxxxxxxxxxxx> > >> --- > >> MAINTAINERS | 7 ++ > >> drivers/pwm/Kconfig | 11 ++ > >> drivers/pwm/Makefile | 1 + > >> drivers/pwm/pwm-ocores.c | 211 +++++++++++++++++++++++++++++++++++++++ > >> 4 files changed, 230 insertions(+) > >> create mode 100644 drivers/pwm/pwm-ocores.c > >> > >> diff --git a/MAINTAINERS b/MAINTAINERS > >> index 6c4cce45a09d..321af8fa7aad 100644 > >> --- a/MAINTAINERS > >> +++ b/MAINTAINERS > >> @@ -16003,6 +16003,13 @@ F: Documentation/i2c/busses/i2c-ocores.rst > >> F: drivers/i2c/busses/i2c-ocores.c > >> F: include/linux/platform_data/i2c-ocores.h > >> > >> +OPENCORES PWM DRIVER > >> +M: William Qiu <william.qiu@xxxxxxxxxxxxxxxx> > >> +M: Hal Feng <hal.feng@xxxxxxxxxxxxxxxx> > >> +S: Supported > >> +F: Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml > >> +F: drivers/pwm/pwm-ocores.c > >> + > >> OPENRISC ARCHITECTURE > >> M: Jonas Bonn <jonas@xxxxxxxxxxxx> > >> M: Stefan Kristiansson <stefan.kristiansson@xxxxxxxxxxxxx> > >> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > >> index 8ebcddf91f7b..cbfbf227d957 100644 > >> --- a/drivers/pwm/Kconfig > >> +++ b/drivers/pwm/Kconfig > >> @@ -434,6 +434,17 @@ config PWM_NTXEC > >> controller found in certain e-book readers designed by the original > >> design manufacturer Netronix. > >> > >> +config PWM_OCORES > >> + tristate "Opencores PWM support" > >> + depends on HAS_IOMEM && OF > >> + depends on COMMON_CLK && RESET_CONTROLLER > > > > Would it make sense to add something like: > > > > depends on ARCH_SOMETHING || COMPILE_TEST > > > > here? > > > But there is no mention of architectural limitations in the OpenCores's > specification. I already guessed that. Still it probably makes no sense to enable that option on most machines. The PWM device found in i.MX SoCs can theoretically also be implemented on AT91 or S390x. In practice it isn't, so there is a dependency on ARCH_MXC || COMPILE_TEST. Consider the role of someone who does a kernel bump for a certain machine (on one end of the spectrum) or a distribution kernel (on the other end). If you take a 6.5 x86_64 allmodconfig + COMPILE_TEST=n and upgrade to v6.6-rc7 and do an oldconfig, you get 90 questions[1]. Just looking quickly through this list, among them are: DRM support for Loongson Graphics (DRM_LOONGSON) [N/m/?] (NEW) Xilinx AXI DMAS Engine (XILINX_DMA) [N/m/y/?] (NEW) Clock driver for Renesas VersaClock 3 devices (COMMON_CLK_VC3) [N/m/y/?] (NEW) Realtek RT1017 SDCA Codec - SDW (SND_SOC_RT1017_SDCA_SDW) [N/m/?] (NEW) I didn't check in detail and maybe one or the other is valid on x86_64, but I'd be surprised if you find two that are sensible to enable on x86_64 to support a real machine. While I think Kconfig cannot be held responsible to only allow generating "real world sensible" configurations, we should work a bit harder to rule out the obvious violators and make it easy for people configuring the kernel where sensible. In my book it's better to have a too strong dependency at first for a new driver (but allow it with COMPILE_TEST). Someone who as a device needing that driver will find it out and speak up. However if you allow to enable the driver everywhere, many people will disable the driver (maybe using yes '' | make oldconfig), some will spend time to research about this option to find which machines actually have such a device and if the machine(s) they care about are in this set. This is a waste of time and opportunities. (And note, this isn't only about people spending time to decide if they enable or disable PWM_OCORES, this is also about people who use yes '' because there are too many questions and so they might miss the handful of useful ones.) Best regards Uwe [1] measured using yes '' | make oldconfig and counting the occurrences of "(NEW)". -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
