| 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
| ||
|
Message-ID: <56201D82.6080106@gmail.com>
Date: Thu, 15 Oct 2015 23:41:22 +0200
From: Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
To: Jisheng Zhang <jszhang@...vell.com>, ulf.hansson@...aro.org,
robh+dt@...nel.org, pawel.moll@....com, mark.rutland@....com,
ijc+devicetree@...lion.org.uk, galak@...eaurora.org,
mturquette@...libre.com, sboyd@...eaurora.org
CC: linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-mmc@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/5] mmc: sdhci-pxav3: fix optional clock name
On 12.10.2015 07:46, Jisheng Zhang wrote:
> Commit 8afdc9cca27f ("mmc: sdhci-pxav3: Get optional core clock") adds
> additional optional clock support, but the clock names isn't correct.
> The current "io" clock is really the PXAv3 SDHCI IP's "core" clock
> which is manadatory. The current "core" clock is really the IP's "axi"
> clock which is optional.
>
> Signed-off-by: Jisheng Zhang <jszhang@...vell.com>
> ---
> .../devicetree/bindings/mmc/sdhci-pxa.txt | 8 ++---
> arch/arm/boot/dts/berlin2.dtsi | 6 ++--
> arch/arm/boot/dts/berlin2cd.dtsi | 2 +-
> arch/arm/boot/dts/berlin2q.dtsi | 2 +-
> drivers/mmc/host/sdhci-pxav3.c | 40 +++++++++++-----------
> 5 files changed, 29 insertions(+), 29 deletions(-)
Jisheng,
Please split the DT changes from the driver changes.
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt b/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt
> index 3d1b449..25d5ba8 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt
> @@ -17,9 +17,9 @@ Required properties:
> - reg names: should be "sdhci", "mbus", "conf-sdio3". only mandatory
> for "marvell,armada-380-sdhci"
> - clocks: Array of clocks required for SDHCI; requires at least one for
> - I/O clock.
> + core clock.
> - clock-names: Array of names corresponding to clocks property; shall be
> - "io" for I/O clock and "core" for optional core clock.
> + "core" for core clock and "axi" for optional axi clock.
s/axi/bus/ ?
> Optional properties:
> - mrvl,clk-delay-cycles: Specify a number of cycles to delay for tuning.
> @@ -32,7 +32,7 @@ sdhci@...80800 {
> bus-width = <8>;
> interrupts = <27>;
> clocks = <&chip CLKID_SDIO1XIN>, <&chip CLKID_SDIO1>;
> - clock-names = "io", "core";
> + clock-names = "core", "axi";
This will break backward compatibility, right?
> non-removable;
> mrvl,clk-delay-cycles = <31>;
> };
> @@ -45,6 +45,6 @@ sdhci@...00 {
> <0x18454 0x4>;
> interrupts = <0 25 0x4>;
> clocks = <&gateclk 17>;
> - clock-names = "io";
> + clock-names = "core";
> mrvl,clk-delay-cycles = <0x1F>;
> };
[...]
> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
> index f5edf9d..cec95f1 100644
> --- a/drivers/mmc/host/sdhci-pxav3.c
> +++ b/drivers/mmc/host/sdhci-pxav3.c
> @@ -59,8 +59,8 @@
> #define SDCE_MISC_INT_EN (1<<1)
>
> struct sdhci_pxa {
> + struct clk *clk_axi;
s/clk_axi/clk_bus/g
> struct clk *clk_core;
> - struct clk *clk_io;
> u8 power_mode;
> void __iomem *sdio3_conf_reg;
> };
> @@ -381,20 +381,20 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
> pltfm_host = sdhci_priv(host);
> pltfm_host->priv = pxa;
>
> - pxa->clk_io = devm_clk_get(dev, "io");
> - if (IS_ERR(pxa->clk_io))
> - pxa->clk_io = devm_clk_get(dev, NULL);
> - if (IS_ERR(pxa->clk_io)) {
> - dev_err(dev, "failed to get io clock\n");
> - ret = PTR_ERR(pxa->clk_io);
> + pxa->clk_core = devm_clk_get(dev, "core");
To maintain backward compatibility, we should still
devm_clk_get(dev, "io") here - or even better, first
probe for "io" to catch old binding semantics including
old "core"/"bus" misnaming.
If we detected old semantics, print a warning that
firmware should be updated.
Sebastian
> + if (IS_ERR(pxa->clk_core))
> + pxa->clk_core = devm_clk_get(dev, NULL);
> + if (IS_ERR(pxa->clk_core)) {
> + dev_err(dev, "failed to get core clock\n");
> + ret = PTR_ERR(pxa->clk_core);
> goto err_clk_get;
> }
> - pltfm_host->clk = pxa->clk_io;
> - clk_prepare_enable(pxa->clk_io);
> + pltfm_host->clk = pxa->clk_core;
> + clk_prepare_enable(pxa->clk_core);
>
> - pxa->clk_core = devm_clk_get(dev, "core");
> - if (!IS_ERR(pxa->clk_core))
> - clk_prepare_enable(pxa->clk_core);
> + pxa->clk_axi = devm_clk_get(dev, "axi");
> + if (!IS_ERR(pxa->clk_axi))
> + clk_prepare_enable(pxa->clk_axi);
>
> /* enable 1/8V DDR capable */
> host->mmc->caps |= MMC_CAP_1_8V_DDR;
> @@ -475,8 +475,8 @@ err_add_host:
> err_of_parse:
> err_cd_req:
> err_mbus_win:
> - clk_disable_unprepare(pxa->clk_io);
> clk_disable_unprepare(pxa->clk_core);
> + clk_disable_unprepare(pxa->clk_axi);
> err_clk_get:
> sdhci_pltfm_free(pdev);
> return ret;
> @@ -494,8 +494,8 @@ static int sdhci_pxav3_remove(struct platform_device *pdev)
>
> sdhci_remove_host(host, 1);
>
> - clk_disable_unprepare(pxa->clk_io);
> clk_disable_unprepare(pxa->clk_core);
> + clk_disable_unprepare(pxa->clk_axi);
>
> sdhci_pltfm_free(pdev);
>
> @@ -542,9 +542,9 @@ static int sdhci_pxav3_runtime_suspend(struct device *dev)
> if (ret)
> return ret;
>
> - clk_disable_unprepare(pxa->clk_io);
> - if (!IS_ERR(pxa->clk_core))
> - clk_disable_unprepare(pxa->clk_core);
> + clk_disable_unprepare(pxa->clk_core);
> + if (!IS_ERR(pxa->clk_axi))
> + clk_disable_unprepare(pxa->clk_axi);
>
> return 0;
> }
> @@ -555,9 +555,9 @@ static int sdhci_pxav3_runtime_resume(struct device *dev)
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_pxa *pxa = pltfm_host->priv;
>
> - clk_prepare_enable(pxa->clk_io);
> - if (!IS_ERR(pxa->clk_core))
> - clk_prepare_enable(pxa->clk_core);
> + clk_prepare_enable(pxa->clk_core);
> + if (!IS_ERR(pxa->clk_axi))
> + clk_prepare_enable(pxa->clk_axi);
>
> return sdhci_runtime_resume_host(host);
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists