Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* MMC quirks relating to performance/lifetime.
From: Arnd Bergmann @ 2011-02-12 18:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AANLkTikxMKm-Xy5du2+MzrD2eLuvfO9hXrSzBKmAkdBm@mail.gmail.com>

On Saturday 12 February 2011 18:33:10 Andrei Warkentin wrote:
> On Sat, Feb 12, 2011 at 11:05 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Friday 11 February 2011 23:33:42 Andrei Warkentin wrote:
> >> On Wed, Feb 9, 2011 at 3:13 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> >> Yes, this is a Toshiba card. I've sent the patch as a reply to Linus' email.
> >>
> >> cid - 02010053454d3332479070cc51451d00
> >> csd - d00f 0032 0f59 03ff ffffffff92404000
> >> erase_size - 524288
> >> fwrev - 0x0
> >> hwrev - 0x0
> >> manfid - 0x000002
> >> name - SEM32G
> >> oemid - 0x0100
> >> preferred_erase_size - 2097152
> >
> 
> This is a Toshiba eMMC part. It is 32GB as far as the OS can see and access.

Ah, right, that explains all the values, which make sense for eMMC4
but not for SDHC ;-)

	Arnd

^ permalink raw reply

* [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption
From: Santosh Shilimkar @ 2011-02-12 17:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AANLkTimkqZ5datA+7shggbWz3+j=TDg5beUkQK=b1AmK@mail.gmail.com>

> -----Original Message-----
> From: Andrei Warkentin [mailto:andreiw at motorola.com]
> Sent: Saturday, February 12, 2011 11:20 PM
> To: Santosh Shilimkar
> Cc: linux-omap at vger.kernel.org; khilman at ti.com; tony at atomide.com;
> linux-arm-kernel at lists.infradead.org; Catalin Marinas
> Subject: Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
> operation can cause data corruption
>
[....]

>
> Can these PL310 errata fixes be made more generic? PL310 is present
> in
> non-OMAP platforms too, which lack the TI hypervisor. And these
> platforms might have the same PL310 rev, and suffer the same
> glitches.
> While ideally there is some kind of hypervisor_ops to modify the
> protected register, at the very least there should be the generic
> debug_write handling the  "I  can write all PL310 regs" case. If
> you're interested I have a patch someplace that tried to do this,
> hopefully I can still find it.

They are kind of generic. If you look at it, the only change
Which is arch specific is the implementation of "debug_writel" function.
Today this code is not in generic PL310 code, but
OMAP specific.

May be we can make this as exported function pointer, which
arch's can populate.

Will that work for you ?

Regards,
Santosh

^ permalink raw reply

* [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption
From: Andrei Warkentin @ 2011-02-12 17:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1297510187-31547-4-git-send-email-santosh.shilimkar@ti.com>

On Sat, Feb 12, 2011 at 5:29 AM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> PL310 implements the Clean & Invalidate by Way L2 cache maintenance
> operation (offset 0x7FC). This operation runs in background so that
> PL310 can handle normal accesses while it is in progress. Under very
> rare circumstances, due to this erratum, write data can be lost when
> PL310 treats a cacheable write transaction during a Clean & Invalidate
> by Way operation.
>
> Workaround:
> Disable Write-Back and Cache Linefill (Debug Control Register)
> Clean & Invalidate by Way (0x7FC)
> Re-enable Write-Back and Cache Linefill (Debug Control Register)
>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> ---
> ?arch/arm/Kconfig ? ? ? ? ? ?| ? 11 +++++++++++
> ?arch/arm/mach-omap2/Kconfig | ? ?1 +
> ?arch/arm/mm/cache-l2x0.c ? ?| ? 16 ++++++++++------
> ?3 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 5cff165..2e6b879 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1177,6 +1177,17 @@ config ARM_ERRATA_743622
> ? ? ? ? ?visible impact on the overall performance or power consumption of the
> ? ? ? ? ?processor.
>
> +config PL310_ERRATA_727915
> + ? ? ? bool "Background Clean & Invalidate by Way operation can cause data corruption"
> + ? ? ? depends on CACHE_L2X0 && ARCH_OMAP4
> + ? ? ? help
> + ? ? ? ? PL310 implements the Clean & Invalidate by Way L2 cache maintenance
> + ? ? ? ? operation (offset 0x7FC). This operation runs in background so that
> + ? ? ? ? PL310 can handle normal accesses while it is in progress. Under very
> + ? ? ? ? rare circumstances, due to this erratum, write data can be lost when
> + ? ? ? ? PL310 treats a cacheable write transaction during a Clean &
> + ? ? ? ? Invalidate by Way operation Note that this errata uses Texas
> + ? ? ? ? Instrument's secure monitor api to implement the work around.
> ?endmenu
>
> ?source "arch/arm/common/Kconfig"
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index f285dd7..1f0ff75 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -46,6 +46,7 @@ config ARCH_OMAP4
> ? ? ? ?select ARM_GIC
> ? ? ? ?select LOCAL_TIMERS
> ? ? ? ?select PL310_ERRATA_588369
> + ? ? ? select PL310_ERRATA_727915
> ? ? ? ?select ARM_ERRATA_720789
> ? ? ? ?select ARCH_HAS_OPP
> ? ? ? ?select PM_OPP if PM
> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index 170c9bb..c7c8fbe 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -67,7 +67,7 @@ static inline void l2x0_inv_line(unsigned long addr)
> ? ? ? ?writel_relaxed(addr, base + L2X0_INV_LINE_PA);
> ?}
>
> -#ifdef CONFIG_PL310_ERRATA_588369
> +#if defined(CONFIG_PL310_ERRATA_588369) || defined(CONFIG_PL310_ERRATA_727915)
> ?static void debug_writel(unsigned long val)
> ?{
> ? ? ? ?extern void omap_smc1(u32 fn, u32 arg);
> @@ -78,7 +78,14 @@ static void debug_writel(unsigned long val)
> ? ? ? ? */
> ? ? ? ?omap_smc1(0x100, val);
> ?}
> +#else
> +/* Optimised out for non-errata case */
> +static inline void debug_writel(unsigned long val)
> +{
> +}
> +#endif
>
> +#ifdef CONFIG_PL310_ERRATA_588369
> ?static inline void l2x0_flush_line(unsigned long addr)
> ?{
> ? ? ? ?void __iomem *base = l2x0_base;
> @@ -91,11 +98,6 @@ static inline void l2x0_flush_line(unsigned long addr)
> ?}
> ?#else
>
> -/* Optimised out for non-errata case */
> -static inline void debug_writel(unsigned long val)
> -{
> -}
> -
> ?static inline void l2x0_flush_line(unsigned long addr)
> ?{
> ? ? ? ?void __iomem *base = l2x0_base;
> @@ -119,9 +121,11 @@ static void l2x0_flush_all(void)
>
> ? ? ? ?/* clean all ways */
> ? ? ? ?spin_lock_irqsave(&l2x0_lock, flags);
> + ? ? ? debug_writel(0x03);
> ? ? ? ?writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_CLEAN_INV_WAY);
> ? ? ? ?cache_wait_way(l2x0_base + L2X0_CLEAN_INV_WAY, l2x0_way_mask);
> ? ? ? ?cache_sync();
> + ? ? ? debug_writel(0x00);
> ? ? ? ?spin_unlock_irqrestore(&l2x0_lock, flags);
> ?}
>
> --
> 1.6.0.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

Can these PL310 errata fixes be made more generic? PL310 is present in
non-OMAP platforms too, which lack the TI hypervisor. And these
platforms might have the same PL310 rev, and suffer the same glitches.
While ideally there is some kind of hypervisor_ops to modify the
protected register, at the very least there should be the generic
debug_write handling the  "I  can write all PL310 regs" case. If
you're interested I have a patch someplace that tried to do this,
hopefully I can still find it.

^ permalink raw reply

* reboot not working on linux-2.6.37 for ARMv7
From: shiraz hashim @ 2011-02-12 17:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110212173216.GC22402@n2100.arm.linux.org.uk>

On Sat, Feb 12, 2011 at 11:02 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sat, Feb 12, 2011 at 10:53:54PM +0530, shiraz hashim wrote:
>> Thanks, we got it. This was missing in linux-2.6.37 where the code was
>>
>> ?void smp_send_stop(void)
>> ?{
>> ? ? ? ? ?cpumask_t mask = cpu_online_map;
>> ? ? ? ? ?cpu_clear(smp_processor_id(), mask);
>> ? ? ? ? ?if (!cpus_empty(mask))
>> ? ? ? ? ? ? ? ? ?send_ipi_message(&mask, IPI_CPU_STOP);
>> ?}
>>
>> I would try this change and see if it works for me.
>
> Probably best to pull the patch out of latest git, which is:
>
> 28e18293cf0f8d23a0950d7b1d2212d11af494dc

Sure, thanks again.

-- 
regards
Shiraz Hashim

^ permalink raw reply

* MMC quirks relating to performance/lifetime.
From: Andrei Warkentin @ 2011-02-12 17:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201102121805.47311.arnd@arndb.de>

On Sat, Feb 12, 2011 at 11:05 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 11 February 2011 23:33:42 Andrei Warkentin wrote:
>> On Wed, Feb 9, 2011 at 3:13 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>
>> Yes, this is a Toshiba card. I've sent the patch as a reply to Linus' email.
>>
>> cid - 02010053454d3332479070cc51451d00
>> csd - d00f00320f5903ffffffffff92404000
>> erase_size - 524288
>> fwrev - 0x0
>> hwrev - 0x0
>> manfid - 0x000002
>> name - SEM32G
>> oemid - 0x0100
>> preferred_erase_size - 2097152
>
> Very interesting. So the manfid is the same as on most Kingston cards,
> but the oemid is different. Most cards have a two-letter ASCII code
> in there, 0x544d ("TM") on Kingston cards, and I always assumed that
> this stood for "Toshiba Memory".
>
> What is even stranger is the size value (among other fields) in the CSD,
> the card claims a size of exactly 32GB, which I find hard to believe,
> given that there are always some bad and reserved blocks.
>
> Are you sure that the card you have is authentic? I've heard a lot about
> fake USB sticks advertising a size that is much larger than the actual
> flash inside of them.
>
> Also this is the first card that I see advertise an allocation unit
> size of 2MB (preferred_erase_size), all other cards seem to advertise
> 4 MB these days, even if they actually have 2 or 8 MB.
>
> ? ? ? ?Arnd
>

This is a Toshiba eMMC part. It is 32GB as far as the OS can see and access.

^ permalink raw reply

