16.11.2020 19:49, Thierry Reding пишет: > On Mon, Nov 16, 2020 at 04:48:39PM +0300, Dmitry Osipenko wrote: >> 16.11.2020 16:20, Thierry Reding пишет: >>> On Wed, Sep 23, 2020 at 03:34:21AM +0300, Dmitry Osipenko wrote: >>>> The DMA subsystem could be entirely disabled in Kconfig and then the >>>> TEGRA20_APB_DMA option isn't available too. Hence kernel configuration >>>> fails if DMADEVICES Kconfig option is disabled due to the unsatisfiable >>>> dependency. >>>> >>>> The FUSE driver isn't a critical driver and currently it only provides >>>> NVMEM interface to userspace which isn't known to be widely used, and >>>> thus, it's fine if FUSE driver fails to load. >>> >>> This isn't entirely accurate. The FUSE driver also provides SKU specific >>> information via tegra_sku_info and exposes some of the FUSE registers to >>> other drivers, such as the calibration data for XUSB. >> >> The SKU data is read out only once during early boot and the DMA is not >> used for this. The FUSE platform driver exists separately from the early >> FUSE code. > > True, but the commit message isn't entirely accurate as-is, because on > later Tegra SoCs the driver does a bit more than that. So if you don't > mind I'll reword this to be a little more accurate if I end up deciding > to apply it. Please feel free to edit it as yo wish. >>> The APB DMA is really only needed to work around an issue on Tegra20, so >>> perhaps this really is okay. On the other hand, could we not just change >>> the dependency to something like >>> >>> select DMADEVICES if ARCH_TEGRA_2x_SOC >>> select TEGRA20_APB_DMA if ARCH_TEGRA_2x_SOC >>> >>> to fix the Kconfig issue but keep the explicit documentation of this >>> dependency? >> >> On Tegra20, the APB DMA is used only for by NVMEM which is exposed via >> sysfs. If DMA is disabled, then NVMEM isn't registered because >> tegra20_fuse_probe() returns -EPROBE_DEFER. > > Again, true. But -EPROBE_DEFER is a silent error, so if somebody indeed > has DMADEVICES disabled and TEGRA20_APB_DMA is not available, then they > will not get FUSE support and they are going to struggle to find out why > that's not working. > >> Hence there is no real need to enforce the extra dependencies. It's >> always better to allow minimizing the build dependencies whenever >> possible, IMO, and it's possible to do it in this case. > > I don't entirely agree with this. Dependencies (and especially selects) > are used to pull in driver and/or features that are generally considered > useful. In this particular case, TEGRA20_APB_DMA is needed for the FUSE > driver to work correctly on Tegra20. Whether FUSE support on Tegra20 is > useful or not isn't quite relevant at this point. > > There's also a balance as to what makes sense and what doesn't. APB DMA > is a useful feature in itself and disabling it is very much discouraged > because there are plenty of other drivers that can make use of it. That > is also the reason why we enable it in the default configuration. So I > don't consider a select on a symbol that's enabled by default an extra > dependency. Instead it's more of an explicit statement that you really > do want this enabled if you want that driver to work. > > And like I said, if we don't have this and the driver will fail to probe > because of -EPROBE_DEFER, the user is going to have a very difficult > time of finding out why exactly that's happening. We're not even > emitting an error for this, so there's no way of knowing, even if they > enable driver debugging, /why/ the FUSE driver isn't working. Recent kernels have /sys/kernel/debug/devices_deferred.
