Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Locking in the clk API
From: Uwe Kleine-König @ 2011-01-12  7:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4D2D1F0D.5040208@codeaurora.org>

Hi Saravana,

On Tue, Jan 11, 2011 at 07:25:01PM -0800, Saravana Kannan wrote:
> On 01/11/2011 06:35 AM, Jeremy Kerr wrote:
> >Hi Paul,
> >
> >>Again, you are approaching it from the angle that an atomic clock is a
> >>special requirement rather than the default behaviour.
> >
> >I'm not considering it a special requirement, but it's still a requirement
> >(that the called function does not sleep).
> >
> >The problem with the inverse logic (clk_enable/clk_enable_sleepable) is that
> >now you've made the caller need to know what kind of clock it has, or might
> >have one day.
> 
> I think it's just a matter of how you interpret the name of the API
> in English. It doesn't make the decision making of the developer any
> easier.
> 
> Just having a _atomic suffix doesn't mean the driver developer
> doesn't need to know what type of clock it is. They are still making
> the assumption that the enable/disable for that clock can be done
> atomically -- namely an "atomic clock".
But there is a difference to 'one function to rule both sleepable and
atomic clocks'.  When calling _atomic on a sleepable clock you get
-ESOMETHING back (and the clock stays off).  With a generic clk_enable
you get an oops and so cannot handle the error.

> Similarly, when a driver developer calls the _sleepable APIs in
> their code, for all practical purposes, they are making an
> assumption that the enable/disable for that clock *needs to* (not
> may) sleep.
IMHO this is not right.  If the driver developer doesn't care if the
clock sleeps or not (which is the norm I think) he calls the _sleepable
function and if the clock happen to be an atomic one it doesn't hurt
him.

And looking at the usage of the sleeping functions in the gpio API, I'd
bet that at least 50% of the calls to gpio_set_value can/should be
gpio_set_value_cansleep.  That's because driver developers don't care or
are not aware of the issue.  If it would be gpio_set_value
vs.gpio_set_value_atomic most developers would use the sleeping variant
and the few that should use the _atomic function would notice that when
seeing the corresponding oops.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH v4 05/10] net/fec: add dual fec support for mx28
From: Uwe Kleine-König @ 2011-01-12  7:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4D2C5A55.7050502@snapgear.com>

Hi Greg,

On Tue, Jan 11, 2011 at 11:25:41PM +1000, Greg Ungerer wrote:
> On 11/01/11 23:07, Uwe Kleine-K?nig wrote:
> >On Tue, Jan 11, 2011 at 10:24:12PM +1000, Greg Ungerer wrote:
> >>On 11/01/11 20:27, Sascha Hauer wrote:
> >>>On Thu, Jan 06, 2011 at 03:13:13PM +0800, Shawn Guo wrote:
> >>>This option is used nowhere and should be removed. Certainly it does not
> >>>have the effect of enabling the second ethernet controller.
> >>
> >>It does for a ColdFire platform...
> >>
> >>grep -r CONFIG_FEC2 *
> >>
> >>arch/m68knommu/configs/m5275evb_defconfig:CONFIG_FEC2=y
> >>arch/m68knommu/platform/527x/config.c:#ifdef CONFIG_FEC2
> >>arch/m68knommu/platform/527x/config.c:#ifdef CONFIG_FEC2
> >IMHO Sascha's comment[1] applies here, too.
> 
> I am not arguing that it doesn't :-)
> 
> Simply that removing that config option and doing nothing
> else would not be the right thing to do.
> 
> Regards
> Greg
> 
> 
> >And someone might want to do what he suggested soon or the patch
> >removing CONFIG_FEC2[2] needs to be commented accordingly.
note that davem took the patch removing CONFIG_FEC2 now.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* BUG: spinlock recursion (sys_chdir, user_path_at, do_path_lookup ...)
From: Uwe Kleine-König @ 2011-01-12  7:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110111110539.GP24920@pengutronix.de>

Hello,

