* [PATCH 1/3] phy: sun4i: check PHY id when poking unknown bit of pmu
From: Maxime Ripard @ 2016-10-25 10:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1773661477366364@web4j.yandex.ru>
Hi,
On Tue, Oct 25, 2016 at 11:32:44AM +0800, Icenowy Zheng wrote:
>
>
> > On Mon, Oct 24, 2016 at 11:59:30AM +0800, Icenowy Zheng wrote:
> >
> >> Allwinner SoC's PHY 0, when used as OTG controller, have no pmu part.
> >> The code that poke some unknown bit of PMU for H3/A64 didn't check
> >> the PHY, and will cause kernel oops when PHY 0 is used.
> >>
> >> Fixes: b3e0d141ca9f (phy: sun4i: add support for A64 usb phy)
> >>
> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> >
> > Cc'ing stable would be nice.
>
> Yes... As it's also used by H3...
>
> >
> > Apart from that, Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>
> If I change it to check whether phy->pmu is null, will you keep the ACK?
Yep, 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: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161025/09742e34/attachment.sig>
^ permalink raw reply
* [PATCH 5/8] ARM: gr8: Add missing pwm channel 1 pin
From: Maxime Ripard @ 2016-10-25 10:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v64Ai+8sUPyH9aTnKUw1ROmQNxzCEqUXrT4v8Q-nipqR3Q@mail.gmail.com>
On Tue, Oct 25, 2016 at 12:10:26PM +0800, Chen-Yu Tsai wrote:
> On Fri, Oct 21, 2016 at 1:07 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Thu, Oct 20, 2016 at 10:10:03PM +0800, Chen-Yu Tsai wrote:
> >> On Thu, Oct 20, 2016 at 4:12 PM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > The PWM controller has two different channels, but only the first pin was
> >> > exposed in the DTSI. Add the other one.
> >> >
> >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>
> >> Acked-by: Chen-Yu Tsai <wens@csie.org>
> >>
> >> > ---
> >> > arch/arm/boot/dts/ntc-gr8.dtsi | 7 +++++++
> >> > 1 file changed, 7 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/arch/arm/boot/dts/ntc-gr8.dtsi b/arch/arm/boot/dts/ntc-gr8.dtsi
> >> > index 74aff795e723..fad7381630f3 100644
> >> > --- a/arch/arm/boot/dts/ntc-gr8.dtsi
> >> > +++ b/arch/arm/boot/dts/ntc-gr8.dtsi
> >> > @@ -854,6 +854,13 @@
> >> > allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
> >> > };
> >> >
> >> > + pwm1_pins_a: pwm1 at 0 {
> >>
> >> Nit: really don't need "_a" and "@0" here.
> >
> > Fixed and applied.
>
> Oops, you forgot to fix the label in the chip-pro dts:
>
> DTC arch/arm/boot/dts/ntc-gr8-chip-pro.dtb
> ERROR (phandle_references): Reference to non-existent node or label
> "pwm1_pins_a"
>
> ERROR: Input tree has errors, aborting (use -f to force output)
> scripts/Makefile.lib:313: recipe for target
> 'arch/arm/boot/dts/ntc-gr8-chip-pro.dtb' failed
Yeah, it was noticed by linux-next too, and I fixed it...
Sorry for that.
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: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161025/7c71abee/attachment.sig>
^ permalink raw reply
* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Thomas Gleixner @ 2016-10-25 10:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdYUVXny57AC9rKX3VRAyooEk_2XLvqe9jOuT3rOaE75rg@mail.gmail.com>
On Tue, 25 Oct 2016, Linus Walleij wrote:
> On Fri, Oct 21, 2016 at 11:06 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
>
> >> Isn't this usecase (also as described in the cover letter) a textbook
> >> example of when you should be using hierarchical irqdomain?
> >>
> >> Please check with Marc et al on hierarchical irqdomains.
> >
> > Linus,
> > Do you mean I should create a new hierarchical irqdomains in each of
> > the two pinctrl instances we have in these SoC, these domains being
> > stacked on the one I just added for controller in irqchip ?
> >
> > I did not understand this is what you meant when I asked you the
> > question at ELCE.
>
> Honestly, I do not understand when and where to properly use
> hierarchical irqdomain, even after Marc's talk at ELC-E.
Hierarchical irqdomains are used when you have several levels of interrupt
hardware to deliver an interrupt.
For example on x86 we have:
device --- [IOAPIC] -- [VECTOR]
and we can have this expanded to
device --- [IOAPIC] -- [IRQ Remapping] -- [VECTOR]
and we have more things hanging off the VECTOR domain
device --- [IOAPIC] ---
|--- [VECTOR]
device --- [PCIMSI] ---
So with irq remapping this might look like this:
device --- [IOAPIC] ---
|-----------------------
device --- [PCIMSI] --- |
|---[VECTOR]
device --- [IOAPIC] --- |
|--[IRQ Remapping]------
device --- [PCIMSI] ---
The important part is that this hierarchy deals with a single Linux virq
and all parts of the hierarchy are required for setup and possibly for
mask/ack/eoi.
This is different from a demultiplex interrupt
device --- [DEMUX] --- [GIC]
where the demultiplex interrupt is a different virq than the device
virq. The demux interrupt chip can have a parent relation ship, which can
be required to propagate information, e.g. wake on a device behind the
demux must keep the gic as a wake irq as well. But it's not hierarchical in
the sense of our hierarchical irq domains.
Hope that helps.
Thanks,
tglx
^ permalink raw reply
* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Marc Zyngier @ 2016-10-25 10:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdYUVXny57AC9rKX3VRAyooEk_2XLvqe9jOuT3rOaE75rg@mail.gmail.com>
On 25/10/16 10:14, Linus Walleij wrote:
> On Fri, Oct 21, 2016 at 11:06 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>>> Isn't this usecase (also as described in the cover letter) a textbook
>>> example of when you should be using hierarchical irqdomain?
>>>
>>> Please check with Marc et al on hierarchical irqdomains.
>>
>> Linus,
>> Do you mean I should create a new hierarchical irqdomains in each of
>> the two pinctrl instances we have in these SoC, these domains being
>> stacked on the one I just added for controller in irqchip ?
>>
>> I did not understand this is what you meant when I asked you the
>> question at ELCE.
>
> Honestly, I do not understand when and where to properly use
> hierarchical irqdomain, even after Marc's talk at ELC-E.
I probably didn't do that good a job explaining it then. Let's try
again. You want to use hierarchical domains when you want to describe an
interrupt whose path traverses multiple controllers without ever being
multiplexed with other signals. As long as you have this 1:1
relationship between controllers, you can use them.
> Which is problematic since quite a few GPIO drivers now
> need to use them.
>
> I will review his slides, in the meantime I would say: whatever
> Marc ACKs is fine with me. I trust this guy 100%. So I guess I
> could ask that he ACK the entire chain of patches
> from GIC->specialchip->GPIO.
Man, I don't even trust myself... ;-)
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH/RFT v2 08/17] ARM: davinci: hawk: add full constraints for ohci plat boot
From: Axel Haslam @ 2016-10-25 10:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <522eeb9a-6fb1-88eb-5c58-7ee209a50fc3@ti.com>
On Tue, Oct 25, 2016 at 12:28 PM, Sekhar Nori <nsekhar@ti.com> wrote:
> On Monday 24 October 2016 10:16 PM, ahaslam at baylibre.com wrote:
>> From: Axel Haslam <ahaslam@baylibre.com>
>>
>> The phy framework requests an optional "phy" regulator. If it does
>> not find one, it returns -EPROBE_DEFER. In the case of non-DT based boot
>> for the omap138-lcdk board, this would prevent the usb11 phy to probe
>> correctly and ohci would not enumerate.
>>
>> By calling "regulator_has_full_constraints", An error would be returned
>
> nit: prefer regulator_has_full_constraints()
>
>> instead of DEFER for the "optional" regulator, and the probe of
>> the phy driver can continue normally without a regulator.
>>
>> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
>
> Looks good to me. Just drop the "hawk: from subject line since you also
> touch da830 evm. I am not sure what "ohci plat boot" means. How about
> the following:
>
> "ARM: davinci: da8xx: fix OHCI PHY probe for non-DT boot"
>
Will do.
Thanks
Axel.
> Thanks,
> Sekhar
^ permalink raw reply
* [PATCH/RFT v2 08/17] ARM: davinci: hawk: add full constraints for ohci plat boot
From: Sekhar Nori @ 2016-10-25 10:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164634.4330-9-ahaslam@baylibre.com>
On Monday 24 October 2016 10:16 PM, ahaslam at baylibre.com wrote:
> From: Axel Haslam <ahaslam@baylibre.com>
>
> The phy framework requests an optional "phy" regulator. If it does
> not find one, it returns -EPROBE_DEFER. In the case of non-DT based boot
> for the omap138-lcdk board, this would prevent the usb11 phy to probe
> correctly and ohci would not enumerate.
>
> By calling "regulator_has_full_constraints", An error would be returned
nit: prefer regulator_has_full_constraints()
> instead of DEFER for the "optional" regulator, and the probe of
> the phy driver can continue normally without a regulator.
>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
Looks good to me. Just drop the "hawk: from subject line since you also
touch da830 evm. I am not sure what "ohci plat boot" means. How about
the following:
"ARM: davinci: da8xx: fix OHCI PHY probe for non-DT boot"
Thanks,
Sekhar
^ permalink raw reply
* [PATCH 2/3] ARM: convert to generated system call tables
From: Arnd Bergmann @ 2016-10-25 10:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161025091210.rjfhvq4mqtuquei5@tower>
On Tuesday, October 25, 2016 10:12:10 PM CEST Michael Cree wrote:
> On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:
> > I see your point, but I think there are serious issues with the current
> > approach as well:
> >
> > - a lot of the less common architectures just don't get updated
> > in time, out of 22 architectures that don't use asm-generic/unistd.h,
> > only 12 have pwritev2 in linux-next, and only three have pkey_mprotect
> >
> > - some architectures that add all syscalls sometimes make a mistake
> > and forget one, e.g. alpha apparently never added __NR_bpf, but it
> > did add the later __NR_execveat.
>
> __NR_bpf was not forgotten on Alpha. It was not wired up because
> extra architecture support is needed which has not been implemented.
>
> But maybe we should just wire it up to sys_ni_syscall in the meantime
> so a syscall number is reserved for it, and user space can call it to
> get -ENOSYS returned.
Ah, I must have misinterpreted the code then. I assumed that the
bpf syscall always works on all architectures, but that only the
jit compiler for it required architecture specific code to make it
more efficient.
The implementation of sys_bfp is compile-time selectable at the moment
and falls back to sys_no_syscall if CONFIG_BPF_SYSCALL is disabled.
If it doesn't work on Alpha, maybe that symbol could have a "depends
on !ALPHA" or "depends on BPF_SUPPORT"?
sys_seccomp is another one that falls into a similar category, but
it already depends on HAVE_ARCH_SECCOMP_FILTER, and most other
architectures have assigned a syscall number but not set this symbol.
This one will actually allow you to set strict seccomp mode even
without the Kconfig symbol, just not allow to set a filter.
Arnd
^ permalink raw reply
* [RFC] shutdown machine when li-ion battery goes below 3V
From: Pavel Machek @ 2016-10-25 10:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201610242348.47484@pali>
On Mon 2016-10-24 23:48:47, Pali Roh?r wrote:
> On Monday 24 October 2016 23:41:52 Pavel Machek wrote:
> > On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> > > Also, the shutdown voltage can depend on external devices
> > > connected. It could be for example 3.3V depending on eMMC on some
> > > devices while devices with no eMMC could have it at 3.0V.
> >
> > Actually, I'd like to shutdown at 3.3V or more (like 3.5V), because
> > going below that is pretty mean to the battery. But if I set
> > threshold too high, GSM activity will push it below that for a very
> > short while, and I'll shutdown too soon.
> >
> > Ideas welcome...
>
> bq27x00 has EDVF flag which means that battery is empty. Maemo with
> bq27x00 driver is configured to issue system shutdown when EDVF is set.
>
> Maybe kernel should issue emergency shutdown e.g. after minute or two
> after EDVF flag is set?
Thanks for pointer.
EDVF seems to be exposed as health. ... but only if battery is
calibrated, AFAICT.
if (has_ci_flag && (cache.flags & BQ27000_FLAG_CI)) {
dev_info_once(di->dev, "battery is not calibrated! ignoring capacity values\n");
...
cache.health = -ENODATA;
Plus, it prioritizes battery cold over battery dead. IMO we don't need
to shutdown on battery cold (we just may not charge the battery), but
we need to shutdown on battery dead.
So something like this?
Pavel
Signed-off-by: Pavel Machek <pavel@ucw.cz>
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 8eb2f8f..5ddf6d7 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -680,10 +680,10 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
/* Unlikely but important to return first */
if (unlikely(bq27xxx_battery_overtemp(di, flags)))
return POWER_SUPPLY_HEALTH_OVERHEAT;
- if (unlikely(bq27xxx_battery_undertemp(di, flags)))
- return POWER_SUPPLY_HEALTH_COLD;
if (unlikely(bq27xxx_battery_dead(di, flags)))
return POWER_SUPPLY_HEALTH_DEAD;
+ if (unlikely(bq27xxx_battery_undertemp(di, flags)))
+ return POWER_SUPPLY_HEALTH_COLD;
return POWER_SUPPLY_HEALTH_GOOD;
}
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161025/199ed265/attachment.sig>
^ permalink raw reply related
* [PATCH] arm/arm64: KVM: Perform local TLB invalidation when multiplexing vcpus on a single CPU
From: Marc Zyngier @ 2016-10-25 10:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024161656.GM15620@leverpostej>
Hi Mark,
On 24/10/16 17:16, Mark Rutland wrote:
> Hi Marc,
>
> On Mon, Oct 24, 2016 at 04:31:28PM +0100, Marc Zyngier wrote:
>> Architecturally, TLBs are private to the (physical) CPU they're
>> associated with. But when multiple vcpus from the same VM are
>> being multiplexed on the same CPU, the TLBs are not private
>> to the vcpus (and are actually shared across the VMID).
>>
>> Let's consider the following scenario:
>>
>> - vcpu-0 maps PA to VA
>> - vcpu-1 maps PA' to VA
>>
>> If run on the same physical CPU, vcpu-1 can hit TLB entries generated
>> by vcpu-0 accesses, and access the wrong physical page.
>
> It might be worth noting that this could also lead to TLB conflicts and
> other such fun usually associated with missing TLB maintenance,
> depending on the two mappings (or the intermediate stages thereof).
>
>> The solution to this is to keep a per-VM map of which vcpu ran last
>> on each given physical CPU, and invalidate local TLBs when switching
>> to a different vcpu from the same VM.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>
> Modulo my comment on preemptiblity of kvm_arch_sched_in, everything
> below is a nit. Assuming that's not preemptible, this looks right to me.
>
> FWIW, with or without the other comments considered:
>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
>
>> ---
>> arch/arm/include/asm/kvm_host.h | 5 +++++
>> arch/arm/include/asm/kvm_hyp.h | 1 +
>> arch/arm/kvm/arm.c | 35 ++++++++++++++++++++++++++++++++++-
>> arch/arm/kvm/hyp/switch.c | 9 +++++++++
>> arch/arm64/include/asm/kvm_host.h | 6 +++++-
>> arch/arm64/kvm/hyp/switch.c | 8 ++++++++
>> 6 files changed, 62 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index 2d19e02..035e744 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -57,6 +57,8 @@ struct kvm_arch {
>> /* VTTBR value associated with below pgd and vmid */
>> u64 vttbr;
>>
>> + int __percpu *last_vcpu_ran;
>> +
>> /* Timer */
>> struct arch_timer_kvm timer;
>>
>> @@ -174,6 +176,9 @@ struct kvm_vcpu_arch {
>> /* vcpu power-off state */
>> bool power_off;
>>
>> + /* TLBI required */
>> + bool requires_tlbi;
>
> A bit of a nit, but it's not clear which class of TLBI is required, or
> why. It's probably worth a comment (and perhaps a bikeshed), like:
>
> /*
> * Local TLBs potentially contain conflicting entries from
> * another vCPU within this VMID. All entries for this VMID must
> * be invalidated from (local) TLBs before we run this vCPU.
> */
> bool tlb_vmid_stale;
Yup, looks good.
>
> [...]
>
>> +void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu)
>> +{
>> + int *last_ran;
>> +
>> + last_ran = per_cpu_ptr(vcpu->kvm->arch.last_vcpu_ran, cpu);
>> +
>> + /*
>> + * If we're very unlucky and get preempted before having ran
>> + * this vcpu for real, we'll end-up in a situation where any
>> + * vcpu that gets scheduled will perform an invalidation (this
>> + * vcpu explicitely requires it, and all the others will have
>> + * a different vcpu_id).
>> + */
>
> Nit: s/explicitely/explicitly/
>
> To bikeshed a little further, perhaps:
>
> /*
> * We might get preempted before the vCPU actually runs, but
> * this is fine. Our TLBI stays pending until we actually make
> * it to __activate_vm, so we won't miss a TLBI. If another vCPU
> * gets scheduled, it will see our vcpu_id in last_ran, and pend
> * a TLBI for itself.
> */
Looks good too. I'll incorporate this into v2.
>
>> + if (*last_ran != vcpu->vcpu_id) {
>> + if (*last_ran != -1)
>> + vcpu->arch.requires_tlbi = true;
>> +
>> + *last_ran = vcpu->vcpu_id;
>> + }
>> +}
>
> I take it this function is run in some non-preemptible context; if so,
> this looks correct to me.
>
> If this is preemptible, then this looks racy.
This function is called from a preempt notifier, which itself isn't
preemptible.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [RFC] ARM: memory: da8xx-ddrctl: new driver
From: Bartosz Golaszewski @ 2016-10-25 10:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024170035.GO15620@leverpostej>
2016-10-24 19:00 GMT+02:00 Mark Rutland <mark.rutland@arm.com>:
> On Mon, Oct 24, 2016 at 06:46:36PM +0200, Bartosz Golaszewski wrote:
>> +
>> + dev = &pdev->dev;
>> + node = dev->of_node;
>> +
>> + /* Find the board name. */
>> + for (parent = node;
>> + !of_node_is_root(parent);
>> + parent = of_get_parent(parent));
>> +
>> + ret = of_property_read_string(parent, "compatible", &board);
>> + if (ret) {
>> + dev_err(dev, "unable to read the soc model\n");
>> + return ret;
>> + }
>
> I can see that you want to expose sysfs knobs for this, but is it really
> necessary to match boards like this? It's very fragile, and commits us
> to maintaining a database of board data (i.e. a board file).
>
> I am very much not keen on that.
>
I Mark,
I don't want to expose any new sysfs interface.
The initial idea was to follow the way the ti-aemif driver is
implemented and expose DT bindings for the memory controller knobs
(initially only the Peripheral Bus Burst Priority Register, tweaking
which is required to make LCDC work correctly on da850 based boards as
mentioned by Kevin). This was rejected as it's not hardware
description but configuration, so it should not be controlled by DT
properties.
The correct approach for this kind of performance knobs doesn't exist
yet. There was a BoF during this year's ELCE in Berlin during which
several ideas were discussed, but no code has been written so far.
Using sysfs would have exactly the same disadvantage - committing to a
stable interface that would have to be maintained indefinitely.
In the end it was decided that a fairly good, temporary solution would
be to create a driver for the da850 DDR controller which would
hardcode the required tweaks for several boards (as the LCDC issue is
known to affect more TI SoCs and boards). Once a framework for
performance knobs is implemented and merged, it would be easy to port
the driver to it as we would not have implemented any stable
interface.
The same solution would be used for the SYSCFG registers on the da8xx SoCs.
Hope that clarifies the need for this patch a bit. I will address all
other issues in v2.
Best regards,
Bartosz Golaszewski
^ permalink raw reply
* [PATCH/RFT v2 02/17] ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration.
From: Sekhar Nori @ 2016-10-25 10:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKXjFTOVuWRL+yuk6sARa2PAd6j-V4Ej5AmPGz7F7JXKidvJrg@mail.gmail.com>
On Tuesday 25 October 2016 03:07 PM, Axel Haslam wrote:
> Hi Sekar,
>
> On Tue, Oct 25, 2016 at 10:10 AM, Sekhar Nori <nsekhar@ti.com> wrote:
>> On Monday 24 October 2016 10:16 PM, ahaslam at baylibre.com wrote:
>>> From: David Lechner <david@lechnology.com>
>>>
>>> The CFGCHIP registers are used by a number of devices, so using a syscon
>>> device to share them. The first consumer of this will by the phy-da8xx-usb
>>> driver.
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>> [Axel: minor fix: change id to -1]
>>
>> Can you please clarify this change? There could be other syscon devices
>> on the chip for other common registers. Why use the singular device-id?
>>
>
> in the case of non DT boot, the phy driver is looking for "syscon" :
>
> d_phy->regmap = syscon_regmap_lookup_by_pdevname("syscon");
>
> if we register the syscon driver with id = 0, the actual name of the syscon
> device will be "syscon.0" and the phy driver will fail to probe, because
> the strncmp match in the syscon driver (syscon_match_pdevname)
> will fail.
>
> should i change the phy driver instead?
Yes, please. Forcing only one syscon region for the whole chip will be
too restrictive, I am pretty sure.
Thanks,
Sekhar
^ permalink raw reply
* [PATCH/RFT v2 07/17] ARM: davinci: da8xx: Enable the usb20 "per" clk on phy_clk_enable
From: Sekhar Nori @ 2016-10-25 10:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164634.4330-8-ahaslam@baylibre.com>
On Monday 24 October 2016 10:16 PM, ahaslam at baylibre.com wrote:
> diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
> index 9e41a7f..982e105 100644
> --- a/arch/arm/mach-davinci/usb-da8xx.c
> +++ b/arch/arm/mach-davinci/usb-da8xx.c
> @@ -53,11 +53,19 @@ int __init da8xx_register_usb_refclkin(int rate)
>
> static void usb20_phy_clk_enable(struct clk *clk)
> {
> + struct clk *usb20_clk;
> u32 val;
> u32 timeout = 500000; /* 500 msec */
>
> val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
>
> + usb20_clk = clk_get(NULL, "usb20");
We should not be using a NULL device pointer here. Can you pass the musb
device pointer available in the same file? Also, da850_clks[] in da850.c
needs to be fixed to add the matching device name.
> + if (IS_ERR(usb20_clk)) {
> + pr_err("could not get usb20 clk\n");
> + return;
> + }
Thanks,
Sekhar
^ permalink raw reply
* [PATCH 4/4] iommu/arm-smmu: Add the device_link between masters and smmu
From: Marek Szyprowski @ 2016-10-25 10:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477070066-15044-5-git-send-email-sricharan@codeaurora.org>
Hi Sricharan,
On 2016-10-21 19:14, Sricharan R wrote:
> The device link between master and its smmu is added so that
> the smmu gets runtime enabled/disabled when the master needs it.
> This is done from add_device callback which gets called once
> when the master is added to the smmu group.
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> ---
> drivers/iommu/arm-smmu.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 578cdc2..71ce4b6 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1470,6 +1470,15 @@ static int arm_smmu_add_device(struct device *dev)
> goto out_free;
> pm_runtime_put_sync(smmu->dev);
>
> + /*
> + * Establish the link between smmu and master, so that the
> + * smmu gets runtime enabled/disabled as per the master's
> + * needs.
> + */
> +
> + device_link_add(dev, smmu->dev, DEVICE_LINK_AVAILABLE,
> + DEVICE_LINK_PM_RUNTIME);
Please update to the latest version of Rafael's patches. In V5 the
initial link
state is not needed anymore and there was an important fix for creating
links
during master's driver probing, what happens after applying your IOMMU
deferred
probe patchset.
> +
> return 0;
>
> out_free:
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* [PATCH 00/10] arm64: move thread_info off of the task stack
From: Mark Rutland @ 2016-10-25 10:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGXu5jKXoo5KWqq0dsWRFvW0Y3dXXs+-ab=ymVUnzcmovUsOkg@mail.gmail.com>
On Mon, Oct 24, 2016 at 11:18:35AM -0700, Kees Cook wrote:
> On Mon, Oct 24, 2016 at 11:15 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Mon, Oct 24, 2016 at 07:09:42PM +0100, Mark Rutland wrote:
> >> It's really crazy how broken a kernel can be yet still "work"; clearly
> >> we better tests are needed. :/
> >
> > Clearly we better grammar need too. :(
>
> Out of curiosity, what workflow would have tripped over the entry.S bug?
There are two bugs:
The issues in [1] would show up if you were attempting to use
breakpoints or watchpoints -- we'd never disable the single step.
The broken 're-entered irq stack' check [2] would be an issue if we were
close to exhausting the stack -- we'd never switch to the IRQ stack when
we take an IRQ in a kernel context. I'm not sure of a particular
workload.
Thanks,
Mark.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/462932.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/462891.html
^ permalink raw reply
* [PATCH/RFT v2 06/17] ARM: davinci: da8xx: Fix some redefined symbol warnings
From: Sekhar Nori @ 2016-10-25 10:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164634.4330-7-ahaslam@baylibre.com>
On Monday 24 October 2016 10:16 PM, ahaslam at baylibre.com wrote:
> From: Alexandre Bailon <abailon@baylibre.com>
>
> Some macro for DA8xx CFGCHIP are defined in usb-davinci.h,
> but da8xx-cfgchip.h intend to replace them.
> The usb-da8xx.c is using both headers, causing redefined symbol warnings.
> Remove the old macros.
>
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
This is a v4.9-rc bug fix. Can you please post it as a separate patch
for Greg to pick up?
You can add:
Acked-by: Sekhar Nori <nsekhar@ti.com>
Thanks,
Sekhar
^ permalink raw reply
* [PATCH/RFT v2 07/17] ARM: davinci: da8xx: Enable the usb20 "per" clk on phy_clk_enable
From: Axel Haslam @ 2016-10-25 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c2d31f5a-32c6-5e93-2d3d-d6ef75b697fc@lechnology.com>
On Tue, Oct 25, 2016 at 4:53 AM, David Lechner <david@lechnology.com> wrote:
> On 10/24/2016 11:46 AM, ahaslam at baylibre.com wrote:
>>
>> From: Axel Haslam <ahaslam@baylibre.com>
>>
>> While probing ochi phy with usb20 phy as a parent clock for usb11_phy,
>> the usb20_phy clock enable would time out. This is because the usb20
>> module clock needs to enabled while trying to lock the usb20_phy PLL.
>>
>> Call clk enable and get for the usb20 peripheral before trying to
>> enable the phy PLL.
>>
>> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
>> ---
>
>
>
> This patch can be combined with "ARM: davinci: da8xx: add usb phy clocks"
> since that patch has not been merged yet.
yes, agree, these should be merged.
>
> If you like, I can resubmit my patches from this series along with the
> changes from this patch.
Ok, if you can resubmit those patches with this change included
i can reference them and start making the series shorter.
i will also submit in separate patches the regulator changes, as requested
by Mark.
Regards
Axel.
>
^ permalink raw reply
* [PATCH] ARM: sti: stih410-clocks: Add PROC_STFE as a critical clock
From: Lee Jones @ 2016-10-25 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161025093926.GA3206@griffinp-ThinkPad-X1-Carbon-2nd>
On Tue, 25 Oct 2016, Peter Griffin wrote:
> Hi Lee,
>
> On Tue, 25 Oct 2016, Lee Jones wrote:
>
> > On Mon, 24 Oct 2016, Peter Griffin wrote:
> >
> > > Hi Lee,
> > >
> > > On Mon, 24 Oct 2016, Lee Jones wrote:
> > > > On Tue, 18 Oct 2016, Peter Griffin wrote:
> > > >
> > > > > Once the ST frontend demux HW IP has been enabled, the clock can't
> > > > > be disabled otherwise the system will hang and the board will
> > > > > be unserviceable.
> > > > >
> > > > > To allow balanced clock enable/disable calls in the driver we use
> > > > > the critical clock infrastructure to take an extra reference on the
> > > > > clock so the clock will never actually be disabled.
> > > >
> > > > This is an abuse of the critical-clocks framework, and is exactly the
> > > > type of hack I promised the clk guys I'd try to prevent.
> > >
> > > I expect the best way to do this would be to write some documentation on the
> > > clock-critical DT binding and/or CRITICAL_CLK flag. The only documentation I can
> > > find currently is with the initial patch series [1] and the comment in
> > > clk-provider.h of
> > >
> > > #define CLK_IS_CRITICAL BIT(11) /* do not gate, ever */
> > >
> > > Or the patch decription
> > >
> > > "Critical clocks are those which must not be gated, else undefined
> > > or catastrophic failure would occur. Here we have chosen to
> > > ensure the prepare/enable counts are correctly incremented, so as
> > > not to confuse users with enabled clocks with no visible users."
> > >
> > > Which is the functionality I want for this clock.
> >
> > No, that's not the functionality you want.
>
> Yes it is :)
No, it's the functionally that is most convenient.
> > You want for the clock not
> > to be RE-gated (big difference). Currently, the STFE clock will never
> > be gated, even when a) it's not used and b) can actually be disabled.
> > You're needlessly wasting power here.
>
> IMO it is *never* safe for Linux to gate this clock, as you have no idea
> on the state of the hardware from the primary and secondary bootloaders.
You can say that with any of the clocks on this platform.
> If the clock is enabled when Linux boots, the Linux clock framework *needs*
> to assume the hardware may have been used in previous boot stages, and it should
> not attempt to disable the clock.
None of the boot loaders we use do this.
I have never seen the STFE clock crash a platform.
> > Also, in your use-case there is a visible user, and the prepare/enable
> > counts will be correct.
>
> Your correct there is a visible user, but this is the same as CLK_EXT2F_A9 where
> there are multiple users of the clock (SPI, I2C, UART etc).
Right, but their clocks can not be turned off, ever. So all the boxes
are ticked for criticalness. Not the case with your clock.
> > > > If this, or
> > > > any other IP has some quirks (i.e. once enabled, if this clock is
> > > > subsequently disabled it will have a catastrophic effect on the
> > > > platform), then they should be worked around in the driver.
> > > >
> > > > The correct thing to do here is craft a clk-keep-on flag and ensure it
> > > > is set to true for the effected platform(s)' platform data.
> > >
> > > I'm always wary of creating a driver specific flag, especially when its
> > > purpose is to do the same thing as an existing mechanism provided by the
> > > subsystem of not gating the clock.
> >
> > Using existing sub-system supplied mechanisms in the way they were not
> > intended is sub-optimal (read "hacky").
>
> I think the scope of this flag has been defined in a very narrow way, which is
> making you want to suggest lots of hacks in the client driver.
The scope was narrowed intentionally, buy the maintainers. I am
merely reflecting their views. If you really wish to contend these,
you should take it up with them.
> > > I can see a couple of problems with what you propose:
> > >
> > > 1) You have to put the clk-keep-on flag in every driver which consumes the
> > > clock. IMO it is much better to have this knowledge in the SoC's
> > > clock driver so every consumer of the clock automatically benefits.
> >
> > That would also be fine(ish). The issue is that this problem is
> > board specific, so the platform clock driver would have to contain
> > board level knowledge.
>
> It is not board specific. It is a SoC HW bug, so by definition present on all
> boards which have this SoC.
Okay SoC specific. Is there a SoC specific clock driver?
> > Also, if you were to implement this, it would
> > too mess up reference counting in the core.
This still stands.
> > > 2) You don't benefit from the CLK_IS_CRITICAL reference counting logic in
> > > clk.c. So then each driver has to also work around that to get sensible reference
> > > counts. e.g.
> > >
> > > if (!__clk_is_enabled(clk) && pdata->clk-keep-on)
> > > clk_enable(clk)
> > >
> > > Which seems to me to be fighting against the subsystem. Given that the only use of
> > > _clk_is_enabled() outside drivers/clk is in an old arch/arm/mach-omap2/pm24xx.c
> > > driver I suspect its use is frowned upon, and it shouldn't really be an EXPORTED_SYMBOL.
> >
> > In this instance, since the STFE clock is only used by this IP, I
> > would choose to handle it in the driver.
>
> There are other IPs within this IP block for which upstream drivers don't yet exist.
If they are many and various, we can discuss alternative solutions.
What are they?
> > This can be done using a
> > single flag stored in pdata which should be fetched using
> > of_match_device(). This way there is no need for any more API abuse;
> > either by incorrectly identifying the STFE clock as critical OR
> > invoking any internal __clk_*() calls.
> >
> > Enable the clock once in .probe(), which you already do.
>
> But these drivers are by default built as modules, so when you rmmod, and insmod the
> driver you now have an ever increasing clock reference count. This is the
> problem I was describing in the previous email, and why what you propose is a
> bad idea.
Then the variable would have to be exported.
> > Then, whenever you do any power saving do:
> >
> > suspend()
> > {
> > if (!ddata->enable_clk_once)
> > clk_disable(clk);
> > }
> >
> > resume()
> > {
> > if (!ddata->enable_clk_once)
> > clk_enable(clk);
> > }
> >
> > However, looking at your driver, I think this point might even be
> > moot, since you don't have any power saving. The only time you
> > disable the clock is in the error path. Just replace that with a
> > comment about the platform's unfortunate errata.
>
> This is exactly what I want to avoid doing. The driver already has these
> hacks as it was waiting for the critical clock patches to land so I could remove
> them and fix the problem properly.
>
> Much like you did with the I2C and SPI drivers, where you removed similar hacks
> and added the clock to the critical clock list.
Right, see above.
> > > [1] https://lkml.org/lkml/2016/1/18/272
> >
> > I'm glad you mentioned this. Let's take a look:
> >
> > > Some platforms contain clocks which if gated, will cause undefined or
> > > catastrophic behaviours. As such they are not to be turned off, ever.
> >
> > Not the case here.
> >
> > This clock *can* be gated and can be turned off *sometimes*.
>
> See above, if the clock is on when Linux boots it can never be assumed that it
> is safe to disable it.
See above.
> > > Many of these such clocks do not have devices, thus device drivers
> > > where clocks may be enabled and references taken to ensure they stay
> > > enabled do not exist. Therefore, we must handle these such cases in
> > > the core.
> >
> > This clock *does* have a driver and correct references *can* be
> > taken.
>
> As above this is the same for CLK_EXT2F_A9, which has multiple users in the
> kernel.
>
> The point is we don't wish to have the knowledge in the individual drivers that
> this clock is critical and can destroy the system if it is disabled.
>
> >
> > [...]
> >
> > All I'm saying is, and it's the same thing I've said many times; these
> > types of issues do not exhibit the same set of symptoms as a critical
> > clock by definition. Critical clocks are those which references can
> > not be taken by any other means.
>
> That is not how you are using it currently. See CLK_EXT2F_A9 and
> CLK_TX_ICN_DMU.
>
> > Think of the critical clock
> > framework as a mechanism to circumvent the requirement of writing a
> > special driver which would *only* handle clocks i.e. an interconnect
> > driver in the ST case.
> >
>
> You didn't need a new flag for that. CLK_IGNORE_UNUSED already allowed you to not
> write a specical driver which only handled for example an interconnect clock and
> would ensure it wouldn't be gated by the disable_unused logic.
>
> IMO the point of the critical clock stuff, and what CLK_IGNORE_UNUSED *did not* allow
> you to do is for situations like CLK_EXT2F_A9, CLK_TX_ICN_DMU and PROC_STFE where users
> *do* exist in the kernel, but the SoC clock driver has *special* knowledge of the
> clock tree on the SoC and knows that despite what the drivers are doing with
> their enable/disable calls the clock should never be disabled.
>
> In fact out of what we currently mark as clock-critical on STi platform most
> of the clocks could have been handled perfectly fine with the CLK_IGNORE_UNUSED
> flag (apart from the fact that there is no way to set the flag from DT). It is
> only CLK_EXT2F_A9 and CLK_TX_ICN_DMU which *do* have kernel users where the
> extra functionality of critical clocks is required (we need the additional
> reference taken by the clk framework to avoid the kernel drivers from disabling
> the clock).
Probably best to take your special use-case up with the Clock
Maintainers. As the author of the critical-clocks patch-set, I would
say that your use-case does not tick all the boxes for use of the
critical-clock mechanism, but at the end of the day, it really isn't
my train-set.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH/RFT v2 13/17] USB: da8xx: use ohci priv data instead of globals
From: Axel Haslam @ 2016-10-25 9:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0cf6f211-90ba-2f7b-978f-f1cfa1b11554@lechnology.com>
On Tue, Oct 25, 2016 at 3:12 AM, David Lechner <david@lechnology.com> wrote:
> On 10/24/2016 11:46 AM, ahaslam at baylibre.com wrote:
>>
>> From: Axel Haslam <ahaslam@baylibre.com>
>> > static const struct ohci_driver_overrides da8xx_overrides __initconst =
>> > {
>> - .reset = ohci_da8xx_reset
>> + .reset = ohci_da8xx_reset,
>> + .extra_priv_size = sizeof(struct da8xx_ohci_hcd),
>
>
> nit: since you are changing both lines anyway, you might as well make the
> ='s line up.
ok, will do.
>
>> };
>>
>> /*
>>
>
^ permalink raw reply
* [PATCH/RFT v2 15/17] usb: host: ohci-da8xx: Add devicetree bindings documentation
From: Axel Haslam @ 2016-10-25 9:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <523928a1-a321-abb6-504f-a1053c88ce53@lechnology.com>
On Tue, Oct 25, 2016 at 3:02 AM, David Lechner <david@lechnology.com> wrote:
> On 10/24/2016 11:46 AM, ahaslam at baylibre.com wrote:
>>
>> From: Axel Haslam <ahaslam@baylibre.com>
>>
>> This patch documents the device tree bindings required for
>> the ohci controller found in TI da8xx family of SoC's
>>
>> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
>> ---
>> .../devicetree/bindings/usb/ohci-da8xx.txt | 39
>> ++++++++++++++++++++++
>> 1 file changed, 39 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
>> b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
>> new file mode 100644
>> index 0000000..4251c84
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
>> @@ -0,0 +1,39 @@
>> +DA8XX USB OHCI controller
>> +
>> +Required properties:
>> +
>> + - compatible: Should be "ti,da830-ohci"
>> + - reg: Should contain one register range i.e. start and length
>> + - interrupts: Description of the interrupt line
>> + - phys: Phandle for the PHY device
>> + - phy-names: Should be "usb-phy"
>> +
>> +Optional properties:
>> + - vbus-supply: Regulator that controls vbus power
>
>
>
> Isn't vbus-supply property required?
>
> If it is really supposed to be optional, the ohci driver needs to use
> devm_regulator_get_optional() and handle the case when there is no
> regulator.
>
> I don't see a problem with making it required though since one can just use
> a dummy supply if there is not a real one.
The regulator framework will use a dummy regulator if none is provided.
>
>> +
>> +Example for omap138-lck:
>> +
>> +vbus_fixed: fixed-regulator-vbus {
>> + compatible = "regulator-fixed";
>> + gpio = <&gpio 109 0>;
>> + oc-gpio = <&gpio 36 0>;
>> + regulator-boot-on;
>> + enable-active-high;
>> + regulator-name = "vbus";
>> + regulator-min-microvolt = <5000000>;
>> + regulator-max-microvolt = <5000000>;
>> +};
>> +
>> +usb_phy: usb-phy {
>> + compatible = "ti,da830-usb-phy";
>> + #phy-cells = <1>;
>> + status = "disabled";
>
>
> why disabled?
>
yes, i copied from the device tree im using, but
i should enable these for the example,
i will fix.
>> +};
>> +usb: usb at 0225000 {
>> + compatible = "ti,da830-ohci";
>> + reg = <0x225000 0x1000>;
>> + interrupts = <59>;
>> + phys = <&usb_phy 1>;
>> + phy-names = "usb-phy";
>
>
> missing vbus-supply property
>
>> + status = "disabled";
>
>
> why disabled?
>
>> +};
>>
>
^ permalink raw reply
* [PATCH v2 3/4] soc: ti: Add ti_sci_pm_domains driver
From: Ulf Hansson @ 2016-10-25 9:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161019203347.17893-4-d-gerlach@ti.com>
On 19 October 2016 at 22:33, Dave Gerlach <d-gerlach@ti.com> wrote:
> Introduce a ti_sci_pm_domains driver to act as a generic pm domain provider
> to allow each device to attach and associate it's ti-sci-id so that it can
> be controlled through the TI SCI protocol.
>
> This driver implements a simple genpd where each device node has
> a phandle to the power domain node and also must provide an index which
> represents the ID to be passed with TI SCI representing the device using a
> ti,sci-id property. Through this interface the genpd dev_ops start and
> stop hooks will use TI SCI to turn on and off each device as determined
> by pm_runtime usage.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Looks good!
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> MAINTAINERS | 1 +
> arch/arm/mach-keystone/Kconfig | 1 +
> drivers/soc/ti/Kconfig | 12 +++
> drivers/soc/ti/Makefile | 1 +
> drivers/soc/ti/ti_sci_pm_domains.c | 198 +++++++++++++++++++++++++++++++++++++
> 5 files changed, 213 insertions(+)
> create mode 100644 drivers/soc/ti/ti_sci_pm_domains.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d894873c2bff..3eaac5ede847 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11894,6 +11894,7 @@ F: drivers/firmware/ti_sci*
> F: include/linux/soc/ti/ti_sci_protocol.h
> F: Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> F: include/dt-bindings/genpd/k2g.h
> +F: drivers/soc/ti/ti_sci_pm_domains.c
>
> THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
> M: Hans Verkuil <hverkuil@xs4all.nl>
> diff --git a/arch/arm/mach-keystone/Kconfig b/arch/arm/mach-keystone/Kconfig
> index 24bd64dabdfc..18d49465cafb 100644
> --- a/arch/arm/mach-keystone/Kconfig
> +++ b/arch/arm/mach-keystone/Kconfig
> @@ -9,6 +9,7 @@ config ARCH_KEYSTONE
> select ARCH_SUPPORTS_BIG_ENDIAN
> select ZONE_DMA if ARM_LPAE
> select PINCTRL
> + select PM_GENERIC_DOMAINS if PM
> help
> Support for boards based on the Texas Instruments Keystone family of
> SoCs.
> diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
> index 3557c5e32a93..39e152abe6b9 100644
> --- a/drivers/soc/ti/Kconfig
> +++ b/drivers/soc/ti/Kconfig
> @@ -38,4 +38,16 @@ config WKUP_M3_IPC
> to communicate and use the Wakeup M3 for PM features like suspend
> resume and boots it using wkup_m3_rproc driver.
>
> +config TI_SCI_PM_DOMAINS
> + tristate "TI SCI PM Domains Driver"
> + depends on TI_SCI_PROTOCOL
> + depends on PM_GENERIC_DOMAINS
> + help
> + Generic power domain implementation for TI device implementing
> + the TI SCI protocol.
> +
> + To compile this as a module, choose M here. The module will be
> + called ti_sci_pm_domains. Note this is needed early in boot before
> + rootfs may be available.
> +
> endif # SOC_TI
> diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
> index 48ff3a79634f..7d572736c86e 100644
> --- a/drivers/soc/ti/Makefile
> +++ b/drivers/soc/ti/Makefile
> @@ -5,3 +5,4 @@ obj-$(CONFIG_KEYSTONE_NAVIGATOR_QMSS) += knav_qmss.o
> knav_qmss-y := knav_qmss_queue.o knav_qmss_acc.o
> obj-$(CONFIG_KEYSTONE_NAVIGATOR_DMA) += knav_dma.o
> obj-$(CONFIG_WKUP_M3_IPC) += wkup_m3_ipc.o
> +obj-$(CONFIG_TI_SCI_PM_DOMAINS) += ti_sci_pm_domains.o
> diff --git a/drivers/soc/ti/ti_sci_pm_domains.c b/drivers/soc/ti/ti_sci_pm_domains.c
> new file mode 100644
> index 000000000000..ec76215d64c7
> --- /dev/null
> +++ b/drivers/soc/ti/ti_sci_pm_domains.c
> @@ -0,0 +1,198 @@
> +/*
> + * TI SCI Generic Power Domain Driver
> + *
> + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
> + * J Keerthy <j-keerthy@ti.com>
> + * Dave Gerlach <d-gerlach@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/slab.h>
> +#include <linux/soc/ti/ti_sci_protocol.h>
> +
> +/**
> + * struct ti_sci_genpd_dev_data: holds data needed for every device attached
> + * to this genpd
> + * @idx: index of the device that identifies it with the system
> + * control processor.
> + */
> +struct ti_sci_genpd_dev_data {
> + int idx;
> +};
> +
> +/**
> + * struct ti_sci_pm_domain: TI specific data needed for power domain
> + * @ti_sci: handle to TI SCI protocol driver that provides ops to
> + * communicate with system control processor.
> + * @dev: pointer to dev for the driver for devm allocs
> + * @pd: generic_pm_domain for use with the genpd framework
> + */
> +struct ti_sci_pm_domain {
> + const struct ti_sci_handle *ti_sci;
> + struct device *dev;
> + struct generic_pm_domain pd;
> +};
> +
> +#define genpd_to_ti_sci_pd(gpd) container_of(gpd, struct ti_sci_pm_domain, pd)
> +
> +/**
> + * ti_sci_dev_id(): get prepopulated ti_sci id from struct dev
> + * @dev: pointer to device associated with this genpd
> + *
> + * Returns device_id stored from ti,sci_id property
> + */
> +static int ti_sci_dev_id(struct device *dev)
> +{
> + struct generic_pm_domain_data *genpd_data = dev_gpd_data(dev);
> + struct ti_sci_genpd_dev_data *sci_dev_data = genpd_data->data;
> +
> + return sci_dev_data->idx;
> +}
> +
> +/**
> + * ti_sci_dev_to_sci_handle(): get pointer to ti_sci_handle
> + * @dev: pointer to device associated with this genpd
> + *
> + * Returns ti_sci_handle to be used to communicate with system
> + * control processor.
> + */
> +static const struct ti_sci_handle *ti_sci_dev_to_sci_handle(struct device *dev)
> +{
> + struct generic_pm_domain *pd = pd_to_genpd(dev->pm_domain);
> + struct ti_sci_pm_domain *ti_sci_genpd = genpd_to_ti_sci_pd(pd);
> +
> + return ti_sci_genpd->ti_sci;
> +}
> +
> +/**
> + * ti_sci_dev_start(): genpd device start hook called to turn device on
> + * @dev: pointer to device associated with this genpd to be powered on
> + */
> +static int ti_sci_dev_start(struct device *dev)
> +{
> + const struct ti_sci_handle *ti_sci = ti_sci_dev_to_sci_handle(dev);
> + int idx = ti_sci_dev_id(dev);
> +
> + return ti_sci->ops.dev_ops.get_device(ti_sci, idx);
> +}
> +
> +/**
> + * ti_sci_dev_stop(): genpd device stop hook called to turn device off
> + * @dev: pointer to device associated with this genpd to be powered off
> + */
> +static int ti_sci_dev_stop(struct device *dev)
> +{
> + const struct ti_sci_handle *ti_sci = ti_sci_dev_to_sci_handle(dev);
> + int idx = ti_sci_dev_id(dev);
> +
> + return ti_sci->ops.dev_ops.put_device(ti_sci, idx);
> +}
> +
> +static int ti_sci_pd_attach_dev(struct generic_pm_domain *domain,
> + struct device *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct ti_sci_pm_domain *ti_sci_genpd = genpd_to_ti_sci_pd(domain);
> + const struct ti_sci_handle *ti_sci = ti_sci_genpd->ti_sci;
> + struct ti_sci_genpd_dev_data *sci_dev_data;
> + struct generic_pm_domain_data *genpd_data;
> + int idx, ret = 0;
> +
> + ret = of_property_read_u32(np, "ti,sci-id", &idx);
> + if (ret) {
> + dev_err(ti_sci_genpd->dev, "Cannot find ti,sci-id for %s\n",
> + dev_name(dev));
> + return -ENODEV;
> + }
> +
> + /*
> + * Check the validity of the requested idx, if the index is not valid
> + * the PMMC will return a NAK here and we will not allocate it.
> + */
> + ret = ti_sci->ops.dev_ops.is_valid(ti_sci, idx);
> + if (ret)
> + return -EINVAL;
> +
> + sci_dev_data = kzalloc(sizeof(*sci_dev_data), GFP_KERNEL);
> + if (!sci_dev_data)
> + return -ENOMEM;
> +
> + sci_dev_data->idx = idx;
> +
> + genpd_data = dev_gpd_data(dev);
> + genpd_data->data = sci_dev_data;
> +
> + return 0;
> +}
> +
> +static void ti_sci_pd_detach_dev(struct generic_pm_domain *domain,
> + struct device *dev)
> +{
> + struct generic_pm_domain_data *genpd_data = dev_gpd_data(dev);
> + struct ti_sci_genpd_dev_data *sci_dev_data = genpd_data->data;
> +
> + kfree(sci_dev_data);
> + genpd_data->data = NULL;
> +}
> +
> +static const struct of_device_id ti_sci_pm_domain_matches[] = {
> + { .compatible = "ti,sci-pm-domain", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, ti_sci_pm_domain_matches);
> +
> +static int ti_sci_pm_domain_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct ti_sci_pm_domain *ti_sci_pd;
> + int ret;
> +
> + ti_sci_pd = devm_kzalloc(dev, sizeof(*ti_sci_pd), GFP_KERNEL);
> + if (!ti_sci_pd)
> + return -ENOMEM;
> +
> + ti_sci_pd->ti_sci = devm_ti_sci_get_handle(dev);
> + if (IS_ERR(ti_sci_pd->ti_sci))
> + return PTR_ERR(ti_sci_pd->ti_sci);
> +
> + ti_sci_pd->dev = dev;
> +
> + ti_sci_pd->pd.attach_dev = ti_sci_pd_attach_dev;
> + ti_sci_pd->pd.detach_dev = ti_sci_pd_detach_dev;
> +
> + ti_sci_pd->pd.dev_ops.start = ti_sci_dev_start;
> + ti_sci_pd->pd.dev_ops.stop = ti_sci_dev_stop;
> +
> + pm_genpd_init(&ti_sci_pd->pd, NULL, true);
> +
> + ret = of_genpd_add_provider_simple(np, &ti_sci_pd->pd);
> +
> + return ret;
> +}
> +
> +static struct platform_driver ti_sci_pm_domains_driver = {
> + .probe = ti_sci_pm_domain_probe,
> + .driver = {
> + .name = "ti_sci_pm_domains",
> + .of_match_table = ti_sci_pm_domain_matches,
> + },
> +};
> +module_platform_driver(ti_sci_pm_domains_driver);
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("TI System Control Interface (SCI) Power Domain driver");
> +MODULE_AUTHOR("Dave Gerlach");
> --
> 2.9.3
>
^ permalink raw reply
* [PATCH v2 1/4] PM / Domains: Add generic data pointer to genpd data struct
From: Ulf Hansson @ 2016-10-25 9:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161019203347.17893-2-d-gerlach@ti.com>
On 19 October 2016 at 22:33, Dave Gerlach <d-gerlach@ti.com> wrote:
> Add a void *data pointer to struct generic_pm_domain_data. Because this
> exists for each device associated with a genpd it will allow us to
> assign per-device data if needed on a platform for control of that
> specific device.
>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> include/linux/pm_domain.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index a09fe5c009c8..9c0a897fe605 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -105,6 +105,7 @@ struct generic_pm_domain_data {
> struct pm_domain_data base;
> struct gpd_timing_data td;
> struct notifier_block nb;
> + void *data;
> };
>
> #ifdef CONFIG_PM_GENERIC_DOMAINS
> --
> 2.9.3
>
^ permalink raw reply
* [PATCH] ARM: sti: stih410-clocks: Add PROC_STFE as a critical clock
From: Peter Griffin @ 2016-10-25 9:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161025074257.GF8574@dell>
Hi Lee,
On Tue, 25 Oct 2016, Lee Jones wrote:
> On Mon, 24 Oct 2016, Peter Griffin wrote:
>
> > Hi Lee,
> >
> > On Mon, 24 Oct 2016, Lee Jones wrote:
> > > On Tue, 18 Oct 2016, Peter Griffin wrote:
> > >
> > > > Once the ST frontend demux HW IP has been enabled, the clock can't
> > > > be disabled otherwise the system will hang and the board will
> > > > be unserviceable.
> > > >
> > > > To allow balanced clock enable/disable calls in the driver we use
> > > > the critical clock infrastructure to take an extra reference on the
> > > > clock so the clock will never actually be disabled.
> > >
> > > This is an abuse of the critical-clocks framework, and is exactly the
> > > type of hack I promised the clk guys I'd try to prevent.
> >
> > I expect the best way to do this would be to write some documentation on the
> > clock-critical DT binding and/or CRITICAL_CLK flag. The only documentation I can
> > find currently is with the initial patch series [1] and the comment in
> > clk-provider.h of
> >
> > #define CLK_IS_CRITICAL BIT(11) /* do not gate, ever */
> >
> > Or the patch decription
> >
> > "Critical clocks are those which must not be gated, else undefined
> > or catastrophic failure would occur. Here we have chosen to
> > ensure the prepare/enable counts are correctly incremented, so as
> > not to confuse users with enabled clocks with no visible users."
> >
> > Which is the functionality I want for this clock.
>
> No, that's not the functionality you want.
Yes it is :)
> You want for the clock not
> to be RE-gated (big difference). Currently, the STFE clock will never
> be gated, even when a) it's not used and b) can actually be disabled.
> You're needlessly wasting power here.
IMO it is *never* safe for Linux to gate this clock, as you have no idea
on the state of the hardware from the primary and secondary bootloaders.
If the clock is enabled when Linux boots, the Linux clock framework *needs*
to assume the hardware may have been used in previous boot stages, and it should
not attempt to disable the clock.
>
> Also, in your use-case there is a visible user, and the prepare/enable
> counts will be correct.
Your correct there is a visible user, but this is the same as CLK_EXT2F_A9 where
there are multiple users of the clock (SPI, I2C, UART etc).
>
> > > If this, or
> > > any other IP has some quirks (i.e. once enabled, if this clock is
> > > subsequently disabled it will have a catastrophic effect on the
> > > platform), then they should be worked around in the driver.
> > >
> > > The correct thing to do here is craft a clk-keep-on flag and ensure it
> > > is set to true for the effected platform(s)' platform data.
> >
> > I'm always wary of creating a driver specific flag, especially when its
> > purpose is to do the same thing as an existing mechanism provided by the
> > subsystem of not gating the clock.
>
> Using existing sub-system supplied mechanisms in the way they were not
> intended is sub-optimal (read "hacky").
I think the scope of this flag has been defined in a very narrow way, which is
making you want to suggest lots of hacks in the client driver.
>
> > I can see a couple of problems with what you propose:
> >
> > 1) You have to put the clk-keep-on flag in every driver which consumes the
> > clock. IMO it is much better to have this knowledge in the SoC's
> > clock driver so every consumer of the clock automatically benefits.
>
> That would also be fine(ish). The issue is that this problem is
> board specific, so the platform clock driver would have to contain
> board level knowledge.
It is not board specific. It is a SoC HW bug, so by definition present on all
boards which have this SoC.
> Also, if you were to implement this, it would
> too mess up reference counting in the core.
>
> > 2) You don't benefit from the CLK_IS_CRITICAL reference counting logic in
> > clk.c. So then each driver has to also work around that to get sensible reference
> > counts. e.g.
> >
> > if (!__clk_is_enabled(clk) && pdata->clk-keep-on)
> > clk_enable(clk)
> >
> > Which seems to me to be fighting against the subsystem. Given that the only use of
> > _clk_is_enabled() outside drivers/clk is in an old arch/arm/mach-omap2/pm24xx.c
> > driver I suspect its use is frowned upon, and it shouldn't really be an EXPORTED_SYMBOL.
>
> In this instance, since the STFE clock is only used by this IP, I
> would choose to handle it in the driver.
There are other IPs within this IP block for which upstream drivers don't yet exist.
> This can be done using a
> single flag stored in pdata which should be fetched using
> of_match_device(). This way there is no need for any more API abuse;
> either by incorrectly identifying the STFE clock as critical OR
> invoking any internal __clk_*() calls.
>
> Enable the clock once in .probe(), which you already do.
But these drivers are by default built as modules, so when you rmmod, and insmod the
driver you now have an ever increasing clock reference count. This is the
problem I was describing in the previous email, and why what you propose is a
bad idea.
>
> Then, whenever you do any power saving do:
>
> suspend()
> {
> if (!ddata->enable_clk_once)
> clk_disable(clk);
> }
>
> resume()
> {
> if (!ddata->enable_clk_once)
> clk_enable(clk);
> }
>
> However, looking at your driver, I think this point might even be
> moot, since you don't have any power saving. The only time you
> disable the clock is in the error path. Just replace that with a
> comment about the platform's unfortunate errata.
This is exactly what I want to avoid doing. The driver already has these
hacks as it was waiting for the critical clock patches to land so I could remove
them and fix the problem properly.
Much like you did with the I2C and SPI drivers, where you removed similar hacks
and added the clock to the critical clock list.
>
> > [1] https://lkml.org/lkml/2016/1/18/272
>
> I'm glad you mentioned this. Let's take a look:
>
> > Some platforms contain clocks which if gated, will cause undefined or
> > catastrophic behaviours. As such they are not to be turned off, ever.
>
> Not the case here.
>
> This clock *can* be gated and can be turned off *sometimes*.
See above, if the clock is on when Linux boots it can never be assumed that it
is safe to disable it.
>
> > Many of these such clocks do not have devices, thus device drivers
> > where clocks may be enabled and references taken to ensure they stay
> > enabled do not exist. Therefore, we must handle these such cases in
> > the core.
>
> This clock *does* have a driver and correct references *can* be
> taken.
As above this is the same for CLK_EXT2F_A9, which has multiple users in the
kernel.
The point is we don't wish to have the knowledge in the individual drivers that
this clock is critical and can destroy the system if it is disabled.
>
> [...]
>
> All I'm saying is, and it's the same thing I've said many times; these
> types of issues do not exhibit the same set of symptoms as a critical
> clock by definition. Critical clocks are those which references can
> not be taken by any other means.
That is not how you are using it currently. See CLK_EXT2F_A9 and
CLK_TX_ICN_DMU.
> Think of the critical clock
> framework as a mechanism to circumvent the requirement of writing a
> special driver which would *only* handle clocks i.e. an interconnect
> driver in the ST case.
>
You didn't need a new flag for that. CLK_IGNORE_UNUSED already allowed you to not
write a specical driver which only handled for example an interconnect clock and
would ensure it wouldn't be gated by the disable_unused logic.
IMO the point of the critical clock stuff, and what CLK_IGNORE_UNUSED *did not* allow
you to do is for situations like CLK_EXT2F_A9, CLK_TX_ICN_DMU and PROC_STFE where users
*do* exist in the kernel, but the SoC clock driver has *special* knowledge of the
clock tree on the SoC and knows that despite what the drivers are doing with
their enable/disable calls the clock should never be disabled.
In fact out of what we currently mark as clock-critical on STi platform most
of the clocks could have been handled perfectly fine with the CLK_IGNORE_UNUSED
flag (apart from the fact that there is no way to set the flag from DT). It is
only CLK_EXT2F_A9 and CLK_TX_ICN_DMU which *do* have kernel users where the
extra functionality of critical clocks is required (we need the additional
reference taken by the clk framework to avoid the kernel drivers from disabling
the clock).
regards,
Peter.
^ permalink raw reply
* [PATCH v3 2/2] power/reset: at91-poweroff: timely shutdown LPDDR memories
From: Alexandre Belloni @ 2016-10-25 9:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161025093759.20913-1-alexandre.belloni@free-electrons.com>
LPDDR memories can only handle up to 400 uncontrolled power off. Ensure the
proper power off sequence is used before shutting down the platform.
Cc: <stable@vger.kernel.org> # 4.4+
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
drivers/power/reset/Kconfig | 2 +-
drivers/power/reset/at91-poweroff.c | 54 +++++++++++++++++++++++++++++++-
drivers/power/reset/at91-sama5d2_shdwc.c | 49 ++++++++++++++++++++++++++++-
3 files changed, 102 insertions(+), 3 deletions(-)
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index c74c3f67b8da..02e46bbcf45d 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -32,7 +32,7 @@ config POWER_RESET_AT91_RESET
config POWER_RESET_AT91_SAMA5D2_SHDWC
tristate "Atmel AT91 SAMA5D2-Compatible shutdown controller driver"
- depends on ARCH_AT91 || COMPILE_TEST
+ depends on ARCH_AT91
default SOC_SAMA5
help
This driver supports the alternate shutdown controller for some Atmel
diff --git a/drivers/power/reset/at91-poweroff.c b/drivers/power/reset/at91-poweroff.c
index e9e24df35f26..2579f025b90b 100644
--- a/drivers/power/reset/at91-poweroff.c
+++ b/drivers/power/reset/at91-poweroff.c
@@ -14,9 +14,12 @@
#include <linux/io.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_address.h>
#include <linux/platform_device.h>
#include <linux/printk.h>
+#include <soc/at91/at91sam9_ddrsdr.h>
+
#define AT91_SHDW_CR 0x00 /* Shut Down Control Register */
#define AT91_SHDW_SHDW BIT(0) /* Shut Down command */
#define AT91_SHDW_KEY (0xa5 << 24) /* KEY Password */
@@ -50,6 +53,7 @@ static const char *shdwc_wakeup_modes[] = {
static void __iomem *at91_shdwc_base;
static struct clk *sclk;
+static void __iomem *mpddrc_base;
static void __init at91_wakeup_status(void)
{
@@ -73,6 +77,29 @@ static void at91_poweroff(void)
writel(AT91_SHDW_KEY | AT91_SHDW_SHDW, at91_shdwc_base + AT91_SHDW_CR);
}
+static void at91_lpddr_poweroff(void)
+{
+ asm volatile(
+ /* Align to cache lines */
+ ".balign 32\n\t"
+
+ /* Ensure AT91_SHDW_CR is in the TLB by reading it */
+ " ldr r6, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
+
+ /* Power down SDRAM0 */
+ " str %1, [%0, #" __stringify(AT91_DDRSDRC_LPR) "]\n\t"
+ /* Shutdown CPU */
+ " str %3, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
+
+ " b .\n\t"
+ :
+ : "r" (mpddrc_base),
+ "r" cpu_to_le32(AT91_DDRSDRC_LPDDR2_PWOFF),
+ "r" (at91_shdwc_base),
+ "r" cpu_to_le32(AT91_SHDW_KEY | AT91_SHDW_SHDW)
+ : "r0");
+}
+
static int at91_poweroff_get_wakeup_mode(struct device_node *np)
{
const char *pm;
@@ -124,6 +151,8 @@ static void at91_poweroff_dt_set_wakeup_mode(struct platform_device *pdev)
static int __init at91_poweroff_probe(struct platform_device *pdev)
{
struct resource *res;
+ struct device_node *np;
+ u32 ddr_type;
int ret;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -150,12 +179,30 @@ static int __init at91_poweroff_probe(struct platform_device *pdev)
pm_power_off = at91_poweroff;
+ np = of_find_compatible_node(NULL, NULL, "atmel,sama5d3-ddramc");
+ if (!np)
+ return 0;
+
+ mpddrc_base = of_iomap(np, 0);
+ of_node_put(np);
+
+ if (!mpddrc_base)
+ return 0;
+
+ ddr_type = readl(mpddrc_base + AT91_DDRSDRC_MDR) & AT91_DDRSDRC_MD;
+ if ((ddr_type == AT91_DDRSDRC_MD_LPDDR2) ||
+ (ddr_type == AT91_DDRSDRC_MD_LPDDR3))
+ pm_power_off = at91_lpddr_poweroff;
+ else
+ iounmap(mpddrc_base);
+
return 0;
}
static int __exit at91_poweroff_remove(struct platform_device *pdev)
{
- if (pm_power_off == at91_poweroff)
+ if (pm_power_off == at91_poweroff ||
+ pm_power_off == at91_lpddr_poweroff)
pm_power_off = NULL;
clk_disable_unprepare(sclk);
@@ -163,6 +210,11 @@ static int __exit at91_poweroff_remove(struct platform_device *pdev)
return 0;
}
+static const struct of_device_id at91_ramc_of_match[] = {
+ { .compatible = "atmel,sama5d3-ddramc", },
+ { /* sentinel */ }
+};
+
static const struct of_device_id at91_poweroff_of_match[] = {
{ .compatible = "atmel,at91sam9260-shdwc", },
{ .compatible = "atmel,at91sam9rl-shdwc", },
diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c b/drivers/power/reset/at91-sama5d2_shdwc.c
index 8a5ac9706c9c..90b0b5a70ce5 100644
--- a/drivers/power/reset/at91-sama5d2_shdwc.c
+++ b/drivers/power/reset/at91-sama5d2_shdwc.c
@@ -22,9 +22,12 @@
#include <linux/io.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_address.h>
#include <linux/platform_device.h>
#include <linux/printk.h>
+#include <soc/at91/at91sam9_ddrsdr.h>
+
#define SLOW_CLOCK_FREQ 32768
#define AT91_SHDW_CR 0x00 /* Shut Down Control Register */
@@ -75,6 +78,7 @@ struct shdwc {
*/
static struct shdwc *at91_shdwc;
static struct clk *sclk;
+static void __iomem *mpddrc_base;
static const unsigned long long sdwc_dbc_period[] = {
0, 3, 32, 512, 4096, 32768,
@@ -108,6 +112,29 @@ static void at91_poweroff(void)
at91_shdwc->at91_shdwc_base + AT91_SHDW_CR);
}
+static void at91_lpddr_poweroff(void)
+{
+ asm volatile(
+ /* Align to cache lines */
+ ".balign 32\n\t"
+
+ /* Ensure AT91_SHDW_CR is in the TLB by reading it */
+ " ldr r6, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
+
+ /* Power down SDRAM0 */
+ " str %1, [%0, #" __stringify(AT91_DDRSDRC_LPR) "]\n\t"
+ /* Shutdown CPU */
+ " str %3, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
+
+ " b .\n\t"
+ :
+ : "r" (mpddrc_base),
+ "r" cpu_to_le32(AT91_DDRSDRC_LPDDR2_PWOFF),
+ "r" (at91_shdwc->at91_shdwc_base),
+ "r" cpu_to_le32(AT91_SHDW_KEY | AT91_SHDW_SHDW)
+ : "r0");
+}
+
static u32 at91_shdwc_debouncer_value(struct platform_device *pdev,
u32 in_period_us)
{
@@ -212,6 +239,8 @@ static int __init at91_shdwc_probe(struct platform_device *pdev)
{
struct resource *res;
const struct of_device_id *match;
+ struct device_node *np;
+ u32 ddr_type;
int ret;
if (!pdev->dev.of_node)
@@ -249,6 +278,23 @@ static int __init at91_shdwc_probe(struct platform_device *pdev)
pm_power_off = at91_poweroff;
+ np = of_find_compatible_node(NULL, NULL, "atmel,sama5d3-ddramc");
+ if (!np)
+ return 0;
+
+ mpddrc_base = of_iomap(np, 0);
+ of_node_put(np);
+
+ if (!mpddrc_base)
+ return 0;
+
+ ddr_type = readl(mpddrc_base + AT91_DDRSDRC_MDR) & AT91_DDRSDRC_MD;
+ if ((ddr_type == AT91_DDRSDRC_MD_LPDDR2) ||
+ (ddr_type == AT91_DDRSDRC_MD_LPDDR3))
+ pm_power_off = at91_lpddr_poweroff;
+ else
+ iounmap(mpddrc_base);
+
return 0;
}
@@ -256,7 +302,8 @@ static int __exit at91_shdwc_remove(struct platform_device *pdev)
{
struct shdwc *shdw = platform_get_drvdata(pdev);
- if (pm_power_off == at91_poweroff)
+ if (pm_power_off == at91_poweroff ||
+ pm_power_off == at91_lpddr_poweroff)
pm_power_off = NULL;
/* Reset values to disable wake-up features */
--
2.9.3
^ permalink raw reply related
* [PATCH v3 1/2] ARM: at91: define LPDDR types
From: Alexandre Belloni @ 2016-10-25 9:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161025093759.20913-1-alexandre.belloni@free-electrons.com>
The Atmel MPDDR controller support LPDDR2 and LPDDR3 memories, add their
types.
Cc: <stable@vger.kernel.org> # 4.4+
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
include/soc/at91/at91sam9_ddrsdr.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/soc/at91/at91sam9_ddrsdr.h b/include/soc/at91/at91sam9_ddrsdr.h
index dc10c52e0e91..393362bdb860 100644
--- a/include/soc/at91/at91sam9_ddrsdr.h
+++ b/include/soc/at91/at91sam9_ddrsdr.h
@@ -81,6 +81,7 @@
#define AT91_DDRSDRC_LPCB_POWER_DOWN 2
#define AT91_DDRSDRC_LPCB_DEEP_POWER_DOWN 3
#define AT91_DDRSDRC_CLKFR (1 << 2) /* Clock Frozen */
+#define AT91_DDRSDRC_LPDDR2_PWOFF (1 << 3) /* LPDDR Power Off */
#define AT91_DDRSDRC_PASR (7 << 4) /* Partial Array Self Refresh */
#define AT91_DDRSDRC_TCSR (3 << 8) /* Temperature Compensated Self Refresh */
#define AT91_DDRSDRC_DS (3 << 10) /* Drive Strength */
@@ -96,7 +97,9 @@
#define AT91_DDRSDRC_MD_SDR 0
#define AT91_DDRSDRC_MD_LOW_POWER_SDR 1
#define AT91_DDRSDRC_MD_LOW_POWER_DDR 3
+#define AT91_DDRSDRC_MD_LPDDR3 5
#define AT91_DDRSDRC_MD_DDR2 6 /* [SAM9 Only] */
+#define AT91_DDRSDRC_MD_LPDDR2 7
#define AT91_DDRSDRC_DBW (1 << 4) /* Data Bus Width */
#define AT91_DDRSDRC_DBW_32BITS (0 << 4)
#define AT91_DDRSDRC_DBW_16BITS (1 << 4)
--
2.9.3
^ permalink raw reply related
* [PATCH v3 0/2] ARM: at91: properly handle LPDDR poweroff
From: Alexandre Belloni @ 2016-10-25 9:37 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
This patch set improves LPDDR support on SoCs using the Atmel MPDDR controller.
LPDDR memories can only handle up to 400 uncontrolled power offs in their
life. The proper power off sequence has to be applied before shutting down the
SoC.
I'm not too happy with the code duplication but this is a design choice
that has been made before because both shutdown controllers are really
different apart from the shutdown itself.
I guess it is still better than slowly killing the LPDDR.
Changes in v3:
- removed COMPILE_TEST for sama5d2_shdwc as it is now ARM specific
Changes in v2:
- Fix typos
- Add a comment for the dummy read access of AT91_SHDW_CR
- Properly set up pm_power_off in at91_poweroff_probe()
Alexandre Belloni (2):
ARM: at91: define LPDDR types
power/reset: at91-poweroff: timely shutdown LPDDR memories
drivers/power/reset/Kconfig | 2 +-
drivers/power/reset/at91-poweroff.c | 54 +++++++++++++++++++++++++++++++-
drivers/power/reset/at91-sama5d2_shdwc.c | 49 ++++++++++++++++++++++++++++-
include/soc/at91/at91sam9_ddrsdr.h | 3 ++
4 files changed, 105 insertions(+), 3 deletions(-)
--
2.9.3
^ 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