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]
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)

Powered by blists - more mailing lists