* [PATCH v3 3/6] pwm: imx: support output polarity inversion
From: Rob Herring @ 2016-10-10 17:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161007151129.6043-4-bhuvanchandra.dv@toradex.com>
On Fri, Oct 07, 2016 at 08:41:26PM +0530, Bhuvanchandra DV wrote:
> From: Lothar Wassmann <LW@KARO-electronics.de>
>
> The i.MX pwm unit on i.MX27 and newer SoCs provides a configurable output
> polarity. This patch adds support to utilize this feature where available.
>
> Signed-off-by: Lothar Wa?mann <LW@KARO-electronics.de>
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> Documentation/devicetree/bindings/pwm/imx-pwm.txt | 6 +--
Acked-by: Rob Herring <robh@kernel.org>
> drivers/pwm/pwm-imx.c | 51 +++++++++++++++++++++--
> 2 files changed, 51 insertions(+), 6 deletions(-)
^ permalink raw reply
* [PATCH 2/3] arm64: hw_breakpoint: Handle inexact watchpoint addresses
From: Pratyush Anand @ 2016-10-10 17:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJt8pk9WOhebSXpbahbbLscqJcY1OOqHiA_9hHC0zmHruRPxWQ@mail.gmail.com>
On Friday 07 October 2016 10:54 PM, Pavel Labath wrote:
> However, I did notice that it does not work if we watch an address
>> which is at some offset from address programmed. For example, it works
>> when byte_mask is 0x3, but it does not work if byte_mask if 0x2 (which
>> is supported by hardware).
>>
>> I do have some patches to resolve that.
>>
>> https://github.com/pratyushanand/linux/commits/perf/upstream_arm64_devel
>>
>> I will send them for review comment after some testing.
This branch has been updated with a test code as well. So far they work
fine at my end. Planning to send them tomorrow for review.
~Pratyush
^ permalink raw reply
* [PATCH v2 5/8] dt/bindings: Update binding for PM domain idle states
From: Lina Iyer @ 2016-10-10 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4e55d6ad-8de7-72d7-7512-5e4ff2aabc79@arm.com>
On Mon, Oct 10 2016 at 09:45 -0600, Sudeep Holla wrote:
>
>
>On 07/10/16 23:36, Lina Iyer wrote:
>>Update DT bindings to describe idle states of PM domains.
>>
>>This patch is based on the original patch by Marc Titinger.
>>
>>Cc: <devicetree@vger.kernel.org>
>>Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>>Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>>Acked-by: Rob Herring <robh@kernel.org>
>>---
>> .../devicetree/bindings/power/power_domain.txt | 38 ++++++++++++++++++++++
>> 1 file changed, 38 insertions(+)
>>
>>diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>>index 025b5e7..7f8f27e 100644
>>--- a/Documentation/devicetree/bindings/power/power_domain.txt
>>+++ b/Documentation/devicetree/bindings/power/power_domain.txt
>>@@ -29,6 +29,10 @@ Optional properties:
>> specified by this binding. More details about power domain specifier are
>> available in the next section.
>>
>>+- domain-idle-states : A phandle of an idle-state that shall be soaked into a
>>+ generic domain power state. The idle state definitions are
>>+ compatible with arm,idle-state specified in [1].
>>+
>
>Please do add the following details to the binding. IMO, this binding is
>not complete in terms of specification as there are few open questions:
>
>1. What not define a standard compatible instead of "arm,idle-state" ?
> I agree it can be used, but as part of this *generic* binding, IMO
> it's better to have something generic and can be used by devices.
> Otherwise, this binding becomes CPU specific, that too ARM CPU
> specific.
>
We had gone down this path of having a separate DT bindings for domains
that is not arm,idle-state. See RFC patches. But the binding did closely
match this and it so was suggested that we use arm,idle-state which is
already defined.
>2. Now taking CPU as a special device, how does this co-exist with the
> cpu-idle-states ? Better to have some description may be in the ARM
> CPU idle binding document(not here of-course)
>
The is a binding for a generic PM domain. This has no bearing on the CPU
or its idle states. Its just that the data is compatible with
arm,idle-state.
>3. I still haven't seen any explanation for not considering complete
> hierarchical power domain representation which was raised in earlier
> versions. I had provided example for the proposal. I just saw them
> already in use in the upstream kernel by Renasas. e.g.:
> arch/arm/boot/dts/r8a73a4.dtsi
>
Hierarchical power domains have been available for few years in DT. The
OF features of domains have always supported it. Platforms are free to
define domains in hierarchy they seem fit for their SoCs. This is a
feature that is available today and is not being modified in these
patches. It will be creating confusion if I talk about hierarchical
domains which are obvious and irrelevant to this series.
> How does that fit with your proposal, though you have not made one
> yet for CPUs in this binding ? In the above file, CPUs have either
> own power domain inside the L2 one which is cluster level power
> domain.
>
Again, this series is not about the CPUs. This is about adding features
to genpd that may be used in other contexts including cpuidle in the
future.
>One must be able to get answers to these above questions with this
>binding. Until then it's *incomplete* though it may be correct.
>
I have always tried to answer all your questions. If anything remains
unclarified pls. bring it up.
Thanks,
Lina
^ permalink raw reply
* [PATCH 2/3] doc: dt: add cyclone-spi binding document
From: atull @ 2016-10-10 16:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAtXAHeZLBT-y8xTKgG4PnZA6DH1z5=FoaM4ipC3pTEVU9KR+Q@mail.gmail.com>
On Fri, 7 Oct 2016, Moritz Fischer wrote:
> > +referred to as "passive serial".
> > +The passive serial link is not technically spi, and might require extra
> > +circuits in order to play nicely with other spi slaves on the same bus.
> > +
> > +See https://www.altera.com/literature/hb/cyc/cyc_c51013.pdf
> > +
> > +Required properties:
> > +- compatible : should contain "altr,cyclonespi-fpga-mgr"
>
> Alan, do you guys have any input on the compat string?
We want to make it clear that this is for fpga configuration
using a specific programming method (passive serial).
How about altr,cyclone-ps-spi ?
>
> I think generally the bindings should go before the actual usage in
> your patch series. Meaning you wanna document the binding
> before you use it. I think this patch should be [1/3].
I agree.
Thanks,
Alan
>
> Cheers,
>
> Moritz
>
^ permalink raw reply
* Restartable Sequences benchmarks (was: Re: [PATCH v2] arm: Added support for getcpu() vDSO using TPIDRURW)
From: Mathieu Desnoyers @ 2016-10-10 16:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161010152948.GD14561@arm.com>
----- On Oct 10, 2016, at 5:29 PM, Will Deacon will.deacon at arm.com wrote:
> Hi Fredrik,
>
> [adding Mathieu -- background is getcpu() in userspace for arm]
>
> On Thu, Oct 06, 2016 at 12:17:07AM +0200, Fredrik Markstr?m wrote:
>> On Wed, Oct 5, 2016 at 9:53 PM, Russell King - ARM Linux <linux@armlinux.org.uk
>> > wrote:
>> > On Wed, Oct 05, 2016 at 06:48:05PM +0100, Robin Murphy wrote:
>> >> On 05/10/16 17:39, Fredrik Markstr?m wrote:
>> >> > The approach I suggested below with the vDSO data page will obviously
>> >> > not work on smp, so suggestions are welcome.
>> >>
>> >> Well, given that it's user-writeable, is there any reason an application
>> >> which cares couldn't simply run some per-cpu threads to call getcpu()
>> >> once and cache the result in TPIDRURW themselves? That would appear to
>> >> both raise no compatibility issues and work with existing kernels.
>> >
>> > There is - the contents of TPIDRURW is thread specific, and it moves
>> > with the thread between CPU cores. So, if a thread was running on CPU0
>> > when it cached the getcpu() value in TPIDRURW, and then migrated to CPU1,
>> > TPIDRURW would still contain 0.
>> >
>> > I'm also not in favour of changing the TPIDRURW usage to be a storage
>> > repository for the CPU number - it's far too specific a usage and seems
>> > like a waste of hardware resources to solve one problem.
>>
>> Ok, but right now it's nothing but a (architecture specific) piece of TLS,
>> which we have generic mechanisms for. From my point of view that is a waste of
>> hardware resources.
>>
>> > As Mark says, it's an ABI breaking change too, even if it is under a config
>> option.
>>
>> I can't argue with that. If it's an ABI it's an ABI, even if I can't imagine
>> why anyone would use it over normal tls... but then again, people probably do.
>>
>> So in conclusion I agree and give up.
>
> Rather than give up, you could take a look at the patches from Mathieu
> Desnoyers, that tackle this in a very different way. It's also the reason
> we've been holding off implementing an optimised getcpu in the arm64 vdso,
> because it might all well be replaced by the new restartable sequences
> approach:
>
> http://lkml.kernel.org/r/1471637274-13583-1-git-send-email-mathieu.desnoyers at efficios.com
>
> He's also got support for arch/arm/ in that series, so you could take
> them for a spin. The main thing missing at the moment is justification
> for the feature using real-world code, as requested by Linus:
>
> http://lkml.kernel.org/r/CA+55aFz+Q33m1+ju3ANaznBwYCcWo9D9WDr2=p0YLEF4gJF12g at mail.gmail.com
>
> so if your per-cpu buffer use-case is compelling in its own right (as
> opposed to a micro-benchmark), then you could chime in over there.
>
> Will
FYI, I've adapted lttng-ust ring buffer (as a POC) to rseq in a dev
branch. I see interesting speedups. See top 3-4 commits of
https://github.com/compudj/lttng-ust-dev/tree/rseq-integration
(start with "Use rseq for...").
On x86-64, we have a 7ns speedup over sched_getcpu on x86-64, and
30ns speedup by using rseq atomics on x86-64, which brings the cost
per event record down to about 100ns/event. This replaces 3 atomic
operations on the fast path. (37% speedup)
On arm32, the cpu_id acceleration gives a 327 ns/event speed increase,
which brings speed to 2000ns/event. Note that reading time on that
system does not use the vDSO (old glibc), so it implies a system call.
This accounts for 857ns/events. I don't observe speed increase nor
slowdown by using rseq instead of ll/sc atomic operations on that
specific board (Cubietruck, only has 2 cores). I suspect that boards
with more core will benefit more of replacing ll/sc by rseq atomics.
If we don't account the overhead of reading time through system call,
we get a 22% speedup.
I have extra benchmarks in this branch:
https://github.com/compudj/rseq-test
Updated ref for current rseq-enabled kernel based on 4.8:
https://github.com/compudj/linux-percpu-dev/tree/rseq-fallback
(ARM64 port would be welcome!) :)
As Will pointed out, what I'm currently looking for is real-life
benchmarks that shows benefits of rseq. I fear that the microbenchmarks
I have for the lttng-ust tracer may be dismissed as being too specific.
Most heavy users of LTTng-UST are closed source applications, so it's
not easy for me to provide numbers in real-life scenarios.
The major use-case besides per-cpu buffering/tracing AFAIU is memory
allocators. It will mainly benefit in use-cases where there are more
threads than cores in a multithreaded application. This mainly makes
sense if threads are either dedicated to specific tasks, and therefore
are often idle, or in use-cases where worker threads are expected to
block (else, if threads are not expected to block, the application
should simply have one thread per core).
Dave Watson had interesting RSS shrinkage on this stress-test program:
http://locklessinc.com/downloads/t-test1.c modified to have 500 threads.
It uses jemalloc modified to use rseq.
I reproduced it on my laptop with 100 threads, 50000 loops:
4-core, 100 threads, 50000 loops.
malloc:
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
10136 compudj 20 0 2857840 24756 1348 R 379.4 0.4 3:49.50 t-test1
real 3m20.830s
user 3m22.164s
sys 9m40.936s
upstream jemalloc:
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
21234 compudj 20 0 2227124 49300 2280 S 306.2 0.7 2:26.97 t-test1
real 1m3.297s
user 3m19.616s
sys 0m8.500s
rseq jemalloc 4.2.1:
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
25652 compudj 20 0 877956 35624 3260 S 301.2 0.5 1:26.07 t-test1
real 0m27.639s
user 1m18.172s
sys 0m1.752s
The next step to translate this into a "real-life" number would be to
run rseq-jemalloc on a facebook node, but Dave has been in vacation for
the past few weeks. Perhaps someone else at Facebook or Google could
look into this ?
Cheers,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* [PATCH RESEND] ARM: dts: keystone-k2*: Increase SPI Flash partition size for U-Boot
From: Santosh Shilimkar @ 2016-10-10 16:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161010143144.GB1041@n2100.armlinux.org.uk>
Vignesh,
On 10/10/2016 7:31 AM, Russell King - ARM Linux wrote:
> On Mon, Oct 10, 2016 at 07:41:41PM +0530, Vignesh R wrote:
>> U-Boot SPI Boot image is now more than 512KB for Keystone2 devices and
>> cannot fit into existing partition. So, increase the SPI Flash partition
>> for U-Boot to 1MB for all Keystone2 devices.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>
>> This was submitted to v4.9 merge window but was never picked up:
>> https://patchwork.kernel.org/patch/9135023/
Another point is, if you want me to pick your patch, please copy
me next time :-). AFAIK, am seeing this patch in my inbox first time.
>
> I think you need to explain why it's safe to change the layout of the
> flash partitions like this.
>
> - What is this "misc" partition?
>
> - Why is it safe to move the "misc" partition in this way?
>
> - Do users need to do anything with data stored in the "misc" partition
> when changing kernels?
>
> If the "misc" partition is simply unused space on the flash device, why
> list it in DT?
>
Thanks Russell. Yes, above clarification would be good to get first.
^ permalink raw reply
* [PATCH] kernel: irq: fix build failure
From: Sudip Mukherjee @ 2016-10-10 15:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475775403-27207-1-git-send-email-sudipm.mukherjee@gmail.com>
On Thursday 06 October 2016 11:06 PM, Sudip Mukherjee wrote:
> The allmodconfig build of powerpc is failing with the error:
> ERROR: ".irq_set_parent" [drivers/mfd/tps65217.ko] undefined!
>
> export the symbol to fix the failure.
Hi Thomas,
powerpc and arm allmodconfig builds still fails with the same error.
Build logs of next-20161010 are at:
arm at https://travis-ci.org/sudipm-mukherjee/parport/jobs/166321467
powerpc at https://travis-ci.org/sudipm-mukherjee/parport/jobs/166321473
Regards
Sudip
>
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> ---
>
> build log is at:
> https://travis-ci.org/sudipm-mukherjee/parport/jobs/165418652
>
> kernel/irq/manage.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 0c5f1a5..5072814 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -721,6 +721,7 @@ int irq_set_parent(int irq, int parent_irq)
> irq_put_desc_unlock(desc, flags);
> return 0;
> }
> +EXPORT_SYMBOL(irq_set_parent);
> #endif
>
> /*
>
^ permalink raw reply
* [PATCH v13 03/15] iommu/dma: Allow MSI-only cookies
From: Robin Murphy @ 2016-10-10 15:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b6771e0c-6f2d-21c1-05ac-8258b2bec462@redhat.com>
On 10/10/16 15:47, Auger Eric wrote:
> Hi Robin,
>
> On 10/10/2016 16:26, Robin Murphy wrote:
>> Hi Alex, Eric,
>>
>> On 06/10/16 21:17, Alex Williamson wrote:
>>> On Thu, 6 Oct 2016 08:45:19 +0000
>>> Eric Auger <eric.auger@redhat.com> wrote:
>>>
>>>> From: Robin Murphy <robin.murphy@arm.com>
>>>>
>>>> IOMMU domain users such as VFIO face a similar problem to DMA API ops
>>>> with regard to mapping MSI messages in systems where the MSI write is
>>>> subject to IOMMU translation. With the relevant infrastructure now in
>>>> place for managed DMA domains, it's actually really simple for other
>>>> users to piggyback off that and reap the benefits without giving up
>>>> their own IOVA management, and without having to reinvent their own
>>>> wheel in the MSI layer.
>>>>
>>>> Allow such users to opt into automatic MSI remapping by dedicating a
>>>> region of their IOVA space to a managed cookie.
>>>>
>>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>
>>>> ---
>>>>
>>>> v1 -> v2:
>>>> - compared to Robin's version
>>>> - add NULL last param to iommu_dma_init_domain
>>>> - set the msi_geometry aperture
>>>> - I removed
>>>> if (base < U64_MAX - size)
>>>> reserve_iova(iovad, iova_pfn(iovad, base + size), ULONG_MAX);
>>>> don't get why we would reserve something out of the scope of the iova domain?
>>>> what do I miss?
>>>> ---
>>>> drivers/iommu/dma-iommu.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>>> include/linux/dma-iommu.h | 9 +++++++++
>>>> 2 files changed, 49 insertions(+)
>>>>
>>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>>> index c5ab866..11da1a0 100644
>>>> --- a/drivers/iommu/dma-iommu.c
>>>> +++ b/drivers/iommu/dma-iommu.c
>>>> @@ -716,3 +716,43 @@ void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
>>>> msg->address_lo += lower_32_bits(msi_page->iova);
>>>> }
>>>> }
>>>> +
>>>> +/**
>>>> + * iommu_get_dma_msi_region_cookie - Configure a domain for MSI remapping only
>>>
>>> Should this perhaps be iommu_setup_dma_msi_region_cookie, or something
>>> along those lines. I'm not sure what we're get'ing. Thanks,
>>
>> What we're getting is private third-party resources for the iommu_domain
>> given in the argument. It's a get/put rather than alloc/free model since
>> we operate opaquely on the domain as a container, rather than on the
>> actual resource in question (an IOVA allocator).
>>
>> Since this particular use case is slightly different from the normal
>> flow and has special initialisation requirements, it seemed a lot
>> cleaner to simply combine that initialisation operation with the
>> prerequisite "get" into a single call. Especially as it helps emphasise
>> that this is not 'normal' DMA cookie usage.
>
> I renamed iommu_get_dma_msi_region_cookie into
> iommu_setup_dma_msi_region. Is it a problem for you?
I'd still prefer not to completely disguise the fact that it's
performing a get_cookie(), which ultimately still needs to be matched by
a put_cookie() somewhere. Really, VFIO should be doing the latter itself
before freeing the domain, as there's not strictly any guarantee that
the underlying IOMMU driver knows anything about this.
Robin.
>>
>>>
>>> Alex
>>>
>>>> + * @domain: IOMMU domain to prepare
>>>> + * @base: Base address of IOVA region to use as the MSI remapping aperture
>>>> + * @size: Size of the desired MSI aperture
>>>> + *
>>>> + * Users who manage their own IOVA allocation and do not want DMA API support,
>>>> + * but would still like to take advantage of automatic MSI remapping, can use
>>>> + * this to initialise their own domain appropriately.
>>>> + */
>>>> +int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>>>> + dma_addr_t base, u64 size)
>>>> +{
>>>> + struct iommu_dma_cookie *cookie;
>>>> + struct iova_domain *iovad;
>>>> + int ret;
>>>> +
>>>> + if (domain->type == IOMMU_DOMAIN_DMA)
>>>> + return -EINVAL;
>>>> +
>>>> + ret = iommu_get_dma_cookie(domain);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + ret = iommu_dma_init_domain(domain, base, size, NULL);
>>>> + if (ret) {
>>>> + iommu_put_dma_cookie(domain);
>>>> + return ret;
>>>> + }
>>
>> It *is* necessary to explicitly reserve the upper part of the IOVA
>> domain here - the aforementioned "special initialisation" - because
>> dma_32bit_pfn is only an optimisation hint to prevent the allocator
>> walking down from the very top of the the tree every time when devices
>> with different DMA masks share a domain (I'm in two minds as to whether
>> to tweak the way the iommu-dma code uses it in this respect, now that I
>> fully understand things). The only actual upper limit to allocation is
>> the DMA mask passed into each alloc_iova() call, so if we want to ensure
>> IOVAs are really allocated within this specific region, we have to carve
>> out everything above it.
>
> thank you for the explanation. So I will restore the reserve then.
>
> Thanks
>
> Eric
>>
>> Robin.
>>
>>>> +
>>>> + domain->msi_geometry.aperture_start = base;
>>>> + domain->msi_geometry.aperture_end = base + size - 1;
>>>> +
>>>> + cookie = domain->iova_cookie;
>>>> + iovad = &cookie->iovad;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +EXPORT_SYMBOL(iommu_get_dma_msi_region_cookie);
>>>> diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
>>>> index 32c5890..1c55413 100644
>>>> --- a/include/linux/dma-iommu.h
>>>> +++ b/include/linux/dma-iommu.h
>>>> @@ -67,6 +67,9 @@ int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
>>>> /* The DMA API isn't _quite_ the whole story, though... */
>>>> void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
>>>>
>>>> +int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>>>> + dma_addr_t base, u64 size);
>>>> +
>>>> #else
>>>>
>>>> struct iommu_domain;
>>>> @@ -90,6 +93,12 @@ static inline void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
>>>> {
>>>> }
>>>>
>>>> +static inline int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>>>> + dma_addr_t base, u64 size)
>>>> +{
>>>> + return -ENODEV;
>>>> +}
>>>> +
>>>> #endif /* CONFIG_IOMMU_DMA */
>>>> #endif /* __KERNEL__ */
>>>> #endif /* __DMA_IOMMU_H */
>>>
>>
>
^ permalink raw reply
* [PATCH v2 5/8] dt/bindings: Update binding for PM domain idle states
From: Sudeep Holla @ 2016-10-10 15:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475879821-8035-6-git-send-email-lina.iyer@linaro.org>
On 07/10/16 23:36, Lina Iyer wrote:
> Update DT bindings to describe idle states of PM domains.
>
> This patch is based on the original patch by Marc Titinger.
>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> .../devicetree/bindings/power/power_domain.txt | 38 ++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index 025b5e7..7f8f27e 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -29,6 +29,10 @@ Optional properties:
> specified by this binding. More details about power domain specifier are
> available in the next section.
>
> +- domain-idle-states : A phandle of an idle-state that shall be soaked into a
> + generic domain power state. The idle state definitions are
> + compatible with arm,idle-state specified in [1].
> +
Please do add the following details to the binding. IMO, this binding is
not complete in terms of specification as there are few open questions:
1. What not define a standard compatible instead of "arm,idle-state" ?
I agree it can be used, but as part of this *generic* binding, IMO
it's better to have something generic and can be used by devices.
Otherwise, this binding becomes CPU specific, that too ARM CPU
specific.
2. Now taking CPU as a special device, how does this co-exist with the
cpu-idle-states ? Better to have some description may be in the ARM
CPU idle binding document(not here of-course)
3. I still haven't seen any explanation for not considering complete
hierarchical power domain representation which was raised in earlier
versions. I had provided example for the proposal. I just saw them
already in use in the upstream kernel by Renasas. e.g.:
arch/arm/boot/dts/r8a73a4.dtsi
How does that fit with your proposal, though you have not made one
yet for CPUs in this binding ? In the above file, CPUs have either
own power domain inside the L2 one which is cluster level power
domain.
One must be able to get answers to these above questions with this
binding. Until then it's *incomplete* though it may be correct.
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH] arm64: mm: Fix memmap to be initialized for the entire section
From: David Daney @ 2016-10-10 15:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475747527-32387-1-git-send-email-rrichter@cavium.com>
On 10/06/2016 02:52 AM, Robert Richter wrote:
> There is a memory setup problem on ThunderX systems with certain
> memory configurations. The symptom is
>
> kernel BUG at mm/page_alloc.c:1848!
>
> This happens for some configs with 64k page size enabled. The bug
> triggers for page zones with some pages in the zone not assigned to
> this particular zone. In my case some pages that are marked as nomap
> were not reassigned to the new zone of node 1, so those are still
> assigned to node 0.
>
> The reason for the mis-configuration is a change in pfn_valid() which
> reports pages marked nomap as invalid:
>
> 68709f45385a arm64: only consider memblocks with NOMAP cleared for linear mapping
>
> This causes pages marked as nomap being no long reassigned to the new
> zone in memmap_init_zone() by calling __init_single_pfn().
>
> Fixing this by restoring the old behavior of pfn_valid() to use
> memblock_is_memory(). Also changing users of pfn_valid() in arm64 code
> to use memblock_is_map_memory() where necessary. This only affects
> code in ioremap.c. The code in mmu.c still can use the new version of
> pfn_valid().
>
> Should be marked stable v4.5..
In that case you should add:
Cc: <stable@vger.kernel.org> # 4.5.x-
here.
>
> Signed-off-by: Robert Richter <rrichter@cavium.com>
[...]
^ permalink raw reply
* [PATCH v2] arm: Added support for getcpu() vDSO using TPIDRURW
From: Will Deacon @ 2016-10-10 15:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKdL+dSt+cBCpwW5q+VCQh+7XeKrnyJgfTsEsuo2nKoUr9ytxw@mail.gmail.com>
Hi Fredrik,
[adding Mathieu -- background is getcpu() in userspace for arm]
On Thu, Oct 06, 2016 at 12:17:07AM +0200, Fredrik Markstr?m wrote:
> On Wed, Oct 5, 2016 at 9:53 PM, Russell King - ARM Linux <linux@armlinux.org.uk
> > wrote:
> > On Wed, Oct 05, 2016 at 06:48:05PM +0100, Robin Murphy wrote:
> >> On 05/10/16 17:39, Fredrik Markstr?m wrote:
> >> > The approach I suggested below with the vDSO data page will obviously
> >> > not work on smp, so suggestions are welcome.
> >>
> >> Well, given that it's user-writeable, is there any reason an application
> >> which cares couldn't simply run some per-cpu threads to call getcpu()
> >> once and cache the result in TPIDRURW themselves? That would appear to
> >> both raise no compatibility issues and work with existing kernels.
> >
> > There is - the contents of TPIDRURW is thread specific, and it moves
> > with the thread between CPU cores. So, if a thread was running on CPU0
> > when it cached the getcpu() value in TPIDRURW, and then migrated to CPU1,
> > TPIDRURW would still contain 0.
> >
> > I'm also not in favour of changing the TPIDRURW usage to be a storage
> > repository for the CPU number - it's far too specific a usage and seems
> > like a waste of hardware resources to solve one problem.
>
> Ok, but right now it's nothing but a (architecture specific) piece of TLS,
> which we have generic mechanisms for. From my point of view that is a waste of
> hardware resources.
>
> > As Mark says, it's an ABI breaking change too, even if it is under a config
> option.
>
> I can't argue with that. If it's an ABI it's an ABI, even if I can't imagine
> why anyone would use it over normal tls... but then again, people probably do.
>
> So in conclusion I agree and give up.
Rather than give up, you could take a look at the patches from Mathieu
Desnoyers, that tackle this in a very different way. It's also the reason
we've been holding off implementing an optimised getcpu in the arm64 vdso,
because it might all well be replaced by the new restartable sequences
approach:
http://lkml.kernel.org/r/1471637274-13583-1-git-send-email-mathieu.desnoyers at efficios.com
He's also got support for arch/arm/ in that series, so you could take
them for a spin. The main thing missing at the moment is justification
for the feature using real-world code, as requested by Linus:
http://lkml.kernel.org/r/CA+55aFz+Q33m1+ju3ANaznBwYCcWo9D9WDr2=p0YLEF4gJF12g at mail.gmail.com
so if your per-cpu buffer use-case is compelling in its own right (as
opposed to a micro-benchmark), then you could chime in over there.
Will
^ permalink raw reply
* [PATCH v4 03/10] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac
From: Rob Herring @ 2016-10-10 15:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161010123023.GG3462@lukather>
On Mon, Oct 10, 2016 at 02:30:23PM +0200, Maxime Ripard wrote:
> On Fri, Oct 07, 2016 at 10:25:50AM +0200, Corentin Labbe wrote:
> > This patch adds documentation for Device-Tree bindings for the
> > Allwinner sun8i-emac driver.
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> > .../bindings/net/allwinner,sun8i-emac.txt | 70 ++++++++++++++++++++++
> > 1 file changed, 70 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > new file mode 100644
> > index 0000000..92e4ef3b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > @@ -0,0 +1,70 @@
> > +* Allwinner sun8i EMAC ethernet controller
> > +
> > +Required properties:
> > +- compatible: should be one of the following string:
> > + "allwinner,sun8i-a83t-emac"
> > + "allwinner,sun8i-h3-emac"
> > + "allwinner,sun50i-a64-emac"
> > +- reg: address and length of the register for the device.
> > +- syscon: A phandle to the syscon of the SoC
> > +- interrupts: interrupt for the device
> > +- clocks: A phandle to the reference clock for this device
> > +- clock-names: should be "ahb"
> > +- resets: A phandle to the reset control for this device
> > +- reset-names: should be "ahb"
> > +- phy-mode: See ethernet.txt
> > +- phy-handle: See ethernet.txt
> > +- #address-cells: shall be 1
> > +- #size-cells: shall be 0
> > +
> > +Optional properties:
> > +- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. Default is 0)
> > +- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. Default is 0)
> > +Both delay properties does not have units, there are arbitrary value.
> > +The TX/RX clock delay chain settings are board specific and could be found
> > +in vendor FEX files.
> > +
> > +Optional properties for "allwinner,sun8i-h3-emac":
> > +- allwinner,leds-active-low: EPHY LEDs are active low
> > +
> > +Required child node of emac:
> > +- mdio bus node: should be named mdio
> > +
> > +Required properties of the mdio node:
> > +- #address-cells: shall be 1
> > +- #size-cells: shall be 0
> > +
> > +The device node referenced by "phy" or "phy-handle" should be a child node
> > +of the mdio node. See phy.txt for the generic PHY bindings.
> > +
> > +Required properties of the phy node with "allwinner,sun8i-h3-emac":
> > +- clocks: an extra phandle to the reference clock for the EPHY
> > +- resets: an extra phandle to the reset control for the EPHY
> > +
> > +Example:
> > +
> > +emac: ethernet at 01c0b000 {
> > + compatible = "allwinner,sun8i-h3-emac";
> > + syscon = <&syscon>;
> > + reg = <0x01c0b000 0x104>;
> > + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> > + resets = <&ccu RST_BUS_EMAC>;
> > + reset-names = "ahb";
> > + clocks = <&ccu CLK_BUS_EMAC>;
> > + clock-names = "ahb";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + phy = <&int_mii_phy>;
> > + phy-mode = "mii";
> > + allwinner,leds-active-low;
> > + mdio: mdio {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + int_mii_phy: ethernet-phy at 1 {
> > + reg = <1>;
> > + clocks = <&ccu CLK_BUS_EPHY>;
> > + resets = <&ccu RST_BUS_EPHY>;
>
> That works for me, let's see how the DT maintainers feel about it.
The phy should have a compatible string since you have extra properties.
Rob
^ permalink raw reply
* [PATCH v4 03/10] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac
From: Rob Herring @ 2016-10-10 15:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475828757-926-4-git-send-email-clabbe.montjoie@gmail.com>
On Fri, Oct 07, 2016 at 10:25:50AM +0200, Corentin Labbe wrote:
> This patch adds documentation for Device-Tree bindings for the
> Allwinner sun8i-emac driver.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> .../bindings/net/allwinner,sun8i-emac.txt | 70 ++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
>
> diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> new file mode 100644
> index 0000000..92e4ef3b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> @@ -0,0 +1,70 @@
> +* Allwinner sun8i EMAC ethernet controller
> +
> +Required properties:
> +- compatible: should be one of the following string:
> + "allwinner,sun8i-a83t-emac"
> + "allwinner,sun8i-h3-emac"
> + "allwinner,sun50i-a64-emac"
> +- reg: address and length of the register for the device.
> +- syscon: A phandle to the syscon of the SoC
> +- interrupts: interrupt for the device
> +- clocks: A phandle to the reference clock for this device
> +- clock-names: should be "ahb"
> +- resets: A phandle to the reset control for this device
> +- reset-names: should be "ahb"
> +- phy-mode: See ethernet.txt
> +- phy-handle: See ethernet.txt
> +- #address-cells: shall be 1
> +- #size-cells: shall be 0
> +
> +Optional properties:
> +- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. Default is 0)
> +- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. Default is 0)
> +Both delay properties does not have units, there are arbitrary value.
They have to have some sort of units. Some number of clocks perhaps. Or
just say what register field they correspond to.
> +The TX/RX clock delay chain settings are board specific and could be found
> +in vendor FEX files.
> +
> +Optional properties for "allwinner,sun8i-h3-emac":
> +- allwinner,leds-active-low: EPHY LEDs are active low
> +
> +Required child node of emac:
> +- mdio bus node: should be named mdio
> +
> +Required properties of the mdio node:
> +- #address-cells: shall be 1
> +- #size-cells: shall be 0
> +
> +The device node referenced by "phy" or "phy-handle" should be a child node
> +of the mdio node. See phy.txt for the generic PHY bindings.
> +
> +Required properties of the phy node with "allwinner,sun8i-h3-emac":
> +- clocks: an extra phandle to the reference clock for the EPHY
> +- resets: an extra phandle to the reset control for the EPHY
> +
> +Example:
> +
> +emac: ethernet at 01c0b000 {
Drop leading 0.
> + compatible = "allwinner,sun8i-h3-emac";
> + syscon = <&syscon>;
> + reg = <0x01c0b000 0x104>;
> + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> + resets = <&ccu RST_BUS_EMAC>;
> + reset-names = "ahb";
> + clocks = <&ccu CLK_BUS_EMAC>;
> + clock-names = "ahb";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy = <&int_mii_phy>;
> + phy-mode = "mii";
> + allwinner,leds-active-low;
> + mdio: mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy at 1 {
> + reg = <1>;
> + clocks = <&ccu CLK_BUS_EPHY>;
> + resets = <&ccu RST_BUS_EPHY>;
> + };
> + };
> +};
> --
> 2.7.3
>
^ permalink raw reply
* [PATCH V5 01/10] Documentation: DT: qcom_hidma: update binding for MSI
From: Rob Herring @ 2016-10-10 15:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475817915-11976-2-git-send-email-okaya@codeaurora.org>
On Fri, Oct 07, 2016 at 01:25:06AM -0400, Sinan Kaya wrote:
> Adding a new binding for qcom,hidma-1.1 to distinguish HW supporting
> MSI interrupts from the older revision.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v2 1/8] PM / Domains: Make genpd state allocation dynamic
From: Lina Iyer @ 2016-10-10 15:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFo1wFdNo3hR9WUZJ=sonKi4_sWb1PUFvPvyHeavbvqLvw@mail.gmail.com>
On Mon, Oct 10 2016 at 02:40 -0600, Ulf Hansson wrote:
>On 8 October 2016 at 00:36, Lina Iyer <lina.iyer@linaro.org> wrote:
>> Allow PM Domain states to be defined dynamically by the drivers. This
>> removes the limitation on the maximum number of states possible for a
>> domain.
>>
>> Cc: Axel Haslam <ahaslam+renesas@baylibre.com>
>> Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> ---
>> arch/arm/mach-imx/gpc.c | 17 ++++++++++-------
>> drivers/base/power/domain.c | 36 ++++++++++++++++++++++++------------
>> include/linux/pm_domain.h | 5 ++---
>> 3 files changed, 36 insertions(+), 22 deletions(-)
>>
>> diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
>> index 0df062d..57a410b 100644
>> --- a/arch/arm/mach-imx/gpc.c
>> +++ b/arch/arm/mach-imx/gpc.c
>> @@ -380,13 +380,6 @@ static struct pu_domain imx6q_pu_domain = {
>> .name = "PU",
>> .power_off = imx6q_pm_pu_power_off,
>> .power_on = imx6q_pm_pu_power_on,
>> - .states = {
>> - [0] = {
>> - .power_off_latency_ns = 25000,
>> - .power_on_latency_ns = 2000000,
>> - },
>> - },
>> - .state_count = 1,
>> },
>> };
>>
>> @@ -430,6 +423,16 @@ static int imx_gpc_genpd_init(struct device *dev, struct regulator *pu_reg)
>> if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS))
>> return 0;
>>
>> + imx6q_pu_domain.base.states = devm_kzalloc(dev,
>> + sizeof(*imx6q_pu_domain.base.states),
>> + GFP_KERNEL);
>> + if (!imx6q_pu_domain.base.states)
>> + return -ENOMEM;
>> +
>> + imx6q_pu_domain.base.states[0].power_off_latency_ns = 25000;
>> + imx6q_pu_domain.base.states[0].power_on_latency_ns = 2000000;
>> + imx6q_pu_domain.base.state_count = 1;
>> +
>> pm_genpd_init(&imx6q_pu_domain.base, NULL, false);
>> return of_genpd_add_provider_onecell(dev->of_node,
>> &imx_gpc_onecell_data);
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index e023066..4e87170 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -1282,6 +1282,21 @@ out:
>> }
>> EXPORT_SYMBOL_GPL(pm_genpd_remove_subdomain);
>>
>> +static int genpd_set_default_power_state(struct generic_pm_domain *genpd)
>> +{
>> + struct genpd_power_state *state;
>> +
>> + state = kzalloc(sizeof(*state), GFP_KERNEL);
>> + if (!state)
>> + return -ENOMEM;
>> +
>> + genpd->states = state;
>> + genpd->state_count = 1;
>> + genpd->free = state;
>> +
>> + return 0;
>> +}
>> +
>> /**
>> * pm_genpd_init - Initialize a generic I/O PM domain object.
>> * @genpd: PM domain object to initialize.
>> @@ -1293,6 +1308,8 @@ EXPORT_SYMBOL_GPL(pm_genpd_remove_subdomain);
>> int pm_genpd_init(struct generic_pm_domain *genpd,
>> struct dev_power_governor *gov, bool is_off)
>> {
>> + int ret;
>> +
>> if (IS_ERR_OR_NULL(genpd))
>> return -EINVAL;
>>
>> @@ -1325,19 +1342,12 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
>> genpd->dev_ops.start = pm_clk_resume;
>> }
>>
>> - if (genpd->state_idx >= GENPD_MAX_NUM_STATES) {
>> - pr_warn("Initial state index out of bounds.\n");
>> - genpd->state_idx = GENPD_MAX_NUM_STATES - 1;
>> - }
>> -
>> - if (genpd->state_count > GENPD_MAX_NUM_STATES) {
>> - pr_warn("Limiting states to %d\n", GENPD_MAX_NUM_STATES);
>> - genpd->state_count = GENPD_MAX_NUM_STATES;
>> - }
>> -
>> /* Use only one "off" state if there were no states declared */
>> - if (genpd->state_count == 0)
>> - genpd->state_count = 1;
>> + if (genpd->state_count == 0) {
>> + ret = genpd_set_default_power_state(genpd);
>> + if (ret)
>> + return ret;
>> + }
>>
>> mutex_lock(&gpd_list_lock);
>> list_add(&genpd->gpd_list_node, &gpd_list);
>> @@ -1374,6 +1384,8 @@ static int genpd_remove(struct generic_pm_domain *genpd)
>> kfree(link);
>> }
>>
>> + kfree(genpd->free);
>> +
>
>To be safe, let's move this after cancel_work_sync() - as to prevent
>no accesses is made to ->states pointer after you have freed it.
>
OK
Thanks,
Lina
>> list_del(&genpd->gpd_list_node);
>> mutex_unlock(&genpd->lock);
>> cancel_work_sync(&genpd->power_off_work);
>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>> index a09fe5c..de1d8f3 100644
>> --- a/include/linux/pm_domain.h
>> +++ b/include/linux/pm_domain.h
>> @@ -19,8 +19,6 @@
>> /* Defines used for the flags field in the struct generic_pm_domain */
>> #define GENPD_FLAG_PM_CLK (1U << 0) /* PM domain uses PM clk */
>>
>> -#define GENPD_MAX_NUM_STATES 8 /* Number of possible low power states */
>> -
>> enum gpd_status {
>> GPD_STATE_ACTIVE = 0, /* PM domain is active */
>> GPD_STATE_POWER_OFF, /* PM domain is off */
>> @@ -70,9 +68,10 @@ struct generic_pm_domain {
>> void (*detach_dev)(struct generic_pm_domain *domain,
>> struct device *dev);
>> unsigned int flags; /* Bit field of configs for genpd */
>> - struct genpd_power_state states[GENPD_MAX_NUM_STATES];
>> + struct genpd_power_state *states;
>> unsigned int state_count; /* number of states */
>> unsigned int state_idx; /* state that genpd will go to when off */
>> + void *free; /* Free the state that was allocated for default */
>>
>> };
>>
>> --
>> 2.7.4
>>
>
>After the minor change suggested above, you may add my ack.
>
>Kind regards
>Uffe
^ permalink raw reply
* [PATCH v2 3/8] PM / Domains: Allow domain power states to be read from DT
From: Lina Iyer @ 2016-10-10 15:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFo62XuwsoqqPyFa7FYMutd2ng-xt9f7aFh_2T6ab=vAZg@mail.gmail.com>
On Mon, Oct 10 2016 at 04:01 -0600, Ulf Hansson wrote:
>On 8 October 2016 at 00:36, Lina Iyer <lina.iyer@linaro.org> wrote:
>> This patch allows domains to define idle states in the DT. SoC's can
>> define domain idle states in DT using the "domain-idle-states" property
>> of the domain provider. Add API to read the idle states from DT that can
>> be set in the genpd object.
>>
>> This patch is based on the original patch by Marc Titinger.
>>
>> Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> ---
>> drivers/base/power/domain.c | 95 +++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/pm_domain.h | 8 ++++
>> 2 files changed, 103 insertions(+)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 4e87170..4208b67 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -1917,6 +1917,101 @@ out:
>> return ret ? -EPROBE_DEFER : 0;
>> }
>> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>> +
>> +static const struct of_device_id idle_state_match[] = {
>> + { .compatible = "arm,idle-state", },
>> + { }
>> +};
>> +
>> +static int genpd_parse_state(struct genpd_power_state *genpd_state,
>> + struct device_node *state_node)
>> +{
>> + int err;
>> + u32 residency;
>> + u32 entry_latency, exit_latency;
>> + const struct of_device_id *match_id;
>> +
>> + match_id = of_match_node(idle_state_match, state_node);
>> + if (!match_id)
>> + return -EINVAL;
>> +
>> + err = of_property_read_u32(state_node, "entry-latency-us",
>> + &entry_latency);
>> + if (err) {
>> + pr_debug(" * %s missing entry-latency-us property\n",
>> + state_node->full_name);
>> + return -EINVAL;
>> + }
>> +
>> + err = of_property_read_u32(state_node, "exit-latency-us",
>> + &exit_latency);
>> + if (err) {
>> + pr_debug(" * %s missing exit-latency-us property\n",
>> + state_node->full_name);
>> + return -EINVAL;
>> + }
>> +
>> + err = of_property_read_u32(state_node, "min-residency-us", &residency);
>> + if (!err)
>> + genpd_state->residency_ns = 1000 * residency;
>> +
>> + genpd_state->power_on_latency_ns = 1000 * exit_latency;
>> + genpd_state->power_off_latency_ns = 1000 * entry_latency;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * of_genpd_parse_idle_states: Return array of idle states for the genpd.
>> + *
>> + * @dn: The genpd device node
>> + * @states: The pointer to which the state array will be saved.
>> + * @n: The count of elements in the array returned from this function.
>> + *
>> + * Returns the device states parsed from the OF node. The memory for the states
>> + * is allocated by this function and is the responsibility of the caller to
>> + * free the memory after use.
>> + */
>> +int of_genpd_parse_idle_states(struct device_node *dn,
>> + struct genpd_power_state **states, int *n)
>
>Instead of taking **states as a parameter, let's instead return it as
>a pointer for the allocated struct. In case of failures, let's return
>ERR_PTR().
>
Hmm.. I thought about it. There are 2 return values from this function.
If we return a pointer to the allocated memory, we still have to return
the size of it as an argument. I wasn't happy splitting the return
values in 2 different places.
>> +{
>> + struct genpd_power_state *st;
>> + struct device_node *np;
>> + int i = 0;
>> + int err, ret;
>> + int count;
>> + struct of_phandle_iterator it;
>> +
>> + count = of_count_phandle_with_args(dn, "domain-idle-states", NULL);
>
>If count is zero or an error, we should return an error code (ERR_PTR()). Right?
>
OK
>> +
>> + st = kcalloc(count, sizeof(*st), GFP_KERNEL);
>> + if (!st)
>> + return -ENOMEM;
>> +
>> + /* Loop over the phandles until all the requested entry is found */
>> + of_for_each_phandle(&it, err, dn, "domain-idle-states", NULL, 0) {
>> + np = of_node_get(it.node);
>
>I don't think you need to increment the usage count for the device
>node as that is already managed by of_for_each_phandle().
>
>It's only in the error case below, when it's needed.
>
Hmm.. Didn't realize that.. will fix.
>> + ret = genpd_parse_state(&st[i++], np);
>> + if (ret) {
>> + pr_err
>> + ("Parsing idle state node %s failed with err %d\n",
>> + np->full_name, ret);
>> + of_node_put(np);
>> + goto fail;
>
>The goto seems unnecessary. Why not deal with all error handling here
>and return the error code?
>
>> + }
>> + of_node_put(np);
>
>According the comment above, you should be able to remove this.
>
>> + }
>> +
>> + *n = count;
>> + *states = st;
>> +
>> + return 0;
>> +fail:
>> + kfree(st);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(of_genpd_parse_idle_states);
>
>Please use EXPORT_SYMBOL_GPL() instead.
>
Hmm.. OK
Thanks,
Lina
>> +
>> #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
>>
>>
>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>> index f4492eb..b489496 100644
>> --- a/include/linux/pm_domain.h
>> +++ b/include/linux/pm_domain.h
>> @@ -205,6 +205,8 @@ extern int of_genpd_add_device(struct of_phandle_args *args,
>> extern int of_genpd_add_subdomain(struct of_phandle_args *parent,
>> struct of_phandle_args *new_subdomain);
>> extern struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
>> +extern int of_genpd_parse_idle_states(struct device_node *dn,
>> + struct genpd_power_state **states, int *n);
>>
>> int genpd_dev_pm_attach(struct device *dev);
>> #else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
>> @@ -234,6 +236,12 @@ static inline int of_genpd_add_subdomain(struct of_phandle_args *parent,
>> return -ENODEV;
>> }
>>
>> +static inline int of_genpd_parse_idle_states(struct device_node *dn,
>> + struct genpd_power_state **states, int *n)
>> +{
>> + return -ENODEV;
>> +}
>> +
>> static inline int genpd_dev_pm_attach(struct device *dev)
>> {
>> return -ENODEV;
>> --
>> 2.7.4
>>
>
>Kind regards
>Uffe
^ permalink raw reply
* [PATCH v13 15/15] vfio/type1: Return the MSI geometry through VFIO_IOMMU_GET_INFO capability chains
From: Auger Eric @ 2016-10-10 15:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161007143811.1fb3ebfe@t450s.home>
Hi Alex,
On 07/10/2016 22:38, Alex Williamson wrote:
> On Fri, 7 Oct 2016 19:10:27 +0200
> Auger Eric <eric.auger@redhat.com> wrote:
>
>> Hi Alex,
>>
>> On 06/10/2016 22:42, Alex Williamson wrote:
>>> On Thu, 6 Oct 2016 14:20:40 -0600
>>> Alex Williamson <alex.williamson@redhat.com> wrote:
>>>
>>>> On Thu, 6 Oct 2016 08:45:31 +0000
>>>> Eric Auger <eric.auger@redhat.com> wrote:
>>>>
>>>>> This patch allows the user-space to retrieve the MSI geometry. The
>>>>> implementation is based on capability chains, now also added to
>>>>> VFIO_IOMMU_GET_INFO.
>>>>>
>>>>> The returned info comprise:
>>>>> - whether the MSI IOVA are constrained to a reserved range (x86 case) and
>>>>> in the positive, the start/end of the aperture,
>>>>> - or whether the IOVA aperture need to be set by the userspace. In that
>>>>> case, the size and alignment of the IOVA window to be provided are
>>>>> returned.
>>>>>
>>>>> In case the userspace must provide the IOVA aperture, we currently report
>>>>> a size/alignment based on all the doorbells registered by the host kernel.
>>>>> This may exceed the actual needs.
>>>>>
>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>>
>>>>> ---
>>>>> v11 -> v11:
>>>>> - msi_doorbell_pages was renamed msi_doorbell_calc_pages
>>>>>
>>>>> v9 -> v10:
>>>>> - move cap_offset after iova_pgsizes
>>>>> - replace __u64 alignment by __u32 order
>>>>> - introduce __u32 flags in vfio_iommu_type1_info_cap_msi_geometry and
>>>>> fix alignment
>>>>> - call msi-doorbell API to compute the size/alignment
>>>>>
>>>>> v8 -> v9:
>>>>> - use iommu_msi_supported flag instead of programmable
>>>>> - replace IOMMU_INFO_REQUIRE_MSI_MAP flag by a more sophisticated
>>>>> capability chain, reporting the MSI geometry
>>>>>
>>>>> v7 -> v8:
>>>>> - use iommu_domain_msi_geometry
>>>>>
>>>>> v6 -> v7:
>>>>> - remove the computation of the number of IOVA pages to be provisionned.
>>>>> This number depends on the domain/group/device topology which can
>>>>> dynamically change. Let's rely instead rely on an arbitrary max depending
>>>>> on the system
>>>>>
>>>>> v4 -> v5:
>>>>> - move msi_info and ret declaration within the conditional code
>>>>>
>>>>> v3 -> v4:
>>>>> - replace former vfio_domains_require_msi_mapping by
>>>>> more complex computation of MSI mapping requirements, especially the
>>>>> number of pages to be provided by the user-space.
>>>>> - reword patch title
>>>>>
>>>>> RFC v1 -> v1:
>>>>> - derived from
>>>>> [RFC PATCH 3/6] vfio: Extend iommu-info to return MSIs automap state
>>>>> - renamed allow_msi_reconfig into require_msi_mapping
>>>>> - fixed VFIO_IOMMU_GET_INFO
>>>>> ---
>>>>> drivers/vfio/vfio_iommu_type1.c | 78 ++++++++++++++++++++++++++++++++++++++++-
>>>>> include/uapi/linux/vfio.h | 32 ++++++++++++++++-
>>>>> 2 files changed, 108 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
>>>>> index dc3ee5d..ce5e7eb 100644
>>>>> --- a/drivers/vfio/vfio_iommu_type1.c
>>>>> +++ b/drivers/vfio/vfio_iommu_type1.c
>>>>> @@ -38,6 +38,8 @@
>>>>> #include <linux/workqueue.h>
>>>>> #include <linux/dma-iommu.h>
>>>>> #include <linux/msi-doorbell.h>
>>>>> +#include <linux/irqdomain.h>
>>>>> +#include <linux/msi.h>
>>>>>
>>>>> #define DRIVER_VERSION "0.2"
>>>>> #define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
>>>>> @@ -1101,6 +1103,55 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu)
>>>>> return ret;
>>>>> }
>>>>>
>>>>> +static int compute_msi_geometry_caps(struct vfio_iommu *iommu,
>>>>> + struct vfio_info_cap *caps)
>>>>> +{
>>>>> + struct vfio_iommu_type1_info_cap_msi_geometry *vfio_msi_geometry;
>>>>> + unsigned long order = __ffs(vfio_pgsize_bitmap(iommu));
>>>>> + struct iommu_domain_msi_geometry msi_geometry;
>>>>> + struct vfio_info_cap_header *header;
>>>>> + struct vfio_domain *d;
>>>>> + bool reserved;
>>>>> + size_t size;
>>>>> +
>>>>> + mutex_lock(&iommu->lock);
>>>>> + /* All domains have same require_msi_map property, pick first */
>>>>> + d = list_first_entry(&iommu->domain_list, struct vfio_domain, next);
>>>>> + iommu_domain_get_attr(d->domain, DOMAIN_ATTR_MSI_GEOMETRY,
>>>>> + &msi_geometry);
>>>>> + reserved = !msi_geometry.iommu_msi_supported;
>>>>> +
>>>>> + mutex_unlock(&iommu->lock);
>>>>> +
>>>>> + size = sizeof(*vfio_msi_geometry);
>>>>> + header = vfio_info_cap_add(caps, size,
>>>>> + VFIO_IOMMU_TYPE1_INFO_CAP_MSI_GEOMETRY, 1);
>>>>> +
>>>>> + if (IS_ERR(header))
>>>>> + return PTR_ERR(header);
>>>>> +
>>>>> + vfio_msi_geometry = container_of(header,
>>>>> + struct vfio_iommu_type1_info_cap_msi_geometry,
>>>>> + header);
>>>>> +
>>>>> + vfio_msi_geometry->flags = reserved;
>>>>
>>>> Use the bit flag VFIO_IOMMU_MSI_GEOMETRY_RESERVED
>>>>
>>>>> + if (reserved) {
>>>>> + vfio_msi_geometry->aperture_start = msi_geometry.aperture_start;
>>>>> + vfio_msi_geometry->aperture_end = msi_geometry.aperture_end;
>>>>
>>>> But maybe nobody has set these, did you intend to use
>>>> iommu_domain_msi_aperture_valid(), which you defined early on but never
>>>> used?
>>>>
>>>>> + return 0;
>>>>> + }
>>>>> +
>>>>> + vfio_msi_geometry->order = order;
>>>>
>>>> I'm tempted to suggest that a user could do the same math on their own
>>>> since we provide the supported bitmap already... could it ever not be
>>>> the same?
>>>>
>>>>> + /*
>>>>> + * we compute a system-wide requirement based on all the registered
>>>>> + * doorbells
>>>>> + */
>>>>> + vfio_msi_geometry->size =
>>>>> + msi_doorbell_calc_pages(order) * ((uint64_t) 1 << order);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> static long vfio_iommu_type1_ioctl(void *iommu_data,
>>>>> unsigned int cmd, unsigned long arg)
>>>>> {
>>>>> @@ -1122,8 +1173,10 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
>>>>> }
>>>>> } else if (cmd == VFIO_IOMMU_GET_INFO) {
>>>>> struct vfio_iommu_type1_info info;
>>>>> + struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
>>>>> + int ret;
>>>>>
>>>>> - minsz = offsetofend(struct vfio_iommu_type1_info, iova_pgsizes);
>>>>> + minsz = offsetofend(struct vfio_iommu_type1_info, cap_offset);
>>>>>
>>>>> if (copy_from_user(&info, (void __user *)arg, minsz))
>>>>> return -EFAULT;
>>>>> @@ -1135,6 +1188,29 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
>>>>>
>>>>> info.iova_pgsizes = vfio_pgsize_bitmap(iommu);
>>>>>
>>>>> + ret = compute_msi_geometry_caps(iommu, &caps);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + if (caps.size) {
>>>>> + info.flags |= VFIO_IOMMU_INFO_CAPS;
>>>>> + if (info.argsz < sizeof(info) + caps.size) {
>>>>> + info.argsz = sizeof(info) + caps.size;
>>>>> + info.cap_offset = 0;
>>>>> + } else {
>>>>> + vfio_info_cap_shift(&caps, sizeof(info));
>>>>> + if (copy_to_user((void __user *)arg +
>>>>> + sizeof(info), caps.buf,
>>>>> + caps.size)) {
>>>>> + kfree(caps.buf);
>>>>> + return -EFAULT;
>>>>> + }
>>>>> + info.cap_offset = sizeof(info);
>>>>> + }
>>>>> +
>>>>> + kfree(caps.buf);
>>>>> + }
>>>>> +
>>>>> return copy_to_user((void __user *)arg, &info, minsz) ?
>>>>> -EFAULT : 0;
>>>>>
>>>>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>>>>> index 4a9dbc2..8dae013 100644
>>>>> --- a/include/uapi/linux/vfio.h
>>>>> +++ b/include/uapi/linux/vfio.h
>>>>> @@ -488,7 +488,35 @@ struct vfio_iommu_type1_info {
>>>>> __u32 argsz;
>>>>> __u32 flags;
>>>>> #define VFIO_IOMMU_INFO_PGSIZES (1 << 0) /* supported page sizes info */
>>>>> - __u64 iova_pgsizes; /* Bitmap of supported page sizes */
>>>>> +#define VFIO_IOMMU_INFO_CAPS (1 << 1) /* Info supports caps */
>>>>> + __u64 iova_pgsizes; /* Bitmap of supported page sizes */
>>>>> + __u32 __resv;
>>>>> + __u32 cap_offset; /* Offset within info struct of first cap */
>>>>> +};
>>>>
>>>> I understand the padding, but not the ordering. Why not end with
>>>> padding?
>>>>
>>>>> +
>>>>> +#define VFIO_IOMMU_TYPE1_INFO_CAP_MSI_GEOMETRY 1
>>>>> +
>>>>> +/*
>>>>> + * The MSI geometry capability allows to report the MSI IOVA geometry:
>>>>> + * - either the MSI IOVAs are constrained within a reserved IOVA aperture
>>>>> + * whose boundaries are given by [@aperture_start, @aperture_end].
>>>>> + * this is typically the case on x86 host. The userspace is not allowed
>>>>> + * to map userspace memory at IOVAs intersecting this range using
>>>>> + * VFIO_IOMMU_MAP_DMA.
>>>>> + * - or the MSI IOVAs are not requested to belong to any reserved range;
>>>>> + * in that case the userspace must provide an IOVA window characterized by
>>>>> + * @size and @alignment using VFIO_IOMMU_MAP_DMA with RESERVED_MSI_IOVA flag.
>>>>> + */
>>>>> +struct vfio_iommu_type1_info_cap_msi_geometry {
>>>>> + struct vfio_info_cap_header header;
>>>>> + __u32 flags;
>>>>> +#define VFIO_IOMMU_MSI_GEOMETRY_RESERVED (1 << 0) /* reserved geometry */
>>>>> + /* not reserved */
>>>>> + __u32 order; /* iommu page order used for aperture alignment*/
>>>>> + __u64 size; /* IOVA aperture size (bytes) the userspace must provide */
>>>>> + /* reserved */
>>>>> + __u64 aperture_start;
>>>>> + __u64 aperture_end;
>>>>
>>>> Should these be a union? We never set them both. Should the !reserved
>>>> case have a flag as well, so the user can positively identify what's
>>>> being provided?
>>>
>>> Actually, is there really any need to fit both of these within the same
>>> structure? Part of the idea of the capability chains is we can create
>>> a capability for each new thing we want to describe. So, we could
>>> simply define a generic reserved IOVA range capability with a 'start'
>>> and 'end' and then another capability to define MSI mapping
>>> requirements. Thanks,
>> Yes your suggested approach makes sense to me.
>>
>> One reason why I proceeded that way is we are mixing things at iommu.h
>> level too. Personally I would have preferred to separate things:
>> 1) add a new IOMMU_CAP_TRANSLATE_MSI capability in iommu_cap
>> 2) rename iommu_msi_supported into "programmable" bool: reporting
>> whether the aperture is reserved or programmable.
>>
>> In the early releases I think it was as above but slightly we moved to a
>> mixed description.
>>
>> What do you think?
>
> The API certainly doesn't seem like it has a cohesive feel to me. It's
> not entirely clear to me how we know when we need to register a DMA MSI
> cookie, or how we know that the MSI doorbell API is actually
> initialized and in use by the MSI/IOMMU layer, or exactly what is the
> MSI geometry telling me. Perhaps this is why the code doesn't seem to
> have a good rejection mechanism for architectures that need it versus
> those that don't, it's too hard to tell.
>
> Maybe we can look at what we think the user API should be and work
> backwards. For x86 we simply have a reserved range of IOVA. I'm not
> entirely sure it adds to the user API to know that it's for MSI, it's
> just a range of IOVAs that we cannot allocate for regular DMA. In
> fact, we currently lack a mechanism for describing the IOVA space of
> the IOMMU at all, so rather than focusing on a mechanism to describe a
> hole in the IOVA space, we might simply want to focus on a mechanism to
> describe the available IOVA space. Everybody needs that, not just
> x86. That sort of sounds like a VFIO_IOMMU_TYPE1_INFO_CAP_IOVA_RANGE
> that perhaps looks like:
>
> struct vfio_iommu_type1_info_cap_iova_range {
> struct vfio_info_cap_header header;
> u64 start;
> u64 end;
> };
>
> Clearly we need to allow multiple of these in the capability chain
> since the existing x86 MSI range bisects this address space.
>
> To support this, we basically need the same information from the IOMMU
> API. We already have DOMAIN_ATTR_GEOMETRY, which should give us the
> base IOVA range, but we don't have anything describing the gaps. We
> don't know how many sources of gaps we'll have in the future, but let's
> keep it simple and assume we can look for MSI gaps and add other
> possible sources of gaps in the future, it's an internal API after all.
> So we can use DOMAIN_ATTR_MSI_GEOMETRY to tell us about the (we assume
> one) MSI range of reserved IOVA within DOMAIN_ATTR_GEOMETRY. For x86
> this is fixed, for SMMU this is a zero range until someone programs it.
>
> Now, what does a user need to know to add a reserved MSI IOVA range?
> They need to know a) that it needs to be done, and b) how big to make
> it (and maybe alignment requirements). Really all we need to describe
> then is b) since b) implies a). So maybe that gives us another
> capability chain entry:
>
> struct vfio_iommu_type1_info_cap_msi_resv {
> struct vfio_info_cap_header header;
> u64 size;
> u64 alignment;
> };
I like the approach and I like the idea to separate the 2 issues in
separate structs, both at VFIO level and IOMMU level. It makes even more
sense now we have the other requirement to handle host PCIe host bridge
window.
>
> It doesn't seem like we need to waste a flag bit on
> vfio_iommu_type1_info.flags for this since the existence of this
> capability would imply that VFIO_IOMMU_MAP_DMA supports an MSI_RESV
> flag.
I agree.
>
> So what do we need from the kernel infrastructure to make that happen?
> Well, we need a) and b) above, and again b) can imply a), so if the
> IOMMU API provided a DOMAIN_ATTR_MSI_RESV, providing the same
> size/alignment, then we're nearly there.
Agreed
Then we just need a way to
> set that range, which I'd probably try to plumb through the IOMMU API
> rather than pulling in separate doorbell APIs and DMA cookie APIs. If
> it's going to pull together all those different things, let's at least
> only do that in one place so we can expose a consistent API through the
> IOMMU API. Obviously once a range is set, DOMAIN_ATTR_MSI_RESV should
> report that range, so if the user were to look at the type1 info
> capability chain again, the available IOVA ranges would reflect the now
> reserved range.
So my plan is to respin the passthrough series with
vfio_iommu_type1_info_cap_msi_resv and associated iommu struct.
I would prefer to send a separate series to report IOVA usable address
space.
Thanks
Eric
>
> Maybe that's more than you're asking for, but that's the approach I
> would take to solidify the API. Thanks,
>
> Alex
>
> _______________________________________________
> 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 5/9] drm/sun4i: Add compatible strings for A31/A31s display pipelines
From: Rob Herring @ 2016-10-10 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161006160629.11198-6-wens@csie.org>
On Fri, Oct 07, 2016 at 12:06:25AM +0800, Chen-Yu Tsai wrote:
> The A31's display pipeline has 2 frontends, 2 backends, and 2 TCONs. It
> also has new display enhancement blocks, such as the DRC (Dynamic Range
> Controller), the DEU (Display Enhancement Unit), and the CMU (Color
> Management Unit). It supports HDMI, MIPI DSI, and 2 LCD/LVDS channels.
>
> The A31s display pipeline is almost the same, just without MIPI DSI.
> Only the TCON seems to be different, due to the missing mux for MIPI
> DSI.
>
> Add compatible strings for both of them.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 4 ++++
> drivers/gpu/drm/sun4i/sun4i_backend.c | 1 +
> drivers/gpu/drm/sun4i/sun4i_drv.c | 3 +++
> 3 files changed, 8 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v13 03/15] iommu/dma: Allow MSI-only cookies
From: Auger Eric @ 2016-10-10 14:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6b944a74-7429-caf2-507d-30d6ae336bec@arm.com>
Hi Robin,
On 10/10/2016 16:26, Robin Murphy wrote:
> Hi Alex, Eric,
>
> On 06/10/16 21:17, Alex Williamson wrote:
>> On Thu, 6 Oct 2016 08:45:19 +0000
>> Eric Auger <eric.auger@redhat.com> wrote:
>>
>>> From: Robin Murphy <robin.murphy@arm.com>
>>>
>>> IOMMU domain users such as VFIO face a similar problem to DMA API ops
>>> with regard to mapping MSI messages in systems where the MSI write is
>>> subject to IOMMU translation. With the relevant infrastructure now in
>>> place for managed DMA domains, it's actually really simple for other
>>> users to piggyback off that and reap the benefits without giving up
>>> their own IOVA management, and without having to reinvent their own
>>> wheel in the MSI layer.
>>>
>>> Allow such users to opt into automatic MSI remapping by dedicating a
>>> region of their IOVA space to a managed cookie.
>>>
>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>
>>> ---
>>>
>>> v1 -> v2:
>>> - compared to Robin's version
>>> - add NULL last param to iommu_dma_init_domain
>>> - set the msi_geometry aperture
>>> - I removed
>>> if (base < U64_MAX - size)
>>> reserve_iova(iovad, iova_pfn(iovad, base + size), ULONG_MAX);
>>> don't get why we would reserve something out of the scope of the iova domain?
>>> what do I miss?
>>> ---
>>> drivers/iommu/dma-iommu.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>> include/linux/dma-iommu.h | 9 +++++++++
>>> 2 files changed, 49 insertions(+)
>>>
>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>> index c5ab866..11da1a0 100644
>>> --- a/drivers/iommu/dma-iommu.c
>>> +++ b/drivers/iommu/dma-iommu.c
>>> @@ -716,3 +716,43 @@ void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
>>> msg->address_lo += lower_32_bits(msi_page->iova);
>>> }
>>> }
>>> +
>>> +/**
>>> + * iommu_get_dma_msi_region_cookie - Configure a domain for MSI remapping only
>>
>> Should this perhaps be iommu_setup_dma_msi_region_cookie, or something
>> along those lines. I'm not sure what we're get'ing. Thanks,
>
> What we're getting is private third-party resources for the iommu_domain
> given in the argument. It's a get/put rather than alloc/free model since
> we operate opaquely on the domain as a container, rather than on the
> actual resource in question (an IOVA allocator).
>
> Since this particular use case is slightly different from the normal
> flow and has special initialisation requirements, it seemed a lot
> cleaner to simply combine that initialisation operation with the
> prerequisite "get" into a single call. Especially as it helps emphasise
> that this is not 'normal' DMA cookie usage.
I renamed iommu_get_dma_msi_region_cookie into
iommu_setup_dma_msi_region. Is it a problem for you?
>
>>
>> Alex
>>
>>> + * @domain: IOMMU domain to prepare
>>> + * @base: Base address of IOVA region to use as the MSI remapping aperture
>>> + * @size: Size of the desired MSI aperture
>>> + *
>>> + * Users who manage their own IOVA allocation and do not want DMA API support,
>>> + * but would still like to take advantage of automatic MSI remapping, can use
>>> + * this to initialise their own domain appropriately.
>>> + */
>>> +int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>>> + dma_addr_t base, u64 size)
>>> +{
>>> + struct iommu_dma_cookie *cookie;
>>> + struct iova_domain *iovad;
>>> + int ret;
>>> +
>>> + if (domain->type == IOMMU_DOMAIN_DMA)
>>> + return -EINVAL;
>>> +
>>> + ret = iommu_get_dma_cookie(domain);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = iommu_dma_init_domain(domain, base, size, NULL);
>>> + if (ret) {
>>> + iommu_put_dma_cookie(domain);
>>> + return ret;
>>> + }
>
> It *is* necessary to explicitly reserve the upper part of the IOVA
> domain here - the aforementioned "special initialisation" - because
> dma_32bit_pfn is only an optimisation hint to prevent the allocator
> walking down from the very top of the the tree every time when devices
> with different DMA masks share a domain (I'm in two minds as to whether
> to tweak the way the iommu-dma code uses it in this respect, now that I
> fully understand things). The only actual upper limit to allocation is
> the DMA mask passed into each alloc_iova() call, so if we want to ensure
> IOVAs are really allocated within this specific region, we have to carve
> out everything above it.
thank you for the explanation. So I will restore the reserve then.
Thanks
Eric
>
> Robin.
>
>>> +
>>> + domain->msi_geometry.aperture_start = base;
>>> + domain->msi_geometry.aperture_end = base + size - 1;
>>> +
>>> + cookie = domain->iova_cookie;
>>> + iovad = &cookie->iovad;
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL(iommu_get_dma_msi_region_cookie);
>>> diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
>>> index 32c5890..1c55413 100644
>>> --- a/include/linux/dma-iommu.h
>>> +++ b/include/linux/dma-iommu.h
>>> @@ -67,6 +67,9 @@ int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
>>> /* The DMA API isn't _quite_ the whole story, though... */
>>> void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
>>>
>>> +int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>>> + dma_addr_t base, u64 size);
>>> +
>>> #else
>>>
>>> struct iommu_domain;
>>> @@ -90,6 +93,12 @@ static inline void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
>>> {
>>> }
>>>
>>> +static inline int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>>> + dma_addr_t base, u64 size)
>>> +{
>>> + return -ENODEV;
>>> +}
>>> +
>>> #endif /* CONFIG_IOMMU_DMA */
>>> #endif /* __KERNEL__ */
>>> #endif /* __DMA_IOMMU_H */
>>
>
^ permalink raw reply
* [PATCH 1/9] drm/sun4i: sun6i-drc: Support DRC on A31 and A31s
From: Rob Herring @ 2016-10-10 14:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161006160629.11198-2-wens@csie.org>
On Fri, Oct 07, 2016 at 12:06:21AM +0800, Chen-Yu Tsai wrote:
> The A31 and A31s also have the DRC as part of the display pipeline.
> As we know virtually nothing about them, just add compatible strings
> for both SoCs to the stub driver.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 2 ++
> drivers/gpu/drm/sun4i/sun6i_drc.c | 2 ++
> 2 files changed, 4 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH] MAINTAINERS: Add ARM64-specific ACPI maintainers entry
From: Sudeep Holla @ 2016-10-10 14:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161005112540.22189-1-lorenzo.pieralisi@arm.com>
On 05/10/16 12:25, Lorenzo Pieralisi wrote:
> The ARM64 architecture defines ARM64 specific ACPI bindings to
> configure and set-up arch specific components. To simplify
> code reviews/updates and streamline the maintainership structure
> supporting the arch specific code, a new arm64 directory was created in
> /drivers/acpi, to contain ACPI code that is specific to ARM64
> architecture.
>
> Add the ARM64-specific ACPI maintainers entry in MAINTAINERS for
> the newly created subdirectory and respective code content.
>
> Lorenzo Pieralisi will be in charge of submitting and managing
> the pull requests on behalf of all maintainers listed.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH] ARM: dts: imx6sx: Fix LCDIF interrupt type
From: Marek Vasut @ 2016-10-10 14:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476106556.2376.12.camel@pengutronix.de>
On 10/10/2016 03:35 PM, Lucas Stach wrote:
> Am Sonntag, den 02.10.2016, 18:44 +0200 schrieb Marek Vasut:
>> The LCDIF interrupt should be triggered by the rising edge of the
>> IRQ line because we only want the interrupt to trigger once per each
>> frame. It seems the LCDIF IRQ line cannot be explicitly de-asserted
>> by software, so the previous behavior before this patch, where the
>> interrupt was triggered by level-high status of the IRQ line, caused
>> the interrupt to fire again immediatelly after it was handled, which
>> caused the system to lock up due to the high rate of interrupts.
>>
> If there is no way to ack the IRQ how is the line going low again? Some
> hardware state machine?
My understanding is that it goes down at the end of VBLANK period.
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> Cc: Shawn Guo <shawnguo@kernel.org>
>> ---
>> arch/arm/boot/dts/imx6sx.dtsi | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
>> index 1a473e8..9526c38 100644
>> --- a/arch/arm/boot/dts/imx6sx.dtsi
>> +++ b/arch/arm/boot/dts/imx6sx.dtsi
>> @@ -1143,7 +1143,7 @@
>> lcdif1: lcdif at 02220000 {
>> compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif";
>> reg = <0x02220000 0x4000>;
>> - interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupts = <GIC_SPI 5 IRQ_TYPE_EDGE_RISING>;
>> clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>,
>> <&clks IMX6SX_CLK_LCDIF_APB>,
>> <&clks IMX6SX_CLK_DISPLAY_AXI>;
>> @@ -1154,7 +1154,7 @@
>> lcdif2: lcdif at 02224000 {
>> compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif";
>> reg = <0x02224000 0x4000>;
>> - interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupts = <GIC_SPI 6 IRQ_TYPE_EDGE_RISING>;
>> clocks = <&clks IMX6SX_CLK_LCDIF2_PIX>,
>> <&clks IMX6SX_CLK_LCDIF_APB>,
>> <&clks IMX6SX_CLK_DISPLAY_AXI>;
>
>
--
Best regards,
Marek Vasut
^ permalink raw reply
* [PATCH v4 2/8] scpi: Add alternative legacy structures, functions and macros
From: Sudeep Holla @ 2016-10-10 14:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475652814-30619-3-git-send-email-narmstrong@baylibre.com>
Hi Neil,
Sorry, I could not reply to your response on v3. Anyways I will review v4.
On 05/10/16 08:33, Neil Armstrong wrote:
> This patch adds support for the Legacy SCPI protocol in early JUNO versions and
> shipped Amlogic ARMv8 based SoCs. Some Rockchip SoC are also known to use this
> version of protocol with extended vendor commands
> .
> In order to support the legacy SCPI protocol variant, add back the structures
> and macros that varies against the final specification.
> Then add indirection table for legacy commands.
> Finally Add bitmap field for channel selection since the Legacy protocol mandates to
> send a selected subset of the commands on the high priority channel instead of the
> low priority channel.
>
> The message sending path differs from the final SCPI procotocol because the
> Amlogic SCP firmware always reply 1 instead of a special value containing the command
> byte and replied rx data length.
> For this reason commands queuing cannot be used and we assume the reply command is
> the head of the rx_pending list since we ensure sequential command sending with a
> separate dedicated mutex.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> drivers/firmware/arm_scpi.c | 221 +++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 199 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
> index 498afa0..6244eb1 100644
> --- a/drivers/firmware/arm_scpi.c
> +++ b/drivers/firmware/arm_scpi.c
[...]
> @@ -307,21 +398,46 @@ static void scpi_process_cmd(struct scpi_chan *ch, u32 cmd)
> return;
> }
>
> - list_for_each_entry(t, &ch->rx_pending, node)
> - if (CMD_XTRACT_UNIQ(t->cmd) == CMD_XTRACT_UNIQ(cmd)) {
> - list_del(&t->node);
> - match = t;
> - break;
> - }
> + /* Command type is not replied by the SCP Firmware in legacy Mode
> + * We should consider that command is the head of pending RX commands
> + * if the list is not empty. In TX only mode, the list would be empty.
> + */
> + if (scpi_info->is_legacy) {
> + match = list_first_entry(&ch->rx_pending, struct scpi_xfer,
> + node);
> + list_del(&match->node);
> + } else {
> + list_for_each_entry(t, &ch->rx_pending, node)
> + if (CMD_XTRACT_UNIQ(t->cmd) == CMD_XTRACT_UNIQ(cmd)) {
> + list_del(&t->node);
> + match = t;
> + break;
> + }
> + }
> /* check if wait_for_completion is in progress or timed-out */
> if (match && !completion_done(&match->done)) {
> - struct scpi_shared_mem *mem = ch->rx_payload;
> - unsigned int len = min(match->rx_len, CMD_SIZE(cmd));
> + unsigned int len;
> +
> + if (scpi_info->is_legacy) {
> + struct legacy_scpi_shared_mem *mem = ch->rx_payload;
> +
> + /* RX Length is not replied by the lagcy Firmware */
> + len = match->rx_len;
> +
> + match->status = le32_to_cpu(mem->status);
> + memcpy_fromio(match->rx_buf, mem->payload, len);
The above 2 seems common to both, no ?
> + } else {
> + struct scpi_shared_mem *mem = ch->rx_payload;
> +
> + len = min(match->rx_len, CMD_SIZE(cmd));
> +
> + match->status = le32_to_cpu(mem->status);
> + memcpy_fromio(match->rx_buf, mem->payload, len);
> + }
>
> - match->status = le32_to_cpu(mem->status);
> - memcpy_fromio(match->rx_buf, mem->payload, len);
> if (match->rx_len > len)
> memset(match->rx_buf + len, 0, match->rx_len - len);
> +
Spurious ?
> complete(&match->done);
> }
> spin_unlock_irqrestore(&ch->rx_lock, flags);
> @@ -331,7 +447,12 @@ static void scpi_handle_remote_msg(struct mbox_client *c, void *msg)
> {
> struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
> struct scpi_shared_mem *mem = ch->rx_payload;
> - u32 cmd = le32_to_cpu(mem->command);
> + u32 cmd;
> +
> + if (scpi_info->is_legacy)
> + cmd = *(u32 *)msg;
Do we need do this if it doesn't contain command ?
> + else
> + cmd = le32_to_cpu(mem->command);
>
> scpi_process_cmd(ch, cmd);
> }
> @@ -343,17 +464,26 @@ static void scpi_tx_prepare(struct mbox_client *c, void *msg)
> struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
> struct scpi_shared_mem *mem = (struct scpi_shared_mem *)ch->tx_payload;
>
> - if (t->tx_buf)
> - memcpy_toio(mem->payload, t->tx_buf, t->tx_len);
> + if (t->tx_buf) {
> + if (scpi_info->is_legacy)
> + memcpy_toio(ch->tx_payload, t->tx_buf, t->tx_len);
> + else
> + memcpy_toio(mem->payload, t->tx_buf, t->tx_len);
> + }
> +
> if (t->rx_buf) {
> if (!(++ch->token))
> ++ch->token;
> ADD_SCPI_TOKEN(t->cmd, ch->token);
> + if (scpi_info->is_legacy)
> + t->slot = t->cmd;
I thought passing token was not an issue from your previous response,
but you are overriding it here, why ?
> spin_lock_irqsave(&ch->rx_lock, flags);
> list_add_tail(&t->node, &ch->rx_pending);
> spin_unlock_irqrestore(&ch->rx_lock, flags);
> }
> - mem->command = cpu_to_le32(t->cmd);
> +
> + if (!scpi_info->is_legacy)
> + mem->command = cpu_to_le32(t->cmd);
> }
>
> static struct scpi_xfer *get_scpi_xfer(struct scpi_chan *ch)
> @@ -396,21 +526,37 @@ static int scpi_send_message(unsigned int offset, void *tx_buf,
>
> cmd = scpi_info->scpi_cmds[offset];
>
> - chan = atomic_inc_return(&scpi_info->next_chan) % scpi_info->num_chans;
> + if (scpi_info->is_legacy)
> + chan = test_bit(cmd, scpi_info->cmd_priority) ? 1 : 0;
> + else
> + chan = atomic_inc_return(&scpi_info->next_chan) %
> + scpi_info->num_chans;
> scpi_chan = scpi_info->channels + chan;
>
> msg = get_scpi_xfer(scpi_chan);
> if (!msg)
> return -ENOMEM;
>
> - msg->slot = BIT(SCPI_SLOT);
> - msg->cmd = PACK_SCPI_CMD(cmd, tx_len);
> + if (scpi_info->is_legacy) {
> + msg->cmd = PACK_LEGACY_SCPI_CMD(cmd, tx_len);
> + msg->slot = msg->cmd;
> + } else {
> + msg->slot = BIT(SCPI_SLOT);
> + msg->cmd = PACK_SCPI_CMD(cmd, tx_len);
> + }
> msg->tx_buf = tx_buf;
> msg->tx_len = tx_len;
> msg->rx_buf = rx_buf;
> msg->rx_len = rx_len;
> init_completion(&msg->done);
>
> + /* Since we cannot distinguish the original command in the
> + * MHU reply stat value from a Legacy SCP firmware, ensure
> + * sequential command sending to the firmware.
> + */
OK this comment now questions the existence of this extra lock.
The mailbox will always send the commands in the sequential order.
It's only firmware that can re-order the response. Since that can't
happen in you case, I really don't see the need for this.
Please explain the race you would see without this locking. Yes I
understand that only one command is supposed to be sent to firmware at a
time. Suppose you allow more callers here, all will wait on the
completion flags and the first in the list gets unblocked right ?
I am just trying to understand if there's real need for this extra
lock when we already have that from the list.
> + if (scpi_info->is_legacy)
> + mutex_lock(&scpi_chan->legacy_lock);
> +
> ret = mbox_send_message(scpi_chan->chan, msg);
> if (ret < 0 || !rx_buf)
> goto out;
> @@ -421,9 +567,13 @@ static int scpi_send_message(unsigned int offset, void *tx_buf,
> /* first status word */
> ret = msg->status;
> out:
> - if (ret < 0 && rx_buf) /* remove entry from the list if timed-out */
> + if (ret < 0 && rx_buf)
> + /* remove entry from the list if timed-out */
> scpi_process_cmd(scpi_chan, msg->cmd);
>
> + if (scpi_info->is_legacy)
> + mutex_unlock(&scpi_chan->legacy_lock);
> +
> put_scpi_xfer(msg, scpi_chan);
> /* SCPI error codes > 0, translate them to Linux scale*/
> return ret > 0 ? scpi_to_linux_errno(ret) : ret;
[...]
> @@ -525,7 +687,6 @@ static struct scpi_dvfs_info *scpi_dvfs_get_info(u8 domain)
>
> info->count = DVFS_OPP_COUNT(buf.header);
> info->latency = DVFS_LATENCY(buf.header) * 1000; /* uS to nS */
> -
Spurious ?
> info->opps = kcalloc(info->count, sizeof(*opp), GFP_KERNEL);
> if (!info->opps) {
> kfree(info);
> @@ -580,9 +741,13 @@ static int scpi_sensor_get_value(u16 sensor, u64 *val)
>
> ret = scpi_send_message(CMD_SENSOR_VALUE, &id, sizeof(id),
> &buf, sizeof(buf));
> - if (!ret)
> - *val = (u64)le32_to_cpu(buf.hi_val) << 32 |
> - le32_to_cpu(buf.lo_val);
> + if (!ret) {
> + if (scpi_info->is_legacy)
> + *val = (u64)le32_to_cpu(buf.lo_val);
> + else
> + *val = (u64)le32_to_cpu(buf.hi_val) << 32 |
> + le32_to_cpu(buf.lo_val);
> + }
Not required as I have mentioned couple of times in previous versions,
it's zero filled by the driver.
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH RESEND] ARM: dts: keystone-k2*: Increase SPI Flash partition size for U-Boot
From: Russell King - ARM Linux @ 2016-10-10 14:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161010141141.21333-1-vigneshr@ti.com>
On Mon, Oct 10, 2016 at 07:41:41PM +0530, Vignesh R wrote:
> U-Boot SPI Boot image is now more than 512KB for Keystone2 devices and
> cannot fit into existing partition. So, increase the SPI Flash partition
> for U-Boot to 1MB for all Keystone2 devices.
>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>
> This was submitted to v4.9 merge window but was never picked up:
> https://patchwork.kernel.org/patch/9135023/
I think you need to explain why it's safe to change the layout of the
flash partitions like this.
- What is this "misc" partition?
- Why is it safe to move the "misc" partition in this way?
- Do users need to do anything with data stored in the "misc" partition
when changing kernels?
If the "misc" partition is simply unused space on the flash device, why
list it in DT?
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH 0/4] soc: renesas: Identify SoC and register with the SoC bus
From: Arnd Bergmann @ 2016-10-10 14:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475572167-29581-1-git-send-email-geert+renesas@glider.be>
On Tuesday, October 4, 2016 11:09:23 AM CEST Geert Uytterhoeven wrote:
> Hi all,
>
> Some Renesas SoCs may exist in different revisions, providing slightly
> different functionalities (e.g. R-Car H3 ES1.x and ES2.0). This needs to
> be catered for by drivers and/or platform code. The recently proposed
> soc_device_match() API seems like a good fit to handle this.
>
> This patch series implements the core infrastructure to provide SoC and
> revision information through the SoC bus for Renesas ARM SoCs. It
> consists of 4 patches:
> - Patch 1 avoids a crash when SoC revision information is needed and
> provided early,
> - Patch 2 (from Arnd) introduces the soc_device_match() API.
> I don't know if, when, and through which channel this patch is
> planned to go upstream,
> - Patch 3 fixes a bug in soc_device_match(), causing a crash when
> trying to match on an SoC attribute that is not provided (seen on
> EMEV2, RZ/A, and R-Car M1A, which lack revision information),
> - Patch 4 identifies Renesas SoCs and registers them with the SoC bus.
>
> Tested on (family, machine, soc_id, optional revision):
>
> Emma Mobile EV2, EMEV2 KZM9D Board, emev2
> RZ/A, Genmai, r7s72100
> R-Mobile, APE6EVM, r8a73a4, ES1.0
> R-Mobile, armadillo 800 eva, r8a7740, ES2.0
> R-Car Gen1, bockw, r8a7778
> R-Car Gen1, marzen, r8a7779, ES1.0
> R-Car Gen2, Lager, r8a7790, ES1.0
> R-Car Gen2, Koelsch, r8a7791, ES1.0
> R-Car Gen2, Gose, r8a7793, ES1.0
> R-Car Gen2, Alt, r8a7794, ES1.0
> R-Car Gen3, Renesas Salvator-X board based on r8a7795, r8a7795, ES1.0
> R-Car Gen3, Renesas Salvator-X board based on r8a7796, r8a7796, ES1.0
> SH-Mobile, KZM-A9-GT, sh73a0, ES2.0
As mentioned in the comment for the driver patch, I think this makes
a lot of sense for the machines that have a revision register, in
particular when the interpretation of that register is always done
the same way, but I'm a bit skeptical about doing it in the same driver
for machines that don't have the register.
Matching by a device rather than the SoC platform also has the advantage
that there is no need to maintain a list of compatible numbers in the
driver.
Arnd
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox