| Message ID | 49E81CED.1000704@monstr.eu (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Grant Likely |
| Headers | show |
On Fri, Apr 17, 2009 at 12:08 AM, Michal Simek <monstr@monstr.eu> wrote: > Hi All, > > I have got email from Ilpo about prom_parse file. > I take this file from powerpc. Who did write prom_parse file and take care about? Posting to the linuxppc-dev list is sufficient to start. There are several people who may be interested. > BTW: What about to move prom_parse file to any generic location as we discussed in past? > Any volunteer? I'm kind of working on it. More specifically, I'm looking at factoring out fdt stuff into common code (drivers/of/of_fdt.c). But I haven't made a whole lot of progress yet. > -------- Original Message -------- > Subject: [RFC!] [PATCH] microblaze: fix bug in error handling > Date: Thu, 16 Apr 2009 23:05:53 +0300 (EEST) > From: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> > To: monstr@monstr.eu > CC: microblaze-uclinux@itee.uq.edu.au > > While some version of the patches were on the lkml I read > some part of the code briefly through but my feedback got > stuck into postponed emails, so here's one correctness > related issue I might have found (the rest were just > cosmetic things). > > I'm not sure if the latter return needs the of_node_put or not > but it seems more likely than not. Yes, it does. This change is applicable to arch/powerpc/kernel/prom_parse.c too. > > Not even compile tested. > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> > --- > arch/microblaze/kernel/prom_parse.c | 11 +++++++---- > 1 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/arch/microblaze/kernel/prom_parse.c b/arch/microblaze/kernel/prom_parse.c > index ae0352e..d1174bc 100644 > --- a/arch/microblaze/kernel/prom_parse.c > +++ b/arch/microblaze/kernel/prom_parse.c > @@ -927,20 +927,23 @@ int of_irq_map_one(struct device_node *device, > /* Get size of interrupt specifier */ > tmp = of_get_property(p, "#interrupt-cells", NULL); > if (tmp == NULL) { > - of_node_put(p); > - return -EINVAL; > + res = -EINVAL; > + goto out; > } > intsize = *tmp; > > pr_debug(" intsize=%d intlen=%d\n", intsize, intlen); > > /* Check index */ > - if ((index + 1) * intsize > intlen) > - return -EINVAL; > + if ((index + 1) * intsize > intlen) { > + res = -EINVAL; > + goto out; > + } > > /* Get new specifier and map it */ > res = of_irq_map_raw(p, intspec + index * intsize, intsize, > addr, out_irq); > +out: > of_node_put(p); > return res; > } > -- > 1.5.6.5 > > > -- > Michal Simek, Ing. (M.Eng) > w: www.monstr.eu p: +42-0-721842854 >
On Fri, 2009-04-17 at 08:08 +0200, Michal Simek wrote: > Hi All, > > I have got email from Ilpo about prom_parse file. > I take this file from powerpc. Who did write prom_parse file and take care about? I wrote it :-) > BTW: What about to move prom_parse file to any generic location as we discussed in past? > Any volunteer? Well.. it relies on some bits and pieces that are rather powerpc specific afaik such as our irq mapping scheme. I suppose we could split those bits and move them to a separate file though. Off hand the patch looks ok but I'll have to look more closely when I get back from vacation. Cheers, Ben.
Grant Likely wrote: > On Fri, Apr 17, 2009 at 12:08 AM, Michal Simek <monstr@monstr.eu> wrote: >> Hi All, >> >> I have got email from Ilpo about prom_parse file. >> I take this file from powerpc. Who did write prom_parse file and take care about? > > Posting to the linuxppc-dev list is sufficient to start. There are > several people who may be interested. > >> BTW: What about to move prom_parse file to any generic location as we discussed in past? >> Any volunteer? > > I'm kind of working on it. More specifically, I'm looking at > factoring out fdt stuff into common code (drivers/of/of_fdt.c). But I > haven't made a whole lot of progress yet. > >> -------- Original Message -------- >> Subject: [RFC!] [PATCH] microblaze: fix bug in error handling >> Date: Thu, 16 Apr 2009 23:05:53 +0300 (EEST) >> From: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> >> To: monstr@monstr.eu >> CC: microblaze-uclinux@itee.uq.edu.au >> >> While some version of the patches were on the lkml I read >> some part of the code briefly through but my feedback got >> stuck into postponed emails, so here's one correctness >> related issue I might have found (the rest were just >> cosmetic things). >> >> I'm not sure if the latter return needs the of_node_put or not >> but it seems more likely than not. > > Yes, it does. This change is applicable to > arch/powerpc/kernel/prom_parse.c too. ok. Ilpo: Can you create patch for both architectures? Thanks, Michal > >> Not even compile tested. >> >> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> >> --- >> arch/microblaze/kernel/prom_parse.c | 11 +++++++---- >> 1 files changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/arch/microblaze/kernel/prom_parse.c b/arch/microblaze/kernel/prom_parse.c >> index ae0352e..d1174bc 100644 >> --- a/arch/microblaze/kernel/prom_parse.c >> +++ b/arch/microblaze/kernel/prom_parse.c >> @@ -927,20 +927,23 @@ int of_irq_map_one(struct device_node *device, >> /* Get size of interrupt specifier */ >> tmp = of_get_property(p, "#interrupt-cells", NULL); >> if (tmp == NULL) { >> - of_node_put(p); >> - return -EINVAL; >> + res = -EINVAL; >> + goto out; >> } >> intsize = *tmp; >> >> pr_debug(" intsize=%d intlen=%d\n", intsize, intlen); >> >> /* Check index */ >> - if ((index + 1) * intsize > intlen) >> - return -EINVAL; >> + if ((index + 1) * intsize > intlen) { >> + res = -EINVAL; >> + goto out; >> + } >> >> /* Get new specifier and map it */ >> res = of_irq_map_raw(p, intspec + index * intsize, intsize, >> addr, out_irq); >> +out: >> of_node_put(p); >> return res; >> } >> -- >> 1.5.6.5 >> >> >> -- >> Michal Simek, Ing. (M.Eng) >> w: www.monstr.eu p: +42-0-721842854 >> > > >
On Fri, 17 Apr 2009, Michal Simek wrote: > Grant Likely wrote: > > On Fri, Apr 17, 2009 at 12:08 AM, Michal Simek <monstr@monstr.eu> wrote: > >> Hi All, > >> > >> I have got email from Ilpo about prom_parse file. > >> I take this file from powerpc. Who did write prom_parse file and take care about? > > > > Posting to the linuxppc-dev list is sufficient to start. There are > > several people who may be interested. > > > >> BTW: What about to move prom_parse file to any generic location as we discussed in past? > >> Any volunteer? > > > > I'm kind of working on it. More specifically, I'm looking at > > factoring out fdt stuff into common code (drivers/of/of_fdt.c). But I > > haven't made a whole lot of progress yet. > > > >> -------- Original Message -------- > >> Subject: [RFC!] [PATCH] microblaze: fix bug in error handling > >> Date: Thu, 16 Apr 2009 23:05:53 +0300 (EEST) > >> From: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> > >> To: monstr@monstr.eu > >> CC: microblaze-uclinux@itee.uq.edu.au > >> > >> While some version of the patches were on the lkml I read > >> some part of the code briefly through but my feedback got > >> stuck into postponed emails, so here's one correctness > >> related issue I might have found (the rest were just > >> cosmetic things). > >> > >> I'm not sure if the latter return needs the of_node_put or not > >> but it seems more likely than not. > > > > Yes, it does. This change is applicable to > > arch/powerpc/kernel/prom_parse.c too. > > ok. > Ilpo: Can you create patch for both architectures? Sure, but tomorrow as today is a deadline day :-).
diff --git a/arch/microblaze/kernel/prom_parse.c b/arch/microblaze/kernel/prom_parse.c index ae0352e..d1174bc 100644 --- a/arch/microblaze/kernel/prom_parse.c +++ b/arch/microblaze/kernel/prom_parse.c @@ -927,20 +927,23 @@ int of_irq_map_one(struct device_node *device, /* Get size of interrupt specifier */ tmp = of_get_property(p, "#interrupt-cells", NULL); if (tmp == NULL) { - of_node_put(p); - return -EINVAL; + res = -EINVAL; + goto out; } intsize = *tmp; pr_debug(" intsize=%d intlen=%d\n", intsize, intlen); /* Check index */ - if ((index + 1) * intsize > intlen) - return -EINVAL; + if ((index + 1) * intsize > intlen) { + res = -EINVAL; + goto out; + } /* Get new specifier and map it */ res = of_irq_map_raw(p, intspec + index * intsize, intsize, addr, out_irq); +out: of_node_put(p); return res; }