[ mediabay.c changes are limited to <linux/ide.h> include removal ]
Linus, please pull from 'for-linus' branch of:
master.kernel.org:/pub/scm/linux/kernel/git/bart/ide-2.6.git for-linus
to receive the following updates:
drivers/ide/ide-cd.c | 2 +-
drivers/ide/ide-gd.c | 17 ++++++++++++++++-
drivers/ide/palm_bk3710.c | 26 ++++++++++++--------------
drivers/macintosh/mediabay.c | 1 -
4 files changed, 29 insertions(+), 17 deletions(-)
Bartlomiej Zolnierkiewicz (1):
mediabay: fix build for CONFIG_BLOCK=n
Bruno Pr?mont (1):
ide: Stop disks on reboot for laptop which cuts power
David Brownell (1):
palm_bk3710: UDMA performance fix
Helge Deller (1):
ide-cd: fix kernel crash on hppa regression
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 3aec19d..3d4e099 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -609,7 +609,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)
struct request *rq = hwif->rq;
ide_expiry_t *expiry = NULL;
int dma_error = 0, dma, thislen, uptodate = 0;
- int write = (rq_data_dir(rq) == WRITE) ? 1 : 0, rc, nsectors;
+ int write = (rq_data_dir(rq) == WRITE) ? 1 : 0, rc = 0, nsectors;
int sense = blk_sense_request(rq);
unsigned int timeout;
u16 len;
diff --git a/drivers/ide/ide-gd.c b/drivers/ide/ide-gd.c
index 1aebdf1..4b6b71e 100644
--- a/drivers/ide/ide-gd.c
+++ b/drivers/ide/ide-gd.c
@@ -7,6 +7,7 @@
#include <linux/mutex.h>
#include <linux/ide.h>
#include <linux/hdreg.h>
+#include <linux/dmi.h>
#if !defined(CONFIG_DEBUG_BLOCK_EXT_DEVT)
#define IDE_DISK_MINORS (1 << PARTN_BITS)
@@ -99,6 +100,19 @@ static void ide_gd_resume(ide_drive_t *drive)
(void)drive->disk_ops->get_capacity(drive);
}
+static const struct dmi_system_id ide_coldreboot_table[] = {
+ {
+ /* Acer TravelMate 66x cuts power during reboot */
+ .ident = "Acer TravelMate 660",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "TravelMate 660"),
+ },
+ },
+
+ { } /* terminate list */
+};
+
static void ide_gd_shutdown(ide_drive_t *drive)
{
#ifdef CONFIG_ALPHA
@@ -115,7 +129,8 @@ static void ide_gd_shutdown(ide_drive_t *drive)
the disk to expire its write cache. */
if (system_state != SYSTEM_POWER_OFF) {
#else
- if (system_state == SYSTEM_RESTART) {
+ if (system_state == SYSTEM_RESTART &&
+ !dmi_check_system(ide_coldreboot_table)) {
#endif
drive->disk_ops->flush(drive);
return;
diff --git a/drivers/ide/palm_bk3710.c b/drivers/ide/palm_bk3710.c
index c7acca0..d1513b4 100644
--- a/drivers/ide/palm_bk3710.c
+++ b/drivers/ide/palm_bk3710.c
@@ -39,14 +39,6 @@
/* Primary Control Offset */
#define IDE_PALM_ATA_PRI_CTL_OFFSET 0x3F6
-/*
- * PalmChip 3710 IDE Controller UDMA timing structure Definition
- */
-struct palm_bk3710_udmatiming {
- unsigned int rptime; /* Ready to pause time */
- unsigned int cycletime; /* Cycle Time */
-};
-
#define BK3710_BMICP 0x00
#define BK3710_BMISP 0x02
#define BK3710_BMIDTP 0x04
@@ -75,13 +67,19 @@ struct palm_bk3710_udmatiming {
static unsigned ideclk_period; /* in nanoseconds */
+struct palm_bk3710_udmatiming {
+ unsigned int rptime; /* tRP -- Ready to pause time (nsec) */
+ unsigned int cycletime; /* tCYCTYP2/2 -- avg Cycle Time (nsec) */
+ /* tENV is always a minimum of 20 nsec */
+};
+
static const struct palm_bk3710_udmatiming palm_bk3710_udmatimings[6] = {
- {160, 240}, /* UDMA Mode 0 */
- {125, 160}, /* UDMA Mode 1 */
- {100, 120}, /* UDMA Mode 2 */
- {100, 90}, /* UDMA Mode 3 */
- {100, 60}, /* UDMA Mode 4 */
- {85, 40}, /* UDMA Mode 5 */
+ {160, 240 / 2,}, /* UDMA Mode 0 */
+ {125, 160 / 2,}, /* UDMA Mode 1 */
+ {100, 120 / 2,}, /* UDMA Mode 2 */
+ {100, 90 / 2,}, /* UDMA Mode 3 */
+ {100, 60 / 2,}, /* UDMA Mode 4 */
+ {85, 40 / 2,}, /* UDMA Mode 5 */
};
static void palm_bk3710_setudmamode(void __iomem *base, unsigned int dev,
diff --git a/drivers/macintosh/mediabay.c b/drivers/macintosh/mediabay.c
index d7e46d3..eca55ef 100644
--- a/drivers/macintosh/mediabay.c
+++ b/drivers/macintosh/mediabay.c
@@ -18,7 +18,6 @@
#include <linux/timer.h>
#include <linux/stddef.h>
#include <linux/init.h>
-#include <linux/ide.h>
#include <linux/kthread.h>
#include <linux/mutex.h>
#include <asm/prom.h>
On Wed, 2009-04-22 at 20:48 +0200, Bartlomiej Zolnierkiewicz wrote:
> diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
> index 3aec19d..3d4e099 100644
> --- a/drivers/ide/ide-cd.c
> +++ b/drivers/ide/ide-cd.c
> @@ -609,7 +609,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)
> struct request *rq = hwif->rq;
> ide_expiry_t *expiry = NULL;
> int dma_error = 0, dma, thislen, uptodate = 0;
> - int write = (rq_data_dir(rq) == WRITE) ? 1 : 0, rc, nsectors;
> + int write = (rq_data_dir(rq) == WRITE) ? 1 : 0, rc = 0, nsectors;
> int sense = blk_sense_request(rq);
> unsigned int timeout;
> u16 len;
I think ide is the only subsystem to use the
initialization style of "?:," with additional
declarations.
Would it be better to use a more standard style?
maybe:
int dma_error = 0, dma, thislen, uptodate = 0, rc = 0, nsectors;
int write = ((rq_data_dir(rq) == WRITE) ? 1 : 0;
> diff --git a/drivers/ide/palm_bk3710.c b/drivers/ide/palm_bk3710.c
> index c7acca0..d1513b4 100644
> --- a/drivers/ide/palm_bk3710.c
> +++ b/drivers/ide/palm_bk3710.c
[]
> @@ -75,13 +67,19 @@ struct palm_bk3710_udmatiming {
>
> static unsigned ideclk_period; /* in nanoseconds */
>
> +struct palm_bk3710_udmatiming {
> + unsigned int rptime; /* tRP -- Ready to pause time (nsec) */
> + unsigned int cycletime; /* tCYCTYP2/2 -- avg Cycle Time (nsec) */
> + /* tENV is always a minimum of 20 nsec */
> +};
> +
> static const struct palm_bk3710_udmatiming palm_bk3710_udmatimings[6] = {
> - {160, 240}, /* UDMA Mode 0 */
> - {125, 160}, /* UDMA Mode 1 */
> - {100, 120}, /* UDMA Mode 2 */
> - {100, 90}, /* UDMA Mode 3 */
> - {100, 60}, /* UDMA Mode 4 */
> - {85, 40}, /* UDMA Mode 5 */
> + {160, 240 / 2,}, /* UDMA Mode 0 */
> + {125, 160 / 2,}, /* UDMA Mode 1 */
> + {100, 120 / 2,}, /* UDMA Mode 2 */
> + {100, 90 / 2,}, /* UDMA Mode 3 */
> + {100, 60 / 2,}, /* UDMA Mode 4 */
> + {85, 40 / 2,}, /* UDMA Mode 5 */
> };
>
> static void palm_bk3710_setudmamode(void __iomem *base, unsigned int dev,
Odd looking commas.
Maybe:
{.rptime = 160, .cycletime = 240 / 2 }, /* UDMA Mode 0 */
etc.
On Wed, Apr 22, 2009 at 12:06:47PM -0700, Joe Perches wrote:
> On Wed, 2009-04-22 at 20:48 +0200, Bartlomiej Zolnierkiewicz wrote:
> > diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
> > index 3aec19d..3d4e099 100644
> > --- a/drivers/ide/ide-cd.c
> > +++ b/drivers/ide/ide-cd.c
> > @@ -609,7 +609,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)
> > struct request *rq = hwif->rq;
> > ide_expiry_t *expiry = NULL;
> > int dma_error = 0, dma, thislen, uptodate = 0;
> > - int write = (rq_data_dir(rq) == WRITE) ? 1 : 0, rc, nsectors;
> > + int write = (rq_data_dir(rq) == WRITE) ? 1 : 0, rc = 0, nsectors;
> > int sense = blk_sense_request(rq);
> > unsigned int timeout;
> > u16 len;
>
> I think ide is the only subsystem to use the
> initialization style of "?:," with additional
> declarations.
>
> Would it be better to use a more standard style?
>
> maybe:
> int dma_error = 0, dma, thislen, uptodate = 0, rc = 0, nsectors;
> int write = ((rq_data_dir(rq) == WRITE) ? 1 : 0;
Better to move assignment after variable definition.
Sam
On Wednesday 22 April 2009 21:06:47 Joe Perches wrote:
> On Wed, 2009-04-22 at 20:48 +0200, Bartlomiej Zolnierkiewicz wrote:
> > diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
> > index 3aec19d..3d4e099 100644
> > --- a/drivers/ide/ide-cd.c
> > +++ b/drivers/ide/ide-cd.c
> > @@ -609,7 +609,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)
> > struct request *rq = hwif->rq;
> > ide_expiry_t *expiry = NULL;
> > int dma_error = 0, dma, thislen, uptodate = 0;
> > - int write = (rq_data_dir(rq) == WRITE) ? 1 : 0, rc, nsectors;
> > + int write = (rq_data_dir(rq) == WRITE) ? 1 : 0, rc = 0, nsectors;
> > int sense = blk_sense_request(rq);
> > unsigned int timeout;
> > u16 len;
>
> I think ide is the only subsystem to use the
> initialization style of "?:," with additional
> declarations.
Well, nothing wrong with it per se.
> Would it be better to use a more standard style?
>
> maybe:
> int dma_error = 0, dma, thislen, uptodate = 0, rc = 0, nsectors;
> int write = ((rq_data_dir(rq) == WRITE) ? 1 : 0;
Personally I would prefer:
int write = !!(rq_data_dir(rq) == WRITE);
However the old code is also fine with me.
> > diff --git a/drivers/ide/palm_bk3710.c b/drivers/ide/palm_bk3710.c
> > index c7acca0..d1513b4 100644
> > --- a/drivers/ide/palm_bk3710.c
> > +++ b/drivers/ide/palm_bk3710.c
> []
> > @@ -75,13 +67,19 @@ struct palm_bk3710_udmatiming {
> >
> > static unsigned ideclk_period; /* in nanoseconds */
> >
> > +struct palm_bk3710_udmatiming {
> > + unsigned int rptime; /* tRP -- Ready to pause time (nsec) */
> > + unsigned int cycletime; /* tCYCTYP2/2 -- avg Cycle Time (nsec) */
> > + /* tENV is always a minimum of 20 nsec */
> > +};
> > +
> > static const struct palm_bk3710_udmatiming palm_bk3710_udmatimings[6] = {
> > - {160, 240}, /* UDMA Mode 0 */
> > - {125, 160}, /* UDMA Mode 1 */
> > - {100, 120}, /* UDMA Mode 2 */
> > - {100, 90}, /* UDMA Mode 3 */
> > - {100, 60}, /* UDMA Mode 4 */
> > - {85, 40}, /* UDMA Mode 5 */
> > + {160, 240 / 2,}, /* UDMA Mode 0 */
> > + {125, 160 / 2,}, /* UDMA Mode 1 */
> > + {100, 120 / 2,}, /* UDMA Mode 2 */
> > + {100, 90 / 2,}, /* UDMA Mode 3 */
> > + {100, 60 / 2,}, /* UDMA Mode 4 */
> > + {85, 40 / 2,}, /* UDMA Mode 5 */
> > };
> >
> > static void palm_bk3710_setudmamode(void __iomem *base, unsigned int dev,
>
> Odd looking commas.
>
> Maybe:
>
> {.rptime = 160, .cycletime = 240 / 2 }, /* UDMA Mode 0 */
> etc.
I really don't consider this an improvement from readability perspective
and I also don't like the added code duplication...
IMO we should just remove extra commas and add some whitespaces.
I have also more general (process oriented) comment:
All patches have been posted to linux-kernel or linux-ide for review before
and it is _much_ more efficient to raise issues (including CodingStyle ones)
during "review phase" instead of during "push to Linus" phase.
Thanks,
Bart
On Wed, 2009-04-22 at 21:43 +0200, Bartlomiej Zolnierkiewicz wrote:
> IMO we should just remove extra commas and add some whitespaces.
Your choice.
Ignore, reject, accept, improve, fine by me.
> I have also more general (process oriented) comment:
>
> All patches have been posted to linux-kernel or linux-ide for review before
> and it is _much_ more efficient to raise issues (including CodingStyle ones)
> during "review phase" instead of during "push to Linus" phase.
Maybe for you, but I'm an intermittent
and random reader.
If you accept an idea at all, queue it
up at your convenience.
cheers, Joe
On Wednesday 22 April 2009 21:55:03 Joe Perches wrote:
> On Wed, 2009-04-22 at 21:43 +0200, Bartlomiej Zolnierkiewicz wrote:
> > IMO we should just remove extra commas and add some whitespaces.
>
> Your choice.
> Ignore, reject, accept, improve, fine by me.
>
> > I have also more general (process oriented) comment:
> >
> > All patches have been posted to linux-kernel or linux-ide for review before
> > and it is _much_ more efficient to raise issues (including CodingStyle ones)
> > during "review phase" instead of during "push to Linus" phase.
>
> Maybe for you, but I'm an intermittent
> and random reader.
For everybody involved in the process -- this includes you.
I'm not the original author of many patches so I simply cannot be
correcting every single issue because:
a) it is physically impossible
b) it doesn't really educate people
Thanks,
Bart
On Wed, Apr 22, 2009 at 2:41 PM, Bartlomiej Zolnierkiewicz
<[email protected]> wrote:
> On Wednesday 22 April 2009 21:55:03 Joe Perches wrote:
>> On Wed, 2009-04-22 at 21:43 +0200, Bartlomiej Zolnierkiewicz wrote:
>> > IMO we should just remove extra commas and add some whitespaces.
>>
>> Your choice.
>> Ignore, reject, accept, improve, fine by me.
>>
>> > I have also more general (process oriented) comment:
>> >
>> > All patches have been posted to linux-kernel or linux-ide for review before
>> > and it is _much_ more efficient to raise issues (including CodingStyle ones)
>> > during "review phase" instead of during "push to Linus" phase.
>>
>> Maybe for you, but I'm an intermittent
>> and random reader.
>
> For everybody involved in the process -- this includes you.
>
> I'm not the original author of many patches so I simply cannot be
> correcting every single issue because:
>
> a) it is physically impossible
>
> b) it doesn't really educate people
His point is that you can queue a follow-up patch to remove the
ugliness. Just because it didn't get caught until the push stage
doesn't mean the code needs to stay set in stone for the rest of
eternity.
What most maintainers do at this stage is say "Great idea! Send me a
patch." Perhaps you could try that tack instead.
On Thursday 23 April 2009 00:02:38 Ray Lee wrote:
> On Wed, Apr 22, 2009 at 2:41 PM, Bartlomiej Zolnierkiewicz
> <[email protected]> wrote:
> > On Wednesday 22 April 2009 21:55:03 Joe Perches wrote:
> >> On Wed, 2009-04-22 at 21:43 +0200, Bartlomiej Zolnierkiewicz wrote:
> >> > IMO we should just remove extra commas and add some whitespaces.
> >>
> >> Your choice.
> >> Ignore, reject, accept, improve, fine by me.
> >>
> >> > I have also more general (process oriented) comment:
> >> >
> >> > All patches have been posted to linux-kernel or linux-ide for review before
> >> > and it is _much_ more efficient to raise issues (including CodingStyle ones)
> >> > during "review phase" instead of during "push to Linus" phase.
> >>
> >> Maybe for you, but I'm an intermittent
> >> and random reader.
> >
> > For everybody involved in the process -- this includes you.
> >
> > I'm not the original author of many patches so I simply cannot be
> > correcting every single issue because:
> >
> > a) it is physically impossible
> >
> > b) it doesn't really educate people
>
> His point is that you can queue a follow-up patch to remove the
> ugliness. Just because it didn't get caught until the push stage
> doesn't mean the code needs to stay set in stone for the rest of
> eternity.
I get his point and of course patches are welcomed.
However my point is that it is still suboptimal way to address
the issue in the long-term and from my perspective the long-term
is what really matters.
Thanks,
Bart
On Thursday 23 April 2009 00:02:38 Ray Lee wrote:
> On Wed, Apr 22, 2009 at 2:41 PM, Bartlomiej Zolnierkiewicz
> <[email protected]> wrote:
> > On Wednesday 22 April 2009 21:55:03 Joe Perches wrote:
> >> On Wed, 2009-04-22 at 21:43 +0200, Bartlomiej Zolnierkiewicz wrote:
> >> > IMO we should just remove extra commas and add some whitespaces.
> >>
> >> Your choice.
> >> Ignore, reject, accept, improve, fine by me.
> >>
> >> > I have also more general (process oriented) comment:
> >> >
> >> > All patches have been posted to linux-kernel or linux-ide for review before
> >> > and it is _much_ more efficient to raise issues (including CodingStyle ones)
> >> > during "review phase" instead of during "push to Linus" phase.
> >>
> >> Maybe for you, but I'm an intermittent
> >> and random reader.
> >
> > For everybody involved in the process -- this includes you.
> >
> > I'm not the original author of many patches so I simply cannot be
> > correcting every single issue because:
> >
> > a) it is physically impossible
> >
> > b) it doesn't really educate people
>
> His point is that you can queue a follow-up patch to remove the
> ugliness. Just because it didn't get caught until the push stage
> doesn't mean the code needs to stay set in stone for the rest of
> eternity.
I get his point and of course patches are welcomed.
However my point is that it is still suboptimal way to address
the issue in the long-term and from my perspective the long-term
is what really matters.
Thanks,
Bart