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

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

 



Hi Jeremy,

On Thu, Aug 2, 2018 at 2:04 AM Jeremy Linton <lintonrjeremy@xxxxxxxxx> wrote:
> 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:
> >> 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...

We already have a description of the hardware and its dependencies in
arch/arm64/boot/dts/hisilicon/hi3660.dtsi:

                dwmmc1: dwmmc1@ff37f000 {
                        ...
                        clocks = <&crg_ctrl HI3660_CLK_GATE_SD>,
                                <&crg_ctrl HI3660_HCLK_GATE_SD>;
                        ...
                        resets = <&crg_rst 0x94 18>;
                        reset-names = "reset";
                        hisilicon,peripheral-syscon = <&sctrl>;
                        ...
                };

                crg_ctrl: crg_ctrl@fff35000 {
                        compatible = "hisilicon,hi3660-crgctrl", "syscon";
                        ...
                }

                crg_rst: crg_rst_controller {
                        compatible = "hisilicon,hi3660-reset";
                        ...
                        hisi,rst-syscon = <&crg_ctrl>;
                };

                sctrl: sctrl@fff0a000 {
                        compatible = "hisilicon,hi3660-sctrl", "syscon";
                        ...
                }

$ git grep -E 'compatible.*=.*"hisilicon,hi3660-(crgctrl|reset|sctrl)"' -- "*c"
drivers/clk/hisilicon/clk-hi3660.c:     { .compatible =
"hisilicon,hi3660-crgctrl",
drivers/clk/hisilicon/clk-hi3660.c:     { .compatible =
"hisilicon,hi3660-sctrl",
drivers/reset/hisilicon/reset-hi3660.c: { .compatible =
"hisilicon,hi3660-reset", },
$ git grep clk-hi3660.o drivers/clk/hisilicon/Makefile
drivers/clk/hisilicon/Makefile:obj-$(CONFIG_COMMON_CLK_HI3660) += clk-hi3660.o
$ git grep reset-hi3660.o drivers/reset/hisilicon/Makefile
drivers/reset/hisilicon/Makefile:obj-$(CONFIG_COMMON_RESET_HI3660) +=
reset-hi3660.o

You can get inspiration to automate the above from my linux-config-from-dt
script at https://github.com/geertu/linux-scripts .

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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 Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux