* [PATCH v3 1/3] ARM: da850: fix infinite loop in clk_set_rate()
From: Bartosz Golaszewski @ 2016-12-06 11:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <689efaa1-d8d9-6937-5880-3ed7a1401268@ti.com>
2016-12-05 11:15 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
> On Monday 05 December 2016 03:39 PM, Bartosz Golaszewski wrote:
>> The aemif clock is added twice to the lookup table in da850.c. This
>> breaks the children list of pll0_sysclk3 as we're using the same list
>> links in struct clk. When calling clk_set_rate(), we get stuck in
>> propagate_rate().
>>
>> Create a separate clock for nand, inheriting the rate of the aemif
>> clock and retrieve it in the davinci_nand module.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>> arch/arm/mach-davinci/da850.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
>> index e770c97..c008e5e 100644
>> --- a/arch/arm/mach-davinci/da850.c
>> +++ b/arch/arm/mach-davinci/da850.c
>> @@ -367,6 +367,11 @@ static struct clk aemif_clk = {
>> .flags = ALWAYS_ENABLED,
>> };
>>
>> +static struct clk aemif_nand_clk = {
>> + .name = "nand",
>> + .parent = &aemif_clk,
>> +};
>> +
>> static struct clk usb11_clk = {
>> .name = "usb11",
>> .parent = &pll0_sysclk4,
>> @@ -537,7 +542,7 @@ static struct clk_lookup da850_clks[] = {
>> CLK("da830-mmc.0", NULL, &mmcsd0_clk),
>> CLK("da830-mmc.1", NULL, &mmcsd1_clk),
>> CLK("ti-aemif", NULL, &aemif_clk),
>> - CLK(NULL, "aemif", &aemif_clk),
>> + CLK(NULL, "aemif", &aemif_nand_clk),
>
> Why use a NULL device name here?
Hi Sekhar,
there's an issue with this bit. I added an of_dev_auxdata entry to
da8xx-dt.c for the nand node, but it didn't work (the nand driver
could not get the clock). When I dug deeper, it turned out, the nand
node is created from aemif_probe() instead of from
da850_init_machine() and the lookup table is not passed as argument to
of_platform_populate().
There are two solutions: one is using "620000000.nand" as dev_id in
the clock lookup table, but that's ugly. The second is leaving dev_id
as NULL - I verified that the nand driver works correctly having only
the connector id. Please let me know which one you prefer or if you
have other ideas.
Best regards,
Bartosz Golaszewski
^ permalink raw reply
* [PATCH 1/2] arm: kernel: Add SMC structure parameter
From: Will Deacon @ 2016-12-06 11:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480405463-23165-2-git-send-email-andy.gross@linaro.org>
Hi Andy,
On Tue, Nov 29, 2016 at 01:44:22AM -0600, Andy Gross wrote:
> This patch adds a quirk parameter to the arm_smccc_smc call. The quirk
> structure allows for specialized SMC operations due to SoC specific
> requirements.
>
> This patch also fixes up all the current users of the arm_smccc_smc API.
>
> This patch and partial implementation was suggested by Will Deacon.
>
> Signed-off-by: Andy Gross <andy.gross@linaro.org>
> ---
> arch/arm/kernel/smccc-call.S | 3 ++-
> arch/arm/mach-artpec/board-artpec6.c | 2 +-
> arch/arm64/kernel/asm-offsets.c | 7 +++++--
> arch/arm64/kernel/smccc-call.S | 3 ++-
> drivers/clk/rockchip/clk-ddr.c | 6 +++---
> drivers/devfreq/rk3399_dmc.c | 6 +++---
> drivers/firmware/meson/meson_sm.c | 2 +-
> drivers/firmware/psci.c | 2 +-
> drivers/firmware/qcom_scm-64.c | 4 ++--
> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> include/linux/arm-smccc.h | 18 ++++++++++++++++--
> 11 files changed, 37 insertions(+), 18 deletions(-)
Thanks for respinning this; I'd forgotten about it!
> diff --git a/arch/arm/kernel/smccc-call.S b/arch/arm/kernel/smccc-call.S
> index 37669e7..e77950a 100644
> --- a/arch/arm/kernel/smccc-call.S
> +++ b/arch/arm/kernel/smccc-call.S
> @@ -47,7 +47,8 @@ UNWIND( .fnend)
> /*
> * void smccc_smc(unsigned long a0, unsigned long a1, unsigned long a2,
> * unsigned long a3, unsigned long a4, unsigned long a5,
> - * unsigned long a6, unsigned long a7, struct arm_smccc_res *res)
> + * unsigned long a6, unsigned long a7, struct arm_smccc_res *res,
> + * struct arm_smccc_quirk *quirk)
I'd not thought of doing it like this -- I envisaged embedding the quirk
structure into arm_smccc_res, but this works too. I wonder if we could avoid
having to pass NULL everywhere if we renamed arm_smccc_{hvc.smc} and added
a default wrapper around them?
For example, rename arm_smccc_smc to __arm_smccc_smc, add a macro called
arm_smccc_smc that passes a NULL argument for the quirk, then finally add
a macro arm_smccc_smc_quirk that takes the additional parameter?
Will
^ permalink raw reply
* [PATCH 2/2] firmware: qcom: scm: Fix interrupted SCM calls
From: Will Deacon @ 2016-12-06 11:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480405463-23165-3-git-send-email-andy.gross@linaro.org>
On Tue, Nov 29, 2016 at 01:44:23AM -0600, Andy Gross wrote:
> This patch adds a Qualcomm specific quirk to the arm_smccc_smc call.
>
> On Qualcomm ARM64 platforms, the SMC call can return before it has
> completed. If this occurs, the call can be restarted, but it requires
> using the returned session ID value from the interrupted SMC call.
>
> The quirk stores off the session ID from the interrupted call in the
> quirk structure so that it can be used by the caller.
>
> This patch folds in a fix given by Sricharan R:
> https://lkml.org/lkml/2016/9/28/272
>
> Signed-off-by: Andy Gross <andy.gross@linaro.org>
> ---
> arch/arm64/kernel/smccc-call.S | 9 ++++++++-
> drivers/firmware/qcom_scm-64.c | 11 +++++++++--
> include/linux/arm-smccc.h | 11 ++++++++---
> 3 files changed, 25 insertions(+), 6 deletions(-)
Acked-by: Will Deacon <will.deacon@arm.com>
Will
^ permalink raw reply
* [PATCH v3] arm64: Add DTS support for FSL's LS1012A SoC
From: Harninder Rai @ 2016-12-06 11:50 UTC (permalink / raw)
To: linux-arm-kernel
LS1012A features an advanced 64-bit ARM v8 CortexA53 processor
with 32 KB of parity protected L1-I cache, 32 KB of ECC protected
L1-D cache, as well as 256 KB of ECC protected L2 cache.
Features summary
One 64-bit ARM-v8 Cortex-A53 core with the following capabilities
- Arranged as a cluster of one core supporting a 256 KB L2 cache with ECC
protection
- Speed up to 800 MHz
- Parity-protected 32 KB L1 instruction cache and 32 KB L1 data cache
- Neon SIMD engine
- ARM v8 cryptography extensions
One 16-bit DDR3L SDRAM memory controller
ARM core-link CCI-400 cache coherent interconnect
Cryptography acceleration (SEC)
One Configurable x3 SerDes
One PCI Express Gen2 controller, supporting x1 operation
One serial ATA (SATA Gen 3.0) controller
One USB 3.0/2.0 controller with integrated PHY
Following levels of DTSI/DTS files have been created for the LS1012A
SoC family:
- fsl-ls1012a.dtsi:
DTS-Include file for FSL LS1012A SoC.
- fsl-ls1012a-frdm.dts:
DTS file for FSL LS1012A FRDM board.
- fsl-ls1012a-qds.dts:
DTS file for FSL LS1012A QDS board.
- fsl-ls1012a-rdb.dts:
DTS file for FSL LS1012A RDB board.
Signed-off-by: Harninder Rai <harninder.rai@nxp.com>
Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
---
Changes in v3: Incorporated Shawn's comments
- Change PPI interrupts to IRQ_TYPE_LEVEL_LOW and
- SPI interrupts to IRQ_TYPE_LEVEL_HIGH
Changes in v2: Incorporated Shawn's comments
- Brief introduction of the SoC in commit message
- Alphabetic ordering of labeled nodes
- Better naming to be used for regulator node
- Make timer node's comments more readable
- Sort nodes with unit-address in order of the address
arch/arm64/boot/dts/freescale/Makefile | 3 +
arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts | 115 ++++++++++
arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts | 128 +++++++++++
arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts | 59 +++++
arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 245 +++++++++++++++++++++
5 files changed, 550 insertions(+)
create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 6602718..39db645 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -1,3 +1,6 @@
+dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-frdm.dtb
+dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-qds.dtb
+dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-rdb.dtb
dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1043a-qds.dtb
dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1043a-rdb.dtb
dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1046a-qds.dtb
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts b/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
new file mode 100644
index 0000000..81bd689
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
@@ -0,0 +1,115 @@
+/*
+ * Device Tree file for Freescale LS1012A Freedom Board.
+ *
+ * Copyright 2016, Freescale Semiconductor
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPLv2 or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+/dts-v1/;
+
+#include "fsl-ls1012a.dtsi"
+
+/ {
+ model = "LS1012A Freedom Board";
+ compatible = "fsl,ls1012a-frdm", "fsl,ls1012a";
+
+ sys_mclk: clock-mclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <25000000>;
+ };
+
+ regulator_1p8v: regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "1P8V";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-always-on;
+ };
+
+ sound {
+ compatible = "simple-audio-card";
+ simple-audio-card,format = "i2s";
+ simple-audio-card,widgets =
+ "Microphone", "Microphone Jack",
+ "Headphone", "Headphone Jack",
+ "Speaker", "Speaker Ext",
+ "Line", "Line In Jack";
+ simple-audio-card,routing =
+ "MIC_IN", "Microphone Jack",
+ "Microphone Jack", "Mic Bias",
+ "LINE_IN", "Line In Jack",
+ "Headphone Jack", "HP_OUT",
+ "Speaker Ext", "LINE_OUT";
+
+ simple-audio-card,cpu {
+ sound-dai = <&sai2>;
+ frame-master;
+ bitclock-master;
+ };
+
+ simple-audio-card,codec {
+ sound-dai = <&codec>;
+ frame-master;
+ bitclock-master;
+ system-clock-frequency = <25000000>;
+ };
+ };
+};
+
+&duart0 {
+ status = "okay";
+};
+
+&i2c0 {
+ status = "okay";
+
+ codec: sgtl5000 at a {
+ #sound-dai-cells = <0>;
+ compatible = "fsl,sgtl5000";
+ reg = <0xa>;
+ VDDA-supply = <®ulator_1p8v>;
+ VDDIO-supply = <®ulator_1p8v>;
+ clocks = <&sys_mclk>;
+ };
+};
+
+&sai2 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
new file mode 100644
index 0000000..b841251
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
@@ -0,0 +1,128 @@
+/*
+ * Device Tree file for Freescale LS1012A QDS Board.
+ *
+ * Copyright 2016, Freescale Semiconductor
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPLv2 or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+/dts-v1/;
+
+#include "fsl-ls1012a.dtsi"
+
+/ {
+ model = "LS1012A QDS Board";
+ compatible = "fsl,ls1012a-qds", "fsl,ls1012a";
+
+ sys_mclk: clock-mclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <24576000>;
+ };
+
+ regulator_3p3v: regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "3P3V";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+
+ sound {
+ compatible = "simple-audio-card";
+ simple-audio-card,format = "i2s";
+ simple-audio-card,widgets =
+ "Microphone", "Microphone Jack",
+ "Headphone", "Headphone Jack",
+ "Speaker", "Speaker Ext",
+ "Line", "Line In Jack";
+ simple-audio-card,routing =
+ "MIC_IN", "Microphone Jack",
+ "Microphone Jack", "Mic Bias",
+ "LINE_IN", "Line In Jack",
+ "Headphone Jack", "HP_OUT",
+ "Speaker Ext", "LINE_OUT";
+
+ simple-audio-card,cpu {
+ sound-dai = <&sai2>;
+ frame-master;
+ bitclock-master;
+ };
+
+ simple-audio-card,codec {
+ sound-dai = <&codec>;
+ frame-master;
+ bitclock-master;
+ system-clock-frequency = <24576000>;
+ };
+ };
+};
+
+&duart0 {
+ status = "okay";
+};
+
+&i2c0 {
+ status = "okay";
+
+ pca9547 at 77 {
+ compatible = "nxp,pca9547";
+ reg = <0x77>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ i2c at 4 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x4>;
+
+ codec: sgtl5000 at a {
+ #sound-dai-cells = <0>;
+ compatible = "fsl,sgtl5000";
+ reg = <0xa>;
+ VDDA-supply = <®ulator_3p3v>;
+ VDDIO-supply = <®ulator_3p3v>;
+ clocks = <&sys_mclk>;
+ };
+ };
+ };
+};
+
+&sai2 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
new file mode 100644
index 0000000..62c5c71
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
@@ -0,0 +1,59 @@
+/*
+ * Device Tree file for Freescale LS1012A RDB Board.
+ *
+ * Copyright 2016, Freescale Semiconductor
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPLv2 or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+/dts-v1/;
+
+#include "fsl-ls1012a.dtsi"
+
+/ {
+ model = "LS1012A RDB Board";
+ compatible = "fsl,ls1012a-rdb", "fsl,ls1012a";
+};
+
+&duart0 {
+ status = "okay";
+};
+
+&i2c0 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
new file mode 100644
index 0000000..92e64f3
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
@@ -0,0 +1,245 @@
+/*
+ * Device Tree Include file for Freescale Layerscape-1012A family SoC.
+ *
+ * Copyright 2016, Freescale Semiconductor
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPLv2 or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+ compatible = "fsl,ls1012a";
+ interrupt-parent = <&gic>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu0: cpu at 0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x0>;
+ clocks = <&clockgen 1 0>;
+ #cooling-cells = <2>;
+ };
+ };
+
+ sysclk: sysclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <100000000>;
+ clock-output-names = "sysclk";
+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+
+ interrupts = <1 13 IRQ_TYPE_LEVEL_LOW>,/* Physical Secure PPI */
+ <1 14 IRQ_TYPE_LEVEL_LOW>,/* Physical Non-Secure PPI */
+ <1 11 IRQ_TYPE_LEVEL_LOW>,/* Virtual PPI */
+ <1 10 IRQ_TYPE_LEVEL_LOW>;/* Hypervisor PPI */
+ };
+
+ pmu {
+ compatible = "arm,armv8-pmuv3";
+ interrupts = <0 106 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ gic: interrupt-controller at 1400000 {
+ compatible = "arm,gic-400";
+ #interrupt-cells = <3>;
+ interrupt-controller;
+ reg = <0x0 0x1401000 0 0x1000>, /* GICD */
+ <0x0 0x1402000 0 0x2000>, /* GICC */
+ <0x0 0x1404000 0 0x2000>, /* GICH */
+ <0x0 0x1406000 0 0x2000>; /* GICV */
+ interrupts = <1 9 IRQ_TYPE_LEVEL_LOW>;
+ };
+
+ reboot {
+ compatible = "syscon-reboot";
+ regmap = <&dcfg>;
+ offset = <0xb0>;
+ mask = <0x02>;
+ };
+
+ soc {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ scfg: scfg at 1570000 {
+ compatible = "fsl,ls1012a-scfg", "syscon";
+ reg = <0x0 0x1570000 0x0 0x10000>;
+ big-endian;
+ };
+
+ dcfg: dcfg at 1ee0000 {
+ compatible = "fsl,ls1012a-dcfg",
+ "syscon";
+ reg = <0x0 0x1ee0000 0x0 0x10000>;
+ big-endian;
+ };
+
+ clockgen: clocking at 1ee1000 {
+ compatible = "fsl,ls1012a-clockgen";
+ reg = <0x0 0x1ee1000 0x0 0x1000>;
+ #clock-cells = <2>;
+ clocks = <&sysclk>;
+ };
+
+ i2c0: i2c at 2180000 {
+ compatible = "fsl,vf610-i2c";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x0 0x2180000 0x0 0x10000>;
+ interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clockgen 4 0>;
+ status = "disabled";
+ };
+
+ i2c1: i2c at 2190000 {
+ compatible = "fsl,vf610-i2c";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x0 0x2190000 0x0 0x10000>;
+ interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clockgen 4 0>;
+ status = "disabled";
+ };
+
+ duart0: serial at 21c0500 {
+ compatible = "fsl,ns16550", "ns16550a";
+ reg = <0x00 0x21c0500 0x0 0x100>;
+ interrupts = <0 54 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clockgen 4 0>;
+ };
+
+ duart1: serial at 21c0600 {
+ compatible = "fsl,ns16550", "ns16550a";
+ reg = <0x00 0x21c0600 0x0 0x100>;
+ interrupts = <0 54 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clockgen 4 0>;
+ };
+
+ gpio0: gpio at 2300000 {
+ compatible = "fsl,qoriq-gpio";
+ reg = <0x0 0x2300000 0x0 0x10000>;
+ interrupts = <0 66 IRQ_TYPE_LEVEL_HIGH>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpio1: gpio at 2310000 {
+ compatible = "fsl,qoriq-gpio";
+ reg = <0x0 0x2310000 0x0 0x10000>;
+ interrupts = <0 67 IRQ_TYPE_LEVEL_HIGH>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ wdog0: wdog at 2ad0000 {
+ compatible = "fsl,ls1012a-wdt",
+ "fsl,imx21-wdt";
+ reg = <0x0 0x2ad0000 0x0 0x10000>;
+ interrupts = <0 83 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clockgen 4 0>;
+ big-endian;
+ };
+
+ sai1: sai at 2b50000 {
+ #sound-dai-cells = <0>;
+ compatible = "fsl,vf610-sai";
+ reg = <0x0 0x2b50000 0x0 0x10000>;
+ interrupts = <0 148 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clockgen 4 3>, <&clockgen 4 3>,
+ <&clockgen 4 3>, <&clockgen 4 3>;
+ clock-names = "bus", "mclk1", "mclk2", "mclk3";
+ dma-names = "tx", "rx";
+ dmas = <&edma0 1 47>,
+ <&edma0 1 46>;
+ status = "disabled";
+ };
+
+ sai2: sai at 2b60000 {
+ #sound-dai-cells = <0>;
+ compatible = "fsl,vf610-sai";
+ reg = <0x0 0x2b60000 0x0 0x10000>;
+ interrupts = <0 149 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clockgen 4 3>, <&clockgen 4 3>,
+ <&clockgen 4 3>, <&clockgen 4 3>;
+ clock-names = "bus", "mclk1", "mclk2", "mclk3";
+ dma-names = "tx", "rx";
+ dmas = <&edma0 1 45>,
+ <&edma0 1 44>;
+ status = "disabled";
+ };
+
+ edma0: edma at 2c00000 {
+ #dma-cells = <2>;
+ compatible = "fsl,vf610-edma";
+ reg = <0x0 0x2c00000 0x0 0x10000>,
+ <0x0 0x2c10000 0x0 0x10000>,
+ <0x0 0x2c20000 0x0 0x10000>;
+ interrupts = <0 103 IRQ_TYPE_LEVEL_HIGH>,
+ <0 103 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "edma-tx", "edma-err";
+ dma-channels = <32>;
+ big-endian;
+ clock-names = "dmamux0", "dmamux1";
+ clocks = <&clockgen 4 3>,
+ <&clockgen 4 3>;
+ };
+
+ sata: sata at 3200000 {
+ compatible = "fsl,ls1012a-ahci", "fsl,ls1043a-ahci";
+ reg = <0x0 0x3200000 0x0 0x10000>;
+ interrupts = <0 69 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clockgen 4 0>;
+ };
+ };
+};
--
1.9.1
^ permalink raw reply related
* [PATCHv4 05/10] arm64: Use __pa_symbol for kernel symbols
From: Catalin Marinas @ 2016-12-06 11:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <72eb08c8-4f2c-6cb9-1e23-0860fd153a2e@gmail.com>
On Mon, Dec 05, 2016 at 04:50:33PM -0800, Florian Fainelli wrote:
> On 11/29/2016 10:55 AM, Laura Abbott wrote:
> > __pa_symbol is technically the marco that should be used for kernel
> > symbols. Switch to this as a pre-requisite for DEBUG_VIRTUAL which
> > will do bounds checking. As part of this, introduce lm_alias, a
> > macro which wraps the __va(__pa(...)) idiom used a few places to
> > get the alias.
> >
> > Signed-off-by: Laura Abbott <labbott@redhat.com>
> > ---
> > v4: Stop calling __va early, conversion of a few more sites. I decided against
> > wrapping the __p*d_populate calls into new functions since the call sites
> > should be limited.
> > ---
>
>
> > - pud_populate(&init_mm, pud, bm_pmd);
> > + if (pud_none(*pud))
> > + __pud_populate(pud, __pa_symbol(bm_pmd), PMD_TYPE_TABLE);
> > pmd = fixmap_pmd(addr);
> > - pmd_populate_kernel(&init_mm, pmd, bm_pte);
> > + __pmd_populate(pmd, __pa_symbol(bm_pte), PMD_TYPE_TABLE);
>
> Is there a particular reason why pmd_populate_kernel() is not changed to
> use __pa_symbol() instead of using __pa()? The other users in the arm64
> kernel is arch/arm64/kernel/hibernate.c which seems to call this against
> kernel symbols as well?
create_safe_exec_page() may allocate a pte from the linear map and
passes such pointer to pmd_populate_kernel(). The copy_pte() function
does something similar. In addition, we have the generic
__pte_alloc_kernel() in mm/memory.c using linear addresses.
--
Catalin
^ permalink raw reply
* [PATCH v9 01/11] arm/arm64: vgic: Implement support for userspace access
From: Auger Eric @ 2016-12-06 11:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161128130512.GC18170@cbox>
Hi,
On 28/11/2016 14:05, Christoffer Dall wrote:
> On Wed, Nov 23, 2016 at 06:31:48PM +0530, vijay.kilari at gmail.com wrote:
>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>
>> Read and write of some registers like ISPENDR and ICPENDR
>> from userspace requires special handling when compared to
>> guest access for these registers.
>>
>> Refer to Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>> for handling of ISPENDR, ICPENDR registers handling.
>>
>> Add infrastructure to support guest and userspace read
>> and write for the required registers
>> Also moved vgic_uaccess from vgic-mmio-v2.c to vgic-mmio.c
>>
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>> ---
>> virt/kvm/arm/vgic/vgic-mmio-v2.c | 25 ----------
>> virt/kvm/arm/vgic/vgic-mmio-v3.c | 102 ++++++++++++++++++++++++++++++++-------
>> virt/kvm/arm/vgic/vgic-mmio.c | 78 +++++++++++++++++++++++++++---
>> virt/kvm/arm/vgic/vgic-mmio.h | 19 ++++++++
>> 4 files changed, 175 insertions(+), 49 deletions(-)
>>
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c
>> index b44b359..0b32f40 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
>> @@ -406,31 +406,6 @@ int vgic_v2_has_attr_regs(struct kvm_device *dev, struct kvm_device_attr *attr)
>> return -ENXIO;
>> }
>>
>> -/*
>> - * When userland tries to access the VGIC register handlers, we need to
>> - * create a usable struct vgic_io_device to be passed to the handlers and we
>> - * have to set up a buffer similar to what would have happened if a guest MMIO
>> - * access occurred, including doing endian conversions on BE systems.
>> - */
>> -static int vgic_uaccess(struct kvm_vcpu *vcpu, struct vgic_io_device *dev,
>> - bool is_write, int offset, u32 *val)
>> -{
>> - unsigned int len = 4;
>> - u8 buf[4];
>> - int ret;
>> -
>> - if (is_write) {
>> - vgic_data_host_to_mmio_bus(buf, len, *val);
>> - ret = kvm_io_gic_ops.write(vcpu, &dev->dev, offset, len, buf);
>> - } else {
>> - ret = kvm_io_gic_ops.read(vcpu, &dev->dev, offset, len, buf);
>> - if (!ret)
>> - *val = vgic_data_mmio_bus_to_host(buf, len);
>> - }
>> -
>> - return ret;
>> -}
>> -
>> int vgic_v2_cpuif_uaccess(struct kvm_vcpu *vcpu, bool is_write,
>> int offset, u32 *val)
>> {
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> index 50f42f0..8e76d04 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> @@ -207,6 +207,66 @@ static unsigned long vgic_mmio_read_v3_idregs(struct kvm_vcpu *vcpu,
>> return 0;
>> }
>>
>> +static unsigned long vgic_v3_uaccess_read_pending(struct kvm_vcpu *vcpu,
>> + gpa_t addr, unsigned int len)
>> +{
>> + u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
>> + u32 value = 0;
>> + int i;
>> +
>> + /*
>> + * A level triggerred interrupt pending state is latched in both
>> + * "soft_pending" and "line_level" variables. Userspace will save
>> + * and restore soft_pending and line_level separately.
>> + * Refer to Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>> + * handling of ISPENDR and ICPENDR.
>> + */
>> + for (i = 0; i < len * 8; i++) {
>> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
>> +
>> + if (irq->config == VGIC_CONFIG_LEVEL && irq->soft_pending)
>> + value |= (1U << i);
>> + if (irq->config == VGIC_CONFIG_EDGE && irq->pending)
>> + value |= (1U << i);
>> +
>> + vgic_put_irq(vcpu->kvm, irq);
>> + }
>> +
>> + return value;
>> +}
>> +
>> +static void vgic_v3_uaccess_write_pending(struct kvm_vcpu *vcpu,
>> + gpa_t addr, unsigned int len,
>> + unsigned long val)
>> +{
>> + u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
>> + int i;
>> +
>> + for (i = 0; i < len * 8; i++) {
>> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
>> +
>> + spin_lock(&irq->irq_lock);
>> + if (test_bit(i, &val)) {
>> + /* soft_pending is set irrespective of irq type
>> + * (level or edge) to avoid dependency that VM should
>> + * restore irq config before pending info.
>> + */
>
> nit: kernel commenting style
>
>> + irq->pending = true;
>> + irq->soft_pending = true;
>> + vgic_queue_irq_unlock(vcpu->kvm, irq);
>> + } else {
>> + irq->soft_pending = false;
>> + if (irq->config == VGIC_CONFIG_EDGE ||
>> + (irq->config == VGIC_CONFIG_LEVEL &&
>> + !irq->line_level))
>> + irq->pending = false;
I am confused by the comment above. Since we test the irq config here
don't we assume the config was restored before the pending state?
>> + spin_unlock(&irq->irq_lock);
>> + }
>> +
>> + vgic_put_irq(vcpu->kvm, irq);
>> + }
>> +}
>> +
>> /* We want to avoid outer shareable. */
>> u64 vgic_sanitise_shareability(u64 field)
>> {
>> @@ -356,7 +416,7 @@ static void vgic_mmio_write_pendbase(struct kvm_vcpu *vcpu,
>> * We take some special care here to fix the calculation of the register
>> * offset.
>> */
>> -#define REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(off, rd, wr, bpi, acc) \
>> +#define REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(off, rd, wr, ur, uw, bpi, acc) \
>> { \
>> .reg_offset = off, \
>> .bits_per_irq = bpi, \
>> @@ -371,6 +431,8 @@ static void vgic_mmio_write_pendbase(struct kvm_vcpu *vcpu,
>> .access_flags = acc, \
>> .read = rd, \
>> .write = wr, \
>> + .uaccess_read = ur, \
>> + .uaccess_write = uw, \
>> }
>>
>> static const struct vgic_register_region vgic_v3_dist_registers[] = {
>> @@ -378,40 +440,42 @@ static void vgic_mmio_write_pendbase(struct kvm_vcpu *vcpu,
>> vgic_mmio_read_v3_misc, vgic_mmio_write_v3_misc, 16,
>> VGIC_ACCESS_32bit),
>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_IGROUPR,
>> - vgic_mmio_read_rao, vgic_mmio_write_wi, 1,
>> + vgic_mmio_read_rao, vgic_mmio_write_wi, NULL, NULL, 1,
>> VGIC_ACCESS_32bit),
>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISENABLER,
>> - vgic_mmio_read_enable, vgic_mmio_write_senable, 1,
>> + vgic_mmio_read_enable, vgic_mmio_write_senable, NULL, NULL, 1,
>> VGIC_ACCESS_32bit),
>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICENABLER,
>> - vgic_mmio_read_enable, vgic_mmio_write_cenable, 1,
>> + vgic_mmio_read_enable, vgic_mmio_write_cenable, NULL, NULL, 1,
>> VGIC_ACCESS_32bit),
>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISPENDR,
>> - vgic_mmio_read_pending, vgic_mmio_write_spending, 1,
>> + vgic_mmio_read_pending, vgic_mmio_write_spending,
>> + vgic_v3_uaccess_read_pending, vgic_v3_uaccess_write_pending, 1,
>> VGIC_ACCESS_32bit),
>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICPENDR,
>> - vgic_mmio_read_pending, vgic_mmio_write_cpending, 1,
>> + vgic_mmio_read_pending, vgic_mmio_write_cpending,
>> + vgic_mmio_read_raz, vgic_mmio_write_wi, 1,
>> VGIC_ACCESS_32bit),
>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISACTIVER,
>> - vgic_mmio_read_active, vgic_mmio_write_sactive, 1,
>> + vgic_mmio_read_active, vgic_mmio_write_sactive, NULL, NULL, 1,
>> VGIC_ACCESS_32bit),
>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICACTIVER,
>> - vgic_mmio_read_active, vgic_mmio_write_cactive, 1,
>> + vgic_mmio_read_active, vgic_mmio_write_cactive, NULL, NULL, 1,
>> VGIC_ACCESS_32bit),
>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_IPRIORITYR,
>> - vgic_mmio_read_priority, vgic_mmio_write_priority, 8,
>> - VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
>> + vgic_mmio_read_priority, vgic_mmio_write_priority, NULL, NULL,
>> + 8, VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ITARGETSR,
>> - vgic_mmio_read_raz, vgic_mmio_write_wi, 8,
>> + vgic_mmio_read_raz, vgic_mmio_write_wi, NULL, NULL, 8,
>> VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICFGR,
>> - vgic_mmio_read_config, vgic_mmio_write_config, 2,
>> + vgic_mmio_read_config, vgic_mmio_write_config, NULL, NULL, 2,
>> VGIC_ACCESS_32bit),
>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_IGRPMODR,
>> - vgic_mmio_read_raz, vgic_mmio_write_wi, 1,
>> + vgic_mmio_read_raz, vgic_mmio_write_wi, NULL, NULL, 1,
>> VGIC_ACCESS_32bit),
>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_IROUTER,
>> - vgic_mmio_read_irouter, vgic_mmio_write_irouter, 64,
>> + vgic_mmio_read_irouter, vgic_mmio_write_irouter, NULL, NULL, 64,
>> VGIC_ACCESS_64bit | VGIC_ACCESS_32bit),
>> REGISTER_DESC_WITH_LENGTH(GICD_IDREGS,
>> vgic_mmio_read_v3_idregs, vgic_mmio_write_wi, 48,
>> @@ -449,11 +513,13 @@ static void vgic_mmio_write_pendbase(struct kvm_vcpu *vcpu,
>> REGISTER_DESC_WITH_LENGTH(GICR_ICENABLER0,
>> vgic_mmio_read_enable, vgic_mmio_write_cenable, 4,
>> VGIC_ACCESS_32bit),
>> - REGISTER_DESC_WITH_LENGTH(GICR_ISPENDR0,
>> - vgic_mmio_read_pending, vgic_mmio_write_spending, 4,
>> + REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_ISPENDR0,
>> + vgic_mmio_read_pending, vgic_mmio_write_spending,
>> + vgic_v3_uaccess_read_pending, vgic_v3_uaccess_write_pending, 4,
>> VGIC_ACCESS_32bit),
>> - REGISTER_DESC_WITH_LENGTH(GICR_ICPENDR0,
>> - vgic_mmio_read_pending, vgic_mmio_write_cpending, 4,
>> + REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_ICPENDR0,
>> + vgic_mmio_read_pending, vgic_mmio_write_cpending,
>> + vgic_mmio_read_raz, vgic_mmio_write_wi, 4,
>> VGIC_ACCESS_32bit),
>> REGISTER_DESC_WITH_LENGTH(GICR_ISACTIVER0,
>> vgic_mmio_read_active, vgic_mmio_write_sactive, 4,
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
>> index ebe1b9f..d5f3ee2 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio.c
>> +++ b/virt/kvm/arm/vgic/vgic-mmio.c
>> @@ -484,6 +484,74 @@ static bool check_region(const struct kvm *kvm,
>> return false;
>> }
>>
>> +static const struct vgic_register_region *
>> +vgic_get_mmio_region(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev,
>> + gpa_t addr, int len)
>> +{
>> + const struct vgic_register_region *region;
>> +
>> + region = vgic_find_mmio_region(iodev->regions, iodev->nr_regions,
>> + addr - iodev->base_addr);
>> + if (!region || !check_region(vcpu->kvm, region, addr, len))
>> + return NULL;
>> +
>> + return region;
>> +}
>> +
>> +static int vgic_uaccess_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
>> + gpa_t addr, u32 *val)
>> +{
>> + struct vgic_io_device *iodev = kvm_to_vgic_iodev(dev);
>> + const struct vgic_register_region *region;
>> + struct kvm_vcpu *r_vcpu;
>> +
>> + region = vgic_get_mmio_region(vcpu, iodev, addr, sizeof(u32));
>> + if (!region) {
>> + *val = 0;
>> + return 0;
do we really want to return 0 here? -ENXIO?
I see that dispatch_mmio_read/write return 0 in that case but I don't
see any reason either? Other kvm_io_device_ops seem to return
-EOPNOTSUPP in such a case.
Thanks
Eric
>> + }
>> +
>> + r_vcpu = iodev->redist_vcpu ? iodev->redist_vcpu : vcpu;
>> + if (region->uaccess_read)
>> + *val = region->uaccess_read(r_vcpu, addr, sizeof(u32));
>> + else
>> + *val = region->read(r_vcpu, addr, sizeof(u32));
>> +
>> + return 0;
>> +}
>> +
>> +static int vgic_uaccess_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
>> + gpa_t addr, const u32 *val)
>> +{
>> + struct vgic_io_device *iodev = kvm_to_vgic_iodev(dev);
>> + const struct vgic_register_region *region;
>> + struct kvm_vcpu *r_vcpu;
>> +
>> + region = vgic_get_mmio_region(vcpu, iodev, addr, sizeof(u32));
>> + if (!region)
>> + return 0;
same?
>> +
>> + r_vcpu = iodev->redist_vcpu ? iodev->redist_vcpu : vcpu;
>> + if (region->uaccess_write)
>> + region->uaccess_write(r_vcpu, addr, sizeof(u32), *val);
>> + else
>> + region->write(r_vcpu, addr, sizeof(u32), *val);
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Userland access to VGIC registers.
>> + */
>> +int vgic_uaccess(struct kvm_vcpu *vcpu, struct vgic_io_device *dev,
>> + bool is_write, int offset, u32 *val)
>> +{
>> + if (is_write)
>> + return vgic_uaccess_write(vcpu, &dev->dev, offset, val);
>> + else
>> + return vgic_uaccess_read(vcpu, &dev->dev, offset, val);
>> +}
>> +
>> static int dispatch_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
>> gpa_t addr, int len, void *val)
>> {
>> @@ -491,9 +559,8 @@ static int dispatch_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
>> const struct vgic_register_region *region;
>> unsigned long data = 0;
>>
>> - region = vgic_find_mmio_region(iodev->regions, iodev->nr_regions,
>> - addr - iodev->base_addr);
>> - if (!region || !check_region(vcpu->kvm, region, addr, len)) {
>> + region = vgic_get_mmio_region(vcpu, iodev, addr, len);
>> + if (!region) {
>> memset(val, 0, len);
>> return 0;
>> }
>> @@ -524,9 +591,8 @@ static int dispatch_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
>> const struct vgic_register_region *region;
>> unsigned long data = vgic_data_mmio_bus_to_host(val, len);
>>
>> - region = vgic_find_mmio_region(iodev->regions, iodev->nr_regions,
>> - addr - iodev->base_addr);
>> - if (!region || !check_region(vcpu->kvm, region, addr, len))
>> + region = vgic_get_mmio_region(vcpu, iodev, addr, len);
>> + if (!region)
>> return 0;
>>
>> switch (iodev->iodev_type) {
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.h b/virt/kvm/arm/vgic/vgic-mmio.h
>> index 84961b4..7b30296 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio.h
>> +++ b/virt/kvm/arm/vgic/vgic-mmio.h
>> @@ -34,6 +34,10 @@ struct vgic_register_region {
>> gpa_t addr, unsigned int len,
>> unsigned long val);
>> };
>> + unsigned long (*uaccess_read)(struct kvm_vcpu *vcpu, gpa_t addr,
>> + unsigned int len);
>> + void (*uaccess_write)(struct kvm_vcpu *vcpu, gpa_t addr,
>> + unsigned int len, unsigned long val);
>> };
>>
>> extern struct kvm_io_device_ops kvm_io_gic_ops;
>> @@ -86,6 +90,18 @@ struct vgic_register_region {
>> .write = wr, \
>> }
>>
>> +#define REGISTER_DESC_WITH_LENGTH_UACCESS(off, rd, wr, urd, uwr, length, acc) \
>> + { \
>> + .reg_offset = off, \
>> + .bits_per_irq = 0, \
>> + .len = length, \
>> + .access_flags = acc, \
>> + .read = rd, \
>> + .write = wr, \
>> + .uaccess_read = urd, \
>> + .uaccess_write = uwr, \
>> + }
>> +
>> int kvm_vgic_register_mmio_region(struct kvm *kvm, struct kvm_vcpu *vcpu,
>> struct vgic_register_region *reg_desc,
>> struct vgic_io_device *region,
>> @@ -158,6 +174,9 @@ void vgic_mmio_write_config(struct kvm_vcpu *vcpu,
>> gpa_t addr, unsigned int len,
>> unsigned long val);
>>
>> +int vgic_uaccess(struct kvm_vcpu *vcpu, struct vgic_io_device *dev,
>> + bool is_write, int offset, u32 *val);
>> +
>> unsigned int vgic_v2_init_dist_iodev(struct vgic_io_device *dev);
>>
>> unsigned int vgic_v3_init_dist_iodev(struct vgic_io_device *dev);
>> --
>> 1.9.1
>>
>
> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH] SCPI (pre-v1.0): fix reading sensor value
From: Sudeep Holla @ 2016-12-06 11:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFBinCD4RexWZPTm2hKjk6hqcmuCHiGoBDhwzgtx+0ns6sE70A@mail.gmail.com>
On 02/12/16 22:54, Martin Blumenstingl wrote:
> Hello Sudeep,
>
> On Fri, Nov 25, 2016 at 1:56 AM, Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
>> On Thu, Nov 24, 2016 at 12:15 PM, Martin Blumenstingl
>> <martin.blumenstingl@googlemail.com> wrote:
>>> On Thu, Nov 24, 2016 at 11:47 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>>
>>>>
>>>> On 24/11/16 00:18, Martin Blumenstingl wrote:
>>>>>
>>>>> I observed the following "strange" value when trying to read the SCPI
>>>>> temperature sensor on my Amlogic GXM S912 device:
>>>>> $ cat /sys/class/hwmon/hwmon0/temp1_input
>>>>> 6875990994467160116
>>>>>
>>>>> The value reported by the original kernel (Amlogic vendor kernel, after
>>>>> a reboot obviously) was 53C.
>>>>> The Amlogic SCPI driver only uses a single 32bit value to read the
>>>>> sensor value, instead of two. After stripping the upper 32bits from
>>>>> above value gives "52" as result, which is basically identical to
>>>>> what the vendor kernel reports.
>>>>
>>>>
>>>> Can you check why the upper 32-bit is not set to 0 ?
>>>>
>>>> In scpi_process_cmd, we memset extra rx_buf length by 0 and that should
>>>> take care. Neil had mentioned that works but now I doubt if firmware
>>>> returns 8 instead of 4 in the size which is wrong as it supports only
>>>> 32-bit.
>>> according to the code "RX Length is not replied by the legacy
>>> Firmware", so for legacy firmwares the "if (match->rx_len > len)"
>>> condition will never be true (because both values are always equal).
>>> in the sensor case we then go and copy 8 byte from mem->payload to
>>> match->rx_buf, but SCPI firmware only wrote 4 bytes to mem->payload.
>>> This means we are simply reading 4 byte (hi_val) of uninitialized
>>> memory - which may be all zeroes if we're lucky - but in my case I got
>>> "garbage" (I guess it's the second byte from the *previous* command
>>> which are leaking here).
>>>
>>> while writing this I see a second (more generic) approach which might
>>> work as well:
>>> scpi_chan does not hold any information about rx_payload/tx_payload
>>> sizes (these are calculated in scpi_probe but not stored anywhere).
>>> (for now, let's assume we had the rx_payload_size available)
>>> we could then go ahead and memset(rx_payload, 0, rx_payload_size) in
>>> scpi_tx_prepare or scpi_send_message.
>>> However, I am not sure if that would have any side-effects (for
>>> example on newer SCPI implementations).
>> I simply tried implementing this solution and I find it better than
>> the old one. However, I am still not sure if there are any
>> side-effects. maybe you can simply review v2 of this series which
>> implements the described approach (the result is the same as with v1:
>> temp1_input contains the correct value).
>
> did you have time to review this yet?
>
I was away, I will look into this today or tomorrow.
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH] arm/arm64: KVM: Check for properly initialized timer on init
From: Marc Zyngier @ 2016-12-06 11:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161205093211.11870-1-christoffer.dall@linaro.org>
On 05/12/16 09:32, Christoffer Dall wrote:
> When the arch timer code fails to initialize (for example because the
> memory mapped timer doesn't work, which is currently seen with the AEM
> model), then KVM just continues happily with a final result that KVM
> eventually does a NULL pointer dereference of the uninitialized cycle
> counter.
>
> Check directly for this in the init path and give the user a reasonable
> error in this case.
>
> Cc: Shih-Wei Li <shihwei@cs.columbia.edu>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> virt/kvm/arm/arch_timer.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 27a1f63..5c12f53 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -425,6 +425,11 @@ int kvm_timer_hyp_init(void)
> info = arch_timer_get_kvm_info();
> timecounter = &info->timecounter;
>
> + if (!timecounter->cc) {
> + kvm_err("arch_timer: uninitialized timecounter\n");
For consistency, I'll change the error message to say "kvm_arch_timer",
just like the below case.
> + return -ENODEV;
> + }
> +
> if (info->virtual_irq <= 0) {
> kvm_err("kvm_arch_timer: invalid virtual timer IRQ: %d\n",
> info->virtual_irq);
>
Otherwise looks good to me. I'll queue it now.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v2] arm/arm64: KVM: VGIC: limit ITARGETSR bits to number of VCPUs
From: Marc Zyngier @ 2016-12-06 11:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161116175716.31578-1-andre.przywara@arm.com>
On 16/11/16 17:57, Andre Przywara wrote:
> The GICv2 spec says in section 4.3.12 that a "CPU targets field bit that
> corresponds to an unimplemented CPU interface is RAZ/WI."
> Currently we allow the guest to write any value in there and it can
> read that back.
> Mask the written value with the proper CPU mask to be spec compliant.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> Changes v1 .. v2:
> - use GENMASK() instead of open-coding mask
> - drop explicit 0xff masking, since cpu_mask is stronger anyway
>
> virt/kvm/arm/vgic/vgic-mmio-v2.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c
> index b44b359..78e34bc 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
> @@ -129,6 +129,7 @@ static void vgic_mmio_write_target(struct kvm_vcpu *vcpu,
> unsigned long val)
> {
> u32 intid = VGIC_ADDR_TO_INTID(addr, 8);
> + u8 cpu_mask = GENMASK(atomic_read(&vcpu->kvm->online_vcpus) - 1, 0);
> int i;
>
> /* GICD_ITARGETSR[0-7] are read-only */
> @@ -141,7 +142,7 @@ static void vgic_mmio_write_target(struct kvm_vcpu *vcpu,
>
> spin_lock(&irq->irq_lock);
>
> - irq->targets = (val >> (i * 8)) & 0xff;
> + irq->targets = (val >> (i * 8)) & cpu_mask;
> target = irq->targets ? __ffs(irq->targets) : 0;
> irq->target_vcpu = kvm_get_vcpu(vcpu->kvm, target);
>
>
Applied, thanks.
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v3] KVM: arm/arm64: Access CNTHCTL_EL2 bit fields correctly
From: Marc Zyngier @ 2016-12-06 11:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480620725-9864-1-git-send-email-jintack@cs.columbia.edu>
On 01/12/16 19:32, Jintack Lim wrote:
> Current KVM world switch code is unintentionally setting wrong bits to
> CNTHCTL_EL2 when E2H == 1, which may allow guest OS to access physical
> timer. Bit positions of CNTHCTL_EL2 are changing depending on
> HCR_EL2.E2H bit. EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is
> not set, but they are 11th and 10th bits respectively when E2H is set.
>
> In fact, on VHE we only need to set those bits once, not for every world
> switch. This is because the host kernel runs in EL2 with HCR_EL2.TGE ==
> 1, which makes those bits have no effect for the host kernel execution.
> So we just set those bits once for guests, and that's it.
>
> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> ---
> v2->v3:
> - Perform the initialization including CPU hotplug case.
> - Move has_vhe() to asm/virt.h
>
> v1->v2:
> - Skip configuring cnthctl_el2 in world switch path on VHE system.
> - Write patch based on linux-next.
> ---
> arch/arm/include/asm/virt.h | 5 +++++
> arch/arm/kvm/arm.c | 3 +++
> arch/arm64/include/asm/virt.h | 10 ++++++++++
> include/kvm/arm_arch_timer.h | 1 +
> virt/kvm/arm/arch_timer.c | 23 +++++++++++++++++++++++
> virt/kvm/arm/hyp/timer-sr.c | 33 +++++++++++++++++++++------------
> 6 files changed, 63 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/include/asm/virt.h b/arch/arm/include/asm/virt.h
> index a2e75b8..6dae195 100644
> --- a/arch/arm/include/asm/virt.h
> +++ b/arch/arm/include/asm/virt.h
> @@ -80,6 +80,11 @@ static inline bool is_kernel_in_hyp_mode(void)
> return false;
> }
>
> +static inline bool has_vhe(void)
> +{
> + return false;
> +}
> +
> /* The section containing the hypervisor idmap text */
> extern char __hyp_idmap_text_start[];
> extern char __hyp_idmap_text_end[];
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 8f92efa..13e54e8 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -1099,6 +1099,9 @@ static void cpu_init_hyp_mode(void *dummy)
> __cpu_init_hyp_mode(pgd_ptr, hyp_stack_ptr, vector_ptr);
> __cpu_init_stage2();
>
> + if (is_kernel_in_hyp_mode())
> + kvm_timer_init_vhe();
> +
> kvm_arm_init_debug();
> }
>
> diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
> index fea1073..b043cfd 100644
> --- a/arch/arm64/include/asm/virt.h
> +++ b/arch/arm64/include/asm/virt.h
> @@ -47,6 +47,7 @@
> #include <asm/ptrace.h>
> #include <asm/sections.h>
> #include <asm/sysreg.h>
> +#include <asm/cpufeature.h>
>
> /*
> * __boot_cpu_mode records what mode CPUs were booted in.
> @@ -80,6 +81,15 @@ static inline bool is_kernel_in_hyp_mode(void)
> return read_sysreg(CurrentEL) == CurrentEL_EL2;
> }
>
> +static inline bool has_vhe(void)
> +{
> +#ifdef CONFIG_ARM64_VHE
Is there a particular reason why this should be guarded by a #ifdef? All
the symbols should always be available, and since this is a static key,
the overhead should be really insignificant (provided that you use a
non-prehistoric compiler...).
> + if (cpus_have_const_cap(ARM64_HAS_VIRT_HOST_EXTN))
> + return true;
> +#endif
> + return false;
> +}
> +
> #ifdef CONFIG_ARM64_VHE
> extern void verify_cpu_run_el(void);
> #else
> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
> index dda39d8..2d54903 100644
> --- a/include/kvm/arm_arch_timer.h
> +++ b/include/kvm/arm_arch_timer.h
> @@ -76,4 +76,5 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
>
> void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu);
>
> +void kvm_timer_init_vhe(void);
> #endif
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 17b8fa5..c7c3bfd 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -24,6 +24,7 @@
>
> #include <clocksource/arm_arch_timer.h>
> #include <asm/arch_timer.h>
> +#include <asm/kvm_hyp.h>
>
> #include <kvm/arm_vgic.h>
> #include <kvm/arm_arch_timer.h>
> @@ -507,3 +508,25 @@ void kvm_timer_init(struct kvm *kvm)
> {
> kvm->arch.timer.cntvoff = kvm_phys_timer_read();
> }
> +
> +/*
> + * On VHE system, we only need to configure trap on physical timer and counter
> + * accesses in EL0 and EL1 once, not for every world switch.
> + * The host kernel runs at EL2 with HCR_EL2.TGE == 1,
> + * and this makes those bits have no effect for the host kernel execution.
> + */
> +void kvm_timer_init_vhe(void)
> +{
> + /* When HCR_EL2.E2H ==1, EL1PCEN and EL1PCTEN are shifted by 10 */
> + u32 cnthctl_shift = 10;
> + u64 val;
> +
> + /*
> + * Disallow physical timer access for the guest.
> + * Physical counter access is allowed.
> + */
> + val = read_sysreg(cnthctl_el2);
> + val &= ~(CNTHCTL_EL1PCEN << cnthctl_shift);
> + val |= (CNTHCTL_EL1PCTEN << cnthctl_shift);
> + write_sysreg(val, cnthctl_el2);
> +}
> diff --git a/virt/kvm/arm/hyp/timer-sr.c b/virt/kvm/arm/hyp/timer-sr.c
> index 798866a..63e28dd 100644
> --- a/virt/kvm/arm/hyp/timer-sr.c
> +++ b/virt/kvm/arm/hyp/timer-sr.c
> @@ -35,10 +35,16 @@ void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
> /* Disable the virtual timer */
> write_sysreg_el0(0, cntv_ctl);
>
> - /* Allow physical timer/counter access for the host */
> - val = read_sysreg(cnthctl_el2);
> - val |= CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN;
> - write_sysreg(val, cnthctl_el2);
> + /*
> + * We don't need to do this for VHE since the host kernel runs in EL2
> + * with HCR_EL2.TGE ==1, which makes those bits have no impact.
> + */
> + if (!has_vhe()) {
> + /* Allow physical timer/counter access for the host */
> + val = read_sysreg(cnthctl_el2);
> + val |= CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN;
> + write_sysreg(val, cnthctl_el2);
> + }
>
> /* Clear cntvoff for the host */
> write_sysreg(0, cntvoff_el2);
> @@ -50,14 +56,17 @@ void __hyp_text __timer_restore_state(struct kvm_vcpu *vcpu)
> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> u64 val;
>
> - /*
> - * Disallow physical timer access for the guest
> - * Physical counter access is allowed
> - */
> - val = read_sysreg(cnthctl_el2);
> - val &= ~CNTHCTL_EL1PCEN;
> - val |= CNTHCTL_EL1PCTEN;
> - write_sysreg(val, cnthctl_el2);
> + /* Those bits are already configured at boot on VHE-system */
> + if (!has_vhe()) {
> + /*
> + * Disallow physical timer access for the guest
> + * Physical counter access is allowed
> + */
> + val = read_sysreg(cnthctl_el2);
> + val &= ~CNTHCTL_EL1PCEN;
> + val |= CNTHCTL_EL1PCTEN;
> + write_sysreg(val, cnthctl_el2);
> + }
>
> if (timer->enabled) {
> write_sysreg(kvm->arch.timer.cntvoff, cntvoff_el2);
>
Otherwise, and assuming you're OK with me fixing the above nit (no need
to resend):
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [RFC PATCH 0/3] clk: at91: audio PLL clock
From: Nicolas Ferre @ 2016-12-06 11:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1481021115.git.nicolas.ferre@atmel.com>
Le 06/12/2016 ? 11:55, Nicolas Ferre a ?crit :
> This series covers the addition of the Audio PLL clock found on AT91 SoCs like
> the SAMA5D2.
> I also added the use of these clocks by the ClassD audi amplifier in both SoC
> and board DT.
>
> The Audio PLL is described in the sama5d2 datasheet chapter "29.8 Audio PLL".
>
> Even if "it works" (!), note that I'm not satisfied with the current code and
> would need some advice from people more accustomed with the CCF and
> particularly composite audio PLL/clocks like these. For example, I do not take
> into account the limits of these clocks (as described in the datasheet) and the
> dependency between the PAD and the PMC child clocks.
>
> Thanks in advance for your inputs.
Note as well that Boris already sent me a review of the first posting of
this patch ("[PATCH] clk: at91: add audio pll clock driver" on 31 Jul.
2015) and most of his comments are still relevant.
https://patchwork.kernel.org/patch/6910111/
https://www.spinics.net/lists/arm-kernel/msg436120.html
But I would like some comments about the whole architecture of these
clocks and how I could rearrange them and maybe use some of the common
facilities of the CCF (composite with gate + fractionnal? + divider?, etc.)
Regards,
> Cyrille Pitchen (2):
> ARM: dts: at91: sama5d2: add classd nodes
> ARM: dts: at91: sama5d2_xplained: add pin muxing and enable classd
>
> Nicolas Ferre (1):
> clk: at91: add audio pll clock driver
>
> .../devicetree/bindings/clock/at91-clock.txt | 10 +
> arch/arm/boot/dts/at91-sama5d2_xplained.dts | 16 ++
> arch/arm/boot/dts/sama5d2.dtsi | 39 +++-
> arch/arm/mach-at91/Kconfig | 4 +
> drivers/clk/at91/Makefile | 2 +
> drivers/clk/at91/clk-audio-pll-pad.c | 238 +++++++++++++++++++
> drivers/clk/at91/clk-audio-pll-pmc.c | 184 +++++++++++++++
> drivers/clk/at91/clk-audio-pll.c | 253 +++++++++++++++++++++
> include/linux/clk/at91_pmc.h | 25 ++
> 9 files changed, 770 insertions(+), 1 deletion(-)
> create mode 100644 drivers/clk/at91/clk-audio-pll-pad.c
> create mode 100644 drivers/clk/at91/clk-audio-pll-pmc.c
> create mode 100644 drivers/clk/at91/clk-audio-pll.c
>
--
Nicolas Ferre
^ permalink raw reply
* [PATCH v2 2/2] soc: zte: pm_domains: Add support for zx296718 board
From: Jun Nie @ 2016-12-06 11:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480987828-20549-2-git-send-email-baoyou.xie@linaro.org>
2016-12-06 9:30 GMT+08:00 Baoyou Xie <baoyou.xie@linaro.org>:
> This patch introduces the power domain driver of zx296718
> which belongs to zte's 2967 family.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> ---
> drivers/soc/zte/Makefile | 2 +-
> drivers/soc/zte/zx296718_pm_domains.c | 182 ++++++++++++++++++++++++++++++++++
> 2 files changed, 183 insertions(+), 1 deletion(-)
> create mode 100644 drivers/soc/zte/zx296718_pm_domains.c
>
> diff --git a/drivers/soc/zte/Makefile b/drivers/soc/zte/Makefile
> index 97ac8ea..2d2a2cc 100644
> --- a/drivers/soc/zte/Makefile
> +++ b/drivers/soc/zte/Makefile
> @@ -1,4 +1,4 @@
> #
> # zx SOC drivers
> #
> -obj-$(CONFIG_ZX_PM_DOMAINS) += pm_domains.o
> +obj-$(CONFIG_ZX_PM_DOMAINS) += pm_domains.o zx296718_pm_domains.o
> diff --git a/drivers/soc/zte/zx296718_pm_domains.c b/drivers/soc/zte/zx296718_pm_domains.c
> new file mode 100644
> index 0000000..ed18b33
> --- /dev/null
> +++ b/drivers/soc/zte/zx296718_pm_domains.c
> @@ -0,0 +1,182 @@
> +/*
> + * Copyright (C) 2015 ZTE Ltd.
> + *
> + * Author: Baoyou Xie <baoyou.xie@linaro.org>
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +#include <dt-bindings/arm/zte_pm_domains.h>
> +#include "pm_domains.h"
> +
> +static u16 zx296718_offsets[REG_ARRAY_SIZE] = {
> + [REG_CLKEN] = 0x18,
> + [REG_ISOEN] = 0x1c,
> + [REG_RSTEN] = 0x20,
> + [REG_PWREN] = 0x24,
> + [REG_ACK_SYNC] = 0x28,
> +};
> +
> +enum {
> + PCU_DM_VOU = 0,
> + PCU_DM_SAPPU,
> + PCU_DM_VDE,
> + PCU_DM_VCE,
> + PCU_DM_HDE,
> + PCU_DM_VIU,
> + PCU_DM_USB20,
> + PCU_DM_USB21,
> + PCU_DM_USB30,
> + PCU_DM_HSIC,
> + PCU_DM_GMAC,
> + PCU_DM_TS,
> +};
> +
> +static struct zx_pm_domain vou_domain = {
> + .dm = {
> + .name = "vou_domain",
> + .power_off = zx_normal_power_off,
> + .power_on = zx_normal_power_on,
> + },
> + .bit = PCU_DM_VOU,
> + .reg_offset = zx296718_offsets,
> +};
> +static struct zx_pm_domain sappu_domain = {
> + .dm = {
> + .name = "sappu_domain",
> + .power_off = zx_normal_power_off,
> + .power_on = zx_normal_power_on,
> + },
> + .bit = PCU_DM_SAPPU,
> + .reg_offset = zx296718_offsets,
> +};
> +static struct zx_pm_domain vde_domain = {
> + .dm = {
> + .name = "vde_domain",
> + .power_off = zx_normal_power_off,
> + .power_on = zx_normal_power_on,
> + },
> + .bit = PCU_DM_VDE,
> + .reg_offset = zx296718_offsets,
> +};
> +static struct zx_pm_domain vce_domain = {
> + .dm = {
> + .name = "vce_domain",
> + .power_off = zx_normal_power_off,
> + .power_on = zx_normal_power_on,
> + },
> + .bit = PCU_DM_VCE,
> + .reg_offset = zx296718_offsets,
> +};
> +static struct zx_pm_domain hde_domain = {
> + .dm = {
> + .name = "hde_domain",
> + .power_off = zx_normal_power_off,
> + .power_on = zx_normal_power_on,
> + },
> + .bit = PCU_DM_HDE,
> + .reg_offset = zx296718_offsets,
> +};
> +
> +static struct zx_pm_domain viu_domain = {
> + .dm = {
> + .name = "viu_domain",
> + .power_off = zx_normal_power_off,
> + .power_on = zx_normal_power_on,
> + },
> + .bit = PCU_DM_VIU,
> + .reg_offset = zx296718_offsets,
> +};
> +static struct zx_pm_domain usb20_domain = {
> + .dm = {
> + .name = "usb20_domain",
> + .power_off = zx_normal_power_off,
> + .power_on = zx_normal_power_on,
> + },
> + .bit = PCU_DM_USB20,
> + .reg_offset = zx296718_offsets,
> +};
> +static struct zx_pm_domain usb21_domain = {
> + .dm = {
> + .name = "usb21_domain",
> + .power_off = zx_normal_power_off,
> + .power_on = zx_normal_power_on,
> + },
> + .bit = PCU_DM_USB21,
> + .reg_offset = zx296718_offsets,
> +};
> +static struct zx_pm_domain usb30_domain = {
> + .dm = {
> + .name = "usb30_domain",
> + .power_off = zx_normal_power_off,
> + .power_on = zx_normal_power_on,
> + },
> + .bit = PCU_DM_USB30,
> + .reg_offset = zx296718_offsets,
> +};
> +static struct zx_pm_domain hsic_domain = {
> + .dm = {
> + .name = "hsic_domain",
> + .power_off = zx_normal_power_off,
> + .power_on = zx_normal_power_on,
> + },
> + .bit = PCU_DM_HSIC,
> + .reg_offset = zx296718_offsets,
> +};
> +static struct zx_pm_domain gmac_domain = {
> + .dm = {
> + .name = "gmac_domain",
> + .power_off = zx_normal_power_off,
> + .power_on = zx_normal_power_on,
> + },
> + .bit = PCU_DM_GMAC,
> + .reg_offset = zx296718_offsets,
> +};
> +static struct zx_pm_domain ts_domain = {
> + .dm = {
> + .name = "ts_domain",
> + .power_off = zx_normal_power_off,
> + .power_on = zx_normal_power_on,
> + },
> + .bit = PCU_DM_TS,
> + .reg_offset = zx296718_offsets,
> +};
> +struct generic_pm_domain *zx296718_pm_domains[] = {
> + [DM_ZX296718_SAPPU] = &sappu_domain.dm,
> + [DM_ZX296718_VDE] = &vde_domain.dm,
> + [DM_ZX296718_VCE] = &vce_domain.dm,
> + [DM_ZX296718_HDE] = &hde_domain.dm,
> + [DM_ZX296718_VIU] = &viu_domain.dm,
> + [DM_ZX296718_USB20] = &usb20_domain.dm,
> + [DM_ZX296718_USB21] = &usb21_domain.dm,
> + [DM_ZX296718_USB30] = &usb30_domain.dm,
> + [DM_ZX296718_HSIC] = &hsic_domain.dm,
> + [DM_ZX296718_GMAC] = &gmac_domain.dm,
> + [DM_ZX296718_TS] = &ts_domain.dm,
> + [DM_ZX296718_VOU] = &vou_domain.dm,
> +};
> +
> +static int zx296718_pd_probe(struct platform_device *pdev)
> +{
> + return zx_pd_probe(pdev,
> + zx296718_pm_domains,
> + ARRAY_SIZE(zx296718_pm_domains));
> +}
> +
> +static const struct of_device_id zx296718_pm_domain_matches[] = {
> + { .compatible = "zte,zx296718-pcu", },
> + { },
> +};
> +
> +static struct platform_driver zx296718_pd_driver = {
> + .driver = {
> + .name = "zx-powerdomain",
> + .owner = THIS_MODULE,
> + .of_match_table = zx296718_pm_domain_matches,
> + },
> + .probe = zx296718_pd_probe,
> +};
> +
> +static int __init zx296718_pd_init(void)
> +{
> + return platform_driver_register(&zx296718_pd_driver);
> +}
> +subsys_initcall(zx296718_pd_init);
> --
> 2.7.4
>
Reviewed-by: Jun Nie <jun.nie@linaro.org>
^ permalink raw reply
* [PATCH 8/9] mfd: wm97xx-core: core support for wm97xx Codec
From: Mark Brown @ 2016-12-06 11:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87y406ppjl.fsf@belgarion.home>
On Sat, Nov 26, 2016 at 10:18:38AM +0100, Robert Jarzmik wrote:
> >> >> +#define WM9705_VENDOR_ID 0x574d4c05
> >> >> +#define WM9712_VENDOR_ID 0x574d4c12
> >> >> +#define WM9713_VENDOR_ID 0x574d4c13
> >> >> +#define WM97xx_VENDOR_ID_MASK 0xffffffff
> >> > These are probably better represented as enums.
> >> These are ids, just as devicetree ids, or PCI ids, I don't think an enum will
> >> fit.
> > That's fine. We can use enums to simply group items of a similar
> > type. Representing these as enums would only serve to benefit code
> > cleanliness. What makes you think they won't fit?
That would be like having all PCI or USB identifiers in an enum, it's
doable but it means having one big table that all drivers need to
modify.
> >> >> +static int wm97xx_ac97_probe(struct ac97_codec_device *adev)
> >> As for the "sound specific part", it's because AC97 bus is mainly used in sound
> >> oriented drivers, but still the codec IPs provide more than just sound, as the
> >> Wolfson codecs for instance.
> > I'd like to get Mark Brown's opinion on this.
I'm not sure what the question is, sorry.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161206/bb87a688/attachment.sig>
^ permalink raw reply
* [PATCH v2 1/2] soc: zte: pm_domains: Prepare for supporting ARMv8 2967 family
From: Jun Nie @ 2016-12-06 11:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480987828-20549-1-git-send-email-baoyou.xie@linaro.org>
2016-12-06 9:30 GMT+08:00 Baoyou Xie <baoyou.xie@linaro.org>:
> The ARMv8 2967 family (296718, 296716 etc) uses different value
> for controlling the power domain on/off registers, Choose the
> value depending on the compatible.
>
> Multiple domains are prepared for the family, they'are privated
> by the drivers of boards.
Domains are SoC specific, not board, right?
>
> This patch prepares the common functions.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> ---
> drivers/soc/Kconfig | 1 +
> drivers/soc/Makefile | 1 +
> drivers/soc/zte/Kconfig | 13 ++++
> drivers/soc/zte/Makefile | 4 ++
> drivers/soc/zte/pm_domains.c | 140 +++++++++++++++++++++++++++++++++++++++++++
> drivers/soc/zte/pm_domains.h | 41 +++++++++++++
> 6 files changed, 200 insertions(+)
> create mode 100644 drivers/soc/zte/Kconfig
> create mode 100644 drivers/soc/zte/Makefile
> create mode 100644 drivers/soc/zte/pm_domains.c
> create mode 100644 drivers/soc/zte/pm_domains.h
>
> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> index f31bceb..f09023f 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -11,5 +11,6 @@ source "drivers/soc/tegra/Kconfig"
> source "drivers/soc/ti/Kconfig"
> source "drivers/soc/ux500/Kconfig"
> source "drivers/soc/versatile/Kconfig"
> +source "drivers/soc/zte/Kconfig"
>
> endmenu
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 50c23d0..05eae52 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_ARCH_TEGRA) += tegra/
> obj-$(CONFIG_SOC_TI) += ti/
> obj-$(CONFIG_ARCH_U8500) += ux500/
> obj-$(CONFIG_PLAT_VERSATILE) += versatile/
> +obj-$(CONFIG_ARCH_ZX) += zte/
> diff --git a/drivers/soc/zte/Kconfig b/drivers/soc/zte/Kconfig
> new file mode 100644
> index 0000000..4953c3fa
> --- /dev/null
> +++ b/drivers/soc/zte/Kconfig
> @@ -0,0 +1,13 @@
> +#
> +# zx SoC drivers
> +#
> +menuconfig SOC_ZX
> + bool "zx SoC driver support"
> +
> +if SOC_ZX
> +
> +config ZX_PM_DOMAINS
> + bool "zx PM domains"
> + depends on PM_GENERIC_DOMAINS
> +
> +endif
> diff --git a/drivers/soc/zte/Makefile b/drivers/soc/zte/Makefile
> new file mode 100644
> index 0000000..97ac8ea
> --- /dev/null
> +++ b/drivers/soc/zte/Makefile
> @@ -0,0 +1,4 @@
> +#
> +# zx SOC drivers
> +#
> +obj-$(CONFIG_ZX_PM_DOMAINS) += pm_domains.o
> diff --git a/drivers/soc/zte/pm_domains.c b/drivers/soc/zte/pm_domains.c
> new file mode 100644
> index 0000000..956c963
> --- /dev/null
> +++ b/drivers/soc/zte/pm_domains.c
> @@ -0,0 +1,140 @@
> +/*
> + * Copyright (C) 2015 ZTE Ltd.
> + *
> + * Author: Baoyou Xie <baoyou.xie@linaro.org>
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include "pm_domains.h"
> +
> +#define PCU_DM_CLKEN(zpd) ((zpd)->reg_offset[REG_CLKEN])
> +#define PCU_DM_ISOEN(zpd) ((zpd)->reg_offset[REG_ISOEN])
> +#define PCU_DM_RSTEN(zpd) ((zpd)->reg_offset[REG_RSTEN])
> +#define PCU_DM_PWREN(zpd) ((zpd)->reg_offset[REG_PWREN])
> +#define PCU_DM_ACK_SYNC(zpd) ((zpd)->reg_offset[REG_ACK_SYNC])
> +
> +static void __iomem *pcubase;
> +
> +int zx_normal_power_on(struct generic_pm_domain *domain)
> +{
> + struct zx_pm_domain *zpd = (struct zx_pm_domain *)domain;
> + unsigned long loop = 20000;
> + u32 val;
> +
> + loop = 1000;
Why change loop value here? You can initialize it to 1000 above.
> + do {
> + val = readl_relaxed(pcubase + PCU_DM_PWREN(zpd));
> + val |= BIT(zpd->bit);
> + writel_relaxed(val, pcubase + PCU_DM_PWREN(zpd));
Can we put this bit set out of loop?
Suggest to support 6702 with this driver via supporting PWREN
polarity, because all other logic are shared among these ZX SoCs.
> +
> + udelay(1);
> + val = readl_relaxed(pcubase
> + + PCU_DM_ACK_SYNC(zpd)) & BIT(zpd->bit);
> + } while (--loop && !val);
> +
> + if (!loop) {
> + pr_err("Error: %s %s fail\n", __func__, domain->name);
> + return -EIO;
> + }
> +
> + val = readl_relaxed(pcubase + PCU_DM_RSTEN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val | BIT(zpd->bit), pcubase + PCU_DM_RSTEN(zpd));
> + udelay(5);
> +
> + val = readl_relaxed(pcubase + PCU_DM_ISOEN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val, pcubase + PCU_DM_ISOEN(zpd));
> + udelay(5);
> +
> + val = readl_relaxed(pcubase + PCU_DM_CLKEN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val | BIT(zpd->bit), pcubase + PCU_DM_CLKEN(zpd));
> + udelay(5);
> +
> + pr_info("normal poweron %s\n", domain->name);
> +
> + return 0;
> +}
> +
> +int zx_normal_power_off(struct generic_pm_domain *domain)
> +{
> + struct zx_pm_domain *zpd = (struct zx_pm_domain *)domain;
> + unsigned long loop = 1000;
> + u32 val;
> +
> + val = readl_relaxed(pcubase + PCU_DM_CLKEN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val, pcubase + PCU_DM_CLKEN(zpd));
> + udelay(5);
> +
> + val = readl_relaxed(pcubase + PCU_DM_ISOEN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val | BIT(zpd->bit), pcubase + PCU_DM_ISOEN(zpd));
> + udelay(5);
> +
> + val = readl_relaxed(pcubase + PCU_DM_RSTEN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val, pcubase + PCU_DM_RSTEN(zpd));
> + udelay(5);
> +
> + loop = 1000;
> + do {
> + val = readl_relaxed(pcubase + PCU_DM_PWREN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val, pcubase + PCU_DM_PWREN(zpd));
Ditto.
> +
> + udelay(1);
> +
> + val = readl_relaxed(pcubase
> + + PCU_DM_ACK_SYNC(zpd)) & BIT(zpd->bit);
> + } while (--loop && val);
> +
> + if (!loop) {
> + pr_err("Error: %s %s fail\n", __func__, domain->name);
> + return -EIO;
> + }
> +
> + pr_info("normal poweroff %s\n", domain->name);
> +
> + return 0;
> +}
> +
> +int
> +zx_pd_probe(struct platform_device *pdev,
> + struct generic_pm_domain **zx_pm_domains,
> + int domain_num)
> +{
> + struct genpd_onecell_data *genpd_data;
> + struct resource *res;
> + int i;
> +
> + genpd_data = devm_kzalloc(&pdev->dev, sizeof(*genpd_data), GFP_KERNEL);
> + if (!genpd_data)
> + return -ENOMEM;
> +
> + genpd_data->domains = zx_pm_domains;
> + genpd_data->num_domains = domain_num;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "no memory resource defined\n");
> + return -ENODEV;
> + }
> +
> + pcubase = devm_ioremap_resource(&pdev->dev, res);
> + if (!pcubase) {
Check error with:
if (IS_ERR(pcubase) {
> + dev_err(&pdev->dev, "ioremap fail.\n");
> + return -EIO;
> + }
> +
> + for (i = 0; i < domain_num; ++i)
> + pm_genpd_init(zx_pm_domains[i], NULL, false);
> +
> + of_genpd_add_provider_onecell(pdev->dev.of_node, genpd_data);
> + dev_info(&pdev->dev, "powerdomain init ok\n");
> + return 0;
> +}
> diff --git a/drivers/soc/zte/pm_domains.h b/drivers/soc/zte/pm_domains.h
> new file mode 100644
> index 0000000..e02518e
> --- /dev/null
> +++ b/drivers/soc/zte/pm_domains.h
> @@ -0,0 +1,41 @@
> +/*
> + * Copyright (c) 2015 ZTE Co., Ltd.
> + * http://www.zte.com.cn
> + *
> + * Header for ZTE's Power Domain Driver support
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __ZTE_PM_DOMAIN_H
> +#define __ZTE_PM_DOMAIN_H
> +
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +
> +enum {
> + REG_CLKEN,
> + REG_ISOEN,
> + REG_RSTEN,
> + REG_PWREN,
> + REG_ACK_SYNC,
> +
> + /* The size of the array - must be last */
> + REG_ARRAY_SIZE,
> +};
> +
> +struct zx_pm_domain {
> + struct generic_pm_domain dm;
> + const u16 bit;
> + const u16 *reg_offset;
> +};
> +
> +extern int zx_normal_power_on(struct generic_pm_domain *domain);
> +extern int zx_normal_power_off(struct generic_pm_domain *domain);
> +extern int
> +zx_pd_probe(struct platform_device *pdev,
> + struct generic_pm_domain **zx_pm_domains,
> + int domain_num);
> +#endif /* __ZTE_PM_DOMAIN_H */
> --
> 2.7.4
>
^ permalink raw reply
* [LINUX RFC v4 3/4] mtd: spi-nor: add stripe support
From: Cyrille Pitchen @ 2016-12-06 11:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <DA6901612C71B84D91459DE817C418AE26E0C61B@XAP-PVEXMBX01.xlnx.xilinx.com>
Le 06/12/2016 ? 07:54, Naga Sureshkumar Relli a ?crit :
> Hi Cyrille,
>
>> -----Original Message-----
>> From: Cyrille Pitchen [mailto:cyrille.pitchen at atmel.com]
>> Sent: Monday, December 05, 2016 6:34 PM
>> To: Naga Sureshkumar Relli <nagasure@xilinx.com>; broonie at kernel.org;
>> michal.simek at xilinx.com; Soren Brinkmann <sorenb@xilinx.com>; Harini
>> Katakam <harinik@xilinx.com>; Punnaiah Choudary Kalluri
>> <punnaia@xilinx.com>
>> Cc: linux-spi at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
>> kernel at vger.kernel.org; linux-mtd at lists.infradead.org
>> Subject: Re: [LINUX RFC v4 3/4] mtd: spi-nor: add stripe support
>>
>> Hi Naga,
>>
>> Le 05/12/2016 ? 08:02, Naga Sureshkumar Relli a ?crit :
>>> Hi Cyrille,
>>>
>>>>> Hi Cyrille,
>>>>>
>>>>>> I have not finished to review the whole series yet but here some
>>>>>> first
>>>>>> comments:
>>>>>
>>>>> Thanks for reviewing these patch series.
>>>>>
>>>>>>
>>>>>> Le 27/11/2016 ? 09:33, Naga Sureshkumar Relli a ?crit :
>>>>>>> This patch adds stripe support and it is needed for GQSPI parallel
>>>>>>> configuration mode by:
>>>>>>>
>>>>>>> - Adding required parameters like stripe and shift to spi_nor
>>>>>>> structure.
>>>>>>> - Initializing all added parameters in spi_nor_scan()
>>>>>>> - Updating read_sr() and read_fsr() for getting status from both
>>>>>>> flashes
>>>>>>> - Increasing page_size, sector_size, erase_size and toatal flash
>>>>>>> size as and when required.
>>>>>>> - Dividing address by 2
>>>>>>> - Updating spi->master->flags for qspi driver to change CS
>>>>>>>
>>>>>>> Signed-off-by: Naga Sureshkumar Relli <nagasure@xilinx.com>
>>>>>>> ---
>>>>>>> Changes for v4:
>>>>>>> - rename isparallel to stripe
>>>>>>> Changes for v3:
>>>>>>> - No change
>>>>>>> Changes for v2:
>>>>>>> - Splitted to separate MTD layer changes from SPI core changes
>>>>>>> ---
>>>>>>> drivers/mtd/spi-nor/spi-nor.c | 130
>>>>>> ++++++++++++++++++++++++++++++++----------
>>>>>>> include/linux/mtd/spi-nor.h | 2 +
>>>>>>> 2 files changed, 103 insertions(+), 29 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c
>>>>>>> b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..4252239 100644
>>>>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>>>>> @@ -22,6 +22,7 @@
>>>>>>> #include <linux/of_platform.h>
>>>>>>> #include <linux/spi/flash.h>
>>>>>>> #include <linux/mtd/spi-nor.h>
>>>>>>> +#include <linux/spi/spi.h>
>>>>>>>
>>>>>>> /* Define max times to check status register before we give up.
>>>>>>> */
>>>>>>>
>>>>>>> @@ -89,15 +90,24 @@ static const struct flash_info
>>>>>>> *spi_nor_match_id(const char *name); static int read_sr(struct
>>>>>>> spi_nor *nor) {
>>>>>>> int ret;
>>>>>>> - u8 val;
>>>>>>> + u8 val[2];
>>>>>>>
>>>>>>> - ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val, 1);
>>>>>>> - if (ret < 0) {
>>>>>>> - pr_err("error %d reading SR\n", (int) ret);
>>>>>>> - return ret;
>>>>>>> + if (nor->stripe) {
>>>>>>> + ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val[0], 2);
>>>>>>> + if (ret < 0) {
>>>>>>> + pr_err("error %d reading SR\n", (int) ret);
>>>>>>> + return ret;
>>>>>>> + }
>>>>>>> + val[0] |= val[1];
>>>>>> Why '|' rather than '&' ?
>>>>>> I guess because of the 'Write In Progress/Busy' bit: when called by
>>>>>> spi_nor_sr_ready(), you want to be sure that this 'BUSY' bit is
>>>>>> cleared on both memories.
>>>>>>
>>>>>> But what about when the Status Register is read for purpose other
>>>>>> than checking the state of the 'BUSY' bit?
>>>>>>
>>>>> Yes you are correct, I will change this.
>>>>>
>>>>>> What about SPI controllers supporting more than 2 memories in
>> parallel?
>>>>>>
>>>>>> This solution might fit the ZynqMP controller but doesn't look so
>> generic.
>>>>>>
>>>>>>> + } else {
>>>>>>> + ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val[0], 1);
>>>>>>> + if (ret < 0) {
>>>>>>> + pr_err("error %d reading SR\n", (int) ret);
>>>>>>> + return ret;
>>>>>>> + }
>>>>>>> }
>>>>>>>
>>>>>>> - return val;
>>>>>>> + return val[0];
>>>>>>> }
>>>>>>>
>>>>>>> /*
>>>>>>> @@ -108,15 +118,24 @@ static int read_sr(struct spi_nor *nor)
>>>>>>> static int read_fsr(struct spi_nor *nor) {
>>>>>>> int ret;
>>>>>>> - u8 val;
>>>>>>> + u8 val[2];
>>>>>>>
>>>>>>> - ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val, 1);
>>>>>>> - if (ret < 0) {
>>>>>>> - pr_err("error %d reading FSR\n", ret);
>>>>>>> - return ret;
>>>>>>> + if (nor->stripe) {
>>>>>>> + ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val[0], 2);
>>>>>>> + if (ret < 0) {
>>>>>>> + pr_err("error %d reading FSR\n", ret);
>>>>>>> + return ret;
>>>>>>> + }
>>>>>>> + val[0] &= val[1];
>>>>>> Same comment here: why '&' rather than '|'?
>>>>>> Surely because of the the 'READY' bit which should be set for both
>>>> memories.
>>>>> I will update this also.
>>>>>>
>>>>>>> + } else {
>>>>>>> + ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val[0], 1);
>>>>>>> + if (ret < 0) {
>>>>>>> + pr_err("error %d reading FSR\n", ret);
>>>>>>> + return ret;
>>>>>>> + }
>>>>>>> }
>>>>>>>
>>>>>>> - return val;
>>>>>>> + return val[0];
>>>>>>> }
>>>>>>>
>>>>>>> /*
>>>>>>> @@ -290,9 +309,16 @@ static int spi_nor_wait_till_ready(struct
>>>>>>> spi_nor
>>>>>> *nor)
>>>>>>> */
>>>>>>> static int erase_chip(struct spi_nor *nor) {
>>>>>>> + u32 ret;
>>>>>>> +
>>>>>>> dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >>
>>>>>>> 10));
>>>>>>>
>>>>>>> - return nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
>>>>>>> + ret = nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0); if
>>>>>>> + (ret)
>>>>>>> + return ret;
>>>>>>> +
>>>>>>> + return ret;
>>>>>>> +
>>>>>>
>>>>>> if (ret)
>>>>>> return ret;
>>>>>> else
>>>>>> return ret;
>>>>>>
>>>>>> This chunk should be removed, it doesn't ease the patch review ;)
>>>>> Ok, I will remove.
>>>>>>
>>>>>>> }
>>>>>>>
>>>>>>> static int spi_nor_lock_and_prep(struct spi_nor *nor, enum
>>>>>>> spi_nor_ops ops) @@ -349,7 +375,7 @@ static int
>>>>>>> spi_nor_erase_sector(struct spi_nor *nor, u32 addr) static int
>>>>>>> spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) {
>>>>>>> struct spi_nor *nor = mtd_to_spi_nor(mtd);
>>>>>>> - u32 addr, len;
>>>>>>> + u32 addr, len, offset;
>>>>>>> uint32_t rem;
>>>>>>> int ret;
>>>>>>>
>>>>>>> @@ -399,9 +425,13 @@ static int spi_nor_erase(struct mtd_info
>>>>>>> *mtd,
>>>>>> struct erase_info *instr)
>>>>>>> /* "sector"-at-a-time erase */
>>>>>>> } else {
>>>>>>> while (len) {
>>>>>>> +
>>>>>>> write_enable(nor);
>>>>>>> + offset = addr;
>>>>>>> + if (nor->stripe)
>>>>>>> + offset /= 2;
>>>>>>
>>>>>> I guess this should be /= 4 for controllers supporting 4 memories
>>>>>> in
>>>> parallel.
>>>>>> Shouldn't you use nor->shift and define shift as an unsigned int
>>>>>> instead of a bool?
>>>>>> offset >>= nor->shift;
>>>>>>
>>>>> Yes we can use this shift, I will update
>>>>>
>>>>>> Anyway, by tuning the address here in spi-nor.c rather than in the
>>>>>> SPI controller driver, you impose a model to support parallel
>>>>>> memories that might not be suited to other controllers.
>>>>>
>>>>> For this ZynqMP GQSPI controller parallel configuration, globally
>>>>> spi-nor should know about this stripe feature And based on that
>>>>> address
>>>> has to change.
>>>>> As I mentioned in cover letter, this controller in parallel
>>>>> configuration will
>>>> work with even addresses only.
>>>>> i.e. Before creating address format(m25p_addr2cmd) in mtd layer,
>>>>> spi-nor
>>>> should change that address based on stripe option.
>>>>>
>>>>> I am updating this offset based on stripe option, and stripe option
>>>>> will
>>>> update by reading dt property in nor_scan().
>>>>> So the controller which doesn't support, then the stripe will be zero.
>>>>> Or Can you please suggest any other way?
>>>>>
>>>>>>
>>>>>>>
>>>>>>> - ret = spi_nor_erase_sector(nor, addr);
>>>>>>> + ret = spi_nor_erase_sector(nor, offset);
>>>>>>> if (ret)
>>>>>>> goto erase_err;
>>>>>>>
>>>>>>> @@ -525,6 +555,8 @@ static int stm_lock(struct spi_nor *nor,
>>>>>>> loff_t ofs,
>>>>>> uint64_t len)
>>>>>>> bool use_top;
>>>>>>> int ret;
>>>>>>>
>>>>>>> + ofs = ofs >> nor->shift;
>>>>>>> +
>>>>>>> status_old = read_sr(nor);
>>>>>>> if (status_old < 0)
>>>>>>> return status_old;
>>>>>>> @@ -610,6 +642,8 @@ static int stm_unlock(struct spi_nor *nor,
>>>>>>> loff_t ofs,
>>>>>> uint64_t len)
>>>>>>> bool use_top;
>>>>>>> int ret;
>>>>>>>
>>>>>>> + ofs = ofs >> nor->shift;
>>>>>>> +
>>>>>>> status_old = read_sr(nor);
>>>>>>> if (status_old < 0)
>>>>>>> return status_old;
>>>>>>> @@ -709,6 +743,8 @@ static int spi_nor_lock(struct mtd_info *mtd,
>>>>>>> loff_t
>>>>>> ofs, uint64_t len)
>>>>>>> if (ret)
>>>>>>> return ret;
>>>>>>>
>>>>>>> + ofs = ofs >> nor->shift;
>>>>>>> +
>>>>>>> ret = nor->flash_lock(nor, ofs, len);
>>>>>>>
>>>>>>> spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_UNLOCK); @@ -
>>>>>> 724,6 +760,8
>>>>>>> @@ static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs,
>>>>>>> uint64_t
>>>>>> len)
>>>>>>> if (ret)
>>>>>>> return ret;
>>>>>>>
>>>>>>> + ofs = ofs >> nor->shift;
>>>>>>> +
>>>>>>> ret = nor->flash_unlock(nor, ofs, len);
>>>>>>>
>>>>>>> spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_LOCK); @@ -
>>>>>> 1018,6 +1056,9
>>>>>>> @@ static const struct flash_info *spi_nor_read_id(struct spi_nor
>> *nor)
>>>>>>> u8 id[SPI_NOR_MAX_ID_LEN];
>>>>>>> const struct flash_info *info;
>>>>>>>
>>>>>>> + nor->spi->master->flags &= ~(SPI_MASTER_BOTH_CS |
>>>>>>> + SPI_MASTER_DATA_STRIPE);
>>>>>>> +
>>>>>>> tmp = nor->read_reg(nor, SPINOR_OP_RDID, id,
>>>>>> SPI_NOR_MAX_ID_LEN);
>>>>>>> if (tmp < 0) {
>>>>>>> dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
>>>>>> @@ -1041,6
>>>>>>> +1082,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t
>>>>>>> +from,
>>>>>>> size_t len, {
>>>>>>> struct spi_nor *nor = mtd_to_spi_nor(mtd);
>>>>>>> int ret;
>>>>>>> + u32 offset = from;
>>>>>>>
>>>>>>> dev_dbg(nor->dev, "from 0x%08x, len %zd\n", (u32)from, len);
>>>>>>>
>>>>>>> @@ -1049,7 +1091,13 @@ static int spi_nor_read(struct mtd_info
>>>>>>> *mtd,
>>>>>> loff_t from, size_t len,
>>>>>>> return ret;
>>>>>>>
>>>>>>> while (len) {
>>>>>>> - ret = nor->read(nor, from, len, buf);
>>>>>>> +
>>>>>>> + offset = from;
>>>>>>> +
>>>>>>> + if (nor->stripe)
>>>>>>> + offset /= 2;
>>>>>>> +
>>>>>>> + ret = nor->read(nor, offset, len, buf);
>>>>>>> if (ret == 0) {
>>>>>>> /* We shouldn't see 0-length reads */
>>>>>>> ret = -EIO;
>>>>>>> @@ -1161,6 +1209,7 @@ static int spi_nor_write(struct mtd_info
>>>>>>> *mtd,
>>>>>> loff_t to, size_t len,
>>>>>>> struct spi_nor *nor = mtd_to_spi_nor(mtd);
>>>>>>> size_t page_offset, page_remain, i;
>>>>>>> ssize_t ret;
>>>>>>> + u32 offset;
>>>>>>>
>>>>>>> dev_dbg(nor->dev, "to 0x%08x, len %zd\n", (u32)to, len);
>>>>>>>
>>>>>>> @@ -1178,9 +1227,13 @@ static int spi_nor_write(struct mtd_info
>>>>>>> *mtd,
>>>>>> loff_t to, size_t len,
>>>>>>> /* the size of data remaining on the first page */
>>>>>>> page_remain = min_t(size_t,
>>>>>>> nor->page_size - page_offset, len -
>>>>>>> i);
>>>>>>> + offset = (to + i);
>>>>>>> +
>>>>>>> + if (nor->stripe)
>>>>>>> + offset /= 2;
>>>>>>>
>>>>>>> write_enable(nor);
>>>>>>> - ret = nor->write(nor, to + i, page_remain, buf + i);
>>>>>>> + ret = nor->write(nor, (offset), page_remain, buf + i);
>>>>>>> if (ret < 0)
>>>>>>> goto write_err;
>>>>>>> written = ret;
>>>>>>> @@ -1302,22 +1355,22 @@ static int spi_nor_check(struct spi_nor
>>>>>>> *nor)
>>>>>>>
>>>>>>> int spi_nor_scan(struct spi_nor *nor, const char *name, enum
>>>>>>> read_mode mode) {
>>>>>>> - const struct flash_info *info = NULL;
>>>>>>> + struct flash_info *info = NULL;
>>>>>>
>>>>>> You should not remove the const and should not try to modify
>>>>>> members of *info.
>>>>>>
>>>>>>> struct device *dev = nor->dev;
>>>>>>> struct mtd_info *mtd = &nor->mtd;
>>>>>>> struct device_node *np = spi_nor_get_flash_node(nor);
>>>>>>> - int ret;
>>>>>>> - int i;
>>>>>>> + struct device_node *np_spi;
>>>>>>> + int ret, i, xlnx_qspi_mode;
>>>>>>>
>>>>>>> ret = spi_nor_check(nor);
>>>>>>> if (ret)
>>>>>>> return ret;
>>>>>>>
>>>>>>> if (name)
>>>>>>> - info = spi_nor_match_id(name);
>>>>>>> + info = (struct flash_info *)spi_nor_match_id(name);
>>>>>>> /* Try to auto-detect if chip name wasn't specified or not found */
>>>>>>> if (!info)
>>>>>>> - info = spi_nor_read_id(nor);
>>>>>>> + info = (struct flash_info *)spi_nor_read_id(nor);
>>>>>>> if (IS_ERR_OR_NULL(info))
>>>>>>> return -ENOENT;
>>>>>>>
>>>>>> Both spi_nor_match_id() and spi_nor_read_id(), when they succeed,
>>>>>> return a pointer to an entry of the spi_nor_ids[] array, which is
>>>>>> located in a read- only memory area.
>>>>>>
>>>>>> Since your patch doesn't remove the const attribute of the
>>>>>> spi_nor_ids[], I wonder whether it has been tested. I expect it not
>>>>>> to work on most architecture.
>>>>>>
>>>>>> Anyway spi_nor_ids[] should remain const. Let's take the case of
>>>>>> eXecution In Place (XIP) from an external memory: if spi_nor_ids[]
>>>>>> is const, it can be read directly from this external (read-only)
>>>>>> memory and we never need to copy the array in RAM, so we save
>> some
>>>>>> KB of
>>>> RAM.
>>>>>> This is just an example but I guess we can find other reasons to
>>>>>> keep this array const.
>>>>>>
>>>>>>> @@ -1341,7 +1394,7 @@ int spi_nor_scan(struct spi_nor *nor, const
>>>>>>> char
>>>>>> *name, enum read_mode mode)
>>>>>>> */
>>>>>>> dev_warn(dev, "found %s, expected %s\n",
>>>>>>> jinfo->name, info->name);
>>>>>>> - info = jinfo;
>>>>>>> + info = (struct flash_info *)jinfo;
>>>>>>> }
>>>>>>> }
>>>>>>>
>>>>>>> @@ -1370,6 +1423,27 @@ int spi_nor_scan(struct spi_nor *nor, const
>>>>>>> char
>>>>>> *name, enum read_mode mode)
>>>>>>> mtd->size = info->sector_size * info->n_sectors;
>>>>>>> mtd->_erase = spi_nor_erase;
>>>>>>> mtd->_read = spi_nor_read;
>>>>>>> +#ifdef CONFIG_OF
>>>>>>> + np_spi = of_get_next_parent(np);
>>>>>>> +
>>>>>>> + if (of_property_read_u32(np_spi, "xlnx,qspi-mode",
>>>>>>> + &xlnx_qspi_mode) < 0) {
>>>>>> This really looks controller specific so should not be placed in
>>>>>> the generic spi- nor.c file.
>>>>>
>>>>> Const is removed in info struct, because to update info members
>>>>> based
>>>> parallel configuration.
>>>>> As I mentioned above, for this parallel configuration, mtd and
>>>>> spi-nor should know the details like
>>>>> mtd->size, info->sectors, sector_size and page_size.
>>>>
>>>> You can tune the values of nor->mtd.size, nor->mtd.erasesize, nor-
>>>>> page_size or whatever member of nor/nor.mtd as needed without ever
>>>> modifying members of *info.
>>>>
>>>> If you modify *info then spi_nor_scan() is called a 2nd time to probe
>>>> and configure SPI memories of the same part but connected to another
>>>> controller, the values of the modified members in this *info would
>>>> not be those expected.
>>>> So *info and the spi_nor_ids[] array must remain const.
>>>>
>>>> The *info structure is not used outside spi_nor_scan(); none of
>>>> spi_nor_read(),
>>>> spi_nor_write() and spi_nor_erase() refers to *info hence every
>>>> relevant value can be set only nor or nor->mtd members.
>>>>
>>>>
>>>> Anyway, I think OR'ing or AND'ing values of memory registers
>>>> depending on the relevant bit we want to check is not the right solution.
>>>> If done in spi-nor.c, there would be a specific case for each memory
>>>> register we read, each register bit would have to be handled differently.
>>>>
>>>> spi-nor.c tries to support as much memory parts as possible, it deals
>>>> with many registers and bits: Status/Control registers, Quad Enable bits...
>>>>
>>>> If we start to OR or AND each of these register values to support the
>>>> stripping mode, spi-nor will become really hard to maintain.
>>>>
>>>> I don't know whether it could be done with the xilinx controller but
>>>> I thought about first configuring the two memories independently
>>>> calling
>>>> spi_nor_scan() twice; one call for each memory.
>>>>
>>>> Then the xilinx driver could register only one struct mtd_info,
>>>> overriding
>>>> mtd->_read() [and likely mtd->_write() and mtd->_erase() too] set by
>>>> spi_nor_scan() with a xilinx driver custom implementation so this
>>>> driver supports its controller stripping mode as it wants.
>>>>
>>>> Of course, this solution assumes that the SPI controller has one
>>>> dedicated chip-select line for each memory and not a single
>>>> chip-select line shared by both memories. The memories should be
>>>> configured independently: you can't assume multiple instances of the
>>>> very same memory part always return the exact same value when reading
>>>> one of their register. Logical AND/OR is not a generic solution, IMHO.
>>>>
>>>> If the xilinx controller has only one shared chip-select line then
>>>> let's see whether 2 GPIO lines could be used as independent chip-select
>> lines.
>>>>
>>>>
>>> In parallel configuration, Physically we have two flashes but mtd will
>>> see as single flash memory (sum of both memories) If we call
>> spi_nor_scan(), twice then read/write will override but nor->mtd.size, nor-
>>> mtd.erasesize, nor->page_size will remain same, I,e they will also override,
>> they won't append.
>>> I tried calling spi_nor_scan() twice by some hacks but nor->mtd.size,
>>> nor->mtd.erasesize, nor->page_size are not changing Also the same issue
>> we are getting for flash address, need to shift the address to work in this
>> configuration.
>>> Also to tune nor->mtd.size, nor->mtd.erasesize, nor->page_size, we
>>> need to touch this spi-nor.c
>>>
>>> Please kindly suggest, if I am wrong.
>>>
>>
>> What I've been suggesting is:
>>
>> {
>> struct spi_nor *nor1, *nor2;
>> struct mtd_info *mtd;
>> enum read_mode mode = SPI_NOR_QUAD;
>> int err;
>>
>> [...]
>>
>> err = spi_nor_scan(nor1, NULL, mode);
>> if (err)
>> return err; /* or handle error properly. */
>>
>> err = spi_nor_scan(nor2, NULL, mode);
>> if (err)
>> return err;
>>
>> mtd = &nor1->mtd;
>> mtd->erasesize <<= 1;
>> mtd->size <<= 1;
>> mtd->writebufsize <<= 1;
>> nor1->page_size <<= 1;
>> /* tune all other relevant members of nor1/mtd. */
>>
>> /* override relevant mtd hooks. */
>> mtd->_read = stripping_read;
>> mtd->_erase = stripping_erase;
>> mtd->_write = stripping_write;
>> mtd->_lock = ...;
>> mtd->_unlock = ...;
>> mtd->_is_lock = ...;
>>
>> /* register a single mtd_info structure. */
>> err = mtd_device_register(mtd, NULL, 0);
>> if (err)
>> return err;
>>
>> [...]
>> }
>>
>
> It's really good for us to have our controller specific mtd hooks instead of changing the layer calls and thanks for this suggestion.
> But spi-zynqmp-gqspi.c is spi driver and all above mentioned parameters and function pointers are related to flash layer.
> So is it ok to update and change flash related stuff in our spi driver?
>
Check with the SPI sub-system people, especially Mark Brown, but I don't
think would be good to put too much mtd/spi-nor stuff in the SPI sub-system.
Anyway, the solution I've proposed is not suited if you use m25p80.c as an
adaptation layer between spi-nor.c and the SPI controller driver.
Indeed, in that case, spi_nor_scan() is called from m25p_probe() in
m25p80.c and I still think there would be too many side effects if we
modified either spi-nor.c or m25p80.c the way you propose to compute
logical operations on register values. This solution is not maintainable
regarding the number memory registers we already manage.
You might need to develop a new driver to substitute m25p80.c and make the
link between spi-nor, mostly spi_nor_scan(), and you SPI controller driver.
Honestly I have no real idea how you could proceed to add support to such a
feature: accessing 2 SPI flashes at the time to perform stripping
operations doesn't sound really reliable to me because I don't see how you
plan to handle errors properly and also cover all the features provided by
SPI flash memory and supported by spi-nor.c (sector/block protection, ...).
>> Best regards,
>>
>> Cyrille
>>
> Thanks,
> Naga Sureshkumar Relli
>
>
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
>
>
^ permalink raw reply
* [RFC PATCH 3/3] ARM: dts: at91: sama5d2_xplained: add pin muxing and enable classd
From: Nicolas Ferre @ 2016-12-06 10:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1481021115.git.nicolas.ferre@atmel.com>
From: Cyrille Pitchen <cyrille.pitchen@atmel.com>
This patch adds the pin muxing for classd and enables it.
Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
arch/arm/boot/dts/at91-sama5d2_xplained.dts | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
index 0b9a59d5fdac..c9a8939ba1ef 100644
--- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
@@ -332,6 +332,14 @@
bias-pull-up;
};
+ pinctrl_classd_default: classd_default {
+ pinmux = <PIN_PB1__CLASSD_R0>,
+ <PIN_PB2__CLASSD_R1>,
+ <PIN_PB3__CLASSD_R2>,
+ <PIN_PB4__CLASSD_R3>;
+ bias-pull-up;
+ };
+
pinctrl_flx0_default: flx0_default {
pinmux = <PIN_PB28__FLEXCOM0_IO0>,
<PIN_PB29__FLEXCOM0_IO1>;
@@ -464,6 +472,14 @@
};
};
+
+ classd: classd at fc048000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_classd_default>;
+ atmel,pwm-type = "diff";
+ atmel,non-overlap-time = <10>;
+ status = "okay";
+ };
};
};
--
2.9.0
^ permalink raw reply related
* [RFC PATCH 2/3] ARM: dts: at91: sama5d2: add classd nodes
From: Nicolas Ferre @ 2016-12-06 10:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1481021115.git.nicolas.ferre@atmel.com>
From: Cyrille Pitchen <cyrille.pitchen@atmel.com>
This patch adds nodes for the classd device and its generated clock.
Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
arch/arm/boot/dts/sama5d2.dtsi | 39 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index ceb9783ff7e1..d6b8d59a913b 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -447,6 +447,24 @@
clocks = <&plla>;
};
+ audio_pll_frac: audiopll_fracck {
+ compatible = "atmel,sama5d2-clk-audio-pll-frac";
+ #clock-cells = <0>;
+ clocks = <&main>;
+ };
+
+ audio_pll_pad: audiopll_padck {
+ compatible = "atmel,sama5d2-clk-audio-pll-pad";
+ #clock-cells = <0>;
+ clocks = <&audio_pll_frac>;
+ };
+
+ audio_pll_pmc: audiopll_pmcck {
+ compatible = "atmel,sama5d2-clk-audio-pll-pmc";
+ #clock-cells = <0>;
+ clocks = <&audio_pll_frac>;
+ };
+
utmi: utmick {
compatible = "atmel,at91sam9x5-clk-utmi";
#clock-cells = <0>;
@@ -836,7 +854,7 @@
#address-cells = <1>;
#size-cells = <0>;
interrupt-parent = <&pmc>;
- clocks = <&clk32k>, <&main>, <&plladiv>, <&utmi>, <&mck>;
+ clocks = <&clk32k>, <&main>, <&plladiv>, <&utmi>, <&mck>, <&audio_pll_pmc>;
sdmmc0_gclk: sdmmc0_gclk {
#clock-cells = <0>;
@@ -880,6 +898,12 @@
#clock-cells = <0>;
reg = <55>;
};
+
+ classd_gclk: classd_gclk {
+ #clock-cells = <0>;
+ reg = <59>;
+ atmel,clk-output-range = <0 100000000>;
+ };
};
};
@@ -1279,6 +1303,19 @@
status = "okay";
};
+ classd: classd at fc048000 {
+ compatible = "atmel,sama5d2-classd";
+ reg = <0xfc048000 0x100>;
+ interrupts = <59 IRQ_TYPE_LEVEL_HIGH 7>;
+ dmas = <&dma0
+ (AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1) |
+ AT91_XDMAC_DT_PERID(47))>;
+ dma-names = "tx";
+ clocks = <&classd_clk>, <&classd_gclk>, <&audio_pll_pmc>;
+ clock-names = "pclk", "gclk", "aclk";
+ status = "disabled";
+ };
+
chipid at fc069000 {
compatible = "atmel,sama5d2-chipid";
reg = <0xfc069000 0x8>;
--
2.9.0
^ permalink raw reply related
* [RFC PATCH 1/3] clk: at91: add audio pll clock driver
From: Nicolas Ferre @ 2016-12-06 10:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1481021115.git.nicolas.ferre@atmel.com>
This new clock driver set allows to have a fractional divided clock that
would generate a precise clock particularly suitable for audio applications.
The main audio pll clock has two children clocks: one that is connected to
the PMC, the other that can directly drive a pad. As these two routes have
different enable bits and different dividers and divider formula, they are
handled by two different drivers. Each of them would modify the rate of
the main audio pll parent.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
.../devicetree/bindings/clock/at91-clock.txt | 10 +
arch/arm/mach-at91/Kconfig | 4 +
drivers/clk/at91/Makefile | 2 +
drivers/clk/at91/clk-audio-pll-pad.c | 238 +++++++++++++++++++
drivers/clk/at91/clk-audio-pll-pmc.c | 184 +++++++++++++++
drivers/clk/at91/clk-audio-pll.c | 253 +++++++++++++++++++++
include/linux/clk/at91_pmc.h | 25 ++
7 files changed, 716 insertions(+)
create mode 100644 drivers/clk/at91/clk-audio-pll-pad.c
create mode 100644 drivers/clk/at91/clk-audio-pll-pmc.c
create mode 100644 drivers/clk/at91/clk-audio-pll.c
diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt b/Documentation/devicetree/bindings/clock/at91-clock.txt
index 5f3ad65daf69..bbf3ed9d9166 100644
--- a/Documentation/devicetree/bindings/clock/at91-clock.txt
+++ b/Documentation/devicetree/bindings/clock/at91-clock.txt
@@ -81,6 +81,16 @@ Required properties:
"atmel,sama5d2-clk-generated":
at91 generated clock
+ "atmel,sama5d2-clk-audio-pll-frac":
+ at91 audio fractional pll
+
+ "atmel,sama5d2-clk-audio-pll-pad":
+ at91 audio pll CLK_AUDIO output pin
+
+ "atmel,sama5d2-clk-audio-pll-pmc"
+ at91 audio pll ouput on AUDIOPLLCLK that feeds the PMC
+ and can be used by peripheral clock or generic clock
+
Required properties for SCKC node:
- reg : defines the IO memory reserved for the SCKC.
- #size-cells : shall be 0 (reg is used to encode clk id).
diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
index 841e924143f9..bc1f56dcda8a 100644
--- a/arch/arm/mach-at91/Kconfig
+++ b/arch/arm/mach-at91/Kconfig
@@ -17,6 +17,7 @@ config SOC_SAMA5D2
select HAVE_AT91_USB_CLK
select HAVE_AT91_H32MX
select HAVE_AT91_GENERATED_CLK
+ select HAVE_AT91_AUDIO_PLL
select PINCTRL_AT91PIO4
help
Select this if ou are using one of Atmel's SAMA5D2 family SoC.
@@ -114,6 +115,9 @@ config HAVE_AT91_H32MX
config HAVE_AT91_GENERATED_CLK
bool
+config HAVE_AT91_AUDIO_PLL
+ bool
+
config SOC_SAM_V4_V5
bool
diff --git a/drivers/clk/at91/Makefile b/drivers/clk/at91/Makefile
index 13e67bd35cff..c9353d17763a 100644
--- a/drivers/clk/at91/Makefile
+++ b/drivers/clk/at91/Makefile
@@ -6,6 +6,8 @@ obj-y += pmc.o sckc.o
obj-y += clk-slow.o clk-main.o clk-pll.o clk-plldiv.o clk-master.o
obj-y += clk-system.o clk-peripheral.o clk-programmable.o
+obj-$(CONFIG_HAVE_AT91_AUDIO_PLL) += clk-audio-pll.o
+obj-$(CONFIG_HAVE_AT91_AUDIO_PLL) += clk-audio-pll-pmc.o clk-audio-pll-pad.o
obj-$(CONFIG_HAVE_AT91_UTMI) += clk-utmi.o
obj-$(CONFIG_HAVE_AT91_USB_CLK) += clk-usb.o
obj-$(CONFIG_HAVE_AT91_SMD) += clk-smd.o
diff --git a/drivers/clk/at91/clk-audio-pll-pad.c b/drivers/clk/at91/clk-audio-pll-pad.c
new file mode 100644
index 000000000000..39cee71f15c6
--- /dev/null
+++ b/drivers/clk/at91/clk-audio-pll-pad.c
@@ -0,0 +1,238 @@
+/*
+ * Copyright (C) 2016 Atmel Corporation,
+ * Nicolas Ferre <nicolas.ferre@atmel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/clk/at91_pmc.h>
+#include <linux/of.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+
+#include "pmc.h"
+
+/*
+ * DOC: PAD output for fractional PLL clock for audio
+ *
+ * Traits of this clock:
+ * enable - clk_enable writes qdpad (which is ext_div and (div2,div3)),
+ * and enables PAD output
+ * rate - rate is adjustable.
+ * clk->rate = parent->rate / (ext_div * (div2,div3))
+ * parent - fixed parent. No clk_set_parent support
+ */
+
+#define AUDIO_PLL_FOUT_MIN 620000000
+#define AUDIO_PLL_FOUT_MAX 700000000
+#define AUDIO_PLL_REFERENCE_FOUT 660000000
+
+#define AUDIO_PLL_QDPAD_MAX ((AT91_PMC_AUDIO_PLL_QDPAD_DIV_MASK >> \
+ AT91_PMC_AUDIO_PLL_QDPAD_DIV_OFFSET) * \
+ AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_MAX)
+#define AUDIO_PLL_QDPAD_EXTDIV_OFFSET (AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_OFFSET - \
+ AT91_PMC_AUDIO_PLL_QDPAD_OFFSET)
+#define AUDIO_PLL_DIV2QD(div, ext_div) ((div) | ((ext_div) << \
+ AUDIO_PLL_QDPAD_EXTDIV_OFFSET))
+#define AUDIO_PLL_QD2DIV(qd) ((qd) & (AT91_PMC_AUDIO_PLL_QDPAD_DIV_MASK >> \
+ AT91_PMC_AUDIO_PLL_QDPAD_DIV_OFFSET))
+#define AUDIO_PLL_QD2EXTDIV(qd) (((qd) >> AUDIO_PLL_QDPAD_EXTDIV_OFFSET) \
+ & (AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_MASK >> \
+ AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_OFFSET))
+
+struct clk_audio_pad {
+ struct clk_hw hw;
+ struct regmap *regmap;
+ spinlock_t *lock;
+ u8 qdpad;
+};
+
+#define to_clk_audio_pad(hw) container_of(hw, struct clk_audio_pad, hw)
+
+static int clk_audio_pll_pad_enable(struct clk_hw *hw)
+{
+ struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
+ unsigned long flags;
+
+ spin_lock_irqsave(apad_ck->lock, flags);
+ regmap_update_bits(apad_ck->regmap, AT91_PMC_AUDIO_PLL1,
+ AT91_PMC_AUDIO_PLL_QDPAD_MASK,
+ AT91_PMC_AUDIO_PLL_QDPAD(apad_ck->qdpad));
+ regmap_update_bits(apad_ck->regmap, AT91_PMC_AUDIO_PLL0,
+ AT91_PMC_AUDIO_PLL_PADEN, AT91_PMC_AUDIO_PLL_PADEN);
+ spin_unlock_irqrestore(apad_ck->lock, flags);
+ return 0;
+}
+
+static void clk_audio_pll_pad_disable(struct clk_hw *hw)
+{
+ struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
+
+ regmap_update_bits(apad_ck->regmap, AT91_PMC_AUDIO_PLL0,
+ AT91_PMC_AUDIO_PLL_PADEN, 0);
+}
+
+static unsigned long clk_audio_pll_pad_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
+ unsigned long apad_rate = 0;
+ u8 tmp_div = AUDIO_PLL_QD2DIV(apad_ck->qdpad);
+ u8 tmp_ext_div = AUDIO_PLL_QD2EXTDIV(apad_ck->qdpad);
+
+ if (tmp_div && tmp_ext_div)
+ apad_rate = parent_rate / (tmp_div * tmp_ext_div);
+
+ pr_debug("A PLL/PAD: %s, apad_rate = %lu (div = %u, ext_div = %u)\n" ,
+ __func__ , apad_rate, tmp_div, tmp_ext_div);
+
+ return apad_rate;
+}
+
+static int clk_audio_pll_compute_qdpad(unsigned long q_rate, unsigned long rate,
+ unsigned long *qd, u8 *div, u8 *ext_div)
+{
+ unsigned long tmp_qd;
+ unsigned long rem2, rem3;
+ unsigned long ldiv, lext_div;;
+
+ if (!rate)
+ return -EINVAL;
+
+ tmp_qd = q_rate / rate;
+ if (!tmp_qd || tmp_qd > AUDIO_PLL_QDPAD_MAX)
+ return -EINVAL;
+
+ if (tmp_qd <= AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_MAX) {
+ ldiv = 1;
+ lext_div = tmp_qd;
+ } else {
+ rem2 = tmp_qd % 2;
+ rem3 = tmp_qd % 3;
+
+ if (rem3 == 0 ||
+ tmp_qd > AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_MAX * 2 ||
+ rem3 < rem2) {
+ ldiv = 3;
+ lext_div = tmp_qd / 3;
+ } else {
+ ldiv = 2;
+ lext_div = tmp_qd >> 1;
+ }
+ }
+
+ pr_debug("A PLL/PAD: %s, qd = %lu (div = %lu, ext_div = %lu)\n" ,
+ __func__ , ldiv * lext_div, ldiv, lext_div);
+
+ /* if we were given variable to store, we can provide them */
+ if (qd)
+ *qd = ldiv * lext_div;
+ if (div && ext_div) {
+ /* we can cast here as we verified the bounds just above */
+ *div = (u8)ldiv;
+ *ext_div = (u8)lext_div;
+ }
+
+ return 0;
+}
+
+static long clk_audio_pll_pad_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ struct clk_hw *pclk = clk_hw_get_parent(hw);
+ long best_rate = -EINVAL;
+ unsigned long best_parent_rate = 0;
+ unsigned long tmp_qd;
+
+ pr_debug("A PLL/PAD: %s, rate = %lu (parent_rate = %lu)\n" ,
+ __func__ , rate, *parent_rate);
+
+ if (clk_audio_pll_compute_qdpad(AUDIO_PLL_REFERENCE_FOUT, rate,
+ &tmp_qd, NULL, NULL))
+ return -EINVAL;
+
+ best_parent_rate = clk_hw_round_rate(pclk, rate * tmp_qd);
+ best_rate = best_parent_rate / tmp_qd;
+
+ pr_debug("A PLL/PAD: %s, best_rate = %ld, best_parent_rate = %lu\n",
+ __func__, best_rate, best_parent_rate);
+
+ *parent_rate = best_parent_rate;
+ return best_rate;
+}
+
+static int clk_audio_pll_pad_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
+ u8 tmp_div, tmp_ext_div;
+ int ret;
+
+ pr_debug("A PLL/PAD: %s, rate = %lu (parent_rate = %lu)\n" ,
+ __func__ , rate, parent_rate);
+
+ ret = clk_audio_pll_compute_qdpad(parent_rate, rate, NULL,
+ &tmp_div, &tmp_ext_div);
+ if (!ret)
+ apad_ck->qdpad = AUDIO_PLL_DIV2QD(tmp_div, tmp_ext_div);
+
+ return ret;
+}
+
+static const struct clk_ops audio_pll_pad_ops = {
+ .enable = clk_audio_pll_pad_enable,
+ .disable = clk_audio_pll_pad_disable,
+ .recalc_rate = clk_audio_pll_pad_recalc_rate,
+ .round_rate = clk_audio_pll_pad_round_rate,
+ .set_rate = clk_audio_pll_pad_set_rate,
+};
+
+static void __init of_sama5d2_clk_audio_pll_pad_setup(struct device_node *np)
+{
+ struct clk_audio_pad *apad_ck;
+ struct clk_init_data init;
+ struct regmap *regmap;
+ const char *parent_name;
+ const char *name = np->name;
+ int ret;
+
+ parent_name = of_clk_get_parent_name(np, 0);
+
+ of_property_read_string(np, "clock-output-names", &name);
+
+ regmap = syscon_node_to_regmap(of_get_parent(np));
+ if (IS_ERR(regmap))
+ return;
+
+ apad_ck = kzalloc(sizeof(*apad_ck), GFP_KERNEL);
+ if (!apad_ck)
+ return;
+
+ init.name = name;
+ init.ops = &audio_pll_pad_ops;
+ init.parent_names = (parent_name ? &parent_name : NULL);
+ init.num_parents = 1;
+ init.flags = CLK_SET_RATE_GATE |
+ CLK_SET_PARENT_GATE | CLK_SET_RATE_PARENT;
+
+ apad_ck->hw.init = &init;
+ apad_ck->regmap = regmap;
+ apad_ck->lock = &pmc_pcr_lock;
+
+ ret = clk_hw_register(NULL, &apad_ck->hw);
+ if (ret)
+ kfree(apad_ck);
+ else
+ of_clk_add_hw_provider(np, of_clk_hw_simple_get, &apad_ck->hw);
+
+ return;
+}
+CLK_OF_DECLARE(of_sama5d2_clk_audio_pll_pad_setup,
+ "atmel,sama5d2-clk-audio-pll-pad",
+ of_sama5d2_clk_audio_pll_pad_setup);
diff --git a/drivers/clk/at91/clk-audio-pll-pmc.c b/drivers/clk/at91/clk-audio-pll-pmc.c
new file mode 100644
index 000000000000..fadc51e25bac
--- /dev/null
+++ b/drivers/clk/at91/clk-audio-pll-pmc.c
@@ -0,0 +1,184 @@
+/*
+ * Copyright (C) 2016 Atmel Corporation,
+ * Nicolas Ferre <nicolas.ferre@atmel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/clk/at91_pmc.h>
+#include <linux/of.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+
+#include "pmc.h"
+
+/*
+ * DOC: PMC output for fractional PLL clock for audio
+ *
+ * Traits of this clock:
+ * enable - clk_enable writes qdpmc, and enables PMC output
+ * rate - rate is adjustable.
+ * clk->rate = parent->rate / (qdpmc + 1)
+ * parent - fixed parent. No clk_set_parent support
+ */
+
+#define AUDIO_PLL_FOUT_MIN 620000000
+#define AUDIO_PLL_FOUT_MAX 700000000
+#define AUDIO_PLL_REFERENCE_FOUT 660000000
+#define AUDIO_PLL_QDPMC_MAX (AT91_PMC_AUDIO_PLL_QDPMC_MASK >> \
+ AT91_PMC_AUDIO_PLL_QDPMC_OFFSET)
+struct clk_audio_pmc {
+ struct clk_hw hw;
+ struct regmap *regmap;
+ u8 qdpmc;
+};
+
+#define to_clk_audio_pmc(hw) container_of(hw, struct clk_audio_pmc, hw)
+
+static int clk_audio_pll_pmc_enable(struct clk_hw *hw)
+{
+ struct clk_audio_pmc *apmc_ck = to_clk_audio_pmc(hw);
+
+ regmap_update_bits(apmc_ck->regmap, AT91_PMC_AUDIO_PLL0,
+ AT91_PMC_AUDIO_PLL_PMCEN |
+ AT91_PMC_AUDIO_PLL_QDPMC_MASK,
+ AT91_PMC_AUDIO_PLL_PMCEN |
+ AT91_PMC_AUDIO_PLL_QDPMC(apmc_ck->qdpmc));
+ return 0;
+}
+
+static void clk_audio_pll_pmc_disable(struct clk_hw *hw)
+{
+ struct clk_audio_pmc *apmc_ck = to_clk_audio_pmc(hw);
+
+ regmap_update_bits(apmc_ck->regmap, AT91_PMC_AUDIO_PLL0,
+ AT91_PMC_AUDIO_PLL_PMCEN, 0);
+}
+
+static unsigned long clk_audio_pll_pmc_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct clk_audio_pmc *apmc_ck = to_clk_audio_pmc(hw);
+ unsigned long apmc_rate = 0;
+
+ apmc_rate = parent_rate / (apmc_ck->qdpmc + 1);
+
+ pr_debug("A PLL/PMC: %s, apmc_rate = %lu (qdpmc = %u)\n" ,
+ __func__ , apmc_rate, apmc_ck->qdpmc);
+
+ return apmc_rate;
+}
+
+static int clk_audio_pll_compute_qdpmc(unsigned long q_rate, unsigned long rate,
+ unsigned long *qd)
+{
+ unsigned long tmp_qd;
+
+ if (!rate)
+ return -EINVAL;
+
+ tmp_qd = q_rate / rate;
+ if (!tmp_qd || tmp_qd > AUDIO_PLL_QDPMC_MAX)
+ return -EINVAL;
+
+ *qd = tmp_qd;
+ return 0;
+}
+
+static long clk_audio_pll_pmc_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ struct clk_hw *pclk = clk_hw_get_parent(hw);
+ long best_rate = -EINVAL;
+ unsigned long best_parent_rate = 0;
+ unsigned long tmp_qd;
+
+ pr_debug("A PLL/PMC: %s, rate = %lu (parent_rate = %lu)\n" ,
+ __func__ , rate, *parent_rate);
+
+ if (clk_audio_pll_compute_qdpmc(AUDIO_PLL_REFERENCE_FOUT, rate, &tmp_qd))
+ return -EINVAL;
+
+ best_parent_rate = clk_hw_round_rate(pclk, rate * tmp_qd);
+ best_rate = best_parent_rate / tmp_qd;
+
+ pr_debug("A PLL/PMC: %s, best_rate = %ld, best_parent_rate = %lu (qd = %lu)\n",
+ __func__, best_rate, best_parent_rate, tmp_qd - 1);
+
+ *parent_rate = best_parent_rate;
+ return best_rate;
+}
+
+static int clk_audio_pll_pmc_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct clk_audio_pmc *apmc_ck = to_clk_audio_pmc(hw);
+ unsigned long tmp_qd;
+ int ret;
+
+ pr_debug("A PLL/PMC: %s, rate = %lu (parent_rate = %lu)\n" ,
+ __func__ , rate, parent_rate);
+
+ ret = clk_audio_pll_compute_qdpmc(parent_rate, rate, &tmp_qd);
+ if (!ret)
+ apmc_ck->qdpmc = tmp_qd - 1;
+
+ return ret;
+}
+
+static const struct clk_ops audio_pll_pmc_ops = {
+ .enable = clk_audio_pll_pmc_enable,
+ .disable = clk_audio_pll_pmc_disable,
+ .recalc_rate = clk_audio_pll_pmc_recalc_rate,
+ .round_rate = clk_audio_pll_pmc_round_rate,
+ .set_rate = clk_audio_pll_pmc_set_rate,
+};
+
+static void __init of_sama5d2_clk_audio_pll_pmc_setup(struct device_node *np)
+{
+ struct clk_audio_pmc *apmc_ck;
+ struct clk_init_data init;
+ struct regmap *regmap;
+ const char *parent_name;
+ const char *name = np->name;
+ int ret;
+
+ parent_name = of_clk_get_parent_name(np, 0);
+
+ of_property_read_string(np, "clock-output-names", &name);
+
+ regmap = syscon_node_to_regmap(of_get_parent(np));
+ if (IS_ERR(regmap))
+ return;
+
+ apmc_ck = kzalloc(sizeof(*apmc_ck), GFP_KERNEL);
+ if (!apmc_ck)
+ return;
+
+ init.name = name;
+ init.ops = &audio_pll_pmc_ops;
+ init.parent_names = (parent_name ? &parent_name : NULL);
+ init.num_parents = 1;
+ init.flags = CLK_SET_RATE_GATE |
+ CLK_SET_PARENT_GATE | CLK_SET_RATE_PARENT;
+
+ apmc_ck->hw.init = &init;
+ apmc_ck->regmap = regmap;
+
+ ret = clk_hw_register(NULL, &apmc_ck->hw);
+ if (ret)
+ kfree(apmc_ck);
+ else
+ of_clk_add_hw_provider(np, of_clk_hw_simple_get, &apmc_ck->hw);
+
+ return;
+}
+CLK_OF_DECLARE(of_sama5d2_clk_audio_pll_pmc_setup,
+ "atmel,sama5d2-clk-audio-pll-pmc",
+ of_sama5d2_clk_audio_pll_pmc_setup);
diff --git a/drivers/clk/at91/clk-audio-pll.c b/drivers/clk/at91/clk-audio-pll.c
new file mode 100644
index 000000000000..76261bcb204f
--- /dev/null
+++ b/drivers/clk/at91/clk-audio-pll.c
@@ -0,0 +1,253 @@
+/*
+ * Copyright (C) 2016 Atmel Corporation,
+ * Songjun Wu <songjun.wu@atmel.com>,
+ * Nicolas Ferre <nicolas.ferre@atmel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/clk/at91_pmc.h>
+#include <linux/of.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+
+#include "pmc.h"
+
+/*
+ * DOC: Fractional PLL clock for audio
+ *
+ * Traits of this clock:
+ * prepare - clk_prepare puts audio PLL in reset state
+ * enable - clk_enable writes nd, fracr parameters and enables PLL
+ * rate - rate is adjustable.
+ * clk->rate = parent->rate * ((nd + 1) + (fracr / 2^22))
+ * parent - fixed parent. No clk_set_parent support
+ */
+
+#define AUDIO_PLL_DIV_FRAC (1 << 22)
+#define AUDIO_PLL_ND_MAX (AT91_PMC_AUDIO_PLL_ND_MASK >> \
+ AT91_PMC_AUDIO_PLL_ND_OFFSET)
+
+struct clk_audio_frac {
+ struct clk_hw hw;
+ struct regmap *regmap;
+ spinlock_t *lock;
+ u32 fracr;
+ u8 nd;
+};
+
+#define to_clk_audio_frac(hw) container_of(hw, struct clk_audio_frac, hw)
+
+/* make sure that pll is in reset state beforehand */
+static int clk_audio_pll_prepare(struct clk_hw *hw)
+{
+ struct clk_audio_frac *fck = to_clk_audio_frac(hw);
+
+ regmap_update_bits(fck->regmap, AT91_PMC_AUDIO_PLL0,
+ AT91_PMC_AUDIO_PLL_RESETN, 0);
+ return 0;
+}
+
+static void clk_audio_pll_unprepare(struct clk_hw *hw)
+{
+ clk_audio_pll_prepare(hw);
+}
+
+static int clk_audio_pll_enable(struct clk_hw *hw)
+{
+ struct clk_audio_frac *fck = to_clk_audio_frac(hw);
+ unsigned long flags;
+
+ spin_lock_irqsave(fck->lock, flags);
+ regmap_update_bits(fck->regmap, AT91_PMC_AUDIO_PLL0,
+ AT91_PMC_AUDIO_PLL_RESETN,
+ AT91_PMC_AUDIO_PLL_RESETN);
+ regmap_update_bits(fck->regmap, AT91_PMC_AUDIO_PLL1,
+ AT91_PMC_AUDIO_PLL_FRACR_MASK, fck->fracr);
+
+ /*
+ * reset and enable have to be done in 2 separated writes
+ * for AT91_PMC_AUDIO_PLL0
+ */
+ regmap_update_bits(fck->regmap, AT91_PMC_AUDIO_PLL0,
+ AT91_PMC_AUDIO_PLL_PLLEN |
+ AT91_PMC_AUDIO_PLL_ND_MASK,
+ AT91_PMC_AUDIO_PLL_PLLEN |
+ AT91_PMC_AUDIO_PLL_ND(fck->nd));
+ spin_unlock_irqrestore(fck->lock, flags);
+ return 0;
+}
+
+static void clk_audio_pll_disable(struct clk_hw *hw)
+{
+ struct clk_audio_frac *fck = to_clk_audio_frac(hw);
+ unsigned long flags;
+
+ spin_lock_irqsave(fck->lock, flags);
+ regmap_update_bits(fck->regmap, AT91_PMC_AUDIO_PLL0,
+ AT91_PMC_AUDIO_PLL_PLLEN, 0);
+ /* do it in 2 separated writes */
+ regmap_update_bits(fck->regmap, AT91_PMC_AUDIO_PLL0,
+ AT91_PMC_AUDIO_PLL_RESETN, 0);
+ spin_unlock_irqrestore(fck->lock, flags);
+}
+
+static unsigned long clk_audio_pll_fout(unsigned long parent_rate,
+ unsigned long nd, unsigned long fracr)
+{
+ unsigned long long fr = (unsigned long long)parent_rate *
+ (unsigned long long)fracr;
+
+ pr_debug("A PLL: %s, fr = %llu\n" ,
+ __func__ , fr);
+
+ fr = DIV_ROUND_CLOSEST_ULL(fr, AUDIO_PLL_DIV_FRAC);
+
+ pr_debug("A PLL: %s, fr = %llu\n" ,
+ __func__ , fr);
+
+ return parent_rate * (nd + 1) + fr;
+}
+
+static unsigned long clk_audio_pll_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct clk_audio_frac *fck = to_clk_audio_frac(hw);
+ unsigned long fout;
+
+ fout = clk_audio_pll_fout(parent_rate, fck->nd, fck->fracr);
+
+ pr_debug("A PLL: %s, fout = %lu (nd = %u, fracr = %lu)\n" ,
+ __func__ , fout, fck->nd, (unsigned long)fck->fracr);
+
+ return fout;
+}
+
+static int clk_audio_pll_compute_frac(unsigned long rate, unsigned long parent_rate,
+ unsigned long *nd, unsigned long *fracr)
+{
+ unsigned long long tmp;
+ unsigned long long r;
+
+ if (!rate)
+ return -EINVAL;
+
+ tmp = rate;
+ r = do_div(tmp, parent_rate);
+ if (tmp == 0 || (tmp - 1) > AUDIO_PLL_ND_MAX)
+ return -EINVAL;
+
+ *nd = tmp - 1;
+
+ tmp = r * AUDIO_PLL_DIV_FRAC;
+ tmp = DIV_ROUND_CLOSEST_ULL(tmp, parent_rate);
+ if (tmp > AT91_PMC_AUDIO_PLL_FRACR_MASK)
+ return -EINVAL;
+
+ /* we can cast here as we verified the bounds just above */
+ *fracr = (unsigned long)tmp;
+
+ return 0;
+}
+
+static long clk_audio_pll_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ long best_rate = -EINVAL;
+ unsigned long fracr;
+ unsigned long nd;
+ int ret;
+
+ pr_debug("A PLL: %s, rate = %lu (parent_rate = %lu)\n" ,
+ __func__ , rate, *parent_rate);
+
+ ret = clk_audio_pll_compute_frac(rate, *parent_rate, &nd, &fracr);
+ if (ret)
+ return ret;
+
+ best_rate = clk_audio_pll_fout(*parent_rate, nd, fracr);
+
+ pr_debug("A PLL: %s, best_rate = %lu (nd = %lu, fracr = %lu)\n" ,
+ __func__ , best_rate, nd, fracr);
+
+ return best_rate;
+}
+
+static int clk_audio_pll_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct clk_audio_frac *fck = to_clk_audio_frac(hw);
+ unsigned long fracr;
+ unsigned long nd;
+ int ret;
+
+ pr_debug("A PLL: %s, rate = %lu (parent_rate = %lu)\n" ,
+ __func__ , rate, parent_rate);
+
+ ret = clk_audio_pll_compute_frac(rate, parent_rate, &nd, &fracr);
+ if (ret)
+ return ret;
+
+ fck->nd = nd;
+ fck->fracr = fracr;
+
+ return 0;
+}
+
+static const struct clk_ops audio_pll_ops = {
+ .prepare = clk_audio_pll_prepare,
+ .unprepare = clk_audio_pll_unprepare,
+ .enable = clk_audio_pll_enable,
+ .disable = clk_audio_pll_disable,
+ .recalc_rate = clk_audio_pll_recalc_rate,
+ .round_rate = clk_audio_pll_round_rate,
+ .set_rate = clk_audio_pll_set_rate,
+};
+
+static void __init of_sama5d2_clk_audio_pll_setup(struct device_node *np)
+{
+ struct clk_audio_frac *fck;
+ struct clk_init_data init;
+ struct regmap *regmap;
+ const char *parent_name;
+ const char *name = np->name;
+ int ret;
+
+ parent_name = of_clk_get_parent_name(np, 0);
+
+ of_property_read_string(np, "clock-output-names", &name);
+
+ regmap = syscon_node_to_regmap(of_get_parent(np));
+ if (IS_ERR(regmap))
+ return;
+
+ fck = kzalloc(sizeof(*fck), GFP_KERNEL);
+ if (!fck)
+ return;
+
+ init.name = name;
+ init.ops = &audio_pll_ops;
+ init.parent_names = (parent_name ? &parent_name : NULL);
+ init.num_parents = 1;
+ init.flags = CLK_SET_RATE_GATE;
+
+ fck->hw.init = &init;
+ fck->regmap = regmap;
+ fck->lock = &pmc_pcr_lock;
+
+ ret = clk_hw_register(NULL, &fck->hw);
+ if (ret)
+ kfree(fck);
+ else
+ of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fck->hw);
+
+ return;
+}
+CLK_OF_DECLARE(of_sama5d2_clk_audio_pll_setup,
+ "atmel,sama5d2-clk-audio-pll-frac",
+ of_sama5d2_clk_audio_pll_setup);
diff --git a/include/linux/clk/at91_pmc.h b/include/linux/clk/at91_pmc.h
index 17f413bbbedf..6aca5ce8a99a 100644
--- a/include/linux/clk/at91_pmc.h
+++ b/include/linux/clk/at91_pmc.h
@@ -185,4 +185,29 @@
#define AT91_PMC_PCR_EN (0x1 << 28) /* Enable */
#define AT91_PMC_PCR_GCKEN (0x1 << 29) /* GCK Enable */
+#define AT91_PMC_AUDIO_PLL0 0x14c
+#define AT91_PMC_AUDIO_PLL_PLLEN (1 << 0)
+#define AT91_PMC_AUDIO_PLL_PADEN (1 << 1)
+#define AT91_PMC_AUDIO_PLL_PMCEN (1 << 2)
+#define AT91_PMC_AUDIO_PLL_RESETN (1 << 3)
+#define AT91_PMC_AUDIO_PLL_ND_OFFSET 8
+#define AT91_PMC_AUDIO_PLL_ND_MASK (0x7f << AT91_PMC_AUDIO_PLL_ND_OFFSET)
+#define AT91_PMC_AUDIO_PLL_ND(n) ((n) << AT91_PMC_AUDIO_PLL_ND_OFFSET)
+#define AT91_PMC_AUDIO_PLL_QDPMC_OFFSET 16
+#define AT91_PMC_AUDIO_PLL_QDPMC_MASK (0x7f << AT91_PMC_AUDIO_PLL_QDPMC_OFFSET)
+#define AT91_PMC_AUDIO_PLL_QDPMC(n) ((n) << AT91_PMC_AUDIO_PLL_QDPMC_OFFSET)
+
+#define AT91_PMC_AUDIO_PLL1 0x150
+#define AT91_PMC_AUDIO_PLL_FRACR_MASK 0x3fffff
+#define AT91_PMC_AUDIO_PLL_QDPAD_OFFSET 24
+#define AT91_PMC_AUDIO_PLL_QDPAD_MASK (0x7f << AT91_PMC_AUDIO_PLL_QDPAD_OFFSET)
+#define AT91_PMC_AUDIO_PLL_QDPAD(n) ((n) << AT91_PMC_AUDIO_PLL_QDPAD_OFFSET)
+#define AT91_PMC_AUDIO_PLL_QDPAD_DIV_OFFSET AT91_PMC_AUDIO_PLL_QDPAD_OFFSET
+#define AT91_PMC_AUDIO_PLL_QDPAD_DIV_MASK (0x3 << AT91_PMC_AUDIO_PLL_QDPAD_DIV_OFFSET)
+#define AT91_PMC_AUDIO_PLL_QDPAD_DIV(n) ((n) << AT91_PMC_AUDIO_PLL_QDPAD_DIV_OFFSET)
+#define AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_OFFSET 26
+#define AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_MAX 0x1f
+#define AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_MASK (AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_MAX << AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_OFFSET)
+#define AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV(n) ((n) << AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_OFFSET)
+
#endif
--
2.9.0
^ permalink raw reply related
* [RFC PATCH 0/3] clk: at91: audio PLL clock
From: Nicolas Ferre @ 2016-12-06 10:55 UTC (permalink / raw)
To: linux-arm-kernel
This series covers the addition of the Audio PLL clock found on AT91 SoCs like
the SAMA5D2.
I also added the use of these clocks by the ClassD audi amplifier in both SoC
and board DT.
The Audio PLL is described in the sama5d2 datasheet chapter "29.8 Audio PLL".
Even if "it works" (!), note that I'm not satisfied with the current code and
would need some advice from people more accustomed with the CCF and
particularly composite audio PLL/clocks like these. For example, I do not take
into account the limits of these clocks (as described in the datasheet) and the
dependency between the PAD and the PMC child clocks.
Thanks in advance for your inputs.
Best regards,
Nicolas
Cyrille Pitchen (2):
ARM: dts: at91: sama5d2: add classd nodes
ARM: dts: at91: sama5d2_xplained: add pin muxing and enable classd
Nicolas Ferre (1):
clk: at91: add audio pll clock driver
.../devicetree/bindings/clock/at91-clock.txt | 10 +
arch/arm/boot/dts/at91-sama5d2_xplained.dts | 16 ++
arch/arm/boot/dts/sama5d2.dtsi | 39 +++-
arch/arm/mach-at91/Kconfig | 4 +
drivers/clk/at91/Makefile | 2 +
drivers/clk/at91/clk-audio-pll-pad.c | 238 +++++++++++++++++++
drivers/clk/at91/clk-audio-pll-pmc.c | 184 +++++++++++++++
drivers/clk/at91/clk-audio-pll.c | 253 +++++++++++++++++++++
include/linux/clk/at91_pmc.h | 25 ++
9 files changed, 770 insertions(+), 1 deletion(-)
create mode 100644 drivers/clk/at91/clk-audio-pll-pad.c
create mode 100644 drivers/clk/at91/clk-audio-pll-pmc.c
create mode 100644 drivers/clk/at91/clk-audio-pll.c
--
2.9.0
^ permalink raw reply
* [PATCH v2 0/3] ARM: dts: imx6: Support Poslab Savageboard dual & quad
From: Milo Kim @ 2016-12-06 10:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5APWxuRmo5=QgNskxtmU-UN-P1pR1kFMZkHFMb2BPrJBQ@mail.gmail.com>
On 12/06/2016 06:42 PM, Fabio Estevam wrote:
> You missed to add imx6q-savageboard.dtb and imx6dl-savageboard.dtb
> entries into arch/arm/boot/dts/Makefile
Oh, I didn't notice because I build the dtbs manually.
Thanks for catching this.
And do you think other patches look OK?
Best regards,
Milo
^ permalink raw reply
* [PATCH v3 0/4] mm: fix the "counter.sh" failure for libhugetlbfs
From: Huang Shijie @ 2016-12-06 10:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161205093100.GF30758@dhcp22.suse.cz>
On Mon, Dec 05, 2016 at 05:31:01PM +0800, Michal Hocko wrote:
> On Mon 05-12-16 17:17:07, Huang Shijie wrote:
> [...]
> > The failure is caused by:
> > 1) kernel fails to allocate a gigantic page for the surplus case.
> > And the gather_surplus_pages() will return NULL in the end.
> >
> > 2) The condition checks for some functions are wrong:
> > return_unused_surplus_pages()
> > nr_overcommit_hugepages_store()
> > hugetlb_overcommit_handler()
>
> OK, so how is this any different from gigantic (1G) hugetlb pages on
I think there is no different from gigantic (1G) hugetlb pages on
x86_64. Do anyone ever tested the 1G hugetlb pages in x86_64 with the "counter.sh"
before?
> x86_64? Do we need the same functionality or is it just 32MB not being
> handled in the same way as 1G?
Yes, we need this functionality for gigantic pages, no matter it is
X86_64 or S390 or arm64, no matter it is 32MB or 1G. :)
But anyway, I will try to find some machine and try the 1G gigantic page
on ARM64.
Thanks
Huang Shijie
^ permalink raw reply
* [PATCH v5] ARM: davinci: da8xx: Fix sleeping function called from invalid context
From: Sekhar Nori @ 2016-12-06 9:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cbf4b99b-e453-acd5-d864-f709708fbab0@baylibre.com>
On Tuesday 06 December 2016 03:21 PM, Alexandre Bailon wrote:
> On 12/06/2016 10:33 AM, Sekhar Nori wrote:
>> On Monday 05 December 2016 07:43 PM, Alexandre Bailon wrote:
>>> Everytime the usb20 phy is enabled, there is a
>>> "sleeping function called from invalid context" BUG.
>>>
>>> clk_enable() from arch/arm/mach-davinci/clock.c uses spin_lock_irqsave()
>>> before to invoke the callback usb20_phy_clk_enable().
>>> usb20_phy_clk_enable() uses clk_get() and clk_enable_prepapre()
>>> which may sleep.
>>> Move clk_get() to da8xx_register_usb20_phy_clk() and
>>> replace clk_prepare_enable() by clk_enable().
>>>
>>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>>
>> This will still cause the recursive locking problem reported by David.
>> Not sure what the point of sending this version was.
>>
>> Thanks,
>> Sekhar
>>
> What am I supposed to do ?
That needs to be resolved between you and David. Perhaps convert the fix
sent by David into a proper patch and base this patch on that. Or wait
for David to send it himself and let him also make the modifications
needed in this patch.
David ?
Thanks,
Sekhar
^ permalink raw reply
* [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer driver in DT
From: Alexandre Torgue @ 2016-12-06 9:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206094835.GB25385@dell.home>
Hi Lee,
On 12/06/2016 10:48 AM, Lee Jones wrote:
> On Mon, 05 Dec 2016, Alexandre Torgue wrote:
>> On 12/02/2016 02:22 PM, Lee Jones wrote:
>>> On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
>>>
>>>> Add general purpose timers and it sub-nodes into DT for stm32f4.
>>>> Define and enable pwm1 and pwm3 for stm32f469 discovery board
>>>>
>>>> version 3:
>>>> - use "st,stm32-timer-trigger" in DT
>>>>
>>>> version 2:
>>>> - use parameters to describe hardware capabilities
>>>> - do not use references for pwm and iio timer subnodes
>>>>
>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>> ---
>>>> arch/arm/boot/dts/stm32f429.dtsi | 333 +++++++++++++++++++++++++++++++++-
>>>> arch/arm/boot/dts/stm32f469-disco.dts | 28 +++
>>>> 2 files changed, 360 insertions(+), 1 deletion(-)
>
> [...]
>
> If you're only commenting on a little piece of the patch, it's always
> a good idea to trim the rest.
>
>>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> index 8a163d7..df4ca7e 100644
>>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> @@ -81,3 +81,31 @@
>>>> &usart3 {
>>>> status = "okay";
>>>> };
>>>> +
>>>> +&gptimer1 {
>>>> + status = "okay";
>>>> +
>>>> + pwm1 at 0 {
>>>> + pinctrl-0 = <&pwm1_pins>;
>>>> + pinctrl-names = "default";
>>>> + status = "okay";
>>>> + };
>>>> +
>>>> + timer1 at 0 {
>>>> + status = "okay";
>>>> + };
>>>> +};
>>>
>>> This is a much *better* format than before.
>>>
>>> I still don't like the '&' syntax though.
>>
>> Please keep "&" format to match with existing nodes.
>
> Right. I wasn't suggesting that he differs from the current format in
> *this* set. I am suggesting that we change the format in a subsequent
> set though.
Why change? Looking at Linux ARM kernel patchwork, new DT board file
contains this format. Did you already discuss with Arnd or Olof about it
?
regards
Alex
>
>>>> +&gptimer3 {
>>>> + status = "okay";
>>>> +
>>>> + pwm3 at 0 {
>>>> + pinctrl-0 = <&pwm3_pins>;
>>>> + pinctrl-names = "default";
>>>> + status = "okay";
>>>> + };
>>>> +
>>>> + timer3 at 0 {
>>>> + status = "okay";
>>>> + };
>>>> +};
>>>
>
^ permalink raw reply
* [bug report v4.8] fs/locks.c: kernel oops during posix lock stress test
From: Ming Lei @ 2016-12-06 9:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161201113031.GB5813@arm.com>
Hi Will,
On Thu, Dec 1, 2016 at 7:30 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Nov 28, 2016 at 11:10:14AM +0800, Ming Lei wrote:
>> When I run stress-ng via the following steps on one ARM64 dual
>> socket system(Cavium Thunder), the kernel oops[1] can often be
>> triggered after running the stress test for several hours(sometimes
>> it may take longer):
>>
>> - git clone git://kernel.ubuntu.com/cking/stress-ng.git
>> - apply the attachment patch which just makes the posix file
>> lock stress test more aggressive
>> - run the test via '~/git/stress-ng$./stress-ng --lockf 128 --aggressive'
>>
>>
>> From the oops log, looks one garbage file_lock node is got
>> from the linked list of 'ctx->flc_posix' when the issue happens.
>>
>> BTW, the issue isn't observed on single socket Cavium Thunder yet,
>> and the same issue can be seen on Ubuntu Xenial(v4.4 based kernel)
>> too.
>
> FWIW, I've been running this on Seattle for 24 hours with your patch applied
> and not seen any problems yet. That said, Thomas did just fix an rt_mutex
> race which only seemed to pop up on Thunder, so you could give those
> patches a try.
>
> https://lkml.kernel.org/r/20161130205431.629977871 at linutronix.de
I applied the patch against Ubuntu Yakkety kernel(v4.8 based), and run
the test again on one dual-socket Cavium ThunderX system, and the
issue can still be triggered.
So looks not a same issue with David Daney's.
Anyway, thank you for providing this input!
Thanks,
Ming
^ permalink raw reply
* [PATCH v5] ARM: davinci: da8xx: Fix sleeping function called from invalid context
From: Alexandre Bailon @ 2016-12-06 9:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2fc7a4ef-02ad-fc15-66ea-9f75756086f8@ti.com>
On 12/06/2016 10:33 AM, Sekhar Nori wrote:
> On Monday 05 December 2016 07:43 PM, Alexandre Bailon wrote:
>> Everytime the usb20 phy is enabled, there is a
>> "sleeping function called from invalid context" BUG.
>>
>> clk_enable() from arch/arm/mach-davinci/clock.c uses spin_lock_irqsave()
>> before to invoke the callback usb20_phy_clk_enable().
>> usb20_phy_clk_enable() uses clk_get() and clk_enable_prepapre()
>> which may sleep.
>> Move clk_get() to da8xx_register_usb20_phy_clk() and
>> replace clk_prepare_enable() by clk_enable().
>>
>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>
> This will still cause the recursive locking problem reported by David.
> Not sure what the point of sending this version was.
>
> Thanks,
> Sekhar
>
What am I supposed to do ?
Thanks,
Alexandre
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox