Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm: proc-v7: pc phy addresses before disable MMU
From: Mika Westerberg @ 2011-09-24  8:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316851370-7284-1-git-send-email-per.forlin@stericsson.com>

On Sat, Sep 24, 2011 at 10:02:50AM +0200, Per Forlin wrote:
> PC runs virtual addresses when calling cpu_v7_reset(). The MMU
> is switched off and "mov pc, r0" sets pc back to
> virtual addresses even though the MMU is switched off.
> This will result in a crash if the pipeline delay after
> MMU disable is one instruction. To prevent this set PC
> to physical addresses before disabling the MMU.
> 
> Signed-off-by: Per Fransson <per.xx.fransson@stericsson.com>
> Signed-off-by: Per Forlin <per.forlin@stericsson.com>
> ---
>  arch/arm/mm/proc-v7.S |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index 9049c07..f26e831 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -64,6 +64,7 @@ ENDPROC(cpu_v7_proc_fin)
>   */
>  	.align	5
>  ENTRY(cpu_v7_reset)
> +	sub	pc, pc, #PAGE_OFFSET+4		@ go to physical addresses

This only works on machines where PHYS_OFFSET is zero.

IIRC, there is a P<->V offset calculated at boot time. Maybe you could use
that instead here?

>  	mrc	p15, 0, r1, c1, c0, 0		@ ctrl register
>  	bic	r1, r1, #0x1			@ ...............m
>   THUMB(	bic	r1, r1, #1 << 30 )		@ SCTLR.TE (Thumb exceptions)
> -- 
> 1.6.3.3
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [GIT PULL] OMAP: Few sparse/bug fixes and clean-up for 3.2
From: Santosh Shilimkar @ 2011-09-24  8:22 UTC (permalink / raw)
  To: linux-arm-kernel

Tony,

Please pull few OMAP sparse/bug fixes and clean-up for 3.2 

Thnaks,
Santosh

The following changes since commit b6fd41e29dea9c6753b1843a77e50433e6123bcb:

  Linux 3.1-rc6 (2011-09-12 14:02:02 -0700)

are available in the git repository at:
  git://gitorious.org/omap-sw-develoment/linux-omap-dev.git for_3_2/omap_misc

Santosh Shilimkar (1):
      OMAP4: Fix the emif and dmm virtual mapping

Todd Poynor (2):
      OMAP: Improve register access in L3 Error handler.
      OMAP: Fix a BUG in l3 error handler.

sricharan (3):
      OMAP: Fix indentation issues in l3 error handler.
      OMAP: Fix sparse warnings in l3 error handler.
      OMAP: Print Initiator name for l3 custom error.

 arch/arm/mach-omap2/omap_l3_noc.c    |  130 ++++++++++----------
 arch/arm/mach-omap2/omap_l3_noc.h    |  224 +++++++++++++++++++---------------
 arch/arm/mach-omap2/omap_l3_smx.c    |   91 +++++++-------
 arch/arm/mach-omap2/omap_l3_smx.h    |  164 ++++++++++++------------
 arch/arm/plat-omap/include/plat/io.h |    4 +-
 5 files changed, 322 insertions(+), 291 deletions(-)

^ permalink raw reply

* [RFC PATCH 0/3] Add accurate boot timing to a Linux system
From: Russell King - ARM Linux @ 2011-09-24  8:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316818998-30711-1-git-send-email-sjg@chromium.org>

On Fri, Sep 23, 2011 at 04:03:15PM -0700, Simon Glass wrote:
> An accurate timer is required to make the numbers meaningful. Many
> modern platforms have a microsecond timer. This patch set uses a
> function called timer_get_us() to read the timer.

Not another 'get a time value' function.  Why do we need soo many?
We have - at least:

ktime_get (and various flavours of it)
do_gettimeofday
getnstimeofday
sched_clock

Do we really need yet another one which will have to be multiplexed
amongst platforms, requiring scaling and so forth from whatever the
platform provides?

Remember that ARM timers are virtually all MMIO mapped, which means
they don't work during early kernel bringup when the MMU mappings for
the hardware have not been setup.  (That's the reason stuff like
sched_clock for printk doesn't work early.)  That can't be solved by
creating yet another per-platform method to get microseconds.

^ permalink raw reply

* [PATCH v7 00/26] gpio/omap: driver cleanup and fixes
From: Santosh Shilimkar @ 2011-09-24  8:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAC83ZvKu5Fo-c88L4WFPdvq91T=usaZY4mzTVVzDLw9HE7g9Kg@mail.gmail.com>

On Saturday 24 September 2011 09:26 AM, DebBarma, Tarun Kanti wrote:
> [...]
>> After debugging this myself a bit, here's what I think may be going on.
>> This may not be the only problem but here's at least one of them.
>>
>> First, debounce clocks are disabled in the runtime_suspend callback.
>>
>> When a GPIO is freed and it's the last one in the bank, bank->mod_usage
>> goes to zero.
>>
>> After that, pm_runtime_put_sync() is called, which will trigger the
>> driver's ->runtime_suspend callback.  The ->runtime_suspend() callback
>> checks bank->mod_usage as well, and if zero, doesn't do anything
>> (notably, it doesn't disable debounce clocks.)
> I need some clarification in reproducing/testing the fix on OMAP3430SDP.
> The first thing I am trying to verify is the code flow of suspend.
> 
> 1) With no debounce clock enabled, when I enable UART timeouts, I
> automatically see
> system going to retention. That is I don't have to type echo mem >
> /sys/power/state
> echo 5 > /sys/devices/platform/omap/omap_uart.0/sleep_timeout
> echo 5 > /sys/devices/platform/omap/omap_uart.1/sleep_timeout
> echo 5 > /sys/devices/platform/omap/omap_uart.2/sleep_timeout
> 
> 2) I am do not see the print in omap_gpio_suspend/resume(), but I see
> the print in
> *_prepare_for_idle()/*_resume_after_idle().
> 
Hmmm,

This is mostly happening because you are missing a below
fix from Kevin in the branch you are testing with.

http://www.mail-archive.com/linux-omap at vger.kernel.org/msg54927.html
{OMAP: omap_device: fix !CONFIG_SUSPEND case in _noirq handlers}

If you rebase, your branch against 3.1-rc6, you should already
have this fix. Commit {126caf1376e7}

Regards
Santosh

^ permalink raw reply

* [PATCH] arm: proc-v7: pc phy addresses before disable MMU
From: Per Förlin @ 2011-09-24  9:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110924081342.GA2639@mwesterb-mobl.ger.corp.intel.com>

On 09/24/2011 10:13 AM, Mika Westerberg wrote:
> On Sat, Sep 24, 2011 at 10:02:50AM +0200, Per Forlin wrote:
>> PC runs virtual addresses when calling cpu_v7_reset(). The MMU
>> is switched off and "mov pc, r0" sets pc back to
>> virtual addresses even though the MMU is switched off.
>> This will result in a crash if the pipeline delay after
>> MMU disable is one instruction. To prevent this set PC
>> to physical addresses before disabling the MMU.
>>
>> Signed-off-by: Per Fransson <per.xx.fransson@stericsson.com>
>> Signed-off-by: Per Forlin <per.forlin@stericsson.com>
>> ---
>>  arch/arm/mm/proc-v7.S |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
>> index 9049c07..f26e831 100644
>> --- a/arch/arm/mm/proc-v7.S
>> +++ b/arch/arm/mm/proc-v7.S
>> @@ -64,6 +64,7 @@ ENDPROC(cpu_v7_proc_fin)
>>   */
>>  	.align	5
>>  ENTRY(cpu_v7_reset)
>> +	sub	pc, pc, #PAGE_OFFSET+4		@ go to physical addresses
> 
> This only works on machines where PHYS_OFFSET is zero.
> 
You are right! Russell was very quick to point out the same thing. 

> IIRC, there is a P<->V offset calculated at boot time. Maybe you could use
> that instead here?
> 
I am trying to figure out how I can get the value of PHYS_OFFSET in assembler code (proc-v7.S).
I guess I could use the value calculated in arch/arm/kernel.head.S but wouldn't that make
this patch dependent on CONFIG_ARM_PATCH_PHYS_VIRT?

Regards,
Per

^ permalink raw reply

* I-cache/D-cache inconsistency issue with page cache
From: Mike Hommey @ 2011-09-24  9:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110923193941.GQ17169@n2100.arm.linux.org.uk>

On Fri, Sep 23, 2011 at 08:39:41PM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 23, 2011 at 01:57:21PM +0200, Mike Hommey wrote:
> > We've been hitting random crashes at startup with Firefox on tegras
> > (under Android), and narrowed it down to a I-cache/D-cache
> > inconsistency. A reduced testcase of the issue looks like the following
> > (compile as ARM, not Thumb):
> 
> If you write code at run time, you need to use the sys_cacheflush
> API to ensure that it's properly synchronized with the I-cache.  It's
> a well known issue, and it applies to any harvard cache structured
> CPU which doesn't automatically ensure coherence (which essentially
> means all ARMs.)

I do agree it's reasonable to have applications doing that to handle
cache synchronization themselves. I wrote such in my message. But I
think the kernel should make sure that its page cache is fresh when 
it maps it PROT_EXEC. I think it's unreasonable to expect applications
doing mmap(PROT_WRITE), inflate, munmap, something, mmap(PROT_EXEC),
and execute something there to have to handle cache synchronisation
themselves. Especially when it's very CPU dependent (the testcase does
not even fail on all ARMs, only tegras, apparently). I'm not talking
actual code generation here, which needs platform-dependent behaviour.

Mike

^ permalink raw reply

* [PATCH 3/4] iommu/exynos: Add iommu driver for Exynos4 Platforms
From: Russell King - ARM Linux @ 2011-09-24  9:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5525949.50001316849744994.JavaMail.weblogic@epml24>

On Sat, Sep 24, 2011 at 07:35:45AM +0000, ??? wrote:
> +# EXYNOS IOMMU support
> +config EXYNOS_IOMMU
> + bool "Exynos IOMMU Support"
> + depends on ARCH_EXYNOS4
> + select IOMMU_API
> + select EXYNOS4_DEV_SYSMMU
> + help

Looks like your mailer converted tabs to one space.  This makes it more
difficult to read this patch as a whole.  Don't expect good reviews as
long as your mailer breaks stuff in this way.

> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 4d4d77d..1eb924f 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -3,3 +3,4 @@ obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o msm_iommu_dev.o
> obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o
> obj-$(CONFIG_DMAR) += dmar.o iova.o intel-iommu.o
> obj-$(CONFIG_INTR_REMAP) += dmar.o intr_remapping.o

Looks like your mailer eliminated spaces at the start of these lines.

> +obj-$(CONFIG_EXYNOS_IOMMU) += exynos_iommu.o
> diff --git a/drivers/iommu/exynos_iommu.c b/drivers/iommu/exynos_iommu.c
> new file mode 100644
> index 0000000..fe5b5d8
> --- /dev/null
> +++ b/drivers/iommu/exynos_iommu.c
> @@ -0,0 +1,859 @@
> +/* linux/drivers/iommu/exynos_iommu.c
> + *
> + * Copyright (c) 2011 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 

Looks like your mailer... well... is totally broken beyond belief.

> +static inline struct sysmmu_platdata *get_platdata(struct device *dev)
> +{
> + return dev_get_platdata(dev);
> +}
> +
> +static inline bool set_sysmmu_active(struct sysmmu_platdata *mmudata)
> +{
> + /* return true if the System MMU was not active previously
> +    and it needs to be initialized */
> +
> + return atomic_inc_return(&mmudata->activations) == 1;
> +}
> +
> +static inline bool set_sysmmu_inactive(struct sysmmu_platdata *mmudata)
> +{
> + /* return true if the System MMU is needed to be disabled */
> + int ref;
> +
> + ref = atomic_dec_return(&mmudata->activations);
> +
> + if (ref == 0)
> + return true;
> +
> + if (WARN_ON(ref < 0)) {
> + /* System MMU is already disabled */
> + atomic_set(&mmudata->activations, 0);
> + ref = 0;
> + }
> +
> + return false;
> +}
> +
> +static inline bool is_sysmmu_active(struct sysmmu_platdata *mmudata)
> +{
> + return atomic_read(&mmudata->activations) != 0;
> +}

This use of an atomic type is total rubbish.  There's nothing magical
about 'atomic_*' stuff which suddenly makes the rest of your code
magically correct.  Think about this:

	Thread0			Thread1
	atomic_inc_return(&v)
		== 1
				atomic_dec_return(&v)
					== 0
				disables MMU
	enables MMU

Now you have the situation where the atomic value is '0' yet the MMU is
enabled.

Repeat after me: atomic types do not make code atomic.  atomic types
just make modifications to the type itself atomic.  Never use atomic
types for code synchronization.

> +static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
> +{
> + /* SYSMMU is in blocked when interrupt occurred. */
> + unsigned long addr;
> + struct sysmmu_platdata *mmudata = dev_id;
> + enum S5P_SYSMMU_INTERRUPT_TYPE itype;
> +
> + WARN_ON(!is_sysmmu_active(mmudata));
> +
> + itype = (enum S5P_SYSMMU_INTERRUPT_TYPE)
> + __ffs(__raw_readl(mmudata->sfrbase + S5P_INT_STATUS));
> +
> + BUG_ON(!((itype >= 0) && (itype < 8)));
> +
> + dev_alert(mmudata->dev, "SYSTEM MMU FAULT!!!.\n");

Do the exclaimation marks do anything for this error message?

> +
> + if (!mmudata->domain)
> + return IRQ_NONE;
> +
> + addr = __raw_readl(mmudata->sfrbase + fault_reg_offset[itype]);
> +
> + if (!report_iommu_fault(mmudata->domain, mmudata->owner, addr, itype)) {
> + __raw_writel(1 << itype, mmudata->sfrbase + S5P_INT_CLEAR);
> + dev_notice(mmudata->dev,
> + "%s is resolved. Retrying translation.\n",

So, this is a notice severity, yet we've printed at alert level.  Either
this condition satisfies being an alert or not.

> + sysmmu_fault_name[itype]);
> + sysmmu_unblock(mmudata->sfrbase);
> + } else {
> + dev_notice(mmudata->dev, "%s is not handled.\n",
> + sysmmu_fault_name[itype]);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +void exynos_sysmmu_set_tablebase_pgd(struct device *owner, unsigned long pgd)
> +{
> + struct sysmmu_platdata *mmudata = NULL;
> +
> + while ((mmudata = get_sysmmu_data(owner, mmudata))) {
> + if (is_sysmmu_active(mmudata)) {
> + sysmmu_block(mmudata->sfrbase);
> + __sysmmu_set_ptbase(mmudata->sfrbase, pgd);
> + sysmmu_unblock(mmudata->sfrbase);
> + dev_dbg(mmudata->dev, "New page table base is %p\n",
> + (void *)pgd);

If it's unsigned long use %08lx and don't cast.

> +static int exynos_sysmmu_probe(struct platform_device *pdev)
> +{
> + struct resource *res, *ioarea;
> + int ret;
> + int irq;
> + struct device *dev;
> + void *sfr;

void __iomem *sfr;

> +
> + dev = &pdev->dev;
> + if (!get_platdata(dev) || (get_platdata(dev)->owner == NULL)) {
> + dev_err(dev, "Failed to probing system MMU: "
> + "Owner device is not set.");
> + return -ENXIO;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(dev,
> + "Failed probing system MMU: failed to get resource.");
> + return -ENOENT;
> + }
> +
> + ioarea = request_mem_region(res->start, resource_size(res), pdev->name);
> + if (ioarea == NULL) {
> + dev_err(dev, "Failed probing system MMU: "
> + "failed to request memory region.");
> + return -ENOMEM;
> + }
> +
> + sfr = ioremap(res->start, resource_size(res));
> + if (!sfr) {
> + dev_err(dev, "Failed probing system MMU: "
> + "failed to call ioremap().");
> + ret = -ENOENT;
> + goto err_ioremap;
> + }
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq <= 0) {
> + dev_err(dev, "Failed probing system MMU: "
> + "failed to get irq resource.");
> + ret = irq;
> + goto err_irq;
> + }
> +
> + if (request_irq(irq, exynos_sysmmu_irq, 0, dev_name(&pdev->dev),
> + dev_get_platdata(dev))) {
> + dev_err(dev, "Failed probing system MMU: "
> + "failed to request irq.");
> + ret = -ENOENT;
> + goto err_irq;
> + }
> +
> + get_platdata(dev)->clk = clk_get(dev, "sysmmu");

It's rather horrible to see drivers modifying their platform data, rather
than drivers using the driver data pointer in struct device.  As far as
drivers are concerned, the platform data should only ever be read.

> +
> + if (IS_ERR_OR_NULL(get_platdata(dev)->clk)) {

IS_ERR() only please.  The *only* allowable failure value for drivers to
be concerned about here is if the return value is an error pointer.
Drivers have no business rejecting a NULL pointer here.

> +static int exynos_sysmmu_remove(struct platform_device *pdev)
> +{
> + return 0;
> +}

That doesn't look good.  If you can't remove it don't provide the
function at all.

> +
> +int exynos_sysmmu_runtime_suspend(struct device *dev)
> +{
> + if (WARN_ON(is_sysmmu_active(dev_get_platdata(dev))))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +int exynos_sysmmu_runtime_resume(struct device *dev)
> +{
> + if (WARN_ON(is_sysmmu_active(dev_get_platdata(dev))))
> + return -EFAULT;
> +
> + return 0;
> +}

Is there much point to having runtime PM support in this driver?

> +static int exynos_iommu_fault_handler(struct iommu_domain *domain,
> + struct device *dev, unsigned long iova, int flags)
> +{
> + struct exynos_iommu_domain *priv = domain->priv;
> +
> + dev_err(priv->dev, "%s occured at %p(Page table base: %p)\n",
> + sysmmu_fault_name[flags], (void *)iova,
> + (void *)(__pa(priv->pgtable)));

Again, you want to get rid of these casts and use a proper format string
for what these are.

> +static int exynos_iommu_attach_device(struct iommu_domain *domain,
> +    struct device *dev)
> +{
> + struct exynos_iommu_domain *priv = domain->priv;
> + int ret;
> +
> + spin_lock(&priv->lock);
> +
> + priv->dev = dev;
> +
> + ret = exynos_sysmmu_enable(domain);
> + if (ret)
> + return ret;
> +
> + spin_unlock(&priv->lock);
> +
> + return 0;

This function has an indeterminant return state for the spinlock - it
sometimes returns with it locked and sometimes with it unlocked.  That's
bad practice.

Getting rid of the 'if (ret) return ret;' and changing this 'return 0;'
to 'return ret;' is a simple way of solving it - and results in less
complex code.

> +static int exynos_iommu_map(struct iommu_domain *domain, unsigned long iova,
> + phys_addr_t paddr, int gfp_order, int prot)
> +{
> + struct exynos_iommu_domain *priv = domain->priv;
> + unsigned long *start_entry, *entry, *end_entry;
> + int num_entry;
> + int ret = 0;
> + unsigned long flags;
> +
> + BUG_ON(priv->pgtable == NULL);
> +
> + spin_lock_irqsave(&priv->pgtablelock, flags);
> +
> + start_entry = entry = priv->pgtable + (iova >> S5P_SECTION_SHIFT);
> +
> + if (gfp_order >= S5P_SECTION_ORDER) {
> + BUG_ON((paddr | iova) & ~S5P_SECTION_MASK);
> + /* 1MiB mapping */
> +
> + num_entry = 1 << (gfp_order - S5P_SECTION_ORDER);
> + end_entry = entry + num_entry;
> +
> + while (entry != end_entry) {
> + if (!section_available(domain, entry))
> + break;
> +
> + MAKE_SECTION_ENTRY(*entry, paddr);
> +
> + paddr += S5P_SECTION_SIZE;
> + entry++;
> + }
> +
> + if (entry != end_entry)
> + goto mapping_error;
> +
> + pgtable_flush(start_entry, entry);
> + goto mapping_done;
> + }
> +
> + if (S5P_FAULT_LV1_ENTRY(*entry)) {
> + unsigned long *l2table;
> +
> + l2table = kmem_cache_zalloc(l2table_cachep, GFP_KERNEL);
> + if (!l2table) {
> + ret = -ENOMEM;
> + goto nomem_error;
> + }
> +
> + pgtable_flush(entry, entry + S5P_LV2TABLE_ENTRIES);
> +
> + MAKE_LV2TABLE_ENTRY(*entry, virt_to_phys(l2table));
> + pgtable_flush(entry, entry + 1);
> + }
> +
> + /* 'entry' points level 2 entries, hereafter */
> + entry = GET_LV2ENTRY(*entry, iova);
> +
> + start_entry = entry;
> + num_entry = 1 << gfp_order;
> + end_entry = entry + num_entry;
> +
> + if (gfp_order >= S5P_LPAGE_ORDER) {
> + /* large page(64KiB) mapping */
> + BUG_ON((paddr | iova) & ~S5P_LPAGE_MASK);
> +
> + while (entry != end_entry) {
> + if (!write_lpage(entry, paddr)) {
> + pr_err("%s: Failed to allocate large page"
> + " entry.\n", __func__);

Don't wrap error messages, irrespective of what checkpatch may tell you.
They make them impossible to grep for.

^ permalink raw reply

* I-cache/D-cache inconsistency issue with page cache
From: Russell King - ARM Linux @ 2011-09-24  9:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110924093544.GA5724@glandium.org>

On Sat, Sep 24, 2011 at 11:35:44AM +0200, Mike Hommey wrote:
> On Fri, Sep 23, 2011 at 08:39:41PM +0100, Russell King - ARM Linux wrote:
> > On Fri, Sep 23, 2011 at 01:57:21PM +0200, Mike Hommey wrote:
> > > We've been hitting random crashes at startup with Firefox on tegras
> > > (under Android), and narrowed it down to a I-cache/D-cache
> > > inconsistency. A reduced testcase of the issue looks like the following
> > > (compile as ARM, not Thumb):
> > 
> > If you write code at run time, you need to use the sys_cacheflush
> > API to ensure that it's properly synchronized with the I-cache.  It's
> > a well known issue, and it applies to any harvard cache structured
> > CPU which doesn't automatically ensure coherence (which essentially
> > means all ARMs.)
> 
> I do agree it's reasonable to have applications doing that to handle
> cache synchronization themselves. I wrote such in my message. But I
> think the kernel should make sure that its page cache is fresh when 
> it maps it PROT_EXEC. I think it's unreasonable to expect applications
> doing mmap(PROT_WRITE), inflate, munmap, something, mmap(PROT_EXEC),
> and execute something there to have to handle cache synchronisation
> themselves. Especially when it's very CPU dependent (the testcase does
> not even fail on all ARMs, only tegras, apparently). I'm not talking
> actual code generation here, which needs platform-dependent behaviour.

Ok.  Which kernel are you trying this with, and which CPU (please
confirm Cortex-A9)?

^ permalink raw reply

* I-cache/D-cache inconsistency issue with page cache
From: Mike Hommey @ 2011-09-24 10:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110924094734.GC17169@n2100.arm.linux.org.uk>

On Sat, Sep 24, 2011 at 10:47:34AM +0100, Russell King - ARM Linux wrote:
> On Sat, Sep 24, 2011 at 11:35:44AM +0200, Mike Hommey wrote:
> > On Fri, Sep 23, 2011 at 08:39:41PM +0100, Russell King - ARM Linux wrote:
> > > On Fri, Sep 23, 2011 at 01:57:21PM +0200, Mike Hommey wrote:
> > > > We've been hitting random crashes at startup with Firefox on tegras
> > > > (under Android), and narrowed it down to a I-cache/D-cache
> > > > inconsistency. A reduced testcase of the issue looks like the following
> > > > (compile as ARM, not Thumb):
> > > 
> > > If you write code at run time, you need to use the sys_cacheflush
> > > API to ensure that it's properly synchronized with the I-cache.  It's
> > > a well known issue, and it applies to any harvard cache structured
> > > CPU which doesn't automatically ensure coherence (which essentially
> > > means all ARMs.)
> > 
> > I do agree it's reasonable to have applications doing that to handle
> > cache synchronization themselves. I wrote such in my message. But I
> > think the kernel should make sure that its page cache is fresh when 
> > it maps it PROT_EXEC. I think it's unreasonable to expect applications
> > doing mmap(PROT_WRITE), inflate, munmap, something, mmap(PROT_EXEC),
> > and execute something there to have to handle cache synchronisation
> > themselves. Especially when it's very CPU dependent (the testcase does
> > not even fail on all ARMs, only tegras, apparently). I'm not talking
> > actual code generation here, which needs platform-dependent behaviour.
> 
> Ok.  Which kernel are you trying this with, and which CPU (please
> confirm Cortex-A9)?

This has been seen on tegra boards under Ubuntu Maverick
(2.6.35.7.something) and under Android (2.6.32.9.something) and on the Asus
Transformer (Android, 2.6.36.3.something). All Cortex-A9 tegras. It has
*not* been reproduced on pandaboards (Cortex-A9 OMAP4430).

Mike

^ permalink raw reply

* I-cache/D-cache inconsistency issue with page cache
From: Russell King - ARM Linux @ 2011-09-24 10:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110924100701.GA5908@glandium.org>

On Sat, Sep 24, 2011 at 12:07:01PM +0200, Mike Hommey wrote:
> This has been seen on tegra boards under Ubuntu Maverick
> (2.6.35.7.something) and under Android (2.6.32.9.something) and on the Asus
> Transformer (Android, 2.6.36.3.something). All Cortex-A9 tegras. It has
> *not* been reproduced on pandaboards (Cortex-A9 OMAP4430).

Ah, your kernels are probably too old.

You need to ensure that you have at least 6012191 (ARM: 6380/1:
Introduce __sync_icache_dcache() for VIPT caches) and 85848dd (ARM:
6381/1: Use lazy cache flushing on ARMv7 SMP systems).  Note that both
these depend on some preceding patches too.

^ permalink raw reply

* I-cache/D-cache inconsistency issue with page cache
From: Siarhei Siamashka @ 2011-09-24 10:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110924093544.GA5724@glandium.org>

On Sat, Sep 24, 2011 at 12:35 PM, Mike Hommey <mh@glandium.org> wrote:
> On Fri, Sep 23, 2011 at 08:39:41PM +0100, Russell King - ARM Linux wrote:
>> On Fri, Sep 23, 2011 at 01:57:21PM +0200, Mike Hommey wrote:
>> > We've been hitting random crashes at startup with Firefox on tegras
>> > (under Android), and narrowed it down to a I-cache/D-cache
>> > inconsistency. A reduced testcase of the issue looks like the following
>> > (compile as ARM, not Thumb):
>>
>> If you write code at run time, you need to use the sys_cacheflush
>> API to ensure that it's properly synchronized with the I-cache. ?It's
>> a well known issue, and it applies to any harvard cache structured
>> CPU which doesn't automatically ensure coherence (which essentially
>> means all ARMs.)
>
> I do agree it's reasonable to have applications doing that to handle
> cache synchronization themselves. I wrote such in my message. But I
> think the kernel should make sure that its page cache is fresh when
> it maps it PROT_EXEC. I think it's unreasonable to expect applications
> doing mmap(PROT_WRITE), inflate, munmap, something, mmap(PROT_EXEC),
> and execute something there to have to handle cache synchronisation
> themselves. Especially when it's very CPU dependent (the testcase does
> not even fail on all ARMs, only tegras, apparently). I'm not talking
> actual code generation here, which needs platform-dependent behaviour.

Unfortunately we can't rely on what is reasonable, but have to
strictly follow how it is specified to work. My understanding is that
'sys_cacheflush' has been always mandatory for arm linux and
orthogonal to 'mmap'/'mprotect', no matter what discussion threads you
find in the mailing list archives. Being new to ARM architecture at
that time, I had been also burned by this issue years ago [1], when
various pieces of documentation did not match each other, there was a
transition from OABI to EABI ongoing, etc.

The way forward could be to try and ask linux man pages maintainers to
update the entries for 'mmap' and 'mprotect' to explicitly state that
certain architectures require mandatory instruction/data caches
synchronization no matter how you play with the protection flags, and
that it can be usually done via gcc '__builtin___clear_cache' function
[2].

1. http://ffmpeg.org/pipermail/ffmpeg-devel/2007-January/027847.html
2. http://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html

-- 
Best regards,
Siarhei Siamashka

^ permalink raw reply

* [PATCH v2 2/2] ARM: smp_scu: remove __init annotation from scu_enable()
From: Russell King - ARM Linux @ 2011-09-24 10:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110924103908.GA16175@S2100-06.ap.freescale.net>

On Sat, Sep 24, 2011 at 06:39:09PM +0800, Shawn Guo wrote:
> On Fri, Sep 23, 2011 at 09:49:55PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Sep 22, 2011 at 11:04:01PM +0800, Shawn Guo wrote:
> > > On Sat, Sep 17, 2011 at 04:32:37PM +0800, Shawn Guo wrote:
> > > > On Tue, Sep 06, 2011 at 01:48:27PM +0800, Shawn Guo wrote:
> > > > > When Cortex-A9 MPCore resumes from Dormant or Shutdown modes,
> > > > > SCU needs to be re-enabled.  This patch removes __init annotation
> > > > > from function scu_enable(), so that platform resume procedure can
> > > > > call it to re-enable SCU.
> > > > > 
> > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > ---
> > > > 
> > > > Hi Russell,
> > > > 
> > > > Do you have any comment on this patch?  Can I put it into patch tracker?
> > > > 
> > > Ping x 2.
> > > 
> > > As imx6q suspend support depends on this patch, I really need to know
> > > if you will merge it.  Otherwise, I will probably have to copy the code
> > > into imx6q platform code just like exynos4 is doing.
> > 
> > It's fine.
> > 
> I'm unsure about this comment.  Does it mean that the patch looks fine
> for being merged, or it's fine to duplicate the code in imx6q platform?

The patch looks fine.

^ permalink raw reply

* [PATCH v2 2/2] ARM: smp_scu: remove __init annotation from scu_enable()
From: Shawn Guo @ 2011-09-24 10:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110923204955.GV17169@n2100.arm.linux.org.uk>

On Fri, Sep 23, 2011 at 09:49:55PM +0100, Russell King - ARM Linux wrote:
> On Thu, Sep 22, 2011 at 11:04:01PM +0800, Shawn Guo wrote:
> > On Sat, Sep 17, 2011 at 04:32:37PM +0800, Shawn Guo wrote:
> > > On Tue, Sep 06, 2011 at 01:48:27PM +0800, Shawn Guo wrote:
> > > > When Cortex-A9 MPCore resumes from Dormant or Shutdown modes,
> > > > SCU needs to be re-enabled.  This patch removes __init annotation
> > > > from function scu_enable(), so that platform resume procedure can
> > > > call it to re-enable SCU.
> > > > 
> > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > ---
> > > 
> > > Hi Russell,
> > > 
> > > Do you have any comment on this patch?  Can I put it into patch tracker?
> > > 
> > Ping x 2.
> > 
> > As imx6q suspend support depends on this patch, I really need to know
> > if you will merge it.  Otherwise, I will probably have to copy the code
> > into imx6q platform code just like exynos4 is doing.
> 
> It's fine.
> 
I'm unsure about this comment.  Does it mean that the patch looks fine
for being merged, or it's fine to duplicate the code in imx6q platform?

-- 
Regards,
Shawn

^ permalink raw reply

* [PATCH v7 00/26] gpio/omap: driver cleanup and fixes
From: DebBarma, Tarun Kanti @ 2011-09-24 10:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4E7D99CB.7090004@ti.com>

Santosh, Kevin,

[...]
>>> After that, pm_runtime_put_sync() is called, which will trigger the
>>> driver's ->runtime_suspend callback. ?The ->runtime_suspend() callback
>>> checks bank->mod_usage as well, and if zero, doesn't do anything
>>> (notably, it doesn't disable debounce clocks.)
>> I need some clarification in reproducing/testing the fix on OMAP3430SDP.
>> The first thing I am trying to verify is the code flow of suspend.
>>
>> 1) With no debounce clock enabled, when I enable UART timeouts, I
>> automatically see
>> system going to retention. That is I don't have to type echo mem >
>> /sys/power/state
>> echo 5 > /sys/devices/platform/omap/omap_uart.0/sleep_timeout
>> echo 5 > /sys/devices/platform/omap/omap_uart.1/sleep_timeout
>> echo 5 > /sys/devices/platform/omap/omap_uart.2/sleep_timeout
>>
>> 2) I am do not see the print in omap_gpio_suspend/resume(), but I see
>> the print in
>> *_prepare_for_idle()/*_resume_after_idle().
>>
> Hmmm,
>
> This is mostly happening because you are missing a below
> fix from Kevin in the branch you are testing with.
>
> http://www.mail-archive.com/linux-omap at vger.kernel.org/msg54927.html
> {OMAP: omap_device: fix !CONFIG_SUSPEND case in _noirq handlers}
>
> If you rebase, your branch against 3.1-rc6, you should already
> have this fix. Commit {126caf1376e7}
Yes, this patch was missing in Kevin's branch and was
causing the suspend issue.

As pointed out by Kevin, debounce clock was not getting disabled.
In my testing I was somehow grepping CORE power domain instead
of PER power domain and hence missed it. The fix for the debounce
clock issue is at the end of the email.

- Have re-based the for_3.2/gpio-cleanup branch against 3.1-rc6.
- Dropped [PATCH 26/26] gpio/omap: add dbclk aliases for all gpio modules
as suggested by Kevin since it's already taken care by hwmod.
- Added the debounce clock fix in the end.

With above, PER is hitting low power state in Suspend and Idle path.

Have pushed a branch at below URL with mentioned changes.
git://gitorious.org/omap-sw-develoment/linux-omap-dev.git
for_3.2/kevin/gpio-cleanup

Regards,
Tarun

^ permalink raw reply

* [PATCH] arm: proc-v7: pc phy addresses before disable MMU
From: Will Deacon @ 2011-09-24 11:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4E7DA3C0.7000208@stericsson.com>

On Sat, Sep 24, 2011 at 10:32:48AM +0100, Per F?rlin wrote:
> I am trying to figure out how I can get the value of PHYS_OFFSET in assembler code (proc-v7.S).
> I guess I could use the value calculated in arch/arm/kernel.head.S but wouldn't that make
> this patch dependent on CONFIG_ARM_PATCH_PHYS_VIRT?

Wouldn't we just be better off passing a physical address to cpu_v7_reset
instead?

Will

^ permalink raw reply

* [PATCH 32/55] i2c: irq: Remove IRQF_DISABLED
From: Jean Delvare @ 2011-09-24 11:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316681962-8217-33-git-send-email-yong.zhang0@gmail.com>

On Thu, 22 Sep 2011 16:58:59 +0800, Yong Zhang wrote:
> Since commit [e58aa3d2: genirq: Run irq handlers with interrupts disabled],
> We run all interrupt handlers with interrupts disabled
> and we even check and yell when an interrupt handler
> returns with interrupts enabled (see commit [b738a50a:
> genirq: Warn when handler enables interrupts]).
> 
> So now this flag is a NOOP and can be removed.
> 
> Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
> Acked-by: Wolfram Sang <w.sang@pengutronix.de>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Acked-by: Barry Song <21cnbao@gmail.com>
> ---
>  drivers/i2c/busses/i2c-bfin-twi.c   |    4 ++--
>  drivers/i2c/busses/i2c-designware.c |    2 +-
>  drivers/i2c/busses/i2c-highlander.c |    2 +-
>  drivers/i2c/busses/i2c-nomadik.c    |    2 +-
>  drivers/i2c/busses/i2c-nuc900.c     |    2 +-
>  drivers/i2c/busses/i2c-pmcmsp.c     |    2 +-
>  drivers/i2c/busses/i2c-s3c2410.c    |    2 +-
>  drivers/i2c/busses/i2c-sh7760.c     |    2 +-
>  drivers/i2c/busses/i2c-sh_mobile.c  |    2 +-
>  drivers/i2c/busses/i2c-stu300.c     |    2 +-
>  10 files changed, 11 insertions(+), 11 deletions(-)

Ben, I'll let you pick this patch as it only touches drivers which are
under your responsibility. This will limit the risk of conflicts
between our trees.

-- 
Jean Delvare

^ permalink raw reply

* [PATCH] arm: proc-v7: pc phy addresses before disable MMU
From: Per Förlin @ 2011-09-24 12:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110924110024.GA20044@e102144-lin.cambridge.arm.com>

On 09/24/2011 01:00 PM, Will Deacon wrote:
> On Sat, Sep 24, 2011 at 10:32:48AM +0100, Per F?rlin wrote:
>> I am trying to figure out how I can get the value of PHYS_OFFSET in assembler code (proc-v7.S).
>> I guess I could use the value calculated in arch/arm/kernel.head.S but wouldn't that make
>> this patch dependent on CONFIG_ARM_PATCH_PHYS_VIRT?
> 
> Wouldn't we just be better off passing a physical address to cpu_v7_reset
> instead?
> 
Good point! I'm fine with that.

/Per

^ permalink raw reply

* [PATCH] arm: proc-v7: pc phy addresses before disable MMU
From: Russell King - ARM Linux @ 2011-09-24 13:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4E7DD34E.3060402@stericsson.com>

On Sat, Sep 24, 2011 at 02:55:42PM +0200, Per F?rlin wrote:
> On 09/24/2011 01:00 PM, Will Deacon wrote:
> > On Sat, Sep 24, 2011 at 10:32:48AM +0100, Per F?rlin wrote:
> >> I am trying to figure out how I can get the value of PHYS_OFFSET in assembler code (proc-v7.S).
> >> I guess I could use the value calculated in arch/arm/kernel.head.S but wouldn't that make
> >> this patch dependent on CONFIG_ARM_PATCH_PHYS_VIRT?
> > 
> > Wouldn't we just be better off passing a physical address to cpu_v7_reset
> > instead?
> > 
> Good point! I'm fine with that.

Alternatively, call it using the phys address.

^ permalink raw reply

* Identifying Primecells
From: Linus Walleij @ 2011-09-24 13:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKON4Ow1-jTM8fRpm12=5+acCt6A-35ZpWGJ_qMmRHi_JS8wAA@mail.gmail.com>

2011/9/23 jonsmirl at gmail.com <jonsmirl@gmail.com>:

> So it looks like NXP made almost all of their own peripherals. Which
> means I have to clean up the drivers for all of them.

Totally from-scratch IP blocks exist but is uncommon.

When there are no helpfull primecell IDs giving the secret
of ancestry away readily, fall back to comparing register maps,
this usually reveals the family of hardware and possible
ancestors. (Like was quickly done for the 8250 derivate
serial.)

Yours,
Linus Walleij

^ permalink raw reply

* [RFC PATCH 0/3] Add accurate boot timing to a Linux system
From: Simon Glass @ 2011-09-24 14:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110924083203.GZ17169@n2100.arm.linux.org.uk>

Hi Russell,

On Sat, Sep 24, 2011 at 1:32 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Sep 23, 2011 at 04:03:15PM -0700, Simon Glass wrote:
>> An accurate timer is required to make the numbers meaningful. Many
>> modern platforms have a microsecond timer. This patch set uses a
>> function called timer_get_us() to read the timer.
>
> Not another 'get a time value' function. ?Why do we need soo many?
> We have - at least:
>
> ktime_get (and various flavours of it)
> do_gettimeofday
> getnstimeofday
> sched_clock
>
> Do we really need yet another one which will have to be multiplexed
> amongst platforms, requiring scaling and so forth from whatever the
> platform provides?

No we don't need another! It is just a convenience for this RFC since
the actual timer mechanism is not clear and not important for this
RFC. Insert your favourite timer mechanism instead.

>
> Remember that ARM timers are virtually all MMIO mapped, which means
> they don't work during early kernel bringup when the MMU mappings for
> the hardware have not been setup. ?(That's the reason stuff like
> sched_clock for printk doesn't work early.) ?That can't be solved by
> creating yet another per-platform method to get microseconds.
>

While it would be useful to measure early things like kernel
decompression, it's hard to make that work cross-platform and there
are other problems like you raise, and where to store timing info.

^ permalink raw reply

* [PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210
From: Sylwester Nawrocki @ 2011-09-24 17:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201108132324.08137.rjw@sisk.pl>

On 08/13/2011 11:24 PM, Rafael J. Wysocki wrote:
> On Thursday, August 11, 2011, Chanwoo Choi wrote:
>> The following patch set use the generic Power domain Framework instead of
>> power domain code depend of Samsung SoC.
>>
>> Chanwoo Choi (4):
>>    ARM: EXYNOS4: Support for generic I/O power domains on EXYNOS4210
>>    ARM: EXYNOS4: Support for generic Clock manipulation PM callbacks
>>    ARM: EXYNOS4: Delete the power-domain code depend on Samsung SoC
>>    ARM: EXYNOS4: Add power domain to use generic Power domain Framework
>>
>>   arch/arm/mach-exynos4/Kconfig                      |   10 +-
>>   arch/arm/mach-exynos4/Makefile                     |    4 +-
>>   arch/arm/mach-exynos4/dev-pd.c                     |  139 --------------
>>   arch/arm/mach-exynos4/include/mach/pm-exynos4210.h |   52 ++++++
>>   arch/arm/mach-exynos4/include/mach/regs-clock.h    |    8 +
>>   arch/arm/mach-exynos4/mach-nuri.c                  |   21 ++-
>>   arch/arm/mach-exynos4/mach-smdkc210.c              |   26 ++-
>>   arch/arm/mach-exynos4/mach-smdkv310.c              |   23 ++-
>>   arch/arm/mach-exynos4/mach-universal_c210.c        |   21 ++-
>>   arch/arm/mach-exynos4/pm-exynos4210.c              |  189 ++++++++++++++++++++
>>   arch/arm/mach-exynos4/pm-runtime.c                 |   56 ++++++
>>   arch/arm/plat-samsung/Kconfig                      |    8 -
>>   arch/arm/plat-samsung/Makefile                     |    4 -
>>   arch/arm/plat-samsung/include/plat/pd.h            |   30 ---
>>   arch/arm/plat-samsung/pd.c                         |   95 ----------
>>   15 files changed, 377 insertions(+), 309 deletions(-)
>>   delete mode 100644 arch/arm/mach-exynos4/dev-pd.c
>>   create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
>>   create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c
>>   create mode 100644 arch/arm/mach-exynos4/pm-runtime.c
>>   delete mode 100644 arch/arm/plat-samsung/include/plat/pd.h
>>   delete mode 100644 arch/arm/plat-samsung/pd.c
> 
> The patchset looks good to me, but please note that some code it
> is based on will most likely change in 3.2 due to this patchset:
> 
> https://lkml.org/lkml/2011/8/8/420

Are we going to have this patch set in 3.2 ? Does it need more work
or is it already merged in someone's -next repository ?

--
Thanks,
Sylwester

^ permalink raw reply

* Identifying Primecells
From: Vitaly Wool @ 2011-09-24 23:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKON4Ow1-jTM8fRpm12=5+acCt6A-35ZpWGJ_qMmRHi_JS8wAA@mail.gmail.com>

Hi,

On Fri, Sep 23, 2011 at 9:11 PM, jonsmirl at gmail.com <jonsmirl@gmail.com> wrote:
> On Fri, Sep 23, 2011 at 10:22 AM, Pawel Moll <pawel.moll@arm.com> wrote:
>>> My registers don't seem to match up with the pl011 documentation...
>> <...>
>>> INTCE W ? ? ? 0xFD8 ? Interrupt ? ? ? Clear Enable Register
>>> INTSE W ? ? ? 0xFDC ? Interrupt ? ? ? Set Enable Register
>>> INTS ?R ? ? ? 0xFE0 ? Interrupt ? ? ? Status Register
>>> INTE ?R ? ? ? 0xFE4 ? Interrupt ? ? ? Enable Register
>>
>> ... nor any other PrimeCell, actually :-)
>>
>> "Compliant" PrimeCells must have the PCellID in the last 4 registers:
>
> I scanned though 0xff0/f for all of the devices. The only one that had
> anything that makes sense is the Multiport Memory Controller -
> 0xb105f00d
>
> So it looks like NXP made almost all of their own peripherals. Which
> means I have to clean up the drivers for all of them.
>

I have a vague recollection that some time ago NXP was using UARTs
that were basically 8250-compatible but with some twist. You may also
want to check the pnx8xxx_uart implementation for another type of
UARTs NXP used to have on their MIPS boards.

~Vitaly

^ permalink raw reply

* Identifying Primecells
From: jonsmirl at gmail.com @ 2011-09-25  3:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMJBoFN+LBncBshhAkmG0Sf_nz=sKeBajf-Zv1CBagTkirCHUw@mail.gmail.com>

On Sat, Sep 24, 2011 at 7:15 PM, Vitaly Wool <vitalywool@gmail.com> wrote:
> Hi,
>
> On Fri, Sep 23, 2011 at 9:11 PM, jonsmirl at gmail.com <jonsmirl@gmail.com> wrote:
>> On Fri, Sep 23, 2011 at 10:22 AM, Pawel Moll <pawel.moll@arm.com> wrote:
>>>> My registers don't seem to match up with the pl011 documentation...
>>> <...>
>>>> INTCE W ? ? ? 0xFD8 ? Interrupt ? ? ? Clear Enable Register
>>>> INTSE W ? ? ? 0xFDC ? Interrupt ? ? ? Set Enable Register
>>>> INTS ?R ? ? ? 0xFE0 ? Interrupt ? ? ? Status Register
>>>> INTE ?R ? ? ? 0xFE4 ? Interrupt ? ? ? Enable Register
>>>
>>> ... nor any other PrimeCell, actually :-)
>>>
>>> "Compliant" PrimeCells must have the PCellID in the last 4 registers:
>>
>> I scanned though 0xff0/f for all of the devices. The only one that had
>> anything that makes sense is the Multiport Memory Controller -
>> 0xb105f00d
>>
>> So it looks like NXP made almost all of their own peripherals. Which
>> means I have to clean up the drivers for all of them.
>>
>
> I have a vague recollection that some time ago NXP was using UARTs
> that were basically 8250-compatible but with some twist. You may also
> want to check the pnx8xxx_uart implementation for another type of
> UARTs NXP used to have on their MIPS boards.

I have a driver for the chip. It is a 8250 compatible that has been
extended with DMA support.

I've drivers for all of the peripherals on the chip. As I start
sending them out for RFC maybe we can identify them as variants of
drivers already in the kernel so that I don't add 20 new drivers if
some are redundant.

-- 
Jon Smirl
jonsmirl at gmail.com

^ permalink raw reply

* [PATCH v2 1/7] clk: Add a generic clock infrastructure
From: Grant Likely @ 2011-09-25  3:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316730422-20027-2-git-send-email-mturquette@ti.com>

On Thu, Sep 22, 2011 at 03:26:56PM -0700, Mike Turquette wrote:
> From: Jeremy Kerr <jeremy.kerr@canonical.com>
> 
> We currently have ~21 definitions of struct clk in the ARM architecture,
> each defined on a per-platform basis. This makes it difficult to define
> platform- (or architecture-) independent clock sources without making
> assumptions about struct clk, and impossible to compile two
> platforms with different struct clks into a single image.
> 
> This change is an effort to unify struct clk where possible, by defining
> a common struct clk, and a set of clock operations. Different clock
> implementations can set their own operations, and have a standard
> interface for generic code. The callback interface is exposed to the
> kernel proper, while the clock implementations only need to be seen by
> the platform internals.
> 
> The interface is split into two halves:
> 
>  * struct clk, which is the generic-device-driver interface. This
>    provides a set of functions which drivers may use to request
>    enable/disable, query or manipulate in a hardware-independent manner.
> 
>  * struct clk_hw and struct clk_hw_ops, which is the hardware-specific
>    interface. Clock drivers implement the ops, which allow the core
>    clock code to implement the generic 'struct clk' API.
> 
> This allows us to share clock code among platforms, and makes it
> possible to dynamically create clock devices in platform-independent
> code.
> 
> Platforms can enable the generic struct clock through
> CONFIG_GENERIC_CLK. In this case, the clock infrastructure consists of a
> common, opaque struct clk, and a set of clock operations (defined per
> type of clock):
> 
>   struct clk_hw_ops {
>   	int		(*prepare)(struct clk_hw *);
>   	void		(*unprepare)(struct clk_hw *);
>   	int		(*enable)(struct clk_hw *);
>   	void		(*disable)(struct clk_hw *);
>   	unsigned long	(*recalc_rate)(struct clk_hw *);
>   	int		(*set_rate)(struct clk_hw *,
>   					unsigned long, unsigned long *);
>   	long		(*round_rate)(struct clk_hw *, unsigned long);
>   	int		(*set_parent)(struct clk_hw *, struct clk *);
>   	struct clk *	(*get_parent)(struct clk_hw *);
>   };
> 
> Platform clock code can register a clock through clk_register, passing a
> set of operations, and a pointer to hardware-specific data:
> 
>   struct clk_hw_foo {
>   	struct clk_hw clk;
>   	void __iomem *enable_reg;
>   };
> 
>   #define to_clk_foo(c) offsetof(c, clk_hw_foo, clk)
> 
>   static int clk_foo_enable(struct clk_hw *clk)
>   {
>   	struct clk_foo *foo = to_clk_foo(clk);
>   	raw_writeb(foo->enable_reg, 1);
>   	return 0;
>   }
> 
>   struct clk_hw_ops clk_foo_ops = {
>   	.enable = clk_foo_enable,
>   };
> 
> And in the platform initialisation code:
> 
>   struct clk_foo my_clk_foo;
> 
>   void init_clocks(void)
>   {
>   	my_clk_foo.enable_reg = ioremap(...);
> 
>   	clk_register(&clk_foo_ops, &my_clk_foo, NULL);

Shouldn't this be:

	clk_register(&clk_foo_ops, &my_clk_foo->clk, NULL);

?

Also, this documentation would be good to have in the Documentation
directory instead of lost in a commit header.

Otherwise looks okay to me.

Reviewed-by: Grant Likely <grant.likely@secretlab.ca>

^ permalink raw reply

* [PATCH v2 4/7] clk: Add simple gated clock
From: Grant Likely @ 2011-09-25  4:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316730422-20027-5-git-send-email-mturquette@ti.com>

On Thu, Sep 22, 2011 at 03:26:59PM -0700, Mike Turquette wrote:
> From: Jeremy Kerr <jeremy.kerr@canonical.com>
> 
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> Signed-off-by: Mike Turquette <mturquette@ti.com>
> ---
> Changes since v1:
> Add copyright header
> Fold in Jamie's patch for set-to-disable clks
> Use BIT macro instead of shift
> 
>  drivers/clk/Kconfig    |    4 ++
>  drivers/clk/Makefile   |    1 +
>  drivers/clk/clk-gate.c |   78 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/clk.h    |   13 ++++++++
>  4 files changed, 96 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/clk/clk-gate.c
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index d8313d7..a78967c 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -12,3 +12,7 @@ config GENERIC_CLK
>  config GENERIC_CLK_FIXED
>  	bool
>  	depends on GENERIC_CLK
> +
> +config GENERIC_CLK_GATE
> +	bool
> +	depends on GENERIC_CLK

I see zero documentation on what a "gated clock" is supposed to be or
how it works, and there are zero comments in the code.  It's kind of
hard to review that way, and even harder to use.

g.

^ 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