* [PATCH] arm64: dts: hisilicon: Add hi3660 cpu capacity-dmips-mhz information
From: Valentin Schneider @ 2017-12-13 14:21 UTC (permalink / raw)
To: linux-arm-kernel
The following dt entries are added:
cpus [0-3] (Cortex A53):
- capacity-dmips-mhz = <592>;
cpus [4-7] (Cortex A73):
- capacity-dmips-mhz = <1024>;
Those values were obtained by running dhrystone 2.1 on a
HiKey960 with the following procedure:
- Offline all CPUs but CPU0 (A53)
- Set CPU0 frequency to maximum
- Run Dhrystone 2.1 for 20 seconds
- Offline all CPUs but CPU4 (A73)
- set CPU4 frequency to maximum
- Run Dhrystone 2.1 for 20 seconds
The results are as follows:
A53: 129633887 loops
A73: 287034147 loops
By scaling those values so that the A73s use 1024, we end up with 462
for the A53s. However, they have different maximum frequencies:
1.844GHz for A53s and 2.362GHz for A73s. Thus, we can scale the A53
value to truly represent dmips per MHz, and we end up with 592.
The impact of this change can be verified on HiKey960:
$ cat /sys/devices/system/cpu/cpu*/cpufreq/scaling_cur_freq
1844000
1844000
1844000
1844000
2362000
2362000
2362000
2362000
$ cat /sys/devices/system/cpu/cpu*/cpu_capacity
462
462
462
462
1024
1024
1024
1024
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index ab0b95b..04a8d28 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -61,6 +61,7 @@
enable-method = "psci";
next-level-cache = <&A53_L2>;
cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP_0>;
+ capacity-dmips-mhz = <592>;
};
cpu1: cpu at 1 {
@@ -70,6 +71,7 @@
enable-method = "psci";
next-level-cache = <&A53_L2>;
cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP_0>;
+ capacity-dmips-mhz = <592>;
};
cpu2: cpu at 2 {
@@ -79,6 +81,7 @@
enable-method = "psci";
next-level-cache = <&A53_L2>;
cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP_0>;
+ capacity-dmips-mhz = <592>;
};
cpu3: cpu at 3 {
@@ -88,6 +91,7 @@
enable-method = "psci";
next-level-cache = <&A53_L2>;
cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP_0>;
+ capacity-dmips-mhz = <592>;
};
cpu4: cpu at 100 {
@@ -101,6 +105,7 @@
&CPU_SLEEP
&CLUSTER_SLEEP_1
>;
+ capacity-dmips-mhz = <1024>;
};
cpu5: cpu at 101 {
@@ -114,6 +119,7 @@
&CPU_SLEEP
&CLUSTER_SLEEP_1
>;
+ capacity-dmips-mhz = <1024>;
};
cpu6: cpu at 102 {
@@ -127,6 +133,7 @@
&CPU_SLEEP
&CLUSTER_SLEEP_1
>;
+ capacity-dmips-mhz = <1024>;
};
cpu7: cpu at 103 {
@@ -140,6 +147,7 @@
&CPU_SLEEP
&CLUSTER_SLEEP_1
>;
+ capacity-dmips-mhz = <1024>;
};
idle-states {
--
2.7.4
^ permalink raw reply related
* [PATCH v2 1/3] arm64: mm: Support Common Not Private translations
From: James Morse @ 2017-12-13 14:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1507724395-13735-2-git-send-email-vladimir.murzin@arm.com>
Hi Vladimir,
On 11/10/17 13:19, Vladimir Murzin wrote:
> Common Not Private (CNP) is a feature of ARMv8.2 extension which
> allows translation table entries to be shared between different PEs in
> the same inner shareable domain, so the hardware can use this fact to
> optimise the caching of such entries in the TLB.
>
> CNP occupies one bit in TTBRx_ELy and VTTBR_EL2, which advertises to
> the hardware that the translation table entries pointed to by this
> TTBR are the same as every PE in the same inner shareable domain for
> which the equivalent TTBR also has CNP bit set. In case CNP bit is set
> but TTBR does not point at the same translation table entries or a
> given ASID and VMID, then the system is mis-configured, so the results
> of translations are UNPREDICTABLE.
>
> This patch adds support for Common Not Private translations on
> different exceptions levels:
>
> (1) For EL0 there are a few cases we need to care of changes in
> TTBR0_EL1:
> - a switch to idmap
> - software emulated PAN
> we rule out latter via Kconfig options and for the former we make
> sure that CNP is set for non-zero ASIDs only.
>
> (2) For EL1 we postpone setting CNP till all cpus are up and rely on
> cpufeature framework to 1) patch the code which is sensitive to
> CNP and 2) update TTBR1_EL1 with CNP bit set. TTBR1_EL1 can be
> reprogrammed as result of hibernation or cpuidle (via __enable_mmu).
> cpuidle's path has been changed to restore CnP and for hibernation
> the code has been changed to save raw TTBR1_EL1 and blindly restore
> it on resume.
While I remember:
This feature is going to be fun for kdump, we may leave secondary CPUs running
if they don't take the IPI to crash-out of the kernel. Worse, if we don't have
PSCI they just spin in a loop while the surviving CPU brings up the crash kernel
and maybe-enables CNP...
I think the best fix for this is to refuse to enable CNP at all if we're a crash
kernel. There is stuff in the DT to indicate this... we should know about the
'elfcorehdr' before cpufeature runs. (I don't think we should rely on the
cmdline option).
kexec is unaffected because it always powers-off the secondary CPUs before
leaving the old kernel. This behaves much more like a normal boot.
Thanks,
James
^ permalink raw reply
* [PATCH 2/3] reset: meson-axg: add compatible string for Meson-AXG SoC
From: Yixun Lan @ 2017-12-13 14:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171110084650.31325-2-yixun.lan@amlogic.com>
Hi Philipp
On 11/10/2017 04:46 PM, Yixun Lan wrote:
> Try to add compatible string explictly to support new Meson-AXG SoC.
>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> ---
> drivers/reset/reset-meson.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
> index c419a3753d00..93cbee1ae8ef 100644
> --- a/drivers/reset/reset-meson.c
> +++ b/drivers/reset/reset-meson.c
> @@ -139,6 +139,8 @@ static const struct of_device_id meson_reset_dt_ids[] = {
> .data = &meson_reset_meson8_ops, },
> { .compatible = "amlogic,meson-gxbb-reset",
> .data = &meson_reset_gx_ops, },
> + { .compatible = "amlogic,meson-axg-reset",
> + .data = &meson_reset_gx_ops, },
> { /* sentinel */ },
> };
>
>
it's generally a ping to the status of these two patches[1], are they
ready to go? or do you have any comment? or do you want me to send
another version with Neil's Reviewed-by added[1]
thanks
Yixun
[1]
http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005278.html
[2]
http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005318.html
http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005319.html
^ permalink raw reply
* [PATCH v2 2/5] i2c: meson: add configurable divider factors
From: Yixun Lan @ 2017-12-13 13:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171120145415.6581-3-yixun.lan@amlogic.com>
Hi Wolfram
On 11/20/2017 10:54 PM, Yixun Lan wrote:
> From: Jian Hu <jian.hu@amlogic.com>
>
> This patch try to add support for I2C controller in Meson-AXG SoC,
> Due to the IP changes between I2C controller, we need to introduce
> a compatible data to make the divider factor configurable.
>
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> ---
> drivers/i2c/busses/i2c-meson.c | 32 ++++++++++++++++++++++++++++----
> 1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
> index 88d15b92ec35..37c4aa76f37a 100644
> --- a/drivers/i2c/busses/i2c-meson.c
> +++ b/drivers/i2c/busses/i2c-meson.c
> @@ -16,6 +16,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/types.h>
>
> @@ -57,6 +58,10 @@ enum {
> STATE_WRITE,
> };
>
> +struct meson_i2c_data {
> + unsigned char div_factor;
> +};
> +
> /**
> * struct meson_i2c - Meson I2C device private data
> *
> @@ -93,6 +98,8 @@ struct meson_i2c {
> struct completion done;
> u32 tokens[2];
> int num_tokens;
> +
> + const struct meson_i2c_data *data;
> };
>
> static void meson_i2c_set_mask(struct meson_i2c *i2c, int reg, u32 mask,
> @@ -128,7 +135,7 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c, unsigned int freq)
> unsigned long clk_rate = clk_get_rate(i2c->clk);
> unsigned int div;
>
> - div = DIV_ROUND_UP(clk_rate, freq * 4);
> + div = DIV_ROUND_UP(clk_rate, freq * i2c->data->div_factor);
>
> /* clock divider has 12 bits */
> if (div >= (1 << 12)) {
> @@ -376,6 +383,9 @@ static int meson_i2c_probe(struct platform_device *pdev)
> spin_lock_init(&i2c->lock);
> init_completion(&i2c->done);
>
> + i2c->data = (const struct meson_i2c_data *)
> + of_device_get_match_data(&pdev->dev);
> +
> i2c->clk = devm_clk_get(&pdev->dev, NULL);
> if (IS_ERR(i2c->clk)) {
> dev_err(&pdev->dev, "can't get device clock\n");
> @@ -440,11 +450,25 @@ static int meson_i2c_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct meson_i2c_data i2c_meson6_data = {
> + .div_factor = 4,
> +};
> +
> +static const struct meson_i2c_data i2c_gxbb_data = {
> + .div_factor = 4,
> +};
> +
> +static const struct meson_i2c_data i2c_axg_data = {
> + .div_factor = 3,
> +};
> +
> static const struct of_device_id meson_i2c_match[] = {
> - { .compatible = "amlogic,meson6-i2c" },
> - { .compatible = "amlogic,meson-gxbb-i2c" },
> - { },
> + { .compatible = "amlogic,meson6-i2c", .data = &i2c_meson6_data },
> + { .compatible = "amlogic,meson-gxbb-i2c", .data = &i2c_gxbb_data },
> + { .compatible = "amlogic,meson-axg-i2c", .data = &i2c_axg_data },
> + {},
> };
> +
> MODULE_DEVICE_TABLE(of, meson_i2c_match);
>
> static struct platform_driver meson_i2c_driver = {
>
this is merely a ping, do you have any comment on this patch?
or could you take this patch along with patch 1 [1](with Rob's Ack) if
you think it's ready/good?
thanks
[1]
http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005405.html
http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005417.html
Yixun
^ permalink raw reply
* [linux-sunxi] [PATCH v2 3/6] ARM: sun4i: Convert to CCU
From: Maxime Ripard @ 2017-12-13 13:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOi56cUeRrKQnJ-akJPB160-60aBzy64bDgFeoqMpHRJSCnDeQ@mail.gmail.com>
On Tue, Dec 12, 2017 at 01:24:52PM -0800, Kevin Hilman wrote:
> On Tue, Dec 12, 2017 at 9:26 AM, Priit Laes <plaes@plaes.org> wrote:
> > On Mon, Dec 11, 2017 at 02:22:30PM -0800, Kevin Hilman wrote:
> >> On Sun, Mar 26, 2017 at 10:20 AM, Priit Laes <plaes@plaes.org> wrote:
> >> > Convert sun4i-a10.dtsi to new CCU driver.
> >> >
> >> > Signed-off-by: Priit Laes <plaes@plaes.org>
> >>
> >> I finally got around to bisecting a mainline boot failure on
> >> sun4i-a10-cubieboard that's been happening for quite a while. Based
> >> on on kernelci.org, it showed up sometime during the v4.15 merge
> >> window[1]. It bisected down to this commit (in mainline as commit
> >> 41193869f2bdb585ce09bfdd16d9482aadd560ad).
> >>
> >> When it fails, there is no output on the serial console, so I don't
> >> know exactly how it's failing, just that it no longer boots.
> >
> > We tried out latest 4.15 with various compilers and it works:
> > - gcc version 7.1.1 20170622 (Red Hat Cross 7.1.1-3) (GCC) - A10 Gemei G9 tablet
> > - gcc 7.2.0-debian - A10 Cubieboard
>
> And you can reproduce the bug with gcc5 or gcc6?
>
> Very strange that a DT only patch would cause a gcc related regression
> and if it does, it should be investigated. I don't think requiring
> gcc7 is an appropriate solution.
>
> @Chen-Yu, @Maxime: are you guys OK with requiring gcc7 for working
> upstream boot for A10?
I'd rather not set that kind of constraints and fix the issue instead.
Priit, can you test with an older compiler?
You can find one here:
http://toolchains.free-electrons.com/
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171213/992234f8/attachment.sig>
^ permalink raw reply
* [xlnx:2017.3_video_ea 6585/6607] warning: (VIDEO_XILINX_HDMI_RX && ..) selects PHY_XILINX_VPHY which has unmet direct dependencies (STAGING && ..)
From: kbuild test robot @ 2017-12-13 13:03 UTC (permalink / raw)
To: linux-arm-kernel
tree: https://github.com/Xilinx/linux-xlnx 2017.3_video_ea
head: af045f9682c65a0c26afb2f638603d3c01079222
commit: b58dab7f9cf13f9004a220128317fc85a1b75b63 [6585/6607] staging: xilinx: Add xilinx hdmi drivers to staging area
config: x86_64-randconfig-x013-12131947 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
git checkout b58dab7f9cf13f9004a220128317fc85a1b75b63
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
warning: (VIDEO_XILINX_HDMI_RX && DRM_XILINX_HDMI) selects PHY_XILINX_VPHY which has unmet direct dependencies (STAGING && ARCH_ZYNQMP)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 24216 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171213/de4e2d41/attachment-0001.gz>
^ permalink raw reply
* [PATCH 1/3] ARM: dts: stm32: add DMA memory pool on MCU which embed a cortex-M7
From: Alexandre TORGUE @ 2017-12-13 12:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ab9a2ef4-4944-c935-af6e-12503b477879@arm.com>
-----Original Message-----
From: Alexandre TORGUE
Sent: mercredi 13 d?cembre 2017 11:28
To: 'Vladimir Murzin' <vladimir.murzin@arm.com>; Maxime Coquelin <mcoquelin.stm32@gmail.com>; arnd at arndb.de; robh+dt at kernel.org; mark.rutland at arm.com; linux at armlinux.org.uk; Patrice CHOTARD <patrice.chotard@st.com>; lee.jones at linaro.org
Cc: devicetree at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: RE: [PATCH 1/3] ARM: dts: stm32: add DMA memory pool on MCU which embed a cortex-M7
-----Original Message-----
From: Vladimir Murzin [mailto:vladimir.murzin at arm.com]
Sent: mercredi 13 d?cembre 2017 11:07
To: Alexandre TORGUE <alexandre.torgue@st.com>; Maxime Coquelin <mcoquelin.stm32@gmail.com>; arnd at arndb.de; robh+dt at kernel.org; mark.rutland at arm.com; linux at armlinux.org.uk; Patrice CHOTARD <patrice.chotard@st.com>; lee.jones at linaro.org
Cc: devicetree at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH 1/3] ARM: dts: stm32: add DMA memory pool on MCU which embed a cortex-M7
On 12/12/17 18:02, Alexandre Torgue wrote:
> On cortex-M7 MCU, DMA have to use a non cache-able memory area. For
> this reason a dedicated memory pool is created for DMA.
> This patch creates a DMA memory pool of 1MB of each STM32 MCU which
> embeds a cortex-M7 expect stm32f746-disco. Indeed, as stm32f746-disco
> has
^^^^^^
except?
Sorry, Is there a typo issue (or just wording issue) ?
Sorry I was not awake this morning. If no v2 I will fix this typo when I will apply patch on my tree.
> only a 8MB SDRAM and it's tricky to reduce memory used by Kernel.
I guess that 1MB is a kind of "should be enough" estimate, probably something along with [1] would give you exact numbers...
Exactly, 1MB is a kind "should be enough" and code is here to show that we need a dedicated memory area for DMA. But this value has to be adapt regarding to use case needed by users. Thanks for the lkml link. It will help users to adapt DMA area and thanks for reviewing.
Regards
Alex
>
> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>
> diff --git a/arch/arm/boot/dts/stm32746g-eval.dts
> b/arch/arm/boot/dts/stm32746g-eval.dts
> index 2d4e717..3f52a7b 100644
> --- a/arch/arm/boot/dts/stm32746g-eval.dts
> +++ b/arch/arm/boot/dts/stm32746g-eval.dts
> @@ -57,6 +57,19 @@
> reg = <0xc0000000 0x2000000>;
> };
>
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + linux,dma {
> + compatible = "shared-dma-pool";
> + linux,dma-default;
> + no-map;
> + reg = <0xc1f00000 0x100000>;
> + };
> + };
> +
> aliases {
> serial0 = &usart1;
> };
> diff --git a/arch/arm/boot/dts/stm32f769-disco.dts
> b/arch/arm/boot/dts/stm32f769-disco.dts
> index 4463ca1..08699a2 100644
> --- a/arch/arm/boot/dts/stm32f769-disco.dts
> +++ b/arch/arm/boot/dts/stm32f769-disco.dts
> @@ -57,6 +57,19 @@
> reg = <0xC0000000 0x1000000>;
> };
>
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + linux,dma {
> + compatible = "shared-dma-pool";
> + linux,dma-default;
> + no-map;
> + reg = <0xc0f00000 0x100000>;
> + };
> + };
> +
> aliases {
> serial0 = &usart1;
> };
> diff --git a/arch/arm/boot/dts/stm32h743i-disco.dts
> b/arch/arm/boot/dts/stm32h743i-disco.dts
> index 79e841d..104545a 100644
> --- a/arch/arm/boot/dts/stm32h743i-disco.dts
> +++ b/arch/arm/boot/dts/stm32h743i-disco.dts
> @@ -57,6 +57,19 @@
> reg = <0xd0000000 0x2000000>;
> };
>
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + linux,dma {
> + compatible = "shared-dma-pool";
> + linux,dma-default;
> + no-map;
> + reg = <0xc1f00000 0x100000>;
> + };
> + };
> +
> aliases {
> serial0 = &usart2;
> };
> diff --git a/arch/arm/boot/dts/stm32h743i-eval.dts
> b/arch/arm/boot/dts/stm32h743i-eval.dts
> index 9f0e72c..5bd4b16 100644
> --- a/arch/arm/boot/dts/stm32h743i-eval.dts
> +++ b/arch/arm/boot/dts/stm32h743i-eval.dts
> @@ -57,6 +57,19 @@
> reg = <0xd0000000 0x2000000>;
> };
>
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + linux,dma {
> + compatible = "shared-dma-pool";
> + linux,dma-default;
> + no-map;
> + reg = <0xc1f00000 0x100000>;
> + };
> + };
> +
> aliases {
> serial0 = &usart1;
> };
>
Usage of dma-default looks correct to me, so FWIW
Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>
[1] https://lkml.org/lkml/2017/7/7/296
Vladimir
^ permalink raw reply
* [PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures
From: Lorenzo Pieralisi @ 2017-12-13 12:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211155059.17062-2-prarit@redhat.com>
[+Mark, Graeme]
In $SUBJECT, s/avialable/available
On Mon, Dec 11, 2017 at 10:50:58AM -0500, Prarit Bhargava wrote:
> Other architectures can use SPCR to setup an early console or console
> but the current code is ARM64 specific.
I see nothing ARM64 specific in current code (apart from some
ACPICA macros with an ARM tag in them) please explain to me
what's preventing you to reuse current code on x86.
> Change the name of parse_spcr() to acpi_parse_spcr(). Add a weak
> function acpi_arch_setup_console() that can be used for arch-specific
> setup. Move flags into ACPI code. Update the Documention on the use of
> the SPCR.
>
> [v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
> Keep ACPI_SPCR_TABLE selected for ARM64. Fix 8-bit port access width
> mmio value. Move baud rate check earlier.
This does not belong in the commit log.
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: linux-doc at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-pm at vger.kernel.org
> Cc: linux-acpi at vger.kernel.org
> Cc: linux-serial at vger.kernel.org
> Cc: Bhupesh Sharma <bhsharma@redhat.com>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86 at kernel.org
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Timur Tabi <timur@codeaurora.org>
> ---
> Documentation/admin-guide/kernel-parameters.txt | 6 +-
> arch/arm64/kernel/acpi.c | 128 ++++++++++++++++-
> drivers/acpi/Kconfig | 7 +-
> drivers/acpi/spcr.c | 175 ++++++------------------
> drivers/tty/serial/earlycon.c | 15 +-
> include/linux/acpi.h | 11 +-
> include/linux/serial_core.h | 2 -
> 7 files changed, 184 insertions(+), 160 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 6571fbfdb2a1..0d173289c67e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -914,9 +914,9 @@
>
> earlycon= [KNL] Output early console device and options.
>
> - When used with no options, the early console is
> - determined by the stdout-path property in device
> - tree's chosen node.
> + [ARM64] The early console is determined by the
> + stdout-path property in device tree's chosen node,
> + or determined by the ACPI SPCR table.
>
> cdns,<addr>[,options]
> Start an early, polled-mode console on a Cadence
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index b3162715ed78..b3e33bbdf3b7 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -25,7 +25,6 @@
> #include <linux/memblock.h>
> #include <linux/of_fdt.h>
> #include <linux/smp.h>
> -#include <linux/serial_core.h>
>
> #include <asm/cputype.h>
> #include <asm/cpu_ops.h>
> @@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
> return ret;
> }
>
> +/*
> + * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> + * occasionally getting stuck as 1. To avoid the potential for a hang, check
> + * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> + * implementations, so only do so if an affected platform is detected in
> + * acpi_parse_spcr().
> + */
> +bool qdf2400_e44_present;
> +EXPORT_SYMBOL(qdf2400_e44_present);
My eyes, this is horrible but it is not introduced by this patch. It
would have been much better if:
drivers/tty/serial/amba-pl011.c
parsed the SPCR table (again) to detect it instead of relying on this
horrible exported flag.
> +
> +/*
> + * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> + * Detect them by examining the OEM fields in the SPCR header, similar to PCI
> + * quirk detection in pci_mcfg.c.
> + */
> +static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> +{
> + if (memcmp(h->oem_id, "QCOM ", ACPI_OEM_ID_SIZE))
> + return false;
> +
> + if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> + return true;
> +
> + if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> + h->oem_revision == 1)
> + return true;
> +
> + return false;
> +}
> +
> +/*
> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> + * register aligned to 32-bit. In addition, the BIOS also encoded the
> + * access width to be 8 bits. This function detects this errata condition.
> + */
> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> +{
> + bool xgene_8250 = false;
> +
> + if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> + return false;
> +
> + if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> + memcmp(tb->header.oem_id, "HPE ", ACPI_OEM_ID_SIZE))
> + return false;
> +
> + if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> + ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> + xgene_8250 = true;
> +
> + if (!memcmp(tb->header.oem_table_id, "ProLiant",
> + ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> + xgene_8250 = true;
> +
> + return xgene_8250;
> +}
> +
> +int acpi_arch_setup_console(struct acpi_table_spcr *table,
> + char *opts, char *uart, char *iotype,
> + int baud_rate, bool earlycon)
> +{
> + if (table->header.revision < 2) {
> + pr_err("wrong table version\n");
> + return -ENOENT;
> + }
> +
> + switch (table->interface_type) {
> + case ACPI_DBG2_ARM_SBSA_32BIT:
> + snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> + /* fall through */
> + case ACPI_DBG2_ARM_PL011:
> + case ACPI_DBG2_ARM_SBSA_GENERIC:
> + case ACPI_DBG2_BCM2835:
> + snprintf(uart, ACPI_SPCR_BUF_SIZE, "pl011");
> + break;
> + default:
> + if (strlen(uart) == 0)
> + return -ENOENT;
> + }
> +
> + /*
> + * If the E44 erratum is required, then we need to tell the pl011
> + * driver to implement the work-around.
> + *
> + * The global variable is used by the probe function when it
> + * creates the UARTs, whether or not they're used as a console.
> + *
> + * If the user specifies "traditional" earlycon, the qdf2400_e44
> + * console name matches the EARLYCON_DECLARE() statement, and
> + * SPCR is not used. Parameter "earlycon" is false.
> + *
> + * If the user specifies "SPCR" earlycon, then we need to update
> + * the console name so that it also says "qdf2400_e44". Parameter
> + * "earlycon" is true.
> + *
> + * For consistency, if we change the console name, then we do it
> + * for everyone, not just earlycon.
> + */
> + if (qdf2400_erratum_44_present(&table->header)) {
> + qdf2400_e44_present = true;
> + if (earlycon)
> + snprintf(uart, ACPI_SPCR_BUF_SIZE, "qdf2400_e44");
> + }
> +
> + if (xgene_8250_erratum_present(table)) {
> + snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> +
> + /* for xgene v1 and v2 we don't know the clock rate of the
> + * UART so don't attempt to change to the baud rate state
> + * in the table because driver cannot calculate the dividers
> + */
> + snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx", uart,
> + iotype, table->serial_port.address);
> + } else {
> + snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart,
> + iotype, table->serial_port.address, baud_rate);
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(acpi_arch_setup_console);
EXPORT_SYMBOL() ? Why ?
BTW, why do we need an arch hook ? I do not see anything that prevents
you from using this code on x86 systems - is there anything arch
specific in the SPCR specification itself ?
> +
> /*
> * acpi_boot_table_init() called from setup_arch(), always.
> * 1. find RSDP and get its address, and then find XSDT
> @@ -230,10 +351,11 @@ void __init acpi_boot_table_init(void)
>
> done:
> if (acpi_disabled) {
> - if (earlycon_init_is_deferred)
> + if (console_acpi_spcr_enable)
> early_init_dt_scan_chosen_stdout();
> } else {
> - parse_spcr(earlycon_init_is_deferred);
> + /* Always enable the ACPI SPCR console */
> + acpi_parse_spcr(console_acpi_spcr_enable);
> if (IS_ENABLED(CONFIG_ACPI_BGRT))
> acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
> }
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 46505396869e..9ae98eeada76 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
> endif
>
> config ACPI_SPCR_TABLE
> - bool
> + bool "ACPI Serial Port Console Redirection Support"
> + default y if ARM64
You need to remove the selection in arch/arm64 then. Also, moving away
from a non-visible config may have consequences on ARM64, Graeme and
Mark are more familiar with the SPCR dependencies so please chime in.
> + help
> + Enable support for Serial Port Console Redirection (SPCR) Table.
> + This table provides information about the configuration of the
> + earlycon console.
>
> config ACPI_LPIT
> bool
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> index 324b35bfe781..f4bb8110e404 100644
> --- a/drivers/acpi/spcr.c
> +++ b/drivers/acpi/spcr.c
> @@ -16,65 +16,18 @@
> #include <linux/kernel.h>
> #include <linux/serial_core.h>
>
> -/*
> - * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> - * occasionally getting stuck as 1. To avoid the potential for a hang, check
> - * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> - * implementations, so only do so if an affected platform is detected in
> - * parse_spcr().
> - */
> -bool qdf2400_e44_present;
> -EXPORT_SYMBOL(qdf2400_e44_present);
> -
> -/*
> - * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> - * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
> - * quirk detection in pci_mcfg.c.
> - */
> -static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> -{
> - if (memcmp(h->oem_id, "QCOM ", ACPI_OEM_ID_SIZE))
> - return false;
> -
> - if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> - return true;
> -
> - if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> - h->oem_revision == 1)
> - return true;
> -
> - return false;
> -}
> -
> -/*
> - * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> - * register aligned to 32-bit. In addition, the BIOS also encoded the
> - * access width to be 8 bits. This function detects this errata condition.
> - */
> -static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> +int __weak acpi_arch_setup_console(struct acpi_table_spcr *table,
> + char *opts, char *uart, char *iotype,
> + int baud_rate, bool earlycon)
> {
> - bool xgene_8250 = false;
> -
> - if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> - return false;
> -
> - if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> - memcmp(tb->header.oem_id, "HPE ", ACPI_OEM_ID_SIZE))
> - return false;
> -
> - if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> - ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> - xgene_8250 = true;
> -
> - if (!memcmp(tb->header.oem_table_id, "ProLiant",
> - ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> - xgene_8250 = true;
> -
> - return xgene_8250;
> + snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart, iotype,
> + table->serial_port.address, baud_rate);
> + return 0;
> }
>
> +bool console_acpi_spcr_enable __initdata;
> /**
> - * parse_spcr() - parse ACPI SPCR table and add preferred console
> + * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
> *
> * @earlycon: set up earlycon for the console specified by the table
> *
> @@ -86,13 +39,13 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> * from arch initialization code as soon as the DT/ACPI decision is made.
> *
> */
> -int __init parse_spcr(bool earlycon)
> +int __init acpi_parse_spcr(bool earlycon)
> {
> - static char opts[64];
> + static char opts[ACPI_SPCR_OPTS_SIZE];
> + static char uart[ACPI_SPCR_BUF_SIZE];
> + static char iotype[ACPI_SPCR_BUF_SIZE];
> struct acpi_table_spcr *table;
> acpi_status status;
> - char *uart;
> - char *iotype;
> int baud_rate;
> int err;
>
> @@ -105,48 +58,6 @@ int __init parse_spcr(bool earlycon)
> if (ACPI_FAILURE(status))
> return -ENOENT;
>
> - if (table->header.revision < 2) {
> - err = -ENOENT;
> - pr_err("wrong table version\n");
> - goto done;
> - }
> -
> - if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> - switch (ACPI_ACCESS_BIT_WIDTH((
> - table->serial_port.access_width))) {
> - default:
> - pr_err("Unexpected SPCR Access Width. Defaulting to byte size\n");
> - case 8:
> - iotype = "mmio";
> - break;
> - case 16:
> - iotype = "mmio16";
> - break;
> - case 32:
> - iotype = "mmio32";
> - break;
> - }
> - } else
> - iotype = "io";
> -
> - switch (table->interface_type) {
> - case ACPI_DBG2_ARM_SBSA_32BIT:
> - iotype = "mmio32";
> - /* fall through */
> - case ACPI_DBG2_ARM_PL011:
> - case ACPI_DBG2_ARM_SBSA_GENERIC:
> - case ACPI_DBG2_BCM2835:
> - uart = "pl011";
> - break;
> - case ACPI_DBG2_16550_COMPATIBLE:
> - case ACPI_DBG2_16550_SUBSET:
> - uart = "uart";
> - break;
> - default:
> - err = -ENOENT;
> - goto done;
> - }
> -
> switch (table->baud_rate) {
> case 3:
> baud_rate = 9600;
> @@ -165,43 +76,36 @@ int __init parse_spcr(bool earlycon)
> goto done;
> }
>
> - /*
> - * If the E44 erratum is required, then we need to tell the pl011
> - * driver to implement the work-around.
> - *
> - * The global variable is used by the probe function when it
> - * creates the UARTs, whether or not they're used as a console.
> - *
> - * If the user specifies "traditional" earlycon, the qdf2400_e44
> - * console name matches the EARLYCON_DECLARE() statement, and
> - * SPCR is not used. Parameter "earlycon" is false.
> - *
> - * If the user specifies "SPCR" earlycon, then we need to update
> - * the console name so that it also says "qdf2400_e44". Parameter
> - * "earlycon" is true.
> - *
> - * For consistency, if we change the console name, then we do it
> - * for everyone, not just earlycon.
> - */
> - if (qdf2400_erratum_44_present(&table->header)) {
> - qdf2400_e44_present = true;
> - if (earlycon)
> - uart = "qdf2400_e44";
> + switch (table->interface_type) {
> + case ACPI_DBG2_16550_COMPATIBLE:
> + case ACPI_DBG2_16550_SUBSET:
> + snprintf(uart, ACPI_SPCR_BUF_SIZE, "uart");
> + break;
> + default:
> + break;
> }
>
> - if (xgene_8250_erratum_present(table)) {
> - iotype = "mmio32";
> + if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> + u8 width = ACPI_ACCESS_BIT_WIDTH((
> + table->serial_port.access_width));
> + switch (width) {
> + default:
> + pr_err("Unexpected SPCR Access Width. Defaulting to byte size\n");
> + case 8:
> + snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio");
> + break;
> + case 16:
> + case 32:
> + snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio%d", width);
> + break;
> + }
> + } else
> + snprintf(iotype, ACPI_SPCR_BUF_SIZE, "io");
>
> - /* for xgene v1 and v2 we don't know the clock rate of the
> - * UART so don't attempt to change to the baud rate state
> - * in the table because driver cannot calculate the dividers
> - */
> - snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
> - table->serial_port.address);
> - } else {
> - snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
> - table->serial_port.address, baud_rate);
> - }
> + err = acpi_arch_setup_console(table, opts, uart, iotype, baud_rate,
> + earlycon);
> + if (err)
> + goto done;
>
> pr_info("console: %s\n", opts);
>
> @@ -209,7 +113,6 @@ int __init parse_spcr(bool earlycon)
> setup_earlycon(opts);
>
> err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
> -
Unintended change.
> done:
> acpi_put_table((struct acpi_table_header *)table);
> return err;
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 4c8b80f1c688..b22afb62c7a3 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -196,26 +196,15 @@ int __init setup_earlycon(char *buf)
> return -ENOENT;
> }
>
> -/*
> - * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
> - * command line does not start DT earlycon immediately, instead it defers
> - * starting it until DT/ACPI decision is made. At that time if ACPI is enabled
> - * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
> - */
> -bool earlycon_init_is_deferred __initdata;
> -
> /* early_param wrapper for setup_earlycon() */
> static int __init param_setup_earlycon(char *buf)
> {
> int err;
>
> - /*
> - * Just 'earlycon' is a valid param for devicetree earlycons;
> - * don't generate a warning from parse_early_params() in that case
> - */
> + /* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
> if (!buf || !buf[0]) {
> if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
> - earlycon_init_is_deferred = true;
> + console_acpi_spcr_enable = true;
I am not familiar with this code, I would ask Graeme and Mark to check
if this change is correct, the logic seems correct to me but I may be
missing some corner cases.
> return 0;
> } else if (!buf) {
> return early_init_dt_scan_chosen_stdout();
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index dc1ebfeeb5ec..875d7327d91c 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1241,10 +1241,17 @@ static inline bool acpi_has_watchdog(void) { return false; }
> #endif
>
> #ifdef CONFIG_ACPI_SPCR_TABLE
> +#define ACPI_SPCR_OPTS_SIZE 64
> +#define ACPI_SPCR_BUF_SIZE 32
> extern bool qdf2400_e44_present;
> -int parse_spcr(bool earlycon);
> +extern bool console_acpi_spcr_enable __initdata;
> +extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
> + char *opts, char *uart, char *iotype,
> + int baud_rate, bool earlycon);
> +int acpi_parse_spcr(bool earlycon);
> #else
> -static inline int parse_spcr(bool earlycon) { return 0; }
> +static const bool console_acpi_spcr_enable;
The assignment in param_setup_earlycon won't compile.
Lorenzo
^ permalink raw reply
* [PATCH] phy: rockchip-typec: Try to turn the PHY on several times
From: Enric Balletbo Serra @ 2017-12-13 12:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211214553.118337-1-dianders@chromium.org>
Hi Doug,
2017-12-11 22:45 GMT+01:00 Douglas Anderson <dianders@chromium.org>:
> Bind / unbind stress testing of the USB controller on rk3399 found
> that we'd often end up with lots of failures that looked like this:
>
> phy phy-ff800000.phy.9: phy poweron failed --> -110
> dwc3 fe900000.dwc3: failed to initialize core
> dwc3: probe of fe900000.dwc3 failed with error -110
>
> Those errors were sometimes seen at bootup too, in which case USB
> peripherals wouldn't work until unplugged and re-plugged in.
>
> I spent some time trying to figure out why the PHY was failing to
> power on but I wasn't able to. Possibly this has to do with the fact
> that the PHY docs say that the USB controller "needs to be held in
> reset to hold pipe power state in P2 before initializing the Type C
> PHY" but that doesn't appear to be easy to do with the dwc3 driver
> today. Messing around with the ordering of the reset vs. the PHY
> initialization in the dwc3 driver didn't seem to fix things.
>
> I did, however, find that if I simply retry the power on it seems to
> have a good chance of working. So let's add some retries. I ran a
> pretty tight bind/unbind loop overnight. When I did so, I found that
> I need to retry between 1% and 2% of the time. Overnight I found only
> a small handful of times where I needed 2 retries. I never found a
> case where I needed 3 retries.
>
> I'm completely aware of the fact that this is quite an ugly hack and I
> wish I didn't have to resort to it, but I have no other real idea how
> to make this hardware reliable. If Rockchip in the future can come up
> with a solution we can always revert this hack. Until then, let's at
> least have something that works.
>
> This patch is tested atop Enric's latest dwc3 patch series ending at:
> https://patchwork.kernel.org/patch/10095527/
> ...but it could be applied independently of that series without any
> bad effects.
>
> For some more details on this bug, you can refer to:
> https://bugs.chromium.org/p/chromium/issues/detail?id=783464
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> drivers/phy/rockchip/phy-rockchip-typec.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> index ee85fa0ca4b0..5c2157156ce1 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
> @@ -349,6 +349,8 @@
> #define MODE_DFP_USB BIT(1)
> #define MODE_DFP_DP BIT(2)
>
> +#define POWER_ON_TRIES 5
> +
I did the test of increase the number of tries to 100 because
unfortunately, even with this patch applied, I can see the problem on
my kevin with current mainline.
[ 244.309094] rockchip-typec-phy ff800000.phy: Turn on failed after 100 loops
That's an extra debug print ^
[ 244.317019] phy phy-ff800000.phy.8: phy poweron failed --> -110
[ 244.323824] dwc3 fe900000.dwc3: failed to initialize core
[ 244.330057] dwc3: probe of fe900000.dwc3 failed with error -110
So I'm wondering if there is something else that I need to apply to
really fix this as you didn't reproduce the issue doing lots of tests
and I can reproduce the issue very easily.
> struct usb3phy_reg {
> u32 offset;
> u32 enable_bit;
> @@ -818,9 +820,8 @@ static int tcphy_get_mode(struct rockchip_typec_phy *tcphy)
> return mode;
> }
>
> -static int rockchip_usb3_phy_power_on(struct phy *phy)
> +static int _rockchip_usb3_phy_power_on(struct rockchip_typec_phy *tcphy)
> {
> - struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
> struct rockchip_usb3phy_port_cfg *cfg = &tcphy->port_cfgs;
> const struct usb3phy_reg *reg = &cfg->pipe_status;
> int timeout, new_mode, ret = 0;
> @@ -867,6 +868,25 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
> return ret;
> }
>
> +static int rockchip_usb3_phy_power_on(struct phy *phy)
> +{
> + struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
> + int ret;
> + int tries;
> +
> + for (tries = 0; tries < POWER_ON_TRIES; tries++) {
> + ret = _rockchip_usb3_phy_power_on(tcphy);
> + if (!ret)
> + break;
> + }
> +
> + if (tries && !ret)
> + dev_info(tcphy->dev, "Needed %d loops to turn on\n", tries);
> +
It's curious that in my case I never see this message, or it works or
it fails after 100 retries. I'll do more longer tests and continue
investigating a little bit.
Regards,
Enric
> + return ret;
> +}
> +
> +
> static int rockchip_usb3_phy_power_off(struct phy *phy)
> {
> struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
> --
> 2.15.1.424.g9478a66081-goog
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 2/2] ARM: dts: sun8i: Add ADC routing
From: Mylène Josserand @ 2017-12-13 12:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213123408.10422-1-mylene.josserand@free-electrons.com>
Add the ADC route between the analog and the digital parts
of sun8i A33. Configure the MIC1 to use MBIAS and MIC2 to use HBIAS.
Signed-off-by: Myl?ne Josserand <mylene.josserand@free-electrons.com>
---
arch/arm/boot/dts/sun8i-a33.dtsi | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
index 22660919bd08..1841eecd5993 100644
--- a/arch/arm/boot/dts/sun8i-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a33.dtsi
@@ -191,7 +191,15 @@
simple-audio-card,aux-devs = <&codec_analog>;
simple-audio-card,routing =
"Left DAC", "AIF1 Slot 0 Left",
- "Right DAC", "AIF1 Slot 0 Right";
+ "Right DAC", "AIF1 Slot 0 Right",
+ "AIF1 Slot 0 Left ADC", "Left ADC",
+ "AIF1 Slot 0 Right ADC", "Right ADC",
+ "Left ADC", "ADC",
+ "Right ADC", "ADC",
+ "Mic", "MBIAS",
+ "Headset Mic", "HBIAS",
+ "MIC1", "Mic",
+ "MIC2", "Headset Mic";
status = "disabled";
simple-audio-card,cpu {
--
2.11.0
^ permalink raw reply related
* [PATCH 1/2] ASoC: sun8i-codec: Add ADC support for a33
From: Mylène Josserand @ 2017-12-13 12:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213123408.10422-1-mylene.josserand@free-electrons.com>
Add ADC support for the sun8i-codec driver.
This driver uses microphones widgets and routes provided by the
analog part (sun8i-codec-analog).
Some digital configurations are needed by creating new ADC widgets
and routes.
Signed-off-by: Myl?ne Josserand <mylene.josserand@free-electrons.com>
---
sound/soc/sunxi/sun8i-codec.c | 82 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 80 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c
index 3dd183be08a4..7a15df924316 100644
--- a/sound/soc/sunxi/sun8i-codec.c
+++ b/sound/soc/sunxi/sun8i-codec.c
@@ -37,9 +37,11 @@
#define SUN8I_SYSCLK_CTL_SYSCLK_SRC 0
#define SUN8I_MOD_CLK_ENA 0x010
#define SUN8I_MOD_CLK_ENA_AIF1 15
+#define SUN8I_MOD_CLK_ENA_ADC 3
#define SUN8I_MOD_CLK_ENA_DAC 2
#define SUN8I_MOD_RST_CTL 0x014
#define SUN8I_MOD_RST_CTL_AIF1 15
+#define SUN8I_MOD_RST_CTL_ADC 3
#define SUN8I_MOD_RST_CTL_DAC 2
#define SUN8I_SYS_SR_CTRL 0x018
#define SUN8I_SYS_SR_CTRL_AIF1_FS 12
@@ -54,9 +56,25 @@
#define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ 4
#define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ_16 (1 << 4)
#define SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT 2
+#define SUN8I_AIF1_ADCDAT_CTRL 0x044
+#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA 15
+#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA 14
#define SUN8I_AIF1_DACDAT_CTRL 0x048
#define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0L_ENA 15
#define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA 14
+#define SUN8I_AIF1_MXR_SRC 0x04c
+#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF1DA0L 15
+#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACL 14
+#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_ADCL 13
+#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACR 12
+#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF1DA0R 11
+#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACR 10
+#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_ADCR 9
+#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACL 8
+#define SUN8I_ADC_DIG_CTRL 0x100
+#define SUN8I_ADC_DIG_CTRL_ENDA 15
+#define SUN8I_ADC_DIG_CTRL_ADOUT_DTS 2
+#define SUN8I_ADC_DIG_CTRL_ADOUT_DLY 1
#define SUN8I_DAC_DIG_CTRL 0x120
#define SUN8I_DAC_DIG_CTRL_ENDA 15
#define SUN8I_DAC_MXR_SRC 0x130
@@ -338,10 +356,30 @@ static const struct snd_kcontrol_new sun8i_dac_mixer_controls[] = {
SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_ADCR, 1, 0),
};
+static const struct snd_kcontrol_new sun8i_input_mixer_controls[] = {
+ SOC_DAPM_DOUBLE("AIF1 Slot 0 Digital ADC Capture Switch",
+ SUN8I_AIF1_MXR_SRC,
+ SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF1DA0L,
+ SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF1DA0R, 1, 0),
+ SOC_DAPM_DOUBLE("AIF2 Digital ADC Capture Switch", SUN8I_AIF1_MXR_SRC,
+ SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACL,
+ SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACR, 1, 0),
+ SOC_DAPM_DOUBLE("AIF1 Data Digital ADC Capture Switch",
+ SUN8I_AIF1_MXR_SRC,
+ SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_ADCL,
+ SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_ADCR, 1, 0),
+ SOC_DAPM_DOUBLE("AIF2 Inv Digital ADC Capture Switch",
+ SUN8I_AIF1_MXR_SRC,
+ SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACR,
+ SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACL, 1, 0),
+};
+
static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = {
- /* Digital parts of the DACs */
+ /* Digital parts of the DACs and ADC */
SND_SOC_DAPM_SUPPLY("DAC", SUN8I_DAC_DIG_CTRL, SUN8I_DAC_DIG_CTRL_ENDA,
0, NULL, 0),
+ SND_SOC_DAPM_SUPPLY("ADC", SUN8I_ADC_DIG_CTRL, SUN8I_ADC_DIG_CTRL_ENDA,
+ 0, NULL, 0),
/* Analog DAC AIF */
SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left", "Playback", 0,
@@ -351,17 +389,31 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = {
SUN8I_AIF1_DACDAT_CTRL,
SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA, 0),
- /* DAC Mixers */
+ /* Analog ADC AIF */
+ SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left ADC", "Capture", 0,
+ SUN8I_AIF1_ADCDAT_CTRL,
+ SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA, 0),
+ SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Right ADC", "Capture", 0,
+ SUN8I_AIF1_ADCDAT_CTRL,
+ SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA, 0),
+
+ /* DAC and ADC Mixers */
SOC_MIXER_ARRAY("Left Digital DAC Mixer", SND_SOC_NOPM, 0, 0,
sun8i_dac_mixer_controls),
SOC_MIXER_ARRAY("Right Digital DAC Mixer", SND_SOC_NOPM, 0, 0,
sun8i_dac_mixer_controls),
+ SOC_MIXER_ARRAY("Left Digital ADC Mixer", SND_SOC_NOPM, 0, 0,
+ sun8i_input_mixer_controls),
+ SOC_MIXER_ARRAY("Right Digital ADC Mixer", SND_SOC_NOPM, 0, 0,
+ sun8i_input_mixer_controls),
/* Clocks */
SND_SOC_DAPM_SUPPLY("MODCLK AFI1", SUN8I_MOD_CLK_ENA,
SUN8I_MOD_CLK_ENA_AIF1, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("MODCLK DAC", SUN8I_MOD_CLK_ENA,
SUN8I_MOD_CLK_ENA_DAC, 0, NULL, 0),
+ SND_SOC_DAPM_SUPPLY("MODCLK ADC", SUN8I_MOD_CLK_ENA,
+ SUN8I_MOD_CLK_ENA_ADC, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("AIF1", SUN8I_SYSCLK_CTL,
SUN8I_SYSCLK_CTL_AIF1CLK_ENA, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("SYSCLK", SUN8I_SYSCLK_CTL,
@@ -378,6 +430,12 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = {
SUN8I_MOD_RST_CTL_AIF1, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("RST DAC", SUN8I_MOD_RST_CTL,
SUN8I_MOD_RST_CTL_DAC, 0, NULL, 0),
+ SND_SOC_DAPM_SUPPLY("RST ADC", SUN8I_MOD_RST_CTL,
+ SUN8I_MOD_RST_CTL_ADC, 0, NULL, 0),
+
+ SND_SOC_DAPM_MIC("Headset Mic", NULL),
+ SND_SOC_DAPM_MIC("Mic", NULL),
+
};
static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = {
@@ -387,11 +445,16 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = {
{ "RST AIF1", NULL, "AIF1 PLL" },
{ "MODCLK AFI1", NULL, "RST AIF1" },
{ "DAC", NULL, "MODCLK AFI1" },
+ { "ADC", NULL, "MODCLK AFI1" },
{ "RST DAC", NULL, "SYSCLK" },
{ "MODCLK DAC", NULL, "RST DAC" },
{ "DAC", NULL, "MODCLK DAC" },
+ { "RST ADC", NULL, "SYSCLK" },
+ { "MODCLK ADC", NULL, "RST ADC" },
+ { "ADC", NULL, "MODCLK ADC" },
+
/* DAC Routes */
{ "AIF1 Slot 0 Right", NULL, "DAC" },
{ "AIF1 Slot 0 Left", NULL, "DAC" },
@@ -401,6 +464,12 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = {
"AIF1 Slot 0 Left"},
{ "Right Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch",
"AIF1 Slot 0 Right"},
+
+ /* ADC routes */
+ { "Left Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch",
+ "AIF1 Slot 0 Left ADC" },
+ { "Right Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch",
+ "AIF1 Slot 0 Right ADC" },
};
static const struct snd_soc_dai_ops sun8i_codec_dai_ops = {
@@ -418,6 +487,15 @@ static struct snd_soc_dai_driver sun8i_codec_dai = {
.rates = SNDRV_PCM_RATE_8000_192000,
.formats = SNDRV_PCM_FMTBIT_S16_LE,
},
+ /* capture capabilities */
+ .capture = {
+ .stream_name = "Capture",
+ .channels_min = 1,
+ .channels_max = 2,
+ .rates = SNDRV_PCM_RATE_8000_192000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE,
+ .sig_bits = 24,
+ },
/* pcm operations */
.ops = &sun8i_codec_dai_ops,
};
--
2.11.0
^ permalink raw reply related
* [PATCH 0/2] sunxi: Add Capture support for sun8i-a33
From: Mylène Josserand @ 2017-12-13 12:34 UTC (permalink / raw)
To: linux-arm-kernel
Hello everyone,
Here is a first series to add support for the capture for
Allwinner Sun8I-A33 SoC (sun8i-codec).
These patches have been tested on Sun8i-r16 parrot board and
the two microphones are working well.
Noticed that the DAPM route is not correct: "Right/Left Digital
ADC Mixer" widgets should be after the "Right ADC" (according to what I
understood from the datasheet). This is currently not the case but when
I tried to update it, I got an error about "failing to add routes".
I will investigate why in next weeks (and send a V2) but as it
can take time, I think it is better to send a V1 and got reviews
(because it is the first time I am implementing such features).
Thank you in advance,
Best regards,
Myl?ne
Myl?ne Josserand (2):
ASoC: sun8i-codec: Add ADC support for a33
ARM: dts: sun8i: Add ADC routing
arch/arm/boot/dts/sun8i-a33.dtsi | 10 ++++-
sound/soc/sunxi/sun8i-codec.c | 82 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 89 insertions(+), 3 deletions(-)
--
2.11.0
^ permalink raw reply
* arm64 crashkernel fails to boot on acpi-only machines due to ACPI regions being no longer mapped as NOMAP
From: Ard Biesheuvel @ 2017-12-13 12:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213121605.GE28046@linaro.org>
On 13 December 2017 at 12:16, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
> On Wed, Dec 13, 2017 at 10:49:27AM +0000, Ard Biesheuvel wrote:
>> On 13 December 2017 at 10:26, AKASHI Takahiro
>> <takahiro.akashi@linaro.org> wrote:
>> > Bhupesh, Ard,
>> >
>> > On Wed, Dec 13, 2017 at 03:21:59AM +0530, Bhupesh Sharma wrote:
>> >> Hi Ard, Akashi
>> >>
>> > (snip)
>> >
>> >> Looking deeper into the issue, since the arm64 kexec-tools uses the
>> >> 'linux,usable-memory-range' dt property to allow crash dump kernel to
>> >> identify its own usable memory and exclude, at its boot time, any
>> >> other memory areas that are part of the panicked kernel's memory.
>> >> (see https://www.kernel.org/doc/Documentation/devicetree/bindings/chosen.txt
>> >> , for details)
>> >
>> > Right.
>> >
>> >> 1). Now when 'kexec -p' is executed, this node is patched up only
>> >> with the crashkernel memory range:
>> >>
>> >> /* add linux,usable-memory-range */
>> >> nodeoffset = fdt_path_offset(new_buf, "/chosen");
>> >> result = fdt_setprop_range(new_buf, nodeoffset,
>> >> PROP_USABLE_MEM_RANGE, &crash_reserved_mem,
>> >> address_cells, size_cells);
>> >>
>> >> (see https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/arch/arm64/kexec-arm64.c#n465
>> >> , for details)
>> >>
>> >> 2). This excludes the ACPI reclaim regions irrespective of whether
>> >> they are marked as System RAM or as RESERVED. As,
>> >> 'linux,usable-memory-range' dt node is patched up only with
>> >> 'crash_reserved_mem' and not 'system_memory_ranges'
>> >>
>> >> 3). As a result when the crashkernel boots up it doesn't find this
>> >> ACPI memory and crashes while trying to access the same:
>> >>
>> >> # kexec -p /boot/vmlinuz-`uname -r` --initrd=/boot/initramfs-`uname
>> >> -r`.img --reuse-cmdline -d
>> >>
>> >> [snip..]
>> >>
>> >> Reserved memory range
>> >> 000000000e800000-000000002e7fffff (0)
>> >>
>> >> Coredump memory ranges
>> >> 0000000000000000-000000000e7fffff (0)
>> >> 000000002e800000-000000003961ffff (0)
>> >> 0000000039d40000-000000003ed2ffff (0)
>> >> 000000003ed60000-000000003fbfffff (0)
>> >> 0000001040000000-0000001ffbffffff (0)
>> >> 0000002000000000-0000002ffbffffff (0)
>> >> 0000009000000000-0000009ffbffffff (0)
>> >> 000000a000000000-000000affbffffff (0)
>> >>
>> >> 4). So if we revert Ard's patch or just comment the fixing up of the
>> >> memory cap'ing passed to the crash kernel inside
>> >> 'arch/arm64/mm/init.c' (see below):
>> >>
>> >> static void __init fdt_enforce_memory_region(void)
>> >> {
>> >> struct memblock_region reg = {
>> >> .size = 0,
>> >> };
>> >>
>> >> of_scan_flat_dt(early_init_dt_scan_usablemem, ®);
>> >>
>> >> if (reg.size)
>> >> //memblock_cap_memory_range(reg.base, reg.size); /*
>> >> comment this out */
>> >> }
>> >
>> > Please just don't do that. It can cause a fatal damage on
>> > memory contents of the *crashed* kernel.
>> >
>> >> 5). Both the above temporary solutions fix the problem.
>> >>
>> >> 6). However exposing all System RAM regions to the crashkernel is not
>> >> advisable and may cause the crashkernel or some crashkernel drivers to
>> >> fail.
>> >>
>> >> 6a). I am trying an approach now, where the ACPI reclaim regions are
>> >> added to '/proc/iomem' separately as ACPI reclaim regions by the
>> >> kernel code and on the other hand the user-space 'kexec-tools' will
>> >> pick up the ACPI reclaim regions from '/proc/iomem' and add it to the
>> >> dt node 'linux,usable-memory-range'
>> >
>> > I still don't understand why we need to carry over the information
>> > about "ACPI Reclaim memory" to crash dump kernel. In my understandings,
>> > such regions are free to be reused by the kernel after some point of
>> > initialization. Why does crash dump kernel need to know about them?
>> >
>>
>> Not really. According to the UEFI spec, they can be reclaimed after
>> the OS has initialized, i.e., when it has consumed the ACPI tables and
>> no longer needs them. Of course, in order to be able to boot a kexec
>> kernel, those regions needs to be preserved, which is why they are
>> memblock_reserve()'d now.
>
> For my better understandings, who is actually accessing such regions
> during boot time, uefi itself or efistub?
>
No, only the kernel. This is where the ACPI tables are stored. For
instance, on QEMU we have
ACPI: RSDP 0x0000000078980000 000024 (v02 BOCHS )
ACPI: XSDT 0x0000000078970000 000054 (v01 BOCHS BXPCFACP 00000001
01000013)
ACPI: FACP 0x0000000078930000 00010C (v05 BOCHS BXPCFACP 00000001
BXPC 00000001)
ACPI: DSDT 0x0000000078940000 0011DA (v02 BOCHS BXPCDSDT 00000001
BXPC 00000001)
ACPI: APIC 0x0000000078920000 000140 (v03 BOCHS BXPCAPIC 00000001
BXPC 00000001)
ACPI: GTDT 0x0000000078910000 000060 (v02 BOCHS BXPCGTDT 00000001
BXPC 00000001)
ACPI: MCFG 0x0000000078900000 00003C (v01 BOCHS BXPCMCFG 00000001
BXPC 00000001)
ACPI: SPCR 0x00000000788F0000 000050 (v02 BOCHS BXPCSPCR 00000001
BXPC 00000001)
ACPI: IORT 0x00000000788E0000 00007C (v00 BOCHS BXPCIORT 00000001
BXPC 00000001)
covered by
efi: 0x0000788e0000-0x00007894ffff [ACPI Reclaim Memory ...]
...
efi: 0x000078970000-0x00007898ffff [ACPI Reclaim Memory ...]
>> So it seems that kexec does not honour the memblock_reserve() table
>> when booting the next kernel.
>
> not really.
>
>> > (In other words, can or should we skip some part of ACPI-related init code
>> > on crash dump kernel?)
>> >
>>
>> I don't think so. And the change to the handling of ACPI reclaim
>> regions only revealed the bug, not created it (given that other
>> memblock_reserve regions may be affected as well)
>
> As whether we should honor such reserved regions over kexec'ing
> depends on each one's specific nature, we will have to take care one-by-one.
> As a matter of fact, no information about "reserved" memblocks is
> exposed to user space (via proc/iomem).
>
That is why I suggested (somewhere in this thread?) to not expose them
as 'System RAM'. Do you think that could solve this?
>
>>
>> >> 6b). The kernel code currently looks like the following:
>> >>
>> >> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
>> >> index 30ad2f085d1f..867bdec7c692 100644
>> >> --- a/arch/arm64/kernel/setup.c
>> >> +++ b/arch/arm64/kernel/setup.c
>> >> @@ -206,6 +206,7 @@ static void __init request_standard_resources(void)
>> >> {
>> >> struct memblock_region *region;
>> >> struct resource *res;
>> >> + phys_addr_t addr_start, addr_end;
>> >>
>> >> kernel_code.start = __pa_symbol(_text);
>> >> kernel_code.end = __pa_symbol(__init_begin - 1);
>> >> @@ -218,9 +219,17 @@ static void __init request_standard_resources(void)
>> >> res->name = "reserved";
>> >> res->flags = IORESOURCE_MEM;
>> >> } else {
>> >> - res->name = "System RAM";
>> >> - res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
>> >> + addr_start =
>> >> __pfn_to_phys(memblock_region_reserved_base_pfn(region));
>> >> + addr_end =
>> >> __pfn_to_phys(memblock_region_reserved_end_pfn(region)) - 1;
>> >> + if ((efi_mem_type(addr_start) == EFI_ACPI_RECLAIM_MEMORY)
>> >> || (efi_mem_type(addr_end) == EFI_ACPI_RECLAIM_MEMORY)) {
>> >> + res->name = "ACPI reclaim region";
>> >> + res->flags = IORESOURCE_MEM;
>> >> + } else {
>> >> + res->name = "System RAM";
>> >> + res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
>> >> + }
>> >> }
>> >> +
>> >> res->start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
>> >> res->end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 1;
>> >>
>> >> @@ -292,6 +301,7 @@ void __init setup_arch(char **cmdline_p)
>> >>
>> >> request_standard_resources();
>> >>
>> >> + efi_memmap_unmap();
>> >> early_ioremap_reset();
>> >>
>> >> if (acpi_disabled)
>> >> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
>> >> index 80d1a885def5..a7c522eac640 100644
>> >> --- a/drivers/firmware/efi/arm-init.c
>> >> +++ b/drivers/firmware/efi/arm-init.c
>> >> @@ -259,7 +259,6 @@ void __init efi_init(void)
>> >>
>> >> reserve_regions();
>> >> efi_esrt_init();
>> >> - efi_memmap_unmap();
>> >>
>> >> memblock_reserve(params.mmap & PAGE_MASK,
>> >> PAGE_ALIGN(params.mmap_size +
>> >>
>> >>
>> >> After this change the ACPI reclaim regions are properly recognized in
>> >> '/proc/iomem':
>> >>
>> >> # cat /proc/iomem | grep -i ACPI
>> >> 396c0000-3975ffff : ACPI reclaim region
>> >> 39770000-397affff : ACPI reclaim region
>> >> 398a0000-398bffff : ACPI reclaim region
>> >>
>> >> 6c). I am currently changing the 'kexec-tools' and will finish the
>> >> testing over the next few days.
>> >>
>> >> I just wanted to know your opinion on this issue, so that I will be
>> >> able to propose a fix on the above lines.
>> >>
>> >> Also Cc'ing kexec mailing list for more inputs on changes proposed to
>> >> kexec-tools.
>> >>
>> >> Thanks,
>> >> Bhupesh
^ permalink raw reply
* arm64 crashkernel fails to boot on acpi-only machines due to ACPI regions being no longer mapped as NOMAP
From: AKASHI Takahiro @ 2017-12-13 12:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu_BmFN9Zg861SCS+R=V4khFykjuOzkmfEknsL=NvWW3Eg@mail.gmail.com>
On Wed, Dec 13, 2017 at 10:49:27AM +0000, Ard Biesheuvel wrote:
> On 13 December 2017 at 10:26, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> > Bhupesh, Ard,
> >
> > On Wed, Dec 13, 2017 at 03:21:59AM +0530, Bhupesh Sharma wrote:
> >> Hi Ard, Akashi
> >>
> > (snip)
> >
> >> Looking deeper into the issue, since the arm64 kexec-tools uses the
> >> 'linux,usable-memory-range' dt property to allow crash dump kernel to
> >> identify its own usable memory and exclude, at its boot time, any
> >> other memory areas that are part of the panicked kernel's memory.
> >> (see https://www.kernel.org/doc/Documentation/devicetree/bindings/chosen.txt
> >> , for details)
> >
> > Right.
> >
> >> 1). Now when 'kexec -p' is executed, this node is patched up only
> >> with the crashkernel memory range:
> >>
> >> /* add linux,usable-memory-range */
> >> nodeoffset = fdt_path_offset(new_buf, "/chosen");
> >> result = fdt_setprop_range(new_buf, nodeoffset,
> >> PROP_USABLE_MEM_RANGE, &crash_reserved_mem,
> >> address_cells, size_cells);
> >>
> >> (see https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/arch/arm64/kexec-arm64.c#n465
> >> , for details)
> >>
> >> 2). This excludes the ACPI reclaim regions irrespective of whether
> >> they are marked as System RAM or as RESERVED. As,
> >> 'linux,usable-memory-range' dt node is patched up only with
> >> 'crash_reserved_mem' and not 'system_memory_ranges'
> >>
> >> 3). As a result when the crashkernel boots up it doesn't find this
> >> ACPI memory and crashes while trying to access the same:
> >>
> >> # kexec -p /boot/vmlinuz-`uname -r` --initrd=/boot/initramfs-`uname
> >> -r`.img --reuse-cmdline -d
> >>
> >> [snip..]
> >>
> >> Reserved memory range
> >> 000000000e800000-000000002e7fffff (0)
> >>
> >> Coredump memory ranges
> >> 0000000000000000-000000000e7fffff (0)
> >> 000000002e800000-000000003961ffff (0)
> >> 0000000039d40000-000000003ed2ffff (0)
> >> 000000003ed60000-000000003fbfffff (0)
> >> 0000001040000000-0000001ffbffffff (0)
> >> 0000002000000000-0000002ffbffffff (0)
> >> 0000009000000000-0000009ffbffffff (0)
> >> 000000a000000000-000000affbffffff (0)
> >>
> >> 4). So if we revert Ard's patch or just comment the fixing up of the
> >> memory cap'ing passed to the crash kernel inside
> >> 'arch/arm64/mm/init.c' (see below):
> >>
> >> static void __init fdt_enforce_memory_region(void)
> >> {
> >> struct memblock_region reg = {
> >> .size = 0,
> >> };
> >>
> >> of_scan_flat_dt(early_init_dt_scan_usablemem, ®);
> >>
> >> if (reg.size)
> >> //memblock_cap_memory_range(reg.base, reg.size); /*
> >> comment this out */
> >> }
> >
> > Please just don't do that. It can cause a fatal damage on
> > memory contents of the *crashed* kernel.
> >
> >> 5). Both the above temporary solutions fix the problem.
> >>
> >> 6). However exposing all System RAM regions to the crashkernel is not
> >> advisable and may cause the crashkernel or some crashkernel drivers to
> >> fail.
> >>
> >> 6a). I am trying an approach now, where the ACPI reclaim regions are
> >> added to '/proc/iomem' separately as ACPI reclaim regions by the
> >> kernel code and on the other hand the user-space 'kexec-tools' will
> >> pick up the ACPI reclaim regions from '/proc/iomem' and add it to the
> >> dt node 'linux,usable-memory-range'
> >
> > I still don't understand why we need to carry over the information
> > about "ACPI Reclaim memory" to crash dump kernel. In my understandings,
> > such regions are free to be reused by the kernel after some point of
> > initialization. Why does crash dump kernel need to know about them?
> >
>
> Not really. According to the UEFI spec, they can be reclaimed after
> the OS has initialized, i.e., when it has consumed the ACPI tables and
> no longer needs them. Of course, in order to be able to boot a kexec
> kernel, those regions needs to be preserved, which is why they are
> memblock_reserve()'d now.
For my better understandings, who is actually accessing such regions
during boot time, uefi itself or efistub?
> So it seems that kexec does not honour the memblock_reserve() table
> when booting the next kernel.
not really.
> > (In other words, can or should we skip some part of ACPI-related init code
> > on crash dump kernel?)
> >
>
> I don't think so. And the change to the handling of ACPI reclaim
> regions only revealed the bug, not created it (given that other
> memblock_reserve regions may be affected as well)
As whether we should honor such reserved regions over kexec'ing
depends on each one's specific nature, we will have to take care one-by-one.
As a matter of fact, no information about "reserved" memblocks is
exposed to user space (via proc/iomem).
-Takahiro AKASHI
>
> >> 6b). The kernel code currently looks like the following:
> >>
> >> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> >> index 30ad2f085d1f..867bdec7c692 100644
> >> --- a/arch/arm64/kernel/setup.c
> >> +++ b/arch/arm64/kernel/setup.c
> >> @@ -206,6 +206,7 @@ static void __init request_standard_resources(void)
> >> {
> >> struct memblock_region *region;
> >> struct resource *res;
> >> + phys_addr_t addr_start, addr_end;
> >>
> >> kernel_code.start = __pa_symbol(_text);
> >> kernel_code.end = __pa_symbol(__init_begin - 1);
> >> @@ -218,9 +219,17 @@ static void __init request_standard_resources(void)
> >> res->name = "reserved";
> >> res->flags = IORESOURCE_MEM;
> >> } else {
> >> - res->name = "System RAM";
> >> - res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> >> + addr_start =
> >> __pfn_to_phys(memblock_region_reserved_base_pfn(region));
> >> + addr_end =
> >> __pfn_to_phys(memblock_region_reserved_end_pfn(region)) - 1;
> >> + if ((efi_mem_type(addr_start) == EFI_ACPI_RECLAIM_MEMORY)
> >> || (efi_mem_type(addr_end) == EFI_ACPI_RECLAIM_MEMORY)) {
> >> + res->name = "ACPI reclaim region";
> >> + res->flags = IORESOURCE_MEM;
> >> + } else {
> >> + res->name = "System RAM";
> >> + res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> >> + }
> >> }
> >> +
> >> res->start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
> >> res->end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 1;
> >>
> >> @@ -292,6 +301,7 @@ void __init setup_arch(char **cmdline_p)
> >>
> >> request_standard_resources();
> >>
> >> + efi_memmap_unmap();
> >> early_ioremap_reset();
> >>
> >> if (acpi_disabled)
> >> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> >> index 80d1a885def5..a7c522eac640 100644
> >> --- a/drivers/firmware/efi/arm-init.c
> >> +++ b/drivers/firmware/efi/arm-init.c
> >> @@ -259,7 +259,6 @@ void __init efi_init(void)
> >>
> >> reserve_regions();
> >> efi_esrt_init();
> >> - efi_memmap_unmap();
> >>
> >> memblock_reserve(params.mmap & PAGE_MASK,
> >> PAGE_ALIGN(params.mmap_size +
> >>
> >>
> >> After this change the ACPI reclaim regions are properly recognized in
> >> '/proc/iomem':
> >>
> >> # cat /proc/iomem | grep -i ACPI
> >> 396c0000-3975ffff : ACPI reclaim region
> >> 39770000-397affff : ACPI reclaim region
> >> 398a0000-398bffff : ACPI reclaim region
> >>
> >> 6c). I am currently changing the 'kexec-tools' and will finish the
> >> testing over the next few days.
> >>
> >> I just wanted to know your opinion on this issue, so that I will be
> >> able to propose a fix on the above lines.
> >>
> >> Also Cc'ing kexec mailing list for more inputs on changes proposed to
> >> kexec-tools.
> >>
> >> Thanks,
> >> Bhupesh
^ permalink raw reply
* [PATCH] clk: imx51: uart4, uart5 gates only exist on imx50, imx53
From: Fabio Estevam @ 2017-12-13 12:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213115748.14106-1-p.zabel@pengutronix.de>
Hi Philipp,
On Wed, Dec 13, 2017 at 9:57 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> i.MX51 only has 3 UARTs and no CCGR7 register. In place of the CCGR7
> register on i.MX50/i.MX53 that contains the ipg and per clock gates
> for UARTs 4 and 5, on i.MX51 there is the CMEOR register.
>
> Without this patch, the code disabling the UART clocks would also clear
> the mod_en_ov_vpu bit in the CMEOR register, among others, which causes
> register accesses to the VPU to lock up the system.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
^ permalink raw reply
* [PATCH] arm64: fix CONFIG_DEBUG_WX address reporting (was: Re: How to debug "insecure W+X mapping"?)
From: Mark Rutland @ 2017-12-13 11:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <680ec27a-1557-f2d9-8159-bd49326bd36c@redhat.com>
On Tue, Dec 12, 2017 at 03:30:00PM -0800, Laura Abbott wrote:
> On 12/12/2017 02:57 PM, Timur Tabi wrote:
> > We have a 4.10-based kernel that occasionally displays an insecure W+X mapping (courtesy of CONFIG_DEBUG_WX):
> >
> > [??? 7.151680] arm64/mm: Found insecure W+X mapping at address 0000345a049d2000/0x345a049d2000
> > ...
> > [??? 7.435481] Checked W+X mappings: FAILED, 4 W+X pages found, 0 non-UXN pages found
> >
> > The number of actual W+X pages varies, e.g. sometimes it says 6 pages.
> >
> > How do I go about debugging this? How do I identify the source of 0000345a049d2000?
>
> That's a funny address. The check was written to scan the init_mm
> page table but that's not a kernel address on arm64. It almost looks
> like something set up a userspace mapping very early in the boot process?
Whoops; I think we forgot to apply the VA_START offset in
ptdump_check_wx(), so we report the address wrong.
Does the below (untested) patch result in a sane-looking report?
Thanks,
Mark.
---->8----
>From b3021b76b9ea1e65388b38dfe622ea956cb18647 Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Wed, 13 Dec 2017 11:45:42 +0000
Subject: [PATCH] arm64: fix CONFIG_DEBUG_WX address reporting
In ptdump_check_wx(), we pass walk_pgd() a start address of 0 (rather
than VA_START) for the init_mm. This means that any reported W&X
addresses are offset by VA_START, which is unexepcted and confusing.
Let's fix this by telling the ptdump code that we're walking init_mm
starting at VA_START. We don't need to update the addr_markers, since
these are still valid bounds regardless.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Laura Abbott <labbott@redhat.com>
Reported-by: Timur Tabi <timur@codeaurora.org>
---
arch/arm64/mm/dump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
index ca74a2aace42..7b60d62ac593 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -389,7 +389,7 @@ void ptdump_check_wx(void)
.check_wx = true,
};
- walk_pgd(&st, &init_mm, 0);
+ walk_pgd(&st, &init_mm, VA_START);
note_page(&st, 0, 0, 0);
if (st.wx_pages || st.uxn_pages)
pr_warn("Checked W+X mappings: FAILED, %lu W+X pages found, %lu non-UXN pages found\n",
--
2.11.0
^ permalink raw reply related
* [PATCH v11 3/3] arm64:dts:hisilicon Disable hisilicon smmu node on hip06/hip07
From: Shameer Kolothum @ 2017-12-13 11:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213115830.61872-1-shameerali.kolothum.thodi@huawei.com>
The HiSilicon erratum 161010801 describes the limitation of
HiSilicon platforms hip06/hip07 to support the SMMUv3 mappings
for MSI transactions.
PCIe controller on these platforms has to differentiate the
MSI payload against other DMA payload and has to modify the
MSI payload. This makes it difficult for these platforms to
have SMMU translation for MSI. In order to workaround this,
ARM SMMUv3 driver requires a quirk to treat the MSI regions
separately. Such a quirk is currently missing for DT based
systems and therefore we need to explicitly disable the
hip06/hip07 smmu entries in dts.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Signed-off-by: Wei Xu <xuwei5@hisilicon.com>
---
arch/arm64/boot/dts/hisilicon/hip06.dtsi | 55 ++++++++++++++++++++++++++++++++
arch/arm64/boot/dts/hisilicon/hip07.dtsi | 24 ++++++++++++++
2 files changed, 79 insertions(+)
diff --git a/arch/arm64/boot/dts/hisilicon/hip06.dtsi b/arch/arm64/boot/dts/hisilicon/hip06.dtsi
index a049b64..d0d5933 100644
--- a/arch/arm64/boot/dts/hisilicon/hip06.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hip06.dtsi
@@ -291,6 +291,13 @@
#interrupt-cells = <2>;
num-pins = <128>;
};
+
+ mbigen_pcie0: intc_pcie0 {
+ msi-parent = <&its_dsa 0x40085>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ num-pins = <10>;
+ };
};
mbigen_dsa at c0080000 {
@@ -312,6 +319,30 @@
};
};
+ /** HiSilicon erratum 161010801: This describes the limitation
+ * of HiSilicon platforms hip06/hip07 to support the SMMUv3
+ * mappings for PCIe MSI transactions.
+ * PCIe controller on these platforms has to differentiate the
+ * MSI payload against other DMA payload and has to modify the
+ * MSI payload. This makes it difficult for these platforms to
+ * have a SMMU translation for MSI. In order to workaround this,
+ * ARM SMMUv3 driver requires a quirk to treat the MSI regions
+ * separately. Such a quirk is currently missing for DT based
+ * systems. Hence please make sure that the smmu pcie node on
+ * hip06 is disabled as this will break the PCIe functionality
+ * when iommu-map entry is used along with the PCIe node.
+ * Refer:https://www.spinics.net/lists/arm-kernel/msg602812.html
+ */
+ smmu0: smmu_pcie {
+ compatible = "arm,smmu-v3";
+ reg = <0x0 0xa0040000 0x0 0x20000>;
+ #iommu-cells = <1>;
+ dma-coherent;
+ smmu-cb-memtype = <0x0 0x1>;
+ hisilicon,broken-prefetch-cmd;
+ status = "disabled";
+ };
+
soc {
compatible = "simple-bus";
#address-cells = <2>;
@@ -676,6 +707,30 @@
<637 1>,<638 1>,<639 1>;
status = "disabled";
};
+
+ pcie0: pcie at a0090000 {
+ compatible = "hisilicon,hip06-pcie-ecam";
+ reg = <0 0xb0000000 0 0x2000000>,
+ <0 0xa0090000 0 0x10000>;
+ bus-range = <0 31>;
+ msi-map = <0x0000 &its_dsa 0x0000 0x2000>;
+ msi-map-mask = <0xffff>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ device_type = "pci";
+ dma-coherent;
+ ranges = <0x02000000 0 0xb2000000 0x0 0xb2000000 0
+ 0x5ff0000 0x01000000 0 0 0 0xb7ff0000
+ 0 0x10000>;
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0xf800 0 0 7>;
+ interrupt-map = <0x0 0 0 1 &mbigen_pcie0 650 4
+ 0x0 0 0 2 &mbigen_pcie0 650 4
+ 0x0 0 0 3 &mbigen_pcie0 650 4
+ 0x0 0 0 4 &mbigen_pcie0 650 4>;
+ status = "disabled";
+ };
+
};
};
diff --git a/arch/arm64/boot/dts/hisilicon/hip07.dtsi b/arch/arm64/boot/dts/hisilicon/hip07.dtsi
index 2c01a21..58fe013 100644
--- a/arch/arm64/boot/dts/hisilicon/hip07.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hip07.dtsi
@@ -1083,6 +1083,30 @@
};
};
+ /** HiSilicon erratum 161010801: This describes the limitation
+ * of HiSilicon platforms hip06/hip07 to support the SMMUv3
+ * mappings for PCIe MSI transactions.
+ * PCIe controller on these platforms has to differentiate the
+ * MSI payload against other DMA payload and has to modify the
+ * MSI payload. This makes it difficult for these platforms to
+ * have a SMMU translation for MSI. In order to workaround this,
+ * ARM SMMUv3 driver requires a quirk to treat the MSI regions
+ * separately. Such a quirk is currently missing for DT based
+ * systems. Hence please make sure that the smmu pcie node on
+ * hip06 is disabled as this will break the PCIe functionality
+ * when iommu-map entry is used along with the PCIe node.
+ * Refer:https://www.spinics.net/lists/arm-kernel/msg602812.html
+ */
+ smmu0: smmu_pcie {
+ compatible = "arm,smmu-v3";
+ reg = <0x0 0xa0040000 0x0 0x20000>;
+ #iommu-cells = <1>;
+ dma-coherent;
+ smmu-cb-memtype = <0x0 0x1>;
+ hisilicon,broken-prefetch-cmd;
+ status = "disabled";
+ };
+
soc {
compatible = "simple-bus";
#address-cells = <2>;
--
1.9.1
^ permalink raw reply related
* [PATCH v11 2/3] iommu/dma: Add HW MSI(GICv3 ITS) address regions reservation
From: Shameer Kolothum @ 2017-12-13 11:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213115830.61872-1-shameerali.kolothum.thodi@huawei.com>
Modified iommu_dma_get_resv_regions() to include GICv3 ITS
region on ACPI based ARM platfiorms which may require HW MSI
reservations.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 25914d3..f05f3cf 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -19,6 +19,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <linux/acpi_iort.h>
#include <linux/device.h>
#include <linux/dma-iommu.h>
#include <linux/gfp.h>
@@ -167,13 +168,18 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
*
* IOMMU drivers can use this to implement their .get_resv_regions callback
* for general non-IOMMU-specific reservations. Currently, this covers host
- * bridge windows for PCI devices.
+ * bridge windows for PCI devices and GICv3 ITS region reservation on ACPI
+ * based ARM platforms that may require HW MSI reservation.
*/
void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
{
struct pci_host_bridge *bridge;
struct resource_entry *window;
+ if (!is_of_node(dev->iommu_fwspec->iommu_fwnode) &&
+ iort_iommu_msi_get_resv_regions(dev, list) < 0)
+ return;
+
if (!dev_is_pci(dev))
return;
--
1.9.1
^ permalink raw reply related
* [PATCH v11 1/3] ACPI/IORT: Add msi address regions reservation helper
From: Shameer Kolothum @ 2017-12-13 11:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213115830.61872-1-shameerali.kolothum.thodi@huawei.com>
On some platforms msi parent address regions have to be excluded from
normal IOVA allocation in that they are detected and decoded in a HW
specific way by system components and so they cannot be considered normal
IOVA address space.
Add a helper function that retrieves ITS address regions - the msi
parent - through IORT device <-> ITS mappings and reserves it so that
these regions will not be translated by IOMMU and will be excluded from
IOVA allocations. The function checks for the smmu model number and
only applies the msi reservation if the platform requires it.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
drivers/acpi/arm64/iort.c | 112 +++++++++++++++++++++++++++++++++++++--
drivers/irqchip/irq-gic-v3-its.c | 3 +-
include/linux/acpi_iort.h | 7 ++-
3 files changed, 117 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 95255ec..3e0ce65 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -39,6 +39,7 @@
struct iort_its_msi_chip {
struct list_head list;
struct fwnode_handle *fw_node;
+ phys_addr_t base_addr;
u32 translation_id;
};
@@ -161,14 +162,16 @@ typedef acpi_status (*iort_find_node_callback)
static DEFINE_SPINLOCK(iort_msi_chip_lock);
/**
- * iort_register_domain_token() - register domain token and related ITS ID
- * to the list from where we can get it back later on.
+ * iort_register_domain_token() - register domain token along with related
+ * ITS ID and base address to the list from where we can get it back later on.
* @trans_id: ITS ID.
+ * @base: ITS base address.
* @fw_node: Domain token.
*
* Returns: 0 on success, -ENOMEM if no memory when allocating list element
*/
-int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node)
+int iort_register_domain_token(int trans_id, phys_addr_t base,
+ struct fwnode_handle *fw_node)
{
struct iort_its_msi_chip *its_msi_chip;
@@ -178,6 +181,7 @@ int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node)
its_msi_chip->fw_node = fw_node;
its_msi_chip->translation_id = trans_id;
+ its_msi_chip->base_addr = base;
spin_lock(&iort_msi_chip_lock);
list_add(&its_msi_chip->list, &iort_msi_chip_list);
@@ -581,6 +585,24 @@ int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
return -ENODEV;
}
+static int __maybe_unused iort_find_its_base(u32 its_id, phys_addr_t *base)
+{
+ struct iort_its_msi_chip *its_msi_chip;
+ int ret = -ENODEV;
+
+ spin_lock(&iort_msi_chip_lock);
+ list_for_each_entry(its_msi_chip, &iort_msi_chip_list, list) {
+ if (its_msi_chip->translation_id == its_id) {
+ *base = its_msi_chip->base_addr;
+ ret = 0;
+ break;
+ }
+ }
+ spin_unlock(&iort_msi_chip_lock);
+
+ return ret;
+}
+
/**
* iort_dev_find_its_id() - Find the ITS identifier for a device
* @dev: The device.
@@ -740,6 +762,25 @@ static int __maybe_unused __get_pci_rid(struct pci_dev *pdev, u16 alias,
return 0;
}
+static __maybe_unused struct acpi_iort_node *iort_get_msi_resv_iommu(
+ struct device *dev)
+{
+ struct acpi_iort_node *iommu;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+
+ iommu = iort_get_iort_node(fwspec->iommu_fwnode);
+
+ if (iommu && (iommu->type == ACPI_IORT_NODE_SMMU_V3)) {
+ struct acpi_iort_smmu_v3 *smmu;
+
+ smmu = (struct acpi_iort_smmu_v3 *)iommu->node_data;
+ if (smmu->model == ACPI_IORT_SMMU_V3_HISILICON_HI161X)
+ return iommu;
+ }
+
+ return NULL;
+}
+
static int arm_smmu_iort_xlate(struct device *dev, u32 streamid,
struct fwnode_handle *fwnode,
const struct iommu_ops *ops)
@@ -782,6 +823,69 @@ static inline int iort_add_device_replay(const struct iommu_ops *ops,
return err;
}
+
+/**
+ * iort_iommu_msi_get_resv_regions - Reserved region driver helper
+ * @dev: Device from iommu_get_resv_regions()
+ * @head: Reserved region list from iommu_get_resv_regions()
+ *
+ * Returns: Number of msi reserved regions on success (0 if platform
+ * doesn't require the reservation or no associated msi regions),
+ * appropriate error value otherwise. The ITS interrupt translation
+ * spaces (ITS_base + SZ_64K, SZ_64K) associated with the device
+ * are the msi reserved regions.
+ */
+int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
+{
+ struct acpi_iort_its_group *its;
+ struct acpi_iort_node *iommu_node, *its_node = NULL;
+ int i, resv = 0;
+
+ iommu_node = iort_get_msi_resv_iommu(dev);
+ if (!iommu_node)
+ return 0;
+
+ /*
+ * Current logic to reserve ITS regions relies on HW topologies
+ * where a given PCI or named component maps its IDs to only one
+ * ITS group; if a PCI or named component can map its IDs to
+ * different ITS groups through IORT mappings this function has
+ * to be reworked to ensure we reserve regions for all ITS groups
+ * a given PCI or named component may map IDs to.
+ */
+
+ for (i = 0; i < dev->iommu_fwspec->num_ids; i++) {
+ its_node = iort_node_map_id(iommu_node,
+ dev->iommu_fwspec->ids[i],
+ NULL, IORT_MSI_TYPE);
+ if (its_node)
+ break;
+ }
+
+ if (!its_node)
+ return 0;
+
+ /* Move to ITS specific data */
+ its = (struct acpi_iort_its_group *)its_node->node_data;
+
+ for (i = 0; i < its->its_count; i++) {
+ phys_addr_t base;
+
+ if (!iort_find_its_base(its->identifiers[i], &base)) {
+ int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+ struct iommu_resv_region *region;
+
+ region = iommu_alloc_resv_region(base + SZ_64K, SZ_64K,
+ prot, IOMMU_RESV_MSI);
+ if (region) {
+ list_add_tail(®ion->list, head);
+ resv++;
+ }
+ }
+ }
+
+ return (resv == its->its_count) ? resv : -ENODEV;
+}
#else
static inline const struct iommu_ops *iort_fwspec_iommu_ops(
struct iommu_fwspec *fwspec)
@@ -789,6 +893,8 @@ static inline const struct iommu_ops *iort_fwspec_iommu_ops(
static inline int iort_add_device_replay(const struct iommu_ops *ops,
struct device *dev)
{ return 0; }
+int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
+{ return 0; }
#endif
static int iort_iommu_xlate(struct device *dev, struct acpi_iort_node *node,
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 4039e64..d4cff12 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3450,7 +3450,8 @@ static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header,
return -ENOMEM;
}
- err = iort_register_domain_token(its_entry->translation_id, dom_handle);
+ err = iort_register_domain_token(its_entry->translation_id, res.start,
+ dom_handle);
if (err) {
pr_err("ITS@%pa: Unable to register GICv3 ITS domain token (ITS ID %d) to IORT\n",
&res.start, its_entry->translation_id);
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 2f7a292..38cd77b 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -26,7 +26,8 @@
#define IORT_IRQ_MASK(irq) (irq & 0xffffffffULL)
#define IORT_IRQ_TRIGGER_MASK(irq) ((irq >> 32) & 0xffffffffULL)
-int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node);
+int iort_register_domain_token(int trans_id, phys_addr_t base,
+ struct fwnode_handle *fw_node);
void iort_deregister_domain_token(int trans_id);
struct fwnode_handle *iort_find_domain_token(int trans_id);
#ifdef CONFIG_ACPI_IORT
@@ -38,6 +39,7 @@
/* IOMMU interface */
void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *size);
const struct iommu_ops *iort_iommu_configure(struct device *dev);
+int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head);
#else
static inline void acpi_iort_init(void) { }
static inline u32 iort_msi_map_rid(struct device *dev, u32 req_id)
@@ -52,6 +54,9 @@ static inline void iort_dma_setup(struct device *dev, u64 *dma_addr,
static inline const struct iommu_ops *iort_iommu_configure(
struct device *dev)
{ return NULL; }
+static inline
+int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
+{ return 0; }
#endif
#endif /* __ACPI_IORT_H__ */
--
1.9.1
^ permalink raw reply related
* [PATCH v11 0/3] iommu/smmu-v3: Workaround for hisilicon 161010801 erratum(reserve HW MSI)
From: Shameer Kolothum @ 2017-12-13 11:58 UTC (permalink / raw)
To: linux-arm-kernel
On certain HiSilicon platforms (hip06/hip07) the GIC ITS and PCIe RC
deviates from the standard implementation and this breaks PCIe MSI
functionality when SMMU is enabled.
The HiSilicon erratum 161010801 describes this limitation of certain
HiSilicon platforms to support the SMMU mappings for MSI transactions.
On these platforms GICv3 ITS translator is presented with the deviceID
by extending the MSI payload data to 64 bits to include the deviceID.
Hence, the PCIe controller on this platforms has to differentiate the MSI
payload against other DMA payload and has to modify the MSI payload.
This basically makes it difficult for this platforms to have a SMMU
translation for MSI.
This patch implements an ACPI based quirk to reserve the hw msi regions
in the smmu-v3 driver which means these address regions will not be
translated and will be excluded from iova allocations.
To implement this quirk, the following changes are incorporated:
1. Added a generic helper function to IORT code to retrieve and reserve
the associated ITS base address from a device IORT node. The function
has a check for smmu model to determine whether the platform requires
the HW MSI reservation or not.
2. Added smmu node entries and explicitly disabled them in hip06/hip07
dts files so that users are warned about the non-DT support for this
erratum.
Changelog:
v10 --> v11
-Addressed comments from Lorenzo(patch#1)
-Added Robin's Reviewed-by to patch #2
v9 --> v10
Addressed comments:
-Moved smmu model check to iort helper function to selectively apply
the msi reservation which will make the fn call generic from iommu-dma.
-Removed PCI blacklisting patch, instead added smmu nodes(disabled)
with comments to hip06/hip07 dts file.
v8 --> v9
-Thanks to Marc, fixed IORT helper function to reserve the ITS
translater region only.
-Removed the DT support for MSI reservation and blacklisted
HiSilicon PCIe controllers on DT based systems when SMMUv3 is
enabled.
v7 --> v8
Addressed comments from Rob and Lorenzo:
-Modified to use DT compatible string for errata.
-Changed logic to retrieve the msi-parent for DT case.
v6 --> v7
Addressed request from Will to add DT support for the erratum:
- added bt binding
- add of_iommu_msi_get_resv_regions()
New arm64 silicon errata entry
Rename iort_iommu_{its->msi}_get_resv_regions
v5 --> v6
Addressed comments from Robin and Lorenzo:
-No change to patch#1 .
-Reverted v5 patch#2 as this might break the platforms where this quirk
is not applicable. Provided a generic function in iommu code and added
back the quirk implementation in SMMU v3 driver(patch#3)
v4 --> v5
Addressed comments from Robin and Lorenzo:
-Added a comment to make it clear that, for now, only straightforward
HW topologies are handled while reserving ITS regions(patch #1).
v3 --> v4
Rebased on 4.13-rc1.
Addressed comments from Robin, Will and Lorenzo:
-As suggested by Robin, moved the ITS msi reservation into
iommu_dma_get_resv_regions().
-Added its_count != resv region failure case(patch #1).
v2 --> v3
Addressed comments from Lorenzo and Robin:
-Removed dev_is_pci() check in smmuV3 driver.
-Don't treat device not having an ITS mapping as an error in
iort helper function.
v1 --> v2
-patch 2/2: Invoke iort helper fn based on fwnode type(acpi).
RFCv2 -->PATCH
-Incorporated Lorenzo's review comments.
RFC v1 --> RFC v2
Based on Robin's review comments,
-Removed the generic erratum framework.
-Using IORT/MADT tables to retrieve the ITS base addr instead
of vendor specific CSRT table.
Shameer Kolothum (3):
ACPI/IORT: Add msi address regions reservation helper
iommu/dma: Add HW MSI(GICv3 ITS) address regions reservation
arm64:dts:hisilicon Disable hisilicon smmu node on hip06/hip07
arch/arm64/boot/dts/hisilicon/hip06.dtsi | 55 +++++++++++++++
arch/arm64/boot/dts/hisilicon/hip07.dtsi | 24 +++++++
drivers/acpi/arm64/iort.c | 112 ++++++++++++++++++++++++++++++-
drivers/iommu/dma-iommu.c | 8 ++-
drivers/irqchip/irq-gic-v3-its.c | 3 +-
include/linux/acpi_iort.h | 7 +-
6 files changed, 203 insertions(+), 6 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH] clk: imx51: uart4, uart5 gates only exist on imx50, imx53
From: Philipp Zabel @ 2017-12-13 11:57 UTC (permalink / raw)
To: linux-arm-kernel
i.MX51 only has 3 UARTs and no CCGR7 register. In place of the CCGR7
register on i.MX50/i.MX53 that contains the ipg and per clock gates
for UARTs 4 and 5, on i.MX51 there is the CMEOR register.
Without this patch, the code disabling the UART clocks would also clear
the mod_en_ov_vpu bit in the CMEOR register, among others, which causes
register accesses to the VPU to lock up the system.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/clk/imx/clk-imx51-imx53.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/imx/clk-imx51-imx53.c b/drivers/clk/imx/clk-imx51-imx53.c
index 7bcaf270db117..c864992e6983f 100644
--- a/drivers/clk/imx/clk-imx51-imx53.c
+++ b/drivers/clk/imx/clk-imx51-imx53.c
@@ -257,10 +257,6 @@ static void __init mx5_clocks_common_init(void __iomem *ccm_base)
clk[IMX5_CLK_VPU_SEL] = imx_clk_mux("vpu_sel", MXC_CCM_CBCMR, 14, 2, vpu_sel, ARRAY_SIZE(vpu_sel));
clk[IMX5_CLK_VPU_GATE] = imx_clk_gate2("vpu_gate", "vpu_sel", MXC_CCM_CCGR5, 6);
clk[IMX5_CLK_VPU_REFERENCE_GATE] = imx_clk_gate2("vpu_reference_gate", "osc", MXC_CCM_CCGR5, 8);
- clk[IMX5_CLK_UART4_IPG_GATE] = imx_clk_gate2("uart4_ipg_gate", "ipg", MXC_CCM_CCGR7, 8);
- clk[IMX5_CLK_UART4_PER_GATE] = imx_clk_gate2("uart4_per_gate", "uart_root", MXC_CCM_CCGR7, 10);
- clk[IMX5_CLK_UART5_IPG_GATE] = imx_clk_gate2("uart5_ipg_gate", "ipg", MXC_CCM_CCGR7, 12);
- clk[IMX5_CLK_UART5_PER_GATE] = imx_clk_gate2("uart5_per_gate", "uart_root", MXC_CCM_CCGR7, 14);
clk[IMX5_CLK_GPC_DVFS] = imx_clk_gate2("gpc_dvfs", "dummy", MXC_CCM_CCGR5, 24);
clk[IMX5_CLK_SSI_APM] = imx_clk_mux("ssi_apm", MXC_CCM_CSCMR1, 8, 2, ssi_apm_sels, ARRAY_SIZE(ssi_apm_sels));
@@ -361,6 +357,10 @@ static void __init mx50_clocks_init(struct device_node *np)
clk[IMX5_CLK_USB_PHY1_GATE] = imx_clk_gate2("usb_phy1_gate", "usb_phy_sel", MXC_CCM_CCGR4, 10);
clk[IMX5_CLK_USB_PHY2_GATE] = imx_clk_gate2("usb_phy2_gate", "usb_phy_sel", MXC_CCM_CCGR4, 12);
clk[IMX5_CLK_I2C3_GATE] = imx_clk_gate2("i2c3_gate", "per_root", MXC_CCM_CCGR1, 22);
+ clk[IMX5_CLK_UART4_IPG_GATE] = imx_clk_gate2("uart4_ipg_gate", "ipg", MXC_CCM_CCGR7, 8);
+ clk[IMX5_CLK_UART4_PER_GATE] = imx_clk_gate2("uart4_per_gate", "uart_root", MXC_CCM_CCGR7, 10);
+ clk[IMX5_CLK_UART5_IPG_GATE] = imx_clk_gate2("uart5_ipg_gate", "ipg", MXC_CCM_CCGR7, 12);
+ clk[IMX5_CLK_UART5_PER_GATE] = imx_clk_gate2("uart5_per_gate", "uart_root", MXC_CCM_CCGR7, 14);
clk[IMX5_CLK_CKO1_SEL] = imx_clk_mux("cko1_sel", MXC_CCM_CCOSR, 0, 4,
mx53_cko1_sel, ARRAY_SIZE(mx53_cko1_sel));
@@ -562,6 +562,10 @@ static void __init mx53_clocks_init(struct device_node *np)
clk[IMX5_CLK_IEEE1588_PRED] = imx_clk_divider("ieee1588_pred", "ieee1588_sel", MXC_CCM_CSCDR2, 6, 3);
clk[IMX5_CLK_IEEE1588_PODF] = imx_clk_divider("ieee1588_podf", "ieee1588_pred", MXC_CCM_CSCDR2, 0, 6);
clk[IMX5_CLK_IEEE1588_GATE] = imx_clk_gate2("ieee1588_serial_gate", "ieee1588_podf", MXC_CCM_CCGR7, 6);
+ clk[IMX5_CLK_UART4_IPG_GATE] = imx_clk_gate2("uart4_ipg_gate", "ipg", MXC_CCM_CCGR7, 8);
+ clk[IMX5_CLK_UART4_PER_GATE] = imx_clk_gate2("uart4_per_gate", "uart_root", MXC_CCM_CCGR7, 10);
+ clk[IMX5_CLK_UART5_IPG_GATE] = imx_clk_gate2("uart5_ipg_gate", "ipg", MXC_CCM_CCGR7, 12);
+ clk[IMX5_CLK_UART5_PER_GATE] = imx_clk_gate2("uart5_per_gate", "uart_root", MXC_CCM_CCGR7, 14);
clk[IMX5_CLK_CKO1_SEL] = imx_clk_mux("cko1_sel", MXC_CCM_CCOSR, 0, 4,
mx53_cko1_sel, ARRAY_SIZE(mx53_cko1_sel));
--
2.11.0
^ permalink raw reply related
* [PATCH v4 08/12] clk: qcom: Add KPSS ACC/GCC driver
From: Sricharan R @ 2017-12-13 11:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171212203813.f3fkjw4uvxm43yok@rob-hp-laptop>
Hi Rob,
On 12/13/2017 2:08 AM, Rob Herring wrote:
> On Fri, Dec 08, 2017 at 03:12:26PM +0530, Sricharan R wrote:
>> From: Stephen Boyd <sboyd@codeaurora.org>
>>
>> The ACC and GCC regions present in KPSSv1 contain registers to
>> control clocks and power to each Krait CPU and L2. For CPUfreq
>> purposes probe these devices and expose a mux clock that chooses
>> between PXO and PLL8.
>>
>> Cc: <devicetree@vger.kernel.org>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>> .../devicetree/bindings/arm/msm/qcom,kpss-acc.txt | 7 ++
>> .../devicetree/bindings/arm/msm/qcom,kpss-gcc.txt | 28 +++++++
>
> Please make bindings a separate patch.
ok.
>
>> drivers/clk/qcom/Kconfig | 8 ++
>> drivers/clk/qcom/Makefile | 1 +
>> drivers/clk/qcom/kpss-xcc.c | 96 ++++++++++++++++++++++
>> 5 files changed, 140 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
>> create mode 100644 drivers/clk/qcom/kpss-xcc.c
>>
>> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
>> index 1333db9..382a574 100644
>> --- a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
>> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
>> @@ -21,10 +21,17 @@ PROPERTIES
>> the register region. An optional second element specifies
>> the base address and size of the alias register region.
>>
>> +- clock-output-names:
>> + Usage: optional
>> + Value type: <string>
>> + Definition: Name of the output clock. Typically acpuX_aux where X is a
>> + CPU number starting at 0.
>> +
>> Example:
>>
>> clock-controller at 2088000 {
>> compatible = "qcom,kpss-acc-v2";
>> reg = <0x02088000 0x1000>,
>> <0x02008000 0x1000>;
>> + clock-output-names = "acpu0_aux";
>> };
>> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
>> new file mode 100644
>> index 0000000..d1e12f1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
>> @@ -0,0 +1,28 @@
>> +Krait Processor Sub-system (KPSS) Global Clock Controller (GCC)
>> +
>> +PROPERTIES
>> +
>> +- compatible:
>> + Usage: required
>> + Value type: <string>
>> + Definition: should be one of:
>> + "qcom,kpss-gcc"
>
> Only one implementation?
hmm, missed "qcom,kpss-acc-v1", will add that too.
Regards,
Sricharan
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
^ permalink raw reply
* [PATCH] IPI performance benchmark
From: Christian Borntraeger @ 2017-12-13 11:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213112355.s3ubggurwx4v3r53@yury-thinkpad>
On 12/13/2017 12:23 PM, Yury Norov wrote:
> On Mon, Dec 11, 2017 at 05:30:25PM +0100, Christian Borntraeger wrote:
>>
>>
>> On 12/11/2017 03:55 PM, Yury Norov wrote:
>>> On Mon, Dec 11, 2017 at 03:35:02PM +0100, Christian Borntraeger wrote:
>>>>
>>>>
>>>> On 12/11/2017 03:16 PM, Yury Norov wrote:
>>>>> This benchmark sends many IPIs in different modes and measures
>>>>> time for IPI delivery (first column), and total time, ie including
>>>>> time to acknowledge the receive by sender (second column).
>>>>>
>>>>> The scenarios are:
>>>>> Dry-run: do everything except actually sending IPI. Useful
>>>>> to estimate system overhead.
>>>>> Self-IPI: Send IPI to self CPU.
>>>>> Normal IPI: Send IPI to some other CPU.
>>>>> Broadcast IPI: Send broadcast IPI to all online CPUs.
>>>>>
>>>>> For virtualized guests, sending and reveiving IPIs causes guest exit.
>>>>> I used this test to measure performance impact on KVM subsystem of
>>>>> Christoffer Dall's series "Optimize KVM/ARM for VHE systems".
>>>>>
>>>>> https://www.spinics.net/lists/kvm/msg156755.html
>>>>>
>>>>> Test machine is ThunderX2, 112 online CPUs. Below the results normalized
>>>>> to host dry-run time. Smaller - better.
>>>>>
>>>>> Host, v4.14:
>>>>> Dry-run: 0 1
>>>>> Self-IPI: 9 18
>>>>> Normal IPI: 81 110
>>>>> Broadcast IPI: 0 2106
>>>>>
>>>>> Guest, v4.14:
>>>>> Dry-run: 0 1
>>>>> Self-IPI: 10 18
>>>>> Normal IPI: 305 525
>>>>> Broadcast IPI: 0 9729
>>>>>
>>>>> Guest, v4.14 + VHE:
>>>>> Dry-run: 0 1
>>>>> Self-IPI: 9 18
>>>>> Normal IPI: 176 343
>>>>> Broadcast IPI: 0 9885
>> [...]
>>>>> +static int __init init_bench_ipi(void)
>>>>> +{
>>>>> + ktime_t ipi, total;
>>>>> + int ret;
>>>>> +
>>>>> + ret = bench_ipi(NTIMES, DRY_RUN, &ipi, &total);
>>>>> + if (ret)
>>>>> + pr_err("Dry-run FAILED: %d\n", ret);
>>>>> + else
>>>>> + pr_err("Dry-run: %18llu, %18llu ns\n", ipi, total);
>>>>
>>>> you do not use NTIMES here to calculate the average value. Is that intended?
>>>
>>> I think, it's more visually to represent all results in number of dry-run
>>> times, like I did in patch description. So on kernel side I expose raw data
>>> and calculate final values after finishing tests.
>>
>> I think it is highly confusing that the output from the patch description does not
>> match the output from the real module. So can you make that match at least?
>
> I think so. That's why I noticed that results are normalized to host dry-run
> time, even more, they are small and better for human perception.
>
> I was recommended not to public raw data, you'd understand. If this is
> the blocker, I can post results from QEMU-hosted kernel.
you could just post some example data from any random x86 laptop. I think it
would just be good to have the patch description output match the real output.
^ permalink raw reply
* [PATCH] IPI performance benchmark
From: Yury Norov @ 2017-12-13 11:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ef696fb0-612c-aee7-319e-eb12f8692ac3@de.ibm.com>
On Mon, Dec 11, 2017 at 05:30:25PM +0100, Christian Borntraeger wrote:
>
>
> On 12/11/2017 03:55 PM, Yury Norov wrote:
> > On Mon, Dec 11, 2017 at 03:35:02PM +0100, Christian Borntraeger wrote:
> >>
> >>
> >> On 12/11/2017 03:16 PM, Yury Norov wrote:
> >>> This benchmark sends many IPIs in different modes and measures
> >>> time for IPI delivery (first column), and total time, ie including
> >>> time to acknowledge the receive by sender (second column).
> >>>
> >>> The scenarios are:
> >>> Dry-run: do everything except actually sending IPI. Useful
> >>> to estimate system overhead.
> >>> Self-IPI: Send IPI to self CPU.
> >>> Normal IPI: Send IPI to some other CPU.
> >>> Broadcast IPI: Send broadcast IPI to all online CPUs.
> >>>
> >>> For virtualized guests, sending and reveiving IPIs causes guest exit.
> >>> I used this test to measure performance impact on KVM subsystem of
> >>> Christoffer Dall's series "Optimize KVM/ARM for VHE systems".
> >>>
> >>> https://www.spinics.net/lists/kvm/msg156755.html
> >>>
> >>> Test machine is ThunderX2, 112 online CPUs. Below the results normalized
> >>> to host dry-run time. Smaller - better.
> >>>
> >>> Host, v4.14:
> >>> Dry-run: 0 1
> >>> Self-IPI: 9 18
> >>> Normal IPI: 81 110
> >>> Broadcast IPI: 0 2106
> >>>
> >>> Guest, v4.14:
> >>> Dry-run: 0 1
> >>> Self-IPI: 10 18
> >>> Normal IPI: 305 525
> >>> Broadcast IPI: 0 9729
> >>>
> >>> Guest, v4.14 + VHE:
> >>> Dry-run: 0 1
> >>> Self-IPI: 9 18
> >>> Normal IPI: 176 343
> >>> Broadcast IPI: 0 9885
> [...]
> >>> +static int __init init_bench_ipi(void)
> >>> +{
> >>> + ktime_t ipi, total;
> >>> + int ret;
> >>> +
> >>> + ret = bench_ipi(NTIMES, DRY_RUN, &ipi, &total);
> >>> + if (ret)
> >>> + pr_err("Dry-run FAILED: %d\n", ret);
> >>> + else
> >>> + pr_err("Dry-run: %18llu, %18llu ns\n", ipi, total);
> >>
> >> you do not use NTIMES here to calculate the average value. Is that intended?
> >
> > I think, it's more visually to represent all results in number of dry-run
> > times, like I did in patch description. So on kernel side I expose raw data
> > and calculate final values after finishing tests.
>
> I think it is highly confusing that the output from the patch description does not
> match the output from the real module. So can you make that match at least?
I think so. That's why I noticed that results are normalized to host dry-run
time, even more, they are small and better for human perception.
I was recommended not to public raw data, you'd understand. If this is
the blocker, I can post results from QEMU-hosted kernel.
> > If you think that average values are preferable, I can do that in v2.
>
> The raw numbers a propably fine, but then you might want to print the number of
> loop iterations in the output.
It's easy to do. But this number is the same for all tests, and what
really interesting is relative numbers, so I decided not to trash output.
If you insist on printing iterations number, just let me know and I'll add it.
> If we want to do something fancy, we could do a combination of a smaller inner
> loop doing the test, then an outer loops redoing the inner loop and then you
> can do some min/max/average calculation. Not s
^ permalink raw reply
* [bpf-next V1-RFC PATCH 09/14] thunderx: setup xdp_rxq_info
From: Jesper Dangaard Brouer @ 2017-12-13 11:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <151316391502.14967.13292358380181773729.stgit@firesoul>
This driver uses a bool scheme for "enable"/"disable" when setting up
different resources. Thus, the hook points for xdp_rxq_info is done
in the same function call nicvf_rcv_queue_config(). This is activated
through enable/disble via nicvf_config_data_transfer(), which is tied
into nicvf_stop()/nicvf_open().
Extending driver packet handler call-path nicvf_rcv_pkt_handler() with
a pointer to the given struct rcv_queue, in-order to access the
xdp_rxq_info data area (in nicvf_xdp_rx()).
Cc: linux-arm-kernel at lists.infradead.org
Cc: Sunil Goutham <sgoutham@cavium.com>
Cc: Robert Richter <rric@kernel.org>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 11 +++++++----
drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 7 +++++++
drivers/net/ethernet/cavium/thunder/nicvf_queues.h | 2 ++
3 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 52b3a6044f85..21618d0d694f 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -521,7 +521,7 @@ static void nicvf_unmap_page(struct nicvf *nic, struct page *page, u64 dma_addr)
static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,
struct cqe_rx_t *cqe_rx, struct snd_queue *sq,
- struct sk_buff **skb)
+ struct rcv_queue *rq, struct sk_buff **skb)
{
struct xdp_buff xdp;
struct page *page;
@@ -545,6 +545,7 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,
xdp.data = (void *)cpu_addr;
xdp_set_data_meta_invalid(&xdp);
xdp.data_end = xdp.data + len;
+ xdp.rxq = &rq->xdp_rxq;
orig_data = xdp.data;
rcu_read_lock();
@@ -698,7 +699,8 @@ static inline void nicvf_set_rxhash(struct net_device *netdev,
static void nicvf_rcv_pkt_handler(struct net_device *netdev,
struct napi_struct *napi,
- struct cqe_rx_t *cqe_rx, struct snd_queue *sq)
+ struct cqe_rx_t *cqe_rx,
+ struct snd_queue *sq, struct rcv_queue *rq)
{
struct sk_buff *skb = NULL;
struct nicvf *nic = netdev_priv(netdev);
@@ -724,7 +726,7 @@ static void nicvf_rcv_pkt_handler(struct net_device *netdev,
/* For XDP, ignore pkts spanning multiple pages */
if (nic->xdp_prog && (cqe_rx->rb_cnt == 1)) {
/* Packet consumed by XDP */
- if (nicvf_xdp_rx(snic, nic->xdp_prog, cqe_rx, sq, &skb))
+ if (nicvf_xdp_rx(snic, nic->xdp_prog, cqe_rx, sq, rq, &skb))
return;
} else {
skb = nicvf_get_rcv_skb(snic, cqe_rx,
@@ -781,6 +783,7 @@ static int nicvf_cq_intr_handler(struct net_device *netdev, u8 cq_idx,
struct cqe_rx_t *cq_desc;
struct netdev_queue *txq;
struct snd_queue *sq = &qs->sq[cq_idx];
+ struct rcv_queue *rq = &qs->rq[cq_idx];
unsigned int tx_pkts = 0, tx_bytes = 0, txq_idx;
spin_lock_bh(&cq->lock);
@@ -811,7 +814,7 @@ static int nicvf_cq_intr_handler(struct net_device *netdev, u8 cq_idx,
switch (cq_desc->cqe_type) {
case CQE_TYPE_RX:
- nicvf_rcv_pkt_handler(netdev, napi, cq_desc, sq);
+ nicvf_rcv_pkt_handler(netdev, napi, cq_desc, sq, rq);
work_done++;
break;
case CQE_TYPE_SEND:
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
index 095c18aeb8d5..2f2a736ea102 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
@@ -760,6 +760,7 @@ static void nicvf_rcv_queue_config(struct nicvf *nic, struct queue_set *qs,
if (!rq->enable) {
nicvf_reclaim_rcv_queue(nic, qs, qidx);
+ xdp_rxq_info_unreg(&rq->xdp_rxq);
return;
}
@@ -772,6 +773,12 @@ static void nicvf_rcv_queue_config(struct nicvf *nic, struct queue_set *qs,
/* all writes of RBDR data to be loaded into L2 Cache as well*/
rq->caching = 1;
+ /* XDP RX-queue info */
+ xdp_rxq_info_init(&rq->xdp_rxq);
+ rq->xdp_rxq.dev = nic->netdev;
+ rq->xdp_rxq.queue_index = qidx;
+ xdp_rxq_info_reg(&rq->xdp_rxq);
+
/* Send a mailbox msg to PF to config RQ */
mbx.rq.msg = NIC_MBOX_MSG_RQ_CFG;
mbx.rq.qs_num = qs->vnic_id;
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
index 178ab6e8e3c5..7d1e4e2aaad0 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
@@ -12,6 +12,7 @@
#include <linux/netdevice.h>
#include <linux/iommu.h>
#include <linux/bpf.h>
+#include <net/xdp.h>
#include "q_struct.h"
#define MAX_QUEUE_SET 128
@@ -255,6 +256,7 @@ struct rcv_queue {
u8 start_qs_rbdr_idx; /* RBDR idx in the above QS */
u8 caching;
struct rx_tx_queue_stats stats;
+ struct xdp_rxq_info xdp_rxq;
} ____cacheline_aligned_in_smp;
struct cmp_queue {
^ permalink raw reply related
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