Patchwork [1/2] drivers: ide: Include appropriate header file in ide-cd_verbose.c

login
register
mail settings
Submitter Rashika Kheria
Date Dec. 17, 2013, 11:08 a.m.
Message ID <d24d7befadfbfb9501d0e873f13e39fd42ec893b.1387278225.git.rashika.kheria@gmail.com>
Download mbox | patch
Permalink /patch/302089/
State Accepted
Delegated to: David Miller
Headers show

Comments

Rashika Kheria - Dec. 17, 2013, 11:08 a.m.
Include appropriate header file ide-cd.h in ide-cd_verbose.c because
function ide_cd_log_error() has its prototype declaration in ide-cd.h.
Also, include linux/ide.h because it contains certain declarations
necessary for including ide-cd.h.

This eliminates the following warnings in ide-cd_verbose.c:
drivers/ide/ide-cd_verbose.c:251:6: warning: no previous prototype for ‘ide_cd_log_error’ [-Wmissing-prototypes]

Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>
---
 drivers/ide/ide-cd_verbose.c |    2 ++
 1 file changed, 2 insertions(+)
Borislav Petkov - Dec. 17, 2013, 11:19 a.m.
On Tue, Dec 17, 2013 at 04:38:16PM +0530, Rashika Kheria wrote:
> Include appropriate header file ide-cd.h in ide-cd_verbose.c because
> function ide_cd_log_error() has its prototype declaration in ide-cd.h.
> Also, include linux/ide.h because it contains certain declarations
> necessary for including ide-cd.h.
> 
> This eliminates the following warnings in ide-cd_verbose.c:
> drivers/ide/ide-cd_verbose.c:251:6: warning: no previous prototype for ‘ide_cd_log_error’ [-Wmissing-prototypes]

drivers/ide/ is open only to fixes for serious, real bugs - nothing
else. I wouldn't waste my time with it if I were you.
Josh Triplett - Dec. 17, 2013, 4:37 p.m.
On Tue, Dec 17, 2013 at 04:38:16PM +0530, Rashika Kheria wrote:
> Include appropriate header file ide-cd.h in ide-cd_verbose.c because
> function ide_cd_log_error() has its prototype declaration in ide-cd.h.
> Also, include linux/ide.h because it contains certain declarations
> necessary for including ide-cd.h.
> 
> This eliminates the following warnings in ide-cd_verbose.c:
> drivers/ide/ide-cd_verbose.c:251:6: warning: no previous prototype for ‘ide_cd_log_error’ [-Wmissing-prototypes]
> 
> Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>

Reviewed-by: Josh Triplett <josh@joshtriplett.org>

>  drivers/ide/ide-cd_verbose.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/ide/ide-cd_verbose.c b/drivers/ide/ide-cd_verbose.c
> index 6490a2d..f079ca2 100644
> --- a/drivers/ide/ide-cd_verbose.c
> +++ b/drivers/ide/ide-cd_verbose.c
> @@ -9,7 +9,9 @@
>  #include <linux/kernel.h>
>  #include <linux/blkdev.h>
>  #include <linux/cdrom.h>
> +#include <linux/ide.h>
>  #include <scsi/scsi.h>
> +#include "ide-cd.h"
>  
>  #ifndef CONFIG_BLK_DEV_IDECD_VERBOSE_ERRORS
>  void ide_cd_log_error(const char *name, struct request *failed_command,
> -- 
> 1.7.9.5
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josh Triplett - Dec. 17, 2013, 4:44 p.m.
On Tue, Dec 17, 2013 at 12:19:21PM +0100, Borislav Petkov wrote:
> On Tue, Dec 17, 2013 at 04:38:16PM +0530, Rashika Kheria wrote:
> > Include appropriate header file ide-cd.h in ide-cd_verbose.c because
> > function ide_cd_log_error() has its prototype declaration in ide-cd.h.
> > Also, include linux/ide.h because it contains certain declarations
> > necessary for including ide-cd.h.
> > 
> > This eliminates the following warnings in ide-cd_verbose.c:
> > drivers/ide/ide-cd_verbose.c:251:6: warning: no previous prototype for ‘ide_cd_log_error’ [-Wmissing-prototypes]
> 
> drivers/ide/ is open only to fixes for serious, real bugs - nothing
> else. I wouldn't waste my time with it if I were you.

A quick check of "git log drivers/ide/" turns up a pile of fixes and
cleanups; for instance:

c2f7d1e ide: pmac: remove unnecessary pci_set_drvdata()
a6fd6063 ide: cs5536: use module_pci_driver()
58e48be ide: pmac: Remove casting the return value which is a void pointer
64110c1 ide: sgiioc4: Staticize ioc4_ide_attach_one()
70ddce8 ide: palm_bk3710: add missing __iomem annotation

The two proposed patches here are extremely low risk, don't actually
change the compiled code in any way like some of those did, and fix real
compiler warnings.

- Josh Triplett
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Borislav Petkov - Dec. 17, 2013, 5:54 p.m.
On Tue, Dec 17, 2013 at 08:44:03AM -0800, Josh Triplett wrote:
> The two proposed patches here are extremely low risk, don't actually
> change the compiled code in any way like some of those did, and fix
> real compiler warnings.

Considering the fact that IDE is about to be deleted (unless something
embedded still wants it which I hardly doubt but hey, I'm always open to
surprises), putting *any* work into that is a complete waste of time,
IMO. But it is your time so I certainly won't tell you how to spend it.

:-)
Sergei Shtylyov - Dec. 17, 2013, 6:11 p.m.
Hello.

