* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
From: Javier Martinez Canillas @ 2016-12-16 12:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201612161246.12155@pali>
Hello Pali,
On 12/16/2016 08:46 AM, Pali Roh?r wrote:
> On Thursday 15 December 2016 01:09:20 Pali Roh?r wrote:
>> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux wrote:
>>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh?r wrote:
>>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
>>>> usage") broke support for setting cmdline on Nokia N900 via
>>>> CONFIG_CMDLINE.
>>>>
>>>> It is because arm code booted in DT mode parse cmdline only via
>>>> function early_init_dt_scan_chosen() and that function does not
>>>> fill variable boot_command_line when DTB does not contain /chosen
>>>> entry. It is called from function early_init_dt_scan_nodes() in
>>>> setup_machine_fdt().
>>>>
>>>> This patch fixes it by explicitly filling boot_command_line in
>>>> function setup_machine_fdt() after calling
>>>> early_init_dt_scan_nodes() in case boot_command_line still
>>>> remains empty.
>>>
>>> This looks like a hack.
>>>
>>> First, the matter of the ATAGs compatibility. The decompressor
>>> relies on there being a pre-existing /chosen node to insert the
>>> command line and other parameters into. If we've dropped it (by
>>> dropping skeleton.dtsi) then we've just regressed more than just
>>> N900 - the decompressor won't be able to merge the ATAGs into the
>>> concatenated FDT.
>>
>> Hm... I did not think about it. But right this can be broken too...
>
> Tony, Javier: are you aware of above ??? problem?
>
> It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
> skeleton.dtsi usage") should be really reverted.
>
I don't think reverting the mentioned commit is the correct fix for your
problem. We are trying to get rid of skeleton.dtsi for many reasons, see
commit commit ("3ebee5a2e141 arm64: dts: kill skeleton.dtsi").
Also, the chosen node is mentioned to be optional in the ePAPR document
and u-boot creates a chosen node if isn't found [0] so this issue is only
present in boards that don't use u-boot like the N900/N950/N9 phones.
So if NOLO doesn't do the same than u-boot and the kernel expects a chosen
node, I suggest to add an empty chosen node in the omap3-n900.dts and
omap3-n950-n9.dtsi device tree source files.
[0]: http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c9f7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* [PATCH v10 8/8] arm/arm64: Documentation: Update arm-vgic-v3.txt
From: Auger Eric @ 2016-12-16 12:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480576187-5012-9-git-send-email-vijay.kilari@gmail.com>
Hi Vijaya,
On 01/12/2016 08:09, vijay.kilari at gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> Update error code returned for Invalid CPU interface register
> value and access in AArch32 mode.
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> ---
> Documentation/virtual/kvm/devices/arm-vgic-v3.txt | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
> index 9348b3c..0f29850 100644
> --- a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
> +++ b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
> @@ -142,10 +142,12 @@ Groups:
> KVM_DEV_ARM_VGIC_CPU_SYSREGS accesses the CPU interface registers for the
> CPU specified by the mpidr field.
>
> + CPU interface registers access is not implemented for AArch32 mode.
> + Error -ENXIO is returned when accessed in AArch32 mode.
> Errors:
> -ENXIO: Getting or setting this register is not yet supported
> -EBUSY: VCPU is running
> - -EINVAL: Invalid mpidr supplied
> + -EINVAL: Invalid mpidr or register value supplied
>
>
> KVM_DEV_ARM_VGIC_GRP_NR_IRQS
> @@ -193,6 +195,11 @@ Groups:
>
> Bit[n] indicates the status for interrupt vINTID + n.
>
> + Getting or setting the level info for an edge-triggered interrupt is
> + not guaranteed to work.
I don't get this statement. is the API applicable to edge triggered IRQs?
Thanks
Eric
To restore the complete state of the VGIC, the
> + configuration (edge/level) of interrupts must be restored before
> + restoring the level.
> +
> SGIs and any interrupt with a higher ID than the number of interrupts
> supported, will be RAZ/WI. LPIs are always edge-triggered and are
> therefore not supported by this interface.
>
^ permalink raw reply
* [PATCH v10 6/8] arm/arm64: vgic: Implement VGICv3 CPU interface access
From: Auger Eric @ 2016-12-16 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480576187-5012-7-git-send-email-vijay.kilari@gmail.com>
Hi Vijaya,
On 01/12/2016 08:09, vijay.kilari at gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> VGICv3 CPU interface registers are accessed using
> KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
> as 64-bit. The cpu MPIDR value is passed along with register id.
> is used to identify the cpu for registers access.
s/is/It is
>
> The VM that supports SEIs expect it on destination machine to handle
> guest aborts and hence checked for ICC_CTLR_EL1.SEIS compatibility.
> Similarly, VM that supports Affinity Level 3 that is required for AArch64
> mode, is required to be supported on destination machine. Hence checked
> for ICC_CTLR_EL1.A3V compatibility.
We make sure ICC_CTLR_EL1 SEIS and A3V are compatible on source and target?
>
> The arch/arm64/kvm/vgic-sys-reg-v3.c handles read and write of VGIC
> CPU registers for AArch64.
>
> For AArch32 mode, arch/arm/kvm/vgic-v3-coproc.c file is created but
> APIs are not implemented.
>
> Updated arch/arm/include/uapi/asm/kvm.h with new definitions
> required to compile for AArch32.
>
> The version of VGIC v3 specification is define here
s/define/defined
> Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> ---
> arch/arm/include/uapi/asm/kvm.h | 2 +
> arch/arm/kvm/Makefile | 4 +-
> arch/arm/kvm/vgic-v3-coproc.c | 35 ++++
> arch/arm64/include/uapi/asm/kvm.h | 3 +
> arch/arm64/kvm/Makefile | 3 +-
> arch/arm64/kvm/vgic-sys-reg-v3.c | 338 ++++++++++++++++++++++++++++++++++++
> include/kvm/arm_vgic.h | 9 +
> virt/kvm/arm/vgic/vgic-kvm-device.c | 28 +++
> virt/kvm/arm/vgic/vgic-mmio-v3.c | 18 ++
> virt/kvm/arm/vgic/vgic-v3.c | 8 +
> virt/kvm/arm/vgic/vgic.h | 4 +
> 11 files changed, 449 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index 0ae6035..98658d9d 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -186,9 +186,11 @@ struct kvm_arch_memory_slot {
> (0xffffffffULL << KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT)
> #define KVM_DEV_ARM_VGIC_OFFSET_SHIFT 0
> #define KVM_DEV_ARM_VGIC_OFFSET_MASK (0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
> +#define KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK (0xffff)
> #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS 3
> #define KVM_DEV_ARM_VGIC_GRP_CTRL 4
> #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
> +#define KVM_DEV_ARM_VGIC_CPU_SYSREGS 6
> #define KVM_DEV_ARM_VGIC_CTRL_INIT 0
>
> /* KVM_IRQ_LINE irq field index values */
> diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile
> index d571243..68fb023 100644
> --- a/arch/arm/kvm/Makefile
> +++ b/arch/arm/kvm/Makefile
> @@ -7,7 +7,7 @@ ifeq ($(plus_virt),+virt)
> plus_virt_def := -DREQUIRES_VIRT=1
> endif
>
> -ccflags-y += -Iarch/arm/kvm
> +ccflags-y += -Iarch/arm/kvm -Ivirt/kvm/arm/vgic
> CFLAGS_arm.o := -I. $(plus_virt_def)
> CFLAGS_mmu.o := -I.
>
> @@ -20,7 +20,7 @@ kvm-arm-y = $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o $(KVM)/vf
> obj-$(CONFIG_KVM_ARM_HOST) += hyp/
> obj-y += kvm-arm.o init.o interrupts.o
> obj-y += arm.o handle_exit.o guest.o mmu.o emulate.o reset.o
> -obj-y += coproc.o coproc_a15.o coproc_a7.o mmio.o psci.o perf.o
> +obj-y += coproc.o coproc_a15.o coproc_a7.o mmio.o psci.o perf.o vgic-v3-coproc.o
> obj-y += $(KVM)/arm/aarch32.o
>
> obj-y += $(KVM)/arm/vgic/vgic.o
> diff --git a/arch/arm/kvm/vgic-v3-coproc.c b/arch/arm/kvm/vgic-v3-coproc.c
> new file mode 100644
> index 0000000..f41abf7
> --- /dev/null
> +++ b/arch/arm/kvm/vgic-v3-coproc.c
> @@ -0,0 +1,35 @@
> +/*
> + * VGIC system registers handling functions for AArch32 mode
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/kvm.h>
> +#include <linux/kvm_host.h>
> +#include <asm/kvm_emulate.h>
> +#include "vgic.h"
> +
> +int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
> + u64 *reg)
> +{
> + /*
> + * TODO: Implement for AArch32
> + */
> + return -ENXIO;
> +}
> +
> +int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write, u64 id,
> + u64 *reg)
> +{
> + /*
> + * TODO: Implement for AArch32
> + */
> + return -ENXIO;
> +}
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 56dc08d..91c7137 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -206,9 +206,12 @@ struct kvm_arch_memory_slot {
> (0xffffffffULL << KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT)
> #define KVM_DEV_ARM_VGIC_OFFSET_SHIFT 0
> #define KVM_DEV_ARM_VGIC_OFFSET_MASK (0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
> +#define KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK (0xffff)
> #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS 3
> #define KVM_DEV_ARM_VGIC_GRP_CTRL 4
> #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
> +#define KVM_DEV_ARM_VGIC_CPU_SYSREGS 6
> +
> #define KVM_DEV_ARM_VGIC_CTRL_INIT 0
>
> /* Device Control API on vcpu fd */
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index d50a82a..d89aa50 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -2,7 +2,7 @@
> # Makefile for Kernel-based Virtual Machine module
> #
>
> -ccflags-y += -Iarch/arm64/kvm
> +ccflags-y += -Iarch/arm64/kvm -Ivirt/kvm/arm/vgic
> CFLAGS_arm.o := -I.
> CFLAGS_mmu.o := -I.
>
> @@ -19,6 +19,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(ARM)/psci.o $(ARM)/perf.o
> kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o
> kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
> kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
> +kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o
> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/aarch32.o
>
> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic.o
> diff --git a/arch/arm64/kvm/vgic-sys-reg-v3.c b/arch/arm64/kvm/vgic-sys-reg-v3.c
> new file mode 100644
> index 0000000..76663f9
> --- /dev/null
> +++ b/arch/arm64/kvm/vgic-sys-reg-v3.c
> @@ -0,0 +1,338 @@
> +/*
> + * VGIC system registers handling functions for AArch64 mode
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/irqchip/arm-gic-v3.h>
> +#include <linux/kvm.h>
> +#include <linux/kvm_host.h>
> +#include <asm/kvm_emulate.h>
> +#include "vgic.h"
> +#include "sys_regs.h"
> +
> +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + u32 host_pri_bits, host_id_bits, host_seis, host_a3v, seis, a3v;
> + struct vgic_cpu *vgic_v3_cpu = &vcpu->arch.vgic_cpu;
> + struct vgic_vmcr vmcr;
> + u64 val;
> +
> + vgic_get_vmcr(vcpu, &vmcr);
> + if (p->is_write) {
> + val = p->regval;
> +
> + /*
> + * Disallow restoring VM state if not supported by this
> + * hardware.
> + */
> + host_pri_bits = ((val & ICC_CTLR_EL1_PRI_BITS_MASK) >>
> + ICC_CTLR_EL1_PRI_BITS_SHIFT) + 1;
I am confused by the "host" terminology. Those are the "source" values
we want to restore and we compare with the destination current value, right?
> + if (host_pri_bits > vgic_v3_cpu->num_pri_bits)
> + return false;
I am lost. Who did set num_pri_bits and num_id_bits we compare with?
Seis and a3v I get this is computed on probe
> +
> + vgic_v3_cpu->num_pri_bits = host_pri_bits;
> +
> + host_id_bits = (val & ICC_CTLR_EL1_ID_BITS_MASK) >>
> + ICC_CTLR_EL1_ID_BITS_SHIFT;
> + if (host_id_bits > vgic_v3_cpu->num_id_bits)
> + return false;
> +
> + vgic_v3_cpu->num_id_bits = host_id_bits;
> +
> + host_seis = ((kvm_vgic_global_state.ich_vtr_el2 &
> + ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT);
> + seis = (val & ICC_CTLR_EL1_SEIS_MASK) >>
> + ICC_CTLR_EL1_SEIS_SHIFT;
> + if (host_seis != seis)
> + return false;
> +
> + host_a3v = ((kvm_vgic_global_state.ich_vtr_el2 &
> + ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT);
> + a3v = (val & ICC_CTLR_EL1_A3V_MASK) >> ICC_CTLR_EL1_A3V_SHIFT;
> + if (host_a3v != a3v)
> + return false;
> +
> + vmcr.ctlr = val & ICC_CTLR_EL1_CBPR_MASK;
> + vmcr.ctlr |= val & ICC_CTLR_EL1_EOImode_MASK;
nit: I still don't get why the vmcr has CPBR and EOImode set with the
ICC_CTLR_EL1 layout and then this gets transformed in the proper vmcr
format by vgic_set_vmcr. This is confusing to me and would at least
deserve a comment attached to struct vgic_vmcr definition.
Why don't we set the vmcr.ctlr directly in its ICH_VMCR format? In
set/get_vmcr all the other struct vgic_vmcr fields are handled in
ICH_VMCR native layout except the ctrl field.
> + vgic_set_vmcr(vcpu, &vmcr);
> + } else {
> + val = 0;
> + val |= (vgic_v3_cpu->num_pri_bits - 1) <<
> + ICC_CTLR_EL1_PRI_BITS_SHIFT;
> + val |= vgic_v3_cpu->num_id_bits << ICC_CTLR_EL1_ID_BITS_SHIFT;
> + val |= ((kvm_vgic_global_state.ich_vtr_el2 &
> + ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT) <<
> + ICC_CTLR_EL1_SEIS_SHIFT;
> + val |= ((kvm_vgic_global_state.ich_vtr_el2 &
> + ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT) <<
> + ICC_CTLR_EL1_A3V_SHIFT;
> + val |= vmcr.ctlr & ICC_CTLR_EL1_CBPR_MASK;
> + val |= vmcr.ctlr & ICC_CTLR_EL1_EOImode_MASK;
> +
> + p->regval = val;
> + }
> +
> + return true;
> +}
> +
> +static bool access_gic_pmr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + struct vgic_vmcr vmcr;
> +
> + vgic_get_vmcr(vcpu, &vmcr);
> + if (p->is_write) {
> + vmcr.pmr = (p->regval & ICC_PMR_EL1_MASK) >> ICC_PMR_EL1_SHIFT;
> + vgic_set_vmcr(vcpu, &vmcr);
> + } else {
> + p->regval = (vmcr.pmr << ICC_PMR_EL1_SHIFT) & ICC_PMR_EL1_MASK;
> + }
> +
> + return true;
> +}
> +
> +static bool access_gic_bpr0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + struct vgic_vmcr vmcr;
> +
> + vgic_get_vmcr(vcpu, &vmcr);
> + if (p->is_write) {
> + vmcr.bpr = (p->regval & ICC_BPR0_EL1_MASK) >>
> + ICC_BPR0_EL1_SHIFT;
> + vgic_set_vmcr(vcpu, &vmcr);
> + } else {
> + p->regval = (vmcr.bpr << ICC_BPR0_EL1_SHIFT) &
> + ICC_BPR0_EL1_MASK;
> + }
> +
> + return true;
> +}
> +
> +static bool access_gic_bpr1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + struct vgic_vmcr vmcr;
> +
> + if (!p->is_write)
> + p->regval = 0;
> +
> + vgic_get_vmcr(vcpu, &vmcr);
> + if (!((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >> ICH_VMCR_CBPR_SHIFT)) {
> + if (p->is_write) {
> + vmcr.abpr = (p->regval & ICC_BPR1_EL1_MASK) >>
> + ICC_BPR1_EL1_SHIFT;
> + vgic_set_vmcr(vcpu, &vmcr);
> + } else {
> + p->regval = (vmcr.abpr << ICC_BPR1_EL1_SHIFT) &
> + ICC_BPR1_EL1_MASK;
> + }
> + } else {
> + if (!p->is_write)
> + p->regval = min((vmcr.bpr + 1), 7U);
> + }
> +
> + return true;
> +}
> +
> +static bool access_gic_grpen0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + struct vgic_vmcr vmcr;
> +
> + vgic_get_vmcr(vcpu, &vmcr);
> + if (p->is_write) {
> + vmcr.grpen0 = (p->regval & ICC_IGRPEN0_EL1_MASK) >>
> + ICC_IGRPEN0_EL1_SHIFT;
> + vgic_set_vmcr(vcpu, &vmcr);
> + } else {
> + p->regval = (vmcr.grpen0 << ICC_IGRPEN0_EL1_SHIFT) &
> + ICC_IGRPEN0_EL1_MASK;
> + }
> +
> + return true;
> +}
> +
> +static bool access_gic_grpen1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + struct vgic_vmcr vmcr;
> +
> + vgic_get_vmcr(vcpu, &vmcr);
> + if (p->is_write) {
> + vmcr.grpen1 = (p->regval & ICC_IGRPEN1_EL1_MASK) >>
> + ICC_IGRPEN1_EL1_SHIFT;
> + vgic_set_vmcr(vcpu, &vmcr);
> + } else {
> + p->regval = (vmcr.grpen1 << ICC_IGRPEN1_EL1_SHIFT) &
> + ICC_IGRPEN1_EL1_MASK;
> + }
> +
> + return true;
> +}
> +
> +static void vgic_v3_access_apr_reg(struct kvm_vcpu *vcpu,
> + struct sys_reg_params *p, u8 apr, u8 idx)
> +{
> + struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
> + uint32_t *ap_reg;
> +
> + if (apr)
> + ap_reg = &vgicv3->vgic_ap1r[idx];
> + else
> + ap_reg = &vgicv3->vgic_ap0r[idx];
> +
> + if (p->is_write)
> + *ap_reg = p->regval;
> + else
> + p->regval = *ap_reg;
> +}
> +
> +static bool access_gic_aprn(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> + const struct sys_reg_desc *r, u8 apr)
> +{
> + struct vgic_cpu *vgic_v3_cpu = &vcpu->arch.vgic_cpu;
> + u8 idx = r->Op2 & 3;
> +
> + /*
> + * num_pri_bits are initialized with HW supported values.
> + * We can rely safely on num_pri_bits even if VM has not
> + * restored ICC_CTLR_EL1 before restoring APnR registers.
> + */
> + switch (vgic_v3_cpu->num_pri_bits) {
> + case 7:
> + vgic_v3_access_apr_reg(vcpu, p, apr, idx);
> + break;
> + case 6:
> + if (idx > 1)
> + goto err;
> + vgic_v3_access_apr_reg(vcpu, p, apr, idx);
> + break;
> + default:
> + if (idx > 0)
> + goto err;
> + vgic_v3_access_apr_reg(vcpu, p, apr, idx);
> + }
> +
> + return true;
> +err:
> + if (!p->is_write)
> + p->regval = 0;
> +
> + return false;
> +}
> +
> +static bool access_gic_ap0r(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +
> +{
> + return access_gic_aprn(vcpu, p, r, 0);
> +}
> +
> +static bool access_gic_ap1r(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + return access_gic_aprn(vcpu, p, r, 1);
> +}
> +
> +static bool access_gic_sre(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
> +
> + /* Validate SRE bit */
> + if (p->is_write) {
> + if (!(p->regval & ICC_SRE_EL1_SRE))
> + return false;
> + } else {
> + p->regval = vgicv3->vgic_sre;
> + }
> +
> + return true;
> +}
> +static const struct sys_reg_desc gic_v3_icc_reg_descs[] = {
> + /* ICC_PMR_EL1 */
> + { Op0(3), Op1(0), CRn(4), CRm(6), Op2(0), access_gic_pmr },
> + /* ICC_BPR0_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(3), access_gic_bpr0 },
> + /* ICC_AP0R0_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(4), access_gic_ap0r },
> + /* ICC_AP0R1_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(5), access_gic_ap0r },
> + /* ICC_AP0R2_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(6), access_gic_ap0r },
> + /* ICC_AP0R3_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(7), access_gic_ap0r },
> + /* ICC_AP1R0_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(0), access_gic_ap1r },
> + /* ICC_AP1R1_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(1), access_gic_ap1r },
> + /* ICC_AP1R2_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(2), access_gic_ap1r },
> + /* ICC_AP1R3_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(3), access_gic_ap1r },
> + /* ICC_BPR1_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(3), access_gic_bpr1 },
> + /* ICC_CTLR_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(4), access_gic_ctlr },
> + /* ICC_SRE_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(5), access_gic_sre },
> + /* ICC_IGRPEN0_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(6), access_gic_grpen0 },
> + /* ICC_GRPEN1_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(7), access_gic_grpen1 },
> +};
> +
> +int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
> + u64 *reg)
> +{
> + struct sys_reg_params params;
> + u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
At the beginning I asked myself why we did not OR with KVM_REG_ARM64 as
stated in include/uapi/linux/kvm.h but then looking at
index_to_params() implementation it looks it is not used.
> +
> + params.regval = *reg;
> + params.is_write = is_write;
> + params.is_aarch32 = false;
> + params.is_32bit = false;
> +
> + if (find_reg_by_id(sysreg, ¶ms, gic_v3_icc_reg_descs,
> + ARRAY_SIZE(gic_v3_icc_reg_descs)))
> + return 0;
> +
> + return -ENXIO;
> +}
> +
> +int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write, u64 id,
> + u64 *reg)
> +{
> + struct sys_reg_params params;
> + const struct sys_reg_desc *r;
> + u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
> +
> + if (is_write)
> + params.regval = *reg;
> + params.is_write = is_write;
> + params.is_aarch32 = false;
> + params.is_32bit = false;
> +
> + r = find_reg_by_id(sysreg, ¶ms, gic_v3_icc_reg_descs,
> + ARRAY_SIZE(gic_v3_icc_reg_descs));
> + if (!r)
> + return -ENXIO;
> +
> + if (!r->access(vcpu, ¶ms, r))
> + return -EINVAL;
> +
> + if (!is_write)
> + *reg = params.regval;
> +
> + return 0;
> +}
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 002f092..730a18a 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -71,6 +71,9 @@ struct vgic_global {
>
> /* GIC system register CPU interface */
> struct static_key_false gicv3_cpuif;
> +
> + /* Cache ICH_VTR_EL2 reg value */
nit: comment does not bring much value I think
> + u32 ich_vtr_el2;
> };
>
> extern struct vgic_global kvm_vgic_global_state;
> @@ -269,6 +272,12 @@ struct vgic_cpu {
> u64 pendbaser;
>
> bool lpis_enabled;
> +
> + /* Cache guest priority bits */
> + u32 num_pri_bits;
> +
> + /* Cache guest interrupt ID bits */
> + u32 num_id_bits;
> };
>
> extern struct static_key_false vgic_v2_cpuif_trap;
> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
> index bc7de95..b6266fe 100644
> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
> @@ -16,6 +16,7 @@
> #include <linux/kvm_host.h>
> #include <kvm/arm_vgic.h>
> #include <linux/uaccess.h>
> +#include <asm/kvm_emulate.h>
not needed I think
Thanks
Eric
> #include <asm/kvm_mmu.h>
> #include "vgic.h"
>
> @@ -501,6 +502,14 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
> if (!is_write)
> *reg = tmp32;
> break;
> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> + u64 regid;
> +
> + regid = (attr->attr & KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK);
> + ret = vgic_v3_cpu_sysregs_uaccess(vcpu, is_write,
> + regid, reg);
> + break;
> + }
> default:
> ret = -EINVAL;
> break;
> @@ -534,6 +543,15 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
> reg = tmp32;
> return vgic_v3_attr_regs_access(dev, attr, ®, true);
> }
> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> + u64 __user *uaddr = (u64 __user *)(long)attr->addr;
> + u64 reg;
> +
> + if (get_user(reg, uaddr))
> + return -EFAULT;
> +
> + return vgic_v3_attr_regs_access(dev, attr, ®, true);
> + }
> }
> return -ENXIO;
> }
> @@ -560,6 +578,15 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
> tmp32 = reg;
> return put_user(tmp32, uaddr);
> }
> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> + u64 __user *uaddr = (u64 __user *)(long)attr->addr;
> + u64 reg;
> +
> + ret = vgic_v3_attr_regs_access(dev, attr, ®, false);
> + if (ret)
> + return ret;
> + return put_user(reg, uaddr);
> + }
> }
>
> return -ENXIO;
> @@ -578,6 +605,7 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
> break;
> case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
> case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS:
> return vgic_v3_has_attr_regs(dev, attr);
> case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
> return 0;
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> index e4799ae..51439c9 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> @@ -642,6 +642,24 @@ int vgic_v3_has_attr_regs(struct kvm_device *dev, struct kvm_device_attr *attr)
> nr_regions = ARRAY_SIZE(vgic_v3_rdbase_registers);
> break;
> }
> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> + u64 reg, id;
> + unsigned long vgic_mpidr, mpidr_reg;
> + struct kvm_vcpu *vcpu;
> +
> + vgic_mpidr = (attr->attr & KVM_DEV_ARM_VGIC_V3_MPIDR_MASK) >>
> + KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT;
> +
> + /* Convert plain mpidr value to MPIDR reg format */
> + mpidr_reg = VGIC_TO_MPIDR(vgic_mpidr);
> +
> + vcpu = kvm_mpidr_to_vcpu(dev->kvm, mpidr_reg);
> + if (!vcpu)
> + return -EINVAL;
> +
> + id = (attr->attr & KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK);
> + return vgic_v3_has_cpu_sysregs_attr(vcpu, 0, id, ®);
> + }
> default:
> return -ENXIO;
> }
> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
> index c21968b..c98a1c5 100644
> --- a/virt/kvm/arm/vgic/vgic-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-v3.c
> @@ -238,6 +238,13 @@ void vgic_v3_enable(struct kvm_vcpu *vcpu)
> vgic_v3->vgic_sre = 0;
> }
>
> + vcpu->arch.vgic_cpu.num_id_bits = (kvm_vgic_global_state.ich_vtr_el2 &
> + ICH_VTR_ID_BITS_MASK) >>
> + ICH_VTR_ID_BITS_SHIFT;
> + vcpu->arch.vgic_cpu.num_pri_bits = ((kvm_vgic_global_state.ich_vtr_el2 &
> + ICH_VTR_PRI_BITS_MASK) >>
> + ICH_VTR_PRI_BITS_SHIFT) + 1;
> +
> /* Get the show on the road... */
> vgic_v3->vgic_hcr = ICH_HCR_EN;
> }
> @@ -338,6 +345,7 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
> */
> kvm_vgic_global_state.nr_lr = (ich_vtr_el2 & 0xf) + 1;
> kvm_vgic_global_state.can_emulate_gicv2 = false;
> + kvm_vgic_global_state.ich_vtr_el2 = ich_vtr_el2;
>
> if (!info->vcpu.start) {
> kvm_info("GICv3: no GICV resource entry\n");
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index 9232791..eac272c 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -140,6 +140,10 @@ int vgic_v3_dist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> int offset, u32 *val);
> int vgic_v3_redist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> int offset, u32 *val);
> +int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> + u64 id, u64 *val);
> +int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
> + u64 *reg);
> int kvm_register_vgic_device(unsigned long type);
> void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
> void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
>
^ permalink raw reply
* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
From: Pali Rohár @ 2016-12-16 12:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <029ec718-89c0-b08e-de7e-279946c1a9bd@osg.samsung.com>
Hi!
On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote:
> Hello Pali,
>
> On 12/16/2016 08:46 AM, Pali Roh?r wrote:
> > On Thursday 15 December 2016 01:09:20 Pali Roh?r wrote:
> >> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux
> >> wrote:
> >>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh?r wrote:
> >>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
> >>>> usage") broke support for setting cmdline on Nokia N900 via
> >>>> CONFIG_CMDLINE.
> >>>>
> >>>> It is because arm code booted in DT mode parse cmdline only via
> >>>> function early_init_dt_scan_chosen() and that function does not
> >>>> fill variable boot_command_line when DTB does not contain
> >>>> /chosen entry. It is called from function
> >>>> early_init_dt_scan_nodes() in setup_machine_fdt().
> >>>>
> >>>> This patch fixes it by explicitly filling boot_command_line in
> >>>> function setup_machine_fdt() after calling
> >>>> early_init_dt_scan_nodes() in case boot_command_line still
> >>>> remains empty.
> >>>
> >>> This looks like a hack.
> >>>
> >>> First, the matter of the ATAGs compatibility. The decompressor
> >>> relies on there being a pre-existing /chosen node to insert the
> >>> command line and other parameters into. If we've dropped it (by
> >>> dropping skeleton.dtsi) then we've just regressed more than just
> >>> N900 - the decompressor won't be able to merge the ATAGs into the
> >>> concatenated FDT.
> >>
> >> Hm... I did not think about it. But right this can be broken
> >> too...
> >
> > Tony, Javier: are you aware of above ??? problem?
> >
> > It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
> > skeleton.dtsi usage") should be really reverted.
>
> I don't think reverting the mentioned commit is the correct fix for
> your problem. We are trying to get rid of skeleton.dtsi for many
> reasons, see commit commit ("3ebee5a2e141 arm64: dts: kill
> skeleton.dtsi").
$ git show 3ebee5a2e141
* The default empty /chosen and /aliases are somewhat useless...
That is not truth, they are not useless as Russell King wrote --
removing them break ATAG support.
(But that commit is for arm64 which probably is not using ATAGs... But I
do not know. At least it is not truth for 32bit arm.)
> Also, the chosen node is mentioned to be optional in the ePAPR
> document and u-boot creates a chosen node if isn't found [0] so this
> issue is only present in boards that don't use u-boot like the
> N900/N950/N9 phones.
Linux arm decompressor does not propagate ATAGs when /chosen is missing.
Sorry, but if for Linux /chosen is required (and without it is broken!)
then some ePARP document does not apply there. Either Linux code needs
to be fixed (so /chosen will be really only optional) or /chosen stay in
Linux required. There is no other option.
And I hope that U-boot is not the only one bootloader which Linux kernel
supports. I thought that I can use *any* bootloader to boot Linux kernel
not just U-Boot which is doing some magic...
With this step you are basically going to break booting Linux kernel
with all others bootloaders... And personally I really dislike this
idea.
> So if NOLO doesn't do the same than u-boot and the kernel expects a
> chosen node, I suggest to add an empty chosen node in the
> omap3-n900.dts and omap3-n950-n9.dtsi device tree source files.
That would fix a problem for N900, N950 and N9. But not for all other
ARM devices which bootloader pass some ATAGs.
IIRC rule of kernel is not to break compatibility and that commit
008a2ebcd677 really did it.
Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just saying
that it cause problems which need to be properly fixed. And if fixing
them is harder and will take more time, then correct option is to revert
008a2ebcd677 due to breaking support for more devices.
> [0]:
> http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c9f
> 7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
>
> Best regards,
--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161216/e81662a3/attachment-0001.sig>
^ permalink raw reply
* [RESEND PATCH 2/2] arm64: make WANT_HUGE_PMD_SHARE depends on HUGETLB_PAGE
From: Will Deacon @ 2016-12-16 12:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5853AF6D.20305@huawei.com>
On Fri, Dec 16, 2016 at 05:10:05PM +0800, zhong jiang wrote:
> On 2016/12/14 22:19, zhongjiang wrote:
> > From: zhong jiang <zhongjiang@huawei.com>
> >
> > when HUGETLB_PAGE is disable, WANT_HUGE_PMD_SHARE contains the
> > fuctions should not be use. therefore, we add the dependency.
> >
> > Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> > ---
> > arch/arm64/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 969ef88..694ca73 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -640,6 +640,7 @@ config SYS_SUPPORTS_HUGETLBFS
> >
> > config ARCH_WANT_HUGE_PMD_SHARE
> > def_bool y if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)
> > + depends on HUGETLB_PAGE
> >
> > config ARCH_HAS_CACHE_LINE_SIZE
> > def_bool y
> Hi,
> I still think it is a issue. Perhaps above changelog is unclear. Further explain is as follows.
> when hugetlb_pages is disable and arch_want_huge_pmd_share is enable, we maybe call
> huge_pmd_sahre in hugetlbpage.c, but the function actually is not definition as it is not
> exported. is it right ??
The only users of ARCH_WANT_HUGE_PMD_SHARE on arm64 are:
arch/arm64/mm/hugetlbpage.c: if (IS_ENABLED(CONFIG_ARCH_WANT_HUGE_PMD_SHARE) &&
mm/hugetlb.c:#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
mm/hugetlb.c:#else /* !CONFIG_ARCH_WANT_HUGE_PMD_SHARE */
mm/hugetlb.c:#endif /* CONFIG_ARCH_WANT_HUGE_PMD_SHARE */
and neither of those files are compiled if !HUGETLB_PAGE.
Are you actually seeing a problem here?
Will
^ permalink raw reply
* [PATCH] pinctrl: stm32: activate strict mux mode
From: Patrice Chotard @ 2016-12-16 12:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481725456-1030-1-git-send-email-gabriel.fernandez@st.com>
On 12/14/2016 03:24 PM, gabriel.fernandez at st.com wrote:
> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>
> This activates strict mode muxing for the STM32 pin controllers,
> as these do not allow GPIO and functions to use the same pin
> simultaneously.
>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> drivers/pinctrl/stm32/pinctrl-stm32.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
> index efc4371..c983a1e 100644
> --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
> +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
> @@ -631,6 +631,7 @@ static int stm32_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
> .get_function_groups = stm32_pmx_get_func_groups,
> .set_mux = stm32_pmx_set_mux,
> .gpio_set_direction = stm32_pmx_gpio_set_direction,
> + .strict = true,
> };
>
> /* Pinconf functions */
>
Hi Gaby
Acked-by: Patrice Chotard <patrice.chotard@st.com>
regards
^ permalink raw reply
* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
From: Javier Martinez Canillas @ 2016-12-16 12:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201612161332.36406@pali>
Hello Pali,
On 12/16/2016 09:32 AM, Pali Roh?r wrote:
> Hi!
>
> On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote:
>> Hello Pali,
>>
>> On 12/16/2016 08:46 AM, Pali Roh?r wrote:
>>> On Thursday 15 December 2016 01:09:20 Pali Roh?r wrote:
>>>> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux
>>>> wrote:
>>>>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh?r wrote:
>>>>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
>>>>>> usage") broke support for setting cmdline on Nokia N900 via
>>>>>> CONFIG_CMDLINE.
>>>>>>
>>>>>> It is because arm code booted in DT mode parse cmdline only via
>>>>>> function early_init_dt_scan_chosen() and that function does not
>>>>>> fill variable boot_command_line when DTB does not contain
>>>>>> /chosen entry. It is called from function
>>>>>> early_init_dt_scan_nodes() in setup_machine_fdt().
>>>>>>
>>>>>> This patch fixes it by explicitly filling boot_command_line in
>>>>>> function setup_machine_fdt() after calling
>>>>>> early_init_dt_scan_nodes() in case boot_command_line still
>>>>>> remains empty.
>>>>>
>>>>> This looks like a hack.
>>>>>
>>>>> First, the matter of the ATAGs compatibility. The decompressor
>>>>> relies on there being a pre-existing /chosen node to insert the
>>>>> command line and other parameters into. If we've dropped it (by
>>>>> dropping skeleton.dtsi) then we've just regressed more than just
>>>>> N900 - the decompressor won't be able to merge the ATAGs into the
>>>>> concatenated FDT.
>>>>
>>>> Hm... I did not think about it. But right this can be broken
>>>> too...
>>>
>>> Tony, Javier: are you aware of above ??? problem?
>>>
>>> It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
>>> skeleton.dtsi usage") should be really reverted.
>>
>> I don't think reverting the mentioned commit is the correct fix for
>> your problem. We are trying to get rid of skeleton.dtsi for many
>> reasons, see commit commit ("3ebee5a2e141 arm64: dts: kill
>> skeleton.dtsi").
>
> $ git show 3ebee5a2e141
>
> * The default empty /chosen and /aliases are somewhat useless...
>
> That is not truth, they are not useless as Russell King wrote --
> removing them break ATAG support.
>
> (But that commit is for arm64 which probably is not using ATAGs... But I
> do not know. At least it is not truth for 32bit arm.)
>
>> Also, the chosen node is mentioned to be optional in the ePAPR
>> document and u-boot creates a chosen node if isn't found [0] so this
>> issue is only present in boards that don't use u-boot like the
>> N900/N950/N9 phones.
>
> Linux arm decompressor does not propagate ATAGs when /chosen is missing.
> Sorry, but if for Linux /chosen is required (and without it is broken!)
> then some ePARP document does not apply there. Either Linux code needs
> to be fixed (so /chosen will be really only optional) or /chosen stay in
> Linux required. There is no other option.
>
> And I hope that U-boot is not the only one bootloader which Linux kernel
> supports. I thought that I can use *any* bootloader to boot Linux kernel
> not just U-Boot which is doing some magic...
>
> With this step you are basically going to break booting Linux kernel
> with all others bootloaders... And personally I really dislike this
> idea.
>
>> So if NOLO doesn't do the same than u-boot and the kernel expects a
>> chosen node, I suggest to add an empty chosen node in the
>> omap3-n900.dts and omap3-n950-n9.dtsi device tree source files.
>
> That would fix a problem for N900, N950 and N9. But not for all other
> ARM devices which bootloader pass some ATAGs.
>
> IIRC rule of kernel is not to break compatibility and that commit
> 008a2ebcd677 really did it.
>
> Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just saying
> that it cause problems which need to be properly fixed. And if fixing
> them is harder and will take more time, then correct option is to revert
> 008a2ebcd677 due to breaking support for more devices.
>
If you think that others boards may have the same issue, then you could
add an empty chosen node to omap3.dtsi. As I said I think that in practice
this will only be needed for the machines using NOLO but you are right
that in theory you could boot them using other bootloaders and having an
empty node doesn't cause any harm anyway.
>> [0]:
>> http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c9f
>> 7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
>>
>> Best regards,
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* [PATCH v9 01/11] arm/arm64: vgic: Implement support for userspace access
From: Auger Eric @ 2016-12-16 12:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CALicx6vgdVFMM7fACsGO6UKsXYA-oE__-e0pxvUyJsExKczoSQ@mail.gmail.com>
Hi Vijaya,
On 15/12/2016 08:36, Vijay Kilari wrote:
> On Tue, Dec 6, 2016 at 5:12 PM, Auger Eric <eric.auger@redhat.com> wrote:
>> Hi,
>>
>> On 28/11/2016 14:05, Christoffer Dall wrote:
>>> On Wed, Nov 23, 2016 at 06:31:48PM +0530, vijay.kilari at gmail.com wrote:
>>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>>
>>>> Read and write of some registers like ISPENDR and ICPENDR
>>>> from userspace requires special handling when compared to
>>>> guest access for these registers.
>>>>
>>>> Refer to Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>>>> for handling of ISPENDR, ICPENDR registers handling.
>>>>
>>>> Add infrastructure to support guest and userspace read
>>>> and write for the required registers
>>>> Also moved vgic_uaccess from vgic-mmio-v2.c to vgic-mmio.c
>>>>
>>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>> ---
>>>> virt/kvm/arm/vgic/vgic-mmio-v2.c | 25 ----------
>>>> virt/kvm/arm/vgic/vgic-mmio-v3.c | 102 ++++++++++++++++++++++++++++++++-------
>>>> virt/kvm/arm/vgic/vgic-mmio.c | 78 +++++++++++++++++++++++++++---
>>>> virt/kvm/arm/vgic/vgic-mmio.h | 19 ++++++++
>>>> 4 files changed, 175 insertions(+), 49 deletions(-)
>>>>
>>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c
>>>> index b44b359..0b32f40 100644
>>>> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
>>>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
>>>> @@ -406,31 +406,6 @@ int vgic_v2_has_attr_regs(struct kvm_device *dev, struct kvm_device_attr *attr)
>>>> return -ENXIO;
>>>> }
>>>>
>>>> -/*
>>>> - * When userland tries to access the VGIC register handlers, we need to
>>>> - * create a usable struct vgic_io_device to be passed to the handlers and we
>>>> - * have to set up a buffer similar to what would have happened if a guest MMIO
>>>> - * access occurred, including doing endian conversions on BE systems.
>>>> - */
>>>> -static int vgic_uaccess(struct kvm_vcpu *vcpu, struct vgic_io_device *dev,
>>>> - bool is_write, int offset, u32 *val)
>>>> -{
>>>> - unsigned int len = 4;
>>>> - u8 buf[4];
>>>> - int ret;
>>>> -
>>>> - if (is_write) {
>>>> - vgic_data_host_to_mmio_bus(buf, len, *val);
>>>> - ret = kvm_io_gic_ops.write(vcpu, &dev->dev, offset, len, buf);
>>>> - } else {
>>>> - ret = kvm_io_gic_ops.read(vcpu, &dev->dev, offset, len, buf);
>>>> - if (!ret)
>>>> - *val = vgic_data_mmio_bus_to_host(buf, len);
>>>> - }
>>>> -
>>>> - return ret;
>>>> -}
>>>> -
>>>> int vgic_v2_cpuif_uaccess(struct kvm_vcpu *vcpu, bool is_write,
>>>> int offset, u32 *val)
>>>> {
>>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>>>> index 50f42f0..8e76d04 100644
>>>> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
>>>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>>>> @@ -207,6 +207,66 @@ static unsigned long vgic_mmio_read_v3_idregs(struct kvm_vcpu *vcpu,
>>>> return 0;
>>>> }
>>>>
>>>> +static unsigned long vgic_v3_uaccess_read_pending(struct kvm_vcpu *vcpu,
>>>> + gpa_t addr, unsigned int len)
>>>> +{
>>>> + u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
>>>> + u32 value = 0;
>>>> + int i;
>>>> +
>>>> + /*
>>>> + * A level triggerred interrupt pending state is latched in both
>>>> + * "soft_pending" and "line_level" variables. Userspace will save
>>>> + * and restore soft_pending and line_level separately.
>>>> + * Refer to Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>>>> + * handling of ISPENDR and ICPENDR.
>>>> + */
>>>> + for (i = 0; i < len * 8; i++) {
>>>> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
>>>> +
>>>> + if (irq->config == VGIC_CONFIG_LEVEL && irq->soft_pending)
>>>> + value |= (1U << i);
>>>> + if (irq->config == VGIC_CONFIG_EDGE && irq->pending)
>>>> + value |= (1U << i);
>>>> +
>>>> + vgic_put_irq(vcpu->kvm, irq);
>>>> + }
>>>> +
>>>> + return value;
>>>> +}
>>>> +
>>>> +static void vgic_v3_uaccess_write_pending(struct kvm_vcpu *vcpu,
>>>> + gpa_t addr, unsigned int len,
>>>> + unsigned long val)
>>>> +{
>>>> + u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < len * 8; i++) {
>>>> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
>>>> +
>>>> + spin_lock(&irq->irq_lock);
>>>> + if (test_bit(i, &val)) {
>>>> + /* soft_pending is set irrespective of irq type
>>>> + * (level or edge) to avoid dependency that VM should
>>>> + * restore irq config before pending info.
>>>> + */
>>>
>>> nit: kernel commenting style
>>>
>>>> + irq->pending = true;
>>>> + irq->soft_pending = true;
>>>> + vgic_queue_irq_unlock(vcpu->kvm, irq);
>>>> + } else {
>>>> + irq->soft_pending = false;
>>>> + if (irq->config == VGIC_CONFIG_EDGE ||
>>>> + (irq->config == VGIC_CONFIG_LEVEL &&
>>>> + !irq->line_level))
>>>> + irq->pending = false;
>> I am confused by the comment above. Since we test the irq config here
>> don't we assume the config was restored before the pending state?
>>>> + spin_unlock(&irq->irq_lock);
>>>> + }
>>>> +
>>>> + vgic_put_irq(vcpu->kvm, irq);
>>>> + }
>>>> +}
>>>> +
>>>> /* We want to avoid outer shareable. */
>>>> u64 vgic_sanitise_shareability(u64 field)
>>>> {
>>>> @@ -356,7 +416,7 @@ static void vgic_mmio_write_pendbase(struct kvm_vcpu *vcpu,
>>>> * We take some special care here to fix the calculation of the register
>>>> * offset.
>>>> */
>>>> -#define REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(off, rd, wr, bpi, acc) \
>>>> +#define REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(off, rd, wr, ur, uw, bpi, acc) \
>>>> { \
>>>> .reg_offset = off, \
>>>> .bits_per_irq = bpi, \
>>>> @@ -371,6 +431,8 @@ static void vgic_mmio_write_pendbase(struct kvm_vcpu *vcpu,
>>>> .access_flags = acc, \
>>>> .read = rd, \
>>>> .write = wr, \
>>>> + .uaccess_read = ur, \
>>>> + .uaccess_write = uw, \
>>>> }
>>>>
>>>> static const struct vgic_register_region vgic_v3_dist_registers[] = {
>>>> @@ -378,40 +440,42 @@ static void vgic_mmio_write_pendbase(struct kvm_vcpu *vcpu,
>>>> vgic_mmio_read_v3_misc, vgic_mmio_write_v3_misc, 16,
>>>> VGIC_ACCESS_32bit),
>>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_IGROUPR,
>>>> - vgic_mmio_read_rao, vgic_mmio_write_wi, 1,
>>>> + vgic_mmio_read_rao, vgic_mmio_write_wi, NULL, NULL, 1,
>>>> VGIC_ACCESS_32bit),
>>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISENABLER,
>>>> - vgic_mmio_read_enable, vgic_mmio_write_senable, 1,
>>>> + vgic_mmio_read_enable, vgic_mmio_write_senable, NULL, NULL, 1,
>>>> VGIC_ACCESS_32bit),
>>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICENABLER,
>>>> - vgic_mmio_read_enable, vgic_mmio_write_cenable, 1,
>>>> + vgic_mmio_read_enable, vgic_mmio_write_cenable, NULL, NULL, 1,
>>>> VGIC_ACCESS_32bit),
>>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISPENDR,
>>>> - vgic_mmio_read_pending, vgic_mmio_write_spending, 1,
>>>> + vgic_mmio_read_pending, vgic_mmio_write_spending,
>>>> + vgic_v3_uaccess_read_pending, vgic_v3_uaccess_write_pending, 1,
>>>> VGIC_ACCESS_32bit),
>>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICPENDR,
>>>> - vgic_mmio_read_pending, vgic_mmio_write_cpending, 1,
>>>> + vgic_mmio_read_pending, vgic_mmio_write_cpending,
>>>> + vgic_mmio_read_raz, vgic_mmio_write_wi, 1,
>>>> VGIC_ACCESS_32bit),
>>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISACTIVER,
>>>> - vgic_mmio_read_active, vgic_mmio_write_sactive, 1,
>>>> + vgic_mmio_read_active, vgic_mmio_write_sactive, NULL, NULL, 1,
>>>> VGIC_ACCESS_32bit),
>>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICACTIVER,
>>>> - vgic_mmio_read_active, vgic_mmio_write_cactive, 1,
>>>> + vgic_mmio_read_active, vgic_mmio_write_cactive, NULL, NULL, 1,
>>>> VGIC_ACCESS_32bit),
>>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_IPRIORITYR,
>>>> - vgic_mmio_read_priority, vgic_mmio_write_priority, 8,
>>>> - VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
>>>> + vgic_mmio_read_priority, vgic_mmio_write_priority, NULL, NULL,
>>>> + 8, VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
>>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ITARGETSR,
>>>> - vgic_mmio_read_raz, vgic_mmio_write_wi, 8,
>>>> + vgic_mmio_read_raz, vgic_mmio_write_wi, NULL, NULL, 8,
>>>> VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
>>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICFGR,
>>>> - vgic_mmio_read_config, vgic_mmio_write_config, 2,
>>>> + vgic_mmio_read_config, vgic_mmio_write_config, NULL, NULL, 2,
>>>> VGIC_ACCESS_32bit),
>>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_IGRPMODR,
>>>> - vgic_mmio_read_raz, vgic_mmio_write_wi, 1,
>>>> + vgic_mmio_read_raz, vgic_mmio_write_wi, NULL, NULL, 1,
>>>> VGIC_ACCESS_32bit),
>>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_IROUTER,
>>>> - vgic_mmio_read_irouter, vgic_mmio_write_irouter, 64,
>>>> + vgic_mmio_read_irouter, vgic_mmio_write_irouter, NULL, NULL, 64,
>>>> VGIC_ACCESS_64bit | VGIC_ACCESS_32bit),
>>>> REGISTER_DESC_WITH_LENGTH(GICD_IDREGS,
>>>> vgic_mmio_read_v3_idregs, vgic_mmio_write_wi, 48,
>>>> @@ -449,11 +513,13 @@ static void vgic_mmio_write_pendbase(struct kvm_vcpu *vcpu,
>>>> REGISTER_DESC_WITH_LENGTH(GICR_ICENABLER0,
>>>> vgic_mmio_read_enable, vgic_mmio_write_cenable, 4,
>>>> VGIC_ACCESS_32bit),
>>>> - REGISTER_DESC_WITH_LENGTH(GICR_ISPENDR0,
>>>> - vgic_mmio_read_pending, vgic_mmio_write_spending, 4,
>>>> + REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_ISPENDR0,
>>>> + vgic_mmio_read_pending, vgic_mmio_write_spending,
>>>> + vgic_v3_uaccess_read_pending, vgic_v3_uaccess_write_pending, 4,
>>>> VGIC_ACCESS_32bit),
>>>> - REGISTER_DESC_WITH_LENGTH(GICR_ICPENDR0,
>>>> - vgic_mmio_read_pending, vgic_mmio_write_cpending, 4,
>>>> + REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_ICPENDR0,
>>>> + vgic_mmio_read_pending, vgic_mmio_write_cpending,
>>>> + vgic_mmio_read_raz, vgic_mmio_write_wi, 4,
>>>> VGIC_ACCESS_32bit),
>>>> REGISTER_DESC_WITH_LENGTH(GICR_ISACTIVER0,
>>>> vgic_mmio_read_active, vgic_mmio_write_sactive, 4,
>>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
>>>> index ebe1b9f..d5f3ee2 100644
>>>> --- a/virt/kvm/arm/vgic/vgic-mmio.c
>>>> +++ b/virt/kvm/arm/vgic/vgic-mmio.c
>>>> @@ -484,6 +484,74 @@ static bool check_region(const struct kvm *kvm,
>>>> return false;
>>>> }
>>>>
>>>> +static const struct vgic_register_region *
>>>> +vgic_get_mmio_region(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev,
>>>> + gpa_t addr, int len)
>>>> +{
>>>> + const struct vgic_register_region *region;
>>>> +
>>>> + region = vgic_find_mmio_region(iodev->regions, iodev->nr_regions,
>>>> + addr - iodev->base_addr);
>>>> + if (!region || !check_region(vcpu->kvm, region, addr, len))
>>>> + return NULL;
>>>> +
>>>> + return region;
>>>> +}
>>>> +
>>>> +static int vgic_uaccess_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
>>>> + gpa_t addr, u32 *val)
>>>> +{
>>>> + struct vgic_io_device *iodev = kvm_to_vgic_iodev(dev);
>>>> + const struct vgic_register_region *region;
>>>> + struct kvm_vcpu *r_vcpu;
>>>> +
>>>> + region = vgic_get_mmio_region(vcpu, iodev, addr, sizeof(u32));
>>>> + if (!region) {
>>>> + *val = 0;
>>>> + return 0;
>> do we really want to return 0 here? -ENXIO?
>> I see that dispatch_mmio_read/write return 0 in that case but I don't
>> see any reason either? Other kvm_io_device_ops seem to return
>> -EOPNOTSUPP in such a case.
>
> Yes, This was discussed and decided to fix it outside of this series.
>
> https://www.spinics.net/lists/arm-kernel/msg533695.html
OK. Sorry I missed that. Other comments I sent on v9 remain applicable
on v10 I think.
Thanks
Eric
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH v10 7/8] arm/arm64: vgic: Implement KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO ioctl
From: Peter Maydell @ 2016-12-16 12:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4272e489-7aa4-b77d-ec09-22b58af78336@redhat.com>
On 16 December 2016 at 12:07, Auger Eric <eric.auger@redhat.com> wrote:
> Hi Vijaya,
>
> On 01/12/2016 08:09, vijay.kilari at gmail.com wrote:
>> +void vgic_write_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid,
>> + const u64 val)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < 32; i++) {
>> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
>> +
> same as above.
>> + spin_lock(&irq->irq_lock);
>> + if (val & (1U << i)) {
>> + if (irq->config == VGIC_CONFIG_LEVEL) {
>> + irq->line_level = true;
>> + irq->pending = true;
>> + vgic_queue_irq_unlock(vcpu->kvm, irq);
>> + } else {
>> + spin_unlock(&irq->irq_lock);
>> + }
>> + } else {
>> + if (irq->config == VGIC_CONFIG_EDGE ||
> can't we just ignore VGIC_CONFIG_EDGE case for which line_level is not
> modeled?
>> + (irq->config == VGIC_CONFIG_LEVEL &&
>> + !irq->soft_pending))
>> + irq->line_level = false;
> To me the line level does not depend on the soft_pending bit. The
> pending state depends on both.
Without having looked at the details, it seems surprising to
me that the implementation of "set line level to X" is not
"set irq->line_level to X; figure out consequences and update
other state as necessary"...
thanks
-- PMM
^ permalink raw reply
* [PATCH] ARM: dts: sun8i-q8-common: enable bluetooth on SDIO Wi-Fi
From: Maxime Ripard @ 2016-12-16 12:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161213233658.atGuNCNY@smtp1h.mail.yandex.net>
On Fri, Dec 09, 2016 at 07:49:00PM +0800, Icenowy Zheng wrote:
>
> 2016?12?9? ??4:07? Maxime Ripard <maxime.ripard@free-electrons.com>???
> >
> > On Tue, Dec 06, 2016 at 04:08:38PM +0800, Icenowy Zheng wrote:
> > > Some SDIO Wi-Fi chips (such as RTL8703AS) have a UART bluetooth, which
> > > has a dedicated enable pin (PL8 in the reference design).
> > >
> > > Enable the pin in the same way as the WLAN enable pins.
> > >
> > > Tested on an A33 Q8 tablet with RTL8703AS.
> > >
> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> > > ---
> > >
> > > This patch should be coupled with the uart1 node patch I send before:
> > > http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/471997.html
> > >
> > > For RTL8703AS, the rtl8723bs bluetooth code is used, which can be retrieve from:
> > > https://github.com/lwfinger/rtl8723bs_bt
> > >
> > >? arch/arm/boot/dts/sun8i-q8-common.dtsi | 2 +-
> > >? 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/boot/dts/sun8i-q8-common.dtsi b/arch/arm/boot/dts/sun8i-q8-common.dtsi
> > > index c676940..4aeb5bb 100644
> > > --- a/arch/arm/boot/dts/sun8i-q8-common.dtsi
> > > +++ b/arch/arm/boot/dts/sun8i-q8-common.dtsi
> > > @@ -88,7 +88,7 @@
> > >?
> > >? &r_pio {
> > >? wifi_pwrseq_pin_q8: wifi_pwrseq_pin at 0 {
> > > - pins = "PL6", "PL7", "PL11";
> > > + pins = "PL6", "PL7", "PL8", "PL11";
> > >? function = "gpio_in";
> > >? bias-pull-up;
> > >? };
> >
> > There's several things wrong here. The first one is that you rely
> > solely on the pinctrl state to maintain a reset line. This is very
> > fragile (especially since the GPIO pinctrl state are likely to go away
> > at some point), but it also means that if your driver wants to recover
> > from that situation at some point, it won't work.
> >
> > The other one is that the bluetooth and wifi chips are two devices in
> > linux, and you assign that pin to the wrong device (wifi).
> >
> > rfkill-gpio is made just for that, so please use it.
>
> The GPIO is not for the radio, but for the full Bluetooth part.
I know.
> If it's set to 0, then the bluetooth part will reset, and the
> hciattach will fail.
Both rfkill-gpio and rfkill-regulator will shutdown when called
(either by poking the reset pin or shutting down the regulator), so
that definitely seems like an expected behavior to put the device in
reset.
> The BSP uses this as a rfkill, and the result is that the bluetooth
> on/off switch do not work properly.
Then rfkill needs fixing, but working around it by hoping that the
core will probe an entirely different device, and enforcing a default
that the rest of the kernel might or might not change is both fragile
and wrong.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161216/7af246ff/attachment.sig>
^ permalink raw reply
* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
From: Pali Rohár @ 2016-12-16 12:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5c56e769-76be-295e-b655-8431dde35370@osg.samsung.com>
On Friday 16 December 2016 13:38:34 Javier Martinez Canillas wrote:
> Hello Pali,
>
> On 12/16/2016 09:32 AM, Pali Roh?r wrote:
> > Hi!
> >
> > On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote:
> >> Hello Pali,
> >>
> >> On 12/16/2016 08:46 AM, Pali Roh?r wrote:
> >>> On Thursday 15 December 2016 01:09:20 Pali Roh?r wrote:
> >>>> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux
> >>>>
> >>>> wrote:
> >>>>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh?r wrote:
> >>>>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
> >>>>>> usage") broke support for setting cmdline on Nokia N900 via
> >>>>>> CONFIG_CMDLINE.
> >>>>>>
> >>>>>> It is because arm code booted in DT mode parse cmdline only
> >>>>>> via function early_init_dt_scan_chosen() and that function
> >>>>>> does not fill variable boot_command_line when DTB does not
> >>>>>> contain /chosen entry. It is called from function
> >>>>>> early_init_dt_scan_nodes() in setup_machine_fdt().
> >>>>>>
> >>>>>> This patch fixes it by explicitly filling boot_command_line in
> >>>>>> function setup_machine_fdt() after calling
> >>>>>> early_init_dt_scan_nodes() in case boot_command_line still
> >>>>>> remains empty.
> >>>>>
> >>>>> This looks like a hack.
> >>>>>
> >>>>> First, the matter of the ATAGs compatibility. The decompressor
> >>>>> relies on there being a pre-existing /chosen node to insert the
> >>>>> command line and other parameters into. If we've dropped it
> >>>>> (by dropping skeleton.dtsi) then we've just regressed more
> >>>>> than just N900 - the decompressor won't be able to merge the
> >>>>> ATAGs into the concatenated FDT.
> >>>>
> >>>> Hm... I did not think about it. But right this can be broken
> >>>> too...
> >>>
> >>> Tony, Javier: are you aware of above ??? problem?
> >>>
> >>> It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
> >>> skeleton.dtsi usage") should be really reverted.
> >>
> >> I don't think reverting the mentioned commit is the correct fix
> >> for your problem. We are trying to get rid of skeleton.dtsi for
> >> many reasons, see commit commit ("3ebee5a2e141 arm64: dts: kill
> >> skeleton.dtsi").
> >
> > $ git show 3ebee5a2e141
> >
> > * The default empty /chosen and /aliases are somewhat useless...
> >
> > That is not truth, they are not useless as Russell King wrote --
> > removing them break ATAG support.
> >
> > (But that commit is for arm64 which probably is not using ATAGs...
> > But I do not know. At least it is not truth for 32bit arm.)
> >
> >> Also, the chosen node is mentioned to be optional in the ePAPR
> >> document and u-boot creates a chosen node if isn't found [0] so
> >> this issue is only present in boards that don't use u-boot like
> >> the N900/N950/N9 phones.
> >
> > Linux arm decompressor does not propagate ATAGs when /chosen is
> > missing. Sorry, but if for Linux /chosen is required (and without
> > it is broken!) then some ePARP document does not apply there.
> > Either Linux code needs to be fixed (so /chosen will be really
> > only optional) or /chosen stay in Linux required. There is no
> > other option.
> >
> > And I hope that U-boot is not the only one bootloader which Linux
> > kernel supports. I thought that I can use *any* bootloader to boot
> > Linux kernel not just U-Boot which is doing some magic...
> >
> > With this step you are basically going to break booting Linux
> > kernel with all others bootloaders... And personally I really
> > dislike this idea.
> >
> >> So if NOLO doesn't do the same than u-boot and the kernel expects
> >> a chosen node, I suggest to add an empty chosen node in the
> >> omap3-n900.dts and omap3-n950-n9.dtsi device tree source files.
> >
> > That would fix a problem for N900, N950 and N9. But not for all
> > other ARM devices which bootloader pass some ATAGs.
> >
> > IIRC rule of kernel is not to break compatibility and that commit
> > 008a2ebcd677 really did it.
> >
> > Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just
> > saying that it cause problems which need to be properly fixed. And
> > if fixing them is harder and will take more time, then correct
> > option is to revert 008a2ebcd677 due to breaking support for more
> > devices.
>
> If you think that others boards may have the same issue, then you
> could add an empty chosen node to omap3.dtsi. As I said I think that
> in practice this will only be needed for the machines using NOLO but
> you are right that in theory you could boot them using other
> bootloaders and having an empty node doesn't cause any harm anyway.
Should not be it part of any arm board? IIRC ATAG support is (or was)
not omap3 specified.
> >> [0]:
> >> http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c
> >> 9f 7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
> >>
> >> Best regards,
>
> Best regards,
--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161216/debc8ab2/attachment.sig>
^ permalink raw reply
* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
From: Javier Martinez Canillas @ 2016-12-16 12:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201612161348.35917@pali>
Hello Pali,
On 12/16/2016 09:48 AM, Pali Roh?r wrote:
> On Friday 16 December 2016 13:38:34 Javier Martinez Canillas wrote:
>> Hello Pali,
>>
>> On 12/16/2016 09:32 AM, Pali Roh?r wrote:
>>> Hi!
>>>
>>> On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote:
>>>> Hello Pali,
>>>>
>>>> On 12/16/2016 08:46 AM, Pali Roh?r wrote:
>>>>> On Thursday 15 December 2016 01:09:20 Pali Roh?r wrote:
>>>>>> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux
>>>>>>
>>>>>> wrote:
>>>>>>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh?r wrote:
>>>>>>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
>>>>>>>> usage") broke support for setting cmdline on Nokia N900 via
>>>>>>>> CONFIG_CMDLINE.
>>>>>>>>
>>>>>>>> It is because arm code booted in DT mode parse cmdline only
>>>>>>>> via function early_init_dt_scan_chosen() and that function
>>>>>>>> does not fill variable boot_command_line when DTB does not
>>>>>>>> contain /chosen entry. It is called from function
>>>>>>>> early_init_dt_scan_nodes() in setup_machine_fdt().
>>>>>>>>
>>>>>>>> This patch fixes it by explicitly filling boot_command_line in
>>>>>>>> function setup_machine_fdt() after calling
>>>>>>>> early_init_dt_scan_nodes() in case boot_command_line still
>>>>>>>> remains empty.
>>>>>>>
>>>>>>> This looks like a hack.
>>>>>>>
>>>>>>> First, the matter of the ATAGs compatibility. The decompressor
>>>>>>> relies on there being a pre-existing /chosen node to insert the
>>>>>>> command line and other parameters into. If we've dropped it
>>>>>>> (by dropping skeleton.dtsi) then we've just regressed more
>>>>>>> than just N900 - the decompressor won't be able to merge the
>>>>>>> ATAGs into the concatenated FDT.
>>>>>>
>>>>>> Hm... I did not think about it. But right this can be broken
>>>>>> too...
>>>>>
>>>>> Tony, Javier: are you aware of above ??? problem?
>>>>>
>>>>> It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
>>>>> skeleton.dtsi usage") should be really reverted.
>>>>
>>>> I don't think reverting the mentioned commit is the correct fix
>>>> for your problem. We are trying to get rid of skeleton.dtsi for
>>>> many reasons, see commit commit ("3ebee5a2e141 arm64: dts: kill
>>>> skeleton.dtsi").
>>>
>>> $ git show 3ebee5a2e141
>>>
>>> * The default empty /chosen and /aliases are somewhat useless...
>>>
>>> That is not truth, they are not useless as Russell King wrote --
>>> removing them break ATAG support.
>>>
>>> (But that commit is for arm64 which probably is not using ATAGs...
>>> But I do not know. At least it is not truth for 32bit arm.)
>>>
>>>> Also, the chosen node is mentioned to be optional in the ePAPR
>>>> document and u-boot creates a chosen node if isn't found [0] so
>>>> this issue is only present in boards that don't use u-boot like
>>>> the N900/N950/N9 phones.
>>>
>>> Linux arm decompressor does not propagate ATAGs when /chosen is
>>> missing. Sorry, but if for Linux /chosen is required (and without
>>> it is broken!) then some ePARP document does not apply there.
>>> Either Linux code needs to be fixed (so /chosen will be really
>>> only optional) or /chosen stay in Linux required. There is no
>>> other option.
>>>
>>> And I hope that U-boot is not the only one bootloader which Linux
>>> kernel supports. I thought that I can use *any* bootloader to boot
>>> Linux kernel not just U-Boot which is doing some magic...
>>>
>>> With this step you are basically going to break booting Linux
>>> kernel with all others bootloaders... And personally I really
>>> dislike this idea.
>>>
>>>> So if NOLO doesn't do the same than u-boot and the kernel expects
>>>> a chosen node, I suggest to add an empty chosen node in the
>>>> omap3-n900.dts and omap3-n950-n9.dtsi device tree source files.
>>>
>>> That would fix a problem for N900, N950 and N9. But not for all
>>> other ARM devices which bootloader pass some ATAGs.
>>>
>>> IIRC rule of kernel is not to break compatibility and that commit
>>> 008a2ebcd677 really did it.
>>>
>>> Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just
>>> saying that it cause problems which need to be properly fixed. And
>>> if fixing them is harder and will take more time, then correct
>>> option is to revert 008a2ebcd677 due to breaking support for more
>>> devices.
>>
>> If you think that others boards may have the same issue, then you
>> could add an empty chosen node to omap3.dtsi. As I said I think that
>> in practice this will only be needed for the machines using NOLO but
>> you are right that in theory you could boot them using other
>> bootloaders and having an empty node doesn't cause any harm anyway.
>
> Should not be it part of any arm board? IIRC ATAG support is (or was)
> not omap3 specified.
>
Yes, but you were talking about commit 008a2ebcd677 which only removed
skeleton.dtsi usage for OMAP3 boards. The same can be done for other
SoCs in its top level dtsi for the SoC family of course.
>>>> [0]:
>>>> http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c
>>>> 9f 7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
>>>>
>>>> Best regards,
>>
>> Best regards,
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* [RESEND PATCH 2/2] arm64: make WANT_HUGE_PMD_SHARE depends on HUGETLB_PAGE
From: zhong jiang @ 2016-12-16 12:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216123512.GF13191@arm.com>
On 2016/12/16 20:35, Will Deacon wrote:
> On Fri, Dec 16, 2016 at 05:10:05PM +0800, zhong jiang wrote:
>> On 2016/12/14 22:19, zhongjiang wrote:
>>> From: zhong jiang <zhongjiang@huawei.com>
>>>
>>> when HUGETLB_PAGE is disable, WANT_HUGE_PMD_SHARE contains the
>>> fuctions should not be use. therefore, we add the dependency.
>>>
>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>> ---
>>> arch/arm64/Kconfig | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index 969ef88..694ca73 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -640,6 +640,7 @@ config SYS_SUPPORTS_HUGETLBFS
>>>
>>> config ARCH_WANT_HUGE_PMD_SHARE
>>> def_bool y if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)
>>> + depends on HUGETLB_PAGE
>>>
>>> config ARCH_HAS_CACHE_LINE_SIZE
>>> def_bool y
>> Hi,
>> I still think it is a issue. Perhaps above changelog is unclear. Further explain is as follows.
>> when hugetlb_pages is disable and arch_want_huge_pmd_share is enable, we maybe call
>> huge_pmd_sahre in hugetlbpage.c, but the function actually is not definition as it is not
>> exported. is it right ??
> The only users of ARCH_WANT_HUGE_PMD_SHARE on arm64 are:
>
> arch/arm64/mm/hugetlbpage.c: if (IS_ENABLED(CONFIG_ARCH_WANT_HUGE_PMD_SHARE) &&
yes
> mm/hugetlb.c:#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
> mm/hugetlb.c:#else /* !CONFIG_ARCH_WANT_HUGE_PMD_SHARE */
> mm/hugetlb.c:#endif /* CONFIG_ARCH_WANT_HUGE_PMD_SHARE */
>
> and neither of those files are compiled if !HUGETLB_PAGE.
Indeed, but The only users maybe use it if !HUGETLB_PAGE.
because include/linux/hugetlb.h can not export the reference if !HUGETLB_PAGE.
IOW, the function is not definition.
is it right ? or I miss the key point.
> Are you actually seeing a problem here?
I review the code and find the issue.
Thanks
zhongjiang
> Will
>
> .
>
^ permalink raw reply
* [PATCH v8 0/8] Introducing Exynos ChipId driver
From: Marek Szyprowski @ 2016-12-16 13:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481375323-29724-1-git-send-email-pankaj.dubey@samsung.com>
Hi Pankaj
On 2016-12-10 14:08, Pankaj Dubey wrote:
> Each Exynos SoC has ChipID block which can give information about SoC's
> product Id and revision number.
>
> This patch series introduces Exynos Chipid SoC driver. At the same time
> it reduces dependency of mach-exynos files from plat-samsung, by removing
> soc_is_exynosMMMM and samsung_rev API. Instead of it now we can use
> soc_device_match API proposed by Arnd and getting discussed in thread [1].
>
> Until now we are using static mapping of Exynos Chipid and using this static
> mapping to know about SoC name and revision via soc_is_exynosMMMM macro. This
> is quite cumbersome and every time new ARMv7 based Exynos SoC supports lands in
> mainline a bunch of such new macros needs to be added. Quite long back during
> support of Exynos5260 it has been discussed to solve this problem.
>
> To solve this issue this patchset replaces use of soc_is_exynosMMMM by either
> of_machine_is_compatible or soc_device_match depending upon usecase.
>
> I have tested this patch series on Exynos4210 based Origen board for normal SMP
> boot.
>
>
> Although I submitted this series as a whole of 8 patchsets, following are dependency
> details among the patches.
>
> Patch 1/8 can be taken without any dependency on other patches.
> Patch 2/8 and 3/8 has dependency on 1/8 and can be taken along with 1/8.
> Patch 4/8 has no dependency and can be taken without chipid driver patch 1/8.
> Patch 5/8 has depency on 1/8
> Patch 6/8 has no dependency and can be taken without any other patches.
> Patch 7/8 has dependency on 6/8 and 1/8
> Patch 8/8 has dependency on rest of patches
Which kernel should I use as a base for applying those patches? I wanted
to test them,
but I got conflicts both for v4.9 and current linux-next (next-20161216).
> [1]: https://patchwork.kernel.org/patch/9361389/
>
> Revision 7 and it's discussion can be found here:
> - https://www.spinics.net/lists/arm-kernel/msg540790.html
>
> Revision 6 and it's discussion can be found here:
> - https://lkml.org/lkml/2016/5/25/101
>
> Revision 5 and it's discussion can be found here:
> - http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/310046.html
>
> Revision 4 and it's discussion can be found here:
> - https://lkml.org/lkml/2014/12/3/115
>
> Changes since v7:
> - Added ARM64 based Exynos7 and Exynos5433 support in chipid driver.
> - Removed Exynos4415 support from chipid driver, as exynos4415 has been removed from tree
> - Added patch to enable chipid driver for ARM64 based Exynos platform
> - Addressed review comments from Arnd for firmware.c, platsmp.c and pm.c files/
> - Splitted changes in firmware.c, platsmp.c and pm.c in separate patches
> for better code review
> - Included suggested code improvement in exynos-chipid.c from Marek Szyprowski
>
> Chances since v6:
> - Removed platform driver from chipid, instead use early_init to register soc_device
> - Removed export functions from exynos chipid driver
> - Replace soc_is_exynosMMMM via either of_machine_is_compatible or soc_device_match in
> files in arm/mach-exynos folder
> - This patchset depends on the following patch series by Geert Uytterhoeven. This series
> includes patch which introduces soc_device_match.
> http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1261739.html
> - Rebased on latest krzk/for-next branch and retested.
>
> Change since v5:
> - Addressed Rob's review comments.
> - Rebased on latest krzk/for-next branch and retested.
>
> Changes since v4:
> - Removed custom sysfs entries as they were not providing any new information
> as pointed out by Arnd.
> - Removed functions exporting product_id and revision, instead we will export
> exynos_chipid_info structure. It will be helpfull when we need to provide more
> fields of chipid outside of chipid, as commented by Yadwinder
> - Converted all funcions as __init.
>
> Change since v3:
> - This patch set contains 5/6 and 6/6 patch from v3 series.
> - Made EXYNOS_CHIPID config option non-user selectable,
> as suggested by Tomasz Figa.
> - Made uniform macro for EXYNOS4/5_SOC_MASK as EXYNOS_SOC_MASK as
> suggested by Tomasz Figa.
> - Made local variables static in chipid driver.
> - Added existing SoC's product id's.
> - Added platform driver support.
>
> Changes since v2:
> - Reorganized patches as suggested by Tomasz Figa.
> - Addressed review comments of Tomasz Figa in i2c-s3c2410.c file.
>
> Pankaj Dubey (8):
> soc: samsung: add exynos chipid driver support
> ARM: EXYNOS: enable exynos_chipid for ARCH_EXYNOS
> ARM64: EXYNOS: enable exynos_chipid for ARCH_EXYNOS
> ARM: EXYNOS: refactor firmware specific routines
> ARM: EXYNOS: refactor power management specific routines
> ARM: EXYNOS: remove secondary startup initialization from
> smp_prepare_cpus
> ARM: EXYNOS: refactor smp specific code and routines
> ARM: EXYNOS: refactor of mach-exynos to use chipid information
>
> arch/arm/mach-exynos/Kconfig | 1 +
> arch/arm/mach-exynos/common.h | 92 ---------
> arch/arm/mach-exynos/exynos.c | 38 ----
> arch/arm/mach-exynos/firmware.c | 100 +++++++---
> arch/arm/mach-exynos/include/mach/map.h | 21 ---
> arch/arm/mach-exynos/platsmp.c | 267 +++++++++++++++++++++------
> arch/arm/mach-exynos/pm.c | 185 ++++++++++++++++---
> arch/arm/plat-samsung/cpu.c | 14 --
> arch/arm/plat-samsung/include/plat/cpu.h | 2 -
> arch/arm/plat-samsung/include/plat/map-s5p.h | 2 -
> arch/arm64/Kconfig.platforms | 1 +
> drivers/soc/samsung/Kconfig | 5 +
> drivers/soc/samsung/Makefile | 1 +
> drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++
> 14 files changed, 563 insertions(+), 282 deletions(-)
> delete mode 100644 arch/arm/mach-exynos/include/mach/map.h
> create mode 100644 drivers/soc/samsung/exynos-chipid.c
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* [RFC PATCH] iommu/arm-smmu: Add global SMR masking property
From: Robin Murphy @ 2016-12-16 13:19 UTC (permalink / raw)
To: linux-arm-kernel
The current SMR masking support using a 2-cell iommu-specifier is
primarily intended to handle individual masters with large and/or
complex Stream ID assignments; it quickly gets a bit clunky in other SMR
use-cases where we just want to consistently mask out the same part of
every Stream ID (e.g. for MMU-500 configurations where the appended TBU
number gets in the way unnecessarily). Let's add a new property to allow
a single global mask value to better fit the latter situation.
CC: Stuart Yoder <stuart.yoder@nxp.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
Compile-tested only...
Documentation/devicetree/bindings/iommu/arm,smmu.txt | 8 ++++++++
drivers/iommu/arm-smmu.c | 4 +++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index e862d1485205..98f5cbe5fdb4 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -60,6 +60,14 @@ conditions.
aliases of secure registers have to be used during
SMMU configuration.
+- stream-match-mask : Specifies a fixed SMR mask value to combine with
+ the Stream ID value from every iommu-specifier. This
+ may be used instead of an "#iommu-cells" value of 2
+ when there is no need for per-master SMR masks, but
+ it is still desired to mask some portion of every
+ Stream ID (e.g. for certain MMU-500 configurations
+ given globally unique external IDs).
+
** Deprecated properties:
- mmu-masters (deprecated in favour of the generic "iommus" binding) :
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 8f7281444551..f1abcb7dde36 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1534,13 +1534,15 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
{
- u32 fwid = 0;
+ u32 mask, fwid = 0;
if (args->args_count > 0)
fwid |= (u16)args->args[0];
if (args->args_count > 1)
fwid |= (u16)args->args[1] << SMR_MASK_SHIFT;
+ else if (!of_property_read_u32(args->np, "stream-match-mask", &mask))
+ fwid |= (u16)mask << SMR_MASK_SHIFT;
return iommu_fwspec_add_ids(dev, &fwid, 1);
}
--
2.10.2.dirty
^ permalink raw reply related
* [RESEND PATCH 2/2] arm64: make WANT_HUGE_PMD_SHARE depends on HUGETLB_PAGE
From: zhong jiang @ 2016-12-16 14:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216123512.GF13191@arm.com>
On 2016/12/16 20:35, Will Deacon wrote:
> On Fri, Dec 16, 2016 at 05:10:05PM +0800, zhong jiang wrote:
>> On 2016/12/14 22:19, zhongjiang wrote:
>>> From: zhong jiang <zhongjiang@huawei.com>
>>>
>>> when HUGETLB_PAGE is disable, WANT_HUGE_PMD_SHARE contains the
>>> fuctions should not be use. therefore, we add the dependency.
>>>
>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>> ---
>>> arch/arm64/Kconfig | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index 969ef88..694ca73 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -640,6 +640,7 @@ config SYS_SUPPORTS_HUGETLBFS
>>>
>>> config ARCH_WANT_HUGE_PMD_SHARE
>>> def_bool y if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)
>>> + depends on HUGETLB_PAGE
>>>
>>> config ARCH_HAS_CACHE_LINE_SIZE
>>> def_bool y
>> Hi,
>> I still think it is a issue. Perhaps above changelog is unclear. Further explain is as follows.
>> when hugetlb_pages is disable and arch_want_huge_pmd_share is enable, we maybe call
>> huge_pmd_sahre in hugetlbpage.c, but the function actually is not definition as it is not
>> exported. is it right ??
> The only users of ARCH_WANT_HUGE_PMD_SHARE on arm64 are:
>
> arch/arm64/mm/hugetlbpage.c: if (IS_ENABLED(CONFIG_ARCH_WANT_HUGE_PMD_SHARE) &&
> mm/hugetlb.c:#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
> mm/hugetlb.c:#else /* !CONFIG_ARCH_WANT_HUGE_PMD_SHARE */
> mm/hugetlb.c:#endif /* CONFIG_ARCH_WANT_HUGE_PMD_SHARE */
>
> and neither of those files are compiled if !HUGETLB_PAGE.
>
> Are you actually seeing a problem here?
>
> Will
>
> .
>
I got it. Please forget the stupid patch and my stupid comments.
but the first patch look reasonable. Is it accepted ?
Thanks
zhongjiang
^ permalink raw reply
* [PATCH] Documentation: dt: Explicitly mark Samsung Exynos SoC bindings as unstable
From: Marek Szyprowski @ 2016-12-16 14:14 UTC (permalink / raw)
To: linux-arm-kernel
Samsung Exynos SoCs and boards related bindings evolved since the initial
introduction, but initially the bindings were minimal and a bit incomplete
(they never described all the hardware modules available in the SoCs).
Since then some significant (not fully compatible) changes have been
already committed a few times (like gpio replaced by pinctrl, display ddc,
mfc reserved memory, some core clocks added to various hardware modules,
added more required nodes).
On the other side there are no boards which have device tree embedded in
the bootloader. Device tree blob is always compiled from the kernel tree
and updated together with the kernel image.
Thus to avoid further adding a bunch of workarounds for old/missing
bindings and allow to make cleanup of the existing code and device tree
files, lets mark Samsung Exynos SoC platform bindings as unstable. This
means that bindings can may change at any time and users should use the
dtb file compiled from the same kernel source tree as the kernel image.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
Documentation/devicetree/bindings/arm/samsung/exynos.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/samsung/exynos.txt
diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos.txt b/Documentation/devicetree/bindings/arm/samsung/exynos.txt
new file mode 100644
index 000000000000..0c606f4c6e85
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos.txt
@@ -0,0 +1,12 @@
+Samsung Exynos SoC Family Device Tree Bindings
+---------------------------------------------------------------
+
+Work in progress statement:
+
+Device tree files and bindings applying to Samsung Exynos SoCs and boards are
+considered "unstable". Any Samsung Exynos device tree binding may change at any
+time. Be sure to use a device tree binary and a kernel image generated from the
+same source tree.
+
+Please refer to Documentation/devicetree/bindings/ABI.txt for a definition of a
+stable binding/ABI.
--
1.9.1
^ permalink raw reply related
* jemalloc testsuite stalls in memset
From: Andreas Schwab @ 2016-12-16 14:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216063940.GA1334@bbox>
On Dez 16 2016, Minchan Kim <minchan@kernel.org> wrote:
> Below helps?
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index e10a4fe..dc37c9a 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1611,6 +1611,7 @@ int madvise_free_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> tlb->fullmm);
> orig_pmd = pmd_mkold(orig_pmd);
> orig_pmd = pmd_mkclean(orig_pmd);
> + orig_pmd = pmd_wrprotect(orig_pmd);
>
> set_pmd_at(mm, addr, pmd, orig_pmd);
> tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
Thanks, this fixes the issue (tested with 4.9).
Andreas.
--
Andreas Schwab, SUSE Labs, schwab at suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply
* [PATCH] Documentation: dt: Explicitly mark Samsung Exynos SoC bindings as unstable
From: Javier Martinez Canillas @ 2016-12-16 14:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481897676-13578-1-git-send-email-m.szyprowski@samsung.com>
Hello Marek,
On 12/16/2016 11:14 AM, Marek Szyprowski wrote:
> Samsung Exynos SoCs and boards related bindings evolved since the initial
> introduction, but initially the bindings were minimal and a bit incomplete
> (they never described all the hardware modules available in the SoCs).
> Since then some significant (not fully compatible) changes have been
> already committed a few times (like gpio replaced by pinctrl, display ddc,
> mfc reserved memory, some core clocks added to various hardware modules,
> added more required nodes).
>
> On the other side there are no boards which have device tree embedded in
> the bootloader. Device tree blob is always compiled from the kernel tree
> and updated together with the kernel image.
>
> Thus to avoid further adding a bunch of workarounds for old/missing
> bindings and allow to make cleanup of the existing code and device tree
> files, lets mark Samsung Exynos SoC platform bindings as unstable. This
> means that bindings can may change at any time and users should use the
> dtb file compiled from the same kernel source tree as the kernel image.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
I completely agree with you on this.
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* [PATCH] Documentation: dt: Explicitly mark Samsung Exynos SoC bindings as unstable
From: Bartlomiej Zolnierkiewicz @ 2016-12-16 14:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481897676-13578-1-git-send-email-m.szyprowski@samsung.com>
Hi,
On Friday, December 16, 2016 03:14:36 PM Marek Szyprowski wrote:
> Samsung Exynos SoCs and boards related bindings evolved since the initial
> introduction, but initially the bindings were minimal and a bit incomplete
> (they never described all the hardware modules available in the SoCs).
> Since then some significant (not fully compatible) changes have been
> already committed a few times (like gpio replaced by pinctrl, display ddc,
> mfc reserved memory, some core clocks added to various hardware modules,
> added more required nodes).
>
> On the other side there are no boards which have device tree embedded in
> the bootloader. Device tree blob is always compiled from the kernel tree
> and updated together with the kernel image.
>
> Thus to avoid further adding a bunch of workarounds for old/missing
> bindings and allow to make cleanup of the existing code and device tree
> files, lets mark Samsung Exynos SoC platform bindings as unstable. This
> means that bindings can may change at any time and users should use the
> dtb file compiled from the same kernel source tree as the kernel image.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
This change is long overdue..
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
> Documentation/devicetree/bindings/arm/samsung/exynos.txt | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/samsung/exynos.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos.txt b/Documentation/devicetree/bindings/arm/samsung/exynos.txt
> new file mode 100644
> index 000000000000..0c606f4c6e85
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos.txt
> @@ -0,0 +1,12 @@
> +Samsung Exynos SoC Family Device Tree Bindings
> +---------------------------------------------------------------
> +
> +Work in progress statement:
> +
> +Device tree files and bindings applying to Samsung Exynos SoCs and boards are
> +considered "unstable". Any Samsung Exynos device tree binding may change at any
> +time. Be sure to use a device tree binary and a kernel image generated from the
> +same source tree.
> +
> +Please refer to Documentation/devicetree/bindings/ABI.txt for a definition of a
> +stable binding/ABI.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* [PATCH] ARM: dts: sun8i-q8-common: enable bluetooth on SDIO Wi-Fi
From: Icenowy Zheng @ 2016-12-16 14:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216124748.rkvnnlo4x5onzpvk@lukather>
16.12.2016, 20:47, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
> On Fri, Dec 09, 2016 at 07:49:00PM +0800, Icenowy Zheng wrote:
>> ?2016?12?9? ??4:07? Maxime Ripard <maxime.ripard@free-electrons.com>???
>> ?>
>> ?> On Tue, Dec 06, 2016 at 04:08:38PM +0800, Icenowy Zheng wrote:
>> ?> > Some SDIO Wi-Fi chips (such as RTL8703AS) have a UART bluetooth, which
>> ?> > has a dedicated enable pin (PL8 in the reference design).
>> ?> >
>> ?> > Enable the pin in the same way as the WLAN enable pins.
>> ?> >
>> ?> > Tested on an A33 Q8 tablet with RTL8703AS.
>> ?> >
>> ?> > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>> ?> > ---
>> ?> >
>> ?> > This patch should be coupled with the uart1 node patch I send before:
>> ?> > http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/471997.html
>> ?> >
>> ?> > For RTL8703AS, the rtl8723bs bluetooth code is used, which can be retrieve from:
>> ?> > https://github.com/lwfinger/rtl8723bs_bt
>> ?> >
>> ?> >? arch/arm/boot/dts/sun8i-q8-common.dtsi | 2 +-
>> ?> >? 1 file changed, 1 insertion(+), 1 deletion(-)
>> ?> >
>> ?> > diff --git a/arch/arm/boot/dts/sun8i-q8-common.dtsi b/arch/arm/boot/dts/sun8i-q8-common.dtsi
>> ?> > index c676940..4aeb5bb 100644
>> ?> > --- a/arch/arm/boot/dts/sun8i-q8-common.dtsi
>> ?> > +++ b/arch/arm/boot/dts/sun8i-q8-common.dtsi
>> ?> > @@ -88,7 +88,7 @@
>> ?> >
>> ?> >? &r_pio {
>> ?> >? wifi_pwrseq_pin_q8: wifi_pwrseq_pin at 0 {
>> ?> > - pins = "PL6", "PL7", "PL11";
>> ?> > + pins = "PL6", "PL7", "PL8", "PL11";
>> ?> >? function = "gpio_in";
>> ?> >? bias-pull-up;
>> ?> >? };
>> ?>
>> ?> There's several things wrong here. The first one is that you rely
>> ?> solely on the pinctrl state to maintain a reset line. This is very
>> ?> fragile (especially since the GPIO pinctrl state are likely to go away
>> ?> at some point), but it also means that if your driver wants to recover
>> ?> from that situation at some point, it won't work.
>> ?>
>> ?> The other one is that the bluetooth and wifi chips are two devices in
>> ?> linux, and you assign that pin to the wrong device (wifi).
>> ?>
>> ?> rfkill-gpio is made just for that, so please use it.
>>
>> ?The GPIO is not for the radio, but for the full Bluetooth part.
>
> I know.
>
>> ?If it's set to 0, then the bluetooth part will reset, and the
>> ?hciattach will fail.
>
> Both rfkill-gpio and rfkill-regulator will shutdown when called
> (either by poking the reset pin or shutting down the regulator), so
> that definitely seems like an expected behavior to put the device in
> reset.
>
>> ?The BSP uses this as a rfkill, and the result is that the bluetooth
>> ?on/off switch do not work properly.
>
> Then rfkill needs fixing, but working around it by hoping that the
> core will probe an entirely different device, and enforcing a default
> that the rest of the kernel might or might not change is both fragile
> and wrong.
I think a rfkill-gpio here works just like the BSP rfkill...
The real problem is that the Realtek UART bluetooth driver is a userspace
program (a modified hciattach), which is not capable of the GPIO reset...
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
^ permalink raw reply
* [RFC v2 PATCH 0/3] Fix dma_alloc_coherent() and friends for NOMMU
From: Alexandre Torgue @ 2016-12-16 14:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481636704-18948-1-git-send-email-vladimir.murzin@arm.com>
Hi Vladimir,
On 12/13/2016 02:45 PM, Vladimir Murzin wrote:
> Hi,
>
> It seem that addition of cache support for M-class cpus uncovered
> latent bug in DMA usage. NOMMU memory model has been treated as being
> always consistent; however, for R/M classes of cpu memory can be
> covered by MPU which in turn might configure RAM as Normal
> i.e. bufferable and cacheable. It breaks dma_alloc_coherent() and
> friends, since data can stuck in caches now or be buffered.
>
> This patch set is trying to address the issue by providing region of
> memory suitable for consistent DMA operations. It is supposed that such
> region is marked by MPU as non-cacheable. Since we have MPU support in
> Linux for R-class only and M-class setting MPU in bootloader, proposed
> interface to advertise such memory is via "memdma=size at start" command
> line option, to avoid clashing with normal memory (which usually comes
> from dts) it'd be safer to use it together with "mem=" command line
> option. Meanwhile, I'm open to suggestions for the better way telling
> Linux of such memory.
>
> For configuration without cache support (like Cortex-M3/M4) dma
> operations are forced to be coherent and wired with dma-noop. Such
> decision is made based on cacheid global variable. In case cpu
> supports caches and no coherent memory region is given - dma is
> disallowed. Probably, some other important checks are missing, so I'll
> all my ears :)
>
> To make life easier NOMMU dma operations are kept in separate
> compilation unit.
>
> Thanks!
>
> Changelog:
>
> RFC v1 -> RFC v2
> - s/dmac_unmap_area/dmac_map_area in __dma_page_cpu_to_dev()
> - removed unrelated changes in nommu.c
>
> Vladimir Murzin (3):
> ARM: NOMMU: introduce dma operations for noMMU
> ARM: NOMMU: set ARM_DMA_MEM_BUFFERABLE for M-class cpus
> ARM: dma-mapping: remove traces of NOMMU code
>
> arch/arm/include/asm/dma-mapping.h | 3 +-
> arch/arm/mm/Kconfig | 2 +-
> arch/arm/mm/Makefile | 5 +-
> arch/arm/mm/dma-mapping-nommu.c | 262 ++++++++++++++++++++++++++++++++++++
> arch/arm/mm/dma-mapping.c | 26 +---
> arch/arm/mm/mm.h | 3 +
> arch/arm/mm/nommu.c | 6 +
> 7 files changed, 278 insertions(+), 29 deletions(-)
> create mode 100644 arch/arm/mm/dma-mapping-nommu.c
>
First, thanks for this series.
I tested it on stm32f746 platform. Main issues related to cache and DMA
are fixed but I still have an issue using dma_zalloc_alloc API.
Allocated memory is not set to zero.
Can you have a look on it please?
Thanks
Alex
^ permalink raw reply
* [RFC v2 PATCH 0/3] Fix dma_alloc_coherent() and friends for NOMMU
From: Vladimir Murzin @ 2016-12-16 15:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <82f0d72e-7029-ad88-3e44-ab48784297fa@st.com>
Hi Alexandre,
On 16/12/16 14:57, Alexandre Torgue wrote:
> Hi Vladimir,
>
> On 12/13/2016 02:45 PM, Vladimir Murzin wrote:
>> Hi,
>>
>> It seem that addition of cache support for M-class cpus uncovered
>> latent bug in DMA usage. NOMMU memory model has been treated as being
>> always consistent; however, for R/M classes of cpu memory can be
>> covered by MPU which in turn might configure RAM as Normal
>> i.e. bufferable and cacheable. It breaks dma_alloc_coherent() and
>> friends, since data can stuck in caches now or be buffered.
>>
>> This patch set is trying to address the issue by providing region of
>> memory suitable for consistent DMA operations. It is supposed that such
>> region is marked by MPU as non-cacheable. Since we have MPU support in
>> Linux for R-class only and M-class setting MPU in bootloader, proposed
>> interface to advertise such memory is via "memdma=size at start" command
>> line option, to avoid clashing with normal memory (which usually comes
>> from dts) it'd be safer to use it together with "mem=" command line
>> option. Meanwhile, I'm open to suggestions for the better way telling
>> Linux of such memory.
>>
>> For configuration without cache support (like Cortex-M3/M4) dma
>> operations are forced to be coherent and wired with dma-noop. Such
>> decision is made based on cacheid global variable. In case cpu
>> supports caches and no coherent memory region is given - dma is
>> disallowed. Probably, some other important checks are missing, so I'll
>> all my ears :)
>>
>> To make life easier NOMMU dma operations are kept in separate
>> compilation unit.
>>
>> Thanks!
>>
>> Changelog:
>>
>> RFC v1 -> RFC v2
>> - s/dmac_unmap_area/dmac_map_area in __dma_page_cpu_to_dev()
>> - removed unrelated changes in nommu.c
>>
>> Vladimir Murzin (3):
>> ARM: NOMMU: introduce dma operations for noMMU
>> ARM: NOMMU: set ARM_DMA_MEM_BUFFERABLE for M-class cpus
>> ARM: dma-mapping: remove traces of NOMMU code
>>
>> arch/arm/include/asm/dma-mapping.h | 3 +-
>> arch/arm/mm/Kconfig | 2 +-
>> arch/arm/mm/Makefile | 5 +-
>> arch/arm/mm/dma-mapping-nommu.c | 262 ++++++++++++++++++++++++++++++++++++
>> arch/arm/mm/dma-mapping.c | 26 +---
>> arch/arm/mm/mm.h | 3 +
>> arch/arm/mm/nommu.c | 6 +
>> 7 files changed, 278 insertions(+), 29 deletions(-)
>> create mode 100644 arch/arm/mm/dma-mapping-nommu.c
>>
>
> First, thanks for this series.
>
> I tested it on stm32f746 platform. Main issues related to cache and DMA are fixed but I still have an issue using dma_zalloc_alloc API. Allocated memory is not set to zero.
> Can you have a look on it please?
Thanks for testing! I think following diff should fix dma_zalloc_alloc():
diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
index f92d98a..1f97bb8 100644
--- a/arch/arm/mm/dma-mapping-nommu.c
+++ b/arch/arm/mm/dma-mapping-nommu.c
@@ -38,6 +38,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
ptr = (void *)gen_pool_alloc(dma_pool, size);
if (ptr) {
+ memset(ptr, 0, size);
*dma_handle = __pa(ptr);
dmac_flush_range(ptr, ptr + size);
outer_flush_range(__pa(ptr), __pa(ptr) + size);
Cheers
Vladimir
>
> Thanks
> Alex
>
>
^ permalink raw reply related
* [RFC v2 PATCH 0/3] Fix dma_alloc_coherent() and friends for NOMMU
From: Alexandre Torgue @ 2016-12-16 15:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <58540199.6000002@arm.com>
Hi,
On 12/16/2016 04:00 PM, Vladimir Murzin wrote:
> Hi Alexandre,
>
> On 16/12/16 14:57, Alexandre Torgue wrote:
>> Hi Vladimir,
>>
>> On 12/13/2016 02:45 PM, Vladimir Murzin wrote:
>>> Hi,
>>>
>>> It seem that addition of cache support for M-class cpus uncovered
>>> latent bug in DMA usage. NOMMU memory model has been treated as being
>>> always consistent; however, for R/M classes of cpu memory can be
>>> covered by MPU which in turn might configure RAM as Normal
>>> i.e. bufferable and cacheable. It breaks dma_alloc_coherent() and
>>> friends, since data can stuck in caches now or be buffered.
>>>
>>> This patch set is trying to address the issue by providing region of
>>> memory suitable for consistent DMA operations. It is supposed that such
>>> region is marked by MPU as non-cacheable. Since we have MPU support in
>>> Linux for R-class only and M-class setting MPU in bootloader, proposed
>>> interface to advertise such memory is via "memdma=size at start" command
>>> line option, to avoid clashing with normal memory (which usually comes
>>> from dts) it'd be safer to use it together with "mem=" command line
>>> option. Meanwhile, I'm open to suggestions for the better way telling
>>> Linux of such memory.
>>>
>>> For configuration without cache support (like Cortex-M3/M4) dma
>>> operations are forced to be coherent and wired with dma-noop. Such
>>> decision is made based on cacheid global variable. In case cpu
>>> supports caches and no coherent memory region is given - dma is
>>> disallowed. Probably, some other important checks are missing, so I'll
>>> all my ears :)
>>>
>>> To make life easier NOMMU dma operations are kept in separate
>>> compilation unit.
>>>
>>> Thanks!
>>>
>>> Changelog:
>>>
>>> RFC v1 -> RFC v2
>>> - s/dmac_unmap_area/dmac_map_area in __dma_page_cpu_to_dev()
>>> - removed unrelated changes in nommu.c
>>>
>>> Vladimir Murzin (3):
>>> ARM: NOMMU: introduce dma operations for noMMU
>>> ARM: NOMMU: set ARM_DMA_MEM_BUFFERABLE for M-class cpus
>>> ARM: dma-mapping: remove traces of NOMMU code
>>>
>>> arch/arm/include/asm/dma-mapping.h | 3 +-
>>> arch/arm/mm/Kconfig | 2 +-
>>> arch/arm/mm/Makefile | 5 +-
>>> arch/arm/mm/dma-mapping-nommu.c | 262 ++++++++++++++++++++++++++++++++++++
>>> arch/arm/mm/dma-mapping.c | 26 +---
>>> arch/arm/mm/mm.h | 3 +
>>> arch/arm/mm/nommu.c | 6 +
>>> 7 files changed, 278 insertions(+), 29 deletions(-)
>>> create mode 100644 arch/arm/mm/dma-mapping-nommu.c
>>>
>>
>> First, thanks for this series.
>>
>> I tested it on stm32f746 platform. Main issues related to cache and DMA are fixed but I still have an issue using dma_zalloc_alloc API. Allocated memory is not set to zero.
>> Can you have a look on it please?
>
> Thanks for testing! I think following diff should fix dma_zalloc_alloc():
Yes. With the patch below, it works fine.
Thanks
Alex
>
> diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
> index f92d98a..1f97bb8 100644
> --- a/arch/arm/mm/dma-mapping-nommu.c
> +++ b/arch/arm/mm/dma-mapping-nommu.c
> @@ -38,6 +38,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
>
> ptr = (void *)gen_pool_alloc(dma_pool, size);
> if (ptr) {
> + memset(ptr, 0, size);
> *dma_handle = __pa(ptr);
> dmac_flush_range(ptr, ptr + size);
> outer_flush_range(__pa(ptr), __pa(ptr) + size);
>
>
> Cheers
> Vladimir
>
>>
>> Thanks
>> Alex
>>
>>
>
^ permalink raw reply
* [PATCH 1/3] dmaengine: xilinx_dma: Check for channel idle state before submitting dma descriptor
From: Laurent Pinchart @ 2016-12-16 15:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481814682-31780-2-git-send-email-appanad@xilinx.com>
Hi Kedareswara,
Thank you for the patch.
On Thursday 15 Dec 2016 20:41:20 Kedareswara rao Appana wrote:
> Add channel idle state to ensure that dma descriptor is not
> submitted when VDMA engine is in progress.
>
> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> ---
> drivers/dma/xilinx/xilinx_dma.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/dma/xilinx/xilinx_dma.c
> b/drivers/dma/xilinx/xilinx_dma.c index 8288fe4..736c2a3 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -321,6 +321,7 @@ struct xilinx_dma_tx_descriptor {
> * @cyclic: Check for cyclic transfers.
> * @genlock: Support genlock mode
> * @err: Channel has errors
> + * @idle: Check for channel idle
> * @tasklet: Cleanup work after irq
> * @config: Device configuration info
> * @flush_on_fsync: Flush on Frame sync
> @@ -351,6 +352,7 @@ struct xilinx_dma_chan {
> bool cyclic;
> bool genlock;
> bool err;
> + bool idle;
> struct tasklet_struct tasklet;
> struct xilinx_vdma_config config;
> bool flush_on_fsync;
> @@ -966,6 +968,7 @@ static void xilinx_dma_halt(struct xilinx_dma_chan
> *chan) chan, dma_ctrl_read(chan, XILINX_DMA_REG_DMASR));
> chan->err = true;
> }
> + chan->idle = true;
> }
>
> /**
> @@ -1007,6 +1010,9 @@ static void xilinx_vdma_start_transfer(struct
> xilinx_dma_chan *chan)
> if (chan->err)
> return;
>
> + if (!chan->idle)
> + return;
Don't you need to perform the same check for the DMA and CDMA channels ? If
so, shouldn't this be moved to common code ?
There's another problem (not strictly introduced by this patch) I wanted to
mention. The append_desc_queue() function, called from your tx_submit handler,
appends descriptors to the pending_list. The DMA engine API states that a
transfer submitted by tx_submit will not be executed until .issue_pending() is
called. However, if a transfer is in progress at tx_submit time, I believe
that the IRQ handler, at transfer completion, will start the next transfer
from the pending_list even if .issue_pending() hasn't been called for it.
> if (list_empty(&chan->pending_list))
> return;
Now that you catch busy channels with a software flag, do you still need the
xilinx_dma_is_running() and xilinx_dma_is_idle() checks ? Three different
checks for the same or very similar conditions are confusing, if you really
need them you should document clearly how they differ.
> @@ -1110,6 +1116,7 @@ static void xilinx_vdma_start_transfer(struct
> xilinx_dma_chan *chan) vdma_desc_write(chan, XILINX_DMA_REG_VSIZE,
> last->hw.vsize);
> }
>
> + chan->idle = false;
(and this too)
> if (!chan->has_sg) {
> list_del(&desc->node);
> list_add_tail(&desc->node, &chan->active_list);
> @@ -1447,6 +1454,7 @@ static irqreturn_t xilinx_dma_irq_handler(int irq,
> void *data) if (status & XILINX_DMA_DMASR_FRM_CNT_IRQ) {
> spin_lock(&chan->lock);
> xilinx_dma_complete_descriptor(chan);
> + chan->idle = true;
> chan->start_transfer(chan);
> spin_unlock(&chan->lock);
> }
> @@ -2327,6 +2335,7 @@ static int xilinx_dma_chan_probe(struct
> xilinx_dma_device *xdev, chan->has_sg = xdev->has_sg;
> chan->desc_pendingcount = 0x0;
> chan->ext_addr = xdev->ext_addr;
> + chan->idle = true;
>
> spin_lock_init(&chan->lock);
> INIT_LIST_HEAD(&chan->pending_list);
--
Regards,
Laurent Pinchart
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox