Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 08/10] pwm: pwm-tiehrpwm: Adding TBCLK gating support.
From: Thierry Reding @ 2012-11-09  8:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1352361197-27442-9-git-send-email-avinashphilip@ti.com>

On Thu, Nov 08, 2012 at 01:23:15PM +0530, Philip, Avinash wrote:
> Some platforms (like AM33XX) requires clock gating from control module
> explicitly for TBCLK. Enabling of this clock required for the
> functioning of the time base sub module in EHRPWM module. So adding
> optional TBCLK handling if DT node populated with tbclkgating. This
> helps the driver can coexist for Davinci platforms.
> 
> Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
> Cc:	Grant Likely <grant.likely@secretlab.ca>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Rob Landley <rob@landley.net>
> ---
> Changes since v1:
> 	- Moved TBCLK enable from probe to .pwm_enable & disable from
> 	  remove to .pwm_disable
> 
> :100644 100644 07911e6... 927a8ed... M	drivers/pwm/pwm-tiehrpwm.c
>  drivers/pwm/pwm-tiehrpwm.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> index 07911e6..927a8ed 100644
> --- a/drivers/pwm/pwm-tiehrpwm.c
> +++ b/drivers/pwm/pwm-tiehrpwm.c
> @@ -126,6 +126,7 @@ struct ehrpwm_pwm_chip {
>  	void __iomem	*mmio_base;
>  	unsigned long period_cycles[NUM_PWM_CHANNEL];
>  	enum pwm_polarity polarity[NUM_PWM_CHANNEL];
> +	struct	clk	*tbclk;
>  };
>  
>  static inline struct ehrpwm_pwm_chip *to_ehrpwm_pwm_chip(struct pwm_chip *chip)
> @@ -346,6 +347,13 @@ static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>  	/* Channels polarity can be configured from action qualifier module */
>  	configure_polarity(pc, pwm->hwpwm);
>  
> +	/*
> +	 * Platforms require explicit clock enabling of TBCLK has
> +	 * to enable TBCLK explicitly before enabling PWM device
> +	 */
> +	if (pc->tbclk)
> +		clk_enable(pc->tbclk);
> +
>  	/* Enable time counter for free_run */
>  	ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_RUN_MASK, TBCTL_FREE_RUN);
>  	return 0;
> @@ -374,6 +382,10 @@ static void ehrpwm_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>  
>  	ehrpwm_modify(pc->mmio_base, AQCSFRC, aqcsfrc_mask, aqcsfrc_val);
>  
> +	/* Disabling TBCLK on PWM disable */
> +	if (pc->tbclk)
> +		clk_disable(pc->tbclk);
> +
>  	/* Stop Time base counter */
>  	ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_RUN_MASK, TBCTL_STOP_NEXT);
>  
> @@ -464,6 +476,16 @@ static int __devinit ehrpwm_pwm_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
>  		return ret;
>  	}
> +
> +	/* Some platforms require explicit tbclk gating */
> +	if (of_property_read_bool(pdev->dev.of_node, "tbclkgating")) {

Is it really necessary to have an extra boolean property for this?
Couldn't this just be handled by not defining a clock for the tbclk
consumer in board setup/DT

> +		pc->tbclk = clk_get(&pdev->dev, "tbclk");

You should be using devm_clk_get() or add a matching clk_put() in
.remove().

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121109/288ac5fd/attachment.sig>

^ permalink raw reply

* [PATCH v2 04/10] pwm: pwm-tiecap: Add device-tree binding support for APWM driver
From: Thierry Reding @ 2012-11-09  8:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121109075219.GA22224@avionic-0098.mockup.avionic-design.de>

On Fri, Nov 09, 2012 at 08:52:19AM +0100, Thierry Reding wrote:
> On Thu, Nov 08, 2012 at 01:23:11PM +0530, Philip, Avinash wrote:
[...]
> >  static int __devinit ecap_pwm_probe(struct platform_device *pdev)
> 
> __devinit can go away.
[...]
> >  static int __devexit ecap_pwm_remove(struct platform_device *pdev)
> 
> No __devexit please.
[...]
> >  	.remove = __devexit_p(ecap_pwm_remove),
> 
> No __devexit_p() please.

Okay, ignore these comments. The annotation were not added in this patch
so they can be removed in a separate patch. Or when Greg finally removes
all traces of them.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121109/8aa58abd/attachment.sig>

^ permalink raw reply

* [PATCH 2/2] ARM: S3C64XX: Statically define parent clock of the "camera" clock
From: Sylwester Nawrocki @ 2012-11-09  8:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAA11ShAo_N5iwPmVqE27NMzhpSD6NZORqf7yvjUyZ-+kGkgYog@mail.gmail.com>

On 11/09/2012 08:42 AM, Andrey Gusakov wrote:
> Hi.
>
> I think .reg_src can be removed? This clock have only one source.

Yes, good point. I'll repost with reg_src removed.
Can you test that patch then ?

Thanks,
Sylwester

> On Thu, Nov 8, 2012 at 2:00 AM, Sylwester Nawrocki
> <sylvester.nawrocki@gmail.com>  wrote:
>> The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null
>> clock source mux control register as it can have only one parent
>> clock. In such cases there is a need to configure the parent clock
>> statically, otherwise s3c_set_clksrc() bails out with an error message
>> "no parent clock specified" leaving the parent clock not configured.
>> Define statically the parent clock so it is possible to get or set rate
>> of the "camera" clock.
>>
>> Reported-by: In-Bae Jeong<kukyakya@gmail.com>
>> Signed-off-by: Sylwester Nawrocki<sylvester.nawrocki@gmail.com>
>> ---
>>   arch/arm/mach-s3c64xx/clock.c |    1 +
>>   1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-s3c64xx/clock.c b/arch/arm/mach-s3c64xx/clock.c
>> index 28041e8..85b9cf1 100644
>> --- a/arch/arm/mach-s3c64xx/clock.c
>> +++ b/arch/arm/mach-s3c64xx/clock.c
>> @@ -744,6 +744,7 @@ static struct clksrc_clk clksrcs[] = {
>>                          .name           = "camera",
>>                          .ctrlbit        = S3C_CLKCON_SCLK_CAM,
>>                          .enable         = s3c64xx_sclk_ctrl,
>> +                       .parent         =&clk_h2,
>>                  },
>>                  .reg_div        = { .reg = S3C_CLK_DIV0, .shift = 20, .size = 4  },
>>                  .reg_src        = { .reg = NULL, .shift = 0, .size = 0  },
>> --
>> 1.7.4.1

^ permalink raw reply

* [PATCH] ARM: S3C64XX: Statically define parent clock of the "camera" clock
From: Sylwester Nawrocki @ 2012-11-09  8:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAA11ShAo_N5iwPmVqE27NMzhpSD6NZORqf7yvjUyZ-+kGkgYog@mail.gmail.com>

The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null
clock source mux control register as it can have only one parent
clock. In such cases there is a need to configure the parent clock
statically, otherwise s3c_set_clksrc() bails out with an error message
"no parent clock specified" leaving the parent clock not configured.
Define statically the parent clock so it is possible to get or set
rate of the "camera" clock.
While at it remove the unneded null reg_src definition.

Reported-by: In-Bae Jeong <kukyakya@gmail.com>
Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
---
 arch/arm/mach-s3c64xx/clock.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-s3c64xx/clock.c b/arch/arm/mach-s3c64xx/clock.c
index 28041e8..2e6d7f9 100644
--- a/arch/arm/mach-s3c64xx/clock.c
+++ b/arch/arm/mach-s3c64xx/clock.c
@@ -744,9 +744,9 @@ static struct clksrc_clk clksrcs[] = {
 			.name		= "camera",
 			.ctrlbit        = S3C_CLKCON_SCLK_CAM,
 			.enable		= s3c64xx_sclk_ctrl,
+			.parent		= &clk_h2,
 		},
 		.reg_div	= { .reg = S3C_CLK_DIV0, .shift = 20, .size = 4  },
-		.reg_src	= { .reg = NULL, .shift = 0, .size = 0  },
 		.sources	= &clkset_camif,
 	},
 };
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 1/1] regulator: gpio-regulator: fix crash when no states property in dt
From: Frank Li @ 2012-11-09  8:34 UTC (permalink / raw)
  To: linux-arm-kernel

Unable to handle kernel NULL pointer dereference at virtual address 00000004
pgd = 80004000
[00000004] *pgd=00000000
Internal error: Oops: 5 [#1] SMP ARM
Modules linked in:
CPU: 0    Not tainted  (3.5.7+ #11)
PC is at of_get_gpio_regulator_config+0x1b0/0x2e0
LR is at of_find_property+0x4c/0x9c
pc : [<8022498c>]    lr : [<80322d44>]    psr: 60000013
sp : bf859de8  ip : bf859dc8  fp : bf859e14
r10: 805d1180  r9 : 8053e208  r8 : 00000001
r7 : 811056ec  r6 : 00000000  r5 : bf8e0208  r4 : bf8f2b50
r3 : 805a7e00  r2 : 000000d0  r1 : 00000000  r0 : 00000000
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 10c53c7d  Table: 1000404a  DAC: 00000017
Process swapper/0 (pid: 1, stack limit = 0xbf8582f0)
Stack: (0xbf859de8 to 0xbf85a000)
9de0:                   bf859e34 bf859df8 8010a040 00000000 bf8e0200 811056ec
9e00: 00000000 805b4d9c bf859e54 bf859e18 80404a14 802247e8 00000000 00000000
9e20: 00000000 00000000 00000000 00000000 00000000 00000000 8061f05c 805b4d9c
9e40: bf8e0208 00000000 bf859e64 bf859e58 8024bc0c 804049e0 bf859e8c bf859e68
9e60: 8024a7d8 8024bbf8 00000000 bf8e0208 805b4d9c bf8e023c 00000000 0000008d
9e80: bf859eac bf859e90 8024a9dc 8024a76c 8024a948 805b4d9c 8024a948 00000000
9ea0: bf859ed4 bf859eb0 80248f10 8024a954 bf83b458 bf8dd934 801f13e0 805b4d9c
9ec0: 805b8af8 bf8e9f00 bf859ee4 bf859ed8 8024a358 80248ec4 bf859f14 bf859ee8
9ee0: 80249f94 8024a344 804eb244 805d1180 805b4d9c 00000004 00000000 805d1180
9f00: 0000008d 805d1180 bf859f3c bf859f18 8024aefc 80249e1c 00000000 bf858000
9f20: 00000004 00000000 805d1180 0000008d bf859f4c bf859f40 8024bedc 8024ae88
9f40: bf859f5c bf859f50 805600b4 8024be9c bf859fb4 bf859f60 800086e4 805600ac
9f60: bf859fb4 bf859f70 805600a0 00000000 00000000 00000004 00000004 8053cb20
9f80: 00000000 804c9788 bf859fb4 805770a0 00000004 80577080 805d1180 0000008d
9fa0: 8053e208 805827a4 bf859ff4 bf859fb8 8053e974 800086b0 00000004 00000004
9fc0: 8053e208 8053e870 80026750 00000000 8053e870 80026750 00000013 00000000
9fe0: 00000000 00000000 00000000 bf859ff8 80026750 8053e87c 9773f7a7 ed28dffe
Backtrace:
[<802247dc>] (of_get_gpio_regulator_config+0x0/0x2e0) from [<80404a14>] (gpio_regulator_probe+0x40/0x2f0)
 r8:805b4d9c r7:00000000 r6:811056ec r5:bf8e0200 r4:00000000
[<804049d4>] (gpio_regulator_probe+0x0/0x2f0) from [<8024bc0c>] (platform_drv_probe+0x20/0x24)
 r7:00000000 r6:bf8e0208 r5:805b4d9c r4:8061f05c
[<8024bbec>] (platform_drv_probe+0x0/0x24) from [<8024a7d8>] (driver_probe_device+0x78/0x1e8)
[<8024a760>] (driver_probe_device+0x0/0x1e8) from [<8024a9dc>] (__driver_attach+0x94/0x98)
 r8:0000008d r7:00000000 r6:bf8e023c r5:805b4d9c r4:bf8e0208
 r3:00000000
[<8024a948>] (__driver_attach+0x0/0x98) from [<80248f10>] (bus_for_each_dev+0x58/0x84)
 r6:00000000 r5:8024a948 r4:805b4d9c r3:8024a948
[<80248eb8>] (bus_for_each_dev+0x0/0x84) from [<8024a358>] (driver_attach+0x20/0x28)
 r6:bf8e9f00 r5:805b8af8 r4:805b4d9c
[<8024a338>] (driver_attach+0x0/0x28) from [<80249f94>] (bus_add_driver+0x184/0x250)
[<80249e10>] (bus_add_driver+0x0/0x250) from [<8024aefc>] (driver_register+0x80/0x134)
[<8024ae7c>] (driver_register+0x0/0x134) from [<8024bedc>] (platform_driver_register+0x4c/0x60)
 r8:0000008d r7:805d1180 r6:00000000 r5:00000004 r4:bf858000
 r3:00000000
[<8024be90>] (platform_driver_register+0x0/0x60) from [<805600b4>] (gpio_regulator_init+0x14/0x1c)
[<805600a0>] (gpio_regulator_init+0x0/0x1c) from [<800086e4>] (do_one_initcall+0x40/0x184)
[<800086a4>] (do_one_initcall+0x0/0x184) from [<8053e974>] (kernel_init+0x104/0x1c8)
[<8053e870>] (kernel_init+0x0/0x1c8) from [<80026750>] (do_exit+0x0/0x7fc)
Code: e3a02000 e1a00007 eb03f8db e3a020d0 (e5903004)

Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
 drivers/regulator/gpio-regulator.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c
index e467d0a..b51e757 100644
--- a/drivers/regulator/gpio-regulator.c
+++ b/drivers/regulator/gpio-regulator.c
@@ -183,7 +183,7 @@ of_get_gpio_regulator_config(struct device *dev, struct device_node *np)
 
 	/* Fetch states. */
 	prop = of_find_property(np, "states", NULL);
-	proplen = prop->length / sizeof(int);
+	proplen = prop ? prop->length / sizeof(int) : 0;
 
 	config->states = devm_kzalloc(dev,
 				sizeof(struct gpio_regulator_state)
-- 
1.7.1

^ permalink raw reply related

* [PATCH] ARM: kprobes: use BUG_ON where possible
From: Jon Medhurst (Tixy) @ 2012-11-09  9:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1352406191-14303-4-git-send-email-sasha.levin@oracle.com>

On Thu, 2012-11-08 at 15:23 -0500, Sasha Levin wrote:
> Just use BUG_ON() instead of constructions such as:
> 
> 	if (...)
> 		BUG()
> 
> A simplified version of the semantic patch that makes this transformation
> is as follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression e;
> @@
> - if (e) BUG();
> + BUG_ON(e);
> // </smpl>
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

I'm not sure that trivial changes like this are worth it, but equally,
they're not worth having a discussion about, so...

Acked-by: Jon Medhurst <tixy@linaro.org>

> ---
>  arch/arm/kernel/kprobes-test.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm/kernel/kprobes-test.c b/arch/arm/kernel/kprobes-test.c
> index 1862d8f..0fb370d 100644
> --- a/arch/arm/kernel/kprobes-test.c
> +++ b/arch/arm/kernel/kprobes-test.c
> @@ -1212,8 +1212,7 @@ static int register_test_probe(struct test_probe *probe)
>  {
>  	int ret;
>  
> -	if (probe->registered)
> -		BUG();
> +	BUG_ON(probe->registered);
>  
>  	ret = register_kprobe(&probe->kprobe);
>  	if (ret >= 0) {

^ permalink raw reply

* [PATCH] ARM: S3C64XX: Statically define parent clock of the "camera" clock
From: Andrey Gusakov @ 2012-11-09  9:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1352449929-20168-1-git-send-email-sylvester.nawrocki@gmail.com>

Hi.

On Fri, Nov 9, 2012 at 12:32 PM, Sylwester Nawrocki
<sylvester.nawrocki@gmail.com> wrote:
> The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null
> clock source mux control register as it can have only one parent
> clock. In such cases there is a need to configure the parent clock
> statically, otherwise s3c_set_clksrc() bails out with an error message
> "no parent clock specified" leaving the parent clock not configured.
> Define statically the parent clock so it is possible to get or set
> rate of the "camera" clock.
> While at it remove the unneded null reg_src definition.
>
> Reported-by: In-Bae Jeong <kukyakya@gmail.com>
> Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
> ---
>  arch/arm/mach-s3c64xx/clock.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-s3c64xx/clock.c b/arch/arm/mach-s3c64xx/clock.c
> index 28041e8..2e6d7f9 100644
> --- a/arch/arm/mach-s3c64xx/clock.c
> +++ b/arch/arm/mach-s3c64xx/clock.c
> @@ -744,9 +744,9 @@ static struct clksrc_clk clksrcs[] = {
>                         .name           = "camera",
>                         .ctrlbit        = S3C_CLKCON_SCLK_CAM,
>                         .enable         = s3c64xx_sclk_ctrl,
> +                       .parent         = &clk_h2,
>                 },
>                 .reg_div        = { .reg = S3C_CLK_DIV0, .shift = 20, .size = 4  },
> -               .reg_src        = { .reg = NULL, .shift = 0, .size = 0  },
>                 .sources        = &clkset_camif,
Just figure out that .sources can be removed to. And seems
"clkset_camif" and "clkset_camif_list" can be removed as unused.

I'll try to test it tiday.

^ permalink raw reply

* [PATCH] arm: zynq: add system level control register manager
From: Daniel Borkmann @ 2012-11-09  9:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121108160125.GA5550@kryptos>

On Thu, Nov 8, 2012 at 5:01 PM, Josh Cartwright <joshc@eso.teric.us> wrote:
> On Wed, Oct 31, 2012 at 07:36:12PM +0100, Daniel Borkmann wrote:
>> This patch for the Xilinx Zynq ARM architecture adds management of system
>> level control register. The code is taken from the Xilinx-internal Linux
>> Git tree and cleaned up a bit for mainline integration. Besides others,
>> this patch is needed in order to integrate further drivers for Zynq such as
>> the Zynq xemacps networking device driver. The patch is aganst the latest
>> arm-soc tree.
>>
>> Signed-off-by: Daniel Borkmann <daniel.borkmann@tik.ee.ethz.ch>
>> Cc: Michal Simek <michals@xilinx.com>
>> Cc: John Linn <john.linn@xilinx.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> ---
>
> These changes will conflict with my pending clk patchset [1], as I'm
> introducing bindings for the SLCR (and its clk interfaces).

Oops, okay. Then your's has priority, of course.

> It seems like a hefty chunk of the out-of-tree slcr driver is related to
> configuring MIO.  I think it would be really nice if this driver was
> reworked to use the pinctrl subsystem, and also to provide suitable
> device tree bindings.

Agreed. If there's a chance to help with further Zynq integration and
to develop this driver with testing on Qemu for ARM Zynq, I'd like to
give it a try. (The board is a bit expensive as a hobby only.)

> But, at an even higher level, it would be nice to have a coordinated
> plan for getting better Zynq support upstream.  My implicit plan (so
> far) has been:
>
>    1. Initial cleanup [get the thing booting] (done)
>    2. Figure out clk drivers and bindings (pending)
>    3. Figure out MIO pinctrl support and bindings (?)
>    4. Adapting out-of-tree peripheral drivers according to 2 & 3 (?)
>    5. Add support for SMP (?)
>
> Step 4 has a dependency on 2 and 3, of course, the point being that we
> get those into a state where we are happy with them before moving all of
> the peripherals over.  I believe step 5 can be done in parallel to the
> others.
>
> Arnd-
>
> At a high level, does this sound like a suitable plan of action?  Is
> there something else you would like to see from those of us working on
> Zynq support?
>
> Thanks,
>    Josh
>
> 1: http://lists.infradead.org/pipermail/linux-arm-kernel/2012-October/129295.html
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] ARM: S3C64XX: Statically define parent clock of the "camera" clock
From: Sylwester Nawrocki @ 2012-11-09 10:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAA11ShABPJhLP5dvx65=HOzSNoiY+jjiiNPAvd_x_5z7K3meVg@mail.gmail.com>

Hi,

On 11/09/2012 10:31 AM, Andrey Gusakov wrote:
> On Fri, Nov 9, 2012 at 12:32 PM, Sylwester Nawrocki
> <sylvester.nawrocki@gmail.com>  wrote:
>> The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null
>> clock source mux control register as it can have only one parent
>> clock. In such cases there is a need to configure the parent clock
>> statically, otherwise s3c_set_clksrc() bails out with an error message
>> "no parent clock specified" leaving the parent clock not configured.
>> Define statically the parent clock so it is possible to get or set
>> rate of the "camera" clock.
>> While at it remove the unneded null reg_src definition.
>>
>> Reported-by: In-Bae Jeong<kukyakya@gmail.com>
>> Signed-off-by: Sylwester Nawrocki<sylvester.nawrocki@gmail.com>
>> ---
>>   arch/arm/mach-s3c64xx/clock.c |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-s3c64xx/clock.c b/arch/arm/mach-s3c64xx/clock.c
>> index 28041e8..2e6d7f9 100644
>> --- a/arch/arm/mach-s3c64xx/clock.c
>> +++ b/arch/arm/mach-s3c64xx/clock.c
>> @@ -744,9 +744,9 @@ static struct clksrc_clk clksrcs[] = {
>>                          .name           = "camera",
>>                          .ctrlbit        = S3C_CLKCON_SCLK_CAM,
>>                          .enable         = s3c64xx_sclk_ctrl,
>> +                       .parent         =&clk_h2,
>>                  },
>>                  .reg_div        = { .reg = S3C_CLK_DIV0, .shift = 20, .size = 4  },
>> -               .reg_src        = { .reg = NULL, .shift = 0, .size = 0  },
>>                  .sources        =&clkset_camif,
> Just figure out that .sources can be removed to. And seems
> "clkset_camif" and "clkset_camif_list" can be removed as unused.

OK, you're right. I think it could be done as a separate patch,
depending on this one to avoid conflicts.

> I'll try to test it tiday.

Thanks.

^ permalink raw reply

* [PATCH] ARM: S3C64XX: Statically define parent clock of the "camera" clock
From: Kukjin Kim @ 2012-11-09 10:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAA11ShABPJhLP5dvx65=HOzSNoiY+jjiiNPAvd_x_5z7K3meVg@mail.gmail.com>

Andrey Gusakov wrote:
> 
> Hi.
> 
> On Fri, Nov 9, 2012 at 12:32 PM, Sylwester Nawrocki
> <sylvester.nawrocki@gmail.com> wrote:
> > The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null
> > clock source mux control register as it can have only one parent
> > clock. In such cases there is a need to configure the parent clock
> > statically, otherwise s3c_set_clksrc() bails out with an error message
> > "no parent clock specified" leaving the parent clock not configured.
> > Define statically the parent clock so it is possible to get or set
> > rate of the "camera" clock.
> > While at it remove the unneded null reg_src definition.
> >
> > Reported-by: In-Bae Jeong <kukyakya@gmail.com>
> > Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
> > ---
> >  arch/arm/mach-s3c64xx/clock.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/mach-s3c64xx/clock.c b/arch/arm/mach-
> s3c64xx/clock.c
> > index 28041e8..2e6d7f9 100644
> > --- a/arch/arm/mach-s3c64xx/clock.c
> > +++ b/arch/arm/mach-s3c64xx/clock.c
> > @@ -744,9 +744,9 @@ static struct clksrc_clk clksrcs[] = {
> >                         .name           = "camera",
> >                         .ctrlbit        = S3C_CLKCON_SCLK_CAM,
> >                         .enable         = s3c64xx_sclk_ctrl,
> > +                       .parent         = &clk_h2,
> >                 },
> >                 .reg_div        = { .reg = S3C_CLK_DIV0, .shift = 20,
.size =
> 4  },
> > -               .reg_src        = { .reg = NULL, .shift = 0, .size = 0
},
> >                 .sources        = &clkset_camif,
> Just figure out that .sources can be removed to. And seems
> "clkset_camif" and "clkset_camif_list" can be removed as unused.
> 
Yeah, in this case, right. And we don't need source field as well because
its source is fixed.

See below.

8<----
diff --git a/arch/arm/mach-s3c64xx/clock.c b/arch/arm/mach-s3c64xx/clock.c
index 28041e8..e963711 100644
--- a/arch/arm/mach-s3c64xx/clock.c
+++ b/arch/arm/mach-s3c64xx/clock.c
@@ -681,15 +681,6 @@ static struct clksrc_sources clkset_audio2 = {
 	.nr_sources	= ARRAY_SIZE(clkset_audio2_list),
 };
 
-static struct clk *clkset_camif_list[] = {
-	&clk_h2,
-};
-
-static struct clksrc_sources clkset_camif = {
-	.sources	= clkset_camif_list,
-	.nr_sources	= ARRAY_SIZE(clkset_camif_list),
-};
-
 static struct clksrc_clk clksrcs[] = {
 	{
 		.clk	= {
@@ -742,12 +733,11 @@ static struct clksrc_clk clksrcs[] = {
 	}, {
 		.clk	= {
 			.name		= "camera",
+			.parent		= &clk_h2,
 			.ctrlbit        = S3C_CLKCON_SCLK_CAM,
 			.enable		= s3c64xx_sclk_ctrl,
 		},
 		.reg_div	= { .reg = S3C_CLK_DIV0, .shift = 20, .size
= 4  },
-		.reg_src	= { .reg = NULL, .shift = 0, .size = 0  },
-		.sources	= &clkset_camif,
 	},
 };
8<----

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

^ permalink raw reply related

* [PATCH v2] ARM: EXYNOS: origen: Add missing USB regulators
From: Kukjin Kim @ 2012-11-09 10:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <11762974.EgWP4VdH3P@amdc1227>

Tomasz Figa wrote:
> 
> This patch adds missing USB OTG regulators needed for s3c-hsotg driver
> to work on Origen board.
> 
> Confirmed with schematics of and tested on Origen board.
> 
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> ---
>  arch/arm/mach-exynos/mach-origen.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/mach-exynos/mach-origen.c b/arch/arm/mach-
> exynos/mach-origen.c
> index d8dc6d7..bf34704 100644
> --- a/arch/arm/mach-exynos/mach-origen.c
> +++ b/arch/arm/mach-exynos/mach-origen.c
> @@ -100,6 +100,7 @@ static struct regulator_consumer_supply __initdata
> ldo3_consumer[] = {
>  	REGULATOR_SUPPLY("vddcore", "s5p-mipi-csis.0"), /* MIPI */
>  	REGULATOR_SUPPLY("vdd", "exynos4-hdmi"), /* HDMI */
>  	REGULATOR_SUPPLY("vdd_pll", "exynos4-hdmi"), /* HDMI */
> +	REGULATOR_SUPPLY("vusb_a", "s3c-hsotg"), /* OTG */
>  };
>  static struct regulator_consumer_supply __initdata ldo6_consumer[] = {
>  	REGULATOR_SUPPLY("vddio", "s5p-mipi-csis.0"), /* MIPI */
> @@ -110,6 +111,7 @@ static struct regulator_consumer_supply __initdata
> ldo7_consumer[] = {
>  static struct regulator_consumer_supply __initdata ldo8_consumer[] = {
>  	REGULATOR_SUPPLY("vdd", "s5p-adc"), /* ADC */
>  	REGULATOR_SUPPLY("vdd_osc", "exynos4-hdmi"), /* HDMI */
> +	REGULATOR_SUPPLY("vusb_d", "s3c-hsotg"), /* OTG */
>  };
>  static struct regulator_consumer_supply __initdata ldo9_consumer[] = {
>  	REGULATOR_SUPPLY("dvdd", "swb-a31"), /* AR6003 WLAN & CSR 8810 BT
> */
> --
> 1.8.0

Applied, thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

^ permalink raw reply

* [PATCH 2/4] gpio: samsung: Skip registration if pinctrl driver is present on Exynos4x12
From: Kukjin Kim @ 2012-11-09 10:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACRpkdaOpzFvs_wqy9sdrmx1=wEs5uxqfh_rOrKxLTXV-q1K2Q@mail.gmail.com>

Linus Walleij wrote:
> 
> On Wed, Nov 7, 2012 at 5:41 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> 
> > A commit 1b6056d6 ("pinctrl: samsung: Include bank-specific eint offset
> in
> > bank struct") which is in your pinctrl tree (samsung branch) changed
> > macro(EXYNOS_PIN_BANK_EINTG) to add offset. Eventually, this series(due
> to
> > 3rd patch, pinctrl: samsung: Add support for Exynos4x12) breaks
> compilation
> > without the commit. So if you don't have a plan to rebase samsung branch
> of
> > your pinctrl tree, I'd like to merge it in my tree. Is it ok to you?
> 
> Sure tell me when you've merged it and I'll drop commit 1b6056d6
> from my tree.
> 

Thanks :-)

(- some guys in Cc...)

But having a problem, the 'it' means the commit 1b6056d6? If so, I couldn't
cherry-pick only that because of dependency with other commits 40ba622 and
3a232ba.

$ git cherry-pick -s 40ba622
[next/dt-exynos4x12 3b1977c] pinctrl: samsung: Assing pin numbers
dynamically
 Author: Tomasz Figa <t.figa@samsung.com>
 3 files changed, 62 insertions(+), 54 deletions(-)

$ git cherry-pick -s 3a232ba
[next/dt-exynos4x12 7fa08a4] pinctrl: samsung: Remove static pin
enumerations
 Author: Tomasz Figa <t.figa@samsung.com>
 1 files changed, 96 insertions(+), 215 deletions(-)
 rewrite drivers/pinctrl/pinctrl-exynos.h (66%)

$ git cherry-pick -s 1b6056d
[next/dt-exynos4x12 86010aa] pinctrl: samsung: Include bank-specific eint
offset in bank struct
 Author: Tomasz Figa <t.figa@samsung.com>
 3 files changed, 30 insertions(+), 29 deletions(-)

I could cherry-pick clearly with 2 more commits.

Can you drop above 3 commits in your tree? If many conflicts happens during
rebasing, how about that I merge your pinctrl/samsung branch into Samsung
tree?...

$ git log --oneline pinctrl/samsung v3.7-rc1..pinctrl/samsung
b33ef91 Documentation: Update samsung-pinctrl device tree bindings
documentation
a19fe2d pinctrl: samsung: Add GPIO to IRQ translation
22b9ba0 pinctrl: exynos: Set pin function to EINT in irq_set_type of wake-up
EINT
a04b07c pinctrl: samsung: Use per-bank IRQ domain for wake-up interrupts
d3a7b9e pinctrl: samsung: Use one GPIO chip per pin bank
595be72 pinctrl: exynos: Use one IRQ domain per pin bank
1b6056d pinctrl: samsung: Include bank-specific eint offset in bank struct
6defe9a pinctrl: samsung: Hold pointer to driver data in bank struct
ab66378 pinctrl: samsung: Match pin banks with their device nodes
a7a8241 ARM: dts: exynos4210-pinctrl: Add nodes for pin banks
724e56a pinctrl: samsung: Distinguish between pin group and bank nodes
3a232ba pinctrl: samsung: Remove static pin enumerations
40ba622 pinctrl: samsung: Assing pin numbers dynamically
62f14c0 pinctrl: samsung: Do not pass gpio_chip to pin_to_reg_bank
7c367d3 pinctrl: samsung: Detect and handle unsupported configuration types

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

^ permalink raw reply

* linux-next: manual merge of the arm-soc tree with the pinctrl tree
From: Linus Walleij @ 2012-11-09 10:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121109133111.1070c1062889461152713978@canb.auug.org.au>

On Fri, Nov 9, 2012 at 3:31 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Today's linux-next merge of the arm-soc tree got a conflict in
> drivers/pinctrl/pinctrl-nomadik.c between commit 241e51ebd3b2
> ("pinctrl/nomadik: make independent of prcmu driver") from the pinctrl
> tree and commit 44e47ccf8ab6 ("Merge branch 'next/multiplatform' into
> for-next") from the arm-soc tree.
>
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).

This looks like the correct fix, thanks a lot Stephen!

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH] i2c: at91: fix compilation issue due to header move
From: ludovic.desroches at atmel.com @ 2012-11-09 10:11 UTC (permalink / raw)
  To: linux-arm-kernel

From: Ludovic Desroches <ludovic.desroches@atmel.com>

arch/arm/mach-at91/include/mach/at_hdmac.h has been move to
include/linux/platform_data/dma-atmel.h, commit 7cdc39e.

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---

Hi,

This is a fix for 3.7.

Regards

Ludovic

 drivers/i2c/busses/i2c-at91.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index a9fbad8..e2e9837 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -32,7 +32,7 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
-#include <mach/at_hdmac.h>
+#include <linux/platform_data/dma-atmel.h>
 
 #define TWI_CLK_HZ		100000			/* max 400 Kbits/s */
 #define AT91_I2C_TIMEOUT	msecs_to_jiffies(100)	/* transfer timeout */
-- 
1.7.11.3

^ permalink raw reply related

* [PATCH] ARM: S3C64XX: Statically define parent clock of the "camera" clock
From: Sylwester Nawrocki @ 2012-11-09 10:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <509CD574.2050909@gmail.com>

On 11/09/2012 11:05 AM, Sylwester Nawrocki wrote:
> Hi,
>
> On 11/09/2012 10:31 AM, Andrey Gusakov wrote:
>> On Fri, Nov 9, 2012 at 12:32 PM, Sylwester Nawrocki
>> <sylvester.nawrocki@gmail.com> wrote:
>>> The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null
>>> clock source mux control register as it can have only one parent
>>> clock. In such cases there is a need to configure the parent clock
>>> statically, otherwise s3c_set_clksrc() bails out with an error message
>>> "no parent clock specified" leaving the parent clock not configured.
>>> Define statically the parent clock so it is possible to get or set
>>> rate of the "camera" clock.
>>> While at it remove the unneded null reg_src definition.
>>>
>>> Reported-by: In-Bae Jeong<kukyakya@gmail.com>
>>> Signed-off-by: Sylwester Nawrocki<sylvester.nawrocki@gmail.com>
>>> ---
>>> arch/arm/mach-s3c64xx/clock.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-s3c64xx/clock.c
>>> b/arch/arm/mach-s3c64xx/clock.c
>>> index 28041e8..2e6d7f9 100644
>>> --- a/arch/arm/mach-s3c64xx/clock.c
>>> +++ b/arch/arm/mach-s3c64xx/clock.c
>>> @@ -744,9 +744,9 @@ static struct clksrc_clk clksrcs[] = {
>>> .name = "camera",
>>> .ctrlbit = S3C_CLKCON_SCLK_CAM,
>>> .enable = s3c64xx_sclk_ctrl,
>>> + .parent =&clk_h2,
>>> },
>>> .reg_div = { .reg = S3C_CLK_DIV0, .shift = 20, .size = 4 },
>>> - .reg_src = { .reg = NULL, .shift = 0, .size = 0 },
>>> .sources =&clkset_camif,
>> Just figure out that .sources can be removed to. And seems
>> "clkset_camif" and "clkset_camif_list" can be removed as unused.
>
> OK, you're right. I think it could be done as a separate patch,
> depending on this one to avoid conflicts.

Or feel free to make a new patch, ignoring this one.

--
Regards,
Sylwester

^ permalink raw reply

* [PATCH 2/4] ARM: EXYNOS: PL330 MDMA1 fix for revision 0 of Exynos4210 SOC
From: Kukjin Kim @ 2012-11-09 10:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201211081054.41649.b.zolnierkie@samsung.com>

Bartlomiej Zolnierkiewicz wrote:
> 
> Ah, okay.  Here is full simplified patch.
> 
> From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Subject: [PATCH v2] ARM: EXYNOS: PL330 MDMA1 fix for revision 0 of
> Exynos4210 SOC
> 
> Commit 8214513 ("ARM: EXYNOS: fix address for EXYNOS4 MDMA1")
> changed EXYNOS specific setup of PL330 DMA engine to use 'non-secure'
> mdma1 address instead of 'secure' one (from 0x12840000 to 0x12850000)
> to fix issue with some Exynos4212 SOCs.  Unfortunately it brakes
> PL330 setup for revision 0 of Exynos4210 SOC (mdma1 device cannot
> be found at 'non-secure' address):
> 
> [    0.566245] dma-pl330 dma-pl330.2: PERIPH_ID 0x0, PCELL_ID 0x0 !
> [    0.566278] dma-pl330: probe of dma-pl330.2 failed with error -22
> 
> Fix it by using 'secure' mdma1 address on Exynos4210 revision 0 SOC.
> 
> Reviewed-by: Tomasz Figa <t.figa@samsung.com>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/mach-exynos/dma.c              |    3 +++
>  arch/arm/mach-exynos/include/mach/map.h |    1 +
>  2 files changed, 4 insertions(+)
> 
> Index: b/arch/arm/mach-exynos/dma.c
> ===================================================================
> --- a/arch/arm/mach-exynos/dma.c	2012-11-07 18:20:36.561743865 +0100
> +++ b/arch/arm/mach-exynos/dma.c	2012-11-08 10:48:23.445067606 +0100
> @@ -275,6 +275,9 @@ static int __init exynos_dma_init(void)
>  		exynos_pdma1_pdata.nr_valid_peri =
>  			ARRAY_SIZE(exynos4210_pdma1_peri);
>  		exynos_pdma1_pdata.peri_id = exynos4210_pdma1_peri;
> +
> +		if (samsung_rev() == EXYNOS4210_REV_0)
> +			exynos_mdma1_device.res.start = EXYNOS4_PA_S_MDMA1;
>  	} else if (soc_is_exynos4212() || soc_is_exynos4412()) {
>  		exynos_pdma0_pdata.nr_valid_peri =
>  			ARRAY_SIZE(exynos4212_pdma0_peri);
> Index: b/arch/arm/mach-exynos/include/mach/map.h
> ===================================================================
> --- a/arch/arm/mach-exynos/include/mach/map.h	2012-11-07
> 18:20:44.801743862 +0100
> +++ b/arch/arm/mach-exynos/include/mach/map.h	2012-11-08
> 10:48:40.597067605 +0100
> @@ -92,6 +92,7 @@
> 
>  #define EXYNOS4_PA_MDMA0		0x10810000
>  #define EXYNOS4_PA_MDMA1		0x12850000
> +#define EXYNOS4_PA_S_MDMA1		0x12840000
>  #define EXYNOS4_PA_PDMA0		0x12680000
>  #define EXYNOS4_PA_PDMA1		0x12690000
>  #define EXYNOS5_PA_MDMA0		0x10800000

Looks good to me, and I think, this can be handled separate from this
series.

Vinod, if you're ok, let me pick this up into Samsung tree.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

^ permalink raw reply

* [PATCH 2/2] pinctrl/nomadik: make independent of prcmu driver
From: Linus Walleij @ 2012-11-09 10:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <509BE7C5.4030109@wwwdotorg.org>

On Thu, Nov 8, 2012 at 6:11 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:

> Do you actually need to store the run-time data in struct
> nmk_pinctrl_soc_data too? I would have expected all the soc_data
> pointers to remain const, and to store the runtime register pointer
> somewhere else, and perhaps pass it as a separate parameter to the
> relevant init functions; wouldn't that make the patch much smaller?

OK point taken, I'm sending a v2...

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v2] pinctrl/nomadik: make independent of prcmu driver
From: Linus Walleij @ 2012-11-09 10:28 UTC (permalink / raw)
  To: linux-arm-kernel

From: Jonas Aaberg <jonas.aberg@stericsson.com>

Currently there are some unnecessary criss-cross
dependencies between the PRCMU driver in MFD and a lot of
other drivers, mainly because other drivers need to poke
around in the PRCM register range.

In cases like this there are actually just a few select
registers that the pinctrl driver need to read/modify/write,
and it turns out that no other driver is actually using
these registers, so there are no concurrency issues
whatsoever.

So: don't let the location of the register range complicate
things, just poke into these registers directly and skip
a layer of indirection.

Take this opportunity to add kerneldoc to the pinctrl
state container.

Cc: Loic Pallardy <loic.pallardy@st.com>
Signed-off-by: Jonas Aaberg <jonas.aberg@stericsson.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Have the pointer in the pin controller state container
  instead of as part of SoC data.
---
 drivers/pinctrl/pinctrl-nomadik.c | 59 ++++++++++++++++++++++++++-------------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
index 22f6937..6a95d04 100644
--- a/drivers/pinctrl/pinctrl-nomadik.c
+++ b/drivers/pinctrl/pinctrl-nomadik.c
@@ -30,20 +30,6 @@
 #include <linux/pinctrl/pinconf.h>
 /* Since we request GPIOs from ourself */
 #include <linux/pinctrl/consumer.h>
-/*
- * For the U8500 archs, use the PRCMU register interface, for the older
- * Nomadik, provide some stubs. The functions using these will only be
- * called on the U8500 series.
- */
-#ifdef CONFIG_ARCH_U8500
-#include <linux/mfd/dbx500-prcmu.h>
-#else
-static inline u32 prcmu_read(unsigned int reg) {
-	return 0;
-}
-static inline void prcmu_write(unsigned int reg, u32 value) {}
-static inline void prcmu_write_masked(unsigned int reg, u32 mask, u32 value) {}
-#endif
 #include <linux/platform_data/pinctrl-nomadik.h>
 
 #include <asm/mach/irq.h>
@@ -82,10 +68,18 @@ struct nmk_gpio_chip {
 	u32 lowemi;
 };
 
+/**
+ * struct nmk_pinctrl - state container for the Nomadik pin controller
+ * @dev: containing device pointer
+ * @pctl: corresponding pin controller device
+ * @soc: SoC data for this specific chip
+ * @prcm_base: PRCM register range virtual base
+ */
 struct nmk_pinctrl {
 	struct device *dev;
 	struct pinctrl_dev *pctl;
 	const struct nmk_pinctrl_soc_data *soc;
+	void __iomem *prcm_base;
 };
 
 static struct nmk_gpio_chip *
@@ -247,6 +241,15 @@ nmk_gpio_disable_lazy_irq(struct nmk_gpio_chip *nmk_chip, unsigned offset)
 	dev_dbg(nmk_chip->chip.dev, "%d: clearing interrupt mask\n", gpio);
 }
 
+static void nmk_write_masked(void __iomem *reg, u32 mask, u32 value)
+{
+	u32 val;
+
+	val = readl(reg);
+	val = ((val & ~mask) | (value & mask));
+	writel(val, reg);
+}
+
 static void nmk_prcm_altcx_set_mode(struct nmk_pinctrl *npct,
 	unsigned offset, unsigned alt_num)
 {
@@ -285,8 +288,8 @@ static void nmk_prcm_altcx_set_mode(struct nmk_pinctrl *npct,
 			if (pin_desc->altcx[i].used == true) {
 				reg = gpiocr_regs[pin_desc->altcx[i].reg_index];
 				bit = pin_desc->altcx[i].control_bit;
-				if (prcmu_read(reg) & BIT(bit)) {
-					prcmu_write_masked(reg, BIT(bit), 0);
+				if (readl(npct->prcm_base + reg) & BIT(bit)) {
+					nmk_write_masked(npct->prcm_base + reg, BIT(bit), 0);
 					dev_dbg(npct->dev,
 						"PRCM GPIOCR: pin %i: alternate-C%i has been disabled\n",
 						offset, i+1);
@@ -314,8 +317,8 @@ static void nmk_prcm_altcx_set_mode(struct nmk_pinctrl *npct,
 		if (pin_desc->altcx[i].used == true) {
 			reg = gpiocr_regs[pin_desc->altcx[i].reg_index];
 			bit = pin_desc->altcx[i].control_bit;
-			if (prcmu_read(reg) & BIT(bit)) {
-				prcmu_write_masked(reg, BIT(bit), 0);
+			if (readl(npct->prcm_base + reg) & BIT(bit)) {
+				nmk_write_masked(npct->prcm_base + reg, BIT(bit), 0);
 				dev_dbg(npct->dev,
 					"PRCM GPIOCR: pin %i: alternate-C%i has been disabled\n",
 					offset, i+1);
@@ -327,7 +330,7 @@ static void nmk_prcm_altcx_set_mode(struct nmk_pinctrl *npct,
 	bit = pin_desc->altcx[alt_index].control_bit;
 	dev_dbg(npct->dev, "PRCM GPIOCR: pin %i: alternate-C%i has been selected\n",
 		offset, alt_index+1);
-	prcmu_write_masked(reg, BIT(bit), BIT(bit));
+	nmk_write_masked(npct->prcm_base + reg, BIT(bit), BIT(bit));
 }
 
 static void __nmk_config_pin(struct nmk_gpio_chip *nmk_chip, unsigned offset,
@@ -693,7 +696,7 @@ static int nmk_prcm_gpiocr_get_mode(struct pinctrl_dev *pctldev, int gpio)
 		if (pin_desc->altcx[i].used == true) {
 			reg = gpiocr_regs[pin_desc->altcx[i].reg_index];
 			bit = pin_desc->altcx[i].control_bit;
-			if (prcmu_read(reg) & BIT(bit))
+			if (readl(npct->prcm_base + reg) & BIT(bit))
 				return NMK_GPIO_ALT_C+i+1;
 		}
 	}
@@ -1851,6 +1854,7 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
 	const struct platform_device_id *platid = platform_get_device_id(pdev);
 	struct device_node *np = pdev->dev.of_node;
 	struct nmk_pinctrl *npct;
+	struct resource *res;
 	unsigned int version = 0;
 	int i;
 
@@ -1872,6 +1876,20 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
 	if (version == PINCTRL_NMK_DB8540)
 		nmk_pinctrl_db8540_init(&npct->soc);
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (res) {
+		npct->prcm_base = devm_ioremap(&pdev->dev, res->start,
+					       resource_size(res));
+		if (!npct->prcm_base) {
+			dev_err(&pdev->dev,
+				"failed to ioremap PRCM registers\n");
+			return -ENOMEM;
+		}
+	} else {
+		dev_info(&pdev->dev,
+			 "No PRCM base, assume no ALT-Cx control is available\n");
+	}
+
 	/*
 	 * We need all the GPIO drivers to probe FIRST, or we will not be able
 	 * to obtain references to the struct gpio_chip * for them, and we
@@ -1888,6 +1906,7 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
 	nmk_pinctrl_desc.pins = npct->soc->pins;
 	nmk_pinctrl_desc.npins = npct->soc->npins;
 	npct->dev = &pdev->dev;
+
 	npct->pctl = pinctrl_register(&nmk_pinctrl_desc, &pdev->dev, npct);
 	if (!npct->pctl) {
 		dev_err(&pdev->dev, "could not register Nomadik pinctrl driver\n");
-- 
1.7.11.3

^ permalink raw reply related

* [PATCH] ARM: tegra: enable data prefetch on L2
From: Peter De Schrijver @ 2012-11-09 10:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1352251597-2936-1-git-send-email-josephl@nvidia.com>

On Wed, Nov 07, 2012 at 02:26:37AM +0100, Joseph Lo wrote:
> Enable the data prefetch on L2. The bit28 in aux ctrl register.
> 
> Signed-off-by: Joseph Lo <josephl@nvidia.com>

Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>

Cheers,

Peter.

^ permalink raw reply

* [PATCH] ARM: backtrace: avoid crash on large invalid fp value
From: Dave Martin @ 2012-11-09 10:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMbhsRR+BVFP7rfyc_3+F0rNQVWFUh6Oca=Bg_TpcQszwF-6Bg@mail.gmail.com>

On Thu, Nov 08, 2012 at 06:05:52PM -0800, Colin Cross wrote:
> On Mon, Nov 5, 2012 at 2:54 AM, Dave Martin <dave.martin@linaro.org> wrote:
> > On Fri, Nov 02, 2012 at 04:47:38PM -0700, Colin Cross wrote:
> >> On Wed, Oct 10, 2012 at 4:15 AM, Dave Martin <dave.martin@linaro.org> wrote:
> >> > On Tue, Oct 09, 2012 at 11:46:12PM -0700, Todd Poynor wrote:
> >> >> Invalid frame pointer (signed) -4 <= fp <= -1 defeats check for too high
> >> >> on overflow.
> >> >>
> >> >> Signed-off-by: Todd Poynor <toddpoynor@google.com>
> >> >> ---
> >> >>  arch/arm/kernel/stacktrace.c |    2 +-
> >> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >> >>
> >> >> diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
> >> >> index 00f79e5..6315162 100644
> >> >> --- a/arch/arm/kernel/stacktrace.c
> >> >> +++ b/arch/arm/kernel/stacktrace.c
> >> >> @@ -31,7 +31,7 @@ int notrace unwind_frame(struct stackframe *frame)
> >> >>       high = ALIGN(low, THREAD_SIZE);
> >> >>
> >> >>       /* check current frame pointer is within bounds */
> >> >> -     if (fp < (low + 12) || fp + 4 >= high)
> >> >> +     if (fp < (low + 12) || fp >= high - 4)
> >> >>               return -EINVAL;
> >> >>
> >> >>       /* restore the registers from the stack frame */
> >> >
> >> > sp and fp can still be complete garbage in the case of a corrupted frame,
> >> > so low + 12 can still overflow and cause us to read beyond the stack base.
> >> >
> >> > A more robust patch might be as follows.  This also checks for misaligned
> >> > fp and sp values, since those indicate corruption and there can be no
> >> > sensible way to interpret the resulting frame in that case.
> >> >
> >> > Also, according to the definition of current_thread_info(),
> >> > IS_ALIGNED(sp, THREAD_SIZE) indicates a full stack extending from sp
> >> > to sp + THREAD_SIZE, and not an empty stack extending from sp -
> >> > THREAD_SIZE to sp.  We cannot backtrace this situation anyway, since
> >> > that would imply that the frame record extends beyond the stack...
> >> > but this patch tidies it up in the interest of clarity.
> >> >
> >> > Cheers
> >> > ---Dave
> >> >
> >> > (untested)
> >> >
> >> > diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
> >> > index 00f79e5..fec82be 100644
> >> > --- a/arch/arm/kernel/stacktrace.c
> >> > +++ b/arch/arm/kernel/stacktrace.c
> >> > @@ -28,10 +28,20 @@ int notrace unwind_frame(struct stackframe *frame)
> >> >
> >> >         /* only go to a higher address on the stack */
> >> >         low = frame->sp;
> >> > -       high = ALIGN(low, THREAD_SIZE);
> >> > +       if (!IS_ALIGNED(fp, 4))
> >> > +               return -EINVAL;
> >> > +
> >> > +       /*
> >> > +        * low + 1 here ensures that high > sp, consistent with the
> >> > +        * definition of current_thread_info().
> >> > +        * We subtract 1 to compute the highest allowable byte address.
> >> > +        * Otherwise, we might get high == 0 which would confuse our
> >> > +        * comparisons.
> >> > +        */
> >> > +       high = ALIGN(low + 1, THREAD_SIZE) - 1;
> 
> ARM eabi stacks are full-descending, meaning that if the sp is a
> multiple of THREAD_SIZE, the stack is empty.  current_thread_info
> takes a short-cut and assumes it can never be called on an empty
> stack, but better not to propagate that anywhere else.

The effect of the code is consistent with current_thread_info():

	low = THREAD_SIZE * X --> high = THREAD_SIZE * (X + 1) - 1
	low = THREAD_SIZE * (X + 1) - 1 --> high = THREAD_SIZE * (X + 1) - 1

i.e., low = THREAD_SIZE * X is treated as a full stack.


The comment relates to the case where the stack is right at the top
of the address space: if we define high as ALIGN(low + 1, THREAD_SIZE),
then high overflow to zero in this case, giving unexpected results
for comparisons "some_address >= high".

Definig high as the address of the last byte of the stack (instead of
the first byte after the stack) avoids this kind of problem, providing
that "some_address >= high" is rewritten as "some_address > high" in
our comparisons.

I don't know whether any stack will be at the top of the address space
in practice, but I prefer to avoid unnecessary assumptions where
possible.


Do you agree with the code as-is, or does something need to be changed/
clarified?

> >> >
> >> >         /* check current frame pointer is within bounds */
> >> > -       if (fp < (low + 12) || fp + 4 >= high)
> >> > +       if (fp < 12 || fp - 12 < low || fp > high)
> >> >                 return -EINVAL;
> >> >
> >> >         /* restore the registers from the stack frame */
> >> > @@ -39,6 +49,10 @@ int notrace unwind_frame(struct stackframe *frame)
> >> >         frame->sp = *(unsigned long *)(fp - 8);
> >> >         frame->pc = *(unsigned long *)(fp - 4);
> >> >
> >> > +       /* Do not claim the frame is valid if if is obviously corrupt: */
> >> > +       if (!IS_ALIGNED(frame->fp, 4))
> >> > +               return -EINVAL;
> >> > +
> >> >         return 0;
> >> >  }
> >> >  #endif
> >> >
> >> >
> >> > _______________________________________________
> >> > linux-arm-kernel mailing list
> >> > linux-arm-kernel at lists.infradead.org
> >> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >>
> >> Dave or Todd, mind reposting this, or should I squash it into my
> >> CONFIG_SMP stacktrace series?
> >
> > I'm happy for you to fold my patch into your series if you agree
> > with it.  Ideally, please fix my typo in the final comment ("if IT is
> > obviously corrupt").
> >
> > Do I assume correctly that you are already testing this stuff?
> 
> I've been testing it by repeatedly dumping the stack of a running
> thread (cat /dev/urandom > /dev/null) and making sure it doesn't
> panic, and by dumping all the threads in a idle system and making sure
> they all end at the normal user or kernel thread initial frames
> (do_exit, kernel_thread_exit, or ret_fast_syscall).

OK -- that's good to know.

I'm still assuming that you're rolling this into your series.  Let me
know if you want me to post a separate patch.

Cheers
---Dave

^ permalink raw reply

* [PATCH v2 01/10] PWMSS: Add PWM Subsystem driver for parent<->child relationship
From: Philip, Avinash @ 2012-11-09 10:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121109072957.GA21991@avionic-0098.mockup.avionic-design.de>

On Fri, Nov 09, 2012 at 12:59:57, Thierry Reding wrote:
> On Thu, Nov 08, 2012 at 01:23:08PM +0530, Philip, Avinash wrote:
> > diff --git a/Documentation/devicetree/bindings/pwm/tipwmss.txt b/Documentation/devicetree/bindings/pwm/tipwmss.txt
> > new file mode 100644
> > index 0000000..b6c2814
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/tipwmss.txt
> > @@ -0,0 +1,30 @@
> [...]
> > +Also child nodes should also populated under PWMSS DT node.
> > +Example:
> 
> Maybe put an blank line between these two lines for readability?

Ok I will add.

> 
> > +pwmss0: pwmss at 48300000 {
> > +	compatible = "ti,am33xx-pwmss";
> > +	reg = <0x48300000 0x10
> > +		0x48300100 0x80
> > +		0x48300180 0x80
> > +		0x48300200 0x80>;
> 
> I don't think you should list the register spaces of the children here.
> From what I understand, all regions listed in the reg property are
> supposed to be requested by the corresponding driver and therefore
> cannot be used by any other device.

I will check & correct it.

> 
> > +	ti,hwmods = "epwmss0";
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +	status = "disabled";
> > +	ranges;
> 
> I think to represent which memory regions go to the children, you should
> put them in this ranges property, which would then look like this:
> 
> 	ranges = <0x48300100 0x48300100 0x80   /* ECAP */
> 		  0x48300180 0x48300180 0x80   /* EQEP */
> 		  0x48300200 0x48300200 0x80>; /* EHRPWM */

I will correct it.

> 
> > +
> > +	/* child nodes go here */
> > +};
> 
> Maybe you should actually list a full set of children here?

Ok.

> 
> > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> > index 6e556c7..384a346 100644
> > --- a/drivers/pwm/Kconfig
> > +++ b/drivers/pwm/Kconfig
> > @@ -136,6 +136,7 @@ config PWM_TEGRA
> >  config  PWM_TIECAP
> >  	tristate "ECAP PWM support"
> >  	depends on SOC_AM33XX
> > +	select PWM_TIPWMSS
> >  	help
> >  	  PWM driver support for the ECAP APWM controller found on AM33XX
> >  	  TI SOC
> > @@ -146,6 +147,7 @@ config  PWM_TIECAP
> >  config  PWM_TIEHRPWM
> >  	tristate "EHRPWM PWM support"
> >  	depends on SOC_AM33XX
> > +	select PWM_TIPWMSS
> >  	help
> >  	  PWM driver support for the EHRPWM controller found on AM33XX
> >  	  TI SOC
> > @@ -153,6 +155,15 @@ config  PWM_TIEHRPWM
> >  	  To compile this driver as a module, choose M here: the module
> >  	  will be called pwm-tiehrpwm.
> >  
> > +config  PWM_TIPWMSS
> > +	tristate "TI PWM Subsytem parent support"
> > +	depends on SOC_AM33XX && (PWM_TIEHRPWM || PWM_TIECAP)
> 
> Since you select the symbol from the PWM_TIECAP and PWM_TIEHRPWM symbols
> there is no need to depend on them, right? Oh, but maybe that's to make
> sure the symbol is deselected automatically if neither user is selected.

I want to make sure that, symbol selected only if either are selected.
> 
> Perhaps this should actually be a hidden symbol (i.e. leave away the
> prompt string in the tristate option) since it's purely a dependency and
> not useful of its own.

Ok I will take care.

> 
> > +	help
> > +	  PWM Subsystem driver support for AM33xx SOC.
> > +
> > +	  PWM submodules require PWM config space access from submodule
> > +	  drivers and require common parent driver support.
> > +
> >  config PWM_TWL6030
> >  	tristate "TWL6030 PWM support"
> >  	depends on TWL4030_CORE
> > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> > index 3b3f4c9..55f6fb2 100644
> > --- a/drivers/pwm/Makefile
> > +++ b/drivers/pwm/Makefile
> > @@ -12,5 +12,6 @@ obj-$(CONFIG_PWM_SPEAR)		+= pwm-spear.o
> >  obj-$(CONFIG_PWM_TEGRA)		+= pwm-tegra.o
> >  obj-$(CONFIG_PWM_TIECAP)	+= pwm-tiecap.o
> >  obj-$(CONFIG_PWM_TIEHRPWM)	+= pwm-tiehrpwm.o
> > +obj-$(CONFIG_PWM_TIPWMSS)	+= tipwmss.o
> 
> This should have a pwm- prefix as well.

Ok I will add.

> 
> >  obj-$(CONFIG_PWM_TWL6030)	+= pwm-twl6030.o
> >  obj-$(CONFIG_PWM_VT8500)	+= pwm-vt8500.o
> > diff --git a/drivers/pwm/tipwmss.c b/drivers/pwm/tipwmss.c
> > new file mode 100644
> > index 0000000..c188348
> > --- /dev/null
> > +++ b/drivers/pwm/tipwmss.c
> > @@ -0,0 +1,142 @@
> [...]
> > +#include "tipwmss.h"
> > +
> > +#define PWMSS_CLKCONFIG		0x8	/* Clock gaitng reg, for PWM submodules */
> 
> "gating"

Ok

> 
> > +#define PWMSS_CLKSTATUS		0xc	/* Clock gating status reg */
> > +
> > +struct pwmss_info {
> > +	void __iomem	*mmio_base;
> > +	struct mutex	pwmss_lock;
> > +	u16				pwmss_clkconfig;
> 
> The indentation looks weird on this last field.

Ok I will correct it.

> 
> > +};
> > +
> > +u16 pwmss_submodule_state_change(struct device *dev, int set)
> > +{
> > +	struct pwmss_info *info = dev_get_drvdata(dev);
> > +	u16 val;
> > +
> > +	val = readw(info->mmio_base + PWMSS_CLKCONFIG);
> > +	val |= set;
> > +	mutex_lock(&info->pwmss_lock);
> > +	writew(val , info->mmio_base + PWMSS_CLKCONFIG);
> > +	mutex_unlock(&info->pwmss_lock);
> 
> The mutex needs to span the whole read-modify-write sequence, not just
> the write.

I missed. I will correct it.

> 
> Also, how do you clear this state?

Here is the hardware trick comes. 
For enabling should write enable bit.
For disabling stop_req bit should set & it will clear enable bit also.

> 
> > +	return readw(info->mmio_base + PWMSS_CLKSTATUS);
> > +}
> > +EXPORT_SYMBOL(pwmss_submodule_state_change);
> > +
> > +static const struct of_device_id pwmss_of_match[] = {
> > +	{
> > +		.compatible	= "ti,am33xx-pwmss",
> > +	},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, pwmss_of_match);
> > +
> > +static int __devinit pwmss_probe(struct platform_device *pdev)
> 
> __dev* annotation usage is deprecated, you should drop it.

Ok

> 
> > +{
> > +	int ret;
> > +	struct resource *r;
> > +	struct pwmss_info *info;
> > +	struct device_node *node = pdev->dev.of_node;
> > +
> > +	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> > +	if (!info) {
> > +		dev_err(&pdev->dev, "failed to allocate memory\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	mutex_init(&info->pwmss_lock);
> > +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> 
> Blank line between those two lines?

Ok

> 
> > +	if (!r) {
> > +		dev_err(&pdev->dev, "no memory resource defined\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	info->mmio_base = devm_request_and_ioremap(&pdev->dev, r);
> > +	if (!info->mmio_base)
> > +		return -EADDRNOTAVAIL;
> > +
> > +	pm_runtime_enable(&pdev->dev);
> > +	pm_runtime_get_sync(&pdev->dev);
> > +	platform_set_drvdata(pdev, info);
> > +
> > +	/* Populate all the child nodes here... */
> > +	ret = of_platform_populate(node, NULL, NULL, &pdev->dev);
> > +	if (ret)
> > +		dev_warn(&pdev->dev, "Doesn't have any child node\n");
> 
> This reads oddly compared to the other error messages, maybe something
> like "no children found" or similar would be more consistent.

Ok

> 
> > +
> > +	return ret;
> 
> Then again, since you return of_platform_populate()'s error code here,
> you may just want to skip the above warning since the driver probe won't
> succeed anyway. Or if you really want to give a hint as to why loading
> failed, maybe it would be better to make it an error message instead.

Ok I will change to error message.

> 
> There is one little problem with registering the children here, which is
> that if you build the drivers as modules, because once the pwm-tipwmss
> module is unloaded, reloading it will fail since it will try to create
> the children again.
> 
> AFAICT there are two solutions to this: a) do not allow the pwm-tipwmss
> code to be built as a module and b) have of_platform_populate() called
> by the architecture initialization code. Both are relatively easy to
> implement. a) can be done by making the PWM_TIPWMSS symbol bool instead
> of tristate, and b) can be done by adding "simple-bus" to the end of the
> compatible list in the DT.

Ok I will go for option a.

> 
> > +}
> > +
> > +static int __devexit pwmss_remove(struct platform_device *pdev)
> 
> Again, no need for __devexit anymore.

I will remove

> 
...
> > +
> > +static const struct dev_pm_ops pwmss_pm_ops = {
> > +	.suspend	= pwmss_suspend,
> > +	.resume		= pwmss_resume,
> > +};
> 
> Shouldn't these functions be conditionalized on CONFIG_PM_SLEEP? And
> maybe you want to use the SIMPLE_DEV_PM_OPS macro here.

I will check and correct it.

> 
> > +
> > +static struct platform_driver pwmss_driver = {
> > +	.driver	= {
> > +		.name	= "pwmss",
> > +		.owner	= THIS_MODULE,
> > +		.pm	= &pwmss_pm_ops,
> > +		.of_match_table	= of_match_ptr(pwmss_of_match),
> 
> You already define the pwmss_of_match table unconditionally, so you
> don't need the of_match_ptr() either.

Will remove

> 
> > +	},
> > +	.probe	= pwmss_probe,
> > +	.remove	= __devexit_p(pwmss_remove),
> 
> __devexit_p() can go away.

Will remove

> 
> > +};
> > +
> > +module_platform_driver(pwmss_driver);
> > +
> > +MODULE_DESCRIPTION("pwmss driver");
> 
> This description could be better.

Will add.
> 
> > +MODULE_AUTHOR("Texas Instruments");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/pwm/tipwmss.h b/drivers/pwm/tipwmss.h
> > new file mode 100644
> > index 0000000..f9cb2e2
> > --- /dev/null
> > +++ b/drivers/pwm/tipwmss.h
> 
> I think this should also get the pwm- prefix for consistency with the
> source file.

Will add the prefix.

> 
> > @@ -0,0 +1,30 @@
> > +/*
> > + * TI PWM Subsystem driver
> > + *
> > + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
> > + *
> > + * 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.
> > + *
> > + */
> > +
> > +#ifndef __TIPWMSS_H
> > +#define __TIPWMSS_H
> > +
> > +#ifdef CONFIG_PWM_TIPWMSS
> > +extern u16 pwmss_submodule_state_change(struct device *dev, int set);
> > +#else
> > +static inline u16 pwmss_submodule_state_change(struct device *dev, int set)
> > +{
> > +	/* return success status value */
> > +	return 0xFFFF;
> > +}
> > +#endif
> > +#endif	/* __TIPWMSS_H */
> 
> Is it really necessary to provide a !PWM_TIPWMSS version of this
> function? All users that want to use it can select it and get the
> correct version, right?

Added the !PWM_TIPWMSS part to support platforms that didn't
have PWM Subsytem support.


I will send next version on nov 19th as I was on leave on next week.

Thanks
Avinash
> 
> Thierry
> 

^ permalink raw reply

* [PATCH v2 08/10] pwm: pwm-tiehrpwm: Adding TBCLK gating support.
From: Philip, Avinash @ 2012-11-09 10:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121109081104.GB26007@avionic-0098.mockup.avionic-design.de>

On Fri, Nov 09, 2012 at 13:41:04, Thierry Reding wrote:
> On Thu, Nov 08, 2012 at 01:23:15PM +0530, Philip, Avinash wrote:
...
> > +	/* Some platforms require explicit tbclk gating */
> > +	if (of_property_read_bool(pdev->dev.of_node, "tbclkgating")) {
> 
> Is it really necessary to have an extra boolean property for this?
> Couldn't this just be handled by not defining a clock for the tbclk
> consumer in board setup/DT

If no clk node defined, driver can still continue expecting this platform 
the requirement is not there. So I will check for tbclk with devm_clk_get()
and continue by removing Boolean property.

> 
> > +		pc->tbclk = clk_get(&pdev->dev, "tbclk");
> 
> You should be using devm_clk_get() or add a matching clk_put() in
> .remove().

Ok

Thanks
Avinash

> 
> Thierry
> 

^ permalink raw reply

* [PATCH v2 04/10] pwm: pwm-tiecap: Add device-tree binding support for APWM driver
From: Philip, Avinash @ 2012-11-09 10:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121109075219.GA22224@avionic-0098.mockup.avionic-design.de>

On Fri, Nov 09, 2012 at 13:22:19, Thierry Reding wrote:
> On Thu, Nov 08, 2012 at 01:23:11PM +0530, Philip, Avinash wrote:
> > This patch
> > 1. Add support for device-tree binding for ECAP APWM driver.
> > 2. Set size of pwm-cells set to 3 to support PWM channel number, PWM
> >    period & polarity configuration from device tree.
> > 3. Add enable/disable clock gating in PWM subsystem common config space.
> > 4. When here set .owner member in platform_driver structure to
> >    THIS_MODULE.
> > 
> > Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
> > Cc:	Grant Likely <grant.likely@secretlab.ca>
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: Rob Landley <rob@landley.net>
> > ---
> > Changes since v1:
> > 	- Add separate patch for pinctrl support
> > 	- Add conditional check for PWM subsystem clock enable.
> > 	- Combined with HWMOD changes & DT bindings.
> > 	- Remove the custom of xlate support.
> > 
> > :000000 100644 0000000... fe24cac... A	Documentation/devicetree/bindings/pwm/pwm-tiecap.txt
> > :100644 100644 d6d4cf0... 0d43266... M	drivers/pwm/pwm-tiecap.c
> >  .../devicetree/bindings/pwm/pwm-tiecap.txt         |   22 +++++++++
> >  drivers/pwm/pwm-tiecap.c                           |   48 +++++++++++++++++++-
> >  2 files changed, 69 insertions(+), 1 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-tiecap.txt b/Documentation/devicetree/bindings/pwm/pwm-tiecap.txt
> > new file mode 100644
> > index 0000000..fe24cac
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-tiecap.txt
> > @@ -0,0 +1,22 @@
> > +TI SOC ECAP based APWM controller
> > +
> > +Required properties:
> > +- compatible: Must be "ti,am33xx-ecap"
> > +- #pwm-cells: Should be 3. Number of cells being used to specify PWM property.
> > +  First cell specifies the per-chip index of the PWM to use, the second
> > +  cell is the period cycle in nanoseconds and bit 0 in the third cell is
> 
> I think this should be "period in nanoseconds". I haven't heard "period
> cycle" before.

Ok

> 
> > +  used to encode the polarity of PWM output.
> 
> Maybe you should explicitly say how this is encoded.

Ok I will add details

> 
...
> >  
> > +#define ECAPCLK_EN			BIT(0)
> > +#define ECAPCLK_STOP_REQ	BIT(1)
> 
> This one doesn't seem to align with the rest. Also, why is bit 0 called
> _EN and bit 1 _STOP_REQ? Couldn't they be made more consistent, i.e.
> _START and _STOP? Or _ENABLE and _DISABLE?

Ok I will change to PWMSS_ECAPCLK_EN & PWMSS_ECAPCLK_STPO_REQ

> 
> > +
> > +#define ECAPCLK_EN_ACK		BIT(0)
> > +
> > +#define PWM_CELL_SIZE		3
> 
> You don't need a define for this.

I remove.

> 
> > +
> >  struct ecap_pwm_chip {
> >  	struct pwm_chip	chip;
> >  	unsigned int	clk_rate;
> > @@ -184,6 +194,16 @@ static const struct pwm_ops ecap_pwm_ops = {
> >  	.owner		= THIS_MODULE,
> >  };
> >  
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id ecap_of_match[] = {
> > +	{
> > +		.compatible	= "ti,am33xx-ecap",
> > +	},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, ecap_of_match);
> > +#endif
> > +
> 
> I'm not sure if I remember correctly, but wasn't AM33xx support supposed
> to be DT only? In that case you don't need the CONFIG_OF guards.

I will remove

> 
...
> >  	pm_runtime_enable(&pdev->dev);
> > +	pm_runtime_get_sync(&pdev->dev);
> 
> Maybe put a blank line after this for readability.

Ok

> 
> > +	if (!(pwmss_submodule_state_change(pdev->dev.parent, ECAPCLK_EN) &
> > +				ECAPCLK_EN_ACK)) {
> 
> This is very hard to read, can you split this up into something like the
> following please?
> 
> 	status = pwmss_submodule_state_change(pdev->dev.parent, ECAPCLK_EN);
> 	if (!(status & ECAPCLK_EN_ACK)) {
> 		...
> 	}
> 

Ok I will correct it.

> > +		dev_err(&pdev->dev, "PWMSS config space clock enable failure\n");
> > +		ret = -EINVAL;
> > +		goto pwmss_clk_failure;
> > +	}
> > +	pm_runtime_put_sync(&pdev->dev);
> 
> Another blank line between the two above would be good.

Ok

> 
...
> > +		.owner	= THIS_MODULE,
> > +		.of_match_table = of_match_ptr(ecap_of_match),
> 
> Here as well, if AM33xx is DT-only, then the of_match_ptr() can be
> dropped.

Ok I will drop.

Thanks
Avinash
> 
> >  	},
> >  	.probe = ecap_pwm_probe,
> >  	.remove = __devexit_p(ecap_pwm_remove),
> 
> No __devexit_p() please.
> 
> Thierry
> 

^ permalink raw reply

* [PATCH v2 1/6] ARM: EXYNOS5: Clock settings for SATA and SATA PHY
From: Kukjin Kim @ 2012-11-09 11:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1351612897-14923-2-git-send-email-vasanthananthan@gmail.com>

Vasanth Ananthan wrote:
> 
> This patch adds neccessary clock entries for SATA, SATA PHY and
> I2C_SATAPHY
> 
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
>  arch/arm/mach-exynos/clock-exynos5.c |   21 ++++++++++++++++++---
>  1 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/clock-exynos5.c b/arch/arm/mach-
> exynos/clock-exynos5.c
> index c44ca1e..124c54f 100644
> --- a/arch/arm/mach-exynos/clock-exynos5.c
> +++ b/arch/arm/mach-exynos/clock-exynos5.c
> @@ -651,15 +651,20 @@ static struct clk exynos5_init_clocks_off[] = {
>  		.ctrlbit	= (1 << 15),
>  	}, {
>  		.name		= "sata",
> -		.devname	= "ahci",
> +		.devname	= "exynos5-sata",
> +		.parent         = &exynos5_clk_aclk_200.clk,
>  		.enable		= exynos5_clk_ip_fsys_ctrl,
>  		.ctrlbit	= (1 << 6),
>  	}, {
> -		.name		= "sata_phy",
> +		.name		= "sata-phy",
> +		.devname	= "exynos5-sata-phy",
> +		.parent         = &exynos5_clk_aclk_200.clk,
>  		.enable		= exynos5_clk_ip_fsys_ctrl,
>  		.ctrlbit	= (1 << 24),
>  	}, {
> -		.name		= "sata_phy_i2c",
> +		.name		= "i2c",
> +		.devname	= "exynos5-sata-phy-i2c",
> +		.parent         = &exynos5_clk_aclk_200.clk,
>  		.enable		= exynos5_clk_ip_fsys_ctrl,
>  		.ctrlbit	= (1 << 25),
>  	}, {
> @@ -1226,6 +1231,16 @@ static struct clksrc_clk exynos5_clksrcs[] = {
>  		.reg_div = { .reg = EXYNOS5_CLKDIV_TOP0, .shift = 24, .size
> = 3 },
>  	}, {
>  		.clk	= {
> +			.name		= "sclk_sata",
> +			.devname	= "exynos5-sata",
> +			.enable		= exynos5_clksrc_mask_fsys_ctrl,
> +			.ctrlbit	= (1 << 24),
> +		},
> +		.sources = &exynos5_clkset_aclk,
> +		.reg_src = { .reg = EXYNOS5_CLKSRC_FSYS, .shift = 24, .size
> = 1 },
> +		.reg_div = { .reg = EXYNOS5_CLKDIV_FSYS0, .shift = 20, .size
> = 4 },
> +	}, {
> +		.clk	= {
>  			.name		= "sclk_gscl_wrap",
>  			.devname	= "s5p-mipi-csis.0",
>  			.enable		= exynos5_clksrc_mask_gscl_ctrl,
> --
> 1.7.4.1

Looks OK to me, need to think again about naming though?, anyway, this can
be handled separate from this series.

Let me take this into Samsung tree, if any problems, please let me know.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

^ permalink raw reply

* S-o-b for 48efdd2 "Add a pm_power_off function that resets us, ..."
From: Dominic Cobley @ 2012-11-09 11:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <509C7328.9030104@wwwdotorg.org>

I'm happy to give permission for s.o.b. to be commits made by me. Is that sufficient?
If that's not sufficient can you give me git commands that will do what you want?

-----Original Message-----
From: Stephen Warren [mailto:swarren at wwwdotorg.org] 
Sent: 09 November 2012 03:06
To: Dominic Cobley
Cc: linux-rpi-kernel at lists.infradead.org; linux-arm-kernel at lists.infradead.org
Subject: Re: S-o-b for 48efdd2 "Add a pm_power_off function that resets us, ..."

On 10/27/2012 09:21 PM, Stephen Warren wrote:
> Dom,
> 
> Are you able to provide your S-o-b for the following commit:
> 
> 48efdd2 Add a pm_power_off function that resets us, and indicates to
> bootcode.bin not to reboot us. Should allow a lower power 'off' state
> 
> I would like to upstream that, but can't without an S-o-b [from you].
> 
> Thanks.

Dom, are you able to provide an S-o-b line for this or any other commits
in you kernel trees? It'd be really helpful if you could add S-o-bs to
any/all commits in your branches where you can. Thanks.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox