* [linux-pm Query] Power Management Device Suspend/ Resume
From: Viresh Kumar @ 2011-09-28 6:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1317181365.1573.1636.camel@vkoul-udesk3>
Vinod,
Thanks for mailing us back.
On 9/28/2011 9:12 AM, Vinod Koul wrote:
> On Wed, 2011-09-28 at 08:55 +0530, Viresh Kumar wrote:
>> On 9/22/2011 4:19 PM, Deepak Sikri wrote:
> are you trying to do runtime power management or traditional suspend and
> resume? IMO both should be done...
We will first be testing traditional suspend/resume, but obviously we would
try to design drivers in order to support runtime PM too.
>>> *Query- 1*. Suppose my platform has a ADC driver under char framework, which internally uses the
>>> DMA driver (channels). Now, ADC driver can be used by other kernel drivers or directly from user application.
>>>
>>> *1. a.* What are the expectations from the suspend and resume routines of both the devices, DMA & ADC ?
>>>
>>> */-- Given that few of the options are/*
>>> */1.a.1. On suspend,/*
>>> In ADC suspend function: ADC releases all the DMA channels, latches its registers; and
>>> DMA suspend function: DMA does nothing except for lataching of registers if required.
>>>
>>> */1.a.2. On suspend, /*
>>> ADC suspend function: ADC just latches its registers and stops R/W through DMA, no DMA channel release;
>>> DMA suspend function: DMA halts its operating channels, and latches its registers if required.
> Well channel release/alloc is not really required. You can goto suspend
> in between as well. if you have queued but not active descriptors on all
> channels then dmac is idle and you can use this time to goto suspend.
But the time suspend is called for DMA, we may in middle of DMA transfers
on few channels. So should we wait for them to finish or simply disable
channel transfer? And in case of resume what will happen to transfers that
were active during suspend?
>>> *1.b* Will the suspend resume for the dependent drivers follow sequencing, i.e. Suspend of ADC followed by
>>> Suspend of DMA? (Assume both of these devices are hooked on to Platform bus, given that ADC uses DMA)
> why do you care?
Suppose suspend of DMA occurs first and we disable its clock in the suspend
routine. Now before the suspend of ADC is called, ADC requests for DMA
transfers and because not every routine of DMA keeps track of clk is on/off,
it may try to access DMA register.
So, we must ensure that all children are suspended before the parent is
requested to suspend. And probably this is the correct way.
So, here we need ADC to suspend before DMA. How is this sequencing ensured?
>>> 2.1 The user space threads freeze first followed by kernel space. In case the user space process issues a a system
>>> call (lets assume an ioctl system call), how will the user space thread respond in case of suspend to ram?
> all user space processes are frozen on STR
Yes, but what will be its state. Will the system calls complete? Will it
close all devices opened by it before freezing?
Suppose a user applications is accessing few ADC channels. ADC controller
and its channel are all configured. Now, suspend is requested. What should
ADC driver do? As power will be turned off, its register will loose their
value.
Should ADC reconfigure all its channel registers by keeping their state?
Also it is not very straight forward to reconfigure channels directly, as
registers are required to be programmed in certain sequence, so there
needs to be good amount of intelligence put into driver for that.
IIRC, i couldn't find any such stuff in other mainline drivers. And that's
why we are confused on this topic.
--
viresh
^ permalink raw reply
* [PATCH] dma/imx-sdma+imx-dma: explicitly #include <linux/module.h>
From: Uwe Kleine-König @ 2011-09-28 6:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E824821.8040105@windriver.com>
[Cc: += linux-kbuild at v.k.o + Michal Marek]
Hello,
On Tue, Sep 27, 2011 at 06:03:13PM -0400, Paul Gortmaker wrote:
> On 11-09-26 02:26 AM, Uwe Kleine-K?nig wrote:
> > This is needed after commit
> >
> > include: replace linux/module.h with "struct module" wherever possible
> >
> > (currently 25215aa in next).
> >
> > Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> >
> > maybe it's sensible for Paul to take that before his commit?
>
> Thanks, I'll blend it into the commit adding module.h to the other
> drivers/dma files. It seems the imx boards don't have a defconfig
> in the arch/arm/configs that enables this (CONFIG_IMX_[S]DMA).
>
> What is really interesting (at least to me anyway) is that I
> didn't get one iMX build in some 6000+ ARM randconfig builds....
>
> I would have thought it would have showed up at least a couple
> times, given that number of builds. Maybe randconfig doesn't
> deal so well with "choice" Kconfig items?
>
> ~/git/module.h/linux-2.6.git$ ls -1 ../build-10*arm |wc -l
> 6399
> ~/git/module.h/linux-2.6.git$ grep CONFIG_ARCH_[A-Z0-9]*=y ../build-10*arm*/.config | sed 's/.*CON/CON/'|sort |uniq
> CONFIG_ARCH_EBSA110=y
> CONFIG_ARCH_H7201=y
> CONFIG_ARCH_H7202=y
> CONFIG_ARCH_H720X=y
> CONFIG_ARCH_VERSATILE=y
I don't understand why you did that sed, but AFAICT it doesn't hurt,
too. Maybe adding -c to uniq in your command line is interesting for the
kbuild people?!
> ~/git/module.h/linux-2.6.git$
>
> No ARCH_MXC, no ARCH_MXS, PXA, or any of the other 20+ variants.
> Definitely not a random spread there, with all 6000 builds falling
> in just 5 buckets.
How did you test? Generate a .config, test without your change and if
that succeeds test with it? If so, maybe the problem isn't that
randconfig doesn't generate more random configs but that you got many
failures in the first run?!
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH v2 2/6] OMAP4: Clock: round_rate and recalc functions for DPLL_ABE
From: Paul Walmsley @ 2011-09-28 6:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316195263-14951-1-git-send-email-jon-hunter@ti.com>
Hi Jon and Mike,
On Fri, 16 Sep 2011, Jon Hunter wrote:
> From: Mike Turquette <mturquette@ti.com>
>
> OMAP4 DPLL_ABE can enable a 4X multipler on top of the normal MN multipler
> and divider. This is achieved by setting CM_CLKMODE_DPLL_ABE.DPLL_REGM4XEN
> bit in CKGEN module of CM1. From the OMAP4 TRM:
>
> Fdpll = Fref x 2 x (4 x M/(N+1)) in case REGM4XEN bit field is set (only
> applicable to DPLL_ABE).
>
> Add new round_rate() and recalc() functions for OMAP4, that check the
> setting of REGM4XEN bit and handle this appropriately. The new functions
> are a simple wrapper on top of the existing omap2_dpll_round_rate() and
> omap2_dpll_get_rate() functions to handle the REGM4XEN bit.
>
> The REGM4XEN bit is only implemented for the ABE DPLL on OMAP4 and so
> only dpll_abe_ck uses omap4_dpll_regm4xen_round_rate() and
> omap4_dpll_regm4xen_recalc() functions.
>
> Signed-off-by: Mike Turquette <mturquette@ti.com>
> Tested-by: Jon Hunter <jon-hunter@ti.com>
Some changes have been made to this patch here, to fix a few minor bugs in
error paths and to add documentation and Jon's Signed-off-by: (since he's
in the submittal chain).
Care to review and send any comments? Otherwise, I plan to queue this
revised version for 3.2.
Thanks for the great changelogs on this, and the other patches in this
series.
regards
- Paul
From: Mike Turquette <mturquette@ti.com>
Date: Wed, 28 Sep 2011 00:00:31 -0600
Subject: [PATCH] ARM: OMAP4: clock: round_rate and recalc functions for
DPLL_ABE
OMAP4 DPLL_ABE can enable a 4X multipler on top of the normal MN multipler
and divider. This is achieved by setting CM_CLKMODE_DPLL_ABE.DPLL_REGM4XEN
bit in CKGEN module of CM1. From the OMAP4 TRM:
Fdpll = Fref x 2 x (4 x M/(N+1)) in case REGM4XEN bit field is set (only
applicable to DPLL_ABE).
Add new round_rate() and recalc() functions for OMAP4, that check the
setting of REGM4XEN bit and handle this appropriately. The new functions
are a simple wrapper on top of the existing omap2_dpll_round_rate() and
omap2_dpll_get_rate() functions to handle the REGM4XEN bit.
The REGM4XEN bit is only implemented for the ABE DPLL on OMAP4 and so
only dpll_abe_ck uses omap4_dpll_regm4xen_round_rate() and
omap4_dpll_regm4xen_recalc() functions.
Signed-off-by: Mike Turquette <mturquette@ti.com>
Tested-by: Jon Hunter <jon-hunter@ti.com>
Signed-off-by: Jon Hunter <jon-hunter@ti.com>
[paul at pwsan.com: fixed attempt to return a negative from a fn returning
unsigned; pass along errors from omap2_dpll_round_rate();
added documentation; added Jon's S-o-b]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
arch/arm/mach-omap2/clock.h | 2 +
arch/arm/mach-omap2/clock44xx.h | 7 +++
arch/arm/mach-omap2/clock44xx_data.c | 4 +-
arch/arm/mach-omap2/dpll44xx.c | 69 ++++++++++++++++++++++++++++++++++
4 files changed, 80 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/clock.h b/arch/arm/mach-omap2/clock.h
index 48ac568..2311bc2 100644
--- a/arch/arm/mach-omap2/clock.h
+++ b/arch/arm/mach-omap2/clock.h
@@ -66,6 +66,8 @@ void omap3_noncore_dpll_disable(struct clk *clk);
int omap4_dpllmx_gatectrl_read(struct clk *clk);
void omap4_dpllmx_allow_gatectrl(struct clk *clk);
void omap4_dpllmx_deny_gatectrl(struct clk *clk);
+long omap4_dpll_regm4xen_round_rate(struct clk *clk, unsigned long target_rate);
+unsigned long omap4_dpll_regm4xen_recalc(struct clk *clk);
#ifdef CONFIG_OMAP_RESET_CLOCKS
void omap2_clk_disable_unused(struct clk *clk);
diff --git a/arch/arm/mach-omap2/clock44xx.h b/arch/arm/mach-omap2/clock44xx.h
index 7ceb870..287a46f 100644
--- a/arch/arm/mach-omap2/clock44xx.h
+++ b/arch/arm/mach-omap2/clock44xx.h
@@ -8,6 +8,13 @@
#ifndef __ARCH_ARM_MACH_OMAP2_CLOCK44XX_H
#define __ARCH_ARM_MACH_OMAP2_CLOCK44XX_H
+/*
+ * OMAP4430_REGM4XEN_MULT: If the CM_CLKMODE_DPLL_ABE.DPLL_REGM4XEN bit is
+ * set, then the DPLL's lock frequency is multiplied by 4 (OMAP4430 TRM
+ * vV Section 3.6.3.3.1 "DPLLs Output Clocks Parameters")
+ */
+#define OMAP4430_REGM4XEN_MULT 4
+
int omap4xxx_clk_init(void);
#endif
diff --git a/arch/arm/mach-omap2/clock44xx_data.c b/arch/arm/mach-omap2/clock44xx_data.c
index c0b6fbd..c98c0a2 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -270,8 +270,8 @@ static struct clk dpll_abe_ck = {
.dpll_data = &dpll_abe_dd,
.init = &omap2_init_dpll_parent,
.ops = &clkops_omap3_noncore_dpll_ops,
- .recalc = &omap3_dpll_recalc,
- .round_rate = &omap2_dpll_round_rate,
+ .recalc = &omap4_dpll_regm4xen_recalc,
+ .round_rate = &omap4_dpll_regm4xen_round_rate,
.set_rate = &omap3_noncore_dpll_set_rate,
};
diff --git a/arch/arm/mach-omap2/dpll44xx.c b/arch/arm/mach-omap2/dpll44xx.c
index 4e4da61..9c6a296 100644
--- a/arch/arm/mach-omap2/dpll44xx.c
+++ b/arch/arm/mach-omap2/dpll44xx.c
@@ -19,6 +19,7 @@
#include <plat/clock.h>
#include "clock.h"
+#include "clock44xx.h"
#include "cm-regbits-44xx.h"
/* Supported only on OMAP4 */
@@ -82,3 +83,71 @@ const struct clkops clkops_omap4_dpllmx_ops = {
.deny_idle = omap4_dpllmx_deny_gatectrl,
};
+/**
+ * omap4_dpll_regm4xen_recalc - compute DPLL rate, considering REGM4XEN bit
+ * @clk: struct clk * of the DPLL to compute the rate for
+ *
+ * Compute the output rate for the OMAP4 DPLL represented by @clk.
+ * Takes the REGM4XEN bit into consideration, which is needed for the
+ * OMAP4 ABE DPLL. Returns the DPLL's output rate (before M-dividers)
+ * upon success, or 0 upon error.
+ */
+unsigned long omap4_dpll_regm4xen_recalc(struct clk *clk)
+{
+ u32 v;
+ unsigned long rate;
+ struct dpll_data *dd;
+
+ if (!clk || !clk->dpll_data)
+ return 0;
+
+ dd = clk->dpll_data;
+
+ rate = omap2_get_dpll_rate(clk);
+
+ /* regm4xen adds a multiplier of 4 to DPLL calculations */
+ v = __raw_readl(dd->control_reg);
+ if (v & OMAP4430_DPLL_REGM4XEN_MASK)
+ rate *= OMAP4430_REGM4XEN_MULT;
+
+ return rate;
+}
+
+/**
+ * omap4_dpll_regm4xen_round_rate - round DPLL rate, considering REGM4XEN bit
+ * @clk: struct clk * of the DPLL to round a rate for
+ * @target_rate: the desired rate of the DPLL
+ *
+ * Compute the rate that would be programmed into the DPLL hardware
+ * for @clk if set_rate() were to be provided with the rate
+ * @target_rate. Takes the REGM4XEN bit into consideration, which is
+ * needed for the OMAP4 ABE DPLL. Returns the rounded rate (before
+ * M-dividers) upon success, -EINVAL if @clk is null or not a DPLL, or
+ * ~0 if an error occurred in omap2_dpll_round_rate().
+ */
+long omap4_dpll_regm4xen_round_rate(struct clk *clk, unsigned long target_rate)
+{
+ u32 v;
+ struct dpll_data *dd;
+ long r;
+
+ if (!clk || !clk->dpll_data)
+ return -EINVAL;
+
+ dd = clk->dpll_data;
+
+ /* regm4xen adds a multiplier of 4 to DPLL calculations */
+ v = __raw_readl(dd->control_reg) & OMAP4430_DPLL_REGM4XEN_MASK;
+
+ if (v)
+ target_rate = target_rate / OMAP4430_REGM4XEN_MULT;
+
+ r = omap2_dpll_round_rate(clk, target_rate);
+ if (r == ~0)
+ return r;
+
+ if (v)
+ clk->dpll_data->last_rounded_rate *= OMAP4430_REGM4XEN_MULT;
+
+ return clk->dpll_data->last_rounded_rate;
+}
--
1.7.6.3
^ permalink raw reply related
* [PATCH v3 3/6] OMAP3+: use DPLL's round_rate when setting rate
From: Paul Walmsley @ 2011-09-28 7:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316195281-14988-1-git-send-email-jon-hunter@ti.com>
Hi,
On Fri, 16 Sep 2011, Jon Hunter wrote:
> From: Mike Turquette <mturquette@ti.com>
>
> omap3_noncore_dpll_set_rate uses omap2_dpll_round_rate explicitly. Instead
> use the struct clk pointer's round_rate function to allow for DPLL's with
> special needs.
>
> Also the rounded rate can differ from target rate, so to better reflect
> reality set clk->rate equal to the rounded rate when setting DPLL frequency.
> This avoids issues where the DPLL frequency is slightly different than what
> debugfs clock tree reports using the old target rate.
>
> An example of both of these needs is DPLL_ABE on OMAP4 which can have a 4x
> multiplier on top of the usual MN dividers depending on register settings.
> This requires a special round_rate function that might yield a rate
> different from the initial target.
>
> Signed-off-by: Mike Turquette <mturquette@ti.com>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
The two separate changes in this patch have been separated out into two
patches - both included below. Please let me know if you have any
comments; otherwise, I'll queue for 3.2.
- Paul
From: Mike Turquette <mturquette@ti.com>
Date: Wed, 28 Sep 2011 00:00:32 -0600
Subject: [PATCH] ARM: OMAP3+: dpll: use DPLL's round_rate when setting rate
omap3_noncore_dpll_set_rate uses omap2_dpll_round_rate explicitly. Instead
use the struct clk pointer's round_rate function to allow for DPLL's with
special needs.
An example of a clock that requires this is DPLL_ABE on OMAP4 which
can have a 4x multiplier on top of the usual MN dividers depending on
register settings. This requires a special round_rate function that
might yield a rate different from the initial target.
Signed-off-by: Mike Turquette <mturquette@ti.com>
Signed-off-by: Jon Hunter <jon-hunter@ti.com>
[paul at pwsan.com: split rate assignment portion into a separate patch]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
arch/arm/mach-omap2/dpll3xxx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/dpll3xxx.c b/arch/arm/mach-omap2/dpll3xxx.c
index f77022b..6b0fa37 100644
--- a/arch/arm/mach-omap2/dpll3xxx.c
+++ b/arch/arm/mach-omap2/dpll3xxx.c
@@ -455,7 +455,7 @@ int omap3_noncore_dpll_set_rate(struct clk *clk, unsigned long rate)
new_parent = dd->clk_bypass;
} else {
if (dd->last_rounded_rate != rate)
- omap2_dpll_round_rate(clk, rate);
+ clk->round_rate(clk, rate);
if (dd->last_rounded_rate == 0)
return -EINVAL;
--
1.7.6.3
--------------------
From: Mike Turquette <mturquette@ti.com>
Date: Wed, 28 Sep 2011 00:00:32 -0600
Subject: [PATCH] ARM: OMAP3+: dpll: assign clk rate from rounded rate during
rate set
The rounded rate can differ from target rate, so to better reflect
reality set clk->rate equal to the rounded rate when setting DPLL frequency.
This avoids issues where the DPLL frequency is slightly different than what
debugfs clock tree reports using the old target rate.
An example of a clock that requires this is DPLL_ABE on OMAP4 which
can have a 4x multiplier on top of the usual MN dividers depending on
register settings. This requires a special round_rate function that
might yield a rate different from the initial target.
Signed-off-by: Mike Turquette <mturquette@ti.com>
Signed-off-by: Jon Hunter <jon-hunter@ti.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
arch/arm/mach-omap2/dpll3xxx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/dpll3xxx.c b/arch/arm/mach-omap2/dpll3xxx.c
index 6b0fa37..73a1595 100644
--- a/arch/arm/mach-omap2/dpll3xxx.c
+++ b/arch/arm/mach-omap2/dpll3xxx.c
@@ -455,7 +455,7 @@ int omap3_noncore_dpll_set_rate(struct clk *clk, unsigned long rate)
new_parent = dd->clk_bypass;
} else {
if (dd->last_rounded_rate != rate)
- clk->round_rate(clk, rate);
+ rate = clk->round_rate(clk, rate);
if (dd->last_rounded_rate == 0)
return -EINVAL;
--
1.7.6.3
^ permalink raw reply related
* [PATCH v2 4/6] OMAP3+: use DPLLs recalc function instead of omap2_get_dpll_rate
From: Paul Walmsley @ 2011-09-28 7:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316195301-15025-1-git-send-email-jon-hunter@ti.com>
On Fri, 16 Sep 2011, Jon Hunter wrote:
> From: Jon Hunter <jon-hunter@ti.com>
>
> This is a continuation of Mike Turquette's patch "OMAP3+: use
> DPLL's round_rate when setting rate".
>
> omap3_noncore_dpll_set_rate() and omap3_noncore_dpll_enable() call
> omap2_get_dpll_rate() explicitly. It may be necessary for some
> DPLLs to use a different function and so use the DPLLs recalc()
> function pointer instead.
>
> An example is the DPLL_ABE on OMAP4 which can have a 4X multiplier
> in addition to the usual MN multipler and dividers and therefore
> uses a different round_rate and recalc function.
>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
Thanks, queued for 3.2.
- Paul
^ permalink raw reply
* [PATCH] dma/imx-sdma+imx-dma: explicitly #include <linux/module.h>
From: Arnaud Lacombe @ 2011-09-28 7:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110928064614.GN20550@pengutronix.de>
Hi,
2011/9/28 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> [Cc: += linux-kbuild at v.k.o + Michal Marek]
>
> Hello,
>
> On Tue, Sep 27, 2011 at 06:03:13PM -0400, Paul Gortmaker wrote:
>> On 11-09-26 02:26 AM, Uwe Kleine-K?nig wrote:
>> > This is needed after commit
>> >
>> > ? ? include: replace linux/module.h with "struct module" wherever possible
>> >
>> > (currently 25215aa in next).
>> >
>> > Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
>> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
>> > ---
>> > Hello,
>> >
>> > maybe it's sensible for Paul to take that before his commit?
>>
>> Thanks, I'll blend it into the commit adding module.h to the other
>> drivers/dma files. ?It seems the imx boards don't have a defconfig
>> in the arch/arm/configs that enables this (CONFIG_IMX_[S]DMA).
>>
>> What is really interesting (at least to me anyway) is that I
>> didn't get one iMX build in some 6000+ ARM randconfig builds....
>>
>> I would have thought it would have showed up at least a couple
>> times, given that number of builds. ?Maybe randconfig doesn't
>> deal so well with "choice" Kconfig items?
>>
>> ~/git/module.h/linux-2.6.git$ ls -1 ../build-10*arm |wc -l
>> 6399
>> ~/git/module.h/linux-2.6.git$ grep CONFIG_ARCH_[A-Z0-9]*=y ../build-10*arm*/.config | sed 's/.*CON/CON/'|sort |uniq
>> CONFIG_ARCH_EBSA110=y
>> CONFIG_ARCH_H7201=y
>> CONFIG_ARCH_H7202=y
>> CONFIG_ARCH_H720X=y
>> CONFIG_ARCH_VERSATILE=y
>
> I don't understand why you did that sed, but AFAICT it doesn't hurt,
> too. Maybe adding -c to uniq in your command line is interesting for the
> kbuild people?!
>
this can be reproduced trivially:
for i in $(seq 1 100); do
make ARCH=arm randconfig
sed -e '/CONFIG_ARCH_INTEGRATOR/,/CONFIG_ARCH_ZYNQ/!d;' \
-e '/=y/!d' .config >> arch_selected
done
sort arch_selected | uniq -c
I may have a trivial explanation for part of the symbol, which depends
on MMU, so if these symbols are selected, but MMU is not, it will
fall-back on the default, VERSATILE, which should happen 50% of the
time. However, there is plenty of symbols which have no specific
dependency, and are still not selected.
On the other side, all the result of a randconfig, on the choice alone
(ie. without any 'select' and any other symbols) is spread
"correctly".
Taken as a whole:
% cat arch_selected | sort | uniq -c | wc -l
3
% cat arch_selected | sort | uniq -c
2 CONFIG_ARCH_EBSA110=y
4 CONFIG_ARCH_H720X=y
100 CONFIG_ARCH_VERSATILE=y
However, when the choice is alone:
% cat arch_selected | sort | uniq -c | wc -l
39
>> ~/git/module.h/linux-2.6.git$
>>
>> No ARCH_MXC, no ARCH_MXS, PXA, or any of the other 20+ variants.
>> Definitely not a random spread there, with all 6000 builds falling
>> in just 5 buckets.
> How did you test? Generate a .config, test without your change and if
> that succeeds test with it? If so, maybe the problem isn't that
> randconfig doesn't generate more random configs but that you got many
> failures in the first run?!
>
it would seem that taken globally, the result of this particular
choice is not random, but I'm not sure of the dynamic involved
precisely.
I'm not a huge fan of the way randconfig works presently, that it by
just iterating over all symbols and setting a value . I'd prefer to
keep traversing all the menu, picking a random value for
user-changeable setting, over and over until all symbol have been
chosen. However, this is a whole lots of change...
- Arnaud
> Best regards
> Uwe
>
> --
> Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? | Uwe Kleine-K?nig ? ? ? ? ? ?|
> Industrial Linux Solutions ? ? ? ? ? ? ? ? | http://www.pengutronix.de/ ?|
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>
^ permalink raw reply
* [PATCH V9 0/5] imx53 ahci driver v9
From: Richard Zhu @ 2011-09-28 7:41 UTC (permalink / raw)
To: linux-arm-kernel
*** BLURB HERE ***
This is the seventh iteration of the imx53 ahci patch. Changes between
v5 and v7 are described in the following description.
ChangeLog v5->v6:
- Move the most ahci initialization codes from the board related files
to the common ahci_sata file.
- Add the imx53 ahci's own ata_port_info definition in ahci_platform
driver, since the 'ahci_pmp_retry_srst_ops' is required when imx53
ahci is present.
V6: http://www.spinics.net/lists/arm-kernel/msg138254.html
ChangeLog v6->v7:
- Define the default imx53 ahci platform data, reduce the modificatons
on the board related files.
- Handle the sata_pwr_en pin obviously on smd board.
V7: http://www.spinics.net/lists/arm-kernel/msg138569.html
ChangeLog V7->V8
- Accept Sascha's suggestions.
* Rename sata_init/sata_exit to imx_sata_init/imx_sata_exit.
* move the init/exit as static functions to platform-ahci-imx.c file.
Make a wrapper on these functions, and avoid to pass the function
hooks by platform data.
V8: http://www.spinics.net/lists/arm-kernel/msg141500.html
ChangeLog V8->V9
- Use gpio_request_one to replace the gpio_request and gpio_direction_output
when enable the sata power on mx53 smd board.
Richard Zhu (5):
AHCI Add the AHCI SATA feature on the MX53 platforms
ahci_plt Add the board_ids and pi refer to different features
MX53 Enable the AHCI SATA on MX53 ARD board
MX53 Enable the AHCI SATA on MX53 LOCO board
MX53 Enable the AHCI SATA on MX53 SMD board
arch/arm/mach-mx5/board-mx53_ard.c | 1 +
arch/arm/mach-mx5/board-mx53_loco.c | 1 +
arch/arm/mach-mx5/board-mx53_smd.c | 16 +++
arch/arm/mach-mx5/clock-mx51-mx53.c | 19 +++
arch/arm/mach-mx5/devices-imx53.h | 2 +
arch/arm/plat-mxc/devices/Kconfig | 4 +
arch/arm/plat-mxc/devices/Makefile | 1 +
arch/arm/plat-mxc/devices/platform-ahci-imx.c | 156 +++++++++++++++++++++++
arch/arm/plat-mxc/include/mach/devices-common.h | 10 ++
drivers/ata/ahci_platform.c | 44 ++++++-
10 files changed, 248 insertions(+), 6 deletions(-)
create mode 100644 arch/arm/plat-mxc/devices/platform-ahci-imx.c
^ permalink raw reply
* [PATCH V9 1/5] AHCI Add the AHCI SATA feature on the MX53 platforms
From: Richard Zhu @ 2011-09-28 7:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1317195717-21174-1-git-send-email-richard.zhu@linaro.org>
Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
Tested-by: Hector Oron Martinez <hector.oron@gmail.com>
---
arch/arm/mach-mx5/clock-mx51-mx53.c | 19 +++
arch/arm/mach-mx5/devices-imx53.h | 2 +
arch/arm/plat-mxc/devices/Kconfig | 4 +
arch/arm/plat-mxc/devices/Makefile | 1 +
arch/arm/plat-mxc/devices/platform-ahci-imx.c | 156 +++++++++++++++++++++++
arch/arm/plat-mxc/include/mach/devices-common.h | 10 ++
6 files changed, 192 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/plat-mxc/devices/platform-ahci-imx.c
diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 7f20308..e1fadaf 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -1397,6 +1397,22 @@ static struct clk esdhc4_mx53_clk = {
.secondary = &esdhc4_ipg_clk,
};
+static struct clk sata_clk = {
+ .parent = &ipg_clk,
+ .enable = _clk_max_enable,
+ .enable_reg = MXC_CCM_CCGR4,
+ .enable_shift = MXC_CCM_CCGRx_CG1_OFFSET,
+ .disable = _clk_max_disable,
+};
+
+static struct clk ahci_phy_clk = {
+ .parent = &usb_phy1_clk,
+};
+
+static struct clk ahci_dma_clk = {
+ .parent = &ahb_clk,
+};
+
DEFINE_CLOCK(mipi_esc_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG5_OFFSET, NULL, NULL, NULL, &pll2_sw_clk);
DEFINE_CLOCK(mipi_hsc2_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG4_OFFSET, NULL, NULL, &mipi_esc_clk, &pll2_sw_clk);
DEFINE_CLOCK(mipi_hsc1_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG3_OFFSET, NULL, NULL, &mipi_hsc2_clk, &pll2_sw_clk);
@@ -1503,6 +1519,9 @@ static struct clk_lookup mx53_lookups[] = {
_REGISTER_CLOCK("imx-ssi.1", NULL, ssi2_clk)
_REGISTER_CLOCK("imx-ssi.2", NULL, ssi3_clk)
_REGISTER_CLOCK("imx-keypad", NULL, dummy_clk)
+ _REGISTER_CLOCK("imx53-ahci.0", "ahci", sata_clk)
+ _REGISTER_CLOCK("imx53-ahci.0", "ahci_phy", ahci_phy_clk)
+ _REGISTER_CLOCK("imx53-ahci.0", "ahci_dma", ahci_dma_clk)
};
static void clk_tree_init(void)
diff --git a/arch/arm/mach-mx5/devices-imx53.h b/arch/arm/mach-mx5/devices-imx53.h
index c27fe8b..1ab399e 100644
--- a/arch/arm/mach-mx5/devices-imx53.h
+++ b/arch/arm/mach-mx5/devices-imx53.h
@@ -40,3 +40,5 @@ extern const struct imx_imx_ssi_data imx53_imx_ssi_data[];
extern const struct imx_imx_keypad_data imx53_imx_keypad_data;
#define imx53_add_imx_keypad(pdata) \
imx_add_imx_keypad(&imx53_imx_keypad_data, pdata)
+
+extern struct platform_device *__init imx53_add_ahci_imx(void);
diff --git a/arch/arm/plat-mxc/devices/Kconfig b/arch/arm/plat-mxc/devices/Kconfig
index bd294ad..f63887b 100644
--- a/arch/arm/plat-mxc/devices/Kconfig
+++ b/arch/arm/plat-mxc/devices/Kconfig
@@ -76,3 +76,7 @@ config IMX_HAVE_PLATFORM_SDHCI_ESDHC_IMX
config IMX_HAVE_PLATFORM_SPI_IMX
bool
+
+config IMX_HAVE_PLATFORM_AHCI
+ bool
+ default y if ARCH_MX53
diff --git a/arch/arm/plat-mxc/devices/Makefile b/arch/arm/plat-mxc/devices/Makefile
index b41bf97..e858ad9 100644
--- a/arch/arm/plat-mxc/devices/Makefile
+++ b/arch/arm/plat-mxc/devices/Makefile
@@ -25,3 +25,4 @@ obj-$(CONFIG_IMX_HAVE_PLATFORM_MXC_RTC) += platform-mxc_rtc.o
obj-$(CONFIG_IMX_HAVE_PLATFORM_MXC_W1) += platform-mxc_w1.o
obj-$(CONFIG_IMX_HAVE_PLATFORM_SDHCI_ESDHC_IMX) += platform-sdhci-esdhc-imx.o
obj-$(CONFIG_IMX_HAVE_PLATFORM_SPI_IMX) += platform-spi_imx.o
+obj-$(CONFIG_IMX_HAVE_PLATFORM_AHCI) += platform-ahci-imx.o
diff --git a/arch/arm/plat-mxc/devices/platform-ahci-imx.c b/arch/arm/plat-mxc/devices/platform-ahci-imx.c
new file mode 100644
index 0000000..d8a56ae
--- /dev/null
+++ b/arch/arm/plat-mxc/devices/platform-ahci-imx.c
@@ -0,0 +1,156 @@
+/*
+ * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * 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.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <asm/sizes.h>
+#include <mach/hardware.h>
+#include <mach/devices-common.h>
+
+#define imx_ahci_imx_data_entry_single(soc, _devid) \
+ { \
+ .devid = _devid, \
+ .iobase = soc ## _SATA_BASE_ADDR, \
+ .irq = soc ## _INT_SATA, \
+ }
+
+#ifdef CONFIG_SOC_IMX53
+const struct imx_ahci_imx_data imx53_ahci_imx_data __initconst =
+ imx_ahci_imx_data_entry_single(MX53, "imx53-ahci");
+#endif
+
+enum {
+ HOST_CAP = 0x00,
+ HOST_CAP_SSS = (1 << 27), /* Staggered Spin-up */
+ HOST_PORTS_IMPL = 0x0c,
+ HOST_TIMER1MS = 0xe0, /* Timer 1-ms */
+};
+
+static struct clk *sata_clk, *sata_ref_clk;
+
+/* AHCI module Initialization, if return 0, initialization is successful. */
+static int imx_sata_init(struct device *dev, void __iomem *addr)
+{
+ u32 tmpdata;
+ int ret = 0;
+ struct clk *clk;
+
+ sata_clk = clk_get(dev, "ahci");
+ if (IS_ERR(sata_clk)) {
+ dev_err(dev, "no sata clock.\n");
+ return PTR_ERR(sata_clk);
+ }
+ ret = clk_enable(sata_clk);
+ if (ret) {
+ dev_err(dev, "can't enable sata clock.\n");
+ goto put_sata_clk;
+ }
+
+ /* Get the AHCI SATA PHY CLK */
+ sata_ref_clk = clk_get(dev, "ahci_phy");
+ if (IS_ERR(sata_ref_clk)) {
+ dev_err(dev, "no sata ref clock.\n");
+ ret = PTR_ERR(sata_ref_clk);
+ goto release_sata_clk;
+ }
+ ret = clk_enable(sata_ref_clk);
+ if (ret) {
+ dev_err(dev, "can't enable sata ref clock.\n");
+ goto put_sata_ref_clk;
+ }
+
+ /* Get the AHB clock rate, and configure the TIMER1MS reg later */
+ clk = clk_get(dev, "ahci_dma");
+ if (IS_ERR(clk)) {
+ dev_err(dev, "no dma clock.\n");
+ ret = PTR_ERR(clk);
+ goto release_sata_ref_clk;
+ }
+ tmpdata = clk_get_rate(clk) / 1000;
+ clk_put(clk);
+
+ writel(tmpdata, addr + HOST_TIMER1MS);
+
+ tmpdata = readl(addr + HOST_CAP);
+ if (!(tmpdata & HOST_CAP_SSS)) {
+ tmpdata |= HOST_CAP_SSS;
+ writel(tmpdata, addr + HOST_CAP);
+ }
+
+ if (!(readl(addr + HOST_PORTS_IMPL) & 0x1))
+ writel((readl(addr + HOST_PORTS_IMPL) | 0x1),
+ addr + HOST_PORTS_IMPL);
+
+ return 0;
+
+release_sata_ref_clk:
+ clk_disable(sata_ref_clk);
+put_sata_ref_clk:
+ clk_put(sata_ref_clk);
+release_sata_clk:
+ clk_disable(sata_clk);
+put_sata_clk:
+ clk_put(sata_clk);
+
+ return ret;
+}
+
+static void imx_sata_exit(struct device *dev)
+{
+ clk_disable(sata_ref_clk);
+ clk_put(sata_ref_clk);
+
+ clk_disable(sata_clk);
+ clk_put(sata_clk);
+
+}
+struct platform_device *__init imx_add_ahci_imx(
+ const struct imx_ahci_imx_data *data,
+ const struct ahci_platform_data *pdata)
+{
+ struct resource res[] = {
+ {
+ .start = data->iobase,
+ .end = data->iobase + SZ_4K - 1,
+ .flags = IORESOURCE_MEM,
+ }, {
+ .start = data->irq,
+ .end = data->irq,
+ .flags = IORESOURCE_IRQ,
+ },
+ };
+
+ return imx_add_platform_device_dmamask(data->devid, 0,
+ res, ARRAY_SIZE(res),
+ pdata, sizeof(*pdata), DMA_BIT_MASK(32));
+}
+
+struct platform_device *__init imx53_add_ahci_imx(void)
+{
+ struct ahci_platform_data pdata = {
+ .init = imx_sata_init,
+ .exit = imx_sata_exit,
+ };
+
+ return imx_add_ahci_imx(&imx53_ahci_imx_data, &pdata);
+}
diff --git a/arch/arm/plat-mxc/include/mach/devices-common.h b/arch/arm/plat-mxc/include/mach/devices-common.h
index 524538a..f04e063 100644
--- a/arch/arm/plat-mxc/include/mach/devices-common.h
+++ b/arch/arm/plat-mxc/include/mach/devices-common.h
@@ -301,3 +301,13 @@ struct platform_device *__init imx_add_spi_imx(
struct platform_device *imx_add_imx_dma(void);
struct platform_device *imx_add_imx_sdma(char *name,
resource_size_t iobase, int irq, struct sdma_platform_data *pdata);
+
+#include <linux/ahci_platform.h>
+struct imx_ahci_imx_data {
+ const char *devid;
+ resource_size_t iobase;
+ resource_size_t irq;
+};
+struct platform_device *__init imx_add_ahci_imx(
+ const struct imx_ahci_imx_data *data,
+ const struct ahci_platform_data *pdata);
--
1.7.1
^ permalink raw reply related
* [PATCH V9 2/5] ahci_plt Add the board_ids and pi refer to different features
From: Richard Zhu @ 2011-09-28 7:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1317195717-21174-1-git-send-email-richard.zhu@linaro.org>
On imx53 AHCI, soft reset fails with IPMS set when PMP
is enabled but SATA HDD/ODD is connected to SATA port,
do soft reset again to port 0.
So the 'ahci_pmp_retry_srst_ops' is required when imx53
ahci is present.
Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
Acked-by: Eric Miao <eric.miao@linaro.org>
---
drivers/ata/ahci_platform.c | 44 +++++++++++++++++++++++++++++++++++++-----
1 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 6fef1fa..c03277d 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -23,6 +23,41 @@
#include <linux/ahci_platform.h>
#include "ahci.h"
+enum ahci_type {
+ AHCI, /* standard platform ahci */
+ IMX53_AHCI, /* ahci on i.mx53 */
+};
+
+static struct platform_device_id ahci_devtype[] = {
+ {
+ .name = "ahci",
+ .driver_data = AHCI,
+ }, {
+ .name = "imx53-ahci",
+ .driver_data = IMX53_AHCI,
+ }, {
+ /* sentinel */
+ }
+};
+MODULE_DEVICE_TABLE(platform, ahci_devtype);
+
+
+static const struct ata_port_info ahci_port_info[] = {
+ /* by features */
+ [AHCI] = {
+ .flags = AHCI_FLAG_COMMON,
+ .pio_mask = ATA_PIO4,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &ahci_ops,
+ },
+ [IMX53_AHCI] = {
+ .flags = AHCI_FLAG_COMMON,
+ .pio_mask = ATA_PIO4,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &ahci_pmp_retry_srst_ops,
+ },
+};
+
static struct scsi_host_template ahci_platform_sht = {
AHCI_SHT("ahci_platform"),
};
@@ -31,12 +66,8 @@ static int __init ahci_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct ahci_platform_data *pdata = dev->platform_data;
- struct ata_port_info pi = {
- .flags = AHCI_FLAG_COMMON,
- .pio_mask = ATA_PIO4,
- .udma_mask = ATA_UDMA6,
- .port_ops = &ahci_ops,
- };
+ const struct platform_device_id *id = platform_get_device_id(pdev);
+ struct ata_port_info pi = ahci_port_info[id->driver_data];
const struct ata_port_info *ppi[] = { &pi, NULL };
struct ahci_host_priv *hpriv;
struct ata_host *host;
@@ -177,6 +208,7 @@ static struct platform_driver ahci_driver = {
.name = "ahci",
.owner = THIS_MODULE,
},
+ .id_table = ahci_devtype,
};
static int __init ahci_init(void)
--
1.7.1
^ permalink raw reply related
* [PATCH V9 3/5] MX53 Enable the AHCI SATA on MX53 ARD board
From: Richard Zhu @ 2011-09-28 7:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1317195717-21174-1-git-send-email-richard.zhu@linaro.org>
Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
---
arch/arm/mach-mx5/board-mx53_ard.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mx5/board-mx53_ard.c b/arch/arm/mach-mx5/board-mx53_ard.c
index 76a67c4..ef2039e 100644
--- a/arch/arm/mach-mx5/board-mx53_ard.c
+++ b/arch/arm/mach-mx5/board-mx53_ard.c
@@ -234,6 +234,7 @@ static void __init mx53_ard_board_init(void)
imx53_add_imx_i2c(1, &mx53_ard_i2c2_data);
imx53_add_imx_i2c(2, &mx53_ard_i2c3_data);
imx_add_gpio_keys(&ard_button_data);
+ imx53_add_ahci_imx();
}
static void __init mx53_ard_timer_init(void)
--
1.7.1
^ permalink raw reply related
* [PATCH V9 4/5] MX53 Enable the AHCI SATA on MX53 LOCO board
From: Richard Zhu @ 2011-09-28 7:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1317195717-21174-1-git-send-email-richard.zhu@linaro.org>
Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
---
arch/arm/mach-mx5/board-mx53_loco.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-mx5/board-mx53_loco.c
index 4e1d51d..940aac9 100644
--- a/arch/arm/mach-mx5/board-mx53_loco.c
+++ b/arch/arm/mach-mx5/board-mx53_loco.c
@@ -273,6 +273,7 @@ static void __init mx53_loco_board_init(void)
imx53_add_sdhci_esdhc_imx(2, &mx53_loco_sd3_data);
imx_add_gpio_keys(&loco_button_data);
gpio_led_register_device(-1, &mx53loco_leds_data);
+ imx53_add_ahci_imx();
}
static void __init mx53_loco_timer_init(void)
--
1.7.1
^ permalink raw reply related
* [PATCH V9 5/5] MX53 Enable the AHCI SATA on MX53 SMD board
From: Richard Zhu @ 2011-09-28 7:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1317195717-21174-1-git-send-email-richard.zhu@linaro.org>
Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
---
arch/arm/mach-mx5/board-mx53_smd.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mx5/board-mx53_smd.c b/arch/arm/mach-mx5/board-mx53_smd.c
index bc02894..efcab68 100644
--- a/arch/arm/mach-mx5/board-mx53_smd.c
+++ b/arch/arm/mach-mx5/board-mx53_smd.c
@@ -35,6 +35,7 @@
#include "devices-imx53.h"
#define SMD_FEC_PHY_RST IMX_GPIO_NR(7, 6)
+#define MX53_SMD_SATA_PWR_EN IMX_GPIO_NR(3, 3)
static iomux_v3_cfg_t mx53_smd_pads[] = {
MX53_PAD_CSI0_DAT10__UART1_TXD_MUX,
@@ -111,6 +112,19 @@ static const struct imxi2c_platform_data mx53_smd_i2c_data __initconst = {
.bitrate = 100000,
};
+static inline void mx53_smd_ahci_pwr_on(void)
+{
+ int ret;
+
+ /* Enable SATA PWR */
+ ret = gpio_request_one(MX53_SMD_SATA_PWR_EN,
+ GPIOF_DIR_OUT | GPIOF_INIT_HIGH, "ahci-sata-pwr");
+ if (ret) {
+ pr_err("failed to enable SATA_PWR_EN: %d\n", ret);
+ return;
+ }
+}
+
static void __init mx53_smd_board_init(void)
{
imx53_soc_init();
@@ -125,6 +139,8 @@ static void __init mx53_smd_board_init(void)
imx53_add_sdhci_esdhc_imx(0, NULL);
imx53_add_sdhci_esdhc_imx(1, NULL);
imx53_add_sdhci_esdhc_imx(2, NULL);
+ mx53_smd_ahci_pwr_on();
+ imx53_add_ahci_imx();
}
static void __init mx53_smd_timer_init(void)
--
1.7.1
^ permalink raw reply related
* [PATCH v3 4/6] arm/imx6q: add smp and cpu hotplug support
From: Shawn Guo @ 2011-09-28 7:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110926110634.GR31404@pengutronix.de>
On Mon, Sep 26, 2011 at 01:06:34PM +0200, Sascha Hauer wrote:
> On Mon, Sep 26, 2011 at 03:20:49PM +0800, Shawn Guo wrote:
> > It adds smp and cpu hotplug support for imx6q.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> > arch/arm/mach-imx/Kconfig | 1 +
> > arch/arm/mach-imx/Makefile | 4 ++
> > arch/arm/mach-imx/head-v7.S | 71 ++++++++++++++++++++++++++
> > arch/arm/mach-imx/hotplug.c | 44 ++++++++++++++++
> > arch/arm/mach-imx/localtimer.c | 35 +++++++++++++
> > arch/arm/mach-imx/platsmp.c | 85 +++++++++++++++++++++++++++++++
> > arch/arm/plat-mxc/include/mach/common.h | 5 ++
> > 7 files changed, 245 insertions(+), 0 deletions(-)
> > create mode 100644 arch/arm/mach-imx/head-v7.S
> > create mode 100644 arch/arm/mach-imx/hotplug.c
> > create mode 100644 arch/arm/mach-imx/localtimer.c
> > create mode 100644 arch/arm/mach-imx/platsmp.c
> >
> > diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> > index af73b3e..6ec758d 100644
> > --- a/arch/arm/mach-imx/Kconfig
> > +++ b/arch/arm/mach-imx/Kconfig
> > @@ -623,6 +623,7 @@ config SOC_IMX6Q
> > bool "i.MX6 Quad support"
> > select ARM_GIC
> > select CPU_V7
> > + select HAVE_ARM_SCU
> > select HAVE_IMX_GPC
> > select HAVE_IMX_MMDC
> > select HAVE_IMX_SRC
> > diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> > index 8c21fda..d46b2e7 100644
> > --- a/arch/arm/mach-imx/Makefile
> > +++ b/arch/arm/mach-imx/Makefile
> > @@ -66,4 +66,8 @@ obj-$(CONFIG_DEBUG_LL) += lluart.o
> > obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
> > obj-$(CONFIG_HAVE_IMX_MMDC) += mmdc.o
> > obj-$(CONFIG_HAVE_IMX_SRC) += src.o
> > +obj-$(CONFIG_CPU_V7) += head-v7.o
>
> Can we have a
>
> AFLAGS_head-v7.o :=-Wa,-march=armv7-a
>
> here?
>
> Then you only have to adjust some Kconfig variables and can compile
> i.MX3 and i.MX6 together. I just gave it a test and it works on i.MX3, I
> don't have a i.MX6 platform to test on though.
>
> The following patch could be a starting point. I merged the imx-features
> branch into it so it probably won't apply cleanly on your series.
>
> 8<----------------------------------------
>
>
> From 010a61373355e6cba6856272e3fd8e22e8ee3ccd Mon Sep 17 00:00:00 2001
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Date: Mon, 26 Sep 2011 13:04:08 +0200
> Subject: [PATCH] merge i.MX3 and i.MX6
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> arch/arm/Makefile | 3 +--
> arch/arm/mach-imx/Kconfig | 7 +++----
> arch/arm/mach-imx/Makefile | 1 +
> arch/arm/mm/Kconfig | 4 ++--
> arch/arm/plat-mxc/Kconfig | 14 ++++++--------
> 5 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 89be85d..42ab385 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -155,9 +155,8 @@ machine-$(CONFIG_ARCH_MMP) := mmp
> machine-$(CONFIG_ARCH_MSM) := msm
> machine-$(CONFIG_ARCH_MV78XX0) := mv78xx0
> machine-$(CONFIG_ARCH_IMX_V4_V5) := imx
> -machine-$(CONFIG_ARCH_MX3) := imx
> +machine-$(CONFIG_ARCH_IMX_V6_V7) := imx
> machine-$(CONFIG_ARCH_MX5) := mx5
> -machine-$(CONFIG_ARCH_MX6) := imx
> machine-$(CONFIG_ARCH_MXS) := mxs
> machine-$(CONFIG_ARCH_NETX) := netx
> machine-$(CONFIG_ARCH_NOMADIK) := nomadik
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 3852f45..27355cb 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -74,6 +74,7 @@ config SOC_IMX31
> select ARCH_MXC_AUDMUX_V2
> select ARCH_MX31
> select MXC_AVIC
> + select SMP_ON_UP if SMP
>
> config SOC_IMX35
> bool
> @@ -83,6 +84,7 @@ config SOC_IMX35
> select HAVE_EPIT
> select ARCH_MX35
> select MXC_AVIC
> + select SMP_ON_UP if SMP
>
>
> if ARCH_IMX_V4_V5
> @@ -351,7 +353,7 @@ config MACH_IMX27IPCAM
>
> endif
>
> -if ARCH_MX3
> +if ARCH_IMX_V6_V7
>
> comment "MX31 platforms:"
>
> @@ -601,9 +603,6 @@ config MACH_VPR200
> Include support for VPR200 platform. This includes specific
> configurations for the board and its peripherals.
>
> -endif
> -
> -if ARCH_MX6
> comment "i.MX6 family:"
>
> config SOC_IMX6Q
> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> index aa1cf0c..b9dffe5 100644
> --- a/arch/arm/mach-imx/Makefile
> +++ b/arch/arm/mach-imx/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_DEBUG_LL) += lluart.o
> obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
> obj-$(CONFIG_HAVE_IMX_MMDC) += mmdc.o
> obj-$(CONFIG_HAVE_IMX_SRC) += src.o
> +AFLAGS_head-v7.o :=-Wa,-march=armv7-a
> obj-$(CONFIG_CPU_V7) += head-v7.o
> obj-$(CONFIG_SMP) += platsmp.o
> obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
> index c3ce146..9ab5be0 100644
> --- a/arch/arm/mm/Kconfig
> +++ b/arch/arm/mm/Kconfig
> @@ -819,10 +819,10 @@ config CACHE_FEROCEON_L2_WRITETHROUGH
> config CACHE_L2X0
> bool "Enable the L2x0 outer cache controller"
> depends on REALVIEW_EB_ARM11MP || MACH_REALVIEW_PB11MP || MACH_REALVIEW_PB1176 || \
> - REALVIEW_EB_A9MP || SOC_IMX35 || SOC_IMX31 || MACH_REALVIEW_PBX || \
> + REALVIEW_EB_A9MP || ARCH_IMX_V6_V7 || MACH_REALVIEW_PBX || \
> ARCH_NOMADIK || ARCH_OMAP4 || ARCH_EXYNOS4 || ARCH_TEGRA || \
> ARCH_U8500 || ARCH_VEXPRESS_CA9X4 || ARCH_SHMOBILE || \
> - ARCH_PRIMA2 || ARCH_ZYNQ || ARCH_CNS3XXX || ARCH_MX6
> + ARCH_PRIMA2 || ARCH_ZYNQ || ARCH_CNS3XXX
> default y
> select OUTER_CACHE
> select OUTER_CACHE_SYNC
> diff --git a/arch/arm/plat-mxc/Kconfig b/arch/arm/plat-mxc/Kconfig
> index 82b0d92..43efc3c 100644
> --- a/arch/arm/plat-mxc/Kconfig
> +++ b/arch/arm/plat-mxc/Kconfig
> @@ -16,10 +16,13 @@ config ARCH_IMX_V4_V5
> This enables support for systems based on the Freescale i.MX ARMv4
> and ARMv5 SoCs
>
> -config ARCH_MX3
> - bool "MX3-based"
Then the following ARCH_MX3 references need to be removed properly.
I will take care of them.
arch/arm/mach-imx/Makefile.boot:17:zreladdr-$(CONFIG_ARCH_MX3) += 0x80008000
arch/arm/mach-imx/Makefile.boot:18:params_phys-$(CONFIG_ARCH_MX3) := 0x80000100
arch/arm/mach-imx/Makefile.boot:19:initrd_phys-$(CONFIG_ARCH_MX3) := 0x80800000
arch/arm/plat-mxc/Kconfig:9: default ARCH_MX3
arch/arm/plat-mxc/include/mach/memory.h:25:# if defined CONFIG_ARCH_MX3
Regards,
Shawn
> +config ARCH_IMX_V6_V7
> + bool "i.MX3, i.MX6"
> + select AUTO_ZRELADDR
> + select ARM_PATCH_PHYS_VIRT
> help
> - This enables support for systems based on the Freescale i.MX3 family
> + This enables support for systems based on the Freescale i.MX3 and i.MX6
> + family.
>
> config ARCH_MX5
> bool "i.MX50, i.MX51, i.MX53"
> @@ -29,11 +32,6 @@ config ARCH_MX5
> This enables support for machines using Freescale's i.MX50 and i.MX51
> processors.
>
> -config ARCH_MX6
> - bool "i.MX6"
> - help
> - This enables support for systems based on the Freescale i.MX6 family
> -
> endchoice
>
> source "arch/arm/mach-imx/Kconfig"
> --
^ permalink raw reply
* [PATCH V2 1/1] dmaengine/amba-pl08x: Add support for s3c64xx DMAC
From: Viresh Kumar @ 2011-09-28 7:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1317189007-23033-2-git-send-email-alim.akhtar@samsung.com>
On 9/28/2011 11:20 AM, Alim Akhtar wrote:
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
> drivers/dma/amba-pl08x.c | 135 ++++++++++++++++++++++++++++++++++++++--------
> 1 files changed, 112 insertions(+), 23 deletions(-)
>
It would be good if you can add pick some part from cover-letter and put it in
commit log too.
> diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
> index cd8df7f..501540f 100644
> --- a/drivers/dma/amba-pl08x.c
> +++ b/drivers/dma/amba-pl08x.c
> @@ -110,6 +129,11 @@ struct pl08x_lli {
> u32 dst;
> u32 lli;
> u32 cctl;
> + /*
> + * Samsung pl080 DMAC has one exrta control register
> + * which is used to hold the transfer_size
> + */
> + u32 cctl1;
Will you write transfer_size in cctl also? What is the purpose of cctl1?
> @@ -215,11 +255,23 @@ static void pl08x_start_txd(struct pl08x_dma_chan *plchan,
> cpu_relax();
>
> /* Do not access config register until channel shows as inactive */
> - val = readl(phychan->base + PL080_CH_CONFIG);
> - while ((val & PL080_CONFIG_ACTIVE) || (val & PL080_CONFIG_ENABLE))
> + if (pl08x->vd->is_pl080_s3c) {
> + val = readl(phychan->base + PL080S_CH_CONFIG);
> + while ((val & PL080_CONFIG_ACTIVE) ||
> + (val & PL080_CONFIG_ENABLE))
> + val = readl(phychan->base + PL080S_CH_CONFIG);
> +
> + writel(val | PL080_CONFIG_ENABLE,
> + phychan->base + PL080S_CH_CONFIG);
> + } else {
> val = readl(phychan->base + PL080_CH_CONFIG);
> + while ((val & PL080_CONFIG_ACTIVE) ||
> + (val & PL080_CONFIG_ENABLE))
> + val = readl(phychan->base + PL080_CH_CONFIG);
>
> - writel(val | PL080_CONFIG_ENABLE, phychan->base + PL080_CH_CONFIG);
> + writel(val | PL080_CONFIG_ENABLE,
> + phychan->base + PL080_CH_CONFIG);
> + }
You have similar stuff in most the the changes in this patch, because offset of
config register changes for s3c, you placed in if,else block.
If you check these changes again, there is a lot of code duplication in this if,else
blocks. The only different thing in if,else is the offset of CH_CONFIG register.
It would be much better if you can do following:
u32 ch_cfg_off;
if (pl08x->vd->is_pl080_s3c)
ch_cfg_off = PL080S_CH_CONFIG;
else
ch_cfg_off = PL080_CH_CONFIG;
Now, this offset can be used in existing code, without much code duplication.
> @@ -569,6 +641,7 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
> u32 cctl, early_bytes = 0;
> size_t max_bytes_per_lli, total_bytes = 0;
> struct pl08x_lli *llis_va;
> + size_t lli_len = 0, target_len, tsize, odd_bytes;
>
> txd->llis_va = dma_pool_alloc(pl08x->pool, GFP_NOWAIT, &txd->llis_bus);
> if (!txd->llis_va) {
> @@ -700,7 +773,7 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
> * width left
> */
> while (bd.remainder > (mbus->buswidth - 1)) {
> - size_t lli_len, tsize, width;
> + size_t width;
>
why do we need above two changes. odd_bytes and target_len are still unused.
> /*
> * If enough left try to send max possible,
> @@ -759,6 +832,9 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
> llis_va[num_llis - 1].lli = 0;
> /* The final LLI element shall also fire an interrupt. */
> llis_va[num_llis - 1].cctl |= PL080_CONTROL_TC_IRQ_EN;
> + /* Keep the TransferSize seperate to fill samsung specific register */
> + if (pl08x->vd->is_pl080_s3c)
> + llis_va[num_llis - 1].cctl1 |= lli_len;
I couldn't get this completely. Why do you keep only length of
last lli in cctl1. what about all other llis. Also why |=
and not directly cctl1 = lli_len
--
viresh
^ permalink raw reply
* [PATCH V2 1/1] dmaengine/amba-pl08x: Add support for s3c64xx DMAC
From: Linus Walleij @ 2011-09-28 8:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1317189007-23033-2-git-send-email-alim.akhtar@samsung.com>
Sorry if I missed a few nitpicks last time, anyway it's looking much better now:
On Wed, Sep 28, 2011 at 7:50 AM, Alim Akhtar <alim.akhtar@samsung.com> wrote:
> + ? ? ? /*
> + ? ? ? ?* Samsung pl080 DMAC has one exrta control register
s/exrta/exstra
> + ? ? ? if (pl08x->vd->is_pl080_s3c) {
> + ? ? ? ? ? ? ? writel(txd->ccfg, phychan->base + PL080S_CH_CONFIG);
> + ? ? ? ? ? ? ? writel(lli->cctl1, phychan->base + PL080S_CH_CONTROL2);
> + ? ? ? } else
> + ? ? ? ? ? ? ? writel(txd->ccfg, phychan->base + PL080_CH_CONFIG);
What do you think about adding a field to the struct pl08x
like
u32 config_reg
that we set to the proper config register (PL080S_CH_CONFIG or
PL080_CH_CONFIG in probe(), so the above
becomes the simpler variant:
writel(txd->ccfg, phychan->base + pl08x->config_reg);
if (pl08x->vd->is_pl080_s3c)
writel(lli->cctl1, phychan->base + PL080S_CH_CONTROL2);
> + ? ? ? if (pl08x->vd->is_pl080_s3c) {
> + ? ? ? ? ? ? ? val = readl(phychan->base + PL080S_CH_CONFIG);
> + ? ? ? ? ? ? ? while ((val & PL080_CONFIG_ACTIVE) ||
> + ? ? ? ? ? ? ? ? ? ? ? (val & PL080_CONFIG_ENABLE))
> + ? ? ? ? ? ? ? ? ? ? ? val = readl(phychan->base + PL080S_CH_CONFIG);
> +
> + ? ? ? ? ? ? ? writel(val | PL080_CONFIG_ENABLE,
> + ? ? ? ? ? ? ? ? ? ? ? phychan->base + PL080S_CH_CONFIG);
> + ? ? ? } else {
> ? ? ? ? ? ? ? ?val = readl(phychan->base + PL080_CH_CONFIG);
> + ? ? ? ? ? ? ? ? ? ? ? while ((val & PL080_CONFIG_ACTIVE) ||
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (val & PL080_CONFIG_ENABLE))
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? val = readl(phychan->base + PL080_CH_CONFIG);
>
> - ? ? ? writel(val | PL080_CONFIG_ENABLE, phychan->base + PL080_CH_CONFIG);
> + ? ? ? ? ? ? ? writel(val | PL080_CONFIG_ENABLE,
> + ? ? ? ? ? ? ? ? ? ? ? phychan->base + PL080_CH_CONFIG);
> + ? ? ? }
This would also become much simpler with that approach I think...
> ? ? ? ?/* Set the HALT bit and wait for the FIFO to drain */
> - ? ? ? val = readl(ch->base + PL080_CH_CONFIG);
> - ? ? ? val |= PL080_CONFIG_HALT;
> - ? ? ? writel(val, ch->base + PL080_CH_CONFIG);
> -
> + ? ? ? if (pl08x->vd->is_pl080_s3c) {
> + ? ? ? ? ? ? ? val = readl(ch->base + PL080S_CH_CONFIG);
> + ? ? ? ? ? ? ? val |= PL080_CONFIG_HALT;
> + ? ? ? ? ? ? ? writel(val, ch->base + PL080S_CH_CONFIG);
> + ? ? ? } else {
> + ? ? ? ? ? ? ? val = readl(ch->base + PL080_CH_CONFIG);
> + ? ? ? ? ? ? ? val |= PL080_CONFIG_HALT;
> + ? ? ? ? ? ? ? writel(val, ch->base + PL080_CH_CONFIG);
> + ? ? ? }
This would become simply:
val = readl(ch->base + pl08x->config_reg);
val |= PL080_CONFIG_HALT;
writel(val, ch->base + pl08x->config_reg);
> ? ? ? ?/* Clear the HALT bit */
> - ? ? ? val = readl(ch->base + PL080_CH_CONFIG);
> - ? ? ? val &= ~PL080_CONFIG_HALT;
> - ? ? ? writel(val, ch->base + PL080_CH_CONFIG);
> + ? ? ? if (pl08x->vd->is_pl080_s3c) {
> + ? ? ? ? ? ? ? val = readl(ch->base + PL080S_CH_CONFIG);
> + ? ? ? ? ? ? ? val &= ~PL080_CONFIG_HALT;
> + ? ? ? ? ? ? ? writel(val, ch->base + PL080S_CH_CONFIG);
> + ? ? ? } else {
> + ? ? ? ? ? ? ? val = readl(ch->base + PL080_CH_CONFIG);
> + ? ? ? ? ? ? ? val &= ~PL080_CONFIG_HALT;
> + ? ? ? ? ? ? ? writel(val, ch->base + PL080_CH_CONFIG);
> + ? ? ? }
This would get rid of the if/else clauses
> + ? ? ? if (pl08x->vd->is_pl080_s3c) {
> + ? ? ? ? ? ? ? u32 val = readl(ch->base + PL080S_CH_CONFIG);
> + ? ? ? ? ? ? ? val &= ~(PL080_CONFIG_ENABLE | PL080_CONFIG_ERR_IRQ_MASK |
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? PL080_CONFIG_TC_IRQ_MASK);
> + ? ? ? ? ? ? ? writel(val, ch->base + PL080S_CH_CONFIG);
> + ? ? ? } else {
> + ? ? ? ? ? ? ? u32 val = readl(ch->base + PL080_CH_CONFIG);
> + ? ? ? ? ? ? ? val &= ~(PL080_CONFIG_ENABLE | PL080_CONFIG_ERR_IRQ_MASK |
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? PL080_CONFIG_TC_IRQ_MASK);
> + ? ? ? ? ? ? ? writel(val, ch->base + PL080_CH_CONFIG);
> + ? ? ? }
As would this...
> + ? ? ? /* Samsung DMAC is PL080 variant*/
> + ? ? ? {
> + ? ? ? ? ? ? ? .id ? ? = 0x00041082,
> + ? ? ? ? ? ? ? .mask ? = 0x000fffff,
> + ? ? ? ? ? ? ? .data ? = &vendor_pl080_s3c,
Does the hardware realy have this primecell number or is it something that is
hardcoded from the board/device tree?
If it is hardcoded then no objections.
In the latter case, replace 0x41 (= 'A', ARM) with something like
0x55 'U' for Samsung or so. Or 0x51 'S'. Or whatever you like.
Then add that to include/linux/amba/bus.h as
AMBA_VENDOR_SAMSUNG = 0x55,
so we have this under some kind of control.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 2/9] regulator: helper routine to extract regulator_init_data
From: Cousson, Benoit @ 2011-09-28 8:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110927150511.GM4289@opensource.wolfsonmicro.com>
On 9/27/2011 5:05 PM, Mark Brown wrote:
> On Tue, Sep 27, 2011 at 08:18:04PM +0530, Rajendra Nayak wrote:
>> On Tuesday 27 September 2011 05:40 PM, Mark Brown wrote:
>>> On Tue, Sep 27, 2011 at 03:42:45PM +0530, Rajendra Nayak wrote:
>
>>>> + init_data = devm_kzalloc(dev, sizeof(struct regulator_init_data),
>>>> + GFP_KERNEL);
>>>> + if (!init_data)
>>>> + return NULL; /* Out of memory? */
>
>>> This means that the init data will be kept around for the entire
>>> lifetime of the device rather than being discarded.
>
>> Wasn't it the same while this was passed around as platform_data?
>
> It was in the past but I remember fixing it at some point. Perhaps I'm
> imagining things.
>
>>>> + init_data->supply_regulator = (char *)of_get_property(dev->of_node,
>>>> + "regulator-supplies", NULL);
>
>>> I'd expect that in the device tree world the supply regulator would
>>> reference the node for that regulator.
>
>> You mean using phandles? Thats what Grant proposed too but
>> I thought you instead had an inclination towards names? Or maybe
>> I misunderstood.
>
> They need both. We need to reference the device that provides the
> supply and use a name to say which of the potentially multiple supplies
> on the consumer device is which.
>
>>> Hrm, I think loosing the signs here is bad karma - negative voltages do
>>> exist after all.
>
>> Oops.. they do? didn't know about that.
>
> Yup, ground is just a reference point.
Yep, we do have a negative charge pump to generate -1.9v from 3.8v to
supply the audio power amplifier part in twl6040 for example.
Benoit
^ permalink raw reply
* [PATCH 8/9] regulator: helper to extract regulator node based on supply name
From: Cousson, Benoit @ 2011-09-28 8:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110927185913.GU4289@opensource.wolfsonmicro.com>
On 9/27/2011 8:59 PM, Mark Brown wrote:
> On Tue, Sep 27, 2011 at 08:19:37PM +0530, Rajendra Nayak wrote:
>> On Tuesday 27 September 2011 05:51 PM, Mark Brown wrote:
>>> On Tue, Sep 27, 2011 at 03:42:51PM +0530, Rajendra Nayak wrote:
>
>>>> + if (!dev)
>>>> + return NULL;
>
>>> So how do we handle CPUs? cpufreq is one of the most active users of
>>> regulators...
>
>> Hmm, never thought of it :(
>> Maybe I should associate a supply name with all
>> regulators and then lookup from the global registered
>> list.
>
> I'm not sure how this should work in a device tree world, I'd *hope*
> we'd get a device tree node for the CPU and could then just make this a
> regular consumer thing but then the cpufreq drivers would need to be
> updated to make use of it. The only reason we allow null devices right
> now is the fact that cpufreq doesn't have a struct device it can use.
That's why we do have a MPU node in OMAP dts, in order to build an
omap_device that will be mainly used for the DVFS on the MPU.
And even before DT migration, we used to build statically some
omap_device to represent the various processors in the system (MPU, DSP,
CortexM3...).
Regards,
Benoit
^ permalink raw reply
* [PATCH 02/13] gpio/omap: Adapt GPIO driver to DT
From: Cousson, Benoit @ 2011-09-28 8:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E8161C7.7020404@ti.com>
Hi Rajendra,
On 9/27/2011 7:40 AM, Nayak, Rajendra wrote:
> On Monday 26 September 2011 10:20 PM, Benoit Cousson wrote:
>> Adapt the GPIO driver to retrieve information from a DT file.
>> Note that since the driver is currently under cleanup, some hacks
>> will have to be removed after.
>>
>> Add documentation for GPIO properties specific to OMAP.
>>
>> Remove an un-needed whitespace.
>>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Grant Likely<grant.likely@secretlab.ca>
>> Cc: Charulatha V<charu@ti.com>
>> Cc: Tarun Kanti DebBarma<tarun.kanti@ti.com>
>> ---
>> .../devicetree/bindings/gpio/gpio-omap.txt | 33 ++++++
>> drivers/gpio/gpio-omap.c | 108 ++++++++++++++++++--
>> 2 files changed, 132 insertions(+), 9 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-omap.txt
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-omap.txt b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
>> new file mode 100644
>> index 0000000..bdd63de
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
>> @@ -0,0 +1,33 @@
>> +OMAP GPIO controller
>> +
>> +Required properties:
>> +- compatible:
>> + - "ti,omap2-gpio" for OMAP2 and OMAP3 controllers
>
> Would it be more readable to have
> "ti,omap2-gpio" for OMAP2 controllers and
> "ti,omap3-gpio" for OMAP3 controllers?
>
>> + - "ti,omap4-gpio" for OMAP4 controller
>> +- #gpio-cells : Should be two.
>> + - first cell is the pin number
>> + - second cell is used to specify optional parameters (unused)
>> +- gpio-controller : Marks the device node as a GPIO controller.
>> +
>> +OMAP specific properties:
>> +- ti,hwmods: Name of the hwmod associated to the GPIO
>> +- id: 32 bits to identify the id (1 based index)
>> +- bank-width: number of pin supported by the controller (16 or 32)
>> +- debounce: set if the controller support the debouce funtionnality
>> +- bank-count: number of controller support by the SoC. This is a temporary
>> + hack until the bank_count is removed from the driver.
>
> Is there a general rule to be followed as to when to use
> "ti,<prop-name>" and when to use just"<prop-name>".
> Since all these are OMAP specific properties, shouldn't all
> of them be "ti,<prop-name>"?
To be honest, I was wondering as well about this rule.
I think that a property that is not purely OMAP specific and that
represents some standard HW information does not have to be prefixed by
"ti,XXX".
So hwmods must be "ti,hwmods", but bank-witdh and bank-count seems to me
quite generic.
>> +Example:
>> +
>> +gpio4: gpio4 {
>> + compatible = "ti,omap4-gpio", "ti,omap-gpio";
>> + ti,hwmods = "gpio4";
>> + id =<4>;
>> + bank-width =<32>;
>> + debounce;
>> + no_idle_on_suspend;
>> + #gpio-cells =<2>;
>> + gpio-controller;
>> +};
>> +
>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index 0599854..f878fa4 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -21,6 +21,8 @@
>> #include<linux/io.h>
>> #include<linux/slab.h>
>> #include<linux/pm_runtime.h>
>> +#include<linux/of.h>
>> +#include<linux/of_device.h>
>>
>> #include<mach/hardware.h>
>> #include<asm/irq.h>
>> @@ -521,7 +523,7 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable)
>> unsigned long flags;
>>
>> if (bank->non_wakeup_gpios& gpio_bit) {
>> - dev_err(bank->dev,
>> + dev_err(bank->dev,
>
> Stray change?
Not anymore, it is part of the changelog :-)
>
>> "Unable to modify wakeup on non-wakeup GPIO%d\n", gpio);
>> return -EINVAL;
>> }
>> @@ -1150,6 +1152,8 @@ static void __devinit omap_gpio_chip_init(struct gpio_bank *bank)
>> irq_set_handler_data(bank->irq, bank);
>> }
>>
>> +static const struct of_device_id omap_gpio_match[];
>> +
>> static int __devinit omap_gpio_probe(struct platform_device *pdev)
>> {
>> static int gpio_init_done;
>> @@ -1157,11 +1161,31 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
>> struct resource *res;
>> int id;
>> struct gpio_bank *bank;
>> + struct device_node *node = pdev->dev.of_node;
>> + const struct of_device_id *match;
>> +
>> + match = of_match_device(omap_gpio_match,&pdev->dev);
>> + if (match) {
>> + pdata = match->data;
>> + /* XXX: big hack until the bank_count is removed */
>> + of_property_read_u32(node, "bank-count",&gpio_bank_count);
>> + if (of_property_read_u32(node, "id",&id))
>
> id should be u32.
Oops, good point.
>
>> + return -EINVAL;
>> + /*
>> + * In an ideal world, the id should not be needed, but since
>> + * the OMAP TRM consider the multiple GPIO controllers as
>> + * multiple banks, the GPIO number is based on the whole set
>> + * of banks. Hence the need to provide an id in order to
>> + * respect the order and the correct GPIO number.
>> + */
>> + id -= 1;
>> + } else {
>> + if (!pdev->dev.platform_data)
>> + return -EINVAL;
>>
>> - if (!pdev->dev.platform_data)
>> - return -EINVAL;
>> -
>> - pdata = pdev->dev.platform_data;
>> + pdata = pdev->dev.platform_data;
>> + id = pdev->id;
>> + }
>>
>> if (!gpio_init_done) {
>> int ret;
>> @@ -1171,7 +1195,6 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
>> return ret;
>> }
>>
>> - id = pdev->id;
>> bank =&gpio_bank[id];
>>
>> res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> @@ -1181,12 +1204,19 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
>> }
>>
>> bank->irq = res->start;
>> - bank->virtual_irq_start = pdata->virtual_irq_start;
>> bank->method = pdata->bank_type;
>> bank->dev =&pdev->dev;
>> - bank->dbck_flag = pdata->dbck_flag;
>> bank->stride = pdata->bank_stride;
>> - bank->width = pdata->bank_width;
>> + if (match) {
>> + of_property_read_u32(node, "bank-width",&bank->width);
>
> Bank width should be u32.
>
>> + if (of_get_property(node, "debounce", NULL))
>
> of_find_property() should suffice.
Yes, indeed.
Thanks,
Benoit
^ permalink raw reply
* [PATCH 8/9] regulator: helper to extract regulator node based on supply name
From: Rajendra Nayak @ 2011-09-28 8:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E82D63A.7030207@ti.com>
On Wednesday 28 September 2011 01:39 PM, Cousson, Benoit wrote:
> On 9/27/2011 8:59 PM, Mark Brown wrote:
>> On Tue, Sep 27, 2011 at 08:19:37PM +0530, Rajendra Nayak wrote:
>>> On Tuesday 27 September 2011 05:51 PM, Mark Brown wrote:
>>>> On Tue, Sep 27, 2011 at 03:42:51PM +0530, Rajendra Nayak wrote:
>>
>>>>> + if (!dev)
>>>>> + return NULL;
>>
>>>> So how do we handle CPUs? cpufreq is one of the most active users of
>>>> regulators...
>>
>>> Hmm, never thought of it :(
>>> Maybe I should associate a supply name with all
>>> regulators and then lookup from the global registered
>>> list.
>>
>> I'm not sure how this should work in a device tree world, I'd *hope*
>> we'd get a device tree node for the CPU and could then just make this a
>> regular consumer thing but then the cpufreq drivers would need to be
>> updated to make use of it. The only reason we allow null devices right
>> now is the fact that cpufreq doesn't have a struct device it can use.
>
> That's why we do have a MPU node in OMAP dts, in order to build an
> omap_device that will be mainly used for the DVFS on the MPU.
>
> And even before DT migration, we used to build statically some
> omap_device to represent the various processors in the system (MPU, DSP,
> CortexM3...).
yes, but clearly not everyone seems to do this. and then there are
also these instances of board files requesting regulators without
associating them with any device :(
>
> Regards,
> Benoit
^ permalink raw reply
* [PATCH 02/13] gpio/omap: Adapt GPIO driver to DT
From: Cousson, Benoit @ 2011-09-28 8:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E8161C7.7020404@ti.com>
I missed one comment...
On 9/27/2011 7:40 AM, Nayak, Rajendra wrote:
> On Monday 26 September 2011 10:20 PM, Benoit Cousson wrote:
[...]
>> +Required properties:
>> +- compatible:
>> + - "ti,omap2-gpio" for OMAP2 and OMAP3 controllers
>
> Would it be more readable to have
> "ti,omap2-gpio" for OMAP2 controllers and
> "ti,omap3-gpio" for OMAP3 controllers?
The point here is to identify the IP version used in various OMAPs.
Since OMAP3 and OMAP2 are using the same IP version, there is no point
to differentiate the OMAP3 version.
What is doable is to put both "ti,omap3-gpio", "ti,omap2-gpio" to avoid
modifying the driver for no reason and still being able to identify the
OMAP3 version.
But I'm not sure we should do that if there is not real difference.
regards,
Benoit
^ permalink raw reply
* [PATCH for 3.1] ARM: at91: add PWM driver selector in Kconfig for 9g45 family
From: Nicolas Ferre @ 2011-09-28 8:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316710934-22405-1-git-send-email-nicolas.ferre@atmel.com>
Le 22/09/2011 19:02, Nicolas Ferre :
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> drivers/misc/Kconfig | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 2d6423c..7ce658e 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -63,7 +63,7 @@ config AD525X_DPOT_SPI
>
> config ATMEL_PWM
> tristate "Atmel AT32/AT91 PWM support"
> - depends on AVR32 || ARCH_AT91SAM9263 || ARCH_AT91SAM9RL || ARCH_AT91CAP9
> + depends on AVR32 || ARCH_AT91SAM9263 || ARCH_AT91SAM9RL || ARCH_AT91SAM9G45 || ARCH_AT91CAP9
I am rethinking about that just planning to remove entirely this "depends" line...
> help
> This option enables device driver support for the PWM channels
> on certain Atmel processors. Pulse Width Modulation is used for
---8<--------
From: Nicolas Ferre <nicolas.ferre@atmel.com>
Date: Wed, 28 Sep 2011 09:38:44 +0200
Subject: [PATCH for 3.1 V2] ARM: at91: add PWM driver selector in Kconfig for 9g45 family
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 2d6423c..7ce658e 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -63,7 +63,6 @@ config AD525X_DPOT_SPI
config ATMEL_PWM
tristate "Atmel AT32/AT91 PWM support"
- depends on AVR32 || ARCH_AT91SAM9263 || ARCH_AT91SAM9RL || ARCH_AT91CAP9
help
This option enables device driver support for the PWM channels
on certain Atmel processors. Pulse Width Modulation is used for
--
Nicolas Ferre
^ permalink raw reply related
* at91 material for 3.1
From: Nicolas Ferre @ 2011-09-28 8:27 UTC (permalink / raw)
To: linux-arm-kernel
Hi Arnd,
I have two little patches about Kconfig and one defconfig that are
suitable for a 3.1 inclusion.
I wonder if you plan a pull request to Linus before 3.1-final?
If yes, would you prefer each patch in a feature branch on its own
(at91-kconfig, at91-defconfig for instance) or a generic at91-fixes?
Thanks, best regards,
--
Nicolas Ferre
^ permalink raw reply
* [PATCH] ARM: mm: Align bank start to MAX_ORDER_NR_PAGES
From: Johan Palsson @ 2011-09-28 8:32 UTC (permalink / raw)
To: linux-arm-kernel
The VM subsystem assumes that there are valid memmap entries from
the bank start aligned to MAX_ORDER_NR_PAGES.
Cc: stable at kernel.org
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Michael Bohan <mbohan@codeaurora.org>
Cc: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Johan Palsson <johan.palsson@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
arch/arm/mm/init.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index cc7e2d8..f8037ba 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -496,6 +496,13 @@ static void __init free_unused_memmap(struct meminfo *mi)
*/
bank_start = min(bank_start,
ALIGN(prev_bank_end, PAGES_PER_SECTION));
+#else
+ /*
+ * Align down here since the VM subsystem insists that the
+ * memmap entries are valid from the bank start aligned to
+ * MAX_ORDER_NR_PAGES.
+ */
+ bank_start = round_down(bank_start, MAX_ORDER_NR_PAGES);
#endif
/*
* If we had a previous bank, and there is a space
--
1.7.3.2
^ permalink raw reply related
* [PATCH V2 1/1] dmaengine/amba-pl08x: Add support for s3c64xx DMAC
From: Alim Akhtar @ 2011-09-28 8:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E82D0A1.8050203@st.com>
Hi Viresh,
Thanks for reviewing the patch.
On Wed, Sep 28, 2011 at 1:15 PM, Viresh Kumar <viresh.kumar@st.com> wrote:
> On 9/28/2011 11:20 AM, Alim Akhtar wrote:
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>> ?drivers/dma/amba-pl08x.c | ?135 ++++++++++++++++++++++++++++++++++++++--------
>> ?1 files changed, 112 insertions(+), 23 deletions(-)
>>
>
> It would be good if you can add pick some part from cover-letter and put it in
> commit log too.
>
Ok, I will write more comments in the commit log.
>> diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
>> index cd8df7f..501540f 100644
>> --- a/drivers/dma/amba-pl08x.c
>> +++ b/drivers/dma/amba-pl08x.c
>> @@ -110,6 +129,11 @@ struct pl08x_lli {
>> ? ? ? u32 dst;
>> ? ? ? u32 lli;
>> ? ? ? u32 cctl;
>> + ? ? /*
>> + ? ? ?* Samsung pl080 DMAC has one exrta control register
>> + ? ? ?* which is used to hold the transfer_size
>> + ? ? ?*/
>> + ? ? u32 cctl1;
>
> Will you write transfer_size in cctl also? What is the purpose of cctl1?
>
The main difference between Primecell PL080 and samsung variant is in
LLI control register bit [0:11] is reserved in case of samsung pl080
and one extra register is add to hold the transfer size at offset
0x10. The purpose of cctl1 is store the transfer_size.
>> @@ -215,11 +255,23 @@ static void pl08x_start_txd(struct pl08x_dma_chan *plchan,
>> ? ? ? ? ? ? ? cpu_relax();
>>
>> ? ? ? /* Do not access config register until channel shows as inactive */
>> - ? ? val = readl(phychan->base + PL080_CH_CONFIG);
>> - ? ? while ((val & PL080_CONFIG_ACTIVE) || (val & PL080_CONFIG_ENABLE))
>> + ? ? if (pl08x->vd->is_pl080_s3c) {
>> + ? ? ? ? ? ? val = readl(phychan->base + PL080S_CH_CONFIG);
>> + ? ? ? ? ? ? while ((val & PL080_CONFIG_ACTIVE) ||
>> + ? ? ? ? ? ? ? ? ? ? (val & PL080_CONFIG_ENABLE))
>> + ? ? ? ? ? ? ? ? ? ? val = readl(phychan->base + PL080S_CH_CONFIG);
>> +
>> + ? ? ? ? ? ? writel(val | PL080_CONFIG_ENABLE,
>> + ? ? ? ? ? ? ? ? ? ? phychan->base + PL080S_CH_CONFIG);
>> + ? ? } else {
>> ? ? ? ? ? ? ? val = readl(phychan->base + PL080_CH_CONFIG);
>> + ? ? ? ? ? ? ? ? ? ? while ((val & PL080_CONFIG_ACTIVE) ||
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? (val & PL080_CONFIG_ENABLE))
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? val = readl(phychan->base + PL080_CH_CONFIG);
>>
>> - ? ? writel(val | PL080_CONFIG_ENABLE, phychan->base + PL080_CH_CONFIG);
>> + ? ? ? ? ? ? writel(val | PL080_CONFIG_ENABLE,
>> + ? ? ? ? ? ? ? ? ? ? phychan->base + PL080_CH_CONFIG);
>> + ? ? }
>
> You have similar stuff in most the the changes in this patch, because offset of
> config register changes for s3c, you placed in if,else block.
>
> If you check these changes again, there is a lot of code duplication in this if,else
> blocks. The only different thing in if,else is the offset of CH_CONFIG register.
>
> It would be much better if you can do following:
>
> u32 ch_cfg_off;
>
> ? ? ? ?if (pl08x->vd->is_pl080_s3c)
> ? ? ? ? ? ? ? ?ch_cfg_off = PL080S_CH_CONFIG;
> ? ? ? ?else
> ? ? ? ? ? ? ? ?ch_cfg_off = PL080_CH_CONFIG;
>
> Now, this offset can be used in existing code, without much code duplication.
>
I will use suggestion and remove the code duplications.
>> @@ -569,6 +641,7 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
>> ? ? ? u32 cctl, early_bytes = 0;
>> ? ? ? size_t max_bytes_per_lli, total_bytes = 0;
>> ? ? ? struct pl08x_lli *llis_va;
>> + ? ? size_t lli_len = 0, target_len, tsize, odd_bytes;
>>
>> ? ? ? txd->llis_va = dma_pool_alloc(pl08x->pool, GFP_NOWAIT, &txd->llis_bus);
>> ? ? ? if (!txd->llis_va) {
>> @@ -700,7 +773,7 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
>> ? ? ? ? ? ? ? ?* width left
>> ? ? ? ? ? ? ? ?*/
>> ? ? ? ? ? ? ? while (bd.remainder > (mbus->buswidth - 1)) {
>> - ? ? ? ? ? ? ? ? ? ? size_t lli_len, tsize, width;
>> + ? ? ? ? ? ? ? ? ? ? size_t width;
>>
>
> why do we need above two changes. odd_bytes and target_len are still unused.
>
sorry, I will remove the used variables.
>> ? ? ? ? ? ? ? ? ? ? ? /*
>> ? ? ? ? ? ? ? ? ? ? ? ?* If enough left try to send max possible,
>> @@ -759,6 +832,9 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
>> ? ? ? llis_va[num_llis - 1].lli = 0;
>> ? ? ? /* The final LLI element shall also fire an interrupt. */
>> ? ? ? llis_va[num_llis - 1].cctl |= PL080_CONTROL_TC_IRQ_EN;
>> + ? ? /* Keep the TransferSize seperate to fill samsung specific register */
>> + ? ? if (pl08x->vd->is_pl080_s3c)
>> + ? ? ? ? ? ? llis_va[num_llis - 1].cctl1 |= lli_len;
>
> I couldn't get this completely. Why do you keep only length of
> last lli in cctl1. what about all other llis. Also why |=
> and not directly cctl1 = lli_len
>
I will write more explanation about it.
> --
> viresh
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH 06/13] mfd: twl-core: Add initial DT support for twl4030/twl6030
From: Cousson, Benoit @ 2011-09-28 8:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E816232.5020502@ti.com>
On 9/27/2011 7:42 AM, Nayak, Rajendra wrote:
> On Monday 26 September 2011 10:20 PM, Benoit Cousson wrote:
>> Add initial device-tree support for twl familly chips.
>
> s/familly/family
Oops.
>> The current version is missing the regulator entries due
>> to the lack of DT regulator bindings for the moment.
>> Only the simple sub-modules that do not depend on
>> platform_data information can be initialized properly.
>>
>> Add documentation for the Texas Instruments TWL Integrated Chip.
>>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Balaji T K<balajitk@ti.com>
>> Cc: Graeme Gregory<gg@slimlogic.co.uk>
>> Cc: Samuel Ortiz<sameo@linux.intel.com>
>> ---
>> .../devicetree/bindings/mfd/twl-familly.txt | 47 +++++++++++++++++
>> drivers/mfd/twl-core.c | 53 ++++++++++++++++++--
>> 2 files changed, 96 insertions(+), 4 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/mfd/twl-familly.txt
>
> s/familly.txt/family.txt
At least I am consistent in my typos ;-)
>> diff --git a/Documentation/devicetree/bindings/mfd/twl-familly.txt b/Documentation/devicetree/bindings/mfd/twl-familly.txt
>> new file mode 100644
>> index 0000000..ff4cacd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/twl-familly.txt
>> @@ -0,0 +1,47 @@
>> +Texas Instruments TWL family
>> +
>> +The TWLs are Integrated Power Management Chips.
>> +Some version might contain much more analog function like
>> +USB transceiver or Audio amplifier.
>> +These chips are connected to an i2c bus.
>> +
>> +
>> +Required properties:
>> +- compatible : Must be "ti,twl4030";
>> + For Integrated power-management/audio CODEC device used in OMAP3
>> + based boards
>> +- compatible : Must be "ti,twl6030";
>> + For Integrated power-management used in OMAP4 based boards
>> +- interrupts : This i2c device has an IRQ line connected to the main SoC
>> +- interrupt-controller : Since the twl support several interrupts internally,
>> + it is considered as an interrupt controller cascaded to the SoC one.
>> +- #interrupt-cells =<1>;
>> +- interrupt-parent : The parent interrupt controller.
>> +
>> +Optional node:
>> +- Child nodes contain in the twl. The twl family is made of severals variants
>> + that support a different number of features.
>> + The children nodes will thus depend of the capabilty of the variant.
>> +
>> +
>> +Example:
>> +/*
>> + * Integrated Power Management Chip
>> + * http://www.ti.com/lit/ds/symlink/twl6030.pdf
>> + */
>> +twl at 48 {
>> + compatible = "ti,twl6030";
>> + reg =<0x48>;
>
> What does the 'reg' property signify here for twl?
The i2c slave address.
>> + interrupts =<39>; /* IRQ_SYS_1N cascaded to gic */
>> + interrupt-controller;
>> + #interrupt-cells =<1>;
>> + interrupt-parent =<&gic>;
>> + #address-cells =<1>;
>> + #size-cells =<0>;
>> +
>> + twl_rtc {
>> + compatible = "ti,twl_rtc";
>> + interrupts =<11>;
>> + reg =<0>;
>
> Does the 'reg' property need to be faked for
> every twl child node, even if it does not have
> any?
No, it was even removed from the DTS. But I forgot to update the
documentation:-(
>> + };
>> +};
>> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
>> index 01ecfee..3ef0b43 100644
>> --- a/drivers/mfd/twl-core.c
>> +++ b/drivers/mfd/twl-core.c
>> @@ -33,6 +33,10 @@
>> #include<linux/platform_device.h>
>> #include<linux/clk.h>
>> #include<linux/err.h>
>> +#include<linux/slab.h>
>> +#include<linux/of_irq.h>
>> +#include<linux/of_platform.h>
>> +#include<linux/irqdomain.h>
>>
>> #include<linux/regulator/machine.h>
>>
>> @@ -1182,22 +1186,53 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
>> int status;
>> unsigned i;
>> struct twl4030_platform_data *pdata = client->dev.platform_data;
>> + struct device_node *node = client->dev.of_node;
>> u8 temp;
>> int ret = 0;
>>
>> + if (node&& !pdata) {
>> + /*
>> + * XXX: Temporary fake pdata until the information
>> + * is correctly retrieved by every TWL modules from DT.
>> + */
>> + pdata = kzalloc(sizeof(struct twl4030_platform_data),
>> + GFP_KERNEL);
>
> devm_kzalloc instead?
Good point.
>> + if (!pdata) {
>> + status = -ENOMEM;
>> + goto exit;
>> + }
>> +
>> + /*
>> + * XXX: For the moment the IRQs for TWL seems to be encoded in
>> + * the global OMAP space. That should be cleaned to allow
>> + * dynamically adding a new IRQ controller.
>> + */
>> + if ((id->driver_data)& TWL6030_CLASS) {
>> + pdata->irq_base = TWL6030_IRQ_BASE;
>> + pdata->irq_end = pdata->irq_base + TWL6030_BASE_NR_IRQS;
>> + } else {
>> + pdata->irq_base = TWL4030_IRQ_BASE;
>> + pdata->irq_end = pdata->irq_base + TWL4030_BASE_NR_IRQS;
>> + }
>> + irq_domain_add_simple(node, pdata->irq_base);
>> + }
>> +
>> if (!pdata) {
>> dev_dbg(&client->dev, "no platform data?\n");
>> - return -EINVAL;
>> + status = -EINVAL;
>> + goto fail_free;
>> }
>>
>> if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C) == 0) {
>> dev_dbg(&client->dev, "can't talk I2C?\n");
>> - return -EIO;
>> + status = -EIO;
>> + goto fail_free;
>> }
>>
>> if (inuse) {
>> dev_dbg(&client->dev, "driver is already in use\n");
>> - return -EBUSY;
>> + status = -EBUSY;
>> + goto fail_free;
>> }
>>
>> for (i = 0; i< TWL_NUM_SLAVES; i++) {
>> @@ -1269,10 +1304,20 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
>> twl_i2c_write_u8(TWL4030_MODULE_INTBR, temp, REG_GPPUPDCTR1);
>> }
>>
>> - status = add_children(pdata, id->driver_data);
>> +#ifdef CONFIG_OF_DEVICE
>
> is the #ifdef really needed?
Yes, because the of_platform_populate does not have a stub in case of
none DT build... But I agree, it is ugly and the of_platform.h should
maybe be fixed instead.
I was too lazy to fix it :-)
Thanks,
Benoit
^ 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