Re: [PATCH] mmc: dw_mmc-k3: Add clk and reset softdep

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, Aug 1, 2018 at 5:21 AM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> On 31 July 2018 at 19:17, Jeremy Linton <lintonrjeremy@xxxxxxxxx> wrote:
>> Hi,
>>
>> Thanks for looking at this!
>>
>> On Mon, Jul 30, 2018 at 10:06 AM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>>> On 18 July 2018 at 02:49, Jeremy Linton <lintonrjeremy@xxxxxxxxx> wrote:
>>>> The mmc/k3 driver is dependent on a number of other linux modules
>>>> which must be built into the initrd in order to use the mmc/sd
>>>> as a boot device for initrd/module based distros.
>>>>
>>>> Normally this would be taken care of with linux's modules.deps
>>>> based on symbolic dependencies but the dwmmc drivers have
>>>> particularly complex relationships that are based on soft
>>>> callback APIs. The result is that dracut and other initrd builders
>>>
>>> If I understand correctly, this is about clock and resets. It doesn't
>>> sound like a special complex relationship, but rather a quite common.
>>
>> Yes...
>>
>>>
>>>> are unable to determine the module dependencies directly.
>>>>
>>>> To solve this problem linux has MODULE_SOFTDEP() so lets softdep
>>>> the hisi clock and reset drivers associated with the k3 implementation.
>>>
>>> Why doesn't the -EPROBE_DEFER mechanism help here? Or you simply don't
>>> want to load modules that isn't needed?
>>
>> The problem happens when you want to boot from the mmc device, and
>> everything is built as modules, with an initrd containing the minimum
>> module set to bring the rootfs/etc online.
>>
>> An initrd builder can't determine the subset of modules required to
>> get this driver to work based solely on the module dependencies and
>> the boot device description. It places the
>> mmc/designware/filesystem/etc modules in the initrd and when the
>> machine boots the designware driver loads but can't start the sd/mmc
>> because the reset driver is missing from the initrd.
>>
>> ARM's already have a special case to include the clk drivers even
>> though on arm/ACPI platforms they are unnecessary. In this case, its
>> really the reset driver which is problematic, because most boot
>> devices don't require them, and its not possible to tell programatically
>> what the dependency tree is.
>
> Sounds like the exact same problem as we have for build in drivers as well.
>
> Hence we just try all of them and when they fails with -EPROBE_DEFER
> we re-try later.
>
> So whats wrong with that here?

If I'm interpreting this correctly your suggesting just throwing
everything we can find in the image and letting the kernel sort it
out?

I'm not sure that is the direction of travel.. A builder like dracut
has a couple different modes it runs in, depending on the target
environment. For an installer image, it tends to do exactly that, but
the resulting images are hundreds of MB/s in size and can take a
fairly long time to load. The other is the hostonly mode which is
optimized to only place the minimum set of drivers/utilities in the
initrd sufficient to get the root filesystem mounted where it
continues to load the remainder of the modules. This is the default
mode once the machine is installed.

It does this both for space as well as size, as they tend to be
related and affect boot time, which can be quite significant if the
firmware doesn't optimally program the transfer settings for an SD
card, or the cores aren't particularly fast when it comes to
compress/decompress.

Anyway, its hostonly mode i'm trying to fix here, as people are lothe
to hard-code platform specific module lists for something like
hostonly mode. Its possible there are other systems with this problem,
but at the moment for whatever reason, this seems to be a HiSi
specific problem with the -k3 driver.


>
>>
>> This isn't really unique to the k3 driver, and is fairly common with a
>> few of these other dw_mmc modules (try playing with `udevadm info -a
>> -n /dev/somedevice` looking at DRIVERS and SUBSYSTEM) which are
>> themselves somewhat unusual because USB/SATA/SCSI/etc device
>> dependencies tend to be fairly clear.. Put another way, the problem is
>> basically that given a block device/driver we need an accurate view of
>> what drivers are required for it to work.
>> AFAIK, when that can't be determined by the symbolic dependencies,
>> that is what SOFTDEP is for.
>
> I think you are simplifying this.
>
> The dw_mmc k3 needs other resources besides the reset to work. In
> needs regulators, clocks, etc as well.

Sure, but those seem to have symbolic dependency chains/etc sufficient
to get them loaded (aka include the clk driver and it pulls in the
regulator or whatever).

>
> SOFTDEP don't sounds like the solution to list all these.

In the dw_mmc-k3 case, its really only the reset driver that isn't
being picked up by the dependency detection, but I listed the clk
driver as well since the two are fairly closely related on this
platform.

I'm open to other ideas for detecting these dependencies...

>
>>
>>>
>>>>
>>>> Signed-off-by: Jeremy Linton <lintonrjeremy@xxxxxxxxx>
>>>> ---
>>>>  drivers/mmc/host/dw_mmc-k3.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c
>>>> index 89cdb3d533bb..cd8f545fa30d 100644
>>>> --- a/drivers/mmc/host/dw_mmc-k3.c
>>>> +++ b/drivers/mmc/host/dw_mmc-k3.c
>>>> @@ -487,3 +487,4 @@ module_platform_driver(dw_mci_k3_pltfm_driver);
>>>>  MODULE_DESCRIPTION("K3 Specific DW-MSHC Driver Extension");
>>>>  MODULE_LICENSE("GPL v2");
>>>>  MODULE_ALIAS("platform:dwmmc_k3");
>>>> +MODULE_SOFTDEP("pre: hi6220_reset clk_hi655x");
>>>
>>> These are platform specific dependencies, but added as generic module
>>> dependencies. It doesn't seem right to me.
>>
>> I agree, its not optimal but In this case, the dwmmc_k3 is the
>> platform specific driver, and its
>
> No, dwmmc_k3 is not a platform specific driver. It's being used on
> more than one Hisilicon SoC.

Which if i'm reading the .dts correctly are all sharing the same
clk/reset driver too? (not 100% sure here)

>
>> being softdep'ed to its platform requirements. That avoids having to
>> hard-code SOC specific rules. I'm not sure how else to describe these
>> relationships with this driver.
>
> Unfortunate, neither do I. Except that you can add more modules to
> your initrd and insmod and try...


Yes, I know that works. I can manually add `--add-drivers
hi6220_reset` and build an initrd that is bootable, but that isn't
really the goal.  The idea is to create a generic process that works
on any machine/platform and this is one of the last remaining hicups
for this particular platform.

OTOH, with that MODULE_SOFTDEP I can type 'make install' in the kernel
source directory and have a bootable image created/installed without
having to create a platform specific dracut module/configuration.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux