* [PATCH resubmit] /drivers/ata ioremap returncode check
@ 2007-08-23 2:47 ` Scott Thompson
0 siblings, 0 replies; 12+ messages in thread
From: Scott Thompson @ 2007-08-23 2:47 UTC (permalink / raw)
To: brandon, linux-kernel, kernel-janitors, linux-ide
Patchset against 2.6.23-rc3. corrects missing ioremap return
checks, resending after making changes suggested....
Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
------------------------------------------------------------
diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
index 4ca7fd6..8dc7c3b 100644
--- a/drivers/ata/pata_ixp4xx_cf.c
+++ b/drivers/ata/pata_ixp4xx_cf.c
@@ -189,6 +189,10 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
+ if (!data->cs0 || !data->cs1) {
+ return -ENOMEM;
+ }
+
irq = platform_get_irq(pdev, 0);
if (irq)
set_irq_type(irq, IRQT_RISING);
> ----- Forwarded message from Brandon Philips <brandon@ifup.org> ----
> -
> The iounmap calls are unnecessary since devm_ioremap will un-
> allocate the space if you return an error from probe. See
> Documentation/driver-model/devres.txt
>
> But, something like this is needed.
>
> + if (!data->cs0 || !data->cs1)
> + return -ENOMEM;
>
> Thanks, Brandon
____________________________________________________________________________________
Pinpoint customers who are looking for what you sell.
http://searchmarketing.yahoo.com/
____________________________________________________________________________________
Building a website is a piece of cake. Yahoo! Small Business gives you all the tools to get online.
http://smallbusiness.yahoo.com/webhosting
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH resubmit] /drivers/ata ioremap returncode check
2007-08-23 2:47 ` Scott Thompson
@ 2007-08-23 5:34 ` Brandon Philips
-1 siblings, 0 replies; 12+ messages in thread
From: Brandon Philips @ 2007-08-23 5:34 UTC (permalink / raw)
To: postfail; +Cc: linux-kernel, kernel-janitors, linux-ide
On 19:47 Wed 22 Aug 2007, Scott Thompson wrote:
> Patchset against 2.6.23-rc3. corrects missing ioremap return
> checks, resending after making changes suggested....
>
> Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
> ------------------------------------------------------------
> diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
> index 4ca7fd6..8dc7c3b 100644
> --- a/drivers/ata/pata_ixp4xx_cf.c
> +++ b/drivers/ata/pata_ixp4xx_cf.c
> @@ -189,6 +189,10 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
> data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
> data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
>
> + if (!data->cs0 || !data->cs1) {
> + return -ENOMEM;
> + }
> +
You aren't following the Kernel CodingStyle there.
See Documentation/CodingStyle:
"Do not unnecessarily use braces where a single statement will do."
+ if (!data->cs0 || !data->cs1)
+ return -ENOMEM;
Thanks,
Brandon
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH resubmit] /drivers/ata ioremap returncode check
@ 2007-08-23 5:34 ` Brandon Philips
0 siblings, 0 replies; 12+ messages in thread
From: Brandon Philips @ 2007-08-23 5:34 UTC (permalink / raw)
To: postfail; +Cc: linux-kernel, kernel-janitors, linux-ide
On 19:47 Wed 22 Aug 2007, Scott Thompson wrote:
> Patchset against 2.6.23-rc3. corrects missing ioremap return
> checks, resending after making changes suggested....
>
> Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
> ------------------------------------------------------------
> diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
> index 4ca7fd6..8dc7c3b 100644
> --- a/drivers/ata/pata_ixp4xx_cf.c
> +++ b/drivers/ata/pata_ixp4xx_cf.c
> @@ -189,6 +189,10 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
> data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
> data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
>
> + if (!data->cs0 || !data->cs1) {
> + return -ENOMEM;
> + }
> +
You aren't following the Kernel CodingStyle there.
See Documentation/CodingStyle:
"Do not unnecessarily use braces where a single statement will do."
+ if (!data->cs0 || !data->cs1)
+ return -ENOMEM;
Thanks,
Brandon
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH resubmit] /drivers/ata ioremap returncode check
2007-08-23 5:34 ` Brandon Philips
@ 2007-08-23 17:42 ` Alexey Dobriyan
-1 siblings, 0 replies; 12+ messages in thread
From: Alexey Dobriyan @ 2007-08-23 17:42 UTC (permalink / raw)
To: Brandon Philips; +Cc: postfail, linux-kernel, kernel-janitors, linux-ide
On Wed, Aug 22, 2007 at 10:34:58PM -0700, Brandon Philips wrote:
> On 19:47 Wed 22 Aug 2007, Scott Thompson wrote:
> > Patchset against 2.6.23-rc3. corrects missing ioremap return
> > checks, resending after making changes suggested....
> >
> > Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
> > ------------------------------------------------------------
> > diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
> > index 4ca7fd6..8dc7c3b 100644
> > --- a/drivers/ata/pata_ixp4xx_cf.c
> > +++ b/drivers/ata/pata_ixp4xx_cf.c
> > @@ -189,6 +189,10 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
> > data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
> > data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
> >
> > + if (!data->cs0 || !data->cs1) {
> > + return -ENOMEM;
> > + }
> > +
>
> You aren't following the Kernel CodingStyle there.
>
> See Documentation/CodingStyle:
>
> "Do not unnecessarily use braces where a single statement will do."
>
> + if (!data->cs0 || !data->cs1)
> + return -ENOMEM;
This and nice printk message absence. :)
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH resubmit] /drivers/ata ioremap returncode check
@ 2007-08-23 17:42 ` Alexey Dobriyan
0 siblings, 0 replies; 12+ messages in thread
From: Alexey Dobriyan @ 2007-08-23 17:42 UTC (permalink / raw)
To: Brandon Philips; +Cc: postfail, linux-kernel, kernel-janitors, linux-ide
On Wed, Aug 22, 2007 at 10:34:58PM -0700, Brandon Philips wrote:
> On 19:47 Wed 22 Aug 2007, Scott Thompson wrote:
> > Patchset against 2.6.23-rc3. corrects missing ioremap return
> > checks, resending after making changes suggested....
> >
> > Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
> > ------------------------------------------------------------
> > diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
> > index 4ca7fd6..8dc7c3b 100644
> > --- a/drivers/ata/pata_ixp4xx_cf.c
> > +++ b/drivers/ata/pata_ixp4xx_cf.c
> > @@ -189,6 +189,10 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
> > data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
> > data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
> >
> > + if (!data->cs0 || !data->cs1) {
> > + return -ENOMEM;
> > + }
> > +
>
> You aren't following the Kernel CodingStyle there.
>
> See Documentation/CodingStyle:
>
> "Do not unnecessarily use braces where a single statement will do."
>
> + if (!data->cs0 || !data->cs1)
> + return -ENOMEM;
This and nice printk message absence. :)
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH resubmit] /drivers/ata ioremap returncode check
2007-08-23 17:42 ` Alexey Dobriyan
@ 2007-08-23 17:53 ` Brandon Philips
-1 siblings, 0 replies; 12+ messages in thread
From: Brandon Philips @ 2007-08-23 17:53 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: postfail, linux-kernel, kernel-janitors, linux-ide
On 21:42 Thu 23 Aug 2007, Alexey Dobriyan wrote:
> On Wed, Aug 22, 2007 at 10:34:58PM -0700, Brandon Philips wrote:
> > On 19:47 Wed 22 Aug 2007, Scott Thompson wrote:
> > > Patchset against 2.6.23-rc3. corrects missing ioremap return
> > > checks, resending after making changes suggested....
> > >
> > > Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
> > > ------------------------------------------------------------
> > > diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
> > > index 4ca7fd6..8dc7c3b 100644
> > > --- a/drivers/ata/pata_ixp4xx_cf.c
> > > +++ b/drivers/ata/pata_ixp4xx_cf.c
> > > @@ -189,6 +189,10 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
> > > data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
> > > data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
> > >
> > > + if (!data->cs0 || !data->cs1) {
> > > + return -ENOMEM;
> > > + }
> > > +
> >
> > You aren't following the Kernel CodingStyle there.
> >
> > See Documentation/CodingStyle:
> >
> > "Do not unnecessarily use braces where a single statement will do."
> >
> > + if (!data->cs0 || !data->cs1)
> > + return -ENOMEM;
>
> This and nice printk message absence. :)
Huh? What do you mean?
Thanks,
Brandon
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH resubmit] /drivers/ata ioremap returncode check
@ 2007-08-23 17:53 ` Brandon Philips
0 siblings, 0 replies; 12+ messages in thread
From: Brandon Philips @ 2007-08-23 17:53 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: postfail, linux-kernel, kernel-janitors, linux-ide
On 21:42 Thu 23 Aug 2007, Alexey Dobriyan wrote:
> On Wed, Aug 22, 2007 at 10:34:58PM -0700, Brandon Philips wrote:
> > On 19:47 Wed 22 Aug 2007, Scott Thompson wrote:
> > > Patchset against 2.6.23-rc3. corrects missing ioremap return
> > > checks, resending after making changes suggested....
> > >
> > > Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
> > > ------------------------------------------------------------
> > > diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
> > > index 4ca7fd6..8dc7c3b 100644
> > > --- a/drivers/ata/pata_ixp4xx_cf.c
> > > +++ b/drivers/ata/pata_ixp4xx_cf.c
> > > @@ -189,6 +189,10 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
> > > data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
> > > data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
> > >
> > > + if (!data->cs0 || !data->cs1) {
> > > + return -ENOMEM;
> > > + }
> > > +
> >
> > You aren't following the Kernel CodingStyle there.
> >
> > See Documentation/CodingStyle:
> >
> > "Do not unnecessarily use braces where a single statement will do."
> >
> > + if (!data->cs0 || !data->cs1)
> > + return -ENOMEM;
>
> This and nice printk message absence. :)
Huh? What do you mean?
Thanks,
Brandon
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH resubmit] /drivers/ata ioremap returncode check
@ 2007-08-23 19:34 ` Scott Thompson
0 siblings, 0 replies; 12+ messages in thread
From: Scott Thompson @ 2007-08-23 19:34 UTC (permalink / raw)
To: brandon, linux-kernel, kernel-janitors, linux-ide
Patchset against 2.6.23-rc3. corrects missing ioremap return
checks, resending after fixing for documentation making changes suggested...
As for suggestions, I did fix the {} that weren't needed. I did not add in a
"printk" err or warn statement as the rest of the module does not contain these
messages. When auditing (as I am for ioremap checking here) I am attempting
to keep changes "in the style of their surroundings". Otherwise it's massive
scope screep on these patches.
Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
---------------------------------------------------------------
diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
index 4ca7fd6..6f9a2b7 100644
--- a/drivers/ata/pata_ixp4xx_cf.c
+++ b/drivers/ata/pata_ixp4xx_cf.c
@@ -189,6 +189,9 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
+ if (!data->cs0 || !data->cs1)
+ return -ENOMEM;
+
irq = platform_get_irq(pdev, 0);
if (irq)
set_irq_type(irq, IRQT_RISING);
____________________________________________________________________________________
Moody friends. Drama queens. Your life? Nope! - their life, your story. Play Sims Stories at
Yahoo! Games.
http://sims.yahoo.com/
____________________________________________________________________________________
Looking for a deal? Find great prices on flights and hotels with Yahoo! FareChase.
http://farechase.yahoo.com/
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH resubmit] /drivers/ata ioremap returncode check
@ 2007-08-23 19:34 ` Scott Thompson
0 siblings, 0 replies; 12+ messages in thread
From: Scott Thompson @ 2007-08-23 19:34 UTC (permalink / raw)
To: brandon, linux-kernel, kernel-janitors, linux-ide
Patchset against 2.6.23-rc3. corrects missing ioremap return
checks, resending after fixing for documentation making changes suggested...
As for suggestions, I did fix the {} that weren't needed. I did not add in a
"printk" err or warn statement as the rest of the module does not contain these
messages. When auditing (as I am for ioremap checking here) I am attempting
to keep changes "in the style of their surroundings". Otherwise it's massive
scope screep on these patches.
Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
---------------------------------------------------------------
diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
index 4ca7fd6..6f9a2b7 100644
--- a/drivers/ata/pata_ixp4xx_cf.c
+++ b/drivers/ata/pata_ixp4xx_cf.c
@@ -189,6 +189,9 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
+ if (!data->cs0 || !data->cs1)
+ return -ENOMEM;
+
irq = platform_get_irq(pdev, 0);
if (irq)
set_irq_type(irq, IRQT_RISING);
____________________________________________________________________________________
Moody friends. Drama queens. Your life? Nope! - their life, your story. Play Sims Stories at
Yahoo! Games.
http://sims.yahoo.com/
____________________________________________________________________________________
Looking for a deal? Find great prices on flights and hotels with Yahoo! FareChase.
http://farechase.yahoo.com/
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH resubmit] /drivers/ata ioremap returncode check
2007-08-23 19:34 ` Scott Thompson
@ 2007-09-03 8:29 ` Tejun Heo
-1 siblings, 0 replies; 12+ messages in thread
From: Tejun Heo @ 2007-09-03 8:29 UTC (permalink / raw)
To: postfail; +Cc: brandon, linux-kernel, kernel-janitors, linux-ide
Scott Thompson wrote:
> Patchset against 2.6.23-rc3. corrects missing ioremap return
> checks, resending after fixing for documentation making changes suggested...
>
> As for suggestions, I did fix the {} that weren't needed. I did not add in a
> "printk" err or warn statement as the rest of the module does not contain these
> messages. When auditing (as I am for ioremap checking here) I am attempting
> to keep changes "in the style of their surroundings". Otherwise it's massive
> scope screep on these patches.
>
> Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
Acked-by: Tejun Heo <htejun@gmail.com>
It would be better to start the subject line with pata_ixp4xx_cf: but
maybe Jeff can fix it up while applying.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH resubmit] /drivers/ata ioremap returncode check
@ 2007-09-03 8:29 ` Tejun Heo
0 siblings, 0 replies; 12+ messages in thread
From: Tejun Heo @ 2007-09-03 8:29 UTC (permalink / raw)
To: postfail; +Cc: brandon, linux-kernel, kernel-janitors, linux-ide
Scott Thompson wrote:
> Patchset against 2.6.23-rc3. corrects missing ioremap return
> checks, resending after fixing for documentation making changes suggested...
>
> As for suggestions, I did fix the {} that weren't needed. I did not add in a
> "printk" err or warn statement as the rest of the module does not contain these
> messages. When auditing (as I am for ioremap checking here) I am attempting
> to keep changes "in the style of their surroundings". Otherwise it's massive
> scope screep on these patches.
>
> Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
Acked-by: Tejun Heo <htejun@gmail.com>
It would be better to start the subject line with pata_ixp4xx_cf: but
maybe Jeff can fix it up while applying.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 12+ messages in thread