Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/3] [v8] pinctrl: qcom: disable GPIO groups with no pins
From: Andy Shevchenko @ 2017-12-13 14:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1513111858-6251-3-git-send-email-timur@codeaurora.org>

On Tue, 2017-12-12 at 14:50 -0600, Timur Tabi wrote:
> pinctrl-msm only accepts an array of GPIOs from 0 to n-1, and it
> expects
> each group to support have only one pin (npins == 1).
> 
> We can support "sparse" GPIO maps if we allow for some groups to have
> zero
> pins (npins == 0).  These pins are "hidden" from the rest of the
> driver
> and gpiolib.
> 
> Access to unavailable GPIOs is blocked via a request callback.  If the
> requested GPIO is unavailable, -EACCES is returned, which prevents
> further access to that GPIO.

We recently have some interesting BIOS/Windows driver design which makes
a need of something similar. Mika did patched pinctrl-intel for that. I
dunno that approach can be used here, or your proposal be utilized in
pinctrl-intel. Mika, any comments?

See some nitpicks below.

>  
>  	seq_printf(s, " %-8s: %-3s %d", g->name, is_out ? "out" :
> "in", func);
>  	seq_printf(s, " %dmA", msm_regval_to_drive(drive));
> -	seq_printf(s, " %s", pulls[pull]);
> +	seq_printf(s, " %s\n", pulls[pull]);

I would rather do

 seq_putc(s, '\n');

which makes code slightly more flexible for maintenance and reading.


>  }
>  
>  static void msm_gpio_dbg_show(struct seq_file *s, struct gpio_chip
> *chip)
> @@ -524,23 +529,36 @@ static void msm_gpio_dbg_show(struct seq_file
> *s, struct gpio_chip *chip)
>  	unsigned gpio = chip->base;
>  	unsigned i;
>  
> -	for (i = 0; i < chip->ngpio; i++, gpio++) {
> +	for (i = 0; i < chip->ngpio; i++, gpio++)
>  		msm_gpio_dbg_show_one(s, NULL, chip, i, gpio);
> -		seq_puts(s, "\n");
> -	}

This kind of change looks like a candidate to a separate patch,
though I mentioned it's just a nit.


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* [PATCH V4 08/12] boot_constraint: Manage deferrable constraints
From: Viresh Kumar @ 2017-12-13 14:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171213103315.GI10595@n2100.armlinux.org.uk>

