* [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space
From: Marc Zyngier @ 2016-09-19 14:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474283695-212421-3-git-send-email-agraf@suse.de>
On 19/09/16 12:14, Alexander Graf wrote:
> We have 2 modes for dealing with interrupts in the ARM world. We can either
> handle them all using hardware acceleration through the vgic or we can emulate
> a gic in user space and only drive CPU IRQ pins from there.
>
> Unfortunately, when driving IRQs from user space, we never tell user space
> about timer events that may result in interrupt line state changes, so we
> lose out on timer events if we run with user space gic emulation.
>
> This patch fixes that by routing vtimer expiration events to user space.
> With this patch I can successfully run edk2 and Linux with user space gic
> emulation.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
>
> ---
>
> v1 -> v2:
>
> - Add back curly brace that got lost
>
> v2 -> v3:
>
> - Split into patch set
>
> v3 -> v4:
>
> - Improve documentation
> ---
> Documentation/virtual/kvm/api.txt | 30 ++++++++-
> arch/arm/include/asm/kvm_host.h | 3 +
> arch/arm/kvm/arm.c | 22 ++++---
> arch/arm64/include/asm/kvm_host.h | 3 +
> include/uapi/linux/kvm.h | 14 +++++
> virt/kvm/arm/arch_timer.c | 125 +++++++++++++++++++++++++++-----------
> 6 files changed, 155 insertions(+), 42 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 23937e0..1c0bd86 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -3202,9 +3202,14 @@ struct kvm_run {
> /* in */
> __u8 request_interrupt_window;
>
> -Request that KVM_RUN return when it becomes possible to inject external
> +[x86] Request that KVM_RUN return when it becomes possible to inject external
> interrupts into the guest. Useful in conjunction with KVM_INTERRUPT.
>
> +[arm*] Bits set to 1 in here mask IRQ lines that would otherwise potentially
> +trigger forever. These lines are available:
> +
> + KVM_IRQWINDOW_VTIMER - Masks hw virtual timer irq while in guest
> +
> __u8 padding1[7];
>
> /* out */
> @@ -3519,6 +3524,18 @@ Hyper-V SynIC state change. Notification is used to remap SynIC
> event/message pages and to enable/disable SynIC messages/events processing
> in userspace.
>
> + /* KVM_EXIT_ARM_TIMER */
> + struct {
> + __u8 timesource;
> + } arm_timer;
> +
> +Indicates that a timer triggered that user space needs to handle and
> +potentially mask with vcpu->run->request_interrupt_window to allow the
> +guest to proceed. This only happens for timers that got enabled through
> +KVM_CAP_ARM_TIMER. The following time sources are available:
> +
> + KVM_ARM_TIMER_VTIMER - virtual cpu timer
> +
> /* Fix the size of the union. */
> char padding[256];
> };
> @@ -3739,6 +3756,17 @@ Once this is done the KVM_REG_MIPS_VEC_* and KVM_REG_MIPS_MSA_* registers can be
> accessed, and the Config5.MSAEn bit is accessible via the KVM API and also from
> the guest.
>
> +6.11 KVM_CAP_ARM_TIMER
> +
> +Architectures: arm, arm64
> +Target: vcpu
> +Parameters: args[0] contains a bitmap of timers to select (see 5.)
> +
> +This capability allows to route per-core timers into user space. When it's
> +enabled and no in-kernel interrupt controller is in use, the timers selected
> +by args[0] trigger KVM_EXIT_ARM_TIMER guest exits when they are pending,
> +unless masked by vcpu->run->request_interrupt_window (see 5.).
> +
> 7. Capabilities that can be enabled on VMs
> ------------------------------------------
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index de338d9..77d1f73 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -180,6 +180,9 @@ struct kvm_vcpu_arch {
>
> /* Detect first run of a vcpu */
> bool has_run_once;
> +
> + /* User space wants timer notifications */
> + bool user_space_arm_timers;
Please move this to the timer structure.
> };
>
> struct kvm_vm_stat {
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index c84b6ad..57bdb71 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -187,6 +187,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> case KVM_CAP_ARM_PSCI_0_2:
> case KVM_CAP_READONLY_MEM:
> case KVM_CAP_MP_STATE:
> + case KVM_CAP_ARM_TIMER:
> r = 1;
> break;
> case KVM_CAP_COALESCED_MMIO:
> @@ -474,13 +475,7 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
> return ret;
> }
>
> - /*
> - * Enable the arch timers only if we have an in-kernel VGIC
> - * and it has been properly initialized, since we cannot handle
> - * interrupts from the virtual timer with a userspace gic.
> - */
> - if (irqchip_in_kernel(kvm) && vgic_initialized(kvm))
> - ret = kvm_timer_enable(vcpu);
> + ret = kvm_timer_enable(vcpu);
>
> return ret;
> }
> @@ -601,6 +596,13 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
> run->exit_reason = KVM_EXIT_INTR;
> }
>
> + if (kvm_check_request(KVM_REQ_PENDING_TIMER, vcpu)) {
Since this is a very unlikely event (in the grand scheme of things), how
about making this unlikely()?
> + /* Tell user space about the pending vtimer */
> + ret = 0;
> + run->exit_reason = KVM_EXIT_ARM_TIMER;
> + run->arm_timer.timesource = KVM_ARM_TIMER_VTIMER;
> + }
More importantly: why does it have to be indirected by a
make_request/check_request, and not be handled as part of the
kvm_timer_sync() call? We do update the state there, and you could
directly find out whether an exit is required.
> +
> if (ret <= 0 || need_new_vmid_gen(vcpu->kvm) ||
> vcpu->arch.power_off || vcpu->arch.pause) {
> local_irq_enable();
> @@ -887,6 +889,12 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
> return -EINVAL;
>
> switch (cap->cap) {
> + case KVM_CAP_ARM_TIMER:
> + r = 0;
> + if (cap->args[0] != KVM_ARM_TIMER_VTIMER)
> + return -EINVAL;
> + vcpu->arch.user_space_arm_timers = true;
> + break;
> default:
> r = -EINVAL;
> break;
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 3eda975..3d01481 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -270,6 +270,9 @@ struct kvm_vcpu_arch {
>
> /* Detect first run of a vcpu */
> bool has_run_once;
> +
> + /* User space wants timer notifications */
> + bool user_space_arm_timers;
> };
>
> #define vcpu_gp_regs(v) (&(v)->arch.ctxt.gp_regs)
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 300ef25..00f4948 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -205,6 +205,7 @@ struct kvm_hyperv_exit {
> #define KVM_EXIT_S390_STSI 25
> #define KVM_EXIT_IOAPIC_EOI 26
> #define KVM_EXIT_HYPERV 27
> +#define KVM_EXIT_ARM_TIMER 28
>
> /* For KVM_EXIT_INTERNAL_ERROR */
> /* Emulate instruction failed. */
> @@ -361,6 +362,10 @@ struct kvm_run {
> } eoi;
> /* KVM_EXIT_HYPERV */
> struct kvm_hyperv_exit hyperv;
> + /* KVM_EXIT_ARM_TIMER */
> + struct {
> + __u8 timesource;
> + } arm_timer;
> /* Fix the size of the union. */
> char padding[256];
> };
> @@ -870,6 +875,7 @@ struct kvm_ppc_smmu_info {
> #define KVM_CAP_S390_USER_INSTR0 130
> #define KVM_CAP_MSI_DEVID 131
> #define KVM_CAP_PPC_HTM 132
> +#define KVM_CAP_ARM_TIMER 133
>
> #ifdef KVM_CAP_IRQ_ROUTING
>
> @@ -1327,4 +1333,12 @@ struct kvm_assigned_msix_entry {
> #define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
> #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
>
> +/* Available with KVM_CAP_ARM_TIMER */
> +
> +/* Bits for run->request_interrupt_window */
> +#define KVM_IRQWINDOW_VTIMER (1 << 0)
> +
> +/* Bits for run->arm_timer.timesource */
> +#define KVM_ARM_TIMER_VTIMER (1 << 0)
> +
> #endif /* __LINUX_KVM_H */
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 4309b60..cbbb50dd 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -170,16 +170,45 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level)
> {
> int ret;
> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> + struct kvm_run *run = vcpu->run;
>
> - BUG_ON(!vgic_initialized(vcpu->kvm));
> + BUG_ON(irqchip_in_kernel(vcpu->kvm) && !vgic_initialized(vcpu->kvm));
>
> timer->active_cleared_last = false;
> timer->irq.level = new_level;
> - trace_kvm_timer_update_irq(vcpu->vcpu_id, timer->irq.irq,
> + trace_kvm_timer_update_irq(vcpu->vcpu_id, host_vtimer_irq,
> timer->irq.level);
> - ret = kvm_vgic_inject_mapped_irq(vcpu->kvm, vcpu->vcpu_id,
> - timer->irq.irq,
> - timer->irq.level);
> +
> + if (irqchip_in_kernel(vcpu->kvm) && vgic_initialized(vcpu->kvm)) {
Given how many times you repeat this idiom in this patch, you should
have a single condition that encapsulate it once and for all.
> + /* Fire the timer in the VGIC */
> +
> + ret = kvm_vgic_inject_mapped_irq(vcpu->kvm, vcpu->vcpu_id,
> + timer->irq.irq,
> + timer->irq.level);
> + } else if (!vcpu->arch.user_space_arm_timers) {
> + /* User space has not activated timer use */
> + ret = 0;
> + } else {
> + /*
> + * Set PENDING_TIMER so that user space can handle the event if
> + *
> + * 1) Level is high
> + * 2) The vtimer is not suppressed by user space
> + * 3) We are not in the timer trigger exit path
> + */
> + if (new_level &&
> + !(run->request_interrupt_window & KVM_IRQWINDOW_VTIMER) &&
> + (run->exit_reason != KVM_EXIT_ARM_TIMER)) {
> + /* KVM_REQ_PENDING_TIMER means vtimer triggered */
> + kvm_make_request(KVM_REQ_PENDING_TIMER, vcpu);
> + }
> +
> + /* Force a new level high check on next entry */
> + timer->irq.level = 0;
I think this could become a bit more simple if you follow the flow I
mentioned earlier involving kvm_timer_sync(). Also, I only see how you
flag the line as being high, but not how you lower it. Care to explain
that flow?
> +
> + ret = 0;
> + }
> +
> WARN_ON(ret);
> }
>
> @@ -197,7 +226,8 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)
> * because the guest would never see the interrupt. Instead wait
> * until we call this function from kvm_timer_flush_hwstate.
> */
> - if (!vgic_initialized(vcpu->kvm) || !timer->enabled)
> + if ((irqchip_in_kernel(vcpu->kvm) && !vgic_initialized(vcpu->kvm)) ||
> + !timer->enabled)
> return -ENODEV;
>
> if (kvm_timer_should_fire(vcpu) != timer->irq.level)
> @@ -275,35 +305,57 @@ void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu)
> * to ensure that hardware interrupts from the timer triggers a guest
> * exit.
> */
> - phys_active = timer->irq.level ||
> - kvm_vgic_map_is_active(vcpu, timer->irq.irq);
> -
> - /*
> - * We want to avoid hitting the (re)distributor as much as
> - * possible, as this is a potentially expensive MMIO access
> - * (not to mention locks in the irq layer), and a solution for
> - * this is to cache the "active" state in memory.
> - *
> - * Things to consider: we cannot cache an "active set" state,
> - * because the HW can change this behind our back (it becomes
> - * "clear" in the HW). We must then restrict the caching to
> - * the "clear" state.
> - *
> - * The cache is invalidated on:
> - * - vcpu put, indicating that the HW cannot be trusted to be
> - * in a sane state on the next vcpu load,
> - * - any change in the interrupt state
> - *
> - * Usage conditions:
> - * - cached value is "active clear"
> - * - value to be programmed is "active clear"
> - */
> - if (timer->active_cleared_last && !phys_active)
> - return;
> + if (irqchip_in_kernel(vcpu->kvm) && vgic_initialized(vcpu->kvm)) {
> + phys_active = timer->irq.level ||
> + kvm_vgic_map_is_active(vcpu, timer->irq.irq);
> +
> + /*
> + * We want to avoid hitting the (re)distributor as much as
> + * possible, as this is a potentially expensive MMIO access
> + * (not to mention locks in the irq layer), and a solution for
> + * this is to cache the "active" state in memory.
> + *
> + * Things to consider: we cannot cache an "active set" state,
> + * because the HW can change this behind our back (it becomes
> + * "clear" in the HW). We must then restrict the caching to
> + * the "clear" state.
> + *
> + * The cache is invalidated on:
> + * - vcpu put, indicating that the HW cannot be trusted to be
> + * in a sane state on the next vcpu load,
> + * - any change in the interrupt state
> + *
> + * Usage conditions:
> + * - cached value is "active clear"
> + * - value to be programmed is "active clear"
> + */
> + if (timer->active_cleared_last && !phys_active)
> + return;
> +
> + ret = irq_set_irqchip_state(host_vtimer_irq,
> + IRQCHIP_STATE_ACTIVE,
> + phys_active);
> + } else {
> + /* User space tells us whether the timer is in active mode */
> + phys_active = vcpu->run->request_interrupt_window &
> + KVM_IRQWINDOW_VTIMER;
No, this just says "mask the interrupt". It doesn't say anything about
the state of the timer. More importantly: you sample the shared page.
What guarantees that the information there is preserved? Is userspace
writing that bit each time the vcpu thread re-enters the kernel with
this interrupt being in flight?
> +
> + /* However if the line is high, we exit anyway, so we want
> + * to keep the IRQ masked */
> + phys_active = phys_active || timer->irq.level;
Why would you force the interrupt to be masked as soon as the timer is
firing? If userspace hasn't masked it, I don't think you should paper
over it.
> +
> + /*
> + * So we can just explicitly mask or unmask the IRQ, gaining
> + * more compatibility with oddball irq controllers.
> + */
> + if (phys_active)
> + disable_percpu_irq(host_vtimer_irq);
> + else
> + enable_percpu_irq(host_vtimer_irq, 0);
Since you are now targeting random irqchips (as opposed to a GIC
specifically), what guarantees that the timer is a per-cpu IRQ?
> +
> + ret = 0;
> + }
>
> - ret = irq_set_irqchip_state(host_vtimer_irq,
> - IRQCHIP_STATE_ACTIVE,
> - phys_active);
> WARN_ON(ret);
>
> timer->active_cleared_last = !phys_active;
> @@ -479,6 +531,10 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
> if (timer->enabled)
> return 0;
>
> + /* No need to route physical IRQs when we don't use the vgic */
> + if (!irqchip_in_kernel(vcpu->kvm))
> + goto no_vgic;
> +
> /*
> * Find the physical IRQ number corresponding to the host_vtimer_irq
> */
> @@ -502,6 +558,7 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
> if (ret)
> return ret;
>
> +no_vgic:
>
> /*
> * There is a potential race here between VCPUs starting for the first
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [GIT PULL 2/5] i.MX soc updates for 4.9
From: Arnd Bergmann @ 2016-09-19 14:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473670948-4265-2-git-send-email-shawnguo@kernel.org>
On Monday, September 12, 2016 5:02:25 PM CEST Shawn Guo wrote:
> i.MX SoC updates for 4.9:
> - Set INT_MEM_CLK_LPM bit to get proper WAIT mode support on i.MX6SX.
> This is a workaround for i.MX6SX WAIT mode hardware issue.
> - Enable cpuidle support with 3 low-power states (WFI, WAIT, POWER-OFF)
> for i.MX6UL.
>
>
Pulled into next/soc, thanks!
Arnd
^ permalink raw reply
* [GIT PULL 1/3] ARM: soc: exynos: Drivers for v4.9
From: Arnd Bergmann @ 2016-09-19 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474216788-17282-2-git-send-email-krzk@kernel.org>
On Sunday, September 18, 2016 6:39:46 PM CEST Krzysztof Kozlowski wrote:
> Samsung drivers/soc update for v4.9:
> 1. Allow compile testing of exynos-mct clocksource driver on ARM64.
> 2. Document Exynos5433 PMU compatible (already used by clkout driver and more
> will be coming soon).
Pulled into next/drivers, thanks
Just for my understanding: why do we need the exynos-mct driver on ARM64
but not the delay-timer portion of it?
Is there an advantage in using MCT over the architected timer on these
chips? If so, should we also have a way to use it as the delay timer?
Arnd
^ permalink raw reply
* [PATCH v3 1/8] scpi: Add cmd indirection table to prepare for legacy commands
From: Neil Armstrong @ 2016-09-19 15:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <213c023d-bac8-5148-4846-8501501bddf2@arm.com>
On 09/19/2016 04:41 PM, Sudeep Holla wrote:
> Hi Neil,
>
> On 07/09/16 16:34, Neil Armstrong wrote:
>> Add indirection table to permit multiple command values for legacy support.
>>
>
> I wrote the most of the patch and you changed the author too ;)
Sorry, forgot this ! v4 will have it !
>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> drivers/firmware/arm_scpi.c | 145 ++++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 127 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
>> index 4388937..9a87687 100644
>> --- a/drivers/firmware/arm_scpi.c
>> +++ b/drivers/firmware/arm_scpi.c
>
> [..]
>
>> @@ -161,6 +194,7 @@ struct scpi_drvinfo {
>> u32 protocol_version;
>> u32 firmware_version;
>> int num_chans;
>> + int *scpi_cmds;
>> atomic_t next_chan;
>> struct scpi_ops *scpi_ops;
>> struct scpi_chan *channels;
>> @@ -390,6 +424,19 @@ static u32 scpi_get_version(void)
>> return scpi_info->protocol_version;
>> }
>>
>> +static inline int check_cmd(unsigned int offset)
>> +{
>> + if (offset >= CMD_MAX_COUNT ||
>
> If we call scpi_send_message internally(as it's static) why is this
> check needed ?
>
>
>> + !scpi_info ||
>> + !scpi_info->scpi_cmds)
>
> Will be even reach to this point if above is true ?
>
>> + return -EINVAL;
>> +
>> + if (scpi_info->scpi_cmds[offset] < 0)
>> + return -EOPNOTSUPP;
>
> IMO just above couple of lines in the beginning of scpi_send_message
> will suffice. You can just add this to my original patch.
Will do.
>
>> static int
>> scpi_clk_get_range(u16 clk_id, unsigned long *min, unsigned long *max)
>> {
>> @@ -397,8 +444,13 @@ scpi_clk_get_range(u16 clk_id, unsigned long *min, unsigned long *max)
>> struct clk_get_info clk;
>> __le16 le_clk_id = cpu_to_le16(clk_id);
>>
>> - ret = scpi_send_message(SCPI_CMD_GET_CLOCK_INFO, &le_clk_id,
>> - sizeof(le_clk_id), &clk, sizeof(clk));
>> + ret = check_cmd(CMD_GET_CLOCK_INFO);
>> + if (ret)
>> + return ret;
>> +
>
> It's totally unnecessary to add check in each and every function calling
> scpi_send_message, why not add it to scpi_send_message instead.
>
This was my first thought, I should have stayed at this !
Thanks,
Neil
^ permalink raw reply
* [PATCH v5 02/16] dt/bindings: Update binding for PM domain idle states
From: Brendan Jackman @ 2016-09-19 15:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7af4c2d7-70a3-1881-2980-8ea49e594e5b@arm.com>
On Fri, Sep 16 2016 at 18:39, Sudeep Holla <sudeep.holla@arm.com> wrote:
> Hi Kevin,
>
> Thanks for looking at this and simplifying various discussions we had so
> far. I was thinking of summarizing something very similar. I couldn't
> due to lack of time.
>
> On 16/09/16 18:13, Kevin Hilman wrote:
>
> [...]
>
>> I think we're having some terminology issues...
>>
>> FWIW, the kernel terminolgy is actually "PM domain", not power domain.
>> This was intentional because the goal of the PM domain was to group
>> devices that some PM features. To be very specific to the kernel, they
>> us the same set of PM callbacks. Today, this is most commonly used to
>> model power domains, where a group of devices share a power rail, but it
>> does not need to be limited to that.
>>
>
> Agreed/Understood.
>
>> That being said, I'm having a hard time understanding the root of the
>> disagreement.
>>
>
> Yes. I tried to convey the same earlier, but have failed. The only
> disagreement is about a small part of this DT bindings. We would like to
> make it completely hierarchical up to CPU nodes. More comments on that
> below.
>
>> It seems that you and Sudeep would like to use domain-idle-states to
>> replace/superceed cpu-idle-states with the primary goal (and benefit)
>> being that it simplifies the DT bindings. Is that correct?
>>
>
> Correct, we want to deprecate cpu-idle-states with the introduction of
> this hierarchical PM bindings. Yes IMO, it simplifies things and avoids
> any ABI break we might trigger if we miss to consider some use-case now.
>
>> The objections have come in because that means that implies that CPUs
>> become their own domains, which may not be the case in hardware in the
>> sense that they share a power rail.
>>
>
> Agreed.
>
>> However, IMO, thinking of a CPU as it's own "PM domain" may make some
>> sense based on the terminology above.
>>
>
> Thanks for that, we do understand that it may not be 100% correct when
> we strictly considers hardware terminologies instead of above ones.
> As along as we see no issues with the above terminologies it should be fine.
>
>> I think the other objection may be that using a genpd to model domain
>> with only a single device in it may be overkill, and I agree with that.
>
> I too agree with that. Just because we represent that in DT in that way
> doesn't mean we need to create a genpd to model domain. We can always
> skip that if not required. That's pure implementation specifics and I
> have tried to convey the same in my previous emails. I must say you have
> summarized it very clearly in this email. Thanks again for that.
>
>> But, I'm not sure if making CPUs use domain-idle-states implies that
>> they necessarily have to use genpd is what you are proposing. Maybe
>> someone could clarify that?
>>
>
> No, I have not proposing anything around implementation in the whole
> discussion so far. I have constrained myself just to DT bindings so far.
> That's the main reason why I was opposed to mentions of OS vs platform
> co-ordinated modes of CPU suspend in this discussion. IMO that's
> completely out of scope of this DT binding we are defining here.
>
> Hope that helps/clarifies the misunderstanding/disagreement.
Indeed. My intention was that the proposal would result in the exact
same kernel behaviour as Lina's current patchset, i.e. there is one
genpd per cluster, and CPU-level idle states are still handled by
cpuidle.
The only change from the current patchset would be in initialisation
code: some coordination would need to be done to determine which idle
states go into cpuidle and which go into the genpds (whereas with the
current bindings, states from cpu-idle-states go into cpuidle and states
from domain-idle-states go into genpd). So you could say that this would
be a trade-off between binding simplicity and implementation simplicity.
Cheers,
Brendan
^ permalink raw reply
* [PATCH v2 4/4] arm64: dts: add Pine64 support
From: Chen-Yu Tsai @ 2016-09-19 15:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c7ad213f-e5d1-d5c5-a83c-af1c38d9c1d6@arm.com>
On Mon, Sep 12, 2016 at 6:11 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> Hi,
>
> On 10/09/16 03:33, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Sat, Sep 10, 2016 at 4:10 AM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>>> From: Andre Przywara <andre.przywara@arm.com>
>>>
>>> The Pine64 is a cost-efficient development board based on the
>>> Allwinner A64 SoC.
>>> There are three models: the basic version with Fast Ethernet and
>>> 512 MB of DRAM (Pine64) and two Pine64+ versions, which both
>>> feature Gigabit Ethernet and additional connectors for touchscreens
>>> and a camera. Or as my son put it: "Those are smaller and these are
>>> missing." ;-)
>>> The two Pine64+ models just differ in the amount of DRAM
>>> (1GB vs. 2GB). Since U-Boot will figure out the right size for us and
>>> patches the DT accordingly we just need to provide one DT for the
>>> Pine64+.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> [Maxime: Removed the common DTSI and include directly the pine64 DTS]
>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>> ---
>>> arch/arm64/boot/dts/Makefile | 1 +
>>> arch/arm64/boot/dts/allwinner/Makefile | 5 ++
>>> .../boot/dts/allwinner/sun50i-a64-pine64-plus.dts | 50 ++++++++++++++++
>>> .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 70 ++++++++++++++++++++++
>>> 4 files changed, 126 insertions(+)
>>> create mode 100644 arch/arm64/boot/dts/allwinner/Makefile
>>> create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
>>> create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
>>>
>>> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
>>> index 6e199c903676..ddcbf5a2c17e 100644
>>> --- a/arch/arm64/boot/dts/Makefile
>>> +++ b/arch/arm64/boot/dts/Makefile
>>> @@ -1,4 +1,5 @@
>>> dts-dirs += al
>>> +dts-dirs += allwinner
>>> dts-dirs += altera
>>> dts-dirs += amd
>>> dts-dirs += amlogic
>>> diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
>>> new file mode 100644
>>> index 000000000000..1e29a5ae8282
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/allwinner/Makefile
>>> @@ -0,0 +1,5 @@
>>> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pine64-plus.dtb sun50i-a64-pine64.dtb
>>> +
>>> +always := $(dtb-y)
>>> +subdir-y := $(dts-dirs)
>>> +clean-files := *.dtb
>>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
>>> new file mode 100644
>>> index 000000000000..790d14daaa6a
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
>>> @@ -0,0 +1,50 @@
>>> +/*
>>> + * Copyright (c) 2016 ARM Ltd.
>>> + *
>>> + * This file is dual-licensed: you can use it either under the terms
>>> + * of the GPL or the X11 license, at your option. Note that this dual
>>> + * licensing only applies to this file, and not this project as a
>>> + * whole.
>>> + *
>>> + * a) This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public License as
>>> + * published by the Free Software Foundation; either version 2 of the
>>> + * License, or (at your option) any later version.
>>> + *
>>> + * This library is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * Or, alternatively,
>>> + *
>>> + * b) Permission is hereby granted, free of charge, to any person
>>> + * obtaining a copy of this software and associated documentation
>>> + * files (the "Software"), to deal in the Software without
>>> + * restriction, including without limitation the rights to use,
>>> + * copy, modify, merge, publish, distribute, sublicense, and/or
>>> + * sell copies of the Software, and to permit persons to whom the
>>> + * Software is furnished to do so, subject to the following
>>> + * conditions:
>>> + *
>>> + * The above copyright notice and this permission notice shall be
>>> + * included in all copies or substantial portions of the Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>>> + * OTHER DEALINGS IN THE SOFTWARE.
>>> + */
>>> +
>>> +#include "sun50i-a64-pine64.dts"
>>> +
>>> +/ {
>>> + model = "Pine64+";
>>> + compatible = "pine64,pine64-plus", "allwinner,sun50i-a64";
>>> +
>>> + /* TODO: Camera, Ethernet PHY, touchscreen, etc. */
>>> +};
>>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
>>> new file mode 100644
>>> index 000000000000..da9bca51f5a9
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
>>> @@ -0,0 +1,70 @@
>>> +/*
>>> + * Copyright (c) 2016 ARM Ltd.
>>> + *
>>> + * This file is dual-licensed: you can use it either under the terms
>>> + * of the GPL or the X11 license, at your option. Note that this dual
>>> + * licensing only applies to this file, and not this project as a
>>> + * whole.
>>> + *
>>> + * a) This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public License as
>>> + * published by the Free Software Foundation; either version 2 of the
>>> + * License, or (at your option) any later version.
>>> + *
>>> + * This library is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * Or, alternatively,
>>> + *
>>> + * b) Permission is hereby granted, free of charge, to any person
>>> + * obtaining a copy of this software and associated documentation
>>> + * files (the "Software"), to deal in the Software without
>>> + * restriction, including without limitation the rights to use,
>>> + * copy, modify, merge, publish, distribute, sublicense, and/or
>>> + * sell copies of the Software, and to permit persons to whom the
>>> + * Software is furnished to do so, subject to the following
>>> + * conditions:
>>> + *
>>> + * The above copyright notice and this permission notice shall be
>>> + * included in all copies or substantial portions of the Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>>> + * OTHER DEALINGS IN THE SOFTWARE.
>>> + */
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include "sun50i-a64.dtsi"
>>> +
>>> +/ {
>>> + model = "Pine64";
>>> + compatible = "pine64,pine64", "allwinner,sun50i-a64";
>>> +
>>> + aliases {
>>> + serial0 = &uart0;
>>> + };
>>> +
>>> + chosen {
>>> + stdout-path = "serial0:115200n8";
>>> + };
>>> +};
>>> +
>>> +&uart0 {
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&uart0_pins_a>;
>>> + status = "okay";
>>> +};
>>> +
>>> +&i2c1 {
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&i2c1_pins>;
>>> + status = "okay";
>>
>> Schematics say this is missing an external pull-up. Has anyone tried it?
>> Without a pull-up any access on the i2c bus should just block.
>
> I tried it a while ago (with an older kernel), though I had an external
> pull-up on the device side, I think. I can give it a try again with
> Maxime's branch once I find this I2C test device in one of my moving
> boxes ;-)
>
> Do other boards providing a Raspi-compatible header have an on-board
> pull-up here? So will (some) hardware extensions for the Raspi fail on
> the Pine64?
IIRC The Bananapi series have pull-ups on the default I2C pins.
>> Also this is on the RPi-2 compatible header. There's also an UART and SPI
>> that are standard for the RPi-2 header. We should consider enabling them
>> as well.
>
> Interesting topic ;-)
> As both interfaces can be configured for GPIO as well, I think Maxime
> voted to not declare them as special function in the upstream DT.
> UART0 is used for the console, so I guess some people may decide to use
> those UART2 header pins for GPIO. Similarly for SPI: if you don't need
> it, you get four (or five) additional GPIO.
>
> A kind of related issue arises for those BT/Wifi headers. 99.9% of the
> users will plug the Pine64 BT/WiFi module in there, at which point we
> could declare MMC1 as SDIO and UART1 as enabled to cover this.
> But then again nothing prevents people from building a custom module
> which uses those pins for something else. Port G at least does not
> multiplex any other IP to those pins - apart from GPIO, of course.
>
> So what is the exact policy here? In the end I think any pin (including
> UART0's PB8 and PB9) can be configured as GPIO, so do we make some
> assumptions about "sensible" or predominant usage?
My personal position is if the vendor explicitly labeled X pin as Y
peripheral, then we should enable it by default, since that is possibly
what many users would be expecting. We can list, but explicitly disable,
other peripherals that the vendor mentioned as optional or changeable.
Obviously most pins are changeable from the SoC PoV, so my focus here
is what the vendor intended.
Unfortunately I've not completely worked this out with Maxime yet,
and I am somewhat busy on other fronts lately. I'll be at ELCE this
year, so hopefully we'll get this sorted out soon.
> Does "# echo $DEVICE > /sys/devices/platform/$DEVICE/driver/unbind" work
> to get those pins free later in case one wants to configure them as
> GPIOs? In this case we may consider being more generous in declaring
> common use cases in the upstream DT for the sake of the majority of users.
I believe it does, but I've never actually tried it.
Regards
ChenYu
^ permalink raw reply
* [PATCH] ASoC: samsung: make audio interface/controller explicitly
From: Krzysztof Kozlowski @ 2016-09-19 15:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <30C748BE-359C-4212-96D3-C2A71E1094C3@soulik.info>
On Mon, Sep 19, 2016 at 09:21:54AM +0800, Ayaka wrote:
>
>
> ??? iPad ??
>
> > Krzysztof Kozlowski <krzk@kernel.org> ? 2016?9?19? ??2:09 ???
> >
> >> On Sun, Sep 18, 2016 at 11:12:34PM +0800, ayaka wrote:
> >>
> >>
> >>> On 09/18/2016 10:42 PM, Krzysztof Kozlowski wrote:
> >>>> On Sun, Sep 18, 2016 at 10:09:11PM +0800, Randy Li wrote:
> >>>> It is simple sound card time, we could assign different codec
> >>>> to a interface without making a specific driver for it.
> >>> The description does not convince me and I do not see an example using
> >>> this. Could you provide one?
> >> Sorry, the board TOPEET iTop 4412 for exynos 4412 I posted supported codec
> >> with I2S interface using the simple sound card. Anyway, it is no harm to
> >> make them explicitly right?
> >
> > kbuild gave you the answer...
> Not sure how comes, even time I sent patches to you, I at lease build and run it once.
Building only one ARM config may be sufficient for trivial changes but
for more complex stuff (especially when dealing with Kconfig) needs more
build coverage.
On Ubuntu you can easily cross compile for x86, x86_64,
arm, arm64 and powerpc. You can also use this cross-compiler collection:
https://www.kernel.org/pub/tools/crosstool
> >
> >> Or I have to enabled those codec support for
> >> SMDK, which is not needed for the other board.
> >
> > If I understand correctly, the i2s/pcm etc are still needed but not
> > built in config choosing only simple-audio-card? I tried now such
> > configuration on Odroid XU and indeed the audio is missing.
> >
> > The patch looks like needed but:
> > 1. You need to describe the rationale in commit message, why it is
> > needed.
> Sorry about English.
> > 2. You need to fix it... kbuild pointed build issues.
> I would check that.
> >
> > Other solution would be to add a user-selectable option for generic
> > sound on Samsung using simple audio card. The option would then select
> > appropriate SND_SAMSUNG* options, just like specific drivers do. I see
> > that sh does like this. Personally this approach seems simpler to me -
> > the defconfig could just choose this generic sound instead of many
> > SND_SAMSUNG_* sub-options.
> I would just what Freescale did. It included those options entries in a sub-menu.
> I don't those options should be bound either, as a board may only use one of interface or controller(like TOPEET iTOP would only use i2s. No place for SPDIF, AC97 nor PCM.)
OK, sounds good for me. Thanks for explanation.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH] arm: dts: fix rk3066a based boards vdd_log voltage initialization
From: Doug Anderson @ 2016-09-19 15:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474274696-28090-1-git-send-email-andy.yan@rock-chips.com>
Hi,
On Mon, Sep 19, 2016 at 1:44 AM, Andy Yan <andy.yan@rock-chips.com> wrote:
> The current rk3066a based boards(Rayeager, Bqcurie2, Marsboard) use
> pwm modulate vdd_logic voltage, but the pwm is default disabled and
> the pwm pin acts as a gpio before pwm regulator probed, so the pwm
> regulator driver will get a zero dutycycle at probe time, so change
> the initial dutycycle to zero to match pwm_regulator_init_state check.
>
> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>
> ---
>
> arch/arm/boot/dts/rk3066a-bqcurie2.dts | 2 +-
> arch/arm/boot/dts/rk3066a-marsboard.dts | 2 +-
> arch/arm/boot/dts/rk3066a-rayeager.dts | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/boot/dts/rk3066a-bqcurie2.dts b/arch/arm/boot/dts/rk3066a-bqcurie2.dts
> index bc674ee..618450d 100644
> --- a/arch/arm/boot/dts/rk3066a-bqcurie2.dts
> +++ b/arch/arm/boot/dts/rk3066a-bqcurie2.dts
> @@ -61,7 +61,7 @@
> regulator-min-microvolt = <1200000>;
> regulator-max-microvolt = <1200000>;
> regulator-always-on;
> - voltage-table = <1000000 100>,
> + voltage-table = <1000000 0>,
In my opinion this isn't quite the right answer. I think that you
should add a new property describing the voltage in the case that the
pin is an input and you should fill that property in, like:
voltage-when-input = <1000000>;
Once you have this property you should ideally be able to read whether
the pin is currently configured as an input or as a special function
at bootup. Note that I don't actually know if this is possible with
the current pinctrl API, but it does seem like the ideal way to do it
since it means you'll work even if the BIOS changes (AKA: if the BIOS
leaves the pin as an input you can keep the voltage the same and if
the BIOS leaves the pin as PWM you can keep the voltage the same).
It's also possible that you could just add a property that says "init
to a certain value at bootup no matter what the BIOS left it as". As
long as that voltage is the maximum (and you'll lower it later) this
ought to be safe and you shouldn't risk temporarily undervolting
things.
Note that, if you haven't already done so, you almost certainly want
to make sure your pinctrl species an "init" state in addition to a
"default" state. See <https://patchwork.kernel.org/patch/7454311/>.
-Doug
^ permalink raw reply
* [GIT PULL 1/2] arm64: X-Gene driver updates queued for 4.9
From: Arnd Bergmann @ 2016-09-19 15:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CADaLNDnbuQWrV6myasn2k5Mi7=jdg7FAjbdbS3GrbXY34cBfNA@mail.gmail.com>
On Friday, September 16, 2016 12:01:26 AM CEST Duc Dang wrote:
> This is the driver updates for X-Gene platforms targeted for 4.9.
>
> The change set includes:
> + X-Gene Soc PMU support patch set from Tai Nguyen (v10 reviewed by
> Mark, DT binding document acked by Rob [1] and was suggested to merge
> via am-soc tree by Will [2])
>
> As we discussed, the X-Gene Soc PMU will go through arm-soc tree this
> time because we don't have a dedicated tree for PMU drivers yet.
> Starting from v4.10, PMU drivers will be collected by Will/Mark before
> sending to arm-soc tree [3].
>
>
Pulled into next/drivers, thanks!
Arnd
^ permalink raw reply
* [PATCH] ARM: multi_v7_defconfig: enable CONFIG_EFI
From: Arnd Bergmann @ 2016-09-19 15:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473949700-12814-1-git-send-email-ard.biesheuvel@linaro.org>
On Thursday, September 15, 2016 3:28:20 PM CEST Ard Biesheuvel wrote:
> This enables CONFIG_EFI for multi_v7_defconfig, which adds support for
> booting via EFI, and for the EFI framebuffer as builtin options. It
> also enables the EFI rtc, the EFI variable pseudo-filesystem and the
> EFI capsule loader as modules.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>
> We have been happily booting multi_v7_defconfig+CONFIG_EFI=y kernels on
> kernelci for months now, so please consider enabling this by default.
> The increase in compressed kernel footprint is ~30 KB, for the uncompressed
> kernel it's ~10 KB, some of which is .init code.
>
Applied to next/defconfig, thanks!
Arnd
^ permalink raw reply
* [PATCH 1/4] drm/sun4i: rgb: Declare RGB encoder and connector as MIPI DPI
From: Chen-Yu Tsai @ 2016-09-19 15:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160918191205.GG17518@lukather>
On Mon, Sep 19, 2016 at 3:12 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Thu, Sep 15, 2016 at 11:13:59PM +0800, Chen-Yu Tsai wrote:
>> The 18 or 24 bit parallel RGB LCD panel interface found on Allwinner
>> SoCs matches the description of MIPI DPI. Declare the RGB encoder and
>> connector as MIPI DPI.
>
> Unfortunately, even it that patch might be true (is there a public
> spec for that standard?), this breaks the user-space, so there's no
Not that I know of. I basically googled the term and looked at other
datasheets and asked on #linux-arm-kernel.
> way we change this.
:(
ChenYu
^ permalink raw reply
* [PATCH 0/8] crypto: omap-sham: convert to sg based data engine
From: Tero Kristo @ 2016-09-19 15:22 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
This series converts the omap-sham buffer handling towards a scatterlist
approach. This avoids the need to have a huge internal buffer within the
driver, and also allows us to properly implement export/import for the
driver. I tried splitting up the changes to some sane patches, but this
was rather difficult due to the fact that this is largely a complete
rewrite of portions of the driver. Patch #6 is a prime example of this
being pretty large, but splitting this up would break bisectability.
Hopefully the patch is still understandable though.
Crypto manager tests work fine at least on omap3/am3/am4/dra7 SoC:s.
(Something is broken with test farm again and could not try omap2/omap4.)
Also tested tcrypt SHA performance on DRA7 and it seems to be working
fine with different buffer sizes.
My test branch is also available here for interested parties:
tree: https://github.com/t-kristo/linux-pm.git
breanch: 4.8-rc1-crypto
-Tero
^ permalink raw reply
* [PATCH 1/8] crypto: omap-sham: add context export/import stubs
From: Tero Kristo @ 2016-09-19 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474298539-23897-1-git-send-email-t-kristo@ti.com>
Initially these just return -ENOTSUPP to indicate that they don't
really do anything yet. Some sort of implementation is required
for the driver to at least probe.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
drivers/crypto/omap-sham.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index cf9f617c..74653c9 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -1340,6 +1340,16 @@ static void omap_sham_cra_exit(struct crypto_tfm *tfm)
}
}
+static int omap_sham_export(struct ahash_request *req, void *out)
+{
+ return -ENOTSUPP;
+}
+
+static int omap_sham_import(struct ahash_request *req, const void *in)
+{
+ return -ENOTSUPP;
+}
+
static struct ahash_alg algs_sha1_md5[] = {
{
.init = omap_sham_init,
@@ -1998,8 +2008,13 @@ static int omap_sham_probe(struct platform_device *pdev)
for (i = 0; i < dd->pdata->algs_info_size; i++) {
for (j = 0; j < dd->pdata->algs_info[i].size; j++) {
- err = crypto_register_ahash(
- &dd->pdata->algs_info[i].algs_list[j]);
+ struct ahash_alg *alg;
+
+ alg = &dd->pdata->algs_info[i].algs_list[j];
+ alg->export = omap_sham_export;
+ alg->import = omap_sham_import;
+ alg->halg.statesize = sizeof(struct omap_sham_reqctx);
+ err = crypto_register_ahash(alg);
if (err)
goto err_algs;
--
1.9.1
^ permalink raw reply related
* [PATCH 2/8] crypto: omap-sham: align algorithms on word offset
From: Tero Kristo @ 2016-09-19 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474298539-23897-1-git-send-email-t-kristo@ti.com>
OMAP HW generally expects data for DMA to be on word boundary, so make the
SHA driver inform crypto framework of the same preference.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
drivers/crypto/omap-sham.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 74653c9..3f2bf98 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -1368,7 +1368,7 @@ static struct ahash_alg algs_sha1_md5[] = {
CRYPTO_ALG_NEED_FALLBACK,
.cra_blocksize = SHA1_BLOCK_SIZE,
.cra_ctxsize = sizeof(struct omap_sham_ctx),
- .cra_alignmask = 0,
+ .cra_alignmask = OMAP_ALIGN_MASK,
.cra_module = THIS_MODULE,
.cra_init = omap_sham_cra_init,
.cra_exit = omap_sham_cra_exit,
@@ -1467,7 +1467,7 @@ static struct ahash_alg algs_sha224_sha256[] = {
CRYPTO_ALG_NEED_FALLBACK,
.cra_blocksize = SHA224_BLOCK_SIZE,
.cra_ctxsize = sizeof(struct omap_sham_ctx),
- .cra_alignmask = 0,
+ .cra_alignmask = OMAP_ALIGN_MASK,
.cra_module = THIS_MODULE,
.cra_init = omap_sham_cra_init,
.cra_exit = omap_sham_cra_exit,
@@ -1489,7 +1489,7 @@ static struct ahash_alg algs_sha224_sha256[] = {
CRYPTO_ALG_NEED_FALLBACK,
.cra_blocksize = SHA256_BLOCK_SIZE,
.cra_ctxsize = sizeof(struct omap_sham_ctx),
- .cra_alignmask = 0,
+ .cra_alignmask = OMAP_ALIGN_MASK,
.cra_module = THIS_MODULE,
.cra_init = omap_sham_cra_init,
.cra_exit = omap_sham_cra_exit,
@@ -1562,7 +1562,7 @@ static struct ahash_alg algs_sha384_sha512[] = {
CRYPTO_ALG_NEED_FALLBACK,
.cra_blocksize = SHA384_BLOCK_SIZE,
.cra_ctxsize = sizeof(struct omap_sham_ctx),
- .cra_alignmask = 0,
+ .cra_alignmask = OMAP_ALIGN_MASK,
.cra_module = THIS_MODULE,
.cra_init = omap_sham_cra_init,
.cra_exit = omap_sham_cra_exit,
@@ -1584,7 +1584,7 @@ static struct ahash_alg algs_sha384_sha512[] = {
CRYPTO_ALG_NEED_FALLBACK,
.cra_blocksize = SHA512_BLOCK_SIZE,
.cra_ctxsize = sizeof(struct omap_sham_ctx),
- .cra_alignmask = 0,
+ .cra_alignmask = OMAP_ALIGN_MASK,
.cra_module = THIS_MODULE,
.cra_init = omap_sham_cra_init,
.cra_exit = omap_sham_cra_exit,
--
1.9.1
^ permalink raw reply related
* [PATCH 3/8] crypto: omap-sham: rename sgl to sgl_tmp for deprecation
From: Tero Kristo @ 2016-09-19 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474298539-23897-1-git-send-email-t-kristo@ti.com>
The current usage of sgl will be deprecated, and will be replaced by an
array required by the sg based driver implementation. Rename the existing
variable as sgl_tmp so that it can be removed from the driver easily later.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
drivers/crypto/omap-sham.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 3f2bf98..33bea52 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -151,7 +151,7 @@ struct omap_sham_reqctx {
/* walk state */
struct scatterlist *sg;
- struct scatterlist sgl;
+ struct scatterlist sgl_tmp;
unsigned int offset; /* offset in current sg */
unsigned int total; /* total request */
@@ -583,18 +583,19 @@ static int omap_sham_xmit_dma(struct omap_sham_dev *dd, dma_addr_t dma_addr,
if (is_sg) {
/*
* The SG entry passed in may not have the 'length' member
- * set correctly so use a local SG entry (sgl) with the
+ * set correctly so use a local SG entry (sgl_tmp) with the
* proper value for 'length' instead. If this is not done,
* the dmaengine may try to DMA the incorrect amount of data.
*/
- sg_init_table(&ctx->sgl, 1);
- sg_assign_page(&ctx->sgl, sg_page(ctx->sg));
- ctx->sgl.offset = ctx->sg->offset;
- sg_dma_len(&ctx->sgl) = len32;
- sg_dma_address(&ctx->sgl) = sg_dma_address(ctx->sg);
-
- tx = dmaengine_prep_slave_sg(dd->dma_lch, &ctx->sgl, 1,
- DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ sg_init_table(&ctx->sgl_tmp, 1);
+ sg_assign_page(&ctx->sgl_tmp, sg_page(ctx->sg));
+ ctx->sgl_tmp.offset = ctx->sg->offset;
+ sg_dma_len(&ctx->sgl_tmp) = len32;
+ sg_dma_address(&ctx->sgl_tmp) = sg_dma_address(ctx->sg);
+
+ tx = dmaengine_prep_slave_sg(dd->dma_lch, &ctx->sgl_tmp, 1,
+ DMA_MEM_TO_DEV,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
} else {
tx = dmaengine_prep_slave_single(dd->dma_lch, dma_addr, len32,
DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
--
1.9.1
^ permalink raw reply related
* [PATCH 4/8] crypto: omap-sham: add support functions for sg based data handling
From: Tero Kristo @ 2016-09-19 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474298539-23897-1-git-send-email-t-kristo@ti.com>
Currently omap-sham uses a huge internal buffer for caching data, and
pushing this out to the DMA as large chunks. This, unfortunately,
doesn't work too well with the export/import functionality required
for ahash algorithms, and must be changed towards more scatterlist
centric approach.
This patch adds support functions for (mostly) scatterlist based data
handling. omap_sham_prepare_request() prepares a scatterlist for DMA
transfer to SHA crypto accelerator. This requires checking the data /
offset / length alignment of the data, splitting the data to SHA block
size granularity, and adding any remaining data back to the buffer.
With this patch, the code doesn't actually go live yet, the support code
will be taken properly into use with additional patches that modify the
SHA driver functionality itself.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
drivers/crypto/omap-sham.c | 263 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 263 insertions(+)
diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 33bea52..8558989 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -112,6 +112,8 @@
#define FLAGS_DMA_READY 6
#define FLAGS_AUTO_XOR 7
#define FLAGS_BE32_SHA1 8
+#define FLAGS_SGS_COPIED 9
+#define FLAGS_SGS_ALLOCED 10
/* context flags */
#define FLAGS_FINUP 16
#define FLAGS_SG 17
@@ -151,8 +153,10 @@ struct omap_sham_reqctx {
/* walk state */
struct scatterlist *sg;
+ struct scatterlist sgl[2];
struct scatterlist sgl_tmp;
unsigned int offset; /* offset in current sg */
+ int sg_len;
unsigned int total; /* total request */
u8 buffer[0] OMAP_ALIGNED;
@@ -223,6 +227,7 @@ struct omap_sham_dev {
struct dma_chan *dma_lch;
struct tasklet_struct done_task;
u8 polling_mode;
+ u8 xmit_buf[BUFLEN];
unsigned long flags;
struct crypto_queue queue;
@@ -626,6 +631,260 @@ static int omap_sham_xmit_dma(struct omap_sham_dev *dd, dma_addr_t dma_addr,
return -EINPROGRESS;
}
+static int omap_sham_copy_sg_lists(struct omap_sham_reqctx *ctx,
+ struct scatterlist *sg, int bs, int new_len)
+{
+ int n = sg_nents(sg);
+ struct scatterlist *tmp;
+ int offset = ctx->offset;
+
+ if (ctx->bufcnt)
+ n++;
+
+ ctx->sg = kmalloc_array(n, sizeof(*sg), GFP_KERNEL);
+ if (!ctx->sg)
+ return -ENOMEM;
+
+ sg_init_table(ctx->sg, n);
+
+ tmp = ctx->sg;
+
+ ctx->sg_len = 0;
+
+ if (ctx->bufcnt) {
+ sg_set_buf(tmp, ctx->dd->xmit_buf, ctx->bufcnt);
+ tmp = sg_next(tmp);
+ ctx->sg_len++;
+ }
+
+ while (sg && new_len) {
+ int len = sg->length - offset;
+
+ if (offset) {
+ offset -= sg->length;
+ if (offset < 0)
+ offset = 0;
+ }
+
+ if (new_len < len)
+ len = new_len;
+
+ if (len > 0) {
+ new_len -= len;
+ sg_set_page(tmp, sg_page(sg), len, sg->offset);
+ if (new_len <= 0)
+ sg_mark_end(tmp);
+ tmp = sg_next(tmp);
+ ctx->sg_len++;
+ }
+
+ sg = sg_next(sg);
+ }
+
+ set_bit(FLAGS_SGS_ALLOCED, &ctx->dd->flags);
+
+ ctx->bufcnt = 0;
+
+ return 0;
+}
+
+static int omap_sham_copy_sgs(struct omap_sham_reqctx *ctx,
+ struct scatterlist *sg, int bs, int new_len)
+{
+ int pages;
+ void *buf;
+ int len;
+
+ len = new_len + ctx->bufcnt;
+
+ pages = get_order(ctx->total);
+
+ buf = (void *)__get_free_pages(GFP_ATOMIC, pages);
+ if (!buf) {
+ pr_err("Couldn't allocate pages for unaligned cases.\n");
+ return -ENOMEM;
+ }
+
+ if (ctx->bufcnt)
+ memcpy(buf, ctx->dd->xmit_buf, ctx->bufcnt);
+
+ scatterwalk_map_and_copy(buf + ctx->bufcnt, sg, ctx->offset,
+ ctx->total - ctx->bufcnt, 0);
+ sg_init_table(ctx->sgl, 1);
+ sg_set_buf(ctx->sgl, buf, len);
+ ctx->sg = ctx->sgl;
+ set_bit(FLAGS_SGS_COPIED, &ctx->dd->flags);
+ ctx->sg_len = 1;
+ ctx->bufcnt = 0;
+ ctx->offset = 0;
+
+ return 0;
+}
+
+static int omap_sham_align_sgs(struct scatterlist *sg,
+ int nbytes, int bs, bool final,
+ struct omap_sham_reqctx *rctx)
+{
+ int n = 0;
+ bool aligned = true;
+ bool list_ok = true;
+ struct scatterlist *sg_tmp = sg;
+ int new_len;
+ int offset = rctx->offset;
+
+ if (!sg || !sg->length || !nbytes)
+ return 0;
+
+ new_len = nbytes;
+
+ if (offset)
+ list_ok = false;
+
+ if (final)
+ new_len = DIV_ROUND_UP(new_len, bs) * bs;
+ else
+ new_len = new_len / bs * bs;
+
+ while (nbytes > 0 && sg_tmp) {
+ n++;
+
+ if (offset < sg_tmp->length) {
+ if (!IS_ALIGNED(offset + sg_tmp->offset, 4)) {
+ aligned = false;
+ break;
+ }
+
+ if (!IS_ALIGNED(sg_tmp->length - offset, bs)) {
+ aligned = false;
+ break;
+ }
+ }
+
+ if (offset) {
+ offset -= sg_tmp->length;
+ if (offset < 0) {
+ nbytes += offset;
+ offset = 0;
+ }
+ } else {
+ nbytes -= sg_tmp->length;
+ }
+
+ sg_tmp = sg_next(sg_tmp);
+
+ if (nbytes < 0) {
+ list_ok = false;
+ break;
+ }
+ }
+
+ if (!aligned)
+ return omap_sham_copy_sgs(rctx, sg, bs, new_len);
+ else if (!list_ok)
+ return omap_sham_copy_sg_lists(rctx, sg, bs, new_len);
+
+ rctx->sg_len = n;
+ rctx->sg = sg;
+
+ return 0;
+}
+
+static int omap_sham_prepare_request(struct ahash_request *req, bool update)
+{
+ struct omap_sham_reqctx *rctx = ahash_request_ctx(req);
+ int bs;
+ int ret;
+ int nbytes;
+ bool final = rctx->flags & BIT(FLAGS_FINUP);
+ int xmit_len, hash_later;
+
+ if (!req)
+ return 0;
+
+ bs = get_block_size(rctx);
+
+ if (update)
+ nbytes = req->nbytes;
+ else
+ nbytes = 0;
+
+ rctx->total = nbytes + rctx->bufcnt;
+
+ if (!rctx->total)
+ return 0;
+
+ if (nbytes && (!IS_ALIGNED(rctx->bufcnt, bs))) {
+ int len = bs - rctx->bufcnt % bs;
+
+ if (len > nbytes)
+ len = nbytes;
+ scatterwalk_map_and_copy(rctx->buffer + rctx->bufcnt, req->src,
+ 0, len, 0);
+ rctx->bufcnt += len;
+ nbytes -= len;
+ rctx->offset = len;
+ }
+
+ if (rctx->bufcnt)
+ memcpy(rctx->dd->xmit_buf, rctx->buffer, rctx->bufcnt);
+
+ ret = omap_sham_align_sgs(req->src, nbytes, bs, final, rctx);
+ if (ret)
+ return ret;
+
+ xmit_len = rctx->total;
+
+ if (!IS_ALIGNED(xmit_len, bs)) {
+ if (final)
+ xmit_len = DIV_ROUND_UP(xmit_len, bs) * bs;
+ else
+ xmit_len = xmit_len / bs * bs;
+ }
+
+ hash_later = rctx->total - xmit_len;
+ if (hash_later < 0)
+ hash_later = 0;
+
+ if (rctx->bufcnt && nbytes) {
+ /* have data from previous operation and current */
+ sg_init_table(rctx->sgl, 2);
+ sg_set_buf(rctx->sgl, rctx->dd->xmit_buf, rctx->bufcnt);
+
+ sg_chain(rctx->sgl, 2, req->src);
+
+ rctx->sg = rctx->sgl;
+
+ rctx->sg_len++;
+ } else if (rctx->bufcnt) {
+ /* have buffered data only */
+ sg_init_table(rctx->sgl, 1);
+ sg_set_buf(rctx->sgl, rctx->dd->xmit_buf, xmit_len);
+
+ rctx->sg = rctx->sgl;
+
+ rctx->sg_len = 1;
+ }
+
+ if (hash_later) {
+ if (req->nbytes) {
+ scatterwalk_map_and_copy(rctx->buffer, req->src,
+ req->nbytes - hash_later,
+ hash_later, 0);
+ } else {
+ memcpy(rctx->buffer, rctx->buffer + xmit_len,
+ hash_later);
+ }
+ rctx->bufcnt = hash_later;
+ } else {
+ rctx->bufcnt = 0;
+ }
+
+ if (!final)
+ rctx->total = xmit_len;
+
+ return 0;
+}
+
static size_t omap_sham_append_buffer(struct omap_sham_reqctx *ctx,
const u8 *data, size_t length)
{
@@ -1040,6 +1299,10 @@ retry:
dd->req = req;
ctx = ahash_request_ctx(req);
+ err = omap_sham_prepare_request(NULL, ctx->op == OP_UPDATE);
+ if (err)
+ goto err1;
+
dev_dbg(dd->dev, "handling new req, op: %lu, nbytes: %d\n",
ctx->op, req->nbytes);
--
1.9.1
^ permalink raw reply related
* [PATCH 5/8] crypto: omap-sham: change the DMA threshold value to a define
From: Tero Kristo @ 2016-09-19 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474298539-23897-1-git-send-email-t-kristo@ti.com>
Currently the threshold value was hardcoded in the driver. Having a define
for it makes it easier to configure.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
drivers/crypto/omap-sham.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 8558989..5c95bf9 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -137,6 +137,7 @@
#define OMAP_ALIGNED __attribute__((aligned(sizeof(u32))))
#define BUFLEN PAGE_SIZE
+#define OMAP_SHA_DMA_THRESHOLD 256
struct omap_sham_dev;
@@ -1435,10 +1436,11 @@ static int omap_sham_final(struct ahash_request *req)
/*
* OMAP HW accel works only with buffers >= 9.
* HMAC is always >= 9 because ipad == block size.
- * If buffersize is less than 240, we use fallback SW encoding,
- * as using DMA + HW in this case doesn't provide any benefit.
+ * If buffersize is less than DMA_THRESHOLD, we use fallback
+ * SW encoding, as using DMA + HW in this case doesn't provide
+ * any benefit.
*/
- if (!ctx->digcnt && ctx->bufcnt < 240)
+ if (!ctx->digcnt && ctx->bufcnt < OMAP_SHA_DMA_THRESHOLD)
return omap_sham_final_shash(req);
else if (ctx->bufcnt)
return omap_sham_enqueue(req, OP_FINAL);
--
1.9.1
^ permalink raw reply related
* [PATCH 6/8] crypto: omap-sham: convert driver logic to use sgs for data xmit
From: Tero Kristo @ 2016-09-19 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474298539-23897-1-git-send-email-t-kristo@ti.com>
Currently, the internal buffer has been used for data transmission. Change
this so that scatterlists are used instead, and change the driver to
actually use the previously introduced helper functions for scatterlist
preparation.
This patch also removes the old buffer handling code which is no longer
needed.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
drivers/crypto/omap-sham.c | 344 ++++++++++-----------------------------------
1 file changed, 74 insertions(+), 270 deletions(-)
diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 5c95bf9..412559e 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -116,7 +116,6 @@
#define FLAGS_SGS_ALLOCED 10
/* context flags */
#define FLAGS_FINUP 16
-#define FLAGS_SG 17
#define FLAGS_MODE_SHIFT 18
#define FLAGS_MODE_MASK (SHA_REG_MODE_ALGO_MASK << FLAGS_MODE_SHIFT)
@@ -150,13 +149,11 @@ struct omap_sham_reqctx {
size_t digcnt;
size_t bufcnt;
size_t buflen;
- dma_addr_t dma_addr;
/* walk state */
struct scatterlist *sg;
struct scatterlist sgl[2];
- struct scatterlist sgl_tmp;
- unsigned int offset; /* offset in current sg */
+ int offset; /* offset in current sg */
int sg_len;
unsigned int total; /* total request */
@@ -516,12 +513,14 @@ static int omap_sham_poll_irq_omap4(struct omap_sham_dev *dd)
SHA_REG_IRQSTATUS_INPUT_RDY);
}
-static int omap_sham_xmit_cpu(struct omap_sham_dev *dd, const u8 *buf,
- size_t length, int final)
+static int omap_sham_xmit_cpu(struct omap_sham_dev *dd, size_t length,
+ int final)
{
struct omap_sham_reqctx *ctx = ahash_request_ctx(dd->req);
int count, len32, bs32, offset = 0;
- const u32 *buffer = (const u32 *)buf;
+ const u32 *buffer;
+ int mlen;
+ struct sg_mapping_iter mi;
dev_dbg(dd->dev, "xmit_cpu: digcnt: %d, length: %d, final: %d\n",
ctx->digcnt, length, final);
@@ -531,6 +530,7 @@ static int omap_sham_xmit_cpu(struct omap_sham_dev *dd, const u8 *buf,
/* should be non-zero before next lines to disable clocks later */
ctx->digcnt += length;
+ ctx->total -= length;
if (final)
set_bit(FLAGS_FINAL, &dd->flags); /* catch last interrupt */
@@ -540,16 +540,35 @@ static int omap_sham_xmit_cpu(struct omap_sham_dev *dd, const u8 *buf,
len32 = DIV_ROUND_UP(length, sizeof(u32));
bs32 = get_block_size(ctx) / sizeof(u32);
+ sg_miter_start(&mi, ctx->sg, ctx->sg_len,
+ SG_MITER_FROM_SG | SG_MITER_ATOMIC);
+
+ mlen = 0;
+
while (len32) {
if (dd->pdata->poll_irq(dd))
return -ETIMEDOUT;
- for (count = 0; count < min(len32, bs32); count++, offset++)
+ for (count = 0; count < min(len32, bs32); count++, offset++) {
+ if (!mlen) {
+ sg_miter_next(&mi);
+ mlen = mi.length;
+ if (!mlen) {
+ pr_err("sg miter failure.\n");
+ return -EINVAL;
+ }
+ offset = 0;
+ buffer = mi.addr;
+ }
omap_sham_write(dd, SHA_REG_DIN(dd, count),
buffer[offset]);
+ mlen -= 4;
+ }
len32 -= min(len32, bs32);
}
+ sg_miter_stop(&mi);
+
return -EINPROGRESS;
}
@@ -561,22 +580,27 @@ static void omap_sham_dma_callback(void *param)
tasklet_schedule(&dd->done_task);
}
-static int omap_sham_xmit_dma(struct omap_sham_dev *dd, dma_addr_t dma_addr,
- size_t length, int final, int is_sg)
+static int omap_sham_xmit_dma(struct omap_sham_dev *dd, size_t length,
+ int final)
{
struct omap_sham_reqctx *ctx = ahash_request_ctx(dd->req);
struct dma_async_tx_descriptor *tx;
struct dma_slave_config cfg;
- int len32, ret, dma_min = get_block_size(ctx);
+ int ret;
dev_dbg(dd->dev, "xmit_dma: digcnt: %d, length: %d, final: %d\n",
ctx->digcnt, length, final);
+ if (!dma_map_sg(dd->dev, ctx->sg, ctx->sg_len, DMA_TO_DEVICE)) {
+ dev_err(dd->dev, "dma_map_sg error\n");
+ return -EINVAL;
+ }
+
memset(&cfg, 0, sizeof(cfg));
cfg.dst_addr = dd->phys_base + SHA_REG_DIN(dd, 0);
cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
- cfg.dst_maxburst = dma_min / DMA_SLAVE_BUSWIDTH_4_BYTES;
+ cfg.dst_maxburst = get_block_size(ctx) / DMA_SLAVE_BUSWIDTH_4_BYTES;
ret = dmaengine_slave_config(dd->dma_lch, &cfg);
if (ret) {
@@ -584,31 +608,12 @@ static int omap_sham_xmit_dma(struct omap_sham_dev *dd, dma_addr_t dma_addr,
return ret;
}
- len32 = DIV_ROUND_UP(length, dma_min) * dma_min;
-
- if (is_sg) {
- /*
- * The SG entry passed in may not have the 'length' member
- * set correctly so use a local SG entry (sgl_tmp) with the
- * proper value for 'length' instead. If this is not done,
- * the dmaengine may try to DMA the incorrect amount of data.
- */
- sg_init_table(&ctx->sgl_tmp, 1);
- sg_assign_page(&ctx->sgl_tmp, sg_page(ctx->sg));
- ctx->sgl_tmp.offset = ctx->sg->offset;
- sg_dma_len(&ctx->sgl_tmp) = len32;
- sg_dma_address(&ctx->sgl_tmp) = sg_dma_address(ctx->sg);
-
- tx = dmaengine_prep_slave_sg(dd->dma_lch, &ctx->sgl_tmp, 1,
- DMA_MEM_TO_DEV,
- DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
- } else {
- tx = dmaengine_prep_slave_single(dd->dma_lch, dma_addr, len32,
- DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
- }
+ tx = dmaengine_prep_slave_sg(dd->dma_lch, ctx->sg, ctx->sg_len,
+ DMA_MEM_TO_DEV,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!tx) {
- dev_err(dd->dev, "prep_slave_sg/single() failed\n");
+ dev_err(dd->dev, "prep_slave_sg failed\n");
return -EINVAL;
}
@@ -618,6 +623,7 @@ static int omap_sham_xmit_dma(struct omap_sham_dev *dd, dma_addr_t dma_addr,
dd->pdata->write_ctrl(dd, length, final, 1);
ctx->digcnt += length;
+ ctx->total -= length;
if (final)
set_bit(FLAGS_FINAL, &dd->flags); /* catch last interrupt */
@@ -886,208 +892,13 @@ static int omap_sham_prepare_request(struct ahash_request *req, bool update)
return 0;
}
-static size_t omap_sham_append_buffer(struct omap_sham_reqctx *ctx,
- const u8 *data, size_t length)
-{
- size_t count = min(length, ctx->buflen - ctx->bufcnt);
-
- count = min(count, ctx->total);
- if (count <= 0)
- return 0;
- memcpy(ctx->buffer + ctx->bufcnt, data, count);
- ctx->bufcnt += count;
-
- return count;
-}
-
-static size_t omap_sham_append_sg(struct omap_sham_reqctx *ctx)
-{
- size_t count;
- const u8 *vaddr;
-
- while (ctx->sg) {
- vaddr = kmap_atomic(sg_page(ctx->sg));
- vaddr += ctx->sg->offset;
-
- count = omap_sham_append_buffer(ctx,
- vaddr + ctx->offset,
- ctx->sg->length - ctx->offset);
-
- kunmap_atomic((void *)vaddr);
-
- if (!count)
- break;
- ctx->offset += count;
- ctx->total -= count;
- if (ctx->offset == ctx->sg->length) {
- ctx->sg = sg_next(ctx->sg);
- if (ctx->sg)
- ctx->offset = 0;
- else
- ctx->total = 0;
- }
- }
-
- return 0;
-}
-
-static int omap_sham_xmit_dma_map(struct omap_sham_dev *dd,
- struct omap_sham_reqctx *ctx,
- size_t length, int final)
-{
- int ret;
-
- ctx->dma_addr = dma_map_single(dd->dev, ctx->buffer, ctx->buflen,
- DMA_TO_DEVICE);
- if (dma_mapping_error(dd->dev, ctx->dma_addr)) {
- dev_err(dd->dev, "dma %u bytes error\n", ctx->buflen);
- return -EINVAL;
- }
-
- ctx->flags &= ~BIT(FLAGS_SG);
-
- ret = omap_sham_xmit_dma(dd, ctx->dma_addr, length, final, 0);
- if (ret != -EINPROGRESS)
- dma_unmap_single(dd->dev, ctx->dma_addr, ctx->buflen,
- DMA_TO_DEVICE);
-
- return ret;
-}
-
-static int omap_sham_update_dma_slow(struct omap_sham_dev *dd)
-{
- struct omap_sham_reqctx *ctx = ahash_request_ctx(dd->req);
- unsigned int final;
- size_t count;
-
- omap_sham_append_sg(ctx);
-
- final = (ctx->flags & BIT(FLAGS_FINUP)) && !ctx->total;
-
- dev_dbg(dd->dev, "slow: bufcnt: %u, digcnt: %d, final: %d\n",
- ctx->bufcnt, ctx->digcnt, final);
-
- if (final || (ctx->bufcnt == ctx->buflen && ctx->total)) {
- count = ctx->bufcnt;
- ctx->bufcnt = 0;
- return omap_sham_xmit_dma_map(dd, ctx, count, final);
- }
-
- return 0;
-}
-
-/* Start address alignment */
-#define SG_AA(sg) (IS_ALIGNED(sg->offset, sizeof(u32)))
-/* SHA1 block size alignment */
-#define SG_SA(sg, bs) (IS_ALIGNED(sg->length, bs))
-
-static int omap_sham_update_dma_start(struct omap_sham_dev *dd)
-{
- struct omap_sham_reqctx *ctx = ahash_request_ctx(dd->req);
- unsigned int length, final, tail;
- struct scatterlist *sg;
- int ret, bs;
-
- if (!ctx->total)
- return 0;
-
- if (ctx->bufcnt || ctx->offset)
- return omap_sham_update_dma_slow(dd);
-
- /*
- * Don't use the sg interface when the transfer size is less
- * than the number of elements in a DMA frame. Otherwise,
- * the dmaengine infrastructure will calculate that it needs
- * to transfer 0 frames which ultimately fails.
- */
- if (ctx->total < get_block_size(ctx))
- return omap_sham_update_dma_slow(dd);
-
- dev_dbg(dd->dev, "fast: digcnt: %d, bufcnt: %u, total: %u\n",
- ctx->digcnt, ctx->bufcnt, ctx->total);
-
- sg = ctx->sg;
- bs = get_block_size(ctx);
-
- if (!SG_AA(sg))
- return omap_sham_update_dma_slow(dd);
-
- if (!sg_is_last(sg) && !SG_SA(sg, bs))
- /* size is not BLOCK_SIZE aligned */
- return omap_sham_update_dma_slow(dd);
-
- length = min(ctx->total, sg->length);
-
- if (sg_is_last(sg)) {
- if (!(ctx->flags & BIT(FLAGS_FINUP))) {
- /* not last sg must be BLOCK_SIZE aligned */
- tail = length & (bs - 1);
- /* without finup() we need one block to close hash */
- if (!tail)
- tail = bs;
- length -= tail;
- }
- }
-
- if (!dma_map_sg(dd->dev, ctx->sg, 1, DMA_TO_DEVICE)) {
- dev_err(dd->dev, "dma_map_sg error\n");
- return -EINVAL;
- }
-
- ctx->flags |= BIT(FLAGS_SG);
-
- ctx->total -= length;
- ctx->offset = length; /* offset where to start slow */
-
- final = (ctx->flags & BIT(FLAGS_FINUP)) && !ctx->total;
-
- ret = omap_sham_xmit_dma(dd, sg_dma_address(ctx->sg), length, final, 1);
- if (ret != -EINPROGRESS)
- dma_unmap_sg(dd->dev, ctx->sg, 1, DMA_TO_DEVICE);
-
- return ret;
-}
-
-static int omap_sham_update_cpu(struct omap_sham_dev *dd)
-{
- struct omap_sham_reqctx *ctx = ahash_request_ctx(dd->req);
- int bufcnt, final;
-
- if (!ctx->total)
- return 0;
-
- omap_sham_append_sg(ctx);
-
- final = (ctx->flags & BIT(FLAGS_FINUP)) && !ctx->total;
-
- dev_dbg(dd->dev, "cpu: bufcnt: %u, digcnt: %d, final: %d\n",
- ctx->bufcnt, ctx->digcnt, final);
-
- if (final || (ctx->bufcnt == ctx->buflen && ctx->total)) {
- bufcnt = ctx->bufcnt;
- ctx->bufcnt = 0;
- return omap_sham_xmit_cpu(dd, ctx->buffer, bufcnt, final);
- }
-
- return 0;
-}
-
static int omap_sham_update_dma_stop(struct omap_sham_dev *dd)
{
struct omap_sham_reqctx *ctx = ahash_request_ctx(dd->req);
+ dma_unmap_sg(dd->dev, ctx->sg, ctx->sg_len, DMA_TO_DEVICE);
- if (ctx->flags & BIT(FLAGS_SG)) {
- dma_unmap_sg(dd->dev, ctx->sg, 1, DMA_TO_DEVICE);
- if (ctx->sg->length == ctx->offset) {
- ctx->sg = sg_next(ctx->sg);
- if (ctx->sg)
- ctx->offset = 0;
- }
- } else {
- dma_unmap_single(dd->dev, ctx->dma_addr, ctx->buflen,
- DMA_TO_DEVICE);
- }
+ clear_bit(FLAGS_DMA_ACTIVE, &dd->flags);
return 0;
}
@@ -1148,6 +959,8 @@ static int omap_sham_init(struct ahash_request *req)
ctx->bufcnt = 0;
ctx->digcnt = 0;
+ ctx->total = 0;
+ ctx->offset = 0;
ctx->buflen = BUFLEN;
if (tctx->flags & BIT(FLAGS_HMAC)) {
@@ -1170,14 +983,19 @@ static int omap_sham_update_req(struct omap_sham_dev *dd)
struct ahash_request *req = dd->req;
struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
int err;
+ bool final = ctx->flags & BIT(FLAGS_FINUP);
dev_dbg(dd->dev, "update_req: total: %u, digcnt: %d, finup: %d\n",
ctx->total, ctx->digcnt, (ctx->flags & BIT(FLAGS_FINUP)) != 0);
+ if (ctx->total < get_block_size(ctx) ||
+ ctx->total < OMAP_SHA_DMA_THRESHOLD)
+ ctx->flags |= BIT(FLAGS_CPU);
+
if (ctx->flags & BIT(FLAGS_CPU))
- err = omap_sham_update_cpu(dd);
+ err = omap_sham_xmit_cpu(dd, ctx->total, final);
else
- err = omap_sham_update_dma_start(dd);
+ err = omap_sham_xmit_dma(dd, ctx->total, final);
/* wait for dma completion before can take more data */
dev_dbg(dd->dev, "update: err: %d, digcnt: %d\n", err, ctx->digcnt);
@@ -1191,7 +1009,7 @@ static int omap_sham_final_req(struct omap_sham_dev *dd)
struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
int err = 0, use_dma = 1;
- if ((ctx->bufcnt <= get_block_size(ctx)) || dd->polling_mode)
+ if ((ctx->total <= get_block_size(ctx)) || dd->polling_mode)
/*
* faster to handle last block with cpu or
* use cpu when dma is not present.
@@ -1199,9 +1017,9 @@ static int omap_sham_final_req(struct omap_sham_dev *dd)
use_dma = 0;
if (use_dma)
- err = omap_sham_xmit_dma_map(dd, ctx, ctx->bufcnt, 1);
+ err = omap_sham_xmit_dma(dd, ctx->total, 1);
else
- err = omap_sham_xmit_cpu(dd, ctx->buffer, ctx->bufcnt, 1);
+ err = omap_sham_xmit_cpu(dd, ctx->total, 1);
ctx->bufcnt = 0;
@@ -1249,6 +1067,17 @@ static void omap_sham_finish_req(struct ahash_request *req, int err)
struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
struct omap_sham_dev *dd = ctx->dd;
+ if (test_bit(FLAGS_SGS_COPIED, &dd->flags))
+ free_pages((unsigned long)sg_virt(ctx->sg),
+ get_order(ctx->sg->length));
+
+ if (test_bit(FLAGS_SGS_ALLOCED, &dd->flags))
+ kfree(ctx->sg);
+
+ ctx->sg = NULL;
+
+ dd->flags &= ~(BIT(FLAGS_SGS_ALLOCED) | BIT(FLAGS_SGS_COPIED));
+
if (!err) {
dd->pdata->copy_hash(req, 1);
if (test_bit(FLAGS_FINAL, &dd->flags))
@@ -1300,7 +1129,7 @@ retry:
dd->req = req;
ctx = ahash_request_ctx(req);
- err = omap_sham_prepare_request(NULL, ctx->op == OP_UPDATE);
+ err = omap_sham_prepare_request(req, ctx->op == OP_UPDATE);
if (err)
goto err1;
@@ -1356,34 +1185,15 @@ static int omap_sham_update(struct ahash_request *req)
{
struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
struct omap_sham_dev *dd = ctx->dd;
- int bs = get_block_size(ctx);
if (!req->nbytes)
return 0;
- ctx->total = req->nbytes;
- ctx->sg = req->src;
- ctx->offset = 0;
-
- if (ctx->flags & BIT(FLAGS_FINUP)) {
- if ((ctx->digcnt + ctx->bufcnt + ctx->total) < 240) {
- /*
- * OMAP HW accel works only with buffers >= 9
- * will switch to bypass in final()
- * final has the same request and data
- */
- omap_sham_append_sg(ctx);
- return 0;
- } else if ((ctx->bufcnt + ctx->total <= bs) ||
- dd->polling_mode) {
- /*
- * faster to use CPU for short transfers or
- * use cpu when dma is not present.
- */
- ctx->flags |= BIT(FLAGS_CPU);
- }
- } else if (ctx->bufcnt + ctx->total < ctx->buflen) {
- omap_sham_append_sg(ctx);
+ if (ctx->total + req->nbytes < ctx->buflen) {
+ scatterwalk_map_and_copy(ctx->buffer + ctx->bufcnt, req->src,
+ 0, req->nbytes, 0);
+ ctx->bufcnt += req->nbytes;
+ ctx->total += req->nbytes;
return 0;
}
@@ -1917,12 +1727,8 @@ static void omap_sham_done_task(unsigned long data)
}
if (test_bit(FLAGS_CPU, &dd->flags)) {
- if (test_and_clear_bit(FLAGS_OUTPUT_READY, &dd->flags)) {
- /* hash or semi-hash ready */
- err = omap_sham_update_cpu(dd);
- if (err != -EINPROGRESS)
- goto finish;
- }
+ if (test_and_clear_bit(FLAGS_OUTPUT_READY, &dd->flags))
+ goto finish;
} else if (test_bit(FLAGS_DMA_READY, &dd->flags)) {
if (test_and_clear_bit(FLAGS_DMA_ACTIVE, &dd->flags)) {
omap_sham_update_dma_stop(dd);
@@ -1934,8 +1740,6 @@ static void omap_sham_done_task(unsigned long data)
if (test_and_clear_bit(FLAGS_OUTPUT_READY, &dd->flags)) {
/* hash or semi-hash ready */
clear_bit(FLAGS_DMA_READY, &dd->flags);
- err = omap_sham_update_dma_start(dd);
- if (err != -EINPROGRESS)
goto finish;
}
}
--
1.9.1
^ permalink raw reply related
* [PATCH 7/8] crypto: omap-sham: add support for export/import
From: Tero Kristo @ 2016-09-19 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474298539-23897-1-git-send-email-t-kristo@ti.com>
Now that the driver has been converted to use scatterlists for data
handling, add proper implementation for the export/import stubs also.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
drivers/crypto/omap-sham.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 412559e..8eefd79 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -1418,12 +1418,21 @@ static void omap_sham_cra_exit(struct crypto_tfm *tfm)
static int omap_sham_export(struct ahash_request *req, void *out)
{
- return -ENOTSUPP;
+ struct omap_sham_reqctx *rctx = ahash_request_ctx(req);
+
+ memcpy(out, rctx, sizeof(*rctx) + rctx->bufcnt);
+
+ return 0;
}
static int omap_sham_import(struct ahash_request *req, const void *in)
{
- return -ENOTSUPP;
+ struct omap_sham_reqctx *rctx = ahash_request_ctx(req);
+ const struct omap_sham_reqctx *ctx_in = in;
+
+ memcpy(rctx, in, sizeof(*rctx) + ctx_in->bufcnt);
+
+ return 0;
}
static struct ahash_alg algs_sha1_md5[] = {
@@ -2083,7 +2092,8 @@ static int omap_sham_probe(struct platform_device *pdev)
alg = &dd->pdata->algs_info[i].algs_list[j];
alg->export = omap_sham_export;
alg->import = omap_sham_import;
- alg->halg.statesize = sizeof(struct omap_sham_reqctx);
+ alg->halg.statesize = sizeof(struct omap_sham_reqctx) +
+ BUFLEN;
err = crypto_register_ahash(alg);
if (err)
goto err_algs;
--
1.9.1
^ permalink raw reply related
* [PATCH 8/8] crypto: omap-sham: shrink the internal buffer size
From: Tero Kristo @ 2016-09-19 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474298539-23897-1-git-send-email-t-kristo@ti.com>
The current internal buffer size is way too large for crypto core, so
shrink it to be smaller. This makes the buffer to fit into the space
reserved for the export/import buffers also.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
drivers/crypto/omap-sham.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 8eefd79..d0b16e5 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -135,7 +135,7 @@
#define OMAP_ALIGN_MASK (sizeof(u32)-1)
#define OMAP_ALIGNED __attribute__((aligned(sizeof(u32))))
-#define BUFLEN PAGE_SIZE
+#define BUFLEN SHA512_BLOCK_SIZE
#define OMAP_SHA_DMA_THRESHOLD 256
struct omap_sham_dev;
--
1.9.1
^ permalink raw reply related
* [PATCH v3 2/8] scpi: Add alternative legacy structures, functions and macros
From: Sudeep Holla @ 2016-09-19 15:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473262477-18045-3-git-send-email-narmstrong@baylibre.com>
On 07/09/16 16:34, Neil Armstrong wrote:
> In order to support the legacy SCPI protocol variant, add back the structures
> and macros that varies against the final specification.
> Add indirection table for legacy commands.
> Add bitmap field for channel selection
> Add support for legacy in scpi_send_message.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> drivers/firmware/arm_scpi.c | 218 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 211 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
> index 9a87687..9ba1020 100644
> --- a/drivers/firmware/arm_scpi.c
> +++ b/drivers/firmware/arm_scpi.c
[..]
> @@ -336,6 +424,39 @@ static void scpi_handle_remote_msg(struct mbox_client *c, void *msg)
> scpi_process_cmd(ch, cmd);
> }
>
> +static void legacy_scpi_process_cmd(struct scpi_chan *ch)
> +{
> + unsigned long flags;
> + struct scpi_xfer *t;
> +
> + spin_lock_irqsave(&ch->rx_lock, flags);
> + if (list_empty(&ch->rx_pending)) {
> + spin_unlock_irqrestore(&ch->rx_lock, flags);
> + return;
> + }
> +
> + t = list_first_entry(&ch->rx_pending, struct scpi_xfer, node);
> + list_del(&t->node);
> +
This is a bad assumption that it will be always first. The legacy SCPI
did support multiple commands at a time and they can be reordered when
SCP responds to them. Except this it's almost same scpi_process_cmd. You
should be able to use it as is if you pass the command.
> + /* check if wait_for_completion is in progress or timed-out */
> + if (t && !completion_done(&t->done)) {
> + struct legacy_scpi_shared_mem *mem = ch->rx_payload;
> + unsigned int len = t->rx_len;
> +
> + t->status = le32_to_cpu(mem->status);
> + memcpy_fromio(t->rx_buf, mem->payload, len);
> + complete(&t->done);
> + }
> + spin_unlock_irqrestore(&ch->rx_lock, flags);
> +}
> +
> +static void legacy_scpi_handle_remote_msg(struct mbox_client *c, void *_msg)
> +{
> + struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
> +
> + legacy_scpi_process_cmd(ch);
You will get the command in *_msg IIRC. So you can just pass that to
scpi_process_cmd. You can even reuse scpi_handle_remote_msg
diff --git i/drivers/firmware/arm_scpi.c w/drivers/firmware/arm_scpi.c
index edf1a3327041..165f2fc3b627 100644
--- i/drivers/firmware/arm_scpi.c
+++ w/drivers/firmware/arm_scpi.c
@@ -419,7 +419,12 @@ static void scpi_handle_remote_msg(struct
mbox_client *c, void *msg)
{
struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
struct scpi_shared_mem *mem = ch->rx_payload;
- u32 cmd = le32_to_cpu(mem->command);
+ u32 cmd;
+
+ if (ch->is_legacy)
+ cmd = *(u32 *)msg;
+ else
+ cmd = le32_to_cpu(mem->command);
scpi_process_cmd(ch, cmd);
}
> +}
> +
> static void scpi_tx_prepare(struct mbox_client *c, void *msg)
> {
> unsigned long flags;
> @@ -356,6 +477,21 @@ static void scpi_tx_prepare(struct mbox_client *c, void *msg)
> mem->command = cpu_to_le32(t->cmd);
> }
>
> +static void legacy_scpi_tx_prepare(struct mbox_client *c, void *msg)
> +{
> + unsigned long flags;
> + struct scpi_xfer *t = msg;
> + struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
> +
> + if (t->tx_buf)
> + memcpy_toio(ch->tx_payload, t->tx_buf, t->tx_len);
> + if (t->rx_buf) {
> + spin_lock_irqsave(&ch->rx_lock, flags);
> + list_add_tail(&t->node, &ch->rx_pending);
> + spin_unlock_irqrestore(&ch->rx_lock, flags);
> + }
> +}
Again here the only difference is token addition. I think we should
retain that as it's helpful in debugging and I don't think it will have
any issues. Worst case we can make it conditional but let's check if we
can retain it first.
> @@ -386,15 +522,25 @@ static int scpi_send_message(u8 cmd, void *tx_buf, unsigned int tx_len,
> struct scpi_xfer *msg;
> struct scpi_chan *scpi_chan;
>
> - chan = atomic_inc_return(&scpi_info->next_chan) % scpi_info->num_chans;
> + if (scpi_info->is_legacy)
> + chan = test_bit(cmd, scpi_info->cmd_priority) ? 1 : 0;
> + else
> + chan = atomic_inc_return(&scpi_info->next_chan) %
> + scpi_info->num_chans;
> scpi_chan = scpi_info->channels + chan;
>
> msg = get_scpi_xfer(scpi_chan);
> if (!msg)
> return -ENOMEM;
>
> - msg->slot = BIT(SCPI_SLOT);
> - msg->cmd = PACK_SCPI_CMD(cmd, tx_len);
> + if (scpi_info->is_legacy) {
> + mutex_lock(&scpi_chan->xfers_lock);
Why does legacy need a different locking scheme ?
[...]
> @@ -635,6 +804,24 @@ static int scpi_sensor_get_value(u16 sensor, u64 *val)
> return ret;
> }
>
> +static int legacy_scpi_sensor_get_value(u16 sensor, u64 *val)
> +{
> + __le16 id = cpu_to_le16(sensor);
> + struct sensor_value buf;
> + int ret;
> +
> + ret = check_cmd(CMD_SENSOR_VALUE);
> + if (ret)
> + return ret;
> +
> + ret = scpi_send_message(scpi_info->scpi_cmds[CMD_SENSOR_VALUE],
> + &id, sizeof(id), &buf, sizeof(buf));
> + if (!ret)
> + *val = (u64)le32_to_cpu(buf.lo_val);
> +
This is not needed as it's backward compatible as discussed before.
Any particular reason you retained it here ?
--
Regards,
Sudeep
^ permalink raw reply related
* [PATCH 1/3] i2c: bcm2835: Fix hang for writing messages larger than 16 bytes
From: Noralf Trønnes @ 2016-09-19 15:26 UTC (permalink / raw)
To: linux-arm-kernel
Writing messages larger than the FIFO size results in a hang, rendering
the machine unusable. This is because the RXD status flag is set on the
first interrupt which results in bcm2835_drain_rxfifo() stealing bytes
from the buffer. The controller continues to trigger interrupts waiting
for the missing bytes, but bcm2835_fill_txfifo() has none to give.
In this situation wait_for_completion_timeout() apparently is unable to
stop the madness.
The BCM2835 ARM Peripherals datasheet has this to say about the flags:
TXD: is set when the FIFO has space for at least one byte of data.
RXD: is set when the FIFO contains at least one byte of data.
TXW: is set during a write transfer and the FIFO is less than full.
RXR: is set during a read transfer and the FIFO is or more full.
Implementing the logic from the downstream i2c-bcm2708 driver solved
the hang problem.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
---
drivers/i2c/busses/i2c-bcm2835.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index d4f3239..f283b71 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -64,6 +64,7 @@ struct bcm2835_i2c_dev {
int irq;
struct i2c_adapter adapter;
struct completion completion;
+ struct i2c_msg *curr_msg;
u32 msg_err;
u8 *msg_buf;
size_t msg_buf_remaining;
@@ -126,14 +127,13 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
return IRQ_HANDLED;
}
- if (val & BCM2835_I2C_S_RXD) {
- bcm2835_drain_rxfifo(i2c_dev);
- if (!(val & BCM2835_I2C_S_DONE))
- return IRQ_HANDLED;
- }
-
if (val & BCM2835_I2C_S_DONE) {
- if (i2c_dev->msg_buf_remaining)
+ if (i2c_dev->curr_msg->flags & I2C_M_RD) {
+ bcm2835_drain_rxfifo(i2c_dev);
+ val = bcm2835_i2c_readl(i2c_dev, BCM2835_I2C_S);
+ }
+
+ if ((val & BCM2835_I2C_S_RXD) || i2c_dev->msg_buf_remaining)
i2c_dev->msg_err = BCM2835_I2C_S_LEN;
else
i2c_dev->msg_err = 0;
@@ -141,11 +141,16 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
return IRQ_HANDLED;
}
- if (val & BCM2835_I2C_S_TXD) {
+ if (val & BCM2835_I2C_S_TXW) {
bcm2835_fill_txfifo(i2c_dev);
return IRQ_HANDLED;
}
+ if (val & BCM2835_I2C_S_RXR) {
+ bcm2835_drain_rxfifo(i2c_dev);
+ return IRQ_HANDLED;
+ }
+
return IRQ_NONE;
}
@@ -155,6 +160,7 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
u32 c;
unsigned long time_left;
+ i2c_dev->curr_msg = msg;
i2c_dev->msg_buf = msg->buf;
i2c_dev->msg_buf_remaining = msg->len;
reinit_completion(&i2c_dev->completion);
--
2.8.2
^ permalink raw reply related
* [PATCH 2/3] i2c: bcm2835: Add support for combined write-read transfer
From: Noralf Trønnes @ 2016-09-19 15:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474298777-5858-1-git-send-email-noralf@tronnes.org>
Some SMBus protocols use Repeated Start Condition to switch from write
mode to read mode. Devices like MMA8451 won't work without it.
When downstream implemented support for this in i2c-bcm2708, it broke
support for some devices, so a module parameter was added and combined
transfer was disabled by default.
See https://github.com/raspberrypi/linux/issues/599
It doesn't seem to have been any investigation into what the problem
really was. Later there was added a timeout on the polling loop.
One of the devices mentioned to partially stop working was DS1307.
I have run thousands of transfers to a DS1307 (rtc), MMA8451 (accel)
and AT24C32 (eeprom) in parallel without problems.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
---
drivers/i2c/busses/i2c-bcm2835.c | 107 +++++++++++++++++++++++++++++++++++----
1 file changed, 98 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index f283b71..b3ce565 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -154,8 +154,39 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
return IRQ_NONE;
}
+/*
+ * Repeated Start Condition (Sr)
+ * The BCM2835 ARM Peripherals datasheet mentions a way to trigger a Sr when it
+ * talks about reading from a slave with 10 bit address. This is achieved by
+ * issuing a write (without enabling interrupts), poll the I2CS.TA flag and
+ * wait for it to be set, and then issue a read.
+ * https://github.com/raspberrypi/linux/issues/254 shows how the firmware does
+ * it and states that it's a workaround for a problem in the state machine.
+ * This is the comment in the firmware code:
+ *
+ * The I2C peripheral samples the values for rw_bit and xfer_count in the
+ * IDLE state if start is set.
+ *
+ * We want to generate a ReSTART not a STOP at the end of the TX phase. In
+ * order to do that we must ensure the state machine goes
+ * RACK1 -> RACK2 -> SRSTRT1 (not RACK1 -> RACK2 -> SSTOP1).
+ *
+ * So, in the RACK2 state when (TX) xfer_count==0 we must therefore have
+ * already set, ready to be sampled:
+ * READ; rw_bit <= I2CC bit 0 - must be "read"
+ * ST; start <= I2CC bit 7 - must be "Go" in order to not issue STOP
+ * DLEN; xfer_count <= I2CDLEN - must be equal to our read amount
+ *
+ * The plan to do this is:
+ * 1. Start the sub-address write, but don't let it finish (keep
+ * xfer_count > 0)
+ * 2. Populate READ, DLEN and ST in preparation for ReSTART read sequence
+ * 3. Let TX finish (write the rest of the data)
+ * 4. Read back data as it arrives
+ */
+
static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
- struct i2c_msg *msg)
+ struct i2c_msg *msg, struct i2c_msg *msg2)
{
u32 c;
unsigned long time_left;
@@ -167,21 +198,70 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, BCM2835_I2C_C_CLEAR);
- if (msg->flags & I2C_M_RD) {
- c = BCM2835_I2C_C_READ | BCM2835_I2C_C_INTR;
- } else {
- c = BCM2835_I2C_C_INTT;
+ if (!(msg->flags & I2C_M_RD))
bcm2835_fill_txfifo(i2c_dev);
- }
- c |= BCM2835_I2C_C_ST | BCM2835_I2C_C_INTD | BCM2835_I2C_C_I2CEN;
bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_A, msg->addr);
bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DLEN, msg->len);
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, c);
+
+ if (!msg2) {
+ if (msg->flags & I2C_M_RD)
+ c = BCM2835_I2C_C_READ | BCM2835_I2C_C_INTR;
+ else
+ c = BCM2835_I2C_C_INTT;
+
+ c |= BCM2835_I2C_C_ST | BCM2835_I2C_C_INTD |
+ BCM2835_I2C_C_I2CEN;
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, c);
+ } else {
+ unsigned long flags;
+ u32 stat, err = 0;
+
+ local_irq_save(flags);
+
+ /* Start write message */
+ c = BCM2835_I2C_C_ST | BCM2835_I2C_C_I2CEN;
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, c);
+
+ /* Wait for the transfer to become active */
+ for (time_left = 100; time_left > 0; time_left--) {
+ stat = bcm2835_i2c_readl(i2c_dev, BCM2835_I2C_S);
+
+ err = stat & (BCM2835_I2C_S_CLKT | BCM2835_I2C_S_ERR);
+ if (err)
+ break;
+
+ if (stat & BCM2835_I2C_S_TA)
+ break;
+ }
+
+ if (err || !time_left) {
+ i2c_dev->msg_err = err;
+ local_irq_restore(flags);
+ goto error;
+ }
+
+ /* Start read message */
+ i2c_dev->curr_msg = msg2;
+ i2c_dev->msg_buf = msg2->buf;
+ i2c_dev->msg_buf_remaining = msg2->len;
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DLEN, msg2->len);
+
+ c = BCM2835_I2C_C_READ | BCM2835_I2C_C_INTR |
+ BCM2835_I2C_C_INTD | BCM2835_I2C_C_ST |
+ BCM2835_I2C_C_I2CEN;
+
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, c);
+
+ local_irq_restore(flags);
+ }
time_left = wait_for_completion_timeout(&i2c_dev->completion,
BCM2835_I2C_TIMEOUT);
+error:
bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, BCM2835_I2C_C_CLEAR);
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_S, BCM2835_I2C_S_CLKT |
+ BCM2835_I2C_S_ERR | BCM2835_I2C_S_DONE);
if (!time_left) {
dev_err(i2c_dev->dev, "i2c transfer timed out\n");
return -ETIMEDOUT;
@@ -209,8 +289,17 @@ static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
int i;
int ret = 0;
+ /* Combined write-read to the same address (smbus) */
+ if (num == 2 && (msgs[0].addr == msgs[1].addr) &&
+ !(msgs[0].flags & I2C_M_RD) && (msgs[1].flags & I2C_M_RD) &&
+ (msgs[0].len <= 16)) {
+ ret = bcm2835_i2c_xfer_msg(i2c_dev, &msgs[0], &msgs[1]);
+
+ return ret ? ret : 2;
+ }
+
for (i = 0; i < num; i++) {
- ret = bcm2835_i2c_xfer_msg(i2c_dev, &msgs[i]);
+ ret = bcm2835_i2c_xfer_msg(i2c_dev, &msgs[i], NULL);
if (ret)
break;
}
--
2.8.2
^ permalink raw reply related
* [PATCH 3/3] i2c: bcm2835: Use ratelimited logging on transfer errors
From: Noralf Trønnes @ 2016-09-19 15:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474298777-5858-1-git-send-email-noralf@tronnes.org>
Writing to an AT24C32 generates on average 2x i2c transfer errors per
32-byte page write. Which amounts to a lot for a 4k write. This is due
to the fact that the chip doesn't respond during it's internal write
cycle when the at24 driver tries and retries the next write.
Reduce this flooding of the log by using dev_err_ratelimited().
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
---
drivers/i2c/busses/i2c-bcm2835.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index b3ce565..dc0a5d5 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -274,7 +274,8 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
(msg->flags & I2C_M_IGNORE_NAK))
return 0;
- dev_err(i2c_dev->dev, "i2c transfer failed: %x\n", i2c_dev->msg_err);
+ dev_err_ratelimited(i2c_dev->dev, "i2c transfer failed: %x\n",
+ i2c_dev->msg_err);
if (i2c_dev->msg_err & BCM2835_I2C_S_ERR)
return -EREMOTEIO;
--
2.8.2
^ permalink raw reply related
* [GIT PULL] ARM: at91: dt for 4.9
From: Arnd Bergmann @ 2016-09-19 15:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160914082812.tmlrrqwkeudcybui@piout.net>
On Wednesday, September 14, 2016 10:28:12 AM CEST Alexandre Belloni wrote:
> DT Changes for 4.9:
> - add ETM and ETB nodes on sama5d2
>
Pulled into next/dt, thanks!
Arnd
^ 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