Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/5] of/irq: introduce of_irq_init
From: Grant Likely @ 2011-09-17 23:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316017900-19918-4-git-send-email-robherring2@gmail.com>

On Wed, Sep 14, 2011 at 11:31:38AM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> of_irq_init will scan the devicetree for matching interrupt controller
> nodes. Then it calls an initialization function for each found controller
> in the proper order with parent nodes initialized before child nodes.
> 
> Based on initial pseudo code from Grant Likely.
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> ---
>  drivers/of/irq.c       |   96 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_irq.h |    1 +
>  2 files changed, 97 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 9f689f1..a0cd7e8 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -19,10 +19,13 @@
>   */
>  
>  #include <linux/errno.h>
> +#include <linux/list.h>
> +#include <linux/list_sort.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
>  #include <linux/string.h>
> +#include <linux/slab.h>
>  
>  /* For archs that don't support NO_IRQ (such as x86), provide a dummy value */
>  #ifndef NO_IRQ
> @@ -386,3 +389,96 @@ int of_irq_to_resource_table(struct device_node *dev, struct resource *res,
>  
>  	return i;
>  }
> +
> +struct intc_desc {
> +	struct list_head	list;
> +	struct device_node	*dev;
> +	struct device_node	*parent;
> +};
> +
> +typedef void (*irq_init_cb_t)(struct device_node *, struct device_node *);
> +
> +static int __init irq_cmp_intc_desc(void *unused, struct list_head *a,
> +				    struct list_head *b)
> +{
> +	const struct intc_desc *da = list_entry(a, typeof(*da), list);
> +	const struct intc_desc *db = list_entry(b, typeof(*db), list);
> +
> +	/* same parent, so order doesn't matter */
> +	if (da->parent == db->parent)
> +		return 0;
> +
> +	/* NULL parent comes first */
> +	if (!da->parent && db->parent)
> +		return -1;
> +	if (!db->parent && da->parent)
> +		return 1;
> +
> +	/* parent node must be before child node */
> +	if (da->dev == db->parent)
> +		return -1;
> +	if (db->dev == da->parent)
> +		return 1;

Does sort_list work for relationships 4 or more levels deep?  ie. if
there was a relationship of A <- B <- C <- D, then B compared with D
would return 0 from this function which could potentially result in an
incorrectly ordered list.

The other option for implementing this would be to take the probe
deferral approach and not try to sort the list, but instead allow
probe functions to fail & request retry if the parent hasn't yet been
probed.  I haven't thought enough about it though to say which would
be the best approach.

> +
> +	return 0;
> +}
> +
> +/**
> + * of_irq_init - Scan the device tree for matching interrupt controllers and
> + * call their initialization functions in order with parents first.
> + * @matches: 0 terminated array of nodes to match and initialization function
> + * to call on match
> + */
> +void __init of_irq_init(const struct of_device_id *matches)
> +{
> +	struct device_node *np;
> +	const struct of_device_id *match;
> +	struct intc_desc *desc;
> +	struct intc_desc *temp_desc;
> +	struct list_head intc_desc_list;
> +
> +	INIT_LIST_HEAD(&intc_desc_list);
> +
> +	for_each_matching_node(np, matches) {
> +		if (!of_find_property(np, "interrupt-controller", NULL))
> +			continue;
> +		/* Here, we allocate and populate an intc_desc with the node
> +		* pointer, interrupt-parent device_node etc. */
> +		desc = kzalloc(sizeof(*desc), GFP_KERNEL);
> +		if (!desc) {
> +			WARN_ON(1);
> +			goto err;
> +		}
> +		desc->dev = np;
> +		desc->parent = of_irq_find_parent(np);
> +		list_add(&desc->list, &intc_desc_list);
> +	}
> +	if (list_empty(&intc_desc_list))
> +		return;
> +
> +	/*
> +	 * The root irq controller is the one without an interrupt-parent.
> +	 * That one goes first, followed by the controllers that reference it,
> +	 * followed by the ones that reference the 2nd level controllers, etc
> +	 */

I don't believe that this actually turns out to be true (and yes I
know it is how I originally described it).  :-)  When the
interrupt-parent property is at the root of the tree, then the root
interrupt controller may very well inherit itself as it's interrupt
parent, and of_irq_find_parent() will still return a value.  This
should probably be considered a bug in of_irq_find_parent(), and it
should return NULL if the parent is itself.

of_irq_find_parent should probably be implemented thusly (completely
untested); although the only functional change is the line:
	return (p == child) ? NULL : p;

/**
 * of_irq_find_parent - Given a device node, find its interrupt parent node
 * @child: pointer to device node
 *
 * Returns a pointer to the interrupt parent node, or NULL if the
 * interrupt parent could not be determined.
 */
struct device_node *of_irq_find_parent(struct device_node *child)
{
	struct device_node *p, *c = child;
	const __be32 *parp;

	if (!of_node_get(c))
		return NULL;

	do {
		p = of_parse_phandle(c, "interrupt-parent", 0);

		if (!p && (of_irq_workarounds & OF_IMAP_NO_PHANDLE) &&
		    of_find_property(c, "interrupt-parent", NULL))
			p = of_node_get(of_irq_dflt_pic);

		if (!p)
			p = of_get_parent(c);

		of_node_put(c);
		c = p;
	} while (p && !of_find_property(p, "#interrupt-cells", NULL));

	return (p == child) ? NULL : p;
}


> +	list_sort(NULL, &intc_desc_list, irq_cmp_intc_desc);
> +
> +	list_for_each_entry_safe(desc, temp_desc, &intc_desc_list, list) {
> +		match = of_match_node(matches, desc->dev);
> +		if (match && match->data) {
> +			irq_init_cb_t irq_init_cb = match->data;
> +			pr_debug("of_irq_init: init %s @ %p, parent %p\n",
> +				 match->compatible, desc->dev, desc->parent);
> +			irq_init_cb(desc->dev, desc->parent);
> +		}
> +		list_del(&desc->list);
> +		kfree(desc);
> +	}
> +	return;
> +
> +err:
> +	list_for_each_entry_safe(desc, temp_desc, &intc_desc_list, list) {
> +		list_del(&desc->list);
> +		kfree(desc);
> +	}
> +}

Overall, I'm pretty happy with how this is looking.  Great job!

g.

> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> index cd2e61c..032d76c 100644
> --- a/include/linux/of_irq.h
> +++ b/include/linux/of_irq.h
> @@ -73,6 +73,7 @@ extern int of_irq_to_resource_table(struct device_node *dev,
>  		struct resource *res, int nr_irqs);
>  extern struct device_node *of_irq_find_parent(struct device_node *child);
>  
> +extern void of_irq_init(const struct of_device_id *matches);
>  
>  #endif /* CONFIG_OF_IRQ */
>  #endif /* CONFIG_OF */
> -- 
> 1.7.5.4
> 

^ permalink raw reply

* [PATCH 5/5] ARM: gic: add OF based initialization
From: Grant Likely @ 2011-09-18  0:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4E70F7BE.6020909@gmail.com>

On Wed, Sep 14, 2011 at 01:51:42PM -0500, Rob Herring wrote:
> On 09/14/2011 01:34 PM, Marc Zyngier wrote:
> > Hi Rob,
> > 
> > On 14/09/11 18:57, Rob Herring wrote:
> >> Marc,
> >>
> >> On 09/14/2011 12:46 PM, Marc Zyngier wrote:
> >>> On 14/09/11 17:31, Rob Herring wrote:
> >>>> From: Rob Herring <rob.herring@calxeda.com>
> >>>>
> >>>> This adds gic initialization using device tree data. The initialization
> >>>> functions are intended to be called by a generic OF interrupt
> >>>> controller parsing function once the right pieces are in place.
> >>>>
> >>>> PPIs are handled using 3rd cell of interrupts properties to specify the cpu
> >>>> mask the PPI is assigned to.
> >>>>
> >>>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> >>>> ---
> >>>>  Documentation/devicetree/bindings/arm/gic.txt |   53 ++++++++++++++++++++++++
> >>>>  arch/arm/common/gic.c                         |   55 +++++++++++++++++++++++--
> >>>>  arch/arm/include/asm/hardware/gic.h           |   10 +++++
> >>>>  3 files changed, 114 insertions(+), 4 deletions(-)
> >>>>  create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> >>>> new file mode 100644
> >>>> index 0000000..6c513de
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> >>>> @@ -0,0 +1,53 @@
> >>>> +* ARM Generic Interrupt Controller
> >>>> +
> >>>> +ARM SMP cores are often associated with a GIC, providing per processor
> >>>> +interrupts (PPI), shared processor interrupts (SPI) and software
> >>>> +generated interrupts (SGI).
> >>>> +
> >>>> +Primary GIC is attached directly to the CPU and typically has PPIs and SGIs.
> >>>> +Secondary GICs are cascaded into the upward interrupt controller and do not
> >>>> +have PPIs or SGIs.
> >>>> +
> >>>> +Main node required properties:
> >>>> +
> >>>> +- compatible : should be one of:
> >>>> +	"arm,cortex-a9-gic"
> >>>> +	"arm,arm11mp-gic"
> >>>> +- interrupt-controller : Identifies the node as an interrupt controller
> >>>> +- #interrupt-cells : Specifies the number of cells needed to encode an
> >>>> +  interrupt source.  The type shall be a <u32> and the value shall be 3.
> >>>> +
> >>>> +  The 1st cell is the interrupt number. 0-15 are reserved for SGIs. 16-31 are
> >>>> +  for PPIs.
> >>>> +
> >>>> +  The 2nd cell is the level-sense information, encoded as follows:
> >>>> +                    1 = low-to-high edge triggered
> >>>> +                    2 = high-to-low edge triggered
> >>>> +                    4 = active high level-sensitive
> >>>> +                    8 = active low level-sensitive
> >>>> +
> >>>> +  Only values of 1 and 4 are valid for GIC 1.0 spec.
> >>>> +
> >>>> +  The 3rd cell contains the mask of the cpu number for the interrupt source.
> >>>> +  The cpu mask is only valid for PPIs and shall be 0 for SPIs. This value shall
> >>>> +  be 0 for PPIs.
> >>>      ^^^^^^^^^^^^^
> >>>
> >>> Typo here ? The way I understand it, it should read "For PPIs, this
> >>> value shall be the mask of the possible CPU numbers for the interrupt
> >>> source" (or something to similar effect...).
> >>>
> >>
> >> Cut and paste error. This sentence goes in the previous paragraph. What
> >> I meant is the 2nd cell should contain 0 for PPIs as you cannot set the
> >> edge/level on PPIs (that is always true, right?). I probably should also
> >> add 0 in the list of values.
> > 
> > Ah, right. It makes sense indeed. You're correct about PPIs polarity,
> > this is defined by the hardware and cannot be configured. But it may be
> > interesting to have the DT to reflect the way the hardware is actually
> > configured (on the Cortex-A9, some PPIs are configured active-low, and
> > others are rising-edge).
> 
> So we should allow specifying what it is as the OS may need to know that.

If it is a difference between level & edge, then the OS absolutely
needs to know about it.

^ permalink raw reply

* [PATCH] arm/dt: Add SoC detection macros
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-09-18  0:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3353131.FJY7smc8i5@wuerfel>

On 22:56 Sat 17 Sep     , Arnd Bergmann wrote:
> On Saturday 17 September 2011 20:19:07 Jean-Christophe PLAGNIOL-VILLARD wrote:
> > 
> > I agree about it I just mean that if you have the same board with 2 SoC which
> > are pin to pin compatible detect the soc type will be usefull as the soc
> > resource may not be the same
> 
> In that scenario, you would put all SOC specific data into one .dtsi
> file and all board data into another .dtsi file and then create a
> description for the combination using a trivial .dts file with two
> include statements. This is all solved outside of the kernel already,
> so there is no need for infrastructure in the kernel.
except in this case you must have 2 machine id which can be avoided if you can
detect the soc at kernel level

Best Regards,
J.

^ permalink raw reply

* [PATCH 2/2] ARM: mmp: add audio sram allocator
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-09-18  0:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4E72A2A1.1020400@marvell.com>

On 09:13 Fri 16 Sep     , Leo Yan wrote:
> 
> 
> On 09/15/2011 10:38 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >On 10:59 Mon 01 Aug     , Leo Yan wrote:
> >>Implement the audio sram allocator with genalloc,
> >>so that can dynamically allocate the buffer
> >>to pcm devices as need.
> >>
> >>Signed-off-by: Leo Yan<leoy@marvell.com>
> >this can be more generic nad move to lib/genalloc.c
> 
> This is an older version, i have updated the patch same as you said.
> pls see below link for the latest patch:
> http://lists.arm.linux.org.uk/lurker/message/20110815.030920.2ad4227b.en.html
I take a look on it it's the same issue this is not mach specific

Best Regards,
J.

^ permalink raw reply

* [PATCH 3/5] of/irq: introduce of_irq_init
From: Rob Herring @ 2011-09-18  1:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110917235328.GA3523@ponder.secretlab.ca>

Grant,

On 09/17/2011 06:53 PM, Grant Likely wrote:
> On Wed, Sep 14, 2011 at 11:31:38AM -0500, Rob Herring wrote:
>> From: Rob Herring <rob.herring@calxeda.com>
>>
>> of_irq_init will scan the devicetree for matching interrupt controller
>> nodes. Then it calls an initialization function for each found controller
>> in the proper order with parent nodes initialized before child nodes.
>>
>> Based on initial pseudo code from Grant Likely.
>>
>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> ---
>>  drivers/of/irq.c       |   96 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/of_irq.h |    1 +
>>  2 files changed, 97 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>> index 9f689f1..a0cd7e8 100644
>> --- a/drivers/of/irq.c
>> +++ b/drivers/of/irq.c
>> @@ -19,10 +19,13 @@
>>   */
>>  
>>  #include <linux/errno.h>
>> +#include <linux/list.h>
>> +#include <linux/list_sort.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/of_irq.h>
>>  #include <linux/string.h>
>> +#include <linux/slab.h>
>>  
>>  /* For archs that don't support NO_IRQ (such as x86), provide a dummy value */
>>  #ifndef NO_IRQ
>> @@ -386,3 +389,96 @@ int of_irq_to_resource_table(struct device_node *dev, struct resource *res,
>>  
>>  	return i;
>>  }
>> +
>> +struct intc_desc {
>> +	struct list_head	list;
>> +	struct device_node	*dev;
>> +	struct device_node	*parent;
>> +};
>> +
>> +typedef void (*irq_init_cb_t)(struct device_node *, struct device_node *);
>> +
>> +static int __init irq_cmp_intc_desc(void *unused, struct list_head *a,
>> +				    struct list_head *b)
>> +{
>> +	const struct intc_desc *da = list_entry(a, typeof(*da), list);
>> +	const struct intc_desc *db = list_entry(b, typeof(*db), list);
>> +
>> +	/* same parent, so order doesn't matter */
>> +	if (da->parent == db->parent)
>> +		return 0;
>> +
>> +	/* NULL parent comes first */
>> +	if (!da->parent && db->parent)
>> +		return -1;
>> +	if (!db->parent && da->parent)
>> +		return 1;
>> +
>> +	/* parent node must be before child node */
>> +	if (da->dev == db->parent)
>> +		return -1;
>> +	if (db->dev == da->parent)
>> +		return 1;
> 
> Does sort_list work for relationships 4 or more levels deep?  ie. if
> there was a relationship of A <- B <- C <- D, then B compared with D
> would return 0 from this function which could potentially result in an
> incorrectly ordered list.
> 

Doh! Um, 3 levels is enough for everyone!? ;)

> The other option for implementing this would be to take the probe
> deferral approach and not try to sort the list, but instead allow
> probe functions to fail & request retry if the parent hasn't yet been
> probed.  I haven't thought enough about it though to say which would
> be the best approach.
> 

Considering the list will typically be only a few entries, it is
probably not so important how efficiently we sort or walk the list.

The only way I see controller code knowing if it needs to defer init is
if of_irq_create_mapping fails. The core code could simply do this
itself. However, I would imagine sorting it would be faster than that path.

How about something like this (untested):

int find_order(struct intc_desc *node)
{
	struct intc_desc *d;

	list_for_each_entry(d, &intc_desc_list, list) {
		if (node->parent != d->dev)
			continue;

		if (d->order < 0)
			find_order(d);

		node->order = d->order + 1;
		break;
	}
}


Then rather than sorting, do this:


	list_for_each_entry(desc, &intc_desc_list, list)
		find_order(desc);

	for (order = 0; !list_empty(&intc_desc_list); order++) {
		list_for_each_entry_safe(desc, temp_desc, &intc_desc_list, list) {
			if (desc->order != order)
				continue;
			
			match = of_match_node(matches, desc->dev);
			if (match && match->data) {
				irq_init_cb_t irq_init_cb = match->data;
				pr_debug("of_irq_init: init %s @ %p, parent %p\n",
					 match->compatible, desc->dev, desc->parent);
				irq_init_cb(desc->dev, desc->parent);
			}
			list_del(&desc->list);
			kfree(desc);
		}
	}


>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * of_irq_init - Scan the device tree for matching interrupt controllers and
>> + * call their initialization functions in order with parents first.
>> + * @matches: 0 terminated array of nodes to match and initialization function
>> + * to call on match
>> + */
>> +void __init of_irq_init(const struct of_device_id *matches)
>> +{
>> +	struct device_node *np;
>> +	const struct of_device_id *match;
>> +	struct intc_desc *desc;
>> +	struct intc_desc *temp_desc;
>> +	struct list_head intc_desc_list;
>> +
>> +	INIT_LIST_HEAD(&intc_desc_list);
>> +
>> +	for_each_matching_node(np, matches) {
>> +		if (!of_find_property(np, "interrupt-controller", NULL))
>> +			continue;
>> +		/* Here, we allocate and populate an intc_desc with the node
>> +		* pointer, interrupt-parent device_node etc. */
>> +		desc = kzalloc(sizeof(*desc), GFP_KERNEL);
>> +		if (!desc) {
>> +			WARN_ON(1);
>> +			goto err;
>> +		}
>> +		desc->dev = np;
>> +		desc->parent = of_irq_find_parent(np);
>> +		list_add(&desc->list, &intc_desc_list);
>> +	}
>> +	if (list_empty(&intc_desc_list))
>> +		return;
>> +
>> +	/*
>> +	 * The root irq controller is the one without an interrupt-parent.
>> +	 * That one goes first, followed by the controllers that reference it,
>> +	 * followed by the ones that reference the 2nd level controllers, etc
>> +	 */
> 
> I don't believe that this actually turns out to be true (and yes I
> know it is how I originally described it).  :-)  When the
> interrupt-parent property is at the root of the tree, then the root
> interrupt controller may very well inherit itself as it's interrupt
> parent, and of_irq_find_parent() will still return a value.  This
> should probably be considered a bug in of_irq_find_parent(), and it
> should return NULL if the parent is itself.

I did hit this exact issue. There is an easy, but not obvious fix to the
device tree. Simply adding "interupt-parent;" to the root interrupt
controller node will do the trick and override the value in the tree root.

> 
> of_irq_find_parent should probably be implemented thusly (completely
> untested); although the only functional change is the line:
> 	return (p == child) ? NULL : p;
> 
> /**
>  * of_irq_find_parent - Given a device node, find its interrupt parent node
>  * @child: pointer to device node
>  *
>  * Returns a pointer to the interrupt parent node, or NULL if the
>  * interrupt parent could not be determined.
>  */
> struct device_node *of_irq_find_parent(struct device_node *child)
> {
> 	struct device_node *p, *c = child;
> 	const __be32 *parp;
> 
> 	if (!of_node_get(c))
> 		return NULL;
> 
> 	do {
> 		p = of_parse_phandle(c, "interrupt-parent", 0);
> 
> 		if (!p && (of_irq_workarounds & OF_IMAP_NO_PHANDLE) &&
> 		    of_find_property(c, "interrupt-parent", NULL))
> 			p = of_node_get(of_irq_dflt_pic);
> 
> 		if (!p)
> 			p = of_get_parent(c);
> 
> 		of_node_put(c);
> 		c = p;
> 	} while (p && !of_find_property(p, "#interrupt-cells", NULL));
> 
> 	return (p == child) ? NULL : p;
> }
> 

This change should probably be implemented as well as this is likely a
common occurrence that will be stumbled over or existing device trees
won't have this. I'll test and add to the next series.

Rob

^ permalink raw reply

* [PATCH 0/19] removal of mach/vmalloc.h and generic optimizations
From: Nicolas Pitre @ 2011-09-18  2:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3319383.WZQsqr8ND4@wuerfel>

On Sat, 17 Sep 2011, Arnd Bergmann wrote:

> On Friday 16 September 2011 03:07:11 Nicolas Pitre wrote:
> > This patch series removes all instances of mach/vmalloc.h in order to
> > have a more unified memory map across all ARM architectures.  To do so,
> > the static mappings are moved inside the vmalloc area.  And finally this
> > allows for a generic optimization to ioremap where static mappings are
> > reused whenever possible, using common code instead of having this
> > duplicated in a couple places.
> > 
> > This also provides a net reduction of more than 1200 lines of code.
> > 
> > Those patches are also available in the following repository:
> > 
> >         git://git.linaro.org/people/nico/linux vmalloc
> 
> Hi Nicolas,
> 
> I like the series a lot. I had planned to work on this myself, but
> I guess you did a better job than I could have anyway.

Thanks!

> Doing some randconfig tests, I noticed that your series is currently broken
> on shmobile, which triggers a 
> 
> 	BUILD_BUG_ON(VMALLOC_END > CONSISTENT_BASE);
> 
> in mem_init(), because the platform has a CONSISTENT_DMA_SIZE of 158MB.
> All other platforms have a CONSISTENT_DMA_SIZE of at most 14MB, which
> works correctly.

Any idea why shmobile requires such an amount of consistent memory?

> My feeling is that a 158MB CONSISTENT_DMA_SIZE causes other problems
> anyway because we cannot map regular RAM both cached and uncached,
> but this is still a regression. The problem might be solved using
> Marek's CMA patches, which should eliminate the need for a huge
> CONSISTENT_DMA_SIZE.

Maybe.  However I don't want for this series to depend on CMA.  I'll 
find some temporary hack in the mean time.


Nicolas

^ permalink raw reply

* [PATCH 3/5] of/irq: introduce of_irq_init
From: Grant Likely @ 2011-09-18  6:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4E754B56.1010404@gmail.com>

On Sat, Sep 17, 2011 at 08:37:26PM -0500, Rob Herring wrote:
> Grant,
> 
> On 09/17/2011 06:53 PM, Grant Likely wrote:
> > On Wed, Sep 14, 2011 at 11:31:38AM -0500, Rob Herring wrote:
> > The other option for implementing this would be to take the probe
> > deferral approach and not try to sort the list, but instead allow
> > probe functions to fail & request retry if the parent hasn't yet been
> > probed.  I haven't thought enough about it though to say which would
> > be the best approach.
> > 
> 
> Considering the list will typically be only a few entries, it is
> probably not so important how efficiently we sort or walk the list.
> 
> The only way I see controller code knowing if it needs to defer init is
> if of_irq_create_mapping fails. The core code could simply do this
> itself. However, I would imagine sorting it would be faster than that path.
> 
> How about something like this (untested):
> 
> int find_order(struct intc_desc *node)
> {
> 	struct intc_desc *d;
> 
> 	list_for_each_entry(d, &intc_desc_list, list) {
> 		if (node->parent != d->dev)
> 			continue;
> 
> 		if (d->order < 0)
> 			find_order(d);
> 
> 		node->order = d->order + 1;
> 		break;
> 	}
> }
> 
> 
> Then rather than sorting, do this:
> 
> 
> 	list_for_each_entry(desc, &intc_desc_list, list)
> 		find_order(desc);
> 
> 	for (order = 0; !list_empty(&intc_desc_list); order++) {
> 		list_for_each_entry_safe(desc, temp_desc, &intc_desc_list, list) {
> 			if (desc->order != order)
> 				continue;
> 			
> 			match = of_match_node(matches, desc->dev);
> 			if (match && match->data) {
> 				irq_init_cb_t irq_init_cb = match->data;
> 				pr_debug("of_irq_init: init %s @ %p, parent %p\n",
> 					 match->compatible, desc->dev, desc->parent);
> 				irq_init_cb(desc->dev, desc->parent);
> 			}
> 			list_del(&desc->list);
> 			kfree(desc);
> 		}
> 	}

I don't think there needs to be a separate pass for calculating order.
What about a breadth-first search with a secondary list for pending
parents?  Something like:


	struct list_head intc_desc_list;
	struct list_head intc_parent_list;
	struct device_node *parent = NULL;

	while (!list_empty(&intc_desc_list)) {
		/*
		 * Process all controllers with the current 'parent'.
		 * First pass will be looking for NULL as the parent.
		 * The assumption is that NULL parent means a root
		 * controller
		 */
		list_for_each_entry_safe(desc, temp_desc, &intc_desc_list, list) {
			if (desc->parent == parent) {
				list_del(&desc->list);
				irq_init_cb_t irq_init_cb = match->data;
				pr_debug("of_irq_init: init %s @ %p, parent %p\n",
					 match->compatible, desc->dev, desc->parent);
				irq_init_cb(desc->dev, desc->parent);

				/*
				 * This one is now set up; add it to
				 * the parent list so its children
				 * can get processed in a subsequent
				 * pass.
				 */
				list_add_tail(&desc->list, &intc_parent_list);
			}
		}

		/* Get the next pending parent that might have children */
		desc = list_first_entry(&intc_parent_list, typeof(*desc), list);
		if (!desc) {
			pr_error("Danger Will Robinson!");
			break;
		}
		list_del(&desc->list);
		parent = desc->list
		kfree(desc);
	}

> >> +	/*
> >> +	 * The root irq controller is the one without an interrupt-parent.
> >> +	 * That one goes first, followed by the controllers that reference it,
> >> +	 * followed by the ones that reference the 2nd level controllers, etc
> >> +	 */
> > 
> > I don't believe that this actually turns out to be true (and yes I
> > know it is how I originally described it).  :-)  When the
> > interrupt-parent property is at the root of the tree, then the root
> > interrupt controller may very well inherit itself as it's interrupt
> > parent, and of_irq_find_parent() will still return a value.  This
> > should probably be considered a bug in of_irq_find_parent(), and it
> > should return NULL if the parent is itself.
> 
> I did hit this exact issue. There is an easy, but not obvious fix to the
> device tree. Simply adding "interupt-parent;" to the root interrupt
> controller node will do the trick and override the value in the tree root.

Hahaha.  Definitely a bug then.

> > of_irq_find_parent should probably be implemented thusly (completely
> > untested); although the only functional change is the line:
> > 	return (p == child) ? NULL : p;
> > 
> > /**
> >  * of_irq_find_parent - Given a device node, find its interrupt parent node
> >  * @child: pointer to device node
> >  *
> >  * Returns a pointer to the interrupt parent node, or NULL if the
> >  * interrupt parent could not be determined.
> >  */
> > struct device_node *of_irq_find_parent(struct device_node *child)
> > {
> > 	struct device_node *p, *c = child;
> > 	const __be32 *parp;
> > 
> > 	if (!of_node_get(c))
> > 		return NULL;
> > 
> > 	do {
> > 		p = of_parse_phandle(c, "interrupt-parent", 0);
> > 
> > 		if (!p && (of_irq_workarounds & OF_IMAP_NO_PHANDLE) &&
> > 		    of_find_property(c, "interrupt-parent", NULL))
> > 			p = of_node_get(of_irq_dflt_pic);
> > 
> > 		if (!p)
> > 			p = of_get_parent(c);
> > 
> > 		of_node_put(c);
> > 		c = p;
> > 	} while (p && !of_find_property(p, "#interrupt-cells", NULL));
> > 
> > 	return (p == child) ? NULL : p;
> > }
> > 
> 
> This change should probably be implemented as well as this is likely a
> common occurrence that will be stumbled over or existing device trees
> won't have this.

Existing device trees probably do have this, but all the powerpc board
support currently hard codes the interrupt controller setup.  They
will hit the same issue if converted to use this approach.

> I'll test and add to the next series.

Awesome, thanks!

g.

^ permalink raw reply

* [PATCH 5/5] ARM: gic: add OF based initialization
From: Grant Likely @ 2011-09-18  6:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAJuYYwQ=tSh8k5ZOi2kx6KbMsQ4eVAvgE=T4kdckRSLjdj3dMQ@mail.gmail.com>

