Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [alsa-devel] [PATCH 5/8] ASoC: uniphier: add support for UniPhier AIO driver
From: Vinod Koul @ 2017-12-11 17:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211151629.GD9854@sirena.org.uk>

On Mon, Dec 11, 2017 at 03:16:29PM +0000, Mark Brown wrote:
> On Mon, Dec 11, 2017 at 06:21:58PM +0900, Katsuhiro Suzuki wrote:
> 
> > But I can't find how to use/map this DAI in machine driver or Device-Tree or
> > something. I think that it's same as PCM DAI, am I correct?
> 
> Yes, that probably makes sense from a binding point of view.
> 
> > I read compress-offload.rst, but I can't find how do I test it. It seems aplay
> > of
> > alsa-util doesn't know compress audio formats. Should I use PulseAudio or
> > Android HAL to test compress audio APIs?
> 
> IIRC tinyalsa has a compressed API test application - Vinod?

I guess it was sheer luck that i saw this :) email in CC reads
vinod.koul at linaro.org! I don't work for Linaro, not yet :D

And to the answer the question, Yes we have compressed API test application
in tinycompress which is located at git.alsa-project.org:tinycompress.git

We have both compressed audio playback as well as record test app, cplay and
crecord.

HTH
-- 
~Vinod

^ permalink raw reply

* [PATCH 5/5] arm_pmu: acpi: request IRQs up-front
From: Mark Rutland @ 2017-12-11 17:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211173647.GA3275@arm.com>

On Mon, Dec 11, 2017 at 05:36:47PM +0000, Will Deacon wrote:
> On Wed, Nov 01, 2017 at 02:12:39PM +0000, Mark Rutland wrote:
> > We can't request IRQs in atomic context, so for ACPI systems we'll have
> > to request them up-front, and later associate them with CPUs.
> > 
> > This patch reorganises the arm_pmu code to do so. As we no longer have
> > the arm_pmu strucutre at probe time, a number of prototypes need to be
> > adjusted, requiring changes to the common arm_pmu code and arm_pmu
> > platform code.

> > +void armpmu_bind_cpu(struct arm_pmu *armpmu, int cpu)
> >  {
> > -	struct arm_pmu_platdata *platdata = armpmu_get_platdata(armpmu);
> > -	unsigned long irq_flags;
> > -	struct pmu_hw_events __percpu *hw_events = armpmu->hw_events;
> > -	int irq = per_cpu(hw_events->irq, cpu);
> > -	if (!irq)
> > -		return 0;
> > -
> > -	if (platdata && platdata->irq_flags) {
> > -		irq_flags = platdata->irq_flags;
> > -	} else {
> > -		irq_flags = ARM_PMU_IRQ_FLAGS;
> > -	}
> > -
> >  	per_cpu(cpu_armpmu, cpu) = armpmu;
> 
> Can we not make the binding implicit in armpmu_{request,free}_irq?

Unfortunately not.

As mentioned in the commit message (typo and all), in the ACPI case, we
need to request/free IRQs before we know the PMU.

Mark.

^ permalink raw reply

* [PATCH v3 00/20] arm64: Unmap the kernel whilst running in userspace (KPTI)
From: Catalin Marinas @ 2017-12-11 17:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1512563739-25239-1-git-send-email-will.deacon@arm.com>

On Wed, Dec 06, 2017 at 12:35:19PM +0000, Will Deacon wrote:
> Patches are also pushed here:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git kpti
> 
> Feedback and testing welcome. At this point, I'd like to start thinking
> about getting this merged for 4.16.

For the record, the fixed up version was pushed by Will here:

git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git kpti

and I queued it for 4.16 in the arm64 for-next/core branch (same tree as
above).

-- 
Catalin

^ permalink raw reply

* [PATCH 2/5] arm_pmu: have armpmu_alloc() take GFP flags
From: Mark Rutland @ 2017-12-11 18:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211173702.GC3275@arm.com>

On Mon, Dec 11, 2017 at 05:37:02PM +0000, Will Deacon wrote:
> On Wed, Nov 01, 2017 at 02:12:36PM +0000, Mark Rutland wrote:
> > diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
> > index 705f1a390e31..a52f5b673a15 100644
> > --- a/drivers/perf/arm_pmu_acpi.c
> > +++ b/drivers/perf/arm_pmu_acpi.c
> > @@ -127,7 +127,7 @@ static struct arm_pmu *arm_pmu_acpi_find_alloc_pmu(void)
> >  		return pmu;
> >  	}
> >  
> > -	pmu = armpmu_alloc();
> > +	pmu = armpmu_alloc(GFP_ATOMIC);
> 
> I think I'd rather have armpmu_alloc_atomic as a wrapper around
> __armpmu_alloc(GFP_ATOMIC) and then leave the armpmu_alloc to pass
> GFP_KERNEL.

Sure; done.

Mark.

^ permalink raw reply

* [PATCH 3/5] arm_pmu: acpi: check for mismatched PPIs
From: Mark Rutland @ 2017-12-11 18:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211173707.GD3275@arm.com>

