Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 7/8] ARM: Move system register accessors to asm/cp15.h
From: Marc Zyngier @ 2016-09-09 17:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473350810-10857-8-git-send-email-vladimir.murzin@arm.com>

On 08/09/16 17:06, Vladimir Murzin wrote:
> Headers linux/irqchip/arm-gic.v3.h and arch/arm/include/asm/kvm_hyp.h
> are included in virt/kvm/arm/hyp/vgic-v3-sr.c and both define macros
> called __ACCESS_CP15 and __ACCESS_CP15_64 which obviously creates a
> conflict. These macros were introduced independently for GIC and KVM
> and, in fact, do the same thing.
> 
> As an option we could add prefixes to KVM and GIC version of macros so
> they won't clash, but it'd introduce code duplication.  Alternatively,
> we could keep macro in, say, GIC header and include it in KVM one (or
> vice versa), but such dependency would not look nicer.
> 
> So we follow arm64 way (it handles this via sysreg.h) and move only
> single set of macros to asm/cp15.h
> 
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  arch/arm/include/asm/arch_gicv3.h |   27 +++++++++++----------------
>  arch/arm/include/asm/cp15.h       |   15 +++++++++++++++
>  arch/arm/include/asm/kvm_hyp.h    |   15 +--------------
>  3 files changed, 27 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch_gicv3.h b/arch/arm/include/asm/arch_gicv3.h
> index e08d151..af25c32 100644
> --- a/arch/arm/include/asm/arch_gicv3.h
> +++ b/arch/arm/include/asm/arch_gicv3.h
> @@ -22,9 +22,7 @@
>  
>  #include <linux/io.h>
>  #include <asm/barrier.h>
> -
> -#define __ACCESS_CP15(CRn, Op1, CRm, Op2)	p15, Op1, %0, CRn, CRm, Op2
> -#define __ACCESS_CP15_64(Op1, CRm)		p15, Op1, %Q0, %R0, CRm
> +#include <asm/cp15.h>
>  
>  #define ICC_EOIR1			__ACCESS_CP15(c12, 0, c12, 1)
>  #define ICC_DIR				__ACCESS_CP15(c12, 0, c11, 1)
> @@ -102,58 +100,55 @@
>  
>  static inline void gic_write_eoir(u32 irq)
>  {
> -	asm volatile("mcr " __stringify(ICC_EOIR1) : : "r" (irq));
> +	write_sysreg(irq, ICC_EOIR1);
>  	isb();
>  }
>  
>  static inline void gic_write_dir(u32 val)
>  {
> -	asm volatile("mcr " __stringify(ICC_DIR) : : "r" (val));
> +	write_sysreg(val, ICC_DIR);
>  	isb();
>  }
>  
>  static inline u32 gic_read_iar(void)
>  {
> -	u32 irqstat;
> +	u32 irqstat = read_sysreg(ICC_IAR1);
>  
> -	asm volatile("mrc " __stringify(ICC_IAR1) : "=r" (irqstat));
>  	dsb(sy);
> +
>  	return irqstat;
>  }
>  
>  static inline void gic_write_pmr(u32 val)
>  {
> -	asm volatile("mcr " __stringify(ICC_PMR) : : "r" (val));
> +	write_sysreg(val, ICC_PMR);
>  }
>  
>  static inline void gic_write_ctlr(u32 val)
>  {
> -	asm volatile("mcr " __stringify(ICC_CTLR) : : "r" (val));
> +	write_sysreg(val, ICC_CTLR);
>  	isb();
>  }
>  
>  static inline void gic_write_grpen1(u32 val)
>  {
> -	asm volatile("mcr " __stringify(ICC_IGRPEN1) : : "r" (val));
> +	write_sysreg(val, ICC_IGRPEN1);
>  	isb();
>  }
>  
>  static inline void gic_write_sgi1r(u64 val)
>  {
> -	asm volatile("mcrr " __stringify(ICC_SGI1R) : : "r" (val));
> +	write_sysreg(val, ICC_SGI1R);
>  }
>  
>  static inline u32 gic_read_sre(void)
>  {
> -	u32 val;
> -
> -	asm volatile("mrc " __stringify(ICC_SRE) : "=r" (val));
> -	return val;
> +	return read_sysreg(ICC_SRE);
>  }
>  
>  static inline void gic_write_sre(u32 val)
>  {
> -	asm volatile("mcr " __stringify(ICC_SRE) : : "r" (val));
> +	write_sysreg(val, ICC_SRE);
>  	isb();
>  }
>  
> diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
> index c3f1152..f661732 100644
> --- a/arch/arm/include/asm/cp15.h
> +++ b/arch/arm/include/asm/cp15.h
> @@ -47,6 +47,21 @@
>  #define vectors_high()	(0)
>  #endif
>  
> +#define __ACCESS_CP15(CRn, Op1, CRm, Op2)	\
> +	"mrc", "mcr", __stringify(p15, Op1, %0, CRn, CRm, Op2), u32
> +#define __ACCESS_CP15_64(Op1, CRm)		\
> +	"mrrc", "mcrr", __stringify(p15, Op1, %Q0, %R0, CRm), u64
> +
> +#define __read_sysreg(r, w, c, t) ({				\
> +	t __val;						\
> +	asm volatile(r " " c : "=r" (__val));			\
> +	__val;							\
> +})
> +#define read_sysreg(...)		__read_sysreg(__VA_ARGS__)
> +
> +#define __write_sysreg(v, r, w, c, t)	asm volatile(w " " c : : "r" ((t)(v)))
> +#define write_sysreg(v, ...)		__write_sysreg(v, __VA_ARGS__)
> +

Shouldn't that be placed after the #ifdef below?

>  #ifdef CONFIG_CPU_CP15
>  
>  extern unsigned long cr_alignment;	/* defined in entry-armv.S */
> diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h
> index bd9434e..0b475d2 100644
> --- a/arch/arm/include/asm/kvm_hyp.h
> +++ b/arch/arm/include/asm/kvm_hyp.h
> @@ -20,26 +20,13 @@
>  
>  #include <linux/compiler.h>
>  #include <linux/kvm_host.h>
> +#include <asm/cp15.h>
>  #include <asm/kvm_mmu.h>
>  #include <asm/vfp.h>
>  
> -#define __ACCESS_CP15(CRn, Op1, CRm, Op2)	\
> -	"mrc", "mcr", __stringify(p15, Op1, %0, CRn, CRm, Op2), u32
> -#define __ACCESS_CP15_64(Op1, CRm)		\
> -	"mrrc", "mcrr", __stringify(p15, Op1, %Q0, %R0, CRm), u64
>  #define __ACCESS_VFP(CRn)			\
>  	"mrc", "mcr", __stringify(p10, 7, %0, CRn, cr0, 0), u32
>  
> -#define __write_sysreg(v, r, w, c, t)	asm volatile(w " " c : : "r" ((t)(v)))
> -#define write_sysreg(v, ...)		__write_sysreg(v, __VA_ARGS__)
> -
> -#define __read_sysreg(r, w, c, t) ({				\
> -	t __val;						\
> -	asm volatile(r " " c : "=r" (__val));			\
> -	__val;							\
> -})
> -#define read_sysreg(...)		__read_sysreg(__VA_ARGS__)
> -
>  #define write_special(v, r)					\
>  	asm volatile("msr " __stringify(r) ", %0" : : "r" (v))
>  #define read_special(r) ({					\
> 

Could you please cc RMK on this, given that this touches a core arch/arm
file?

Thanks,

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

^ permalink raw reply

* [PATCH] hwmon: xgene: access mailbox as RAM
From: Hoan Tran @ 2016-09-09 17:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160909165855.GB18952@roeck-us.net>

On Fri, Sep 9, 2016 at 9:58 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> Hi Arnd,
>
> On Fri, Sep 09, 2016 at 05:38:58PM +0200, Arnd Bergmann wrote:
>> The newly added hwmon driver fails to build in an allmodconfig
>> kernel:
>>
>>       1  ERROR: "memblock_is_memory" [drivers/hwmon/xgene-hwmon.ko] undefined!
>>
>> According to comments in the code, the mailbox is a shared memory region,
>> not a set of MMIO registers, so we should use memremap() for mapping it
>> instead of ioremap or acpi_os_ioremap, and pointer dereferences instead
>> of readl/writel.
>>
>> The driver already uses plain kernel pointers, so it's a bit unusual
>> to work with functions that operate on __iomem pointers, and this
>> fixes that part too.
>>
>> I'm using READ_ONCE/WRITE_ONCE here to keep the existing behavior
>> regarding the ordering of the accesses from the CPU, but note that
>> there are no barriers (also unchanged from before).
>>
>> I'm also keeping the endianess behavior, though I'm unsure whether
>> the message data was supposed to be in LE32 format in the first
>> place, it's possible this was meant to be interpreted as a byte
>> stream instead.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>
>
> Thanks a lot for looking into this.
>
> I'll apply this patch to address the build problem. Much better than
> my rude "depends on BROKEN". It would be great to get a Tested-by:
> from someone with access to the hardware.
>

Hi Arnd and Guenter,

Thanks for the patch. I'm testing it out.

Hoan