On Fri, Sep 16, 2011 at 03:04:11PM +0530, Thomas Abraham wrote:
> Hi Rob,
> 
> On 15 September 2011 18:24, Rob Herring <robherring2@gmail.com> wrote:
> > On 09/15/2011 02:55 AM, Thomas Abraham wrote:
> >>> +void __init gic_of_init(struct device_node *node, struct device_node *parent)
> >>> +{
> >>> + ? ? ? void __iomem *cpu_base;
> >>> + ? ? ? void __iomem *dist_base;
> >>> + ? ? ? int irq;
> >>> + ? ? ? struct irq_domain *domain = &gic_data[gic_cnt].domain;
> >>> +
> >>> + ? ? ? if (WARN_ON(!node))
> >>> + ? ? ? ? ? ? ? return;
> >>> +
> >>> + ? ? ? dist_base = of_iomap(node, 0);
> >>> + ? ? ? WARN(!dist_base, "unable to map gic dist registers\n");
> >>> +
> >>> + ? ? ? cpu_base = of_iomap(node, 1);
> >>> + ? ? ? WARN(!cpu_base, "unable to map gic cpu registers\n");
> >>> +
> >>> + ? ? ? domain->nr_irq = gic_irq_count(dist_base);
> >>> + ? ? ? domain->irq_base = irq_alloc_descs(-1, 0, domain->nr_irq, numa_node_id());
> >>
> >> For exynos4, all the interrupts originating from GIC are statically
> >> mapped to start from 32 in the linux virq space (GIC SPI interrupts
> >> start from 64). In the above code, since irq_base would be 0 for
> >> exynos4, the interrupt mapping is not working correctly. In your
> >> previous version of the patch, you have given a option to the platform
> >> code to choose the offset. Could that option be added to this series
> >> also. Or a provision to use platform specific translate function
> >> instead of the irq_domain_simple translator.
> >>
> >
> > So I guess you have the A9 external nIRQ hooked up to another
> > controller? Why can't the 0-31 interrupts get mapped to after the gic
> > interrupts? Ultimately we want h/w irq numbers completely decoupled from
> > linux irq numbers. So you will want to put that controller in devicetree
> > and have an DT init function for it as well.
> 
> There are chained interrupt handlers mapped in between linux irq
> number 0 to 31. So the offset for GIC interrupts was set to 32 (SGI[0]
> = 32). The interrupt chaining for the interrupts mapped between 0 to
> 31 seems unnecessary though. I will try removing them and check.

Please note; when using the DT, the linux virq number should be
dynamically assigned and therefore will not matter.  Historically
Exynos may have started from irq 32, but it doesn't really have any
relevance when all IRQ references are via DT irq specifiers.

Plus, for dynamically allocated irq_descs, I really want to make sure
that irq 0 never gets assigned.  We're not supposed to be using it,
and that becomes an easy rule to enforce when interrupt numbers are no
longer assigned with #defines.

g.

^ permalink raw reply

* [PATCH 5/5] ARM: gic: add OF based initialization
From: Grant Likely @ 2011-09-18  6:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4E71CE5D.9030900@ti.com>

On Thu, Sep 15, 2011 at 12:07:25PM +0200, Cousson, Benoit wrote:
> Hi Rob,
> 
> On 9/15/2011 9:55 AM, Thomas Abraham wrote:
> >Hi Rob,
> >
> >On 14 September 2011 22:01, Rob Herring<robherring2@gmail.com>  wrote:
> >>From: Rob Herring<rob.herring@calxeda.com>
> >>
> >>This adds gic initialization using device tree data. The initialization
> >>functions are intended to be called by a generic OF interrupt
> >>controller parsing function once the right pieces are in place.
> >>
> >>PPIs are handled using 3rd cell of interrupts properties to specify the cpu
> >>mask the PPI is assigned to.
> >>
> >>Signed-off-by: Rob Herring<rob.herring@calxeda.com>
> >>---
> >>  Documentation/devicetree/bindings/arm/gic.txt |   53 ++++++++++++++++++++++++
> >>  arch/arm/common/gic.c                         |   55 +++++++++++++++++++++++--
> >>  arch/arm/include/asm/hardware/gic.h           |   10 +++++
> >>  3 files changed, 114 insertions(+), 4 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
> >
> >[...]
> >
> >
> >>diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> >>index d1ccc72..14de380 100644
> >>--- a/arch/arm/common/gic.c
> >>+++ b/arch/arm/common/gic.c
> >
> >[...]
> >
> >>+void __init gic_of_init(struct device_node *node, struct device_node *parent)
> >>+{
> >>+       void __iomem *cpu_base;
> >>+       void __iomem *dist_base;
> >>+       int irq;
> >>+       struct irq_domain *domain =&gic_data[gic_cnt].domain;
> >>+
> >>+       if (WARN_ON(!node))
> >>+               return;
> >>+
> >>+       dist_base = of_iomap(node, 0);
> >>+       WARN(!dist_base, "unable to map gic dist registers\n");
> >>+
> >>+       cpu_base = of_iomap(node, 1);
> >>+       WARN(!cpu_base, "unable to map gic cpu registers\n");
> >>+
> >>+       domain->nr_irq = gic_irq_count(dist_base);
> >>+       domain->irq_base = irq_alloc_descs(-1, 0, domain->nr_irq, numa_node_id());
> >
> >For exynos4, all the interrupts originating from GIC are statically
> >mapped to start from 32 in the linux virq space (GIC SPI interrupts
> >start from 64). In the above code, since irq_base would be 0 for
> >exynos4, the interrupt mapping is not working correctly. In your
> >previous version of the patch, you have given a option to the platform
> >code to choose the offset. Could that option be added to this series
> >also. Or a provision to use platform specific translate function
> >instead of the irq_domain_simple translator.
> 
> I have another concern on a similar topic.
> 
> On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset
> of 32. Only the internal PPI are between 0 and 31.
> 
> For the moment we add 32 to every SoC interrupts in the irq.h
> define, but I'm assuming that this offset calculation should be done
> thanks to a dedicated irq domain for the SPI.
> The real HW physical number start at 0, and thus this is that value
> that should be in the irq binding of the device.

Yes.

> So ideally we should have a irq domain for the PPI starting at 0 and
> another one for the SPI starting at 32. Or 32 and 64 for the exynos4
> case, but it looks like the PPI/SPI offset is always 32.

Part of the purpose behind irq_domains is to have a translator
callback that can take care of complex mappings, such as mapping each
of the GIC irq ranges onto the Linux irq space.  Plus, by being based
on the DT irq specifiers and dynamically assigning the linux numbers,
the actual mapping that the kernel chooses to use shouldn't actually
have any relevance.  So whether or not the driver uses an offset is 32
becomes an implementation detail.

^ permalink raw reply

* [PATCH 5/5] ARM: gic: add OF based initialization
From: Grant Likely @ 2011-09-18  6:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110916160939.GA2100@arm.com>

On Fri, Sep 16, 2011 at 05:09:39PM +0100, Dave Martin wrote:
> For now, we express the mapping by putting an interrupt-map in the
> core-tile DT, but this feels inelegant as well as wasteful -- expressing
> "+ 32" using a table which is about 1K in size and duplicates that
> information 43 times.
> 
> Using a dedicated irq domain or a fake interrupt controller node to
> encapsulate the motherboard interrupts feels like a cleaner approach,
> but for now I'm not clear on the best way to do it.

An irq nexus node would indeed be something to investigate for your
particular case.  Look for examples of interrupt-map.  It is most
often used for handling IRQ swizzling on PCI busses.

g.

^ permalink raw reply

* [PATCH 4/5] ARM: gic: allow irq_start to be 0
From: Grant Likely @ 2011-09-18  6:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316017900-19918-5-git-send-email-robherring2@gmail.com>

On Wed, Sep 14, 2011 at 11:31:39AM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> There's really no need to set irq_start per platform for the primary gic.
> The SGIs and PPIs are not handled as normal irqs, so how irqs 0-31 are
> setup doesn't really matter. So allow irq_start to be set to 0 to match
> the linux irq numbering.

Is this really what you want?  Linux irq 0 is not supposed to be used.
With the transition to dynamically allocated irq_descs, I want to make
sure 0 never gets assigned.

g.

> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
>  arch/arm/common/gic.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> index 666b278..d1ccc72 100644
> --- a/arch/arm/common/gic.c
> +++ b/arch/arm/common/gic.c
> @@ -359,7 +359,7 @@ void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
>  	gic = &gic_data[gic_nr];
>  	gic->dist_base = dist_base;
>  	gic->cpu_base = cpu_base;
> -	gic->irq_offset = (irq_start - 1) & ~31;
> +	gic->irq_offset = irq_start ? (irq_start - 1) & ~31 : 0;
>  
>  	if (gic_nr == 0)
>  		gic_cpu_base_addr = cpu_base;
> -- 
> 1.7.5.4
> 

^ permalink raw reply

* [PATCH 5/5] ARM: gic: add OF based initialization
From: Grant Likely @ 2011-09-18  6:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316017900-19918-6-git-send-email-robherring2@gmail.com>

On Wed, Sep 14, 2011 at 11:31:40AM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> This adds gic initialization using device tree data. The initialization
> functions are intended to be called by a generic OF interrupt
> controller parsing function once the right pieces are in place.
> 
> PPIs are handled using 3rd cell of interrupts properties to specify the cpu
> mask the PPI is assigned to.
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
>  Documentation/devicetree/bindings/arm/gic.txt |   53 ++++++++++++++++++++++++
>  arch/arm/common/gic.c                         |   55 +++++++++++++++++++++++--
>  arch/arm/include/asm/hardware/gic.h           |   10 +++++
>  3 files changed, 114 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> new file mode 100644
> index 0000000..6c513de
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -0,0 +1,53 @@
> +* ARM Generic Interrupt Controller
> +
> +ARM SMP cores are often associated with a GIC, providing per processor
> +interrupts (PPI), shared processor interrupts (SPI) and software
> +generated interrupts (SGI).
> +
> +Primary GIC is attached directly to the CPU and typically has PPIs and SGIs.
> +Secondary GICs are cascaded into the upward interrupt controller and do not
> +have PPIs or SGIs.
> +
> +Main node required properties:
> +
> +- compatible : should be one of:
> +	"arm,cortex-a9-gic"
> +	"arm,arm11mp-gic"
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> +  interrupt source.  The type shall be a <u32> and the value shall be 3.
> +
> +  The 1st cell is the interrupt number. 0-15 are reserved for SGIs. 16-31 are
> +  for PPIs.
> +
> +  The 2nd cell is the level-sense information, encoded as follows:
> +                    1 = low-to-high edge triggered
> +                    2 = high-to-low edge triggered
> +                    4 = active high level-sensitive
> +                    8 = active low level-sensitive
> +
> +  Only values of 1 and 4 are valid for GIC 1.0 spec.
> +
> +  The 3rd cell contains the mask of the cpu number for the interrupt source.
> +  The cpu mask is only valid for PPIs and shall be 0 for SPIs. This value shall
> +  be 0 for PPIs.

THe binding looks good, but I really think the first cell should be
the mask; followed by the irq number in the 2nd and the flags in the
3rd.  That would better match with existing practice.

> +
> +- reg : Specifies base physical address(s) and size of the GIC registers. The
> +  first 2 values are the GIC distributor register base and size. The 2nd 2

... first region is the GIC distributor...

... The 2nd region is the GIC cpu ...


I've only looked at the code changes superficially, but it looks good
to me.

g.

^ permalink raw reply

* [PATCH 3/7] ASoC: edb93xx: convert to use snd_soc_register_card()
From: Mika Westerberg @ 2011-09-18  7:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316260700.17769.8.camel@r60e>

On Sat, Sep 17, 2011 at 01:58:20PM +0200, Alexander Sverdlin wrote:
> 
> I've checked mainline linux-next without your patches, it's broken too.
> So your patches are not the root cause for that.

Ok thanks.

Still it would be good to find out which causes it to fail. Are you able to
load all the modules manually? Does it work then?

^ permalink raw reply

* [PATCH 0/19] removal of mach/vmalloc.h and generic optimizations
From: Arnd Bergmann @ 2011-09-18  8:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.00.1109172238070.20358@xanadu.home>

On Saturday 17 September 2011 22:46:33 Nicolas Pitre wrote:
> On Sat, 17 Sep 2011, Arnd Bergmann wrote:
> 
> > On Friday 16 September 2011 03:07:11 Nicolas Pitre wrote:
> > > This patch series removes all instances of mach/vmalloc.h in order to
> > > have a more unified memory map across all ARM architectures.  To do so,
> > > the static mappings are moved inside the vmalloc area.  And finally this
> > > allows for a generic optimization to ioremap where static mappings are
> > > reused whenever possible, using common code instead of having this
> > > duplicated in a couple places.
> > > 
> > > This also provides a net reduction of more than 1200 lines of code.

> 
> > Doing some randconfig tests, I noticed that your series is currently broken
> > on shmobile, which triggers a 
> > 
> > 	BUILD_BUG_ON(VMALLOC_END > CONSISTENT_BASE);
> > 
> > in mem_init(), because the platform has a CONSISTENT_DMA_SIZE of 158MB.
> > All other platforms have a CONSISTENT_DMA_SIZE of at most 14MB, which
> > works correctly.
> 
> Any idea why shmobile requires such an amount of consistent memory?

No, I couldn't find anything in the code or the changelog why this was
done. Magnus changed it in this commit:

commit 28f0721a79046056ced3b9bd79c319c5c417ec30
Author: Magnus Damm <damm@opensource.se>
Date:   Wed Apr 28 08:25:30 2010 +0000

    ARM: mach-shmobile: Set CONSISTENT_DMA_SIZE to 158 MB
    
    This patch sets CONSISTENT_DMA_SIZE to 158 MB
    for all SH-Mobile ARM processors.
    
    The DMA area is mapped at 0xf6000000 - 0xffdfffff,
    on top of the 256 MB I/O window at 0xe6000000.
    
    Signed-off-by: Magnus Damm <damm@opensource.se>
    Signed-off-by: Paul Mundt <lethal@linux-sh.org>

Maybe he can give a better explanation and can say whether it's
actually required. The patch series introducing it contained
the introduction of the shdma dmaengine driver and some changes
to video drivers, but I could not tell how they are related
to this change.

> > My feeling is that a 158MB CONSISTENT_DMA_SIZE causes other problems
> > anyway because we cannot map regular RAM both cached and uncached,
> > but this is still a regression. The problem might be solved using
> > Marek's CMA patches, which should eliminate the need for a huge
> > CONSISTENT_DMA_SIZE.
> 
> Maybe.  However I don't want for this series to depend on CMA.  I'll 
> find some temporary hack in the mean time.

Ok

	Arnd

^ permalink raw reply

* [PATCH 20/25] OMAP4: PM: Add L2X0 cache lowpower support
From: Santosh @ 2011-09-18  8:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <87iposqu3j.fsf@ti.com>

On Friday 16 September 2011 10:53 PM, Kevin Hilman wrote:
> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
> 
>> When MPUSS hits off-mode e, L2 cache is lost. This patch adds L2X0
>                           ^^^
> extra 'e' ?
> 
>> necessary maintenance operations and context restoration in the
>> low power code.
>>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Cc: Kevin Hilman <khilman@ti.com>
> 
> [...]
> 
>> @@ -135,6 +138,33 @@ static void scu_pwrst_prepare(unsigned int cpu_id, unsigned int cpu_state)
>>  	__raw_writel(scu_pwr_st, pm_info->scu_sar_addr);
>>  }
>>  
>> +/*
>> + * Store the CPU cluster state for L2X0 low power operations.
>> + */
>> +static void l2x0_pwrst_prepare(unsigned int cpu_id, unsigned int save_state)
>> +{
>> +	struct omap4_cpu_pm_info *pm_info = &per_cpu(omap4_pm_info, cpu_id);
>> +
>> +	__raw_writel(save_state, pm_info->l2x0_sar_addr);
>> +}
>> +
>> +/*
>> + * Save the L2X0 AUXCTRL and POR value to SAR memory. Its used to
>> + * in every restore MPUSS OFF path.
>> + */
>> +static void save_l2x0_context(void)
>> +{
>> +#ifdef CONFIG_CACHE_L2X0
>> +	u32 val;
>> +	void __iomem *l2x0_base = omap4_get_l2cache_base();
>> +
>> +	val = __raw_readl(l2x0_base + L2X0_AUX_CTRL);
>> +	__raw_writel(val, sar_base + L2X0_AUXCTRL_OFFSET);
>> +	val = __raw_readl(l2x0_base + L2X0_PREFETCH_CTRL);
>> +	__raw_writel(val, sar_base + L2X0_PREFETCH_CTRL_OFFSET);
>> +#endif
> 
> nit:  (c.f. '#ifdefs are ugly' in Documentatin/SubmittingPatches)
> 
> This should probably be more like
> 
> #ifdef CONFIG_CACHE_L2X0
> static void save_l2x0_context(void)
> {
>         /* real function */
> }
> #else
> static void save_l2x0_context(void) {}
> #endif
> 
Ok. Will fix this.

Regards
Santosh

^ permalink raw reply

* [PATCH 23/25] OMAP4: PM: Add CPUidle support
From: Santosh @ 2011-09-18  8:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <87k498pejf.fsf@ti.com>

On Friday 16 September 2011 11:15 PM, Kevin Hilman wrote:
> Hi Santosh,
> 
> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
> 
>> Add OMAP4 CPUIDLE support. CPU1 is left with defualt idle and
>> the low power state for it is managed via cpu-hotplug.
>>
>> This patch adds MPUSS low power states in cpuidle.
>>
>> 	C1 - CPU0 ON + CPU1 ON + MPU ON
>> 	C2 - CPU0 OFF + CPU1 OFF + MPU CSWR
>> 	C3 - CPU0 OFF + CPU1 OFF + MPU OSWR
>>
>> OMAP4460 onwards, MPUSS power domain doesn't support OFF state any more
>> anymore just like CORE power domain. The deepest state supported is OSWr.
>> Ofcourse when MPUSS and CORE PD transitions to OSWR along with device
>> off mode, even the memory contemts are lost which is as good as
>> the PD off state.
>>
>> On OMAP4 because of hardware constraints, no low power states are
>> targeted when both CPUs are online and in SMP mode. The low power
>> states are attempted only when secondary CPU gets offline to OFF
>> through hotplug infrastructure.
>>
>> Thanks to Nicole Chalhoub <n-chalhoub@ti.com> for doing exhaustive
>> C-state latency profiling.
>>
>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Cc: Kevin Hilman <khilman@ti.com>
> 
> A handful of minor comments below...
> 
Will take care of them.

Regards
Santosh

^ permalink raw reply

* [PATCH 24/25] OMAP4: cpuidle: Switch to gptimer from twd in deeper C-states.
From: Santosh @ 2011-09-18  8:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <878vpope90.fsf@ti.com>

On Friday 16 September 2011 11:21 PM, Kevin Hilman wrote:
> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
> 
>> CPU local timer(TWD) stops when the CPU is transitioning into
>> deeper C-States. Since these timers are not wakeup capable, we
>> need the wakeup capable global timer to program the wakeup time
>> depending on the next timer expiry.
>>
>> It can be handled by registering a global wakeup capable timer along
>> with local timers marked with (mis)feature flag CLOCK_EVT_FEAT_C3STOP.
> 
> nit: this comment makes is sound like this patch adds the registration
> of this timer, when all this patch does is add the notification.
> 
> Changelog should be updated to make it clear that the global timer exists
> already, and is already marked with the C3STOP flag.
> 
 Will add couple of lines in the change log to make it clear.

>> Then notify the clock events layer from idle code using
>> CLOCK_EVT_NOTIFY_BROADCAST_ENTER/EXIT).
>>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Cc: Kevin Hilman <khilman@ti.com>
> 
> Other than that
> 
> Acked-by: Kevin Hilman <khilman@ti.com>
> 
Thanks.

Regards
Santosh

^ permalink raw reply

* [PATCH] arm/dt: Add SoC detection macros
From: Arnd Bergmann @ 2011-09-18  9:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110918004615.GC16141@game.jcrosoft.org>

On Sunday 18 September 2011 02:46:15 Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 22:56 Sat 17 Sep     , Arnd Bergmann wrote:
> > On Saturday 17 September 2011 20:19:07 Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > 
> > > I agree about it I just mean that if you have the same board with 2 SoC which
> > > are pin to pin compatible detect the soc type will be usefull as the soc
> > > resource may not be the same
> > 
> > In that scenario, you would put all SOC specific data into one .dtsi
> > file and all board data into another .dtsi file and then create a
> > description for the combination using a trivial .dts file with two
> > include statements. This is all solved outside of the kernel already,
> > so there is no need for infrastructure in the kernel.
> except in this case you must have 2 machine id which can be avoided if you can
> detect the soc at kernel level

But when you move to device tree based probing, you don't use machine numbers
at all, so it doesn't matter.

Detecting the soc in the kernel is pointless, since you describe it in
the device tree anyway. Ideally, the kernel should not need to know about
the possible SoCs at all, it just needs to know the components on it.

While we first want to get to the point where the device tree only needs
to describe the off-chip devices, in the long run it's much better to
describe all of the hardware in the device tree, as we do with new platforms
(zynq, prima2, ...) already. I don't think we need to introduce
infrastructure now when we know that we want to get rid of it later.

	Arnd

^ permalink raw reply

* [PATCH v3] ARM: kprobes: Add some benchmarking to test module
From: Tixy @ 2011-09-18 11:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.00.1109112241310.20358@xanadu.home>

From: Jon Medhurst <tixy@yxit.co.uk>

These benchmarks show the basic speed of kprobes and verify the success
of optimisations done to the emulation of typical function entry
instructions (i.e. push/stmdb).

Signed-off-by: Jon Medhurst <tixy@yxit.co.uk>
Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---

Changes since v2:
- Modify benchmark() to use sched_clock() rather than do_gettimeofday()
  as this has lower overheads and gives simpler code - as suggested by
  Nicolas Pitre.

---
 arch/arm/kernel/kprobes-test.c |  161 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 161 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/kprobes-test.c b/arch/arm/kernel/kprobes-test.c
index fe169e9..e5740c7 100644
--- a/arch/arm/kernel/kprobes-test.c
+++ b/arch/arm/kernel/kprobes-test.c
@@ -185,6 +185,9 @@
 #include "kprobes-test.h"
 
 
+#define BENCHMARKING	1
+
+
 /*
  * Test basic API
  */
@@ -444,6 +447,157 @@ static int run_api_tests(long (*func)(long, long))
 
 
 /*
+ * Benchmarking
+ */
+
+#if BENCHMARKING
+
+static void __naked benchmark_nop(void)
+{
+	__asm__ __volatile__ (
+		"nop		\n\t"
+		"bx	lr"
+	);
+}
+
+#ifdef CONFIG_THUMB2_KERNEL
+#define wide ".w"
+#else
+#define wide
+#endif
+
+static void __naked benchmark_pushpop1(void)
+{
+	__asm__ __volatile__ (
+		"stmdb"wide"	sp!, {r3-r11,lr}  \n\t"
+		"ldmia"wide"	sp!, {r3-r11,pc}"
+	);
+}
+
+static void __naked benchmark_pushpop2(void)
+{
+	__asm__ __volatile__ (
+		"stmdb"wide"	sp!, {r0-r8,lr}  \n\t"
+		"ldmia"wide"	sp!, {r0-r8,pc}"
+	);
+}
+
+static void __naked benchmark_pushpop3(void)
+{
+	__asm__ __volatile__ (
+		"stmdb"wide"	sp!, {r4,lr}  \n\t"
+		"ldmia"wide"	sp!, {r4,pc}"
+	);
+}
+
+static void __naked benchmark_pushpop4(void)
+{
+	__asm__ __volatile__ (
+		"stmdb"wide"	sp!, {r0,lr}  \n\t"
+		"ldmia"wide"	sp!, {r0,pc}"
+	);
+}
+
+
+#ifdef CONFIG_THUMB2_KERNEL
+
+static void __naked benchmark_pushpop_thumb(void)
+{
+	__asm__ __volatile__ (
+		"push.n	{r0-r7,lr}  \n\t"
+		"pop.n	{r0-r7,pc}"
+	);
+}
+
+#endif
+
+static int __kprobes
+benchmark_pre_handler(struct kprobe *p, struct pt_regs *regs)
+{
+	return 0;
+}
+
+static int benchmark(void(*fn)(void))
+{
+	unsigned n, i, t, t0;
+
+	for (n = 1000; ; n *= 2) {
+		t0 = sched_clock();
+		for (i = n; i > 0; --i)
+			fn();
+		t = sched_clock() - t0;
+		if (t >= 250000000)
+			break; /* Stop once we took more than 0.25 seconds */
+	}
+	return t / n; /* Time for one iteration in nanoseconds */
+};
+
+static int kprobe_benchmark(void(*fn)(void), unsigned offset)
+{
+	struct kprobe k = {
+		.addr		= (kprobe_opcode_t *)((uintptr_t)fn + offset),
+		.pre_handler	= benchmark_pre_handler,
+	};
+
+	int ret = register_kprobe(&k);
+	if (ret < 0) {
+		pr_err("FAIL: register_kprobe failed with %d\n", ret);
+		return ret;
+	}
+
+	ret = benchmark(fn);
+
+	unregister_kprobe(&k);
+	return ret;
+};
+
+struct benchmarks {
+	void		(*fn)(void);
+	unsigned	offset;
+	const char	*title;
+};
+
+static int run_benchmarks(void)
+{
+	int ret;
+	struct benchmarks list[] = {
+		{&benchmark_nop, 0, "nop"},
+		/*
+		 * benchmark_pushpop{1,3} will have the optimised
+		 * instruction emulation, whilst benchmark_pushpop{2,4} will
+		 * be the equivalent unoptimised instructions.
+		 */
+		{&benchmark_pushpop1, 0, "stmdb	sp!, {r3-r11,lr}"},
+		{&benchmark_pushpop1, 4, "ldmia	sp!, {r3-r11,pc}"},
+		{&benchmark_pushpop2, 0, "stmdb	sp!, {r0-r8,lr}"},
+		{&benchmark_pushpop2, 4, "ldmia	sp!, {r0-r8,pc}"},
+		{&benchmark_pushpop3, 0, "stmdb	sp!, {r4,lr}"},
+		{&benchmark_pushpop3, 4, "ldmia	sp!, {r4,pc}"},
+		{&benchmark_pushpop4, 0, "stmdb	sp!, {r0,lr}"},
+		{&benchmark_pushpop4, 4, "ldmia	sp!, {r0,pc}"},
+#ifdef CONFIG_THUMB2_KERNEL
+		{&benchmark_pushpop_thumb, 0, "push.n	{r0-r7,lr}"},
+		{&benchmark_pushpop_thumb, 2, "pop.n	{r0-r7,pc}"},
+#endif
+		{0}
+	};
+
+	struct benchmarks *b;
+	for (b = list; b->fn; ++b) {
+		ret = kprobe_benchmark(b->fn, b->offset);
+		if (ret < 0)
+			return ret;
+		pr_info("    %dns for kprobe %s\n", ret, b->title);
+	}
+
+	pr_info("\n");
+	return 0;
+}
+
+#endif /* BENCHMARKING */
+
+
+/*
  * Decoding table self-consistency tests
  */
 
@@ -1526,6 +1680,13 @@ static int __init run_all_tests(void)
 		goto out;
 	}
 
+#if BENCHMARKING
+	pr_info("Benchmarks\n");
+	ret = run_benchmarks();
+	if (ret)
+		goto out;
+#endif
+
 #if __LINUX_ARM_ARCH__ >= 7
 	/* We are able to run all test cases so coverage should be complete */
 	if (coverage_fail) {
-- 
1.7.2.5

^ permalink raw reply related

* [PATCH v12 0/3] add the GPMI controller driver for IMX23/IMX28
From: Huang Shijie @ 2011-09-18 11:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20083.21122.830407.146310@ipc1.ka-ro>

Hi Lothar:

On Fri, Sep 16, 2011 at 9:43 PM, Lothar Wa?mann <LW@karo-electronics.de> wrote:
> Hi,
>
> Huang Shijie writes:
>> Hi,
>> > Hi,
>> >
>> > Huang Shijie writes:
>> >> The patch set is based on Artem's tree:
>> >> ? ?http://git.infradead.org/users/dedekind/l2-mtd-2.6.git
>> >>
>> >> The general-purpose media interface(GPMI) controller is a flexible interface
>> >> to up to several NAND flashs.
>> >>
>> >> The Bose Ray-Choudhury Hocquenghem(BCH) module is a hardware ECC accelerator.
>> >>
>> >> With the help of BCH, the GPMI controller can choose to do the hardware ECC or
>> >> not.
>> >>
>> >> This driver is a _pure_ MTD NAND controller driver now.
>> >>
>> >>
>> >> The driver depends on another GPMI-NAND device patch set, you can find them at :
>> >> ? ?[1] http://marc.info/?l=linux-arm-kernel&m=131416901319573&w=2
>> >> ? ?[2] http://marc.info/?l=linux-arm-kernel&m=131416912319668&w=2
>> >> ? ?[3] http://marc.info/?l=linux-arm-kernel&m=131416891119504&w=2
>> >> ? ?[4] http://marc.info/?l=linux-arm-kernel&m=131416896219539&w=2
>> >>
>> >> Test environment:
>> >> ? ?Using imx23 and imx28 boards for test.
>> >>
>> >> ? ?imx23 :
>> >> ? ?console=ttyAMA0,115200 mtdparts=gpmi-nfc:20m(boot),-(user)
>> >> ? ?Tested by USB boot and NAND boot.
>> >>
>> >> ? ? ? ? ?imx28 :
>> >> ? ?#console=ttyAMA0,115200 root=/dev/mmcblk0p3 rw rootwait
>> >> ? ?Tested by SD card boot mode.
>> >>
>> > How did you perform your tests?
>> >
>> > I tried this driver on our TX28 board with the result that with
>> > concurrent access of the SD card and the NAND flash, I still get the
>> > dreaded DMA timeouts within seconds:
>> > DMA timeout, last DMA :1
>> > GPMI regs:
>> > ? ? ? ? ? ? ?HW_GPMI_CTRL0[000]=21800001
>> > ? ? ? ? ? ?HW_GPMI_COMPARE[010]=00000000
>> > ? ? ? ? ? ?HW_GPMI_ECCCTRL[020]=00000000
>> > ? ? ? ? ? HW_GPMI_ECCCOUNT[030]=00000840
>> > ? ? ? ? ? ?HW_GPMI_PAYLOAD[040]=46578000
>> > ? ? ? ? ?HW_GPMI_AUXILIARY[050]=46578800
>> > ? ? ? ? ? ? ?HW_GPMI_CTRL1[060]=0004000c
>> > ? ? ? ? ? ?HW_GPMI_TIMING0[070]=00010203
>> > ? ? ? ? ? ?HW_GPMI_TIMING1[080]=05000000
>> > ? ? ? ? ? ?HW_GPMI_TIMING2[090]=09020101
>> > ? ? ? ? ? ? ? HW_GPMI_STAT[0b0]=0d000003
>> > ? ? ? ? ? ? ?HW_GPMI_DEBUG[0c0]=01000000
>> > BCH regs:
>> > ? ? ? ? ? ? ? ?HW_BCH_CTRL[000]=00000100
>> > ? ? ? ? ? ? HW_BCH_STATUS0[010]=00000010
>> > ? ? ? ? ? ? ? ?HW_BCH_MODE[020]=00000000
>> > ? ? ? ? ? HW_BCH_ENCODEPTR[030]=00000000
>> > ? ? ? ? ? ? HW_BCH_DATAPTR[040]=00000000
>> > ? ? ? ? ? ? HW_BCH_METAPTR[050]=00000000
>> > ? ? ? ?HW_BCH_LAYOUTSELECT[070]=00000000
>> > ? ? ? HW_BCH_FLASH0LAYOUT0[080]=030a4200
>> > ? ? ? HW_BCH_FLASH0LAYOUT1[090]=08404200
>> > ? ? ? ? ? ? ?HW_BCH_DEBUG0[100]=00000000
>> > DMA regs:
>> > ? ? ? ? ? ? HW_APBHX_CTRL0[000]=30000000
>> > ? ? ? ? ? ? HW_APBHX_CTRL1[010]=ffff0000
>> > ? ? ? ? ? ? HW_APBHX_CTRL2[020]=00000000
>> > ? ? ?HW_APBHX_CHANNEL_CTRL[030]=00000000
>> > ? ? ?HW_APBHX_CHn_CURCMDAR(0)[100]=46e1c000
>> > ? ? ?HW_APBHX_CHn_NXTCMDAR(0)[110]=46e1c04c
>> > ? ? ? ? ? HW_APBHX_CHn_CMD(0)[120]=000001c8
>> > ? ? ? ? ? HW_APBHX_CHn_BAR(0)[130]=00000000
>> > ? ? ? ? ?HW_APBHX_CHn_SEMA(0)[140]=00000000
>> > ? ? ? ?HW_APBHX_CHn_DEBUG1(0)[150]=00a0001e
>> > ? ? ? ?HW_APBHX_CHn_DEBUG2(0)[160]=00000000
>> > ? ? ?HW_APBHX_CHn_CURCMDAR(4)[2c0]=4652c098
>> > ? ? ?HW_APBHX_CHn_NXTCMDAR(4)[2d0]=4652c0e4
>> > ? ? ? ? ? HW_APBHX_CHn_CMD(4)[2e0]=000001c9
>> > ? ? ? ? ? HW_APBHX_CHn_BAR(4)[2f0]=46553241
>> > ? ? ? ? ?HW_APBHX_CHn_SEMA(4)[300]=00010000
>> > ? ? ? ?HW_APBHX_CHn_DEBUG1(4)[310]=27a00015
>> > ? ? ? ?HW_APBHX_CHn_DEBUG2(4)[320]=00000000
>> > BCH Geometry :
>> > GF length ? ? ? ? ? ? ?: 13
>> > ECC Strength ? ? ? ? ? : 8
>> > Page Size in Bytes ? ? : 2112
>> > Metadata Size in Bytes : 10
>> > ECC Chunk Size in Bytes: 512
>> > ECC Chunk Count ? ? ? ?: 4
>> > Payload Size in Bytes ?: 2048
>> > Auxiliary Size in Bytes: 16
>> > Auxiliary Status Offset: 12
>> > Block Mark Byte Offset : 1999
>> > Block Mark Bit Offset ?: 0
>> >
>> > I'm doing a:
>> > dd if=/dev/mtd0> ?/dev/null& ?dd if=/dev/mmcblk0> ?/dev/null
>> > Sometimes the 'dd' accessing the SD card will stall indefinitely.
>> > Also copying data from SD card to flash will produce the error, though
>> > less likely.
>> >
>> it reproduces in my side too.
>> > This looks like the problem arises in the DMA driver when multiple
>> anyway, I will debug it.
>>
>> but i will on vacation next week.
>>
>> I am not sure I can fix it tomorrow.
>>
> I now found, that selecting 'CONFIG_PREEMPT_NONE' in the kernel config
> dramatically decreases the chance of hitting this problem.
>
> I've checked the mxs-dma and gpmi-nand drivers, but could not find
> anything fishy wrt preemption.
thanks for your hard work.

I think you could focus on the mxs-dma driver.
I read code of sd driver.
The only common place which gpmi and sd uses is the dma submit code.

The DMA maybe become unstabe when the gpmi and sd submit the DMA
operation at the same time. :(
I remember you ever submit a patch about the mxs-dma.
Could you try it with your patch?

thanks

Huang Shijie




>
>
> Lothar Wa?mann
> --
> ___________________________________________________________
>
> Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
> Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
> Gesch?ftsf?hrer: Matthias Kaussen
> Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
>
> www.karo-electronics.de | info at karo-electronics.de
> ___________________________________________________________
>

^ permalink raw reply

* [PATCH 3/7] ASoC: edb93xx: convert to use snd_soc_register_card()
From: Alexander Sverdlin @ 2011-09-18 11:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110918073852.GA2625@mwesterb-mobl.ger.corp.intel.com>

Hello!

On Sun, 2011-09-18 at 10:38 +0300, Mika Westerberg wrote:
> On Sat, Sep 17, 2011 at 01:58:20PM +0200, Alexander Sverdlin wrote:
> > 
> > I've checked mainline linux-next without your patches, it's broken too.
> > So your patches are not the root cause for that.
> 
> Ok thanks.
> 
> Still it would be good to find out which causes it to fail. Are you able to
> load all the modules manually? Does it work then?
> 

I've limited access to hardware at the moment, so I'll find out that,
but it will not be so fast )

^ permalink raw reply

* [PATCH 0/4] add fec support for imx6q
From: Shawn Guo @ 2011-09-18 11:54 UTC (permalink / raw)
  To: linux-arm-kernel

This series adds imx6q enet support.  The imx6q enet is a derivative of
imx28 enet controller.  It fixed the frame endian issue found on imx28,
and added 1 Gbps support.

It's based on v3.1-rc6.

Shawn Guo (4):
      net/fec: change phy-reset-gpio request warning to debug message
      net/fec: fix fec1 check in fec_enet_mii_init()
      net/fec: set phy_speed to the optimal frequency 2.5 MHz
      net/fec: add imx6q enet support

 drivers/net/Kconfig |    3 +-
 drivers/net/fec.c   |   65 ++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 52 insertions(+), 16 deletions(-)

^ permalink raw reply

* [PATCH 1/4] net/fec: change phy-reset-gpio request warning to debug message
From: Shawn Guo @ 2011-09-18 11:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316346852-17090-1-git-send-email-shawn.guo@linaro.org>

FEC can work without a phy reset on some platforms, which means not
very platform necessarily have a phy-reset gpio encoded in device tree.
So it makes more sense to have the phy-reset-gpio request failure as
a debug message rather than a warning.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/net/fec.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index e8266cc..6a638e9 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -1422,7 +1422,7 @@ static int __devinit fec_reset_phy(struct platform_device *pdev)
 	phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
 	err = gpio_request_one(phy_reset, GPIOF_OUT_INIT_LOW, "phy-reset");
 	if (err) {
-		pr_warn("FEC: failed to get gpio phy-reset: %d\n", err);
+		pr_debug("FEC: failed to get gpio phy-reset: %d\n", err);
 		return err;
 	}
 	msleep(1);
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 2/4] net/fec: fix fec1 check in fec_enet_mii_init()
From: Shawn Guo @ 2011-09-18 11:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316346852-17090-1-git-send-email-shawn.guo@linaro.org>