On Mon, Dec 11, 2017 at 05:37:07PM +0000, Will Deacon wrote:
> On Wed, Nov 01, 2017 at 02:12:37PM +0000, Mark Rutland wrote:
> > The arm_pmu platform code explicitly checks for mismatched PPIs at probe
> > time, while the ACPI code leaves this to the core code. Future
> > refactoring will make this difficult for the core code to check, so
> > let's have the ACPI code check this explicitly.
> > 
> > As before, upon a failure we'll continue on without an interrupt. Ho
> > hum.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > ---
> >  drivers/perf/arm_pmu.c      | 16 ++++------------
> >  drivers/perf/arm_pmu_acpi.c | 42 ++++++++++++++++++++++++++++++++++++++----
> >  2 files changed, 42 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> > index 3d6d4c5f2356..e0242103d904 100644
> > --- a/drivers/perf/arm_pmu.c
> > +++ b/drivers/perf/arm_pmu.c
> > @@ -557,18 +557,10 @@ int armpmu_request_irq(struct arm_pmu *armpmu, int cpu)
> >  	if (!irq)
> >  		return 0;
> >  
> > -	if (irq_is_percpu_devid(irq) && cpumask_empty(&armpmu->active_irqs)) {
> > -		err = request_percpu_irq(irq, handler, "arm-pmu",
> > -					 &hw_events->percpu_pmu);
> > -	} else if (irq_is_percpu_devid(irq)) {
> > -		int other_cpu = cpumask_first(&armpmu->active_irqs);
> > -		int other_irq = per_cpu(hw_events->irq, other_cpu);
> > -
> > -		if (irq != other_irq) {
> > -			pr_warn("mismatched PPIs detected.\n");
> > -			err = -EINVAL;
> > -			goto err_out;
> > -		}
> > +	if (irq_is_percpu_devid(irq)) {
> > +		if (cpumask_empty(&armpmu->active_irqs))
> 
> Why not leave this as before, with a '&&' operator?

Because then we'd fall into the else case (for SPIs), were the
active_irqs mask empty.

Previously, that would have been caught by the irq_is_percpu_devid(irq)
case that got removed.

I can instead make this:

	if (irq_is_percpu_devid(irq) && cpumask_empty(&armpmu->active_irqs)) {
		err = request_percpu_irq(irq, handler, "arm-pmu",
					 &hw_events->percpu_pmu);
	} else if (irq_is_percpu_devid(irq)) {
		/* nothing to do */
	} else  { 
		< SPI case >
	}

... but that seemed more painful to read.

Mark.

^ permalink raw reply

* [PATCH 4/5] arm_pmu: note IRQs/PMUs per-cpu
From: Mark Rutland @ 2017-12-11 18:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211173657.GB3275@arm.com>

On Mon, Dec 11, 2017 at 05:36:58PM +0000, Will Deacon wrote:
> On Wed, Nov 01, 2017 at 02:12:38PM +0000, Mark Rutland wrote:
> > @@ -531,40 +530,56 @@ int perf_num_counters(void)
> >  }
> >  EXPORT_SYMBOL_GPL(perf_num_counters);
> >  
> > -void armpmu_free_irq(struct arm_pmu *armpmu, int cpu)
> > +int armpmu_count_irq_users(const int irq)
> >  {
> > -	struct pmu_hw_events __percpu *hw_events = armpmu->hw_events;
> > -	int irq = per_cpu(hw_events->irq, cpu);
> > +	int cpu, count = 0;
> > +
> > +	for_each_possible_cpu(cpu) {
> > +		if (per_cpu(cpu_irq, cpu) == irq)
> > +			count++;
> > +	}
> > +
> > +	return count;
> > +}
> >  
> > -	if (!cpumask_test_and_clear_cpu(cpu, &armpmu->active_irqs))
> > +void __armpmu_free_irq(int irq, int cpu)
> > +{
> > +	if (per_cpu(cpu_irq, cpu) == 0)
> > +		return;
> > +	if (WARN_ON(irq != per_cpu(cpu_irq, cpu)))
> >  		return;
> >  
> >  	if (irq_is_percpu_devid(irq)) {
> > -		free_percpu_irq(irq, &hw_events->percpu_pmu);
> > -		cpumask_clear(&armpmu->active_irqs);
> > -		return;
> > +		if (armpmu_count_irq_users(irq) == 1)
> > +			free_percpu_irq(irq, &cpu_armpmu);
> 
> Do you actually need the count, or could you just free the irq the first
> time this is called and set all of the cpu_irqs to 0?

It might be safe to blat all the matching cpu_irq entries. I'll take a
look.

Thanks,
Mark.

^ permalink raw reply

* [PATCH 3/5] arm_pmu: acpi: check for mismatched PPIs
From: Will Deacon @ 2017-12-11 18:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211180830.firkcy3jptw3bou3@lakrids.cambridge.arm.com>

On Mon, Dec 11, 2017 at 06:08:31PM +0000, Mark Rutland wrote:
> On Mon, Dec 11, 2017 at 05:37:07PM +0000, Will Deacon wrote:
> > On Wed, Nov 01, 2017 at 02:12:37PM +0000, Mark Rutland wrote:
> > > The arm_pmu platform code explicitly checks for mismatched PPIs at probe
> > > time, while the ACPI code leaves this to the core code. Future
> > > refactoring will make this difficult for the core code to check, so
> > > let's have the ACPI code check this explicitly.
> > > 
> > > As before, upon a failure we'll continue on without an interrupt. Ho
> > > hum.
> > > 
> > > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Will Deacon <will.deacon@arm.com>
> > > ---
> > >  drivers/perf/arm_pmu.c      | 16 ++++------------
> > >  drivers/perf/arm_pmu_acpi.c | 42 ++++++++++++++++++++++++++++++++++++++----
> > >  2 files changed, 42 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> > > index 3d6d4c5f2356..e0242103d904 100644
> > > --- a/drivers/perf/arm_pmu.c
> > > +++ b/drivers/perf/arm_pmu.c
> > > @@ -557,18 +557,10 @@ int armpmu_request_irq(struct arm_pmu *armpmu, int cpu)
> > >  	if (!irq)
> > >  		return 0;
> > >  
> > > -	if (irq_is_percpu_devid(irq) && cpumask_empty(&armpmu->active_irqs)) {
> > > -		err = request_percpu_irq(irq, handler, "arm-pmu",
> > > -					 &hw_events->percpu_pmu);
> > > -	} else if (irq_is_percpu_devid(irq)) {
> > > -		int other_cpu = cpumask_first(&armpmu->active_irqs);
> > > -		int other_irq = per_cpu(hw_events->irq, other_cpu);
> > > -
> > > -		if (irq != other_irq) {
> > > -			pr_warn("mismatched PPIs detected.\n");
> > > -			err = -EINVAL;
> > > -			goto err_out;
> > > -		}
> > > +	if (irq_is_percpu_devid(irq)) {
> > > +		if (cpumask_empty(&armpmu->active_irqs))
> > 
> > Why not leave this as before, with a '&&' operator?
> 
> Because then we'd fall into the else case (for SPIs), were the
> active_irqs mask empty.
> 
> Previously, that would have been caught by the irq_is_percpu_devid(irq)
> case that got removed.
> 
> I can instead make this:
> 
> 	if (irq_is_percpu_devid(irq) && cpumask_empty(&armpmu->active_irqs)) {
> 		err = request_percpu_irq(irq, handler, "arm-pmu",
> 					 &hw_events->percpu_pmu);
> 	} else if (irq_is_percpu_devid(irq)) {
> 		/* nothing to do */
> 	} else  { 
> 		< SPI case >
> 	}
> 
> ... but that seemed more painful to read.

Yeah, that's crazy :)

How about:

  if (!irq_is_percpu_devid(irq)) {
	/* SPI case */
  } else if (cpumask_empty(&armpmu->active_irqs)) {
	/* PPI case */
  }

?

Will

^ permalink raw reply

* [PATCH 5/5] arm_pmu: acpi: request IRQs up-front
From: Will Deacon @ 2017-12-11 18:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211175539.jqjxyoa4vtwrakl3@lakrids.cambridge.arm.com>

On Mon, Dec 11, 2017 at 05:55:40PM +0000, Mark Rutland wrote:
> On Mon, Dec 11, 2017 at 05:36:47PM +0000, Will Deacon wrote:
> > On Wed, Nov 01, 2017 at 02:12:39PM +0000, Mark Rutland wrote:
> > > We can't request IRQs in atomic context, so for ACPI systems we'll have
> > > to request them up-front, and later associate them with CPUs.
> > > 
> > > This patch reorganises the arm_pmu code to do so. As we no longer have
> > > the arm_pmu strucutre at probe time, a number of prototypes need to be
> > > adjusted, requiring changes to the common arm_pmu code and arm_pmu
> > > platform code.
> 
> > > +void armpmu_bind_cpu(struct arm_pmu *armpmu, int cpu)
> > >  {
> > > -	struct arm_pmu_platdata *platdata = armpmu_get_platdata(armpmu);
> > > -	unsigned long irq_flags;
> > > -	struct pmu_hw_events __percpu *hw_events = armpmu->hw_events;
> > > -	int irq = per_cpu(hw_events->irq, cpu);
> > > -	if (!irq)
> > > -		return 0;
> > > -
> > > -	if (platdata && platdata->irq_flags) {
> > > -		irq_flags = platdata->irq_flags;
> > > -	} else {
> > > -		irq_flags = ARM_PMU_IRQ_FLAGS;
> > > -	}
> > > -
> > >  	per_cpu(cpu_armpmu, cpu) = armpmu;
> > 
> > Can we not make the binding implicit in armpmu_{request,free}_irq?
> 
> Unfortunately not.
> 
> As mentioned in the commit message (typo and all), in the ACPI case, we
> need to request/free IRQs before we know the PMU.

Urgh. This is hideous! Just try reading the imnplementation of
armpmu_bind_cpu out loud.

Could we use {enable,disable}_irq in the hotplug notifier and request the
interrupts with NOAUTOEN instead? That would mean we have a similar flow
for SPI and PPIs and could potentially hide some of the book-keeping behind
armpmu_{enable,disable}_irq functions.

Will

^ permalink raw reply

* [PATCH] ARM: pxa/lubbock: add GPIO driver for LUB_MISC_WR register
From: Russell King @ 2017-12-11 18:56 UTC (permalink / raw)
  To: linux-arm-kernel

Add a gpio driver for the lubbock miscellaneous write IO register so we
can take advantage of subsystems modelled around gpiolib, rather than
having to provide platform specific callbacks.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-pxa/Kconfig   |  1 +
 arch/arm/mach-pxa/lubbock.c | 24 ++++++++++++++++--------
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig
index ce7d97babb0f..a68b34183107 100644
--- a/arch/arm/mach-pxa/Kconfig
+++ b/arch/arm/mach-pxa/Kconfig
@@ -40,6 +40,7 @@ config MACH_PXA3XX_DT
 
 config ARCH_LUBBOCK
 	bool "Intel DBPXA250 Development Platform (aka Lubbock)"
+	select GPIO_REG
 	select PXA25x
 	select SA1111
 
diff --git a/arch/arm/mach-pxa/lubbock.c b/arch/arm/mach-pxa/lubbock.c
index df45682e99a5..fe2ef9b78602 100644
--- a/arch/arm/mach-pxa/lubbock.c
+++ b/arch/arm/mach-pxa/lubbock.c
@@ -13,6 +13,7 @@
  */
 #include <linux/clkdev.h>
 #include <linux/gpio.h>
+#include <linux/gpio/gpio-reg.h>
 #include <linux/gpio/machine.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -110,20 +111,18 @@ static unsigned long lubbock_pin_config[] __initdata = {
 };
 
 #define LUB_HEXLED		__LUB_REG(LUBBOCK_FPGA_PHYS + 0x010)
-#define LUB_MISC_WR		__LUB_REG(LUBBOCK_FPGA_PHYS + 0x080)
 
 void lubbock_set_hexled(uint32_t value)
 {
 	LUB_HEXLED = value;
 }
 
+static struct gpio_chip *lubbock_misc_wr_gc;
+
 void lubbock_set_misc_wr(unsigned int mask, unsigned int set)
 {
-	unsigned long flags;
-
-	local_irq_save(flags);
-	LUB_MISC_WR = (LUB_MISC_WR & ~mask) | (set & mask);
-	local_irq_restore(flags);
+	unsigned long m = mask, v = set;
+	lubbock_misc_wr_gc->set_multiple(lubbock_misc_wr_gc, &m, &v);
 }
 EXPORT_SYMBOL(lubbock_set_misc_wr);
 
@@ -452,9 +451,9 @@ static void lubbock_irda_transceiver_mode(struct device *dev, int mode)
 
 	local_irq_save(flags);
 	if (mode & IR_SIRMODE) {
-		LUB_MISC_WR &= ~(1 << 4);
+		lubbock_set_misc_wr(BIT(4), 0);
 	} else if (mode & IR_FIRMODE) {
-		LUB_MISC_WR |= 1 << 4;
+		lubbock_set_misc_wr(BIT(4), BIT(4));
 	}
 	pxa2xx_transceiver_mode(dev, mode);
 	local_irq_restore(flags);
@@ -472,6 +471,15 @@ static void __init lubbock_init(void)
 
 	pxa2xx_mfp_config(ARRAY_AND_SIZE(lubbock_pin_config));
 
+	lubbock_misc_wr_gc = gpio_reg_init(NULL, (void *)&LUB_MISC_WR,
+					   -1, 16, "lubbock", 0, LUB_MISC_WR,
+					   NULL, NULL, NULL);
+	if (IS_ERR(lubbock_misc_wr_gc)) {
+		pr_err("Lubbock: unable to register lubbock GPIOs: %ld\n",
+		       PTR_ERR(lubbock_misc_wr_gc));
+		lubbock_misc_wr_gc = NULL;
+	}
+
 	pxa_set_ffuart_info(NULL);
 	pxa_set_btuart_info(NULL);
 	pxa_set_stuart_info(NULL);
-- 
2.7.4

^ permalink raw reply related

* [PATCH] ARM: sa1100/assabet: add BCR/BSR GPIO driver
From: Russell King @ 2017-12-11 18:56 UTC (permalink / raw)
  To: linux-arm-kernel

Add a GPIO driver for the board control register/board status register
for the sa1100/assabet platform.  This allows us to transition a range
of drivers to use the gpiod APIs rather than the platform private
ASSABET_BCR_* interfaces.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-sa1100/Kconfig   |  1 +
 arch/arm/mach-sa1100/assabet.c | 71 ++++++++++++++++++++++++++++++++----------
 2 files changed, 55 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-sa1100/Kconfig b/arch/arm/mach-sa1100/Kconfig
index 36e3c79f4973..07df3a59b13f 100644
--- a/arch/arm/mach-sa1100/Kconfig
+++ b/arch/arm/mach-sa1100/Kconfig
@@ -5,6 +5,7 @@ menu "SA11x0 Implementations"
 config SA1100_ASSABET
 	bool "Assabet"
 	select ARM_SA1110_CPUFREQ
+	select GPIO_REG
 	help
 	  Say Y here if you are using the Intel(R) StrongARM(R) SA-1110
 	  Microprocessor Development Board (also known as the Assabet).
diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c
index d28ecb9ef172..f68241d995f2 100644
--- a/arch/arm/mach-sa1100/assabet.c
+++ b/arch/arm/mach-sa1100/assabet.c
@@ -13,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/errno.h>
+#include <linux/gpio/gpio-reg.h>
 #include <linux/ioport.h>
 #include <linux/platform_data/sa11x0-serial.h>
 #include <linux/serial_core.h>
@@ -61,20 +62,45 @@
 unsigned long SCR_value = ASSABET_SCR_INIT;
 EXPORT_SYMBOL(SCR_value);
 
-static unsigned long BCR_value = ASSABET_BCR_DB1110;
+static struct gpio_chip *assabet_bcr_gc;
+
+static const char *assabet_names[] = {
+	"cf_pwr", "cf_gfx_reset", "nsoft_reset", "irda_fsel",
+	"irda_md0", "irda_md1", "stereo_loopback", "ncf_bus_on",
+	"audio_pwr_on", "light_pwr_on", "lcd16data", "lcd_pwr_on",
+	"rs232_on", "nred_led", "ngreen_led", "vib_on",
+	"com_dtr", "com_rts", "radio_wake_mod", "i2c_enab",
+	"tvir_enab", "qmute", "radio_pwr_on", "spkr_off",
+	"rs232_valid", "com_dcd", "com_cts", "com_dsr",
+	"radio_cts", "radio_dsr", "radio_dcd", "radio_ri",
+};
 
+/* The old deprecated interface */
 void ASSABET_BCR_frob(unsigned int mask, unsigned int val)
 {
-	unsigned long flags;
+	unsigned long m = mask, v = val;
 
-	local_irq_save(flags);
-	BCR_value = (BCR_value & ~mask) | val;
-	ASSABET_BCR = BCR_value;
-	local_irq_restore(flags);
+	assabet_bcr_gc->set_multiple(assabet_bcr_gc, &m, &v);
 }
-
 EXPORT_SYMBOL(ASSABET_BCR_frob);
 
+static int __init assabet_init_gpio(void __iomem *reg, u32 def_val)
+{
+	struct gpio_chip *gc;
+
+	writel_relaxed(def_val, reg);
+
+	gc = gpio_reg_init(NULL, reg, -1, 32, "assabet", 0xff000000, def_val,
+			   assabet_names, NULL, NULL);
+
+	if (IS_ERR(gc))
+		return PTR_ERR(gc);
+
+	assabet_bcr_gc = gc;
+
+	return gc->base;
+}
+
 /*
  * The codec reset goes to three devices, so we need to release
  * the rest when any one of these requests it.  However, that
@@ -146,7 +172,7 @@ static void adv7171_write(unsigned reg, unsigned val)
 	unsigned gpdr = GPDR;
 	unsigned gplr = GPLR;
 
-	ASSABET_BCR = BCR_value | ASSABET_BCR_AUDIO_ON;
+	ASSABET_BCR_frob(ASSABET_BCR_AUDIO_ON, ASSABET_BCR_AUDIO_ON);
 	udelay(100);
 
 	GPCR = SDA | SCK | MOD; /* clear L3 mode to ensure UDA1341 doesn't respond */
@@ -457,14 +483,6 @@ static void __init assabet_init(void)
 	sa11x0_ppc_configure_mcp();
 
 	if (machine_has_neponset()) {
-		/*
-		 * Angel sets this, but other bootloaders may not.
-		 *
-		 * This must precede any driver calls to BCR_set()
-		 * or BCR_clear().
-		 */
-		ASSABET_BCR = BCR_value = ASSABET_BCR_DB1111;
-
 #ifndef CONFIG_ASSABET_NEPONSET
 		printk( "Warning: Neponset detected but full support "
 			"hasn't been configured in the kernel\n" );
@@ -748,12 +766,31 @@ static int __init assabet_leds_init(void)
 fs_initcall(assabet_leds_init);
 #endif
 
+void __init assabet_init_irq(void)
+{
+	u32 def_val;
+
+	sa1100_init_irq();
+
+	if (machine_has_neponset())
+		def_val = ASSABET_BCR_DB1111;
+	else
+		def_val = ASSABET_BCR_DB1110;
+
+	/*
+	 * Angel sets this, but other bootloaders may not.
+	 *
+	 * This must precede any driver calls to BCR_set() or BCR_clear().
+	 */
+	assabet_init_gpio((void *)&ASSABET_BCR, def_val);
+}
+
 MACHINE_START(ASSABET, "Intel-Assabet")
 	.atag_offset	= 0x100,
 	.fixup		= fixup_assabet,
 	.map_io		= assabet_map_io,
 	.nr_irqs	= SA1100_NR_IRQS,
-	.init_irq	= sa1100_init_irq,
+	.init_irq	= assabet_init_irq,
 	.init_time	= sa1100_timer_init,
 	.init_machine	= assabet_init,
 	.init_late	= sa11x0_init_late,
-- 
2.7.4

^ permalink raw reply related

* [PATCH] ARM: sa1100/neponset: add GPIO drivers for control and modem registers
From: Russell King @ 2017-12-11 18:56 UTC (permalink / raw)
  To: linux-arm-kernel

The NCR, MDM_CTL* and AUD registers manipulate the state of external
signals (eg, the RTS, DTR signals and the ethernet oscillator enable
signal) or indicate the state of external signals (eg, CTS, DSR).

Where these registers can be written, the current value can be read
back, which relieves us from having to maintain a software copy of
the current state.

Model these registers as fixed-direction GPIO registers.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-sa1100/neponset.c | 159 ++++++++++++++++++++++++----------------
 1 file changed, 96 insertions(+), 63 deletions(-)

diff --git a/arch/arm/mach-sa1100/neponset.c b/arch/arm/mach-sa1100/neponset.c
index a61a2432766b..b1823f445358 100644
--- a/arch/arm/mach-sa1100/neponset.c
+++ b/arch/arm/mach-sa1100/neponset.c
@@ -3,6 +3,8 @@
  * linux/arch/arm/mach-sa1100/neponset.c
  */
 #include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/gpio-reg.h>
 #include <linux/init.h>
 #include <linux/ioport.h>
 #include <linux/irq.h>
@@ -45,10 +47,13 @@
 #define IRR_USAR	(1 << 1)
 #define IRR_SA1111	(1 << 2)
 
+#define NCR_NGPIO	7
+
 #define MDM_CTL0_RTS1	(1 << 0)
 #define MDM_CTL0_DTR1	(1 << 1)
 #define MDM_CTL0_RTS2	(1 << 2)
 #define MDM_CTL0_DTR2	(1 << 3)
+#define MDM_CTL0_NGPIO	4
 
 #define MDM_CTL1_CTS1	(1 << 0)
 #define MDM_CTL1_DSR1	(1 << 1)
@@ -56,80 +61,87 @@
 #define MDM_CTL1_CTS2	(1 << 3)
 #define MDM_CTL1_DSR2	(1 << 4)
 #define MDM_CTL1_DCD2	(1 << 5)
+#define MDM_CTL1_NGPIO	6
 
 #define AUD_SEL_1341	(1 << 0)
 #define AUD_MUTE_1341	(1 << 1)
+#define AUD_NGPIO	2
 
 extern void sa1110_mb_disable(void);
 
+#define to_neponset_gpio_chip(x) container_of(x, struct neponset_gpio_chip, gc)
+
+static const char *neponset_ncr_names[] = {
+	"gp01_off", "tp_power", "ms_power", "enet_osc",
+	"spi_kb_wk_up", "a0vpp", "a1vpp"
+};
+
+static const char *neponset_mdmctl0_names[] = {
+	"rts3", "dtr3", "rts1", "dtr1",
+};
+
+static const char *neponset_mdmctl1_names[] = {
+	"cts3", "dsr3", "dcd3", "cts1", "dsr1", "dcd1"
+};
+
+static const char *neponset_aud_names[] = {
+	"sel_1341", "mute_1341",
+};
+
 struct neponset_drvdata {
 	void __iomem *base;
 	struct platform_device *sa1111;
 	struct platform_device *smc91x;
 	unsigned irq_base;
-#ifdef CONFIG_PM_SLEEP
-	u32 ncr0;
-	u32 mdm_ctl_0;
-#endif
+	struct gpio_chip *gpio[4];
 };
 
-static void __iomem *nep_base;
+static struct neponset_drvdata *nep;
 
 void neponset_ncr_frob(unsigned int mask, unsigned int val)
 {
-	void __iomem *base = nep_base;
-
-	if (base) {
-		unsigned long flags;
-		unsigned v;
-
-		local_irq_save(flags);
-		v = readb_relaxed(base + NCR_0);
-		writeb_relaxed((v & ~mask) | val, base + NCR_0);
-		local_irq_restore(flags);
-	} else {
-		WARN(1, "nep_base unset\n");
-	}
+	struct neponset_drvdata *n = nep;
+	unsigned long m = mask, v = val;
+
+	if (nep)
+		n->gpio[0]->set_multiple(n->gpio[0], &m, &v);
+	else
+		WARN(1, "nep unset\n");
 }
 EXPORT_SYMBOL(neponset_ncr_frob);
 
 static void neponset_set_mctrl(struct uart_port *port, u_int mctrl)
 {
-	void __iomem *base = nep_base;
-	u_int mdm_ctl0;
+	struct neponset_drvdata *n = nep;
+	unsigned long mask, val = 0;
 
-	if (!base)
+	if (!n)
 		return;
 
-	mdm_ctl0 = readb_relaxed(base + MDM_CTL_0);
 	if (port->mapbase == _Ser1UTCR0) {
-		if (mctrl & TIOCM_RTS)
-			mdm_ctl0 &= ~MDM_CTL0_RTS2;
-		else
-			mdm_ctl0 |= MDM_CTL0_RTS2;
-
-		if (mctrl & TIOCM_DTR)
-			mdm_ctl0 &= ~MDM_CTL0_DTR2;
-		else
-			mdm_ctl0 |= MDM_CTL0_DTR2;
+		mask = MDM_CTL0_RTS2 | MDM_CTL0_DTR2;
+
+		if (!(mctrl & TIOCM_RTS))
+			val |= MDM_CTL0_RTS2;
+
+		if (!(mctrl & TIOCM_DTR))
+			val |= MDM_CTL0_DTR2;
 	} else if (port->mapbase == _Ser3UTCR0) {
-		if (mctrl & TIOCM_RTS)
-			mdm_ctl0 &= ~MDM_CTL0_RTS1;
-		else
-			mdm_ctl0 |= MDM_CTL0_RTS1;
-
-		if (mctrl & TIOCM_DTR)
-			mdm_ctl0 &= ~MDM_CTL0_DTR1;
-		else
-			mdm_ctl0 |= MDM_CTL0_DTR1;
+		mask = MDM_CTL0_RTS1 | MDM_CTL0_DTR1;
+
+		if (!(mctrl & TIOCM_RTS))
+			val |= MDM_CTL0_RTS1;
+
+		if (!(mctrl & TIOCM_DTR))
+			val |= MDM_CTL0_DTR1;
 	}
 
-	writeb_relaxed(mdm_ctl0, base + MDM_CTL_0);
+	n->gpio[1]->set_multiple(n->gpio[1], &mask, &val);
 }
 
 static u_int neponset_get_mctrl(struct uart_port *port)
 {
-	void __iomem *base = nep_base;
+	void __iomem *base = nep->base;
 	u_int ret = TIOCM_CD | TIOCM_CTS | TIOCM_DSR;
 	u_int mdm_ctl1;
 
@@ -231,6 +243,22 @@ static struct irq_chip nochip = {
 	.irq_unmask = nochip_noop,
 };
 
+static int neponset_init_gpio(struct gpio_chip **gcp,
+	struct device *dev, const char *label, void __iomem *reg,
+	unsigned num, bool in, const char *const * names)
+{
+	struct gpio_chip *gc;
+
+	gc = gpio_reg_init(dev, reg, -1, num, label, in ? 0xffffffff : 0,
+			   readl_relaxed(reg), names, NULL, NULL);
+	if (IS_ERR(gc))
+		return PTR_ERR(gc);
+
+	*gcp = gc;
+
+	return 0;
+}
+
 static struct sa1111_platform_data sa1111_info = {
 	.disable_devs	= SA1111_DEVID_PS2_MSE,
 };
@@ -274,7 +302,7 @@ static int neponset_probe(struct platform_device *dev)
 	};
 	int ret, irq;
 
-	if (nep_base)
+	if (nep)
 		return -EBUSY;
 
 	irq = ret = platform_get_irq(dev, 0);
@@ -330,6 +358,22 @@ static int neponset_probe(struct platform_device *dev)
 	irq_set_irq_type(irq, IRQ_TYPE_EDGE_RISING);
 	irq_set_chained_handler_and_data(irq, neponset_irq_handler, d);
 
+	/* Disable GPIO 0/1 drivers so the buttons work on the Assabet */
+	writeb_relaxed(NCR_GP01_OFF, d->base + NCR_0);
+
+	neponset_init_gpio(&d->gpio[0], &dev->dev, "neponset-ncr",
+			   d->base + NCR_0, NCR_NGPIO, false,
+			   neponset_ncr_names);
+	neponset_init_gpio(&d->gpio[1], &dev->dev, "neponset-mdm-ctl0",
+			   d->base + MDM_CTL_0, MDM_CTL0_NGPIO, false,
+			   neponset_mdmctl0_names);
+	neponset_init_gpio(&d->gpio[2], &dev->dev, "neponset-mdm-ctl1",
+			   d->base + MDM_CTL_1, MDM_CTL1_NGPIO, true,
+			   neponset_mdmctl1_names);
+	neponset_init_gpio(&d->gpio[3], &dev->dev, "neponset-aud-ctl",
+			   d->base + AUD_CTL, AUD_NGPIO, false,
+			   neponset_aud_names);
+
 	/*
 	 * We would set IRQ_GPIO25 to be a wake-up IRQ, but unfortunately
 	 * something on the Neponset activates this IRQ on sleep (eth?)
@@ -340,16 +384,13 @@ static int neponset_probe(struct platform_device *dev)
 
 	dev_info(&dev->dev, "Neponset daughter board, providing IRQ%u-%u\n",
 		 d->irq_base, d->irq_base + NEP_IRQ_NR - 1);
-	nep_base = d->base;
+	nep = d;
 
 	sa1100_register_uart_fns(&neponset_port_fns);
 
 	/* Ensure that the memory bus request/grant signals are setup */
 	sa1110_mb_disable();
 
-	/* Disable GPIO 0/1 drivers so the buttons work on the Assabet */
-	writeb_relaxed(NCR_GP01_OFF, d->base + NCR_0);
-
 	sa1111_resources[0].parent = sa1111_res;
 	sa1111_resources[1].start = d->irq_base + NEP_IRQ_SA1111;
 	sa1111_resources[1].end = d->irq_base + NEP_IRQ_SA1111;
@@ -385,7 +426,7 @@ static int neponset_remove(struct platform_device *dev)
 		platform_device_unregister(d->smc91x);
 	irq_set_chained_handler(irq, NULL);
 	irq_free_descs(d->irq_base, NEP_IRQ_NR);
-	nep_base = NULL;
+	nep = NULL;
 	iounmap(d->base);
 	kfree(d);
 
@@ -393,30 +434,22 @@ static int neponset_remove(struct platform_device *dev)
 }
 
 #ifdef CONFIG_PM_SLEEP
-static int neponset_suspend(struct device *dev)
-{
-	struct neponset_drvdata *d = dev_get_drvdata(dev);
-
-	d->ncr0 = readb_relaxed(d->base + NCR_0);
-	d->mdm_ctl_0 = readb_relaxed(d->base + MDM_CTL_0);
-
-	return 0;
-}
-
 static int neponset_resume(struct device *dev)
 {
 	struct neponset_drvdata *d = dev_get_drvdata(dev);
+	int i, ret = 0;
 
-	writeb_relaxed(d->ncr0, d->base + NCR_0);
-	writeb_relaxed(d->mdm_ctl_0, d->base + MDM_CTL_0);
+	for (i = 0; i < ARRAY_SIZE(d->gpio); i++) {
+		ret = gpio_reg_resume(d->gpio[i]);
+		if (ret)
+			break;
+	}
 
-	return 0;
+	return ret;
 }
 
 static const struct dev_pm_ops neponset_pm_ops = {
-	.suspend_noirq = neponset_suspend,
 	.resume_noirq = neponset_resume,
-	.freeze_noirq = neponset_suspend,
 	.restore_noirq = neponset_resume,
 };
 #define PM_OPS &neponset_pm_ops
-- 
2.7.4

^ permalink raw reply related

* [RESEND PATCH] arm64: v8.4: Support for new floating point multiplication variant
From: Suzuki K Poulose @ 2017-12-11 18:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211132914.GJ22781@e103592.cambridge.arm.com>

Hi gengdongjiu

Sorry for the late response. I have a similar patch to add the support 
for "FHM", which I was about to post it this week.

On 11/12/17 13:29, Dave Martin wrote:
> On Mon, Dec 11, 2017 at 08:47:00PM +0800, gengdongjiu wrote:
>>
>> On 2017/12/11 19:59, Dave P Martin wrote:
>>> On Sat, Dec 09, 2017 at 03:28:42PM +0000, Dongjiu Geng wrote:
>>>> ARM v8.4 extensions include support for new floating point
>>>> multiplication variant instructions to the AArch64 SIMD
>>>
>>> Do we have any human-readable description of what the new instructions
>>> do?
>>>
>>> Since the v8.4 spec itself only describes these as "New Floating
>>> Point Multiplication Variant", I wonder what "FHM" actually stands
>>> for.
>> Thanks for the point out.
>> In fact, this feature only adds two instructions:
>> FP16 * FP16 + FP32
>> FP16 * FP16 - FP32
>>
>> The spec call this bit to ID_AA64ISAR0_EL1.FHM, I do not know why it
>> will call "FHM", I  think call it "FMLXL" may be better, which can
>> stand for FMLAL/FMLSL instructions.
> 
> Although "FHM" is cryptic, I think it makes sense to keep this as "FHM"
> to match the ISAR0 field name -- we've tended to follow this policy
> for other extension names unless there's a much better or more obvious
> name available.
> 
> For "FMLXL", new instructions might be added in the future that match
> the same pattern, and then "FMLXL" could become ambiguous.  So maybe
> this is not the best choice.

I think the FHM stands for "FP Half precision Multiplication 
instructions". I vote for keeping the feature bit in sync with the 
register bit definition. i.e, FHM.

However, my version of the patch names the HWCAP bit "asimdfml", 
following the compiler name for the feature option "fp16fml", which
is not perfect either. I think FHM is the safe option here.

> 
>>> Maybe something like "widening half-precision floating-point multiply
>>> accumulate" is acceptable wording consistent with the existing
>>> architecture, but I just made that up, so it's not official ;)
>>
>> how about something like "performing a multiplication of each FP16
>> element of one vector with the corresponding FP16 element of a second
>> vector, and to add or subtract this without an intermediate rounding
>> to the corresponding FP32 element in a third vector."?
> 
> We could have that, I guess.
> 

I agree, and that matches the feature description.

^ permalink raw reply

* [PATCH net-next 2/2 v8] net: ethernet: Add a driver for Gemini gigabit ethernet
From: David Miller @ 2017-12-11 19:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171210224558.27122-2-linus.walleij@linaro.org>

From: Linus Walleij <linus.walleij@linaro.org>
Date: Sun, 10 Dec 2017 23:45:58 +0100

> diff --git a/drivers/net/ethernet/cortina/Kconfig b/drivers/net/ethernet/cortina/Kconfig
> new file mode 100644
> index 000000000000..7d279ac4357d
> --- /dev/null
> +++ b/drivers/net/ethernet/cortina/Kconfig
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Cortina ethernet devices
> +
> +config NET_VENDOR_CORTINA
> +	bool "Cortina Gemini devices"
> +	depends on (ARM || COMPILE_TEST)
> +	default ARCH_GEMINI
> +	---help---
> +	  If you have a network (Ethernet) card belonging to this class, say Y
> +	  and read the Ethernet-HOWTO, available from
> +	  <http://www.tldp.org/docs.html#howto>.

Vendor Kconfig guards should default to 'y'.  Remove the depends guard.

> +
> +if NET_VENDOR_CORTINA
> +
> +config GEMINI_ETHERNET
> +	tristate "Gemini Gigabit Ethernet support"
> +	depends on ARCH_GEMINI
> +	depends on OF
> +	select PHYLIB
> +	select CRC32
> +	---help---
> +	  This driver supports StorLink SL351x (Gemini) dual Gigabit Ethernet.

Make this driver buildable anywhere, you don't use any platform architecture
specific features.

Otherwise your driver will have bad build coverage and will likely
stop building when core APIs are changed.

^ permalink raw reply

* [PATCH v5 15/30] arm64/sve: Signal handling support
From: Kees Cook @ 2017-12-11 19:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211140720.GE2141@arm.com>

On Mon, Dec 11, 2017 at 6:07 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Thu, Dec 07, 2017 at 10:50:38AM -0800, Kees Cook wrote:
>> My question is mainly: why not just use copy_*() everywhere instead?
>> Having these things so spread out makes it fragile, and there's very
>> little performance benefit from using __copy_*() over copy_*().
>
> I think that's more of a general question. Why not just remove the __
> versions from the kernel entirely if they're not worth the perf?

That has been something Linus has strongly suggested in the past, so
I've kind of been looking for easy places to drop the __copy_*
versions. :)

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply

* [kernel-hardening][PATCH] arm: hw_breakpoint: Mark variables as __ro_after_init
From: Kees Cook @ 2017-12-11 19:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211125043.GA17299@pjb1027-Latitude-E5410>

On Mon, Dec 11, 2017 at 4:50 AM, Jinbum Park <jinb.park7@gmail.com> wrote:
> core_num_brps, core_num_wrps, debug_arch, has_ossr,
> max_watchpoint_len are setup once while init stage,
> and never changed after that.
> so it is good candidate for __ro_after_init.
>
> Signed-off-by: Jinbum Park <jinb.park7@gmail.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

(Probably good to toss this into the ARM patch tracker.)

-Kees

> ---
>  arch/arm/kernel/hw_breakpoint.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
> index af2a7f1..629e251 100644
> --- a/arch/arm/kernel/hw_breakpoint.c
> +++ b/arch/arm/kernel/hw_breakpoint.c
> @@ -44,17 +44,17 @@
>  static DEFINE_PER_CPU(struct perf_event *, wp_on_reg[ARM_MAX_WRP]);
>
>  /* Number of BRP/WRP registers on this CPU. */
> -static int core_num_brps;
> -static int core_num_wrps;
> +static int core_num_brps __ro_after_init;
> +static int core_num_wrps __ro_after_init;
>
>  /* Debug architecture version. */
> -static u8 debug_arch;
> +static u8 debug_arch __ro_after_init;
>
>  /* Does debug architecture support OS Save and Restore? */
> -static bool has_ossr;
> +static bool has_ossr __ro_after_init;
>
>  /* Maximum supported watchpoint length. */
> -static u8 max_watchpoint_len;
> +static u8 max_watchpoint_len __ro_after_init;
>
>  #define READ_WB_REG_CASE(OP2, M, VAL)                  \
>         case ((OP2 << 4) + M):                          \
> --
> 1.9.1
>



-- 
Kees Cook
Pixel Security

^ permalink raw reply

* [RFC] KVM API extensions for SVE
From: Christoffer Dall @ 2017-12-11 19:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211145135.GK22781@e103592.cambridge.arm.com>

On Mon, Dec 11, 2017 at 02:51:36PM +0000, Dave Martin wrote:
> On Fri, Nov 24, 2017 at 03:45:38PM +0100, Christoffer Dall wrote:
> > On Thu, Nov 23, 2017 at 06:40:50PM +0000, Dave Martin wrote:
> > > On Wed, Nov 22, 2017 at 08:52:30PM +0100, Christoffer Dall wrote:
> > > > Hi Dave,
> > > > 
> > > > On Tue, Nov 21, 2017 at 01:49:16PM +0000, Dave Martin wrote:
> > > > > Hi all,
> > > > > 
> > > > > SVE adds some new registers, and their size depends on the hardware ando
> > > > > on runtime sysreg settings.
> > > > > 
> > > > > Before coding something, I'd like to get people's views on my current
> > > > > approach here.
> > > > > 
> > > > > --8<--
> > > > > 
> > > > > New vcpu feature flag:
> > > > > /*
> > > > >  * userspace can support regs up to at least 2048 bits in size via ioctl,
> > > > >  * and will honour the size field in the reg iD
> > > > >  */
> > > > > #define KVM_ARM_VCPU_LARGE_REGS	4
> > > > > 
> > > > > Should we just error out of userspace fails to set this on a system that
> > > > > supports SVE, or is that too brutal?  
> > > > 
> > > > That would break older userspace on newer kernels on certain hardware,
> > > > which would certainly not be acceptable.  Or did I misunderstand?
> > > 
> > > Yes, which is probably bad.
> > > 
> > > I'm still trying to gauge the policy regarding backwards compatibility.
> > > 
> > 
> > Think if QEMU as any other userspace application.  We should really
> > never regress userspace.
> > 
> > > 
> > > So, I guess you're saying is that we should disable SVE for the guest
> > > but still run it in this case.
> > > 
> > 
> > That's slightly more debatable, but doing it any other way would break
> > any form of migration that relies on the guest configuration of SVE and
> > userspace would have no way to know.  I think this sounds like a bad
> > idea.
> > 
> > > 
> > > This creates another issue: if SVE is supported by the host kernel
> > > but not enabled for the guest, do I need to hist the SVE regs from
> > > KVM_GET_REG_LIST?
> > 
> > I don't think so.  We should check with QEMU and kvmtool, but I think
> > the way it works is that userspace matches the KVM list with its own
> > internal list, and matches up anything it knows about, and discards the
> > rest.  Certainly in the past we haven't been afraid of adding registers
> > to KVM_GET_REG_LIST.
> > 
> > > 
> > > If not, a guest that doesn't have SVE created on a host that supports
> > > SVE would not be migratable to a host kernel that doesn't support SVE
> > > but otherwise could run the guest:  as I understand it, the attempt to
> > > restore the SVE regs on the target node would fail because the host
> > > kernel doesn't recognise those regs.
> > > 
> > > Or do we not claim to support backwards compatibility for migration?
> > > 
> > 
> > I think QEMU and higher level tools like libvirt would have the
> > knowledge to figure this out and implement what they want, so from the
> > kernel's point of view, I think we can simply export the registers when
> > SVE is supported.
> > 
> > > > 
> > > > > If we do treat that as an error,
> > > > > then we can unconditionally enable SVE for the guest when the host
> > > > > supports it -- which cuts down on unnecessary implementation complexity.
> > > > 
> > > > I think it makes sense to be able to disable SVE, especially if there's
> > > > high overhead related to context switching the state.  Since you say the
> > > > implementation complexity is unnecessary, I may be missing some larger
> > > > point?
> > > 
> > > I don't think it's all that bad, but there doesn't seem to be any
> > > example of an optional architecture feature for a guest today --
> > > so I wanted to check before setting a precedent here.
> > 
> > We don't enable the GIC unless userspace asks for it, same with the
> > PMU...
> > 
> > > 
> > > Would "SVE enabled" be better as an attribute, rather than a
> > > feature, or does it not really matter?
> > > 
> > 
> > Doesn't matter.  It's a question of what you need in terms of the ABI.
> > 
> > > > > 
> > > > > Alternatively, we would need logic to disable SVE if userspace is too
> > > > > old, i.e., doesn't set this flag.  Then we might need to enforce that
> > > > > the flag is set the same on every vcpu -- but from the kernel's PoV
> > > > > it probably doesn't matter.
> > > > 
> > > > Not sure I understand why it doesn't matter from the kernel's PoV.
> > > > 
> > > > I think SVE should be disabled by default (as it is now) and then we
> > > > should expose a capability (potentially simply via the vcpu attributes
> > > > being present), and let userspace enable SVE and set a vector length.
> > > 
> > > Yes, but aren't all the attributes/features per-vcpu?
> > > 
> > 
> > Yes, so the kernel should check that everything is configured
> > consistently across all VCPUs.
> > 
> > > > It makes sense that userspace needs to know about SVE for VMs to use it,
> > > > doesn't it?
> > > 
> > > Yes and no.  Except for debug purposes I don't see why userspace needs
> > > to know anything execpt how to handle large registers through the ioctl
> > > interface.
> > > 
> > 
> > Migration is another reason.
> > 
> > > > I assume SVE systems will have SVE on all CPUs in an SMP system, or am I
> > > > being way too optimistic about the ARM ecosystem here?  Just like we
> > > > don't model big.LITTLE, I think we should enforce in the kernel, that
> > > 
> > > The kernel follows the same policy: if SVE is not present on all
> > > physical CPUs we disable it completely and hide it from guests and
> > > userspace.
> > > 
> > > For vector length I'm a little more permissive: the max vector length
> > > would be clamped to the minimum commonly supported vector length.
> > > 
> > 
> > Ok, so KVM could implement the same.  Or we could just be reasonable and
> > require userspace to configure all VCPUs the same.
> > 
> > > > userspace configures all VCPUs with the same SVE properties.
> > > 
> > > OK, so long as you think it's not superfluous to do it, then I'm happy
> > > to do it.
> > > 
> > > > > 
> > > > > /*
> > > > >  * For the SVE regs, we add some new reg IDs.
> > > > >  * Zn are presented in 2048-bit slices; Pn, FFR are presented in 256-bit
> > > > >  * slices.  This is sufficient for only a single slice to be required
> > > > >  * per register for SVE, but ensures expansion room in case future arch
> > > > >  * versions grow the maximum size.
> > > > >  */
> > > > 
> > > > I don't understand the last part of this comment?
> > > 
> > > This may be explained better in by response below.
> > > 
> > > > > #define KVM_REG_SIZE_U2048 (ULL(8) << KVM_REG_SIZE_MASK)
> > > > 
> > > > Shift left by KVM_REG_SIZE_MASK?  I'm confused.
> > > > 
> > > > > #define KVM_REG_ARM64_SVE_Z(n, i) /* Zn[2048 * (i + 1) - 1 : 2048 * i] */ \
> > > > > 	((0x0014 << KVM_REG_ARM_COPROC_SHIFT) | KVM_REG_SIZE_U2048 |	\
> > > > > 		((n) << 5) | (i))
> > > > > #define KVM_REG_ARM64_SVE_P(n, i) /* Pn[256 * (i + 1) - 1 : 256 * i] */	\
> > > > > 	((0x0014 << KVM_REG_ARM_COPROC_SHIFT) | KVM_REG_SIZE_U256 |	\
> > > > > 		(((n) + 32) << 5) | (i))
> > > > > #define KVM_REG_ARM64_SVE_FFR(i) /* FFR[256 * (i + 1) - 1 : 256 * i] */	\
> > > > > 	 KVM_REG_ARM64_SVE_P(16, i)
> > > > > 
> > > > > For j in [0,3], KVM_REG_ARM64_SVE_Z(n, 0) bits [32(j + 1) - 1 : 32 * j]
> > > > > 	alias KVM_REG_ARM_CORE_REG(fp_regs.vregs[n]) + j
> > > > > 
> > > > 
> > > > This is hard to read and understand the way presented here.  I would
> > > > suggest you formulate this suggestion in the form of an RFC patch to
> > > > Documentation/virtual/kvm/api.txt plus the header definitions.
> > > 
> > > Sure, I hadn't figured out the best way to present this: I was thinking
> > > aloud.
> > > 
> > > > (I'm not sure where to look to look to decode the "<< 5" and the
> > > > " + 32) << 5)" stuff above.
> > > 
> > > The idea here is that we have 49 registers: Z0-Z31, P0-P15 and FFR.
> > > They are numbered serially.
> > > 
> > > However, the SVE architecture leaves the possibility of future
> > > expansion open, up to 32 times the current maximum.
> > > 
> > > The KVM reg API doesn't support such ridiculously huge registers,
> > > so my proposal is to slice them up, indexed by the value in the
> > > bottom 5 bits of the reg ID.  This requires the "register ID"
> > > field to be shifted up by 5 bits.
> > > 
> > > If the regs are not oversized (never, for the current architecture),
> > > then we simply don't list those extra slices via KVM_GET_REG_LIST.
> > > 
> > > > > Bits above the max vector length could be
> > > > 
> > > > Which bits are these and where are they, and why do we have them?
> > > 
> > > The KVM register size via ioctl is fixed at 2048 bits here.  Since
> > > the system might not support vectors that large, then bits 2047:1024
> > > in the ioctl payload wouldn't map to any register bits in the hardware.
> > > Should KVM still store them somewhere?  Should they logically exist
> > > for the purpose of the ioctl() API?
> > > 
> > > Making the size dynamic to avoid surplus bits doesn't work, because KVM
> > > only supports power-of-two reg sizes, whereas SVE can support non-power-
> > > of-two sizes.
> > > 
> > > > Is the max vector length the max architecturally (current version)
> > > > defined length, or what is chosen for this VM?
> > > 
> > > For now, that's an open question.
> > > 
> > > > >  * don't care (or not copied at all) on read; ignored on write
> > > > >  * zero on read; ignored on write
> > > > >  * zero on read; must be zero on write
> > > > > 
> > > > > Bits between the current and max vector length are trickier to specify:
> > > > > the "current" vector length for ioctl access is ill-defined, because
> > > > > we would need to specify ordering dependencies between Zn/Pn/FFR access
> > > > > and access to ZCR_EL1.
> > > > 
> > > > I think you want userspace to be able to read/write these values in any
> > > > order compared to configuring SVE for the VM, and then fix up whatever
> > > > needs masking etc. in the kernel later, if possible.  Ordering
> > > > requirements to userspace accesses have shown to be hard to enforce and
> > > > get right in the past.
> > > 
> > > So I've heard from other people.
> > > 
> > > > What happens on hardware if you give a certain vector length to EL0, EL0
> > > > writes a value of the full length, and then EL1 restricts the length to
> > > > something smaller, and subsequently expands it again?  Is the original
> > > > full value visible or are some bits potentially lost?  IOW, can't we
> > > > rely on what the architecture says here?
> > > 
> > > The architecture says that bits that are hidden and then revealed again
> > > are either preserved whilst hidden, or zeroed.
> > > 
> > > Opinion differs on whether that's a good thing to expose in ABI: Will
> > > considered it unacceptable to expose this kind of behaviour around
> > > syscalls from userspace for example, so I currently always zero the
> > > bits in that case even though it's slightly more expensive.
> > > 
> > > The concern here was that userspace might unintentionally rely on
> > > the affected register bits being preserved around syscalls when this
> > > is not guaranteed by the implementation.
> > > 
> > > This does not mean that similar design considerations apply to the KVM
> > > ioctl interface though.
> > > 
> > 
> > It sounds to me that the most simple thing is that the register
> > interface to userspace exposes the full possible register width in both
> > directions, and we apply a mask whenever we need to.
> > 
> > > > > 
> > > > > So, it may be simpler to expose the full maximum supported vector size
> > > > > unconditionally through ioctl, and pack/unpack as necessary.
> > > > 
> > > > yes, I think this was what I tried to say.
> > > > 
> > > > > 
> > > > > Currently, data is packed in the vcpu struct in a vector length
> > > > > dependent format, since this seems optimal for low-level save/restore,
> > > > > so there will be potential data loss / zero padding when converting.
> > > > > 
> > > > > This may cause some unexpected effects.  For example:
> > > > > 
> > > > > KVM_SET_ONE_REG(ZCR_EL1, 0) 
> > > > > /* Guest's current vector length will be 128 bits when started */
> > > > > 
> > > > > KVM_SET_ONE_REG(Z0, (uint256_t)1 << 128)
> > > > > KVM_GET_ONE_REG(Z0) /* yields (uint256_t)1 << 128 */
> > > > > 
> > > > > KVM_RUN /* reg data packed down to 128-bit in vcpu struct */
> > > > > 
> > > > > KVM_GET_ONE_REG(Z0) /* yields 0 even if guest doesn't use SVE */
> > > > > 
> > > > 
> > > > I really don't know how to parse this or what the point here is?  Sorry.
> > > 
> > > It means that for the ioctl interface, "obvious" guarantees like "if you
> > > read a register you get back the last value written" don't work quite as
> > > expected.  Some bits may have disappeared, or not, depending on the
> > > precise scenario.
> > > 
> > > > > 
> > > > > Since the guest should be treated mostly as a black box, I'm not sure
> > > > > how big a deal this is.  The guest _might_ have explicitly set those
> > > > > bits to 0... who are we to say?
> > > > 
> > > > How big a deal what is?  I'm lost.
> > > > 
> > > > > 
> > > > > Can anyone think of a scenario where effects like this would matter?
> > > > > 
> > > > 
> > > > I think we need more information.
> > > 
> > > See if my comments above throw any light on this.
> > > 
> > 
> > So you're saying even if we try the "expose full width and read back
> > hidden values" approach, those hidden values may be changed when
> > executing the guest, due to the KVM implementation or the way hardware
> > works, is that the point?
> 
> Basically yes.
> 
> > I think the KVM interface should be designed similarly to being able to
> > probe a hardware CPU's register state at various stages of execution.
> > 
> > So, for example, if you write content to hidden bits in the SVE
> > registers from EL2 on real hardware and limit the length using ZCR_EL2,
> > and then run a bunch of code in EL1/0, and then come back to EL2 and
> > examine the registers again, then we should model that behavior in
> > software.
> > 
> > In other words, I think we have to model this more closely to what
> > guarantees ZCR_EL2 gives us, and not ZCR_EL1, and choose something
> > architecturally compliant which is reasonable to implement.
> 
> So, we imagine that provided the vcpu is not run in the meantime,
> all accesses to SVE regs via the KVM reg API act like they are executed
> at EL2?

Yes, userspace probing virtual EL1 state should be like EL2 probing EL1
state on hardware.

> 
> That doesn't seem unreasonable, and it removes any ordering requirement
> between ZCR_EL1 and the SVE regs providing that the vcpu isn't set
> running in the meantime.  There is no userspace access to ZCR_EL2 at
> all, if we go with the model of configuring that via attributes that
> must be configured before vcpu startup -- in which case there is no
> ordering requirement there.
> 
> The extra bits beyond ZCR_EL1.LEN may disappear as soon as the vcpu
> is run, but that is architecturally consistent behaviour at least.
> 

Yes, I think we agree here.  It will all be interesting with nested
virtualization where we have to start exposing ZCR_EL2, but that's not
for today.

Thanks,
-Christoffer

^ permalink raw reply

* [PATCH 16/20] ARM: dts: aspeed: Add Witherspoon BMC machine
From: Brandon Wyman @ 2017-12-11 19:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211050704.20621-17-joel@jms.id.au>

On Sun, Dec 10, 2017 at 11:07 PM, Joel Stanley <joel@jms.id.au> wrote:
> The Witherspoon BMC is an ASPEED ast2500 based BMC that is part of an
> OpenPower Power9 server.
>
> This adds the device tree description for most upstream components. It
> is a squashed commit from the OpenBMC kernel tree.
>
> Signed-off-by: Brandon Wyman <bjwyman@gmail.com>
> Signed-off-by: Matt Spinler <spinler@us.ibm.com>
> Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> Signed-off-by: C?dric Le Goater <clg@kaod.org>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  arch/arm/boot/dts/Makefile                       |   4 +-
>  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 547 +++++++++++++++++++++++
>  2 files changed, 550 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 5d1e9d37bf3a..15a9207319c1 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1104,5 +1104,7 @@ dtb-$(CONFIG_ARCH_ZX) += zx296702-ad1.dtb
>  dtb-$(CONFIG_ARCH_ASPEED) += \
>         aspeed-ast2500-evb.dtb \
>         aspeed-bmc-opp-palmetto.dtb \
> -       aspeed-bmc-opp-romulus.dtb
> +       aspeed-bmc-opp-romulus.dtb \
> +       aspeed-bmc-opp-witherspoon.dtb
> +
>  endif
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> new file mode 100644
> index 000000000000..9a0937512e5b
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> @@ -0,0 +1,547 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +#include "aspeed-g5.dtsi"
> +#include <dt-bindings/leds/leds-pca955x.h>
> +
> +/ {
> +       model = "Witherspoon BMC";
> +       compatible = "ibm,witherspoon-bmc", "aspeed,ast2500";
> +
> +       chosen {
> +               stdout-path = &uart5;
> +               bootargs = "console=ttyS4,115200 earlyprintk";
> +       };
> +
> +       memory {
> +               reg = <0x80000000 0x20000000>;
> +       };
> +
> +       reserved-memory {
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               ranges;
> +
> +               flash_memory: region at 98000000 {
> +                       no-map;
> +                       reg = <0x98000000 0x04000000>; /* 64M */
> +               };
> +       };
> +
> +       gpio-keys-polled {
> +               compatible = "gpio-keys-polled";
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               poll-interval = <1000>;
> +
> +               fan0-presence {
> +                       label = "fan0-presence";
> +                       gpios = <&pca0 4 GPIO_ACTIVE_LOW>;
> +                       linux,code = <4>;
> +               };
> +
> +               fan1-presence {
> +                       label = "fan1-presence";
> +                       gpios = <&pca0 5 GPIO_ACTIVE_LOW>;
> +                       linux,code = <5>;
> +               };
> +
> +               fan2-presence {
> +                       label = "fan2-presence";
> +                       gpios = <&pca0 6 GPIO_ACTIVE_LOW>;
> +                       linux,code = <6>;
> +               };
> +
> +               fan3-presence {
> +                       label = "fan3-presence";
> +                       gpios = <&pca0 7 GPIO_ACTIVE_LOW>;
> +                       linux,code = <7>;
> +               };
> +       };
> +
> +       leds {
> +               compatible = "gpio-leds";
> +
> +               fan0 {
> +                       retain-state-shutdown;
> +                       default-state = "keep";
> +                       gpios = <&pca0 0 GPIO_ACTIVE_LOW>;
> +               };
> +
> +               fan1 {
> +                       retain-state-shutdown;
> +                       default-state = "keep";
> +                       gpios = <&pca0 1 GPIO_ACTIVE_LOW>;
> +               };
> +
> +               fan2 {
> +                       retain-state-shutdown;
> +                       default-state = "keep";
> +                       gpios = <&pca0 2 GPIO_ACTIVE_LOW>;
> +               };
> +
> +               fan3 {
> +                       retain-state-shutdown;
> +                       default-state = "keep";
> +                       gpios = <&pca0 3 GPIO_ACTIVE_LOW>;
> +               };
> +
> +               front-fault {
> +                       retain-state-shutdown;
> +                       default-state = "keep";
> +                       gpios = <&pca0 13 GPIO_ACTIVE_LOW>;
> +               };
> +
> +               front-power {
> +                       retain-state-shutdown;
> +                       default-state = "keep";
> +                       gpios = <&pca0 14 GPIO_ACTIVE_LOW>;
> +               };
> +
> +               front-id {
> +                       retain-state-shutdown;
> +                       default-state = "keep";
> +                       gpios = <&pca0 15 GPIO_ACTIVE_LOW>;
> +               };
> +
> +               rear-fault {
> +                       gpios = <&gpio ASPEED_GPIO(N, 2) GPIO_ACTIVE_LOW>;
> +               };
> +
> +               rear-id {
> +                       gpios = <&gpio ASPEED_GPIO(N, 4) GPIO_ACTIVE_LOW>;
> +               };
> +
> +               rear-power {
> +                       gpios = <&gpio ASPEED_GPIO(N, 3) GPIO_ACTIVE_LOW>;
> +               };
> +
> +               power-button {
> +                       gpios = <&gpio ASPEED_GPIO(R, 5) GPIO_ACTIVE_LOW>;
> +               };
> +       };
> +
> +       fsi: gpio-fsi {
> +               compatible = "fsi-master-gpio", "fsi-master";
> +               #address-cells = <2>;
> +               #size-cells = <0>;
> +
> +               clock-gpios = <&gpio ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
> +               data-gpios = <&gpio ASPEED_GPIO(E, 0) GPIO_ACTIVE_HIGH>;
> +               mux-gpios = <&gpio ASPEED_GPIO(A, 6) GPIO_ACTIVE_HIGH>;
> +               enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
> +               trans-gpios = <&gpio ASPEED_GPIO(R, 2) GPIO_ACTIVE_HIGH>;
> +       };
> +
> +       iio-hwmon-dps310 {
> +               compatible = "iio-hwmon";
> +               io-channels = <&dps 0>;
> +       };
> +
> +       iio-hwmon-bmp280 {
> +               compatible = "iio-hwmon";
> +               io-channels = <&bmp 1>;
> +       };
> +
> +};
> +
> +&fmc {
> +       status = "okay";
> +
> +       flash at 0 {
> +               status = "okay";
> +               label = "bmc";
> +               m25p,fast-read;
> +#include "openbmc-flash-layout.dtsi"
> +       };
> +
> +       flash at 1 {
> +               status = "okay";
> +               label = "alt";
> +               m25p,fast-read;
> +       };
> +};
> +
> +&spi1 {
> +       status = "okay";
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_spi1_default>;
> +
> +       flash at 0 {
> +               status = "okay";
> +               label = "pnor";
> +               m25p,fast-read;
> +       };
> +};
> +
> +&uart1 {
> +       /* Rear RS-232 connector */
> +       status = "okay";
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_txd1_default
> +                       &pinctrl_rxd1_default
> +                       &pinctrl_nrts1_default
> +                       &pinctrl_ndtr1_default
> +                       &pinctrl_ndsr1_default
> +                       &pinctrl_ncts1_default
> +                       &pinctrl_ndcd1_default
> +                       &pinctrl_nri1_default>;
> +};
> +
> +&uart2 {
> +       /* APSS */
> +       status = "okay";
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_txd2_default &pinctrl_rxd2_default>;
> +};
> +
> +&uart5 {
> +       status = "okay";
> +};
> +
> +&lpc_ctrl {
> +       status = "okay";
> +       memory-region = <&flash_memory>;
> +       flash = <&spi1>;
> +};
> +
> +&mac0 {
> +       status = "okay";
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_rmii1_default>;
> +       use-ncsi;
> +};
> +
> +&i2c2 {
> +       status = "okay";
> +
> +       /* MUX ->
> +        *    Samtec 1
> +        *    Samtec 2
> +        */
> +};
> +
> +&i2c3 {
> +       status = "okay";
> +
> +       bmp: bmp280 at 77 {
> +               compatible = "bosch,bmp280";
> +               reg = <0x77>;
> +               #io-channel-cells = <1>;
> +       };
> +
> +       max31785 at 52 {
> +               compatible = "maxim,max31785a";
> +               reg = <0x52>;
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +       };
> +
> +       dps: dps310 at 76 {
> +               compatible = "infineon,dps310";
> +               reg = <0x76>;
> +               #io-channel-cells = <0>;
> +       };
> +
> +       pca0: pca9552 at 60 {
> +               compatible = "nxp,pca9552";
> +               reg = <0x60>;
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               gpio-controller;
> +               #gpio-cells = <2>;
> +
> +               gpio at 0 {
> +                       reg = <0>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 1 {
> +                       reg = <1>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 2 {
> +                       reg = <2>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 3 {
> +                       reg = <3>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 4 {
> +                       reg = <4>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 5 {
> +                       reg = <5>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 6 {
> +                       reg = <6>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 7 {
> +                       reg = <7>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 8 {
> +                       reg = <8>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 9 {
> +                       reg = <9>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 10 {
> +                       reg = <10>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 11 {
> +                       reg = <11>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 12 {
> +                       reg = <12>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 13 {
> +                       reg = <13>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 14 {
> +                       reg = <14>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 15 {
> +                       reg = <15>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +       };
> +
> +       power-supply at 68 {
> +               compatible = "ibm,cffps1";
> +               reg = <0x68>;
> +       };
> +
> +       power-supply at 69 {
> +               compatible = "ibm,cffps1";
> +               reg = <0x69>;
> +       };
> +};
> +
> +&i2c4 {
> +       status = "okay";
> +
> +       tmp423a at 4c {
> +               compatible = "ti,tmp423";
> +               reg = <0x4c>;
> +       };
> +
> +       ir35221 at 70 {
> +               compatible = "infineon,ir35221";
> +               reg = <0x70>;
> +       };
> +
> +       ir35221 at 71 {
> +               compatible = "infineon,ir35221";
> +               reg = <0x71>;
> +       };
> +};
> +
> +
> +&i2c5 {
> +       status = "okay";
> +
> +       tmp423a at 4c {
> +               compatible = "ti,tmp423";
> +               reg = <0x4c>;
> +       };
> +
> +       ir35221 at 70 {
> +               compatible = "infineon,ir35221";
> +               reg = <0x70>;
> +       };
> +
> +       ir35221 at 71 {
> +               compatible = "infineon,ir35221";
> +               reg = <0x71>;
> +       };
> +};
> +
> +&i2c9 {
> +       status = "okay";
> +
> +       tmp275 at 4a {
> +               compatible = "ti,tmp275";
> +               reg = <0x4a>;
> +       };
> +};
> +
> +&i2c10 {
> +       /* MUX
> +        *   -> PCIe Slot 3
> +        *   -> PCIe Slot 4
> +        */
> +       status = "okay";
> +};
> +
> +&i2c11 {
> +       status = "okay";
> +
> +       pca9552: pca9552 at 60 {
> +               compatible = "nxp,pca9552";
> +               reg = <0x60>;
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               gpio-controller;
> +               #gpio-cells = <2>;
> +
> +               gpio-line-names = "PS_SMBUS_RESET_N", "APSS_RESET_N",
> +                       "GPU0_TH_OVERT_N_BUFF", "GPU1_TH_OVERT_N_BUFF",
> +                       "GPU2_TH_OVERT_N_BUFF", "GPU3_TH_OVERT_N_BUFF",
> +                       "GPU4_TH_OVERT_N_BUFF", "GPU5_TH_OVERT_N_BUFF",
> +                       "GPU0_PWR_GOOD_BUFF", "GPU1_PWR_GOOD_BUFF",
> +                       "GPU2_PWR_GOOD_BUFF", "GPU3_PWR_GOOD_BUFF",
> +                       "GPU4_PWR_GOOD_BUFF", "GPU5_PWR_GOOD_BUFF",
> +                       "12V_BREAKER_FLT_N", "THROTTLE_UNLATCHED_N";
> +
> +               gpio at 0 {
> +                       reg = <0>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 1 {
> +                       reg = <1>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 2 {
> +                       reg = <2>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 3 {
> +                       reg = <3>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 4 {
> +                       reg = <4>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 5 {
> +                       reg = <5>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 6 {
> +                       reg = <6>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 7 {
> +                       reg = <7>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 8 {
> +                       reg = <8>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 9 {
> +                       reg = <9>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 10 {
> +                       reg = <10>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 11 {
> +                       reg = <11>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 12 {
> +                       reg = <12>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 13 {
> +                       reg = <13>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 14 {
> +                       reg = <14>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio at 15 {
> +                       reg = <15>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +       };
> +
> +       rtc at 32 {
> +               compatible = "epson,rx8900";
> +               reg = <0x32>;
> +       };
> +
> +       eeprom at 51 {
> +               compatible = "atmel,24c64";
> +               reg = <0x51>;
> +       };
> +
> +       ucd90160 at 64 {
> +               compatible = "ti,ucd90160";
> +               reg = <0x64>;
> +       };
> +};
> +
> +&i2c12 {
> +       status = "okay";
> +};
> +
> +&i2c13 {
> +       status = "okay";
> +};
> +
> +&vuart {
> +       status = "okay";
> +};
> +
> +&gfx {
> +       status = "okay";
> +};
> +
> +&pinctrl {
> +       aspeed,external-nodes = <&gfx &lhc>;
> +};
> +
> +&wdt1 {
> +       aspeed,reset-type = "none";
> +       aspeed,external-signal;
> +       aspeed,ext-push-pull;
> +       aspeed,ext-active-high;
> +
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_wdtrst1_default>;
> +};
> --
> 2.14.1
>
Reviewed-by: Brandon Wyman <bjwyman@gmail.com>

^ permalink raw reply

* [PATCH 0/4] Sunxi: Add SMP support on A83T
From: Corentin Labbe @ 2017-12-11 19:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211075001.6100-1-mylene.josserand@free-electrons.com>

On Mon, Dec 11, 2017 at 08:49:57AM +0100, Myl?ne Josserand wrote:
> Hello everyone,
> 
> This series adds SMP support for Allwinner Sun8i-a83t
> with MCPM (Multi-Cluster Power Management).
> Series information:
> 	- Based on last linux-next (next-20171211)
> 	- Had dependencies on Chen Yu's patch that add MCPM
> 	support:
> 	https://patchwork.kernel.org/patch/6402801/
> 
> Patch 01: Convert the mcpm driver (initially for A80) to be able
> to use it for A83T. This SoC has a bit flip that needs to be handled.
> Patch 02: Add registers nodes (prcm, cpucfg and r_cpucfg) needed
> for MCPM.
> Patch 03: Add CCI-400 node for a83t.
> Patch 04: Fix the use of virtual timers that hangs the kernel in
> case of SMP support.
> 
> If you have any remarks/questions, let me know.
> Thank you in advance,
> Myl?ne
> 

Hello

As we discussed in private, Chen Yu's patch should be added in your series.

Furthermore, MCPM is not automaticaly selected via imply.

With all patchs I hit a bug:
[    0.898668] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:238
[    0.911162] in_atomic(): 1, irqs_disabled(): 0, pid: 1, name: swapper/0
[    0.917776] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.15.0-rc2-next-20171211+ #73
[    0.925418] Hardware name: Allwinner sun8i Family
[    0.930118] Backtrace: 
[    0.932596] [<c010cc50>] (dump_backtrace) from [<c010cf0c>] (show_stack+0x18/0x1c)
[    0.940158]  r7:c0b261e4 r6:60000013 r5:00000000 r4:c0b51958
[    0.945820] [<c010cef4>] (show_stack) from [<c06baccc>] (dump_stack+0x8c/0xa0)
[    0.953045] [<c06bac40>] (dump_stack) from [<c0149d40>] (___might_sleep+0x150/0x170)
[    0.960779]  r7:c0b261e4 r6:00000000 r5:000000ee r4:ee844000
[    0.966437] [<c0149bf0>] (___might_sleep) from [<c0149dc8>] (__might_sleep+0x68/0xa0)
[    0.974253]  r4:c0861690
[    0.976796] [<c0149d60>] (__might_sleep) from [<c06d2918>] (mutex_lock+0x24/0x68)
[    0.984269]  r6:c0892f6c r5:ffffffff r4:c0b1bb24
[    0.988891] [<c06d28f4>] (mutex_lock) from [<c01ccb6c>] (perf_pmu_register+0x24/0x3e4)
[    0.996795]  r5:ffffffff r4:ee98b014
[    1.000375] [<c01ccb48>] (perf_pmu_register) from [<c03efabc>] (cci_pmu_probe+0x340/0x484)
[    1.008631]  r10:c0892f6c r9:c0bfd5f0 r8:eea19010 r7:c0b261e4 r6:c0b26240 r5:eea19000
[    1.016447]  r4:ee98b010
[    1.018989] [<c03ef77c>] (cci_pmu_probe) from [<c045e21c>] (platform_drv_probe+0x58/0xb8)
[    1.027158]  r10:00000000 r9:c0b2610c r8:00000000 r7:fffffdfb r6:c0b2610c r5:ffffffed
[    1.034974]  r4:eea19010
[    1.037511] [<c045e1c4>] (platform_drv_probe) from [<c045c984>] (driver_probe_device+0x254/0x330)
[    1.046371]  r7:00000000 r6:c0bff498 r5:c0bff494 r4:eea19010
[    1.052026] [<c045c730>] (driver_probe_device) from [<c045cbc4>] (__device_attach_driver+0xa0/0xd4)
[    1.061062]  r10:00000000 r9:c0bff470 r8:00000000 r7:00000001 r6:eea19010 r5:ee845ac0
[    1.068879]  r4:c0b2610c r3:00000000
[    1.072454] [<c045cb24>] (__device_attach_driver) from [<c045ad68>] (bus_for_each_drv+0x68/0x9c)
[    1.081228]  r7:00000001 r6:c045cb24 r5:ee845ac0 r4:00000000
[    1.086883] [<c045ad00>] (bus_for_each_drv) from [<c045c60c>] (__device_attach+0xb8/0x11c)
[    1.095135]  r6:c0b3e848 r5:eea19044 r4:eea19010
[    1.099750] [<c045c554>] (__device_attach) from [<c045cc44>] (device_initial_probe+0x14/0x18)
[    1.108263]  r7:c0b0a4c8 r6:c0b3e848 r5:eea19010 r4:eea19018
[    1.113919] [<c045cc30>] (device_initial_probe) from [<c045bb58>] (bus_probe_device+0x8c/0x94)
[    1.122523] [<c045bacc>] (bus_probe_device) from [<c0459db8>] (device_add+0x40c/0x5a0)
[    1.130429]  r7:c0b0a4c8 r6:eea19010 r5:eea18a10 r4:eea19018
[    1.136089] [<c04599ac>] (device_add) from [<c0582a58>] (of_device_add+0x3c/0x44)
[    1.143564]  r10:00000000 r9:00000000 r8:00000000 r7:eedf21a4 r6:eea18a10 r5:00000000
[    1.151380]  r4:eea19000
[    1.153915] [<c0582a1c>] (of_device_add) from [<c0582f80>] (of_platform_device_create_pdata+0x7c/0xac)
[    1.163210] [<c0582f04>] (of_platform_device_create_pdata) from [<c0583100>] (of_platform_bus_create+0xf4/0x1f0)
[    1.173372]  r9:00000000 r8:00000000 r7:00000001 r6:00000000 r5:eedf2154 r4:00000000
[    1.181107] [<c058300c>] (of_platform_bus_create) from [<c0583374>] (of_platform_populate+0x74/0xd4)
[    1.190229]  r10:00000001 r9:eea18a10 r8:00000000 r7:00000000 r6:00000000 r5:eedf1d04
[    1.198045]  r4:eedf2154
[    1.200580] [<c0583300>] (of_platform_populate) from [<c03ef2a8>] (cci_platform_probe+0x3c/0x54)
[    1.209356]  r10:00000000 r9:c0b26168 r8:00000000 r7:fffffdfb r6:c0b26168 r5:ffffffed
[    1.217172]  r4:eea18a00
[    1.219708] [<c03ef26c>] (cci_platform_probe) from [<c045e21c>] (platform_drv_probe+0x58/0xb8)
[    1.228306]  r5:ffffffed r4:eea18a10
[    1.231881] [<c045e1c4>] (platform_drv_probe) from [<c045c984>] (driver_probe_device+0x254/0x330)
[    1.240742]  r7:00000000 r6:c0bff498 r5:c0bff494 r4:eea18a10
[    1.246397] [<c045c730>] (driver_probe_device) from [<c045cbc4>] (__device_attach_driver+0xa0/0xd4)
[    1.255433]  r10:00000000 r9:c0bff470 r8:00000000 r7:00000001 r6:eea18a10 r5:ee845ce8
[    1.263250]  r4:c0b26168 r3:00000000
[    1.266825] [<c045cb24>] (__device_attach_driver) from [<c045ad68>] (bus_for_each_drv+0x68/0x9c)
[    1.275598]  r7:00000001 r6:c045cb24 r5:ee845ce8 r4:00000000
[    1.281253] [<c045ad00>] (bus_for_each_drv) from [<c045c60c>] (__device_attach+0xb8/0x11c)
[    1.289506]  r6:c0b3e848 r5:eea18a44 r4:eea18a10
[    1.294120] [<c045c554>] (__device_attach) from [<c045cc44>] (device_initial_probe+0x14/0x18)
[    1.302633]  r7:c0b0a4c8 r6:c0b3e848 r5:eea18a10 r4:eea18a18
[    1.308288] [<c045cc30>] (device_initial_probe) from [<c045bb58>] (bus_probe_device+0x8c/0x94)
[    1.316890] [<c045bacc>] (bus_probe_device) from [<c0459db8>] (device_add+0x40c/0x5a0)
[    1.324796]  r7:c0b0a4c8 r6:eea18a10 r5:ee993810 r4:eea18a18
[    1.330450] [<c04599ac>] (device_add) from [<c0582a58>] (of_device_add+0x3c/0x44)
[    1.337926]  r10:00000000 r9:c07759d8 r8:00000000 r7:eedf1d54 r6:ee993810 r5:00000000
[    1.345743]  r4:eea18a00
[    1.348277] [<c0582a1c>] (of_device_add) from [<c0582f80>] (of_platform_device_create_pdata+0x7c/0xac)
[    1.357572] [<c0582f04>] (of_platform_device_create_pdata) from [<c0583100>] (of_platform_bus_create+0xf4/0x1f0)
[    1.367734]  r9:c07759d8 r8:00000000 r7:00000001 r6:00000000 r5:eedf1d04 r4:00000000
[    1.375469] [<c058300c>] (of_platform_bus_create) from [<c058315c>] (of_platform_bus_create+0x150/0x1f0)
[    1.384938]  r10:ee993810 r9:c07759d8 r8:00000000 r7:00000001 r6:00000000 r5:eedefe1c
[    1.392754]  r4:eedf1d04
[    1.395289] [<c058300c>] (of_platform_bus_create) from [<c0583374>] (of_platform_populate+0x74/0xd4)
[    1.404411]  r10:00000001 r9:00000000 r8:00000000 r7:c07759d8 r6:00000000 r5:eedee844
[    1.412228]  r4:eedefe1c
[    1.414769] [<c0583300>] (of_platform_populate) from [<c0a25ee8>] (of_platform_default_populate_init+0x80/0x94)
[    1.424844]  r10:c0a37848 r9:00000000 r8:c0b59680 r7:c0a37834 r6:ffffe000 r5:c0775ce8
[    1.432661]  r4:00000000
[    1.435200] [<c0a25e68>] (of_platform_default_populate_init) from [<c0102794>] (do_one_initcall+0x5c/0x194)
[    1.444925]  r5:c0a25e68 r4:c0b0a4c8
[    1.448506] [<c0102738>] (do_one_initcall) from [<c0a00f88>] (kernel_init_freeable+0x1d4/0x268)
[    1.457195]  r9:00000004 r8:c0b59680 r7:c0a37834 r6:c0b59680 r5:c0a47308 r4:c090cfb8
[    1.464932] [<c0a00db4>] (kernel_init_freeable) from [<c06cf3b0>] (kernel_init+0x10/0x118)
[    1.473187]  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c06cf3a0
[    1.481004]  r4:00000000
[    1.483540] [<c06cf3a0>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
[    1.491098] Exception stack(0xee845fb0 to 0xee845ff8)
[    1.496146] 5fa0:                                     00000000 00000000 00000000 00000000
[    1.504313] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    1.512480] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    1.519084]  r5:c06cf3a0 r4:00000000
[    1.522737] ARM CCI_400_r1 PMU driver probed

And only CPU 0 show up.

Regards

^ permalink raw reply

* [PATCH 0/3] irqchip: irq-bcm2836: add support for DT interrupt polarity
From: Stefan Wahren @ 2017-12-11 20:39 UTC (permalink / raw)
  To: linux-arm-kernel

This patch series implements DT polarity support for the 1st level interrupt
controller.

Stefan Wahren (3):
  dt-bindings: bcm2836-l1-intc: add interrupt polarity support
  irqchip: irq-bcm2836: add support for DT interrupt polarity
  ARM: dts: bcm283x: Define polarity of per-cpu interrupts

 .../interrupt-controller/brcm,bcm2836-l1-intc.txt  |  4 +-
 arch/arm/boot/dts/bcm2836.dtsi                     | 14 +++----
 arch/arm/boot/dts/bcm2837.dtsi                     | 12 +++---
 arch/arm/boot/dts/bcm283x.dtsi                     |  1 +
 drivers/irqchip/irq-bcm2836.c                      | 46 +++++++++++++---------
 5 files changed, 44 insertions(+), 33 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH 1/3] dt-bindings: bcm2836-l1-intc: add interrupt polarity support
From: Stefan Wahren @ 2017-12-11 20:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1513024752-11246-1-git-send-email-stefan.wahren@i2se.com>

This increases the interrupt cells for the 1st level interrupt controller
binding in order to describe the polarity like on the other ARM platforms.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt
index f320dcd..8ced169 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt
@@ -12,7 +12,7 @@ Required properties:
 			  registers
 - interrupt-controller:	Identifies the node as an interrupt controller
 - #interrupt-cells:	Specifies the number of cells needed to encode an
-			  interrupt source. The value shall be 1
+			  interrupt source. The value shall be 2
 
 Please refer to interrupts.txt in this directory for details of the common
 Interrupt Controllers bindings used by client devices.
@@ -32,6 +32,6 @@ local_intc: local_intc {
 	compatible = "brcm,bcm2836-l1-intc";
 	reg = <0x40000000 0x100>;
 	interrupt-controller;
-	#interrupt-cells = <1>;
+	#interrupt-cells = <2>;
 	interrupt-parent = <&local_intc>;
 };
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/3] irqchip: irq-bcm2836: add support for DT interrupt polarity
From: Stefan Wahren @ 2017-12-11 20:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1513024752-11246-1-git-send-email-stefan.wahren@i2se.com>

In order to properly define the polarity of the per-cpu interrupts,
we need to support for a second property cell. But this must be
optional to keep backward compatibility with old DT blobs.

Suggested-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/irqchip/irq-bcm2836.c | 46 ++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
index 667b9e1..dfe4a46 100644
--- a/drivers/irqchip/irq-bcm2836.c
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -98,13 +98,35 @@ static struct irq_chip bcm2836_arm_irqchip_gpu = {
 	.irq_unmask	= bcm2836_arm_irqchip_unmask_gpu_irq,
 };
 
-static void bcm2836_arm_irqchip_register_irq(int hwirq, struct irq_chip *chip)
-{
-	int irq = irq_create_mapping(intc.domain, hwirq);
+static int bcm2836_map(struct irq_domain *d, unsigned int irq,
+		       irq_hw_number_t hw)
+{
+	struct irq_chip *chip;
+
+	switch (hw) {
+	case LOCAL_IRQ_CNTPSIRQ:
+	case LOCAL_IRQ_CNTPNSIRQ:
+	case LOCAL_IRQ_CNTHPIRQ:
+	case LOCAL_IRQ_CNTVIRQ:
+		chip = &bcm2836_arm_irqchip_timer;
+		break;
+	case LOCAL_IRQ_GPU_FAST:
+		chip = &bcm2836_arm_irqchip_gpu;
+		break;
+	case LOCAL_IRQ_PMU_FAST:
+		chip = &bcm2836_arm_irqchip_pmu;
+		break;
+	default:
+		pr_warn_once("Unexpected hw irq: %lu\n", hw);
+		return -EINVAL;
+	}
 
 	irq_set_percpu_devid(irq);
-	irq_set_chip_and_handler(irq, chip, handle_percpu_devid_irq);
+	irq_domain_set_info(d, irq, hw, chip, d->host_data,
+			    handle_percpu_devid_irq, NULL, NULL);
 	irq_set_status_flags(irq, IRQ_NOAUTOEN);
+
+	return 0;
 }
 
 static void
@@ -165,7 +187,8 @@ static int bcm2836_cpu_dying(unsigned int cpu)
 #endif
 
 static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
-	.xlate = irq_domain_xlate_onecell
+	.xlate = irq_domain_xlate_onetwocell,
+	.map = bcm2836_map,
 };
 
 static void
@@ -218,19 +241,6 @@ static int __init bcm2836_arm_irqchip_l1_intc_of_init(struct device_node *node,
 	if (!intc.domain)
 		panic("%pOF: unable to create IRQ domain\n", node);
 
-	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_CNTPSIRQ,
-					 &bcm2836_arm_irqchip_timer);
-	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_CNTPNSIRQ,
-					 &bcm2836_arm_irqchip_timer);
-	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_CNTHPIRQ,
-					 &bcm2836_arm_irqchip_timer);
-	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_CNTVIRQ,
-					 &bcm2836_arm_irqchip_timer);
-	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_GPU_FAST,
-					 &bcm2836_arm_irqchip_gpu);
-	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_PMU_FAST,
-					 &bcm2836_arm_irqchip_pmu);
-
 	bcm2836_arm_irqchip_smp_init();
 
 	set_handle_irq(bcm2836_arm_irqchip_handle_irq);
-- 
2.7.4

^ permalink raw reply related

* [PATCH 3/3] ARM: dts: bcm283x: Define polarity of per-cpu interrupts
From: Stefan Wahren @ 2017-12-11 20:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1513024752-11246-1-git-send-email-stefan.wahren@i2se.com>

This patch define the polarity of the per-cpu interrupts on BCM2836
and BCM2837 in order to avoid the warnings from ARM arch timer code:

    arch_timer: WARNING: Invalid trigger for IRQ19, assuming level low
    arch_timer: WARNING: Please fix your firmware
    arch_timer: cp15 timer(s) running at 19.20MHz (virt).

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 arch/arm/boot/dts/bcm2836.dtsi | 14 +++++++-------
 arch/arm/boot/dts/bcm2837.dtsi | 12 ++++++------
 arch/arm/boot/dts/bcm283x.dtsi |  1 +
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/arch/arm/boot/dts/bcm2836.dtsi b/arch/arm/boot/dts/bcm2836.dtsi
index 61e1580..1dfd764 100644
--- a/arch/arm/boot/dts/bcm2836.dtsi
+++ b/arch/arm/boot/dts/bcm2836.dtsi
@@ -13,24 +13,24 @@
 			compatible = "brcm,bcm2836-l1-intc";
 			reg = <0x40000000 0x100>;
 			interrupt-controller;
-			#interrupt-cells = <1>;
+			#interrupt-cells = <2>;
 			interrupt-parent = <&local_intc>;
 		};
 
 		arm-pmu {
 			compatible = "arm,cortex-a7-pmu";
 			interrupt-parent = <&local_intc>;
-			interrupts = <9>;
+			interrupts = <9 IRQ_TYPE_LEVEL_HIGH>;
 		};
 	};
 
 	timer {
 		compatible = "arm,armv7-timer";
 		interrupt-parent = <&local_intc>;
-		interrupts = <0>, // PHYS_SECURE_PPI
-			     <1>, // PHYS_NONSECURE_PPI
-			     <3>, // VIRT_PPI
-			     <2>; // HYP_PPI
+		interrupts = <0 IRQ_TYPE_LEVEL_HIGH>, // PHYS_SECURE_PPI
+			     <1 IRQ_TYPE_LEVEL_HIGH>, // PHYS_NONSECURE_PPI
+			     <3 IRQ_TYPE_LEVEL_HIGH>, // VIRT_PPI
+			     <2 IRQ_TYPE_LEVEL_HIGH>; // HYP_PPI
 		always-on;
 	};
 
@@ -76,7 +76,7 @@
 	compatible = "brcm,bcm2836-armctrl-ic";
 	reg = <0x7e00b200 0x200>;
 	interrupt-parent = <&local_intc>;
-	interrupts = <8>;
+	interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
 };
 
 &cpu_thermal {
diff --git a/arch/arm/boot/dts/bcm2837.dtsi b/arch/arm/boot/dts/bcm2837.dtsi
index bc1cca5..efa7d33 100644
--- a/arch/arm/boot/dts/bcm2837.dtsi
+++ b/arch/arm/boot/dts/bcm2837.dtsi
@@ -12,7 +12,7 @@
 			compatible = "brcm,bcm2836-l1-intc";
 			reg = <0x40000000 0x100>;
 			interrupt-controller;
-			#interrupt-cells = <1>;
+			#interrupt-cells = <2>;
 			interrupt-parent = <&local_intc>;
 		};
 	};
@@ -20,10 +20,10 @@
 	timer {
 		compatible = "arm,armv7-timer";
 		interrupt-parent = <&local_intc>;
-		interrupts = <0>, // PHYS_SECURE_PPI
-			     <1>, // PHYS_NONSECURE_PPI
-			     <3>, // VIRT_PPI
-			     <2>; // HYP_PPI
+		interrupts = <0 IRQ_TYPE_LEVEL_HIGH>, // PHYS_SECURE_PPI
+			     <1 IRQ_TYPE_LEVEL_HIGH>, // PHYS_NONSECURE_PPI
+			     <3 IRQ_TYPE_LEVEL_HIGH>, // VIRT_PPI
+			     <2 IRQ_TYPE_LEVEL_HIGH>; // HYP_PPI
 		always-on;
 	};
 
@@ -73,7 +73,7 @@
 	compatible = "brcm,bcm2836-armctrl-ic";
 	reg = <0x7e00b200 0x200>;
 	interrupt-parent = <&local_intc>;
-	interrupts = <8>;
+	interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
 };
 
 &cpu_thermal {
diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index e08203c..0d43bd4 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -2,6 +2,7 @@
 #include <dt-bindings/clock/bcm2835.h>
 #include <dt-bindings/clock/bcm2835-aux.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
 
 /* firmware-provided startup stubs live here, where the secondary CPUs are
  * spinning.
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 0/8] Handle forwarded level-triggered interrupts
From: Auger Eric @ 2017-12-11 20:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171207105418.22428-1-christoffer.dall@linaro.org>

Hi Christoffer,

On 07/12/17 11:54, Christoffer Dall wrote:
> This series is an alternative approach to Eric Auger's direct EOI setup
> patches [1] in terms of the KVM VGIC support.
> 
> The idea is to maintain existing semantics for the VGIC for mapped
> level-triggered IRQs and also support the timer using mapped IRQs with
> the same VGIC support as VFIO interrupts.
> 
> Based on v4.15-rc1.
> 
> Also available at:
> git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git level-mapped-v7
> 
> Changes since v6:
>  - Removed double semi-colon
>  - Changed another confusing conditional in patch 6
>  - Fixed typos in commit message and comments
> 
> Changes since v5:
>  - Rebased on v4.15-rc1
>  - Changed comment on preemption code as suggested by Andre
>  - Fixed white space and confusing conditionals as suggested by Drew
> 
> Changes since v4:
>  - Rebased on the timer optimization series merged in the v4.15 merge
>    window, which caused a fair amount of modifications to patch 3.
>  - Added a static key to disable the sync operations when no VMs are
>    using userspace irqchips to further optimize the performance
>  - Fixed extra semicolon in vgic-mmio.c
>  - Added commentary as requested during review
>  - Dropped what was patch 4, because it was merged as part of GICv4
>    support.
>  - Factored out the VGIC input level function change as separate patch
>    (helps bisect and debugging), before providing a function for the
>    timer.
> 
> Changes since v3:
>  - Added a number of patches and moved patches around a bit.
>  - Check for uaccesses in the mmio handler functions
>  - Fixed bugs in the mmio handler functions
> 
> Changes since v2:
>  - Removed patch 5 from v2 and integrating the changes in what's now
>    patch 5 to make it easier to reuse code when adding VFIO integration.
>  - Changed the virtual distributor MMIO handling to use the
>    pending_latch and more closely match the semantics of SPENDR and
>    CPENDR for both level and edge mapped interrupts.
> 
> Changes since v1:
>  - Added necessary changes to the timer (Patch 1)
>  - Added handling of guest MMIO accesses to the virtual distributor
>    (Patch 4)
>  - Addressed Marc's comments from the initial RFC (mostly renames)
> 
> Thanks,
> -Christoffer
> 
> [1]: https://lists.cs.columbia.edu/pipermail/kvmarm/2017-June/026072.html
> 
> Christoffer Dall (8):
>   KVM: arm/arm64: Remove redundant preemptible checks
>   KVM: arm/arm64: Factor out functionality to get vgic mmio
>     requester_vcpu
>   KVM: arm/arm64: Don't cache the timer IRQ level
>   KVM: arm/arm64: vgic: Support level-triggered mapped interrupts
>   KVM: arm/arm64: Support a vgic interrupt line level sample function
>   KVM: arm/arm64: Support VGIC dist pend/active changes for mapped IRQs
>   KVM: arm/arm64: Provide a get_input_level for the arch timer
>   KVM: arm/arm64: Avoid work when userspace iqchips are not used
> 
>  include/kvm/arm_arch_timer.h  |   2 +
>  include/kvm/arm_vgic.h        |  13 ++++-
>  virt/kvm/arm/arch_timer.c     | 105 +++++++++++++++++++++-----------------
>  virt/kvm/arm/arm.c            |   2 -
>  virt/kvm/arm/vgic/vgic-mmio.c | 115 ++++++++++++++++++++++++++++++++++--------
>  virt/kvm/arm/vgic/vgic-v2.c   |  29 +++++++++++
>  virt/kvm/arm/vgic/vgic-v3.c   |  29 +++++++++++
>  virt/kvm/arm/vgic/vgic.c      |  42 +++++++++++++--
>  virt/kvm/arm/vgic/vgic.h      |   8 +++
>  9 files changed, 270 insertions(+), 75 deletions(-)
> 

I tested the series with AMD xgbe assignment using Direct EOI and I
don't see any regression. However most interesting assigned IRQs - DMA
related - are edge sensitive ones.

Tested-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric

^ permalink raw reply

* [PATCH v7 3/8] KVM: arm/arm64: Don't cache the timer IRQ level
From: Auger Eric @ 2017-12-11 20:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171207105418.22428-4-christoffer.dall@linaro.org>

Hi Christoffer,
On 07/12/17 11:54, Christoffer Dall wrote:
> The timer was modeled after a strict idea of modelling an interrupt line
> level in software, meaning that only transitions in the level needed to
> be reported to the VGIC.  This works well for the timer, because the
> arch timer code is in complete control of the device and can track the
> transitions of the line.
> 
> However, as we are about to support using the HW bit in the VGIC not
> just for the timer, but also for VFIO which cannot track transitions of
> the interrupt line, we have to decide on an interface for level
> triggered mapped interrupts to the GIC, which both the timer and VFIO
> can use.
> 
> VFIO only sees an asserting transition of the physical interrupt line,
> and tells the VGIC when that happens.  That means that part of the
> interrupt flow is offloaded to the hardware.
> 
> To use the same interface for VFIO devices and the timer, we therefore
> have to change the timer (we cannot change VFIO because it doesn't know
> the details of the device it is assigning to a VM).
> 
> Luckily, changing the timer is simple, we just need to stop 'caching'
> the line level, but instead let the VGIC know the state of the timer
> every time there is a potential change in the line level, and when the
> line level should be asserted from the timer ISR.  The VGIC can ignore
> extra notifications using its validate mechanism.

I was confused by the fact we say we stop caching the line level but
vtimer->irq.level still exists, is updated in the vtimer host ISR and
kvm_timer_update_state() and read in many places.

I feel difficult to figure out if each time we use the vtimer->irq.level
value it is safe to use it.

Also for the validate() to succeed we need the vgic irq->line_level to
to be 0. I understand this is properly handled for mapped level irqs in
next patch which does that on the populate_lr. However I currently fail
to understand why the timer level sensitive mapped IRQ does not require
the next patch to work.

Thanks

Eric

> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
>  virt/kvm/arm/arch_timer.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 4151250ce8da..dd5aca05c500 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -99,11 +99,9 @@ static irqreturn_t kvm_arch_timer_handler(int irq, void *dev_id)
>  	}
>  	vtimer = vcpu_vtimer(vcpu);
>  
> -	if (!vtimer->irq.level) {
> -		vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);
> -		if (kvm_timer_irq_can_fire(vtimer))
> -			kvm_timer_update_irq(vcpu, true, vtimer);
> -	}
> +	vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);
> +	if (kvm_timer_irq_can_fire(vtimer))
> +		kvm_timer_update_irq(vcpu, true, vtimer);
>  
>  	if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
>  		kvm_vtimer_update_mask_user(vcpu);
> @@ -324,12 +322,20 @@ static void kvm_timer_update_state(struct kvm_vcpu *vcpu)
>  	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
>  	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
>  	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> +	bool level;
>  
>  	if (unlikely(!timer->enabled))
>  		return;
>  
> -	if (kvm_timer_should_fire(vtimer) != vtimer->irq.level)
> -		kvm_timer_update_irq(vcpu, !vtimer->irq.level, vtimer);
> +	/*
> +	 * The vtimer virtual interrupt is a 'mapped' interrupt, meaning part
> +	 * of its lifecycle is offloaded to the hardware, and we therefore may
> +	 * not have lowered the irq.level value before having to signal a new
> +	 * interrupt, but have to signal an interrupt every time the level is
> +	 * asserted.
> +	 */
> +	level = kvm_timer_should_fire(vtimer);
> +	kvm_timer_update_irq(vcpu, level, vtimer);
>  
>  	if (kvm_timer_should_fire(ptimer) != ptimer->irq.level)
>  		kvm_timer_update_irq(vcpu, !ptimer->irq.level, ptimer);
> 

^ permalink raw reply

* [PATCH 1/1] tty: serial: imx: allow breaks to be received when using dma
From: Troy Kisky @ 2017-12-11 20:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <a3f0140a-92bb-ed59-ee79-971385634d4d@boundarydevices.com>

On 10/20/2017 3:17 PM, Troy Kisky wrote:
> On 10/20/2017 3:13 PM, Troy Kisky wrote:
>> This allows me to login after sending a break when service
>> serial-getty at ttymxc0.service is running
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>> ---
>>  drivers/tty/serial/imx.c | 20 +++++++++++++-------
>>  1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index 506fcd742b47..39033f460a24 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -934,7 +934,6 @@ static void dma_rx_callback(void *data)
>>  	status = dmaengine_tx_status(chan, (dma_cookie_t)0, &state);
>>  
>>  	if (status == DMA_ERROR) {
>> -		dev_err(sport->port.dev, "DMA transaction error.\n");
>>  		clear_rx_errors(sport);
>>  		return;
>>  	}
>> @@ -1035,6 +1034,7 @@ static int start_rx_dma(struct imx_port *sport)
>>  
>>  static void clear_rx_errors(struct imx_port *sport)
>>  {
>> +	struct tty_port *port = &sport->port.state->port;
>>  	unsigned int status_usr1, status_usr2;
>>  
>>  	status_usr1 = readl(sport->port.membase + USR1);
>> @@ -1043,12 +1043,18 @@ static void clear_rx_errors(struct imx_port *sport)
>>  	if (status_usr2 & USR2_BRCD) {
>>  		sport->port.icount.brk++;
>>  		writel(USR2_BRCD, sport->port.membase + USR2);
>> -	} else if (status_usr1 & USR1_FRAMERR) {
>> -		sport->port.icount.frame++;
>> -		writel(USR1_FRAMERR, sport->port.membase + USR1);
>> -	} else if (status_usr1 & USR1_PARITYERR) {
>> -		sport->port.icount.parity++;
>> -		writel(USR1_PARITYERR, sport->port.membase + USR1);
>> +		if (tty_insert_flip_char(port, 0, TTY_BREAK) == 0)
>> +			sport->port.icount.buf_overrun++;
>> +		tty_flip_buffer_push(port);
>> +	} else {
>> +		dev_err(sport->port.dev, "DMA transaction error.\n");
>> +		if (status_usr1 & USR1_FRAMERR) {
>> +			sport->port.icount.frame++;
>> +			writel(USR1_FRAMERR, sport->port.membase + USR1);
>> +		} else if (status_usr1 & USR1_PARITYERR) {
>> +			sport->port.icount.parity++;
>> +			writel(USR1_PARITYERR, sport->port.membase + USR1);
>> +		}
>>  	}
>>  
>>  	if (status_usr2 & USR2_ORE) {
>>
> 
> 
> 
> Does this need to use
>  	spin_lock_irqsave(&sport->port.lock, flags);
> 
> 
> I would have, but dma_rx_callback doesn't.
> 
> BR
> Troy
> 
> 

I think the path is fine as is. Should I send a rebased version ?

Thanks
Troy

^ permalink raw reply


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