Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] arm64: fpsimd: improve stacking logic in non-interruptible context
From: Ard Biesheuvel @ 2016-12-08 15:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161208155051.GA35383@MBP.local>

On 8 December 2016 at 15:50, Catalin Marinas <catalin.marinas@arm.com> wrote:
> Hi Ard,
>
> On Wed, Dec 07, 2016 at 10:14:08AM +0000, Ard Biesheuvel wrote:
>>  void kernel_neon_begin_partial(u32 num_regs)
>>  {
>> -     if (in_interrupt()) {
>> -             struct fpsimd_partial_state *s = this_cpu_ptr(
>> -                     in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
>> +     struct fpsimd_partial_state *s;
>> +     int level;
>> +
>> +     preempt_disable();
>> +
>> +     level = this_cpu_read(kernel_neon_nesting_level);
>> +     BUG_ON(level > 2);
>> +
>> +     if (level > 0) {
>> +             s = this_cpu_ptr(nested_fpsimdstate);
>>
>> -             BUG_ON(num_regs > 32);
>> -             fpsimd_save_partial_state(s, roundup(num_regs, 2));
>> +             WARN_ON_ONCE(num_regs > 32);
>> +             num_regs = min(roundup(num_regs, 2), 32U);
>> +
>> +             fpsimd_save_partial_state(&s[level - 1], num_regs);
>>       } else {
>>               /*
>>                * Save the userland FPSIMD state if we have one and if we
>> @@ -241,24 +256,29 @@ void kernel_neon_begin_partial(u32 num_regs)
>>                * that there is no longer userland FPSIMD state in the
>>                * registers.
>>                */
>> -             preempt_disable();
>>               if (current->mm &&
>>                   !test_and_set_thread_flag(TIF_FOREIGN_FPSTATE))
>>                       fpsimd_save_state(&current->thread.fpsimd_state);
>>               this_cpu_write(fpsimd_last_state, NULL);
>>       }
>> +     this_cpu_write(kernel_neon_nesting_level, level + 1);
>>  }
>
> I'm slightly confused with the potential race with an interrupt here.
> Let's say the above is running in the process context, sets the
> TIF_FOREIGN_FPSTATE but is interrupted before fpsimd_save_state(). The
> interrupt handler calling kernel_neon_begin_partial() is seeing level 0
> and TIF_FOREIGN_FPSTATE and decides that it is safe to corrupt the Neon
> state without any further saving.
>
> I think the kernel_neon_nesting_level should be incremented early on in
> this function.
>

Good point, I hadn't considered that.

^ permalink raw reply

* [PATCH v2] arm64: fpsimd: improve stacking logic in non-interruptible context
From: Catalin Marinas @ 2016-12-08 15:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481105648-19775-1-git-send-email-ard.biesheuvel@linaro.org>

Hi Ard,

On Wed, Dec 07, 2016 at 10:14:08AM +0000, Ard Biesheuvel wrote:
>  void kernel_neon_begin_partial(u32 num_regs)
>  {
> -	if (in_interrupt()) {
> -		struct fpsimd_partial_state *s = this_cpu_ptr(
> -			in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
> +	struct fpsimd_partial_state *s;
> +	int level;
> +
> +	preempt_disable();
> +
> +	level = this_cpu_read(kernel_neon_nesting_level);
> +	BUG_ON(level > 2);
> +
> +	if (level > 0) {
> +		s = this_cpu_ptr(nested_fpsimdstate);
>  
> -		BUG_ON(num_regs > 32);
> -		fpsimd_save_partial_state(s, roundup(num_regs, 2));
> +		WARN_ON_ONCE(num_regs > 32);
> +		num_regs = min(roundup(num_regs, 2), 32U);
> +
> +		fpsimd_save_partial_state(&s[level - 1], num_regs);
>  	} else {
>  		/*
>  		 * Save the userland FPSIMD state if we have one and if we
> @@ -241,24 +256,29 @@ void kernel_neon_begin_partial(u32 num_regs)
>  		 * that there is no longer userland FPSIMD state in the
>  		 * registers.
>  		 */
> -		preempt_disable();
>  		if (current->mm &&
>  		    !test_and_set_thread_flag(TIF_FOREIGN_FPSTATE))
>  			fpsimd_save_state(&current->thread.fpsimd_state);
>  		this_cpu_write(fpsimd_last_state, NULL);
>  	}
> +	this_cpu_write(kernel_neon_nesting_level, level + 1);
>  }

I'm slightly confused with the potential race with an interrupt here.
Let's say the above is running in the process context, sets the
TIF_FOREIGN_FPSTATE but is interrupted before fpsimd_save_state(). The
interrupt handler calling kernel_neon_begin_partial() is seeing level 0
and TIF_FOREIGN_FPSTATE and decides that it is safe to corrupt the Neon
state without any further saving.

I think the kernel_neon_nesting_level should be incremented early on in
this function.

-- 
Catalin

^ permalink raw reply

* Tearing down DMA transfer setup after DMA client has finished
From: Vinod Koul @ 2016-12-08 15:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <yw1xshpyzk6p.fsf@unicorn.mansr.com>

On Thu, Dec 08, 2016 at 12:20:30PM +0000, M?ns Rullg?rd wrote:
> >
> > I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but
> > it does request DMA channels at open time, not at probe time.
> 
> In the part quoted above, I said most drivers request dma channels in
> their probe or open functions.  For the purposes of this discussion,
> that distinction is irrelevant.  In either case, the channel is held
> indefinitely.  

And the answer was it is wrong and not _all_ do that!!

> If this wasn't the correct way to use the dmaengine,
> there would be no need for the virt-dma helpers which are specifically
> designed for cases the one currently at hand.

That is incorrect.

virt-dma helps to have multiple request from various clients. For many
controllers which implement a SW mux, they can transfer data for one client
and then next transfer can be for some other one.

This allows better utilization of dma channels and helps in case where we
have fewer dma channels than users. This was _not_ developed to let people
grab channels in probe.

> The only problem we have is that nobody envisioned hardware where the
> dma engine indicates completion slightly too soon.  I suspect there's a
> fifo or such somewhere, and the interrupt is triggered when the last
> byte has been placed in the fifo rather than when it has been removed
> which would have been more correct.

That is pretty common hardware optimization but usually hardware shows up
with flush commands to let in flight transactions be completed.

One other example of this implementations has already been pointed in this
thread

-- 
~Vinod

^ permalink raw reply

* [PATCH] i2c: rk3x: keep i2c irq ON in suspend
From: David.Wu @ 2016-12-08 15:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e34bab0d-b4b1-ff0d-3a27-c3701b8206c1@ti.com>

Hi Grygorii, Doug and Heiko,

Thanks for your replies.
I will do 2 steps:

1. Add "suspended" flag in suspend_noirq()/resume_noirq() callback to 
prevent new i2c started, and use i2c_lock_adapter() to wait for current 
i2c transfer finished.

2. IRQF_NO_SUSPEND added could make i2c work well during the time 
between suspend_device_irqs() and i2c_suspend_noirq() callback. In the 
other side, it is the the time between resume_device_irqs() and 
i2c_resume_noirq() callback.

If any i2c client try to access I2C after suspend_noirq() or before 
resume_noirq() callback, print the warning, and they should fix it, not 
to start i2c access and the moment.

? 2016/12/8 0:27, Grygorii Strashko ??:
>
>
> On 12/06/2016 09:37 PM, David.Wu wrote:
>> Hi Doug,
>>
>> ? 2016/12/7 0:31, Doug Anderson ??:
>>> Hi,
>>>
>>> On Tue, Dec 6, 2016 at 12:12 AM, David.Wu <david.wu@rock-chips.com>
>>> wrote:
>>>> Hi Heiko,
>>>>
>>>> ? 2016/12/5 18:54, Heiko Stuebner ??:
>>>>>
>>>>> Hi David,
>>>>>
>>>>> Am Montag, 5. Dezember 2016, 16:02:59 CET schrieb David Wu:
>>>>>>
>>>>>> During suspend there may still be some i2c access happening.
>>>>>> And if we don't keep i2c irq ON, there may be i2c access timeout if
>>>>>> i2c is in irq mode of operation.
>>>>>
>>>>>
>>>>> can you describe the issue you're trying to fix a bit more please?
>>>>
>>>>
>>>> Sometimes we could see the i2c timeout errors during suspend/resume,
>>>> which
>>>> makes the duration of suspend/resume too longer.
>>>>
>>>> [  484.171541] CPU4: Booted secondary processor [410fd082]
>>>> [  485.172777] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
>>>> [  486.172760] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
>>>> [  487.172759] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
>>>> [  487.172840] cpu cpu4: _set_opp_voltage: failed to set voltage (800000
>>>> 800000 800000 mV): -110
>>>> [  487.172874] cpu cpu4: failed to set volt 800000
>>>>
>>>>>
>>>>> I.e. I'd think the i2c-core does suspend i2c-client devices first,
>>>>> so that
>>>>> these should be able to finish up their ongoing transfers and not start
>>>>> any
>>>>> new ones instead?
>>>>>
>>>>> Your irq can still happen slightly after the system started going to
>>>>> actually
>>>>> sleep, so to me it looks like you just widened the window where irqs
>>>>> can
>>>>> be
>>>>> handled. Especially as your irq could also just simply stem from the
>>>>> start
>>>>> state, so you cannot even be sure if your transaction actually is
>>>>> finished.
>>>>
>>>>
>>>> Okay, you are right. I want to give it a double insurance at first,
>>>> but it
>>>> may hide the unhappend issue.
>>>>
>>>>>
>>>>> So to me it looks like the i2c-connected device driver should be fixed
>>>>> instead?
>>>>
>>>>
>>>> I tell them to fix it in rk808 driver.
>>>
>>> To me it seems like perhaps cpufreq should not be changing frequencies
>>> until it is resumed properly.  Presumably if all the ordering is done
>>> right then cpufreq should be resumed _after_ the i2c regulator so you
>>> should be OK.  ...or am I somehow confused about that?
>>
>> yes?the cpufreq and regulator should start i2c job after they resume
>> properly.
>>
>>>
>>> Also note that previous i2c busses I worked with simply returned -EIO
>>> in the case where they were called when suspended.  See
>>> "i2c-exynos5.c" and "i2c-s3c2410.c".
>>
>> In "i2c-exynos5.c", it seems that using the "i2c->suspended" to protect
>> i2c transfer works most of the time. Of course it could prevent the next
>> new i2c transfer to start. But in one case, if the current i2c job was
>> not finished until the i2c irq was disabled by system suspend, the i2c
>> timeout error would also happen, as the current i2c job may have a large
>> data to transfer and it lasts from a long time.
>
> And this means you have bug in some of I2C client drivers which do not stop
> their activities during suspend properly (most usual case - driver uses work
> and this work still active during suspend and can run on one CPU while suspend
> runs on another).
>
> At the moment .suspend_noirq() callback is called there should be no active
> I2C transactions in general.
>
>>
>> So is it necessary to add a mutex lock to wait the current job to be
>> finished before the "i2c->suspended" is changed in i2c_suspend_noirq()?
>>
>
> You need to catch and fix all driver who will try to access I2C after your
> I2C bus driver passes suspend_noirq stage. Smth, like [1], uses i2c_lock_adapter().
>
>
> [1] https://git.ti.com/android-sdk/kernel-omap/commit/125ef8f7016e7b205886f93862288a45a312b1d8
>

^ permalink raw reply

* [Question] New mmap64 syscall?
From: Florian Weimer @ 2016-12-08 15:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161207154811.GA15248@yury-N73SV>

On 12/07/2016 04:48 PM, Yury Norov wrote:
> On Wed, Dec 07, 2016 at 02:23:55PM +0100, Florian Weimer wrote:
>> On 12/06/2016 07:54 PM, Yury Norov wrote:
>>> 3. Introduce new mmap64() syscall like this:
>>> sys_mmap64(void *addr, size_t len, int prot, int flags, int fd, struct off_pair *off);
>>> (The pointer here because otherwise we have 7 args, if simply pass off_hi and
>>> off_lo in registers.)
>>
>> I would prefer a batched mmap/munmap/mremap/mprotect/madvise interface, so
>> that VM changes can be coalesced and the output reduced.  This interface
>> could then be used to implement mmap on 32-bit architectures as well because
>> the offset restrictions would not apply there.
>
> Hi Florian,
>
> I frankly don't understand what you mean, All syscalls you mentioned
> doesn't take off_t or other 64-bit arguments. 'VM changes' - virtual
> memory? If so, I don't see any changes in VM with this approach, just
> correct handling of big offsets.

What I was trying to suggest is a completely different interface which 
is not subject to register size constraints and which has been requested 
before (a mechanism for batching mm updates).

Thanks,
Florian

^ permalink raw reply

* [RFC v3 00/10] KVM PCIe/MSI passthrough on ARM/ARM64 and IOVA reserved regions
From: Robin Murphy @ 2016-12-08 15:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <48e7d75a-8499-7f41-42d0-08fe081c192b@redhat.com>

On 08/12/16 13:36, Auger Eric wrote:
> Hi Robin,
> 
> On 08/12/2016 14:14, Robin Murphy wrote:
>> On 08/12/16 09:36, Auger Eric wrote:
>>> Hi,
>>>
>>> On 15/11/2016 14:09, Eric Auger wrote:
>>>> Following LPC discussions, we now report reserved regions through
>>>> iommu-group sysfs reserved_regions attribute file.
>>>
>>>
>>> While I am respinning this series into v4, here is a tentative summary
>>> of technical topics for which no consensus was reached at this point.
>>>
>>> 1) Shall we report the usable IOVA range instead of reserved IOVA
>>>    ranges. Not discussed at/after LPC.
>>>    x I currently report reserved regions. Alex expressed the need to
>>>      report the full usable IOVA range instead (x86 min-max range
>>>      minus MSI APIC window). I think this is meaningful for ARM
>>>      too where arm-smmu might not support the full 64b range.
>>>    x Any objection we report the usable IOVA regions instead?
>>
>> The issue with that is that we can't actually report "the usable
>> regions" at the moment, as that involves pulling together disjoint
>> properties of arbitrary hardware unrelated to the IOMMU. We'd be
>> reporting "the not-definitely-unusable regions, which may have some
>> unusable holes in them still". That seems like an ABI nightmare - I'd
>> still much rather say "here are some, but not necessarily all, regions
>> you definitely can't use", because saying "here are some regions which
>> you might be able to use most of, probably" is what we're already doing
>> today, via a single implicit region from 0 to ULONG_MAX ;)
>>
>> The address space limits are definitely useful to know, but I think it
>> would be better to expose them separately to avoid the ambiguity. At
>> worst, I guess it would be reasonable to express the limits via an
>> "out-of-range" reserved region type for 0 to $base and $top to
>> ULONG-MAX. To *safely* expose usable regions, we'd have to start out
>> with a very conservative assumption (e.g. only IOVAs matching physical
>> RAM), and only expand them once we're sure we can detect every possible
>> bit of problematic hardware in the system - that's just too limiting to
>> be useful. And if we expose something knowingly inaccurate, we risk
>> having another "bogoMIPS in /proc/cpuinfo" ABI burden on our hands, and
>> nobody wants that...
> Makes sense to me. "out-of-range reserved region type for 0 to $base and
> $top to ULONG-MAX" can be an alternative to fulfill the requirement.
>>
>>> 2) Shall the kernel check collision with MSI window* when userspace
>>>    calls VFIO_IOMMU_MAP_DMA?
>>>    Joerg/Will No; Alex yes
>>>    *for IOVA regions consumed downstream to the IOMMU: everyone says NO
>>
>> If we're starting off by having the SMMU drivers expose it as a fake
>> fixed region, I don't think we need to worry about this yet. We all seem
>> to agree that as long as we communicate the fixed regions to userspace,
>> it's then userspace's job to work around them. Let's come back to this
>> one once we actually get to the point of dynamically sizing and
>> allocating 'real' MSI remapping region(s).
>>
>> Ultimately, the kernel *will* police collisions either way, because an
>> underlying iommu_map() is going to fail if overlapping IOVAs are ever
>> actually used, so it's really just a question of whether to have a more
>> user-friendly failure mode.
> That's true on ARM but not on x86 where the APIC MSI region is not
> mapped I think.

Yes, but the APIC interrupt region is fixed, i.e. it falls under "IOVA
regions consumed downstream to the IOMMU" since the DMAR units are
physically incapable of remapping those addresses. I take "MSI window"
to mean specifically the thing we have to set aside and get a magic
remapping cookie for, which is also why it warrants its own special
internal type - we definitely *don't* want VFIO trying to set up a
remapping cookie on x86. We just don't let userspace know about the
difference, yet (if ever).

Robin.

>>> 3) RMRR reporting in the iommu group sysfs? Joerg: yes; Don: no
>>>    My current series does not expose them in iommu group sysfs.
>>>    I understand we can expose the RMRR regions in the iomm group sysfs
>>>    without necessarily supporting RMRR requiring device assignment.
>>>    We can also add this support later.
>>
>> As you say, reporting them doesn't necessitate allowing device
>> assignment, and it's information which can already be easily grovelled
>> out of dmesg (for intel-iommu at least) - there doesn't seem to be any
>> need to hide them, but the x86 folks can have the final word on that.
> agreed
> 
> Thanks
> 
> Eric
>>
>> Robin.
>>
>>> Thanks
>>>
>>> Eric
>>>
>>>
>>>>
>>>> Reserved regions are populated through the IOMMU get_resv_region callback
>>>> (former get_dm_regions), now implemented by amd-iommu, intel-iommu and
>>>> arm-smmu.
>>>>
>>>> The intel-iommu reports the [FEE0_0000h - FEF0_000h] MSI window as an
>>>> IOMMU_RESV_NOMAP reserved region.
>>>>
>>>> arm-smmu reports the MSI window (arbitrarily located at 0x8000000 and
>>>> 1MB large) and the PCI host bridge windows.
>>>>
>>>> The series integrates a not officially posted patch from Robin:
>>>> "iommu/dma: Allow MSI-only cookies".
>>>>
>>>> This series currently does not address IRQ safety assessment.
>>>>
>>>> Best Regards
>>>>
>>>> Eric
>>>>
>>>> Git: complete series available at
>>>> https://github.com/eauger/linux/tree/v4.9-rc5-reserved-rfc-v3
>>>>
>>>> History:
>>>> RFC v2 -> v3:
>>>> - switch to an iommu-group sysfs API
>>>> - use new dummy allocator provided by Robin
>>>> - dummy allocator initialized by vfio-iommu-type1 after enumerating
>>>>   the reserved regions
>>>> - at the moment ARM MSI base address/size is left unchanged compared
>>>>   to v2
>>>> - we currently report reserved regions and not usable IOVA regions as
>>>>   requested by Alex
>>>>
>>>> RFC v1 -> v2:
>>>> - fix intel_add_reserved_regions
>>>> - add mutex lock/unlock in vfio_iommu_type1
>>>>
>>>>
>>>> Eric Auger (10):
>>>>   iommu/dma: Allow MSI-only cookies
>>>>   iommu: Rename iommu_dm_regions into iommu_resv_regions
>>>>   iommu: Add new reserved IOMMU attributes
>>>>   iommu: iommu_alloc_resv_region
>>>>   iommu: Do not map reserved regions
>>>>   iommu: iommu_get_group_resv_regions
>>>>   iommu: Implement reserved_regions iommu-group sysfs file
>>>>   iommu/vt-d: Implement reserved region get/put callbacks
>>>>   iommu/arm-smmu: Implement reserved region get/put callbacks
>>>>   vfio/type1: Get MSI cookie
>>>>
>>>>  drivers/iommu/amd_iommu.c       |  20 +++---
>>>>  drivers/iommu/arm-smmu.c        |  52 +++++++++++++++
>>>>  drivers/iommu/dma-iommu.c       | 116 ++++++++++++++++++++++++++-------
>>>>  drivers/iommu/intel-iommu.c     |  50 ++++++++++----
>>>>  drivers/iommu/iommu.c           | 141 ++++++++++++++++++++++++++++++++++++----
>>>>  drivers/vfio/vfio_iommu_type1.c |  26 ++++++++
>>>>  include/linux/dma-iommu.h       |   7 ++
>>>>  include/linux/iommu.h           |  49 ++++++++++----
>>>>  8 files changed, 391 insertions(+), 70 deletions(-)
>>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