In function fec_enet_mii_init(), it uses non-zero pdev->id as part
of the condition to check the second fec instance (fec1).  This works
before the driver supports device tree probe.  But in case of device
tree probe, pdev->id is -1 which is also non-zero, so the logic becomes
broken when device tree probe gets supported.

The patch change the logic to check "pdev->id > 0" as the part of the
condition for identifying fec1.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/net/fec.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 6a638e9..5ef0e34 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -996,7 +996,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
 	 * mdio interface in board design, and need to be configured by
 	 * fec0 mii_bus.
 	 */
-	if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && pdev->id) {
+	if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && pdev->id > 0) {
 		/* fec1 uses fec0 mii_bus */
 		fep->mii_bus = fec0_mii_bus;
 		return 0;
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 3/4] net/fec: set phy_speed to the optimal frequency 2.5 MHz
From: Shawn Guo @ 2011-09-18 11:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316346852-17090-1-git-send-email-shawn.guo@linaro.org>

With the unnecessary 1 bit left-shift on fep->phy_speed during the
calculation, the phy_speed always runs at the half frequency of the
optimal one 2.5 MHz.

The patch removes that 1 bit left-shift to get the optimal phy_speed.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/net/fec.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 5ef0e34..04206e4 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -1007,7 +1007,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
 	/*
 	 * Set MII speed to 2.5 MHz (= clk_get_rate() / 2 * phy_speed)
 	 */
-	fep->phy_speed = DIV_ROUND_UP(clk_get_rate(fep->clk), 5000000) << 1;
+	fep->phy_speed = DIV_ROUND_UP(clk_get_rate(fep->clk), 5000000);
 	writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
 
 	fep->mii_bus = mdiobus_alloc();
-- 
1.7.4.1

^ permalink raw reply related


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