On 2023/10/24 19:45, Uwe Kleine-König wrote: > 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)". > I see, I'll think about it. Maybe depend on STARFIVE'S SoCs first? Best regards, William