On 13-12-17, 10:33, Russell King - ARM Linux wrote:
> On Wed, Dec 13, 2017 at 03:57:07PM +0530, Viresh Kumar wrote:
> > On 13-12-17, 10:53, Greg Kroah-Hartman wrote:
> > > On Sun, Oct 29, 2017 at 07:18:56PM +0530, Viresh Kumar wrote:
> > > > +static void add_deferrable_of_single(struct device_node *np,
> > > > +				     struct dev_boot_constraint *constraints,
> > > > +				     int count)
> > > > +{
> > > > +	struct device *dev;
> > > > +	int ret;
> > > > +
> > > > +	if (!of_device_is_available(np))
> > > > +		return;
> > > > +
> > > > +	ret = of_platform_bus_create(np, NULL, NULL, NULL, false);
> > > > +	if (ret)
> > > > +		return;
> > > > +
> > > > +	if (of_device_is_compatible(np, "arm,primecell")) {
> > > 
> > > Why is "arm,primecell" in the core code here?
> > 
> > All we need here is a struct device pointer to add constraints. But how we get
> > the device node depends on what bus type the device corresponds to. Currently
> > this only support amba and platform devices, but we may need to get spi, i2c,
> > etc later on.
> > 
> > How do you suggest to keep this stuff out of core here ? Are you asking me to
> > add a generic API in the OF core to find the struct device pointer using a node
> > pointer ?
> 
> Why do we need this?  Why can't we lookup the "struct device" by DT
> node, and then look at the device's bus type and decide what to do
> from that?

My requirement is only to get the struct device * for the DT node and I don't
really need to get into the bus specific details at all. I was not sure if there
is a way to lookup for the "struct device" by its DT node currently and so
depended on helpers like of_find_device_by_node(). Can you please point me to
the routine (or the way we can traverse all devices) ?

> Wouldn't a better solution be to use fwnode stuff for this, and
> make the bus-type handling a property of the bus type itself,
> pushing the bus specific code into the bus layer?

As I said earlier, I don't really need to work at the bus level. I just need the
device structure and so that may not be required.

-- 
viresh

^ permalink raw reply

* [PATCH v4 0/4] pwm support for allwinner sun8i R40/V40/T3 SOCs.
From: hao_zhang @ 2017-12-13 14:38 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset add pwm support for allwinner sun8i R40/V40/T3 SOCs.

all has been teat on T3 board.

Some features
	8 PWM channels outputs(4 PWM pairs) 
	Supports capturing input 
	Supports three kinds of output waveforms: continuous waveform, pulse
	waveform and complementarity pair 
	Programmable deadzone generator and controllable dead-time 
	0% to 100% adjustable duty cycle 
	Up to 24/100MHz output frequency 
	Minimum resolution is 1/65536 
	Supports interrupt for PWM output and capturing input


DOC:
https://github.com/tinalinux/docs/blob/r40-v1.y/Allwinner_R40_User_Manual_V1.0.pdf
https://github.com/tinalinux/docs/blob/r40-v1.y/R40_Datasheet_V1.0.pdf

Regards;-)
Hao Zhang

hao_zhang (4):
  dt-bindings: pwm: binding allwinner R40, V40, T3
  ARM: PWM: add allwinner sun8i R40/V40/T3 pwm support.
  ARM: dts: add pwm node for r40.
  ARM: pinctrl: sunxi-pinctrl: fix pin funtion can not be match
    correctly.

 .../devicetree/bindings/pwm/pwm-sun8i.txt          |  18 +
 arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts  |   6 +
 arch/arm/boot/dts/sun8i-r40.dtsi                   |  13 +
 drivers/pinctrl/sunxi/pinctrl-sunxi.c              |   6 +-
 drivers/pwm/Kconfig                                |  10 +
 drivers/pwm/Makefile                               |   1 +
 drivers/pwm/pwm-sun8i-r40.c                        | 449 +++++++++++++++++++++
 7 files changed, 501 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
 create mode 100644 drivers/pwm/pwm-sun8i-r40.c

-- 
2.7.4

^ permalink raw reply

* [PATCH 4/4] [v4] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002
From: Andy Shevchenko @ 2017-12-13 14:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <88349f2e-5243-8061-cc72-d01fa70e6f2e@codeaurora.org>

On Tue, 2017-12-12 at 14:27 -0600, Timur Tabi wrote:
> On 12/12/2017 05:07 AM, Andy Shevchenko wrote:
> 
> > Not ACPI standards as of my knowledge. ACPI standard defines a
> > common
> > scheme how to define properties, it doesn't tell anything about
> > property
> > names or any mappings between names to values or names to "OS
> > subsystem").
> 
> There was an attempt a while back to standardize this like we do for 
> device tree, but it fell apart.  Device-specific ACPI-only properties 
> are not standarized.  This driver is initialized only on ACPI
> systems. 
> It has no device tree binding.

It should follow DT *de facto* standard bindings like "ngpios" (though
it's not needed in ACPI case IIRC) and other properties.

> > As for GPIO we just follow *de facto* what DT has right now, i.e.
> > "xxx-
> > gpio" or "xxx-gpios" pattern is used to map ACPI standard resource
> > to a
> > GPIO name. That's how GPIO ACPI lib is being developed.
> 
> GPIOs in device tree are defined completely differently than in ACPI. 
> On DT, the kernel controls the pin muxing.  On ACPI, pins are muxed
> by 
> firmware and never re-muxed by the operating system.  So all this
> driver 
> does is expose a few pins as simple GPIOs.

Wait, runtime muxing is a matter of requesting another function (usually
GPIO) and putting it back afterwards. Do you really need anything like
this at *runtime*?

Pin control design is not compatible with hardware (too abstract), but
that is the problem of DT as well: I'm referring here to not carefully
designed so called "pin states". This is another story and has nothing
specific for ACPI.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* [PATCH 4/4] [v4] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002
From: Andy Shevchenko @ 2017-12-13 14:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <bae16fa3-1bdf-74b1-97b7-f6b2db212621@codeaurora.org>

On Tue, 2017-12-12 at 14:17 -0600, Timur Tabi wrote:
> On 12/12/2017 04:05 AM, Andy Shevchenko wrote:
> > > +static const struct acpi_device_id qdf2xxx_acpi_ids[] = {
> > > +	{"QCOM8001", QDF2XXX_V1},
> > > +	{"QCOM8002", QDF2XXX_V2},
> > > +	{},
> > > +};
> > > +MODULE_DEVICE_TABLE(acpi, qdf2xxx_acpi_ids);
> > > 
> > > +	const struct acpi_device_id *id =
> > > +		acpi_match_device(qdf2xxx_acpi_ids, &pdev->dev);
> > 
> > JFYI: there is no need to move IDs like this.
> > Use members of struct device_driver wisely.
> 
> I have to move it, otherwise I get:
> 
> drivers/pinctrl/qcom/pinctrl-qdf2xxx.c:49:21: error:
> 'qdf2xxx_acpi_ids' 
> undeclared (first use in this function); did you mean
> 'qdf2xxx_pinctrl'?
> 
> I reference the structure in qdf2xxx_pinctrl_probe().

Please, read my comment again. The key part of the phrase: 
"Use members of struct device_driver"

So, do not move the IDs. There are examples in the kernel how to access
it.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* [PATCH v2 07/19] arm64: insn: Add encoder for bitwise operations using litterals
From: Marc Zyngier @ 2017-12-13 14:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5A3020DA.8010309@arm.com>

Hi James,

On 12/12/17 18:32, James Morse wrote:
> Hi Marc,
> 
> On 11/12/17 14:49, Marc Zyngier wrote:
>> We lack a way to encode operations such as AND, ORR, EOR that take
>> an immediate value. Doing so is quite involved, and is all about
>> reverse engineering the decoding algorithm described in the
>> pseudocode function DecodeBitMasks().
> 
> 
> As this is over my head, I've been pushing random encodings through gas/objdump
> and then tracing them through here.... can this encode 0xf80000000fffffff?
> 
> gas thinks this is legal:
> |   0:   92458000        and     x0, x0, #0xf80000000fffffff
> 
> I make that N=1, S=0x20, R=0x05.
> (I'm still working out what 'S' means)
> 
> 
>> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
>> index 7e432662d454..326b17016485 100644
>> --- a/arch/arm64/kernel/insn.c
>> +++ b/arch/arm64/kernel/insn.c
> 
>> +static u32 aarch64_encode_immediate(u64 imm,
>> +				    enum aarch64_insn_variant variant,
>> +				    u32 insn)
>> +{
>> +	unsigned int immr, imms, n, ones, ror, esz, tmp;
>> +	u64 mask;
> 
> [...]
> 
>> +	/* N is only set if we're encoding a 64bit value */
>> +	n = esz == 64;
>> +
>> +	/* Trim imm to the element size */
>> +	mask = BIT(esz - 1) - 1;
>> +	imm &= mask;
> 
> Won't this lose the top bit of a 64bit immediate?

Humfff... Yup, nicely spotted.

> 
> (but then you put it back later, so something funny is going on)
> 
> This becomes 0x780000000fffffff,
> 
> 
>> +
>> +	/* That's how many ones we need to encode */
>> +	ones = hweight64(imm);
> 
> meaning we're short a one here,
> 
> 
>> +
>> +	/*
>> +	 * imms is set to (ones - 1), prefixed with a string of ones
>> +	 * and a zero if they fit. Cap it to 6 bits.
>> +	 */
>> +	imms  = ones - 1;
>> +	imms |= 0xf << ffs(esz);
>> +	imms &= BIT(6) - 1;
> 
> so imms is 0x1f, not 0x20.
> 
> 
>> +	/* Compute the rotation */
>> +	if (range_of_ones(imm)) {
>> +		/*
>> +		 * Pattern: 0..01..10..0
>> +		 *
>> +		 * Compute how many rotate we need to align it right
>> +		 */
>> +		ror = ffs(imm) - 1;
> 
> (how come range_of_ones() uses __ffs64() on the same value?)

News flash: range_of_ones is completely buggy. It will fail on the 
trivial value 1 (__ffs64(1) = 0; 0 - 1 = -1; val >> -1 is... ermmmm).
I definitely got mixed up between the two.

>> +	} else {
>> +		/*
>> +		 * Pattern: 0..01..10..01..1
>> +		 *
>> +		 * Fill the unused top bits with ones, and check if
>> +		 * the result is a valid immediate (all ones with a
>> +		 * contiguous ranges of zeroes).
>> +		 */
> 
>> +		imm |= ~mask;
> 
> but here we put the missing one back,
> 
> 
>> +		if (!range_of_ones(~imm))
>> +			return AARCH64_BREAK_FAULT;
> 
> meaning we pass this check and carry on, (even though 0x780000000fffffff isn't a
> legal value)
> 
> 
> (this next bit I haven't worked out yet)
>> +		/*
>> +		 * Compute the rotation to get a continuous set of
>> +		 * ones, with the first bit set at position 0
>> +		 */
>> +		ror = fls(~imm);
>> +	}
>> +
>> +	/*
>> +	 * immr is the number of bits we need to rotate back to the
>> +	 * original set of ones. Note that this is relative to the
>> +	 * element size...
>> +	 */
>> +	immr = (esz - ror) & (esz - 1);
> 
> 
> If I've followed this through correctly, this results in:
> |   0:   92457c00        and     x0, x0, #0xf800000007ffffff
> 
> ... which wasn't the immediate I started with.
> 
> 
> Unless I've gone wrong, I think the 'Trim imm to the element size' code needs to
> move up into the esz-reducing loop so it doesn't happen for a 64bit immediate.

Yup. I've stashed the following patch:

diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index b8fb2d89b3a6..e58be1c57f18 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -1503,8 +1503,7 @@ pstate_check_t * const aarch32_opcode_cond_checks[16] = {
 static bool range_of_ones(u64 val)
 {
 	/* Doesn't handle full ones or full zeroes */
-	int x = __ffs64(val) - 1;
-	u64 sval = val >> x;
+	u64 sval = val >> __ffs64(val);
 
 	/* One of Sean Eron Anderson's bithack tricks */
 	return ((sval + 1) & (sval)) == 0;
@@ -1515,7 +1514,7 @@ static u32 aarch64_encode_immediate(u64 imm,
 				    u32 insn)
 {
 	unsigned int immr, imms, n, ones, ror, esz, tmp;
-	u64 mask;
+	u64 mask = ~0UL;
 
 	/* Can't encode full zeroes or full ones */
 	if (!imm || !~imm)
@@ -1543,8 +1542,12 @@ static u32 aarch64_encode_immediate(u64 imm,
 	for (tmp = esz; tmp > 2; tmp /= 2) {
 		u64 emask = BIT(tmp / 2) - 1;
 
-		if ((imm & emask) != ((imm >> (tmp / 2)) & emask))
+		if ((imm & emask) != ((imm >> (tmp / 2)) & emask)) {
+			/* Trim imm to the element size */
+			mask = BIT(esz - 1) - 1;
+			imm &= mask;
 			break;
+		}
 
 		esz = tmp;
 	}
@@ -1552,10 +1555,6 @@ static u32 aarch64_encode_immediate(u64 imm,
 	/* N is only set if we're encoding a 64bit value */
 	n = esz == 64;
 
-	/* Trim imm to the element size */
-	mask = BIT(esz - 1) - 1;
-	imm &= mask;
-
 	/* That's how many ones we need to encode */
 	ones = hweight64(imm);
 
I really need to run this against gas in order to make sure
I get the same parameters for all the possible values.

Many thanks for this careful review!

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply related

* [PATCH] ARM: dts: imx51: add CodaHx4 VPU
From: Philipp Zabel @ 2017-12-13 14:24 UTC (permalink / raw)
  To: linux-arm-kernel

Add the CodaHx4 VPU to the i.MX51 device tree.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
This patch should only be applied once the patch to fix VPU register access on
i.MX51 [1] is merged. The coda driver with patches [2] applied will start
accessing VPU registers on i.MX51, which hangs the system without [1].

[1] https://patchwork.kernel.org/patch/10109781/
    ("clk: imx51: uart4, uart5 gates only exist on imx50, imx53")
[2] https://patchwork.linuxtv.org/patch/45929/
    ("media: dt-bindings: coda: Add compatible for CodaHx4 on i.MX51")
    https://patchwork.linuxtv.org/patch/45930/
    ("media: coda: Add i.MX51 (CodaHx4) support")
---
 arch/arm/boot/dts/imx51.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index 378be720b3c76..6d6c19ede2be0 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -597,6 +597,17 @@
 				clock-names = "ipg", "ahb", "ptp";
 				status = "disabled";
 			};
+
+			vpu at 83ff4000 {
+				compatible = "fsl,imx51-vpu", "cnm,codahx4";
+				reg = <0x83ff4000 0x1000>;
+				interrupts = <9>;
+				clocks = <&clks IMX5_CLK_VPU_REFERENCE_GATE>,
+					 <&clks IMX5_CLK_VPU_GATE>;
+				clock-names = "per", "ahb";
+				resets = <&src 1>;
+				iram = <&iram>;
+			};
 		};
 	};
 };
-- 
2.11.0

^ permalink raw reply related

* [PATCH v6 3/6] kernel/reboot.c: export pm_power_off_prepare
From: Leonard Crestez @ 2017-12-13 14:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <b39d3ae2-5a93-854f-a0ff-aefd405da78b@pengutronix.de>

On Thu, 2017-12-07 at 06:36 +0100, Oleksij Rempel wrote:
> 
> On 07.12.2017 00:11, Christoph Hellwig wrote:
> > 
> > > 
> > > ?void (*pm_power_off_prepare)(void);
> > > +EXPORT_SYMBOL(pm_power_off_prepare);
> > EXPORT_SYMBOL_GPL for something this deeply internal, please.
> Ok,
> probably all other symbols should be converted in this file in to
> EXPORT_SYMBOL_GPL as well?
> 
> grep EXPORT_SYMBOL kernel/reboot.c
> EXPORT_SYMBOL(cad_pid);
> EXPORT_SYMBOL(pm_power_off_prepare);
> EXPORT_SYMBOL_GPL(emergency_restart);
> EXPORT_SYMBOL(register_reboot_notifier);
> EXPORT_SYMBOL(unregister_reboot_notifier);
> EXPORT_SYMBOL(devm_register_reboot_notifier);
> EXPORT_SYMBOL(register_restart_handler);
> EXPORT_SYMBOL(unregister_restart_handler);
> EXPORT_SYMBOL_GPL(kernel_restart);
> EXPORT_SYMBOL_GPL(kernel_halt);
> EXPORT_SYMBOL_GPL(kernel_power_off);
> EXPORT_SYMBOL_GPL(orderly_poweroff);
> EXPORT_SYMBOL_GPL(orderly_reboot);

This call looks much more deeply internal than those other functions,
it's not like reboot_notifier and restart_handler.

It seems that the only user of pm_power_off_prepare right now is the
ACPI core and your patch uses it in a regulator driver. This looks
extremely strange. Maybe this is why imx maintainers are stalling on
this patch?

It might help if the ACPI/PM maintainers ack/review this part
explicitly, and the usage in part 5:

https://patchwork.kernel.org/patch/9799615/

--
Regards,
Leonard

^ permalink raw reply

* [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, &reg);
>> >>
>> >>         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, &reg);
> >>
> >>         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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox