* [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
* [PATCH/RFT v2 03/17] ARM: davinci: da8xx: Add USB PHY platform declaration
From: Axel Haslam @ 2016-10-25 9:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4129f1be-42f6-f182-c8a2-84befe01d298@ti.com>
On Tue, Oct 25, 2016 at 11:18 AM, Sekhar Nori <nsekhar@ti.com> wrote:
> On Monday 24 October 2016 10:16 PM, ahaslam at baylibre.com wrote:
>> +static struct platform_device da8xx_usb_phy = {
>> + .name = "da8xx-usb-phy",
>> + .id = 0,
>
> There is a single phy control in the system for both 1.1 and 2.0 PHYs.
> so this can be a singular device (id -1).
>
Ok.
> Thanks,
> Sekhar
>
^ permalink raw reply
* [PATCH/RFT v2 02/17] ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration.
From: Axel Haslam @ 2016-10-25 9:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7f9efe9d-0912-e10a-3e45-24c5d2b455ab@ti.com>
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?
Regards,
Axel.
>> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
>
> Thanks,
> Sekhar
>
^ permalink raw reply
* [PATCH 0/4] i2c: Fix module autoload for some i2c busses platform drivers
From: Wolfram Sang @ 2016-10-25 9:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476824508-4679-1-git-send-email-javier@osg.samsung.com>
On Tue, Oct 18, 2016 at 06:01:44PM -0300, Javier Martinez Canillas wrote:
> Hello Wolfram,
>
> I noticed that module autoload won't be working in some of the i2c
> busses drivers. This patch series contains the fixes for these.
>
> Best regards,
> Javier
Applied to for-current, thank you very much for doing this
subsystem-wide!
-------------- 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/20161025/a53437d0/attachment.sig>
^ permalink raw reply
* [PATCH] ahci: use pci_alloc_irq_vectors
From: Robert Richter @ 2016-10-25 9:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161022141123.GA25500@lst.de>
On 22.10.16 16:11:23, Christoph Hellwig wrote:
> Hi Robert,
>
> is this a controller that's using MSI-X?
>
> If so can you try the patch below?
Great, that fixes the issue. Thanks.
Tested-by: Robert Richter <rrichter@cavium.com>
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index ba5f11c..5fe852d 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1617,7 +1617,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> /* legacy intx interrupts */
> pci_intx(pdev, 1);
> }
> - hpriv->irq = pdev->irq;
> + hpriv->irq = pci_irq_vector(pdev, 0);
>
> if (!(hpriv->cap & HOST_CAP_SSS) || ahci_ignore_sss)
> host->flags |= ATA_HOST_PARALLEL_SCAN;
^ permalink raw reply
* Disabling an interrupt in the handler locks the system up
From: Thomas Gleixner @ 2016-10-25 9:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <580F17E7.5060603@laposte.net>
On Tue, 25 Oct 2016, Sebastian Frias wrote:
> On 10/24/2016 06:55 PM, Thomas Gleixner wrote:
> > On Mon, 24 Oct 2016, Mason wrote:
> >>
> >> For the record, setting the IRQ_DISABLE_UNLAZY flag for this device
> >> makes the system lock-up disappear.
> >
> > The way how lazy irq disabling works is:
> >
> > 1) Interrupt is marked disabled in software, but the hardware is not masked
> >
> > 2) If the interrupt fires befor the interrupt is reenabled, then it's
> > masked at the hardware level in the low level interrupt flow handler.
> >
> Would you mind explaining what is the intention behind?
> Because it does not seem obvious why there isn't a direct map between
> "disable_irq*()" and "mask_irq()"
Two reasons for this:
1) If you mask edge type interrupts then you might race with an incoming
interrupt which then gets lost and eventually you won't get another
interrupt from that device. Even if there is no race, then on many irq
chips edge type interrupts are not latched when the interrupt line is
masked. That also can result in a stale interrupt line.
With the lazy disabling we mask only if an interrupt fires while it's
disabled in software. We note that it is pending and resend it when the
interrupt gets reenabled.
2) Accessing irq chip hardware can be slow and we have situations where
interrupts are disabled/enabled fast. So it's an optimization to avoid
the hardware access, which is sensible as we do not expect an interrupt
to fire in most cases. If it fires then we mask it when the interrupt
handler sees the disabled flag.
That should really work on any hardware and the IRQ_DISABLE_UNLAZY flag is
just there to deal with devices which are known to keep the (level based)
irq line active. In that case we know that we always take an interrupt to
mask it right away, so we can avoid the overhead.
Though you should not set that flag on edge type interrupts, unless your
hardware guarantees to avoid the issues described in #1.
Hope that helps.
Thanks,
tglx
^ permalink raw reply
* [PATCH/RFT v2 03/17] ARM: davinci: da8xx: Add USB PHY platform declaration
From: Sekhar Nori @ 2016-10-25 9:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164634.4330-4-ahaslam@baylibre.com>
On Monday 24 October 2016 10:16 PM, ahaslam at baylibre.com wrote:
> +static struct platform_device da8xx_usb_phy = {
> + .name = "da8xx-usb-phy",
> + .id = 0,
There is a single phy control in the system for both 1.1 and 2.0 PHYs.
so this can be a singular device (id -1).
Thanks,
Sekhar
^ permalink raw reply
* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Linus Walleij @ 2016-10-25 9:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477040798.15560.96.camel@baylibre.com>
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.
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.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] i2c: rk3x: Give the tuning value 0 during rk3x_i2c_v0_calc_timings
From: Wolfram Sang @ 2016-10-25 9:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477125822-30644-1-git-send-email-david.wu@rock-chips.com>
On Sat, Oct 22, 2016 at 04:43:42PM +0800, David Wu wrote:
> We found a bug that i2c transfer sometimes failed on 3066a board with
> stabel-4.8, the con register would be updated by uninitialized tuning
> value, it made the i2c transfer failed.
>
> So give the tuning value to be zero during rk3x_i2c_v0_calc_timings.
>
> Signed-off-by: David Wu <david.wu@rock-chips.com>
Added stable and applied to for-current, thanks!
-------------- 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/20161025/0184c0eb/attachment.sig>
^ 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