* [PATCH] arm: spin one more cycle in timer-based delays
From: Russell King - ARM Linux @ 2016-11-18 12:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118120630.GJ13470@arm.com>
On Fri, Nov 18, 2016 at 12:06:30PM +0000, Will Deacon wrote:
> On Tue, Nov 15, 2016 at 02:36:33PM +0100, Mason wrote:
> > When polling a tick counter in a busy loop, one might sample the
> > counter just *before* it is updated, and then again just *after*
> > it is updated. In that case, while mathematically v2-v1 equals 1,
> > only epsilon has really passed.
> >
> > So, if a caller requests an N-cycle delay, we spin until v2-v1
> > is strictly greater than N to avoid these random corner cases.
> >
> > Signed-off-by: Mason <slash.tmp@free.fr>
> > ---
> > arch/arm/lib/delay.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c
> > index 69aad80a3af4..3f1cd15ca102 100644
> > --- a/arch/arm/lib/delay.c
> > +++ b/arch/arm/lib/delay.c
> > @@ -60,7 +60,7 @@ static void __timer_delay(unsigned long cycles)
> > {
> > cycles_t start = get_cycles();
> >
> > - while ((get_cycles() - start) < cycles)
> > + while ((get_cycles() - start) <= cycles)
> > cpu_relax();
> > }
>
> I thought a bit about this last night. It is well known that the delay
> routines are not guaranteed to be accurate, and I don't *think* that's
> limited to over-spinning, so arguably this isn't a bug. However, taking
> the approach that "drivers should figure it out" is also unhelpful,
> because the frequency of the underlying counter isn't generally known
> and so drivers can't simply adjust the delay parameter.
I don't think this change makes any sense whatsoever. udelay() is
inaccurate, period. It _will_ give delays shorter than requested
for many reasons, many of which can't be fixed.
Having a super-accurate version just encourages people to write broken
drivers which assume (eg) that udelay(10) will give _at least_ a 10us
delay. However, there is no such guarantee.
So, having udelay() for timers return slightly short is actually a good
thing - it causes people not to make the mistake to be soo accurate
with their delay specifications.
So, NAK on this change. udelay is not super-accurate.
Reference (and this is the most important one):
http://lists.openwall.net/linux-kernel/2011/01/12/372
--
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 V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Gabriele Paoloni @ 2016-11-18 12:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2822893.F0LqNAm9bT@wuerfel>
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 18 November 2016 12:24
> To: Gabriele Paoloni
> Cc: liviu.dudau at arm.com; linux-arm-kernel at lists.infradead.org;
> Yuanzhichang; mark.rutland at arm.com; devicetree at vger.kernel.org;
> lorenzo.pieralisi at arm.com; minyard at acm.org; linux-pci at vger.kernel.org;
> benh at kernel.crashing.org; John Garry; will.deacon at arm.com; linux-
> kernel at vger.kernel.org; xuwei (O); Linuxarm; zourongrong at gmail.com;
> robh+dt at kernel.org; kantyzc at 163.com; linux-serial at vger.kernel.org;
> catalin.marinas at arm.com; olof at lixom.net; bhelgaas at go og le.com;
> zhichang.yuan02 at gmail.com; Jason Gunthorpe; Thomas Petazzoni
> Subject: Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on
> Hip06
>
> On Friday, November 18, 2016 12:07:28 PM CET Gabriele Paoloni wrote:
> > > From: Arnd Bergmann [mailto:arnd at arndb.de]
> > > On Monday, November 14, 2016 11:26:25 AM CET liviu.dudau at arm.com
> wrote:
> > > > On Mon, Nov 14, 2016 at 08:26:42AM +0000, Gabriele Paoloni wrote:
> > > > > > Nope, that is not what it means. It means that PCI devices
> can
> > > see I/O
> > > > > > addresses
> > > > > > on the bus that start from 0. There never was any usage for
> non-
> > > PCI
> > > > > > controllers
> > > > >
> > > > > So I am a bit confused...
> > > > > From http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> > > > > It seems that ISA buses operate on cpu I/O address range [0,
> > > 0xFFF].
> > > > > I thought that was the reason why for most architectures we
> have
> > > > > PCIBIOS_MIN_IO equal to 0x1000 (so I thought that ISA
> controllers
> > > > > usually use [0, PCIBIOS_MIN_IO - 1] )
> > > >
> > > > First of all, cpu I/O addresses is an x86-ism. ARM architectures
> and
> > > others
> > > > have no separate address space for I/O, it is all merged into
> one
> > > unified
> > > > address space. So, on arm/arm64 for example, PCIBIOS_MIN_IO = 0
> could
> > > mean
> > > > that we don't care about ISA I/O because the platform does not
> > > support having
> > > > an ISA bus (e.g.).
> > >
> > > I think to be more specific, PCIBIOS_MIN_IO=0 would indicate that
> you
> > > cannot
> > > have a PCI-to-ISA or PCI-to-LPC bridge in any PCI domain. This is
> > > different
> > > from having an LPC master outside of PCI, as that lives in its own
> > > domain
> > > and has a separately addressable I/O space.
> >
> > Yes correct so if we go for the single domain solution arch that
> > have PCIBIOS_MIN_IO=0 cannot support special devices such as LPC
> > unless we also redefine PCIBIOS_MIN_IO, right?
>
> This is what I was referring to below as the difference between
> a) and b): Setting PCIBIOS_MIN_IO=0 means you cannot have LPC
> behind PCI, but it shouldn't stop you from having a separate
> LPC bridge.
>
> > > The PCIBIOS_MIN_DIRECT_IO name still suggests having something
> related
> > > to
> > > PCIBIOS_MIN_IO, but it really isn't. We are talking about multiple
> > > concepts here that are not the same but that are somewhat related:
> > >
> > > a) keeping PCI devices from allocating low I/O ports on the PCI bus
> > > that would conflict with ISA devices behind a bridge of the
> > > same bus.
> > >
> > > b) reserving the low 0x0-0xfff range of the Linux-internal I/O
> > > space abstraction to a particular LPC or PCI domain to make
> > > legacy device drivers work that hardcode a particular port
> > > number.
> > >
> > > c) Redirecting inb/outb to call a domain-specific accessor function
> > > rather than doing the normal MMIO window for an LPC master or
> > > more generally any arbitrary LPC or PCI domain that has a
> > > nonstandard I/O space.
> > > [side note: actually if we generalized this, we could avoid
> > > assigning an MMIO range for the I/O space on the pci-mvebu
> > > driver, and that would help free up some other remapping
> > > windows]
> > >
> > > I think there is no need to change a) here, we have PCIBIOS_MIN_IO
> > > today and even if we don't need it, there is no obvious downside.
> > > I would also argue that we can ignore b) for the discussion of
> > > the HiSilicon LPC driver, we just need to assign some range
> > > of logical addresses to each domain.
> > >
> > > That means solving c) is the important problem here, and it
> > > shouldn't be so hard. We can do this either with a single
> > > special domain as in the v5 patch series, or by generalizing it
> > > so that any I/O space mapping gets looked up through the device
> > > pointer of the bus master.
> >
> > I am not very on the "generalized" multi-domain solution...
> > Currently the IO accessors prototypes have an unsigned long addr
> > as input parameter. If we live in a multi-domain IO system
> > how can we distinguish inside the accessor which domain addr
> > belongs to?
>
> The easiest change compared to the v5 code would be to walk
> a linked list of 'struct extio_ops' structures rather than
> assuming there is only ever one of them. I think one of the
> earlier versions actually did this.
Right but if my understanding is correct if we live in a multi-
domain I/O space world when you have an input addr in the I/O
accessors this addr can be duplicated (for example for the standard
PCI IO domain and for our special LPC domain).
So effectively even if you walk a linked list there is a problem
of disambiguation...am I right?
>
> Another option the IA64 approach mentioned in another subthread
> today, looking up the operations based on an index from the
> upper bits of the port number. If we do this, we probably
> want to do that for all PIO access and replace the entire
> virtual address remapping logic with that. I think Bjorn
> in the past argued in favor of such an approach, while I
> advocated the current scheme for simplicity based on how
> every I/O space these days is just memory mapped (which now
> turned out to be false, both on powerpc and arm64).
This seems really complex...I am a bit worried that possibly
we end up in having the maintainers saying that it is not worth
to re-invent the world just for this special LPC device...
To be honest with you I would keep things simple for this
LPC and introduce more complex reworks later if more devices
need to be introduced.
What if we stick on a single domain now where we introduce a
reserved threshold for the IO space (say INDIRECT_MAX_IO).
We define INDIRECT_MAX_IO as 0 in "include/linux/extio.h" and
we define INDIRECT_MAX_IO as 0x1000 in "arch/arm64/include/asm/io.h"
So effectively this threshold can change according to the
architecture and so far we only define it for ARM64 as we need
it for ARM64...
Thoughts?
Thanks again
Gab
>
> Arnd
^ permalink raw reply
* [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU
From: Russell King - ARM Linux @ 2016-11-18 12:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <9481995.AMBiYg893F@wuerfel>
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.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [GIT PULL 2/2] ARM: dts: exynos: Topic dts for v4.10
From: Krzysztof Kozlowski @ 2016-11-18 12:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479473075-9787-1-git-send-email-krzk@kernel.org>
Hi,
Topic branch with a GIC interrupt fixes and cleanup (human-friendly symbols).
Possible rather trivial conflicts:
diff --cc arch/arm/boot/dts/exynos4x12.dtsi
index 0074f566cd3b,505f047e81c6..85a7122658f1
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@@ -147,9 -147,10 +147,10 @@@
};
fimc_is: fimc-is at 12000000 {
- compatible = "samsung,exynos4212-fimc-is", "simple-bus";
+ compatible = "samsung,exynos4212-fimc-is";
reg = <0x12000000 0x260000>;
- interrupts = <0 90 0>, <0 95 0>;
+ interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
power-domains = <&pd_isp>;
clocks = <&clock CLK_FIMC_LITE0>,
<&clock CLK_FIMC_LITE1>, <&clock CLK_PPMUISPX>,
diff --cc arch/arm/boot/dts/exynos5410.dtsi
index 9a91685d8890,2501249d97aa..bb90bbdbe2d9
--- a/arch/arm/boot/dts/exynos5410.dtsi
+++ b/arch/arm/boot/dts/exynos5410.dtsi
@@@ -190,58 -181,8 +190,58 @@@
pinctrl_3: pinctrl at 03860000 {
compatible = "samsung,exynos5410-pinctrl";
reg = <0x03860000 0x1000>;
- interrupts = <0 47 0>;
+ interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
};
+
+ amba {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "simple-bus";
+ interrupt-parent = <&gic>;
+ ranges;
Best regards,
Krzysztof
The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git tags/samsung-dt-gic-flags-4.10
for you to fetch changes up to 04a886727ca7e841afa2fbc5d87aff81ae256dbf:
ARM: dts: exynos: Use human-friendly symbols for interrupt properties in exynos5440 (2016-11-03 22:44:56 +0200)
----------------------------------------------------------------
Topic branch with DT changes for v4.10.
Fix invalid GIC interrupt flags - type IRQ_TYPE_NONE is not allowed for GIC
interrupts. Although this was working but with error messages like:
genirq: Setting trigger mode 0 for irq 16 failed
Use level high interrupt instead of type none. The choice of level high was
rather an arbitrary decision hoping it will work on each platform. Tests shown
no issues so far.
----------------------------------------------------------------
Krzysztof Kozlowski (14):
ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4
ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4210
ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4x12
ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos3250
ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5
ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5250
ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5410/exynos542x
ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5260
ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5440
ARM: dts: exynos: Use human-friendly symbols for interrupt properties in exynos3250
ARM: dts: exynos: Use human-friendly symbols for interrupt properties in exynos4
ARM: dts: exynos: Use human-friendly symbols for interrupt properties in exynos5
ARM: dts: exynos: Use human-friendly symbols for interrupt properties in exynos5260
ARM: dts: exynos: Use human-friendly symbols for interrupt properties in exynos5440
arch/arm/boot/dts/exynos3250-pinctrl.dtsi | 20 +++++--
arch/arm/boot/dts/exynos3250.dtsi | 97 ++++++++++++++++++-------------
arch/arm/boot/dts/exynos4.dtsi | 95 ++++++++++++++++--------------
arch/arm/boot/dts/exynos4210-pinctrl.dtsi | 20 +++++--
arch/arm/boot/dts/exynos4210.dtsi | 36 ++++++++----
arch/arm/boot/dts/exynos4x12-pinctrl.dtsi | 20 +++++--
arch/arm/boot/dts/exynos4x12.dtsi | 48 ++++++++++-----
arch/arm/boot/dts/exynos5.dtsi | 64 ++++++++++++++------
arch/arm/boot/dts/exynos5250.dtsi | 80 ++++++++++++-------------
arch/arm/boot/dts/exynos5260.dtsi | 43 +++++++++-----
arch/arm/boot/dts/exynos5410.dtsi | 26 ++++-----
arch/arm/boot/dts/exynos5420.dtsi | 78 ++++++++++++-------------
arch/arm/boot/dts/exynos5440.dtsi | 70 +++++++++++++---------
arch/arm/boot/dts/exynos54xx.dtsi | 34 +++++------
14 files changed, 437 insertions(+), 294 deletions(-)
^ permalink raw reply
* [GIT PULL 1/2] arm64: dts: exynos: Topic dts arm64 for v4.10
From: Krzysztof Kozlowski @ 2016-11-18 12:44 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Topic branch with a GIC interrupt fix and cleanup. No conflicts expected.
No dependencies.
Best regards,
Krzysztof
The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git tags/samsung-dt64-gic-flags-4.10
for you to fetch changes up to 86bb573d0b2a8e876a12d1348bd0b6e377c1043e:
arm64: dts: exynos: Use human-friendly symbols for interrupt properties in exynos7 (2016-11-03 22:40:39 +0200)
----------------------------------------------------------------
Topic branch with DT arm64 changes for v4.10.
Fix invalid GIC interrupt flags - type IRQ_TYPE_NONE is not allowed for GIC
interrupts. Although this was working but with error messages like:
genirq: Setting trigger mode 0 for irq 16 failed
Use level high interrupt instead of type none. The choice of level high was
rather an arbitrary decision hoping it will work on each platform. Tests shown
no issues so far.
----------------------------------------------------------------
Krzysztof Kozlowski (2):
arm64: dts: exynos: Fix invalid GIC interrupt flags in exynos7
arm64: dts: exynos: Use human-friendly symbols for interrupt properties in exynos7
arch/arm64/boot/dts/exynos/exynos7-pinctrl.dtsi | 20 +++++--
arch/arm64/boot/dts/exynos/exynos7.dtsi | 69 +++++++++++++------------
2 files changed, 51 insertions(+), 38 deletions(-)
^ permalink raw reply
* arasan,sdhci.txt "compatibility" DT binding
From: Mason @ 2016-11-18 12:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFd313x6MJeO90_+rxditdPn762g7TqZc0_D6ojGTKmtGJKjPA@mail.gmail.com>
On 18/11/2016 11:49, Rameshwar Sahu wrote:
> Mason wrote:
>
>> Suman/APM added "arasan,sdhci-4.9a" in 308f3f8d8112
>> @Suman, @Rameshwar: what specific IP block does your SoC embed?
>> What does 4.9a refer to? The documentation revision number?
>
> We have Arasan SD3.0/ SDIO3.0/ eMMC4.41 AHB Host Controller IP
> embedded in our SoC and 4.9a is documentation revision number which
> was given by Arasan.
> FYI this documentation date was May, 2012.
Hello Ram,
Thanks for the information.
Xilinx is using "SD2.0/SDIO2.0/MMC3.31 AHB Host Controller"
APM is using "SD3.0/SDIO3.0/eMMC4.41 AHB Host Controller"
Sigma is using "SD3.0/SDIO3.0/eMMC4.4 AHB Host Controller"
Rockchip is using ? (Shawn could you specify exactly which IP you use?)
And it looks like Arasan is using different revision numbers for
every documentation they have.
The "SD3.0/SDIO3.0/eMMC4.4 AHB Host Controller" doc I have is
at rev number 6.0, dated 2010-02-19.
Regards.
^ permalink raw reply
* [PATCH] ARM: dts: qcom: Add apq8064 CoreSight components
From: Georgi Djakov @ 2016-11-18 12:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d466f591-baac-c734-a49b-8aab763c1876@codeaurora.org>
On 11/18/2016 12:16 AM, Stephen Boyd wrote:
> On 11/17/2016 07:36 AM, Georgi Djakov wrote:
>> From: "Ivan T. Ivanov" <ivan.ivanov@linaro.org>
>>
>> Add initial set of CoreSight components found on Qualcomm's
>> 8064 chipset.
>>
>> Signed-off-by: Ivan T. Ivanov <ivan.ivanov@linaro.org>
>> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
>> ---
>> arch/arm/boot/dts/qcom-apq8064-coresight.dtsi | 196 ++++++++++++++++++++++++++
>
> Why not put this inside the soc file? This would be the first time we
> add a new file for something that's inside the SoC node that probably
> won't change thereafter. At least for pins I slightly agree with having
> a different file, we're adding more and more things there so it's nice
> to have a consolidated place of all possible configurations to choose
> from. But here it's mostly a static device description so what's the gain?
Ok, after thinking more about this and looking at other soc dtsi files i
agree with you. Will update!
Thanks!
Georgi
^ permalink raw reply
* [PATCH v4] arm64: dts: qcom: Add msm8916 CoreSight components
From: Georgi Djakov @ 2016-11-18 12:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161117185918.GA7884@linaro.org>
On 11/17/2016 08:59 PM, Mathieu Poirier wrote:
> On Thu, Nov 17, 2016 at 05:35:22PM +0200, Georgi Djakov wrote:
>> From: "Ivan T. Ivanov" <ivan.ivanov@linaro.org>
>>
>> Add initial set of CoreSight components found on Qualcomm's 8x16 chipset.
>
> Hello Georgi,
>
> Could you add a better desccription for the SoC? To me "8x16" doesn't
> say much.
Ok, will do it. Thanks!
>
> With that change:
> Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
^ permalink raw reply
* [PATCH] arm: spin one more cycle in timer-based delays
From: Mason @ 2016-11-18 12:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118120630.GJ13470@arm.com>
On 18/11/2016 13:06, Will Deacon wrote:
> On Tue, Nov 15, 2016 at 02:36:33PM +0100, Mason wrote:
>
>> When polling a tick counter in a busy loop, one might sample the
>> counter just *before* it is updated, and then again just *after*
>> it is updated. In that case, while mathematically v2-v1 equals 1,
>> only epsilon has really passed.
>>
>> So, if a caller requests an N-cycle delay, we spin until v2-v1
>> is strictly greater than N to avoid these random corner cases.
>>
>> Signed-off-by: Mason <slash.tmp@free.fr>
>> ---
>> arch/arm/lib/delay.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c
>> index 69aad80a3af4..3f1cd15ca102 100644
>> --- a/arch/arm/lib/delay.c
>> +++ b/arch/arm/lib/delay.c
>> @@ -60,7 +60,7 @@ static void __timer_delay(unsigned long cycles)
>> {
>> cycles_t start = get_cycles();
>>
>> - while ((get_cycles() - start) < cycles)
>> + while ((get_cycles() - start) <= cycles)
>> cpu_relax();
>> }
>
> I thought a bit about this last night. It is well known that the delay
> routines are not guaranteed to be accurate, and I don't *think* that's
> limited to over-spinning, so arguably this isn't a bug. However, taking
> the approach that "drivers should figure it out" is also unhelpful,
> because the frequency of the underlying counter isn't generally known
> and so drivers can't simply adjust the delay parameter.
>
> If you want to go ahead with this change, I do think that you should fix
> the other architectures for consistency (particularly arm64, which is
> likely to share drivers with arch/arm/). However, given that I don't
> think you've seen a failure in practice, it might be a hard sell to get
> the patches picked up, especially given the deliberately vague guarantees
> offered by the delay loop.
Hello Will,
Thanks for the in-depth analysis.
This is, conceptually, the first patch in a 2-patch series, and perhaps
the intent would have been clearer had I posted the series, along with
full rationale in the cover letter. I'll do that next week, so everyone
can judge with all the information in hand.
Regards.
^ permalink raw reply
* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-11-18 12:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <EE11001F9E5DDD47B7634E2F8A612F2E1F921146@lhreml507-mbx>
On Friday, November 18, 2016 12:07:28 PM CET Gabriele Paoloni wrote:
> > From: Arnd Bergmann [mailto:arnd at arndb.de]
> > On Monday, November 14, 2016 11:26:25 AM CET liviu.dudau at arm.com wrote:
> > > On Mon, Nov 14, 2016 at 08:26:42AM +0000, Gabriele Paoloni wrote:
> > > > > Nope, that is not what it means. It means that PCI devices can
> > see I/O
> > > > > addresses
> > > > > on the bus that start from 0. There never was any usage for non-
> > PCI
> > > > > controllers
> > > >
> > > > So I am a bit confused...
> > > > From http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> > > > It seems that ISA buses operate on cpu I/O address range [0,
> > 0xFFF].
> > > > I thought that was the reason why for most architectures we have
> > > > PCIBIOS_MIN_IO equal to 0x1000 (so I thought that ISA controllers
> > > > usually use [0, PCIBIOS_MIN_IO - 1] )
> > >
> > > First of all, cpu I/O addresses is an x86-ism. ARM architectures and
> > others
> > > have no separate address space for I/O, it is all merged into one
> > unified
> > > address space. So, on arm/arm64 for example, PCIBIOS_MIN_IO = 0 could
> > mean
> > > that we don't care about ISA I/O because the platform does not
> > support having
> > > an ISA bus (e.g.).
> >
> > I think to be more specific, PCIBIOS_MIN_IO=0 would indicate that you
> > cannot
> > have a PCI-to-ISA or PCI-to-LPC bridge in any PCI domain. This is
> > different
> > from having an LPC master outside of PCI, as that lives in its own
> > domain
> > and has a separately addressable I/O space.
>
> Yes correct so if we go for the single domain solution arch that
> have PCIBIOS_MIN_IO=0 cannot support special devices such as LPC
> unless we also redefine PCIBIOS_MIN_IO, right?
This is what I was referring to below as the difference between
a) and b): Setting PCIBIOS_MIN_IO=0 means you cannot have LPC
behind PCI, but it shouldn't stop you from having a separate
LPC bridge.
> > The PCIBIOS_MIN_DIRECT_IO name still suggests having something related
> > to
> > PCIBIOS_MIN_IO, but it really isn't. We are talking about multiple
> > concepts here that are not the same but that are somewhat related:
> >
> > a) keeping PCI devices from allocating low I/O ports on the PCI bus
> > that would conflict with ISA devices behind a bridge of the
> > same bus.
> >
> > b) reserving the low 0x0-0xfff range of the Linux-internal I/O
> > space abstraction to a particular LPC or PCI domain to make
> > legacy device drivers work that hardcode a particular port
> > number.
> >
> > c) Redirecting inb/outb to call a domain-specific accessor function
> > rather than doing the normal MMIO window for an LPC master or
> > more generally any arbitrary LPC or PCI domain that has a
> > nonstandard I/O space.
> > [side note: actually if we generalized this, we could avoid
> > assigning an MMIO range for the I/O space on the pci-mvebu
> > driver, and that would help free up some other remapping
> > windows]
> >
> > I think there is no need to change a) here, we have PCIBIOS_MIN_IO
> > today and even if we don't need it, there is no obvious downside.
> > I would also argue that we can ignore b) for the discussion of
> > the HiSilicon LPC driver, we just need to assign some range
> > of logical addresses to each domain.
> >
> > That means solving c) is the important problem here, and it
> > shouldn't be so hard. We can do this either with a single
> > special domain as in the v5 patch series, or by generalizing it
> > so that any I/O space mapping gets looked up through the device
> > pointer of the bus master.
>
> I am not very on the "generalized" multi-domain solution...
> Currently the IO accessors prototypes have an unsigned long addr
> as input parameter. If we live in a multi-domain IO system
> how can we distinguish inside the accessor which domain addr
> belongs to?
The easiest change compared to the v5 code would be to walk
a linked list of 'struct extio_ops' structures rather than
assuming there is only ever one of them. I think one of the
earlier versions actually did this.
Another option the IA64 approach mentioned in another subthread
today, looking up the operations based on an index from the
upper bits of the port number. If we do this, we probably
want to do that for all PIO access and replace the entire
virtual address remapping logic with that. I think Bjorn
in the past argued in favor of such an approach, while I
advocated the current scheme for simplicity based on how
every I/O space these days is just memory mapped (which now
turned out to be false, both on powerpc and arm64).
Arnd
^ permalink raw reply
* [arm-soc:qcom/arm64 13/13] arch/arm64/boot/dts/qcom/msm8994.dtsi:14:48: fatal error: dt-bindings/clock/qcom,gcc-msm8994.h: No such file or directory
From: kbuild test robot @ 2016-11-18 12:20 UTC (permalink / raw)
To: linux-arm-kernel
tree: https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git qcom/arm64
head: feeaf56ac78d283efe65ea60ec999d4bf3cf395e
commit: feeaf56ac78d283efe65ea60ec999d4bf3cf395e [13/13] arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout feeaf56ac78d283efe65ea60ec999d4bf3cf395e
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
In file included from arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts:16:0:
>> arch/arm64/boot/dts/qcom/msm8994.dtsi:14:48: fatal error: dt-bindings/clock/qcom,gcc-msm8994.h: No such file or directory
#include <dt-bindings/clock/qcom,gcc-msm8994.h>
^
compilation terminated.
vim +14 arch/arm64/boot/dts/qcom/msm8994.dtsi
8 * but WITHOUT ANY WARRANTY; without even the implied warranty of
9 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
10 * GNU General Public License for more details.
11 */
12
13 #include <dt-bindings/interrupt-controller/arm-gic.h>
> 14 #include <dt-bindings/clock/qcom,gcc-msm8994.h>
15
16 / {
17 model = "Qualcomm Technologies, Inc. MSM 8994";
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 31923 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161118/0ec2fd4b/attachment-0001.gz>
^ permalink raw reply
* [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU
From: Arnd Bergmann @ 2016-11-18 12:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <62d93a1f-fd34-771b-4280-6b7cde5c61ad@samsung.com>
On Friday, November 18, 2016 8:54:30 AM CET pankaj.dubey wrote:
> >> Please let me know if any concern in this approach.
> >
> > I think ideally we wouldn't even need to know the virtual address
> > outside of smp_scu.c. If we can move all users of the address
> > into that file directly, it could become a local variable and
> > we change scu_power_mode() and scu_get_core_count() instead to
> > not require the address argument.
> >
>
> If we change scu_get_core_count() without address argument, what about
> those platforms which are calling this API very early boot (mostly in
> smp_init_cpus), during this stage we can't use iomap. These platforms
> are doing static mapping of SCU base, and passing virtual address.
>
> Currently following are platforms which needs SCU virtual address at
> very early stage mostly for calling scu_get_core_count(scu_address):
> IMX, ZYNQ, SPEAr, and OMAP2.
Ah, you are right, I missed how this is done earlier than the
scu_enable() call.
> I can think of handling of these platform's early need of SCU in the
> following way:
>
> Probably we need something like early_a9_scu_get_core_count() which can
> be handled in this way in smp_scu.c file itself:
>
> +static struct map_desc scu_map __initdata = {
> + .length = SZ_256,
> + .type = MT_DEVICE,
> +};
> +
> +static void __iomem *early_scu_map_io(void)
> +{
> + unsigned long base;
> + void __iomem *scu_base;
> +
> + base = scu_a9_get_base();
> + scu_map.pfn = __phys_to_pfn(base);
> + scu_map.virtual = base;
> + iotable_init(&scu_map, 1);
> + scu_base = (void __iomem *)base;
> + return scu_base;
> +}
Unfortunately, this doesn't work because scu_map.virtual must be
picked by the platform code in a way that doesn't conflict
with anything else. Setting up the iotable is probably not
something we should move into the smp_scu.c file but leave where
it currently is. You copied the trick from zynq that happens
to work there but probably not on the other three.
At some point we decided that at least new platforms should
always get the core count from DT rather than from the SCU,
but I don't know if we have to keep supporting old DTB files
without a full description of the CPUs on any of the
four platforms.
I see that there are four other users of scu_get_core_count()
that all call it from ->prepare_cpus, and we can probably
just use num_possible_cpus() at that point as the count
must have been initialized from DT already.
> +/*
> + * early_a9_scu_get_core_count - Get number of CPU cores at very early boot
> + * Only platforms which needs number_of_cores during early boot should
> call this
> + */
> +unsigned int __init early_a9_scu_get_core_count(void)
> +{
> + void __iomem *scu_base;
> +
> + scu_base = early_scu_map_io();
> + return scu_get_core_count(scu_base);
> +}
> +
>
> Please let me know how about using above approach to simplify platform
> specific code of early users of SCU address.
>
> Also for rest platforms, which are not using scu base at very early
> stage, but are using virtual address which is mapped either from SCU
> device node or using s9_scu_get_base() to map to SCU virtual address. To
> separate these two we discussed to separate scu_enable in two helper
> APIs as of_scu_enable and s9_scu_enable. So if we change
> scu_get_core_count which do not requires address argument, this also
> needs to be separated in two as of_scu_get_core_count and
> s9_scu_get_core_count.
We can probably leave get_core_count code alone for now, or
we change it so that we pass a virtual address into it
from the platform and have the SCU mapped there. One nice
property of the early mapping is that the later ioremap()
will just return the same virtual address, so we don't get
a double mapping when calling the scu_enable variant later.
Adding two functions of each doesn't sound too great though,
it would make the API more complicated when the intention was
to make it simpler by leaving out the address argument.
Maybe something like this?
diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c
index 72f9241ad5db..c248a16e980f 100644
--- a/arch/arm/kernel/smp_scu.c
+++ b/arch/arm/kernel/smp_scu.c
@@ -24,6 +24,8 @@
#define SCU_INVALIDATE 0x0c
#define SCU_FPGA_REVISION 0x10
+static void __iomem *scu_base_addr;
+
#ifdef CONFIG_SMP
/*
* Get the number of CPU cores from the SCU configuration
@@ -41,6 +43,9 @@ void scu_enable(void __iomem *scu_base)
{
u32 scu_ctrl;
+ if (scu_base)
+ scu_base = scu_base_addr;
+
#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;
+
if (mode > 3 || mode == 1 || cpu > 3)
return -EINVAL;
@@ -94,3 +102,31 @@ int scu_power_mode(void __iomem *scu_base, unsigned int mode)
return 0;
}
+
+int __init scu_probe_a9(void)
+{
+ phys_addr_t base;
+
+ if (!scu_a9_has_base)
+ return -ENODEV;
+
+ base = scu_a9_get_base()
+ if (!base)
+ return -ENODEV;
+
+ scu_base_addr = ioremap(base, PAGE_SIZE);
+ if (scu_base_addr)
+ return -ENOMEM;
+
+ return scu_get_core_count(scu_base_addr);
+}
+
+int __init scu_probe_dt(void)
+{
+ ...
+ scu_base_addr = of_iomap(np, 0);
+ if (scu_base_addr)
+ return -ENOMEM;
+
+ return scu_get_core_count(scu_base_addr);
+}
Arnd
^ permalink raw reply related
* [RFC 1/6] drm/etnaviv: add binding for the gc320 found in ti socs
From: Russell King - ARM Linux @ 2016-11-18 12:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0bf288ee-33bc-c3b5-389f-08d9bb89ccb5@ti.com>
On Thu, Nov 17, 2016 at 08:53:38PM -0600, Nishanth Menon wrote:
> >diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> >index a6799b0..ce51270 100644
> >--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> >+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> >@@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev)
> > static const struct of_device_id dt_match[] = {
> > { .compatible = "fsl,imx-gpu-subsystem" },
> > { .compatible = "marvell,dove-gpu-subsystem" },
> >+ { .compatible = "ti,gc320-gpu-subsystem" },
We need to get away from this ever-increasing set of compatible
strings here, as this is not long-term maintainable.
What we should have is a common compatible which describes that
the node is compatible with this driver, and then use SoC specific
compatible strings later if we need to (eg, because of some GPU
subsystem SoC specifics.)
So, I'd suggest that we update the documentation and add:
"vivante,gc-gpu-subsystem"
as a common compatible now, and if necessary move on to more specific
compatibles if we need to later.
Also, I'd strongly suggest that no compatibles should contain the ID
number of the GPU core for exactly the same reason - Vivante GPU cores
vary according to features, and we don't want to end up with a long
list of specific compatibles (eg)
"ti,gc2000-and-gc320-and-gc355-gpu-subsystem" because TI
decides to integrate a 3d, 2d and VG core.
--
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 v27 1/9] memblock: add memblock_cap_memory_range()
From: Will Deacon @ 2016-11-18 12:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <582DF05A.9050601@arm.com>
On Thu, Nov 17, 2016 at 06:00:58PM +0000, James Morse wrote:
> On 17/11/16 11:19, Will Deacon wrote:
> > It looks much better, thanks! Just one question below.
> >
>
> > On Thu, Nov 17, 2016 at 02:34:24PM +0900, AKASHI Takahiro wrote:
> >> diff --git a/mm/memblock.c b/mm/memblock.c
> >> index 7608bc3..fea1688 100644
> >> --- a/mm/memblock.c
> >> +++ b/mm/memblock.c
> >> @@ -1514,11 +1514,37 @@ void __init memblock_enforce_memory_limit(phys_addr_t limit)
> >> (phys_addr_t)ULLONG_MAX);
> >> }
> >>
> >> +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> >> +{
> >> + int start_rgn, end_rgn;
> >> + int i, ret;
> >> +
> >> + if (!size)
> >> + return;
> >> +
> >> + ret = memblock_isolate_range(&memblock.memory, base, size,
> >> + &start_rgn, &end_rgn);
> >> + if (ret)
> >> + return;
> >> +
> >> + /* remove all the MAP regions */
> >> + for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> >> + if (!memblock_is_nomap(&memblock.memory.regions[i]))
> >> + memblock_remove_region(&memblock.memory, i);
> >
> > In the case that we have only one, giant memblock that covers base all
> > of base + size, can't we end up with start_rgn = end_rgn = 0? In which
>
> Can this happen? If we only have one memblock that exactly spans
> base:(base+size), memblock_isolate_range() will hit the '@rgn is fully
> contained, record it' code and set start_rgn=0,end_rgn=1. (rbase==base,
> rend==end). We only go round the loop once.
>
> If we only have one memblock that is bigger than base:(base+size) we end up with
> three regions, start_rgn=1,end_rgn=2. The trickery here is the '@rgn intersects
> from above' code decreases the loop counter so we process the same entry twice,
> hitting '@rgn is fully contained, record it' the second time round... so we go
> round the loop four times.
>
> I can't see how we hit the:
> > if (rbase >= end)
> > break;
> > if (rend <= base)
> > continue;
>
> code in either case...
I consistently misread that as rend >= end and rbase <= base! In which case,
I agree with your analysis:
Reviewed-by: Will Deacon <will.deacon@arm.com>
The patch could probably still use an ack from an mm person.
Will
^ permalink raw reply
* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Gabriele Paoloni @ 2016-11-18 12:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2281986.miqFuFkAbO@wuerfel>
Hi Arnd many thanks for your help here
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 18 November 2016 10:18
> To: liviu.dudau at arm.com
> Cc: Gabriele Paoloni; linux-arm-kernel at lists.infradead.org;
> Yuanzhichang; mark.rutland at arm.com; devicetree at vger.kernel.org;
> lorenzo.pieralisi at arm.com; minyard at acm.org; linux-pci at vger.kernel.org;
> benh at kernel.crashing.org; John Garry; will.deacon at arm.com; linux-
> kernel at vger.kernel.org; xuwei (O); Linuxarm; zourongrong at gmail.com;
> robh+dt at kernel.org; kantyzc at 163.com; linux-serial at vger.kernel.org;
> catalin.marinas at arm.com; olof at lixom.net; bhelgaas at go ogle.com;
> zhichang.yuan02 at gmail.com; Jason Gunthorpe; Thomas Petazzoni
> Subject: Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on
> Hip06
>
> On Monday, November 14, 2016 11:26:25 AM CET liviu.dudau at arm.com wrote:
> > On Mon, Nov 14, 2016 at 08:26:42AM +0000, Gabriele Paoloni wrote:
> > > > Nope, that is not what it means. It means that PCI devices can
> see I/O
> > > > addresses
> > > > on the bus that start from 0. There never was any usage for non-
> PCI
> > > > controllers
> > >
> > > So I am a bit confused...
> > > From http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> > > It seems that ISA buses operate on cpu I/O address range [0,
> 0xFFF].
> > > I thought that was the reason why for most architectures we have
> > > PCIBIOS_MIN_IO equal to 0x1000 (so I thought that ISA controllers
> > > usually use [0, PCIBIOS_MIN_IO - 1] )
> >
> > First of all, cpu I/O addresses is an x86-ism. ARM architectures and
> others
> > have no separate address space for I/O, it is all merged into one
> unified
> > address space. So, on arm/arm64 for example, PCIBIOS_MIN_IO = 0 could
> mean
> > that we don't care about ISA I/O because the platform does not
> support having
> > an ISA bus (e.g.).
>
> I think to be more specific, PCIBIOS_MIN_IO=0 would indicate that you
> cannot
> have a PCI-to-ISA or PCI-to-LPC bridge in any PCI domain. This is
> different
> from having an LPC master outside of PCI, as that lives in its own
> domain
> and has a separately addressable I/O space.
Yes correct so if we go for the single domain solution arch that
have PCIBIOS_MIN_IO=0 cannot support special devices such as LPC
unless we also redefine PCIBIOS_MIN_IO, right?
>
> > > As said before this series forbid IO tokens to be in [0,
> PCIBIOS_MIN_IO)
> > > to allow special ISA controllers to use that range with special
> > > accessors.
> > > Having a variable threshold would make life much more difficult
> > > as there would be a probe dependency between the PCI controller and
> > > the special ISA one (PCI to wait for the special ISA device to be
> > > probed and set the right threshold value from DT or ACPI table).
> > >
> > > Instead using PCIBIOS_MIN_IO is easier and should not impose much
> > > constraint as [PCIBIOS_MIN_IO, IO_SPACE_LIMIT] is available to
> > > the PCI controller for I/O tokens...
> >
> > What I am suggesting is to leave PCIBIOS_MIN_IO alone which still
> reserves
> > space for ISA controller and add a PCIBIOS_MIN_DIRECT_IO that will
> reserve
> > space for your direct address I/O on top of PCIBIOS_MIN_IO.
>
> The PCIBIOS_MIN_DIRECT_IO name still suggests having something related
> to
> PCIBIOS_MIN_IO, but it really isn't. We are talking about multiple
> concepts here that are not the same but that are somewhat related:
>
> a) keeping PCI devices from allocating low I/O ports on the PCI bus
> that would conflict with ISA devices behind a bridge of the
> same bus.
>
> b) reserving the low 0x0-0xfff range of the Linux-internal I/O
> space abstraction to a particular LPC or PCI domain to make
> legacy device drivers work that hardcode a particular port
> number.
>
> c) Redirecting inb/outb to call a domain-specific accessor function
> rather than doing the normal MMIO window for an LPC master or
> more generally any arbitrary LPC or PCI domain that has a
> nonstandard I/O space.
> [side note: actually if we generalized this, we could avoid
> assigning an MMIO range for the I/O space on the pci-mvebu
> driver, and that would help free up some other remapping
> windows]
>
> I think there is no need to change a) here, we have PCIBIOS_MIN_IO
> today and even if we don't need it, there is no obvious downside.
> I would also argue that we can ignore b) for the discussion of
> the HiSilicon LPC driver, we just need to assign some range
> of logical addresses to each domain.
>
> That means solving c) is the important problem here, and it
> shouldn't be so hard. We can do this either with a single
> special domain as in the v5 patch series, or by generalizing it
> so that any I/O space mapping gets looked up through the device
> pointer of the bus master.
I am not very on the "generalized" multi-domain solution...
Currently the IO accessors prototypes have an unsigned long addr
as input parameter. If we live in a multi-domain IO system
how can we distinguish inside the accessor which domain addr
belongs to?
Thanks
Gab
>
> Arnd
^ permalink raw reply
* [PATCH] arm: spin one more cycle in timer-based delays
From: Will Deacon @ 2016-11-18 12:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <582B0F61.6030903@free.fr>
On Tue, Nov 15, 2016 at 02:36:33PM +0100, Mason wrote:
> When polling a tick counter in a busy loop, one might sample the
> counter just *before* it is updated, and then again just *after*
> it is updated. In that case, while mathematically v2-v1 equals 1,
> only epsilon has really passed.
>
> So, if a caller requests an N-cycle delay, we spin until v2-v1
> is strictly greater than N to avoid these random corner cases.
>
> Signed-off-by: Mason <slash.tmp@free.fr>
> ---
> arch/arm/lib/delay.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c
> index 69aad80a3af4..3f1cd15ca102 100644
> --- a/arch/arm/lib/delay.c
> +++ b/arch/arm/lib/delay.c
> @@ -60,7 +60,7 @@ static void __timer_delay(unsigned long cycles)
> {
> cycles_t start = get_cycles();
>
> - while ((get_cycles() - start) < cycles)
> + while ((get_cycles() - start) <= cycles)
> cpu_relax();
> }
I thought a bit about this last night. It is well known that the delay
routines are not guaranteed to be accurate, and I don't *think* that's
limited to over-spinning, so arguably this isn't a bug. However, taking
the approach that "drivers should figure it out" is also unhelpful,
because the frequency of the underlying counter isn't generally known
and so drivers can't simply adjust the delay parameter.
If you want to go ahead with this change, I do think that you should fix
the other architectures for consistency (particularly arm64, which is
likely to share drivers with arch/arm/). However, given that I don't
think you've seen a failure in practice, it might be a hard sell to get
the patches picked up, especially given the deliberately vague guarantees
offered by the delay loop.
Will
^ permalink raw reply
* [PATCH v3 06/13] ARM: dts: armada-375: Fixup sa-ram DT warning
From: Gregory CLEMENT @ 2016-11-18 12:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118095925.770496c3@free-electrons.com>
Hi Thomas,
On ven., nov. 18 2016, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Fri, 18 Nov 2016 00:08:23 +0100, Gregory CLEMENT wrote:
>
>> - crypto_sram0: sa-sram0 {
>> + /* The following unit addresses (for sa-sram) are composed of
>> + * the target value (bit [40-47]), attributes value (bits
>> + * [32-39], and the address value in the window memory: [0-31].
>> + */
>
> The "address value in the window memory" part doesn't make a lot of
> sense. Maybe:
>
> "The following unit addresses are composed of the window target ID
> (bits 40-47), the window target attributes (bits 32-39) and the offset
> inside the window."
I'm fine with it
>
> Also, the comment formatting is not compliant with the coding style,
> should be:
>
> /*
> * ...
> * ...
> */
>
Hum yes I need to teach emacs how to properly format the comments.
> But do we really want this comment above each node? Couldn't we instead
> add this explanation in the mvebu-mbus.txt DT binding?
We could but I fear that nobody will read it.
Indeed if you know that in order to understand the unit address, you will
have to have a look an the binding of the mvebu-mbus, then it means that
you already are an expert and actually you barely need to read it!
In order to have less change we could at least put it near the MBUS_ID
macro and if the mvebu-mbus.txt DT binding too.
Gregory
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH 15/20] ARM/hw_breakpoint: Convert to hotplug state machine
From: Will Deacon @ 2016-11-18 12:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161117183541.8588-16-bigeasy@linutronix.de>
On Thu, Nov 17, 2016 at 07:35:36PM +0100, Sebastian Andrzej Siewior wrote:
> Install the callbacks via the state machine and let the core invoke
> the callbacks on the already online CPUs.
>
> smp_call_function_single() has been removed because the function is already
> invoked on the target CPU.
>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> arch/arm/kernel/hw_breakpoint.c | 44 ++++++++++++++++++-----------------------
> 1 file changed, 19 insertions(+), 25 deletions(-)
[...]
> static int __init arch_hw_breakpoint_init(void)
> {
> + int ret;
> +
> debug_arch = get_debug_arch();
>
> if (!debug_arch_supported()) {
> @@ -1072,8 +1069,6 @@ static int __init arch_hw_breakpoint_init(void)
> core_num_brps = get_num_brps();
> core_num_wrps = get_num_wrps();
>
> - cpu_notifier_register_begin();
> -
> /*
> * We need to tread carefully here because DBGSWENABLE may be
> * driven low on this core and there isn't an architected way to
> @@ -1082,15 +1077,18 @@ static int __init arch_hw_breakpoint_init(void)
> register_undef_hook(&debug_reg_hook);
>
> /*
> - * Reset the breakpoint resources. We assume that a halting
> - * debugger will leave the world in a nice state for us.
> + * Register CPU notifier which resets the breakpoint resources. We
> + * assume that a halting debugger will leave the world in a nice state
> + * for us.
> */
> - on_each_cpu(reset_ctrl_regs, NULL, 1);
> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "arm/hw_breakpoint:online",
> + dbg_reset_online, NULL);
I'm slightly unsure about this. The dbg_reset_online callback can execute
undefined instructions (unfortunately, there's no way to probe the presence
of some of the debug registers), so it absolutely has to run within the
register_undef_hook/unregister_undef_hook calls that are in this function.
With this patch, I worry that the callback can be postponed to ONLINE time
for other CPUs, and then the kernel will panic.
Will
^ permalink raw reply
* [GIT PULL v2] firmware: SCPI updates for v4.10
From: Sudeep Holla @ 2016-11-18 11:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161028112920.GB12241@e107155-lin>
Hi ARM-SoC Team,
I have decoupled the platform specific binding from generic SCPI. Also
I have renamed "arm,legacy-scpi" to "arm,scpi-pre-1.0". Since I haven't
heard back any objections from Olof/Rob for my response, I am sending
the pull request now.
--
Regards,
Sudeep
The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git tags/scpi-updates-4.10
for you to fetch changes up to 8358c6b5fc8c160d0af8654f313b8a7745f8e304:
firmware: arm_scpi: add support for pre-v1.0 SCPI compatible (2016-11-17 16:31:20 +0000)
----------------------------------------------------------------
SCPI updates for v4.10
1. Adds support for pre-v1.0 SCPI protocol versions
2. Adds support for SCPI used on Amlogic GXBB SoC platforms using the
newly added pre-v1.0 SCPI protocol
3. Decouples some platform specific details from generic SCPI binding
----------------------------------------------------------------
Neil Armstrong (4):
firmware: arm_scpi: increase MAX_DVFS_OPPS to 16 entries
firmware: arm_scpi: add alternative legacy structures, functions and macros
firmware: arm_scpi: allow firmware with get_capabilities not implemented
Documentation: bindings: Add support for Amlogic GXBB SCPI protocol
Sudeep Holla (4):
firmware: arm_scpi: add command indirection to support legacy commands
Documentation: bindings: decouple juno specific details from generic binding
Documentation: bindings: add compatible specific to pre v1.0 SCPI protocols
firmware: arm_scpi: add support for pre-v1.0 SCPI compatible
.../devicetree/bindings/arm/amlogic,scpi.txt | 20 ++
Documentation/devicetree/bindings/arm/arm,scpi.txt | 25 +-
.../devicetree/bindings/arm/juno,scpi.txt | 26 ++
drivers/firmware/arm_scpi.c | 276 ++++++++++++++++++---
4 files changed, 301 insertions(+), 46 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/amlogic,scpi.txt
create mode 100644 Documentation/devicetree/bindings/arm/juno,scpi.txt
^ permalink raw reply
* [GIT PULL 2/3] ARM64: dts: exynos: DT for v4.10
From: Krzysztof Kozlowski @ 2016-11-18 11:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118015954.GP8578@localhost>
On Thu, Nov 17, 2016 at 05:59:54PM -0800, Olof Johansson wrote:
> On Tue, Nov 08, 2016 at 08:26:29PM +0200, Krzysztof Kozlowski wrote:
> > Hi,
> >
> > Exynos5433 + two boards using it. Mobile boards! :)
> >
> > I am really happy to push it. I know that it has been a lot of effort
> > in Samsung to mainline this.
> >
> > Best regards,
> > Krzysztof
> >
> >
> > The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
> >
> > Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
> >
> > are available in the git repository at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git tags/samsung-dt64-4.10
> >
> > for you to fetch changes up to 8ac46fc57df82efbc19194dddd335b6c7a960c31:
> >
> > arm64: dts: exynos: Add dts file for Exynos5433-based TM2E board (2016-11-03 22:19:57 +0200)
> >
> > ----------------------------------------------------------------
> > Finally, I am really pleased to announce adding support for Exynos5433 ARMv8
> > SoC along with two boards. A lot of Samsung people contributed into this
> > but the final work and commits were done by Chanwoo Choi.
> >
> > This means that for v4.10 we got:
> > 1. Exynos5433 DTSI.
> > 2. Two boards: TM2 and TM2E. These are (almost fully) working mobile phones.
>
> Awesome! Looks like TM2 is a Tizen reference board? Great to see the support,
> even if it's taken a while.
Yes, I think these were planned to be reference devices. Userspace
packages and snapshots are published for some quite long time
(http://download.tizen.org/snapshots/tizen/3.0-mobile/latest/images/arm64-wayland/).
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-11-18 11:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <582469C8.9090609@hisilicon.com>
[found this old mail in my drafts folder, might as well send it now]
On Thursday, November 10, 2016 8:36:24 PM CET zhichang.yuan wrote:
> Sorry! I can't catch your idea yet:(
>
> When to register the I/O range? Is it done just after the successfully
> of_translate_address() during the children scanning?
No, you do it when first finding the bus itself, just like we do for
PCI host bridges.
> If yes, when a child is scanning, there is no range data in arm64_extio_ops. The
> addr_is_indirect_io() calling in of_get_isa_indirect_io() don't need. All we can
> check is just whether the address to be translated is IO and is under a parent
> device which has no 'ranges' property.
The children should only be scanned after the I/O range has been
registered for the parent.
> > Your current version has
> >
> > if (arm64_extio_ops->pfout) \
> > arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> > addr, value, sizeof(type)); \
> >
> > Instead, just subtract the start of the range from the logical
> > port number to transform it back into a bus-local port number:
> >
> > if (arm64_extio_ops->pfout) \
> > arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> > addr - arm64_extio_ops->start, value, sizeof(type)); \
> >
> I think there is some information needed sync.
> In the old patch-set, we don't bypass the pci_address_to_pio() after
> successfully of_translate_address(). In this way, we don't need to reserve any
> PIO space for our LPC since the logical port are from the same mapping
> algorithm. Based on this way, the port number in the device resource is logical
> one, then we need to subtract the start of the resource to get back the
> bus-local port.
>
> From V3, we don't apply the mapping based on pci_address_to_pio(), the
> of_translate_address() return the bus-local port directly and store into
> relevant device resource. So, in the current arm64_extio_ops->pfout(), the
> reverse translation don't need anymore. The input "addr" is bus-local port now.
Ok, so this would have to be changed again: If we want to support multiple
bus domains, of_translate_address() must translate between the bus specific
address and the general Linux I/O port number. Even without doing that,
it seems nicer to not overlap the range of the first PCI host bridge.
Arnd
^ permalink raw reply
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Arnd Bergmann @ 2016-11-18 11:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <582EE223.4090605@hisilicon.com>
On Friday, November 18, 2016 7:12:35 PM CET zhichang.yuan wrote:
> Hi, Arnd,
>
>
> On 2016/11/18 17:20, Arnd Bergmann wrote:
> > On Friday, November 11, 2016 6:07:07 PM CET zhichang.yuan wrote:
> >>
> >> I have similar idea as your PPC MMIO.
> >>
> >> We notice the prototype of {in/out()} is something like that:
> >>
> >> static inline u8 inb(unsigned long addr)
> >> static inline void outb(u8 value, unsigned long addr)
> >>
> >> The type of parameter 'addr' is unsigned long. For I/O space, it is big enough.
> >> So, could you divide this 'addr' into several bit segments? The top 8 bits is
> >> defined as bus index. For normal direct IO, the bus index is 0. For those bus
> >> device which need indirectIO or some special I/O accessors, when these devices
> >> are initializing, can request to allocate an unique ID to them, and register
> >> their own accessors to the entry which is corresponding to the ID.
> >
> > Ah, have you looked at the IA64 code? It does exactly this.
> > For ARM64 we decided to use the same basic approach as powerpc with
> > a single range of virtual memory for mapping it as that somewhat
> > simplified all cases we knew about at the time.
>
> Yes. I spent some time to trace how to work on PPC. But the code is a bit long,
> I am not clear on how the indirectIO there was supported.
>
> I noticed there are CONFIG_PPC_INDIRECT_PIO and CONFIG_PPC_INDIRECT_MMIO on PPC.
> It seems that only CONFIG_PPC_INDIRECT_MMIO applied some MSB to store the bus
> tokens which are used to get iowa_busses[] for specific operation helpers.
> I can not find how CONFIG_PPC_INDIRECT_PIO support multiple ISA domains. It
> seems only Opal-lpc.c adopt this INDIRECT_PIO method.
>
> Although CONFIG_PPC_INDIRECT_MMIO is for MMIO, seems not suitable for ISA/LPC
> I/O. But this idea is helpful.
>
> what else did I miss??
I mentioned two different things here: ia64 IIRC uses some bits of the
port number to look up the domain, while powerpc traditionally had no
support for any such lookup, it did the same thing as ARM64 with
virtual remapping of MMIO ranges into an address range starting at
a fixed virtual address.
CONFIG_PPC_INDIRECT_PIO is a fairly recent addition, I was not thinking
of that.
> >> In this way, we can support multiple domains, I think.
> >> But I am not sure whether it is feasible, for example, are there some
> >> architectures/platforms had populated the top 8 bits? Do we need to request IO
> >> region from ioport_resource for those devices? etc...
> >
> > On a 64-bit architecture, the top 32 bits of the port number are
> > definitely free to use for this, and 8 bits are probably sufficient.
> >
> > Even on 32 bit architectures, I can't see why we'd ever need more than
> > 16 bits worth of addressing within a domain, so using 8 bit domain
> > and 16 bit address leaves 8 or 40 unused bits.
>
> Yes. 8 bits are enough.
> But the maximal PIO on some architectures are defined as ~0 or -1. There is no
> any bare space left. Probably we can not ensure the upper 8 bits available.
Right, we clearly can't use it across all architectures. The trick with
architectures using ULONG_MAX as the limit for port numbers is that they
treat it as a 1:1 mapping between port numbers and virtual addresses,
which is yet another way to handle the MMIO-based devices, but that has
a number of downsides we don't need to get into now.
What I think the code should do is a generic workaround handling that
architectures can opt-in to. We'd start doing this on ARM64 only,
and can then decide whether to change ARM or PowerPC over to use
that as well.
Arnd
^ permalink raw reply
* [arm-soc:qcom/arm64 11/13] arch/arm64/boot/dts/qcom/msm8992.dtsi:14:48: fatal error: dt-bindings/clock/qcom,gcc-msm8994.h: No such file or directory
From: kbuild test robot @ 2016-11-18 11:30 UTC (permalink / raw)
To: linux-arm-kernel
tree: https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git qcom/arm64
head: feeaf56ac78d283efe65ea60ec999d4bf3cf395e
commit: 6a6d1978f9c0d818b4370903e1f3eecf1681c932 [11/13] arm64: dts: msm8992 SoC and LG Bullhead (Nexus 5X) support
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 6a6d1978f9c0d818b4370903e1f3eecf1681c932
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
In file included from arch/arm64/boot/dts/qcom/msm8992-bullhead-rev-101.dts:16:0:
>> arch/arm64/boot/dts/qcom/msm8992.dtsi:14:48: fatal error: dt-bindings/clock/qcom,gcc-msm8994.h: No such file or directory
#include <dt-bindings/clock/qcom,gcc-msm8994.h>
^
compilation terminated.
vim +14 arch/arm64/boot/dts/qcom/msm8992.dtsi
8 * but WITHOUT ANY WARRANTY; without even the implied warranty of
9 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
10 * GNU General Public License for more details.
11 */
12
13 #include <dt-bindings/interrupt-controller/arm-gic.h>
> 14 #include <dt-bindings/clock/qcom,gcc-msm8994.h>
15
16 / {
17 model = "Qualcomm Technologies, Inc. MSM 8992";
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 31923 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161118/3411caec/attachment-0001.gz>
^ permalink raw reply
* [PATCH v3] ARM: Drop fixed 200 Hz timer requirement from Samsung platforms
From: Krzysztof Kozlowski @ 2016-11-18 11:15 UTC (permalink / raw)
To: linux-arm-kernel
All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
200 Hz. Unfortunately in case of multiplatform image this affects also
other platforms when Exynos is enabled.
This looks like an very old legacy code, dating back to initial
upstreaming of S3C24xx. Probably it was required for s3c24xx timer
driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
unused plat-samsung/time.c").
Since then, this fixed 200 Hz spread everywhere, including out-of-tree
Samsung kernels (SoC vendor's and Tizen's). I believe this choice
was rather an effect of coincidence instead of conscious choice.
On S3C24xx, the PWM counter is only 16 bit wide, and with the
typical 12MHz input clock that overflows every 5.5ms. This works
with HZ=200 or higher but not with HZ=100 which needs a 10ms
interval between ticks. On Later chips (S3C64xx, S5P and EXYNOS),
the counter is 32 bits and does not have this problem.
The new samsung_pwm_timer driver solves the problem by scaling the input
clock by a factor of 50 on S3C24xx, which makes it less accurate but
allows HZ=100 as well as CONFIG_NO_HZ with fewer wakeups.
Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
other values.
Reported-by: Lee Jones <lee.jones@linaro.org>
[Dropping of 200_HZ from S3C/S5P was suggested by Arnd]
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Kukjin Kim <kgene@kernel.org>
[Tested on Exynos5800]
Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Kukjin Kim <kgene@kernel.org>
[Tested on S3C2440]
Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
Changes since v2:
1. Extend message.
2. Add Kukjin's ack.
3. Add Sylwester's tested-by.
Changes since v1:
1. Add Javier's tested-by.
2. Drop HZ_FIXED also from ARCH_S5PV210 and ARCH_S3C24XX after Arnd
suggestions and analysis.
---
arch/arm/Kconfig | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b5d529fdffab..ced2e08a9d08 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1496,8 +1496,7 @@ source kernel/Kconfig.preempt
config HZ_FIXED
int
- default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
- ARCH_S5PV210 || ARCH_EXYNOS4
+ default 200 if ARCH_EBSA110
default 128 if SOC_AT91RM9200
default 0
--
2.7.4
^ permalink raw reply related
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: zhichang.yuan @ 2016-11-18 11:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2436881.9RqUVYxmDf@wuerfel>
Hi, Arnd,
On 2016/11/18 17:20, Arnd Bergmann wrote:
> On Friday, November 11, 2016 6:07:07 PM CET zhichang.yuan wrote:
>>
>> I have similar idea as your PPC MMIO.
>>
>> We notice the prototype of {in/out()} is something like that:
>>
>> static inline u8 inb(unsigned long addr)
>> static inline void outb(u8 value, unsigned long addr)
>>
>> The type of parameter 'addr' is unsigned long. For I/O space, it is big enough.
>> So, could you divide this 'addr' into several bit segments? The top 8 bits is
>> defined as bus index. For normal direct IO, the bus index is 0. For those bus
>> device which need indirectIO or some special I/O accessors, when these devices
>> are initializing, can request to allocate an unique ID to them, and register
>> their own accessors to the entry which is corresponding to the ID.
>
> Ah, have you looked at the IA64 code? It does exactly this.
> For ARM64 we decided to use the same basic approach as powerpc with
> a single range of virtual memory for mapping it as that somewhat
> simplified all cases we knew about at the time.
Yes. I spent some time to trace how to work on PPC. But the code is a bit long,
I am not clear on how the indirectIO there was supported.
I noticed there are CONFIG_PPC_INDIRECT_PIO and CONFIG_PPC_INDIRECT_MMIO on PPC.
It seems that only CONFIG_PPC_INDIRECT_MMIO applied some MSB to store the bus
tokens which are used to get iowa_busses[] for specific operation helpers.
I can not find how CONFIG_PPC_INDIRECT_PIO support multiple ISA domains. It
seems only Opal-lpc.c adopt this INDIRECT_PIO method.
Although CONFIG_PPC_INDIRECT_MMIO is for MMIO, seems not suitable for ISA/LPC
I/O. But this idea is helpful.
what else did I miss??
>
>> In this way, we can support multiple domains, I think.
>> But I am not sure whether it is feasible, for example, are there some
>> architectures/platforms had populated the top 8 bits? Do we need to request IO
>> region from ioport_resource for those devices? etc...
>
> On a 64-bit architecture, the top 32 bits of the port number are
> definitely free to use for this, and 8 bits are probably sufficient.
>
> Even on 32 bit architectures, I can't see why we'd ever need more than
> 16 bits worth of addressing within a domain, so using 8 bit domain
> and 16 bit address leaves 8 or 40 unused bits.
Yes. 8 bits are enough.
But the maximal PIO on some architectures are defined as ~0 or -1. There is no
any bare space left. Probably we can not ensure the upper 8 bits available.
Thanks,
Zhichang
>
> 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