* [PATCH v9 5/9] KVM: arm/arm64: Support a vgic interrupt line level sample function
From: Christoffer Dall @ 2017-12-20 11:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171220113606.7030-1-christoffer.dall@linaro.org>
The GIC sometimes need to sample the physical line of a mapped
interrupt. As we know this to be notoriously slow, provide a callback
function for devices (such as the timer) which can do this much faster
than talking to the distributor, for example by comparing a few
in-memory values. Fall back to the good old method of poking the
physical GIC if no callback is provided.
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
include/kvm/arm_vgic.h | 13 ++++++++++++-
virt/kvm/arm/arch_timer.c | 3 ++-
virt/kvm/arm/vgic/vgic.c | 13 +++++++++----
3 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 8c896540a72c..cdbd142ca7f2 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -130,6 +130,17 @@ struct vgic_irq {
u8 priority;
enum vgic_irq_config config; /* Level or edge */
+ /*
+ * Callback function pointer to in-kernel devices that can tell us the
+ * state of the input level of mapped level-triggered IRQ faster than
+ * peaking into the physical GIC.
+ *
+ * Always called in non-preemptible section and the functions can use
+ * kvm_arm_get_running_vcpu() to get the vcpu pointer for private
+ * IRQs.
+ */
+ bool (*get_input_level)(int vintid);
+
void *owner; /* Opaque pointer to reserve an interrupt
for in-kernel devices. */
};
@@ -331,7 +342,7 @@ void kvm_vgic_init_cpu_hardware(void);
int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,
bool level, void *owner);
int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,
- u32 vintid);
+ u32 vintid, bool (*get_input_level)(int vindid));
int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int vintid);
bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, unsigned int vintid);
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 9376fe03bf2e..fb0533ed903d 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -834,7 +834,8 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
return -EINVAL;
}
- ret = kvm_vgic_map_phys_irq(vcpu, host_vtimer_irq, vtimer->irq.irq);
+ ret = kvm_vgic_map_phys_irq(vcpu, host_vtimer_irq, vtimer->irq.irq,
+ NULL);
if (ret)
return ret;
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index 4318e9edd075..d57b3bf8eb36 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -144,13 +144,15 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)
kfree(irq);
}
-/* Get the input level of a mapped IRQ directly from the physical GIC */
bool vgic_get_phys_line_level(struct vgic_irq *irq)
{
bool line_level;
BUG_ON(!irq->hw);
+ if (irq->get_input_level)
+ return irq->get_input_level(irq->intid);
+
WARN_ON(irq_get_irqchip_state(irq->host_irq,
IRQCHIP_STATE_PENDING,
&line_level));
@@ -436,7 +438,8 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,
/* @irq->irq_lock must be held */
static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
- unsigned int host_irq)
+ unsigned int host_irq,
+ bool (*get_input_level)(int vindid))
{
struct irq_desc *desc;
struct irq_data *data;
@@ -456,6 +459,7 @@ static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
irq->hw = true;
irq->host_irq = host_irq;
irq->hwintid = data->hwirq;
+ irq->get_input_level = get_input_level;
return 0;
}
@@ -464,10 +468,11 @@ static inline void kvm_vgic_unmap_irq(struct vgic_irq *irq)
{
irq->hw = false;
irq->hwintid = 0;
+ irq->get_input_level = NULL;
}
int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,
- u32 vintid)
+ u32 vintid, bool (*get_input_level)(int vindid))
{
struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, vintid);
unsigned long flags;
@@ -476,7 +481,7 @@ int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,
BUG_ON(!irq);
spin_lock_irqsave(&irq->irq_lock, flags);
- ret = kvm_vgic_map_irq(vcpu, irq, host_irq);
+ ret = kvm_vgic_map_irq(vcpu, irq, host_irq, get_input_level);
spin_unlock_irqrestore(&irq->irq_lock, flags);
vgic_put_irq(vcpu->kvm, irq);
--
2.14.2
^ permalink raw reply related
* [PATCH v9 4/9] KVM: arm/arm64: vgic: Support level-triggered mapped interrupts
From: Christoffer Dall @ 2017-12-20 11:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171220113606.7030-1-christoffer.dall@linaro.org>
Level-triggered mapped IRQs are special because we only observe rising
edges as input to the VGIC, and we don't set the EOI flag and therefore
are not told when the level goes down, so that we can re-queue a new
interrupt when the level goes up.
One way to solve this problem is to side-step the logic of the VGIC and
special case the validation in the injection path, but it has the
unfortunate drawback of having to peak into the physical GIC state
whenever we want to know if the interrupt is pending on the virtual
distributor.
Instead, we can maintain the current semantics of a level triggered
interrupt by sort of treating it as an edge-triggered interrupt,
following from the fact that we only observe an asserting edge. This
requires us to be a bit careful when populating the LRs and when folding
the state back in though:
* We lower the line level when populating the LR, so that when
subsequently observing an asserting edge, the VGIC will do the right
thing.
* If the guest never acked the interrupt while running (for example if
it had masked interrupts at the CPU level while running), we have
to preserve the pending state of the LR and move it back to the
line_level field of the struct irq when folding LR state.
If the guest never acked the interrupt while running, but changed the
device state and lowered the line (again with interrupts masked) then
we need to observe this change in the line_level.
Both of the above situations are solved by sampling the physical line
and set the line level when folding the LR back.
* Finally, if the guest never acked the interrupt while running and
sampling the line reveals that the device state has changed and the
line has been lowered, we must clear the physical active state, since
we will otherwise never be told when the interrupt becomes asserted
again.
This has the added benefit of making the timer optimization patches
(https://lists.cs.columbia.edu/pipermail/kvmarm/2017-July/026343.html) a
bit simpler, because the timer code doesn't have to clear the active
state on the sync anymore. It also potentially improves the performance
of the timer implementation because the GIC knows the state or the LR
and only needs to clear the
active state when the pending bit in the LR is still set, where the
timer has to always clear it when returning from running the guest with
an injected timer interrupt.
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++++++++++++++++++++
virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++++++++++++++++++++
virt/kvm/arm/vgic/vgic.c | 23 +++++++++++++++++++++++
virt/kvm/arm/vgic/vgic.h | 7 +++++++
4 files changed, 88 insertions(+)
diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c
index 80897102da26..c32d7b93ffd1 100644
--- a/virt/kvm/arm/vgic/vgic-v2.c
+++ b/virt/kvm/arm/vgic/vgic-v2.c
@@ -105,6 +105,26 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
irq->pending_latch = false;
}
+ /*
+ * Level-triggered mapped IRQs are special because we only
+ * observe rising edges as input to the VGIC.
+ *
+ * If the guest never acked the interrupt we have to sample
+ * the physical line and set the line level, because the
+ * device state could have changed or we simply need to
+ * process the still pending interrupt later.
+ *
+ * If this causes us to lower the level, we have to also clear
+ * the physical active state, since we will otherwise never be
+ * told when the interrupt becomes asserted again.
+ */
+ if (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT)) {
+ irq->line_level = vgic_get_phys_line_level(irq);
+
+ if (!irq->line_level)
+ vgic_irq_set_phys_active(irq, false);
+ }
+
spin_unlock_irqrestore(&irq->irq_lock, flags);
vgic_put_irq(vcpu->kvm, irq);
}
@@ -162,6 +182,15 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
val |= GICH_LR_EOI;
}
+ /*
+ * Level-triggered mapped IRQs are special because we only observe
+ * rising edges as input to the VGIC. We therefore lower the line
+ * level here, so that we can take new virtual IRQs. See
+ * vgic_v2_fold_lr_state for more info.
+ */
+ if (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT))
+ irq->line_level = false;
+
/* The GICv2 LR only holds five bits of priority. */
val |= (irq->priority >> 3) << GICH_LR_PRIORITY_SHIFT;
diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index f47e8481fa45..6b329414e57a 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -96,6 +96,26 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
irq->pending_latch = false;
}
+ /*
+ * Level-triggered mapped IRQs are special because we only
+ * observe rising edges as input to the VGIC.
+ *
+ * If the guest never acked the interrupt we have to sample
+ * the physical line and set the line level, because the
+ * device state could have changed or we simply need to
+ * process the still pending interrupt later.
+ *
+ * If this causes us to lower the level, we have to also clear
+ * the physical active state, since we will otherwise never be
+ * told when the interrupt becomes asserted again.
+ */
+ if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) {
+ irq->line_level = vgic_get_phys_line_level(irq);
+
+ if (!irq->line_level)
+ vgic_irq_set_phys_active(irq, false);
+ }
+
spin_unlock_irqrestore(&irq->irq_lock, flags);
vgic_put_irq(vcpu->kvm, irq);
}
@@ -145,6 +165,15 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
val |= ICH_LR_EOI;
}
+ /*
+ * Level-triggered mapped IRQs are special because we only observe
+ * rising edges as input to the VGIC. We therefore lower the line
+ * level here, so that we can take new virtual IRQs. See
+ * vgic_v3_fold_lr_state for more info.
+ */
+ if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))
+ irq->line_level = false;
+
/*
* We currently only support Group1 interrupts, which is a
* known defect. This needs to be addressed at some point.
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index ecb8e25f5fe5..4318e9edd075 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -144,6 +144,29 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)
kfree(irq);
}
+/* Get the input level of a mapped IRQ directly from the physical GIC */
+bool vgic_get_phys_line_level(struct vgic_irq *irq)
+{
+ bool line_level;
+
+ BUG_ON(!irq->hw);
+
+ WARN_ON(irq_get_irqchip_state(irq->host_irq,
+ IRQCHIP_STATE_PENDING,
+ &line_level));
+ return line_level;
+}
+
+/* Set/Clear the physical active state */
+void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active)
+{
+
+ BUG_ON(!irq->hw);
+ WARN_ON(irq_set_irqchip_state(irq->host_irq,
+ IRQCHIP_STATE_ACTIVE,
+ active));
+}
+
/**
* kvm_vgic_target_oracle - compute the target vcpu for an irq
*
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index efbcf8f96f9c..d0787983a357 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -104,6 +104,11 @@ static inline bool irq_is_pending(struct vgic_irq *irq)
return irq->pending_latch || irq->line_level;
}
+static inline bool vgic_irq_is_mapped_level(struct vgic_irq *irq)
+{
+ return irq->config == VGIC_CONFIG_LEVEL && irq->hw;
+}
+
/*
* This struct provides an intermediate representation of the fields contained
* in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC
@@ -140,6 +145,8 @@ vgic_get_mmio_region(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev,
struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu,
u32 intid);
void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq);
+bool vgic_get_phys_line_level(struct vgic_irq *irq);
+void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active);
bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq,
unsigned long flags);
void vgic_kick_vcpus(struct kvm *kvm);
--
2.14.2
^ permalink raw reply related
* [PATCH v9 3/9] KVM: arm/arm64: Don't cache the timer IRQ level
From: Christoffer Dall @ 2017-12-20 11:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171220113606.7030-1-christoffer.dall@linaro.org>
The timer logic was designed after a strict idea of modeling an
interrupt line level in software, meaning that only transitions in the
level need to be reported to the VGIC. This works well for the timer,
because the arch timer code is in complete control of the device and can
track the transitions of the line.
However, as we are about to support using the HW bit in the VGIC not
just for the timer, but also for VFIO which cannot track transitions of
the interrupt line, we have to decide on an interface between the GIC
and other subsystems for level triggered mapped interrupts, which both
the timer and VFIO can use.
VFIO only sees an asserting transition of the physical interrupt line,
and tells the VGIC when that happens. That means that part of the
interrupt flow is offloaded to the hardware.
To use the same interface for VFIO devices and the timer, we therefore
have to change the timer (we cannot change VFIO because it doesn't know
the details of the device it is assigning to a VM).
Luckily, changing the timer is simple, we just need to stop 'caching'
the line level, but instead let the VGIC know the state of the timer
every time there is a potential change in the line level, and when the
line level should be asserted from the timer ISR. The VGIC can ignore
extra notifications using its validate mechanism.
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Julien Thierry <julien.thierry@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
virt/kvm/arm/arch_timer.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index f9555b1e7f15..9376fe03bf2e 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -99,11 +99,9 @@ static irqreturn_t kvm_arch_timer_handler(int irq, void *dev_id)
}
vtimer = vcpu_vtimer(vcpu);
- if (!vtimer->irq.level) {
- vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);
- if (kvm_timer_irq_can_fire(vtimer))
- kvm_timer_update_irq(vcpu, true, vtimer);
- }
+ vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);
+ if (kvm_timer_irq_can_fire(vtimer))
+ kvm_timer_update_irq(vcpu, true, vtimer);
if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
kvm_vtimer_update_mask_user(vcpu);
@@ -324,12 +322,20 @@ static void kvm_timer_update_state(struct kvm_vcpu *vcpu)
struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
+ bool level;
if (unlikely(!timer->enabled))
return;
- if (kvm_timer_should_fire(vtimer) != vtimer->irq.level)
- kvm_timer_update_irq(vcpu, !vtimer->irq.level, vtimer);
+ /*
+ * The vtimer virtual interrupt is a 'mapped' interrupt, meaning part
+ * of its lifecycle is offloaded to the hardware, and we therefore may
+ * not have lowered the irq.level value before having to signal a new
+ * interrupt, but have to signal an interrupt every time the level is
+ * asserted.
+ */
+ level = kvm_timer_should_fire(vtimer);
+ kvm_timer_update_irq(vcpu, level, vtimer);
if (kvm_timer_should_fire(ptimer) != ptimer->irq.level)
kvm_timer_update_irq(vcpu, !ptimer->irq.level, ptimer);
--
2.14.2
^ permalink raw reply related
* [PATCH v9 2/9] KVM: arm/arm64: Factor out functionality to get vgic mmio requester_vcpu
From: Christoffer Dall @ 2017-12-20 11:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171220113606.7030-1-christoffer.dall@linaro.org>
We are about to distinguish between userspace accesses and mmio traps
for a number of the mmio handlers. When the requester vcpu is NULL, it
means we are handling a userspace access.
Factor out the functionality to get the request vcpu into its own
function, mostly so we have a common place to document the semantics of
the return value.
Also take the chance to move the functionality outside of holding a
spinlock and instead explicitly disable and enable preemption. This
supports PREEMPT_RT kernels as well.
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
virt/kvm/arm/vgic/vgic-mmio.c | 44 +++++++++++++++++++++++++++----------------
1 file changed, 28 insertions(+), 16 deletions(-)
diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
index deb51ee16a3d..fdad95f62fa3 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.c
+++ b/virt/kvm/arm/vgic/vgic-mmio.c
@@ -122,6 +122,27 @@ unsigned long vgic_mmio_read_pending(struct kvm_vcpu *vcpu,
return value;
}
+/*
+ * This function will return the VCPU that performed the MMIO access and
+ * trapped from within the VM, and will return NULL if this is a userspace
+ * access.
+ *
+ * We can disable preemption locally around accessing the per-CPU variable,
+ * and use the resolved vcpu pointer after enabling preemption again, because
+ * even if the current thread is migrated to another CPU, reading the per-CPU
+ * value later will give us the same value as we update the per-CPU variable
+ * in the preempt notifier handlers.
+ */
+static struct kvm_vcpu *vgic_get_mmio_requester_vcpu(void)
+{
+ struct kvm_vcpu *vcpu;
+
+ preempt_disable();
+ vcpu = kvm_arm_get_running_vcpu();
+ preempt_enable();
+ return vcpu;
+}
+
void vgic_mmio_write_spending(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val)
@@ -184,24 +205,10 @@ unsigned long vgic_mmio_read_active(struct kvm_vcpu *vcpu,
static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
bool new_active_state)
{
- struct kvm_vcpu *requester_vcpu;
unsigned long flags;
- spin_lock_irqsave(&irq->irq_lock, flags);
+ struct kvm_vcpu *requester_vcpu = vgic_get_mmio_requester_vcpu();
- /*
- * The vcpu parameter here can mean multiple things depending on how
- * this function is called; when handling a trap from the kernel it
- * depends on the GIC version, and these functions are also called as
- * part of save/restore from userspace.
- *
- * Therefore, we have to figure out the requester in a reliable way.
- *
- * When accessing VGIC state from user space, the requester_vcpu is
- * NULL, which is fine, because we guarantee that no VCPUs are running
- * when accessing VGIC state from user space so irq->vcpu->cpu is
- * always -1.
- */
- requester_vcpu = kvm_arm_get_running_vcpu();
+ spin_lock_irqsave(&irq->irq_lock, flags);
/*
* If this virtual IRQ was written into a list register, we
@@ -213,6 +220,11 @@ static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
* vgic_change_active_prepare) and still has to sync back this IRQ,
* so we release and re-acquire the spin_lock to let the other thread
* sync back the IRQ.
+ *
+ * When accessing VGIC state from user space, requester_vcpu is
+ * NULL, which is fine, because we guarantee that no VCPUs are running
+ * when accessing VGIC state from user space so irq->vcpu->cpu is
+ * always -1.
*/
while (irq->vcpu && /* IRQ may have state in an LR somewhere */
irq->vcpu != requester_vcpu && /* Current thread is not the VCPU thread */
--
2.14.2
^ permalink raw reply related
* [PATCH v9 1/9] KVM: arm/arm64: Remove redundant preemptible checks
From: Christoffer Dall @ 2017-12-20 11:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171220113606.7030-1-christoffer.dall@linaro.org>
The __this_cpu_read() and __this_cpu_write() functions already implement
checks for the required preemption levels when using
CONFIG_DEBUG_PREEMPT which gives you nice error messages and such.
Therefore there is no need to explicitly check this using a BUG_ON() in
the code (which we don't do for other uses of per cpu variables either).
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
virt/kvm/arm/arm.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 6b60c98a6e22..3610e132df8b 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -71,7 +71,6 @@ static DEFINE_PER_CPU(unsigned char, kvm_arm_hardware_enabled);
static void kvm_arm_set_running_vcpu(struct kvm_vcpu *vcpu)
{
- BUG_ON(preemptible());
__this_cpu_write(kvm_arm_running_vcpu, vcpu);
}
@@ -81,7 +80,6 @@ static void kvm_arm_set_running_vcpu(struct kvm_vcpu *vcpu)
*/
struct kvm_vcpu *kvm_arm_get_running_vcpu(void)
{
- BUG_ON(preemptible());
return __this_cpu_read(kvm_arm_running_vcpu);
}
--
2.14.2
^ permalink raw reply related
* [PATCH v9 0/9] Handle forwarded level-triggered interrupts
From: Christoffer Dall @ 2017-12-20 11:35 UTC (permalink / raw)
To: linux-arm-kernel
This series is an alternative approach to Eric Auger's direct EOI setup
patches [1] in terms of the KVM VGIC support.
The idea is to maintain existing semantics for the VGIC for mapped
level-triggered IRQs and also support the timer using mapped IRQs with
the same VGIC support as VFIO interrupts.
Based on v4.15-rc3.
Also available at:
git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git level-mapped-v9
Changes since v8:
- Addd kvm_timer_update_irq(vcpu, false, vtimer) call in
unmask_vtimer_irq_user in patch 7 to fix userspace irqchip support
with these patches.
- Delete outdated documentation instead of erroneously attempting to
update it.
- Fixed weirdo commit message in patch 3
- Attempted alternate approach in patch 8
Changes since v7:
- Cleanup stale commentary
- Updated documentation (patch 9/9 is new in this version)
- Added Eric's reviewed-by tags
Changes since v6:
- Removed double semi-colon
- Changed another confusing conditional in patch 6
- Fixed typos in commit message and comments
Changes since v5:
- Rebased on v4.15-rc1
- Changed comment on preemption code as suggested by Andre
- Fixed white space and confusing conditionals as suggested by Drew
Changes since v4:
- Rebased on the timer optimization series merged in the v4.15 merge
window, which caused a fair amount of modifications to patch 3.
- Added a static key to disable the sync operations when no VMs are
using userspace irqchips to further optimize the performance
- Fixed extra semicolon in vgic-mmio.c
- Added commentary as requested during review
- Dropped what was patch 4, because it was merged as part of GICv4
support.
- Factored out the VGIC input level function change as separate patch
(helps bisect and debugging), before providing a function for the
timer.
Changes since v3:
- Added a number of patches and moved patches around a bit.
- Check for uaccesses in the mmio handler functions
- Fixed bugs in the mmio handler functions
Changes since v2:
- Removed patch 5 from v2 and integrating the changes in what's now
patch 5 to make it easier to reuse code when adding VFIO integration.
- Changed the virtual distributor MMIO handling to use the
pending_latch and more closely match the semantics of SPENDR and
CPENDR for both level and edge mapped interrupts.
Changes since v1:
- Added necessary changes to the timer (Patch 1)
- Added handling of guest MMIO accesses to the virtual distributor
(Patch 4)
- Addressed Marc's comments from the initial RFC (mostly renames)
Thanks,
-Christoffer
[1]: https://lists.cs.columbia.edu/pipermail/kvmarm/2017-June/026072.html
Christoffer Dall (9):
KVM: arm/arm64: Remove redundant preemptible checks
KVM: arm/arm64: Factor out functionality to get vgic mmio
requester_vcpu
KVM: arm/arm64: Don't cache the timer IRQ level
KVM: arm/arm64: vgic: Support level-triggered mapped interrupts
KVM: arm/arm64: Support a vgic interrupt line level sample function
KVM: arm/arm64: Support VGIC dist pend/active changes for mapped IRQs
KVM: arm/arm64: Provide a get_input_level for the arch timer
KVM: arm/arm64: Avoid work when userspace iqchips are not used
KVM: arm/arm64: Delete outdated forwarded irq documentation
Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 187 ---------------------
arch/arm/include/asm/kvm_host.h | 2 +
arch/arm64/include/asm/kvm_host.h | 2 +
include/kvm/arm_arch_timer.h | 2 +
include/kvm/arm_vgic.h | 13 +-
virt/kvm/arm/arch_timer.c | 111 ++++++------
virt/kvm/arm/arm.c | 61 ++++---
virt/kvm/arm/vgic/vgic-mmio.c | 115 ++++++++++---
virt/kvm/arm/vgic/vgic-v2.c | 29 ++++
virt/kvm/arm/vgic/vgic-v3.c | 29 ++++
virt/kvm/arm/vgic/vgic.c | 41 ++++-
virt/kvm/arm/vgic/vgic.h | 8 +
12 files changed, 313 insertions(+), 287 deletions(-)
delete mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
--
2.14.2
^ permalink raw reply
* [PATCH] MAINTAINERS: Add self as extended maintainer for a slew of files
From: Linus Walleij @ 2017-12-20 11:30 UTC (permalink / raw)
To: linux-arm-kernel
Take over sole maintenance of Nomadik, U300 and Ux500. Since all are
Device Tree converted and using standard format drivers this is not
burdensome. Alessandro is not working on this platform any more.
Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ARM SoC folks: please apply this directly where appropriate.
---
MAINTAINERS | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index aa71ab52fd76..ac14e81ed38b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1634,14 +1634,18 @@ ARM/NEC MOBILEPRO 900/c MACHINE SUPPORT
M: Michael Petchkovsky <mkpetch@internode.on.net>
S: Maintained
-ARM/NOMADIK ARCHITECTURE
-M: Alessandro Rubini <rubini@unipv.it>
+ARM/NOMADIK/U300/Ux500 ARCHITECTURE
M: Linus Walleij <linus.walleij@linaro.org>
L: linux-arm-kernel at lists.infradead.org (moderated for non-subscribers)
S: Maintained
F: arch/arm/mach-nomadik/
+F: arch/arm/mach-u300/
+F: arch/arm/mach-ux500/
+F: arch/arm/boot/dts/ste-*
F: drivers/pinctrl/nomadik/
F: drivers/i2c/busses/i2c-nomadik.c
+F: Documentation/devicetree/bindings/arm/ste-*
+F: Documentation/devicetree/bindings/arm/ux500/
T: git git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik.git
ARM/NUVOTON W90X900 ARM ARCHITECTURE
--
2.14.3
^ permalink raw reply related
* next/master boot: 270 boots: 35 failed, 213 passed with 20 offline, 2 untried/unknown (next-20171207)
From: Marek Szyprowski @ 2017-12-20 11:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171219200523.GE7997@codeaurora.org>
Hi Stephen,
On 2017-12-19 21:05, Stephen Boyd wrote:
> On 12/11, Marek Szyprowski wrote:
>> On 2017-12-08 17:59, Stephen Boyd wrote:
>>> On 12/08, Marek Szyprowski wrote:
>>>> On 2017-12-08 13:33, Krzysztof Kozlowski wrote:
>>>>> On Fri, Dec 8, 2017 at 1:27 PM, Mark Brown <broonie@kernel.org> wrote:
>>>>>> On Fri, Dec 08, 2017 at 12:20:07PM +0000, Mark Brown wrote:
>>>>>>> On Thu, Dec 07, 2017 at 03:54:47PM -0800, kernelci.org bot wrote:
>>>>>>>
>>>>>>> Today's -next failed to boot on peach-pi:
>>>>>>>
>>>>>>>> exynos_defconfig:
>>>>>>>> exynos5800-peach-pi:
>>>>>>>> lab-collabora: new failure (last pass: next-20171205)
>>>>>>> with details at https://kernelci.org/boot/id/5a2a2e7859b5141bc2afa17c/
>>>>>>> (including logs and comparisons with other boots, the last good boot was
>>>>>>> Wednesday). It looks like it hangs somewhere late on in boot, the last
>>>>>>> output on the console is:
>>>>>>>
>>>>>>> [ 4.827139] smsc95xx 3-1.1:1.0 eth0: register 'smsc95xx' at usb-xhci-hcd.3.auto-1.1, smsc95xx USB 2.0 Ethernet, 94:eb:2c:00:03:c0
>>>>>>> [ 5.781037] dma-pl330 3880000.adma: Loaded driver for PL330 DMAC-241330
>>>>>>> [ 5.786247] dma-pl330 3880000.adma: DBUFF-4x8bytes Num_Chans-6 Num_Peri-16 Num_Events-6
>>>>>>> [ 5.819200] dma-pl330 3880000.adma: PM domain MAU will not be powered off
>>>>>>> [ 64.529228] random: crng init done
>>>>>>>
>>>>>>> and there's failures earlier to instantiate the display.
>>>>>> I just noticed that further up the log there's a lockdep splat with a
>>>>>> conflict between the genpd and clock API locking - an ABBA issue with
>>>>>> genpd->mlock and the clock API prepare_lock.
>>>>> +Cc Marek Szyprowski,
>>>>>
>>>>> The lockdep issue and display failures (including regulator warning)
>>>>> were present for some time. They also appear in boot log for
>>>>> next-20171206 (https://storage.kernelci.org/next/master/next-20171206/arm/exynos_defconfig/lab-collabora/boot-exynos5800-peach-pi.html).
>>>>> The difference is that 20171208 hangs on "random: crng init done"
>>>>> which did not appear before at all.
>>> I haven't looked at the lockdep splat yet, but is that happening
>>> because of runtime PM usage by the clk framework?
>> This is a false positive. The deplock doesn't distinguish each
>> domain instance.
>> Only some instances of exynos power domains use clocks (as an old
>> workaround of
>> the lack possibility to integrate proper clock rate/topology
>> restoration after
>> power off/on cycle in the clock provider driver).
>>
>> Those clock controllers, which implements runtime pm, are assigned to power
>> domain, which doesn't touch clocks at all.
>>
>> I still have no idea how to fix the code to make deplock happy.
>>
> Right. Once lockdep complains lockdep turns itself off, so we
> lose the ability to detect other problems. Even if it's a false
> positive, it's a potential problem on some device so it's
> concerning that runtime PM usage from clk framework has created
> this potential problem.
>
> Is it possible to remove the clk operations from the exynos power
> domains? You say it's to deal with the lack of rate/topology
> restoration so maybe it can be changed.
Yes, it can be changed. Those rate/topology restoration should be done
in exynos5420 clk driver, which should also implement runtime PM.
However there is still one issue to be resolved. Current runtime
PM / generic power domains bindings doesn't allow to assign a device
(clock controller in this case) to more than one power domain. To
get it working we would need to have a clock controller object
(subdevice?) for each power domain.
Such approach has been already rejected by You in the initial Exynos4
clock controller runtime PM patches.
Exynos4 case could have been modeled in a different, frankly speaking
a bit more close to real hardware details. Exynos4 ISP clocks
registers (that part which is in fact under power domain) are in
separate address space region, so this in the end has been modeled
as separate clock controller, which was easily assigned to respective
power domain.
Exynos5420/5422 (also partially Exynos5250) is much more problematic,
because registers of all clocks are mixed together and MUXes which
loose state after power domain suspend are in common register
(SRC_TOP3). It cannot be modeled with multiple clock controllers, one
per each power domain.
> That will at least allow
> lockdep to continue working here and detect the "real" deadlock
> here. Otherwise, do we need to revert runtime PM for clk
> framework and back out all the Samsung changes on top of that? If
> we need to do that, we need to do it soon.
I would like to avoid this if possible.
> We'll need to think about how to resolve the cross-subsystem
> locking problem regardless. We definitely want to have genpd be
> able to do CCF things, and CCF to use runtime PM and genpds too.
> It seems that we have a classic AB-BA deadlock potential between
> the clk prepare lock and the genpd domain mutex. Both frameworks
> are holding a mutex across the operations of their providers
> (either clk_ops or genpd power_on/off) so we can't have the CCF
> call genpd things and genpd call CCF things or lockdep will
> complain.
One thing to notice is locking granularity. CCF have a single,
common mutex with non-standard semantic (thread re-entrant), while
genpd has a standard per object mutex.
Changing CCF to use per-clock mutex instead of global prepare lock
will probably solve the possible dead-lock, but it will not make
deplock happy, because deplock doesn't distinguish object instances.
> I was worried about runtime PM usage by CCF causing
> this problem, but I missed that genpd was behind runtime PM so I
> didn't consider the locks in that part of the chain. Ugh.
>
> Maybe we can have runtime PM things done outside of the prepare
> lock in CCF, that way we aren't holding any locks that genpd may
> need to use. That would fix the problem, but would expose us to
> clk tree topology changes happening while we enable runtime PM
> for clks. It would be great if we could drop all framework level
> locks when we call into provider drivers. I'm not sure how to do
> that yet, but that's probably the end goal.
>
> Anyway, this needs some thought to figure out how to redesign the
> CCF locking scheme so this problem doesn't exist.
CCF locking scheme already suffers from the other issues, like long
waiting on common prepare lock and possible AB-BA deadlocks, which
were mentioned in the "clk: Add per-controller locks to fix
deadlocks" thread: https://lkml.org/lkml/2016/8/16/442
I thought the we will be able to continue that work, but sadly there
were other more urgent issue to resolve first.
After some more thoughts about that patchset, it should be quite easy
to switch to per-clock mutexes (instead of per-controller/per-provider).
This would however not solve all the discussed issues.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* [PATCH] drm/rockchip: analogix_dp: Ensure that the bridge is powered before poking it
From: Andrzej Hajda @ 2017-12-20 11:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <17fd8819-2771-7dd2-03c1-ba23326961d8@arm.com>
On 19.12.2017 12:42, Marc Zyngier wrote:
> On 19/12/17 07:55, Andrzej Hajda wrote:
>> On 18.12.2017 12:28, Marc Zyngier wrote:
>>> Stopping the X display manager on a kevin platform results in the
>>> following crash:
>>>
>>> [ 674.833536] Synchronous External Abort: synchronous external abort (0x96000010) at 0xffff00000c970640
>>> [ 674.843886] Internal error: : 96000010 [#1] PREEMPT SMP
>>> [ 674.849744] Modules linked in:
>>> [ 674.849755] CPU: 1 PID: 86 Comm: kworker/1:1 Not tainted 4.15.0-rc3-00057-gff24f8cf492d-dirty #3
>>> [ 674.849760] detected fb_set_par error, error code: -16
>>> [ 674.849761] Hardware name: Google Kevin (DT)
>>> [ 674.849773] Workqueue: events analogix_dp_psr_work
>>> [ 674.849778] pstate: 60000005 (nZCv daif -PAN -UAO)
>>> [ 674.849784] pc : analogix_dp_send_psr_spd+0x8/0x168
>>> [ 674.849788] lr : analogix_dp_enable_psr+0x54/0x60
>>> [ 674.849789] sp : ffff000009b2bd60
>>> [ 674.849790] x29: ffff000009b2bd60 x28: 0000000000000000
>>> [ 674.849794] x27: ffff000009913d20 x26: ffff00000900fbf0
>>> [ 674.849797] x25: ffff8000f1b30000 x24: ffff8000f0c21d98
>>> [ 674.849800] x23: 0000000000000000 x22: ffff8000f7d3aa00
>>> [ 674.849803] x21: ffff8000f7d36980 x20: ffff8000f0c21c18
>>> [ 674.849806] x19: ffff8000f0c21db8 x18: 0000000000000001
>>> [ 674.849809] x17: 0000ffff89f2ed58 x16: ffff000008222908
>>> [ 674.849812] x15: 0000000000000000 x14: 0000000000000400
>>> [ 674.849815] x13: 0000000000000400 x12: 0000000000000000
>>> [ 674.849817] x11: 0000000000001414 x10: 0000000000000a00
>>> [ 674.849820] x9 : ffff000009b2bbb0 x8 : ffff8000f1b30a60
>>> [ 674.849823] x7 : 0000000000080000 x6 : 0000000000000001
>>> [ 674.849826] x5 : 0000000000000010 x4 : 0000000000000007
>>> [ 674.849829] x3 : 0000000000000002 x2 : ffff00000c970640
>>> [ 674.849832] x1 : ffff000009b2bd78 x0 : ffff8000f1624018
>>> [ 674.849836] Process kworker/1:1 (pid: 86, stack limit = 0x0000000083e5f7c3)
>>> [ 674.849838] Call trace:
>>> [ 674.849842] analogix_dp_send_psr_spd+0x8/0x168
>>> [ 674.849844] analogix_dp_psr_work+0x9c/0xa0
>>> [ 674.849849] process_one_work+0x1cc/0x328
>>> [ 674.849852] worker_thread+0x50/0x450
>>> [ 674.849856] kthread+0xf8/0x128
>>> [ 674.849860] ret_from_fork+0x10/0x18
>>> [ 674.849864] Code: b9000001 d65f03c0 f9445802 91190042 (b9400042)
>>>
>>> Further investigation show that this happens because the the workqueue
>>> races with the analogix_dp_bridge_disable() call from the core DRM code,
>>> and end up trying to write to the DP bridge that has already been powered
>>> down. This result is a very black screen, and a hard reset.
>>>
>>> Instead of counting on luck to keep the bridge alive, let's use the
>>> pm_runtime framework and take a reference on the device when we're about
>>> to poke it. That is a fairly big hammer, but one that allows the system
>>> to stay alive across dozens of X start/stop sequences.
>> Wouldn't be better to cancel the work in analogix_dp_bridge_disable, it
>> looks safer.
> Not sure. That would only cancel a single work that would be in flight
> right when we hit disable, but won't prevent the work from being queued
> right after the cancel.
>
> In summary, I think you're trading a race between pm_runtime_put_sync
> and analogix_dp_send_psr_spd for another between cancel_work_sync and
> analogix_dp_send_psr_spd. Also, I seem to remember that the disable can
> occur in its own work queue:
>
> commit_tail -> drm_atomic_helper_commit_modeset_disables ->
> disable_outputs -> drm_bridge_disable -> analogix_dp_bridge_disable
>
> making it racy by nature. But I'm no DRM expert (as you can probably tell).
>
> My approach is to guarantee that analogix_dp_send_psr_spd cannot fault
> due to the IP being powered off, which feels a bit more bullet proof.
I suspect the worker should not be executed during/after disable, at
least its body suggests it.
And if it will be guaranteed, runtime dance in the worker is pointless.
Regards
Andrzej
>
> Please shoot me down if I got it wrong!
>
> Thanks,
>
> M.
^ permalink raw reply
* [PATCH v4] staging: fsl-mc: move bus driver out of staging
From: Greg KH @ 2017-12-20 11:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5A3A4103.6050502@nxp.com>
On Wed, Dec 20, 2017 at 10:52:52AM +0000, Laurentiu Tudor wrote:
>
>
> On 12/20/2017 12:42 PM, Greg KH wrote:
> > On Wed, Dec 20, 2017 at 10:26:49AM +0000, Laurentiu Tudor wrote:
> >> On 12/19/2017 06:10 PM, Greg KH wrote:
> >>>>> But all of these .h files are only used by the code in this specific
> >>>>> directory, no where else.
> >>>>
> >>>> They are also used by our ethernet driver, see:
> >>>> drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
> >>>
> >>> Ick, really? Then they should not be buried in a bus-specific
> >>> location, but rather be in include/linux/SOMEWHERE, right?
> >>
> >> Right. The goal is that in the end, all headers be moved to the already
> >> existing include/linux/fsl/. For now I've left these in staging because
> >> they are not part of the bus "core" infrastructure.
> >
> > Then shouldn't they be in the drivers/staging/fsl-mc/include/ directory
> > now to show this?
>
> Not sure i get your comment. Aren't we talking about the headers in there?
>
> This was your original comment:
>
> > Also, what's up with the .h files in drivers/staging/fsl-bus/include?
> > You didn't touch those with this movement, right? Why?
Ok, yeah, I'm getting confused now. Let's just see what you do with
your next set of patches and we can go from there :)
thanks,
greg k-h
^ permalink raw reply
* [PATCH v5 07/11] thermal: armada: Add support for Armada CP110
From: Gregory CLEMENT @ 2017-12-20 10:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171219135719.9531-8-miquel.raynal@free-electrons.com>
Hi Miquel,
On mar., d?c. 19 2017, Miquel Raynal <miquel.raynal@free-electrons.com> wrote:
> From: Baruch Siach <baruch@tkos.co.il>
>
> The CP110 component is integrated in the Armada 8k and 7k lines of
> processors.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> [<miquel.raynal@free-electrons.com>: renamed the register pointers as
> well as some definitions related to the new register names and
> simplified the init sequence for Armada 380]
> Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Here again, on Armada 7040 DB and Machitobin, the temperature value
looks coherent:
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Gregory
> ---
> drivers/thermal/armada_thermal.c | 33 ++++++++++++++++++++++++++-------
> 1 file changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index c7dcac39cbf9..3785b5248bf5 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -37,7 +37,6 @@
> #define A375_UNIT_CONTROL_MASK 0x7
> #define A375_READOUT_INVERT BIT(15)
> #define A375_HW_RESETn BIT(8)
> -#define A380_HW_RESET BIT(8)
>
> /* Legacy bindings */
> #define LEGACY_CONTROL_MEM_LEN 0x4
> @@ -52,6 +51,10 @@
> #define CONTROL0_TSEN_RESET BIT(1)
> #define CONTROL0_TSEN_ENABLE BIT(2)
>
> +/* EXT_TSEN refers to the external temperature sensors, out of the AP */
> +#define CONTROL1_EXT_TSEN_SW_RESET BIT(7)
> +#define CONTROL1_EXT_TSEN_HW_RESETn BIT(8)
> +
> struct armada_thermal_data;
>
> /* Marvell EBU Thermal Sensor Dev Structure */
> @@ -153,12 +156,11 @@ static void armada380_init_sensor(struct platform_device *pdev,
> {
> u32 reg = readl_relaxed(priv->control1);
>
> - /* Reset hardware once */
> - if (!(reg & A380_HW_RESET)) {
> - reg |= A380_HW_RESET;
> - writel(reg, priv->control1);
> - msleep(10);
> - }
> + /* Disable the HW/SW reset */
> + reg |= CONTROL1_EXT_TSEN_HW_RESETn;
> + reg &= ~CONTROL1_EXT_TSEN_SW_RESET;
> + writel(reg, priv->control1);
> + msleep(10);
> }
>
> static void armada_ap806_init_sensor(struct platform_device *pdev,
> @@ -279,6 +281,19 @@ static const struct armada_thermal_data armada_ap806_data = {
> .needs_control0 = true,
> };
>
> +static const struct armada_thermal_data armada_cp110_data = {
> + .is_valid = armada_is_valid,
> + .init_sensor = armada380_init_sensor,
> + .is_valid_bit = BIT(10),
> + .temp_shift = 0,
> + .temp_mask = 0x3ff,
> + .coef_b = 1172499100ULL,
> + .coef_m = 2000096ULL,
> + .coef_div = 4201,
> + .inverted = true,
> + .needs_control0 = true,
> +};
> +
> static const struct of_device_id armada_thermal_id_table[] = {
> {
> .compatible = "marvell,armadaxp-thermal",
> @@ -301,6 +316,10 @@ static const struct of_device_id armada_thermal_id_table[] = {
> .data = &armada_ap806_data,
> },
> {
> + .compatible = "marvell,armada-cp110-thermal",
> + .data = &armada_cp110_data,
> + },
> + {
> /* sentinel */
> },
> };
> --
> 2.11.0
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH v5 06/11] thermal: armada: Add support for Armada AP806
From: Gregory CLEMENT @ 2017-12-20 10:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171219135719.9531-7-miquel.raynal@free-electrons.com>
Hi Miquel,
On mar., d?c. 19 2017, Miquel Raynal <miquel.raynal@free-electrons.com> wrote:
> From: Baruch Siach <baruch@tkos.co.il>
>
> The AP806 component is integrated in the Armada 8K and 7K lines of
> processors.
>
> The thermal sensor sample field on the status register is a signed
> value. Extend armada_get_temp() and the driver structure to handle
> signed values.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> [<miquel.raynal@free-electrons.com>: Changes when applying over the
> previous patches, including the register names changes, also switched
> the coefficients values to s64 instead of unsigned long to deal with
> negative values and used do_div instead of the traditionnal '/']
> Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
On Armada 7040 DB and Machitobin, the temperature value looks coherent:
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Gregory
> ---
> drivers/thermal/armada_thermal.c | 74 ++++++++++++++++++++++++++++++++--------
> 1 file changed, 59 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index ceebabf45c53..c7dcac39cbf9 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -47,6 +47,11 @@
> #define CONTROL0_OFFSET 0x0
> #define CONTROL1_OFFSET 0x4
>
> +/* TSEN refers to the temperature sensors within the AP */
> +#define CONTROL0_TSEN_START BIT(0)
> +#define CONTROL0_TSEN_RESET BIT(1)
> +#define CONTROL0_TSEN_ENABLE BIT(2)
> +
> struct armada_thermal_data;
>
> /* Marvell EBU Thermal Sensor Dev Structure */
> @@ -66,10 +71,11 @@ struct armada_thermal_data {
> bool (*is_valid)(struct armada_thermal_priv *);
>
> /* Formula coeficients: temp = (b - m * reg) / div */
> - unsigned long coef_b;
> - unsigned long coef_m;
> - unsigned long coef_div;
> + s64 coef_b;
> + s64 coef_m;
> + u32 coef_div;
> bool inverted;
> + bool signed_sample;
>
> /* Register shift and mask to access the sensor temperature */
> unsigned int temp_shift;
> @@ -155,6 +161,18 @@ static void armada380_init_sensor(struct platform_device *pdev,
> }
> }
>
> +static void armada_ap806_init_sensor(struct platform_device *pdev,
> + struct armada_thermal_priv *priv)
> +{
> + u32 reg;
> +
> + reg = readl_relaxed(priv->control0);
> + reg &= ~CONTROL0_TSEN_RESET;
> + reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
> + writel(reg, priv->control0);
> + msleep(10);
> +}
> +
> static bool armada_is_valid(struct armada_thermal_priv *priv)
> {
> u32 reg = readl_relaxed(priv->status);
> @@ -166,8 +184,8 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
> int *temp)
> {
> struct armada_thermal_priv *priv = thermal->devdata;
> - unsigned long reg;
> - unsigned long m, b, div;
> + u32 reg, div;
> + s64 sample, b, m;
>
> /* Valid check */
> if (priv->data->is_valid && !priv->data->is_valid(priv)) {
> @@ -178,6 +196,11 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
>
> reg = readl_relaxed(priv->status);
> reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask;
> + if (priv->data->signed_sample)
> + /* The most significant bit is the sign bit */
> + sample = sign_extend32(reg, fls(priv->data->temp_mask) - 1);
> + else
> + sample = reg;
>
> /* Get formula coeficients */
> b = priv->data->coef_b;
> @@ -185,9 +208,12 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
> div = priv->data->coef_div;
>
> if (priv->data->inverted)
> - *temp = ((m * reg) - b) / div;
> + *temp = (m * sample) - b;
> else
> - *temp = (b - (m * reg)) / div;
> + *temp = b - (m * sample);
> +
> + do_div(*temp, div);
> +
> return 0;
> }
>
> @@ -199,8 +225,8 @@ static const struct armada_thermal_data armadaxp_data = {
> .init_sensor = armadaxp_init_sensor,
> .temp_shift = 10,
> .temp_mask = 0x1ff,
> - .coef_b = 3153000000UL,
> - .coef_m = 10000000UL,
> + .coef_b = 3153000000ULL,
> + .coef_m = 10000000ULL,
> .coef_div = 13825,
> };
>
> @@ -210,8 +236,8 @@ static const struct armada_thermal_data armada370_data = {
> .is_valid_bit = BIT(9),
> .temp_shift = 10,
> .temp_mask = 0x1ff,
> - .coef_b = 3153000000UL,
> - .coef_m = 10000000UL,
> + .coef_b = 3153000000ULL,
> + .coef_m = 10000000ULL,
> .coef_div = 13825,
> };
>
> @@ -221,8 +247,8 @@ static const struct armada_thermal_data armada375_data = {
> .is_valid_bit = BIT(10),
> .temp_shift = 0,
> .temp_mask = 0x1ff,
> - .coef_b = 3171900000UL,
> - .coef_m = 10000000UL,
> + .coef_b = 3171900000ULL,
> + .coef_m = 10000000ULL,
> .coef_div = 13616,
> .needs_control0 = true,
> };
> @@ -233,12 +259,26 @@ static const struct armada_thermal_data armada380_data = {
> .is_valid_bit = BIT(10),
> .temp_shift = 0,
> .temp_mask = 0x3ff,
> - .coef_b = 1172499100UL,
> - .coef_m = 2000096UL,
> + .coef_b = 1172499100ULL,
> + .coef_m = 2000096ULL,
> .coef_div = 4201,
> .inverted = true,
> };
>
> +static const struct armada_thermal_data armada_ap806_data = {
> + .is_valid = armada_is_valid,
> + .init_sensor = armada_ap806_init_sensor,
> + .is_valid_bit = BIT(16),
> + .temp_shift = 0,
> + .temp_mask = 0x3ff,
> + .coef_b = -150000LL,
> + .coef_m = 423ULL,
> + .coef_div = 1,
> + .inverted = true,
> + .signed_sample = true,
> + .needs_control0 = true,
> +};
> +
> static const struct of_device_id armada_thermal_id_table[] = {
> {
> .compatible = "marvell,armadaxp-thermal",
> @@ -257,6 +297,10 @@ static const struct of_device_id armada_thermal_id_table[] = {
> .data = &armada380_data,
> },
> {
> + .compatible = "marvell,armada-ap806-thermal",
> + .data = &armada_ap806_data,
> + },
> + {
> /* sentinel */
> },
> };
> --
> 2.11.0
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH v2 3/8] media: v4l2-async: simplify v4l2_async_subdev structure
From: Niklas Söderlund @ 2017-12-20 10:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <9702fbf0c9dd2f6a657aff0c7fff3ca849d76713.1513682135.git.mchehab@s-opensource.com>
Hi Mauro,
On 2017-12-19 09:18:19 -0200, Mauro Carvalho Chehab wrote:
> The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> match criteria requires just a device name.
>
> So, it doesn't make sense to enclose those into structs,
> as the criteria can go directly into the union.
>
> That makes easier to document it, as we don't need to document
> weird senseless structs.
>
> At drivers, this makes even clearer about the match criteria.
>
> Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Acked-by: Benoit Parrot <bparrot@ti.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
> drivers/media/platform/am437x/am437x-vpfe.c | 6 +++---
> drivers/media/platform/atmel/atmel-isc.c | 2 +-
> drivers/media/platform/atmel/atmel-isi.c | 2 +-
> drivers/media/platform/davinci/vpif_capture.c | 4 ++--
> drivers/media/platform/exynos4-is/media-dev.c | 4 ++--
> drivers/media/platform/pxa_camera.c | 2 +-
> drivers/media/platform/qcom/camss-8x16/camss.c | 2 +-
> drivers/media/platform/rcar-vin/rcar-core.c | 2 +-
For rcar-vin:
Acked-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
> drivers/media/platform/rcar_drif.c | 4 ++--
> drivers/media/platform/soc_camera/soc_camera.c | 2 +-
> drivers/media/platform/stm32/stm32-dcmi.c | 2 +-
> drivers/media/platform/ti-vpe/cal.c | 2 +-
> drivers/media/platform/xilinx/xilinx-vipp.c | 2 +-
> drivers/media/v4l2-core/v4l2-async.c | 16 ++++++++--------
> drivers/media/v4l2-core/v4l2-fwnode.c | 10 +++++-----
> drivers/staging/media/imx/imx-media-dev.c | 4 ++--
> include/media/v4l2-async.h | 8 ++------
> 17 files changed, 35 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
> index 0997c640191d..601ae6487617 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> @@ -2304,8 +2304,8 @@ vpfe_async_bound(struct v4l2_async_notifier *notifier,
> vpfe_dbg(1, vpfe, "vpfe_async_bound\n");
>
> for (i = 0; i < ARRAY_SIZE(vpfe->cfg->asd); i++) {
> - if (vpfe->cfg->asd[i]->match.fwnode.fwnode ==
> - asd[i].match.fwnode.fwnode) {
> + if (vpfe->cfg->asd[i]->match.fwnode ==
> + asd[i].match.fwnode) {
> sdinfo = &vpfe->cfg->sub_devs[i];
> vpfe->sd[i] = subdev;
> vpfe->sd[i]->grp_id = sdinfo->grp_id;
> @@ -2510,7 +2510,7 @@ vpfe_get_pdata(struct platform_device *pdev)
> }
>
> pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
> - pdata->asd[i]->match.fwnode.fwnode = of_fwnode_handle(rem);
> + pdata->asd[i]->match.fwnode = of_fwnode_handle(rem);
> of_node_put(rem);
> }
>
> diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
> index 0c2635647f69..34676409ca08 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -2088,7 +2088,7 @@ static int isc_parse_dt(struct device *dev, struct isc_device *isc)
> subdev_entity->pfe_cfg0 |= ISC_PFE_CFG0_PPOL_LOW;
>
> subdev_entity->asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> - subdev_entity->asd->match.fwnode.fwnode =
> + subdev_entity->asd->match.fwnode =
> of_fwnode_handle(rem);
> list_add_tail(&subdev_entity->list, &isc->subdev_entities);
> }
> diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
> index e900995143a3..9958918e2449 100644
> --- a/drivers/media/platform/atmel/atmel-isi.c
> +++ b/drivers/media/platform/atmel/atmel-isi.c
> @@ -1128,7 +1128,7 @@ static int isi_graph_parse(struct atmel_isi *isi, struct device_node *node)
> /* Remote node to connect */
> isi->entity.node = remote;
> isi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> - isi->entity.asd.match.fwnode.fwnode = of_fwnode_handle(remote);
> + isi->entity.asd.match.fwnode = of_fwnode_handle(remote);
> return 0;
> }
> }
> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> index e45916f69def..e1c273c8b9a6 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -1390,7 +1390,7 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
>
> for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
> struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
> - const struct fwnode_handle *fwnode = _asd->match.fwnode.fwnode;
> + const struct fwnode_handle *fwnode = _asd->match.fwnode;
>
> if (fwnode == subdev->fwnode) {
> vpif_obj.sd[i] = subdev;
> @@ -1595,7 +1595,7 @@ vpif_capture_get_pdata(struct platform_device *pdev)
> }
>
> pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
> - pdata->asd[i]->match.fwnode.fwnode = of_fwnode_handle(rem);
> + pdata->asd[i]->match.fwnode = of_fwnode_handle(rem);
> of_node_put(rem);
> }
>
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index 0ef583cfc424..78b48a1fa26c 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -456,7 +456,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
> }
>
> fmd->sensor[index].asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> - fmd->sensor[index].asd.match.fwnode.fwnode = of_fwnode_handle(rem);
> + fmd->sensor[index].asd.match.fwnode = of_fwnode_handle(rem);
> fmd->async_subdevs[index] = &fmd->sensor[index].asd;
>
> fmd->num_sensors++;
> @@ -1364,7 +1364,7 @@ static int subdev_notifier_bound(struct v4l2_async_notifier *notifier,
>
> /* Find platform data for this sensor subdev */
> for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++)
> - if (fmd->sensor[i].asd.match.fwnode.fwnode ==
> + if (fmd->sensor[i].asd.match.fwnode ==
> of_fwnode_handle(subdev->dev->of_node))
> si = &fmd->sensor[i];
>
> diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
> index 305cf1cac210..f028084f0775 100644
> --- a/drivers/media/platform/pxa_camera.c
> +++ b/drivers/media/platform/pxa_camera.c
> @@ -2335,7 +2335,7 @@ static int pxa_camera_pdata_from_dt(struct device *dev,
> asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> remote = of_graph_get_remote_port(np);
> if (remote) {
> - asd->match.fwnode.fwnode = of_fwnode_handle(remote);
> + asd->match.fwnode = of_fwnode_handle(remote);
> of_node_put(remote);
> } else {
> dev_notice(dev, "no remote for %pOF\n", np);
> diff --git a/drivers/media/platform/qcom/camss-8x16/camss.c b/drivers/media/platform/qcom/camss-8x16/camss.c
> index 390a42c17b66..05f06c98aa64 100644
> --- a/drivers/media/platform/qcom/camss-8x16/camss.c
> +++ b/drivers/media/platform/qcom/camss-8x16/camss.c
> @@ -341,7 +341,7 @@ static int camss_of_parse_ports(struct device *dev,
> }
>
> csd->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> - csd->asd.match.fwnode.fwnode = of_fwnode_handle(remote);
> + csd->asd.match.fwnode = of_fwnode_handle(remote);
> }
>
> return notifier->num_subdevs;
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 108d776f3265..f1fc7978d6d1 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -187,7 +187,7 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
> return -ENODEV;
>
> vin_dbg(vin, "Found digital subdevice %pOF\n",
> - to_of_node(vin->digital->asd.match.fwnode.fwnode));
> + to_of_node(vin->digital->asd.match.fwnode));
>
> vin->notifier.ops = &rvin_digital_notify_ops;
> ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
> diff --git a/drivers/media/platform/rcar_drif.c b/drivers/media/platform/rcar_drif.c
> index 63c94f4028a7..b2e080ef5391 100644
> --- a/drivers/media/platform/rcar_drif.c
> +++ b/drivers/media/platform/rcar_drif.c
> @@ -1107,7 +1107,7 @@ static int rcar_drif_notify_bound(struct v4l2_async_notifier *notifier,
> struct rcar_drif_sdr *sdr =
> container_of(notifier, struct rcar_drif_sdr, notifier);
>
> - if (sdr->ep.asd.match.fwnode.fwnode !=
> + if (sdr->ep.asd.match.fwnode !=
> of_fwnode_handle(subdev->dev->of_node)) {
> rdrif_err(sdr, "subdev %s cannot bind\n", subdev->name);
> return -EINVAL;
> @@ -1235,7 +1235,7 @@ static int rcar_drif_parse_subdevs(struct rcar_drif_sdr *sdr)
> return -EINVAL;
> }
>
> - sdr->ep.asd.match.fwnode.fwnode = fwnode;
> + sdr->ep.asd.match.fwnode = fwnode;
> sdr->ep.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> notifier->num_subdevs++;
>
> diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
> index 916ff68b73d4..d13e2c5fb06f 100644
> --- a/drivers/media/platform/soc_camera/soc_camera.c
> +++ b/drivers/media/platform/soc_camera/soc_camera.c
> @@ -1517,7 +1517,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
> if (!info)
> return -ENOMEM;
>
> - info->sasd.asd.match.fwnode.fwnode = of_fwnode_handle(remote);
> + info->sasd.asd.match.fwnode = of_fwnode_handle(remote);
> info->sasd.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> info->subdev = &info->sasd.asd;
>
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
> index ac4c450a6c7d..9460b3080dca 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -1520,7 +1520,7 @@ static int dcmi_graph_parse(struct stm32_dcmi *dcmi, struct device_node *node)
> /* Remote node to connect */
> dcmi->entity.node = remote;
> dcmi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> - dcmi->entity.asd.match.fwnode.fwnode = of_fwnode_handle(remote);
> + dcmi->entity.asd.match.fwnode = of_fwnode_handle(remote);
> return 0;
> }
> }
> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> index 719ed1d79957..d1febe5baa6d 100644
> --- a/drivers/media/platform/ti-vpe/cal.c
> +++ b/drivers/media/platform/ti-vpe/cal.c
> @@ -1702,7 +1702,7 @@ static int of_cal_create_instance(struct cal_ctx *ctx, int inst)
> goto cleanup_exit;
> }
> asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> - asd->match.fwnode.fwnode = of_fwnode_handle(sensor_node);
> + asd->match.fwnode = of_fwnode_handle(sensor_node);
>
> remote_ep = of_graph_get_remote_endpoint(ep_node);
> if (!remote_ep) {
> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
> index f4c3e48ed2c0..6bb28cd49dae 100644
> --- a/drivers/media/platform/xilinx/xilinx-vipp.c
> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c
> @@ -387,7 +387,7 @@ static int xvip_graph_parse_one(struct xvip_composite_device *xdev,
>
> entity->node = remote;
> entity->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> - entity->asd.match.fwnode.fwnode = of_fwnode_handle(remote);
> + entity->asd.match.fwnode = of_fwnode_handle(remote);
> list_add_tail(&entity->list, &xdev->entities);
> xdev->num_subdevs++;
> }
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index e5acfab470a5..2b08d03b251d 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -68,12 +68,12 @@ static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> static bool match_devname(struct v4l2_subdev *sd,
> struct v4l2_async_subdev *asd)
> {
> - return !strcmp(asd->match.device_name.name, dev_name(sd->dev));
> + return !strcmp(asd->match.device_name, dev_name(sd->dev));
> }
>
> static bool match_fwnode(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> {
> - return sd->fwnode == asd->match.fwnode.fwnode;
> + return sd->fwnode == asd->match.fwnode;
> }
>
> static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> @@ -319,7 +319,7 @@ static bool __v4l2_async_notifier_fwnode_has_async_subdev(
> if (asd->match_type != V4L2_ASYNC_MATCH_FWNODE)
> continue;
>
> - if (asd->match.fwnode.fwnode == fwnode)
> + if (asd->match.fwnode == fwnode)
> return true;
> }
>
> @@ -330,7 +330,7 @@ static bool __v4l2_async_notifier_fwnode_has_async_subdev(
> if (sd->asd->match_type != V4L2_ASYNC_MATCH_FWNODE)
> continue;
>
> - if (sd->asd->match.fwnode.fwnode == fwnode)
> + if (sd->asd->match.fwnode == fwnode)
> return true;
> }
>
> @@ -355,8 +355,8 @@ static bool v4l2_async_notifier_fwnode_has_async_subdev(
> struct v4l2_async_subdev *other_asd = notifier->subdevs[j];
>
> if (other_asd->match_type == V4L2_ASYNC_MATCH_FWNODE &&
> - asd->match.fwnode.fwnode ==
> - other_asd->match.fwnode.fwnode)
> + asd->match.fwnode ==
> + other_asd->match.fwnode)
> return true;
> }
>
> @@ -395,7 +395,7 @@ static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier)
> break;
> case V4L2_ASYNC_MATCH_FWNODE:
> if (v4l2_async_notifier_fwnode_has_async_subdev(
> - notifier, asd->match.fwnode.fwnode, i)) {
> + notifier, asd->match.fwnode, i)) {
> dev_err(dev,
> "fwnode has already been registered or in notifier's subdev list\n");
> ret = -EEXIST;
> @@ -510,7 +510,7 @@ void v4l2_async_notifier_cleanup(struct v4l2_async_notifier *notifier)
>
> switch (asd->match_type) {
> case V4L2_ASYNC_MATCH_FWNODE:
> - fwnode_handle_put(asd->match.fwnode.fwnode);
> + fwnode_handle_put(asd->match.fwnode);
> break;
> default:
> WARN_ON_ONCE(true);
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index fb72c7ac04d4..d630640642ee 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -359,9 +359,9 @@ static int v4l2_async_notifier_fwnode_parse_endpoint(
> return -ENOMEM;
>
> asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> - asd->match.fwnode.fwnode =
> + asd->match.fwnode =
> fwnode_graph_get_remote_port_parent(endpoint);
> - if (!asd->match.fwnode.fwnode) {
> + if (!asd->match.fwnode) {
> dev_warn(dev, "bad remote port parent\n");
> ret = -EINVAL;
> goto out_err;
> @@ -393,7 +393,7 @@ static int v4l2_async_notifier_fwnode_parse_endpoint(
> return 0;
>
> out_err:
> - fwnode_handle_put(asd->match.fwnode.fwnode);
> + fwnode_handle_put(asd->match.fwnode);
> kfree(asd);
>
> return ret == -ENOTCONN ? 0 : ret;
> @@ -566,7 +566,7 @@ static int v4l2_fwnode_reference_parse(
> }
>
> notifier->subdevs[notifier->num_subdevs] = asd;
> - asd->match.fwnode.fwnode = args.fwnode;
> + asd->match.fwnode = args.fwnode;
> asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> notifier->num_subdevs++;
> }
> @@ -853,7 +853,7 @@ static int v4l2_fwnode_reference_parse_int_props(
> }
>
> notifier->subdevs[notifier->num_subdevs] = asd;
> - asd->match.fwnode.fwnode = fwnode;
> + asd->match.fwnode = fwnode;
> asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> notifier->num_subdevs++;
> }
> diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
> index 2800700482d6..f7ed5f506fa9 100644
> --- a/drivers/staging/media/imx/imx-media-dev.c
> +++ b/drivers/staging/media/imx/imx-media-dev.c
> @@ -48,7 +48,7 @@ find_async_subdev(struct imx_media_dev *imxmd,
> asd = &imxasd->asd;
> switch (asd->match_type) {
> case V4L2_ASYNC_MATCH_FWNODE:
> - if (fwnode && asd->match.fwnode.fwnode == fwnode)
> + if (fwnode && asd->match.fwnode == fwnode)
> return asd;
> break;
> case V4L2_ASYNC_MATCH_DEVNAME:
> @@ -104,7 +104,7 @@ int imx_media_add_async_subdev(struct imx_media_dev *imxmd,
>
> if (fwnode) {
> asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> - asd->match.fwnode.fwnode = fwnode;
> + asd->match.fwnode = fwnode;
> } else {
> asd->match_type = V4L2_ASYNC_MATCH_DEVNAME;
> asd->match.device_name.name = devname;
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 6152434cbe82..a010af5134b2 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -58,12 +58,8 @@ enum v4l2_async_match_type {
> struct v4l2_async_subdev {
> enum v4l2_async_match_type match_type;
> union {
> - struct {
> - struct fwnode_handle *fwnode;
> - } fwnode;
> - struct {
> - const char *name;
> - } device_name;
> + struct fwnode_handle *fwnode;
> + const char *device_name;
> struct {
> int adapter_id;
> unsigned short address;
> --
> 2.14.3
>
--
Regards,
Niklas S?derlund
^ permalink raw reply
* [PATCH v4] staging: fsl-mc: move bus driver out of staging
From: Laurentiu Tudor @ 2017-12-20 10:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171220104203.GA29824@kroah.com>
On 12/20/2017 12:42 PM, Greg KH wrote:
> On Wed, Dec 20, 2017 at 10:26:49AM +0000, Laurentiu Tudor wrote:
>> On 12/19/2017 06:10 PM, Greg KH wrote:
>>>>> But all of these .h files are only used by the code in this specific
>>>>> directory, no where else.
>>>>
>>>> They are also used by our ethernet driver, see:
>>>> drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
>>>
>>> Ick, really? Then they should not be buried in a bus-specific
>>> location, but rather be in include/linux/SOMEWHERE, right?
>>
>> Right. The goal is that in the end, all headers be moved to the already
>> existing include/linux/fsl/. For now I've left these in staging because
>> they are not part of the bus "core" infrastructure.
>
> Then shouldn't they be in the drivers/staging/fsl-mc/include/ directory
> now to show this?
Not sure i get your comment. Aren't we talking about the headers in there?
This was your original comment:
> Also, what's up with the .h files in drivers/staging/fsl-bus/include?
> You didn't touch those with this movement, right? Why?
---
Best Regards, Laurentiu
^ permalink raw reply
* [PATCH 2/2] mmc: sunxi: Add runtime_pm support
From: Maxime Ripard @ 2017-12-20 10:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.ac66a754953bedbb7529436b40a7996f4ba3cb95.1513766964.git-series.maxime.ripard@free-electrons.com>
So far, even if our card was not in use, we didn't shut down our main
clock, which meant that it was still output on the MMC bus.
While this obviously means that we could save some power there, it also
created issues when it comes with EMC control since we'll have a perfect
peak at the card clock rate.
Let's implement runtime_pm with autosuspend so that we will shut down the
clock when it's not been in use for quite some time.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
drivers/mmc/host/sunxi-mmc.c | 90 ++++++++++++++++++++++++-------------
1 file changed, 60 insertions(+), 30 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 3ce46ebd3488..c6a0bd0e0476 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -35,6 +35,7 @@
#include <linux/of_gpio.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/regulator/consumer.h>
#include <linux/reset.h>
#include <linux/scatterlist.h>
@@ -1217,29 +1218,11 @@ static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host,
return ret;
}
- ret = clk_prepare_enable(host->clk_mmc);
- if (ret) {
- dev_err(&pdev->dev, "Enable mmc clk err %d\n", ret);
- goto error_disable_clk_ahb;
- }
-
- ret = clk_prepare_enable(host->clk_output);
- if (ret) {
- dev_err(&pdev->dev, "Enable output clk err %d\n", ret);
- goto error_disable_clk_mmc;
- }
-
- ret = clk_prepare_enable(host->clk_sample);
- if (ret) {
- dev_err(&pdev->dev, "Enable sample clk err %d\n", ret);
- goto error_disable_clk_output;
- }
-
if (!IS_ERR(host->reset)) {
ret = reset_control_reset(host->reset);
if (ret) {
dev_err(&pdev->dev, "reset err %d\n", ret);
- goto error_disable_clk_sample;
+ goto error_disable_clk_ahb;
}
}
@@ -1258,12 +1241,6 @@ static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host,
error_assert_reset:
if (!IS_ERR(host->reset))
reset_control_assert(host->reset);
-error_disable_clk_sample:
- clk_disable_unprepare(host->clk_sample);
-error_disable_clk_output:
- clk_disable_unprepare(host->clk_output);
-error_disable_clk_mmc:
- clk_disable_unprepare(host->clk_mmc);
error_disable_clk_ahb:
clk_disable_unprepare(host->clk_ahb);
return ret;
@@ -1280,6 +1257,7 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "mmc alloc host failed\n");
return -ENOMEM;
}
+ platform_set_drvdata(pdev, mmc);
host = mmc_priv(mmc);
host->mmc = mmc;
@@ -1340,12 +1318,16 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
if (ret)
goto error_free_dma;
+ pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
+ pm_runtime_use_autosuspend(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);
+
ret = mmc_add_host(mmc);
if (ret)
goto error_free_dma;
dev_info(&pdev->dev, "base:0x%p irq:%u\n", host->reg_base, host->irq);
- platform_set_drvdata(pdev, mmc);
+
return 0;
error_free_dma:
@@ -1361,27 +1343,75 @@ static int sunxi_mmc_remove(struct platform_device *pdev)
struct sunxi_mmc_host *host = mmc_priv(mmc);
mmc_remove_host(mmc);
+ pm_runtime_force_suspend(&pdev->dev);
disable_irq(host->irq);
sunxi_mmc_reset_host(host);
if (!IS_ERR(host->reset))
reset_control_assert(host->reset);
- clk_disable_unprepare(host->clk_sample);
+ dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma);
+ mmc_free_host(mmc);
+
+ return 0;
+}
+
+static int sunxi_mmc_runtime_resume(struct device *dev)
+{
+ struct mmc_host *mmc = dev_get_drvdata(dev);
+ struct sunxi_mmc_host *host = mmc_priv(mmc);
+ int ret;
+
+ ret = clk_prepare_enable(host->clk_mmc);
+ if (ret) {
+ dev_err(dev, "Enable mmc clk err %d\n", ret);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(host->clk_output);
+ if (ret) {
+ dev_err(dev, "Enable output clk err %d\n", ret);
+ goto error_disable_clk_mmc;
+ }
+
+ ret = clk_prepare_enable(host->clk_sample);
+ if (ret) {
+ dev_err(dev, "Enable sample clk err %d\n", ret);
+ goto error_disable_clk_output;
+ }
+
+ return 0;
+
+error_disable_clk_output:
clk_disable_unprepare(host->clk_output);
+error_disable_clk_mmc:
clk_disable_unprepare(host->clk_mmc);
- clk_disable_unprepare(host->clk_ahb);
+ return ret;
+}
- dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma);
- mmc_free_host(mmc);
+static int sunxi_mmc_runtime_suspend(struct device *dev)
+{
+ struct mmc_host *mmc = dev_get_drvdata(dev);
+ struct sunxi_mmc_host *host = mmc_priv(mmc);
+
+ clk_disable_unprepare(host->clk_sample);
+ clk_disable_unprepare(host->clk_output);
+ clk_disable_unprepare(host->clk_mmc);
return 0;
}
+static const struct dev_pm_ops sunxi_mmc_pm_ops = {
+ SET_RUNTIME_PM_OPS(sunxi_mmc_runtime_suspend,
+ sunxi_mmc_runtime_resume,
+ NULL)
+};
+
static struct platform_driver sunxi_mmc_driver = {
.driver = {
.name = "sunxi-mmc",
.of_match_table = of_match_ptr(sunxi_mmc_of_match),
+ .pm = &sunxi_mmc_pm_ops,
},
.probe = sunxi_mmc_probe,
.remove = sunxi_mmc_remove,
--
git-series 0.9.1
^ permalink raw reply related
* [PATCH 1/2] mmc: sunxi: Reorder the headers
From: Maxime Ripard @ 2017-12-20 10:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.ac66a754953bedbb7529436b40a7996f4ba3cb95.1513766964.git-series.maxime.ripard@free-electrons.com>
Our headers sort algorithm has had pretty chaotic results. Let's fix that.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
drivers/mmc/host/sunxi-mmc.c | 43 +++++++++++++++++--------------------
1 file changed, 20 insertions(+), 23 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index cc98355dbdb9..3ce46ebd3488 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -13,36 +13,33 @@
* the License, or (at your option) any later version.
*/
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/io.h>
-#include <linux/device.h>
-#include <linux/interrupt.h>
-#include <linux/delay.h>
-#include <linux/err.h>
-
#include <linux/clk.h>
#include <linux/clk/sunxi-ng.h>
-#include <linux/gpio.h>
-#include <linux/platform_device.h>
-#include <linux/spinlock.h>
-#include <linux/scatterlist.h>
+#include <linux/delay.h>
+#include <linux/device.h>
#include <linux/dma-mapping.h>
-#include <linux/slab.h>
-#include <linux/reset.h>
-#include <linux/regulator/consumer.h>
-
-#include <linux/of_address.h>
-#include <linux/of_gpio.h>
-#include <linux/of_platform.h>
-
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mmc/card.h>
+#include <linux/mmc/core.h>
+#include <linux/mmc/mmc.h>
#include <linux/mmc/host.h>
#include <linux/mmc/sd.h>
#include <linux/mmc/sdio.h>
-#include <linux/mmc/mmc.h>
-#include <linux/mmc/core.h>
-#include <linux/mmc/card.h>
#include <linux/mmc/slot-gpio.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
/* register offset definitions */
#define SDXC_REG_GCTRL (0x00) /* SMC Global Control Register */
--
git-series 0.9.1
^ permalink raw reply related
* [PATCH 0/2] mmc: sunxi: Add runtime PM support
From: Maxime Ripard @ 2017-12-20 10:50 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Since it's introduction, our MMC controller has had its external clocks
running all the time.
While that was working great, the power usage and most importantly the EMI
that it generated was pretty bad.
Let's implement some runtime_pm hooks with an autosuspend to cut the
external clock when the MMC is not active.
I didn't get all the way to also shutdown the bus clocks since on some
older SoCs (before the A31), it also means that the controler will be reset
which has some quite important implications obviously. And I couldn't make
it work reliably at the moment. Anyway, it can always be implemented as a
second step if needed.
(Quite simple) benchmarks have shown no noticeable regressions since the
controller state is maintained.
Let me know what you think,
Maxime
Maxime Ripard (2):
mmc: sunxi: Reorder the headers
mmc: sunxi: Add runtime_pm support
drivers/mmc/host/sunxi-mmc.c | 133 +++++++++++++++++++++---------------
1 file changed, 80 insertions(+), 53 deletions(-)
base-commit: 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323
--
git-series 0.9.1
^ permalink raw reply
* [PATCH v4] staging: fsl-mc: move bus driver out of staging
From: Greg KH @ 2017-12-20 10:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5A3A3AE6.6080905@nxp.com>
On Wed, Dec 20, 2017 at 10:26:49AM +0000, Laurentiu Tudor wrote:
> On 12/19/2017 06:10 PM, Greg KH wrote:
> >>> But all of these .h files are only used by the code in this specific
> >>> directory, no where else.
> >>
> >> They are also used by our ethernet driver, see:
> >> drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
> >
> > Ick, really? Then they should not be buried in a bus-specific
> > location, but rather be in include/linux/SOMEWHERE, right?
>
> Right. The goal is that in the end, all headers be moved to the already
> existing include/linux/fsl/. For now I've left these in staging because
> they are not part of the bus "core" infrastructure.
Then shouldn't they be in the drivers/staging/fsl-mc/include/ directory
now to show this?
thanks,
greg k-h
^ permalink raw reply
* Linux Kernel handling AXI DECERR/SLVERR
From: Bharat Kumar Gogada @ 2017-12-20 10:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171219113848.6dpb3fosbznguala@lakrids.cambridge.arm.com>
On Tue, Dec 19, 2017 at 11:28:49AM +0000, Bharat Kumar Gogada wrote:
> In our case the peripheral returns SLVERR first time and we see the following print but kernel do not hang.
> [ 231.484186] Unhandled fault: synchronous external abort
> (0x92000210) at 0x0000007f9241f880 Bus error
>
> And from simulation we know that subsequent access to peripheral
> returns OKAY response, however we see subsequent access fail with same
> above bus error when we boot Linux.
>
> Is there a way to handle these synchronous abort gracefully in Linux
> or are these fatal ?
We don't currently have any mechanism to handle these, though it might be possible for synchronous abort.
Since currently there is no mechanism to handle, if once synchronous abort is received from a peripheral,
consecutive access will show up same error even peripheral responds with OKAY ?
What are the possible ways for handling these ?
Do you know why the device is returning SLVERR in this case?
There is an error being detected by the device.
Bharat
^ permalink raw reply
* [PATCH V1 1/1] iommu: Make sure device's ID array elements are unique
From: Tomasz Nowicki @ 2017-12-20 10:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171219093726.432e16e7@t450s.home>
On 19.12.2017 17:37, Alex Williamson wrote:
> On Tue, 19 Dec 2017 16:20:21 +0100
> Tomasz Nowicki <tomasz.nowicki@caviumnetworks.com> wrote:
>
>> While iterating over DMA aliases for a PCI device, for some rare cases
>> (i.e. PCIe-to-PCI/X bridges) we may get exactly the same ID as initial child
>> device. In turn, the same ID may get registered for a device multiple times.
>> Eventually IOMMU driver may try to configure the same ID within domain
>> multiple times too which for some IOMMU drivers is illegal and causes kernel
>> panic.
>>
>> Rule out ID duplication prior to device ID array registration.
>>
>> CC: stable at vger.kernel.org # v4.14+
>
> You've identified a release, is there a specific commit this fixes?
Yes, it was triggered by converting drm_pci_init() to
pci_register_driver() in ast_drv.c
Fixes: 10631d724def ("drm/pci: Deprecate drm_pci_init/exit completely
")
>
>> Signed-off-by: Tomasz Nowicki <tomasz.nowicki@caviumnetworks.com>
>> ---
>> drivers/iommu/iommu.c | 28 ++++++++++++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 3de5c0b..9b2c138 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -1945,6 +1945,31 @@ void iommu_fwspec_free(struct device *dev)
>> }
>> EXPORT_SYMBOL_GPL(iommu_fwspec_free);
>>
>> +static void iommu_fwspec_remove_ids_dup(struct device *dev, u32 *ids,
>> + int *num_ids)
>> +{
>> + struct iommu_fwspec *fwspec = dev->iommu_fwspec;
>> + int i, j, k, valid_ids = *num_ids;
>> +
>> + for (i = 0; i < valid_ids; i++) {
>> + for (j = 0; j < fwspec->num_ids; j++) {
>> + if (ids[i] != fwspec->ids[j])
>> + continue;
>> +
>> + dev_info(dev, "found 0x%x ID duplication, skipped\n",
>> + ids[i]);
>> +
>> + for (k = i + 1; k < valid_ids; k++)
>> + ids[k - 1] = ids[k];
>
> Use memmove()?
Right.
>
>> +
>> + valid_ids--;
>> + break;
>
> At this point ids[i] is not the ids[i] that we tested for dupes, it's
> what was ids[i + 1], but we're going to i++ on the next iteration and
> we therefore never test that entry.
Good point.
Now the fundamental question is where we should put the patch, here or
in SMMUv3 driver as per Robin suggestion.
Thanks,
Tomasz
^ permalink raw reply
* [PATCH V1 0/1] Fix kernel panic caused by device ID duplication presented to the IOMMU
From: Tomasz Nowicki @ 2017-12-20 10:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7f7fca76-88f8-6ee7-c402-fe4300c62253@arm.com>
Hi Robin,
On 19.12.2017 17:34, Robin Murphy wrote:
> Hi Tomasz,
>
> On 19/12/17 15:13, Tomasz Nowicki wrote:
>> Here is my lspci output of ThunderX2 for which I am observing kernel
>> panic coming from
>> SMMUv3 driver -> arm_smmu_write_strtab_ent() -> BUG_ON(ste_live):
>>
>> # lspci -vt
>> -[0000:00]-+-00.0-[01-1f]--+ [...]
>> ??????????????????????????? + [...]
>> ??????????????????????????? \-00.0-[1e-1f]----00.0-[1f]----00.0
>> ASPEED Technology, Inc. ASPEED Graphics Family
>>
>> ASP device -> 1f:00.0 VGA compatible controller: ASPEED Technology,
>> Inc. ASPEED Graphics Family
>> PCI-Express to PCI/PCI-X Bridge -> 1e:00.0 PCI bridge: ASPEED
>> Technology, Inc. AST1150 PCI-to-PCI Bridge
>> While setting up ASP device SID in IORT dirver:
>> iort_iommu_configure() -> pci_for_each_dma_alias()
>> we need to walk up and iterate over each device which alias
>> transaction from
>> downstream devices.
>>
>> AST device (1f:00.0) gets BDF=0x1f00 and corresponding SID=0x1f00 from
>> IORT.
>> Bridge (1e:00.0) is the first alias. Following PCI Express to
>> PCI/PCI-X Bridge
>> spec: PCIe-to-PCI/X bridges alias transactions from downstream devices
>> using
>> the subordinate bus number. For bridge (1e:00.0), the subordinate is
>> equal
>> to 0x1f. This gives BDF=0x1f00 and SID=1f00 which is the same as
>> downstream
>> device. So it is possible to have two identical SIDs. The question is
>> what we
>> should do about such case. Presented patch prevents from registering
>> the same
>> ID so that SMMUv3 is not complaining later on.
>
> Ooh, subtle :( There is logic in arm_smmu_attach_device() to tolerate
> grouped devices aliasing to the same ID, but I guess I overlooked the
> distinction of a device sharing an alias ID with itself. I'm not sure
> I really like trying to work around this in generic code, since
> fwspec->ids is essentially opaque data in a driver-specific format - in
> theory a driver is free to encode a single logical ID into multiple
> fwspec elements (I think I did that in an early draft of SMMUv2 SMR
> support), at which point this approach might corrupt things massively.
I don't have strong favourite here, the fix in SMMUv3 driver would work
too. Initially we can fix things for SMMUv3 only and if ever face the
same issue for other IOMMU driver, then we can move it to generic layer.
>
> Does the (untested) diff below suffice?
>
> Robin.
>
> ----->8-----diff --git a/drivers/iommu/arm-smmu-v3.c
> b/drivers/iommu/arm-smmu-v3.c
> index f122071688fd..d8a730d83401 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -1731,7 +1731,7 @@ static __le64 *arm_smmu_get_step_for_sid(struct
> arm_smmu_device *smmu, u32 sid)
>
> ?static void arm_smmu_install_ste_for_dev(struct iommu_fwspec *fwspec)
> ?{
> -??? int i;
> +??? int i, j;
> ???? struct arm_smmu_master_data *master = fwspec->iommu_priv;
> ???? struct arm_smmu_device *smmu = master->smmu;
>
> @@ -1739,6 +1739,13 @@ static void arm_smmu_install_ste_for_dev(struct
> iommu_fwspec *fwspec)
> ???????? u32 sid = fwspec->ids[i];
> ???????? __le64 *step = arm_smmu_get_step_for_sid(smmu, sid);
>
> +??????? /* Bridged PCI devices may end up with duplicated IDs */
> +??????? for (j = 0; j < i; j++)
> +??????????? if (fwspec->ids[j] == sid)
> +??????????????? break;
> +??????? if (j < i)
> +??????????? continue;
> +
> ???????? arm_smmu_write_strtab_ent(smmu, sid, step, &master->ste);
> ???? }
> ?}
Yes, worked for me.
Thanks,
Tomasz
^ permalink raw reply
* [PATCH v5 06/11] thermal: armada: Add support for Armada AP806
From: Gregory CLEMENT @ 2017-12-20 10:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171219135719.9531-7-miquel.raynal@free-electrons.com>
Hi Miquel,
On mar., d?c. 19 2017, Miquel Raynal <miquel.raynal@free-electrons.com> wrote:
> From: Baruch Siach <baruch@tkos.co.il>
>
> The AP806 component is integrated in the Armada 8K and 7K lines of
> processors.
>
> The thermal sensor sample field on the status register is a signed
> value. Extend armada_get_temp() and the driver structure to handle
> signed values.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> [<miquel.raynal@free-electrons.com>: Changes when applying over the
> previous patches, including the register names changes, also switched
> the coefficients values to s64 instead of unsigned long to deal with
> negative values and used do_div instead of the traditionnal '/']
> Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> drivers/thermal/armada_thermal.c | 74 ++++++++++++++++++++++++++++++++--------
> 1 file changed, 59 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index ceebabf45c53..c7dcac39cbf9 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -47,6 +47,11 @@
> #define CONTROL0_OFFSET 0x0
> #define CONTROL1_OFFSET 0x4
>
> +/* TSEN refers to the temperature sensors within the AP */
> +#define CONTROL0_TSEN_START BIT(0)
> +#define CONTROL0_TSEN_RESET BIT(1)
> +#define CONTROL0_TSEN_ENABLE BIT(2)
> +
> struct armada_thermal_data;
>
> /* Marvell EBU Thermal Sensor Dev Structure */
> @@ -66,10 +71,11 @@ struct armada_thermal_data {
> bool (*is_valid)(struct armada_thermal_priv *);
>
> /* Formula coeficients: temp = (b - m * reg) / div */
> - unsigned long coef_b;
> - unsigned long coef_m;
> - unsigned long coef_div;
> + s64 coef_b;
> + s64 coef_m;
> + u32 coef_div;
> bool inverted;
> + bool signed_sample;
>
> /* Register shift and mask to access the sensor temperature */
> unsigned int temp_shift;
> @@ -155,6 +161,18 @@ static void armada380_init_sensor(struct platform_device *pdev,
> }
> }
>
> +static void armada_ap806_init_sensor(struct platform_device *pdev,
> + struct armada_thermal_priv *priv)
> +{
> + u32 reg;
> +
> + reg = readl_relaxed(priv->control0);
> + reg &= ~CONTROL0_TSEN_RESET;
> + reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
> + writel(reg, priv->control0);
> + msleep(10);
> +}
> +
> static bool armada_is_valid(struct armada_thermal_priv *priv)
> {
> u32 reg = readl_relaxed(priv->status);
> @@ -166,8 +184,8 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
> int *temp)
> {
> struct armada_thermal_priv *priv = thermal->devdata;
> - unsigned long reg;
> - unsigned long m, b, div;
> + u32 reg, div;
> + s64 sample, b, m;
>
> /* Valid check */
> if (priv->data->is_valid && !priv->data->is_valid(priv)) {
> @@ -178,6 +196,11 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
>
> reg = readl_relaxed(priv->status);
> reg = (reg >> priv->data->temp_shift) & priv->data->temp_mask;
> + if (priv->data->signed_sample)
> + /* The most significant bit is the sign bit */
> + sample = sign_extend32(reg, fls(priv->data->temp_mask) - 1);
> + else
> + sample = reg;
>
> /* Get formula coeficients */
> b = priv->data->coef_b;
> @@ -185,9 +208,12 @@ static int armada_get_temp(struct thermal_zone_device *thermal,
> div = priv->data->coef_div;
>
> if (priv->data->inverted)
> - *temp = ((m * reg) - b) / div;
> + *temp = (m * sample) - b;
> else
> - *temp = (b - (m * reg)) / div;
> + *temp = b - (m * sample);
> +
> + do_div(*temp, div);
I wanted to test in on ARMv7 and this line failed to compile:
In file included from arch/arm/include/asm/div64.h:127:0,
from include/linux/kernel.h:173,
from include/linux/list.h:9,
from include/linux/kobject.h:20,
from include/linux/device.h:17,
from drivers/thermal/armada_thermal.c:16:
drivers/thermal/armada_thermal.c: In function ?armada_get_temp?:
include/asm-generic/div64.h:208:28: warning: comparison of distinct pointer types lacks a cast
(void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
^
drivers/thermal/armada_thermal.c:247:2: note: in expansion of macro ?do_div?
do_div(*temp, div);
^~~~~~
In file included from include/linux/ioport.h:13:0,
from include/linux/device.h:16,
from drivers/thermal/armada_thermal.c:16:
include/asm-generic/div64.h:221:25: warning: right shift count >= width of type [-Wshift-count-overflow]
} else if (likely(((n) >> 32) == 0)) { \
^
include/linux/compiler.h:175:40: note: in definition of macro ?likely?
# define likely(x) __builtin_expect(!!(x), 1)
^
drivers/thermal/armada_thermal.c:247:2: note: in expansion of macro ?do_div?
do_div(*temp, div);
^~~~~~
In file included from arch/arm/include/asm/div64.h:127:0,
from include/linux/kernel.h:173,
from include/linux/list.h:9,
from include/linux/kobject.h:20,
from include/linux/device.h:17,
from drivers/thermal/armada_thermal.c:16:
include/asm-generic/div64.h:225:22: error: passing argument 1 of ?__div64_32? from incompatible pointer type [-Werror=incompatible-pointer-types]
__rem = __div64_32(&(n), __base); \
^
drivers/thermal/armada_thermal.c:247:2: note: in expansion of macro ?do_div?
do_div(*temp, div);
^~~~~~
In file included from include/linux/kernel.h:173:0,
from include/linux/list.h:9,
from include/linux/kobject.h:20,
from include/linux/device.h:17,
from drivers/thermal/armada_thermal.c:16:
arch/arm/include/asm/div64.h:33:24: note: expected ?uint64_t * {aka long long unsigned int *}? but argument is of type ?int *?
static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
Gregory
> +
> return 0;
> }
>
> @@ -199,8 +225,8 @@ static const struct armada_thermal_data armadaxp_data = {
> .init_sensor = armadaxp_init_sensor,
> .temp_shift = 10,
> .temp_mask = 0x1ff,
> - .coef_b = 3153000000UL,
> - .coef_m = 10000000UL,
> + .coef_b = 3153000000ULL,
> + .coef_m = 10000000ULL,
> .coef_div = 13825,
> };
>
> @@ -210,8 +236,8 @@ static const struct armada_thermal_data armada370_data = {
> .is_valid_bit = BIT(9),
> .temp_shift = 10,
> .temp_mask = 0x1ff,
> - .coef_b = 3153000000UL,
> - .coef_m = 10000000UL,
> + .coef_b = 3153000000ULL,
> + .coef_m = 10000000ULL,
> .coef_div = 13825,
> };
>
> @@ -221,8 +247,8 @@ static const struct armada_thermal_data armada375_data = {
> .is_valid_bit = BIT(10),
> .temp_shift = 0,
> .temp_mask = 0x1ff,
> - .coef_b = 3171900000UL,
> - .coef_m = 10000000UL,
> + .coef_b = 3171900000ULL,
> + .coef_m = 10000000ULL,
> .coef_div = 13616,
> .needs_control0 = true,
> };
> @@ -233,12 +259,26 @@ static const struct armada_thermal_data armada380_data = {
> .is_valid_bit = BIT(10),
> .temp_shift = 0,
> .temp_mask = 0x3ff,
> - .coef_b = 1172499100UL,
> - .coef_m = 2000096UL,
> + .coef_b = 1172499100ULL,
> + .coef_m = 2000096ULL,
> .coef_div = 4201,
> .inverted = true,
> };
>
> +static const struct armada_thermal_data armada_ap806_data = {
> + .is_valid = armada_is_valid,
> + .init_sensor = armada_ap806_init_sensor,
> + .is_valid_bit = BIT(16),
> + .temp_shift = 0,
> + .temp_mask = 0x3ff,
> + .coef_b = -150000LL,
> + .coef_m = 423ULL,
> + .coef_div = 1,
> + .inverted = true,
> + .signed_sample = true,
> + .needs_control0 = true,
> +};
> +
> static const struct of_device_id armada_thermal_id_table[] = {
> {
> .compatible = "marvell,armadaxp-thermal",
> @@ -257,6 +297,10 @@ static const struct of_device_id armada_thermal_id_table[] = {
> .data = &armada380_data,
> },
> {
> + .compatible = "marvell,armada-ap806-thermal",
> + .data = &armada_ap806_data,
> + },
> + {
> /* sentinel */
> },
> };
> --
> 2.11.0
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH v4] staging: fsl-mc: move bus driver out of staging
From: Laurentiu Tudor @ 2017-12-20 10:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171219161045.GA18839@kroah.com>
On 12/19/2017 06:10 PM, Greg KH wrote:
> On Tue, Dec 19, 2017 at 03:39:44PM +0000, Laurentiu Tudor wrote:
>> On 12/19/2017 05:29 PM, Greg KH wrote:
>>> On Tue, Dec 19, 2017 at 03:21:19PM +0000, Laurentiu Tudor wrote:
>>>>
>>>>
>>>> On 12/19/2017 04:48 PM, Greg KH wrote:
>>>>> On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tudor at nxp.com wrote:
>>>>>> From: Stuart Yoder <stuart.yoder@nxp.com>
>>>>>>
>>>>>> Move the source files out of staging into their final locations:
>>>>>> -include files in drivers/staging/fsl-mc/include go to include/linux/fsl
>>>>>> -irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>>>>>> -source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
>>>>>> -README.txt, providing and overview of DPAA goes to
>>>>>> Documentation/dpaa2/overview.txt
>>>>>>
>>>>>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
>>>>>> Update dpaa2_eth and dpio staging drivers.
>>>>>>
>>>>>> Signed-off-by: Stuart Yoder <stuyoder@gmail.com>
>>>>>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>>>>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
>>>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>>>> Cc: Jason Cooper <jason@lakedaemon.net>
>>>>>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>>>>>> ---
>>>>>> Notes:
>>>>>> -v4:
>>>>>> - regenerated patch with renames detection disabled (Andrew Lunn)
>>>>>> -v3:
>>>>>> - rebased
>>>>>
>>>>> Ok, meta-comments on the structure of the code.
>>>>>
>>>>> You have 8 .h files that are "private" to your bus logic. That's 7 too
>>>>> many, some of them have a bigger license header than actual content :)
>>>>>
>>>>> Please consolidate into 1.
>>>>>
>>>>> Also, the headers should be moved to SPDX format to get rid of the
>>>>> boilerplate. I _think_ it's BSD/GPL, right? Hard to tell :(
>>>>
>>>> It's 3-clause BSD and GPLv2. Will make it clear when moving to SPDX.
>>>
>>> Thanks.
>>>
>>>>> Your "public" .h file does not need to go into a subdirectory, just name
>>>>> it fsl-mc.h and put it in include/linux/.
>>>>
>>>> There's already a "fsl" subdirectory in include/linux/ so it seemed to
>>>> make sense to use it.
>>>
>>> Ah, missed that. Ok, nevermind :)`
>>>
>>>>> One comment on the fields in your .h file, all of the user/kernel
>>>>> crossing boundry structures need to use the "__" variant of types, like
>>>>> "__u8" and the like. You mix and match them for some reason, you need
>>>>> to be consistent.
>>>>>
>>>>> Also, what's up with the .h files in drivers/staging/fsl-bus/include?
>>>>> You didn't touch those with this movement, right? Why?
>>>>
>>>> Those are not part of the bus "core". Some of them are part of the DPBP
>>>> and DPCON device types APIs and are used by drivers probing on this bus
>>>> and the rest are part of the DPIO driver which is also used by other
>>>> drivers. Since these devices (DPBP, DPCON, DPIO) are interfaces used by
>>>> all the other drivers it made sense to group them together with the bus.
>>>
>>> But all of these .h files are only used by the code in this specific
>>> directory, no where else.
>>
>> They are also used by our ethernet driver, see:
>> drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
>
> Ick, really? Then they should not be buried in a bus-specific
> location, but rather be in include/linux/SOMEWHERE, right?
Right. The goal is that in the end, all headers be moved to the already
existing include/linux/fsl/. For now I've left these in staging because
they are not part of the bus "core" infrastructure.
---
Best Regards, Laurentiu
^ permalink raw reply
* [PATCH 00/13] replace print_symbol() with printk()-s
From: Sergey Senozhatsky @ 2017-12-20 10:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211125025.2270-1-sergey.senozhatsky@gmail.com>
On (12/11/17 21:50), Sergey Senozhatsky wrote:
>
> A rather automatic replacement of print_symbol()
> with direct printk() calls. print_symbol() uses extra stack
> buffer (KSYM_SYMBOL_LEN 128 bytes) and, basically, should
> be identical to printk(%pS).
>
> I can't test all of the patches, because I don't
> own any of those exotic arch-s. Sorry for the inconvenience.
>
> Sergey Senozhatsky (13):
> arm: do not use print_symbol()
> arm64: do not use print_symbol()
> c6x: do not use print_symbol()
> ia64: do not use print_symbol()
> mn10300: do not use print_symbol()
> sh: do not use print_symbol()
> unicore32: do not use print_symbol()
> x86: do not use print_symbol()
> drivers: do not use print_symbol()
> sysfs: do not use print_symbol()
> irq debug: do not use print_symbol()
> lib: do not use print_symbol()
> arc: do not use __print_symbol()
Hello,
can we please have more reviews/acks/etc?
-ss
^ permalink raw reply
* [GIT PULL] ARM: at91: Fixes for 4.15
From: Alexandre Belloni @ 2017-12-20 10:16 UTC (permalink / raw)
To: linux-arm-kernel
Arnd, Olof,
A single fix for 4.15. The driver part of it will land in 4.15 through
the hwmon tree.
The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:
Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git tags/at91-ab-4.15-dt-fixes
for you to fetch changes up to bc53e3aa88e8240823c1c440e6bab3c3a5ba5f59:
ARM: dts: at91: disable the nxp,se97b SMBUS timeout on the TSE-850 (2017-12-04 20:30:38 +0100)
----------------------------------------------------------------
Fixes for 4.15:
- tse850-3: fix an i2c timeout issue
----------------------------------------------------------------
Peter Rosin (1):
ARM: dts: at91: disable the nxp,se97b SMBUS timeout on the TSE-850
arch/arm/boot/dts/at91-tse850-3.dts | 1 +
1 file changed, 1 insertion(+)
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ 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