^ permalink raw reply

* [PATCH 1/2] Documentation: sample averaging for imx6ul_tsc
From: Rob Herring @ 2016-12-08 15:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <82b04890-692d-4e22-36c1-a21affad5126@mobi-wize.com>

On Thu, Dec 8, 2016 at 9:15 AM, Guy Shapiro <guy.shapiro@mobi-wize.com> wrote:
> On 01/12/2016 18:13, Rob Herring wrote:
>>
>> On Sun, Nov 27, 2016 at 09:44:57AM +0200, Guy Shapiro wrote:
>>>
>>> The i.MX6UL internal touchscreen controller contains an option to
>>> average upon samples. This feature reduces noise from the produced
>>> touch locations.
>>>
>>> This patch introduces a new device tree optional property for this
>>> feature. It provides control over the amount of averaged samples per
>>> touch event.
>>>
>>> The property was inspired by a similar property on the
>>> "brcm,iproc-touchscreen" binding.
>>>
>>> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
>>> ---
>>>   .../devicetree/bindings/input/touchscreen/imx6ul_tsc.txt          | 8
>>
>> ++++++++
>>>
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git
>>
>> a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>> b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>>>
>>> index 853dff9..a66069f 100644
>>> --- a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>>> +++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>>> @@ -17,6 +17,13 @@ Optional properties:
>>>     This value depends on the touch screen.
>>>   - pre-charge-time: the touch screen need some time to precharge.
>>>     This value depends on the touch screen.
>>> +- average-samples: Number of data samples which are averaged for each
>>
>> read.
>>>
>>> +       Valid values 0-4
>>> +       0 =  1 sample
>>> +       1 =  4 samples
>>> +       2 =  8 samples
>>> +       3 = 16 samples
>>> +       4 = 32 samples
>>
>> Either this needs a vendor prefix or should be documented as a generic
>> property. In the latter case, you should use actual number of samples
>> (1-32) for the values.
>
> In the term "generic property", do you mean to document it on
> bindings/input/touchscreen/touchscreen.txt ?

Yes.

> If so, should I add the "touchscreen-" prefix like all the other properties
> in that file?

Yes.

> Grepping bindings/input/touchscreen/, I found two other device drivers that
> implement a
> similar property - "ti-tsc-adc" and "brcm,iproc-touchscreen" (The latter,
> BTW, uses a non
> vendor prefixed property name).

Unfortunately, the brcm one doesn't look directly usable.

> Do we want to move from per-vendor properties to a generic one?

No. Those are set already. I just don't want to get more vendor properties.

Rob

^ permalink raw reply

* Tearing down DMA transfer setup after DMA client has finished
From: Mason @ 2016-12-08 15:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161208154018.GD6408@localhost>

On 08/12/2016 16:40, Vinod Koul wrote:

> FWIW look at ALSA-dmaengine lib, thereby every audio driver that uses it. I
> could find other examples and go on and on, but that's besides the point and
> looks like you don't want to listen to people telling you something..

Hello Vinod,

FWIW, your system clock seems to be 10 minutes ahead ;-)

I am willing to learn. I have a few outstanding questions
in the thread. Could you have a look at them?

Regards.

^ permalink raw reply

* Tearing down DMA transfer setup after DMA client has finished
From: Vinod Koul @ 2016-12-08 15:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <yw1x60mu1w7e.fsf@unicorn.mansr.com>

On Thu, Dec 08, 2016 at 11:44:53AM +0000, M?ns Rullg?rd wrote:
> Vinod Koul <vinod.koul@intel.com> writes:
> 
> > On Wed, Dec 07, 2016 at 04:45:58PM +0000, M?ns Rullg?rd wrote:
> >> Vinod Koul <vinod.koul@intel.com> writes:
> >> 
> >> > On Tue, Dec 06, 2016 at 01:14:20PM +0000, M?ns Rullg?rd wrote:
> >> >> 
> >> >> That's not going to work very well.  Device drivers typically request
> >> >> dma channels in their probe functions or when the device is opened.
> >> >> This means that reserving one of the few channels there will inevitably
> >> >> make some other device fail to operate.
> >> >
> >> > No that doesnt make sense at all, you should get a channel only when you
> >> > want to use it and not in probe!
> >> 
> >> Tell that to just about every single driver ever written.
> >
> > Not really, few do yes which is wrong but not _all_ do that.
> 
> Every driver I ever looked at does.  Name one you consider "correct."

That only tells me you haven't looked enough and want to rant!

FWIW look at ALSA-dmaengine lib, thereby every audio driver that uses it. I
could find other examples and go on and on, but that's besides the point and
looks like you don't want to listen to people telling you something..

-- 
~Vinod

^ permalink raw reply

* [PATCH renesas/devel] arm64: dts: r8a7795: Use Gen 3 fallback compat string for PCIE
From: Simon Horman @ 2016-12-08 15:29 UTC (permalink / raw)
  To: linux-arm-kernel

