LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 2/5] of/device: make for_each_node* check status properties
From: Grant Likely @ 2010-12-31  7:46 UTC (permalink / raw)
  To: Deepak Saxena; +Cc: devicetree-discuss, linuxppc-dev
In-Reply-To: <20101208192957.GF32473@mentor.com>

On Wed, Dec 08, 2010 at 11:29:57AM -0800, Deepak Saxena wrote:
> Some early device drivers don't actually implement a struct of_platform_driver,
> and instead use the for_each_node* iterators to search for matching device
> nodes. Disabled device nodes will no longer be returned by these iterators.
> 
> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
> Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com>

As already discussed, the generic device tree iterator and search
functions should not be filtering the results as done here.

g.

> ---
>  arch/powerpc/kernel/pci_of_scan.c         |    2 --
>  arch/powerpc/platforms/83xx/usb.c         |    2 +-
>  arch/powerpc/platforms/85xx/mpc85xx_mds.c |   13 ++-----------
>  arch/powerpc/sysdev/fsl_pci.c             |    5 -----
>  arch/powerpc/sysdev/ppc4xx_pci.c          |   15 ---------------
>  drivers/of/base.c                         |   25 +++++++++++++++++++++++--
>  6 files changed, 26 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
> index e751506..fa4f719 100644
> --- a/arch/powerpc/kernel/pci_of_scan.c
> +++ b/arch/powerpc/kernel/pci_of_scan.c
> @@ -310,8 +310,6 @@ static void __devinit __of_scan_bus(struct device_node *node,
>  	/* Scan direct children */
>  	for_each_child_of_node(node, child) {
>  		pr_debug("  * %s\n", child->full_name);
> -		if (!of_device_is_available(child))
> -			continue;
>  		reg = of_get_property(child, "reg", &reglen);
>  		if (reg == NULL || reglen < 20)
>  			continue;
> diff --git a/arch/powerpc/platforms/83xx/usb.c b/arch/powerpc/platforms/83xx/usb.c
> index 3ba4bb7..749c70b 100644
> --- a/arch/powerpc/platforms/83xx/usb.c
> +++ b/arch/powerpc/platforms/83xx/usb.c
> @@ -211,7 +211,7 @@ int mpc837x_usb_cfg(void)
>  	int ret = 0;
>  
>  	np = of_find_compatible_node(NULL, NULL, "fsl-usb2-dr");
> -	if (!np || !of_device_is_available(np))
> +	if (!np)
>  		return -ENODEV;
>  	prop = of_get_property(np, "phy_type", NULL);
>  
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> index aa34cac..5d1a33f 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> @@ -170,10 +170,8 @@ static void __init mpc85xx_publish_qe_devices(void)
>  	struct device_node *np;
>  
>  	np = of_find_compatible_node(NULL, NULL, "fsl,qe");
> -	if (!of_device_is_available(np)) {
> -		of_node_put(np);
> +	if (!np)
>  		return;
> -	}
>  
>  	of_platform_bus_probe(NULL, mpc85xx_qe_ids, NULL);
>  }
> @@ -267,11 +265,6 @@ static void __init mpc85xx_mds_qe_init(void)
>  			return;
>  	}
>  
> -	if (!of_device_is_available(np)) {
> -		of_node_put(np);
> -		return;
> -	}
> -
>  	qe_reset();
>  	of_node_put(np);
>  
> @@ -328,10 +321,8 @@ static void __init mpc85xx_mds_qeic_init(void)
>  	struct device_node *np;
>  
>  	np = of_find_compatible_node(NULL, NULL, "fsl,qe");
> -	if (!of_device_is_available(np)) {
> -		of_node_put(np);
> +	if (!np)
>  		return;
> -	}
>  
>  	np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic");
>  	if (!np) {
> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> index 818f7c6..fba4ec7 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -623,11 +623,6 @@ int __init mpc83xx_add_bridge(struct device_node *dev)
>  
>  	is_mpc83xx_pci = 1;
>  
> -	if (!of_device_is_available(dev)) {
> -		pr_warning("%s: disabled by the firmware.\n",
> -			   dev->full_name);
> -		return -ENODEV;
> -	}
>  	pr_debug("Adding PCI host bridge %s\n", dev->full_name);
>  
>  	/* Fetch host bridge registers address */
> diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c b/arch/powerpc/sysdev/ppc4xx_pci.c
> index 156aa7d..cf2adec 100644
> --- a/arch/powerpc/sysdev/ppc4xx_pci.c
> +++ b/arch/powerpc/sysdev/ppc4xx_pci.c
> @@ -321,13 +321,6 @@ static void __init ppc4xx_probe_pci_bridge(struct device_node *np)
>  	const int *bus_range;
>  	int primary = 0;
>  
> -	/* Check if device is enabled */
> -	if (!of_device_is_available(np)) {
> -		printk(KERN_INFO "%s: Port disabled via device-tree\n",
> -		       np->full_name);
> -		return;
> -	}
> -
>  	/* Fetch config space registers address */
>  	if (of_address_to_resource(np, 0, &rsrc_cfg)) {
>  		printk(KERN_ERR "%s: Can't get PCI config register base !",
> @@ -1890,14 +1883,6 @@ static void __init ppc4xx_probe_pciex_bridge(struct device_node *np)
>  	port = &ppc4xx_pciex_ports[portno];
>  	port->index = portno;
>  
> -	/*
> -	 * Check if device is enabled
> -	 */
> -	if (!of_device_is_available(np)) {
> -		printk(KERN_INFO "PCIE%d: Port disabled via device-tree\n", port->index);
> -		return;
> -	}
> -
>  	port->node = of_node_get(np);
>  	pval = of_get_property(np, "sdr-base", NULL);
>  	if (pval == NULL) {
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 710b53b..5d269a4 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -344,6 +344,8 @@ EXPORT_SYMBOL(of_get_next_child);
>   *
>   *	Returns a node pointer with refcount incremented, use
>   *	of_node_put() on it when done.
> + *
> + *	Does not return nodes marked unavailable by a status property.
>   */
>  struct device_node *of_find_node_by_path(const char *path)
>  {
> @@ -352,6 +354,7 @@ struct device_node *of_find_node_by_path(const char *path)
>  	read_lock(&devtree_lock);
>  	for (; np; np = np->allnext) {
>  		if (np->full_name && (of_node_cmp(np->full_name, path) == 0)
> +			&& of_device_is_available(np)
>  		    && of_node_get(np))
>  			break;
>  	}
> @@ -370,6 +373,8 @@ EXPORT_SYMBOL(of_find_node_by_path);
>   *
>   *	Returns a node pointer with refcount incremented, use
>   *	of_node_put() on it when done.
> + *
> + *	Does not return nodes marked unavailable by a status property.
>   */
>  struct device_node *of_find_node_by_name(struct device_node *from,
>  	const char *name)
> @@ -380,6 +385,7 @@ struct device_node *of_find_node_by_name(struct device_node *from,
>  	np = from ? from->allnext : allnodes;
>  	for (; np; np = np->allnext)
>  		if (np->name && (of_node_cmp(np->name, name) == 0)
> +			&& of_device_is_available(np)
>  		    && of_node_get(np))
>  			break;
>  	of_node_put(from);
> @@ -399,6 +405,8 @@ EXPORT_SYMBOL(of_find_node_by_name);
>   *
>   *	Returns a node pointer with refcount incremented, use
>   *	of_node_put() on it when done.
> + *
> + *	Does not return nodes marked unavailable by a status property.
>   */
>  struct device_node *of_find_node_by_type(struct device_node *from,
>  	const char *type)
> @@ -409,6 +417,7 @@ struct device_node *of_find_node_by_type(struct device_node *from,
>  	np = from ? from->allnext : allnodes;
>  	for (; np; np = np->allnext)
>  		if (np->type && (of_node_cmp(np->type, type) == 0)
> +			&& of_device_is_available(np)
>  		    && of_node_get(np))
>  			break;
>  	of_node_put(from);
> @@ -430,6 +439,8 @@ EXPORT_SYMBOL(of_find_node_by_type);
>   *
>   *	Returns a node pointer with refcount incremented, use
>   *	of_node_put() on it when done.
> + *
> + *	Does not return nodes marked unavailable by a status property.
>   */
>  struct device_node *of_find_compatible_node(struct device_node *from,
>  	const char *type, const char *compatible)
> @@ -442,7 +453,9 @@ struct device_node *of_find_compatible_node(struct device_node *from,
>  		if (type
>  		    && !(np->type && (of_node_cmp(np->type, type) == 0)))
>  			continue;
> -		if (of_device_is_compatible(np, compatible) && of_node_get(np))
> +		if (of_device_is_compatible(np, compatible)
> +			&& of_device_is_available(np)
> +			&& of_node_get(np))
>  			break;
>  	}
>  	of_node_put(from);
> @@ -462,6 +475,8 @@ EXPORT_SYMBOL(of_find_compatible_node);
>   *
>   *	Returns a node pointer with refcount incremented, use
>   *	of_node_put() on it when done.
> + *
> + *	Does not return nodes marked unavailable by a status property.
>   */
>  struct device_node *of_find_node_with_property(struct device_node *from,
>  	const char *prop_name)
> @@ -472,6 +487,8 @@ struct device_node *of_find_node_with_property(struct device_node *from,
>  	read_lock(&devtree_lock);
>  	np = from ? from->allnext : allnodes;
>  	for (; np; np = np->allnext) {
> +		if (!of_device_is_available(np))
> +			continue;
>  		for (pp = np->properties; pp != 0; pp = pp->next) {
>  			if (of_prop_cmp(pp->name, prop_name) == 0) {
>  				of_node_get(np);
> @@ -526,6 +543,8 @@ EXPORT_SYMBOL(of_match_node);
>   *
>   *	Returns a node pointer with refcount incremented, use
>   *	of_node_put() on it when done.
> + *
> + *	Does not return nodes marked unavailable by a status property.
>   */
>  struct device_node *of_find_matching_node(struct device_node *from,
>  					  const struct of_device_id *matches)
> @@ -535,7 +554,9 @@ struct device_node *of_find_matching_node(struct device_node *from,
>  	read_lock(&devtree_lock);
>  	np = from ? from->allnext : allnodes;
>  	for (; np; np = np->allnext) {
> -		if (of_match_node(matches, np) && of_node_get(np))
> +		if (of_match_node(matches, np)
> +			&& of_device_is_available(np)
> +			&& of_node_get(np))
>  			break;
>  	}
>  	of_node_put(from);
> -- 
> 1.6.3.3
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply

* Re: [PATCH 1/5] of/device: Centralize checking of 'status' properties
From: Grant Likely @ 2010-12-31  7:43 UTC (permalink / raw)
  To: Deepak Saxena; +Cc: devicetree-discuss, linuxppc-dev
In-Reply-To: <20101208192913.GB32473@mentor.com>

On Wed, Dec 08, 2010 at 11:29:13AM -0800, Deepak Saxena wrote:
> Don't call any of_platform_driver's probe() function if the device node is
> not accessible (as denoted in the status property).
> 
> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
> Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com>
> ---
>  arch/powerpc/platforms/83xx/suspend.c |    3 ---
>  drivers/mmc/host/sdhci-of-core.c      |    3 ---
>  drivers/net/gianfar.c                 |    2 +-
>  drivers/net/ibm_newemac/core.c        |    2 +-
>  drivers/of/platform.c                 |    7 +++++--
>  5 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
> index 75ae77f..a5a54aa 100644
> --- a/arch/powerpc/platforms/83xx/suspend.c
> +++ b/arch/powerpc/platforms/83xx/suspend.c
> @@ -326,9 +326,6 @@ static int pmc_probe(struct platform_device *ofdev,
>  	struct pmc_type *type = match->data;
>  	int ret = 0;
>  
> -	if (!of_device_is_available(np))
> -		return -ENODEV;
> -
>  	has_deep_sleep = type->has_deep_sleep;
>  	immrbase = get_immrbase();
>  	pmc_dev = ofdev;
> diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
> index c51b711..51218d4 100644
> --- a/drivers/mmc/host/sdhci-of-core.c
> +++ b/drivers/mmc/host/sdhci-of-core.c
> @@ -126,9 +126,6 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev,
>  	int size;
>  	int ret;
>  
> -	if (!of_device_is_available(np))
> -		return -ENODEV;
> -
>  	host = sdhci_alloc_host(&ofdev->dev, sizeof(*of_host));
>  	if (IS_ERR(host))
>  		return -ENOMEM;
> diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> index d1bec62..9918914 100644
> --- a/drivers/net/gianfar.c
> +++ b/drivers/net/gianfar.c
> @@ -620,7 +620,7 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
>  	unsigned int num_tx_qs, num_rx_qs;
>  	u32 *tx_queues, *rx_queues;
>  
> -	if (!np || !of_device_is_available(np))
> +	if (!np)
>  		return -ENODEV;
>  
>  	/* parse the num of tx and rx queues */
> diff --git a/drivers/net/ibm_newemac/core.c b/drivers/net/ibm_newemac/core.c
> index 06bb9b7..290fff2 100644
> --- a/drivers/net/ibm_newemac/core.c
> +++ b/drivers/net/ibm_newemac/core.c
> @@ -2732,7 +2732,7 @@ static int __devinit emac_probe(struct platform_device *ofdev,
>  	 * property here for now, but new flat device trees should set a
>  	 * status property to "disabled" instead.
>  	 */
> -	if (of_get_property(np, "unused", NULL) || !of_device_is_available(np))
> +	if (of_get_property(np, "unused", NULL))
>  		return -ENODEV;
>  
>  	/* Find ourselves in the bootlist if we are there */
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 5b4a07f..14370a0 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -56,7 +56,10 @@ static int platform_driver_probe_shim(struct platform_device *pdev)
>  	 * come up empty.  Return -EINVAL in this case so other drivers get
>  	 * the chance to bind. */
>  	match = of_match_device(pdev->dev.driver->of_match_table, &pdev->dev);
> -	return match ? ofpdrv->probe(pdev, match) : -EINVAL;
> +	if (match && of_device_is_available(pdev->dev.of_node))
> +		return ofpdrv->probe(pdev, match);
> +
> +	return -EINVAL;

Hi Deepak,

This works for drivers still using the deprecated of_platform_driver
structure, but all of those users are being converted into
platform_drivers which do not use this shim.  Fixing it here will not
work after conversion.

I think it would be better to inhibit the devices from getting registered in
the first place instead of short circuiting the probe path.

g.

>  }
>  
>  static void platform_driver_shutdown_shim(struct platform_device *pdev)
> @@ -142,7 +145,7 @@ static int of_platform_device_probe(struct device *dev)
>  	of_dev_get(of_dev);
>  
>  	match = of_match_device(drv->driver.of_match_table, dev);
> -	if (match)
> +	if (match && of_device_is_available(dev->of_node))
>  		error = drv->probe(of_dev, match);
>  	if (error)
>  		of_dev_put(of_dev);
> -- 
> 1.6.3.3
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply

* Re: [PATCH 3/5] of/device: Make of_get_next_child() check status properties
From: Grant Likely @ 2010-12-31  7:39 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Scott Wood, devicetree-discuss, linuxppc-dev, Hollis Blanchard
In-Reply-To: <1292452855.6133.5.camel@concordia>

On Thu, Dec 16, 2010 at 09:40:55AM +1100, Michael Ellerman wrote:
> On Wed, 2010-12-15 at 10:35 -0800, Hollis Blanchard wrote:
> > On Wed, Dec 8, 2010 at 7:09 PM, David Gibson
> > <david@gibson.dropbear.id.au> wrote:
> > > On Thu, Dec 09, 2010 at 12:33:22PM +1100, Michael Ellerman wrote:
> > >> On Wed, 2010-12-08 at 15:01 -0600, Scott Wood wrote:
> > >> > On Wed, 8 Dec 2010 11:29:44 -0800
> > >> > Deepak Saxena <deepak_saxena@mentor.com> wrote:
> > >> >
> > >> > > We only return the next child if the device is available.
> > >> > >
> > >> > > Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
> > >> > > Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com>
> > >> > > ---
> > >> > >  drivers/of/base.c |    4 +++-
> > >> > >  1 files changed, 3 insertions(+), 1 deletions(-)
> > >> > >
> > >> > > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > >> > > index 5d269a4..81b2601 100644
> > >> > > --- a/drivers/of/base.c
> > >> > > +++ b/drivers/of/base.c
> > >> > > @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct device_node *node)
> > >> > >   *
> > >> > >   *       Returns a node pointer with refcount incremented, use
> > >> > >   *       of_node_put() on it when done.
> > >> > > + *
> > >> > > + *       Does not return nodes marked unavailable by a status property.
> > >> > >   */
> > >> > >  struct device_node *of_get_next_child(const struct device_node *node,
> > >> > >   struct device_node *prev)
> > >> > > @@ -330,7 +332,7 @@ struct device_node *of_get_next_child(const struct device_node *node,
> > >> > >   read_lock(&devtree_lock);
> > >> > >   next = prev ? prev->sibling : node->child;
> > >> > >   for (; next; next = next->sibling)
> > >> > > -         if (of_node_get(next))
> > >> > > +         if (of_device_is_available(next) && of_node_get(next))
> > >> > >                   break;
> > >> > >   of_node_put(prev);
> > >> > >   read_unlock(&devtree_lock);
> > >> >
> > >> > This seems like too low-level a place to put this.  Some code may know
> > >> > how to un-disable a device in certain situations, or it may be part of
> > >> > debug code trying to dump the whole device tree, etc.  Looking
> > >> > further[1], I see a raw version of this function, but not other things
> > >> > like of_find_compatible_node.
> > >>
> > >> Yeah I agree. I think we'll eventually end up with __ versions of all or
> > >> lots of them. Not to mention there might be cases you've missed where
> > >> code expects to see unavailable nodes. The right approach is to add
> > >> _new_ routines that don't return unavailable nodes, and convert code
> > >> that you know wants to use them.
> > >
> > > Actually, I don't think we really want these status-skipping
> > > iterators at all.  The device tree iterators should give us the device
> > > tree, as it is.  Those old-style drivers which seach for a node rather
> > > than using the bus probing logic can keep individual checks of the
> > > status property until they're converted to the new scheme.
> > 
> > So the patch should look something like this?
> > 
> > @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct
> > device_node *node)
> >  *
> >  *     Returns a node pointer with refcount incremented, use
> >  *     of_node_put() on it when done.
> > + *
> > + *     Do not use this function.
> >  */
> >  struct device_node *of_get_next_child(const struct device_node *node,
> >        struct device_node *prev)
> 
> Haha. No it should say "this function doesn't lie to you".
> 
> And the patch should say "this patch _doesn't_ subtly change all callers
> of of_get_next_child() without carefully auditing them".

Heh, Yes. The comments made on this patch are totally on-base.  Not
all nodes are devices, and not all callers will want to skip nodes;
regardless of the reason for skipping.  Case in point: the
/proc/device-tree support code.

If a caller needs a version of the function that skips unavailable
nodes, then that behaviour should be explicitly asked for.  In this
case it should be a new function with a new name.  Don't change the
behaviour out from under the existing users.

g.

^ permalink raw reply

* [PATCH 4/5]drivers:staging:rtl8192u:ieee80211:rtl819x_HTProc.c Typo change singal to signal
From: Justin P. Mattock @ 2010-12-31  0:09 UTC (permalink / raw)
  To: trivial; +Cc: gregkh, linuxppc-dev, linux-kernel, Justin P. Mattock
In-Reply-To: <1293754184-1928-3-git-send-email-justinmattock@gmail.com>

The patches below fixes a typo "singal" to "signal". Please let me 
know if anything needs to be changed.

Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

---
 .../staging/rtl8192u/ieee80211/rtl819x_HTProc.c    |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 50f4f59..5ef816f 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -921,7 +921,7 @@ u8 HTFilterMCSRate( struct ieee80211_device* ieee, u8* pSupportMCS, u8* pOperate
 	// TODO: adjust our operational rate set  according to our channel bandwidth, STBC and Antenna number
 
 	// TODO: fill suggested rate adaptive rate index and give firmware info using Tx command packet
-	// we also shall suggested the first start rate set according to our singal strength
+	// we also shall suggested the first start rate set according to our signal strength
 	HT_PickMCSRate(ieee, pOperateMCS);
 
 	// For RTL819X, if pairwisekey = wep/tkip, we support only MCS0~7.
-- 
1.6.5.2.180.gc5b3e

^ permalink raw reply related

* [PATCH 5/5]drivers:staging:rtl8187se:r8180_core.c Typo change singal to signal
From: Justin P. Mattock @ 2010-12-31  0:09 UTC (permalink / raw)
  To: trivial; +Cc: gregkh, linuxppc-dev, linux-kernel, Justin P. Mattock
In-Reply-To: <1293754184-1928-4-git-send-email-justinmattock@gmail.com>

The patches below fixes a typo "singal" to "signal". Please let me 
know if anything needs to be changed.

Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

---
 drivers/staging/rtl8187se/r8180_core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8187se/r8180_core.c b/drivers/staging/rtl8187se/r8180_core.c
index 70ab008..1ccae4b 100644
--- a/drivers/staging/rtl8187se/r8180_core.c
+++ b/drivers/staging/rtl8187se/r8180_core.c
@@ -1329,7 +1329,7 @@ u16 N_DBPSOfRate(u16 DataRate)
 }
 
 /*
- * For Netgear case, they want good-looking singal strength.
+ * For Netgear case, they want good-looking signal strength.
  */
 long NetgearSignalStrengthTranslate(long LastSS, long CurrSS)
 {
@@ -1625,7 +1625,7 @@ void rtl8180_rx(struct net_device *dev)
 				 * of correctness. */
 				PerformUndecoratedSignalSmoothing8185(priv, bCckRate);
 
-				/* For good-looking singal strength. */
+				/* For good-looking signal strength. */
 				SignalStrengthIndex = NetgearSignalStrengthTranslate(
 								priv->LastSignalStrengthInPercent,
 								priv->SignalStrength);
-- 
1.6.5.2.180.gc5b3e

^ permalink raw reply related

* [PATCH 3/5]drivers:staging:tidspbridge:core:tiomap3430.c Typo change singal to signal
From: Justin P. Mattock @ 2010-12-31  0:09 UTC (permalink / raw)
  To: trivial; +Cc: gregkh, linuxppc-dev, linux-kernel, Justin P. Mattock
In-Reply-To: <1293754184-1928-2-git-send-email-justinmattock@gmail.com>

The patches below fixes a typo "singal" to "signal". Please let me 
know if anything needs to be changed.

Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

---
 drivers/staging/tidspbridge/core/tiomap3430.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/tidspbridge/core/tiomap3430.c b/drivers/staging/tidspbridge/core/tiomap3430.c
index 1be081f..7e56ff6 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430.c
@@ -1796,7 +1796,7 @@ static int mem_map_vmalloc(struct bridge_dev_context *dev_context,
 
 /*
  *  ======== wait_for_start ========
- *      Wait for the singal from DSP that it has started, or time out.
+ *      Wait for the signal from DSP that it has started, or time out.
  */
 bool wait_for_start(struct bridge_dev_context *dev_context, u32 dw_sync_addr)
 {
-- 
1.6.5.2.180.gc5b3e

^ permalink raw reply related

* [PATCH 2/5]drivers:staging:rtl8192e:ieee80211:rtl819x_HTProc.c Typo change singal to signal
From: Justin P. Mattock @ 2010-12-31  0:09 UTC (permalink / raw)
  To: trivial; +Cc: gregkh, linuxppc-dev, linux-kernel, Justin P. Mattock
In-Reply-To: <1293754184-1928-1-git-send-email-justinmattock@gmail.com>

The patches below fixes a typo "singal" to "signal". Please let me 
know if anything needs to be changed.

Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

---
 .../staging/rtl8192e/ieee80211/rtl819x_HTProc.c    |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/rtl8192e/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192e/ieee80211/rtl819x_HTProc.c
index b0c9c78..1be2b13 100644
--- a/drivers/staging/rtl8192e/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192e/ieee80211/rtl819x_HTProc.c
@@ -1007,7 +1007,7 @@ u8 HTFilterMCSRate( struct ieee80211_device* ieee, u8* pSupportMCS, u8* pOperate
 	// TODO: adjust our operational rate set  according to our channel bandwidth, STBC and Antenna number
 
 	// TODO: fill suggested rate adaptive rate index and give firmware info using Tx command packet
-	// we also shall suggested the first start rate set according to our singal strength
+	// we also shall suggested the first start rate set according to our signal strength
 	HT_PickMCSRate(ieee, pOperateMCS);
 
 	// For RTL819X, if pairwisekey = wep/tkip, we support only MCS0~7.
-- 
1.6.5.2.180.gc5b3e

^ permalink raw reply related

* [PATCH 1/5]arch:powerpc:platforms:85xx:mpc85xx_mds.c Typo change singal to signal
From: Justin P. Mattock @ 2010-12-31  0:09 UTC (permalink / raw)
  To: trivial; +Cc: gregkh, linuxppc-dev, linux-kernel, Justin P. Mattock

The patches below fixes a typo "singal" to "signal". Please let me 
know if anything needs to be changed.

Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

---
 arch/powerpc/platforms/85xx/mpc85xx_mds.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index aa34cac..747d1ee 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -309,7 +309,7 @@ static void __init mpc85xx_mds_qe_init(void)
 			/* P1021 has pins muxed for QE and other functions. To
 			 * enable QE UEC mode, we need to set bit QE0 for UCC1
 			 * in Eth mode, QE0 and QE3 for UCC5 in Eth mode, QE9
-			 * and QE12 for QE MII management singals in PMUXCR
+			 * and QE12 for QE MII management signals in PMUXCR
 			 * register.
 			 */
 				setbits32(pmuxcr, MPC85xx_PMUXCR_QE0 |
-- 
1.6.5.2.180.gc5b3e

^ permalink raw reply related

* Re: [PATCH 3/3 v2] of/device: Register children with a compatible value in of_platform_bus_probe()
From: Grant Likely @ 2010-12-30 19:37 UTC (permalink / raw)
  To: Lan Chunhe; +Cc: akpm, linuxppc-dev, dougthompson
In-Reply-To: <op.voioo8pqz91x4h@localhost.localdomain>

On Thu, Dec 30, 2010 at 04:04:10PM +0800, Lan Chunhe wrote:
> On Thu, 30 Dec 2010 15:31:57 +0800, Grant Likely
> <grant.likely@secretlab.ca> wrote:
> 
> >On Thu, Dec 30, 2010 at 03:23:11PM +0800, Lan Chunhe wrote:
> >>On Thu, 30 Dec 2010 14:50:45 +0800, Grant Likely
> >><grant.likely@secretlab.ca> wrote:
> >>
> >>>On Fri, Dec 10, 2010 at 06:52:20PM +0800, Lan Chunhe wrote:
> >>>>Currently, of_platform_bus_probe() completely skips nodes which do not
> >>>>explicitly match the 'matches' table passed in.  Or, if the root node
> >>>>matches, then it registers all the children unconditionally.  However,
> >>>>there are situations, such as registering devices from the root node,
> >>>>when it is desirable to register child nodes, but only if they
> >>actually
> >>>>represent devices.  For example, the root node may contain
> >>both a local
> >>>>bus and a PCI device, but it also contains the chosen, aliases
> >>and cpus
> >>>>nodes which don't represent real devices.
> >>>>
> >>>>This patch changes of_platform_bus_probe() to register all
> >>nodes at the
> >>>>top level if they either match the matches table (the current
> >>>>behaviour),
> >>>>or if they have a 'compatible' value (indicating it represents a
> >>>>device).
> >>>>
> >>>>Signed-off-by: Lan Chunhe <b25806@freescale.com>
> >>>
> >>>I believe this is my patch I wrote and pushed out to my tree back in
> >>>October.  Was this cherry-picked out of the test-devicetree branch?
> >>
> >>   Yes, I will add the signed off with you.
> >>
> >>     Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> >>>>Signed-off-by: Lan Chunhe <b25806@freescale.com>
> >>
> >>   At present I think that it is cherry-picked, and I have
> >>tested it which
> >>   is OK.
> >>   Do you have better method?
> >
> >You need to be very careful about stuff like this.  Posting a patch
> >without attribution of the original author and has the original
> >signed-off-by lines stripped off is an absolute no-no.  In literary
> >circles this would be called plagiarism.
> >
> >I don't think you did it intentionally, but proper attribution is a
> >must, particularly because there are copyright issues involved.
> 
>    Sorry, thank you that you remind me.
> 
> >As for this particular patch, I'm not particularly happy with it and I
> >was planning to rewrite it.  It is an inelegant solution and I think
> >the registration of devices from the device tree can be made simpler
> >instead of more complex like in this patch.
> 
>    Now this patch registers devices from the device tree.
>    Then which function of which file do you will modify?

I'm thinking about deprecating the whole recursive device registration
entirely and instead making child device registration always the
responsibility of a device driver.  That would mean each bus node
would bind to a simple device driver that registers the child platform
devices in the .probe() hook.

g.

^ permalink raw reply

* Re: [PATCH] spufs: use simple_write_to_buffer
From: Arnd Bergmann @ 2010-12-30 13:04 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: cbe-oss-dev, linux-kernel, Paul Mackerras, Jeremy Kerr,
	linuxppc-dev
In-Reply-To: <1293257039-2580-1-git-send-email-akinobu.mita@gmail.com>

On Saturday 25 December 2010, Akinobu Mita wrote:
> Simplify several write fileoperations for spufs by using
> simple_write_to_buffer().
> 
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Jeremy Kerr <jk@ozlabs.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: cbe-oss-dev@lists.ozlabs.org

Looks good to me.

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* Re: PowerPC BUG: using smp_processor_id() in preemptible code
From: Benjamin Herrenschmidt @ 2010-12-30 10:45 UTC (permalink / raw)
  To: Hugh Dickins; +Cc: Jeremy Fitzhardinge, linuxppc-dev, Nick Piggin, linux-kernel
In-Reply-To: <alpine.LSU.2.00.1012291413360.22939@sister.anvils>

On Wed, 2010-12-29 at 14:54 -0800, Hugh Dickins wrote:
> With recent 2.6.37-rc, with CONFIG_PREEMPT=y CONFIG_DEBUG_PREEMPT=y
> on the PowerPC G5, I get spammed by BUG warnings each time I swapoff:
> 
> BUG: using smp_processor_id() in preemptible [00000000] code: swapoff/3974
> caller is .hpte_need_flush+0x4c/0x2e8
> Call Trace:
> [c0000001b4a3f830] [c00000000000f3cc] .show_stack+0x6c/0x16c (unreliable)
> [c0000001b4a3f8e0] [c00000000023eda0] .debug_smp_processor_id+0xe4/0x11c
> [c0000001b4a3f970] [c00000000002f2f4] .hpte_need_flush+0x4c/0x2e8
> [c0000001b4a3fa30] [c0000000000e7ef8] .vunmap_pud_range+0x148/0x200
> [c0000001b4a3fb10] [c0000000000e8058] .vunmap_page_range+0xa8/0xd4
> [c0000001b4a3fbb0] [c0000000000e80a4] .free_unmap_vmap_area+0x20/0x38
> [c0000001b4a3fc40] [c0000000000e8138] .remove_vm_area+0x7c/0xb4
> [c0000001b4a3fcd0] [c0000000000e8308] .__vunmap+0x50/0x104
> [c0000001b4a3fd60] [c0000000000ef3fc] .SyS_swapoff+0x59c/0x6a8
> [c0000001b4a3fe30] [c0000000000075a8] syscall_exit+0x0/0x40
> 
> I notice hpte_need_flush() itself acknowledges
>  * Must be called from within some kind of spinlock/non-preempt region...

Yes, we assume that the PTE lock is always held when modifying page
tables...

> Though I didn't actually bisect, I believe this is since Jeremy's
> 64141da587241301ce8638cc945f8b67853156ec "vmalloc: eagerly clear ptes
> on vunmap", which moves a call to vunmap_page_range() from one place
> (which happened to be inside a spinlock) to another (where it's not).
> 
> I guess my warnings would be easily silenced by moving that call to
> vunmap_page_range() down just inside the spinlock below it; but I'm
> dubious that that's the right fix - it looked as if there are other
> paths through vmalloc.c where vunmap_page_range() has been getting
> called without preemption disabled, long before Jeremy's change,
> just paths that I never happen to go down in my limited testing.
> 
> For the moment I'm using the obvious patch below to keep it quiet;
> but I doubt that this is the right patch either.  I'm hoping that
> ye who understand the importance of hpte_need_flush() will be best
> able to judge what to do.  Or might there be other architectures
> expecting to be unpreemptible there?

Well, it looks like our kernel mappings tend to take some nasty
shortcuts with the PTE locking, which I suppose are legit but do break
some of my assumptions there. I need to have a closer look. Thanks for
the report !

Cheers,
Ben.

> Thanks,
> Hugh
> 
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -37,11 +37,13 @@ static void vunmap_pte_range(pmd_t *pmd,
>  {
>  	pte_t *pte;
>  
> +	preempt_disable(); /* Stop __vunmap() triggering smp_processor_id() in preemptible from hpte_need_flush() */
>  	pte = pte_offset_kernel(pmd, addr);
>  	do {
>  		pte_t ptent = ptep_get_and_clear(&init_mm, addr, pte);
>  		WARN_ON(!pte_none(ptent) && !pte_present(ptent));
>  	} while (pte++, addr += PAGE_SIZE, addr != end);
> +	preempt_enable();
>  }
>  
>  static void vunmap_pmd_range(pud_t *pud, unsigned long addr, unsigned long end)

^ permalink raw reply

* Re: [PATCH 3/3 v2] of/device: Register children with a compatible value in of_platform_bus_probe()
From: Lan Chunhe @ 2010-12-30  8:04 UTC (permalink / raw)
  To: Grant Likely; +Cc: akpm, linuxppc-dev, dougthompson
In-Reply-To: <20101230073157.GC843@angua.secretlab.ca>

On Thu, 30 Dec 2010 15:31:57 +0800, Grant Likely  
<grant.likely@secretlab.ca> wrote:

> On Thu, Dec 30, 2010 at 03:23:11PM +0800, Lan Chunhe wrote:
>> On Thu, 30 Dec 2010 14:50:45 +0800, Grant Likely
>> <grant.likely@secretlab.ca> wrote:
>>
>> >On Fri, Dec 10, 2010 at 06:52:20PM +0800, Lan Chunhe wrote:
>> >>Currently, of_platform_bus_probe() completely skips nodes which do not
>> >>explicitly match the 'matches' table passed in.  Or, if the root node
>> >>matches, then it registers all the children unconditionally.  However,
>> >>there are situations, such as registering devices from the root node,
>> >>when it is desirable to register child nodes, but only if they  
>> actually
>> >>represent devices.  For example, the root node may contain both a  
>> local
>> >>bus and a PCI device, but it also contains the chosen, aliases and  
>> cpus
>> >>nodes which don't represent real devices.
>> >>
>> >>This patch changes of_platform_bus_probe() to register all nodes at  
>> the
>> >>top level if they either match the matches table (the current
>> >>behaviour),
>> >>or if they have a 'compatible' value (indicating it represents a
>> >>device).
>> >>
>> >>Signed-off-by: Lan Chunhe <b25806@freescale.com>
>> >
>> >I believe this is my patch I wrote and pushed out to my tree back in
>> >October.  Was this cherry-picked out of the test-devicetree branch?
>>
>>    Yes, I will add the signed off with you.
>>
>>      Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> >>Signed-off-by: Lan Chunhe <b25806@freescale.com>
>>
>>    At present I think that it is cherry-picked, and I have tested it  
>> which
>>    is OK.
>>    Do you have better method?
>
> You need to be very careful about stuff like this.  Posting a patch
> without attribution of the original author and has the original
> signed-off-by lines stripped off is an absolute no-no.  In literary
> circles this would be called plagiarism.
>
> I don't think you did it intentionally, but proper attribution is a
> must, particularly because there are copyright issues involved.

    Sorry, thank you that you remind me.

> As for this particular patch, I'm not particularly happy with it and I
> was planning to rewrite it.  It is an inelegant solution and I think
> the registration of devices from the device tree can be made simpler
> instead of more complex like in this patch.

    Now this patch registers devices from the device tree.
    Then which function of which file do you will modify?

    Thanks.
    -Lan
> g.
>
>>
>>    Thanks.
>>    -Lan
>>
>> >>---
>> >> drivers/of/platform.c |   28 +++++++++++++++++++++++-----
>> >> 1 files changed, 23 insertions(+), 5 deletions(-)
>> >>
>> >>diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> >>index 5b4a07f..02755ab 100644
>> >>--- a/drivers/of/platform.c
>> >>+++ b/drivers/of/platform.c
>> >>@@ -714,6 +714,8 @@ int of_platform_bus_probe(struct device_node  
>> *root,
>> >> 	struct device_node *child;
>> >> 	struct platform_device *dev;
>> >> 	int rc = 0;
>> >>+	const void *compat;
>> >>+	const struct of_device_id *match;
>> >>
>> >> 	if (WARN_ON(!matches || matches == OF_NO_DEEP_PROBE))
>> >> 		return -EINVAL;
>> >>@@ -741,16 +743,32 @@ int of_platform_bus_probe(struct
>> >>device_node *root,
>> >> 		rc = of_platform_bus_create(root, matches, &dev->dev);
>> >> 		goto bail;
>> >> 	}
>> >>+
>> >>+	/*
>> >>+	 * Register each child node if either:
>> >>+	 *  a) it has a 'compatible' value indicating they are a device, or
>> >>+	 *  b) it is specified by the 'matches' table (by name or  
>> device_type)
>> >>+	 * If a node is specified in the matches table, then all its  
>> children
>> >>+	 * also get registered.
>> >>+	 */
>> >> 	for_each_child_of_node(root, child) {
>> >>-		if (!of_match_node(matches, child))
>> >>+		compat = of_get_property(child, "compatible", NULL);
>> >>+		match = of_match_node(matches, child);
>> >>+		if (!compat && !match)
>> >> 			continue;
>> >>
>> >>-		pr_debug("  match: %s\n", child->full_name);
>> >>+		pr_debug("  register device: %s\n", child->full_name);
>> >> 		dev = of_platform_device_create(child, NULL, parent);
>> >>-		if (dev == NULL)
>> >>+		if (!dev) {
>> >> 			rc = -ENOMEM;
>> >>-		else
>> >>-			rc = of_platform_bus_create(child, matches, &dev->dev);
>> >>+			of_node_put(child);
>> >>+			break;
>> >>+		}
>> >>+		if (!match)
>> >>+			continue;
>> >>+
>> >>+		pr_debug("  register children of: %s\n", child->full_name);
>> >>+		rc = of_platform_bus_create(child, matches, &dev->dev);
>> >> 		if (rc) {
>> >> 			of_node_put(child);
>> >> 			break;
>> >>--
>> >>1.5.4.5
>> >>
>> >>
>> >
>>
>>
>

^ permalink raw reply

* Re: [PATCH 3/3 v2] of/device: Register children with a compatible value in of_platform_bus_probe()
From: Grant Likely @ 2010-12-30  7:31 UTC (permalink / raw)
  To: Lan Chunhe; +Cc: akpm, linuxppc-dev, dougthompson
In-Reply-To: <op.voimsxy9z91x4h@localhost.localdomain>

On Thu, Dec 30, 2010 at 03:23:11PM +0800, Lan Chunhe wrote:
> On Thu, 30 Dec 2010 14:50:45 +0800, Grant Likely
> <grant.likely@secretlab.ca> wrote:
> 
> >On Fri, Dec 10, 2010 at 06:52:20PM +0800, Lan Chunhe wrote:
> >>Currently, of_platform_bus_probe() completely skips nodes which do not
> >>explicitly match the 'matches' table passed in.  Or, if the root node
> >>matches, then it registers all the children unconditionally.  However,
> >>there are situations, such as registering devices from the root node,
> >>when it is desirable to register child nodes, but only if they actually
> >>represent devices.  For example, the root node may contain both a local
> >>bus and a PCI device, but it also contains the chosen, aliases and cpus
> >>nodes which don't represent real devices.
> >>
> >>This patch changes of_platform_bus_probe() to register all nodes at the
> >>top level if they either match the matches table (the current
> >>behaviour),
> >>or if they have a 'compatible' value (indicating it represents a
> >>device).
> >>
> >>Signed-off-by: Lan Chunhe <b25806@freescale.com>
> >
> >I believe this is my patch I wrote and pushed out to my tree back in
> >October.  Was this cherry-picked out of the test-devicetree branch?
> 
>    Yes, I will add the signed off with you.
> 
>      Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> >>Signed-off-by: Lan Chunhe <b25806@freescale.com>
> 
>    At present I think that it is cherry-picked, and I have tested it which
>    is OK.
>    Do you have better method?

You need to be very careful about stuff like this.  Posting a patch
without attribution of the original author and has the original
signed-off-by lines stripped off is an absolute no-no.  In literary
circles this would be called plagiarism.

I don't think you did it intentionally, but proper attribution is a
must, particularly because there are copyright issues involved.

As for this particular patch, I'm not particularly happy with it and I
was planning to rewrite it.  It is an inelegant solution and I think
the registration of devices from the device tree can be made simpler
instead of more complex like in this patch.

g.

> 
>    Thanks.
>    -Lan
> 
> >>---
> >> drivers/of/platform.c |   28 +++++++++++++++++++++++-----
> >> 1 files changed, 23 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> >>index 5b4a07f..02755ab 100644
> >>--- a/drivers/of/platform.c
> >>+++ b/drivers/of/platform.c
> >>@@ -714,6 +714,8 @@ int of_platform_bus_probe(struct device_node *root,
> >> 	struct device_node *child;
> >> 	struct platform_device *dev;
> >> 	int rc = 0;
> >>+	const void *compat;
> >>+	const struct of_device_id *match;
> >>
> >> 	if (WARN_ON(!matches || matches == OF_NO_DEEP_PROBE))
> >> 		return -EINVAL;
> >>@@ -741,16 +743,32 @@ int of_platform_bus_probe(struct
> >>device_node *root,
> >> 		rc = of_platform_bus_create(root, matches, &dev->dev);
> >> 		goto bail;
> >> 	}
> >>+
> >>+	/*
> >>+	 * Register each child node if either:
> >>+	 *  a) it has a 'compatible' value indicating they are a device, or
> >>+	 *  b) it is specified by the 'matches' table (by name or device_type)
> >>+	 * If a node is specified in the matches table, then all its children
> >>+	 * also get registered.
> >>+	 */
> >> 	for_each_child_of_node(root, child) {
> >>-		if (!of_match_node(matches, child))
> >>+		compat = of_get_property(child, "compatible", NULL);
> >>+		match = of_match_node(matches, child);
> >>+		if (!compat && !match)
> >> 			continue;
> >>
> >>-		pr_debug("  match: %s\n", child->full_name);
> >>+		pr_debug("  register device: %s\n", child->full_name);
> >> 		dev = of_platform_device_create(child, NULL, parent);
> >>-		if (dev == NULL)
> >>+		if (!dev) {
> >> 			rc = -ENOMEM;
> >>-		else
> >>-			rc = of_platform_bus_create(child, matches, &dev->dev);
> >>+			of_node_put(child);
> >>+			break;
> >>+		}
> >>+		if (!match)
> >>+			continue;
> >>+
> >>+		pr_debug("  register children of: %s\n", child->full_name);
> >>+		rc = of_platform_bus_create(child, matches, &dev->dev);
> >> 		if (rc) {
> >> 			of_node_put(child);
> >> 			break;
> >>--
> >>1.5.4.5
> >>
> >>
> >
> 
> 

^ permalink raw reply

* Re: [PATCH 3/3 v2] of/device: Register children with a compatible value in of_platform_bus_probe()
From: Lan Chunhe @ 2010-12-30  7:23 UTC (permalink / raw)
  To: Grant Likely; +Cc: akpm, linuxppc-dev, dougthompson
In-Reply-To: <20101230065045.GB843@angua.secretlab.ca>

On Thu, 30 Dec 2010 14:50:45 +0800, Grant Likely  
<grant.likely@secretlab.ca> wrote:

> On Fri, Dec 10, 2010 at 06:52:20PM +0800, Lan Chunhe wrote:
>> Currently, of_platform_bus_probe() completely skips nodes which do not
>> explicitly match the 'matches' table passed in.  Or, if the root node
>> matches, then it registers all the children unconditionally.  However,
>> there are situations, such as registering devices from the root node,
>> when it is desirable to register child nodes, but only if they actually
>> represent devices.  For example, the root node may contain both a local
>> bus and a PCI device, but it also contains the chosen, aliases and cpus
>> nodes which don't represent real devices.
>>
>> This patch changes of_platform_bus_probe() to register all nodes at the
>> top level if they either match the matches table (the current  
>> behaviour),
>> or if they have a 'compatible' value (indicating it represents a  
>> device).
>>
>> Signed-off-by: Lan Chunhe <b25806@freescale.com>
>
> I believe this is my patch I wrote and pushed out to my tree back in
> October.  Was this cherry-picked out of the test-devicetree branch?

    Yes, I will add the signed off with you.

      Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> Signed-off-by: Lan Chunhe <b25806@freescale.com>

    At present I think that it is cherry-picked, and I have tested it which
    is OK.
    Do you have better method?

    Thanks.
    -Lan

>> ---
>>  drivers/of/platform.c |   28 +++++++++++++++++++++++-----
>>  1 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> index 5b4a07f..02755ab 100644
>> --- a/drivers/of/platform.c
>> +++ b/drivers/of/platform.c
>> @@ -714,6 +714,8 @@ int of_platform_bus_probe(struct device_node *root,
>>  	struct device_node *child;
>>  	struct platform_device *dev;
>>  	int rc = 0;
>> +	const void *compat;
>> +	const struct of_device_id *match;
>>
>>  	if (WARN_ON(!matches || matches == OF_NO_DEEP_PROBE))
>>  		return -EINVAL;
>> @@ -741,16 +743,32 @@ int of_platform_bus_probe(struct device_node  
>> *root,
>>  		rc = of_platform_bus_create(root, matches, &dev->dev);
>>  		goto bail;
>>  	}
>> +
>> +	/*
>> +	 * Register each child node if either:
>> +	 *  a) it has a 'compatible' value indicating they are a device, or
>> +	 *  b) it is specified by the 'matches' table (by name or device_type)
>> +	 * If a node is specified in the matches table, then all its children
>> +	 * also get registered.
>> +	 */
>>  	for_each_child_of_node(root, child) {
>> -		if (!of_match_node(matches, child))
>> +		compat = of_get_property(child, "compatible", NULL);
>> +		match = of_match_node(matches, child);
>> +		if (!compat && !match)
>>  			continue;
>>
>> -		pr_debug("  match: %s\n", child->full_name);
>> +		pr_debug("  register device: %s\n", child->full_name);
>>  		dev = of_platform_device_create(child, NULL, parent);
>> -		if (dev == NULL)
>> +		if (!dev) {
>>  			rc = -ENOMEM;
>> -		else
>> -			rc = of_platform_bus_create(child, matches, &dev->dev);
>> +			of_node_put(child);
>> +			break;
>> +		}
>> +		if (!match)
>> +			continue;
>> +
>> +		pr_debug("  register children of: %s\n", child->full_name);
>> +		rc = of_platform_bus_create(child, matches, &dev->dev);
>>  		if (rc) {
>>  			of_node_put(child);
>>  			break;
>> --
>> 1.5.4.5
>>
>>
>

^ permalink raw reply

* Re: [PATCH 3/3 v2] of/device: Register children with a compatible value in of_platform_bus_probe()
From: Grant Likely @ 2010-12-30  6:50 UTC (permalink / raw)
  To: Lan Chunhe; +Cc: akpm, linuxppc-dev, dougthompson
In-Reply-To: <1291978340-15933-1-git-send-email-b25806@freescale.com>

On Fri, Dec 10, 2010 at 06:52:20PM +0800, Lan Chunhe wrote:
> Currently, of_platform_bus_probe() completely skips nodes which do not
> explicitly match the 'matches' table passed in.  Or, if the root node
> matches, then it registers all the children unconditionally.  However,
> there are situations, such as registering devices from the root node,
> when it is desirable to register child nodes, but only if they actually
> represent devices.  For example, the root node may contain both a local
> bus and a PCI device, but it also contains the chosen, aliases and cpus
> nodes which don't represent real devices.
> 
> This patch changes of_platform_bus_probe() to register all nodes at the
> top level if they either match the matches table (the current behaviour),
> or if they have a 'compatible' value (indicating it represents a device).
> 
> Signed-off-by: Lan Chunhe <b25806@freescale.com>

I believe this is my patch I wrote and pushed out to my tree back in
October.  Was this cherry-picked out of the test-devicetree branch?

> ---
>  drivers/of/platform.c |   28 +++++++++++++++++++++++-----
>  1 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 5b4a07f..02755ab 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -714,6 +714,8 @@ int of_platform_bus_probe(struct device_node *root,
>  	struct device_node *child;
>  	struct platform_device *dev;
>  	int rc = 0;
> +	const void *compat;
> +	const struct of_device_id *match;
>  
>  	if (WARN_ON(!matches || matches == OF_NO_DEEP_PROBE))
>  		return -EINVAL;
> @@ -741,16 +743,32 @@ int of_platform_bus_probe(struct device_node *root,
>  		rc = of_platform_bus_create(root, matches, &dev->dev);
>  		goto bail;
>  	}
> +
> +	/*
> +	 * Register each child node if either:
> +	 *  a) it has a 'compatible' value indicating they are a device, or
> +	 *  b) it is specified by the 'matches' table (by name or device_type)
> +	 * If a node is specified in the matches table, then all its children
> +	 * also get registered.
> +	 */
>  	for_each_child_of_node(root, child) {
> -		if (!of_match_node(matches, child))
> +		compat = of_get_property(child, "compatible", NULL);
> +		match = of_match_node(matches, child);
> +		if (!compat && !match)
>  			continue;
>  
> -		pr_debug("  match: %s\n", child->full_name);
> +		pr_debug("  register device: %s\n", child->full_name);
>  		dev = of_platform_device_create(child, NULL, parent);
> -		if (dev == NULL)
> +		if (!dev) {
>  			rc = -ENOMEM;
> -		else
> -			rc = of_platform_bus_create(child, matches, &dev->dev);
> +			of_node_put(child);
> +			break;
> +		}
> +		if (!match)
> +			continue;
> +
> +		pr_debug("  register children of: %s\n", child->full_name);
> +		rc = of_platform_bus_create(child, matches, &dev->dev);
>  		if (rc) {
>  			of_node_put(child);
>  			break;
> -- 
> 1.5.4.5
> 
> 

^ permalink raw reply

* Re: [PATCH] eSPI: change the read behavior of the SPIRF
From: Grant Likely @ 2010-12-30  6:04 UTC (permalink / raw)
  To: Mingkai Hu; +Cc: linuxppc-dev, kumar.gala, spi-devel-general
In-Reply-To: <1291195758-12322-1-git-send-email-Mingkai.hu@freescale.com>

On Wed, Dec 01, 2010 at 05:29:18PM +0800, Mingkai Hu wrote:
> The user must read N bytes of SPIRF (1 <= N <= 4) that do not exceed the
> amount of data in the receive FIFO, so read the SPIRF byte by byte when
> the data in receive FIFO is less than 4 bytes.
> 
> On Simics, when read N bytes that exceed the amout of data in receive
> FIFO, we can't read the data out, that is we can't clear the rx FIFO,
> then the CPU will loop on the espi rx interrupt.
> 
> Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>

Applied for -next, thanks.

g.

> ---
>  drivers/spi/spi_fsl_espi.c |   19 ++++++++++++++++---
>  1 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/spi_fsl_espi.c b/drivers/spi/spi_fsl_espi.c
> index e3b4f64..ae78926 100644
> --- a/drivers/spi/spi_fsl_espi.c
> +++ b/drivers/spi/spi_fsl_espi.c
> @@ -507,16 +507,29 @@ void fsl_espi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events)
>  
>  	/* We need handle RX first */
>  	if (events & SPIE_NE) {
> -		u32 rx_data;
> +		u32 rx_data, tmp;
> +		u8 rx_data_8;
>  
>  		/* Spin until RX is done */
>  		while (SPIE_RXCNT(events) < min(4, mspi->len)) {
>  			cpu_relax();
>  			events = mpc8xxx_spi_read_reg(&reg_base->event);
>  		}
> -		mspi->len -= 4;
>  
> -		rx_data = mpc8xxx_spi_read_reg(&reg_base->receive);
> +		if (mspi->len >= 4) {
> +			rx_data = mpc8xxx_spi_read_reg(&reg_base->receive);
> +		} else {
> +			tmp = mspi->len;
> +			rx_data = 0;
> +			while (tmp--) {
> +				rx_data_8 = in_8((u8 *)&reg_base->receive);
> +				rx_data |= (rx_data_8 << (tmp * 8));
> +			}
> +
> +			rx_data <<= (4 - mspi->len) * 8;
> +		}
> +
> +		mspi->len -= 4;
>  
>  		if (mspi->rx)
>  			mspi->get_rx(rx_data, mspi);
> -- 
> 1.7.0.4
> 
> 

^ permalink raw reply

* Re: [PATCH 4/6] mpc512x_dma: try to free descriptors in case of allocation failure
From: Grant Likely @ 2010-12-30  5:36 UTC (permalink / raw)
  To: Ilya Yanok; +Cc: vlad, linuxppc-dev, wd, dzu, Piotr Ziecik
In-Reply-To: <1288137180-3220-5-git-send-email-yanok@emcraft.com>

On Wed, Oct 27, 2010 at 01:52:58AM +0200, Ilya Yanok wrote:
> Currently completed descriptors are processed in the tasklet. This can
> lead to dead lock in case of CONFIG_NET_DMA enabled (new requests are
> submitted from softirq context and dma_memcpy_to_iovec() busy loops until
> the requests is submitted). To prevent this we should process completed
> descriptors from the allocation failure path in prepare_memcpy too.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>

Merged for -next, thanks.

g.

> ---
>  drivers/dma/mpc512x_dma.c |   79 +++++++++++++++++++++++++-------------------
>  1 files changed, 45 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 97b92ec..59c2701 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -328,19 +328,55 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> -/* DMA Tasklet */
> -static void mpc_dma_tasklet(unsigned long data)
> +/* proccess completed descriptors */
> +static void mpc_dma_process_completed(struct mpc_dma *mdma)
>  {
> -	struct mpc_dma *mdma = (void *)data;
>  	dma_cookie_t last_cookie = 0;
>  	struct mpc_dma_chan *mchan;
>  	struct mpc_dma_desc *mdesc;
>  	struct dma_async_tx_descriptor *desc;
>  	unsigned long flags;
>  	LIST_HEAD(list);
> -	uint es;
>  	int i;
>  
> +	for (i = 0; i < mdma->dma.chancnt; i++) {
> +		mchan = &mdma->channels[i];
> +
> +		/* Get all completed descriptors */
> +		spin_lock_irqsave(&mchan->lock, flags);
> +		if (!list_empty(&mchan->completed))
> +			list_splice_tail_init(&mchan->completed, &list);
> +		spin_unlock_irqrestore(&mchan->lock, flags);
> +
> +		if (list_empty(&list))
> +			continue;
> +
> +		/* Execute callbacks and run dependencies */
> +		list_for_each_entry(mdesc, &list, node) {
> +			desc = &mdesc->desc;
> +
> +			if (desc->callback)
> +				desc->callback(desc->callback_param);
> +
> +			last_cookie = desc->cookie;
> +			dma_run_dependencies(desc);
> +		}
> +
> +		/* Free descriptors */
> +		spin_lock_irqsave(&mchan->lock, flags);
> +		list_splice_tail_init(&list, &mchan->free);
> +		mchan->completed_cookie = last_cookie;
> +		spin_unlock_irqrestore(&mchan->lock, flags);
> +	}
> +}
> +
> +/* DMA Tasklet */
> +static void mpc_dma_tasklet(unsigned long data)
> +{
> +	struct mpc_dma *mdma = (void *)data;
> +	unsigned long flags;
> +	uint es;
> +
>  	spin_lock_irqsave(&mdma->error_status_lock, flags);
>  	es = mdma->error_status;
>  	mdma->error_status = 0;
> @@ -379,35 +415,7 @@ static void mpc_dma_tasklet(unsigned long data)
>  			dev_err(mdma->dma.dev, "- Destination Bus Error\n");
>  	}
>  
> -	for (i = 0; i < mdma->dma.chancnt; i++) {
> -		mchan = &mdma->channels[i];
> -
> -		/* Get all completed descriptors */
> -		spin_lock_irqsave(&mchan->lock, flags);
> -		if (!list_empty(&mchan->completed))
> -			list_splice_tail_init(&mchan->completed, &list);
> -		spin_unlock_irqrestore(&mchan->lock, flags);
> -
> -		if (list_empty(&list))
> -			continue;
> -
> -		/* Execute callbacks and run dependencies */
> -		list_for_each_entry(mdesc, &list, node) {
> -			desc = &mdesc->desc;
> -
> -			if (desc->callback)
> -				desc->callback(desc->callback_param);
> -
> -			last_cookie = desc->cookie;
> -			dma_run_dependencies(desc);
> -		}
> -
> -		/* Free descriptors */
> -		spin_lock_irqsave(&mchan->lock, flags);
> -		list_splice_tail_init(&list, &mchan->free);
> -		mchan->completed_cookie = last_cookie;
> -		spin_unlock_irqrestore(&mchan->lock, flags);
> -	}
> +	mpc_dma_process_completed(mdma);
>  }
>  
>  /* Submit descriptor to hardware */
> @@ -587,8 +595,11 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
>  	}
>  	spin_unlock_irqrestore(&mchan->lock, iflags);
>  
> -	if (!mdesc)
> +	if (!mdesc) {
> +		/* try to free completed descriptors */
> +		mpc_dma_process_completed(mdma);
>  		return NULL;
> +	}
>  
>  	mdesc->error = 0;
>  	tcd = mdesc->tcd;
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH 2/6] mpc512x_dma: fix the hanged transfer issue
From: Grant Likely @ 2010-12-30  5:36 UTC (permalink / raw)
  To: Ilya Yanok; +Cc: vlad, linuxppc-dev, wd, dzu, Piotr Ziecik
In-Reply-To: <1288137180-3220-3-git-send-email-yanok@emcraft.com>

On Wed, Oct 27, 2010 at 01:52:56AM +0200, Ilya Yanok wrote:
> Current code clears interrupt active status _after_ submiting new
> transfers. This leaves a possibility of clearing the interrupt for this
> new transfer (if it is triggered fast enough) and thus lose this
> interrupt. We want to clear interrupt active status _before_ new
> transfers is submited and for current channel only.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>

Merged for -next, thanks.

g.

> ---
>  drivers/dma/mpc512x_dma.c |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 1bc04aa..0717527 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -276,6 +276,9 @@ static void mpc_dma_irq_process(struct mpc_dma *mdma, u32 is, u32 es, int off)
>  
>  		spin_lock(&mchan->lock);
>  
> +		out_8(&mdma->regs->dmacint, ch + off);
> +		out_8(&mdma->regs->dmacerr, ch + off);
> +
>  		/* Check error status */
>  		if (es & (1 << ch))
>  			list_for_each_entry(mdesc, &mchan->active, node)
> @@ -309,12 +312,6 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
>  	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmaintl),
>  					in_be32(&mdma->regs->dmaerrl), 0);
>  
> -	/* Ack interrupt on all channels */
> -	out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
> -
>  	/* Schedule tasklet */
>  	tasklet_schedule(&mdma->tasklet);
>  
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH 5/6] MPC8308RDB: add DMA controller device-tree node
From: Grant Likely @ 2010-12-30  5:36 UTC (permalink / raw)
  To: Ilya Yanok; +Cc: vlad, linuxppc-dev, wd, dzu
In-Reply-To: <1288137180-3220-6-git-send-email-yanok@emcraft.com>

On Wed, Oct 27, 2010 at 01:52:59AM +0200, Ilya Yanok wrote:
> MPC8308 has DMA controller compatible with mpc512x_dma driver. This
> patch adds device-tree node to support DMA controller on MPC8308RDB
> board.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>

Merged for -next, thanks.

g.

> ---
>  arch/powerpc/boot/dts/mpc8308rdb.dts |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/mpc8308rdb.dts b/arch/powerpc/boot/dts/mpc8308rdb.dts
> index 1e2b888..a0bd188 100644
> --- a/arch/powerpc/boot/dts/mpc8308rdb.dts
> +++ b/arch/powerpc/boot/dts/mpc8308rdb.dts
> @@ -265,6 +265,14 @@
>  			interrupt-parent = < &ipic >;
>  		};
>  
> +		dma@2c000 {
> +			compatible = "fsl,mpc8308-dma", "fsl,mpc5121-dma";
> +			reg = <0x2c000 0x1800>;
> +			interrupts = <3 0x8
> +					94 0x8>;
> +			interrupt-parent = < &ipic >;
> +		};
> +
>  	};
>  
>  	pci0: pcie@e0009000 {
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH 6/6] mpc8308_p1m: add DMA controller device-tree node
From: Grant Likely @ 2010-12-30  5:35 UTC (permalink / raw)
  To: Ilya Yanok; +Cc: vlad, linuxppc-dev, wd, dzu
In-Reply-To: <1288137180-3220-7-git-send-email-yanok@emcraft.com>

On Wed, Oct 27, 2010 at 01:53:00AM +0200, Ilya Yanok wrote:
> MPC8308 has DMA controller compatible with mpc512x_dma driver. This
> patch adds device-tree node to support DMA controller on MPC8308 P1M
> board.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>

Merged for -next, thanks.

g.

> ---
>  arch/powerpc/boot/dts/mpc8308_p1m.dts |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/mpc8308_p1m.dts b/arch/powerpc/boot/dts/mpc8308_p1m.dts
> index 05a76cc..697b3f6 100644
> --- a/arch/powerpc/boot/dts/mpc8308_p1m.dts
> +++ b/arch/powerpc/boot/dts/mpc8308_p1m.dts
> @@ -297,6 +297,14 @@
>  			interrupt-parent = < &ipic >;
>  		};
>  
> +		dma@2c000 {
> +			compatible = "fsl,mpc8308-dma", "fsl,mpc5121-dma";
> +			reg = <0x2c000 0x1800>;
> +			interrupts = <3 0x8
> +					94 0x8>;
> +			interrupt-parent = < &ipic >;
> +		};
> +
>  	};
>  
>  	pci0: pcie@e0009000 {
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH 3/6] mpc512x_dma: add MPC8308 support
From: Grant Likely @ 2010-12-30  5:35 UTC (permalink / raw)
  To: Ilya Yanok; +Cc: vlad, linuxppc-dev, wd, dzu, Piotr Ziecik
In-Reply-To: <1288137180-3220-4-git-send-email-yanok@emcraft.com>

On Wed, Oct 27, 2010 at 01:52:57AM +0200, Ilya Yanok wrote:
> MPC8308 has pretty much the same DMA controller as MPC5121 and
> this patch adds support for MPC8308 to the mpc512x_dma driver.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>

Merged for -next, thanks.

g.

> ---
>  drivers/dma/Kconfig       |    2 +-
>  drivers/dma/mpc512x_dma.c |   95 +++++++++++++++++++++++++++++++++-----------
>  2 files changed, 72 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 9520cf0..5c5e95b 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -100,7 +100,7 @@ config FSL_DMA
>  
>  config MPC512X_DMA
>  	tristate "Freescale MPC512x built-in DMA engine support"
> -	depends on PPC_MPC512x
> +	depends on PPC_MPC512x || PPC_MPC831x
>  	select DMA_ENGINE
>  	---help---
>  	  Enable support for the Freescale MPC512x built-in DMA engine.
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 0717527..97b92ec 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -1,6 +1,7 @@
>  /*
>   * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
>   * Copyright (C) Semihalf 2009
> + * Copyright (C) Ilya Yanok, Emcraft Systems 2010
>   *
>   * Written by Piotr Ziecik <kosmo@semihalf.com>. Hardware description
>   * (defines, structures and comments) was taken from MPC5121 DMA driver
> @@ -70,6 +71,8 @@
>  #define MPC_DMA_DMAES_SBE	(1 << 1)
>  #define MPC_DMA_DMAES_DBE	(1 << 0)
>  
> +#define MPC_DMA_DMAGPOR_SNOOP_ENABLE	(1 << 6)
> +
>  #define MPC_DMA_TSIZE_1		0x00
>  #define MPC_DMA_TSIZE_2		0x01
>  #define MPC_DMA_TSIZE_4		0x02
> @@ -104,7 +107,10 @@ struct __attribute__ ((__packed__)) mpc_dma_regs {
>  	/* 0x30 */
>  	u32 dmahrsh;		/* DMA hw request status high(ch63~32) */
>  	u32 dmahrsl;		/* DMA hardware request status low(ch31~0) */
> -	u32 dmaihsa;		/* DMA interrupt high select AXE(ch63~32) */
> +	union {
> +		u32 dmaihsa;	/* DMA interrupt high select AXE(ch63~32) */
> +		u32 dmagpor;	/* (General purpose register on MPC8308) */
> +	};
>  	u32 dmailsa;		/* DMA interrupt low select AXE(ch31~0) */
>  	/* 0x40 ~ 0xff */
>  	u32 reserve0[48];	/* Reserved */
> @@ -195,7 +201,9 @@ struct mpc_dma {
>  	struct mpc_dma_regs __iomem	*regs;
>  	struct mpc_dma_tcd __iomem	*tcd;
>  	int				irq;
> +	int				irq2;
>  	uint				error_status;
> +	int				is_mpc8308;
>  
>  	/* Lock for error_status field in this structure */
>  	spinlock_t			error_status_lock;
> @@ -307,8 +315,10 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
>  	spin_unlock(&mdma->error_status_lock);
>  
>  	/* Handle interrupt on each channel */
> -	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmainth),
> +	if (mdma->dma.chancnt > 32) {
> +		mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmainth),
>  					in_be32(&mdma->regs->dmaerrh), 32);
> +	}
>  	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmaintl),
>  					in_be32(&mdma->regs->dmaerrl), 0);
>  
> @@ -562,6 +572,7 @@ static struct dma_async_tx_descriptor *
>  mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
>  					size_t len, unsigned long flags)
>  {
> +	struct mpc_dma *mdma = dma_chan_to_mpc_dma(chan);
>  	struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan);
>  	struct mpc_dma_desc *mdesc = NULL;
>  	struct mpc_dma_tcd *tcd;
> @@ -590,7 +601,8 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
>  		tcd->dsize = MPC_DMA_TSIZE_32;
>  		tcd->soff = 32;
>  		tcd->doff = 32;
> -	} else if (IS_ALIGNED(src | dst | len, 16)) {
> +	} else if (!mdma->is_mpc8308 && IS_ALIGNED(src | dst | len, 16)) {
> +		/* MPC8308 doesn't support 16 byte transfers */
>  		tcd->ssize = MPC_DMA_TSIZE_16;
>  		tcd->dsize = MPC_DMA_TSIZE_16;
>  		tcd->soff = 16;
> @@ -650,6 +662,15 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
>  		return -EINVAL;
>  	}
>  
> +	if (of_device_is_compatible(dn, "fsl,mpc8308-dma")) {
> +		mdma->is_mpc8308 = 1;
> +		mdma->irq2 = irq_of_parse_and_map(dn, 1);
> +		if (mdma->irq2 == NO_IRQ) {
> +			dev_err(dev, "Error mapping IRQ!\n");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	retval = of_address_to_resource(dn, 0, &res);
>  	if (retval) {
>  		dev_err(dev, "Error parsing memory region!\n");
> @@ -680,11 +701,23 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
>  		return -EINVAL;
>  	}
>  
> +	if (mdma->is_mpc8308) {
> +		retval = devm_request_irq(dev, mdma->irq2, &mpc_dma_irq, 0,
> +				DRV_NAME, mdma);
> +		if (retval) {
> +			dev_err(dev, "Error requesting IRQ2!\n");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	spin_lock_init(&mdma->error_status_lock);
>  
>  	dma = &mdma->dma;
>  	dma->dev = dev;
> -	dma->chancnt = MPC_DMA_CHANNELS;
> +	if (!mdma->is_mpc8308)
> +		dma->chancnt = MPC_DMA_CHANNELS;
> +	else
> +		dma->chancnt = 16; /* MPC8308 DMA has only 16 channels */
>  	dma->device_alloc_chan_resources = mpc_dma_alloc_chan_resources;
>  	dma->device_free_chan_resources = mpc_dma_free_chan_resources;
>  	dma->device_issue_pending = mpc_dma_issue_pending;
> @@ -720,26 +753,40 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
>  	 * - Round-robin group arbitration,
>  	 * - Round-robin channel arbitration.
>  	 */
> -	out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_EDCG |
> -				MPC_DMA_DMACR_ERGA | MPC_DMA_DMACR_ERCA);
> -
> -	/* Disable hardware DMA requests */
> -	out_be32(&mdma->regs->dmaerqh, 0);
> -	out_be32(&mdma->regs->dmaerql, 0);
> -
> -	/* Disable error interrupts */
> -	out_be32(&mdma->regs->dmaeeih, 0);
> -	out_be32(&mdma->regs->dmaeeil, 0);
> -
> -	/* Clear interrupts status */
> -	out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
> -
> -	/* Route interrupts to IPIC */
> -	out_be32(&mdma->regs->dmaihsa, 0);
> -	out_be32(&mdma->regs->dmailsa, 0);
> +	if (!mdma->is_mpc8308) {
> +		out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_EDCG |
> +					MPC_DMA_DMACR_ERGA | MPC_DMA_DMACR_ERCA);
> +
> +		/* Disable hardware DMA requests */
> +		out_be32(&mdma->regs->dmaerqh, 0);
> +		out_be32(&mdma->regs->dmaerql, 0);
> +
> +		/* Disable error interrupts */
> +		out_be32(&mdma->regs->dmaeeih, 0);
> +		out_be32(&mdma->regs->dmaeeil, 0);
> +
> +		/* Clear interrupts status */
> +		out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
> +		out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
> +		out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
> +		out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
> +
> +		/* Route interrupts to IPIC */
> +		out_be32(&mdma->regs->dmaihsa, 0);
> +		out_be32(&mdma->regs->dmailsa, 0);
> +	} else {
> +		/* MPC8308 has 16 channels and lacks some registers */
> +		out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_ERCA);
> +
> +		/* enable snooping */
> +		out_be32(&mdma->regs->dmagpor, MPC_DMA_DMAGPOR_SNOOP_ENABLE);
> +		/* Disable error interrupts */
> +		out_be32(&mdma->regs->dmaeeil, 0);
> +
> +		/* Clear interrupts status */
> +		out_be32(&mdma->regs->dmaintl, 0xFFFF);
> +		out_be32(&mdma->regs->dmaerrl, 0xFFFF);
> +	}
>  
>  	/* Register DMA engine */
>  	dev_set_drvdata(dev, mdma);
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH 1/6] mpc512x_dma: scatter/gather fix
From: Grant Likely @ 2010-12-30  5:35 UTC (permalink / raw)
  To: Ilya Yanok; +Cc: vlad, linuxppc-dev, wd, dzu, Piotr Ziecik
In-Reply-To: <1288137180-3220-2-git-send-email-yanok@emcraft.com>

On Wed, Oct 27, 2010 at 01:52:55AM +0200, Ilya Yanok wrote:
> While testing mpc512x-dma driver with dmatest module I've found that
> I can hang the mpc512x-dma issueing request from multiple threads to
> the single channel.
> (insmod dmatest.ko max_channels=1 threads_per_chan=16)
> After investingating this case I've managed to find that this happens
> if and only if we have more than one quequed requests.
> In this case the driver tries to make use of hardware scatter/gather
> functionality. I've found two problems with scatter/gather:
>  1. When TCD is copied form RAM to the TCD register space with memcpy_io()
> e_sg bit eventually gets cleared. This results in only first TCD being
> executed. I've added setting of e_sg bit excplicitly in the TCD registers.
> BTW, what is the correct way to do this? (How can I use setbits with bitfield
> structure?) After that hardware loads consecutive TCDs and we hit the
> second issue.
>  2. Existing code clears int_maj bit in the last TCD so we never get
> an interrupt on transfefr completion.
> 
> With these fixes my tests with many threads of single channel succeed but
> tests that use many channels simultaneously still don't work reliable.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>

Merged for -next, thanks.

g.

> ---
>  drivers/dma/mpc512x_dma.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 4e9cbf3..1bc04aa 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -252,11 +252,13 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
>  		prev = mdesc;
>  	}
>  
> -	prev->tcd->start = 0;
>  	prev->tcd->int_maj = 1;
>  
>  	/* Send first descriptor in chain into hardware */
>  	memcpy_toio(&mdma->tcd[cid], first->tcd, sizeof(struct mpc_dma_tcd));
> +
> +	if (first != prev)
> +		mdma->tcd[cid].e_sg = 1;
>  	out_8(&mdma->regs->dmassrt, cid);
>  }
>  
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: PowerPC BUG: using smp_processor_id() in preemptible code
From: Jeremy Fitzhardinge @ 2010-12-30  1:26 UTC (permalink / raw)
  To: Hugh Dickins; +Cc: Andrew Morton, linuxppc-dev, Nick Piggin, linux-kernel
In-Reply-To: <alpine.LSU.2.00.1012291413360.22939@sister.anvils>

On 12/30/2010 09:54 AM, Hugh Dickins wrote:
> With recent 2.6.37-rc, with CONFIG_PREEMPT=y CONFIG_DEBUG_PREEMPT=y
> on the PowerPC G5, I get spammed by BUG warnings each time I swapoff:
>
> BUG: using smp_processor_id() in preemptible [00000000] code: swapoff/3974
> caller is .hpte_need_flush+0x4c/0x2e8
> Call Trace:
> [c0000001b4a3f830] [c00000000000f3cc] .show_stack+0x6c/0x16c (unreliable)
> [c0000001b4a3f8e0] [c00000000023eda0] .debug_smp_processor_id+0xe4/0x11c
> [c0000001b4a3f970] [c00000000002f2f4] .hpte_need_flush+0x4c/0x2e8
> [c0000001b4a3fa30] [c0000000000e7ef8] .vunmap_pud_range+0x148/0x200
> [c0000001b4a3fb10] [c0000000000e8058] .vunmap_page_range+0xa8/0xd4
> [c0000001b4a3fbb0] [c0000000000e80a4] .free_unmap_vmap_area+0x20/0x38
> [c0000001b4a3fc40] [c0000000000e8138] .remove_vm_area+0x7c/0xb4
> [c0000001b4a3fcd0] [c0000000000e8308] .__vunmap+0x50/0x104
> [c0000001b4a3fd60] [c0000000000ef3fc] .SyS_swapoff+0x59c/0x6a8
> [c0000001b4a3fe30] [c0000000000075a8] syscall_exit+0x0/0x40
>
> I notice hpte_need_flush() itself acknowledges
>  * Must be called from within some kind of spinlock/non-preempt region...
>
> Though I didn't actually bisect, I believe this is since Jeremy's
> 64141da587241301ce8638cc945f8b67853156ec "vmalloc: eagerly clear ptes
> on vunmap", which moves a call to vunmap_page_range() from one place
> (which happened to be inside a spinlock) to another (where it's not).

Yes.  I was  bit worried by the interaction between vb->lock and the pte
locks, and thought it safer to keep it outside.  Though I suppose kernel
mappings don't have pte locks, so that's a non-issue.

> I guess my warnings would be easily silenced by moving that call to
> vunmap_page_range() down just inside the spinlock below it; but I'm
> dubious that that's the right fix - it looked as if there are other
> paths through vmalloc.c where vunmap_page_range() has been getting
> called without preemption disabled, long before Jeremy's change,
> just paths that I never happen to go down in my limited testing.
>
> For the moment I'm using the obvious patch below to keep it quiet;
> but I doubt that this is the right patch either.  I'm hoping that
> ye who understand the importance of hpte_need_flush() will be best
> able to judge what to do.  Or might there be other architectures
> expecting to be unpreemptible there?

Since kernel mappings don't have a formal pte lock to rely on for
synchronization, each subsystem defines its own ad-hoc one.  In this
case that's probably vb->lock anyway, so the fix is to move
vunmap_page_range() back into its protection.

> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -37,11 +37,13 @@ static void vunmap_pte_range(pmd_t *pmd,
>  {
>  	pte_t *pte;
>  
> +	preempt_disable(); /* Stop __vunmap() triggering smp_processor_id() in preemptible from hpte_need_flush() */
>  	pte = pte_offset_kernel(pmd, addr);
>  	do {
>  		pte_t ptent = ptep_get_and_clear(&init_mm, addr, pte);
>  		WARN_ON(!pte_none(ptent) && !pte_present(ptent));
>  	} while (pte++, addr += PAGE_SIZE, addr != end);
> +	preempt_enable();
>  }
>  
>  static void vunmap_pmd_range(pud_t *pud, unsigned long addr, unsigned long end)

That looks OK, but it interferes with my plans to use
apply_to_page_range(_batch) to replace all this code.

    J

^ permalink raw reply

* Re: [PATCH 7/7] fdt.c: Refactor unflatten_device_tree and add fdt_unflatten_tree
From: Grant Likely @ 2010-12-30  0:43 UTC (permalink / raw)
  To: Stephen Neuendorffer
  Cc: dirk.brandewie, microblaze-uclinux, devicetree-discuss,
	linuxppc-dev
In-Reply-To: <8aead437-fb77-4e72-8b66-2b1626f6316d@SG2EHSMHS008.ehs.local>

On Thu, Nov 18, 2010 at 03:55:02PM -0800, Stephen Neuendorffer wrote:
> unflatten_device_tree has two dependencies on things that happen
> during boot time.  Firstly, it references the initial device tree
> directly. Secondly, it allocates memory using the early boot
> allocator.  This patch factors out these dependencies and uses
> the new __unflatten_device_tree function to implement a driver-visible
> fdt_unflatten_tree function, which can be used to unflatten a
> blob after boot time.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

Merged, thanks.  One comment below, please fix with a followup patch.

g.

> 
> --
> 
> V2: remove extra __va() call
> 	 make dt_alloc functions return void *.  This doesn't fix the general
>     strangeness in this code that constantly casts back and forth between
>     unsigned long and __be32 *
> ---
>  drivers/of/fdt.c       |  149 +++++++++++++++++++++++++++++++----------------
>  include/linux/of_fdt.h |    2 +
>  2 files changed, 100 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 236a8c9..e29dbe2 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -11,10 +11,12 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/initrd.h>
> +#include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_fdt.h>
>  #include <linux/string.h>
>  #include <linux/errno.h>
> +#include <linux/slab.h>
>  
>  #ifdef CONFIG_PPC
>  #include <asm/machdep.h>
> @@ -312,6 +314,94 @@ unsigned long unflatten_dt_node(struct boot_param_header *blob,
>  	return mem;
>  }
>  
> +/**
> + * __unflatten_device_tree - create tree of device_nodes from flat blob
> + *
> + * unflattens a device-tree, creating the
> + * tree of struct device_node. It also fills the "name" and "type"
> + * pointers of the nodes so the normal device-tree walking functions
> + * can be used.
> + * @blob: The blob to expand
> + * @mynodes: The device_node tree created by the call
> + * @dt_alloc: An allocator that provides a virtual address to memory
> + * for the resulting tree
> + */
> +void __unflatten_device_tree(struct boot_param_header *blob,
> +			     struct device_node **mynodes,
> +			     void * (*dt_alloc)(u64 size, u64 align))
> +{
> +	unsigned long start, mem, size;
> +	struct device_node **allnextp = mynodes;
> +
> +	pr_debug(" -> unflatten_device_tree()\n");
> +
> +	if (!blob) {
> +		pr_debug("No device tree pointer\n");
> +		return;
> +	}
> +
> +	pr_debug("Unflattening device tree:\n");
> +	pr_debug("magic: %08x\n", be32_to_cpu(blob->magic));
> +	pr_debug("size: %08x\n", be32_to_cpu(blob->totalsize));
> +	pr_debug("version: %08x\n", be32_to_cpu(blob->version));
> +
> +	if (be32_to_cpu(blob->magic) != OF_DT_HEADER) {
> +		pr_err("Invalid device tree blob header\n");
> +		return;
> +	}
> +
> +	/* First pass, scan for size */
> +	start = ((unsigned long)blob) +
> +		be32_to_cpu(blob->off_dt_struct);
> +	size = unflatten_dt_node(blob, 0, &start, NULL, NULL, 0);
> +	size = (size | 3) + 1;
> +
> +	pr_debug("  size is %lx, allocating...\n", size);
> +
> +	/* Allocate memory for the expanded device tree */
> +	mem = (unsigned long)
> +		dt_alloc(size + 4, __alignof__(struct device_node));
> +
> +	((__be32 *)mem)[size / 4] = cpu_to_be32(0xdeadbeef);
> +
> +	pr_debug("  unflattening %lx...\n", mem);
> +
> +	/* Second pass, do actual unflattening */
> +	start = ((unsigned long)blob) +
> +		be32_to_cpu(blob->off_dt_struct);
> +	unflatten_dt_node(blob, mem, &start, NULL, &allnextp, 0);
> +	if (be32_to_cpup((__be32 *)start) != OF_DT_END)
> +		pr_warning("Weird tag at end of tree: %08x\n", *((u32 *)start));
> +	if (be32_to_cpu(((__be32 *)mem)[size / 4]) != 0xdeadbeef)
> +		pr_warning("End of tree marker overwritten: %08x\n",
> +			   be32_to_cpu(((__be32 *)mem)[size / 4]));
> +	*allnextp = NULL;
> +
> +	pr_debug(" <- unflatten_device_tree()\n");
> +}
> +
> +static void *kernel_tree_alloc(u64 size, u64 align)
> +{
> +	return kzalloc(size, GFP_KERNEL);
> +}
> +
> +/**
> + * of_fdt_unflatten_tree - create tree of device_nodes from flat blob
> + *
> + * unflattens the device-tree passed by the firmware, creating the
> + * tree of struct device_node. It also fills the "name" and "type"
> + * pointers of the nodes so the normal device-tree walking functions
> + * can be used.
> + */
> +void of_fdt_unflatten_tree(unsigned long *blob,
> +			struct device_node **mynodes)
> +{
> +	struct boot_param_header *device_tree =
> +		(struct boot_param_header *)blob;
> +	__unflatten_device_tree(device_tree, mynodes, &kernel_tree_alloc);
> +}
> +EXPORT_SYMBOL_GPL(of_fdt_unflatten_tree);

The blob parameter to of_fdt_unflatten_tree should either be void* or
boot_param_header*.  What's the reason for unsigned long *?  Also,
blob can be passed directly into the __unflatten_device_tree() call.
No reason for the device_tree = blob assignment.  In fact, you could
probably eliminate this function entirely and rename
__unflatten_device_tree to of_fdt_unflatten_tree.

> +
>  /* Everything below here references initial_boot_params directly. */
>  int __initdata dt_root_addr_cells;
>  int __initdata dt_root_size_cells;
> @@ -569,6 +659,12 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>  	return 1;
>  }
>  
> +static void *__init early_device_tree_alloc(u64 size, u64 align)
> +{
> +	unsigned long mem = early_init_dt_alloc_memory_arch(size, align);
> +	return __va(mem);
> +}
> +
>  /**
>   * unflatten_device_tree - create tree of device_nodes from flat blob
>   *
> @@ -579,62 +675,13 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>   */
>  void __init unflatten_device_tree(void)
>  {
> -	unsigned long start, mem, size;
> -	struct device_node **allnextp = &allnodes;
> -
> -	pr_debug(" -> unflatten_device_tree()\n");
> -
> -	if (!initial_boot_params) {
> -		pr_debug("No device tree pointer\n");
> -		return;
> -	}
> -
> -	pr_debug("Unflattening device tree:\n");
> -	pr_debug("magic: %08x\n", be32_to_cpu(initial_boot_params->magic));
> -	pr_debug("size: %08x\n", be32_to_cpu(initial_boot_params->totalsize));
> -	pr_debug("version: %08x\n", be32_to_cpu(initial_boot_params->version));
> -
> -	if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
> -		pr_err("Invalid device tree blob header\n");
> -		return;
> -	}
> -
> -	/* First pass, scan for size */
> -	start = ((unsigned long)initial_boot_params) +
> -		be32_to_cpu(initial_boot_params->off_dt_struct);
> -	size = unflatten_dt_node(initial_boot_params, 0, &start,
> -				 NULL, NULL, 0);
> -	size = (size | 3) + 1;
> -
> -	pr_debug("  size is %lx, allocating...\n", size);
> -
> -	/* Allocate memory for the expanded device tree */
> -	mem = early_init_dt_alloc_memory_arch(size + 4,
> -			__alignof__(struct device_node));
> -	mem = (unsigned long) __va(mem);
> -
> -	((__be32 *)mem)[size / 4] = cpu_to_be32(0xdeadbeef);
> -
> -	pr_debug("  unflattening %lx...\n", mem);
> -
> -	/* Second pass, do actual unflattening */
> -	start = ((unsigned long)initial_boot_params) +
> -		be32_to_cpu(initial_boot_params->off_dt_struct);
> -	unflatten_dt_node(initial_boot_params, mem, &start,
> -			  NULL, &allnextp, 0);
> -	if (be32_to_cpup((__be32 *)start) != OF_DT_END)
> -		pr_warning("Weird tag at end of tree: %08x\n", *((u32 *)start));
> -	if (be32_to_cpu(((__be32 *)mem)[size / 4]) != 0xdeadbeef)
> -		pr_warning("End of tree marker overwritten: %08x\n",
> -			   be32_to_cpu(((__be32 *)mem)[size / 4]));
> -	*allnextp = NULL;
> +	__unflatten_device_tree(initial_boot_params, &allnodes,
> +				early_device_tree_alloc);
>  
>  	/* Get pointer to OF "/chosen" node for use everywhere */
>  	of_chosen = of_find_node_by_path("/chosen");
>  	if (of_chosen == NULL)
>  		of_chosen = of_find_node_by_path("/chosen@0");
> -
> -	pr_debug(" <- unflatten_device_tree()\n");
>  }
>  
>  #endif /* CONFIG_EARLY_FLATTREE */
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 70c5b73..9ce5dfd 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -68,6 +68,8 @@ extern void *of_fdt_get_property(struct boot_param_header *blob,
>  extern int of_fdt_is_compatible(struct boot_param_header *blob,
>  				unsigned long node,
>  				const char *compat);
> +extern void of_fdt_unflatten_tree(unsigned long *blob,
> +			       struct device_node **mynodes);
>  
>  /* TBD: Temporary export of fdt globals - remove when code fully merged */
>  extern int __initdata dt_root_addr_cells;
> -- 
> 1.5.6.6
> 
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> 

^ permalink raw reply

* Re: [PATCH 6/7] fdt.c: Reorder unflatten_dt_node
From: Grant Likely @ 2010-12-30  0:36 UTC (permalink / raw)
  To: Stephen Neuendorffer
  Cc: dirk.brandewie, microblaze-uclinux, devicetree-discuss,
	linuxppc-dev
In-Reply-To: <972516c0-f170-469f-937f-bfa6be8dd04a@SG2EHSMHS004.ehs.local>

On Thu, Nov 18, 2010 at 03:55:01PM -0800, Stephen Neuendorffer wrote:
> Move unflatten_dt_node to be grouped with non-__init functions.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

Merged, thanks.

g.

> ---
>  drivers/of/fdt.c |  218 +++++++++++++++++++++++++++---------------------------
>  1 files changed, 109 insertions(+), 109 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index a07fd1a..236a8c9 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -97,115 +97,6 @@ int of_fdt_is_compatible(struct boot_param_header *blob,
>  	return 0;
>  }
>  
> -/* Everything below here references initial_boot_params directly. */
> -int __initdata dt_root_addr_cells;
> -int __initdata dt_root_size_cells;
> -
> -struct boot_param_header *initial_boot_params;
> -
> -#ifdef CONFIG_EARLY_FLATTREE
> -
> -/**
> - * of_scan_flat_dt - scan flattened tree blob and call callback on each.
> - * @it: callback function
> - * @data: context data pointer
> - *
> - * This function is used to scan the flattened device-tree, it is
> - * used to extract the memory information at boot before we can
> - * unflatten the tree
> - */
> -int __init of_scan_flat_dt(int (*it)(unsigned long node,
> -				     const char *uname, int depth,
> -				     void *data),
> -			   void *data)
> -{
> -	unsigned long p = ((unsigned long)initial_boot_params) +
> -		be32_to_cpu(initial_boot_params->off_dt_struct);
> -	int rc = 0;
> -	int depth = -1;
> -
> -	do {
> -		u32 tag = be32_to_cpup((__be32 *)p);
> -		char *pathp;
> -
> -		p += 4;
> -		if (tag == OF_DT_END_NODE) {
> -			depth--;
> -			continue;
> -		}
> -		if (tag == OF_DT_NOP)
> -			continue;
> -		if (tag == OF_DT_END)
> -			break;
> -		if (tag == OF_DT_PROP) {
> -			u32 sz = be32_to_cpup((__be32 *)p);
> -			p += 8;
> -			if (be32_to_cpu(initial_boot_params->version) < 0x10)
> -				p = ALIGN(p, sz >= 8 ? 8 : 4);
> -			p += sz;
> -			p = ALIGN(p, 4);
> -			continue;
> -		}
> -		if (tag != OF_DT_BEGIN_NODE) {
> -			pr_err("Invalid tag %x in flat device tree!\n", tag);
> -			return -EINVAL;
> -		}
> -		depth++;
> -		pathp = (char *)p;
> -		p = ALIGN(p + strlen(pathp) + 1, 4);
> -		if ((*pathp) == '/') {
> -			char *lp, *np;
> -			for (lp = NULL, np = pathp; *np; np++)
> -				if ((*np) == '/')
> -					lp = np+1;
> -			if (lp != NULL)
> -				pathp = lp;
> -		}
> -		rc = it(p, pathp, depth, data);
> -		if (rc != 0)
> -			break;
> -	} while (1);
> -
> -	return rc;
> -}
> -
> -/**
> - * of_get_flat_dt_root - find the root node in the flat blob
> - */
> -unsigned long __init of_get_flat_dt_root(void)
> -{
> -	unsigned long p = ((unsigned long)initial_boot_params) +
> -		be32_to_cpu(initial_boot_params->off_dt_struct);
> -
> -	while (be32_to_cpup((__be32 *)p) == OF_DT_NOP)
> -		p += 4;
> -	BUG_ON(be32_to_cpup((__be32 *)p) != OF_DT_BEGIN_NODE);
> -	p += 4;
> -	return ALIGN(p + strlen((char *)p) + 1, 4);
> -}
> -
> -/**
> - * of_get_flat_dt_prop - Given a node in the flat blob, return the property ptr
> - *
> - * This function can be used within scan_flattened_dt callback to get
> - * access to properties
> - */
> -void *__init of_get_flat_dt_prop(unsigned long node, const char *name,
> -				 unsigned long *size)
> -{
> -	return of_fdt_get_property(initial_boot_params, node, name, size);
> -}
> -
> -/**
> - * of_flat_dt_is_compatible - Return true if given node has compat in compatible list
> - * @node: node to test
> - * @compat: compatible string to compare with compatible list.
> - */
> -int __init of_flat_dt_is_compatible(unsigned long node, const char *compat)
> -{
> -	return of_fdt_is_compatible(initial_boot_params, node, compat);
> -}
> -
>  static void *unflatten_dt_alloc(unsigned long *mem, unsigned long size,
>  				       unsigned long align)
>  {
> @@ -421,6 +312,115 @@ unsigned long unflatten_dt_node(struct boot_param_header *blob,
>  	return mem;
>  }
>  
> +/* Everything below here references initial_boot_params directly. */
> +int __initdata dt_root_addr_cells;
> +int __initdata dt_root_size_cells;
> +
> +struct boot_param_header *initial_boot_params;
> +
> +#ifdef CONFIG_EARLY_FLATTREE
> +
> +/**
> + * of_scan_flat_dt - scan flattened tree blob and call callback on each.
> + * @it: callback function
> + * @data: context data pointer
> + *
> + * This function is used to scan the flattened device-tree, it is
> + * used to extract the memory information at boot before we can
> + * unflatten the tree
> + */
> +int __init of_scan_flat_dt(int (*it)(unsigned long node,
> +				     const char *uname, int depth,
> +				     void *data),
> +			   void *data)
> +{
> +	unsigned long p = ((unsigned long)initial_boot_params) +
> +		be32_to_cpu(initial_boot_params->off_dt_struct);
> +	int rc = 0;
> +	int depth = -1;
> +
> +	do {
> +		u32 tag = be32_to_cpup((__be32 *)p);
> +		char *pathp;
> +
> +		p += 4;
> +		if (tag == OF_DT_END_NODE) {
> +			depth--;
> +			continue;
> +		}
> +		if (tag == OF_DT_NOP)
> +			continue;
> +		if (tag == OF_DT_END)
> +			break;
> +		if (tag == OF_DT_PROP) {
> +			u32 sz = be32_to_cpup((__be32 *)p);
> +			p += 8;
> +			if (be32_to_cpu(initial_boot_params->version) < 0x10)
> +				p = ALIGN(p, sz >= 8 ? 8 : 4);
> +			p += sz;
> +			p = ALIGN(p, 4);
> +			continue;
> +		}
> +		if (tag != OF_DT_BEGIN_NODE) {
> +			pr_err("Invalid tag %x in flat device tree!\n", tag);
> +			return -EINVAL;
> +		}
> +		depth++;
> +		pathp = (char *)p;
> +		p = ALIGN(p + strlen(pathp) + 1, 4);
> +		if ((*pathp) == '/') {
> +			char *lp, *np;
> +			for (lp = NULL, np = pathp; *np; np++)
> +				if ((*np) == '/')
> +					lp = np+1;
> +			if (lp != NULL)
> +				pathp = lp;
> +		}
> +		rc = it(p, pathp, depth, data);
> +		if (rc != 0)
> +			break;
> +	} while (1);
> +
> +	return rc;
> +}
> +
> +/**
> + * of_get_flat_dt_root - find the root node in the flat blob
> + */
> +unsigned long __init of_get_flat_dt_root(void)
> +{
> +	unsigned long p = ((unsigned long)initial_boot_params) +
> +		be32_to_cpu(initial_boot_params->off_dt_struct);
> +
> +	while (be32_to_cpup((__be32 *)p) == OF_DT_NOP)
> +		p += 4;
> +	BUG_ON(be32_to_cpup((__be32 *)p) != OF_DT_BEGIN_NODE);
> +	p += 4;
> +	return ALIGN(p + strlen((char *)p) + 1, 4);
> +}
> +
> +/**
> + * of_get_flat_dt_prop - Given a node in the flat blob, return the property ptr
> + *
> + * This function can be used within scan_flattened_dt callback to get
> + * access to properties
> + */
> +void *__init of_get_flat_dt_prop(unsigned long node, const char *name,
> +				 unsigned long *size)
> +{
> +	return of_fdt_get_property(initial_boot_params, node, name, size);
> +}
> +
> +/**
> + * of_flat_dt_is_compatible - Return true if given node has compat in compatible list
> + * @node: node to test
> + * @compat: compatible string to compare with compatible list.
> + */
> +int __init of_flat_dt_is_compatible(unsigned long node, const char *compat)
> +{
> +	return of_fdt_is_compatible(initial_boot_params, node, compat);
> +}
> +
>  #ifdef CONFIG_BLK_DEV_INITRD
>  /**
>   * early_init_dt_check_for_initrd - Decode initrd location from flat tree
> -- 
> 1.5.6.6
> 
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> 

^ 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