On 12/17/2013 02:08 PM, Rashika Kheria wrote:

> Include appropriate header file ide-cd.h in ide-cd_verbose.c because
> function ide_cd_log_error() has its prototype declaration in ide-cd.h.
> Also, include linux/ide.h because it contains certain declarations
> necessary for including ide-cd.h.

    You should fix "ide-cd.h" if it depends on <linux/ide.h>, not the .c file 
including it.

> This eliminates the following warnings in ide-cd_verbose.c:
> drivers/ide/ide-cd_verbose.c:251:6: warning: no previous prototype for ‘ide_cd_log_error’ [-Wmissing-prototypes]

> Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>

WBR, Sergei


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Borislav Petkov - Dec. 17, 2013, 7:06 p.m.
On Tue, Dec 17, 2013 at 10:42:54PM +0300, Sergei Shtylyov wrote:
> You're right in your doubting: there are quite a lot of drivers used
> by embedded platforms that nobody has bothered to convert to libata
> thus far.

Hold on, what does that mean? IDE is back from the dead and used only
for embedded stuff?

Or we force people to convert drivers to libata by *deleting* ide?

:-)
Sergei Shtylyov - Dec. 17, 2013, 7:42 p.m.
Hello.

On 12/17/2013 08:54 PM, Borislav Petkov wrote:

>> The two proposed patches here are extremely low risk, don't actually
>> change the compiled code in any way like some of those did, and fix
>> real compiler warnings.

> Considering the fact that IDE is about to be deleted (unless something
> embedded still wants it which I hardly doubt

    You're right in your doubting: there are quite a lot of drivers used by 
embedded platforms that nobody has bothered to convert to libata thus far.

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartlomiej Zolnierkiewicz - Dec. 18, 2013, 1:23 p.m.
Hi,

On Tuesday, December 17, 2013 08:06:10 PM Borislav Petkov wrote:
> On Tue, Dec 17, 2013 at 10:42:54PM +0300, Sergei Shtylyov wrote:
> > You're right in your doubting: there are quite a lot of drivers used
> > by embedded platforms that nobody has bothered to convert to libata
> > thus far.
> 
> Hold on, what does that mean? IDE is back from the dead and used only
> for embedded stuff?

No, it means that there are legacy PATA chipsets (some used by embedded
platforms) that are only supported by IDE host drivers.

> Or we force people to convert drivers to libata by *deleting* ide?
> 
> :-)

If there is a need I should be able to help in converting these leftover
drivers before we delete IDE.  Once I find some time I will go over all
IDE host drivers, make a list of affected hardware and start a discussion
on IDE removal.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Borislav Petkov - Dec. 18, 2013, 1:33 p.m.
Hey Bart,

long time no see! :-)

On Wed, Dec 18, 2013 at 02:23:52PM +0100, Bartlomiej Zolnierkiewicz wrote:
> No, it means that there are legacy PATA chipsets (some used by embedded
> platforms) that are only supported by IDE host drivers.

I know what it means - I was asking a provocative question. :)

> If there is a need I should be able to help in converting these
> leftover drivers before we delete IDE. Once I find some time I will go
> over all IDE host drivers, make a list of affected hardware and start
> a discussion on IDE removal.

While this is a generous offer, I'd leave it to the people who actually
need that functionality to do the conversion themselves. But this is
just me.
Sergei Shtylyov - Dec. 18, 2013, 6:35 p.m.
On 12/17/2013 10:06 PM, Borislav Petkov wrote:

>> You're right in your doubting: there are quite a lot of drivers used
>> by embedded platforms that nobody has bothered to convert to libata
>> thus far.

> Hold on, what does that mean? IDE is back from the dead and used only
> for embedded stuff?

    Like jazz, IDE "is not dead, it just smells funny".

> Or we force people to convert drivers to libata by *deleting* ide?

    I hope not. To me, it would be a shame e.g. to delete more or less 
well-designed IDE taskfile interface to the ad-hockery that libata has in its 
place...

> :-)

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Jan. 29, 2014, 7:36 a.m.
From: Rashika Kheria <rashika.kheria@gmail.com>

Date: Tue, 17 Dec 2013 16:38:16 +0530

> Include appropriate header file ide-cd.h in ide-cd_verbose.c because

> function ide_cd_log_error() has its prototype declaration in ide-cd.h.

> Also, include linux/ide.h because it contains certain declarations

> necessary for including ide-cd.h.

> 

> This eliminates the following warnings in ide-cd_verbose.c:

> drivers/ide/ide-cd_verbose.c:251:6: warning: no previous prototype for ‘ide_cd_log_error’ [-Wmissing-prototypes]

> 

> Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>


Applied.

Patch

diff --git a/drivers/ide/ide-cd_verbose.c b/drivers/ide/ide-cd_verbose.c
index 6490a2d..f079ca2 100644
--- a/drivers/ide/ide-cd_verbose.c
+++ b/drivers/ide/ide-cd_verbose.c
@@ -9,7 +9,9 @@ 
 #include <linux/kernel.h>
 #include <linux/blkdev.h>
 #include <linux/cdrom.h>
+#include <linux/ide.h>
 #include <scsi/scsi.h>
+#include "ide-cd.h"
 
 #ifndef CONFIG_BLK_DEV_IDECD_VERBOSE_ERRORS
 void ide_cd_log_error(const char *name, struct request *failed_command,