Use recently added en 3 fallback compat string for PCIE
in r8a7795 DT.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
Based on renesas-devel-20161207-v4.9-rc8
---
 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index 598bec1629cf..e8718e7d8d97 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -1259,7 +1259,8 @@
 		};
 
 		pciec0: pcie at fe000000 {
-			compatible = "renesas,pcie-r8a7795";
+			compatible = "renesas,pcie-r8a7795",
+				     "renesas,pcie-rcar-gen3";
 			reg = <0 0xfe000000 0 0x80000>;
 			#address-cells = <3>;
 			#size-cells = <2>;
@@ -1284,7 +1285,8 @@
 		};
 
 		pciec1: pcie at ee800000 {
-			compatible = "renesas,pcie-r8a7795";
+			compatible = "renesas,pcie-r8a7795",
+				     "renesas,pcie-rcar-gen3";
 			reg = <0 0xee800000 0 0x80000>;
 			#address-cells = <3>;
 			#size-cells = <2>;
-- 
2.7.0.rc3.207.g0ac5344

^ permalink raw reply related

* [GIT PULL 1/3] ARM: exynos: Soc/mach for v4.10
From: Pankaj Dubey @ 2016-12-08 15:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161203170347.GA3553@kozik-lap>

On 3 December 2016 at 22:33, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Fri, Dec 02, 2016 at 10:52:57PM +0100, Arnd Bergmann wrote:
>> On Thursday, December 1, 2016 8:34:04 PM CET Krzysztof Kozlowski wrote:
>> > On Thu, Nov 24, 2016 at 08:08:27AM +0200, Krzysztof Kozlowski wrote:
>> > > Hi,
>> > >
>> > > This contains previous dts branch because SCU node in dts is needed
>> > > prior to removing it from mach code.
>> > >
>> > > Below you will find full pull request and one stripped from dependency.
>> > >
>> >
>> > Hi Arnd, Kevin and Olof,
>> >
>> > What about this pull from the set?
>> >
>>
>> Sorry, I initially deferred it and then didn't get back to it.
>>
>> The dependency on the .dts changes made me a bit nervous about
>> taking it, mostly because the changelog fails to explain the
>> exact dependencies.
>>
>> This breaks compatibility with existing .dtb files, right?
>
> No, strictly speaking not. There was no dt-bindings change here, no DT
> properties for SCU before. We are converting our drivers to DTB so this
> is the same as before when switching for pinctrl, clocks or all other
> drivers to DT.
>
> We are not braking DTB ABI because there was no ABI around it before.
> Otherwise, one would say that lack of SCU DT node was an ABI. That is
> wrong, because DT should describe the hardware and SCU is in hardware.
>
>> What I'd like to see here is an explanation about:
>>
>> - what the upside of breaking compatibility is
>
> DTBs which do not have SCU are not proper because they skip that part of
> hardware. However we are breaking them in the way the SMP won't work
> there. It is not an ABI break, as I mentioned above.
>
>> - what exactly stops working with an old dtb
>> - why we don't keep a fallback for handling old dtb files
>
> What is the point for it? This is not an ABI break. Even if it was,
> Samsung guys don't care for ABI breaks at all (and in fact we wanted to
> mark the platform experimental...).
>
>> It would also be helpful to have a separate pull request for
>> the commits require the new dtb, and the stuff that is unrelated.
>
> I can do that but the pull will be small.
>

Arnd,

Any update on this? Intention of this change is to improve
Exynos SoC's DT support in mainline kernel. This will help in removing static
mapping from exynos machine files and simplify mach-exynos code-base.

Thanks,
Pankaj Dubey

> Best regards,
> Krzysztof
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH V1] pinctrl:pxa:pinctrl-pxa2xx:- No need of devm functions
From: Robin Murphy @ 2016-12-08 15:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <28f80351-a89d-06b4-bd8e-d4a1810ee3c5@arm.com>

On 08/12/16 15:20, Robin Murphy wrote:
> On 08/12/16 14:35, Arvind Yadav wrote:
>> In functions pxa2xx_build_functions, the memory allocated for
>> 'functions' is live within the function only. After the
>> allocation it is immediately freed with devm_kfree. There is
>> no need to allocate memory for 'functions' with devm function
>> so replace devm_kcalloc  with kcalloc and devm_kfree with kfree.
>>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>> ---
>>  drivers/pinctrl/pxa/pinctrl-pxa2xx.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pxa/pinctrl-pxa2xx.c b/drivers/pinctrl/pxa/pinctrl-pxa2xx.c
>> index 866aa3c..47b8e3a 100644
>> --- a/drivers/pinctrl/pxa/pinctrl-pxa2xx.c
>> +++ b/drivers/pinctrl/pxa/pinctrl-pxa2xx.c
>> @@ -277,7 +277,7 @@ static int pxa2xx_build_functions(struct pxa_pinctrl *pctl)
>>  	 * alternate function, 6 * npins is an absolute high limit of the number
>>  	 * of functions.
>>  	 */
>> -	functions = devm_kcalloc(pctl->dev, pctl->npins * 6,
>> +	functions = kcalloc(pctl->npins * 6,
>>  				 sizeof(*functions), GFP_KERNEL);
> 
> AFAICS, this is allocating a mere 72 bytes. Why not just declare
> 
> 	struct pxa_pinctrl_function functions[6] = {0};
> 
> locally and save *all* the bother?

Bah, ignore me, that was an incredible comprehension failure.

Sorry for the noise.

Robin.

>>  	if (!functions)
>>  		return -ENOMEM;
>> @@ -289,10 +289,12 @@ static int pxa2xx_build_functions(struct pxa_pinctrl *pctl)
>>  	pctl->functions = devm_kmemdup(pctl->dev, functions,
>>  				       pctl->nfuncs * sizeof(*functions),
>>  				       GFP_KERNEL);
>> -	if (!pctl->functions)
>> +	if (!pctl->functions) {
>> +		kfree(functions);
>>  		return -ENOMEM;
>> +	}
>>  
>> -	devm_kfree(pctl->dev, functions);
>> +	kfree(functions);
>>  	return 0;
>>  }
>>  
>>
> 

^ permalink raw reply

* [PATCH V1] pinctrl:pxa:pinctrl-pxa2xx:- No need of devm functions
From: Robin Murphy @ 2016-12-08 15:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481207730-6332-1-git-send-email-arvind.yadav.cs@gmail.com>

On 08/12/16 14:35, Arvind Yadav wrote:
> In functions pxa2xx_build_functions, the memory allocated for
> 'functions' is live within the function only. After the
> allocation it is immediately freed with devm_kfree. There is
> no need to allocate memory for 'functions' with devm function
> so replace devm_kcalloc  with kcalloc and devm_kfree with kfree.
> 
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
>  drivers/pinctrl/pxa/pinctrl-pxa2xx.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/pxa/pinctrl-pxa2xx.c b/drivers/pinctrl/pxa/pinctrl-pxa2xx.c
> index 866aa3c..47b8e3a 100644
> --- a/drivers/pinctrl/pxa/pinctrl-pxa2xx.c
> +++ b/drivers/pinctrl/pxa/pinctrl-pxa2xx.c
> @@ -277,7 +277,7 @@ static int pxa2xx_build_functions(struct pxa_pinctrl *pctl)
>  	 * alternate function, 6 * npins is an absolute high limit of the number
>  	 * of functions.
>  	 */
> -	functions = devm_kcalloc(pctl->dev, pctl->npins * 6,
> +	functions = kcalloc(pctl->npins * 6,
>  				 sizeof(*functions), GFP_KERNEL);

AFAICS, this is allocating a mere 72 bytes. Why not just declare

	struct pxa_pinctrl_function functions[6] = {0};

locally and save *all* the bother?

Robin.

>  	if (!functions)
>  		return -ENOMEM;
> @@ -289,10 +289,12 @@ static int pxa2xx_build_functions(struct pxa_pinctrl *pctl)
>  	pctl->functions = devm_kmemdup(pctl->dev, functions,
>  				       pctl->nfuncs * sizeof(*functions),
>  				       GFP_KERNEL);
> -	if (!pctl->functions)
> +	if (!pctl->functions) {
> +		kfree(functions);
>  		return -ENOMEM;
> +	}
>  
> -	devm_kfree(pctl->dev, functions);
> +	kfree(functions);
>  	return 0;
>  }
>  
> 

^ permalink raw reply

* [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU
From: Pankaj Dubey @ 2016-12-08 15:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2339821.zs8sl71mMv@wuerfel>

On 18 November 2016 at 19:02, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday, November 18, 2016 12:48:07 PM CET Russell King - ARM Linux wrote:
>> On Fri, Nov 18, 2016 at 01:14:35PM +0100, Arnd Bergmann wrote:
>> > @@ -41,6 +43,9 @@ void scu_enable(void __iomem *scu_base)
>> >  {
>> >       u32 scu_ctrl;
>> >
>> > +     if (scu_base)
>> > +             scu_base = scu_base_addr;
>> > +
>>
>> This looks to me like nonsense.
>>
>> >  #ifdef CONFIG_ARM_ERRATA_764369
>> >       /* Cortex-A9 only */
>> >       if ((read_cpuid_id() & 0xff0ffff0) == 0x410fc090) {
>> > @@ -85,6 +90,9 @@ int scu_power_mode(void __iomem *scu_base, unsigned int mode)
>> >       unsigned int val;
>> >       int cpu = MPIDR_AFFINITY_LEVEL(cpu_logical_map(smp_processor_id()), 0);
>> >
>> > +     if (scu_base)
>> > +             scu_base = scu_base_addr;
>> > +
>>
>> Ditto.
>>
>> Rather than doing this, I'd much prefer to always store the SCU base in
>> the SCU code, and remove the "void __iomem *scu_base" argment from all
>> these functions.
>
> Ok, then we just need one scu_probe_*() variant for each of the
> four methods of initializing it (iotable, of_iomap,
> ioremap(scu_a9_get_base) and hardcoded.
>
> The intention of doing the fallback for the NULL argument was
> to avoid having to add lots of new API while also allowing
> the change to be done one platform at a time.
>
> If we remove the argument from the other functions, they either
> need to get a new name, or we change them all to the new prototype
> at once. Either way works fine, do you have a preference between
> them?
>

Russell,

Any opinion on this. Are you OK, with the approach suggested by Arnd?

Thanks,
Pankaj Dubey

>         Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 1/2] Documentation: sample averaging for imx6ul_tsc
From: Guy Shapiro @ 2016-12-08 15:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161201161343.g7p2maxiatp4m5di@rob-hp-laptop>

On 01/12/2016 18:13, Rob Herring wrote:
> On Sun, Nov 27, 2016 at 09:44:57AM +0200, Guy Shapiro wrote:
>> The i.MX6UL internal touchscreen controller contains an option to
>> average upon samples. This feature reduces noise from the produced
>> touch locations.
>>
>> This patch introduces a new device tree optional property for this
>> feature. It provides control over the amount of averaged samples per
>> touch event.
>>
>> The property was inspired by a similar property on the
>> "brcm,iproc-touchscreen" binding.
>>
>> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
>> ---
>>   .../devicetree/bindings/input/touchscreen/imx6ul_tsc.txt          | 8
> ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git
> a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>> index 853dff9..a66069f 100644
>> --- a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>> @@ -17,6 +17,13 @@ Optional properties:
>>     This value depends on the touch screen.
>>   - pre-charge-time: the touch screen need some time to precharge.
>>     This value depends on the touch screen.
>> +- average-samples: Number of data samples which are averaged for each
> read.
>> +	Valid values 0-4
>> +	0 =  1 sample
>> +	1 =  4 samples
>> +	2 =  8 samples
>> +	3 = 16 samples
>> +	4 = 32 samples
> Either this needs a vendor prefix or should be documented as a generic
> property. In the latter case, you should use actual number of samples
> (1-32) for the values.
In the term "generic property", do you mean to document it on 
bindings/input/touchscreen/touchscreen.txt ?
If so, should I add the "touchscreen-" prefix like all the other 
properties in that file?

Grepping bindings/input/touchscreen/, I found two other device drivers 
that implement a
similar property - "ti-tsc-adc" and "brcm,iproc-touchscreen" (The 
latter, BTW, uses a non
vendor prefixed property name).

Do we want to move from per-vendor properties to a generic one?
If we do, should we deprecate the existing vendor specific properties?

^ permalink raw reply

* [PATCH 1/2] ASoC: zte: spdif and i2s drivers are not zx296702 specific
From: Jun Nie @ 2016-12-08 14:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481186655-8213-1-git-send-email-shawnguo@kernel.org>

2016-12-08 16:44 GMT+08:00 Shawn Guo <shawnguo@kernel.org>:
> From: Shawn Guo <shawn.guo@linaro.org>
>
> ZTE ZX SPDIF and I2S drivers can work on not only ZX296702 but also
> other ZTE ZX family SoCs like ZX296718, which is an arm64 platform.
> Let's make a few renaming and tweak the Kconfig a bit to get the drivers
> available for other ZTE ZX platforms.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  sound/soc/zte/Kconfig                          | 16 ++++++++--------
>  sound/soc/zte/Makefile                         |  4 ++--
>  sound/soc/zte/{zx296702-i2s.c => zx-i2s.c}     |  0
>  sound/soc/zte/{zx296702-spdif.c => zx-spdif.c} |  0
>  4 files changed, 10 insertions(+), 10 deletions(-)
>  rename sound/soc/zte/{zx296702-i2s.c => zx-i2s.c} (100%)
>  rename sound/soc/zte/{zx296702-spdif.c => zx-spdif.c} (100%)
>
> diff --git a/sound/soc/zte/Kconfig b/sound/soc/zte/Kconfig
> index c47eb25e441f..6d8a90d36315 100644
> --- a/sound/soc/zte/Kconfig
> +++ b/sound/soc/zte/Kconfig
> @@ -1,17 +1,17 @@
> -config ZX296702_SPDIF
> -       tristate "ZX296702 spdif"
> -       depends on SOC_ZX296702 || COMPILE_TEST
> +config ZX_SPDIF
> +       tristate "ZTE ZX SPDIF Driver Support"
> +       depends on ARCH_ZX || COMPILE_TEST
>         depends on COMMON_CLK
>         select SND_SOC_GENERIC_DMAENGINE_PCM
>         help
>           Say Y or M if you want to add support for codecs attached to the
> -         zx296702 spdif interface
> +         ZTE ZX SPDIF interface
>
> -config ZX296702_I2S
> -       tristate "ZX296702 i2s"
> -       depends on SOC_ZX296702 || COMPILE_TEST
> +config ZX_I2S
> +       tristate "ZTE ZX I2S Driver Support"
> +       depends on ARCH_ZX || COMPILE_TEST
>         depends on COMMON_CLK
>         select SND_SOC_GENERIC_DMAENGINE_PCM
>         help
>           Say Y or M if you want to add support for codecs attached to the
> -         zx296702 i2s interface
> +         ZTE ZX I2S interface
> diff --git a/sound/soc/zte/Makefile b/sound/soc/zte/Makefile
> index 254ed2c8c1a0..77768f5fd10c 100644
> --- a/sound/soc/zte/Makefile
> +++ b/sound/soc/zte/Makefile
> @@ -1,2 +1,2 @@
> -obj-$(CONFIG_ZX296702_SPDIF)   += zx296702-spdif.o
> -obj-$(CONFIG_ZX296702_I2S)     += zx296702-i2s.o
> +obj-$(CONFIG_ZX_SPDIF) += zx-spdif.o
> +obj-$(CONFIG_ZX_I2S)   += zx-i2s.o
> diff --git a/sound/soc/zte/zx296702-i2s.c b/sound/soc/zte/zx-i2s.c
> similarity index 100%
> rename from sound/soc/zte/zx296702-i2s.c
> rename to sound/soc/zte/zx-i2s.c
> diff --git a/sound/soc/zte/zx296702-spdif.c b/sound/soc/zte/zx-spdif.c
> similarity index 100%
> rename from sound/soc/zte/zx296702-spdif.c
> rename to sound/soc/zte/zx-spdif.c
> --
> 1.9.1
>

Reviewed-by: Jun Nie <jun.nie@linaro.org>

^ permalink raw reply

* [PATCH v4 4/4] arm64: dts: marvell: Enable spi0 on the board Armada-3720-db
From: Romain Perier @ 2016-12-08 14:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161208145847.7794-1-romain.perier@free-electrons.com>

This commit enables the device node spi0 on the official development
board for the Marvell Armada 3700. It also adds sub-node for the 128Mb
SPI-NOR present on the board.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---

Changes in v3:
 - Added tag "Tested-by" by Gregory

 arch/arm64/boot/dts/marvell/armada-3720-db.dts | 30 ++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 1372e9a6..0c4eb98 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -67,6 +67,36 @@
 	status = "okay";
 };
 
+&spi0 {
+	status = "okay";
+
+	m25p80 at 0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <108000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
+
+		partitions {
+			compatible = "fixed-partitions";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			partition at 0 {
+				label = "bootloader";
+				reg = <0x0 0x200000>;
+			};
+			partition at 200000 {
+				label = "U-boot Env";
+				reg = <0x200000 0x10000>;
+			};
+			partition at 210000 {
+				label = "Linux";
+				reg = <0x210000 0xDF0000>;
+			};
+		};
+	};
+};
+
 /* Exported on the micro USB connector CON32 through an FTDI */
 &uart0 {
 	status = "okay";
-- 
2.9.3

^ permalink raw reply related

* [PATCH v4 3/4] arm64: dts: marvell: Add definition of SPI controller for Armada 3700
From: Romain Perier @ 2016-12-08 14:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161208145847.7794-1-romain.perier@free-electrons.com>

Armada 3700 SoC has an SPI Controller, this commit adds the definition
of the SPI device node at the SoC level.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---

Changes in v3:
 - Fixed wrong register size for spi0, as suggested by the maintainer
   on the ML.
 - Added tag "Tested-by" by Gregory

Changes in v2:
 - Removed properties max-frequency and clock-frequency, it is no
   longer required and not used by the DT-bindings.

 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index e9bd587..fcef9a5 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -98,6 +98,17 @@
 			/* 32M internal register @ 0xd000_0000 */
 			ranges = <0x0 0x0 0xd0000000 0x2000000>;
 
+			spi0: spi at 10600 {
+				compatible = "marvell,armada-3700-spi";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x10600 0xA00>;
+				clocks = <&nb_periph_clk 7>;
+				interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+				num-cs = <4>;
+				status = "disabled";
+			};
+
 			uart0: serial at 12000 {
 				compatible = "marvell,armada-3700-uart";
 				reg = <0x12000 0x400>;
-- 
2.9.3

^ permalink raw reply related

* [PATCH v4 2/4] spi: armada-3700: Add documentation for the Armada 3700 SPI Controller
From: Romain Perier @ 2016-12-08 14:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161208145847.7794-1-romain.perier@free-electrons.com>

This adds the devicetree bindings documentation for the SPI controller
present in the Marvell Armada 3700 SoCs.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Acked-by: Rob Herring <robh@kernel.org>
---

Changes in v4:
 - Added tag "Acked-by" by Rob Herring

Changes in v3:
 - Added tag "Tested-by" by Gregory
 - Fixed commit title, as requested by Mark Brown

 .../devicetree/bindings/spi/spi-armada-3700.txt    | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt

diff --git a/Documentation/devicetree/bindings/spi/spi-armada-3700.txt b/Documentation/devicetree/bindings/spi/spi-armada-3700.txt
new file mode 100644
index 0000000..1564aa8
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-armada-3700.txt
@@ -0,0 +1,25 @@
+* Marvell Armada 3700 SPI Controller
+
+Required Properties:
+
+- compatible: should be "marvell,armada-3700-spi"
+- reg: physical base address of the controller and length of memory mapped
+       region.
+- interrupts: The interrupt number. The interrupt specifier format depends on
+	      the interrupt controller and of its driver.
+- clocks: Must contain the clock source, usually from the North Bridge clocks.
+- num-cs: The number of chip selects that is supported by this SPI Controller
+- #address-cells: should be 1.
+- #size-cells: should be 0.
+
+Example:
+
+	spi0: spi at 10600 {
+		compatible = "marvell,armada-3700-spi";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10600 0x5d>;
+		clocks = <&nb_perih_clk 7>;
+		interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+		num-cs = <4>;
+	};
-- 
2.9.3

^ permalink raw reply related

* [PATCH v4 1/4] spi: Add support for Armada 3700 SPI Controller
From: Romain Perier @ 2016-12-08 14:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161208145847.7794-1-romain.perier@free-electrons.com>

Marvell Armada 3700 SoC comprises an SPI Controller. This Controller
supports up to 4 SPI slave devices, with dedicated chip selects,supports
SPI mode 0/1/2 and 3, CPIO or Fifo mode with DMA transfers and different
SPI transfer mode (Single, Dual or Quad).

This commit adds basic driver support for FIFO mode. In this mode,
dedicated registers are used to store the instruction, the address, the
read mode and the data. Write and Read FIFO are used to store the
outcoming or incoming data. The data FIFOs are accessible via DMA or by
the CPU. Only the CPU is supported for now.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---

Changes in v4:
 - Added missing COMPILE_TEST in KConfig
 - Replaced the busy wait for loop by a single busy wait in spi_init()
 - Handle IRQ_NONE in the interrupt handler 
 - Assign pdev->id to master->cs_num directly (no conditionnal operator)
 - Moved clock gating into prepare/unprepare callbacks
 - Removed dev_info() in the end of the probe() function
 - Added flag to the spi master for single duplex transfers
 - Squashed patch 1/5 and 2/5 together: remove the non-fifo mode, just keep
   the more efficient mode
 - So removed the module parameter

Changes in v3:
 - Don't enable the fifo mode based on the compatible string, we introduce
   a module parameter "pio_mode". By default this option is set to zero, so
   the fifo mode is enabled. Pass pio_mode=1 to the driver enables the PIO
   mode.
 - Added tag "Tested-by" by Gregory
 - Fixed wrong variable passed as MODULE_DEVICE_TABLE
 - Added missing null terminated entry in a3700_spi_dt_ids 

Changes in v2:
 - Removed a3700_spi_bytelen_set from the setup function, it was accidentally
   let here and not required, as it is configured in the prepare callback now
   (defaults to 4 for fifo mode). It solves unrecognized spi-nor flash memory
   detection with jedec.

 drivers/spi/Kconfig           |   7 +
 drivers/spi/Makefile          |   1 +
 drivers/spi/spi-armada-3700.c | 923 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 931 insertions(+)
 create mode 100644 drivers/spi/spi-armada-3700.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index b799547..1faad2ce 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -67,6 +67,13 @@ config SPI_ATH79
 	  This enables support for the SPI controller present on the
 	  Atheros AR71XX/AR724X/AR913X SoCs.
 
+config SPI_ARMADA_3700
+	tristate "Marvell Armada 3700 SPI Controller"
+	depends on (ARCH_MVEBU && OF) || COMPILE_TEST
+	help
+	  This enables support for the SPI controller present on the
+	  Marvell Armada 3700 SoCs.
+
 config SPI_ATMEL
 	tristate "Atmel SPI Controller"
 	depends on HAS_DMA
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index aa939d9..140ca45 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SPI_LOOPBACK_TEST)		+= spi-loopback-test.o
 
 # SPI master controller drivers (bus)
 obj-$(CONFIG_SPI_ALTERA)		+= spi-altera.o
+obj-$(CONFIG_SPI_ARMADA_3700)		+= spi-armada-3700.o
 obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
 obj-$(CONFIG_SPI_ATH79)			+= spi-ath79.o
 obj-$(CONFIG_SPI_AU1550)		+= spi-au1550.o
diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
new file mode 100644
index 0000000..e89da0a
--- /dev/null
+++ b/drivers/spi/spi-armada-3700.c
@@ -0,0 +1,923 @@
+/*
+ * Marvell Armada-3700 SPI controller driver
+ *
+ * Copyright (C) 2016 Marvell Ltd.
+ *
+ * Author: Wilson Ding <dingwei@marvell.com>
+ * Author: Romain Perier <romain.perier@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/spi/spi.h>
+
+#define DRIVER_NAME			"armada_3700_spi"
+
+#define A3700_SPI_TIMEOUT		10
+
+/* SPI Register Offest */
+#define A3700_SPI_IF_CTRL_REG		0x00
+#define A3700_SPI_IF_CFG_REG		0x04
+#define A3700_SPI_DATA_OUT_REG		0x08
+#define A3700_SPI_DATA_IN_REG		0x0C
+#define A3700_SPI_IF_INST_REG		0x10
+#define A3700_SPI_IF_ADDR_REG		0x14
+#define A3700_SPI_IF_RMODE_REG		0x18
+#define A3700_SPI_IF_HDR_CNT_REG	0x1C
+#define A3700_SPI_IF_DIN_CNT_REG	0x20
+#define A3700_SPI_IF_TIME_REG		0x24
+#define A3700_SPI_INT_STAT_REG		0x28
+#define A3700_SPI_INT_MASK_REG		0x2C
+
+/* A3700_SPI_IF_CTRL_REG */
+#define A3700_SPI_EN			BIT(16)
+#define A3700_SPI_ADDR_NOT_CONFIG	BIT(12)
+#define A3700_SPI_WFIFO_OVERFLOW	BIT(11)
+#define A3700_SPI_WFIFO_UNDERFLOW	BIT(10)
+#define A3700_SPI_RFIFO_OVERFLOW	BIT(9)
+#define A3700_SPI_RFIFO_UNDERFLOW	BIT(8)
+#define A3700_SPI_WFIFO_FULL		BIT(7)
+#define A3700_SPI_WFIFO_EMPTY		BIT(6)
+#define A3700_SPI_RFIFO_FULL		BIT(5)
+#define A3700_SPI_RFIFO_EMPTY		BIT(4)
+#define A3700_SPI_WFIFO_RDY		BIT(3)
+#define A3700_SPI_RFIFO_RDY		BIT(2)
+#define A3700_SPI_XFER_RDY		BIT(1)
+#define A3700_SPI_XFER_DONE		BIT(0)
+
+/* A3700_SPI_IF_CFG_REG */
+#define A3700_SPI_WFIFO_THRS		BIT(28)
+#define A3700_SPI_RFIFO_THRS		BIT(24)
+#define A3700_SPI_AUTO_CS		BIT(20)
+#define A3700_SPI_DMA_RD_EN		BIT(18)
+#define A3700_SPI_FIFO_MODE		BIT(17)
+#define A3700_SPI_SRST			BIT(16)
+#define A3700_SPI_XFER_START		BIT(15)
+#define A3700_SPI_XFER_STOP		BIT(14)
+#define A3700_SPI_INST_PIN		BIT(13)
+#define A3700_SPI_ADDR_PIN		BIT(12)
+#define A3700_SPI_DATA_PIN1		BIT(11)
+#define A3700_SPI_DATA_PIN0		BIT(10)
+#define A3700_SPI_FIFO_FLUSH		BIT(9)
+#define A3700_SPI_RW_EN			BIT(8)
+#define A3700_SPI_CLK_POL		BIT(7)
+#define A3700_SPI_CLK_PHA		BIT(6)
+#define A3700_SPI_BYTE_LEN		BIT(5)
+#define A3700_SPI_CLK_PRESCALE		BIT(0)
+#define A3700_SPI_CLK_PRESCALE_MASK	(0x1f)
+
+#define A3700_SPI_WFIFO_THRS_BIT	28
+#define A3700_SPI_RFIFO_THRS_BIT	24
+#define A3700_SPI_FIFO_THRS_MASK	0x7
+
+#define A3700_SPI_DATA_PIN_MASK		0x3
+
+/* A3700_SPI_IF_HDR_CNT_REG */
+#define A3700_SPI_DUMMY_CNT_BIT		12
+#define A3700_SPI_DUMMY_CNT_MASK	0x7
+#define A3700_SPI_RMODE_CNT_BIT		8
+#define A3700_SPI_RMODE_CNT_MASK	0x3
+#define A3700_SPI_ADDR_CNT_BIT		4
+#define A3700_SPI_ADDR_CNT_MASK		0x7
+#define A3700_SPI_INSTR_CNT_BIT		0
+#define A3700_SPI_INSTR_CNT_MASK	0x3
+
+/* A3700_SPI_IF_TIME_REG */
+#define A3700_SPI_CLK_CAPT_EDGE		BIT(7)
+
+/* Flags and macros for struct a3700_spi */
+#define A3700_INSTR_CNT			1
+#define A3700_ADDR_CNT			3
+#define A3700_DUMMY_CNT			1
+
+struct a3700_spi {
+	struct spi_master *master;
+	void __iomem *base;
+	struct clk *clk;
+	unsigned int irq;
+	unsigned int flags;
+	bool xmit_data;
+	const u8 *tx_buf;
+	u8 *rx_buf;
+	size_t buf_len;
+	u8 byte_len;
+	u32 wait_mask;
+	struct completion done;
+	u32 addr_cnt;
+	u32 instr_cnt;
+	size_t hdr_cnt;
+};
+
+static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
+{
+	return readl(a3700_spi->base + offset);
+}
+
+static void spireg_write(struct a3700_spi *a3700_spi, u32 offset, u32 data)
+{
+	writel(data, a3700_spi->base + offset);
+}
+
+static void a3700_spi_auto_cs_unset(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~A3700_SPI_AUTO_CS;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_activate_cs(struct a3700_spi *a3700_spi, unsigned int cs)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	val |= (A3700_SPI_EN << cs);
+	spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static void a3700_spi_deactivate_cs(struct a3700_spi *a3700_spi,
+				    unsigned int cs)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	val &= ~(A3700_SPI_EN << cs);
+	spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static int a3700_spi_pin_mode_set(struct a3700_spi *a3700_spi,
+				  unsigned int pin_mode)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~(A3700_SPI_INST_PIN | A3700_SPI_ADDR_PIN);
+	val &= ~(A3700_SPI_DATA_PIN0 | A3700_SPI_DATA_PIN1);
+
+	switch (pin_mode) {
+	case 1:
+		break;
+	case 2:
+		val |= A3700_SPI_DATA_PIN0;
+		break;
+	case 4:
+		val |= A3700_SPI_DATA_PIN1;
+		break;
+	default:
+		dev_err(&a3700_spi->master->dev, "wrong pin mode %u", pin_mode);
+		return -EINVAL;
+	}
+
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	return 0;
+}
+
+static void a3700_spi_fifo_mode_set(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val |= A3700_SPI_FIFO_MODE;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_mode_set(struct a3700_spi *a3700_spi,
+			       unsigned int mode_bits)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+
+	if (mode_bits & SPI_CPOL)
+		val |= A3700_SPI_CLK_POL;
+	else
+		val &= ~A3700_SPI_CLK_POL;
+
+	if (mode_bits & SPI_CPHA)
+		val |= A3700_SPI_CLK_PHA;
+	else
+		val &= ~A3700_SPI_CLK_PHA;
+
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_clock_set(struct a3700_spi *a3700_spi,
+				unsigned int speed_hz, u16 mode)
+{
+	u32 val;
+	u32 prescale;
+
+	prescale = DIV_ROUND_UP(clk_get_rate(a3700_spi->clk), speed_hz);
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val = val & ~A3700_SPI_CLK_PRESCALE_MASK;
+
+	val = val | (prescale & A3700_SPI_CLK_PRESCALE_MASK);
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	if (prescale <= 2) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_TIME_REG);
+		val |= A3700_SPI_CLK_CAPT_EDGE;
+		spireg_write(a3700_spi, A3700_SPI_IF_TIME_REG, val);
+	}
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~(A3700_SPI_CLK_POL | A3700_SPI_CLK_PHA);
+
+	if (mode & SPI_CPOL)
+		val |= A3700_SPI_CLK_POL;
+
+	if (mode & SPI_CPHA)
+		val |= A3700_SPI_CLK_PHA;
+
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_bytelen_set(struct a3700_spi *a3700_spi, unsigned int len)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	if (len == 4)
+		val |= A3700_SPI_BYTE_LEN;
+	else
+		val &= ~A3700_SPI_BYTE_LEN;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	a3700_spi->byte_len = len;
+}
+
+static int a3700_spi_fifo_flush(struct a3700_spi *a3700_spi)
+{
+	int timeout = A3700_SPI_TIMEOUT;
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val |= A3700_SPI_FIFO_FLUSH;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	while (--timeout) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		if (!(val & A3700_SPI_FIFO_FLUSH))
+			return 0;
+		udelay(1);
+	}
+
+	return -ETIMEDOUT;
+}
+
+static int a3700_spi_init(struct a3700_spi *a3700_spi)
+{
+	struct spi_master *master = a3700_spi->master;
+	u32 val;
+	int i, ret = 0;
+
+	/* Reset SPI unit */
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val |= A3700_SPI_SRST;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	udelay(A3700_SPI_TIMEOUT);
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~A3700_SPI_SRST;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	/* Disable AUTO_CS and deactivate all chip-selects */
+	a3700_spi_auto_cs_unset(a3700_spi);
+	for (i = 0; i < master->num_chipselect; i++)
+		a3700_spi_deactivate_cs(a3700_spi, i);
+
+	/* Enable FIFO mode */
+	a3700_spi_fifo_mode_set(a3700_spi);
+
+	/* Set SPI mode */
+	a3700_spi_mode_set(a3700_spi, master->mode_bits);
+
+	/* Reset counters */
+	spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG, 0);
+
+	/* Mask the interrupts and clear cause bits */
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, ~0U);
+
+	return ret;
+}
+
+static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
+{
+	struct spi_master *master = dev_id;
+	struct a3700_spi *a3700_spi;
+	u32 cause;
+
+	a3700_spi = spi_master_get_devdata(master);
+
+	/* Get interrupt causes */
+	cause = spireg_read(a3700_spi, A3700_SPI_INT_STAT_REG);
+
+	if (!cause || !(a3700_spi->wait_mask & cause))
+		return IRQ_NONE;
+
+	/* mask and acknowledge the SPI interrupts */
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, cause);
+
+	/* Wake up the transfer */
+	if (a3700_spi->wait_mask & cause)
+		complete(&a3700_spi->done);
+
+	return IRQ_HANDLED;
+}
+
+static bool a3700_spi_wait_completion(struct spi_device *spi)
+{
+	struct a3700_spi *a3700_spi;
+	unsigned int timeout;
+	unsigned int ctrl_reg;
+	unsigned long timeout_jiffies;
+
+	a3700_spi = spi_master_get_devdata(spi->master);
+
+	/* SPI interrupt is edge-triggered, which means an interrupt will
+	 * be generated only when detecting a specific status bit changed
+	 * from '0' to '1'. So when we start waiting for a interrupt, we
+	 * need to check status bit in control reg first, if it is already 1,
+	 * then we do not need to wait for interrupt
+	 */
+	ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	if (a3700_spi->wait_mask & ctrl_reg)
+		return true;
+
+	reinit_completion(&a3700_spi->done);
+
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG,
+		     a3700_spi->wait_mask);
+
+	timeout_jiffies = msecs_to_jiffies(A3700_SPI_TIMEOUT);
+	timeout = wait_for_completion_timeout(&a3700_spi->done,
+					      timeout_jiffies);
+
+	a3700_spi->wait_mask = 0;
+
+	if (timeout)
+		return true;
+
+	/* there might be the case that right after we checked the
+	 * status bits in this routine and before start to wait for
+	 * interrupt by wait_for_completion_timeout, the interrupt
+	 * happens, to avoid missing it we need to double check
+	 * status bits in control reg, if it is already 1, then
+	 * consider that we have the interrupt successfully and
+	 * return true.
+	 */
+	ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	if (a3700_spi->wait_mask & ctrl_reg)
+		return true;
+
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+
+	return true;
+}
+
+static bool a3700_spi_transfer_wait(struct spi_device *spi,
+				    unsigned int bit_mask)
+{
+	struct a3700_spi *a3700_spi;
+
+	a3700_spi = spi_master_get_devdata(spi->master);
+	a3700_spi->wait_mask = bit_mask;
+
+	return a3700_spi_wait_completion(spi);
+}
+
+static void a3700_spi_fifo_thres_set(struct a3700_spi *a3700_spi,
+				     unsigned int bytes)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_RFIFO_THRS_BIT);
+	val |= (bytes - 1) << A3700_SPI_RFIFO_THRS_BIT;
+	val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_WFIFO_THRS_BIT);
+	val |= (7 - bytes) << A3700_SPI_WFIFO_THRS_BIT;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_transfer_setup(struct spi_device *spi,
+				    struct spi_transfer *xfer)
+{
+	struct a3700_spi *a3700_spi;
+	unsigned int byte_len;
+
+	a3700_spi = spi_master_get_devdata(spi->master);
+
+	a3700_spi_clock_set(a3700_spi, xfer->speed_hz, spi->mode);
+
+	byte_len = xfer->bits_per_word >> 3;
+
+	a3700_spi_fifo_thres_set(a3700_spi, byte_len);
+}
+
+static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(spi->master);
+
+	if (!enable)
+		a3700_spi_activate_cs(a3700_spi, spi->chip_select);
+	else
+		a3700_spi_deactivate_cs(a3700_spi, spi->chip_select);
+}
+
+static void a3700_spi_header_set(struct a3700_spi *a3700_spi)
+{
+	u32 instr_cnt = 0, addr_cnt = 0, dummy_cnt = 0;
+	u32 val = 0;
+
+	/* Clear the header registers */
+	spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_IF_RMODE_REG, 0);
+
+	/* Set header counters */
+	if (a3700_spi->tx_buf) {
+		if (a3700_spi->buf_len <= a3700_spi->instr_cnt) {
+			instr_cnt = a3700_spi->buf_len;
+		} else if (a3700_spi->buf_len <= (a3700_spi->instr_cnt +
+						  a3700_spi->addr_cnt)) {
+			instr_cnt = a3700_spi->instr_cnt;
+			addr_cnt = a3700_spi->buf_len - instr_cnt;
+		} else if (a3700_spi->buf_len <= a3700_spi->hdr_cnt) {
+			instr_cnt = a3700_spi->instr_cnt;
+			addr_cnt = a3700_spi->addr_cnt;
+			/* Need to handle the normal write case with 1 byte
+			 * data
+			 */
+			if (!a3700_spi->tx_buf[instr_cnt + addr_cnt])
+				dummy_cnt = a3700_spi->buf_len - instr_cnt -
+					    addr_cnt;
+		}
+		val |= ((instr_cnt & A3700_SPI_INSTR_CNT_MASK)
+			<< A3700_SPI_INSTR_CNT_BIT);
+		val |= ((addr_cnt & A3700_SPI_ADDR_CNT_MASK)
+			<< A3700_SPI_ADDR_CNT_BIT);
+		val |= ((dummy_cnt & A3700_SPI_DUMMY_CNT_MASK)
+			<< A3700_SPI_DUMMY_CNT_BIT);
+	}
+	spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, val);
+
+	/* Update the buffer length to be transferred */
+	a3700_spi->buf_len -= (instr_cnt + addr_cnt + dummy_cnt);
+
+	/* Set Instruction */
+	val = 0;
+	while (instr_cnt--) {
+		val = (val << 8) | a3700_spi->tx_buf[0];
+		a3700_spi->tx_buf++;
+	}
+	spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, val);
+
+	/* Set Address */
+	val = 0;
+	while (addr_cnt--) {
+		val = (val << 8) | a3700_spi->tx_buf[0];
+		a3700_spi->tx_buf++;
+	}
+	spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, val);
+}
+
+static int a3700_is_wfifo_full(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	return (val & A3700_SPI_WFIFO_FULL);
+}
+
+static int a3700_spi_fifo_write(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+	int i = 0;
+
+	while (!a3700_is_wfifo_full(a3700_spi) && a3700_spi->buf_len) {
+		val = 0;
+		if (a3700_spi->buf_len >= 4) {
+			val = cpu_to_le32(*(u32 *)a3700_spi->tx_buf);
+			spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+
+			a3700_spi->buf_len -= 4;
+			a3700_spi->tx_buf += 4;
+		} else {
+			/*
+			 * If the remained buffer length is less than 4-bytes,
+			 * we should pad the write buffer with all ones. So that
+			 * it avoids overwrite the unexpected bytes following
+			 * the last one.
+			 */
+			val = GENMASK(31, 0);
+			while (a3700_spi->buf_len) {
+				val &= ~(0xff << (8 * i));
+				val |= *a3700_spi->tx_buf++ << (8 * i);
+				i++;
+				a3700_spi->buf_len--;
+
+				spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG,
+					     val);
+			}
+			break;
+		}
+	}
+
+	return 0;
+}
+
+static int a3700_is_rfifo_empty(struct a3700_spi *a3700_spi)
+{
+	u32 val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+
+	return (val & A3700_SPI_RFIFO_EMPTY);
+}
+
+static int a3700_spi_fifo_read(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	while (!a3700_is_rfifo_empty(a3700_spi) && a3700_spi->buf_len) {
+		val = spireg_read(a3700_spi, A3700_SPI_DATA_IN_REG);
+		if (a3700_spi->buf_len >= 4) {
+			u32 data = le32_to_cpu(val);
+			memcpy(a3700_spi->rx_buf, &data, 4);
+
+			a3700_spi->buf_len -= 4;
+			a3700_spi->rx_buf += 4;
+		} else {
+			/*
+			 * When remain bytes is not larger than 4, we should
+			 * avoid memory overwriting and just write the left rx
+			 * buffer bytes.
+			 */
+			while (a3700_spi->buf_len) {
+				*a3700_spi->rx_buf = val & 0xff;
+				val >>= 8;
+
+				a3700_spi->buf_len--;
+				a3700_spi->rx_buf++;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static void a3700_spi_transfer_abort_fifo(struct a3700_spi *a3700_spi)
+{
+	int timeout = A3700_SPI_TIMEOUT;
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val |= A3700_SPI_XFER_STOP;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	while (--timeout) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		if (!(val & A3700_SPI_XFER_START))
+			break;
+		udelay(1);
+	}
+
+	a3700_spi_fifo_flush(a3700_spi);
+
+	val &= ~A3700_SPI_XFER_STOP;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static int a3700_spi_prepare_message(struct spi_master *master,
+				     struct spi_message *message)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	struct spi_device *spi = message->spi;
+	int ret;
+
+	ret = clk_enable(a3700_spi->clk);
+	if (ret) {
+		dev_err(&spi->dev, "failed to enable clk with error %d\n", ret);
+		return ret;
+	}
+
+	/* Flush the FIFOs */
+	ret = a3700_spi_fifo_flush(a3700_spi);
+	if (ret)
+		return ret;
+
+	a3700_spi_bytelen_set(a3700_spi, 4);
+
+	return 0;
+}
+
+static int a3700_spi_transfer_one(struct spi_master *master,
+				  struct spi_device *spi,
+				  struct spi_transfer *xfer)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	int ret = 0, timeout = A3700_SPI_TIMEOUT;
+	unsigned int nbits = 0;
+	u32 val;
+
+	a3700_spi_transfer_setup(spi, xfer);
+
+	a3700_spi->tx_buf  = xfer->tx_buf;
+	a3700_spi->rx_buf  = xfer->rx_buf;
+	a3700_spi->buf_len = xfer->len;
+
+	/* SPI transfer headers */
+	a3700_spi_header_set(a3700_spi);
+
+	if (xfer->tx_buf)
+		nbits = xfer->tx_nbits;
+	else if (xfer->rx_buf)
+		nbits = xfer->rx_nbits;
+
+	a3700_spi_pin_mode_set(a3700_spi, nbits);
+
+	if (xfer->rx_buf) {
+		/* Set read data length */
+		spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
+			     a3700_spi->buf_len);
+		/* Start READ transfer */
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val &= ~A3700_SPI_RW_EN;
+		val |= A3700_SPI_XFER_START;
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	} else if (xfer->tx_buf) {
+		/* Start Write transfer */
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val |= (A3700_SPI_XFER_START | A3700_SPI_RW_EN);
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+		/*
+		 * If there are data to be written to the SPI device, xmit_data
+		 * flag is set true; otherwise the instruction in SPI_INSTR does
+		 * not require data to be written to the SPI device, then
+		 * xmit_data flag is set false.
+		 */
+		a3700_spi->xmit_data = (a3700_spi->buf_len != 0);
+	}
+
+	while (a3700_spi->buf_len) {
+		if (a3700_spi->tx_buf) {
+			/* Wait wfifo ready */
+			if (!a3700_spi_transfer_wait(spi,
+						     A3700_SPI_WFIFO_RDY)) {
+				dev_err(&spi->dev,
+					"wait wfifo ready timed out\n");
+				ret = -ETIMEDOUT;
+				goto error;
+			}
+			/* Fill up the wfifo */
+			ret = a3700_spi_fifo_write(a3700_spi);
+			if (ret)
+				goto error;
+		} else if (a3700_spi->rx_buf) {
+			/* Wait rfifo ready */
+			if (!a3700_spi_transfer_wait(spi,
+						     A3700_SPI_RFIFO_RDY)) {
+				dev_err(&spi->dev,
+					"wait rfifo ready timed out\n");
+				ret = -ETIMEDOUT;
+				goto error;
+			}
+			/* Drain out the rfifo */
+			ret = a3700_spi_fifo_read(a3700_spi);
+			if (ret)
+				goto error;
+		}
+	}
+
+	/*
+	 * Stop a write transfer in fifo mode:
+	 *	- wait all the bytes in wfifo to be shifted out
+	 *	 - set XFER_STOP bit
+	 *	- wait XFER_START bit clear
+	 *	- clear XFER_STOP bit
+	 * Stop a read transfer in fifo mode:
+	 *	- the hardware is to reset the XFER_START bit
+	 *	   after the number of bytes indicated in DIN_CNT
+	 *	   register
+	 *	- just wait XFER_START bit clear
+	 */
+	if (a3700_spi->tx_buf) {
+		if (a3700_spi->xmit_data) {
+			/*
+			 * If there are data written to the SPI device, wait
+			 * until SPI_WFIFO_EMPTY is 1 to wait for all data to
+			 * transfer out of write FIFO.
+			 */
+			if (!a3700_spi_transfer_wait(spi,
+						     A3700_SPI_WFIFO_EMPTY)) {
+				dev_err(&spi->dev, "wait wfifo empty timed out\n");
+				return -ETIMEDOUT;
+			}
+		} else {
+			/*
+			 * If the instruction in SPI_INSTR does not require data
+			 * to be written to the SPI device, wait until SPI_RDY
+			 * is 1 for the SPI interface to be in idle.
+			 */
+			if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+				dev_err(&spi->dev, "wait xfer ready timed out\n");
+				return -ETIMEDOUT;
+			}
+		}
+
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val |= A3700_SPI_XFER_STOP;
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	}
+
+	while (--timeout) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		if (!(val & A3700_SPI_XFER_START))
+			break;
+		udelay(1);
+	}
+
+	if (timeout == 0) {
+		dev_err(&spi->dev, "wait transfer start clear timed out\n");
+		ret = -ETIMEDOUT;
+		goto error;
+	}
+
+	val &= ~A3700_SPI_XFER_STOP;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	goto out;
+
+error:
+	a3700_spi_transfer_abort_fifo(a3700_spi);
+out:
+	spi_finalize_current_transfer(master);
+
+	return ret;
+}
+
+static int a3700_spi_unprepare_message(struct spi_master *master,
+				       struct spi_message *message)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+
+	clk_disable(a3700_spi->clk);
+
+	return 0;
+}
+
+static const struct of_device_id a3700_spi_dt_ids[] = {
+	{ .compatible = "marvell,armada-3700-spi", .data = NULL },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, a3700_spi_dt_ids);
+
+static int a3700_spi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *of_node = dev->of_node;
+	struct resource *res;
+	struct spi_master *master;
+	struct a3700_spi *spi;
+	u32 num_cs = 0;
+	int ret = 0;
+
+	master = spi_alloc_master(dev, sizeof(*spi));
+	if (!master) {
+		dev_err(dev, "master allocation failed\n");
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	if (of_property_read_u32(of_node, "num-cs", &num_cs)) {
+		dev_err(dev, "could not find num-cs\n");
+		ret = -ENXIO;
+		goto error;
+	}
+
+	master->bus_num = pdev->id;
+	master->dev.of_node = of_node;
+	master->mode_bits = SPI_MODE_3;
+	master->num_chipselect = num_cs;
+	master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(32);
+	master->prepare_message =  a3700_spi_prepare_message;
+	master->transfer_one = a3700_spi_transfer_one;
+	master->unprepare_message = a3700_spi_unprepare_message;
+	master->set_cs = a3700_spi_set_cs;
+	master->flags = SPI_MASTER_HALF_DUPLEX;
+	master->mode_bits |= (SPI_RX_DUAL | SPI_RX_DUAL |
+			      SPI_RX_QUAD | SPI_TX_QUAD);
+
+	platform_set_drvdata(pdev, master);
+
+	spi = spi_master_get_devdata(master);
+	memset(spi, 0, sizeof(struct a3700_spi));
+
+	spi->master = master;
+	spi->instr_cnt = A3700_INSTR_CNT;
+	spi->addr_cnt = A3700_ADDR_CNT;
+	spi->hdr_cnt = A3700_INSTR_CNT + A3700_ADDR_CNT +
+		       A3700_DUMMY_CNT;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	spi->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(spi->base)) {
+		ret = PTR_ERR(spi->base);
+		goto error;
+	}
+
+	spi->irq = platform_get_irq(pdev, 0);
+	if (spi->irq < 0) {
+		dev_err(dev, "could not get irq: %d\n", spi->irq);
+		ret = -ENXIO;
+		goto error;
+	}
+
+	init_completion(&spi->done);
+
+	spi->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(spi->clk)) {
+		dev_err(dev, "could not find clk: %ld\n", PTR_ERR(spi->clk));
+		goto error;
+	}
+
+	ret = clk_prepare(spi->clk);
+	if (ret) {
+		dev_err(dev, "could not prepare clk: %d\n", ret);
+		goto error;
+	}
+
+	ret = a3700_spi_init(spi);
+	if (ret)
+		goto error_clk;
+
+	ret = devm_request_irq(dev, spi->irq, a3700_spi_interrupt, 0,
+			       dev_name(dev), master);
+	if (ret) {
+		dev_err(dev, "could not request IRQ: %d\n", ret);
+		goto error_clk;
+	}
+
+	ret = devm_spi_register_master(dev, master);
+	if (ret) {
+		dev_err(dev, "Failed to register master\n");
+		goto error_clk;
+	}
+
+	return 0;
+
+error_clk:
+	clk_disable_unprepare(spi->clk);
+error:
+	spi_master_put(master);
+out:
+	return ret;
+}
+
+static int a3700_spi_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct a3700_spi *spi = spi_master_get_devdata(master);
+
+	clk_unprepare(spi->clk);
+	spi_master_put(master);
+
+	return 0;
+}
+
+static struct platform_driver a3700_spi_driver = {
+	.driver = {
+		.name	= DRIVER_NAME,
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(a3700_spi_dt_ids),
+	},
+	.probe		= a3700_spi_probe,
+	.remove		= a3700_spi_remove,
+};
+
+module_platform_driver(a3700_spi_driver);
+
+MODULE_DESCRIPTION("Armada-3700 SPI driver");
+MODULE_AUTHOR("Wilson Ding <dingwei@marvell.com>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
2.9.3

^ permalink raw reply related

* [PATCH v4 0/4] Add support for the Armada 3700 SPI controller
From: Romain Perier @ 2016-12-08 14:58 UTC (permalink / raw)
  To: linux-arm-kernel

The Marvell Armada 3700 SoC includes an SPI controller. This controller
supports up to 4 SPI slave devices, with dedicated chip selects, CPIO or
FIFO mode with DMA or CPU transfers and different SPI transfer modes
(Standard single, Dual or Quad).

This set of patches adds a basic support for the FIFO mode, (CPU-side
only, DMA not supported yet). It also adds the required definitions of
the spi nodes to the devicetree.

Romain Perier (4):
  spi: Add support for Armada 3700 SPI Controller
  spi: armada-3700: Add documentation for the Armada 3700 SPI Controller
  arm64: dts: marvell: Add definition of SPI controller for Armada 3700
  arm64: dts: marvell: Enable spi0 on the board Armada-3720-db

 .../devicetree/bindings/spi/spi-armada-3700.txt    |  25 +
 arch/arm64/boot/dts/marvell/armada-3720-db.dts     |  30 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi       |  11 +
 drivers/spi/Kconfig                                |   7 +
 drivers/spi/Makefile                               |   1 +
 drivers/spi/spi-armada-3700.c                      | 923 +++++++++++++++++++++
 6 files changed, 997 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt
 create mode 100644 drivers/spi/spi-armada-3700.c

-- 
2.9.3

^ permalink raw reply

* [PATCH 2/2] ASoC: zte: spdif: correct ZX_SPDIF_CLK_RAT define
From: Jun Nie @ 2016-12-08 14:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481186655-8213-2-git-send-email-shawnguo@kernel.org>

2016-12-08 16:44 GMT+08:00 Shawn Guo <shawnguo@kernel.org>:
> From: Shawn Guo <shawn.guo@linaro.org>
>
> The macro ZX_SPDIF_CLK_RAT should be 2 instead of 4.  With this
> fix, we can get correct audio output on HDMI through SPDIF interface.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  sound/soc/zte/zx-spdif.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/soc/zte/zx-spdif.c b/sound/soc/zte/zx-spdif.c
> index 26265ce4caca..9fa6463ce5d7 100644
> --- a/sound/soc/zte/zx-spdif.c
> +++ b/sound/soc/zte/zx-spdif.c
> @@ -71,7 +71,7 @@
>  #define ZX_VALID_RIGHT_TRACK           (2 << 0)
>  #define ZX_VALID_TRACK_MASK            (3 << 0)
>
> -#define ZX_SPDIF_CLK_RAT               (4 * 32)
> +#define ZX_SPDIF_CLK_RAT               (2 * 32)
>
>  struct zx_spdif_info {
>         struct snd_dmaengine_dai_dma_data       dma_data;
> --
> 1.9.1
>

Acked-by: Jun Nie <jun.nie@linaro.org>

^ permalink raw reply

* [PATCH 2/2] clk: zte: add audio clocks for zx296718
From: Jun Nie @ 2016-12-08 14:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481189157-8995-2-git-send-email-shawnguo@kernel.org>

2016-12-08 17:25 GMT+08:00 Shawn Guo <shawnguo@kernel.org>:
> From: Jun Nie <jun.nie@linaro.org>
>
> The audio related clock support is missing from the existing zx296718
> clock driver.  Let's add it, so that the upstream ZX SPDIF driver can
> work for HDMI audio support.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  drivers/clk/zte/clk-zx296718.c | 150 +++++++++++++++++++++++++++++++++++++++++
>  drivers/clk/zte/clk.c          | 149 ++++++++++++++++++++++++++++++++++++++++
>  drivers/clk/zte/clk.h          |  28 ++++++++
>  3 files changed, 327 insertions(+)
>
> diff --git a/drivers/clk/zte/clk-zx296718.c b/drivers/clk/zte/clk-zx296718.c
> index 707d62956e9b..eed8581b1b25 100644
> --- a/drivers/clk/zte/clk-zx296718.c
> +++ b/drivers/clk/zte/clk-zx296718.c
> @@ -888,10 +888,160 @@ static int __init lsp1_clocks_init(struct device_node *np)
>         return 0;
>  }
>
> +PNAME(audio_wclk_common_p) = {
> +       "audio_99m",
> +       "audio_24m",
> +};
> +
> +PNAME(audio_timer_p) = {
> +       "audio_24m",
> +       "audio_32k",
> +};
> +
> +static struct zx_clk_mux audio_mux_clk[] = {
> +       MUX(0, "i2s0_wclk_mux", audio_wclk_common_p, AUDIO_I2S0_CLK, 0, 1),
> +       MUX(0, "i2s1_wclk_mux", audio_wclk_common_p, AUDIO_I2S1_CLK, 0, 1),
> +       MUX(0, "i2s2_wclk_mux", audio_wclk_common_p, AUDIO_I2S2_CLK, 0, 1),
> +       MUX(0, "i2s3_wclk_mux", audio_wclk_common_p, AUDIO_I2S3_CLK, 0, 1),
> +       MUX(0, "i2c0_wclk_mux", audio_wclk_common_p, AUDIO_I2C0_CLK, 0, 1),
> +       MUX(0, "spdif0_wclk_mux", audio_wclk_common_p, AUDIO_SPDIF0_CLK, 0, 1),
> +       MUX(0, "spdif1_wclk_mux", audio_wclk_common_p, AUDIO_SPDIF1_CLK, 0, 1),
> +       MUX(0, "timer_wclk_mux", audio_timer_p, AUDIO_TIMER_CLK, 0, 1),
> +};
> +
> +struct zx_clk_audio_div_table i2s_wclk_div_table[] = {
> +       {2048000, 0x3000030, 0xffff5700},
> +       {4096000, 0x3000018, 0xffff2b80},
> +       {2822400, 0x3000011, 0xffff89cb},
> +       {3072000, 0x3000010, 0xffff1d00},
> +       {4096000, 0x300000c, 0xffff15c0},
> +       {5644800, 0x3000008, 0xffffc4e5},
> +       {6144000, 0x3000008, 0xffff0e80},
> +       {11289600, 0x3000004, 0xffff6273},
> +       {12288000, 0x3000004, 0xffff0740},
> +       {22579200, 0x3000002, 0xffff3139},
> +       {24576000, 0x3000002, 0xffff03a0},
> +};
> +
> +struct zx_clk_audio_div_table spdif_wclk_div_table[] = {
> +       {2822400, 0x00023, 0xffff1397},
> +       {3072000, 0x00020, 0xffff3a00},
> +       {4096000, 0x00018, 0xffff2b80},
> +       {5644800, 0x00011, 0xffff89cb},
> +       {6144000, 0x00010, 0xffff1d00},
> +       {11289600, 0x00008, 0xffffc4e5},
> +       {12288000, 0x00008, 0xffff0e80},
> +       {22579200, 0x00004, 0xffff6273},
> +       {24576000, 0x00004, 0xffff0740},
> +};

You can remove these two tables and table pointer member as I already
removed table pointer assignment in macro AUDIO_DIV in this code. I am
sorry for not cleaning code properly.

> +
> +struct clk_zx_audio_divider audio_adiv_clk[] = {
> +       AUDIO_DIV(0, "i2s0_wclk_div", "i2s0_wclk_mux", AUDIO_I2S0_DIV_CFG1, i2s_wclk_div_table),
> +       AUDIO_DIV(0, "i2s1_wclk_div", "i2s1_wclk_mux", AUDIO_I2S1_DIV_CFG1, i2s_wclk_div_table),
> +       AUDIO_DIV(0, "i2s2_wclk_div", "i2s2_wclk_mux", AUDIO_I2S2_DIV_CFG1, i2s_wclk_div_table),
> +       AUDIO_DIV(0, "i2s3_wclk_div", "i2s3_wclk_mux", AUDIO_I2S3_DIV_CFG1, i2s_wclk_div_table),
> +       AUDIO_DIV(0, "spdif0_wclk_div", "spdif0_wclk_mux", AUDIO_SPDIF0_DIV_CFG1, spdif_wclk_div_table),
> +       AUDIO_DIV(0, "spdif1_wclk_div", "spdif1_wclk_mux", AUDIO_SPDIF1_DIV_CFG1, spdif_wclk_div_table),
> +};
> +
> +struct zx_clk_div audio_div_clk[] = {
> +       DIV_T(0, "tdm_wclk_div", "audio_16m384", AUDIO_TDM_CLK, 8, 4, 0, common_div_table),
> +};
> +
> +struct zx_clk_gate audio_gate_clk[] = {
> +       GATE(AUDIO_I2S0_WCLK, "i2s0_wclk", "i2s0_wclk_div", AUDIO_I2S0_CLK, 9, CLK_SET_RATE_PARENT, 0),
> +       GATE(AUDIO_I2S1_WCLK, "i2s1_wclk", "i2s1_wclk_div", AUDIO_I2S1_CLK, 9, CLK_SET_RATE_PARENT, 0),
> +       GATE(AUDIO_I2S2_WCLK, "i2s2_wclk", "i2s2_wclk_div", AUDIO_I2S2_CLK, 9, CLK_SET_RATE_PARENT, 0),
> +       GATE(AUDIO_I2S3_WCLK, "i2s3_wclk", "i2s3_wclk_div", AUDIO_I2S3_CLK, 9, CLK_SET_RATE_PARENT, 0),
> +       GATE(AUDIO_I2C0_WCLK, "i2c0_wclk", "i2c0_wclk_mux", AUDIO_I2C0_CLK, 9, CLK_SET_RATE_PARENT, 0),
> +       GATE(AUDIO_SPDIF0_WCLK, "spdif0_wclk", "spdif0_wclk_div", AUDIO_SPDIF0_CLK, 9, CLK_SET_RATE_PARENT, 0),
> +       GATE(AUDIO_SPDIF1_WCLK, "spdif1_wclk", "spdif1_wclk_div", AUDIO_SPDIF1_CLK, 9, CLK_SET_RATE_PARENT, 0),
> +       GATE(AUDIO_TDM_WCLK, "tdm_wclk", "tdm_wclk_div", AUDIO_TDM_CLK, 17, CLK_SET_RATE_PARENT, 0),
> +       GATE(AUDIO_TS_PCLK, "tempsensor_pclk", "clk49m5", AUDIO_TS_CLK, 1, 0, 0),
> +};
> +
> +static struct clk_hw_onecell_data audio_hw_onecell_data = {
> +       .num = AUDIO_NR_CLKS,
> +       .hws = {
> +               [AUDIO_NR_CLKS - 1] = NULL,
> +       },
> +};
> +
> +static int __init audio_clocks_init(struct device_node *np)
> +{
> +       void __iomem *reg_base;
> +       int i, ret;
> +
> +       reg_base = of_iomap(np, 0);
> +       if (!reg_base) {
> +               pr_err("%s: Unable to map audio clk base\n", __func__);
> +               return -ENXIO;
> +       }
> +
> +       for (i = 0; i < ARRAY_SIZE(audio_mux_clk); i++) {
> +               if (audio_mux_clk[i].id)
> +                       audio_hw_onecell_data.hws[audio_mux_clk[i].id] =
> +                                       &audio_mux_clk[i].mux.hw;
> +
> +               audio_mux_clk[i].mux.reg += (u64)reg_base;

Fix build test failure on 32bit system.
 audio_mux_clk[i].mux.reg +=  (uintptr_t)reg_base;

> +               ret = clk_hw_register(NULL, &audio_mux_clk[i].mux.hw);
> +               if (ret) {
> +                       pr_warn("audio clk %s init error!\n",
> +                               audio_mux_clk[i].mux.hw.init->name);
> +               }
> +       }
> +
> +       for (i = 0; i < ARRAY_SIZE(audio_adiv_clk); i++) {
> +               if (audio_adiv_clk[i].id)
> +                       audio_hw_onecell_data.hws[audio_adiv_clk[i].id] =
> +                                       &audio_adiv_clk[i].hw;
> +
> +               audio_adiv_clk[i].reg_base += (u64)reg_base;

The same to this line and below cases.

> +               ret = clk_hw_register(NULL, &audio_adiv_clk[i].hw);
> +               if (ret) {
> +                       pr_warn("audio clk %s init error!\n",
> +                               audio_adiv_clk[i].hw.init->name);
> +               }
> +       }
> +
> +       for (i = 0; i < ARRAY_SIZE(audio_div_clk); i++) {
> +               if (audio_div_clk[i].id)
> +                       audio_hw_onecell_data.hws[audio_div_clk[i].id] =
> +                                       &audio_div_clk[i].div.hw;
> +
> +               audio_div_clk[i].div.reg += (u64)reg_base;
> +               ret = clk_hw_register(NULL, &audio_div_clk[i].div.hw);
> +               if (ret) {
> +                       pr_warn("audio clk %s init error!\n",
> +                               audio_div_clk[i].div.hw.init->name);
> +               }
> +       }
> +
> +       for (i = 0; i < ARRAY_SIZE(audio_gate_clk); i++) {
> +               if (audio_gate_clk[i].id)
> +                       audio_hw_onecell_data.hws[audio_gate_clk[i].id] =
> +                                       &audio_gate_clk[i].gate.hw;
> +
> +               audio_gate_clk[i].gate.reg += (u64)reg_base;
> +               ret = clk_hw_register(NULL, &audio_gate_clk[i].gate.hw);
> +               if (ret) {
> +                       pr_warn("audio clk %s init error!\n",
> +                               audio_gate_clk[i].gate.hw.init->name);
> +               }
> +       }
> +
> +       if (of_clk_add_hw_provider(np, of_clk_hw_onecell_get, &audio_hw_onecell_data))
> +               panic("could not register clk provider\n");
> +       pr_info("audio-clk init over, nr:%d\n", AUDIO_NR_CLKS);
> +
> +       return 0;
> +}
> +
>  static const struct of_device_id zx_clkc_match_table[] = {
>         { .compatible = "zte,zx296718-topcrm", .data = &top_clocks_init },
>         { .compatible = "zte,zx296718-lsp0crm", .data = &lsp0_clocks_init },
>         { .compatible = "zte,zx296718-lsp1crm", .data = &lsp1_clocks_init },
> +       { .compatible = "zte,zx296718-audiocrm", .data = &audio_clocks_init },
>         { }
>  };
>
> diff --git a/drivers/clk/zte/clk.c b/drivers/clk/zte/clk.c
> index c4c1251bc1e7..ea97024b37aa 100644
> --- a/drivers/clk/zte/clk.c
> +++ b/drivers/clk/zte/clk.c
> @@ -9,6 +9,7 @@
>
>  #include <linux/clk-provider.h>
>  #include <linux/err.h>
> +#include <linux/gcd.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
>  #include <linux/slab.h>
> @@ -310,3 +311,151 @@ struct clk *clk_register_zx_audio(const char *name,
>
>         return clk;
>  }
> +
> +#define CLK_AUDIO_DIV_FRAC     BIT(0)
> +#define CLK_AUDIO_DIV_INT      BIT(1)
> +#define CLK_AUDIO_DIV_UNCOMMON BIT(1)
> +
> +#define CLK_AUDIO_DIV_FRAC_NSHIFT      16
> +#define CLK_AUDIO_DIV_INT_FRAC_RE      BIT(16)
> +#define CLK_AUDIO_DIV_INT_FRAC_MAX     (0xffff)
> +#define CLK_AUDIO_DIV_INT_FRAC_MIN     (0x2)
> +#define CLK_AUDIO_DIV_INT_INT_SHIFT    24
> +#define CLK_AUDIO_DIV_INT_INT_WIDTH    4
> +
> +#define to_clk_zx_audio_div(_hw) container_of(_hw, struct clk_zx_audio_divider, hw)
> +
> +static unsigned long audio_calc_rate(struct clk_zx_audio_divider *audio_div,
> +                                    u32 reg_frac, u32 reg_int,
> +                                    unsigned long parent_rate)
> +{
> +       unsigned long rate, m, n;
> +
> +       if (audio_div->table) {
> +               const struct zx_clk_audio_div_table *divt = audio_div->table;
> +
> +               for (; divt->rate; divt++) {
> +                       if ((divt->int_reg == reg_int) && (divt->frac_reg == reg_frac))
> +                               return divt->rate;
> +               }
> +       }
> +       if (audio_div->table)
> +               pr_warn("cannot found the config(int_reg:0x%x, frac_reg:0x%x) in table, we will caculate it\n",
> +                       reg_int, reg_frac);

Logic of register value table can be removed now.

> +
> +       m = reg_frac & 0xffff;
> +       n = (reg_frac >> 16) & 0xffff;
> +
> +       m = (reg_int & 0xffff) * n + m;
> +       rate = (parent_rate * n) / m;
> +
> +       return rate;
> +}
> +
> +static void audio_calc_reg(struct clk_zx_audio_divider *audio_div,
> +                          struct zx_clk_audio_div_table *div_table,
> +                          unsigned long rate, unsigned long parent_rate)
> +{
> +       unsigned int reg_int, reg_frac;
> +       unsigned long m, n, div;
> +
> +       if (audio_div->table) {
> +               const struct zx_clk_audio_div_table *divt = audio_div->table;
> +
> +               for (; divt->rate; divt++) {
> +                       if (divt->rate == rate) {
> +                               div_table->rate = divt->rate;
> +                               div_table->int_reg = divt->int_reg;
> +                               div_table->frac_reg = divt->frac_reg;
> +                               return;
> +                       }
> +               }
> +       }
> +       if (audio_div->table)
> +               pr_warn("cannot found the rate(%ld) in table, we will caculate the config\n",
> +                       rate);

Table is not used here actually neither.

> +
> +       reg_int = parent_rate / rate;
> +
> +       if (reg_int > CLK_AUDIO_DIV_INT_FRAC_MAX)
> +               reg_int = CLK_AUDIO_DIV_INT_FRAC_MAX;
> +       else if (reg_int < CLK_AUDIO_DIV_INT_FRAC_MIN)
> +               reg_int = 0;
> +       m = parent_rate - rate * reg_int;
> +       n = rate;
> +
> +       div = gcd(m, n);
> +       m = m / div;
> +       n = n / div;
> +
> +       if ((m >> 16) || (n >> 16)) {
> +               if (m > n) {
> +                       n = n * 0xffff / m;
> +                       m = 0xffff;
> +               } else {
> +                       m = m * 0xffff / n;
> +                       n = 0xffff;
> +               }
> +       }
> +       reg_frac = m | (n << 16);
> +
> +       div_table->rate = (ulong)(parent_rate * n) / ((ulong)reg_int * n + m);
> +       div_table->int_reg = reg_int;
> +       div_table->frac_reg = reg_frac;
> +}
> +
> +static unsigned long zx_audio_div_recalc_rate(struct clk_hw *hw,
> +                                         unsigned long parent_rate)
> +{
> +       struct clk_zx_audio_divider *zx_audio_div = to_clk_zx_audio_div(hw);
> +       u32 reg_frac, reg_int;
> +
> +       reg_frac = readl_relaxed(zx_audio_div->reg_base);
> +       reg_int = readl_relaxed(zx_audio_div->reg_base + 0x4);
> +
> +       return audio_calc_rate(zx_audio_div, reg_frac, reg_int, parent_rate);
> +}
> +
> +static long zx_audio_div_round_rate(struct clk_hw *hw, unsigned long rate,
> +                               unsigned long *prate)
> +{
> +       struct clk_zx_audio_divider *zx_audio_div = to_clk_zx_audio_div(hw);
> +       struct zx_clk_audio_div_table divt;
> +
> +       audio_calc_reg(zx_audio_div, &divt, rate, *prate);
> +
> +       return audio_calc_rate(zx_audio_div, divt.frac_reg, divt.int_reg, *prate);
> +}
> +
> +static int zx_audio_div_set_rate(struct clk_hw *hw, unsigned long rate,
> +                                   unsigned long parent_rate)
> +{
> +       struct clk_zx_audio_divider *zx_audio_div = to_clk_zx_audio_div(hw);
> +       struct zx_clk_audio_div_table divt;
> +       unsigned int val;
> +
> +       audio_calc_reg(zx_audio_div, &divt, rate, parent_rate);
> +       if (divt.rate != rate)
> +               pr_info("the real rate is:%ld", divt.rate);
> +
> +       writel_relaxed(divt.frac_reg, zx_audio_div->reg_base);
> +
> +       val = readl_relaxed(zx_audio_div->reg_base + 0x4);
> +       val &= ~0xffff;
> +       val |= divt.int_reg | CLK_AUDIO_DIV_INT_FRAC_RE;
> +       writel_relaxed(val, zx_audio_div->reg_base + 0x4);
> +
> +       mdelay(1);
> +
> +       val = readl_relaxed(zx_audio_div->reg_base + 0x4);
> +       val &= ~CLK_AUDIO_DIV_INT_FRAC_RE;
> +       writel_relaxed(val, zx_audio_div->reg_base + 0x4);
> +
> +       return 0;
> +}
> +
> +const struct clk_ops zx_audio_div_ops = {
> +       .recalc_rate = zx_audio_div_recalc_rate,
> +       .round_rate = zx_audio_div_round_rate,
> +       .set_rate = zx_audio_div_set_rate,
> +};
> diff --git a/drivers/clk/zte/clk.h b/drivers/clk/zte/clk.h
> index 0df3474b2cf3..6e7ccb752c24 100644
> --- a/drivers/clk/zte/clk.h
> +++ b/drivers/clk/zte/clk.h
> @@ -153,6 +153,32 @@ struct zx_clk_div {
>         .id = _id,                                                      \
>  }
>
> +struct zx_clk_audio_div_table {
> +       unsigned long rate;
> +       unsigned int int_reg;
> +       unsigned int frac_reg;
> +};
> +
> +struct clk_zx_audio_divider {
> +       struct clk_hw                           hw;
> +       void __iomem                            *reg_base;
> +       const struct zx_clk_audio_div_table     *table;
> +       unsigned int                            rate_count;
> +       spinlock_t                              *lock;
> +       u16                                     id;
> +};
> +
> +#define AUDIO_DIV(_id, _name, _parent, _reg, _table)                   \

Remove unused table here.

> +{                                                                      \
> +       .reg_base       = (void __iomem *) _reg,                        \
> +       .lock           = &clk_lock,                                    \
> +       .hw.init        = CLK_HW_INIT(_name,                            \
> +                                     _parent,                          \
> +                                     &zx_audio_div_ops,                \
> +                                     0),                               \
> +       .id = _id,                                                      \
> +}
> +
>  struct clk *clk_register_zx_pll(const char *name, const char *parent_name,
>         unsigned long flags, void __iomem *reg_base,
>         const struct zx_pll_config *lookup_table, int count, spinlock_t *lock);
> @@ -167,4 +193,6 @@ struct clk *clk_register_zx_audio(const char *name,
>                                   unsigned long flags, void __iomem *reg_base);
>
>  extern const struct clk_ops zx_pll_ops;
> +extern const struct clk_ops zx_audio_div_ops;
> +
>  #endif
> --
> 1.9.1
>

^ permalink raw reply

* [RFC PATCH net-next v3 1/2] macb: Add 1588 support in Cadence GEM.
From: Andrei.Pistirica at microchip.com @ 2016-12-08 14:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161207210416.GA27622@netboy>



> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran at gmail.com]
> Sent: Wednesday, December 07, 2016 11:04 PM
> To: Andrei Pistirica - M16132
> Cc: netdev at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org; davem at davemloft.net;
> nicolas.ferre at atmel.com; harinikatakamlinux at gmail.com;
> harini.katakam at xilinx.com; punnaia at xilinx.com; michals at xilinx.com;
> anirudh at xilinx.com; boris.brezillon at free-electrons.com;
> alexandre.belloni at free-electrons.com; tbultel at pixelsurmer.com;
> rafalo at cadence.com
> Subject: Re: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in
> Cadence GEM.
> 
> On Wed, Dec 07, 2016 at 08:39:09PM +0100, Richard Cochran wrote:
> > > +static s32 gem_ptp_max_adj(unsigned int f_nom) {
> > > +	u64 adj;
> > > +
> > > +	/* The 48 bits of seconds for the GEM overflows every:
> > > +	 * 2^48/(365.25 * 24 * 60 *60) =~ 8 925 512 years (~= 9 mil years),
> > > +	 * thus the maximum adjust frequency must not overflow CNS
> register:
> > > +	 *
> > > +	 * addend  = 10^9/nominal_freq
> > > +	 * adj_max = +/- addend*ppb_max/10^9
> > > +	 * max_ppb = (2^8-1)*nominal_freq-10^9
> > > +	 */
> > > +	adj = f_nom;
> > > +	adj *= 0xffff;
> > > +	adj -= 1000000000ULL;
> >
> > What is this computation, and how does it relate to the comment?

I considered the following simple equation: increment value at nominal frequency (which is 10^9/nominal frequency nsecs) + the maximum drift value (nsecs) <= maximum increment value@nominal frequency (which is 8bit:0xffff).
If maximum drift is written as function of nominal frequency and maximum ppb, then the equation above yields that the maximum ppb is: (2^8 - 1) *nominal_frequency - 10^9. The equation is also simplified by the fact that the drift is written as ppm + 16bit_fractions and the increment value is written as nsec + 16bit_fractions.

Rafal said that this value is hardcoded: 0x64E6, while Harini said: 250000000.

I need to dig into this...

> 
> I am not sure what you meant, but it sounds like you are on the wrong track.
> Let me explain...

Thanks.

> 
> The max_adj has nothing at all to do with the width of the time register.
> Rather, it should reflect the maximum possible change in the tuning word.
> 
> For example, with a nominal 8 ns period, the tuning word is 0x80000.
> Looking at running the clock more slowly, the slowest possible word is
> 0x00001, meaning a difference of 0x7FFFF.  This implies an adjustment of
> 0x7FFFF/0x80000 or 999998092 ppb.  Running more quickly, we can already
> have 0x100000, twice as fast, or just under 2 billion ppb.
> 
> You should consider the extreme cases to determine the most limited
> (smallest) max_adj value:
> 
> Case 1 - high frequency
> ~~~~~~~~~~~~~~~~~~~~~~~
> 
> With a nominal 1 ns period, we have the nominal tuning word 0x10000.
> The smallest is 0x1 for a difference of 0xFFFF.  This corresponds to an
> adjustment of 0xFFFF/0x10000 = .9999847412109375 or 999984741 ppb.
> 
> Case 2 - low frequency
> ~~~~~~~~~~~~~~~~~~~~~~
> 
> With a nominal 255 ns period, the nominal word is 0xFF0000, the largest
> 0xFFFFFF, and the difference is 0xFFFF.  This corresponds to and adjustment
> of 0xFFFF/0xFF0000 = .0039215087890625 or 3921508 ppb.
> 
> Since 3921508 ppb is a huge adjustment, you can simply use that as a safe
> maximum, ignoring the actual input clock.
> 
> Thanks,
> Richard
> 
> 

Regards,
Andrei

^ permalink raw reply

* [PATCH V1] pinctrl:pxa:pinctrl-pxa2xx:- No need of devm functions
From: Arvind Yadav @ 2016-12-08 14:35 UTC (permalink / raw)
  To: linux-arm-kernel

In functions pxa2xx_build_functions, the memory allocated for
'functions' is live within the function only. After the
allocation it is immediately freed with devm_kfree. There is
no need to allocate memory for 'functions' with devm function
so replace devm_kcalloc  with kcalloc and devm_kfree with kfree.

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/pinctrl/pxa/pinctrl-pxa2xx.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pxa/pinctrl-pxa2xx.c b/drivers/pinctrl/pxa/pinctrl-pxa2xx.c
index 866aa3c..47b8e3a 100644
--- a/drivers/pinctrl/pxa/pinctrl-pxa2xx.c
+++ b/drivers/pinctrl/pxa/pinctrl-pxa2xx.c
@@ -277,7 +277,7 @@ static int pxa2xx_build_functions(struct pxa_pinctrl *pctl)
 	 * alternate function, 6 * npins is an absolute high limit of the number
 	 * of functions.
 	 */
-	functions = devm_kcalloc(pctl->dev, pctl->npins * 6,
+	functions = kcalloc(pctl->npins * 6,
 				 sizeof(*functions), GFP_KERNEL);
 	if (!functions)
 		return -ENOMEM;
@@ -289,10 +289,12 @@ static int pxa2xx_build_functions(struct pxa_pinctrl *pctl)
 	pctl->functions = devm_kmemdup(pctl->dev, functions,
 				       pctl->nfuncs * sizeof(*functions),
 				       GFP_KERNEL);
-	if (!pctl->functions)
+	if (!pctl->functions) {
+		kfree(functions);
 		return -ENOMEM;
+	}
 
-	devm_kfree(pctl->dev, functions);
+	kfree(functions);
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox