* [RFC] arm64: Enforce observed order for spinlock and data
From: bdegraaf at codeaurora.org @ 2016-10-04 17:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004101208.GA18083@leverpostej>
On 2016-10-04 06:12, Mark Rutland wrote:
> On Mon, Oct 03, 2016 at 03:20:57PM -0400, bdegraaf at codeaurora.org
> wrote:
>> On 2016-10-01 14:11, Mark Rutland wrote:
>> >Hi Brent,
>> >
>> >Evidently my questions weren't sufficiently clear; even with your
>> >answers it's not clear to me what precise issue you're attempting to
>> >solve. I've tried to be more specific this time.
>> >
>> >At a high-level, can you clarify whether you're attempting to solve is:
>> >
>> >(a) a functional correctness issue (e.g. data corruption)
>> >(b) a performance issue
>> >
>> >And whether this was seen in practice, or found through code
>> >inspection?
>
>> Thinking about this, as the reader/writer code has no known "abuse"
>> case, I'll remove it from the patchset, then provide a v2 patchset
>> with a detailed explanation for the lockref problem using the commits
>> you provided as an example, as well as performance consideration.
>
> If there's a functional problem, let's consider that in isolation
> first.
> Once we understand that, then we can consider doing what is optimal.
>
> As should be obvious from the above, I'm confused because this patch
> conflates functional details with performance optimisations which (to
> me) sound architecturally dubious.
>
> I completely agree with Peter that if the problem lies with lockref, it
> should be solved in the lockref code.
>
> Thanks,
> Mark.
After looking at this, the problem is not with the lockref code per se:
it is
a problem with arch_spin_value_unlocked(). In the out-of-order case,
arch_spin_value_unlocked() can return TRUE for a spinlock that is in
fact
locked but the lock is not observable yet via an ordinary load. Other
than
ensuring order on the locking side (as the prior patch did), there is a
way
to make arch_spin_value_unlock's TRUE return value deterministic, but it
requires that it does a write-back to the lock to ensure we didn't
observe
the unlocked value while another agent was in process of writing back a
locked value.
Brent
^ permalink raw reply
* [PATCH v2 1/2] clocksource: arm_arch_timer: Don't assume clock runs in suspend
From: Brian Norris @ 2016-10-04 18:12 UTC (permalink / raw)
To: linux-arm-kernel
The ARM ARM specifies that the system counter "must be implemented in an
always-on power domain," and so we try to use the counter as a source of
timekeeping across suspend/resume. Unfortunately, some SoCs (e.g.,
Rockchip's RK3399) do not keep the counter ticking properly when
switched from their high-power clock to the lower-power clock used in
system suspend. Support this quirk by adding a new device tree property.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2:
* add new device tree property, instead of re-using the "always-on"
property (which has different meaning)
Documentation/devicetree/bindings/arm/arch_timer.txt | 5 +++++
drivers/clocksource/arm_arch_timer.c | 9 ++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
index ef5fbe9a77c7..ad440a2b8051 100644
--- a/Documentation/devicetree/bindings/arm/arch_timer.txt
+++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
@@ -38,6 +38,11 @@ to deliver its interrupts via SPIs.
architecturally-defined reset values. Only supported for 32-bit
systems which follow the ARMv7 architected reset values.
+- arm,no-tick-in-suspend : The main counter does not tick when the system is in
+ low-power system suspend on some SoCs. This behavior does not match the
+ Architecture Reference Manual's specification that the system counter "must
+ be implemented in an always-on power domain."
+
Example:
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 73c487da6d2a..a2503db7e533 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -81,6 +81,7 @@ static struct clock_event_device __percpu *arch_timer_evt;
static enum ppi_nr arch_timer_uses_ppi = VIRT_PPI;
static bool arch_timer_c3stop;
static bool arch_timer_mem_use_virtual;
+static bool arch_counter_suspend_stop;
static bool evtstrm_enable = IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EVTSTREAM);
@@ -576,7 +577,7 @@ static struct clocksource clocksource_counter = {
.rating = 400,
.read = arch_counter_read,
.mask = CLOCKSOURCE_MASK(56),
- .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
+ .flags = CLOCK_SOURCE_IS_CONTINUOUS,
};
static struct cyclecounter cyclecounter = {
@@ -616,6 +617,8 @@ static void __init arch_counter_register(unsigned type)
arch_timer_read_counter = arch_counter_get_cntvct_mem;
}
+ if (!arch_counter_suspend_stop)
+ clocksource_counter.flags |= CLOCK_SOURCE_SUSPEND_NONSTOP;
start_count = arch_timer_read_counter();
clocksource_register_hz(&clocksource_counter, arch_timer_rate);
cyclecounter.mult = clocksource_counter.mult;
@@ -907,6 +910,10 @@ static int __init arch_timer_of_init(struct device_node *np)
of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
arch_timer_uses_ppi = PHYS_SECURE_PPI;
+ /* On some systems, the counter stops ticking when in suspend. */
+ arch_counter_suspend_stop = of_property_read_bool(np,
+ "arm,no-tick-in-suspend");
+
return arch_timer_init();
}
CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_of_init);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 2/2] arm64: dts: rockchip: arch counter doesn't tick in system suspend
From: Brian Norris @ 2016-10-04 18:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475604730-140264-1-git-send-email-briannorris@chromium.org>
The "arm,no-tick-in-suspend" property was introduced to note
implementations where the system counter does not quite follow the ARM
specification that it "must be implemented in an always-on power
domain".
Particularly, RK3399's counter stops ticking when we switch from the
24MHz clock to the 32KHz clock in low-power suspend, so let's mark it as
such.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: new in v2
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 76b28649f0b0..401c8be8c8ac 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -174,6 +174,7 @@
<GIC_PPI 14 IRQ_TYPE_LEVEL_LOW 0>,
<GIC_PPI 11 IRQ_TYPE_LEVEL_LOW 0>,
<GIC_PPI 10 IRQ_TYPE_LEVEL_LOW 0>;
+ arm,no-tick-in-suspend;
};
xin24m: xin24m {
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [RFC] arm64: Enforce observed order for spinlock and data
From: bdegraaf at codeaurora.org @ 2016-10-04 18:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <884bd5d3a9a1bcf2a276130ffc17412a@codeaurora.org>
On 2016-10-04 13:53, bdegraaf at codeaurora.org wrote:
> On 2016-10-04 06:12, Mark Rutland wrote:
>> On Mon, Oct 03, 2016 at 03:20:57PM -0400, bdegraaf at codeaurora.org
>> wrote:
>>> On 2016-10-01 14:11, Mark Rutland wrote:
>>> >Hi Brent,
>>> >
>>> >Evidently my questions weren't sufficiently clear; even with your
>>> >answers it's not clear to me what precise issue you're attempting to
>>> >solve. I've tried to be more specific this time.
>>> >
>>> >At a high-level, can you clarify whether you're attempting to solve is:
>>> >
>>> >(a) a functional correctness issue (e.g. data corruption)
>>> >(b) a performance issue
>>> >
>>> >And whether this was seen in practice, or found through code
>>> >inspection?
>>
>>> Thinking about this, as the reader/writer code has no known "abuse"
>>> case, I'll remove it from the patchset, then provide a v2 patchset
>>> with a detailed explanation for the lockref problem using the commits
>>> you provided as an example, as well as performance consideration.
>>
>> If there's a functional problem, let's consider that in isolation
>> first.
>> Once we understand that, then we can consider doing what is optimal.
>>
>> As should be obvious from the above, I'm confused because this patch
>> conflates functional details with performance optimisations which (to
>> me) sound architecturally dubious.
>>
>> I completely agree with Peter that if the problem lies with lockref,
>> it
>> should be solved in the lockref code.
>>
>> Thanks,
>> Mark.
>
> After looking at this, the problem is not with the lockref code per se:
> it is
> a problem with arch_spin_value_unlocked(). In the out-of-order case,
> arch_spin_value_unlocked() can return TRUE for a spinlock that is in
> fact
> locked but the lock is not observable yet via an ordinary load. Other
> than
> ensuring order on the locking side (as the prior patch did), there is a
> way
> to make arch_spin_value_unlock's TRUE return value deterministic, but
> it
> requires that it does a write-back to the lock to ensure we didn't
> observe
> the unlocked value while another agent was in process of writing back a
> locked value.
>
> Brent
Scratch that--things get complicated as the lock itself gets "cloned,"
which
could happen during the out-of-order window. I'll post back later after
I've
analyzed it fully.
^ permalink raw reply
* [PATCH] ARM: dts: rockchip: Reserve unusable memory region on rk3066
From: Heiko Stübner @ 2016-10-04 18:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <81d1fb8b-ee0e-35a7-77db-5e15c3f46449@gmail.com>
Hi Pawe?,
Am Dienstag, 4. Oktober 2016, 13:56:07 schrieb Pawe? Jarosz:
> >>>> I don't think this is a sane workaround, but it is at best difficult to
> >>>> tell, given there's no reason given for why this memory is unusable.
> >>>>
> >>>> For instance, if bus accesses to this address hang, then this patch
> >>>> only
> >>>> makes the hand less likely, since the kernel will still map the region
> >>>> (and
> >>>> therefore the CPU can perform speculative accesses).
> >>>>
> >>>> Are issues with this memory consistently seen in practice?
> >>>>
> >>>> Can you enable CONFIG_MEMTEST and pass 'memtest' to the kernel, to
> >>>> determine if the memory is returning erroneous values?
> >>>
> >>> just for the sake of completeness, on the rk3288 the issue was the dma
> >>> not
> >>> being able to access the specific memory region (interestingly also the
> >>> last 16MB but of the 4GB area supported on the rk3288). So memory itself
> >>> was ok, just dma access to it failed.
> >>
> >> How odd.
> >>
> >>> We didn't find any other sane solution to limit the dma access in a
> >>> general way at the time, so opted for just blocking the memory region
> >>> (as
> >>> it was similarly only
> >>
> >> I was under the impression that dma-ranges could describe this kind of
> >> DMA addressing limitation. Was there some problem with that? Perhaps the
> >> driver is not acquiring/configuring its mask correctly?
> >
> > I remember looking at (and trying) different options back then.
> >
> > dma-mask wanted power-of-2 values (so it's either 4GB or 2GB (or lower)),
> > zone-dma was a 32bit (and non-dt) thing and dma-ranges seem to simply also
> > calculate a dma-mask from the value, so you're down to 2GB again.
> >
> > So just blocking of those 16MB at the end for 4GB devices somehow sounded
> > nicer than limiting dma access to only half the memory.
> >
> > I may be overlooking something but that was what I came up with last year.
> >
> >
> > Heiko
>
> Is there a chance to accept this patch?
>
> I know it's not the best solution to this problem, but i don't know
> a better one.
there is always a "chance". But with changes like these, we always try to find
a real cause first, before resorting to solutions like this. So it's definitly
not off the table, but I'd like to investigate further first, so that we don't
accumulate unnecessary hacks over time.
Especially that your region seems to be in the middle of the designated ram
area is strange.
Could you please tell which board you're using (and how much memory it has)
Thanks
Heiko
^ permalink raw reply
* [RFC] arm64: Enforce observed order for spinlock and data
From: Mark Rutland @ 2016-10-04 19:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <884bd5d3a9a1bcf2a276130ffc17412a@codeaurora.org>
Hi Brent,
Could you *please* clarify if you are trying to solve:
(a) a correctness issue (e.g. data corruption) seen in practice.
(b) a correctness issue (e.g. data corruption) found by inspection.
(c) A performance issue, seen in practice.
(d) A performance issue, found by inspection.
Any one of these is fine; we just need to know in order to be able to
help effectively, and so far it hasn't been clear.
On Tue, Oct 04, 2016 at 01:53:35PM -0400, bdegraaf at codeaurora.org wrote:
> After looking at this, the problem is not with the lockref code per
> se: it is a problem with arch_spin_value_unlocked(). In the
> out-of-order case, arch_spin_value_unlocked() can return TRUE for a
> spinlock that is in fact locked but the lock is not observable yet via
> an ordinary load.
Given arch_spin_value_unlocked() doesn't perform any load itself, I
assume the ordinary load that you are referring to is the READ_ONCE()
early in CMPXCHG_LOOP().
It's worth noting that even if we ignore ordering and assume a
sequentially-consistent machine, READ_ONCE() can give us a stale value.
We could perform the read, then another agent can acquire the lock, then
we can move onto the cmpxchg(), i.e.
CPU0 CPU1
old = READ_ONCE(x.lock_val)
spin_lock(x.lock)
cmpxchg(x.lock_val, old, new)
spin_unlock(x.lock)
If the 'old' value is stale, the cmpxchg *must* fail, and the cmpxchg
should return an up-to-date value which we will then retry with.
> Other than ensuring order on the locking side (as the prior patch
> did), there is a way to make arch_spin_value_unlock's TRUE return
> value deterministic,
In general, this cannot be made deterministic. As above, there is a race
that cannot be avoided.
> but it requires that it does a write-back to the lock to ensure we
> didn't observe the unlocked value while another agent was in process
> of writing back a locked value.
The cmpxchg gives us this guarantee. If it successfully stores, then the
value it observed was the same as READ_ONCE() saw, and the update was
atomic.
There *could* have been an intervening sequence between the READ_ONCE
and cmpxchg (e.g. put(); get()) but that's not problematic for lockref.
Until you've taken your reference it was possible that things changed
underneath you.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v3 2/7] i2c: bcm2835: Protect against unexpected TXW/RXR interrupts
From: Noralf Trønnes @ 2016-10-04 19:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8760p9xlqy.fsf@eliezer.anholt.net>
Den 03.10.2016 21:42, skrev Eric Anholt:
> Noralf Tr?nnes <noralf@tronnes.org> writes:
>
>> Den 29.09.2016 00:00, skrev Eric Anholt:
>>> Noralf Tr?nnes <noralf@tronnes.org> writes:
>>>
>>>> If an unexpected TXW or RXR interrupt occurs (msg_buf_remaining == 0),
>>>> the driver has no way to fill/drain the FIFO to stop the interrupts.
>>>> In this case the controller has to be disabled and the transfer
>>>> completed to avoid hang.
>>>>
>>>> (CLKT | ERR) and DONE interrupts are completed in their own paths, and
>>>> the controller is disabled in the transfer function after completion.
>>>> Unite the code paths and do disabling inside the interrupt routine.
>>>>
>>>> Clear interrupt status bits in the united completion path instead of
>>>> trying to do it on every interrupt which isn't necessary.
>>>> Only CLKT, ERR and DONE can be cleared that way.
>>>>
>>>> Add the status value to the error value in case of TXW/RXR errors to
>>>> distinguish them from the other S_LEN error.
>>> I was surprised that not writing the TXW/RXR bits on handling their
>>> interrupts was OK, given that we were doing so before, but it's a level
>>> interrupt and those bits are basically ignored on write.
>>>
>>> This patch and 3, 4, and 6 are:
>>>
>>> Reviewed-by: Eric Anholt <eric@anholt.net>
>>>
>>> Patch 5 is:
>>>
>>> Acked-by: Eric Anholt <eric@anholt.net>
>>>
>>> Note for future debug: The I2C_C_CLEAR on errors will take some time to
>>> resolve -- if you were in non-idle state and I2C_C_READ, it sets an
>>> abort_rx flag and runs through the state machine to send a NACK and a
>>> STOP, I think. Since we're setting CLEAR without I2CEN, that NACK will
>>> be hanging around queued up for next time we start the engine.
>> Maybe you're able to explain the issues I had with reset:
>> https://github.com/raspberrypi/linux/issues/1653
> One of the questions I think you might have is "what state does the
> controller end up in after the various interrupts?"
>
> ERR:
> - produced if we get a nack that's not at the end of a read.
>
> - Proceeds to repeated start if BCM2835_I2C_C_ST is queued, otherwise
> stop.
>
> CLKT:
> - Triggered by a counter outside of the state machine when stretching
> happens and then times out.
>
> - Sets cs_override, which causes proceeding through the state machine as
> if the clock wasn't getting stretched, until the end of the next byte
> sent/received.
>
> - According to Wolfram we shouldn't be timing out on clock stretching
> for i2c, just on the transfer as a whole
> (https://patchwork.kernel.org/patch/9148431/), so I wrote
> https://github.com/anholt/linux/commit/894200276239d2e4c60b378bdc52164fcb13af8d.
> However, I don't see an obvious way to get back to IDLE while the
> slave is still stretching, without triggering the clock stretching
> timeout path.
If the transfer times out, whatever the reason, we clear the fifo
(and disable). Doesn't that get us back to IDLE?
Code with my patches:
static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
int num)
{
[...]
bcm2835_i2c_start_transfer(i2c_dev);
time_left = wait_for_completion_timeout(&i2c_dev->completion,
adap->timeout);
if (!time_left) {
bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C,
BCM2835_I2C_C_CLEAR);
dev_err(i2c_dev->dev, "i2c transfer timed out\n");
return -ETIMEDOUT;
}
> DONE:
> - Signaled at STOP, and just moves to IDLE state which keeps scl/sda
> high and waits for a BCM2835_I2C_C_ST while we're not clearing the
> FIFOs (if you do signal start while the fifos are clearing, the start
> will hang around until the fifo clear is done). This is the only way
> to get to IDLE.
>
> I'm don't think I have an answer to the "what should I do?" question you
> had, but hopefully this helps.
^ permalink raw reply
* [PATCH 07/15] clk: sunxi: mod0: improve function-level documentation
From: Maxime Ripard @ 2016-10-04 21:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475351192-27079-8-git-send-email-Julia.Lawall@lip6.fr>
On Sat, Oct 01, 2016 at 09:46:24PM +0200, Julia Lawall wrote:
> Use the actual function name in the function documentation.
>
> Issue detected using Coccinelle (http://coccinelle.lip6.fr/)
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Applied, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161004/6642e966/attachment.sig>
^ permalink raw reply
* [PATCH V2 0/3] ACPI,PCI,IRQ: revert penalty calculation for SCI
From: Sinan Kaya @ 2016-10-04 21:15 UTC (permalink / raw)
To: linux-arm-kernel
Restoring the old behavior for IRQ < 256 and the dynamic penalty behavior
will remain effective for IRQ >= 256.
By the time ACPI gets initialized, this code tries to determine an
IRQ number based on penalty values in this array. It will try to locate
the IRQ with the least penalty assignment so that interrupt sharing is
avoided if possible.
A couple of notes about the external APIs:
1. These API can be called before the ACPI is started. Therefore, one
cannot assume that the PCI link objects are initialized for calculating
penalties.
2. The polarity and trigger information passed via the
acpi_penalize_sci_irq from the BIOS may not match what the IRQ subsystem
is reporting as the call might have been placed before the IRQ is
registered by the interrupt subsystem.
The reverted changes were in the direction to remove these external API and
try to calculate the penalties at runtime for the ISA, SCI as well as PCI
IRQS. This didn't work out well with the existing platforms.
Changes from V1 (https://lkml.org/lkml/2016/10/1/106):
* Commit message updates
Sinan Kaya (3):
Revert "ACPI,PCI,IRQ: reduce static IRQ array size to 16"
ACPI, PCI IRQ: add PCI_USING penalty for ISA interrupts
Revert "ACPI,PCI,IRQ: remove SCI penalize function"
arch/x86/kernel/acpi/boot.c | 1 +
drivers/acpi/pci_link.c | 71 ++++++++++++++++++++++-----------------------
include/linux/acpi.h | 1 +
3 files changed, 37 insertions(+), 36 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH V2 1/3] Revert "ACPI, PCI, IRQ: reduce static IRQ array size to 16"
From: Sinan Kaya @ 2016-10-04 21:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475615720-31047-1-git-send-email-okaya@codeaurora.org>
This reverts commit 5c5087a55390 ("ACPI,PCI,IRQ: reduce static IRQ array
size to 16").
The code maintains a fixed size array for IRQ penalties. The array
gets updated by external calls such as acpi_penalize_sci_irq,
acpi_penalize_isa_irq to reflect the actual interrupt usage of the
system. Since the IRQ distribution is platform specific, this is
not known ahead of time. The IRQs get updated based on the SCI
interrupt number BIOS has chosen or the ISA IRQs that were assigned
to existing peripherals.
By the time ACPI gets initialized, this code tries to determine an
IRQ number based on penalty values in this array. It will try to locate
the IRQ with the least penalty assignment so that interrupt sharing is
avoided if possible.
A couple of notes about the external APIs:
1. These API can be called before the ACPI is started. Therefore, one
cannot assume that the PCI link objects are initialized for calculating
penalties.
2. The polarity and trigger information passed via the
acpi_penalize_sci_irq from the BIOS may not match what the IRQ subsystem
is reporting as the call might have been placed before the IRQ is
registered by the interrupt subsystem.
The previous change was in the direction to remove these external API and
try to calculate the penalties at runtime for the ISA path as well. This
didn't work out well with the existing platforms.
Restoring the old behavior for IRQ < 256 and the new behavior will remain
effective for IRQ >= 256.
Tested-by: Jonathan Liu <net147@gmail.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Link: http://www.gossamer-threads.com/lists/linux/kernel/2537016#2537016
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/acpi/pci_link.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index c983bf7..f3792f4 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -438,6 +438,7 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
* enabled system.
*/
+#define ACPI_MAX_IRQS 256
#define ACPI_MAX_ISA_IRQS 16
#define PIRQ_PENALTY_PCI_POSSIBLE (16*16)
@@ -446,7 +447,7 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
#define PIRQ_PENALTY_ISA_USED (16*16*16*16*16)
#define PIRQ_PENALTY_ISA_ALWAYS (16*16*16*16*16*16)
-static int acpi_isa_irq_penalty[ACPI_MAX_ISA_IRQS] = {
+static int acpi_irq_penalty[ACPI_MAX_IRQS] = {
PIRQ_PENALTY_ISA_ALWAYS, /* IRQ0 timer */
PIRQ_PENALTY_ISA_ALWAYS, /* IRQ1 keyboard */
PIRQ_PENALTY_ISA_ALWAYS, /* IRQ2 cascade */
@@ -511,7 +512,7 @@ static int acpi_irq_get_penalty(int irq)
}
if (irq < ACPI_MAX_ISA_IRQS)
- return penalty + acpi_isa_irq_penalty[irq];
+ return penalty + acpi_irq_penalty[irq];
penalty += acpi_irq_pci_sharing_penalty(irq);
return penalty;
@@ -538,14 +539,14 @@ int __init acpi_irq_penalty_init(void)
for (i = 0; i < link->irq.possible_count; i++) {
if (link->irq.possible[i] < ACPI_MAX_ISA_IRQS)
- acpi_isa_irq_penalty[link->irq.
+ acpi_irq_penalty[link->irq.
possible[i]] +=
penalty;
}
} else if (link->irq.active &&
- (link->irq.active < ACPI_MAX_ISA_IRQS)) {
- acpi_isa_irq_penalty[link->irq.active] +=
+ (link->irq.active < ACPI_MAX_IRQS)) {
+ acpi_irq_penalty[link->irq.active] +=
PIRQ_PENALTY_PCI_POSSIBLE;
}
}
@@ -828,7 +829,7 @@ static void acpi_pci_link_remove(struct acpi_device *device)
}
/*
- * modify acpi_isa_irq_penalty[] from cmdline
+ * modify acpi_irq_penalty[] from cmdline
*/
static int __init acpi_irq_penalty_update(char *str, int used)
{
@@ -837,24 +838,24 @@ static int __init acpi_irq_penalty_update(char *str, int used)
for (i = 0; i < 16; i++) {
int retval;
int irq;
- int new_penalty;
retval = get_option(&str, &irq);
if (!retval)
break; /* no number found */
- /* see if this is a ISA IRQ */
- if ((irq < 0) || (irq >= ACPI_MAX_ISA_IRQS))
+ if (irq < 0)
+ continue;
+
+ if (irq >= ARRAY_SIZE(acpi_irq_penalty))
continue;
if (used)
- new_penalty = acpi_irq_get_penalty(irq) +
- PIRQ_PENALTY_ISA_USED;
+ acpi_irq_penalty[irq] = acpi_irq_get_penalty(irq) +
+ PIRQ_PENALTY_ISA_USED;
else
- new_penalty = 0;
+ acpi_irq_penalty[irq] = 0;
- acpi_isa_irq_penalty[irq] = new_penalty;
if (retval != 2) /* no next number */
break;
}
@@ -870,14 +871,14 @@ static int __init acpi_irq_penalty_update(char *str, int used)
*/
void acpi_penalize_isa_irq(int irq, int active)
{
- if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty)))
- acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) +
- (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING);
+ if (irq >= 0 && irq < ARRAY_SIZE(acpi_irq_penalty))
+ acpi_irq_penalty[irq] = acpi_irq_get_penalty(irq) +
+ (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING);
}
bool acpi_isa_irq_available(int irq)
{
- return irq >= 0 && (irq >= ARRAY_SIZE(acpi_isa_irq_penalty) ||
+ return irq >= 0 && (irq >= ARRAY_SIZE(acpi_irq_penalty) ||
acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS);
}
--
1.9.1
^ permalink raw reply related
* [PATCH V2 2/3] ACPI, PCI IRQ: add PCI_USING penalty for ISA interrupts
From: Sinan Kaya @ 2016-10-04 21:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475615720-31047-1-git-send-email-okaya@codeaurora.org>
The change introduced in commit 103544d86976 ("ACPI,PCI,IRQ: reduce
resource requirements") removed PCI_USING penalty from
acpi_pci_link_allocate function as there is no longer a fixed size penalty
array for both PCI and IRQ interrupts.
We need to add the PCI_USING penalty for ISA interrupts too if the link is
in use and matches our ISA IRQ number.
Tested-by: Jonathan Liu <net147@gmail.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/acpi/pci_link.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index f3792f4..06c2a11 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -620,6 +620,10 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link)
acpi_device_bid(link->device));
return -ENODEV;
} else {
+ if (link->irq.active < ACPI_MAX_IRQS)
+ acpi_irq_penalty[link->irq.active] +=
+ PIRQ_PENALTY_PCI_USING;
+
printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n",
acpi_device_name(link->device),
acpi_device_bid(link->device), link->irq.active);
--
1.9.1
^ permalink raw reply related
* [PATCH V2 3/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function"
From: Sinan Kaya @ 2016-10-04 21:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475615720-31047-1-git-send-email-okaya@codeaurora.org>
The SCI function was removed in two steps (first refactor and then remove).
This patch does the revert in one step.
The commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements")
refactored the original code so that SCI penalty is calculated dynamically
by the time get_penalty function is called. This patch does a partial
revert for the SCI functionality only.
The commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") is
for the removal of the function. SCI penalty API was replaced by the
runtime penalty calculation based on the value of
acpi_gbl_FADT.sci_interrupt.
The IRQ type does not get updated at the right time for some platforms and
results in incorrect penalty assignment for PCI IRQs as
irq_get_trigger_type returns the wrong type.
The register_gsi function delivers the IRQ found in the ACPI table to
the interrupt controller driver. Penalties are calculated before a
link object is enabled to find out which interrupt has the least number
of users. By the time penalties are calculated, the IRQ is not registered
yet and the API returns the wrong type.
Tested-by: Jonathan Liu <net147@gmail.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Link: https://lkml.org/lkml/2016/10/4/283
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
arch/x86/kernel/acpi/boot.c | 1 +
drivers/acpi/pci_link.c | 34 ++++++++++++++--------------------
include/linux/acpi.h | 1 +
3 files changed, 16 insertions(+), 20 deletions(-)
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 90d84c3..0ffd26e 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -453,6 +453,7 @@ static void __init acpi_sci_ioapic_setup(u8 bus_irq, u16 polarity, u16 trigger,
polarity = acpi_sci_flags & ACPI_MADT_POLARITY_MASK;
mp_override_legacy_irq(bus_irq, polarity, trigger, gsi);
+ acpi_penalize_sci_irq(bus_irq, trigger, polarity);
/*
* stash over-ride to indicate we've been here
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index 06c2a11..6a2af19 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -495,27 +495,10 @@ static int acpi_irq_pci_sharing_penalty(int irq)
static int acpi_irq_get_penalty(int irq)
{
- int penalty = 0;
-
- /*
- * Penalize IRQ used by ACPI SCI. If ACPI SCI pin attributes conflict
- * with PCI IRQ attributes, mark ACPI SCI as ISA_ALWAYS so it won't be
- * use for PCI IRQs.
- */
- if (irq == acpi_gbl_FADT.sci_interrupt) {
- u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK;
-
- if (type != IRQ_TYPE_LEVEL_LOW)
- penalty += PIRQ_PENALTY_ISA_ALWAYS;
- else
- penalty += PIRQ_PENALTY_PCI_USING;
- }
-
- if (irq < ACPI_MAX_ISA_IRQS)
- return penalty + acpi_irq_penalty[irq];
+ if (irq < ACPI_MAX_IRQS)
+ return acpi_irq_penalty[irq];
- penalty += acpi_irq_pci_sharing_penalty(irq);
- return penalty;
+ return acpi_irq_pci_sharing_penalty(irq);
}
int __init acpi_irq_penalty_init(void)
@@ -886,6 +869,17 @@ bool acpi_isa_irq_available(int irq)
acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS);
}
+void acpi_penalize_sci_irq(int irq, int trigger, int polarity)
+{
+ if (irq >= 0 && irq < ARRAY_SIZE(acpi_irq_penalty)) {
+ if (trigger != ACPI_MADT_TRIGGER_LEVEL ||
+ polarity != ACPI_MADT_POLARITY_ACTIVE_LOW)
+ acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_ALWAYS;
+ else
+ acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING;
+ }
+}
+
/*
* Over-ride default table to reserve additional IRQs for use by ISA
* e.g. acpi_irq_isa=5
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4d8452c..85ac7d5 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -318,6 +318,7 @@ struct pci_dev;
int acpi_pci_irq_enable (struct pci_dev *dev);
void acpi_penalize_isa_irq(int irq, int active);
bool acpi_isa_irq_available(int irq);
+void acpi_penalize_sci_irq(int irq, int trigger, int polarity);
void acpi_pci_irq_disable (struct pci_dev *dev);
extern int ec_read(u8 addr, u8 *val);
--
1.9.1
^ permalink raw reply related
* [PATCH] efi/arm: fix absolute relocation detection for older toolchains
From: Matt Fleming @ 2016-10-04 21:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu94ZEuGyrcGTKMvTPKpvaR+R3AAXNY=diX28RDhc=Cv7Q@mail.gmail.com>
On Tue, 04 Oct, at 11:34:31AM, Ard Biesheuvel wrote:
>
> These relocations are harmless, since the debug ones are only
> interpreted by the debugger, and the ones generated by
> EXPORT_SYMBOL(sort) will never be referenced, since the symbols they
> contain are either renamed to __efistub_xxx (arm64), or they are not
> part of the kernel proper (arm)
>
> So both cases are false positives, but the diagnostic is important,
> and so breaking the build is appropriate for any other absolute
> relocation that may appear.
>
> The effect of the patch is not that the diagnostic is ignored, but
> that these relocations are not generated in the first place (-g0) or
> removed explicitly (ksymtab/krcrctab+sort) rather than via a wildcard.
> So other than not breaking the build, this patch should have no user
> observeable differences.
Thanks Ard, sounds reasonable. Feel free to take this through
whichever tree you think is best.
Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>
^ permalink raw reply
* ks_dw_pcie_initiate_link_train() question
From: Murali Karicheri @ 2016-10-04 21:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004174051.GE3449@localhost>
On 10/04/2016 01:40 PM, Bjorn Helgaas wrote:
> Hi Murali,
>
> This code looks suspicious. Can you comment?
>
> void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie)
> {
> u32 val;
>
> /* Disable Link training */
> val = readl(ks_pcie->va_app_base + CMD_STATUS);
> val &= ~LTSSM_EN_VAL;
> writel(LTSSM_EN_VAL | val, ks_pcie->va_app_base + CMD_STATUS);
>
> Here we cleared the LTSSM_EN_VAL bit in "val", but then we add it
> right back in before writing it back to CMD_STATUS.
>
> That looks like a cut and paste error to me, but of course I don't
> know the hardware.
>
> /* Initiate Link Training */
> val = readl(ks_pcie->va_app_base + CMD_STATUS);
> writel(LTSSM_EN_VAL | val, ks_pcie->va_app_base + CMD_STATUS);
> }
>
>
Bjorn,
Good catch! That is a cut-n-paste error. Here is the description from
device manual
================
Disable link training by de-asserting the LTSSM_EN bit in the PCIESS
Command Status Register (CMD_STATUS[LTSSM_EN]=0). Upon reset, the
LTSSM_EN is de-asserted automatically by hardware.
Initiate link training can be initiated by asserting LTSSM_EN bit in the
CMD_STATUS register (CMD_STATUS[LTSSM_EN]=1).
================================================
Probably it works because it is de-asserted automatically upon
reset by hardware. Let me test this and send you a patch?
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply
* [PATCH v2 1/2] clocksource: arm_arch_timer: Don't assume clock runs in suspend
From: Doug Anderson @ 2016-10-04 22:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475604730-140264-1-git-send-email-briannorris@chromium.org>
Hi,
On Tue, Oct 4, 2016 at 11:12 AM, Brian Norris <briannorris@chromium.org> wrote:
> The ARM ARM specifies that the system counter "must be implemented in an
> always-on power domain," and so we try to use the counter as a source of
> timekeeping across suspend/resume. Unfortunately, some SoCs (e.g.,
> Rockchip's RK3399) do not keep the counter ticking properly when
> switched from their high-power clock to the lower-power clock used in
> system suspend. Support this quirk by adding a new device tree property.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> v2:
> * add new device tree property, instead of re-using the "always-on"
> property (which has different meaning)
>
> Documentation/devicetree/bindings/arm/arch_timer.txt | 5 +++++
> drivers/clocksource/arm_arch_timer.c | 9 ++++++++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
FWIW:
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply
* [PATCH v2 2/2] arm64: dts: rockchip: arch counter doesn't tick in system suspend
From: Doug Anderson @ 2016-10-04 22:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475604730-140264-2-git-send-email-briannorris@chromium.org>
Hi,
On Tue, Oct 4, 2016 at 11:12 AM, Brian Norris <briannorris@chromium.org> wrote:
> The "arm,no-tick-in-suspend" property was introduced to note
> implementations where the system counter does not quite follow the ARM
> specification that it "must be implemented in an always-on power
> domain".
>
> Particularly, RK3399's counter stops ticking when we switch from the
> 24MHz clock to the 32KHz clock in low-power suspend, so let's mark it as
> such.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> v2: new in v2
>
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply
* [PATCH] ARM: dts: lpc32xx: set pwm1 & pwm2 default clock rate
From: Vladimir Zapolskiy @ 2016-10-05 2:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474916056-11336-1-git-send-email-slemieux.tyco@gmail.com>
Hi Sylvain,
On 26.09.2016 21:54, Sylvain Lemieux wrote:
> From: Sylvain Lemieux <slemieux@tycoint.com>
>
> Probably most of NXP LPC32xx boards have 13MHz main oscillator
> and therefore for HCLK PLL and ARM core clock rate default
> hardware setting of 16 * 13MHz = 208MHz and the AHB bus clock
> rate of 208MHz / 2 = 104MHz.
>
> The change explicitly defines the peripheral PWM1/PWM2 default
> clock output rate of 104MHz. If needed it can be redefined
> in a board DTS file.
>
> Signed-off-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
> ---
> Note:
> * There is a dependency on the following patch:
> "ARM: dts: lpc32xx: set default parent clock for pwm1 & pwm2"
> http://www.spinics.net/lists/arm-kernel/msg530277.html
> * This patch should be apply after
> "ARM: dts: lpc32xx: add pwm-cells to base dts file"
> http://www.spinics.net/lists/arm-kernel/msg534050.html
> - There is no dependency between the patches.
>
> arch/arm/boot/dts/lpc32xx.dtsi | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi
> index c031c94..d669200 100644
> --- a/arch/arm/boot/dts/lpc32xx.dtsi
> +++ b/arch/arm/boot/dts/lpc32xx.dtsi
> @@ -471,6 +471,7 @@
> clocks = <&clk LPC32XX_CLK_PWM1>;
> assigned-clocks = <&clk LPC32XX_CLK_PWM1>;
> assigned-clock-parents = <&clk LPC32XX_CLK_PERIPH>;
> + assigned-clock-rates = <104000000>;
PWM controller clock source can be 32KHz or CLK_PERIPH, the latter is
equal either to SYSCLK or HCLK PLL divided by HCLKDIV_CTRL[6:2].
Do you set the divider to 1? If yes, then I would say
1) this is very specific to your board, generally CLK_PERIPH
is set to be about 10-13MHz,
2) HCLKDIV or PERIPH clock configuration shall not be done in pwm device node.
104MHz is too high value to be set by default for PWM clock.
> status = "disabled";
> #pwm-cells = <2>;
> };
> @@ -481,6 +482,7 @@
> clocks = <&clk LPC32XX_CLK_PWM2>;
> assigned-clocks = <&clk LPC32XX_CLK_PWM2>;
> assigned-clock-parents = <&clk LPC32XX_CLK_PERIPH>;
> + assigned-clock-rates = <104000000>;
> status = "disabled";
> #pwm-cells = <2>;
> };
>
--
With best wishes,
Vladimir
^ permalink raw reply
* [PATCH] ARM: dts: rockchip: Reserve unusable memory region on rk3066
From: Huang, Tao @ 2016-10-05 2:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161001140939.GA31220@vaio-ubuntu>
Hi, Pawe?:
On 2016?10?01? 22:09, =?UTF-8?q?Pawe=C5=82=20Jarosz?= wrote:
> For some reason accessing memory region above 0xfe000000 freezes
> system on rk3066. There is similiar bug on later rockchip soc (rk3288)
RK3066 only support 2GB memory from 0x60000000 to 0xE0000000, can not access
above 0xfe000000. I think you mean 0x9F000000?
> solved same way.
>
> Signed-off-by: Pawe? Jarosz <paweljarosz3691@gmail.com>
> ---
> arch/arm/boot/dts/rk3066a.dtsi | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3066a.dtsi b/arch/arm/boot/dts/rk3066a.dtsi
> index 0d0dae3..44c8956 100644
> --- a/arch/arm/boot/dts/rk3066a.dtsi
> +++ b/arch/arm/boot/dts/rk3066a.dtsi
> @@ -93,6 +93,19 @@
> };
> };
>
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + /*
> + * The rk3066 cannot use the memory area above 0x9F000000
> + * for some unknown reason.
> + */
I don't remember RK3066 has such limit. I will double check with our IC
design team.
Do you know which master can not access this memory area
[0x9F000000~0xA0000000)?
> + unusable at 9F000000 {
> + reg = <0x9F000000 0x1000000>;
> + };
> + };
> +
> i2s0: i2s at 10118000 {
> compatible = "rockchip,rk3066-i2s";
> reg = <0x10118000 0x2000>;
^ permalink raw reply
* [PATCH] clk: lpc32xx: fix pwm clock divider computation
From: Vladimir Zapolskiy @ 2016-10-05 2:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474915467-11101-1-git-send-email-slemieux.tyco@gmail.com>
Hi Sylvain,
On 26.09.2016 21:44, Sylvain Lemieux wrote:
> From: Sylvain Lemieux <slemieux@tycoint.com>
>
> A zero value in the PWM clock divider register
> (PWM1_FREQ/PWM2_FREQ) turn off the PWM clock.
>
> The "CLK_DIVIDER_ALLOW_ZERO" option is used for hardware that handle
> the zero divider by not modifying their clock input (i.e. bypass).
> See "/include/linux/clk-provider.h" for details.
the problem is that the divider value is not set to some non-zero
value when the clock is enabled, right?
I think it does not matter if the clock rate value is set to parent
clock rate or any other value when divider is 0 *and* clock is gated.
Enabling or disabling a clock is a gate control, so I suggest two
alternative options at your choice (my preference is option 2):
1) add a custom clk_pwm_gate_enable(), clk_pwm_gate_disable() and
clk_pwm_gate_is_enabled() functions combined under lpc32xx_clk_pwm_gate_ops.
Next instead of adding one more define for a single exception
please reassign .ops for two PWM clocks in runtime in
lpc32xx_clk_init() function before calling lpc32xx_clk_register()
in a loop.
But this option is too invasive, a simpler solution is below.
2) in lpc32xx_clk_init() before clock registrations check for zero
dividers of PWM clocks, then if a divider is 0 and clock is gated
set divider to 1, if the divider is 0 and clock is not gated then
gate the clock and set divider to 1, in other cases do nothing.
> Remove the CLK_DIVIDER_ALLOW_ZERO option and add support to handle
> the clock rate computation of the PWM clock divider 0 value.
>
> Signed-off-by: Sylvain Lemieux <slemieux@tycoint.com>
> ---
> Note:
> * Should we include a new CLK_DIVIDER option for this case
> (i.e. clock off when zero ) in "clk-provider.h"?
>
> drivers/clk/nxp/clk-lpc32xx.c | 52 +++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 48 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
> index 34c9735..3ca3a14 100644
> --- a/drivers/clk/nxp/clk-lpc32xx.c
> +++ b/drivers/clk/nxp/clk-lpc32xx.c
> @@ -959,6 +959,25 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
> divider->flags);
> }
>
> +static unsigned long clk_divider_pwm_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct lpc32xx_clk_div *divider = to_lpc32xx_div(hw);
> + unsigned int val;
> +
> + regmap_read(clk_regmap, divider->reg, &val);
> +
> + val >>= divider->shift;
> + val &= div_mask(divider->width);
> +
> + /* Handle 0 divider -> PWM clock is off. */
> + if(val == 0)
No space in front of the open parenthesis.
> + return 0;
> +
> + return divider_recalc_rate(hw, parent_rate, val, divider->table,
> + divider->flags);
> +}
> +
> static long clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
> unsigned long *prate)
> {
> @@ -999,6 +1018,12 @@ static const struct clk_ops lpc32xx_clk_divider_ops = {
> .set_rate = clk_divider_set_rate,
> };
>
> +static const struct clk_ops lpc32xx_clk_pwm_divider_ops = {
> + .recalc_rate = clk_divider_pwm_recalc_rate,
> + .round_rate = clk_divider_round_rate,
> + .set_rate = clk_divider_set_rate,
> +};
> +
> static u8 clk_mux_get_parent(struct clk_hw *hw)
> {
> struct lpc32xx_clk_mux *mux = to_lpc32xx_mux(hw);
> @@ -1151,6 +1176,25 @@ struct clk_hw_proto {
> }, \
> }
>
> +#define LPC32XX_DEFINE_PWM_DIV(_idx, _reg, _shift, _width, _tab, _fl) \
> +[CLK_PREFIX(_idx)] = { \
> + .type = CLK_DIV, \
> + { \
> + .hw0 = { \
> + .ops = &lpc32xx_clk_pwm_divider_ops, \
> + { \
> + .div = { \
> + .reg = LPC32XX_CLKPWR_ ## _reg, \
> + .shift = (_shift), \
> + .width = (_width), \
> + .table = (_tab), \
> + .flags = (_fl), \
> + }, \
> + }, \
> + }, \
> + }, \
> +}
> +
> #define LPC32XX_DEFINE_GATE(_idx, _reg, _bit, _flags) \
> [CLK_PREFIX(_idx)] = { \
> .type = CLK_GATE, \
> @@ -1281,14 +1325,14 @@ static struct clk_hw_proto clk_hw_proto[LPC32XX_CLK_HW_MAX] = {
> LPC32XX_DEFINE_GATE(MCPWM, TIMCLK_CTRL1, 6, 0),
>
> LPC32XX_DEFINE_MUX(PWM1_MUX, PWMCLK_CTRL, 1, 0x1, NULL, 0),
> - LPC32XX_DEFINE_DIV(PWM1_DIV, PWMCLK_CTRL, 4, 4, NULL,
> - CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO),
> + LPC32XX_DEFINE_PWM_DIV(PWM1_DIV, PWMCLK_CTRL, 4, 4, NULL,
> + CLK_DIVIDER_ONE_BASED),
> LPC32XX_DEFINE_GATE(PWM1_GATE, PWMCLK_CTRL, 0, 0),
> LPC32XX_DEFINE_COMPOSITE(PWM1, PWM1_MUX, PWM1_DIV, PWM1_GATE),
>
> LPC32XX_DEFINE_MUX(PWM2_MUX, PWMCLK_CTRL, 3, 0x1, NULL, 0),
> - LPC32XX_DEFINE_DIV(PWM2_DIV, PWMCLK_CTRL, 8, 4, NULL,
> - CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO),
> + LPC32XX_DEFINE_PWM_DIV(PWM2_DIV, PWMCLK_CTRL, 8, 4, NULL,
> + CLK_DIVIDER_ONE_BASED),
> LPC32XX_DEFINE_GATE(PWM2_GATE, PWMCLK_CTRL, 2, 0),
> LPC32XX_DEFINE_COMPOSITE(PWM2, PWM2_MUX, PWM2_DIV, PWM2_GATE),
>
>
--
With best wishes,
Vladimir
^ permalink raw reply
* [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: Horng-Shyang Liao @ 2016-10-05 2:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475228829.3658.1.camel@mtksdaap41>
On Fri, 2016-09-30 at 17:47 +0800, Horng-Shyang Liao wrote:
> On Fri, 2016-09-30 at 17:11 +0800, CK Hu wrote:
> > Hi, HS:
> >
> > One comment inline
> >
> > On Fri, 2016-09-30 at 16:56 +0800, Horng-Shyang Liao wrote:
> > > Hi CK,
> > >
> > > Please see my inline reply.
> > >
> > > On Fri, 2016-09-30 at 11:06 +0800, CK Hu wrote:
> > > > Hi, HS:
> > > >
> > > > On Mon, 2016-09-05 at 09:44 +0800, HS Liao wrote:
> > > > > This patch is first version of Mediatek Command Queue(CMDQ) driver. The
> > > > > CMDQ is used to help write registers with critical time limitation,
> > > > > such as updating display configuration during the vblank. It controls
> > > > > Global Command Engine (GCE) hardware to achieve this requirement.
> > > > > Currently, CMDQ only supports display related hardwares, but we expect
> > > > > it can be extended to other hardwares for future requirements.
> > > > >
> > > > > Signed-off-by: HS Liao <hs.liao@mediatek.com>
> > > > > Signed-off-by: CK Hu <ck.hu@mediatek.com>
> > > > > ---
> > > >
> > > > [snip...]
> > > >
> > > > > +
> > > > > +struct cmdq_task {
> > > > > + struct cmdq *cmdq;
> > > > > + struct list_head list_entry;
> > > > > + void *va_base;
> > > > > + dma_addr_t pa_base;
> > > > > + size_t cmd_buf_size; /* command occupied size */
> > > > > + size_t buf_size; /* real buffer size */
> > > > > + bool finalized;
> > > > > + struct cmdq_thread *thread;
> > > >
> > > > I think thread info could be removed from cmdq_task. Only
> > > > cmdq_task_handle_error() and cmdq_task_insert_into_thread() use
> > > > task->thread and caller of both function has the thread info. So you
> > > > could just pass thread info into these two function and remove thread
> > > > info in cmdq_task.
> > >
> > > This modification will remove 1 pointer but add 2 pointers. Moreover,
> > > more pointers will need to be delivered between functions for future
> > > extension. IMHO, it would be better to keep thread pointer inside
> > > cmdq_task.
> > >
> > > > > + struct cmdq_task_cb cb;
> > > >
> > > > I think this callback function is equal to mailbox client tx_done
> > > > callback. It's better to use already-defined interface rather than
> > > > creating your own.
> > >
> > > This is because CMDQ driver allows different callback functions for
> > > different tasks, but mailbox only allows one callback function per
> > > channel. But, I think I can add a wrapper for tx_done to call CMDQ
> > > callback functions. So, I will use tx_done in CMDQ v15.
> >
> > Up to now, one callback function for one channel is enough for DRM. So
> > 'different callback function for different sent-message' looks like an
> > advanced function. Maybe you should not include it in first patch.
> >
> > Regards,
> > CK
>
> Hi CK,
>
> OK. I will do it.
>
> Thanks,
> HS
>
> [snip...]
Hi CK,
After I trace mailbox driver, I realize that CMDQ driver cannot use
tx_done.
CMDQ clients will flush many tasks into CMDQ driver, and then CMDQ
driver will apply these tasks into GCE HW "immediately". These tasks,
which are queued in GCE HW, may not execute immediately since they
may need to wait event(s), e.g. vsync.
However, in mailbox driver, mailbox uses a software buffer to queue
sent messages. It only sends next message until previous message is
done. This cannot fulfill CMDQ's requirement.
Quote some code from mailbox driver. Please notice "active_req" part.
static void msg_submit(struct mbox_chan *chan)
{
...
if (!chan->msg_count || chan->active_req)
goto exit;
...
err = chan->mbox->ops->send_data(chan, data);
if (!err) {
chan->active_req = data;
chan->msg_count--;
}
...
}
static void tx_tick(struct mbox_chan *chan, int r)
{
...
spin_lock_irqsave(&chan->lock, flags);
mssg = chan->active_req;
chan->active_req = NULL;
spin_unlock_irqrestore(&chan->lock, flags);
...
}
Current workable CMDQ driver uses mbox_client_txdone() to prevent
this issue, and then uses self callback functions to handle done tasks.
int cmdq_task_flush_async(struct cmdq_client *client, struct cmdq_task
*task, cmdq_async_flush_cb cb, void *data)
{
...
mbox_send_message(client->chan, task);
/* We can send next task immediately, so just call txdone. */
mbox_client_txdone(client->chan, 0);
...
}
Another solution is to use rx_callback; i.e. CMDQ mailbox controller
call mbox_chan_received_data() when CMDQ task is done. But, this may
violate the design of mailbox. What do you think?
Thanks,
HS
Hi Jassi,
Do you have any suggestion about previous situation?
Thanks,
HS
^ permalink raw reply
* [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec documentation
From: Chen-Yu Tsai @ 2016-10-05 2:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004162430.GQ5228@lukather>
On Wed, Oct 5, 2016 at 12:24 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Tue, Oct 04, 2016 at 11:46:21AM +0200, Myl?ne Josserand wrote:
>> Add the documentation for dt-binding of the analog audiocodec
>> driver for SUN8I SoC.
>>
>> Signed-off-by: Myl?ne Josserand <mylene.josserand@free-electrons.com>
>> ---
>> .../devicetree/bindings/sound/sun8i-codec-analog.txt | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
>>
>> diff --git a/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
>> new file mode 100644
>> index 0000000..a03ec20
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
>> @@ -0,0 +1,20 @@
>> +* Allwinner A23/A33 Analog Codec
>> +
>> +This codec must be handled as a PRCM subnode.
>
> Like Mark was saying, you should probably reference the sun6i-prcm.txt
> binding here
>
>> +Required properties:
>> +- compatible: must be either "allwinner,sun8i-codec-analog"
>
> Our compatible prefix is <family>-<soc>, and using the older SoC that
> introduced that block.
>
> In this case, that would be sun6i-a31, I think?
sun6i-a31s actually, but a31s has extra line out controls,
so the right one would be sun8i-a23. Both are listed in my
original driver.
ChenYu
>
> Thanks,
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
^ permalink raw reply
* [PATCH v2 2/4] net: phy: dp83867: add support for MAC impedance configuration
From: Mugunthan V N @ 2016-10-05 3:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004131032.GL11677@lunn.ch>
On Tuesday 04 October 2016 06:40 PM, Andrew Lunn wrote:
>> + if (of_property_read_bool(of_node, "ti,max-output-imepdance"))
>> + dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX;
>> + else if (of_property_read_bool(of_node, "ti,min-output-imepdance"))
>
> Did you really test this? Or did you make the same typos in your device
> tree file?
>
I have tested this and attached the log in cover letter. Since there is
a typo error on both dts and driver it worked as expected. Will send a
v3 ASAP.
Regards
Mugunthan V N
^ permalink raw reply
* [PATCH v2 3/4] ARM: dts: dra72-evm-revc: fix correct phy delay and impedance settings
From: Mugunthan V N @ 2016-10-05 3:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a4ef7fc1-73f7-7136-0d60-6ce8a8ff33b9@ti.com>
On Tuesday 04 October 2016 06:41 PM, Lokesh Vutla wrote:
>
> On Tuesday 04 October 2016 06:26 PM, Mugunthan V N wrote:
>> > The default impedance settings of the phy is not the optimal
>> > value, due to this the second ethernet is not working. Fix it
>> > with correct values which makes the second ethernet port to work.
>> >
>> > Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> > ---
>> > arch/arm/boot/dts/dra72-evm-revc.dts | 2 ++
>> > 1 file changed, 2 insertions(+)
>> >
>> > diff --git a/arch/arm/boot/dts/dra72-evm-revc.dts b/arch/arm/boot/dts/dra72-evm-revc.dts
>> > index f9cfd3b..d626cd7 100644
>> > --- a/arch/arm/boot/dts/dra72-evm-revc.dts
>> > +++ b/arch/arm/boot/dts/dra72-evm-revc.dts
>> > @@ -62,6 +62,7 @@
>> > ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
>> > ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
>> > ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
>> > + ti,min-output-imepdance;
> s/imepdance/impedance
>
Thanks for quick catch. Will fix this in v3.
Regards
Mugunthan V N
^ permalink raw reply
* [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: Jassi Brar @ 2016-10-05 3:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475636064.21937.25.camel@mtksdaap41>
On 5 October 2016 at 08:24, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:
> On Fri, 2016-09-30 at 17:47 +0800, Horng-Shyang Liao wrote:
>> On Fri, 2016-09-30 at 17:11 +0800, CK Hu wrote:
>
> After I trace mailbox driver, I realize that CMDQ driver cannot use
> tx_done.
>
> CMDQ clients will flush many tasks into CMDQ driver, and then CMDQ
> driver will apply these tasks into GCE HW "immediately". These tasks,
> which are queued in GCE HW, may not execute immediately since they
> may need to wait event(s), e.g. vsync.
>
> However, in mailbox driver, mailbox uses a software buffer to queue
> sent messages. It only sends next message until previous message is
> done. This cannot fulfill CMDQ's requirement.
>
I understand
a) GCE HW can internally queue many tasks in some 'FIFO'
b) Execution of some task may have to wait until some external event
occurs (like vsync)
c) GCE does not generate irq/flag for each task executed (?)
If so, may be your tx_done should return 'true' so long as the GCE HW
can accept tasks in its 'FIFO'. For mailbox api, any task that is
queued on GCE, is assumed to be transmitted.
> Quote some code from mailbox driver. Please notice "active_req" part.
>
> static void msg_submit(struct mbox_chan *chan)
> {
> ...
> if (!chan->msg_count || chan->active_req)
> goto exit;
> ...
> err = chan->mbox->ops->send_data(chan, data);
> if (!err) {
> chan->active_req = data;
> chan->msg_count--;
> }
> ...
> }
>
> static void tx_tick(struct mbox_chan *chan, int r)
> {
> ...
> spin_lock_irqsave(&chan->lock, flags);
> mssg = chan->active_req;
> chan->active_req = NULL;
> spin_unlock_irqrestore(&chan->lock, flags);
> ...
> }
>
> Current workable CMDQ driver uses mbox_client_txdone() to prevent
> this issue, and then uses self callback functions to handle done tasks.
>
> int cmdq_task_flush_async(struct cmdq_client *client, struct cmdq_task
> *task, cmdq_async_flush_cb cb, void *data)
> {
> ...
> mbox_send_message(client->chan, task);
> /* We can send next task immediately, so just call txdone. */
> mbox_client_txdone(client->chan, 0);
> ...
> }
>
> Another solution is to use rx_callback; i.e. CMDQ mailbox controller
> call mbox_chan_received_data() when CMDQ task is done. But, this may
> violate the design of mailbox. What do you think?
>
If my point (c) above does not hold, maybe look at implementing
tx_done() callback and submit next task from the callback of last
done.
^ permalink raw reply
* PROBLEM: DWC3 USB 3.0 not working on Odroid-XU4 with Exynos 5422
From: Vivek Gautam @ 2016-10-05 4:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANAwSgQ2j+NtDwgn972-1UdY=j4DEsB+0d7VdhoC9i9WHRc9yQ@mail.gmail.com>
Hi Anand,
On Tue, Oct 4, 2016 at 8:39 PM, Anand Moon <linux.amoon@gmail.com> wrote:
> Hi Vivek,
>
[snip]
>
> What I feel is that their need to be some reset of usb phy so that
> device are assigned to respective bus ports.
The phy resets are what we do in the phy-exynos5-usbdrd driver. In
addition to what we
have currently in this phy driver, we just need the phy calibration
patch [1] for phy configurations.
[1] https://lkml.org/lkml/2015/2/2/259
> odroid at odroid:~$ lsusb -t
> /: Bus 06.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M
> /: Bus 05.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 480M
> |__ Port 1: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 480M
This shows the ethernet device gets detected on the high-speed port of one
of the controller.
The lsusb output for kernel v4.7.x posted by Michael show that the
ethernet device got detected on super-speed port of the controller.
So, there seems to be a difference between the two.
Or, is this how it is behaving ?
Is this lsusb output on 4.8 kernel with the patch [1] ?
> /: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M
> |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/2p, 5000M
> |__ Port 1: Dev 3, If 0, Class=Mass Storage, Driver=usb-storage, 5000M
> |__ Port 2: Dev 4, If 0, Class=Mass Storage, Driver=usb-storage, 5000M
> /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 480M
> |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/2p, 480M
> /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=exynos-ohci/3p, 12M
> /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=exynos-ehci/3p, 480M
> |__ Port 1: Dev 3, If 0, Class=Mass Storage, Driver=usb-storage, 480M
>
>
> Bus 06.Port should register the Realtek Ethernet r8153 device.
> But I am not able to trace out how it's should happen.
If i understand, below is how the configuration looks like on the board?
+-----------------------+
+------>| |
| | Bus 6 |-------+
+-----------+ | (super-speed) | |
| | +-----------------------+ |
|Controller | | --------> Ethernet device
| 2 | |
| | +-----------------------+ |
+-----------+ | | |
| | Bus 5 |-------+
+------>| (high-speed) |
+-----------------------+
+-----------------------+
+------>| |
| | Bus 4 |-------+
+-----------+ | (super-speed) | |
| | +-----------------------+ |
|Controller | | --------> (On board
hub ?? _OR_ external hub with downstream devices) ???
| 1 | |
| | +-----------------------+ |
+-----------+ | | |
| | Bus 3 |-------+
+------>| (high-speed) |
+-----------------------+
Thanks
Vivek
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ 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