Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default

[Marek Szyprowski]

Hi Saravana,

On 18.12.2020 04:17, Saravana Kannan wrote:
> Cyclic dependencies in some firmware was one of the last remaining
> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> dependencies don't block probing, set fw_devlink=on by default.
>
> Setting fw_devlink=on by default brings a bunch of benefits (currently,
> only for systems with device tree firmware):
> * Significantly cuts down deferred probes.
> * Device probe is effectively attempted in graph order.
> * Makes it much easier to load drivers as modules without having to
>    worry about functional dependencies between modules (depmod is still
>    needed for symbol dependencies).
>
> If this patch prevents some devices from probing, it's very likely due
> to the system having one or more device drivers that "probe"/set up a
> device (DT node with compatible property) without creating a struct
> device for it.  If we hit such cases, the device drivers need to be
> fixed so that they populate struct devices and probe them like normal
> device drivers so that the driver core is aware of the devices and their
> status. See [1] for an example of such a case.
>
> [1] - https://lore.kernel.org/lkml/CAGETcx9PiX==mLxB9PO8Myyk6u2vhPVwTMsA5NkD-ywH5xhusw@xxxxxxxxxxxxxx/
> Signed-off-by: Saravana Kannan <saravanak@xxxxxxxxxx>

This patch landed recently in linux next-20210111 as commit e590474768f1 
("driver core: Set fw_devlink=on by default"). Sadly it breaks Exynos 
IOMMU operation, what causes lots of devices being deferred and not 
probed at all. I've briefly checked and noticed that 
exynos_sysmmu_probe() is never called after this patch. This is really 
strange for me, as the SYSMMU controllers on Exynos platform are regular 
platform devices registered by the OF code. The driver code is here: 
drivers/iommu/exynos-iommu.c, example dts: 
arch/arm/boot/dts/exynos3250.dtsi (compatible = "samsung,exynos-sysmmu").

> ---
>   drivers/base/core.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 4cc030361165..803bfa6eb823 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1457,7 +1457,7 @@ static void device_links_purge(struct device *dev)
>   #define FW_DEVLINK_FLAGS_RPM		(FW_DEVLINK_FLAGS_ON | \
>   					 DL_FLAG_PM_RUNTIME)
>   
> -static u32 fw_devlink_flags = FW_DEVLINK_FLAGS_PERMISSIVE;
> +static u32 fw_devlink_flags = FW_DEVLINK_FLAGS_ON;
>   static int __init fw_devlink_setup(char *arg)
>   {
>   	if (!arg)

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland