| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 23 Feb 2011 12:45:10 -0700
From: Grant Likely <grant.likely@...retlab.ca>
To: Andres Salomon <dilinger@...ued.net>
Cc: Daniel Drake <dsd@...top.org>,
David Woodhouse <dwmw2@...radead.org>, cbou@...l.ru,
linux-kernel@...r.kernel.org, x86@...nel.org, tglx@...utronix.de,
mingo@...hat.com, hpa@...or.com, dmitry.torokhov@...il.com,
devicetree-discuss@...ts.ozlabs.org,
Mitch Bradley <wmb@...top.org>,
"David S. Miller" <davem@...emloft.net>, sparclinux@...r.kernel.org
Subject: Re: [PATCH] of/pdt: don't bother parsing pkg2path results, return
as-is
On Fri, Feb 18, 2011 at 07:12:25PM -0800, Andres Salomon wrote:
> On Fri, 18 Feb 2011 19:06:59 -0800
> Andres Salomon <dilinger@...ued.net> wrote:
>
> > On Fri, 18 Feb 2011 23:42:57 +0000
> > Daniel Drake <dsd@...top.org> wrote:
> >
> > > On 16 February 2011 22:44, David Woodhouse <dwmw2@...radead.org>
> > > wrote:
> > > > On Wed, 2011-02-16 at 22:28 +0000, Daniel Drake wrote:
> > > >>
> > > >> +static int __init add_common_platform_devices(void)
> > > >> +{
> > > >> + struct platform_device *pdev;
> > > >> +
> > > >> + pdev = platform_device_register_simple("olpc-battery",
> > > >> -1, NULL, 0);
> > > >> + if (IS_ERR(pdev))
> > > >> + return PTR_ERR(pdev);
> > > >> +
> > > >> + return 0;
> > > >> +}
> > > >> +
> > > >
> > > > Still kind of sucks that you have to do this, and can't bind to
> > > > something in the device-tree.
> > >
> > > OK, feel free to put this patch on hold for now. I started looking
> > > at the device tree approach today. It looks doable but first we
> > > have to fix a DT bug/inconsistency that is preventing us from
> > > correctly binding to the tree's devices.
> > >
> > > Daniel
> >
> >
> > Mea culpa. The patch below fixes a bug I introduced earlier.
> > Cc'ing the sparc folks, as this probably affects them
> > (although I would think that it fixes broken behavior for them..?)
> >
> >
> >
>
> And this is a followup to the prior patch; an optimization based
> upon what we're doing with pkg2path stuff.
>
> The sparc folks don't use pkg2path, so this shouldn't affect them at
> all.
>
> Cc'ing Mitch as well, in case there's some reason why I should be
> wary of doing this. :)
>
>
>
>
> From: Andres Salomon <dilinger@...ued.net>
>
> The of_pdt_try_pkg2path() function currently allocates memory for
> and fetches the full path name from OFW (of the form '/foo/bar/baz@0').
> It then returns only the name ('baz@0)', and the caller re-constructs
> the full name (for dp->full_name) based upon dp->parent->full_name and
> what was returned by of_pdt_try_pkg2path(). Oh, and it allocates more
> memory for it.
>
> OLPC is the only architecture which fills in the pkg2path hook, so
> this shouldn't affect any other users (ie, sparc). Since in practice
> (dp->parent->full_name + '/' + strrchr(pkg2path, '/')) and the result
> from pkg2path end up being exactly the same, just short circuit the rest
> of the of_pdt_build_full_name logic and set dp->full_name to the result
> of pkg2path. This means we don't have to parse it nor allocate more
> memory for it.
>
> This saves time, code, and memory.
>
> Signed-off-by: Andres Salomon <dilinger@...ued.net>
> ---
> drivers/of/pdt.c | 17 +++++++----------
> 1 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/of/pdt.c b/drivers/of/pdt.c
> index b39d584..605834b 100644
> --- a/drivers/of/pdt.c
> +++ b/drivers/of/pdt.c
> @@ -134,7 +134,7 @@ static char * __init of_pdt_get_one_property(phandle node, const char *name)
>
> static char * __init of_pdt_try_pkg2path(phandle node)
> {
> - char *res, *buf = NULL;
> + char *buf = NULL;
> int len;
>
> if (!of_pdt_prom_ops->pkg2path)
> @@ -147,13 +147,7 @@ static char * __init of_pdt_try_pkg2path(phandle node)
> pr_err("%s: package-to-path failed\n", __func__);
> return NULL;
> }
> -
> - res = strrchr(buf, '/');
> - if (!res) {
> - pr_err("%s: couldn't find / in %s\n", __func__, buf);
> - return NULL;
> - }
> - return res+1;
> + return buf;
> }
>
> static struct device_node * __init of_pdt_create_node(phandle node,
> @@ -193,10 +187,13 @@ static char * __init of_pdt_build_full_name(struct device_node *dp,
> * name property (ie, 'battery'), we want the name we see with
> * package-to-path (ie, 'battery@0').
> */
> + n = of_pdt_try_pkg2path(node);
> + if (n)
> + return n;
> +
> + /* Older methods for determining full name */
> name = of_pdt_node_name(dp);
> if (!name)
> - name = of_pdt_try_pkg2path(node);
> - if (!name)
Ah, this addresses one of the comments I just made on the first patch.
I'd squash the two patches together before you repost.
g.
> name = dp->name;
>
> plen = strlen(dp->parent->full_name);
> --
> 1.7.2.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists