On Wed, 16 Jan 2019, Uwe Kleine-König wrote:
> On Wed, Jan 16, 2019 at 09:18:45AM -0800, Paul Walmsley wrote: > > On Wed, 16 Jan 2019, Uwe Kleine-König wrote: > > > > > On Wed, Jan 16, 2019 at 04:40:42PM +0530, Yash Shah wrote: > > > > On Wed, Jan 16, 2019 at 3:30 AM Uwe Kleine-König > > > > <u.kleine-koe...@pengutronix.de> wrote: > > > > > On Fri, Jan 11, 2019 at 01:52:44PM +0530, Yash Shah wrote: > > > > > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > > > > > > index a8f47df..3bcaf6a 100644 > > > > > > --- a/drivers/pwm/Kconfig > > > > > > +++ b/drivers/pwm/Kconfig > > > > > > @@ -380,6 +380,16 @@ config PWM_SAMSUNG > > > > > > To compile this driver as a module, choose M here: the > > > > > > module > > > > > > will be called pwm-samsung. > > > > > > > > > > > > +config PWM_SIFIVE > > > > > > + tristate "SiFive PWM support" > > > > > > + depends on OF > > > > > > + depends on COMMON_CLK > > > > > > > > > > I'd say add: > > > > > > > > > > depends on MACH_SIFIVE || COMPILE_TEST > > > > > > > > > > (I guess "MACH_SIFIVE" is wrong, but I assume you get what I mean.) > > > > > > > > As of now, MACH_SIFIVE/ARCH_SIFIVE isn't available. > > > > @Paul, Do you have any comments on this? > > > > > > If this is not going to be available at least protect it by > > > > > > depends RISCV || COMPILE_TEST > > > > There's nothing RISC-V or SiFive SoC-specific about this driver or IP > > block. The HDL for this IP block is open-source and posted on Github. > > The IP block and driver would work unchanged on an ARM or MIPS SoC, and in > > fact, SiFive does design ARM-based SoCs as well. Likewise, any other SoC > > vendor could take the HDL for this IP block from the git tree and > > implement it on their own SoC. > > > > More generally: it's a basic principle of Linux device drivers that they > > should be buildable for any architecture. The idea here is to prevent > > developers from burying architecture or SoC-specific hacks into the > > driver. So there shouldn't be any architecture or SoC-specific code in > > any device driver, unless it's abstracted in some way - ideally through a > > common framework. > > > > So from this point of view, neither "depends MACH_SIFIVE" nor "depends > > RISCV" would be appropriate. Similarly, the equivalents for other > > architectures (e.g. "ARCH_ARM") or SoC manufacturers (e.g., > > "MACH_SAMSUNG") wouldn't be appropriate for any generic IP block device > > driver like this one. > > The dependency serves two purposes: > > a) ensure that the build requirements are fulfilled. > b) restrict configuration to actually existing machines > > When considering b) it doesn't make sense to enable the driver for (say) > a machine that is powered by an ARM SoC by TI. If you still want to > compile test the sifive driver for ARM, enable COMPILE_TEST. That's what > this symbol is there for. COMPILE_TEST made slightly more sense before the broad availability of open-source soft cores, SoC integration, and IP; and before powerful, inexpensive FPGAs and SoCs with FPGA fabrics were common. Even back then, COMPILE_TEST was starting to look questionable. IP blocks from CPU- and SoC vendor-independent libraries, like DesignWare and the Cadence IP libraries, were integrated on SoCs across varying CPU architectures. (Fortunately, looking at the tree, subsystem maintainers with DesignWare drivers seem to have largely avoided adding architecture or SoC-specific Kconfig restrictions there - and thus have also avoided the need for COMPILE_TEST.) If an unnecessary architecture Kconfig dependency was added for a leaf driver, that Kconfig line would either need to be patched out by hand, or if present, COMPILE_TEST would need to be enabled. This was less of a problem then. There were very few FPGA Linux users, and they were mostly working on internal proprietary projects. FPGAs that could run Linux at a reasonable speed, including MMUs and FPUs, were expensive or were missing good tool support. So most FPGA Linux development was restricted to ASIC vendors - the Samsungs, Qualcomms, NVIDIAs of the world - for prototyping, and those FPGA platforms never made it outside those companies. The situation is different now. The major FPGA vendors have inexpensive FPGA SoCs with full-featured CPU hard macros. The development boards can be quite affordable (< USD 300 for the Xilinx Ultra96). There are also now open-source SoC HDL implementations - including MMUs, FPUs, and peripherals like PWM and UART - for the conventional FPGA market. These can run at mid-1990's clock rates - slow by modern standards but still quite usable. So or vendor-specific IP blocks that are unlikely to ever be reused by anyone else, like the NVIDIA or TI PWM blocks, maybe there's still some justification for COMPILE_TEST. But for drivers for open-source, SoC-independent peripheral IP blocks - or even for proprietary IP blocks from library vendors like Synopsys or Cadence - like this PWM driver, we shouldn't add unnecessary architecture, SoC vendor, or COMPILE_TEST Kconfig dependencies. They will just force users to patch the kernel or enable COMPILE_TEST for kernels that are actually meant to run on real hardware. - Paul
