linux-kernel - Re: [PATCH v6 2/2] mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFBinCAPGwb4YKJvUSyvzSC7hpVO0z-Ju_pBWx-06QzYXc0orw@mail.gmail.com>
Date:   Sun, 10 May 2020 22:52:25 +0200
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     Jerome Brunet <jbrunet@...libre.com>
Cc:     Ulf Hansson <ulf.hansson@...aro.org>,
        Stephen Boyd <sboyd@...nel.org>,
        "open list:ARM/Amlogic Meson..." <linux-amlogic@...ts.infradead.org>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        DTML <devicetree@...r.kernel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Jianxin Pan <jianxin.pan@...ogic.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        yinxin_1989@...yun.com,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        lnykww@...il.com, Anand Moon <linux.amoon@...il.com>
Subject: Re: [PATCH v6 2/2] mmc: host: meson-mx-sdhc: new driver for the
 Amlogic Meson SDHC host

Hi Jerome,

On Tue, May 5, 2020 at 6:05 PM Jerome Brunet <jbrunet@...libre.com> wrote:
[...]
> > 2. Keep the existing approach, with devm_clk_get(). I am fine with
> > this as well, we can always switch to 1) later on.
>
> I have a problem with this approach.
> The dt-bindings would include "#clock-cells = <1>" for a device that
> does not actually provide and only needs it has a temporary work around.
> Those bindings are supposed to be stable ...
actually I don't see a problem here because this specific MMC
controller has a clock controller built into it.
Rob also didn't see any problem with this when he reviewed the dt-bindings

> I have proposed 2 other short term solutions, let's see how it goes
since I was also curious how this turns out I first implemented your
suggestion to use a similar clk_hw registration style as
dwmac-meson8b.
That made the code easier to read - thank you for the suggestion!

On top of that I switched from clk_hw_onecell_data to directly
accessing "clk_hw.clk".
Unfortunately the diffstat is not as great as I hoped, it saves 21
lines (11 in the driver code, 6 in the soc.dtsi, 5 in the dt-bindings)
Once devm_clk_hw_get_clk() is implemented 8 lines have to be added
again for error checking.
I attached the patch for the drivers/mmc/host/meson-mx-sdhc* parts if
you are curious (it'll apply only on top of my github branch, not on
this series).

Please let me know if you want me to submit an updated series where I
directly access "clk_hw.clk" to get the "struct clk" or if I should
keep clk_hw_onecell_data.
If it's the former then I'll also add a TODO comment for the
conversion to devm_clk_hw_get_clk() so it's easy to find.


Regards,
Martin

View attachment "temp.diff" of type "text/x-patch" (3726 bytes)