On Tue, Jan 11, 2011 at 12:05:39PM +0100, Uwe Kleine-K?nig wrote:
> when testing yesterday's Linus' master branch
> (a08948812b30653eb2c536ae613b635a989feb6f + some arch support including
> Trond's latest nfsfix[1]) I hit the following reproducibly:
> 
> [    5.580000] BUG: spinlock recursion on CPU#0, init/1
> [    5.580000]  lock: c7487e10, .magic: dead4ead, .owner: init/1, .owner_cpu: 0
> [    5.590000] Backtrace: 
> [    5.590000] [<c0037c2c>] (dump_backtrace+0x0/0x110) from [<c028240c>] (dump_stack+0x1c/0x20)
> [    5.600000]  r7:c7487e10 r6:c0321368 r5:c7487e10 r4:c7848000
> [    5.610000] [<c02823f0>] (dump_stack+0x0/0x20) from [<c01b516c>] (spin_bug+0x90/0xa4)
> [    5.620000] [<c01b50dc>] (spin_bug+0x0/0xa4) from [<c01b52d4>] (do_raw_spin_lock+0x50/0x154)
> [    5.620000]  r6:c7487e10 r5:c7487e10 r4:00000000
> [    5.630000] [<c01b5284>] (do_raw_spin_lock+0x0/0x154) from [<c028524c>] (_raw_spin_lock_nested+0x40/0x48)
> [    5.640000] [<c028520c>] (_raw_spin_lock_nested+0x0/0x48) from [<c00f436c>] (nameidata_dentry_drop_rcu+0x90/0x1a4)
> [    5.650000]  r5:c7843efc r4:c7487dc0
> [    5.650000] [<c00f42dc>] (nameidata_dentry_drop_rcu+0x0/0x1a4) from [<c00f44c0>] (d_revalidate+0x40/0x68)
> [    5.660000] [<c00f4480>] (d_revalidate+0x0/0x68) from [<c00f6ed4>] (link_path_walk+0xb84/0xbf0)
> [    5.670000]  r6:c7843efc r5:c7843efc r4:00000000
> [    5.680000] [<c00f6350>] (link_path_walk+0x0/0xbf0) from [<c00f7164>] (do_path_lookup+0x48/0xd4)
> [    5.680000] [<c00f711c>] (do_path_lookup+0x0/0xd4) from [<c00f7c08>] (user_path_at+0x64/0x9c)
> [    5.690000] [<c00f7ba4>] (user_path_at+0x0/0x9c) from [<c00e9614>] (sys_chdir+0x2c/0x78)
> [    5.700000]  r8:c0034108 r7:0000000c r6:be961ee4 r5:c7843f88 r4:00063015
> [    5.710000] [<c00e95e8>] (sys_chdir+0x0/0x78) from [<c0033e80>] (ret_fast_syscall+0x0/0x44)
> [    5.720000]  r5:be961ee4 r4:00063015
> [   11.720000] BUG: spinlock lockup on CPU#0, init/1, c7487e10
> [   11.730000] Backtrace: 
> [   11.730000] [<c0037c2c>] (dump_backtrace+0x0/0x110) from [<c028240c>] (dump_stack+0x1c/0x20)
> [   11.740000]  r7:c7842000 r6:c7487e10 r5:00000000 r4:00000000
> [   11.740000] [<c02823f0>] (dump_stack+0x0/0x20) from [<c01b539c>] (do_raw_spin_lock+0x118/0x154)
> [   11.750000] [<c01b5284>] (do_raw_spin_lock+0x0/0x154) from [<c028524c>] (_raw_spin_lock_nested+0x40/0x48)
> [   11.760000] [<c028520c>] (_raw_spin_lock_nested+0x0/0x48) from [<c00f436c>] (nameidata_dentry_drop_rcu+0x90/0x1a4)
> [   11.770000]  r5:c7843efc r4:c7487dc0
> [   11.780000] [<c00f42dc>] (nameidata_dentry_drop_rcu+0x0/0x1a4) from [<c00f44c0>] (d_revalidate+0x40/0x68)
> [   11.790000] [<c00f4480>] (d_revalidate+0x0/0x68) from [<c00f6ed4>] (link_path_walk+0xb84/0xbf0)
> [   11.790000]  r6:c7843efc r5:c7843efc r4:00000000
> [   11.800000] [<c00f6350>] (link_path_walk+0x0/0xbf0) from [<c00f7164>] (do_path_lookup+0x48/0xd4)
> [   11.810000] [<c00f711c>] (do_path_lookup+0x0/0xd4) from [<c00f7c08>] (user_path_at+0x64/0x9c)
> [   11.820000] [<c00f7ba4>] (user_path_at+0x0/0x9c) from [<c00e9614>] (sys_chdir+0x2c/0x78)
> [   11.820000]  r8:c0034108 r7:0000000c r6:be961ee4 r5:c7843f88 r4:00063015
> [   11.830000] [<c00e95e8>] (sys_chdir+0x0/0x78) from [<c0033e80>] (ret_fast_syscall+0x0/0x44)
> [   11.840000]  r5:be961ee4 r4:00063015
> [   75.280000] BUG: soft lockup - CPU#0 stuck for 64s! [init:1]
> [   75.280000] Modules linked in:
> [   75.280000] irq event stamp: 113662
> [   75.280000] hardirqs last  enabled at (113662): [<c0285a7c>] _raw_spin_unlock_irqrestore+0x48/0x50
> [   75.280000] hardirqs last disabled at (113661): [<c0285398>] _raw_spin_lock_irqsave+0x30/0x64
> [   75.280000] softirqs last  enabled at (113509): [<c026447c>] rpc_wake_up_next+0x1b0/0x1c4
> [   75.280000] softirqs last disabled at (113507): [<c02854f0>] _raw_spin_lock_bh+0x20/0x58
> [   75.280000] 
> [   75.280000] Pid: 1, comm:                 init
> [   75.280000] CPU: 0    Not tainted  (2.6.37-04021-gb8b018c-dirty #41)
> [   75.280000] PC is at do_raw_spin_lock+0xac/0x154
> [   75.280000] LR is at do_raw_spin_lock+0xc0/0x154
> [   75.280000] pc : [<c01b5330>]    lr : [<c01b5344>]    psr: 20000013
> [   75.280000] sp : c7843dd0  ip : c7843cd4  fp : c7843e04
> [   75.280000] r10: 06bd0000  r9 : 00000000  r8 : 00000000
> [   75.280000] r7 : c7842000  r6 : c7487e10  r5 : 00000000  r4 : 03dd5aca
> [   75.280000] r3 : 00000000  r2 : 00000001  r1 : c0285a74  r0 : 00000001
> [   75.280000] Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
> [   75.280000] Control: 0005317f  Table: 479a8000  DAC: 00000015
> [   75.280000] [<c00356c4>] (show_regs+0x0/0x54) from [<c0089dac>] (watchdog_timer_fn+0x13c/0x1a4)
> [   75.280000]  r4:c7842000
> [   75.280000] [<c0089c70>] (watchdog_timer_fn+0x0/0x1a4) from [<c006cb58>] (__run_hrtimer+0x114/0x1f0)
> [   75.280000] [<c006ca44>] (__run_hrtimer+0x0/0x1f0) from [<c006ced8>] (hrtimer_interrupt+0x154/0x338)
> [   75.280000] [<c006cd84>] (hrtimer_interrupt+0x0/0x338) from [<c003e36c>] (mxs_timer_interrupt+0x28/0x34)
> [   75.280000] [<c003e344>] (mxs_timer_interrupt+0x0/0x34) from [<c008a408>] (handle_IRQ_event+0x7c/0x1a8)
> [   75.280000] [<c008a38c>] (handle_IRQ_event+0x0/0x1a8) from [<c008c948>] (handle_level_irq+0xc8/0x148)
> [   75.280000] [<c008c880>] (handle_level_irq+0x0/0x148) from [<c002d320>] (asm_do_IRQ+0x80/0xa4)
> [   75.280000]  r7:c7842000 r6:c7487e10 r5:00000000 r4:00000030
> [   75.280000] [<c002d2a0>] (asm_do_IRQ+0x0/0xa4) from [<c0033ab8>] (__irq_svc+0x38/0x80)
> [   75.280000] Exception stack(0xc7843d88 to 0xc7843dd0)
> [   75.280000] 3d80:                   00000001 c0285a74 00000001 00000000 03dd5aca 00000000
> [   75.280000] 3da0: c7487e10 c7842000 00000000 00000000 06bd0000 c7843e04 c7843cd4 c7843dd0
> [   75.280000] 3dc0: c01b5344 c01b5330 20000013 ffffffff
> [   75.280000]  r5:f5000000 r4:ffffffff
> [   75.280000] [<c01b5284>] (do_raw_spin_lock+0x0/0x154) from [<c028524c>] (_raw_spin_lock_nested+0x40/0x48)
> [   75.280000] [<c028520c>] (_raw_spin_lock_nested+0x0/0x48) from [<c00f436c>] (nameidata_dentry_drop_rcu+0x90/0x1a4)
> [   75.280000]  r5:c7843efc r4:c7487dc0
> [   75.280000] [<c00f42dc>] (nameidata_dentry_drop_rcu+0x0/0x1a4) from [<c00f44c0>] (d_revalidate+0x40/0x68)
> [   75.280000] [<c00f4480>] (d_revalidate+0x0/0x68) from [<c00f6ed4>] (link_path_walk+0xb84/0xbf0)
> [   75.280000]  r6:c7843efc r5:c7843efc r4:00000000
> [   75.280000] [<c00f6350>] (link_path_walk+0x0/0xbf0) from [<c00f7164>] (do_path_lookup+0x48/0xd4)
> [   75.280000] [<c00f711c>] (do_path_lookup+0x0/0xd4) from [<c00f7c08>] (user_path_at+0x64/0x9c)
> [   75.280000] [<c00f7ba4>] (user_path_at+0x0/0x9c) from [<c00e9614>] (sys_chdir+0x2c/0x78)
> [   75.280000]  r8:c0034108 r7:0000000c r6:be961ee4 r5:c7843f88 r4:00063015
> [   75.280000] [<c00e95e8>] (sys_chdir+0x0/0x78) from [<c0033e80>] (ret_fast_syscall+0x0/0x44)
> [   75.280000]  r5:be961ee4 r4:00063015
> 
> I started to bisect, but already the first test case showed a different
> error (my getty dying every few seconds).
I bisected this one now, the first bad commit is

	9c0729d (x86: Eliminate bp argument from the stack tracing routines)

.  It made a x86 specific change to include/linux/stacktrace.h.

According to tglx the lockup above "is related to nicks scalability
stuff".  I havn't researched yet the offending commit.  Is that
necessary?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH v2] ARM: Change misleading warning when CONFIG_CMDLINE_FORCE is used
From: Uwe Kleine-König @ 2011-01-12  8:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1294789556-3870-1-git-send-email-holler@ahsoftware.de>

Hello,

On Wed, Jan 12, 2011 at 12:45:56AM +0100, Alexander Holler wrote:
> When CONFIG_CMDLINE_FORCE is used, the warning
> 
>   Ignoring unrecognised tag 0x54410009
> 
> was displayed. Change this to
> 
>   Ignoring tag cmdline (using the default kernel command line)
> 
> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
> ---
>  arch/arm/kernel/setup.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 336f14e..eed2425 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -649,15 +649,17 @@ static int __init parse_tag_revision(const struct tag *tag)
>  
>  __tagtable(ATAG_REVISION, parse_tag_revision);
>  
> -#ifndef CONFIG_CMDLINE_FORCE
>  static int __init parse_tag_cmdline(const struct tag *tag)
>  {
> +#ifndef CONFIG_CMDLINE_FORCE
>  	strlcpy(default_command_line, tag->u.cmdline.cmdline, COMMAND_LINE_SIZE);
> +#else
> +	pr_warning("Ignoring tag cmdline (using the default kernel command line)\n");
> +#endif /* CONFIG_CMDLINE_FORCE */
Does it make sense at all to disable CONFIG_CMDLINE_FORCE?  If not, I
suggest to remove it.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH] ARM: Thumb-2: Fix out-of-range offset for Thumb-2 in proc-v7.S
From: Uwe Kleine-König @ 2011-01-12  8:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1294790613-17101-1-git-send-email-dave.martin@linaro.org>

On Tue, Jan 11, 2011 at 06:03:33PM -0600, Dave Martin wrote:
> The following patch introduces a pre-increment addressing
> offset which is out of range for Thumb-2:
>     ARM: pgtable: switch order of Linux vs hardware page tables
a commit id would be nice here

Uwe
> 
> 162:	str	r3, [r0, #2048]!
> 
> Thumb-2 only permits offsets <256 for pre-increment addressing.
> 
> This patch replaces the store instruction with a suitable add-str
> pair for the Thumb-2 case.
> 
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
> KernelVersion: v2.6.37
> 
>  arch/arm/mm/proc-v7.S |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index b49fab2..0c1172b 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -159,7 +159,9 @@ ENTRY(cpu_v7_set_pte_ext)
>  	tstne	r1, #L_PTE_PRESENT
>  	moveq	r3, #0
>  
> -	str	r3, [r0, #2048]!
> + ARM(	str	r3, [r0, #2048]! )
> + THUMB(	add	r0, r0, #2048 )
> + THUMB(	str	r3, [r0] )
>  	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
>  #endif
>  	mov	pc, lr
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [RFC] ARM: Let GIC to route IRQs to any allowed CPUs
From: TAO HU @ 2011-01-12  8:18 UTC (permalink / raw)
  To: linux-arm-kernel



^ permalink raw reply

* [PATCH] watchdog: add support for the Synopsys DesignWare WDT
From: Jamie Iles @ 2011-01-12  8:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AANLkTikbxyCSX5eKiwtZyW+gYToTD7Jcucst6su2fEjN@mail.gmail.com>

Hi Viresh,

On Wed, Jan 12, 2011 at 10:46:28AM +0530, viresh kumar wrote:
> Jamie,
> 
> Sorry for sending comments on V5 !!

No problem!  All feedback appreciated!

> I don't know why, but checkpatch used to give few errors which it is
> not giving now.
> Like:
> - Mixing spaces and tabs
> - Line over 80 columns.
> 
> There are few places in this patch where i have seen these issues, but
> checkpatch doesn't
> report them at all.

Hmm, the only lines over 80 chars are printk lines and I can't see any 
with mixed spaces and tabs...


> On Mon, Jan 10, 2011 at 9:29 PM, Jamie Iles <jamie@jamieiles.com> wrote:
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 8a3aa2f..62c607a 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -331,6 +331,15 @@ config IMX2_WDT
> > ? ? ? ? ?To compile this driver as a module, choose M here: the
> > ? ? ? ? ?module will be called imx2_wdt.
> >
> > +config DW_WATCHDOG
> > + ? ? ? tristate "Synopsys DesignWare watchdog"
> > + ? ? ? depends on ARM
> 
> Correct me if i am wrong, but can't this peripheral be used for any
> other architecture.
> We are adding it as a platform device and not amba device.
> Might be synopsys give some other interface here, other than AMBA.

Well this is an APB peripheral so I think that it is fair to have the 
dependency on ARM for now.  If someone wants to use it on another 
architecture then we can easily widen this if we need to.

> > + ? ? ? help
> > + ? ? ? ? Say Y here if to include support for the Synopsys DesignWare
> > + ? ? ? ? watchdog timer found in many ARM chips.
> > + ? ? ? ? To compile this driver as a module, choose M here: the
> > + ? ? ? ? module will be called dw_wdt.
> > +
> > ?# AVR32 Architecture
> >
> 
> (...)
> 
> > diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> > new file mode 100644
> > index 0000000..45fa1fb
> > --- /dev/null
> > +++ b/drivers/watchdog/dw_wdt.c
> > @@ -0,0 +1,405 @@
> > +/*
> > + * Copyright 2010-2011 Picochip Ltd., Jamie Iles
> > + * http://www.picochip.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 file implements a driver for the Synopsys DesignWare watchdog device
> > + * in the many ARM subsystems. The watchdog has 16 different timeout periods
> > + * and these are a function of the input clock frequency.
> > + *
> > + * The DesignWare watchdog cannot be stopped once it has been started so we
> > + * use a software timer to implement a ping that will keep the watchdog alive.
> > + * If we receive an expected close for the watchdog then we keep the timer
> > + * running, otherwise the timer is stopped and the watchdog will expire.
> > + */
> > +#define pr_fmt(fmt) "dw_wdt: " fmt
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/clk.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/fs.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/module.h>
> > +#include <linux/moduleparam.h>
> > +#include <linux/pm.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/timer.h>
> > +#include <linux/uaccess.h>
> > +#include <linux/watchdog.h>
> > +
> 
> can remove this blank line.

Ok, will do.

> > +#include <linux/platform_data/dw_wdt.h>
> > +
> > +#define WDOG_CONTROL_REG_OFFSET ? ? ? ? ? ? 0x00
> > +#define WDOG_CONTROL_REG_WDT_EN_MASK ? ? ? 0x01
> > +#define WDOG_TIMEOUT_RANGE_REG_OFFSET ? ? ? 0x04
> > +#define WDOG_CURRENT_COUNT_REG_OFFSET ? ? ? 0x08
> > +#define WDOG_COUNTER_RESTART_REG_OFFSET ? ? 0x0c
> > +#define WDOG_COUNTER_RESTART_KICK_VALUE ? ? ? ? ? ?0x76
> 
> I don't know why, but i have seen a lot of comment that people give to
> align macro's.
> Use tabs instead of spaces between macro name and its value. Also keep
> them aligned.
> I am not sure if its absolutely necessary.

Ahh, yes, there are the spaces.  I'll fix this up, they should be 
aligned.

> (...)
> 
> > +static int dw_wdt_set_top(unsigned top_s)
> > +{
> > + ? ? ? int i, top_val = -1;
> 
> We can assign top_val = DW_WDT_MAX_TOP, here and
> 
> > +
> > + ? ? ? /*
> > + ? ? ? ?* Iterate over the timeout values until we find the closest match. We
> > + ? ? ? ?* always look for >=.
> > + ? ? ? ?*/
> > + ? ? ? for (i = 0; i <= DW_WDT_MAX_TOP; ++i)
> > + ? ? ? ? ? ? ? if (dw_wdt_top_in_seconds(i) >= top_s) {
> > + ? ? ? ? ? ? ? ? ? ? ? top_val = i;
> > + ? ? ? ? ? ? ? ? ? ? ? break;
> > + ? ? ? ? ? ? ? }
> > +
> > + ? ? ? /*
> > + ? ? ? ?* If we didn't find a suitable value, it must have been too large. Go
> > + ? ? ? ?* with the biggest that we can.
> > + ? ? ? ?*/
> > + ? ? ? if (top_val < 0)
> > + ? ? ? ? ? ? ? top_val = DW_WDT_MAX_TOP;
> 
> remove this check.

That's cleaner.  Thanks.

> > + ? ? ? /* Set the new value in the watchdog. */
> > + ? ? ? writel(top_val, dw_wdt.regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
> > +
> > + ? ? ? dw_wdt_set_next_heartbeat();
> > +
> > + ? ? ? return dw_wdt_top_in_seconds(top_val);
> > +}
> > +
> 
> (...)
> 
> > +static int __devinit dw_wdt_drv_probe(struct platform_device *pdev)
> > +{
> > + ? ? ? int ret;
> > + ? ? ? struct resource *mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + ? ? ? struct dw_wdt_platform_data *pdata = pdev->dev.platform_data;
> > +
> > + ? ? ? if (!mem)
> > + ? ? ? ? ? ? ? return -EINVAL;
> > +
> > + ? ? ? if (!devm_request_mem_region(&pdev->dev, mem->start, resource_size(mem),
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"dw_wdt"))
> > + ? ? ? ? ? ? ? return -ENOMEM;
> > +
> > + ? ? ? dw_wdt.regs = devm_ioremap(&pdev->dev, mem->start,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?resource_size(mem));
> 
> this can come in single line.

Ok.

> > + ? ? ? if (!dw_wdt.regs)
> > + ? ? ? ? ? ? ? return -ENOMEM;
> > +
> > + ? ? ? dw_wdt.clk = clk_get(&pdev->dev, NULL);
> > + ? ? ? if (IS_ERR(dw_wdt.clk))
> > + ? ? ? ? ? ? ? return -ENODEV;
> 
> shouldn't we use the error value returned from clk_get here instead of defining
> a new value -ENODEV?

Yes, that's probably better.

> > +
> > + ? ? ? ret = clk_enable(dw_wdt.clk);
> 
> should we call this routine if clk is NULL? Probably we will get error here.
> But as we discussed earlier, we should support machines who don't support clk
> framework. So, shouldn't we check clk for NULL here and call this
> routine only if we have a valid clk pointer, otherwise continue 
> instead of returning error.

I think this is right, it's perfectly valid for a platform to return 
NULL as a clk and you can pass that to clk_enable().

> > + ? ? ? if (ret)
> > + ? ? ? ? ? ? ? goto out_put_clk;
> > +
> > + ? ? ? /*
> > + ? ? ? ?* The timeout period of the watchdog is derived from the input clock
> > + ? ? ? ?* frequency. ?For platforms that don't have a clk for the watchdog,
> > + ? ? ? ?* they can specify the WDT clock rate through the clk_rate field of
> > + ? ? ? ?* the struct dw_wdt_platform_data platform data.
> > + ? ? ? ?*/
> > + ? ? ? if (pdata && pdata->clk_rate > 0)
> > + ? ? ? ? ? ? ? dw_wdt.clk_rate = pdata->clk_rate;
> > + ? ? ? else
> > + ? ? ? ? ? ? ? dw_wdt.clk_rate = clk_get_rate(dw_wdt.clk);
> > +
> 
> I know it doesn't make sense for a platform to have support for clk framework
> and pass rate through plat data. But this code will always take 
> pdata->rate, if it is passed.
> Wouldn't it be better if we reverse the sequence.
> 
> if (clk)
>  rate = clk_get_rate(...);
> else {
>  pdata = pdev->dev.platform_data;
>  if (pdata)
>   rate = pdata->rate;
> }
> 
> and then following code

That's what I did first (without the test that clk is not NULL) but 
switched it the other way round so that the platform can override the 
frequency by setting platform_data->clk_rate to nonzero.  That just 
seems a little more flexible and easy for testing but I'm happy to 
switch the order if you feel that's important.

> > + ? ? ? if (!dw_wdt.clk_rate) {
> > + ? ? ? ? ? ? ? dev_err(&pdev->dev, "no clk rate defined for watchdog, cannot enable\n");
> > + ? ? ? ? ? ? ? ret = -EINVAL;
> > + ? ? ? ? ? ? ? goto out_disable_clk;
> > + ? ? ? }
> > +
> > + ? ? ? spin_lock_init(&dw_wdt.lock);
> > +
> > + ? ? ? ret = misc_register(&dw_wdt_miscdev);
> > + ? ? ? if (ret)
> > + ? ? ? ? ? ? ? goto out_put_clk;
> 
> shouldn't it be out_disable_clk??

Yes, thanks.

> > +
> > + ? ? ? dw_wdt_set_next_heartbeat();
> > + ? ? ? setup_timer(&dw_wdt.timer, dw_wdt_ping, 0);
> > + ? ? ? mod_timer(&dw_wdt.timer, jiffies + WDT_TIMEOUT);
> > +
> > + ? ? ? return 0;
> > +
> > +out_disable_clk:
> > + ? ? ? clk_disable(dw_wdt.clk);
> > +out_put_clk:
> > + ? ? ? clk_put(dw_wdt.clk);
> > +
> > + ? ? ? return ret;
> > +}
> > +
> > +static int __devexit dw_wdt_drv_remove(struct platform_device *pdev)
> > +{
> > + ? ? ? misc_deregister(&dw_wdt_miscdev);
> > +
> > + ? ? ? clk_disable(dw_wdt.clk);
> > + ? ? ? clk_put(dw_wdt.clk);
> > +
> > + ? ? ? return 0;
> > +}
> > +
> > +static struct platform_driver dw_wdt_driver = {
> > + ? ? ? .probe ? ? ? ? ?= dw_wdt_drv_probe,
> > + ? ? ? .remove ? ? ? ? = __devexit_p(dw_wdt_drv_remove),
> > + ? ? ? .driver ? ? ? ? = {
> > + ? ? ? ? ? ? ? .name ? = "dw_wdt",
> > + ? ? ? ? ? ? ? .owner ?= THIS_MODULE,
> > +#ifdef CONFIG_PM
> > + ? ? ? ? ? ? ? .pm ? ? = &dw_wdt_pm_ops,
> > +#endif /* CONFIG_PM */
> > + ? ? ? },
> > +};
> > +
> > +static int __init dw_wdt_watchdog_init(void)
> > +{
> > + ? ? ? return platform_driver_register(&dw_wdt_driver);
> > +}
> > +module_init(dw_wdt_watchdog_init);
> > +
> > +static void __exit dw_wdt_watchdog_exit(void)
> > +{
> > + ? ? ? platform_driver_unregister(&dw_wdt_driver);
> > +}
> > +module_exit(dw_wdt_watchdog_exit);
> > +
> > +MODULE_AUTHOR("Jamie Iles");
> > +MODULE_DESCRIPTION("Synopsys DesignWare Watchdog Driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
> > diff --git a/include/linux/platform_data/dw_wdt.h b/include/linux/platform_data/dw_wdt.h
> > new file mode 100644
> > index 0000000..0af10ef
> > --- /dev/null
> > +++ b/include/linux/platform_data/dw_wdt.h
> 
> It looks you have created a separate generic folder platform_data.
> But generally we put these files directly into existing folders.
> I am not quite sure if that's fine or we should avoid this.

Yes, this was suggested by Greg K-H to stop include/linux getting filled 
with platform data (https://lkml.org/lkml/2011/1/7/323).

Thanks again!

Jamie

^ permalink raw reply

* [PATCH v3 0/4] ARM: i.MX53: spi related patches
From: yong.shen at freescale.com @ 2011-01-12  8:35 UTC (permalink / raw)
  To: linux-arm-kernel


change log:
iomux setting name changes according to Uwe's comments.

^ permalink raw reply

* [PATCH v3 1/4] ARM: i.MX53: enable spi support
From: yong.shen at freescale.com @ 2011-01-12  8:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1294821324-5688-1-git-send-email-yong.shen@freescale.com>

From: Yong Shen <yong.shen@freescale.com>

1. register spi clock
2. add macro definitions for spi platform data
3. fix some bugs of register and iomux definition

Signed-off-by: Yong Shen <yong.shen@freescale.com>
---
 arch/arm/mach-mx5/clock-mx51-mx53.c          |    3 +++
 arch/arm/mach-mx5/devices-imx53.h            |    4 ++++
 arch/arm/plat-mxc/devices/platform-spi_imx.c |   12 ++++++++++++
 arch/arm/plat-mxc/include/mach/iomux-mx53.h  |    6 +++---
 arch/arm/plat-mxc/include/mach/mx53.h        |   10 +++++-----
 5 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index d0f58a3..a20d0c0 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -1330,6 +1330,9 @@ static struct clk_lookup mx53_lookups[] = {
 	_REGISTER_CLOCK("imx-i2c.1", NULL, i2c2_clk)
 	_REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk)
 	_REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_clk)
+	_REGISTER_CLOCK("imx53-ecspi.0", NULL, ecspi1_clk)
+	_REGISTER_CLOCK("imx53-ecspi.1", NULL, ecspi2_clk)
+	_REGISTER_CLOCK("imx53-cspi.0", NULL, cspi_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 f7c89ef..8639735 100644
--- a/arch/arm/mach-mx5/devices-imx53.h
+++ b/arch/arm/mach-mx5/devices-imx53.h
@@ -25,3 +25,7 @@ extern const struct imx_sdhci_esdhc_imx_data
 imx53_sdhci_esdhc_imx_data[] __initconst;
 #define imx53_add_sdhci_esdhc_imx(id, pdata)	\
 	imx_add_sdhci_esdhc_imx(&imx53_sdhci_esdhc_imx_data[id], pdata)
+
+extern const struct imx_spi_imx_data imx53_ecspi_data[] __initconst;
+#define imx53_add_ecspi(id, pdata)	\
+	imx_add_spi_imx(&imx53_ecspi_data[id], pdata)
diff --git a/arch/arm/plat-mxc/devices/platform-spi_imx.c b/arch/arm/plat-mxc/devices/platform-spi_imx.c
index 8ea49ad..013c85f 100644
--- a/arch/arm/plat-mxc/devices/platform-spi_imx.c
+++ b/arch/arm/plat-mxc/devices/platform-spi_imx.c
@@ -81,6 +81,18 @@ const struct imx_spi_imx_data imx51_ecspi_data[] __initconst = {
 };
 #endif /* ifdef CONFIG_SOC_IMX51 */
 
+#ifdef CONFIG_SOC_IMX53
+const struct imx_spi_imx_data imx53_cspi_data __initconst =
+	imx_spi_imx_data_entry_single(MX53, CSPI, "imx53-cspi", 0, , SZ_4K);
+
+const struct imx_spi_imx_data imx53_ecspi_data[] __initconst = {
+#define imx53_ecspi_data_entry(_id, _hwid)				\
+	imx_spi_imx_data_entry(MX53, ECSPI, "imx53-ecspi", _id, _hwid, SZ_4K)
+	imx53_ecspi_data_entry(0, 1),
+	imx53_ecspi_data_entry(1, 2),
+};
+#endif /* ifdef CONFIG_SOC_IMX53 */
+
 struct platform_device *__init imx_add_spi_imx(
 		const struct imx_spi_imx_data *data,
 		const struct spi_imx_master *pdata)
diff --git a/arch/arm/plat-mxc/include/mach/iomux-mx53.h b/arch/arm/plat-mxc/include/mach/iomux-mx53.h
index 2673007..0b3df33 100644
--- a/arch/arm/plat-mxc/include/mach/iomux-mx53.h
+++ b/arch/arm/plat-mxc/include/mach/iomux-mx53.h
@@ -131,9 +131,9 @@ typedef enum iomux_config {
 #define MX53_PAD_JTAG_TDO__JTAG_TDO		IOMUX_PAD(0x454, NON_MUX_I, IOMUX_CONFIG_ALT0, 0x0, 0, NO_PAD_CTRL)
 #define MX53_PAD_EIM_A25__GPIO_5_2		IOMUX_PAD(0x458, 0x110,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
 #define MX53_PAD_EIM_EB2__GPIO_2_30		IOMUX_PAD(0x45C, 0x114,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
-#define MX53_PAD_EIM_D16__GPIO_3_16		IOMUX_PAD(0x460, 0x118,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
-#define MX53_PAD_EIM_D17__GPIO_3_17		IOMUX_PAD(0x464, 0x11C,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
-#define MX53_PAD_EIM_D18__GPIO_3_18		IOMUX_PAD(0x468, 0x120,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
+#define MX53_PAD_EIM_D16__CSPI1_SCLK		IOMUX_PAD(0x460, 0x118,IOMUX_CONFIG_ALT4, 0x79c, 3, NO_PAD_CTRL)
+#define MX53_PAD_EIM_D17__CSPI1_MISO		IOMUX_PAD(0x464, 0x11C,IOMUX_CONFIG_ALT4, 0x7a0, 3, NO_PAD_CTRL)
+#define MX53_PAD_EIM_D18__CSPI1_MOSI		IOMUX_PAD(0x468, 0x120,IOMUX_CONFIG_ALT4, 0x7a4, 3, NO_PAD_CTRL)
 #define MX53_PAD_EIM_D19__GPIO_3_19		IOMUX_PAD(0x46C, 0x124,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
 #define MX53_PAD_EIM_D20__GPIO_3_20		IOMUX_PAD(0x470, 0x128,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
 #define MX53_PAD_EIM_D21__GPIO_3_21		IOMUX_PAD(0x474, 0x12C,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
diff --git a/arch/arm/plat-mxc/include/mach/mx53.h b/arch/arm/plat-mxc/include/mach/mx53.h
index 340937f..d7a8e52 100644
--- a/arch/arm/plat-mxc/include/mach/mx53.h
+++ b/arch/arm/plat-mxc/include/mach/mx53.h
@@ -56,7 +56,7 @@
 #define MX53_ESDHC1_BASE_ADDR	(MX53_SPBA0_BASE_ADDR + 0x00004000)
 #define MX53_ESDHC2_BASE_ADDR	(MX53_SPBA0_BASE_ADDR + 0x00008000)
 #define MX53_UART3_BASE_ADDR		(MX53_SPBA0_BASE_ADDR + 0x0000C000)
-#define MX53_CSPI1_BASE_ADDR		(MX53_SPBA0_BASE_ADDR + 0x00010000)
+#define MX53_ECSPI1_BASE_ADDR		(MX53_SPBA0_BASE_ADDR + 0x00010000)
 #define MX53_SSI2_BASE_ADDR		(MX53_SPBA0_BASE_ADDR + 0x00014000)
 #define MX53_ESDHC3_BASE_ADDR	(MX53_SPBA0_BASE_ADDR + 0x00020000)
 #define MX53_ESDHC4_BASE_ADDR	(MX53_SPBA0_BASE_ADDR + 0x00024000)
@@ -117,12 +117,12 @@
 #define MX53_ARM_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000A0000)
 #define MX53_OWIRE_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000A4000)
 #define MX53_FIRI_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000A8000)
-#define MX53_CSPI2_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000AC000)
+#define MX53_ECSPI2_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000AC000)
 #define MX53_SDMA_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000B0000)
 #define MX53_SCC_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000B4000)
 #define MX53_ROMCP_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000B8000)
 #define MX53_RTIC_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000BC000)
-#define MX53_CSPI3_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000C0000)
+#define MX53_CSPI_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000C0000)
 #define MX53_I2C2_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000C4000)
 #define MX53_I2C1_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000C8000)
 #define MX53_SSI1_BASE_ADDR	(MX53_AIPS2_BASE_ADDR + 0x000CC000)
@@ -264,8 +264,8 @@
 #define MX53_INT_UART3	33
 #define MX53_INT_RESV34	34
 #define MX53_INT_RESV35	35
-#define MX53_INT_CSPI1	36
-#define MX53_INT_CSPI2	37
+#define MX53_INT_ECSPI1	36
+#define MX53_INT_ECSPI2	37
 #define MX53_INT_CSPI	38
 #define MX53_INT_GPT	39
 #define MX53_INT_EPIT1	40
-- 
1.7.1

^ permalink raw reply related

* [PATCH v3 2/4] ARM: i.MX53 EVK: add spi device
From: yong.shen at freescale.com @ 2011-01-12  8:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1294821324-5688-1-git-send-email-yong.shen@freescale.com>

From: Yong Shen <yong.shen@freescale.com>

1. add platform data for spi device
2. Kconfig change

Signed-off-by: Yong Shen <yong.shen@freescale.com>
---
 arch/arm/mach-mx5/Kconfig          |    1 +
 arch/arm/mach-mx5/board-mx53_evk.c |   21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
index 23b0e3f..777740b 100644
--- a/arch/arm/mach-mx5/Kconfig
+++ b/arch/arm/mach-mx5/Kconfig
@@ -126,6 +126,7 @@ config MACH_MX53_EVK
 	select IMX_HAVE_PLATFORM_IMX_UART
 	select IMX_HAVE_PLATFORM_IMX_I2C
 	select IMX_HAVE_PLATFORM_SDHCI_ESDHC_IMX
+	select IMX_HAVE_PLATFORM_SPI_IMX
 	help
 	  Include support for MX53 EVK platform. This includes specific
 	  configurations for the board and its peripherals.
diff --git a/arch/arm/mach-mx5/board-mx53_evk.c b/arch/arm/mach-mx5/board-mx53_evk.c
index 4043451..157fe12 100644
--- a/arch/arm/mach-mx5/board-mx53_evk.c
+++ b/arch/arm/mach-mx5/board-mx53_evk.c
@@ -33,6 +33,8 @@
 #include <mach/iomux-mx53.h>
 
 #define SMD_FEC_PHY_RST		IMX_GPIO_NR(7, 6)
+#define EVK_ECSPI1_CS0		IMX_GPIO_NR(2, 30)
+#define EVK_ECSPI1_CS1		IMX_GPIO_NR(3, 19)
 
 #include "crm_regs.h"
 #include "devices-imx53.h"
@@ -52,6 +54,13 @@ static iomux_v3_cfg_t mx53_evk_pads[] = {
 	MX53_PAD_ATA_CS_1__UART3_RXD,
 	MX53_PAD_ATA_DA_1__UART3_CTS,
 	MX53_PAD_ATA_DA_2__UART3_RTS,
+
+	MX53_PAD_EIM_D16__CSPI1_SCLK,
+	MX53_PAD_EIM_D17__CSPI1_MISO,
+	MX53_PAD_EIM_D18__CSPI1_MOSI,
+
+	MX53_PAD_EIM_EB2__GPIO_2_30,
+	MX53_PAD_EIM_D19__GPIO_3_19,
 };
 
 static const struct imxuart_platform_data mx53_evk_uart_pdata __initconst = {
@@ -89,6 +98,16 @@ static struct fec_platform_data mx53_evk_fec_pdata = {
 	.phy = PHY_INTERFACE_MODE_RMII,
 };
 
+static int mx53_evk_spi_cs[] = {
+	EVK_ECSPI1_CS0,
+	EVK_ECSPI1_CS1,
+};
+
+static const struct spi_imx_master mx53_evk_spi_data __initconst = {
+	.chipselect     = mx53_evk_spi_cs,
+	.num_chipselect = ARRAY_SIZE(mx53_evk_spi_cs),
+};
+
 static void __init mx53_evk_board_init(void)
 {
 	mxc_iomux_v3_setup_multiple_pads(mx53_evk_pads,
@@ -102,6 +121,8 @@ static void __init mx53_evk_board_init(void)
 
 	imx53_add_sdhci_esdhc_imx(0, NULL);
 	imx53_add_sdhci_esdhc_imx(1, NULL);
+
+	imx53_add_ecspi(0, &mx53_evk_spi_data);
 }
 
 static void __init mx53_evk_timer_init(void)
-- 
1.7.1

^ permalink raw reply related

* [PATCH v3 3/4] ARM: i.MX53 EVK: add spi nor device
From: yong.shen at freescale.com @ 2011-01-12  8:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1294821324-5688-1-git-send-email-yong.shen@freescale.com>

From: Yong Shen <yong.shen@freescale.com>

add spi board information for spi nor device

Signed-off-by: Yong Shen <yong.shen@freescale.com>
---
 arch/arm/mach-mx5/board-mx53_evk.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/board-mx53_evk.c b/arch/arm/mach-mx5/board-mx53_evk.c
index 157fe12..f85f206 100644
--- a/arch/arm/mach-mx5/board-mx53_evk.c
+++ b/arch/arm/mach-mx5/board-mx53_evk.c
@@ -24,6 +24,8 @@
 #include <linux/fec.h>
 #include <linux/delay.h>
 #include <linux/gpio.h>
+#include <linux/spi/flash.h>
+#include <linux/spi/spi.h>
 #include <mach/common.h>
 #include <mach/hardware.h>
 #include <asm/mach-types.h>
@@ -98,6 +100,17 @@ static struct fec_platform_data mx53_evk_fec_pdata = {
 	.phy = PHY_INTERFACE_MODE_RMII,
 };
 
+static struct spi_board_info mx53_evk_spi_board_info[] __initdata = {
+	{
+		.modalias = "mtd_dataflash",
+		.max_speed_hz = 25000000,
+		.bus_num = 0,
+		.chip_select = 1,
+		.mode = SPI_MODE_0,
+		.platform_data = NULL,
+	},
+};
+
 static int mx53_evk_spi_cs[] = {
 	EVK_ECSPI1_CS0,
 	EVK_ECSPI1_CS1,
@@ -122,6 +135,8 @@ static void __init mx53_evk_board_init(void)
 	imx53_add_sdhci_esdhc_imx(0, NULL);
 	imx53_add_sdhci_esdhc_imx(1, NULL);
 
+	spi_register_board_info(mx53_evk_spi_board_info,
+		ARRAY_SIZE(mx53_evk_spi_board_info));
 	imx53_add_ecspi(0, &mx53_evk_spi_data);
 }
 
-- 
1.7.1

^ permalink raw reply related

* [PATCH v3 4/4] ARM: i.MX5: uart clock bug fix
From: yong.shen at freescale.com @ 2011-01-12  8:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1294821324-5688-1-git-send-email-yong.shen@freescale.com>

From: Yong Shen <yong.shen@freescale.com>

uart clk is from pll3 on mx53 instead of mx51

Signed-off-by: Yong Shen <yong.shen@freescale.com>
---
 arch/arm/mach-mx5/clock-mx51-mx53.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index a20d0c0..3fb7ff8 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -1370,7 +1370,6 @@ int __init mx51_clocks_init(unsigned long ckil, unsigned long osc,
 
 	clk_tree_init();
 
-	clk_set_parent(&uart_root_clk, &pll3_sw_clk);
 	clk_enable(&cpu_clk);
 	clk_enable(&main_bus_clk);
 
@@ -1413,6 +1412,7 @@ int __init mx53_clocks_init(unsigned long ckil, unsigned long osc,
 
 	clk_tree_init();
 
+	clk_set_parent(&uart_root_clk, &pll3_sw_clk);
 	clk_enable(&cpu_clk);
 	clk_enable(&main_bus_clk);
 
-- 
1.7.1

^ permalink raw reply related

* [RFC] ARM: Let GIC to route IRQs to any allowed CPUs
From: Russell King - ARM Linux @ 2011-01-12  8:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AANLkTimNMcs_6-NMTSY+yZuDUSB2eC2R_NX35gq00f7E@mail.gmail.com>

On Wed, Jan 12, 2011 at 04:18:51PM +0800, TAO HU wrote:
> The current logic will always route IRQs to the first allowed CPU
> Following two commands have the same result (all IRQs to CPU0)
> cmd1: $ echo 3 > proc/irq/xxx/smp_affinity
> cmd2: $ echo 1 > proc/irq/xxx/smp_affinity
> While below command will route all IRQs to CPU1
> cmd3: $ echo 2 > proc/irq/xxx/smp_affinity
> 
> With this patch, cmd1 will route IRQs to both CPU0 and CPU1
> Meanwhile, cmd2 and cmd3 will work as before

There's a reason we don't do this...

We don't want to deliver one IRQ to both CPU0 and CPU1 at the same time,
which is what will happen with this patch.  The GIC hardware doesn't
do IRQ balancing between CPUs.

The result will be CPUs fighting over servicing such interrupts.

^ permalink raw reply

* [PATCH] watchdog: add support for the Synopsys DesignWare WDT
From: viresh kumar @ 2011-01-12  8:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110112082416.GE3181@pulham.picochip.com>

Jamie,

On Wed, Jan 12, 2011 at 1:54 PM, Jamie Iles <jamie@jamieiles.com> wrote:
>> I don't know why, but checkpatch used to give few errors which it is
>> not giving now.
>> Like:
>> - Mixing spaces and tabs
>> - Line over 80 columns.
>>
>> There are few places in this patch where i have seen these issues, but
>> checkpatch doesn't
>> report them at all.
>
> Hmm, the only lines over 80 chars are printk lines and I can't see any
> with mixed spaces and tabs...

There are few in file drivers/watchdog/dw_wdt.c
line 40-45, 127, 133, 156, 167, 204, 216, 315, 319.

I can check it easily as i have used following in my vimrc file:
au FileType c match error /\s\+$\|\%>80v.\+\|[ ][ ]\+\|\n\n\n\+\|,[^
]\|^[ ]\+[^\*]\|(\s\+\|\s\+)/
au FileType c highlight error ctermbg=red guibg=red ctermfg=blue guifg=blue
au FileType h match error /\s\+$\|\%>80v.\+\|[ ][ ]\+\|\n\n\n\+\|,[^
]\|^[ ]\+[^\*]\|(\s\+\|\s\+)/
au FileType h highlight error ctermbg=red guibg=red ctermfg=blue guifg=blue

and so these sections are highlighted.

(...)

>> > + ? ? ? dw_wdt.clk = clk_get(&pdev->dev, NULL);
>> > + ? ? ? if (IS_ERR(dw_wdt.clk))
>> > + ? ? ? ? ? ? ? return -ENODEV;
>> > +
>> > + ? ? ? ret = clk_enable(dw_wdt.clk);
>>
>> should we call this routine if clk is NULL? Probably we will get error here.
>> But as we discussed earlier, we should support machines who don't support clk
>> framework. So, shouldn't we check clk for NULL here and call this
>> routine only if we have a valid clk pointer, otherwise continue
>> instead of returning error.
>
> I think this is right, it's perfectly valid for a platform to return
> NULL as a clk and you can pass that to clk_enable().
>

I agree on the first part: NULL can be returned from clk_get(), but
passing NULL to clk_enable and clk_get_rate doesn't look fine to me.
As they will always return error in that case and code will return from below
error check. That's why i suggest we should check for NULL before
clk_enable/disable and clk_get_rate.
What do you say?

>> > + ? ? ? if (ret)
>> > + ? ? ? ? ? ? ? goto out_put_clk;
>> > +
>> > + ? ? ? /*
>> > + ? ? ? ?* The timeout period of the watchdog is derived from the input clock
>> > + ? ? ? ?* frequency. ?For platforms that don't have a clk for the watchdog,
>> > + ? ? ? ?* they can specify the WDT clock rate through the clk_rate field of
>> > + ? ? ? ?* the struct dw_wdt_platform_data platform data.
>> > + ? ? ? ?*/
>> > + ? ? ? if (pdata && pdata->clk_rate > 0)
>> > + ? ? ? ? ? ? ? dw_wdt.clk_rate = pdata->clk_rate;
>> > + ? ? ? else
>> > + ? ? ? ? ? ? ? dw_wdt.clk_rate = clk_get_rate(dw_wdt.clk);
>> > +
>>
>> I know it doesn't make sense for a platform to have support for clk framework
>> and pass rate through plat data. But this code will always take
>> pdata->rate, if it is passed.
>> Wouldn't it be better if we reverse the sequence.
>>
>> if (clk)
>> ?rate = clk_get_rate(...);
>> else {
>> ?pdata = pdev->dev.platform_data;
>> ?if (pdata)
>> ? rate = pdata->rate;
>> }
>>
>> and then following code
>
> That's what I did first (without the test that clk is not NULL) but
> switched it the other way round so that the platform can override the
> frequency by setting platform_data->clk_rate to nonzero. ?That just
> seems a little more flexible and easy for testing but I'm happy to
> switch the order if you feel that's important.
>

Problem will occur if rate is dynamically changed and we are still believing
on platform code's clk_rate.
Would be better if we switch order. i.e. give priority to clk_get_rate over
pdata->rate.

>> > diff --git a/include/linux/platform_data/dw_wdt.h b/include/linux/platform_data/dw_wdt.h
>> > new file mode 100644
>> > index 0000000..0af10ef
>> > --- /dev/null
>> > +++ b/include/linux/platform_data/dw_wdt.h
>>
>> It looks you have created a separate generic folder platform_data.
>> But generally we put these files directly into existing folders.
>> I am not quite sure if that's fine or we should avoid this.
>
> Yes, this was suggested by Greg K-H to stop include/linux getting filled
> with platform data (https://lkml.org/lkml/2011/1/7/323).
>

Oh. That's good.

--
viresh
ST Microelectronics

^ permalink raw reply

* Locking in the clk API
From: Russell King - ARM Linux @ 2011-01-12  9:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4D2D184A.8020405@codeaurora.org>

On Tue, Jan 11, 2011 at 06:56:10PM -0800, Saravana Kannan wrote:
> On 01/11/2011 01:16 AM, Russell King - ARM Linux wrote:
>> On Tue, Jan 11, 2011 at 10:16:42AM +0800, Jeremy Kerr wrote:
>>> At present, we can satisfy these with:
>>>
>>> * clk_enable: may sleep
>>
>> I object to this as one of the purposes behind the clk API is to allow
>> power savings to be made, and unless we can perform clk enable/disable
>> from atomic contexts, the best you can do is enable the clock when the
>> device is probed and disable it when it's released.
>
> Since dev_pm_ops.suspend is not atomic anymore (am I wrong?), what is  

That never has been, and that is called for the _system_ going into
suspend.  That's not what I'm talking about.  I'm talking about drivers
doing their own power management in response to (a) their knowledge of
how the device behaves and (b) in response to the user state they know.

In the case of a UART, that means enabling the clock when the user opens
the device and turning it off when the user closes the device - and in
the case of the UART being used as the system console, enabled when
printk calls it, disabled when finished outputting the message.

The latter is a case where you're called in atomic context.

Another case - a storage device.  While you may receive open/close calls,
these are meaningless for power management - you'll receive an open call
when you mount a filesystem, and a close call when you finally unmount it.
That doesn't mean it's going to be used.  Your request handler will
generally run in an atomic context, so in order to do dynamic power
saving you need to be able to enable/disable clocks in that context.

A touchscreen controller which you communicate with over a dedicated bus.
The touchscreen controller doesn't require the host to be constantly
clocked - it only needs to be clocked when communicating with the
touchscreen.  The touchscreen controller raises an interrupt for service.
You'd want to enable the clock in the interrupt handler to communicate
with the device and turn it off afterwards.

Shall I continue?

^ permalink raw reply

* [RFC] ARM: Let GIC to route IRQs to any allowed CPUs
From: TAO HU @ 2011-01-12  9:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110112085110.GR11039@n2100.arm.linux.org.uk>

Hi, Russell King

Thanks.

For "The result will be CPUs fighting over servicing such interrupts."
What's the consequence? The performance downgrade?

Does it imply a design flaw in GIC or in ARM SMP?

On Wed, Jan 12, 2011 at 4:51 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jan 12, 2011 at 04:18:51PM +0800, TAO HU wrote:
>> The current logic will always route IRQs to the first allowed CPU
>> Following two commands have the same result (all IRQs to CPU0)
>> cmd1: $ echo 3 > proc/irq/xxx/smp_affinity
>> cmd2: $ echo 1 > proc/irq/xxx/smp_affinity
>> While below command will route all IRQs to CPU1
>> cmd3: $ echo 2 > proc/irq/xxx/smp_affinity
>>
>> With this patch, cmd1 will route IRQs to both CPU0 and CPU1
>> Meanwhile, cmd2 and cmd3 will work as before
>
> There's a reason we don't do this...
>
> We don't want to deliver one IRQ to both CPU0 and CPU1 at the same time,
> which is what will happen with this patch. ?The GIC hardware doesn't
> do IRQ balancing between CPUs.
>
> The result will be CPUs fighting over servicing such interrupts.
>



-- 
Best Regards
Hu Tao

^ permalink raw reply

* [PATCH] watchdog: add support for the Synopsys DesignWare WDT
From: Russell King - ARM Linux @ 2011-01-12  9:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AANLkTim2LJVA+qJNqYd2uPNKjP54kuyvsZ7-T3jSJsta@mail.gmail.com>

On Wed, Jan 12, 2011 at 02:27:47PM +0530, viresh kumar wrote:
> > I think this is right, it's perfectly valid for a platform to return
> > NULL as a clk and you can pass that to clk_enable().
> >
> 
> I agree on the first part: NULL can be returned from clk_get(), but
> passing NULL to clk_enable and clk_get_rate doesn't look fine to me.

Whatever non-error cookie clk_get() passes out, clk_enable() etc must
eat, even if it contains dead flies rather than currants.  It's not
for the driver to make decisions on what clk_enable() et.al. accept,
other than as documented by the clk API - which says that errors from
clk_get() are indicated by IS_ERR(clk).

If IS_ERR(clk) is false, the driver _must_ assume that there is no error
and clk_enable() etc will accept the clk.

^ permalink raw reply

* ARM i.MX53 enable SMD&LOCO board bootup
From: yong.shen at freescale.com @ 2011-01-12  9:14 UTC (permalink / raw)
  To: linux-arm-kernel

change log:
remove gpio_set_value(SMD_FEC_PHY_RST, 0) according to discussion.

^ permalink raw reply

* [PATCH v2 1/2] ARM i.MX53 enable SMD board bootup
From: yong.shen at freescale.com @ 2011-01-12  9:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1294823687-20520-1-git-send-email-yong.shen@freescale.com>

From: Yong Shen <yong.shen@freescale.com>

1. Add Kconfig and Makefile entries
2. Add board definition
3. enable uart and fec for SMD board

Signed-off-by: Yong Shen <yong.shen@freescale.com>
---
 arch/arm/mach-mx5/Kconfig          |    7 ++
 arch/arm/mach-mx5/Makefile         |    1 +
 arch/arm/mach-mx5/board-mx53_smd.c |  111 ++++++++++++++++++++++++++++++++++++
 3 files changed, 119 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-mx5/board-mx53_smd.c

diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
index 777740b..f9706df 100644
--- a/arch/arm/mach-mx5/Kconfig
+++ b/arch/arm/mach-mx5/Kconfig
@@ -131,6 +131,13 @@ config MACH_MX53_EVK
 	  Include support for MX53 EVK platform. This includes specific
 	  configurations for the board and its peripherals.
 
+config MACH_MX53_SMD
+	bool "Support MX53 SMD platforms"
+	select SOC_IMX53
+	select IMX_HAVE_PLATFORM_IMX_UART
+	help
+	  Include support for MX53 SMD platform. This includes specific
+	  configurations for the board and its peripherals.
 
 config MACH_MX50_RDP
 	bool "Support MX50 reference design platform"
diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
index 0c398ba..895bfe3 100644
--- a/arch/arm/mach-mx5/Makefile
+++ b/arch/arm/mach-mx5/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_CPU_FREQ_IMX)    += cpu_op-mx51.o
 obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o
 obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o
 obj-$(CONFIG_MACH_MX53_EVK) += board-mx53_evk.o
+obj-$(CONFIG_MACH_MX53_SMD) += board-mx53_smd.o
 obj-$(CONFIG_MACH_EUKREA_CPUIMX51) += board-cpuimx51.o
 obj-$(CONFIG_MACH_EUKREA_MBIMX51_BASEBOARD) += eukrea_mbimx51-baseboard.o
 obj-$(CONFIG_MACH_EUKREA_CPUIMX51SD) += board-cpuimx51sd.o
diff --git a/arch/arm/mach-mx5/board-mx53_smd.c b/arch/arm/mach-mx5/board-mx53_smd.c
new file mode 100644
index 0000000..7970f7a
--- /dev/null
+++ b/arch/arm/mach-mx5/board-mx53_smd.c
@@ -0,0 +1,111 @@
+/*
+ * 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/init.h>
+#include <linux/clk.h>
+#include <linux/fec.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+
+#include <mach/common.h>
+#include <mach/hardware.h>
+#include <mach/imx-uart.h>
+#include <mach/iomux-mx53.h>
+
+#include <asm/mach-types.h>
+#include <asm/mach/arch.h>
+#include <asm/mach/time.h>
+
+#include "crm_regs.h"
+#include "devices-imx53.h"
+
+#define SMD_FEC_PHY_RST		IMX_GPIO_NR(7, 6)
+
+static iomux_v3_cfg_t mx53_smd_pads[] = {
+	MX53_PAD_CSI0_D10__UART1_TXD,
+	MX53_PAD_CSI0_D11__UART1_RXD,
+	MX53_PAD_ATA_DIOW__UART1_TXD,
+	MX53_PAD_ATA_DMACK__UART1_RXD,
+
+	MX53_PAD_ATA_BUFFER_EN__UART2_RXD,
+	MX53_PAD_ATA_DMARQ__UART2_TXD,
+	MX53_PAD_ATA_DIOR__UART2_RTS,
+	MX53_PAD_ATA_INTRQ__UART2_CTS,
+
+	MX53_PAD_ATA_CS_0__UART3_TXD,
+	MX53_PAD_ATA_CS_1__UART3_RXD,
+	MX53_PAD_ATA_DA_1__UART3_CTS,
+	MX53_PAD_ATA_DA_2__UART3_RTS,
+};
+
+static const struct imxuart_platform_data mx53_smd_uart_data __initconst = {
+	.flags = IMXUART_HAVE_RTSCTS,
+};
+
+static inline void mx53_smd_init_uart(void)
+{
+	imx53_add_imx_uart(0, &mx53_smd_uart_data);
+	imx53_add_imx_uart(1, &mx53_smd_uart_data);
+	imx53_add_imx_uart(2, &mx53_smd_uart_data);
+}
+
+static inline void mx53_smd_fec_reset(void)
+{
+	int ret;
+
+	/* reset FEC PHY */
+	ret = gpio_request(SMD_FEC_PHY_RST, "fec-phy-reset");
+	if (ret) {
+		printk(KERN_ERR"failed to get GPIO_FEC_PHY_RESET: %d\n", ret);
+		return;
+	}
+	gpio_direction_output(SMD_FEC_PHY_RST, 0);
+	msleep(1);
+	gpio_set_value(SMD_FEC_PHY_RST, 1);
+}
+
+static struct fec_platform_data mx53_smd_fec_data = {
+	.phy = PHY_INTERFACE_MODE_RMII,
+};
+
+static void __init mx53_smd_board_init(void)
+{
+	mxc_iomux_v3_setup_multiple_pads(mx53_smd_pads,
+					ARRAY_SIZE(mx53_smd_pads));
+	mx53_smd_init_uart();
+	mx53_smd_fec_reset();
+	imx53_add_fec(&mx53_smd_fec_data);
+}
+
+static void __init mx53_smd_timer_init(void)
+{
+	mx53_clocks_init(32768, 24000000, 22579200, 0);
+}
+
+static struct sys_timer mx53_smd_timer = {
+	.init	= mx53_smd_timer_init,
+};
+
+MACHINE_START(MX53_SMD, "Freescale MX53 SMD Board")
+	.map_io = mx53_map_io,
+	.init_irq = mx53_init_irq,
+	.init_machine = mx53_smd_board_init,
+	.timer = &mx53_smd_timer,
+MACHINE_END
-- 
1.7.1

^ permalink raw reply related

* [PATCH v2 2/2] ARM i.MX53 enable LOCO board bootup
From: yong.shen at freescale.com @ 2011-01-12  9:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1294823687-20520-1-git-send-email-yong.shen@freescale.com>

From: Yong Shen <yong.shen@freescale.com>

1. Add Kconfig and Makefile entries
2. Add board definition
3. enable uart and fec for LOCO board

Signed-off-by: Yong Shen <yong.shen@freescale.com>
---
 arch/arm/mach-mx5/Kconfig           |    8 +++
 arch/arm/mach-mx5/Makefile          |    1 +
 arch/arm/mach-mx5/board-mx53_loco.c |  111 +++++++++++++++++++++++++++++++++++
 3 files changed, 120 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-mx5/board-mx53_loco.c

diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
index f9706df..8f4bf9b 100644
--- a/arch/arm/mach-mx5/Kconfig
+++ b/arch/arm/mach-mx5/Kconfig
@@ -139,6 +139,14 @@ config MACH_MX53_SMD
 	  Include support for MX53 SMD platform. This includes specific
 	  configurations for the board and its peripherals.
 
+config MACH_MX53_LOCO
+	bool "Support MX53 LOCO platforms"
+	select SOC_IMX53
+	select IMX_HAVE_PLATFORM_IMX_UART
+	help
+	  Include support for MX53 LOCO platform. This includes specific
+	  configurations for the board and its peripherals.
+
 config MACH_MX50_RDP
 	bool "Support MX50 reference design platform"
 	depends on BROKEN
diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
index 895bfe3..0d43be9 100644
--- a/arch/arm/mach-mx5/Makefile
+++ b/arch/arm/mach-mx5/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o
 obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o
 obj-$(CONFIG_MACH_MX53_EVK) += board-mx53_evk.o
 obj-$(CONFIG_MACH_MX53_SMD) += board-mx53_smd.o
+obj-$(CONFIG_MACH_MX53_LOCO) += board-mx53_loco.o
 obj-$(CONFIG_MACH_EUKREA_CPUIMX51) += board-cpuimx51.o
 obj-$(CONFIG_MACH_EUKREA_MBIMX51_BASEBOARD) += eukrea_mbimx51-baseboard.o
 obj-$(CONFIG_MACH_EUKREA_CPUIMX51SD) += board-cpuimx51sd.o
diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-mx5/board-mx53_loco.c
new file mode 100644
index 0000000..d1348e0
--- /dev/null
+++ b/arch/arm/mach-mx5/board-mx53_loco.c
@@ -0,0 +1,111 @@
+/*
+ * 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/init.h>
+#include <linux/clk.h>
+#include <linux/fec.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+
+#include <mach/common.h>
+#include <mach/hardware.h>
+#include <mach/imx-uart.h>
+#include <mach/iomux-mx53.h>
+
+#include <asm/mach-types.h>
+#include <asm/mach/arch.h>
+#include <asm/mach/time.h>
+
+#include "crm_regs.h"
+#include "devices-imx53.h"
+
+#define LOCO_FEC_PHY_RST		IMX_GPIO_NR(7, 6)
+
+static iomux_v3_cfg_t mx53_loco_pads[] = {
+	MX53_PAD_CSI0_D10__UART1_TXD,
+	MX53_PAD_CSI0_D11__UART1_RXD,
+	MX53_PAD_ATA_DIOW__UART1_TXD,
+	MX53_PAD_ATA_DMACK__UART1_RXD,
+
+	MX53_PAD_ATA_BUFFER_EN__UART2_RXD,
+	MX53_PAD_ATA_DMARQ__UART2_TXD,
+	MX53_PAD_ATA_DIOR__UART2_RTS,
+	MX53_PAD_ATA_INTRQ__UART2_CTS,
+
+	MX53_PAD_ATA_CS_0__UART3_TXD,
+	MX53_PAD_ATA_CS_1__UART3_RXD,
+	MX53_PAD_ATA_DA_1__UART3_CTS,
+	MX53_PAD_ATA_DA_2__UART3_RTS,
+};
+
+static const struct imxuart_platform_data mx53_loco_uart_data __initconst = {
+	.flags = IMXUART_HAVE_RTSCTS,
+};
+
+static inline void mx53_loco_init_uart(void)
+{
+	imx53_add_imx_uart(0, &mx53_loco_uart_data);
+	imx53_add_imx_uart(1, &mx53_loco_uart_data);
+	imx53_add_imx_uart(2, &mx53_loco_uart_data);
+}
+
+static inline void mx53_loco_fec_reset(void)
+{
+	int ret;
+
+	/* reset FEC PHY */
+	ret = gpio_request(LOCO_FEC_PHY_RST, "fec-phy-reset");
+	if (ret) {
+		printk(KERN_ERR"failed to get GPIO_FEC_PHY_RESET: %d\n", ret);
+		return;
+	}
+	gpio_direction_output(LOCO_FEC_PHY_RST, 0);
+	msleep(1);
+	gpio_set_value(LOCO_FEC_PHY_RST, 1);
+}
+
+static struct fec_platform_data mx53_loco_fec_data = {
+	.phy = PHY_INTERFACE_MODE_RMII,
+};
+
+static void __init mx53_loco_board_init(void)
+{
+	mxc_iomux_v3_setup_multiple_pads(mx53_loco_pads,
+					ARRAY_SIZE(mx53_loco_pads));
+	mx53_loco_init_uart();
+	mx53_loco_fec_reset();
+	imx53_add_fec(&mx53_loco_fec_data);
+}
+
+static void __init mx53_loco_timer_init(void)
+{
+	mx53_clocks_init(32768, 24000000, 0, 0);
+}
+
+static struct sys_timer mx53_loco_timer = {
+	.init	= mx53_loco_timer_init,
+};
+
+MACHINE_START(MX53_LOCO, "Freescale MX53 LOCO Board")
+	.map_io = mx53_map_io,
+	.init_irq = mx53_init_irq,
+	.init_machine = mx53_loco_board_init,
+	.timer = &mx53_loco_timer,
+MACHINE_END
-- 
1.7.1

^ permalink raw reply related

* [PATCH v2] ARM: Change misleading warning when CONFIG_CMDLINE_FORCE is used
From: Russell King - ARM Linux @ 2011-01-12  9:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110112081147.GA24920@pengutronix.de>

On Wed, Jan 12, 2011 at 09:11:47AM +0100, Uwe Kleine-K?nig wrote:
> > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > index 336f14e..eed2425 100644
> > --- a/arch/arm/kernel/setup.c
> > +++ b/arch/arm/kernel/setup.c
> > @@ -649,15 +649,17 @@ static int __init parse_tag_revision(const struct tag *tag)
> >  
> >  __tagtable(ATAG_REVISION, parse_tag_revision);
> >  
> > -#ifndef CONFIG_CMDLINE_FORCE
> >  static int __init parse_tag_cmdline(const struct tag *tag)
> >  {
> > +#ifndef CONFIG_CMDLINE_FORCE
> >  	strlcpy(default_command_line, tag->u.cmdline.cmdline, COMMAND_LINE_SIZE);
> > +#else
> > +	pr_warning("Ignoring tag cmdline (using the default kernel command line)\n");
> > +#endif /* CONFIG_CMDLINE_FORCE */
> Does it make sense at all to disable CONFIG_CMDLINE_FORCE?  If not, I
> suggest to remove it.

Eeh?  Do you understand what CONFIG_CMDLINE_FORCE is and what effect it
has?

When CONFIG_CMDLINE_FORCE is enabled, we ignore the command line passed
in from the boot loader and always use the built-in command line.  When
CONFIG_CMDLINE_FORCE is disabled, we will use the boot loader supplied
command line if present.

"Does it make sense at all to disable CONFIG_CMDLINE_FORCE" so, if you
always have it enabled, you can never pass in a command line from a boot
loader.  That's an absolutely idiotic idea.

_Most_ of us want to pass in command lines from the boot loader so we can
configure things.

Please, if you don't understand, ask questions rather than making
completely misleading review comments which may confuse patch submitters.

^ permalink raw reply

* [PATCH v2] ARM: Change misleading warning when CONFIG_CMDLINE_FORCE is used
From: Russell King - ARM Linux @ 2011-01-12  9:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1294789556-3870-1-git-send-email-holler@ahsoftware.de>

On Wed, Jan 12, 2011 at 12:45:56AM +0100, Alexander Holler wrote:
> When CONFIG_CMDLINE_FORCE is used, the warning
> 
>   Ignoring unrecognised tag 0x54410009
> 
> was displayed. Change this to
> 
>   Ignoring tag cmdline (using the default kernel command line)

Patch is fine, can you send it to the patch system please?

Thanks.

^ permalink raw reply

* [PATCH 0/0] RFC: ARM: Thumb-2: Symbol manipulation macros for function body copying
From: Russell King - ARM Linux @ 2011-01-12  9:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1294790551-17069-1-git-send-email-dave.martin@linaro.org>

On Tue, Jan 11, 2011 at 06:02:30PM -0600, Dave Martin wrote:
> To make it easier to deal with cases like this, I've had a
> go at writing some macros to make copying function bodies
> easier, while being robust for ARM and Thumb-2.

How about instead providing some infrastructure which coes the
copy too?  Something like:

#define copy_fn_to_sram(to, fn, size) ({		\
	__typeof__(fn) f;				\
	unsigned long ptr;				\
	__asm__("" : "=r" (ptr) : "0" (fn));		\
	memcpy(to, (void *)(ptr & ~1), size);		\
	ptr = (ptr & 1) | (unsigned long)to;		\
	__asm__("" : "=r" (f) : "0" (ptr));		\
	f;						\
})

Used by:
extern void my_func(int foo);
extern int my_func_size;

void call_my_func(void *to, int arg)
{
	void (*fn)(int);

	fn = copy_fn_to_sram(to, my_func, my_func_size);
	fn(arg);
}

Then if you need to fix the way the copies are done for some
architectural reason, there's only one place to do it.

I'm not sure asm/unified.h is the right place - I don't think this has
anything to do with the unified assembler syntax.  Please create a new
header for this.

^ permalink raw reply

* [PATCH v2] ARM: Change misleading warning when CONFIG_CMDLINE_FORCE is used
From: Uwe Kleine-König @ 2011-01-12  9:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110112091716.GU11039@n2100.arm.linux.org.uk>

Hello Russell,

On Wed, Jan 12, 2011 at 09:17:16AM +0000, Russell King - ARM Linux wrote:
> > Does it make sense at all to disable CONFIG_CMDLINE_FORCE?  If not, I
> > suggest to remove it.
> 
> Eeh?  Do you understand what CONFIG_CMDLINE_FORCE is and what effect it
> has?
> 
> When CONFIG_CMDLINE_FORCE is enabled, we ignore the command line passed
> in from the boot loader and always use the built-in command line.  When
> CONFIG_CMDLINE_FORCE is disabled, we will use the boot loader supplied
> command line if present.
> 
> "Does it make sense at all to disable CONFIG_CMDLINE_FORCE" so, if you
> always have it enabled, you can never pass in a command line from a boot
> loader.  That's an absolutely idiotic idea.
> 
> _Most_ of us want to pass in command lines from the boot loader so we can
> configure things.
ah, I thought the effect of CONFIG_CMDLINE_FORCE would be its inverse.
And ok, this is probably about broken boot loaders that I'm just not
used to.
 
> Please, if you don't understand, ask questions rather than making
> completely misleading review comments which may confuse patch submitters.
IMHO "Does it make sense at all to disable CONFIG_CMDLINE_FORCE?" is a
question, isn't it?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH v3 1/4] ARM: i.MX53: enable spi support
From: Uwe Kleine-König @ 2011-01-12  9:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1294821324-5688-2-git-send-email-yong.shen@freescale.com>

Hello Yong,

On Wed, Jan 12, 2011 at 04:35:21PM +0800, yong.shen at freescale.com wrote:
> --- a/arch/arm/plat-mxc/include/mach/iomux-mx53.h
> +++ b/arch/arm/plat-mxc/include/mach/iomux-mx53.h
> @@ -131,9 +131,9 @@ typedef enum iomux_config {
>  #define MX53_PAD_JTAG_TDO__JTAG_TDO		IOMUX_PAD(0x454, NON_MUX_I, IOMUX_CONFIG_ALT0, 0x0, 0, NO_PAD_CTRL)
>  #define MX53_PAD_EIM_A25__GPIO_5_2		IOMUX_PAD(0x458, 0x110,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
>  #define MX53_PAD_EIM_EB2__GPIO_2_30		IOMUX_PAD(0x45C, 0x114,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
> -#define MX53_PAD_EIM_D16__GPIO_3_16		IOMUX_PAD(0x460, 0x118,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
> -#define MX53_PAD_EIM_D17__GPIO_3_17		IOMUX_PAD(0x464, 0x11C,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
> -#define MX53_PAD_EIM_D18__GPIO_3_18		IOMUX_PAD(0x468, 0x120,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
> +#define MX53_PAD_EIM_D16__CSPI1_SCLK		IOMUX_PAD(0x460, 0x118,IOMUX_CONFIG_ALT4, 0x79c, 3, NO_PAD_CTRL)
> +#define MX53_PAD_EIM_D17__CSPI1_MISO		IOMUX_PAD(0x464, 0x11C,IOMUX_CONFIG_ALT4, 0x7a0, 3, NO_PAD_CTRL)
> +#define MX53_PAD_EIM_D18__CSPI1_MOSI		IOMUX_PAD(0x468, 0x120,IOMUX_CONFIG_ALT4, 0x7a4, 3, NO_PAD_CTRL)
>  #define MX53_PAD_EIM_D19__GPIO_3_19		IOMUX_PAD(0x46C, 0x124,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
>  #define MX53_PAD_EIM_D20__GPIO_3_20		IOMUX_PAD(0x470, 0x128,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
>  #define MX53_PAD_EIM_D21__GPIO_3_21		IOMUX_PAD(0x474, 0x12C,IOMUX_CONFIG_ALT1, 0x0, 0, NO_PAD_CTRL)
Why do you remove the GPIO definitions?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ 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