> Guenter
>
>> diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
>> index bc78a5d10182..e834dfb3acca 100644
>> --- a/drivers/hwmon/xgene-hwmon.c
>> +++ b/drivers/hwmon/xgene-hwmon.c
>> @@ -34,7 +34,8 @@
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/platform_device.h>
>> -#include <acpi/acpi_io.h>
>> +#include <linux/io.h>
>> +
>>  #include <acpi/pcc.h>
>>
>>  /* SLIMpro message defines */
>> @@ -126,10 +127,10 @@ static u16 xgene_word_tst_and_clr(u16 *addr, u16 mask)
>>  {
>>       u16 ret, val;
>>
>> -     val = readw_relaxed(addr);
>> +     val = le16_to_cpu(READ_ONCE(*addr));
>>       ret = val & mask;
>>       val &= ~mask;
>> -     writew_relaxed(val, addr);
>> +     WRITE_ONCE(*addr, cpu_to_le16(val));
>>
>>       return ret;
>>  }
>> @@ -137,7 +138,7 @@ static u16 xgene_word_tst_and_clr(u16 *addr, u16 mask)
>>  static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev *ctx, u32 *msg)
>>  {
>>       struct acpi_pcct_shared_memory *generic_comm_base = ctx->pcc_comm_addr;
>> -     void *ptr = generic_comm_base + 1;
>> +     u32 *ptr = (void*)(generic_comm_base + 1);
>>       int rc, i;
>>       u16 val;
>>
>> @@ -146,21 +147,21 @@ static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev *ctx, u32 *msg)
>>       ctx->resp_pending = true;
>>
>>       /* Write signature for subspace */
>> -     writel_relaxed(PCC_SIGNATURE_MASK | ctx->mbox_idx,
>> -                    &generic_comm_base->signature);
>> +     WRITE_ONCE(generic_comm_base->signature,
>> +                cpu_to_le32(PCC_SIGNATURE_MASK | ctx->mbox_idx));
>>
>>       /* Write to the shared command region */
>> -     writew_relaxed(MSG_TYPE(msg[0]) | PCCC_GENERATE_DB_INT,
>> -                    &generic_comm_base->command);
>> +     WRITE_ONCE(generic_comm_base->command,
>> +                cpu_to_le16(MSG_TYPE(msg[0]) | PCCC_GENERATE_DB_INT));
>>
>>       /* Flip CMD COMPLETE bit */
>> -     val = readw_relaxed(&generic_comm_base->status);
>> +     val = le16_to_cpu(READ_ONCE(generic_comm_base->status));
>>       val &= ~PCCS_CMD_COMPLETE;
>> -     writew_relaxed(val, &generic_comm_base->status);
>> +     WRITE_ONCE(generic_comm_base->status, cpu_to_le16(val));
>>
>>       /* Copy the message to the PCC comm space */
>>       for (i = 0; i < sizeof(struct slimpro_resp_msg) / 4; i++)
>> -             writel_relaxed(msg[i], ptr + i * 4);
>> +             WRITE_ONCE(ptr[i], cpu_to_le32(msg[i]));
>>
>>       /* Ring the doorbell */
>>       rc = mbox_send_message(ctx->mbox_chan, msg);
>> @@ -652,9 +653,9 @@ static int xgene_hwmon_probe(struct platform_device *pdev)
>>                */
>>               ctx->comm_base_addr = cppc_ss->base_address;
>>               if (ctx->comm_base_addr) {
>> -                     ctx->pcc_comm_addr =
>> -                                     acpi_os_ioremap(ctx->comm_base_addr,
>> -                                                     cppc_ss->length);
>> +                     ctx->pcc_comm_addr = memremap(ctx->comm_base_addr,
>> +                                                     cppc_ss->length,
>> +                                                     MEMREMAP_WT);
>>               } else {
>>                       dev_err(&pdev->dev, "Failed to get PCC comm region\n");
>>                       rc = -ENODEV;
>>

^ permalink raw reply

* [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
From: Kevin Hilman @ 2016-09-09 17:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAFBinCCDDwydB+RP0CZKoAW5Suxotn224ZFVWvt2OGW3gNJR1g@mail.gmail.com>

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>> However, the problem with all of the solutions proposed (runtime PM ones
>> included) is that we're forcing a board-specific design issue (2 devices
>> sharing a reset line) into a driver that should not have any
>> board-specific assumptions in it.
>>
>> For example, if this driver is used on another platform where different
>> PHYs have different reset lines, then one of them (the unlucky one who
>> is not probed first) will never get reset.  So any form of per-device
>> ref-counting is not a portable solution.
>
> maybe we should also consider Ben's solution: he played with the USB
> PHY on his Meson8b board. His approach was to have only one USB PHY
> driver instance which exposes two PHYs.
> The downside of this: the driver would have to know the offset of the
> PHYs (0x0 for the first PHY, 0x20 for the second), but we could handle
> the reset using runtime PM without any hacks.

> I checked the USB PHY reference driver: it seems that there will be a
> new USB PHY with the GXL/GXM SoCs.
> So maybe we could live with the assumption that the PHYs are at
> consecutive addresses.

But isn't that also forcing us to make board-specific assumptions inside
the driver.

Kevin

^ permalink raw reply

* [RFC PATCH 0/5] arm64: Signal context expansion
From: Florian Weimer @ 2016-09-09 17:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160909152047.GA3958@e103592.cambridge.arm.com>

On 09/09/2016 05:21 PM, Dave Martin wrote:

>> Do you add this extra information only if the stack is sufficiently large?
>>
>> x86_64 adds the new information even for small stacks set up with
>> sigaltstack, leading to memory corruption on bleeding-edge hardware:
>>
>>   <https://bugzilla.kernel.org/show_bug.cgi?id=153531>
>
> Hmmm, not yet.  We already check that the whole frame is writable user
> memory, but this isn't sufficient to avoid user corruption in the case
> of alternate signal stacks.  I'll fix this -- thanks for flagging it.
>
> If the stack isn't large enough, we'll still have to SEGV the task
> though.

You could skip copying the data and not install a pointer to it in the 
existing signal context.

> We can (and should) bump up the SIG{,MIN}STKSZ constants when adding
> the SVE support proper to the kernel,

That's a userspace ABI change (libraries use these constants to size 
struct members), and not a good idea.  You might get away with at this 
stage, but you can't do this every time you add some new process state 
you want to add to signal handlers.

> I wonder whether we should make the signal stack size runtime
> discoverable through sysconf() instead...

That might be a good idea, yes.

Florian

^ permalink raw reply

* next build: 143 builds: 2 failed, 141 passed, 2 errors, 2 warnings (next-20160909)
From: Kevin Hilman @ 2016-09-09 17:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2582793.W17ckCts1B@wuerfel>

Arnd Bergmann <arnd@arndb.de> writes:

> On Friday, September 9, 2016 12:25:54 AM CEST kernelci. org bot wrote:
>> next build: 143 builds: 2 failed, 141 passed, 2 errors, 2 warnings (next-20160909)
>
> This is pretty good, here is what's left:
>
>> rpc_defconfig (arm) ? FAIL, 1 error, 0 warnings, 0 section mismatches
>> 
>> Errors:
>>     fs/fat/dir.c:758:424: internal compiler error: Segmentation fault
>
> Could we please update the gcc build on kernelci.org from "Linaro GCC 5.3-2016.02"
> to the following version (Linaro GCC 5.3-2016.05) or later?

I finally found the release tarballs for Linaro toolchains (the "latest"
link on the linaro releases was stale, and pointing to 2016.02).

So it's now 5.3-2016.05 on all the kCI builders.

Kevin

^ permalink raw reply

* [PATCH v3 6/8] ARM: Change MPIDR_AFFINITY_LEVEL to ignore Aff3
From: Marc Zyngier @ 2016-09-09 16:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473350810-10857-7-git-send-email-vladimir.murzin@arm.com>

On 08/09/16 17:06, Vladimir Murzin wrote:
> vgic-v3 driver queries CPU affinity level up to Aff3, which is valid
> for arm64.  However, for arm up to Aff2 levels are supported, so
> querying for third level ends with upper bits of MPIDR are treated as
> valid affinity level which is not true. Make sure we report zero for
> any affinity level above two.
> 
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  arch/arm/include/asm/cputype.h |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
> index 1ee94c7..f08fac4 100644
> --- a/arch/arm/include/asm/cputype.h
> +++ b/arch/arm/include/asm/cputype.h
> @@ -55,9 +55,10 @@
>  
>  #define MPIDR_LEVEL_BITS 8
>  #define MPIDR_LEVEL_MASK ((1 << MPIDR_LEVEL_BITS) - 1)
> +#define MPIDR_LEVEL_SHIFT(level) (MPIDR_LEVEL_BITS * level)
>  
>  #define MPIDR_AFFINITY_LEVEL(mpidr, level) \
> -	((mpidr >> (MPIDR_LEVEL_BITS * level)) & MPIDR_LEVEL_MASK)
> +	(((mpidr & MPIDR_HWID_BITMASK) >> MPIDR_LEVEL_SHIFT(level)) & MPIDR_LEVEL_MASK)
>  
>  #define ARM_CPU_IMP_ARM			0x41
>  #define ARM_CPU_IMP_INTEL		0x69
> 

There is something I don't quite get. Is this patch really necessary?
Are there cases where we construct a MPIDR that can have Aff3 set on 32bit?

Thanks,

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

^ permalink raw reply

* [PATCH] hwmon: xgene: access mailbox as RAM
From: Guenter Roeck @ 2016-09-09 16:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2529114.aAikiSWl11@wuerfel>

Hi Arnd,

On Fri, Sep 09, 2016 at 05:38:58PM +0200, Arnd Bergmann wrote:
> The newly added hwmon driver fails to build in an allmodconfig
> kernel:
> 
>       1  ERROR: "memblock_is_memory" [drivers/hwmon/xgene-hwmon.ko] undefined!
> 
> According to comments in the code, the mailbox is a shared memory region,
> not a set of MMIO registers, so we should use memremap() for mapping it
> instead of ioremap or acpi_os_ioremap, and pointer dereferences instead
> of readl/writel.
> 
> The driver already uses plain kernel pointers, so it's a bit unusual
> to work with functions that operate on __iomem pointers, and this
> fixes that part too.
> 
> I'm using READ_ONCE/WRITE_ONCE here to keep the existing behavior
> regarding the ordering of the accesses from the CPU, but note that
> there are no barriers (also unchanged from before).
> 
> I'm also keeping the endianess behavior, though I'm unsure whether
> the message data was supposed to be in LE32 format in the first
> place, it's possible this was meant to be interpreted as a byte
> stream instead.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 

Thanks a lot for looking into this.

I'll apply this patch to address the build problem. Much better than
my rude "depends on BROKEN". It would be great to get a Tested-by:
from someone with access to the hardware.

Guenter

> diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
> index bc78a5d10182..e834dfb3acca 100644
> --- a/drivers/hwmon/xgene-hwmon.c
> +++ b/drivers/hwmon/xgene-hwmon.c
> @@ -34,7 +34,8 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> -#include <acpi/acpi_io.h>
> +#include <linux/io.h>
> +
>  #include <acpi/pcc.h>
>  
>  /* SLIMpro message defines */
> @@ -126,10 +127,10 @@ static u16 xgene_word_tst_and_clr(u16 *addr, u16 mask)
>  {
>  	u16 ret, val;
>  
> -	val = readw_relaxed(addr);
> +	val = le16_to_cpu(READ_ONCE(*addr));
>  	ret = val & mask;
>  	val &= ~mask;
> -	writew_relaxed(val, addr);
> +	WRITE_ONCE(*addr, cpu_to_le16(val));
>  
>  	return ret;
>  }
> @@ -137,7 +138,7 @@ static u16 xgene_word_tst_and_clr(u16 *addr, u16 mask)
>  static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev *ctx, u32 *msg)
>  {
>  	struct acpi_pcct_shared_memory *generic_comm_base = ctx->pcc_comm_addr;
> -	void *ptr = generic_comm_base + 1;
> +	u32 *ptr = (void*)(generic_comm_base + 1);
>  	int rc, i;
>  	u16 val;
>  
> @@ -146,21 +147,21 @@ static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev *ctx, u32 *msg)
>  	ctx->resp_pending = true;
>  
>  	/* Write signature for subspace */
> -	writel_relaxed(PCC_SIGNATURE_MASK | ctx->mbox_idx,
> -		       &generic_comm_base->signature);
> +	WRITE_ONCE(generic_comm_base->signature,
> +		   cpu_to_le32(PCC_SIGNATURE_MASK | ctx->mbox_idx));
>  
>  	/* Write to the shared command region */
> -	writew_relaxed(MSG_TYPE(msg[0]) | PCCC_GENERATE_DB_INT,
> -		       &generic_comm_base->command);
> +	WRITE_ONCE(generic_comm_base->command,
> +		   cpu_to_le16(MSG_TYPE(msg[0]) | PCCC_GENERATE_DB_INT));
>  
>  	/* Flip CMD COMPLETE bit */
> -	val = readw_relaxed(&generic_comm_base->status);
> +	val = le16_to_cpu(READ_ONCE(generic_comm_base->status));
>  	val &= ~PCCS_CMD_COMPLETE;
> -	writew_relaxed(val, &generic_comm_base->status);
> +	WRITE_ONCE(generic_comm_base->status, cpu_to_le16(val));
>  
>  	/* Copy the message to the PCC comm space */
>  	for (i = 0; i < sizeof(struct slimpro_resp_msg) / 4; i++)
> -		writel_relaxed(msg[i], ptr + i * 4);
> +		WRITE_ONCE(ptr[i], cpu_to_le32(msg[i]));
>  
>  	/* Ring the doorbell */
>  	rc = mbox_send_message(ctx->mbox_chan, msg);
> @@ -652,9 +653,9 @@ static int xgene_hwmon_probe(struct platform_device *pdev)
>  		 */
>  		ctx->comm_base_addr = cppc_ss->base_address;
>  		if (ctx->comm_base_addr) {
> -			ctx->pcc_comm_addr =
> -					acpi_os_ioremap(ctx->comm_base_addr,
> -							cppc_ss->length);
> +			ctx->pcc_comm_addr = memremap(ctx->comm_base_addr,
> +							cppc_ss->length,
> +							MEMREMAP_WT);
>  		} else {
>  			dev_err(&pdev->dev, "Failed to get PCC comm region\n");
>  			rc = -ENODEV;
> 

^ permalink raw reply

* [PATCH v3 5/8] KVM: arm: vgic: Support 64-bit data manipulation on 32-bit host systems
From: Marc Zyngier @ 2016-09-09 16:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473350810-10857-6-git-send-email-vladimir.murzin@arm.com>

On 08/09/16 17:06, Vladimir Murzin wrote:
> We have couple of 64-bit registers defined in GICv3 architecture, so
> unsigned long accesses to these registers will only access a single
> 32-bit part of that regitser. On the other hand these registers can't
> be accessed as 64-bit with a single instruction like ldrd/strd or
> ldmia/stmia if we run a 32-bit host because KVM does not support
> access to MMIO space done by these instructions.
> 
> It means that a 32-bit guest accesses these registers in 32-bit
> chunks, so the only thing we need to do is to ensure that
> extract_bytes() always takes 64-bit data.
> 
> Since we are here fix couple of other width related issues catched by
> gcc
> 
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  virt/kvm/arm/vgic/vgic-mmio-v3.c |    6 +++---
>  virt/kvm/arm/vgic/vgic-mmio.h    |    2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> index acbe691..0d3c76a 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> @@ -23,7 +23,7 @@
>  #include "vgic-mmio.h"
>  
>  /* extract @num bytes at @offset bytes offset in data */
> -unsigned long extract_bytes(unsigned long data, unsigned int offset,
> +unsigned long extract_bytes(u64 data, unsigned int offset,
>  			    unsigned int num)
>  {
>  	return (data >> (offset * 8)) & GENMASK_ULL(num * 8 - 1, 0);
> @@ -181,7 +181,7 @@ static unsigned long vgic_mmio_read_v3r_typer(struct kvm_vcpu *vcpu,
>  	int target_vcpu_id = vcpu->vcpu_id;
>  	u64 value;
>  
> -	value = (mpidr & GENMASK(23, 0)) << 32;
> +	value = (u64)(mpidr & GENMASK(23, 0)) << 32;
>  	value |= ((target_vcpu_id & 0xffff) << 8);
>  	if (target_vcpu_id == atomic_read(&vcpu->kvm->online_vcpus) - 1)
>  		value |= GICR_TYPER_LAST;
> @@ -611,7 +611,7 @@ void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg)
>  	bool broadcast;
>  
>  	sgi = (reg & ICC_SGI1R_SGI_ID_MASK) >> ICC_SGI1R_SGI_ID_SHIFT;
> -	broadcast = reg & BIT(ICC_SGI1R_IRQ_ROUTING_MODE_BIT);
> +	broadcast = reg & BIT_ULL(ICC_SGI1R_IRQ_ROUTING_MODE_BIT);
>  	target_cpus = (reg & ICC_SGI1R_TARGET_LIST_MASK) >> ICC_SGI1R_TARGET_LIST_SHIFT;
>  	mpidr = SGI_AFFINITY_LEVEL(reg, 3);
>  	mpidr |= SGI_AFFINITY_LEVEL(reg, 2);
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.h b/virt/kvm/arm/vgic/vgic-mmio.h
> index 0b3ecf9..80f92ce 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.h
> +++ b/virt/kvm/arm/vgic/vgic-mmio.h
> @@ -96,7 +96,7 @@ unsigned long vgic_data_mmio_bus_to_host(const void *val, unsigned int len);
>  void vgic_data_host_to_mmio_bus(void *buf, unsigned int len,
>  				unsigned long data);
>  
> -unsigned long extract_bytes(unsigned long data, unsigned int offset,
> +unsigned long extract_bytes(u64 data, unsigned int offset,
>  			    unsigned int num);
>  
>  u64 update_64bit_reg(u64 reg, unsigned int offset, unsigned int len,
> 

My personal preference would be to split this in two patches. One that
changes extract_bytes to work on 64bit quantities, and another one that
addresses the 64bit issues. Not a big deal though.

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

Thanks,

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

^ permalink raw reply

* [PATCH v5] i2c: imx: make bus recovery through pinctrl optional
From: Stefan Agner @ 2016-09-09 16:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CADRPPNR4oiRVgFqaM=1=Zn_JzrVJDVmozWsouLByXRtaHCh=qQ@mail.gmail.com>

On 2016-09-08 16:57, Leo Li wrote:
> On Thu, Sep 8, 2016 at 5:39 PM, Stefan Agner <stefan@agner.ch> wrote:
>> On 2016-09-06 15:40, Leo Li wrote:
>>> On Tue, Sep 6, 2016 at 4:51 PM, Stefan Agner <stefan@agner.ch> wrote:
>>>> On 2016-09-06 13:06, Leo Li wrote:
>>>>> On Tue, Sep 6, 2016 at 1:58 PM, Uwe Kleine-K?nig
>>>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>>>> On Fri, Aug 19, 2016 at 05:05:22PM -0500, Li Yang wrote:
>> <snip>
>>>>>>> @@ -1081,8 +1090,11 @@ static int i2c_imx_probe(struct platform_device *pdev)
>>>>>>>               return ret;
>>>>>>>       }
>>>>>>>
>>>>>>> +     /* optional bus recovery feature through pinctrl */
>>>>>>>       i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
>>>>>>> -     if (IS_ERR(i2c_imx->pinctrl)) {
>>>>>>> +     /* bailout on -ENOMEM or -EPROBE_DEFER, continue for other errors */
>>>>>>> +     if (PTR_ERR(i2c_imx->pinctrl) == -ENOMEM ||
>>>>>>> +                     PTR_ERR(i2c_imx->pinctrl) == -EPROBE_DEFER) {
>>>>>>>               ret = PTR_ERR(i2c_imx->pinctrl);
>>>>>>>               goto clk_disable;
>>>>>>>       }
>>>>>>
>>>>>> devm_pinctrl_get might return the following error-valued pointers:
>>>>>>  - -EINVAL
>>>>>>  - -ENOMEM
>>>>>>  - -ENODEV
>>>>>>  - -EPROBE_DEFER
>>>>>>
>>>>>> There are several error paths returning -EINVAL, one is when an invalid
>>>>>> phandle is used. Do you really want to ignore that?
>>>>>>
>>>>>> IMO error handling is better done with inverse logic, that is continue
>>>>>> on some explicit error, bail out on all unknown stuff. This tends to be
>>>>>> more robust. Also the comment should be improved to not explain that for
>>>>>> -ENOMEM and -EPROBE_DEFER we bail out (which should be obvious for
>>>>>> anyone who can read C) but to explain why.
>>>>>
>>>>> What you said is true for normal error handling, but in this scenario
>>>>> it is intentional to ignore all pinctrl related errors except critical
>>>>> ones because failing to have pinctrl for an optional feature shouldn't
>>>>> impact the function of normal i2c.  We choose to catch -ENOMEM because
>>>>> the error could also cause problem for i2c probe, and -EPROBE_DEFER
>>>>> because it's possible that the pinctrl will be ready later and we want
>>>>> to give it a chance.  The i2c driver really don't care why the pinctrl
>>>>> was not usable.  I thought I added comment before the
>>>>
>>>> I don't agree. E.g. -EINVAL would appear if you pass devm_pinctrl_get an
>>>> invalid device. Currently you would silently ignore that, which is not
>>>> what you want.
>>>
>>> It is not silently ignored, there will be a message printed out saying
>>> pinctrl is not available and bus recovery is not supported.  On the
>>> contrary, without this change the entire i2c driver fails to work
>>> silently if pinctrl is somehow not working.  And if the system is so
>>> broken that the pointer to the i2c device is NULL, the probe of i2c
>>> would have already failed before this point.  We shouldn't count on an
>>> optional function of the driver to catch fundamental issues like this.
>>>
>>>>
>>>> You want to get the pinctrl in any case expect there isn't one. And that
>>>> is how you should formulate your if statement.
>>>>
>>>> /*
>>>>  * It is ok if no pinctrl device is available. We'll not be able to use
>>>> the
>>>>  * bus recovery feature, but otherwise the driver works fine...
>>>>  */
>>>> if (PTR_ERR(i2c_imx->pinctrl) != -ENODEV)
>>>
>>> I agree that there could be other possibilities that the pinctrl
>>> failed to work beside the reason I described in the commit
>>> message(platform doesn't support pinctrl at all).  But I don't think
>>> any of them other than the -ENOMEM and -EPROBE_DEFER deserves a bail
>>> out for the entire i2c driver.
>>
>> FWIW, I disagree. If there is pinctrl defined, you want be sure that it
>> gets applied properly, no matter what. E.g. when devm_pinctrl_get return
>> EINVAL (Uwe's example) the driver will continue and likely fail in
>> mysterious ways later on because the pins have not been muxed properly.
>> The driver should not load in that situation so that the developer is
>> forced to fix his mistakes. The only reason to bail out here is if there
>> is no pin controller (ENODEV). And it seems that Uwe also tends to that
>> solution.
> 
> With this patch the i2c bus recovery feature will be disabled if the
> devm_pinctrl_get() fails.  The pin mux setting will not be changed in
> either i2c probe stage or at runtime.  I don't think it can cause any
> trouble to normal I2C operation.  IMO, it is not good to *force*

If you have a pin controller, and you make a typo in your device tree
which leads to a wrong phandle and devm_pinctrl_get returning -EINVAL,
the system won't mux the pins... And that will certainly affect normal
I2C operation!

> people fix problem that they don't really care by deliberately enlarge
> the problem.  That's why we don't panic() on any error we found.  For
> those who do care about the bus recovery, they can get the information
> from the console.

IMHO, it is just stupid to ignore errors and then let the developer
later on trace back what the initial issue was. Error out early is a
common sense software design principle...

I am not asking for a panic(), I am just suggesting to only ignore
pinctrl if it returns -ENODEV, the case you care are about.

--
Stefan

^ permalink raw reply

* [PATCH v3 4/8] KVM: arm64: vgic-its: Introduce config option to guard ITS specific code
From: Marc Zyngier @ 2016-09-09 16:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473350810-10857-5-git-send-email-vladimir.murzin@arm.com>

On 08/09/16 17:06, Vladimir Murzin wrote:
> By now ITS code guarded with KVM_ARM_VGIC_V3 config option which was
> introduced to hide everything specific to vgic-v3 from 32-bit world.
> We are going to support vgic-v3 in 32-bit world and KVM_ARM_VGIC_V3
> will gone, but we don't have support for ITS there yet and we need to
> continue keeping ITS away.
> Introduce the new config option to prevent ITS code being build in
> 32-bit mode when support for vgic-v3 is done.
> 
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

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

^ permalink raw reply

* [PATCH v3 8/8] ARM: KVM: Support vgic-v3
From: Marc Zyngier @ 2016-09-09 16:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473350810-10857-9-git-send-email-vladimir.murzin@arm.com>

On 08/09/16 17:06, Vladimir Murzin wrote:
> This patch allows to build and use vgic-v3 in 32-bit mode.
> 
> Unfortunately, it can not be split in several steps without extra
> stubs to keep patches independent and bisectable.
> For instance, virt/kvm/arm/vgic/vgic-v3.c uses function from
> vgic-v3-sr.c, handling access to GICv3 cpu interface from the guest
> requires vgic_v3.vgic_sre to be already defined.
> 
> It is how support has been done:
> 
> * vgic-v3 save/restore routines are written in such way that they map
>   arm64 system register naming nicely, but it does not fit to arm
>   world. To keep virt/kvm/arm/hyp/vgic-v3-sr.c untouched we create a
>   mapping with a function for each register mapping the 32-bit to the
>   64-bit accessors
> 
> * report configured SRE on access to GICv3 cpu interface from the guest
> 
> * required vgic-v3 macros are provided via uapi.h
> 
> * static keys are used to select GIC backend
> 
> * to make vgic-v3 build KVM_ARM_VGIC_V3 guard is removed along with
>   the static inlines
> 
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  arch/arm/include/asm/arch_gicv3.h   |   64 +++++++++++++++++++++++++++++++++++
>  arch/arm/include/asm/kvm_asm.h      |    3 ++
>  arch/arm/include/asm/kvm_hyp.h      |    3 ++
>  arch/arm/include/uapi/asm/kvm.h     |    7 ++++
>  arch/arm/kvm/Makefile               |    2 ++
>  arch/arm/kvm/coproc.c               |   36 ++++++++++++++++++++
>  arch/arm/kvm/hyp/Makefile           |    1 +
>  arch/arm/kvm/hyp/switch.c           |   12 +++++--
>  arch/arm64/kvm/Kconfig              |    4 ---
>  include/kvm/arm_vgic.h              |    8 -----
>  virt/kvm/arm/vgic/vgic-kvm-device.c |    8 -----
>  virt/kvm/arm/vgic/vgic-mmio.c       |    2 --
>  virt/kvm/arm/vgic/vgic-mmio.h       |    2 --
>  virt/kvm/arm/vgic/vgic.h            |   54 -----------------------------
>  14 files changed, 126 insertions(+), 80 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch_gicv3.h b/arch/arm/include/asm/arch_gicv3.h
> index af25c32..f93f6bd 100644
> --- a/arch/arm/include/asm/arch_gicv3.h
> +++ b/arch/arm/include/asm/arch_gicv3.h
> @@ -96,6 +96,70 @@
>  #define ICH_AP1R2			__AP1Rx(2)
>  #define ICH_AP1R3			__AP1Rx(3)
>  
> +/* A32-to-A64 mappings used by VGIC save/restore */
> +
> +#define CPUIF_MAP(a32, a64)			\
> +static inline void write_ ## a64(u32 val)	\
> +{						\
> +	write_sysreg(val, a32);			\
> +}						\
> +static inline u32 read_ ## a64(void)		\
> +{						\
> +	return read_sysreg(a32); 		\
> +}						\
> +
> +#define CPUIF_MAP_LO_HI(a32lo, a32hi, a64)	\
> +static inline void write_ ## a64(u64 val)	\
> +{						\
> +	write_sysreg((u32)val, a32lo);		\
> +	write_sysreg((u32)(val >> 32), a32hi);	\

Please use {lower,upper}_32_bits, which make the casting/shifting go away.

> +}						\
> +static inline u64 read_ ## a64(void)		\
> +{						\
> +	u64 val = read_sysreg(a32lo);		\
> +						\
> +	val |=	(u64)read_sysreg(a32hi) << 32;	\
> +						\
> +	return val; 				\
> +}
> +
> +CPUIF_MAP(ICH_HCR, ICH_HCR_EL2)
> +CPUIF_MAP(ICH_VTR, ICH_VTR_EL2)
> +CPUIF_MAP(ICH_MISR, ICH_MISR_EL2)
> +CPUIF_MAP(ICH_EISR, ICH_EISR_EL2)
> +CPUIF_MAP(ICH_ELSR, ICH_ELSR_EL2)
> +CPUIF_MAP(ICH_VMCR, ICH_VMCR_EL2)
> +CPUIF_MAP(ICH_AP0R3, ICH_AP0R3_EL2)
> +CPUIF_MAP(ICH_AP0R2, ICH_AP0R2_EL2)
> +CPUIF_MAP(ICH_AP0R1, ICH_AP0R1_EL2)
> +CPUIF_MAP(ICH_AP0R0, ICH_AP0R0_EL2)
> +CPUIF_MAP(ICH_AP1R3, ICH_AP1R3_EL2)
> +CPUIF_MAP(ICH_AP1R2, ICH_AP1R2_EL2)
> +CPUIF_MAP(ICH_AP1R1, ICH_AP1R1_EL2)
> +CPUIF_MAP(ICH_AP1R0, ICH_AP1R0_EL2)
> +CPUIF_MAP(ICC_HSRE, ICC_SRE_EL2)
> +CPUIF_MAP(ICC_SRE, ICC_SRE_EL1)
> +
> +CPUIF_MAP_LO_HI(ICH_LR15, ICH_LRC15, ICH_LR15_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR14, ICH_LRC14, ICH_LR14_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR13, ICH_LRC13, ICH_LR13_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR12, ICH_LRC12, ICH_LR12_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR11, ICH_LRC11, ICH_LR11_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR10, ICH_LRC10, ICH_LR10_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR9, ICH_LRC9, ICH_LR9_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR8, ICH_LRC8, ICH_LR8_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR7, ICH_LRC7, ICH_LR7_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR6, ICH_LRC6, ICH_LR6_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR5, ICH_LRC5, ICH_LR5_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR4, ICH_LRC4, ICH_LR4_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR3, ICH_LRC3, ICH_LR3_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR2, ICH_LRC2, ICH_LR2_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR1, ICH_LRC1, ICH_LR1_EL2)
> +CPUIF_MAP_LO_HI(ICH_LR0, ICH_LRC0, ICH_LR0_EL2)
> +
> +#define read_gicreg(r)                 read_##r()
> +#define write_gicreg(v, r)             write_##r(v)
> +

Can you make this change a separate patch? It will make it easier to
merge if I can ack it as a standalone change. It will also give the last
patch a fantastic diffstat... ;-)

>  /* Low-level accessors */
>  
>  static inline void gic_write_eoir(u32 irq)
> diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
> index 58faff5..dfccf94 100644
> --- a/arch/arm/include/asm/kvm_asm.h
> +++ b/arch/arm/include/asm/kvm_asm.h
> @@ -68,6 +68,9 @@ extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
>  extern void __init_stage2_translation(void);
>  
>  extern void __kvm_hyp_reset(unsigned long);
> +
> +extern u64 __vgic_v3_get_ich_vtr_el2(void);
> +extern void __vgic_v3_init_lrs(void);
>  #endif
>  
>  #endif /* __ARM_KVM_ASM_H__ */
> diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h
> index 0b475d2..ed96cf9 100644
> --- a/arch/arm/include/asm/kvm_hyp.h
> +++ b/arch/arm/include/asm/kvm_hyp.h
> @@ -104,6 +104,9 @@ void __vgic_v2_restore_state(struct kvm_vcpu *vcpu);
>  void __sysreg_save_state(struct kvm_cpu_context *ctxt);
>  void __sysreg_restore_state(struct kvm_cpu_context *ctxt);
>  
> +void __vgic_v3_save_state(struct kvm_vcpu *vcpu);
> +void __vgic_v3_restore_state(struct kvm_vcpu *vcpu);
> +
>  void asmlinkage __vfp_save_state(struct vfp_hard_struct *vfp);
>  void asmlinkage __vfp_restore_state(struct vfp_hard_struct *vfp);
>  static inline bool __vfp_enabled(void)
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index a2b3eb3..b38c10c 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -84,6 +84,13 @@ struct kvm_regs {
>  #define KVM_VGIC_V2_DIST_SIZE		0x1000
>  #define KVM_VGIC_V2_CPU_SIZE		0x2000
>  
> +/* Supported VGICv3 address types  */
> +#define KVM_VGIC_V3_ADDR_TYPE_DIST	2
> +#define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
> +
> +#define KVM_VGIC_V3_DIST_SIZE		SZ_64K
> +#define KVM_VGIC_V3_REDIST_SIZE		(2 * SZ_64K)
> +
>  #define KVM_ARM_VCPU_POWER_OFF		0 /* CPU is started in OFF state */
>  #define KVM_ARM_VCPU_PSCI_0_2		1 /* CPU uses PSCI v0.2 */
>  
> diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile
> index 10d77a6..043d817f 100644
> --- a/arch/arm/kvm/Makefile
> +++ b/arch/arm/kvm/Makefile
> @@ -26,8 +26,10 @@ obj-y += $(KVM)/arm/vgic/vgic.o
>  obj-y += $(KVM)/arm/vgic/vgic-init.o
>  obj-y += $(KVM)/arm/vgic/vgic-irqfd.o
>  obj-y += $(KVM)/arm/vgic/vgic-v2.o
> +obj-y += $(KVM)/arm/vgic/vgic-v3.o
>  obj-y += $(KVM)/arm/vgic/vgic-mmio.o
>  obj-y += $(KVM)/arm/vgic/vgic-mmio-v2.o
> +obj-y += $(KVM)/arm/vgic/vgic-mmio-v3.o
>  obj-y += $(KVM)/arm/vgic/vgic-kvm-device.o
>  obj-y += $(KVM)/irqchip.o
>  obj-y += $(KVM)/arm/arch_timer.o
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 1bb2b79..10c0244 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -228,6 +228,36 @@ bool access_vm_reg(struct kvm_vcpu *vcpu,
>  	return true;
>  }
>  
> +static bool access_gic_sgi(struct kvm_vcpu *vcpu,
> +			   const struct coproc_params *p,
> +			   const struct coproc_reg *r)
> +{
> +	u64 reg;
> +
> +	if (!p->is_write)
> +		return read_from_write_only(vcpu, p);
> +
> +	reg = *vcpu_reg(vcpu, p->Rt2);
> +	reg <<= 32;

nit: can you write this as

	reg = (u64)*vcpu_reg(vcpu, p->Rt2) << 32;

which I find easier to read...

> +	reg |= *vcpu_reg(vcpu, p->Rt1) ;
> +
> +	vgic_v3_dispatch_sgi(vcpu, reg);
> +
> +	return true;
> +}
> +
> +static bool access_gic_sre(struct kvm_vcpu *vcpu,
> +			   const struct coproc_params *p,
> +			   const struct coproc_reg *r)
> +{
> +	if (p->is_write)
> +		return ignore_write(vcpu, p);
> +
> +	*vcpu_reg(vcpu, p->Rt1) = vcpu->arch.vgic_cpu.vgic_v3.vgic_sre;
> +
> +	return true;
> +}
> +
>  /*
>   * We could trap ID_DFR0 and tell the guest we don't support performance
>   * monitoring.  Unfortunately the patch to make the kernel check ID_DFR0 was
> @@ -361,10 +391,16 @@ static const struct coproc_reg cp15_regs[] = {
>  	{ CRn(10), CRm( 3), Op1( 0), Op2( 1), is32,
>  			access_vm_reg, reset_unknown, c10_AMAIR1},
>  
> +	/* ICC_SGI1R */
> +	{ CRm64(12), Op1( 0), is64, access_gic_sgi},
> +
>  	/* VBAR: swapped by interrupt.S. */
>  	{ CRn(12), CRm( 0), Op1( 0), Op2( 0), is32,
>  			NULL, reset_val, c12_VBAR, 0x00000000 },
>  
> +	/* ICC_SRE */
> +	{ CRn(12), CRm(12), Op1( 0), Op2(5), is32, access_gic_sre },
> +
>  	/* CONTEXTIDR/TPIDRURW/TPIDRURO/TPIDRPRW: swapped by interrupt.S. */
>  	{ CRn(13), CRm( 0), Op1( 0), Op2( 1), is32,
>  			access_vm_reg, reset_val, c13_CID, 0x00000000 },
> diff --git a/arch/arm/kvm/hyp/Makefile b/arch/arm/kvm/hyp/Makefile
> index 8dfa5f7..3023bb5 100644
> --- a/arch/arm/kvm/hyp/Makefile
> +++ b/arch/arm/kvm/hyp/Makefile
> @@ -5,6 +5,7 @@
>  KVM=../../../../virt/kvm
>  
>  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v2-sr.o
> +obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v3-sr.o
>  obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/timer-sr.o
>  
>  obj-$(CONFIG_KVM_ARM_HOST) += tlb.o
> diff --git a/arch/arm/kvm/hyp/switch.c b/arch/arm/kvm/hyp/switch.c
> index b13caa9..8409dd5 100644
> --- a/arch/arm/kvm/hyp/switch.c
> +++ b/arch/arm/kvm/hyp/switch.c
> @@ -14,6 +14,7 @@
>   * You should have received a copy of the GNU General Public License
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
> +#include <linux/jump_label.h>
>  
>  #include <asm/kvm_asm.h>
>  #include <asm/kvm_hyp.h>
> @@ -74,14 +75,21 @@ static void __hyp_text __deactivate_vm(struct kvm_vcpu *vcpu)
>  	write_sysreg(read_sysreg(MIDR), VPIDR);
>  }
>  
> +
>  static void __hyp_text __vgic_save_state(struct kvm_vcpu *vcpu)
>  {
> -	__vgic_v2_save_state(vcpu);
> +	if (static_branch_unlikely(&kvm_gicv3_cpuif))
> +		__vgic_v3_save_state(vcpu);
> +	else
> +		__vgic_v2_save_state(vcpu);
>  }
>  
>  static void __hyp_text __vgic_restore_state(struct kvm_vcpu *vcpu)
>  {
> -	__vgic_v2_restore_state(vcpu);
> +	if (static_branch_unlikely(&kvm_gicv3_cpuif))
> +		__vgic_v3_restore_state(vcpu);
> +	else
> +		__vgic_v2_restore_state(vcpu);
>  }
>  
>  static bool __hyp_text __populate_fault_info(struct kvm_vcpu *vcpu)
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index 7ba9164..6eaf12c 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -19,9 +19,6 @@ if VIRTUALIZATION
>  config KVM_ARM_VGIC_V3_ITS
>  	bool
>  
> -config KVM_ARM_VGIC_V3
> -	bool
> -
>  config KVM
>  	bool "Kernel-based Virtual Machine (KVM) support"
>  	depends on OF
> @@ -37,7 +34,6 @@ config KVM
>  	select KVM_VFIO
>  	select HAVE_KVM_EVENTFD
>  	select HAVE_KVM_IRQFD
> -	select KVM_ARM_VGIC_V3
>  	select KVM_ARM_VGIC_V3_ITS
>  	select KVM_ARM_PMU if HW_PERF_EVENTS
>  	select HAVE_KVM_MSI
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 19b698e..7462138 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -217,7 +217,6 @@ struct vgic_v2_cpu_if {
>  };
>  
>  struct vgic_v3_cpu_if {
> -#ifdef CONFIG_KVM_ARM_VGIC_V3
>  	u32		vgic_hcr;
>  	u32		vgic_vmcr;
>  	u32		vgic_sre;	/* Restored only, change ignored */
> @@ -227,7 +226,6 @@ struct vgic_v3_cpu_if {
>  	u32		vgic_ap0r[4];
>  	u32		vgic_ap1r[4];
>  	u64		vgic_lr[VGIC_V3_MAX_LRS];
> -#endif
>  };
>  
>  struct vgic_cpu {
> @@ -294,13 +292,7 @@ bool kvm_vcpu_has_pending_irqs(struct kvm_vcpu *vcpu);
>  void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu);
>  void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu);
>  
> -#ifdef CONFIG_KVM_ARM_VGIC_V3
>  void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
> -#else
> -static inline void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg)
> -{
> -}
> -#endif
>  
>  /**
>   * kvm_vgic_get_max_vcpus - Get the maximum number of VCPUs allowed by HW
> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
> index f3811b3..4dc026a 100644
> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
> @@ -71,7 +71,6 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write)
>  		addr_ptr = &vgic->vgic_cpu_base;
>  		alignment = SZ_4K;
>  		break;
> -#ifdef CONFIG_KVM_ARM_VGIC_V3
>  	case KVM_VGIC_V3_ADDR_TYPE_DIST:
>  		type_needed = KVM_DEV_TYPE_ARM_VGIC_V3;
>  		addr_ptr = &vgic->vgic_dist_base;
> @@ -82,7 +81,6 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write)
>  		addr_ptr = &vgic->vgic_redist_base;
>  		alignment = SZ_64K;
>  		break;
> -#endif
>  	default:
>  		r = -ENODEV;
>  		goto out;
> @@ -219,7 +217,6 @@ int kvm_register_vgic_device(unsigned long type)
>  		ret = kvm_register_device_ops(&kvm_arm_vgic_v2_ops,
>  					      KVM_DEV_TYPE_ARM_VGIC_V2);
>  		break;
> -#ifdef CONFIG_KVM_ARM_VGIC_V3
>  	case KVM_DEV_TYPE_ARM_VGIC_V3:
>  		ret = kvm_register_device_ops(&kvm_arm_vgic_v3_ops,
>  					      KVM_DEV_TYPE_ARM_VGIC_V3);
> @@ -230,7 +227,6 @@ int kvm_register_vgic_device(unsigned long type)
>  		ret = kvm_vgic_register_its_device();
>  #endif
>  		break;
> -#endif
>  	}
>  
>  	return ret;
> @@ -392,8 +388,6 @@ struct kvm_device_ops kvm_arm_vgic_v2_ops = {
>  
>  /* V3 ops */
>  
> -#ifdef CONFIG_KVM_ARM_VGIC_V3
> -
>  static int vgic_v3_set_attr(struct kvm_device *dev,
>  			    struct kvm_device_attr *attr)
>  {
> @@ -436,5 +430,3 @@ struct kvm_device_ops kvm_arm_vgic_v3_ops = {
>  	.get_attr = vgic_v3_get_attr,
>  	.has_attr = vgic_v3_has_attr,
>  };
> -
> -#endif /* CONFIG_KVM_ARM_VGIC_V3 */
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
> index 3bad3c5..e18b30d 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio.c
> @@ -550,11 +550,9 @@ int vgic_register_dist_iodev(struct kvm *kvm, gpa_t dist_base_address,
>  	case VGIC_V2:
>  		len = vgic_v2_init_dist_iodev(io_device);
>  		break;
> -#ifdef CONFIG_KVM_ARM_VGIC_V3
>  	case VGIC_V3:
>  		len = vgic_v3_init_dist_iodev(io_device);
>  		break;
> -#endif
>  	default:
>  		BUG_ON(1);
>  	}
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.h b/virt/kvm/arm/vgic/vgic-mmio.h
> index 80f92ce..4c34d39 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.h
> +++ b/virt/kvm/arm/vgic/vgic-mmio.h
> @@ -162,12 +162,10 @@ unsigned int vgic_v2_init_dist_iodev(struct vgic_io_device *dev);
>  
>  unsigned int vgic_v3_init_dist_iodev(struct vgic_io_device *dev);
>  
> -#ifdef CONFIG_KVM_ARM_VGIC_V3
>  u64 vgic_sanitise_outer_cacheability(u64 reg);
>  u64 vgic_sanitise_inner_cacheability(u64 reg);
>  u64 vgic_sanitise_shareability(u64 reg);
>  u64 vgic_sanitise_field(u64 reg, u64 field_mask, int field_shift,
>  			u64 (*sanitise_fn)(u64));
> -#endif
>  
>  #endif
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index 100045f..9d9e014 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -72,7 +72,6 @@ static inline void vgic_get_irq_kref(struct vgic_irq *irq)
>  	kref_get(&irq->refcount);
>  }
>  
> -#ifdef CONFIG_KVM_ARM_VGIC_V3
>  void vgic_v3_process_maintenance(struct kvm_vcpu *vcpu);
>  void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu);
>  void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr);
> @@ -91,60 +90,7 @@ bool vgic_has_its(struct kvm *kvm);
>  int kvm_vgic_register_its_device(void);
>  void vgic_enable_lpis(struct kvm_vcpu *vcpu);
>  int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi);
> -#endif
> -
>  #else
> -static inline void vgic_v3_process_maintenance(struct kvm_vcpu *vcpu)
> -{
> -}
> -
> -static inline void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
> -{
> -}
> -
> -static inline void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,
> -				       struct vgic_irq *irq, int lr)
> -{
> -}
> -
> -static inline void vgic_v3_clear_lr(struct kvm_vcpu *vcpu, int lr)
> -{
> -}
> -
> -static inline void vgic_v3_set_underflow(struct kvm_vcpu *vcpu)
> -{
> -}
> -
> -static inline
> -void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr)
> -{
> -}
> -
> -static inline
> -void vgic_v3_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr)
> -{
> -}
> -
> -static inline void vgic_v3_enable(struct kvm_vcpu *vcpu)
> -{
> -}
> -
> -static inline int vgic_v3_probe(const struct gic_kvm_info *info)
> -{
> -	return -ENODEV;
> -}
> -
> -static inline int vgic_v3_map_resources(struct kvm *kvm)
> -{
> -	return -ENODEV;
> -}
> -
> -static inline int vgic_register_redist_iodevs(struct kvm *kvm,
> -					      gpa_t dist_base_address)
> -{
> -	return -ENODEV;
> -}
> -
>  static inline int vgic_register_its_iodevs(struct kvm *kvm)
>  {
>  	return -ENODEV;
> 

It otherwise looks good to me.

Thanks,

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

^ permalink raw reply

* [PATCH 4/8] ARM: dts: bcm283x: Add a new EMMC pin group from the downstream tree.
From: Scott Branden @ 2016-09-09 16:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e61a4efa-3685-6694-5fda-3272f6605665@i2se.com>

Hi Stefan,

On 16-09-08 11:58 PM, Stefan Wahren wrote:
> Hi Scott,
>
> Am 08.09.2016 um 21:50 schrieb Scott Branden:
>> Hi Gerd/Eric
>>
>> On 16-09-08 12:44 AM, Gerd Hoffmann wrote:
>>> On Mi, 2016-09-07 at 20:13 +0200, Stefan Wahren wrote:
>>>>> Gerd Hoffmann <kraxel@redhat.com> hat am 7. September 2016 um 12:31
>>>>> geschrieben:
>>>>>
>>>>>
>>>>> From: Eric Anholt <eric@anholt.net>
>>>>>
>>>>> This will be used for having EMMC (sdhci-bcm2835.c) drive the
>>>>> wireless.
>>>>
>>>> sdhci-bcm2835.c has been replaced by sdhci-iproc.c
>>>
>>> Ah, right, I'll update the commit msg for v2.
>>>
>> Should we entirely remove sdhci-bcm2835.c to reduce confusion going
>> forward?
>
> did i missed something?
No - looks like I did.  Everything is cleaned up in latest.
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=46cd175c9788e39a1c57698ab58f34844d216428
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=7eb16493ecaa41ceeb448717ee2c59690cd9047f
>
>
>>
>>>> How about adding pull defines to include/dt-bindings/pinctrl/bcm2835.h?
>>>
>>> Already there (or do you mean something other than '#define
>>> BCM2835_FSEL_*' ?)
>>>
>>> cheers,
>>>   Gerd
>>>
>>
>> Thanks,
>>  Scott
>

^ permalink raw reply

* Improper TTBCR for arm 32bit kernel decompression
From: Srinivas Ramana @ 2016-09-09 16:34 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

While trying to boot arm-32 bit kernel, I came across a problem where 
TTBCR is in improper state. If the bootloader uses the long descriptor 
format and jumps to kernel decompressor code, TTBCR may not be in the 
right state. So, as soon as the MMU is enabled, execution can not 
proceed further.

Before enabling the MMU, it is required to clear the TTBCR.PD0 field to 
use TTBR0 for translation table walks. Also, TTBCR.N should be reset to 
'0' to indicate the correct base address width. The 'commit 
dbece45894d3a ("ARM: 7501/1: decompressor: reset ttbcr for VMSA ARMv7 
cores")' does the reset of TTBCR.N, but doesn't consider all the bits 
for the size of TTBCR.N.

when i tried the below change where i explicitly clear TTBCR.PD0 and use 
correct mask for TTBCR.N, I see proper memory after MMU is enabled and 
decompression succeeds.

Request your comments on the change below. If it looks good, I can 
submit a patch for inclusion.

---------------------8<----------------------------------
diff --git a/arch/arm/boot/compressed/head.S 
b/arch/arm/boot/compressed/head.S
index af11c2f..5769f1f 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -779,7 +779,8 @@ __armv7_mmu_cache_on:
                 orrne   r0, r0, #1              @ MMU enabled
                 movne   r1, #0xfffffffd         @ domain 0 = client
                 bic     r6, r6, #1 << 31        @ 32-bit translation system
-               bic     r6, r6, #3 << 0         @ use only ttbr0
+               bic     r6, r6, #7 << 0         @ width of base address 
field
+               bic     r6, r6, #1 << 4         @ use only ttbr0
                 mcrne   p15, 0, r3, c2, c0, 0   @ load page table pointer
                 mcrne   p15, 0, r1, c3, c0, 0   @ load domain access 
control
                 mcrne   p15, 0, r6, c2, c0, 2   @ load ttb control
---------------------8<----------------------------------

Thanks,
-- Srinivas R

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative 
Project.

^ permalink raw reply related

* [PATCH v3 3/8] arm64: KVM: Move vgic-v3 save/restore to virt/kvm/arm/hyp
From: Marc Zyngier @ 2016-09-09 16:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473350810-10857-4-git-send-email-vladimir.murzin@arm.com>

On 08/09/16 17:06, Vladimir Murzin wrote:
> So we can reuse the code under arch/arm
> 
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

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

^ permalink raw reply

* [PATCH v3 2/8] arm64: KVM: Move GIC accessors to arch_gicv3.h
From: Marc Zyngier @ 2016-09-09 16:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473350810-10857-3-git-send-email-vladimir.murzin@arm.com>

On 08/09/16 17:06, Vladimir Murzin wrote:
> Since we are going to share vgic-v3 save/restore code with ARM keep
> arch specific accessors separately.
> 
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

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

^ permalink raw reply

* [PATCH v2 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
From: Mark Rutland @ 2016-09-09 16:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160908125123.ypkye5nclsi5vx4f@localhost>

On Thu, Sep 08, 2016 at 01:51:24PM +0100, Catalin Marinas wrote:
> On Wed, Sep 07, 2016 at 04:20:55PM -0700, Kees Cook wrote:
> > On Fri, Sep 2, 2016 at 8:02 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > This is the second version of the arm64 PAN emulation by disabling
> > > TTBR0_EL1 accesses. The major change from v1 is the use of a thread_info
> > > member to store the real TTBR0_EL1 value. The advantage is slightly
> > > simpler assembler macros for uaccess_enable with the downside that
> > > switch_mm() must always update the saved ttbr0 even if there is no mm
> > > switch.
> > 
> > Is arm64 thread_info attached to the kernel stack? (i.e. is this
> > introducing a valuable target for stack-based attacks?)
> 
> Currently yes, thread_info is on the kernel stack. At some point we'll
> decouple it in a similar way to what x86 are doing/planning.

FWIW, I'm currently working on this (atop of Andy's x86 patches). The
IRQ stack work largely removed out dependence on the stack pointer to
find thread_info, and I have a plan for the remaining places.

There's a fair amount of ground work to do first (e.g. reworking headers
to avoid circular dependencies), but hopefully I'll have something that
I can share soon.

Thanks,
Mark.

^ permalink raw reply

* [PATCH v7 2/2] sdhci-of-arasan: Set controller to test mode when xlnx, fails-without-test-cd is present
From: Zach Brown @ 2016-09-09 16:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473438483-3857-1-git-send-email-zach.brown@ni.com>

The sdhci controller on xilinx zynq devices will not function unless
the CD bit is provided. http://www.xilinx.com/support/answers/61064.html
In cases where it is impossible to provide the CD bit in hardware,
setting the controller to test mode and then setting inserted to true
will get the controller to function without the CD bit.

When the device has the property xlnx,fails-without-test-cd the driver
changes the controller to test mode and sets test inserted to true to
make the controller function.

Signed-off-by: Zach Brown <zach.brown@ni.com>
---
 drivers/mmc/host/sdhci-of-arasan.c | 27 ++++++++++++++++++++++++++-
 drivers/mmc/host/sdhci.h           |  2 ++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 33601a8..da8e40a 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -26,6 +26,7 @@
 #include <linux/phy/phy.h>
 #include <linux/regmap.h>
 #include "sdhci-pltfm.h"
+#include <linux/of.h>
 
 #define SDHCI_ARASAN_CLK_CTRL_OFFSET	0x2c
 #define SDHCI_ARASAN_VENDOR_REGISTER	0x78
@@ -98,6 +99,10 @@ struct sdhci_arasan_data {
 
 	struct regmap	*soc_ctl_base;
 	const struct sdhci_arasan_soc_ctl_map *soc_ctl_map;
+	unsigned int	quirks; /* Arasan deviations from spec */
+
+/* Controller does not have CD wired and will not function normally without */
+#define SDHCI_ARASAN_QUIRK_FORCE_CDTEST	BIT(0)
 };
 
 static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {
@@ -245,12 +250,27 @@ static void sdhci_arasan_hs400_enhanced_strobe(struct mmc_host *mmc,
 	writel(vendor, host->ioaddr + SDHCI_ARASAN_VENDOR_REGISTER);
 }
 
+void sdhci_arasan_reset(struct sdhci_host *host, u8 mask)
+{
+	u8 ctrl;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
+
+	sdhci_reset(host, mask);
+
+	if (sdhci_arasan->quirks & SDHCI_ARASAN_QUIRK_FORCE_CDTEST) {
+		ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+		ctrl |= SDHCI_CTRL_CDTEST_INS | SDHCI_CTRL_CDTEST_EN;
+		sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+	}
+}
+
 static struct sdhci_ops sdhci_arasan_ops = {
 	.set_clock = sdhci_arasan_set_clock,
 	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
 	.get_timeout_clock = sdhci_arasan_get_timeout_clock,
 	.set_bus_width = sdhci_set_bus_width,
-	.reset = sdhci_reset,
+	.reset = sdhci_arasan_reset,
 	.set_uhs_signaling = sdhci_set_uhs_signaling,
 };
 
@@ -545,6 +565,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 	struct sdhci_host *host;
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_arasan_data *sdhci_arasan;
+	struct device_node *np = pdev->dev.of_node;
 
 	host = sdhci_pltfm_init(pdev, &sdhci_arasan_pdata,
 				sizeof(*sdhci_arasan));
@@ -599,6 +620,10 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 	}
 
 	sdhci_get_of_property(pdev);
+
+	if (of_property_read_bool(np, "xlnx,fails-without-test-cd"))
+		sdhci_arasan->quirks |= SDHCI_ARASAN_QUIRK_FORCE_CDTEST;
+
 	pltfm_host->clk = clk_xin;
 
 	if (of_device_is_compatible(pdev->dev.of_node,
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index a2bc9e1..c722cd2 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -84,6 +84,8 @@
 #define   SDHCI_CTRL_ADMA32	0x10
 #define   SDHCI_CTRL_ADMA64	0x18
 #define   SDHCI_CTRL_8BITBUS	0x20
+#define  SDHCI_CTRL_CDTEST_INS	0x40
+#define  SDHCI_CTRL_CDTEST_EN	0x80
 
 #define SDHCI_POWER_CONTROL	0x29
 #define  SDHCI_POWER_ON		0x01
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 1/2] sdhci-of-arasan: Add device tree parameter xlnx, fails-without-test-cd bit
From: Zach Brown @ 2016-09-09 16:28 UTC (permalink / raw)
  To: linux-arm-kernel

The sdhci controller on xilinx zynq devices will not function unless
the CD bit is provided. http://www.xilinx.com/support/answers/61064.html
In cases where it is impossible to provide the CD bit in hardware,
setting the controller to test mode and then setting inserted to true
will get the controller to function without the CD bit.

The device property "xlnx,fails-without-test-cd" will let the arasan
driver know the controller does not have the CD line wired and that the
controller does not function without it.

Signed-off-by: Zach Brown <zach.brown@ni.com>
---
v2:
 * improved commit messages
 * removed fake-cd device property
 * removed fake-cd quirk
 * use broken-cd device property
 * documented new usage of broken-cd
v3:
 * removed new usage of broken-cd
 * created fails-without-test-cd device property
 * created arasan controller specific quirk
v4:
 * changed fails-without-test-cd to xlnx-fails-without-test-cd
 * removed extra blank line
v5:
 * Fixed style mistake
 * Changed (1 << 0 ) to BIT(0)
v6:
 * Fixed style mistakes
 * Condensed unnecessarily long variable names
 * Removed line wraps that were no longer necessary.
 * Rebased changes off Ulf's mmc tree's next branch.
v7:
 * Removed erroneous re-creation of Optional Properties section
 * Changed xlnx-fails-without-test-cd to xlnx,fails-without-test-cd
 * Changed of_get_property to of_property_read_bool


 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
index 3404afa..49df630 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -36,6 +36,9 @@ Optional Properties:
   - #clock-cells: If specified this should be the value <0>.  With this property
     in place we will export a clock representing the Card Clock.  This clock
     is expected to be consumed by our PHY.  You must also specify
+  - xlnx,fails-without-test-cd: when present, the controller doesn't work when
+    the CD line is not connected properly, and the line is not connected
+    properly. Test mode can be used to force the controller to function.
 
 Example:
 	sdhci at e0100000 {
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 1/2] ARM: dts: Remove use of skeleton.dtsi from bcm283x.dtsi
From: Ian Campbell @ 2016-09-09 16:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5852905.snAkjWmlVZ@wuerfel>

On Fri, 2016-09-09 at 17:47 +0200, Arnd Bergmann wrote:
> On Friday, September 9, 2016 3:45:56 PM CEST Ian Campbell wrote:
> > 
> > On Tue, 2016-08-23 at 12:09 +0200, Arnd Bergmann wrote:
> > > 
> > > I had skipped the new message as well while sorting through 5000
> > > messages after my vacation, but I've now put it into my TODO
> > > folder
> > > and will get to it eventually.
> > 
> > Is it still on your TODO or should I do...
> > 
> > > 
> > > We sometimes miss stuff that is meant for arm-soc when it comes
> > > from people that don't normally send us patches. If you want to
> > > be sure to catch the attention, stick a 'GIT PULL' into the
> > > subject
> > > or send a ping on IRC.
> > 
> > ... one of these?
> > 
> > It'd be great to get this fixed in v4.8 so that the corresponding
> > device-tree.git tag will build.
> 
> Oh, it ended??up in my v4.9-todo folder, not 4.8, I misunderstood
> the urgency, sorry.

No worries.

> I've applied it to the fixes branch now.

Thank you!

Ian.

^ permalink raw reply

* [PATCH 8/8] ARM: pxa/lubbock: add pcmcia clock
From: Robert Jarzmik @ 2016-09-09 16:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160907091027.GI1041@n2100.armlinux.org.uk>

Russell King - ARM Linux <linux@armlinux.org.uk> writes:

> On Tue, Sep 06, 2016 at 07:17:09PM +0200, Robert Jarzmik wrote:
>> Russell King <rmk+kernel@armlinux.org.uk> writes:
>> 
>> > Add the required PCMCIA clock for the SA1111 "1800" device.  This clock
>> > is used to compute timing information for the PCMCIA interface in the
>> > SoC device, rather than the SA1111.  Hence, the provision of this clock
>> > is a convenience for the driver and does not reflect the hardware, so
>> > this must not be copied into DT.
>> >
>> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>> Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
>> But more importantly:
>> Tested-by: Robert Jarzmik <robert.jarzmik@free.fr>
>
> Thanks Robert.  I guess no one else is be interested in these patches,
> so I'm going to drop them into -next today, and send them on Friday or
> over this weekend.

Great !!!!

I'll keep the irq_get_chip() workaround in my private tree for now so that my
automated testing farm can also test the compact flash card. And yes, /dev/hda1
appeared on my lubbock, with a good old MSDOS filesystem :))))

Thanks for the time taken for explanations, I appreciated.

Cheers.

-- 
Robert

^ permalink raw reply

* [PATCH v3 1/8] arm64: KVM: Use static keys for selecting the GIC backend
From: Vladimir Murzin @ 2016-09-09 16:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57D2D486.3090100@arm.com>

On 09/09/16 16:25, Marc Zyngier wrote:
> On 09/09/16 16:14, Vladimir Murzin wrote:
>> On 09/09/16 15:17, Marc Zyngier wrote:
>>> On 09/09/16 14:45, Vladimir Murzin wrote:
>>>> On 09/09/16 10:33, Vladimir Murzin wrote:
>>>>> Hi Marc,
>>>>>
>>>>> On 09/09/16 10:19, Marc Zyngier wrote:
>>>>>>> Hi Vladimir,
>>>>>>>
>>>>> ...
>>>>>>>>>  
>>>>>>>>> +extern struct static_key_false kvm_gicv3_cpuif;
>>>>>>>
>>>>>>> I think we should follow the model set by kvm_vgic_global_state, which
>>>>>>> is declared in arm_vgic.h. Even better, we should *embed* the static key
>>>>>>> in this structure. This will reduce the clutter and we wouldn't have to
>>>>>>> deal with all the section stuff (the hyp_data thing is a good cleanup,
>>>>>>> but I'd like to see it as a separate patch if possible).
>>>>> Yes, it is what I was thinking about too, but was not sure about which
>>>>> way to go, so hyp_data seemed me something we might reuse latter.
>>>>> However, I agree that we can defer hyp_data thing...
>>>>>
>>>>
>>>> I've just tried it out and it seems that static keys are not happy to
>>>> accept a key after kern_hyp_va is applied at &kvm_vgic_global_state:
>>>
>>> Ah, there is a trick. You do not need kern_hyp_va at all, because this
>>> is not evaluated as an expression at runtime (so the pointer doesn't matter).
>>>
>>
>> Ah, right, thank for a tip! ;)
>>
>>>>> In file included from ./include/linux/jump_label.h:105:0,
>>>>>                  from arch/arm64/kvm/hyp/switch.c:19:
>>>>> ./arch/arm64/include/asm/jump_label.h: In function ?__guest_run?:
>>>>> ./arch/arm64/include/asm/jump_label.h:31:2: warning: asm operand 0 probably doesn?t match constraints
>>>>>   asm goto("1: nop\n\t"
>>>>>   ^
>>>>> ./arch/arm64/include/asm/jump_label.h:31:2: warning: asm operand 0 probably doesn?t match constraints
>>>>>   asm goto("1: nop\n\t"
>>>>>   ^
>>>>> ./arch/arm64/include/asm/jump_label.h:31:2: error: impossible constraint in ?asm?
>>>>>   asm goto("1: nop\n\t"
>>>>>   ^
>>>>> ./arch/arm64/include/asm/jump_label.h:31:2: error: impossible constraint in ?asm?
>>>>>   asm goto("1: nop\n\t"
>>>>>   ^
>>>>> make[1]: *** [arch/arm64/kvm/hyp/switch.o] Error 1
>>>>> make: *** [arch/arm64/kvm/hyp/switch.o] Error 2
>>>>
>>>> it looks like we cannot avoid hyp_data thing... if you don't mind I can
>>>> do hyp_data clean-up in separate patch. Alternatively, we can do
>>>> conversion to static keys for both architectures later as an
>>>> optimisation step.
>>>
>>> Can you try the above first? I've just tried the same approach with my
>>> vgic-trap series, and it compiles fine (untested though):
>>
>> I was about to try it out, but didn't manage to find a branch with
>> vgic-trap series, so I did a quick fixup for my series and now it is
>> running non-VHE boot tests and I don't expect issues with VHE one. I
>> think diff bellow should work runtime too, but if you do want me to give
>> it a try it'd be handy to have a branch I can pull from ;)
> 
> I think Christoffer has pulled it into kvmarm/queue.

Nothing has exploded so far for both architectures.

Cheers
Vladimir

^ permalink raw reply

* [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
From: Martin Blumenstingl @ 2016-09-09 16:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <m2sht9gkau.fsf@baylibre.com>

On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman <khilman@baylibre.com> wrote:
> However, the problem with all of the solutions proposed (runtime PM ones
> included) is that we're forcing a board-specific design issue (2 devices
> sharing a reset line) into a driver that should not have any
> board-specific assumptions in it.
>
> For example, if this driver is used on another platform where different
> PHYs have different reset lines, then one of them (the unlucky one who
> is not probed first) will never get reset.  So any form of per-device
> ref-counting is not a portable solution.
maybe we should also consider Ben's solution: he played with the USB
PHY on his Meson8b board. His approach was to have only one USB PHY
driver instance which exposes two PHYs.
The downside of this: the driver would have to know the offset of the
PHYs (0x0 for the first PHY, 0x20 for the second), but we could handle
the reset using runtime PM without any hacks.

I checked the USB PHY reference driver: it seems that there will be a
new USB PHY with the GXL/GXM SoCs.
So maybe we could live with the assumption that the PHYs are at
consecutive addresses.

> I'm not sure yet how the reset framework is supposed to handle shared
> reset lines, but that needs some investigation.  I quick glance and it
> seems that reset controllers can have shared lines, so that should be
> investigated.
unfortunately shared resets are not allowed to use reset_control_reset, see [0]


[0] http://lxr.free-electrons.com/source/drivers/reset/core.c#L102

^ permalink raw reply

* [PATCH 1/3] ARM: pxa: pxa_cplds: fix interrupt handling
From: Robert Jarzmik @ 2016-09-09 16:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473015587-15589-1-git-send-email-robert.jarzmik@free.fr>

Robert Jarzmik <robert.jarzmik@free.fr> writes:

> Since its initial commit, the driver is buggy for multiple interrupts
> handling. The translation from the former lubbock.c file was not
> complete, and might stall all interrupt handling when multiple
> interrupts occur.
>
> This is especially true when inside the interrupt handler and if a new
> interrupt comes and is not handled, leaving the output line still held,
> and not creating a transition as the GPIO block behind would expect to
> trigger another cplds_irq_handler() call.
...
Applied to pxa/for-next.

Cheers.

-- 
Robert

^ permalink raw reply

* [PATCH] [media] platform: constify vb2_ops structures
From: Benoit Parrot @ 2016-09-09 16:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473379150-17315-1-git-send-email-Julia.Lawall@lip6.fr>

Hi,

Thanks for the patch.

Julia Lawall <Julia.Lawall@lip6.fr> wrote on Fri [2016-Sep-09 01:59:10 +0200]:
> Check for vb2_ops structures that are only stored in the ops field of a
> vb2_queue structure.  That field is declared const, so vb2_ops structures
> that have this property can be declared as const also.
> 
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r disable optional_qualifier@
> identifier i;
> position p;
> @@
> static struct vb2_ops i at p = { ... };
> 
> @ok@
> identifier r.i;
> struct vb2_queue e;
> position p;
> @@
> e.ops = &i at p;
> 
> @bad@
> position p != {r.p,ok.p};
> identifier r.i;
> struct vb2_ops e;
> @@
> e at i@p
> 
> @depends on !bad disable optional_qualifier@
> identifier r.i;
> @@
> static
> +const
>  struct vb2_ops i = { ... };
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
>  drivers/media/platform/exynos-gsc/gsc-m2m.c              |    2 +-
>  drivers/media/platform/exynos4-is/fimc-capture.c         |    2 +-
>  drivers/media/platform/exynos4-is/fimc-m2m.c             |    2 +-
>  drivers/media/platform/m2m-deinterlace.c                 |    2 +-
>  drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c       |    2 +-
>  drivers/media/platform/mx2_emmaprp.c                     |    2 +-
>  drivers/media/platform/rcar-vin/rcar-dma.c               |    2 +-
>  drivers/media/platform/rcar_jpu.c                        |    2 +-
>  drivers/media/platform/s5p-g2d/g2d.c                     |    2 +-
>  drivers/media/platform/s5p-jpeg/jpeg-core.c              |    2 +-
>  drivers/media/platform/sh_vou.c                          |    2 +-
>  drivers/media/platform/soc_camera/atmel-isi.c            |    2 +-
>  drivers/media/platform/soc_camera/rcar_vin.c             |    2 +-
>  drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c |    2 +-
>  drivers/media/platform/sti/bdisp/bdisp-v4l2.c            |    2 +-

For the following 2 drivers,

>  drivers/media/platform/ti-vpe/cal.c                      |    2 +-
>  drivers/media/platform/ti-vpe/vpe.c                      |    2 +-

Reviewed-by: Benoit Parrot <bparrot@ti.com>

>  drivers/media/platform/vim2m.c                           |    2 +-
>  drivers/media/platform/xilinx/xilinx-dma.c               |    2 +-
>  19 files changed, 19 insertions(+), 19 deletions(-)

Regards,
Benoit Parrot

^ permalink raw reply

* [PATCH 3/3] tty/serial: at91: fix hardware handshake when DMA is not used
From: Richard Genoud @ 2016-09-09 16:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <15e0dbe0-6636-6bec-9622-be00056dae5e@atmel.com>

Hi Alex, Cyrille,


I did some tests to clear this HW handcheck history out.
NB: all those tests are with a 4.8-rc5 kernel on a AT91SAM9G35-CM board.

I couldn't test the PDC since, as Cyrille said, there's no such thing on
SAM9G35 USARTs.

The modes that DON'T WORK are:
ATMEL_US_USMODE_HWHS + DMA (RTS won't go up when the data are not read anymore)
ATMEL_US_USMODE_HWHS + PIO (RTS won't even go down when the port is open)

The modes that WORK are:
ATMEL_US_USMODE_NORMAL + DMA (no error, perfect transfer)
ATMEL_US_USMODE_NORMAL + PIO (some chars are eaten or nullified)


So, it seems that Atmel HW's guys where right when they said that the
HW handshake is completely broken on SAM9x5 (updating the manual with a
note saying that ATMEL_US_USMODE_HWHS shouldn't be use would be a good
idea !).

2016-09-07 18:59 GMT+02:00 Alexandre Belloni
<alexandre.belloni@free-electrons.com>:
> Hi,
>
> You forgot that the PDC can properly drive the RTS pin so all the
> platforms that are using the PDC will break after your patch.
You're right, I missed the platforms with PDC.
But right know, all SAM9x5 platforms are broken (and I guess SAM92xx
also), so I guess we'll find a way to correct that.

> Also, I believe the controller is able to drive the RTS and CTS pins
> when simply using PIOs (it knows when one complete character has been
> received and US_RHR has not yet been read). However, I didn't test.
> Maybe someone at Atmel can confirm.
I did see anything like that in SAM9G35 datasheet, and the tests I've done show
the contrary.
Actually, in SAM9G35 datasheet:
"When a character reception is completed, it is transferred to the
Receive Holding register (US_RHR) and the RXRDY bit in US_CSR rises.
If a character is completed while the RXRDY is set, the OVRE
(Overrun Error) bit is set.
The last character is transferred into US_RHR and overwrites the previous one."

> I think that 5be605ac9af9 is doing the right thing but I still can be
> convince otherwise ;).
I'll get to that.

2016-09-07 19:28 GMT+02:00 Cyrille Pitchen <cyrille.pitchen@atmel.com>:
> Hi Richard,
>
> For usart without FIFOs (hence before sama5d2), according to our designers, the
> RTS line could only been controlled by an internal PDC signal which doesn't
> exist with the DMA controller. Referring to its datasheet, the sam9g35 embeds
> DMA controllers. So if you enable the hardware handshaking feature on some
> usart, its RTS line won't be monitored at all. The hardware handshaking is
> broken on all SoCs using DMA controllers instead of PDC.
>
> With the sama5d2, our latest MPU, we fixed this issue by introducing an
> alternative mechanism which relies on 2 thresholds on the RX FIFO. So when
> FIFOs are available, those thresholds can be used to control the level of RTS
> line.
>
> Indeed I think a better test should be:
> if (!atmel_use_pdc_rx(port) && !atmel_use_fifo(port)) {
>         dev_info(port->dev, "not enabling hardware flow control
>         because neither the PDC nor the FIFO are available");
>         termios->c_cflags &= ~CRTSCTS;
> }
Maybe that's ok for SAMAD2+ platform, but not for older ones.
Actually, from all the tests I've done, the only mode that works for
SAM9x5 is:
mode |= ATMEL_US_USMODE_NORMAL;
and the CRTSCTS flag should *NOT* be removed.

So, IHMO, test should be:

if (atmel_use_pdc_rx(port) || atmel_use_fifo(port)) {
    mode |= ATMEL_US_USMODE_HWHS;
} else {
    mode |= ATMEL_US_USMODE_NORMAL;
}

Because nothing prevents the driver to drive the RTS pin itself, as it
is done in atmel_set_mctrl().
So, IHMO, commit 5be605ac9af9 is all wrong.
It basically says "If the controller can't handle RTS/CTS, then we don't
do HW handshake."
NO ! If the controller can't do it, the driver can do it for him, can't it ?!
(Well, from here, it truly seems it can !)


If you want to grab an atmel board and a scope to do some testing,
here's what I've done:

On one side:
stty -F /dev/ttyS2 115200 crtscts -opost clocal cread
exec 3>/dev/ttyS2
dd if=/home/rgenoud/dev/linux/MAINTAINERS bs=1M count=1 > /dev/ttyS2
exec 3>&-

On the board:
stty -F /dev/ttyS1 115200 crtscts -opost clocal cread
exec 4</dev/ttyS1
rm -f /tmp/rcv
cat <&4 >> /tmp/rcv
# then ctrl-C, and "cat <&4 >> /tmp/rcv" again
# at the ctrl-C, the RTS pin should go up

To enable dma in sam9x5 dts, I added
atmel,use-dma-rx;
in usart0: serial at f801c000


>
> We can double check once again with our designers to confirm that without FIFO
> the only way for the hardware handshaking to work is to use a PDC.
> So IMHO, your patch should not be applied, sorry!
>
> Best regards,
>
> Cyrille
>

Best regards.
Richard.

^ 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