* reboot not working on linux-2.6.37 for ARMv7
From: Russell King - ARM Linux @ 2011-02-12 17:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AANLkTimbQMrYqKj_FcfiOaYV=MJGpX=HvSvt-AVMdv0G@mail.gmail.com>

On Sat, Feb 12, 2011 at 10:53:54PM +0530, shiraz hashim wrote:
> Thanks, we got it. This was missing in linux-2.6.37 where the code was
> 
>  void smp_send_stop(void)
>  {
>          cpumask_t mask = cpu_online_map;
>          cpu_clear(smp_processor_id(), mask);
>          if (!cpus_empty(mask))
>                  send_ipi_message(&mask, IPI_CPU_STOP);
>  }
> 
> I would try this change and see if it works for me.

Probably best to pull the patch out of latest git, which is:

28e18293cf0f8d23a0950d7b1d2212d11af494dc

^ permalink raw reply

* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
From: Arnd Bergmann @ 2011-02-12 17:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110212183726.GC9821@S2100-06.ap.freescale.net>

On Saturday 12 February 2011 19:37:27 Shawn Guo wrote:
> > If you use a flattened device tree, you can avoid the need for
> > platform data by adding the phandle of the dma engine device to
> > a property of the mmc driver, along with the channel number inside
> > of that device. I think that would be the cleanest approach, but
> > some people still need to be convinced that changing drivers to
> > use fdt data is the right direction for ARM.
> > 
> Could we keep the current the approach for now and go to the cleanest
> way directly when Grant's dt patches get merged?

Sounds good to me. If you are planning to use device trees anyway,
I don't have a problem with leaving it your way in the meantime.

	Arnd

^ permalink raw reply

* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
From: Shawn Guo @ 2011-02-12 17:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201102120959.22707.arnd@arndb.de>

On Sat, Feb 12, 2011 at 09:59:22AM +0100, Arnd Bergmann wrote:
> On Saturday 12 February 2011 15:04:19 Shawn Guo wrote:
> > > and tmio_mmc.c more or less do what I'm suggesting you do instead.
> > > 
> > > Looking at sh_mmcif:
> > > 
> > >       host->chan_tx = dma_request_channel(mask, sh_mmcif_filter,
> > >                                           &pdata->dma->chan_priv_tx);
> > > 
> > > 
> > > This is the only place where dma engine specific data is used
> > > in the driver, and chan_priv_tx is part of the platform data, so the
> > > mmc driver can simply pass it down as a void pointer without knowing
> > > the type. The platform data as defined in the machine file ties
> > > both the dma controller and the mmc device together, but neither
> > > of the two drivers needs to know anything about the implementation
> > > of the other.
> > > 
> > Not really.  Though mmc does not need to know anything about dma
> > driver, dma knows that mmc driver has to pass .slave_id via
> > chan->private.  The snippet below is copied from shdma.
> > 
> >         struct sh_dmae_slave_config {
> >                 unsigned int                    slave_id;
> >                 dma_addr_t                      addr;
> >                 u32                             chcr;
> >                 char                            mid_rid;
> >         };
> > ---
> >         if (chan->private) {
> >                 /* The caller is holding dma_list_mutex */
> >                 struct sh_dmae_slave *param = chan->private;
> >                 clear_bit(param->slave_id, sh_dmae_slave_used);
> >         }
> > 
> > And it only works when slave_id is the first member of
> > sh_dmae_slave_config.
> >
> > For me, it's more natural to define device's dma related things like
> > dma channel id and irq as its resources than platform data.  So I
> > still maintain the current approach.
> 
> I'm not arguing against passing the data as platform data (well, not any
> more, but it was never the main objection anyway).
> 
> You are right that sh_dmae_slave_config contains all the private data
> for the DMA controller, and I have no objection to you doing the
> same. However, sh_mmcif.c does not know the defintion of
> sh_dmae_slave_config, it just gets it via a void pointer from the
> platform data and passes it to the dma_request_channel function via
> another void pointer. That function calls into the sh_dmae driver, which
> casts it back to struct sh_dmae_slave_config, and that is same that I'm
> suggesting you to do.
> 
> It should really be a trivial change to move your struct mxs_dma_data
> from struct mxs_mmc_host to your platform_data:
> 
> struct mxs_mmc_platform_data {
> 	int wp_gpio;	/* write protect pin */
> 	void *dma_data;
> };
> 
> static struct mxs_dma_data mxs_mmc_dma_data = {
> 	.chan_irq = MMC_DMA_IRQ,
> };
> 
> static struct mxs_mmc_platform_data mxs_mmc_pdata = {
> 	.wp_gpio = SOME_CONSTANT,
> 	.dma_data = &mxs_mmc_dma_data,
> }
> 
> Now only the platform definition needs to know about both
> mxs_mmc_platform_data and mxs_dma_data, while the dma engine driver
> and the mmc driver each just need to know about their own data.
> 
Well, we are removing inclusion of mach/dma.h from mmc driver, but
adding it to every mxs based machine code.  This makes mmc driver
clean but machine code becomes not.  For some dma client devices
coming later, the platform data could be saved at all, if they do not
have any.  But with the approach you are suggesting, every single
client device will have to get platform data.

Regards,
Shawn

^ permalink raw reply

* reboot not working on linux-2.6.37 for ARMv7
From: shiraz hashim @ 2011-02-12 17:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110212162148.GA22402@n2100.arm.linux.org.uk>

Hello Russell,

On Sat, Feb 12, 2011 at 9:51 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sat, Feb 12, 2011 at 09:45:45PM +0530, viresh kumar wrote:

[...]

>> We are not modifying flush_cache_all. L2 cache is not flushed from there.
>> Actually as soon as L1 Cache is flushed and disabled, spin_lock is taken from
>> some other thread. This creates problem for our platform, as we do not have
>> monitors support at L2 thus failing strex instruction.
>>
>> If we disable L2 then reboot works fine, as expected.
>
> Again, this should not be happening:
>
> void smp_send_stop(void)
> {
> ? ? ? ?unsigned long timeout;
>
> ? ? ? ?if (num_online_cpus() > 1) {
> ? ? ? ? ? ? ? ?cpumask_t mask = cpu_online_map;
> ? ? ? ? ? ? ? ?cpu_clear(smp_processor_id(), mask);
>
> ? ? ? ? ? ? ? ?smp_cross_call(&mask, IPI_CPU_STOP);
> ? ? ? ?}
>
> ? ? ? ?/* Wait up to one second for other CPUs to stop */
> ? ? ? ?timeout = USEC_PER_SEC;
> ? ? ? ?while (num_online_cpus() > 1 && timeout--)
> ? ? ? ? ? ? ? ?udelay(1);
>
> ? ? ? ?if (num_online_cpus() > 1)
> ? ? ? ? ? ? ? ?pr_warning("SMP: failed to stop secondary CPUs\n");
> }
>
> void machine_shutdown(void)
> {
> #ifdef CONFIG_SMP
> ? ? ? ?smp_send_stop();
> #endif
> }
>
> void machine_restart(char *cmd)
> {
> ? ? ? ?machine_shutdown();
> ? ? ? ?arm_pm_restart(reboot_mode, cmd);
> }
>
> smp_send_stop() brings the other CPUs down to a quiescent state, waits for
> up to one second for that to happen, and then starts flushing caches etc.
> No other CPU should be active at the point where arm_pm_restart() is
> called. ?There should be no other threads active on either this or any
> other CPU.

Thanks, we got it. This was missing in linux-2.6.37 where the code was

 void smp_send_stop(void)
 {
         cpumask_t mask = cpu_online_map;
         cpu_clear(smp_processor_id(), mask);
         if (!cpus_empty(mask))
                 send_ipi_message(&mask, IPI_CPU_STOP);
 }

I would try this change and see if it works for me.

-- 
regards
Shiraz Hashim

^ permalink raw reply

* [PATCH] ARM: SAMSUNG: Removing dependency on CONFIG_PM_DEBUG for clock debugging
From: Amit Daniel Kachhap @ 2011-02-12 17:06 UTC (permalink / raw)
  To: linux-arm-kernel

Enabling the macro CONFIG_PM_DEBUG is causing compilation error as all PM components
are included which is not in mainline for samsung V310 platform, therefore, this patch
removes the dependency on macro CONFIG_PM_DEBUG for clock debugging through debugfs
interface.

Signed-off-by: Amit Daniel Kachhap <amit.kachhap@linaro.org>
---
 arch/arm/plat-samsung/clock.c              |    4 ++--
 arch/arm/plat-samsung/include/plat/clock.h |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-samsung/clock.c b/arch/arm/plat-samsung/clock.c
index 7728928..aca5a25 100644
--- a/arch/arm/plat-samsung/clock.c
+++ b/arch/arm/plat-samsung/clock.c
@@ -450,7 +450,7 @@ int __init s3c24xx_register_baseclocks(unsigned long xtal)
 	return 0;
 }
 
-#if defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS)
+#if defined(CONFIG_DEBUG_FS)
 /* debugfs support to trace clock tree hierarchy and attributes */
 
 static struct dentry *clk_debugfs_root;
@@ -538,4 +538,4 @@ err_out:
 }
 late_initcall(clk_debugfs_init);
 
-#endif /* defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS) */
+#endif /* defined(CONFIG_DEBUG_FS) */
diff --git a/arch/arm/plat-samsung/include/plat/clock.h b/arch/arm/plat-samsung/include/plat/clock.h
index 9a82b88..f6180ab 100644
--- a/arch/arm/plat-samsung/include/plat/clock.h
+++ b/arch/arm/plat-samsung/include/plat/clock.h
@@ -47,7 +47,7 @@ struct clk {
 
 	struct clk_ops		*ops;
 	int		    (*enable)(struct clk *, int enable);
-#if defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS)
+#if defined(CONFIG_DEBUG_FS)
 	struct dentry		*dent;	/* For visible tree hierarchy */
 #endif
 };
-- 
1.7.1

^ permalink raw reply related

* MMC quirks relating to performance/lifetime.
From: Arnd Bergmann @ 2011-02-12 17:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AANLkTinaY2RM86tX4ex+zsMwFKUdBNVpdZ8fFrt-e12_@mail.gmail.com>

On Friday 11 February 2011 23:33:42 Andrei Warkentin wrote:
> On Wed, Feb 9, 2011 at 3:13 AM, Arnd Bergmann <arnd@arndb.de> wrote:

> Yes, this is a Toshiba card. I've sent the patch as a reply to Linus' email.
> 
> cid - 02010053454d3332479070cc51451d00
> csd - d00f00320f5903ffffffffff92404000
> erase_size - 524288
> fwrev - 0x0
> hwrev - 0x0
> manfid - 0x000002
> name - SEM32G
> oemid - 0x0100
> preferred_erase_size - 2097152

Very interesting. So the manfid is the same as on most Kingston cards,
but the oemid is different. Most cards have a two-letter ASCII code
in there, 0x544d ("TM") on Kingston cards, and I always assumed that
this stood for "Toshiba Memory".

What is even stranger is the size value (among other fields) in the CSD,
the card claims a size of exactly 32GB, which I find hard to believe,
given that there are always some bad and reserved blocks.

Are you sure that the card you have is authentic? I've heard a lot about
fake USB sticks advertising a size that is much larger than the actual
flash inside of them.

Also this is the first card that I see advertise an allocation unit
size of 2MB (preferred_erase_size), all other cards seem to advertise
4 MB these days, even if they actually have 2 or 8 MB.

	Arnd

^ permalink raw reply

* MMC quirks relating to performance/lifetime.
From: Russell King - ARM Linux @ 2011-02-12 16:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201102121728.32585.arnd@arndb.de>

On Sat, Feb 12, 2011 at 05:28:32PM +0100, Arnd Bergmann wrote:
> On Saturday 12 February 2011 11:59:18 Russell King - ARM Linux wrote:
> > Unrelated, I have a USB based device which provides an emulated FAT
> > filesystem - all files except one on this filesystem are read-only.
> > The writable file is a textual configuration file.  It can be reliably
> > updated by Windows based systems, but updates from Linux based systems
> > are ignored - presumably because updates to the FAT/directory/data
> > clusters are occuring in a different order.
> 
> Fun. I think qemu also comes with one of these FAT emulation layers,
> as do some mp3 players, but from what I have heard, they are not as
> broken.

Given that it is a secure GPS/barographic flight logger which has
approval for ratifing world record flight claims, you may understand why
it has to be extremely picky about how it interfaces with the external
world.  Especially restricting updates to modification of the
configuration file, while not allowing any of the logged data files to
be changed in any way.

^ permalink raw reply

* MMC quirks relating to performance/lifetime.
From: Arnd Bergmann @ 2011-02-12 16:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110212105918.GG15616@n2100.arm.linux.org.uk>

On Saturday 12 February 2011 11:59:18 Russell King - ARM Linux wrote:
> On Sat, Feb 12, 2011 at 11:45:41AM +0100, Arnd Bergmann wrote:
> > * The FAT location is clearly visible in a number of tests
> >   done inside of an allocation unit. It's normally slower for
> >   linear access, but faster for random access. Sometimes
> >   reading the FAT is also slower than reading elsewhere.
> 
> I wouldn't also be surprised if there's some cards out there which parse
> the FAT being written, and start activities (such as erasing clusters)
> based upon changes therein.  Such cards would be unsuitable for use with
> non-FAT filesystems.
> 
> It might be worth devising some sort of check for this kind of behaviour.

Possible, but doesn't seem to happen with any of the cards I have
tested, the controllers in there appear to be too simplistic.
Also, the recommendations for SD cards are to issue explicit erase
requests, which would make this unnecessary.

OTOH, SD cards do specify exactly where the FAT should be stored on
the medium, so it would be possible to make this kind of assumption.

USB sticks and CF cards might be smart enough to actually do it,
some of them have more sophisticated logic than SD cards (most
do not), and there is no usb mass storage command for erase.

> Unrelated, I have a USB based device which provides an emulated FAT
> filesystem - all files except one on this filesystem are read-only.
> The writable file is a textual configuration file.  It can be reliably
> updated by Windows based systems, but updates from Linux based systems
> are ignored - presumably because updates to the FAT/directory/data
> clusters are occuring in a different order.

Fun. I think qemu also comes with one of these FAT emulation layers,
as do some mp3 players, but from what I have heard, they are not as
broken.

	Arnd

^ permalink raw reply

* reboot not working on linux-2.6.37 for ARMv7
From: Russell King - ARM Linux @ 2011-02-12 16:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AANLkTikvBRJyDJb4=puFzbTCXNYGSEXDRkAZ=1PZuneD@mail.gmail.com>

On Sat, Feb 12, 2011 at 09:45:45PM +0530, viresh kumar wrote:
> On 2/11/11, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > On Fri, Feb 11, 2011 at 05:01:06PM +0530, shiraz hashim wrote:
> >> I am using linux-2.6.37 on our ARM Cortex A9 (dual core) SMP platform
> >> with PL310 as Level 2 cache. We observe that on reboot the control
> >> is not able to reach to the arch_reset.
> >>
> >> The problem happens when L1 cache is disabled in
> >> arm_machine_restart, through cpu_proc_fin() and L2 cache is flushed.
> >
> > Where's the L2 cache flush?  The sequence is:
> >
> >         /* Clean and invalidate caches */
> >         flush_cache_all();
> >         /* Turn off caching */
> >         cpu_proc_fin();
> >         /* Push out any further dirty data, and ensure cache is empty */
> >         flush_cache_all();
> >
> > and flush_cache_all() calls v7_flush_kern_cache_all() in
> > arch/arm/mm/cache-v7.S.
> >
> > I do hope you're not modifying flush_cache_all() to also call the L2
> > cache functions because that's wrong.
> >
> 
> Russell,
> 
> We are not modifying flush_cache_all. L2 cache is not flushed from there.
> Actually as soon as L1 Cache is flushed and disabled, spin_lock is taken from
> some other thread. This creates problem for our platform, as we do not have
> monitors support at L2 thus failing strex instruction.
> 
> If we disable L2 then reboot works fine, as expected.

Again, this should not be happening:

void smp_send_stop(void)
{
        unsigned long timeout;

        if (num_online_cpus() > 1) {
                cpumask_t mask = cpu_online_map;
                cpu_clear(smp_processor_id(), mask);

                smp_cross_call(&mask, IPI_CPU_STOP);
        }

        /* Wait up to one second for other CPUs to stop */
        timeout = USEC_PER_SEC;
        while (num_online_cpus() > 1 && timeout--)
                udelay(1);

        if (num_online_cpus() > 1)
                pr_warning("SMP: failed to stop secondary CPUs\n");
}

void machine_shutdown(void)
{
#ifdef CONFIG_SMP
        smp_send_stop();
#endif
}

void machine_restart(char *cmd)
{
        machine_shutdown();
        arm_pm_restart(reboot_mode, cmd);
}

smp_send_stop() brings the other CPUs down to a quiescent state, waits for
up to one second for that to happen, and then starts flushing caches etc.
No other CPU should be active at the point where arm_pm_restart() is
called.  There should be no other threads active on either this or any
other CPU.

^ permalink raw reply

* reboot not working on linux-2.6.37 for ARMv7
From: viresh kumar @ 2011-02-12 16:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110211120305.GD23404@n2100.arm.linux.org.uk>

On 2/11/11, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Fri, Feb 11, 2011 at 05:01:06PM +0530, shiraz hashim wrote:
>> I am using linux-2.6.37 on our ARM Cortex A9 (dual core) SMP platform
>> with PL310 as Level 2 cache. We observe that on reboot the control
>> is not able to reach to the arch_reset.
>>
>> The problem happens when L1 cache is disabled in
>> arm_machine_restart, through cpu_proc_fin() and L2 cache is flushed.
>
> Where's the L2 cache flush?  The sequence is:
>
>         /* Clean and invalidate caches */
>         flush_cache_all();
>         /* Turn off caching */
>         cpu_proc_fin();
>         /* Push out any further dirty data, and ensure cache is empty */
>         flush_cache_all();
>
> and flush_cache_all() calls v7_flush_kern_cache_all() in
> arch/arm/mm/cache-v7.S.
>
> I do hope you're not modifying flush_cache_all() to also call the L2
> cache functions because that's wrong.
>

Russell,

We are not modifying flush_cache_all. L2 cache is not flushed from there.
Actually as soon as L1 Cache is flushed and disabled, spin_lock is taken from
some other thread. This creates problem for our platform, as we do not have
monitors support at L2 thus failing strex instruction.

If we disable L2 then reboot works fine, as expected.

--
viresh

^ permalink raw reply

* [PATCH v3 2/5] ARM: pm: add generic CPU suspend/resume support
From: Santosh Shilimkar @ 2011-02-12 15:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110212145010.GK15616@n2100.arm.linux.org.uk>

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> Sent: Saturday, February 12, 2011 8:20 PM
> To: Santosh Shilimkar
> Cc: Colin Cross; Kukjin Kim; saeed bishara; linux-arm-
> kernel at lists.infradead.org
> Subject: Re: [PATCH v3 2/5] ARM: pm: add generic CPU suspend/resume
> support
>
> On Fri, Feb 11, 2011 at 05:37:04PM +0530, Santosh Shilimkar wrote:
> > There is a Monitor secure API, needs to be called from non-secure
> > software to set this diagnostic registers in resume path.
>
> It would be an idea to get the OMAP sleep code up to date so that I
> can
> look at OMAPs requirements for this to be useful.
>
> As the current code stands, I don't see any reason why the sleep34xx
> code
> can't use this infrastructure, but I'm loathed to start modifying
> that if there's outstanding code changes in that area.

Yep. There are few issues out there with sleep34xx code.
	- Secure APIs
	- Current code needs to be cleaned up to remove
	unwanted registers save restore
	- Some part of the code on OMAP3 must be run from
	SRAM. It can't run from DDR
	- AUXCTLR, Diagnostic registers aren't accessible
	in secure mode.
	- L2 cache needs to be handled with secure APIs.
	- Code sequence needs to handle errata's handling
	which accesses OMAP PM registers.

Few of the above are getting addressed for this merge window.

So my plan was to take a look at generic suspend after the
merge window. By that time your generic stuff and omap
cleanup would have got merged hopefully.

Regards,
Santosh

^ permalink raw reply

* [PATCH V3 0/6] pxa3xx_nand update series set
From: Lei Wen @ 2011-02-12 15:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AANLkTinZG8KBA-KumJm6ENi_Hfm2YCs1CHOgw-245L5c@mail.gmail.com>

Hi Eric,

On Sat, Feb 12, 2011 at 10:57 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
> On Sat, Dec 25, 2010 at 12:31 PM, Lei Wen <adrian.wenl@gmail.com> wrote:
>> Ping...
>>
>> Should we restart this review?...
>>
>
> Sure. Do you have any update to this patch series? If so,
> please post.
>

Nothing new for this patch set. Just wondering whether it could be accepted now
or need further refine. :(

Best regards,
Lei

^ permalink raw reply

* [PATCH V3 0/6] pxa3xx_nand update series set
From: Eric Miao @ 2011-02-12 14:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AANLkTinj0_+iHQCLiSt4aRZKeoK-VODnW+rcO3VOESv8@mail.gmail.com>

On Sat, Dec 25, 2010 at 12:31 PM, Lei Wen <adrian.wenl@gmail.com> wrote:
> Ping...
>
> Should we restart this review?...
>

Sure. Do you have any update to this patch series? If so,
please post.

^ permalink raw reply

* [PATCH v3 2/5] ARM: pm: add generic CPU suspend/resume support
From: Russell King - ARM Linux @ 2011-02-12 14:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <998315b4ab88b66cc0e32e73d10a0015@mail.gmail.com>

On Fri, Feb 11, 2011 at 05:37:04PM +0530, Santosh Shilimkar wrote:
> There is a Monitor secure API, needs to be called from non-secure
> software to set this diagnostic registers in resume path.

It would be an idea to get the OMAP sleep code up to date so that I can
look at OMAPs requirements for this to be useful.

As the current code stands, I don't see any reason why the sleep34xx code
can't use this infrastructure, but I'm loathed to start modifying that if
there's outstanding code changes in that area.

^ permalink raw reply

* [RFC PATCH 1/3] ARM: Add cpu power management notifiers
From: Russell King - ARM Linux @ 2011-02-12 14:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1297373487-23902-2-git-send-email-ccross@android.com>

On Thu, Feb 10, 2011 at 01:31:25PM -0800, Colin Cross wrote:
> +int cpu_pm_enter(void)
> +{
> +	int nr_calls;
> +	int ret;
> +
> +	spin_lock(&idle_notifier_lock);
> +	ret = __idle_notify(CPU_PM_ENTER, -1, &nr_calls);
> +	if (ret) {
> +		__idle_notify(CPU_PM_ENTER_FAILED, nr_calls - 1, NULL);
> +		spin_unlock(&idle_notifier_lock);
> +		return ret;
> +	}
> +	spin_unlock(&idle_notifier_lock);
> +
> +	return 0;

Wouldn't:

	spin_lock(&idle_notifier_lock);
	ret = __idle_notify(CPU_PM_ENTER, -1, &nr_calls);
	if (ret)
		__idle_notify(CPU_PM_ENTER_FAILED, nr_calls - 1, NULL);
	spin_unlock(&idle_notifier_lock);

	return ret;

be easier reading?

^ permalink raw reply

* [PATCH 1/2] atmel/macb: fix device name when SOFT/HARD_IRQ enabled
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-02-12 14:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4D53EEFA.3030801@ru.mvista.com>

On 16:58 Thu 10 Feb     , Sergei Shtylyov wrote:
> Hello.
> 
> On 09-02-2011 17:44, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> >>>From: Paul Chavent<paul.chavent@fnac.net>
> 
> >>>When listing processes on a system with SOFT/HARD_IRQ enabled,
> >>>the name of the ethernet device is [irq/eth%d] (instead of [irq/eth0] for example).
> 
> >>>This patch call the request_irq function after having initialized the name of the device.
> 
> >>>Signed-off-by: Paul Chavent<paul.chavent@fnac.net>
> >>>Signed-off-by: Nicolas Ferre<nicolas.ferre@atmel.com>
> >>>Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj@jcrosoft.com>
> >>[...]
> 
> >>>diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> >>>index f69e73e..d642e08 100644
> >>>--- a/drivers/net/macb.c
> >>>+++ b/drivers/net/macb.c
> >>[...]
> >>>@@ -1219,13 +1209,23 @@ static int __init macb_probe(struct platform_device *pdev)
> >>>  	err = register_netdev(dev);
> >>>  	if (err) {
> >>>  		dev_err(&pdev->dev, "Cannot register net device, aborting.\n");
> >>>-		goto err_out_free_irq;
> >>>+		goto err_out_iounmap;
> >>>  	}
> >>>
> >>>-	if (macb_mii_init(bp) != 0) {
> >>>+	dev->irq = platform_get_irq(pdev, 0);
> 
> >>    platform_get_irq() can fail...
> 
> >request_irq will fail too so do we really need to check it?
> 
>    You then will get the following printed:
> 
> macb: Unable to request IRQ -6 (error -22)
> 
>    If this is acceptable, then platfrom_get_irq()'s result can be ignored indeed.
yes it's

Best Regards,
J.

^ permalink raw reply

* [PATCH 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
From: Shawn Guo @ 2011-02-12 14:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201102112104.23643.arnd@arndb.de>

On Fri, Feb 11, 2011 at 09:04:23PM +0100, Arnd Bergmann wrote:
> On Saturday 12 February 2011 01:55:07 Shawn Guo wrote:
> > On Fri, Feb 11, 2011 at 04:51:35PM +0100, Arnd Bergmann wrote:
> > > I have not looked much at other dmaengine drivers, but I'd be
> > > surprised if they require the device driver to be written
> > > for a specific implementation. If that was the case, you would
> > > not even need a dmaengine API but could just as well write
> > > to the DMA controller registers from the device driver directly.
> > > 
> > We need a specific implementation, but it's not so specific that we
> > have to access dma controller directly.  Even it is, we still need
> > an API/interface, as there are so many client devices need to do the
> > same thing, right? ;)
> 
> I looked at all mmc drivers that use the dmaengine API: 
> atmel-mci.c does the same as what you propose here, while sh_mmcif.c

Actually, it's not me who propose the implementation.  Sascha sent a
patch (under review) to change mxcmmc.c to use generic damengine api
several weeks ago.  I followed that as the example.

> and tmio_mmc.c more or less do what I'm suggesting you do instead.
> 
> Looking at sh_mmcif:
> 
>       host->chan_tx = dma_request_channel(mask, sh_mmcif_filter,
>                                           &pdata->dma->chan_priv_tx);
> 
> 
> This is the only place where dma engine specific data is used
> in the driver, and chan_priv_tx is part of the platform data, so the
> mmc driver can simply pass it down as a void pointer without knowing
> the type. The platform data as defined in the machine file ties
> both the dma controller and the mmc device together, but neither
> of the two drivers needs to know anything about the implementation
> of the other.
> 
Not really.  Though mmc does not need to know anything about dma
driver, dma knows that mmc driver has to pass .slave_id via
chan->private.  The snippet below is copied from shdma.

	struct sh_dmae_slave_config {
		unsigned int                    slave_id;
		dma_addr_t                      addr;
		u32                             chcr;
		char                            mid_rid;
	};
---
        if (chan->private) {
                /* The caller is holding dma_list_mutex */
                struct sh_dmae_slave *param = chan->private;
                clear_bit(param->slave_id, sh_dmae_slave_used);
        }

And it only works when slave_id is the first member of
sh_dmae_slave_config.

For me, it's more natural to define device's dma related things like
dma channel id and irq as its resources than platform data.  So I
still maintain the current approach.

Regards,
Shawn

^ permalink raw reply

* TI DaVinci support under new maintainer: Sekhar Nori
From: Raffaele Recalcati @ 2011-02-12 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <87pqr2kx9d.fsf@ti.com>

Hi Kevin,

On Tue, Feb 8, 2011 at 10:54 PM, Kevin Hilman <khilman@ti.com> wrote:
> Hello,
>
> I will be stepping aside as maintainer of the TI DaVinci family of
> SoCs and Sekhar Nori from TI will be taking over these responsibilities.
>
> Sekhar has long been an active developer, primary contributor and
> reviewer so taking over the maintainer role is a logical next step for
> him.
>
> I will aid in the transition for a couple merge windows to help make a
> smooth transition, but will be fading away from an active role in
> davinci.

Your help has been very useful in the past to guide me to achieve a
small community role..
I can understand that you have new interests.
I think that Nori Sekhar has a deep experience and I'm happy for this decision.
See you in some other ml.

bye,
Raffaele
Raffaele

^ permalink raw reply

* [PATCH V2 3/4] ARM: Xilinx: base header files and assembly macros
From: Russell King - ARM Linux @ 2011-02-12 12:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e19bc079-5f8a-4622-b7cd-668cede3cb97@VA3EHSMHS032.ehs.local>

On Wed, Feb 09, 2011 at 10:00:44AM -0700, John Linn wrote:
> +#ifndef __MACH_VMALLOC_H__
> +#define __MACH_VMALLOC_H__
> +
> +#define VMALLOC_END       0xE0000000

Should have a UL suffix on this.

^ permalink raw reply

* [PATCH V2 1/4] ARM: Xilinx: Adding Xilinx board support
From: Russell King - ARM Linux @ 2011-02-12 12:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <0e8b21c6-60ff-4f82-a8e6-5c2a0fe80372@VA3EHSMHS005.ehs.local>

On Wed, Feb 09, 2011 at 10:00:42AM -0700, John Linn wrote:
> +/* arch/arm/mach-xilinx/board_ep107.c
> + *
> + * This file contains code specific to the Xilinx EP107 board.
> + *
> + *  Copyright (C) 2011 Xilinx
> + *
> + * based on /arch/arm/mach-realview/core.c
> + *
> + *  Copyright (C) 1999 - 2003 ARM Limited
> + *  Copyright (C) 2000 Deep Blue Solutions Ltd
> + *
> + * 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.
> + *
> + * 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

You probably don't want to include this paragraph - their address keeps
changing, so will require files to be constantly updated.

> + */
> +
> +#include <linux/platform_device.h>
> +#include <asm/mach-types.h>
> +#include <asm/mach/arch.h>
> +#include <mach/xilinx_soc.h>
> +#include <mach/irqs.h>
> +#include <linux/clkdev.h>

Please group linux/ includes together, then asm/ includes, lastly mach/
includes.

> @@ -0,0 +1,113 @@
> +/* arch/arm/mach-xilinx/common.c
...
> +/**
> + * system_init - System specific initialization, intended to be called from
> + *			board specific initialization.
> + *
> + **/
> +void __init system_init(void)

Is there a better name for this - maybe prefixing it with xilinx_ ?

> +{
> +#ifdef CONFIG_CACHE_L2X0
> +	/*
> +	 * 64KB way size, 8-way associativity, parity disabled
> +	 */
> +	l2x0_init(PL310_L2CC_BASE, 0x02060000, 0xF0F0FFFF);
> +#endif
> +}
> +
> +/**
> + * irq_init - Interrupt controller initialization for the GIC.
> + *
> + **/
> +void __init irq_init(void)

Ditto.

> +/**
> + * map_io - Create memory mappings needed for early I/O.
> + *
> + **/
> +void __init map_io(void)

Ditto.

^ 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