* [PATCH v4 04/10] KVM: arm64: vgic-its: Introduce config option to guard ITS specific code
From: Vladimir Murzin @ 2016-09-12 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473691764-29424-1-git-send-email-vladimir.murzin@arm.com>
By now ITS code guarded with KVM_ARM_VGIC_V3 config option which was
introduced to hide everything specific to vgic-v3 from 32-bit world.
We are going to support vgic-v3 in 32-bit world and KVM_ARM_VGIC_V3
will gone, but we don't have support for ITS there yet and we need to
continue keeping ITS away.
Introduce the new config option to prevent ITS code being build in
32-bit mode when support for vgic-v3 is done.
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/kvm/Kconfig | 4 ++++
virt/kvm/arm/vgic/vgic-kvm-device.c | 3 +++
virt/kvm/arm/vgic/vgic-mmio-v3.c | 2 ++
virt/kvm/arm/vgic/vgic.h | 4 ++++
4 files changed, 13 insertions(+)
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 9c9edc9..7ba9164 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -16,6 +16,9 @@ menuconfig VIRTUALIZATION
if VIRTUALIZATION
+config KVM_ARM_VGIC_V3_ITS
+ bool
+
config KVM_ARM_VGIC_V3
bool
@@ -35,6 +38,7 @@ config KVM
select HAVE_KVM_EVENTFD
select HAVE_KVM_IRQFD
select KVM_ARM_VGIC_V3
+ select KVM_ARM_VGIC_V3_ITS
select KVM_ARM_PMU if HW_PERF_EVENTS
select HAVE_KVM_MSI
select HAVE_KVM_IRQCHIP
diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
index 1813f93..f3811b3 100644
--- a/virt/kvm/arm/vgic/vgic-kvm-device.c
+++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
@@ -223,9 +223,12 @@ int kvm_register_vgic_device(unsigned long type)
case KVM_DEV_TYPE_ARM_VGIC_V3:
ret = kvm_register_device_ops(&kvm_arm_vgic_v3_ops,
KVM_DEV_TYPE_ARM_VGIC_V3);
+
+#ifdef CONFIG_KVM_ARM_VGIC_V3_ITS
if (ret)
break;
ret = kvm_vgic_register_its_device();
+#endif
break;
#endif
}
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index 90d8181..acbe691 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -42,6 +42,7 @@ u64 update_64bit_reg(u64 reg, unsigned int offset, unsigned int len,
return reg | ((u64)val << lower);
}
+#ifdef CONFIG_KVM_ARM_VGIC_V3_ITS
bool vgic_has_its(struct kvm *kvm)
{
struct vgic_dist *dist = &kvm->arch.vgic;
@@ -51,6 +52,7 @@ bool vgic_has_its(struct kvm *kvm)
return dist->has_its;
}
+#endif
static unsigned long vgic_mmio_read_v3_misc(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len)
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index 6c4625c..100045f 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -84,11 +84,15 @@ void vgic_v3_enable(struct kvm_vcpu *vcpu);
int vgic_v3_probe(const struct gic_kvm_info *info);
int vgic_v3_map_resources(struct kvm *kvm);
int vgic_register_redist_iodevs(struct kvm *kvm, gpa_t dist_base_address);
+
+#ifdef CONFIG_KVM_ARM_VGIC_V3_ITS
int vgic_register_its_iodevs(struct kvm *kvm);
bool vgic_has_its(struct kvm *kvm);
int kvm_vgic_register_its_device(void);
void vgic_enable_lpis(struct kvm_vcpu *vcpu);
int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi);
+#endif
+
#else
static inline void vgic_v3_process_maintenance(struct kvm_vcpu *vcpu)
{
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 03/10] arm64: KVM: Move vgic-v3 save/restore to virt/kvm/arm/hyp
From: Vladimir Murzin @ 2016-09-12 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473691764-29424-1-git-send-email-vladimir.murzin@arm.com>
So we can reuse the code under arch/arm
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/kvm/hyp/Makefile | 2 +-
{arch/arm64/kvm => virt/kvm/arm}/hyp/vgic-v3-sr.c | 0
2 files changed, 1 insertion(+), 1 deletion(-)
rename {arch/arm64/kvm => virt/kvm/arm}/hyp/vgic-v3-sr.c (100%)
diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
index 0c85feb..aaf42ae 100644
--- a/arch/arm64/kvm/hyp/Makefile
+++ b/arch/arm64/kvm/hyp/Makefile
@@ -5,9 +5,9 @@
KVM=../../../../virt/kvm
obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v2-sr.o
+obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v3-sr.o
obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/timer-sr.o
-obj-$(CONFIG_KVM_ARM_HOST) += vgic-v3-sr.o
obj-$(CONFIG_KVM_ARM_HOST) += sysreg-sr.o
obj-$(CONFIG_KVM_ARM_HOST) += debug-sr.o
obj-$(CONFIG_KVM_ARM_HOST) += entry.o
diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/virt/kvm/arm/hyp/vgic-v3-sr.c
similarity index 100%
rename from arch/arm64/kvm/hyp/vgic-v3-sr.c
rename to virt/kvm/arm/hyp/vgic-v3-sr.c
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 02/10] arm64: KVM: Move GIC accessors to arch_gicv3.h
From: Vladimir Murzin @ 2016-09-12 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473691764-29424-1-git-send-email-vladimir.murzin@arm.com>
Since we are going to share vgic-v3 save/restore code with ARM keep
arch specific accessors separately.
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/include/asm/arch_gicv3.h | 13 +++++++++++++
arch/arm64/kvm/hyp/vgic-v3-sr.c | 13 -------------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
index 8ec88e5..ae7dbd7 100644
--- a/arch/arm64/include/asm/arch_gicv3.h
+++ b/arch/arm64/include/asm/arch_gicv3.h
@@ -79,6 +79,19 @@
#include <linux/stringify.h>
#include <asm/barrier.h>
+#define read_gicreg(r) \
+ ({ \
+ u64 reg; \
+ asm volatile("mrs_s %0, " __stringify(r) : "=r" (reg)); \
+ reg; \
+ })
+
+#define write_gicreg(v,r) \
+ do { \
+ u64 __val = (v); \
+ asm volatile("msr_s " __stringify(r) ", %0" : : "r" (__val));\
+ } while (0)
+
/*
* Low-level accessors
*
diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index 5f8f80b..f2dbd2e 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -24,19 +24,6 @@
#define vtr_to_max_lr_idx(v) ((v) & 0xf)
#define vtr_to_nr_pri_bits(v) (((u32)(v) >> 29) + 1)
-#define read_gicreg(r) \
- ({ \
- u64 reg; \
- asm volatile("mrs_s %0, " __stringify(r) : "=r" (reg)); \
- reg; \
- })
-
-#define write_gicreg(v,r) \
- do { \
- u64 __val = (v); \
- asm volatile("msr_s " __stringify(r) ", %0" : : "r" (__val));\
- } while (0)
-
static u64 __hyp_text __gic_v3_get_lr(unsigned int lr)
{
switch (lr & 0xf) {
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 01/10] arm64: KVM: Use static keys for selecting the GIC backend
From: Vladimir Murzin @ 2016-09-12 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473691764-29424-1-git-send-email-vladimir.murzin@arm.com>
Currently GIC backend is selected via alternative framework and this
is fine. We are going to introduce vgic-v3 to 32-bit world and there
we don't have patching framework in hand, so we can either check
support for GICv3 every time we need to choose which backend to use or
try to optimise it by using static keys. The later looks quite
promising because we can share logic involved in selecting GIC backend
between architectures if both uses static keys.
This patch moves arm64 from alternative to static keys framework for
selecting GIC backend. For that we embed static key into vgic_global
and enable the key during vgic initialisation based on what has
already been exposed by the host GIC driver.
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
---
arch/arm64/kvm/hyp/switch.c | 21 +++++++++++----------
include/kvm/arm_vgic.h | 4 ++++
virt/kvm/arm/vgic/vgic-init.c | 4 ++++
virt/kvm/arm/vgic/vgic.c | 2 +-
4 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 5a84b45..d5c4cc5 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -16,6 +16,8 @@
*/
#include <linux/types.h>
+#include <linux/jump_label.h>
+
#include <asm/kvm_asm.h>
#include <asm/kvm_hyp.h>
@@ -126,17 +128,13 @@ static void __hyp_text __deactivate_vm(struct kvm_vcpu *vcpu)
write_sysreg(0, vttbr_el2);
}
-static hyp_alternate_select(__vgic_call_save_state,
- __vgic_v2_save_state, __vgic_v3_save_state,
- ARM64_HAS_SYSREG_GIC_CPUIF);
-
-static hyp_alternate_select(__vgic_call_restore_state,
- __vgic_v2_restore_state, __vgic_v3_restore_state,
- ARM64_HAS_SYSREG_GIC_CPUIF);
-
static void __hyp_text __vgic_save_state(struct kvm_vcpu *vcpu)
{
- __vgic_call_save_state()(vcpu);
+ if (static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif))
+ __vgic_v3_save_state(vcpu);
+ else
+ __vgic_v2_save_state(vcpu);
+
write_sysreg(read_sysreg(hcr_el2) & ~HCR_INT_OVERRIDE, hcr_el2);
}
@@ -149,7 +147,10 @@ static void __hyp_text __vgic_restore_state(struct kvm_vcpu *vcpu)
val |= vcpu->arch.irq_lines;
write_sysreg(val, hcr_el2);
- __vgic_call_restore_state()(vcpu);
+ if (static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif))
+ __vgic_v3_restore_state(vcpu);
+ else
+ __vgic_v2_restore_state(vcpu);
}
static bool __hyp_text __true_value(void)
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 19b698e..994665a 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -23,6 +23,7 @@
#include <linux/types.h>
#include <kvm/iodev.h>
#include <linux/list.h>
+#include <linux/jump_label.h>
#define VGIC_V3_MAX_CPUS 255
#define VGIC_V2_MAX_CPUS 8
@@ -63,6 +64,9 @@ struct vgic_global {
/* Only needed for the legacy KVM_CREATE_IRQCHIP */
bool can_emulate_gicv2;
+
+ /* GIC system register CPU interface */
+ struct static_key_false gicv3_cpuif;
};
extern struct vgic_global kvm_vgic_global_state;
diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index 83777c1..14d6718 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -405,6 +405,10 @@ int kvm_vgic_hyp_init(void)
break;
case GIC_V3:
ret = vgic_v3_probe(gic_kvm_info);
+ if (!ret) {
+ static_branch_enable(&kvm_vgic_global_state.gicv3_cpuif);
+ kvm_info("GIC system register CPU interface\n");
+ }
break;
default:
ret = -ENODEV;
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index e83b7fe..8a529a7 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -29,7 +29,7 @@
#define DEBUG_SPINLOCK_BUG_ON(p)
#endif
-struct vgic_global __section(.hyp.text) kvm_vgic_global_state;
+struct vgic_global __section(.hyp.text) kvm_vgic_global_state = {.gicv3_cpuif = STATIC_KEY_FALSE_INIT,};
/*
* Locking order is always:
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 00/10] ARM: KVM: Support for vgic-v3
From: Vladimir Murzin @ 2016-09-12 14:49 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
This is an attempt to make use vgic-v3 under arch/arm since
save-restore functionality got re-written in C and can be shared
between arm/arm64 like it has already been done for vgic-v2 and timer.
With this patches I'm able to get 32 core an AArch32 ARMv8 guest boot:
...
GICv3: CPU31: found redistributor 703 region 0:0x000000003ffd0000
CPU31: thread -1, cpu 3, socket 7, mpidr 80000703
Brought up 32 CPUs
SMP: Total of 32 processors activated (768.00 BogoMIPS).
CPU: All CPU(s) started in SVC mode.
...
Additionally, quite lightweight test based on Self IPI guest test[1]
has been run with up to 255 cpus.
[1] http://www.spinics.net/lists/kvm/msg128974.html
Changelog:
v3 -> v4
- rebased on v4.8-rc6
- dropped hyp_data in preference to embedding the static key
in kvm_vgic_global_state
- GICv3 cpu sysreg mappings were moved out to separate patch
- vgic patch was split into two: changes extract_bytes() and
fixes for compiler's warnings
- change for MPIDR_AFFINITY_LEVEL was dropped
- 32-bit system register accessors were moved under
CONFIG_CPU_CP15
v2 -> v3
- rebased on v4.8-rc5
- commit messages are reworked to be more precise and clear (I
hope so)
- MPIDR_HWID_BITMASK is used to discard Aff3 in
MPIDR_AFFINITY_LEVEL macro
- cast to u64 is used instead of abuse of GENMASK_ULL while
building value for typer register
- static keys are used to select GIC backend
- config option to guard ITS code is moved to separate patch
v1 -> v2
- rebased on v4.8-rc2
- introduced guard for ITS code
Thanks!
Vladimir Murzin (10):
arm64: KVM: Use static keys for selecting the GIC backend
arm64: KVM: Move GIC accessors to arch_gicv3.h
arm64: KVM: Move vgic-v3 save/restore to virt/kvm/arm/hyp
KVM: arm64: vgic-its: Introduce config option to guard ITS specific
code
KVM: arm: vgic: Fix compiler warnings when built for 32-bit
KVM: arm: vgic: Support 64-bit data manipulation on 32-bit host
systems
ARM: Introduce MPIDR_LEVEL_SHIFT macro
ARM: Move system register accessors to asm/cp15.h
ARM: gic-v3: Introduce 32-to-64-bit mappings for GICv3 cpu registers
ARM: KVM: Support vgic-v3
arch/arm/include/asm/arch_gicv3.h | 91 +++++++++++++++++----
arch/arm/include/asm/cp15.h | 15 ++++
arch/arm/include/asm/cputype.h | 1 +
arch/arm/include/asm/kvm_asm.h | 3 +
arch/arm/include/asm/kvm_host.h | 5 ++
arch/arm/include/asm/kvm_hyp.h | 18 +---
arch/arm/include/uapi/asm/kvm.h | 7 ++
arch/arm/kvm/Makefile | 2 +
arch/arm/kvm/coproc.c | 35 ++++++++
arch/arm/kvm/hyp/Makefile | 1 +
arch/arm/kvm/hyp/switch.c | 12 ++-
arch/arm64/include/asm/arch_gicv3.h | 13 +++
arch/arm64/kvm/Kconfig | 4 +-
arch/arm64/kvm/hyp/Makefile | 2 +-
arch/arm64/kvm/hyp/switch.c | 21 ++---
include/kvm/arm_vgic.h | 12 +--
{arch/arm64/kvm => virt/kvm/arm}/hyp/vgic-v3-sr.c | 13 ---
virt/kvm/arm/vgic/vgic-init.c | 4 +
virt/kvm/arm/vgic/vgic-kvm-device.c | 11 +--
virt/kvm/arm/vgic/vgic-mmio-v3.c | 8 +-
virt/kvm/arm/vgic/vgic-mmio.c | 2 -
virt/kvm/arm/vgic/vgic-mmio.h | 4 +-
virt/kvm/arm/vgic/vgic.c | 2 +-
virt/kvm/arm/vgic/vgic.h | 54 +-----------
24 files changed, 205 insertions(+), 135 deletions(-)
rename {arch/arm64/kvm => virt/kvm/arm}/hyp/vgic-v3-sr.c (96%)
--
1.7.9.5
^ permalink raw reply
* [PATCH] Revert "Input: bma150 - extend chip detection for bma180"
From: Hans de Goede @ 2016-09-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2C735D34-7559-4A62-A635-A5719C356D94@goldelico.com>
Hi,
On 12-09-16 16:31, H. Nikolaus Schaller wrote:
> Hi,
>
>> Am 11.09.2016 um 18:43 schrieb Hans de Goede <hdegoede@redhat.com>:
>>
>> This reverts commit ef3714fdbc8d ("Input: bma150 - extend chip
>> detection for bma180").
>>
>> The bma180 is not compatible with the bma150 at all, it has 14 bits
>> resolution instead of 10, and it has quite different control registers.
>>
>> Treating the bma180 as a bma150 wrt its data registers will just result
>> in throwing away the lowest 4 bits, which is not too bad. But the ctrl
>> registers are a different story.
>>
>> It may be that things happen to just work (I don't have a bma180 to
>> test with) but that certainly does not make this right.
>
> Yes, looks as if your observation is right. Thanks for pointing this out!
>
> This did need me some research to find an answer...
>
> If I remember correctly, the original patch was based on a recommendation
> from someone I don't remember, who said that the chips are the same in a
> different package. So we added the chip_id and it worked immediately as
> expected. It looks as if we did not check the data sheets. So we did not
> question this recommendation.
>
>>
>> Removing the bma180 id also removes overlap wrt the ids in the iio
>> bma180 driver which does treat the bma180 properly.
>
> Nack.
>
> The problem we get is that an iio driver is not an input driver and can't
> easily replace it.
Actually almost all accelerometer drivers in the Linux kernel are
iio drivers, the bma150 driver is the odd duck out, that is why
we've iio-sensor-proxy for apps which want the accelerometer
to behave as an input device:
https://github.com/hadess/iio-sensor-proxy
> An input driver can be used for gesture applications (e.g. detecting
> the device has been turned upside down) and can report X/Y/Z coordinates
> to e.g. X11 for games similar to a mouse or joystick (which the iio driver
> doesn't).
See above.
> So it should remain configurable which of both driver options is loaded,
> to match user space API needs.
>
> BTW: id overlap is only a problem if both drivers are configured in parallel.
Right, so it is "only" a problem to any generic distro which tries to
support both bma150 and bma250 accelerometers, as both the input
bma150 as well as the iio bma250 driver claim to be bma180 compatible.
> Next, I have tried to find out which devices really use the bma150 and bma180.
>
> It appears that no in-tree device uses the bma150 while the GTA04 uses
> the bma180 (in DT), but not in any defconfig (the GTA04 specific config
> is not upstreamable since omap2plus_defconfig exists).
>
> In user-space the GTA04 requires and actively uses this input driver for Replicant.
>
> So I would conclude that this revert does not improve/fix any device using a
> bma150 (if it exists at all), but breaks an existing device.
>
> What options do we have?
>
> a) add proper register number constants and choose conditionally (where they differ)
> b) drop bma150 support completely and change registers for bma180
> c) clone the bma150 input driver into an bma180 input driver and fix registers
> d) extend the bma180 iio driver to optionally provide an input device
> e) write a generic input/iio-accel wrapper (which should work with any iio accelerometer)
>
> I would favour approach e)
Good, because that solution already exists :) See:
https://github.com/hadess/iio-sensor-proxy
> but it has an issue I have no solution for:
>
> how to define in DT (or CONFIG?) which iio accelerometer(s) should
> be wrapped and presented as input device(s).
I believe currently iio-sensor-proxy simply wraps all accelerometers
it can find, which seems the right thing to do. Usually we're running
a generic desktop-ish OS / distro which wants these devices
to be available as input devices; for special cases like actual
robots and stuff running Linux, iio-sensor-proxy can simply be
disabled; or not installed at all.
So all in all I believe that this is a solved problem, since
solution e. from above is already implemented.
Regards,
Hans
>
> Well, it could be as simple as defining a virtual "input-iio-accel" wrapper
> driver with no real hardware behind and provide a reference to the iio DT node.
> But I think such virtual devices are against DT style.
>
> So I don't know how to implement it in an acceptable way.
>
> Ideas?
>
> BR and thanks,
> Nikolaus Schaller
>
>>
>> Cc: Dr. H. Nikolaus Schaller <hns@goldelico.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/input/misc/bma150.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
>> index b0d4453..cae4832 100644
>> --- a/drivers/input/misc/bma150.c
>> +++ b/drivers/input/misc/bma150.c
>> @@ -70,7 +70,6 @@
>> #define BMA150_CFG_5_REG 0x11
>>
>> #define BMA150_CHIP_ID 2
>> -#define BMA180_CHIP_ID 3
>> #define BMA150_CHIP_ID_REG BMA150_DATA_0_REG
>>
>> #define BMA150_ACC_X_LSB_REG BMA150_DATA_2_REG
>> @@ -539,7 +538,7 @@ static int bma150_probe(struct i2c_client *client,
>> }
>>
>> chip_id = i2c_smbus_read_byte_data(client, BMA150_CHIP_ID_REG);
>> - if (chip_id != BMA150_CHIP_ID && chip_id != BMA180_CHIP_ID) {
>> + if (chip_id != BMA150_CHIP_ID) {
>> dev_err(&client->dev, "BMA150 chip id error: %d\n", chip_id);
>> return -EINVAL;
>> }
>> @@ -643,7 +642,6 @@ static UNIVERSAL_DEV_PM_OPS(bma150_pm, bma150_suspend, bma150_resume, NULL);
>>
>> static const struct i2c_device_id bma150_id[] = {
>> { "bma150", 0 },
>> - { "bma180", 0 },
>> { "smb380", 0 },
>> { "bma023", 0 },
>> { }
>> --
>> 2.9.3
>>
>
^ permalink raw reply
* [GIT PULL] ARM: module PLT optimizations
From: Ard Biesheuvel @ 2016-09-12 14:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912143747.GX1041@n2100.armlinux.org.uk>
On 12 September 2016 at 15:37, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Mon, Sep 12, 2016 at 03:35:53PM +0100, Ard Biesheuvel wrote:
>> On 30 August 2016 at 17:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > Hello Russell,
>> >
>> > The following changes since commit 3eab887a55424fc2c27553b7bfe32330df83f7b8:
>> >
>> > Linux 4.8-rc4 (2016-08-28 15:04:33 -0700)
>> >
>> > are available in the git repository at:
>> >
>> > git://git.linaro.org/people/ard.biesheuvel/linux-arm.git \
>> > tags/arm-plt-optimizations-for-v4.9
>> >
>> > for you to fetch changes up to 66e94ba3c8ea5ff5f1443a50441f953ef44010b1:
>> >
>> > ARM: kernel: avoid brute force search on PLT generation
>> > (2016-08-30 17:45:34 +0100)
>> >
>> > ----------------------------------------------------------------
>> > This series of 4 patches optimizes the ARM PLT generation code that
>> > is invoked at module load time, to get rid of the O(n^2) algorithm
>> > that results in pathological load times of 10 seconds or more for
>> > large modules on certain STB platforms
>> >
>> > ----------------------------------------------------------------
>> > Ard Biesheuvel (4):
>> > ARM: kernel: merge core and init PLTs
>> > ARM: kernel: allocate PLT entries only for external symbols
>> > ARM: kernel: sort relocation sections before allocating PLTs
>> > ARM: kernel: avoid brute force search on PLT generation
>> >
>> > arch/arm/include/asm/module.h | 6 +-
>> > arch/arm/kernel/module-plts.c | 243 +++++++++++++++++++++++++-----------------
>> > arch/arm/kernel/module.lds | 3 +-
>> > 3 files changed, 147 insertions(+), 105 deletions(-)
>> >
>>
>> Ping?
>
> It's been pulled, been through testing and I'm going to be pushing it
> out into -next soon.
>
OK, thanks.
^ permalink raw reply
* [GIT PULL] ARM: module PLT optimizations
From: Russell King - ARM Linux @ 2016-09-12 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu_q0BOYqNmap-BGe_p2Cm7qgRMuJ_eKZFMANpJiMwqbVg@mail.gmail.com>
On Mon, Sep 12, 2016 at 03:35:53PM +0100, Ard Biesheuvel wrote:
> On 30 August 2016 at 17:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > Hello Russell,
> >
> > The following changes since commit 3eab887a55424fc2c27553b7bfe32330df83f7b8:
> >
> > Linux 4.8-rc4 (2016-08-28 15:04:33 -0700)
> >
> > are available in the git repository at:
> >
> > git://git.linaro.org/people/ard.biesheuvel/linux-arm.git \
> > tags/arm-plt-optimizations-for-v4.9
> >
> > for you to fetch changes up to 66e94ba3c8ea5ff5f1443a50441f953ef44010b1:
> >
> > ARM: kernel: avoid brute force search on PLT generation
> > (2016-08-30 17:45:34 +0100)
> >
> > ----------------------------------------------------------------
> > This series of 4 patches optimizes the ARM PLT generation code that
> > is invoked at module load time, to get rid of the O(n^2) algorithm
> > that results in pathological load times of 10 seconds or more for
> > large modules on certain STB platforms
> >
> > ----------------------------------------------------------------
> > Ard Biesheuvel (4):
> > ARM: kernel: merge core and init PLTs
> > ARM: kernel: allocate PLT entries only for external symbols
> > ARM: kernel: sort relocation sections before allocating PLTs
> > ARM: kernel: avoid brute force search on PLT generation
> >
> > arch/arm/include/asm/module.h | 6 +-
> > arch/arm/kernel/module-plts.c | 243 +++++++++++++++++++++++++-----------------
> > arch/arm/kernel/module.lds | 3 +-
> > 3 files changed, 147 insertions(+), 105 deletions(-)
> >
>
> Ping?
It's been pulled, been through testing and I'm going to be pushing it
out into -next soon.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [GIT PULL] ARM: module PLT optimizations
From: Ard Biesheuvel @ 2016-09-12 14:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu-=C_WwO30D=0nZCx9FMhj7qrHFzvkk4xnG81Y0t1bdUw@mail.gmail.com>
On 30 August 2016 at 17:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Hello Russell,
>
> The following changes since commit 3eab887a55424fc2c27553b7bfe32330df83f7b8:
>
> Linux 4.8-rc4 (2016-08-28 15:04:33 -0700)
>
> are available in the git repository at:
>
> git://git.linaro.org/people/ard.biesheuvel/linux-arm.git \
> tags/arm-plt-optimizations-for-v4.9
>
> for you to fetch changes up to 66e94ba3c8ea5ff5f1443a50441f953ef44010b1:
>
> ARM: kernel: avoid brute force search on PLT generation
> (2016-08-30 17:45:34 +0100)
>
> ----------------------------------------------------------------
> This series of 4 patches optimizes the ARM PLT generation code that
> is invoked at module load time, to get rid of the O(n^2) algorithm
> that results in pathological load times of 10 seconds or more for
> large modules on certain STB platforms
>
> ----------------------------------------------------------------
> Ard Biesheuvel (4):
> ARM: kernel: merge core and init PLTs
> ARM: kernel: allocate PLT entries only for external symbols
> ARM: kernel: sort relocation sections before allocating PLTs
> ARM: kernel: avoid brute force search on PLT generation
>
> arch/arm/include/asm/module.h | 6 +-
> arch/arm/kernel/module-plts.c | 243 +++++++++++++++++++++++++-----------------
> arch/arm/kernel/module.lds | 3 +-
> 3 files changed, 147 insertions(+), 105 deletions(-)
>
Ping?
^ permalink raw reply
* [PATCH v5 2/3] mfd: add support for Allwinner SoCs ADC
From: Maxime Ripard @ 2016-09-12 14:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912135655.GC9789@dell>
On Mon, Sep 12, 2016 at 02:56:55PM +0100, Lee Jones wrote:
> > >>> Then use .data = <defined model ID> and set up a switch() in .probe().
> > >>
> > >> Uh? Why? It just adds a non-standard indirection, while using
> > >> of_match_device is very standard, and used extensively in Linux.
> > >
> > > You still use of_match_device() to obtain the ID.
> > >
> > > The "don't mix DT with the MFD API" is there to prevent some of the
> > > nasty hacks I've seen previously. This particular example doesn't
> > > seem so bad, but it's a gateway to ridiculous hackery!
> >
> > How am I supposed to get the .data without of_match_node then?
> > What's more hackish in using .data field for specific data for each
> > compatible than in using a random ID in .data and switching on it? The
> > result is exactly the same, the switching case being more verbose and
> > adding complexity to something that can be done in a straightforward manner.
>
> I've already agreed that your implementation isn't terrible, but I'd
> still like to remain strict on the rules.
>
> Better still, can you can dynamically test which platform you're on,
> via a version register or similar?
>
> Failing that, see how everyone else does it:
>
> `git grep "\.data" -- drivers/mfd/`
Just to make sure, you prefer something like
static struct my_struct data = {
};
static struct my_struct data2 = {
};
struct of_device_id matches[] = {
{ compatible = "...", data = <ID> },
{ compatible = "...", data = <ID2> },
};
of_id = of_match_device (dev, matches);
switch (of_id->data) {
case <ID>:
function(data);
case <ID2>:
function(data2);
};
over
static struct my_struct data = {
};
static struct my_struct data2 = {
};
struct of_device_id matches[] = {
{ compatible = "...", data = data },
{ compatible = "...", data = data2 },
};
of_id = of_match_device (dev, matches);
function(of_id->data);
?
This is the *only* time this is going to be used in that driver. I can
understand the need for a version if you need to apply quirks in
several functions, but here it clearly looks suboptimal.
And we are indeed using this construct in the AXP MFD, and it just
doesn't scale either and become quite difficult to maintain when you
have a significant number of variants, and then you have to patch
*all* the switch instances to get something done.
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: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160912/a8dbac92/attachment.sig>
^ permalink raw reply
* [PATCH] ARM: dma-mapping: add in missing white space in error message text
From: Russell King - ARM Linux @ 2016-09-12 14:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <db1758f8-0335-6661-e797-b2021682a087@arm.com>
On Mon, Sep 12, 2016 at 02:06:09PM +0100, Robin Murphy wrote:
> On 12/09/16 13:52, Colin King wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > A dev_warn message spans two lines and the literal string is missing
> > a white space between words. Add the white space.
> >
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > ---
> > arch/arm/common/dmabounce.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/common/dmabounce.c b/arch/arm/common/dmabounce.c
> > index 3012816..f47767b 100644
> > --- a/arch/arm/common/dmabounce.c
> > +++ b/arch/arm/common/dmabounce.c
> > @@ -547,7 +547,7 @@ void dmabounce_unregister_dev(struct device *dev)
> >
> > if (!device_info) {
> > dev_warn(dev,
> > - "Never registered with dmabounce but attempting"
> > + "Never registered with dmabounce but attempting "
> > "to unregister!\n");
>
> Per Documentation/CodingStyle, better to put the whole thing onto one
> line so that, say, "git grep 'attempting to unregister'" doesn't leave
> one scratching ones head in confusion.
>
> Of course, even better would be to get rid of the whole thing and
> convert ARM to use SWIOTLB, but hey, one thing at a time ;)
I doubt that can happen - dmabounce does a little more than swiotlb
because it copes with DMA masks that are _not_ a number of zeros
followed by one bits. It was written to support things like SA1111
DMA, where the device has a bug in that it can't drive certain
SDRAM address bits correctly.
It also gets used for ITE8512 PCI controllers and IXP4xx platforms,
although these should probably be converted to SWIOTLB. However,
without having test systems, I'd recommend leaving them as-is.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH] Revert "Input: bma150 - extend chip detection for bma180"
From: H. Nikolaus Schaller @ 2016-09-12 14:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160911164321.23122-1-hdegoede@redhat.com>
Hi,
> Am 11.09.2016 um 18:43 schrieb Hans de Goede <hdegoede@redhat.com>:
>
> This reverts commit ef3714fdbc8d ("Input: bma150 - extend chip
> detection for bma180").
>
> The bma180 is not compatible with the bma150 at all, it has 14 bits
> resolution instead of 10, and it has quite different control registers.
>
> Treating the bma180 as a bma150 wrt its data registers will just result
> in throwing away the lowest 4 bits, which is not too bad. But the ctrl
> registers are a different story.
>
> It may be that things happen to just work (I don't have a bma180 to
> test with) but that certainly does not make this right.
Yes, looks as if your observation is right. Thanks for pointing this out!
This did need me some research to find an answer...
If I remember correctly, the original patch was based on a recommendation
from someone I don't remember, who said that the chips are the same in a
different package. So we added the chip_id and it worked immediately as
expected. It looks as if we did not check the data sheets. So we did not
question this recommendation.
>
> Removing the bma180 id also removes overlap wrt the ids in the iio
> bma180 driver which does treat the bma180 properly.
Nack.
The problem we get is that an iio driver is not an input driver and can't
easily replace it.
An input driver can be used for gesture applications (e.g. detecting
the device has been turned upside down) and can report X/Y/Z coordinates
to e.g. X11 for games similar to a mouse or joystick (which the iio driver
doesn't).
So it should remain configurable which of both driver options is loaded,
to match user space API needs.
BTW: id overlap is only a problem if both drivers are configured in parallel.
Next, I have tried to find out which devices really use the bma150 and bma180.
It appears that no in-tree device uses the bma150 while the GTA04 uses
the bma180 (in DT), but not in any defconfig (the GTA04 specific config
is not upstreamable since omap2plus_defconfig exists).
In user-space the GTA04 requires and actively uses this input driver for Replicant.
So I would conclude that this revert does not improve/fix any device using a
bma150 (if it exists at all), but breaks an existing device.
What options do we have?
a) add proper register number constants and choose conditionally (where they differ)
b) drop bma150 support completely and change registers for bma180
c) clone the bma150 input driver into an bma180 input driver and fix registers
d) extend the bma180 iio driver to optionally provide an input device
e) write a generic input/iio-accel wrapper (which should work with any iio accelerometer)
I would favour approach e) but it has an issue I have no solution for:
how to define in DT (or CONFIG?) which iio accelerometer(s) should
be wrapped and presented as input device(s).
Well, it could be as simple as defining a virtual "input-iio-accel" wrapper
driver with no real hardware behind and provide a reference to the iio DT node.
But I think such virtual devices are against DT style.
So I don't know how to implement it in an acceptable way.
Ideas?
BR and thanks,
Nikolaus Schaller
>
> Cc: Dr. H. Nikolaus Schaller <hns@goldelico.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/input/misc/bma150.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
> index b0d4453..cae4832 100644
> --- a/drivers/input/misc/bma150.c
> +++ b/drivers/input/misc/bma150.c
> @@ -70,7 +70,6 @@
> #define BMA150_CFG_5_REG 0x11
>
> #define BMA150_CHIP_ID 2
> -#define BMA180_CHIP_ID 3
> #define BMA150_CHIP_ID_REG BMA150_DATA_0_REG
>
> #define BMA150_ACC_X_LSB_REG BMA150_DATA_2_REG
> @@ -539,7 +538,7 @@ static int bma150_probe(struct i2c_client *client,
> }
>
> chip_id = i2c_smbus_read_byte_data(client, BMA150_CHIP_ID_REG);
> - if (chip_id != BMA150_CHIP_ID && chip_id != BMA180_CHIP_ID) {
> + if (chip_id != BMA150_CHIP_ID) {
> dev_err(&client->dev, "BMA150 chip id error: %d\n", chip_id);
> return -EINVAL;
> }
> @@ -643,7 +642,6 @@ static UNIVERSAL_DEV_PM_OPS(bma150_pm, bma150_suspend, bma150_resume, NULL);
>
> static const struct i2c_device_id bma150_id[] = {
> { "bma150", 0 },
> - { "bma180", 0 },
> { "smb380", 0 },
> { "bma023", 0 },
> { }
> --
> 2.9.3
>
^ permalink raw reply
* [PATCH][V2] dma-mapping: add in missing white space in error message text
From: Russell King - ARM Linux @ 2016-09-12 14:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912130952.2778-1-colin.king@canonical.com>
On Mon, Sep 12, 2016 at 02:09:52PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> A dev_warn message spans two lines and the literal string is missing
> a white space between words. Add the white space and reformat the
> message to not span multiple lines.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> arch/arm/common/dmabounce.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/arm/common/dmabounce.c b/arch/arm/common/dmabounce.c
> index 3012816..afeb5e3 100644
> --- a/arch/arm/common/dmabounce.c
> +++ b/arch/arm/common/dmabounce.c
> @@ -546,9 +546,7 @@ void dmabounce_unregister_dev(struct device *dev)
> set_dma_ops(dev, NULL);
>
> if (!device_info) {
> - dev_warn(dev,
> - "Never registered with dmabounce but attempting"
> - "to unregister!\n");
> + dev_warn(dev, "Never registered with dmabounce but attempting to unregister!\n");
Almost!
dev_warn(dev,
"Never registered with dmabounce but attempting to unregister!\n");
The point is that the string is as greppable as possible, while trying to
keep to the rest of the style.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* ARM, SoC: About the use DT-defined properties by 3rd-party drivers
From: Warner Losh @ 2016-09-12 14:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912135549.GA14165@leverpostej>
On Mon, Sep 12, 2016 at 8:01 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>> Since the question seems understood, do you have an example of other SoC's
>> doing something similar?
>
> I do not have an example. I know that others are using DT for data
> beyond what Linux or another OS requires, but it's my understanding that
> that is typically in a separate DTB.
Just to clarify: FreeBSD uses, for the most part, the DTB's that the
'vendor' ships, which is quite often the same ones included in Linux.
There's some exceptions where the bindings weren't really hardware
independent, or where the abstraction model was really Linux specific
(for things like the HDMI stack).
However, with the advent of overlays, one would think that a vendor
could easily include an overlay with the DTB data for the devices they
don't wish to, or cannot for other reasons release. It seems like the
perfect mechanism to comply with the rules about inclusion of nodes in
the DTS. Vendors are free to document these nodes and don't require
the Linux kernel include them in the Documents directory to do so.
There have been recent efforts to move this documentation to a third
party to maintain.
Warner
^ permalink raw reply
* [PATCH] arm64: mm: move zero page from .bss to right before swapper_pg_dir
From: Catalin Marinas @ 2016-09-12 14:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912125710.GA13899@leverpostej>
On Mon, Sep 12, 2016 at 01:57:10PM +0100, Mark Rutland wrote:
> On Sun, Sep 11, 2016 at 03:38:34PM +0100, Ard Biesheuvel wrote:
> > Move the statically allocated zero page from the .bss section to right
> > before swapper_pg_dir. This allows us to refer to its physical address
> > by simply reading TTBR1_EL1 (which always points to swapper_pg_dir and
> > always has its ASID field cleared), and subtracting PAGE_SIZE.
>
> On a conflicting note, I was hoping to move the zero page into .rodata
> so as to catch any erroneous modification.
>
> Given that we can't rely on TTBR1 poiting at the swapper_pg_dir, that
> leaves us with Image size reduction vs RO-ification.
I think zero page in .rodata + reserved_ttbr0 would increase the image
size by 2 pages vs current mainline. That's not much but I'm not sure
it's worth; we have other types of writable data with security
implications, probably more than the zero page.
--
Catalin
^ permalink raw reply
* [PATCH] arm64: mm: move zero page from .bss to right before swapper_pg_dir
From: Ard Biesheuvel @ 2016-09-12 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912125710.GA13899@leverpostej>
On 12 September 2016 at 13:57, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi,
>
> On Sun, Sep 11, 2016 at 03:38:34PM +0100, Ard Biesheuvel wrote:
>> Move the statically allocated zero page from the .bss section to right
>> before swapper_pg_dir. This allows us to refer to its physical address
>> by simply reading TTBR1_EL1 (which always points to swapper_pg_dir and
>> always has its ASID field cleared), and subtracting PAGE_SIZE.
>
> On a conflicting note, I was hoping to move the zero page into .rodata
> so as to catch any erroneous modification.
>
> Given that we can't rely on TTBR1 poiting at the swapper_pg_dir, that
> leaves us with Image size reduction vs RO-ification.
>
> Any thoughts/preference?
>
That's a good point. v2 coming up ...
>> Inspired-by: http://marc.info/?l=linux-arm-kernel&m=147282867511801
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> arch/arm64/include/asm/mmu_context.h | 10 ++++++----
>> arch/arm64/kernel/head.S | 1 -
>> arch/arm64/kernel/vmlinux.lds.S | 2 ++
>> arch/arm64/mm/mmu.c | 1 -
>> 4 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
>> index b1892a0dbcb0..94461ba5febd 100644
>> --- a/arch/arm64/include/asm/mmu_context.h
>> +++ b/arch/arm64/include/asm/mmu_context.h
>> @@ -49,13 +49,15 @@ static inline void contextidr_thread_switch(struct task_struct *next)
>> */
>> static inline void cpu_set_reserved_ttbr0(void)
>> {
>> - unsigned long ttbr = virt_to_phys(empty_zero_page);
>> + unsigned long dummy;
>>
>> - asm(
>> + asm volatile(
>> + " mrs %0, ttbr1_el1 // get TTBR1\n"
>> + " sub %0, %0, %1 // subtract PAGE_SIZE\n"
>> " msr ttbr0_el1, %0 // set TTBR0\n"
>> " isb"
>> - :
>> - : "r" (ttbr));
>> + : "=&r" (dummy)
>> + : "I" (PAGE_SIZE));
>> }
>>
>> /*
>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index 8bc9458f9add..6020b884b076 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -449,7 +449,6 @@ __primary_switched:
>> adr_l x2, __bss_stop
>> sub x2, x2, x0
>> bl __pi_memset
>> - dsb ishst // Make zero page visible to PTW
>>
>> #ifdef CONFIG_KASAN
>> bl kasan_early_init
>> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
>> index 659963d40bb4..a14eb8ff5144 100644
>> --- a/arch/arm64/kernel/vmlinux.lds.S
>> +++ b/arch/arm64/kernel/vmlinux.lds.S
>> @@ -193,6 +193,8 @@ SECTIONS
>> . = ALIGN(PAGE_SIZE);
>> idmap_pg_dir = .;
>> . += IDMAP_DIR_SIZE;
>> + empty_zero_page = .;
>> + . += PAGE_SIZE;
>> swapper_pg_dir = .;
>> . += SWAPPER_DIR_SIZE;
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 4989948d1feb..539ce9d11325 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -53,7 +53,6 @@ EXPORT_SYMBOL(kimage_voffset);
>> * Empty_zero_page is a special page that is used for zero-initialized data
>> * and COW.
>> */
>> -unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)] __page_aligned_bss;
>> EXPORT_SYMBOL(empty_zero_page);
>>
>> static pte_t bm_pte[PTRS_PER_PTE] __page_aligned_bss;
>> --
>> 2.7.4
>>
>>
>> _______________________________________________
>> 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] arm64: mm: move zero page from .bss to right before swapper_pg_dir
From: Ard Biesheuvel @ 2016-09-12 14:16 UTC (permalink / raw)
To: linux-arm-kernel
Move the statically allocated zero page from the .bss section to right
before swapper_pg_dir. This allows us to refer to its physical address
by simply reading TTBR1_EL1 (which always points to swapper_pg_dir and
always has its ASID field cleared), and subtracting PAGE_SIZE.
To protect the zero page from inadvertent modification, carve out a
segment that covers it as well as idmap_pg_dir[], and mark it read-only
in both the primary and the linear mappings of the kernel.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v2: make empty_zero_page[] read-only
make idmap_pg_dir[] read-only as well
fix issue in v1 with cpu_reserved_ttbr0()
This is perhaps becoming a bit unwieldy, but I agree with Mark that having
a read-only zero page is a significant improvement.
arch/arm64/include/asm/mmu_context.h | 19 +++----
arch/arm64/include/asm/sections.h | 1 +
arch/arm64/kernel/vmlinux.lds.S | 14 ++++-
arch/arm64/mm/mmu.c | 56 ++++++++++++--------
4 files changed, 57 insertions(+), 33 deletions(-)
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index b1892a0dbcb0..1fe4c4422f0a 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -49,13 +49,12 @@ static inline void contextidr_thread_switch(struct task_struct *next)
*/
static inline void cpu_set_reserved_ttbr0(void)
{
- unsigned long ttbr = virt_to_phys(empty_zero_page);
-
- asm(
- " msr ttbr0_el1, %0 // set TTBR0\n"
- " isb"
- :
- : "r" (ttbr));
+ /*
+ * The zero page is located right before swapper_pg_dir, whose
+ * physical address we can easily fetch from TTBR1_EL1.
+ */
+ write_sysreg(read_sysreg(ttbr1_el1) - PAGE_SIZE, ttbr0_el1);
+ isb();
}
/*
@@ -109,7 +108,8 @@ static inline void cpu_uninstall_idmap(void)
{
struct mm_struct *mm = current->active_mm;
- cpu_set_reserved_ttbr0();
+ write_sysreg(virt_to_phys(empty_zero_page), ttbr0_el1);
+ isb();
local_flush_tlb_all();
cpu_set_default_tcr_t0sz();
@@ -119,7 +119,8 @@ static inline void cpu_uninstall_idmap(void)
static inline void cpu_install_idmap(void)
{
- cpu_set_reserved_ttbr0();
+ write_sysreg(virt_to_phys(empty_zero_page), ttbr0_el1);
+ isb();
local_flush_tlb_all();
cpu_set_idmap_tcr_t0sz();
diff --git a/arch/arm64/include/asm/sections.h b/arch/arm64/include/asm/sections.h
index 4e7e7067afdb..44e94e234ba0 100644
--- a/arch/arm64/include/asm/sections.h
+++ b/arch/arm64/include/asm/sections.h
@@ -26,5 +26,6 @@ extern char __hyp_text_start[], __hyp_text_end[];
extern char __idmap_text_start[], __idmap_text_end[];
extern char __irqentry_text_start[], __irqentry_text_end[];
extern char __mmuoff_data_start[], __mmuoff_data_end[];
+extern char __robss_start[], __robss_end[];
#endif /* __ASM_SECTIONS_H */
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 5ce9b2929e0d..eae5036dc725 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -209,9 +209,19 @@ SECTIONS
BSS_SECTION(0, 0, 0)
- . = ALIGN(PAGE_SIZE);
+ . = ALIGN(SEGMENT_ALIGN);
+ __robss_start = .;
idmap_pg_dir = .;
- . += IDMAP_DIR_SIZE;
+ . = ALIGN(. + IDMAP_DIR_SIZE + PAGE_SIZE, SEGMENT_ALIGN);
+ __robss_end = .;
+
+ /*
+ * Put the zero page right before swapper_pg_dir so we can easily
+ * obtain its physical address by subtracting PAGE_SIZE from the
+ * contents of TTBR1_EL1.
+ */
+ empty_zero_page = __robss_end - PAGE_SIZE;
+
swapper_pg_dir = .;
. += SWAPPER_DIR_SIZE;
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index e634a0f6d62b..adb00035a6a4 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -54,7 +54,6 @@ EXPORT_SYMBOL(kimage_voffset);
* Empty_zero_page is a special page that is used for zero-initialized data
* and COW.
*/
-unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)] __page_aligned_bss;
EXPORT_SYMBOL(empty_zero_page);
static pte_t bm_pte[PTRS_PER_PTE] __page_aligned_bss;
@@ -321,16 +320,18 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end)
{
- unsigned long kernel_start = __pa(_text);
- unsigned long kernel_end = __pa(__init_begin);
+ unsigned long text_start = __pa(_text);
+ unsigned long text_end = __pa(__init_begin);
+ unsigned long robss_start = __pa(__robss_start);
+ unsigned long robss_end = __pa(__robss_end);
/*
* Take care not to create a writable alias for the
- * read-only text and rodata sections of the kernel image.
+ * read-only text/rodata/robss sections of the kernel image.
*/
- /* No overlap with the kernel text/rodata */
- if (end < kernel_start || start >= kernel_end) {
+ /* No overlap with the kernel text/rodata/robss */
+ if (end < text_start || start >= robss_end) {
__create_pgd_mapping(pgd, start, __phys_to_virt(start),
end - start, PAGE_KERNEL,
early_pgtable_alloc,
@@ -342,27 +343,32 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end
* This block overlaps the kernel text/rodata mappings.
* Map the portion(s) which don't overlap.
*/
- if (start < kernel_start)
- __create_pgd_mapping(pgd, start,
- __phys_to_virt(start),
- kernel_start - start, PAGE_KERNEL,
+ if (start < text_start)
+ __create_pgd_mapping(pgd, start, __phys_to_virt(start),
+ text_start - start, PAGE_KERNEL,
early_pgtable_alloc,
!debug_pagealloc_enabled());
- if (kernel_end < end)
- __create_pgd_mapping(pgd, kernel_end,
- __phys_to_virt(kernel_end),
- end - kernel_end, PAGE_KERNEL,
+ if (robss_end < end)
+ __create_pgd_mapping(pgd, robss_end, __phys_to_virt(robss_end),
+ end - robss_end, PAGE_KERNEL,
early_pgtable_alloc,
!debug_pagealloc_enabled());
/*
- * Map the linear alias of the [_text, __init_begin) interval as
- * read-only/non-executable. This makes the contents of the
- * region accessible to subsystems such as hibernate, but
- * protects it from inadvertent modification or execution.
+ * Map the linear alias of the intervals [_text, __init_begin) and
+ * [robss_start, robss_end) as read-only/non-executable. This makes
+ * the contents of these regions accessible to subsystems such
+ * as hibernate, but protects them from inadvertent modification or
+ * execution.
*/
- __create_pgd_mapping(pgd, kernel_start, __phys_to_virt(kernel_start),
- kernel_end - kernel_start, PAGE_KERNEL_RO,
+ __create_pgd_mapping(pgd, text_start, __phys_to_virt(text_start),
+ text_end - text_start, PAGE_KERNEL_RO,
+ early_pgtable_alloc, !debug_pagealloc_enabled());
+ __create_pgd_mapping(pgd, text_end, __phys_to_virt(text_end),
+ robss_start - text_end, PAGE_KERNEL,
+ early_pgtable_alloc, !debug_pagealloc_enabled());
+ __create_pgd_mapping(pgd, robss_start, __phys_to_virt(robss_start),
+ robss_end - robss_start, PAGE_KERNEL_RO,
early_pgtable_alloc, !debug_pagealloc_enabled());
}
@@ -436,13 +442,19 @@ static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
*/
static void __init map_kernel(pgd_t *pgd)
{
- static struct vm_struct vmlinux_text, vmlinux_rodata, vmlinux_init, vmlinux_data;
+ static struct vm_struct vmlinux_text, vmlinux_rodata, vmlinux_init,
+ vmlinux_data, vmlinux_robss, vmlinux_tail;
map_kernel_segment(pgd, _text, _etext, PAGE_KERNEL_EXEC, &vmlinux_text);
map_kernel_segment(pgd, __start_rodata, __init_begin, PAGE_KERNEL, &vmlinux_rodata);
map_kernel_segment(pgd, __init_begin, __init_end, PAGE_KERNEL_EXEC,
&vmlinux_init);
- map_kernel_segment(pgd, _data, _end, PAGE_KERNEL, &vmlinux_data);
+ map_kernel_segment(pgd, _data, __robss_start, PAGE_KERNEL,
+ &vmlinux_data);
+ map_kernel_segment(pgd, __robss_start, __robss_end, PAGE_KERNEL_RO,
+ &vmlinux_robss);
+ map_kernel_segment(pgd, __robss_end, _end, PAGE_KERNEL,
+ &vmlinux_tail);
if (!pgd_val(*pgd_offset_raw(pgd, FIXADDR_START))) {
/*
--
2.7.4
^ permalink raw reply related
* [PATCH] BUG: atmel_serial: Interrupts not disabled on close
From: Uwe Kleine-König @ 2016-09-12 14:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912133441.14597-1-richard.genoud@gmail.com>
On Mon, Sep 12, 2016 at 03:34:41PM +0200, Richard Genoud wrote:
> Since commit 18dfef9c7f87 ("serial: atmel: convert to irq handling
> provided mctrl-gpio"), interrupts from GPIOs are not disabled any more
> when the serial port is closed, leading to an oops when the one of the
> input pin is toggled (CTS/DSR/DCD/RNG).
>
> This is only the case if those pins are used as GPIOs, i.e. declared
> like that:
> usart1: serial at f8020000 {
> /* CTS and DTS will be handled by GPIO */
> status = "okay";
> rts-gpios = <&pioB 17 GPIO_ACTIVE_LOW>;
> cts-gpios = <&pioB 16 GPIO_ACTIVE_LOW>;
> dtr-gpios = <&pioB 14 GPIO_ACTIVE_LOW>;
> dsr-gpios = <&pioC 31 GPIO_ACTIVE_LOW>;
> rng-gpios = <&pioB 12 GPIO_ACTIVE_LOW>;
> dcd-gpios = <&pioB 15 GPIO_ACTIVE_LOW>;
> };
>
> That's because modem interrupts used to be freed in atmel_shutdown().
> After commit 18dfef9c7f87 ("serial: atmel: convert to irq handling
> provided mctrl-gpio"), this code was just removed.
> Calling atmel_disable_ms() disables the interrupts and everything works
> fine again.
>
> Tested on at91sam9g35-cm
>
> (This patch doesn't apply on -stable kernels, fixes for 4.4 and 4.7 will
> be sent after this one is applied.)
AFAIK this is no reason to not put stable on Cc. If then
$stablemaintainer fails to backport the change, he will contact you.
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> Fixes: 18dfef9c7f87 ("serial: atmel: convert to irq handling provided mctrl-gpio")
Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
Thanks for caring
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH] arm64: hw_breakpoint: Handle inexact watchpoint addresses
From: Pavel Labath @ 2016-09-12 14:07 UTC (permalink / raw)
To: linux-arm-kernel
Arm64 hardware does not always report a watchpoint hit address that
matches one of the watchpoints set. It can also report an address
"near" the watchpoint if a single instruction access both watched and
unwatched addresses. There is no straight-forward way, short of
disassembling the offending instruction, to map that address back to
the watchpoint.
Previously, when the hardware reported a watchpoint hit on an address
that did not match our watchpoint (this happens in case of instructions
which access large chunks of memory such as "stp") the process would
enter a loop where we would be continually resuming it (because we did
not recognise that watchpoint hit) and it would keep hitting the
watchpoint again and again. The tracing process would never get
notified of the watchpoint hit.
This commit fixes the problem by looking at the watchpoints near the
address reported by the hardware. If the address does not exactly match
one of the watchpoints we have set, it attributes the hit to the
nearest watchpoint we have. This heuristic is a bit dodgy, but I don't
think we can do much more, given the hardware limitations.
I include a kernel selftest which triggers this code.
Signed-off-by: Pavel Labath <labath@google.com>
---
arch/arm64/kernel/hw_breakpoint.c | 104 +++++++---
tools/testing/selftests/breakpoints/Makefile | 5 +-
.../selftests/breakpoints/breakpoint_test-arm.c | 217 +++++++++++++++++++++
3 files changed, 298 insertions(+), 28 deletions(-)
create mode 100644 tools/testing/selftests/breakpoints/breakpoint_test-arm.c
diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index 26a6bf7..d0ebfe6 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c
@@ -661,50 +661,80 @@ unlock:
}
NOKPROBE_SYMBOL(breakpoint_handler);
+/*
+ * Arm64 hardware does not always report a watchpoint hit address that matches
+ * one of the watchpoints set. It can also report an address "near" the
+ * watchpoint if a single instruction access both watched and unwatched
+ * addresses. There is no straight-forward way, short of disassembling the
+ * offending instruction, to map that address back to the watchpoint. This
+ * function computes the distance of the memory access from the watchpoint as a
+ * heuristic for the likelyhood that a given access triggered the watchpoint.
+ *
+ * See Section D2.10.5 "Determining the memory location that caused a Watchpoint
+ * exception" of ARMv8 Architecture Reference Manual for details.
+ *
+ * The function returns the distance of the address from the bytes watched by
+ * the watchpoint. In case of an exact match, it returns 0.
+ */
+static u64 get_distance_from_watchpoint(unsigned long addr, int i,
+ struct arch_hw_breakpoint *info)
+{
+ u64 val, alignment_mask, wp_low, wp_high;
+ u32 ctrl_reg;
+ int first_bit;
+ struct arch_hw_breakpoint_ctrl ctrl;
+
+ /* AArch32 watchpoints are either 4 or 8 bytes aligned. */
+ if (is_compat_task()) {
+ if (info->ctrl.len == ARM_BREAKPOINT_LEN_8)
+ alignment_mask = 0x7;
+ else
+ alignment_mask = 0x3;
+ } else {
+ alignment_mask = 0x7;
+ }
+
+ val = read_wb_reg(AARCH64_DBG_REG_WVR, i) & ~alignment_mask;
+
+ ctrl_reg = read_wb_reg(AARCH64_DBG_REG_WCR, i);
+ decode_ctrl_reg(ctrl_reg, &ctrl);
+ first_bit = ffs(ctrl.len);
+ if (first_bit == 0)
+ return -1;
+ wp_low = val + first_bit - 1;
+ wp_high = val + fls(ctrl.len) - 1;
+ if (addr < wp_low)
+ return wp_low - addr;
+ else if (addr > wp_high)
+ return addr - wp_high;
+ else
+ return 0;
+
+}
+
static int watchpoint_handler(unsigned long addr, unsigned int esr,
struct pt_regs *regs)
{
- int i, step = 0, *kernel_step, access;
- u32 ctrl_reg;
- u64 val, alignment_mask;
+ int i, step = 0, *kernel_step, access, closest_match;
+ u64 min_dist = -1, dist;
struct perf_event *wp, **slots;
struct debug_info *debug_info;
struct arch_hw_breakpoint *info;
- struct arch_hw_breakpoint_ctrl ctrl;
slots = this_cpu_ptr(wp_on_reg);
debug_info = ¤t->thread.debug;
+ /*
+ * Find all watchpoints that match the reported address. If no exact
+ * match is found. Attribute the hit to the closest watchpoint.
+ */
for (i = 0; i < core_num_wrps; ++i) {
rcu_read_lock();
wp = slots[i];
-
if (wp == NULL)
goto unlock;
- info = counter_arch_bp(wp);
- /* AArch32 watchpoints are either 4 or 8 bytes aligned. */
- if (is_compat_task()) {
- if (info->ctrl.len == ARM_BREAKPOINT_LEN_8)
- alignment_mask = 0x7;
- else
- alignment_mask = 0x3;
- } else {
- alignment_mask = 0x7;
- }
-
- /* Check if the watchpoint value matches. */
- val = read_wb_reg(AARCH64_DBG_REG_WVR, i);
- if (val != (addr & ~alignment_mask))
- goto unlock;
-
- /* Possible match, check the byte address select to confirm. */
- ctrl_reg = read_wb_reg(AARCH64_DBG_REG_WCR, i);
- decode_ctrl_reg(ctrl_reg, &ctrl);
- if (!((1 << (addr & alignment_mask)) & ctrl.len))
- goto unlock;
-
/*
* Check that the access type matches.
* 0 => load, otherwise => store
@@ -714,6 +744,17 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
if (!(access & hw_breakpoint_type(wp)))
goto unlock;
+ info = counter_arch_bp(wp);
+
+ dist = get_distance_from_watchpoint(addr, i, info);
+ if (dist < min_dist) {
+ min_dist = dist;
+ closest_match = i;
+ }
+ /* Is this an exact match? */
+ if (dist != 0)
+ goto unlock;
+
info->trigger = addr;
perf_bp_event(wp, regs);
@@ -724,6 +765,15 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
unlock:
rcu_read_unlock();
}
+ if (min_dist > 0 && min_dist != -1) {
+ /* No exact match found. */
+ rcu_read_lock();
+ wp = slots[closest_match];
+ info = counter_arch_bp(wp);
+ info->trigger = addr;
+ perf_bp_event(wp, regs);
+ rcu_read_unlock();
+ }
if (!step)
return 0;
diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
index 74e533f..458a31a 100644
--- a/tools/testing/selftests/breakpoints/Makefile
+++ b/tools/testing/selftests/breakpoints/Makefile
@@ -5,6 +5,9 @@ ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
ifeq ($(ARCH),x86)
TEST_PROGS := breakpoint_test
endif
+ifeq ($(ARCH),arm64)
+TEST_PROGS := breakpoint_test-arm
+endif
TEST_PROGS += step_after_suspend_test
@@ -13,4 +16,4 @@ all: $(TEST_PROGS)
include ../lib.mk
clean:
- rm -fr breakpoint_test step_after_suspend_test
+ rm -fr breakpoint_test breakpoint_test-arm step_after_suspend_test
diff --git a/tools/testing/selftests/breakpoints/breakpoint_test-arm.c b/tools/testing/selftests/breakpoints/breakpoint_test-arm.c
new file mode 100644
index 0000000..9f00ea6
--- /dev/null
+++ b/tools/testing/selftests/breakpoints/breakpoint_test-arm.c
@@ -0,0 +1,217 @@
+/*
+ * Copyright (C) 2016 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ */
+
+#define _GNU_SOURCE
+
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <sys/ptrace.h>
+#include <sys/uio.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <elf.h>
+#include <signal.h>
+
+#include "../kselftest.h"
+
+enum Test {
+ TEST_WRITE_1, TEST_WRITE_2, TEST_WRITE_4, TEST_WRITE_8,
+ TEST_WRITE_16, TEST_WRITE_32, TEST_MAX
+};
+
+struct Data {
+ union {
+ uint8_t u8[32];
+ uint16_t u16[16];
+ uint32_t u32[8];
+ uint64_t u64[4];
+ };
+};
+volatile struct Data var __aligned(32);
+
+
+void child(enum Test test)
+{
+ if (ptrace(PTRACE_TRACEME, 0, NULL, NULL) != 0) {
+ perror("ptrace(PTRACE_TRACEME) failed");
+ _exit(1);
+ }
+
+ if (raise(SIGSTOP) != 0) {
+ perror("raise(SIGSTOP) failed");
+ _exit(1);
+ }
+
+ switch (test) {
+ case TEST_WRITE_1:
+ var.u8[31] = 47;
+ break;
+ case TEST_WRITE_2:
+ var.u16[15] = 47;
+ break;
+ case TEST_WRITE_4:
+ var.u32[7] = 47;
+ break;
+ case TEST_WRITE_8:
+ var.u64[3] = 47;
+ break;
+ case TEST_WRITE_16:
+ __asm__ volatile ("stp x29, x30, %0" : "=m" (var.u64[2]));
+ break;
+ case TEST_WRITE_32:
+ __asm__ volatile ("stp q29, q30, %0" : "=m" (var));
+ break;
+ }
+
+ _exit(0);
+}
+
+static bool set_watchpoint(pid_t pid, const volatile void *address,
+ size_t size)
+{
+ const unsigned byte_mask = (1 << size) - 1;
+ const unsigned type = 2; /* Write */
+ const unsigned enable = 1;
+ const unsigned control = byte_mask << 5 | type << 3 | enable;
+ struct user_hwdebug_state dreg_state;
+ struct iovec iov;
+
+ memset(&dreg_state, 0, sizeof(dreg_state));
+ dreg_state.dbg_regs[0].addr = (uintptr_t)address;
+ dreg_state.dbg_regs[0].ctrl = control;
+ iov.iov_base = &dreg_state;
+ iov.iov_len = offsetof(struct user_hwdebug_state, dbg_regs) +
+ sizeof(dreg_state.dbg_regs[0]);
+ if (ptrace(PTRACE_SETREGSET, pid, NT_ARM_HW_WATCH, &iov) == 0)
+ return true;
+
+ if (errno == EIO) {
+ printf("ptrace(PTRACE_SETREGSET, NT_ARM_HW_WATCH) "
+ "not supported on this hardware\n");
+ ksft_exit_skip();
+ }
+ perror("ptrace(PTRACE_SETREGSET, NT_ARM_HW_WATCH) failed");
+ return false;
+}
+
+
+bool run_test(enum Test test)
+{
+ int status;
+ siginfo_t siginfo;
+ pid_t pid = fork();
+ pid_t wpid;
+
+ if (pid < 0) {
+ perror("fork() failed");
+ return false;
+ }
+ if (pid == 0)
+ child(test);
+
+ wpid = waitpid(pid, &status, __WALL);
+ if (wpid != pid) {
+ perror("waitpid() failed");
+ return false;
+ }
+ if (!WIFSTOPPED(status)) {
+ printf("child did not stop\n");
+ return false;
+ }
+ if (WSTOPSIG(status) != SIGSTOP) {
+ printf("child did not stop with SIGSTOP\n");
+ return false;
+ }
+
+ if (!set_watchpoint(pid, &var.u64[3], 8))
+ return false;
+
+ if (ptrace(PTRACE_CONT, pid, NULL, NULL) < 0) {
+ perror("ptrace(PTRACE_SINGLESTEP) failed");
+ return false;
+ }
+
+ alarm(3);
+ wpid = waitpid(pid, &status, __WALL);
+ if (wpid != pid) {
+ perror("waitpid() failed");
+ return false;
+ }
+ alarm(0);
+ if (WIFEXITED(status)) {
+ printf("child did not single-step\n");
+ return false;
+ }
+ if (!WIFSTOPPED(status)) {
+ printf("child did not stop\n");
+ return false;
+ }
+ if (WSTOPSIG(status) != SIGTRAP) {
+ printf("child did not stop with SIGTRAP\n");
+ return false;
+ }
+ if (ptrace(PTRACE_GETSIGINFO, pid, NULL, &siginfo) != 0) {
+ perror("ptrace(PTRACE_GETSIGINFO)");
+ return false;
+ }
+ if (siginfo.si_code != TRAP_HWBKPT) {
+ printf("Unexpected si_code %d\n", siginfo.si_code);
+ return false;
+ }
+
+ kill(pid, SIGKILL);
+ wpid = waitpid(pid, &status, 0);
+ if (wpid != pid) {
+ perror("waitpid() failed");
+ return false;
+ }
+ return true;
+}
+
+void sigalrm(int sig)
+{
+}
+
+int main(int argc, char **argv)
+{
+ int opt;
+ bool succeeded = true;
+ enum Test test;
+ struct sigaction act;
+
+ act.sa_handler = sigalrm;
+ sigemptyset(&act.sa_mask);
+ act.sa_flags = 0;
+ sigaction(SIGALRM, &act, NULL);
+ for (test = 0; test < TEST_MAX; ++test) {
+ printf("Test %d ", test);
+ if (run_test(test)) {
+ printf("[OK]\n");
+ ksft_inc_pass_cnt();
+ } else {
+ printf("[FAILED]\n");
+ ksft_inc_fail_cnt();
+ succeeded = false;
+ }
+ }
+
+ ksft_print_cnts();
+ if (succeeded)
+ ksft_exit_pass();
+ else
+ ksft_exit_fail();
+}
+
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [linux-sunxi] Re: [RFC] misc: Add Allwinner Q8 tablet hardware manager
From: Rob Herring @ 2016-09-12 14:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <268244d0-55c3-167d-a70f-0e35e6b3dae6@redhat.com>
On Sat, Sep 10, 2016 at 08:12:32PM +0200, Hans de Goede wrote:
> Hi,
>
> On 09-09-16 23:41, Rob Herring wrote:
> >On Thu, Sep 1, 2016 at 2:08 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> >>Allwinnner A13 / A23 / A33 based Q8 tablets are popular cheap 7" tablets
> >>of which a new batch is produced every few weeks. Each batch uses a
> >>different mix of touchscreen, accelerometer and wifi peripherals.
> >>
> >>Given that each batch is different creating a devicetree for each variant
> >>is not desirable. This commit adds a Q8 tablet hardware manager which
> >>auto-detects the touchscreen and accelerometer so that a single generic
> >>dts can be used for these tablets.
> >>
> >>The wifi is connected to a discoverable bus (sdio or usb) and will be
> >>autodetected by the mmc resp. usb subsystems.
> >>
> >>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>---
> >> .../misc/allwinner,sunxi-q8-hardwaremgr.txt | 52 +++
> >> drivers/misc/Kconfig | 12 +
> >> drivers/misc/Makefile | 1 +
> >> drivers/misc/q8-hardwaremgr.c | 512 +++++++++++++++++++++
> >> 4 files changed, 577 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/misc/allwinner,sunxi-q8-hardwaremgr.txt
> >> create mode 100644 drivers/misc/q8-hardwaremgr.c
> >>
> >>diff --git a/Documentation/devicetree/bindings/misc/allwinner,sunxi-q8-hardwaremgr.txt b/Documentation/devicetree/bindings/misc/allwinner,sunxi-q8-hardwaremgr.txt
> >>new file mode 100644
> >>index 0000000..f428bf5
> >>--- /dev/null
> >>+++ b/Documentation/devicetree/bindings/misc/allwinner,sunxi-q8-hardwaremgr.txt
> >>@@ -0,0 +1,52 @@
> >>+Q8 tablet hardware manager
> >>+--------------------------
> >>+
> >>+Allwinnner A13 / A23 / A33 based Q8 tablets are popular cheap 7" tablets of
> >>+which a new batch is produced every few weeks. Each batch uses a different
> >>+mix of touchscreen, accelerometer and wifi peripherals.
> >>+
> >>+Given that each batch is different creating a devicetree for each variant is
> >>+not desirable. The Q8 tablet hardware manager bindings are bindings for an os
> >>+module which auto-detects the touchscreen so that a single
> >>+generic dts can be used for these tablets.
> >>+
> >>+The wifi is connected to a discoverable bus and will be autodetected by the os.
> >>+
> >>+Required properties:
> >>+ - compatible : "allwinner,sunxi-q8-hardwaremgr"
> >>+ - touchscreen : phandle of a template touchscreen node, this must be a
> >>+ child node of the touchscreen i2c bus
> >>+
> >>+Optional properties:
> >>+ - touchscreen-supply : regulator phandle for the touchscreen vdd supply
> >
> >While I said I think you should be using overlays here, you could also
> >do it without.
>
> Good, because I believe that doing things without overlays will
> be much easier. If you look at the actual implementation you will
> see that it sets a lot of properties (touchscreen width, height,
> inversion, axis-swapping, firmware-name) based on various sources,
> the explosion of possible overlays from this is huge, and if
> we're going to runtime patch the overlays then why use them at all
> and not just runtime generate all the info ?
That's fine if run-time makes more sense. I'd just like to move in the
direction of applying a hunk of changes rather than C code creating
nodes and properties one-by-one.
> >However, this node has to go. It is not h/w, and you
> >are putting it here purely to instantiate a driver.
>
> With "this" in "this node has to go", do you mean the node
> with the "allwinner,sunxi-q8-hardwaremgr" compatible, or
> do you mean the touchscreen template node ?
Given that this binding section above is concerning
allwinner,sunxi-q8-hardwaremgr, that is what I'm referring to.
>
> >For the
> >touchscreen property, surely you know where the touchscreen is located
> >in the DT?
>
> Nope q8 tablets come with A13, A23 or A33 SoCs, A23/A33 are pin
> compatible with each other, but the A13 is not, this leads to
> the touchscreen being on a different i2c bus, so I cannot hardcode
> things, likewise the power-gpios and interrupt found in the
> touchscreen node differ between the A13 vs A23/A33.
Either the touchscreen node name is "touchscreen" or you can have
compatible string for q8-touchscreen and use of_find_node_by_name or
of_find_compatible_node. Doesn't matter what bus it is on. It only
requires that there is only 1 node.
> I actually had a touchscreen-i2c-bus property containing a
> phandle to the i2c controller for the bus which has the
> touchscreen in an earlier revision + touchscreen-gpios
> and touchscreen-supply properties, I can switch to that
> if you prefer that over having a touchscreen template node.
>
> > If not, of_find_node_by_name()?
>
> Still a bit confused about what you do not like,
> (touchscreen template node vs q8-hardwaremgr node).
>
> I guess (based on context) you do not want the
> q8-hardwaremgr node ?
>
> So the q8-hardwaremgr code should activate based on
> the machine compatible I presume?
Either that, or if you have the board specific compatible in the
touchscreen node, then you could have a driver for it. That driver would
need to probe the device and then create the "real" device.
> How does that
> work with module autoloading ?
Can't non-DT based platform drivers do autoloading? Regardless, not
really my problem from a binding perspective.
> If you do not want the q8-hardwaremgr node and thus
> no touchscreen property them yes I can use
> of_find_node_by_name(), but I thought that was
> generally frowned up on?
Generally we do prefer finding things by compatible strings. But there
are exceptions and doing fix-ups like this is a good example.
> >For touchscreen-supply, I
> >assume this is to turn on the supply so you can talk to the touch
> >controller. There's no reason the supply can't just be in the
> >touchscreen node itself.
>
> Only a few q8 tablets actually use the regulator, so
> the hardwaremgr first tries detecting the touchscreen
> without it, and only if it does not find anything then
> tries with it, and adds a property to the touchscreen
> node for it, but I guess I can do this other way
> around and have it be present in the (incomplete /
> template) touchscreen node and delete it from it if
> not necessary.
Right.
Rob
^ permalink raw reply
* [PATCHv6 0/3] pwm: imx: support output polarity inversion
From: Uwe Kleine-König @ 2016-09-12 14:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912124553.aqgv7b4vsx4urogi@piout.net>
Hello,
On Mon, Sep 12, 2016 at 02:45:53PM +0200, Alexandre Belloni wrote:
> Isn't a properly designed PWM putting a high level on its pin when
> disabled and configured with inversed polarity ?
it's not well defined. When trying several times over the years to
properly define and document it, I didn't manage to agree with Thierry
what is the right thing to define.
IMHO it would be sensible to make it explicitly undefined what happens
when a PMW is disabled. This would simplify drivers from
pwm_config(mypwm, value, period);
if (!value)
pwm_disable(mypwm)
else
pwm_enable(led_dat->pwm);
to
pwm_config(mypwm, value, period);
and let the pwm driver disable it's clock (or whatever) when value is 0
and there are energy saving benefits that don't hurt the expected
behaviour of the pin. So the hardware specific stuff is handled in the
hardware specific driver and usage in pwm-consumers is simplified.
Moreover this also simplifies some pwm drivers because they don't have
to catch in software the cases where the hardware differs from the
expectation[1].
Looking at drivers/leds/leds-pwm.c it doesn't ensure that each
pwm_enable is paired by an pwm_disable (e.g. on .remove). Is this a bug?
With my purposed semantics of .config and .disable this would be much
easier to fix.
Regarding your question: Yeah, maybe all properly designed PWMs behave
like you expect. But reality isn't only about properly designed
hardware, so I wouldn't expect all hardware to behave. The inverse
property might be software emulated and so on pwm_disable the pin might
become 0.
The obvious downside of my suggestion is that this is a change in what
most people expect (because it was "safe" to call pwm_enable before),
but the resulting code is simpler and cleaner.
Today it's a (maybe small) bug, when a pwm consumer calls pwm_config with
value=0 and doesn't disable it afterwards. IMHO that's a bug in the pwm
API that pwm_config with value=0 doesn't imply (the wanted effects of)
pwm_disable.
Best regards
Uwe
[1] This might even be impossible: Consider a PWM that gets 0 (or
high-z) on hw-disable independent of configured duty or inversion. The
driver now sees for an inverted pwm: pwm_config(this, 0, 100);
pwm_disable(this); The driver cannot know if it should continue to drive
the pin at 1, or if the pwm consumer stopped caring about the pwm and
disabling the hardware is OK.
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* ARM, SoC: About the use DT-defined properties by 3rd-party drivers
From: Mark Rutland @ 2016-09-12 14:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D6AA54.6000208@laposte.net>
Hi,
On Mon, Sep 12, 2016 at 03:15:00PM +0200, Sebastian Frias wrote:
> On 09/12/2016 02:38 PM, Mark Rutland wrote:
> >>
> >> 3rd party users of said SoC could then write kernel modules for such HW
> >> blocks using the DT description. The DT would thus become the authoritative
> >> source of information regarding register programming for the SoC.
> >
> > I don't follow this part entirely. Why are you expecting thrid parties
> > to write a driver for those blocks rather than upstreaming a driver for
> > them?
>
> 3rd parties could choose to write a driver (as opposed to use say, a user-mode
> library) if it fits their programming model better, if they think they would
> have better performance, or other reasons.
A vendor can always choose to "add value" in this manner. The general
expectation of *some* driver being upstreamed remains.
> > I would generally agree that the hardware should be described in DT.
> > The difficulty is that without a 'real' user it's not always possible to
> > tell if we're describing the thing correctly.
>
> That may be true, but so far we are not discussing changing DT's API so it
> should not have big ramifications.
You're not changing the code, but you are creating a binding. Bindings
are intended to be stable (i.e. a working DTB from today should continue
to work in future), and thus there are ramifications.
Few devices these says are entirely independent, and most devices can be
instantiated multiple times (even if there happens to only be a single
instance in practice). For the example of a userspace driver there are
very real ABI concerns, such as how the device(s) are discovered, how
any related components like regulators and clocks are controlled, etc.
There are ramifications here, and it's a dangerous over simplification
to say that this doesn't matter because we're not changing kernel code.
> Besides, what "makes sense now" may "not make sense tomorrow" depending on
> how the HW is modified.
That's always the case when a new generation of hardware comes out, so I
don't think that's relevant to the topic at hand.
> We have somehow learned the hard-way that "le mieux est l'ennemi du bien"
> (the better is the enemy of the good) and we are trying to take a more
> practical (and flexible) approach.
>
> > Putting smoething together that's only sufficient to support some
> > out-of-tree driver with implicit assumptions that we are not aware of is
> > far from fantastic.
>
> That does not seem very positive and it is not the case anyway, otherwise we
> would not be consulting here :-)
> Agreed, right now this whole thing seems like a really hypothetical question,
> but the intention is good.
>
> Actually, I think it would encourage more SoC manufacturers to use DT as a way
> to document their HW, which is a good thing.
Writing and reviewing bindings is a very tricky topic, as it can require
fairly intimate knowledge of a piece of hardware. I've repeatedly found
that binding descriptions did not match the realities of the hardware,
and I've only managed to do so by looking at accompanying driver code.
Given that manuals and other information on devices are often not freely
available (if they exist at all), the proposal effectively limits myself
and others to spot common (anti)patterns, which is far less than ideal,
and will result in more mistakes.
As it stands, the proposal asks for effort for the community (in terms
of review and maintenance of bindings), with no benefit to the kernel
community, and a number of pitfalls that we would rather avoid.
In an ideal world, writing and reviewing bindings would be a simple
affair, and this could happen separately from work on any particular OS.
In practice, things are sufficiently complicated that you need *some*
demonstration that a binding is suitable, which is what I'm personally
after when I ask for a Linux driver.
> >> However, after discussing over IRC, it looks like there was no guidance on
> >> this. Some people think submitting DT properties/nodes without a corresponding
> >> Linux driver is frowned upon, while others thought it was an odd limitation
> >> and suggested asking here.
> >
> > Unfortunately, I think that the area is sufficiently vague that there
> > simply is no clear and general answer.
> >
> > For the sake of discussion, an example of a particular block, along with
> > what you expect/need to describe would be helpful.
>
> I don't have a more concrete example now.
For this discussion to go somewhere, we need an example. Otherwise we're
all coming at this with differing implicit assumptions and no clear
evidence for any assertions.
> As I stated, right now HW description is not centralised, and thus different
> bits of information are cherry-picked by hand from HW description into DT for
> bootloader, DT for Linux, Documentation/headers for 3rd-parties, etc.
>
> But if I understood correctly your comment, you are basically saying that
> without an example is hard to say.
> Since the question seems understood, do you have an example of other SoC's
> doing something similar?
I do not have an example. I know that others are using DT for data
beyond what Linux or another OS requires, but it's my understanding that
that is typically in a separate DTB.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v5 2/3] mfd: add support for Allwinner SoCs ADC
From: Lee Jones @ 2016-09-12 13:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c322d309-993c-a572-5ffd-bf4012e26134@free-electrons.com>
On Mon, 12 Sep 2016, Quentin Schulz wrote:
> On 12/09/2016 12:49, Lee Jones wrote:
> > On Mon, 12 Sep 2016, Maxime Ripard wrote:
> >
> >> On Mon, Sep 12, 2016 at 10:59:23AM +0100, Lee Jones wrote:
> >>>>>> +static const struct of_device_id sun4i_gpadc_mfd_of_match[] = {
> >>>>>> + {
> >>>>>> + .compatible = "allwinner,sun4i-a10-ts",
> >>>>>> + .data = &sun4i_gpadc_mfd_cells,
> >>>>>> + }, {
> >>>>>> + .compatible = "allwinner,sun5i-a13-ts",
> >>>>>> + .data = &sun5i_gpadc_mfd_cells,
> >>>>>> + }, {
> >>>>>> + .compatible = "allwinner,sun6i-a31-ts",
> >>>>>> + .data = &sun6i_gpadc_mfd_cells,
> >>>>>> + }, { /* sentinel */ }
> >>>>>> +};
> >>>>>
> >>>>> Don't mix OF and MFD functionality.
> >>>>>
> >>>>> Why don't you create a node for "iio_hwmon" and have
> >>>>> platform_of_populate() do your bidding?
> >>>>>
> >>>>
> >>>> We are using a stable binding which we cannot modify. This means, the DT
> >>>> in its current state can only be modified to add features, which is not
> >>>> the case of this driver (it is a rewriting of an existing driver which
> >>>> uses the rtp node).
> >>>
> >>> Then use .data = <defined model ID> and set up a switch() in .probe().
> >>
> >> Uh? Why? It just adds a non-standard indirection, while using
> >> of_match_device is very standard, and used extensively in Linux.
> >
> > You still use of_match_device() to obtain the ID.
> >
> > The "don't mix DT with the MFD API" is there to prevent some of the
> > nasty hacks I've seen previously. This particular example doesn't
> > seem so bad, but it's a gateway to ridiculous hackery!
>
> How am I supposed to get the .data without of_match_node then?
> What's more hackish in using .data field for specific data for each
> compatible than in using a random ID in .data and switching on it? The
> result is exactly the same, the switching case being more verbose and
> adding complexity to something that can be done in a straightforward manner.
I've already agreed that your implementation isn't terrible, but I'd
still like to remain strict on the rules.
Better still, can you can dynamically test which platform you're on,
via a version register or similar?
Failing that, see how everyone else does it:
`git grep "\.data" -- drivers/mfd/`
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH v5 2/3] mfd: add support for Allwinner SoCs ADC
From: Lee Jones @ 2016-09-12 13:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5dec118f-45ae-4fb6-dd1e-2028947add53@free-electrons.com>
On Mon, 12 Sep 2016, Quentin Schulz wrote:
> On 12/09/2016 11:59, Lee Jones wrote:
> > On Mon, 12 Sep 2016, Quentin Schulz wrote:
> >
> >> On 12/09/2016 11:18, Lee Jones wrote:
> >>> On Thu, 08 Sep 2016, Quentin Schulz wrote:
> >>>
> >> [...]
> >
> > [...]
> >
> >>>> +++ b/drivers/mfd/sun4i-gpadc-mfd.c
> >
> > [...]
> >
> >>>> +static struct mfd_cell sun4i_gpadc_mfd_cells[] = {
> >>>> + {
> >>>> + .name = "sun4i-a10-gpadc-iio",
> >>>> + .resources = adc_resources,
> >>>> + .num_resources = ARRAY_SIZE(adc_resources),
> >>>> + }, {
> >>>> + .name = "iio_hwmon",
> >>>> + }
> >>>
> >>> Single line please
> >>>
> >>> { .name = "iio_hwmon" }
> >>>
> >>
> >> + {
> >> + .name = "sun4i-a10-gpadc-iio",
> >> + .resources = adc_resources,
> >> + .num_resources = ARRAY_SIZE(adc_resources),
> >> + }, { .name = "iio_hwmon" }
> >>
> >> or
> >>
> >> + {
> >> + .name = "sun4i-a10-gpadc-iio",
> >> + .resources = adc_resources,
> >> + .num_resources = ARRAY_SIZE(adc_resources),
> >> + },
> >> + { .name = "iio_hwmon" }
> >>
> >> ?
> >
> > The latter.
> >
> > [...]
> >
> >>>> +static const struct of_device_id sun4i_gpadc_mfd_of_match[] = {
> >>>> + {
> >>>> + .compatible = "allwinner,sun4i-a10-ts",
> >>>> + .data = &sun4i_gpadc_mfd_cells,
> >>>> + }, {
> >>>> + .compatible = "allwinner,sun5i-a13-ts",
> >>>> + .data = &sun5i_gpadc_mfd_cells,
> >>>> + }, {
> >>>> + .compatible = "allwinner,sun6i-a31-ts",
> >>>> + .data = &sun6i_gpadc_mfd_cells,
> >>>> + }, { /* sentinel */ }
> >>>> +};
> >>>
> >>> Don't mix OF and MFD functionality.
> >>>
> >>> Why don't you create a node for "iio_hwmon" and have
> >>> platform_of_populate() do your bidding?
> >>>
> >>
> >> We are using a stable binding which we cannot modify. This means, the DT
> >> in its current state can only be modified to add features, which is not
> >> the case of this driver (it is a rewriting of an existing driver which
> >> uses the rtp node).
> >
> > Then use .data = <defined model ID> and set up a switch() in .probe().
> >
> >>>> +static int sun4i_gpadc_mfd_probe(struct platform_device *pdev)
> >>>
> >>> Remove all mention of "mfd" from this file.
> >>>
> >>> (Accept the calls to the MFD API of course).
> >>>
> >> [...]
> >>>> +
> >>>> +MODULE_DEVICE_TABLE(of, sun4i_gpadc_mfd_of_match);
> >>>
> >>> Place this directly under the table.
> >>>
> >>>> +static struct platform_driver sun4i_gpadc_mfd_driver = {
> >>>> + .driver = {
> >>>> + .name = "sun4i-adc-mfd",
> >>>> + .of_match_table = of_match_ptr(sun4i_gpadc_mfd_of_match),
> >>>> + },
> >>>> + .probe = sun4i_gpadc_mfd_probe,
> >>>
> >>> No .remove?
> >>>
> >>
> >> No, everything in probe is handled with devm functions.
> >
> > Don't you need to undo the register write you did?
> >
>
> The regmap_write I use is there to disable all interrupts on hardware
> side before the irq_chip handles all interrupts by itself. The
> interrupts are not used in the MFD driver.
>
> Thus, I chose to disable the hardware interrupts in the remove function
> of drivers using the interrupts (only the IIO yet but the touchscreen
> driver later also which will be using a third interrupt). When the MFD
> driver is removed, the MFD cells will all be removed, thus calling their
> own remove functions, thus disabling hardware interrupts used in each
> driver. So the hardware interrupts disabling would be called twice.
This does send some little alarm bells ringing. I'd normally expect
the .remove function to undo everything you did in .probe. So, if you
are disabling the IRQs from within the leaf drivers, shouldn't you be
initialising them in the leaf driver's respective .probes?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCHv6 0/3] pwm: imx: support output polarity inversion
From: Vladimir Zapolskiy @ 2016-09-12 13:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160909091857.7a263220@ipc1.ka-ro>
Hi Lothar,
On 09/09/2016 10:18 AM, Lothar Wa?mann wrote:
> Hi,
>
> On Thu, 08 Sep 2016 15:15:57 -0700 Stefan Agner wrote:
>> On 2014-10-10 07:22, Lothar Wa?mann wrote:
>>> This patch series adds support for polarity inversion to the pwm-imx
>>> driver. The patches have been tested on i.MX6, i.MX53 and with the
>>> ti-ehrpwm.c driver.
>>
>> Do you know what prevented this patchset from getting merged?
>>
> No idea.
>
>> We are looking for Polarity support in PWM for too, this is especially
>> useful for backlight control.
>>
> Actually the PWM driver may be the wrong place to achieve this. When
> the backlight driver sets the brightness to 0 to switch the backlight
> off, it will disable the PWM. This will make the PWM pin go LOW and
> thus turn the backlight to full brightness rather than off (unless there
> is an additional GPIO that controls a backlight enable pin on the LCD).
>
I've just realized that I had submitted practically the same change
(excluding iMX specifics) and about the same time in October 2014, but
my v1 is one and a half hours later than yours preceding v6 :)
Since I've subscribed to the linux-pwm right before sending my changes,
I don't have your changes in my mailbox. Would you mind to review my
v3 "pwm: support backward compatibility of DTB extending PWM args":
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/303833.html
then incorporate anything you find useful into your series and resend
v7? Or just resend the rebased v6 if nothing is found attracting?
In my turn I'll spend time to review the series and test it on iMX.
--
With best wishes,
Vladimir
^ 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