Re: ide-tape redux (was: Re:)

2008-02-05 Thread Borislav Petkov
... and while we're at it ...

commit c824f79fe4040f7541d7e35c546bb57a22d2fe11
Author: Borislav Petkov <[EMAIL PROTECTED]>
Date:   Wed Feb 6 06:23:10 2008 +0100

ide-tape: move all struct and other defs to the top

Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]>

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 9455ce4..398aea8 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -225,6 +225,69 @@ enum {
PC_FL_WRITING   = (1 << 5),
 };
 
+/* Tape door status */
+#define DOOR_UNLOCKED  0
+#define DOOR_LOCKED1
+#define DOOR_EXPLICITLY_LOCKED 2
+
+/* Tape flag bits values. */
+enum {
+   IDETAPE_FL_IGNORE_DSC   = (1 << 0),
+   /* 0 When the tape position is unknown */
+   IDETAPE_FL_ADDRESS_VALID= (1 << 1),
+   /* Device already opened */
+   IDETAPE_FL_BUSY = (1 << 2),
+   /* Error detected in a pipeline stage */
+   IDETAPE_FL_PIPELINE_ERR = (1 << 3),
+   /* Attempt to auto-detect the current user block size */
+   IDETAPE_FL_DETECT_BS= (1 << 4),
+   /* Currently on a filemark */
+   IDETAPE_FL_FILEMARK = (1 << 5),
+   /* DRQ interrupt device */
+   IDETAPE_FL_DRQ_INTERRUPT= (1 << 6),
+   /* pipeline active */
+   IDETAPE_FL_PIPELINE_ACTIVE  = (1 << 7),
+   /* 0 = no tape is loaded, so we don't rewind after ejecting */
+   IDETAPE_FL_MEDIUM_PRESENT   = (1 << 8),
+};
+
+/* A define for the READ BUFFER command */
+#define IDETAPE_RETRIEVE_FAULTY_BLOCK  6
+
+/* Some defines for the SPACE command */
+#define IDETAPE_SPACE_OVER_FILEMARK1
+#define IDETAPE_SPACE_TO_EOD   3
+
+/* Some defines for the LOAD UNLOAD command */
+#define IDETAPE_LU_LOAD_MASK   1
+#define IDETAPE_LU_RETENSION_MASK  2
+#define IDETAPE_LU_EOT_MASK4
+
+/*
+ * Special requests for our block device strategy routine.
+ *
+ * In order to service a character device command, we add special requests to
+ * the tail of our block device request queue and wait for their completion.
+ */
+
+enum {
+   REQ_IDETAPE_PC1 = (1 << 0), /* packet command (first stage) */
+   REQ_IDETAPE_PC2 = (1 << 1), /* packet command (second stage) */
+   REQ_IDETAPE_READ= (1 << 2),
+   REQ_IDETAPE_WRITE   = (1 << 3),
+   REQ_IDETAPE_READ_BUFFER = (1 << 4),
+};
+
+/* Error codes returned in rq->errors to the higher part of the driver. */
+#defineIDETAPE_ERROR_GENERAL   101
+#defineIDETAPE_ERROR_FILEMARK  102
+#defineIDETAPE_ERROR_EOD   103
+
+/* Structures related to the SELECT SENSE / MODE SENSE packet commands. */
+#define IDETAPE_BLOCK_DESCRIPTOR   0
+#defineIDETAPE_CAPABILITIES_PAGE   0x2a
+
+
 /* A pipeline stage. */
 typedef struct idetape_stage_s {
struct request rq;  /* The corresponding request */
@@ -445,68 +508,6 @@ static void ide_tape_put(struct ide_tape_obj *tape)
mutex_unlock(&idetape_ref_mutex);
 }
 
-/* Tape door status */
-#define DOOR_UNLOCKED  0
-#define DOOR_LOCKED1
-#define DOOR_EXPLICITLY_LOCKED 2
-
-/* Tape flag bits values. */
-enum {
-   IDETAPE_FL_IGNORE_DSC   = (1 << 0),
-   /* 0 When the tape position is unknown */
-   IDETAPE_FL_ADDRESS_VALID= (1 << 1),
-   /* Device already opened */
-   IDETAPE_FL_BUSY = (1 << 2),
-   /* Error detected in a pipeline stage */
-   IDETAPE_FL_PIPELINE_ERR = (1 << 3),
-   /* Attempt to auto-detect the current user block size */
-   IDETAPE_FL_DETECT_BS= (1 << 4),
-   /* Currently on a filemark */
-   IDETAPE_FL_FILEMARK = (1 << 5),
-   /* DRQ interrupt device */
-   IDETAPE_FL_DRQ_INTERRUPT= (1 << 6),
-   /* pipeline active */
-   IDETAPE_FL_PIPELINE_ACTIVE  = (1 << 7),
-   /* 0 = no tape is loaded, so we don't rewind after ejecting */
-   IDETAPE_FL_MEDIUM_PRESENT   = (1 << 8),
-};
-
-/* A define for the READ BUFFER command */
-#define IDETAPE_RETRIEVE_FAULTY_BLOCK  6
-
-/* Some defines for the SPACE command */
-#define IDETAPE_SPACE_OVER_FILEMARK1
-#define IDETAPE_SPACE_TO_EOD   3
-
-/* Some defines for the LOAD UNLOAD command */
-#define IDETAPE_LU_LOAD_MASK   1
-#define IDETAPE_LU_RETENSION_MASK  2
-#define IDETAPE_LU_EOT_MASK4
-
-/*
- * Special requests for our block device strategy routine.
- *
- * In order to service a character device command, we add special requests to
- * the tail of our block device request queue and wait for their completion.
- */
-
-enum {
-   REQ_IDETAPE_PC1 = (1 << 0), /* packet command (first stage) */
-   REQ_IDETAPE_PC2 = (1 << 1), /* packet command (second stage) */
-   REQ_IDETAPE_READ= (1 << 2),
-

Re: ide-tape redux (was: Re:)

2008-02-05 Thread Borislav Petkov
On Tue, Feb 05, 2008 at 02:20:22AM +0100, Bartlomiej Zolnierkiewicz wrote:

[...]

> w.r.t. #11 ide-tape uses char devices and supports DSC so it is not as obvious
> as in ide-floppy case that all atomic bitops can be just removed (extra audit
> and some time -mm are required) so please resync/resubmit

Ok, here's what i think we should do here: There are two flags that handle DSC:
PC_FL_WAIT_FOR_DSC and IDETAPE_FL_IGNORE_DSC. The first one is per pc and is 
set in
all the packet command init functions ..create_bla_cmd() after their callers 
have
created a pc on the stack and reached its ptr down for initialization. This case
is carefree since the bit will be tested first in the interrupt handler and this
happens only after the pc is queued (ide_do_drive_cmd()) into the request 
buffer.

The other flag, IDETAPE_FL_IGNORE_DSC, is polled for in the request handler and
can be set when a pc is being retried and we should leave only those atomic
tests intact, imho, but i'm definitely gonna need a second opinion here.

---

commit 1ed8ae92249d5dff7af4ee88710ea08ff3f3356f
Author: Borislav Petkov <[EMAIL PROTECTED]>
Date:   Tue Feb 5 08:05:35 2008 +0100

ide-tape: remove atomic test/set macros

Also, since the driver supports DSC, leave the atomic tests
for the IDETAPE_FL_IGNORE_DSC bit untouched because this is polled
for in the request handler and can be set in the interrupt
handler through idetape_retry_pc() after enabling interrupts.

Finally, remove flag IDETAPE_READ_ERROR since it is unused.

Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]>

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index e59e49e..9455ce4 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -206,24 +206,24 @@ typedef struct idetape_packet_command_s {
/* Temporary buffer */
u8 pc_buffer[IDETAPE_PC_BUFFER_SIZE];
/* Status/Action bit flags: long for set_bit */
-   unsigned long flags;
+   unsigned int flags;
 } idetape_pc_t;
 
-/*
- * Packet command flag bits.
- */
-/* Set when an error is considered normal - We won't retry */
-#definePC_ABORT0
-/* 1 When polling for DSC on a media access command */
-#define PC_WAIT_FOR_DSC1
-/* 1 when we prefer to use DMA if possible */
-#define PC_DMA_RECOMMENDED 2
-/* 1 while DMA in progress */
-#definePC_DMA_IN_PROGRESS  3
-/* 1 when encountered problem during DMA */
-#definePC_DMA_ERROR4
-/* Data direction */
-#definePC_WRITING  5
+/* Packet command flag bits. */
+enum {
+   /* Set when an error is considered normal - We won't retry */
+   PC_FL_ABORT = (1 << 0),
+   /* 1 When polling for DSC on a media access command */
+   PC_FL_WAIT_FOR_DSC  = (1 << 1),
+   /* 1 when we prefer to use DMA if possible */
+   PC_FL_DMA_RECOMMENDED   = (1 << 2),
+   /* 1 while DMA in progress */
+   PC_FL_DMA_IN_PROGRESS   = (1 << 3),
+   /* 1 when encountered problem during DMA */
+   PC_FL_DMA_ERROR = (1 << 4),
+   /* Data direction */
+   PC_FL_WRITING   = (1 << 5),
+};
 
 /* A pipeline stage. */
 typedef struct idetape_stage_s {
@@ -357,8 +357,7 @@ typedef struct ide_tape_obj {
/* Wasted space in each stage */
int excess_bh_size;
 
-   /* Status/Action flags: long for set_bit */
-   unsigned long flags;
+   unsigned int flags;
/* protects the ide-tape queue */
spinlock_t lock;
 
@@ -451,20 +450,26 @@ static void ide_tape_put(struct ide_tape_obj *tape)
 #define DOOR_LOCKED1
 #define DOOR_EXPLICITLY_LOCKED 2
 
-/*
- * Tape flag bits values.
- */
-#define IDETAPE_IGNORE_DSC 0
-#define IDETAPE_ADDRESS_VALID  1   /* 0 When the tape position is 
unknown */
-#define IDETAPE_BUSY   2   /* Device already opened */
-#define IDETAPE_PIPELINE_ERROR 3   /* Error detected in a pipeline 
stage */
-#define IDETAPE_DETECT_BS  4   /* Attempt to auto-detect the 
current user block size */
-#define IDETAPE_FILEMARK   5   /* Currently on a filemark */
-#define IDETAPE_DRQ_INTERRUPT  6   /* DRQ interrupt device */
-#define IDETAPE_READ_ERROR 7
-#define IDETAPE_PIPELINE_ACTIVE8   /* pipeline active */
-/* 0 = no tape is loaded, so we don't rewind after ejecting */
-#define IDETAPE_MEDIUM_PRESENT 9
+/* Tape flag bits values. */
+enum {
+   IDETAPE_FL_IGNORE_DSC   = (1 << 0),
+   /* 0 When the tape position is unknown */
+   IDETAPE_FL_ADDRESS_VALID= (1 << 1),
+   /* Device already opened */
+   IDETAPE_FL_BUSY = (1 << 2),
+   /* Error detected in a pipeline stage */
+   IDETAPE_FL_PIPELINE_ERR = (1 << 3),
+   /* Attempt to auto-detect the 

Re: [PATCH] enclosure: add support for enclosure services

2008-02-05 Thread James Bottomley
On Tue, 2008-02-05 at 16:12 -0800, Andrew Morton wrote:
> On Sun, 03 Feb 2008 18:16:51 -0600
> James Bottomley <[EMAIL PROTECTED]> wrote:
> 
> > 
> > From: James Bottomley <[EMAIL PROTECTED]>
> > Date: Sun, 3 Feb 2008 15:40:56 -0600
> > Subject: [SCSI] enclosure: add support for enclosure services
> > 
> > The enclosure misc device is really just a library providing sysfs
> > support for physical enclosure devices and their components.
> > 
> 
> Thanks for sending it out for review.
> 
> > +struct enclosure_device *enclosure_find(struct device *dev)
> > +{
> > +   struct enclosure_device *edev = NULL;
> > +
> > +   mutex_lock(&container_list_lock);
> > +   list_for_each_entry(edev, &container_list, node) {
> > +   if (edev->cdev.dev == dev) {
> > +   mutex_unlock(&container_list_lock);
> > +   return edev;
> > +   }
> > +   }
> > +   mutex_unlock(&container_list_lock);
> > +
> > +   return NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(enclosure_find);
> 
> This looks a little odd.  We don't take a ref on the object after looking
> it up, so what prevents some other thread of control from freeing or
> otherwise altering the returned object while the caller is playing with it?

The use case is for enclosure destruction, so the free should never
happen, but I take the point; I've added a class_device_get().

> > +/**
> > + * enclosure_for_each_device - calls a function for each enclosure
> > + * @fn:the function to call
> > + * @data:  the data to pass to each call
> > + *
> > + * Loops over all the enclosures calling the function.
> > + *
> > + * Note, this function uses a mutex which will be held across calls to
> > + * @fn, so it must have user context, and @fn should not sleep or
> 
> Probably "non atomic context" would be more accurate.
> 
> fn() actually _can_ sleep.

"should" to me means you don't have to do this but ought to. I'll add a
may (but should not).

> > +   if (!cb) {
> > +   kfree(edev);
> > +   return ERR_PTR(-EINVAL);
> > +   }
> 
> It would be less fuss if this were to test cb before doing the kzalloc().
> 
> Can cb==NULL actually and legitimately happen?

Not really ... I'll make it a BUG_ON.

> > +void enclosure_unregister(struct enclosure_device *edev)
> > +{
> > +   int i;
> > +
> > +   if (!edev)
> > +   return;
> 
> Is this legal?

No ... it'll oops on the null deref later ... I'll remove this.

> > +   mutex_lock(&container_list_lock);
> > +   list_del(&edev->node);
> > +   mutex_unlock(&container_list_lock);
> 
> See, right now, someone who found this enclosure_device via
> enclosure_find() could still be playing with it?

Yes, fixed.

> > +   if (!edev || number >= edev->components)
> > +   return ERR_PTR(-EINVAL);
> 
> Is !edev possible and legitimate?

It shouldn't be, no ... I can remove it.

> > +   snprintf(cdev->class_id, BUS_ID_SIZE, "%d", number);
> 
> %u :)

Nitpicker!

> > +   return snprintf(buf, 40, "%d\n", edev->components);
> > +}
> 
> "40"?

I just followed precedence ;-P

There doesn't seem to be a define for this maximum length, so 40 is the
most commonly picked constant.

> > +static char *enclosure_type [] = {
> > +   [ENCLOSURE_COMPONENT_DEVICE] = "device",
> > +   [ENCLOSURE_COMPONENT_ARRAY_DEVICE] = "array device",
> > +};
> 
> One could play with const here, if sufficiently keen.

One will try to summon up the enthusiasm.

> > +static ssize_t set_component_fault(struct class_device *cdev, const char 
> > *buf,
> > +  size_t count)
> > +{
> > +   struct enclosure_device *edev = to_enclosure_device(cdev->parent);
> > +   struct enclosure_component *ecomp = to_enclosure_component(cdev);
> > +   int val = simple_strtoul(buf, NULL, 0);
> 
> hrm, we do this conversion about 1e99 times in the kernel and we have to go
> and pass three args where only one was needed. katoi()?

Yes ... I'll add it to the todo list.

> > +   for (i = 0; enclosure_status[i]; i++) {
> > +   if (strncmp(buf, enclosure_status[i],
> > +   strlen(enclosure_status[i])) == 0 &&
> > +   buf[strlen(enclosure_status[i])] == '\n')
> > +   break;
> > +   }
> 
> So if an application does
> 
>   write(fd, "foo", 3)
> 
> it won't work?  Thye have to do
> 
>   write(fd, "foo\n", 4)
> 
> ?

No ... it's designed for echo; however, I'll add a check for '\0' which
will catch the write case.

> > +#define to_enclosure_device(x) container_of((x), struct enclosure_device, 
> > cdev)
> > +#define to_enclosure_component(x) container_of((x), struct 
> > enclosure_component, cdev)
> 
> These could be C functions...

OK ... I was just following precedence again, but I can make them
inlines.

> Nice looking driver.

Thanks,

James

---

Here's the incremental diff.

diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 42e6e43..6fcb0e9 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -39,7 +39,8

Re: Problem burning DVDs with Marvell 88SE6121 on pata_marvell

2008-02-05 Thread Mike Hokenson


On Tuesday, February 05, 2008 at 03:21PM, Andrew Morton wrote:


(added linux-ide)


thanks


On Sat, 2 Feb 2008 16:30:04 -0600
Mike Hokenson <[EMAIL PROTECTED]> wrote:


I recently put together a new system with a MSI P35 PLATINUM and although
reading from data CDs, DVDs, and watching DVD movies is working fine, DVD
burning isn't. MSI's manual says "1 IDE port by Marvell 88SE6111", but
lspci says it's a 88SE6121. I have two DVD burners, a SONY DRU-510A and
a DRU-820A. They were both working fine with TDK DVD+R media on my ASUS
K8V SE DELUXE (VIA IDE controller) prior to the upgrade.

Here's what I see with dvd+rw-tools version 7.0-9:

sh# dvd+rw-mediainfo /dev/dvd > /dev/null  # this is blank media
:-[ GET CURRENT PERFORMACE failed with SK=5h/ASC=24h/ACQ=00h]: Input/output 
error
:-[ READ TOC failed with SK=5h/ASC=24h/ACQ=00h]: Input/output error

sh# growisofs -dvd-compat -speed=2.4 -Z /dev/dvd -rJ -joliet-long -quiet "burn"
Executing 'genisoimage -rJ -joliet-long -quiet burn | builtin_dd of=/dev/dvd 
obs=32k seek=0'
/dev/dvd: "Current Write Speed" is 2.5x1352KBps.
:-[ [EMAIL PROTECTED] failed with SK=3h/ASC=0Ch/ACQ=00h]: Input/output error
:-( write failed: Input/output error
/dev/dvd: flushing cache
/dev/dvd: closing track
:-[ CLOSE TRACK failed with SK=5h/ASC=30h/ACQ=05h]: Wrong medium type
/dev/dvd: closing disc
:-[ CLOSE DISC failed with SK=5h/ASC=30h/ACQ=05h]: Wrong medium type

sh# dvd+rw-mediainfo /dev/dvd > /dev/null
:-[ READ TRACK INFORMATION failed with SK=3h/ASC=11h/ACQ=05h]: Input/output 
error


(this is 2.6.24)

If nothing happens with this report in the next few days, please create an
entry at bugzilla.kernel.org so we can keep an eye on it, thanks.

Trying older kernels might be interesting, find out if it's a regression or
if it was always this way.


2.6.13.14 shows the same problems and 2.6.22 doesn't see the controller.

It looks like most of the changes happened betwen it's introduction in
2.6.20 (v0.1.1) and 2.6.22 (v0.1.4), with some minor updates for internal
changes after that...

Mike
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Palmchip BK3710 IDE driver

2008-02-05 Thread Bartlomiej Zolnierkiewicz
On Tuesday 05 February 2008, Anton Salnikov wrote:
> I've tested the driver on 2.6.24.
> 
> I've made the changes you asked for. But when I tested them on 
> arm/mach-davinci 

Thanks!

> architecture on the current Linus' git there was link error undefined 
> reference 
> to symbol ide_hwif_setup_dma().
> However on x86_64 architecture it compiles well without any warnings with 
> respect to link error of undefined clk_get() clk_enable() clk_get_rate(), 
> which 
> are not present in this architecture.

ide_hwif_setup_dma() comes from setup-pci.c which depends on BLK_DEV_IDEPCI.
palm_bk3710 host driver selects BLK_DEV_IDEDMA_PCI which should also select
BLK_DEV_IDEPCI but PCI doesn't even seem to be supported by this ARM platform.

I (hopefully) workarounded the issue by adding an additional #ifdef to
 but it is a unmaintanable and ugly hack:

Index: b/include/linux/ide.h
===
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1014,7 +1014,8 @@ extern int __ide_pci_register_driver(str
 void ide_pci_setup_ports(struct pci_dev *, const struct ide_port_info *, int, 
u8 *);
 void ide_setup_pci_noise(struct pci_dev *, const struct ide_port_info *);
 
-#ifdef CONFIG_BLK_DEV_IDEDMA_PCI
+/* FIXME: palm_bk3710 uses BLK_DEV_IDEDMA_PCI without BLK_DEV_IDEPCI! */
+#if defined(CONFIG_BLK_DEV_IDEPCI) && defined(CONFIG_BLK_DEV_IDEDMA_PCI)
 void ide_hwif_setup_dma(ide_hwif_t *, const struct ide_port_info *);
 #else
 static inline void ide_hwif_setup_dma(ide_hwif_t *hwif,


Anton/Sergei: please look into fixing it properly - we probably just need to
to have a new config option (CONFIG_IDE_SFF_DMA sounds neat) to be selected by
both BLK_DEV_{IDEDMA_PCI,PALMCHIP_BK3710} and to replace BLK_DEV_IDEDMA_PCI
#ifdefs in ide-dma.c.

> > I've just noticed that there is no cable detection for UDMA modes > UDMA2?
> For the present controller in documentation: Cable Select (CSEL) - 
> unsupported 
> IDE controller signal. This signal is not necessary because an 80-conductor 
> cable must be used with the DM644x.
> 
> Signed-off-by: Anton Salnikov <[EMAIL PROTECTED]>

Minor issue: the original patch description got lost somehow so I just used
the one from the previous version.

> ---
> 
>  drivers/ide/Kconfig   |9 
>  drivers/ide/arm/Makefile  |1 
>  drivers/ide/arm/palm_bk3710.c |  420 
> ++
>  drivers/ide/ide-proc.c|1 
>  include/linux/ide.h   |2 
>  5 files changed, 432 insertions(+), 1 deletion(-)
> 
> Index: 2.6.25.ide/drivers/ide/Kconfig
> ===
> --- 2.6.25.ide.orig/drivers/ide/Kconfig
> +++ 2.6.25.ide/drivers/ide/Kconfig
> @@ -1009,6 +1009,15 @@ config BLK_DEV_Q40IDE
> normally be on; disable it only if you are running a custom hard
> drive subsystem through an expansion card.
>  
> +config BLK_DEV_PALMCHIP_BK3710
> + tristate "Palmchip bk3710 IDE controller support"
> + depends on ARCH_DAVINCI
> + select BLK_DEV_IDEDMA_PCI
> + help
> +   Say Y here if you want to support the onchip IDE controller on the
> +   TI DaVinci SoC
> +
> +
>  config BLK_DEV_MPC8xx_IDE
>   tristate "MPC8xx IDE support"
>   depends on 8xx && (LWMON || IVMS8 || IVML24 || TQM8xxL) && IDE=y && 
> BLK_DEV_IDE=y && !PPC_MERGE
> Index: 2.6.25.ide/drivers/ide/arm/Makefile
> ===
> --- 2.6.25.ide.orig/drivers/ide/arm/Makefile
> +++ 2.6.25.ide/drivers/ide/arm/Makefile
> @@ -2,6 +2,7 @@
>  obj-$(CONFIG_BLK_DEV_IDE_ICSIDE) += icside.o
>  obj-$(CONFIG_BLK_DEV_IDE_RAPIDE) += rapide.o
>  obj-$(CONFIG_BLK_DEV_IDE_BAST)   += bast-ide.o
> +obj-$(CONFIG_BLK_DEV_PALMCHIP_BK3710)+= palm_bk3710.o
>  
>  ifeq ($(CONFIG_IDE_ARM), m)
>   obj-m += ide_arm.o
> Index: 2.6.25.ide/drivers/ide/arm/palm_bk3710.c
> ===
> --- /dev/null
> +++ 2.6.25.ide/drivers/ide/arm/palm_bk3710.c

[...]

> +/*static inline u8 hwif_read8(u32 base, u32 reg)
> +{
> + return readb(base + reg);
> +}
> +
> +static inline u16 hwif_read16(u32 base, u32 reg)
> +{
> + return readw(base + reg);
> +}
> +
> +static inline void hwif_write16(u32 base, u16 val, u32 reg)
> +{
> + writew(val, base + reg);
> +}
> +
> +static inline u32 hwif_read32(u32 base, u32 reg)
> +{
> + return readl(base + reg);
> +}
> +
> +static inline void hwif_write32(u32 base, u32 val, u32 reg)
> +{
> + writel(val, base + reg);
> +}*/

I removed this chunk while merging the patch (no need for a dead code)

also added "Reviewed-by:" tags crediting Alan & Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ide-pci-generic: kill the unused ifdef/endif/MODULE code

2008-02-05 Thread Bartlomiej Zolnierkiewicz
On Saturday 02 February 2008, Denis Cheng wrote:
> with module_param macro, the __setup code can be killed now:
>   const __setup("all-generic-ide", ide_generic_all_on);
> 
> and the module name "generic.ko" is not descriptive to its functionality,
> can be changed in Makefile, the "ide-pci-generic.ko" is better.
> 
> the ide-pci-generic.all-generic-ide parameter also documented
> in Documentation/kernel-parameters.txt
> 
> Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>

applied, thanks

PS the other patch will take same more time to review
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [git patches] IDE updates part #4

2008-02-05 Thread Bartlomiej Zolnierkiewicz

Hi Linus,

On Tuesday 05 February 2008, Linus Torvalds wrote:
> 
> On Sat, 2 Feb 2008, Bartlomiej Zolnierkiewicz wrote:
> > 
> > * next part of IDE probing code re-organization saga
> >   (that would be me)
> 
> This seems to cause very irritating and bogus messages for me:
> 
>   Probing IDE interface ide0...
>   Probing IDE interface ide1...
>   ide2: I/O resource 0x0-0x7 not free.
>   ide2: ports already in use, skipping probe
>   ide3: I/O resource 0x0-0x7 not free.
>   ide3: ports already in use, skipping probe
>   ide4: I/O resource 0x0-0x7 not free.
>   ide4: ports already in use, skipping probe
>   ide5: I/O resource 0x0-0x7 not free.
>   ide5: ports already in use, skipping probe
>   ide6: I/O resource 0x0-0x7 not free.
>   ide6: ports already in use, skipping probe
>   ide7: I/O resource 0x0-0x7 not free.
>   ide7: ports already in use, skipping probe
>   ide8: I/O resource 0x0-0x7 not free.
>   ide8: ports already in use, skipping probe
>   ide9: I/O resource 0x0-0x7 not free.
>   ide9: ports already in use, skipping probe
> 
> and that's just totally bogus. It shouldn't even request that region, 
> since it's not been allocated!
> 
> So that "ide_device_add_all()" is missing some checks. Should it check the 
> probe[] array like ideprobe_init() used to, or what?

This is ide-generic problem exhibited by recent ide_device_add_all() changes.
Fix below (it works for me) - you may merge the patch as it is or wait an hour
or so for the next IDE tree pull request.

Also sorry for the issue in the first place (it turned out that it slipped
my testing because I has been running with IDE_MAX_HWIFS=2 for some time :).


From: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]>
Subject: [PATCH] ide-generic: probing bugfix

On Tuesday 05 February 2008, Linus Torvalds wrote:
> 
> On Sat, 2 Feb 2008, Bartlomiej Zolnierkiewicz wrote:
> > 
> > * next part of IDE probing code re-organization saga
> >   (that would be me)
> 
> This seems to cause very irritating and bogus messages for me:
> 
>   Probing IDE interface ide0...
>   Probing IDE interface ide1...
>   ide2: I/O resource 0x0-0x7 not free.
>   ide2: ports already in use, skipping probe
>   ide3: I/O resource 0x0-0x7 not free.
>   ide3: ports already in use, skipping probe
>   ide4: I/O resource 0x0-0x7 not free.
>   ide4: ports already in use, skipping probe
>   ide5: I/O resource 0x0-0x7 not free.
>   ide5: ports already in use, skipping probe
>   ide6: I/O resource 0x0-0x7 not free.
>   ide6: ports already in use, skipping probe
>   ide7: I/O resource 0x0-0x7 not free.
>   ide7: ports already in use, skipping probe
>   ide8: I/O resource 0x0-0x7 not free.
>   ide8: ports already in use, skipping probe
>   ide9: I/O resource 0x0-0x7 not free.
>   ide9: ports already in use, skipping probe
> 
> and that's just totally bogus. It shouldn't even request that region, 
> since it's not been allocated!

The commit 139ddfcab50e5eabcc88341c8743a990ac1be6a2 ("ide: move handling of
I/O resources out of ide_probe_port()") changed the ordering of hwif->noprobe
check vs ide_hwif_request_regions() call (so that we now reserve I/O regions
before checking for hwif->noprobe).  However ide-generic host driver depended
on hwif->noprobe to be set for skipping probing of empty ide_hwifs[] slots.

Fix it by passing only indexes of non-empty slots to ide_device_add_all()
from ide_generic_init().

Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]>
---
 drivers/ide/ide-generic.c |   10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

Index: b/drivers/ide/ide-generic.c
===
--- a/drivers/ide/ide-generic.c
+++ b/drivers/ide/ide-generic.c
@@ -20,8 +20,14 @@ static int __init ide_generic_init(void)
if (ide_hwifs[0].io_ports[IDE_DATA_OFFSET])
ide_get_lock(NULL, NULL); /* for atari only */
 
-   for (i = 0; i < MAX_HWIFS; i++)
-   idx[i] = ide_hwifs[i].present ? 0xff : i;
+   for (i = 0; i < MAX_HWIFS; i++) {
+   ide_hwif_t *hwif = &ide_hwifs[i];
+
+   if (hwif->io_ports[IDE_DATA_OFFSET] && !hwif->present)
+   idx[i] = i;
+   else
+   idx[i] = 0xff;
+   }
 
ide_device_add_all(idx, NULL);
 
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] enclosure: add support for enclosure services

2008-02-05 Thread Andrew Morton
On Sun, 03 Feb 2008 18:16:51 -0600
James Bottomley <[EMAIL PROTECTED]> wrote:

> 
> From: James Bottomley <[EMAIL PROTECTED]>
> Date: Sun, 3 Feb 2008 15:40:56 -0600
> Subject: [SCSI] enclosure: add support for enclosure services
> 
> The enclosure misc device is really just a library providing sysfs
> support for physical enclosure devices and their components.
> 

Thanks for sending it out for review.

> +struct enclosure_device *enclosure_find(struct device *dev)
> +{
> + struct enclosure_device *edev = NULL;
> +
> + mutex_lock(&container_list_lock);
> + list_for_each_entry(edev, &container_list, node) {
> + if (edev->cdev.dev == dev) {
> + mutex_unlock(&container_list_lock);
> + return edev;
> + }
> + }
> + mutex_unlock(&container_list_lock);
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(enclosure_find);

This looks a little odd.  We don't take a ref on the object after looking
it up, so what prevents some other thread of control from freeing or
otherwise altering the returned object while the caller is playing with it?

> +/**
> + * enclosure_for_each_device - calls a function for each enclosure
> + * @fn:  the function to call
> + * @data:the data to pass to each call
> + *
> + * Loops over all the enclosures calling the function.
> + *
> + * Note, this function uses a mutex which will be held across calls to
> + * @fn, so it must have user context, and @fn should not sleep or

Probably "non atomic context" would be more accurate.

fn() actually _can_ sleep.

> + * otherwise cause the mutex to be held for indefinite periods
> + */
> +int enclosure_for_each_device(int (*fn)(struct enclosure_device *, void *),
> +   void *data)
> +{
> + int error = 0;
> + struct enclosure_device *edev;
> +
> + mutex_lock(&container_list_lock);
> + list_for_each_entry(edev, &container_list, node) {
> + error = fn(edev, data);
> + if (error)
> + break;
> + }
> + mutex_unlock(&container_list_lock);
> +
> + return error;
> +}
> +EXPORT_SYMBOL_GPL(enclosure_for_each_device);
> +
> +/**
> + * enclosure_register - register device as an enclosure
> + *
> + * @dev: device containing the enclosure
> + * @components:  number of components in the enclosure
> + *
> + * This sets up the device for being an enclosure.  Note that @dev does
> + * not have to be a dedicated enclosure device.  It may be some other type
> + * of device that additionally responds to enclosure services
> + */
> +struct enclosure_device *
> +enclosure_register(struct device *dev, const char *name, int components,
> +struct enclosure_component_callbacks *cb)
> +{
> + struct enclosure_device *edev =
> + kzalloc(sizeof(struct enclosure_device) +
> + sizeof(struct enclosure_component)*components,
> + GFP_KERNEL);
> + int err, i;
> +
> + if (!edev)
> + return ERR_PTR(-ENOMEM);
> +
> + if (!cb) {
> + kfree(edev);
> + return ERR_PTR(-EINVAL);
> + }

It would be less fuss if this were to test cb before doing the kzalloc().

Can cb==NULL actually and legitimately happen?

> + edev->components = components;
> +
> + edev->cdev.class = &enclosure_class;
> + edev->cdev.dev = get_device(dev);
> + edev->cb = cb;
> + snprintf(edev->cdev.class_id, BUS_ID_SIZE, "%s", name);
> + err = class_device_register(&edev->cdev);
> + if (err)
> + goto err;
> +
> + for (i = 0; i < components; i++)
> + edev->component[i].number = -1;
> +
> + mutex_lock(&container_list_lock);
> + list_add_tail(&edev->node, &container_list);
> + mutex_unlock(&container_list_lock);
> +
> + return edev;
> +
> + err:
> + put_device(edev->cdev.dev);
> + kfree(edev);
> + return ERR_PTR(err);
> +}
> +EXPORT_SYMBOL_GPL(enclosure_register);
> +
> +static struct enclosure_component_callbacks enclosure_null_callbacks;
> +
> +/**
> + * enclosure_unregister - remove an enclosure
> + *
> + * @edev:the registered enclosure to remove;
> + */
> +void enclosure_unregister(struct enclosure_device *edev)
> +{
> + int i;
> +
> + if (!edev)
> + return;

Is this legal?

> + mutex_lock(&container_list_lock);
> + list_del(&edev->node);
> + mutex_unlock(&container_list_lock);

See, right now, someone who found this enclosure_device via
enclosure_find() could still be playing with it?

> + for (i = 0; i < edev->components; i++)
> + if (edev->component[i].number != -1)
> + class_device_unregister(&edev->component[i].cdev);
> +
> + /* prevent any callbacks into service user */
> + edev->cb = &enclosure_null_callbacks;
> + class_device_unregister(&edev->cdev);
> +}
> +EXPORT_SYMBOL_GPL(enclosure_unregister);
> +
> +/**
> + * enclosure_component_

Re: Problem burning DVDs with Marvell 88SE6121 on pata_marvell

2008-02-05 Thread Andrew Morton

(added linux-ide)

On Sat, 2 Feb 2008 16:30:04 -0600
Mike Hokenson <[EMAIL PROTECTED]> wrote:

> I recently put together a new system with a MSI P35 PLATINUM and although
> reading from data CDs, DVDs, and watching DVD movies is working fine, DVD
> burning isn't. MSI's manual says "1 IDE port by Marvell 88SE6111", but
> lspci says it's a 88SE6121. I have two DVD burners, a SONY DRU-510A and
> a DRU-820A. They were both working fine with TDK DVD+R media on my ASUS
> K8V SE DELUXE (VIA IDE controller) prior to the upgrade.
> 
> Here's what I see with dvd+rw-tools version 7.0-9:
> 
> sh# dvd+rw-mediainfo /dev/dvd > /dev/null  # this is blank media
> :-[ GET CURRENT PERFORMACE failed with SK=5h/ASC=24h/ACQ=00h]: Input/output 
> error
> :-[ READ TOC failed with SK=5h/ASC=24h/ACQ=00h]: Input/output error
> 
> sh# growisofs -dvd-compat -speed=2.4 -Z /dev/dvd -rJ -joliet-long -quiet 
> "burn"
> Executing 'genisoimage -rJ -joliet-long -quiet burn | builtin_dd of=/dev/dvd 
> obs=32k seek=0'
> /dev/dvd: "Current Write Speed" is 2.5x1352KBps.
> :-[ [EMAIL PROTECTED] failed with SK=3h/ASC=0Ch/ACQ=00h]: Input/output error
> :-( write failed: Input/output error
> /dev/dvd: flushing cache
> /dev/dvd: closing track
> :-[ CLOSE TRACK failed with SK=5h/ASC=30h/ACQ=05h]: Wrong medium type
> /dev/dvd: closing disc
> :-[ CLOSE DISC failed with SK=5h/ASC=30h/ACQ=05h]: Wrong medium type
> 
> sh# dvd+rw-mediainfo /dev/dvd > /dev/null
> :-[ READ TRACK INFORMATION failed with SK=3h/ASC=11h/ACQ=05h]: Input/output 
> error

(this is 2.6.24)

If nothing happens with this report in the next few days, please create an
entry at bugzilla.kernel.org so we can keep an eye on it, thanks.

Trying older kernels might be interesting, find out if it's a regression or
if it was always this way.


> And from cdrecord version 1.1.6-1:
> 
> sh# mkisofs -rJ -joliet-long -quiet "burn" | cdrecord -v dev=/dev/dvd -
> wodim: No write mode specified.
> wodim: Asuming -tao mode.
> wodim: Future versions of wodim may have different drive dependent defaults.
> TOC Type: 1 = CD-ROM
> scsidev: '/dev/dvd'
> devname: '/dev/dvd'
> scsibus: -2 target: -2 lun: -2
> Linux sg driver version: 3.5.27
> Wodim version: 1.1.6
> SCSI buffer size: 64512
> Device type: Removable CD-ROM
> Version: 5
> Response Format: 2
> Capabilities   :
> Vendor_info: 'SONY'
> Identification : 'DVD RW DRU-820A '
> Revision   : '1.0c'
> Device seems to be: Generic mmc2 DVD-R/DVD-RW.
> Current: 0x0009 (CD-R)
> Profile: 0x002B (DVD+R/DL)
> Profile: 0x001B (DVD+R)
> Profile: 0x001A (DVD+RW)
> Profile: 0x0016 (DVD-R/DL layer jump recording)
> Profile: 0x0015 (DVD-R/DL sequential recording)
> Profile: 0x0014 (DVD-RW sequential recording)
> Profile: 0x0013 (DVD-RW restricted overwrite)
> Profile: 0x0012 (DVD-RAM)
> Profile: 0x0011 (DVD-R sequential recording)
> Profile: 0x0010 (DVD-ROM)
> Profile: 0x000A (CD-RW)
> Profile: 0x0009 (CD-R) (current)
> Profile: 0x0008 (CD-ROM)
> Profile: 0x0002 (Removable disk)
> Using generic SCSI-3/mmc   CD-R/CD-RW driver (mmc_cdr).
> Driver flags   : MMC-3 SWABAUDIO BURNFREE
> Supported modes: TAO PACKET SAO SAO/R96P SAO/R96R RAW/R16 RAW/R96P RAW/R96R
> Drive buf size : 1114112 = 1088 KB
> Beginning DMA speed test. Set CDR_NODMATEST environment variable if device
> communication breaks or freezes immediately after that.
> FIFO size  : 12582912 = 12288 KB
> Track 01: data  unknown length
> Total size:0 MB (00:00.00) = 0 sectors
> Lout start:0 MB (00:02/00) = 0 sectors
> Current Secsize: 2048
> ATIP info from disk:
>   Indicated writing power: 4
>   Is not unrestricted
>   Is not erasable
>   Disk sub type: Medium Type A, high Beta category (A+) (3)
>   ATIP start of lead in:  -11849 (97:24/01)
>   ATIP start of lead out: 359847 (79:59/72)
> Disk type:Long strategy type (Cyanine, AZO or similar)
> Manuf. index: 25
> Manufacturer: Taiyo Yuden Company Limited
> wodim: WARNING: Total disk size unknown. Data may not fit on disk.
> Speed set to 7056 KB/s
> Starting to write CD/DVD at speed  40.0 in real TAO mode for single session.
> Last chance to quit, starting real write i   0 seconds. Operation starts.
> Waiting for reader process to fill input buffer ... input buffer ready.
> Performing OPC...
> Starting new track at sector: 0
> Track 01:1 MB written (fifo 100%) [buf  97%]   5.4x.Errno: 5 
> (Input/output error), write_g1 scsi sendcmd: no error
> CDB:  2A 00 00 00 03 64 00 00 1F 00
> status: 0x2 (CHECK CONDITION)
> Sense Bytes: 70 00 04 00 00 00 00 0A CF DC 00 00 08 03 00 00
> Sense Key: 0x4 Hardware Error, Segment 0
> Sense Code: 0x08 Qual 0x03 (logical unit communication crc error 
> (ultra-dma/32)) Fru 0x0
> Sense flags: Blk 0 (not valid)
> cmd finished after 30.186s timeout 40s
> 
> write track data: error after 1777664 bytes
> wodim: A write error occured.
> wodim: Please properly read the error message above.
> Writing  time:   46.572s
> Fixating...
> Errno: 5 (Input/output error), close track/session 

Re: [PATCH] enclosure: add support for enclosure services

2008-02-05 Thread Luben Tuikov
--- On Tue, 2/5/08, James Bottomley <[EMAIL PROTECTED]> wrote:
> > > Wrong ... we don't export non-SCSI devices as
> SCSI
> > > (with the single and
> > > rather annoying exception of ATA via SAT).
> > 
> > I didn't say you should do that.  I had already
> > mentioned that vendors export such controls
> > as either enclosure or processor type devices,
> > and this is why I told you that that is what
> > needs to be exported, which incidentally is
> > a device node of that type.
> > 
> > Without a common usage model already in the kernel
> > to abstract (e.g. sd for block device, since you
> brought
> > that up) your abstraction seems redundant and
> arbitrary.
> 
> Exactly, so the first patch in this series (a while ago
^^^

See last paragraph.

> now) was a
> common usage model abstraction of enclosures, and the
> second was an
> implementation in terms of SES.   I will do one in terms of
> SGPIO as
> well ... assuming I ever find a SGPIO enclosure ...

The vendor would've abstracted that away most commonly
using SES.

> 
> > Your kernel code already uses READ DIAGNOSTIC, etc,
> > and I'd rather leave that to user-space.
> 
> You can do it in user space as well.  It's just a bit
> difficult to get
> information out of a SES enclosure without using it, and
> getting some of
> the information is a requirement of the abstraction.

You missed my point.  Your abstraction is redundant and
arbitrary -- it is not based on any known, in-practice,
usage model, already in place that needs a better, common
way of doing XYZ, and therefore needs an abstraction.

   Luben

-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] enclosure: add support for enclosure services

2008-02-05 Thread James Bottomley
On Tue, 2008-02-05 at 11:33 -0800, Luben Tuikov wrote:
> > Wrong ... we don't export non-SCSI devices as SCSI
> > (with the single and
> > rather annoying exception of ATA via SAT).
> 
> I didn't say you should do that.  I had already
> mentioned that vendors export such controls
> as either enclosure or processor type devices,
> and this is why I told you that that is what
> needs to be exported, which incidentally is
> a device node of that type.
> 
> Without a common usage model already in the kernel
> to abstract (e.g. sd for block device, since you brought
> that up) your abstraction seems redundant and arbitrary.

Exactly, so the first patch in this series (a while ago now) was a
common usage model abstraction of enclosures, and the second was an
implementation in terms of SES.   I will do one in terms of SGPIO as
well ... assuming I ever find a SGPIO enclosure ...

> Your kernel code already uses READ DIAGNOSTIC, etc,
> and I'd rather leave that to user-space.

You can do it in user space as well.  It's just a bit difficult to get
information out of a SES enclosure without using it, and getting some of
the information is a requirement of the abstraction.

James


-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] enclosure: add support for enclosure services

2008-02-05 Thread Luben Tuikov
--- On Tue, 2/5/08, James Bottomley <[EMAIL PROTECTED]> wrote:
> > > > I guess the same could be said for STGT and
> SCST,
> > > right?
> > > 
> > > You mean both of their kernel pieces are modular?
> 
> > > That's correct.
> > 
> > No, you know very well what I mean.
> > 
> > By the same logic you're preaching to include your
> > solution part of the kernel, you can also apply to
> > SCST.
> 
> Ah, but it's not ... the current patch is merely
> exporting an interface.
> The debate in STGT vs SCST is not whether to export an
> interface but
> where to draw the line.

"draw the line" -- I see.
BTW, what is wrong with "exporting the interface"?

What is wrong if both implementations are in the kernel
and then let the users and distros decide which one
they like best and use more?  It'll not be the fist time
this has happened in the kernel.  Both are actively
maintained.

It seems highly arbitrary to say: "X is in the kernel, Y
is not. If you want Y, just forget about it and fix X."
Give people choice at config time.

This is off topic anyway.

> You could also argue in the same vein that sd is redundant
> because a
> filesystem could talk directly to the device via /dev/sgX
> (in fact OSD
> based filesystems already do this).

Yes, I've mentioned this thing before on this list.  Oh, maybe 3
years ago.  This is why I had wanted for transport protocols
to export ... (oh, let's not get this off topic).

(Apparently it takes 3 years...)

> The argument is true,
> but misses
> the bigger picture that the interfaces exported by sd are
> more portable
> (apply to non-SCSI block devices) and easier to use.

It isn't quite the same thing.  It's like comparing
apples to oranges.

> 
> > > > Yes, for which the transport layer,
> implements the
> > > > scsi device node for the SES device.  It
> doesn't
> > > really
> > > > matter if the SCSI commands sent to the SES
> device go
> > > > over SGPIO or FC or SAS or Bluetooth or I2C,
> etc, the
> > > > transport layer can implement that and
> present the
> > > > /dev/sgX node.
> > > 
> > > But it does matter if the enclosure device
> doesn't
> > > speak SCSI.
> > 
> > Enclosure management isn't as simple as you're
> > portraying it here.  The enclosure management
> > device speaks either SES or SAF-TE.  The transport
> > protocol to access it could be SGPIO or I2C or...
> 
> Look, just read the spec; SGPIO is a bus for driving
> enclosures ...

I thought Serial General Purpose Input Output
(SGPIO) was a method to serialize general purpose
IO signals.

> it
> doesn't require SES or SAF-TE or even any SCSI
> protocol.

That's true.  And this is why I mentioned a couple
of emails ago to simply export a sgpio device node *IF*
this is what is needed.  Of course devices that use SGPIO
abstract it away for their functional purpose, e.g.
enclosures, LED, etc, and provide a more general way to
control it -- highly hardware specific on one side.

Your abstraction currently deals with "SES" devices
and I'd rather leave that to user-space.  Alternatively,
which I presume is what you're thinking, a HW specific
core would be using your "abstraction" to provide
some unified access to raw features, and that "unified
access" isn't defined anywhere, and would likely not
be.  Alternatively that "unified access" is things
like SES and SAF-TE, which is what vendors prefer
to export, or they prefer to drive this directly
via other means.

That is, I fail to see the kernel bloat, for things
that aren't necessary in the kernel.

If you want your abstraction to fly, it first needs
a common usage model to abstract, and the latter is
missing _from the kernel_.

Unless I don't know the details and you've been
asked to implement this for a single vendor's HW solution.

> > >  SGPIO
> > > isn't a SCSI protocol ... it's a general
> purpose
> > > serial bus protocol.
> > > It's pretty simple and register based, but it
> might (or
> > > might not) be
> > > accessible via a SCSI bridge.
> > 
> > I see.  You've just discovered SGPIO -- good for
> you.
> > 
> > At any rate, I told you already that what is needed
> > is not what you've provided but a _device node_
> > exported by the kernel, either a processor or
> > enclosure type.
> 
> Wrong ... we don't export non-SCSI devices as SCSI
> (with the single and
> rather annoying exception of ATA via SAT).

I didn't say you should do that.  I had already
mentioned that vendors export such controls
as either enclosure or processor type devices,
and this is why I told you that that is what
needs to be exported, which incidentally is
a device node of that type.

Without a common usage model already in the kernel
to abstract (e.g. sd for block device, since you brought
that up) your abstraction seems redundant and arbitrary.

Your kernel code already uses READ DIAGNOSTIC, etc,
and I'd rather leave that to user-space.

   Luben

-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at 

Re: [PATCH] ide: another possible ide panic fix for blk-end-request

2008-02-05 Thread Kiyoshi Ueda
Hi Boris,

On Tue, 5 Feb 2008 07:01:57 +0100, Borislav Petkov wrote:
> On Mon, Feb 04, 2008 at 02:53:12PM -0500, Kiyoshi Ueda wrote:
> > Hi Jens, Bart, Boris,
> > 
> > I have reviewed all blk-end-request patches again to confirm whether
> > there are any similar problems with the last week's ide-cd panic:
> > http://lkml.org/lkml/2008/1/29/140
> > 
> > And I found a possible similar bug in ide-io change:
> > ide_end_drive_cmd() could be called for blk_pc_request() which could
> > have bios.
> 
> You mean ide_abort() and ide_error(), right?
> Because ide{-tape,-floppy,-scsi} do call already ide_end_request()
> for non-special rq's (!blk_special_request()), except ide-scsi
> filters also on !blk_sense_request().

That's right.

Thanks,
Kiyoshi Ueda
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3 2.6.24-git] ARM/RPC: Use HAVE_PATA_PLATFORM to select pata platform driver

2008-02-05 Thread Russell King - ARM Linux
On Sat, Feb 02, 2008 at 04:21:35PM +, Ben Dooks wrote:
> Use HAVE_PATA_PLATFORM for ARCH_RPC 
> 
> Cc: Linux ARM Kernel <[EMAIL PROTECTED]>
> Cc: Russell King <[EMAIL PROTECTED]>
> Signed-off-by: Ben Dooks <[EMAIL PROTECTED]>

Patch is fine.

Acked-by: Russell King <[EMAIL PROTECTED]>

Thanks.
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [git patches] IDE updates part #4

2008-02-05 Thread Linus Torvalds


On Sat, 2 Feb 2008, Bartlomiej Zolnierkiewicz wrote:
> 
> * next part of IDE probing code re-organization saga
>   (that would be me)

This seems to cause very irritating and bogus messages for me:

Probing IDE interface ide0...
Probing IDE interface ide1...
ide2: I/O resource 0x0-0x7 not free.
ide2: ports already in use, skipping probe
ide3: I/O resource 0x0-0x7 not free.
ide3: ports already in use, skipping probe
ide4: I/O resource 0x0-0x7 not free.
ide4: ports already in use, skipping probe
ide5: I/O resource 0x0-0x7 not free.
ide5: ports already in use, skipping probe
ide6: I/O resource 0x0-0x7 not free.
ide6: ports already in use, skipping probe
ide7: I/O resource 0x0-0x7 not free.
ide7: ports already in use, skipping probe
ide8: I/O resource 0x0-0x7 not free.
ide8: ports already in use, skipping probe
ide9: I/O resource 0x0-0x7 not free.
ide9: ports already in use, skipping probe

and that's just totally bogus. It shouldn't even request that region, 
since it's not been allocated!

So that "ide_device_add_all()" is missing some checks. Should it check the 
probe[] array like ideprobe_init() used to, or what?

Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: libata pm

2008-02-05 Thread [EMAIL PROTECTED]
Hi everybody,

it looks like this will be a never ending story...

>>> After I got the new PSU and the raid was in full sync without any
>>> error for 48h, I thought all problems were gone. Today the sata errors
>>>  reappeared and whenever the load is high enough I get the following:
>>>
I exchanged two (probably failing) of the eight harddrives with new ones.
All remaining disks have a good smart state and are fully readable when
the raid is not active. As long as I mount the filesystem on the raid
readonly there wont happen any error, but the moment I mount it rw and try
to copy something to the fs on the raid I get the already known timeout.
At least I get a little bit desperate now...

ata10.00: failed to read SCR 1 (Emask=0x40)
ata10.01: failed to read SCR 1 (Emask=0x40)
ata10.02: failed to read SCR 1 (Emask=0x40)
ata10.03: failed to read SCR 1 (Emask=0x40)
ata10.04: failed to read SCR 1 (Emask=0x40)
ata10.05: failed to read SCR 1 (Emask=0x40)
ata10.00: exception Emask 0x100 SAct 0x0 SErr 0x0 action 0x6 frozen
ata10.01: exception Emask 0x100 SAct 0x0 SErr 0x0 action 0x6 frozen
ata10.01: cmd ea/00:00:00:00:00/00:00:00:00:00/a0 tag 1 cdb 0x0 data 0
 res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata10.01: status: { DRDY }
ata10.02: exception Emask 0x100 SAct 0x0 SErr 0x0 action 0x6 frozen
ata10.03: exception Emask 0x100 SAct 0x0 SErr 0x0 action 0x6 frozen
ata10.04: exception Emask 0x100 SAct 0x0 SErr 0x0 action 0x6 frozen
ata10.05: exception Emask 0x100 SAct 0x0 SErr 0x0 action 0x6 frozen
ata10.15: hard resetting link
ata10.15: SATA link up 3.0 Gbps (SStatus 123 SControl 0)
ata10.00: hard resetting link
ata10.00: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
ata10.01: hard resetting link
ata10.01: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
ata10.02: hard resetting link
ata10.02: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
ata10.03: hard resetting link
ata10.03: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
ata10.04: hard resetting link
ata10.04: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
ata10.05: hard resetting link
ata10.05: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
ata10.00: configured for UDMA/100
ata10.01: configured for UDMA/100
ata10.03: configured for UDMA/100
ata10.04: configured for UDMA/100
ata10: EH complete
sd 9:0:0:0: [sdl] 976773168 512-byte hardware sectors (500108 MB)
sd 9:0:0:0: [sdl] Write Protect is off
sd 9:0:0:0: [sdl] Mode Sense: 00 3a 00 00
sd 9:0:0:0: [sdl] Write cache: enabled, read cache: enabled, doesn't
support DPO or FUA
sd 9:1:0:0: [sdm] 976773168 512-byte hardware sectors (500108 MB)
sd 9:1:0:0: [sdm] Write Protect is off
sd 9:1:0:0: [sdm] Mode Sense: 00 3a 00 00
sd 9:1:0:0: [sdm] Write cache: enabled, read cache: enabled, doesn't
support DPO or FUA
sd 9:2:0:0: [sdn] 976773168 512-byte hardware sectors (500108 MB)
sd 9:2:0:0: [sdn] Write Protect is off
sd 9:2:0:0: [sdn] Mode Sense: 00 3a 00 00
sd 9:2:0:0: [sdn] Write cache: enabled, read cache: enabled, doesn't
support DPO or FUA
sd 9:3:0:0: [sdo] 976773168 512-byte hardware sectors (500108 MB)
sd 9:3:0:0: [sdo] Write Protect is off
sd 9:3:0:0: [sdo] Mode Sense: 00 3a 00 00
sd 9:3:0:0: [sdo] Write cache: enabled, read cache: enabled, doesn't
support DPO or FUA
sd 9:4:0:0: [sdp] 976773168 512-byte hardware sectors (500108 MB)
sd 9:4:0:0: [sdp] Write Protect is off
sd 9:4:0:0: [sdp] Mode Sense: 00 3a 00 00
sd 9:4:0:0: [sdp] Write cache: enabled, read cache: enabled, doesn't
support DPO or FUA


-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: (fwd) Bug#11922: I/O error on blank tapes

2008-02-05 Thread Kai Makisara
On Mon, 4 Feb 2008, James Bottomley wrote:

> 
> On Mon, 2008-02-04 at 22:28 +0100, Borislav Petkov wrote:
> > On Mon, Feb 04, 2008 at 03:22:06PM +0100, maximilian attems wrote:
> > 
> > (Added Bart to CC)
> > 
> > > hello borislav,
> > > 
> > > may i forward you that *old* Debian kernel bug,
> > > have seen you working on ide-tape:
> > > http://bugs.debian.org/11922
> > > no we don't carry any ide patches anymore.
> > > 
> > > maybe you've already fixed it in latest?
> > > 
> > > thanks
> > > 
> > > -- 
> > > maks
> > > 
> > > - Forwarded message from Stephen Kitt <[EMAIL PROTECTED]> -
> > > 
> > > Subject: Bug#11922: I/O error on blank tapes
> > > Date: Sat, 1 Dec 2007 19:06:18 +0100
> > > From: Stephen Kitt <[EMAIL PROTECTED]>
> > > To: [EMAIL PROTECTED]
> > > 
> > > Hi,
> > > 
> > > This does still occur with 2.6.22; with a blank tape in my HP DDS-4 drive:
> > > 
> > > $ tar tzvf /dev/nst0
> > > tar: /dev/nst0: Cannot read: Input/output error
> 
> That's a SCSI tape, not an IDE one.  I cc'd the SCSI list
> 
This is not a bug, it is a feature. There is _nothing_ on the tape and if 
you try to read something, you get an error. The same thing applies to 
reading after the last filemark. Note that after writing a filemark at the 
beginning of the tape, the situation is different. Now there is a file and 
the normal EOF semantics apply although there still is no data.

I admit that the error return could be more descriptive but the st driver 
tries to be compatible with other Unices.

The behavior can be changed if Linux does not match other Unices. I don't 
remember if I have tested just this with other Unices. I will try to test 
this with Tru64 tomorrow. If anyone has data on other Unices, it would be 
helpful.

-- 
Kai
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Palmchip BK3710 IDE driver

2008-02-05 Thread Anton Salnikov
I've tested the driver on 2.6.24.

I've made the changes you asked for. But when I tested them on arm/mach-davinci 
architecture on the current Linus' git there was link error undefined reference 
to symbol ide_hwif_setup_dma().
However on x86_64 architecture it compiles well without any warnings with 
respect to link error of undefined clk_get() clk_enable() clk_get_rate(), which 
are not present in this architecture.


> I've just noticed that there is no cable detection for UDMA modes > UDMA2?
For the present controller in documentation: Cable Select (CSEL) - unsupported 
IDE controller signal. This signal is not necessary because an 80-conductor 
cable must be used with the DM644x.

Signed-off-by: Anton Salnikov <[EMAIL PROTECTED]>
---

 drivers/ide/Kconfig   |9 
 drivers/ide/arm/Makefile  |1 
 drivers/ide/arm/palm_bk3710.c |  420 ++
 drivers/ide/ide-proc.c|1 
 include/linux/ide.h   |2 
 5 files changed, 432 insertions(+), 1 deletion(-)

Index: 2.6.25.ide/drivers/ide/Kconfig
===
--- 2.6.25.ide.orig/drivers/ide/Kconfig
+++ 2.6.25.ide/drivers/ide/Kconfig
@@ -1009,6 +1009,15 @@ config BLK_DEV_Q40IDE
  normally be on; disable it only if you are running a custom hard
  drive subsystem through an expansion card.
 
+config BLK_DEV_PALMCHIP_BK3710
+   tristate "Palmchip bk3710 IDE controller support"
+   depends on ARCH_DAVINCI
+   select BLK_DEV_IDEDMA_PCI
+   help
+ Say Y here if you want to support the onchip IDE controller on the
+ TI DaVinci SoC
+
+
 config BLK_DEV_MPC8xx_IDE
tristate "MPC8xx IDE support"
depends on 8xx && (LWMON || IVMS8 || IVML24 || TQM8xxL) && IDE=y && 
BLK_DEV_IDE=y && !PPC_MERGE
Index: 2.6.25.ide/drivers/ide/arm/Makefile
===
--- 2.6.25.ide.orig/drivers/ide/arm/Makefile
+++ 2.6.25.ide/drivers/ide/arm/Makefile
@@ -2,6 +2,7 @@
 obj-$(CONFIG_BLK_DEV_IDE_ICSIDE)   += icside.o
 obj-$(CONFIG_BLK_DEV_IDE_RAPIDE)   += rapide.o
 obj-$(CONFIG_BLK_DEV_IDE_BAST) += bast-ide.o
+obj-$(CONFIG_BLK_DEV_PALMCHIP_BK3710)  += palm_bk3710.o
 
 ifeq ($(CONFIG_IDE_ARM), m)
obj-m += ide_arm.o
Index: 2.6.25.ide/drivers/ide/arm/palm_bk3710.c
===
--- /dev/null
+++ 2.6.25.ide/drivers/ide/arm/palm_bk3710.c
@@ -0,0 +1,420 @@
+/*
+ * Palmchip bk3710 IDE controller
+ *
+ * Copyright (C) 2006 Texas Instruments.
+ * Copyright (C) 2007 MontaVista Software, Inc., <[EMAIL PROTECTED]>
+ *
+ * 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ * 
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Offset of the primary interface registers */
+#define IDE_PALM_ATA_PRI_REG_OFFSET 0x1F0
+
+/* 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
+#define BK3710_BMICS   0x08
+#define BK3710_BMISS   0x0A
+#define BK3710_BMIDTS  0x0C
+#define BK3710_IDETIMP 0x40
+#define BK3710_IDETIMS 0x42
+#define BK3710_SIDETIM 0x44
+#define BK3710_SLEWCTL 0x45
+#define BK3710_IDESTATUS   0x47
+#define BK3710_UDMACTL 0x48
+#define BK3710_UDMATIM 0x4A
+#define BK3710_MISCCTL 0x50
+#define BK3710_REGSTB  0x54
+#define BK3710_REGRCVR 0x58
+#define BK3710_DATSTB  0x5C
+#define BK3710_DATRCVR 0x60
+#define BK3710_DMASTB  0x64
+#define BK3710_DMARCVR 0x68
+#define BK3710_UDMASTB 0x6C
+#define BK3710_UDMATRP 0x70
+#define BK3710_UDMAENV 0x74
+#define BK3710_IORDYTMP0x78
+#define BK3710_IORDYT

Re: [PATCH] enclosure: add support for enclosure services

2008-02-05 Thread James Bottomley
On Mon, 2008-02-04 at 21:35 -0800, Luben Tuikov wrote:
> > > I guess the same could be said for STGT and SCST,
> > right?
> > 
> > You mean both of their kernel pieces are modular? 
> > That's correct.
> 
> No, you know very well what I mean.
> 
> By the same logic you're preaching to include your
> solution part of the kernel, you can also apply to
> SCST.

Ah, but it's not ... the current patch is merely exporting an interface.
The debate in STGT vs SCST is not whether to export an interface but
where to draw the line.

You could also argue in the same vein that sd is redundant because a
filesystem could talk directly to the device via /dev/sgX (in fact OSD
based filesystems already do this).  The argument is true, but misses
the bigger picture that the interfaces exported by sd are more portable
(apply to non-SCSI block devices) and easier to use.

> > > Yes, for which the transport layer, implements the
> > > scsi device node for the SES device.  It doesn't
> > really
> > > matter if the SCSI commands sent to the SES device go
> > > over SGPIO or FC or SAS or Bluetooth or I2C, etc, the
> > > transport layer can implement that and present the
> > > /dev/sgX node.
> > 
> > But it does matter if the enclosure device doesn't
> > speak SCSI.
> 
> Enclosure management isn't as simple as you're
> portraying it here.  The enclosure management
> device speaks either SES or SAF-TE.  The transport
> protocol to access it could be SGPIO or I2C or...

Look, just read the spec; SGPIO is a bus for driving enclosures ... it
doesn't require SES or SAF-TE or even any SCSI protocol.

> >  SGPIO
> > isn't a SCSI protocol ... it's a general purpose
> > serial bus protocol.
> > It's pretty simple and register based, but it might (or
> > might not) be
> > accessible via a SCSI bridge.
> 
> I see.  You've just discovered SGPIO -- good for you.
> 
> At any rate, I told you already that what is needed
> is not what you've provided but a _device node_
> exported by the kernel, either a processor or
> enclosure type.

Wrong ... we don't export non-SCSI devices as SCSI (with the single and
rather annoying exception of ATA via SAT).

James


-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: AHCI driver preferring nr_ports over port map

2008-02-05 Thread Jan Beulich
>Does the following patch fix the problem?

Yes, it does - thanks!

Jan

*

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index e75966b..39627c7 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -679,24 +679,20 @@ static void ahci_save_initial_config(struct pci_dev *pdev,
 
/* cross check port_map and cap.n_ports */
if (port_map) {
-   u32 tmp_port_map = port_map;
-   int n_ports = ahci_nr_ports(cap);
+   int map_ports = 0;
 
-   for (i = 0; i < AHCI_MAX_PORTS && n_ports; i++) {
-   if (tmp_port_map & (1 << i)) {
-   n_ports--;
-   tmp_port_map &= ~(1 << i);
-   }
-   }
+   for (i = 0; i < AHCI_MAX_PORTS; i++)
+   if (port_map & (1 << i))
+   map_ports++;
 
-   /* If n_ports and port_map are inconsistent, whine and
-* clear port_map and let it be generated from n_ports.
+   /* If PI has more ports than n_ports, whine and clear
+* port_map and let it be generated from n_ports.
 */
-   if (n_ports || tmp_port_map) {
+   if (map_ports > ahci_nr_ports(cap)) {
dev_printk(KERN_WARNING, &pdev->dev,
-  "nr_ports (%u) and implemented port map "
-  "(0x%x) don't match, using nr_ports\n",
-  ahci_nr_ports(cap), port_map);
+  "implemented port map (0x%x) contains more "
+  "ports than nr_ports (%u), using nr_ports\n",
+  port_map, ahci_nr_ports(cap));
port_map = 0;
}
}

-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


slow resume from s2ram

2008-02-05 Thread Brian Keck

Hello,

I can't get my otherwise lovely debian/sid Vaio TX17 to resume promptly
from suspend-to-ram.

Until recently it resumed at all maybe 1 in 4 times from
's2ram -f -p' in X, always taking several minutes, apparently 
due to the hard disk (1.8" Toshiba HDD1544/MK6006GAH).   

Last week I tried 2 things:

  * added /etc/modprobe.d/libata with
options libata noacpi=0

  * built a kernel + initrd from debian linux-source-2.6.24 with
CONFIG_BLK_DEV_IDEACPI=y

This boots OK, with ...

   $ cat /sys/module/libata/parameters/noacpi
   0

But the only improvement is that, from X, it resumes every time (maybe
10 times so far).  It still takes about 90 seconds (the disc light is on
for the 1st 30 seconds, then after another 60 seconds it comes on again
& the display comes back).

This is with the s2ram from debian/sid current uswsusp.
Oddly, s2ram has always worked best from X.

dmesg below ... presumably the delay is connected to the lines ...
  hda: lost interrupt
  hda: dma_timer_expiry: dma status == 0x21

Any ideas?

Thanks,
Brian Keck

--

Linux version 2.6.24 (2.6.24-1) ([EMAIL PROTECTED]) (gcc version 4.2.1 (Debian 
4.2.1-5)) #2 SMP Fri Feb 1 20:28:43 EST 2008
BIOS-provided physical RAM map:
 BIOS-e820:  - 0009f800 (usable)
 BIOS-e820: 0009f800 - 000a (reserved)
 BIOS-e820: 000d8000 - 0010 (reserved)
 BIOS-e820: 0010 - 3f69 (usable)
 BIOS-e820: 3f69 - 3f69d000 (ACPI data)
 BIOS-e820: 3f69d000 - 3f70 (ACPI NVS)
 BIOS-e820: 3f70 - 4000 (reserved)
 BIOS-e820: e000 - f0006000 (reserved)
 BIOS-e820: f0008000 - f000c000 (reserved)
 BIOS-e820: fed2 - fed9 (reserved)
 BIOS-e820: ff00 - 0001 (reserved)
118MB HIGHMEM available.
896MB LOWMEM available.
Entering add_active_range(0, 0, 259728) 0 entries of 256 used
Zone PFN ranges:
  DMA 0 -> 4096
  Normal   4096 ->   229376
  HighMem229376 ->   259728
Movable zone start PFN for each node
early_node_map[1] active PFN ranges
0:0 ->   259728
On node 0 totalpages: 259728
  DMA zone: 32 pages used for memmap
  DMA zone: 0 pages reserved
  DMA zone: 4064 pages, LIFO batch:0
  Normal zone: 1760 pages used for memmap
  Normal zone: 223520 pages, LIFO batch:31
  HighMem zone: 237 pages used for memmap
  HighMem zone: 30115 pages, LIFO batch:7
  Movable zone: 0 pages used for memmap
DMI 2.3 present.
ACPI: RSDP 000F6790, 0014 (r0 PTLTD )
ACPI: RSDT 3F697E7C, 0044 (r1   Sony   V1 20050805 PTL 0)
ACPI: FACP 3F69CE78, 0084 (r2   Sony   V1 20050805 PTL50)
ACPI: DSDT 3F69896F, 4509 (r1   Sony   V1 20050805 PTL   10E)
ACPI: FACS 3F6ADFC0, 0040
ACPI: APIC 3F69CEFC, 0068 (r1   Sony   V1 20050805 PTL50)
ACPI: BOOT 3F69CFD8, 0028 (r1   Sony   V1 20050805 PTL 1)
ACPI: MCFG 3F69CF9C, 003C (r1   Sony   V1 20050805 PTL5F)
ACPI: SSDT 3F69873D, 022E (r1   Sony   V1 20050805 PTL  20030224)
ACPI: SSDT 3F6982F8, 0235 (r1   Sony   V1 20050805 PTL  20030224)
ACPI: SSDT 3F6980D9, 021F (r1   Sony   V1 20050805 PTL  20030224)
ACPI: SSDT 3F697EC0, 0219 (r1   Sony   V1 20050805 PTL  20030224)
ACPI: DMI detected: Sony
ACPI: PM-Timer IO Port: 0x1008
ACPI: Local APIC address 0xfee0
ACPI: LAPIC (acpi_id[0x00] lapic_id[0x00] enabled)
Processor #0 6:13 APIC version 20
ACPI: LAPIC (acpi_id[0x01] lapic_id[0x01] disabled)
ACPI: LAPIC_NMI (acpi_id[0x00] high edge lint[0x1])
ACPI: LAPIC_NMI (acpi_id[0x01] high edge lint[0x1])
ACPI: IOAPIC (id[0x01] address[0xfec0] gsi_base[0])
IOAPIC[0]: apic_id 1, version 32, address 0xfec0, GSI 0-23
ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
ACPI: IRQ0 used by override.
ACPI: IRQ2 used by override.
ACPI: IRQ9 used by override.
Enabling APIC mode:  Flat.  Using 1 I/O APICs
Using ACPI (MADT) for SMP configuration information
Allocating PCI resources starting at 5000 (gap: 4000:a000)
Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 257699
Kernel command line: root=/dev/hda2 ro
mapped APIC to b000 (fee0)
mapped IOAPIC to a000 (fec0)
Enabling fast FPU save and restore... done.
Enabling unmasked SIMD FPU exception support... done.
Initializing CPU#0
PID hash table entries: 4096 (order: 12, 16384 bytes)
Detected 1197.028 MHz processor.
Console: colour VGA+ 80x25
console [tty0] enabled
Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
Memory: 1020860k/1038912k available (1692k kernel code, 17256k reserved, 671k 
data, 216k init, 121408k highmem)
virtual kernel memory layout:
fixmap  : 0xfff4d000 - 0xf000   ( 712 kB)
pkmap   : 0xff80 - 0x

Re: AHCI driver preferring nr_ports over port map

2008-02-05 Thread Tejun Heo
Jan Beulich wrote:
> It does, in the description for bits 4:0 of the host capabilities register:
> "Number of Ports (NPS)" RO. Hardwired to 5h to indicate support for 6
> ports. Note that the number of ports indicated in this field may be more
> than the number of ports indicated in the PI (ABAR + 0Ch) register."

Oops, you're right, NP can go over PI.  Somehow I've been believing it
should match PI.  Oh well, that's me being delusional again.  :-(

>From ahci 1.1.

 Number of Ports (NP): 0's based value indicating the maximum number
 of ports supported by the HBA silicon. A maximum of 32 ports can be
 supported. A value of Impl.  '0h', indicating one port, is the
 minimum requirement. Note that the number of ports Spec.  indicated
 in this field may be more than the number of ports indicated in the
 GHC.PI register.

Does the following patch fix the problem?

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index e75966b..39627c7 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -679,24 +679,20 @@ static void ahci_save_initial_config(struct pci_dev *pdev,
 
/* cross check port_map and cap.n_ports */
if (port_map) {
-   u32 tmp_port_map = port_map;
-   int n_ports = ahci_nr_ports(cap);
+   int map_ports = 0;
 
-   for (i = 0; i < AHCI_MAX_PORTS && n_ports; i++) {
-   if (tmp_port_map & (1 << i)) {
-   n_ports--;
-   tmp_port_map &= ~(1 << i);
-   }
-   }
+   for (i = 0; i < AHCI_MAX_PORTS; i++)
+   if (port_map & (1 << i))
+   map_ports++;
 
-   /* If n_ports and port_map are inconsistent, whine and
-* clear port_map and let it be generated from n_ports.
+   /* If PI has more ports than n_ports, whine and clear
+* port_map and let it be generated from n_ports.
 */
-   if (n_ports || tmp_port_map) {
+   if (map_ports > ahci_nr_ports(cap)) {
dev_printk(KERN_WARNING, &pdev->dev,
-  "nr_ports (%u) and implemented port map "
-  "(0x%x) don't match, using nr_ports\n",
-  ahci_nr_ports(cap), port_map);
+  "implemented port map (0x%x) contains more "
+  "ports than nr_ports (%u), using nr_ports\n",
+  port_map, ahci_nr_ports(cap));
port_map = 0;
}
}
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: AHCI driver preferring nr_ports over port map

2008-02-05 Thread Jan Beulich
>>> Tejun Heo <[EMAIL PROTECTED]> 05.02.08 13:21 >>>
>Jan Beulich wrote:
>>> Yes, we can be more smart if necessary.  I don't know.  The hardware is
>>> clearly violating the spec which requires those two values to agree.
>> 
>> So are you saying the ESB2 spec is violating a higher level spec? I know
>> almost nothing about AHCI, so please forgive that question...
>
>n_ports and PI should agree with each other.  That's what the ahci spec
>requires.  If ESB2 spec has different opinion about n_ports, it's in
>violation of AHCI spec but I don't think it explicitly state such
>things, does it?

It does, in the description for bits 4:0 of the host capabilities register:
"Number of Ports (NPS) – RO. Hardwired to 5h to indicate support for 6
ports. Note that the number of ports indicated in this field may be more
than the number of ports indicated in the PI (ABAR + 0Ch) register."

>I'd like to see more output including leading controller detection but
>yeah, it seems there's no silicon implemented for the last port.  The
>SStatus value 4 indicates that PHY is offline which usually happens when
>the PHY is turned off from SControl.  Hmm... weird.  How many ports does
>the machine actually have?  I agree we'll need to adjust PI handling for
>the controller.

<7>libata version 2.00 loaded.
<7>ahci :00:1f.2: version 2.0
<6>ACPI: PCI Interrupt :00:1f.2[C] -> GSI 20 (level, low) -> IRQ 66
<7>PCI: Setting latency timer of device :00:1f.2 to 64
<6>ahci :00:1f.2: AHCI 0001.0100 32 slots 6 ports 3 Gbps 0x1f impl SATA mode
<6>ahci :00:1f.2: flags: 64bit ncq pm led slum part 
<6>ata1: SATA max UDMA/133 cmd 0xF882E100 ctl 0x0 bmdma 0x0 irq 66
<6>ata2: SATA max UDMA/133 cmd 0xF882E180 ctl 0x0 bmdma 0x0 irq 66
<6>ata3: SATA max UDMA/133 cmd 0xF882E200 ctl 0x0 bmdma 0x0 irq 66
<6>ata4: SATA max UDMA/133 cmd 0xF882E280 ctl 0x0 bmdma 0x0 irq 66
<6>ata5: SATA max UDMA/133 cmd 0xF882E300 ctl 0x0 bmdma 0x0 irq 66
<6>ata6: SATA max UDMA/133 cmd 0xF882E380 ctl 0x0 bmdma 0x0 irq 66
<6>scsi0 : ahci
<6>ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
<6>ata1.00: ATA-7, max UDMA/133, 31250 sectors: LBA48 NCQ (depth 31/32)
<6>ata1.00: configured for UDMA/133
<6>scsi1 : ahci
<6>ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
<6>ata2.00: ATA-7, max UDMA/133, 31250 sectors: LBA48 NCQ (depth 31/32)
<6>ata2.00: configured for UDMA/133
<6>scsi2 : ahci
<6>ata3: SATA link down (SStatus 0 SControl 300)
<6>scsi3 : ahci
<6>ata4: SATA link down (SStatus 0 SControl 300)
<6>scsi4 : ahci
<6>ata5: SATA link down (SStatus 4 SControl 300)
<6>scsi5 : ahci
<6>ata6: SATA link down (SStatus 0 SControl 0)
<5>  Vendor: ATA   Model: ST3160815AS   Rev: 3.AD
<5>  Type:   Direct-Access  ANSI SCSI revision: 05
<5>SCSI device sda: 31250 512-byte hdwr sectors (16 MB)
<5>sda: Write Protect is off
<7>sda: Mode Sense: 00 3a 00 00
<5>SCSI device sda: drive cache: write back
<5>SCSI device sda: 31250 512-byte hdwr sectors (16 MB)
<5>sda: Write Protect is off
<7>sda: Mode Sense: 00 3a 00 00
<5>SCSI device sda: drive cache: write back
<6> sda: sda1 sda2 sda3 sda4 < sda5 sda6 sda7 sda8 sda9 sda10 >
<5>sd 0:0:0:0: Attached scsi disk sda
<5>  Vendor: ATA   Model: ST3160815AS   Rev: 3.AD
<5>  Type:   Direct-Access  ANSI SCSI revision: 05
<5>sd 0:0:0:0: Attached scsi generic sg0 type 0
<5>SCSI device sdb: 31250 512-byte hdwr sectors (16 MB)
<5>sdb: Write Protect is off
<7>sdb: Mode Sense: 00 3a 00 00
<5>SCSI device sdb: drive cache: write back
<5>SCSI device sdb: 31250 512-byte hdwr sectors (16 MB)
<5>sdb: Write Protect is off
<7>sdb: Mode Sense: 00 3a 00 00
<5>SCSI device sdb: drive cache: write back
<6> sdb: sdb1
<5>sd 1:0:0:0: Attached scsi disk sdb
<5>sd 1:0:0:0: Attached scsi generic sg1 type 0

Jan
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: AHCI driver preferring nr_ports over port map

2008-02-05 Thread Tejun Heo
Jan Beulich wrote:
>> Yes, we can be more smart if necessary.  I don't know.  The hardware is
>> clearly violating the spec which requires those two values to agree.
> 
> So are you saying the ESB2 spec is violating a higher level spec? I know
> almost nothing about AHCI, so please forgive that question...

n_ports and PI should agree with each other.  That's what the ahci spec
requires.  If ESB2 spec has different opinion about n_ports, it's in
violation of AHCI spec but I don't think it explicitly state such
things, does it?

>> What status values are you seeing?  Hardware vendors usually don't get
>> n_ports wrong from the start, they probably have forgotten to decrement
>> it by one when one of the ports is plugged for some reason.  I bet the
>> silicon for the port is there and reporting offline PHY, right?
> 
> This is output from our SLE10SP2 kernel, the output is similar for others:
> 
> <6>scsi2 : ahci
> <6>ata3: SATA link down (SStatus 0 SControl 300)
> <6>scsi3 : ahci
> <6>ata4: SATA link down (SStatus 0 SControl 300)
> <6>scsi4 : ahci
> <6>ata5: SATA link down (SStatus 4 SControl 300)
> <6>scsi5 : ahci
> <6>ata6: SATA link down (SStatus 0 SControl 0)
>
> Even the message relating to ata5 seems a little dubious to me, as it's
> not in sync with what the other unused ports say (and also not in sync
> with what I see on other boxes - SStatus is always 0 for such ports).

I'd like to see more output including leading controller detection but
yeah, it seems there's no silicon implemented for the last port.  The
SStatus value 4 indicates that PHY is offline which usually happens when
the PHY is turned off from SControl.  Hmm... weird.  How many ports does
the machine actually have?  I agree we'll need to adjust PI handling for
the controller.

Thanks.

-- 
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 0/7] blk_end_request: full I/O completion handler

2008-02-05 Thread S, Chandrakala (STSD)
Hello,

We would like to know in which kernel version these patches are
available.  
 
Thanks,
Chandrakala

-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Jens Axboe
Sent: Monday, September 03, 2007 1:16 PM
To: Kiyoshi Ueda
Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED];
linux-ide@vger.kernel.org; Miller, Mike (OS Dev);
[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]
Subject: Re: [PATCH 0/7] blk_end_request: full I/O completion handler

On Fri, Aug 31 2007, Kiyoshi Ueda wrote:
> Hello,
> 
> This set of patches changes request completion interface between 
> device drivers and block layer to 1 step procedure from current 2 step

> procedures using end_that_request_{first/chunk} and 
> end_that_request_last().
> 
> This change allows request-based multipath to hook in before 
> completing each chunk of request, check errors for it and retry it 
> using another path if error is detected.
> 
> Summaries of each patch are below:
>   1/7: add new request completion interface, blk_end_request()
>   2/7: add some macros to get the size of request in bytes
>   3/7: convert normal drivers to use blk_end_request()
>   4/7: convert odd drivers like cciss/cpqarray/xsysace to use
>blk_end_request()
>   5/7: convert ide-cd (cdrom_newpc_intr) to use blk_end_request()
>   6/7: remove/unexport no longer needed end_that_request_*
>   7/7: change rq->end_io to cover request completion as a whole
> 
> I have tested the patch on two machines, ia64+QLA1280+QLA2200 and 
> x86_64+SATA+IDE-CDROM.
> I can't test other device drivers for which I don't have hardware.
> So testing help and any comments would be very much appreciated.
> 
> The interface change causes code modifications of *ALL DEVICE DRIVERS*

> which are using end_that_request_{first/chunk/last} to complete
request.
> But it should not affect the behavior.
> 
> Please review and apply if no problem.
> This patch-set should be applied on top of 2.6.23-rc3-mm1.
> 
> 
> BACKGROUND
> ==
> The patch is necessary to allow device stacking at request level, that

> is request-based device-mapper multipath.
> Currently, device-mapper is implemented as a stacking block device at 
> BIO level.  OTOH, request-based DM will stack at request level to 
> allow better multipathing decision.
> To allow device stacking at request level, the completion procedure 
> need to provide a hook for it.
> For example, dm-multipath has to check errors and retry with other 
> paths if necessary before returning the I/O result to upper layer.
> struct request has 'end_io' hook currently.  But it's called at the 
> very late stage of completion handling where the I/O result is already

> returned to the upper layer.
> So we need something here.
> 
> The first approach to hook in completion of each chunk of request was 
> adding a new rq->end_io_first() hook and calling it on the top of 
> __end_that_request_first().
>   - http://marc.theaimsgroup.com/?l=linux-scsi&m=115520444515914&w=2
>   - http://marc.theaimsgroup.com/?l=linux-kernel&m=116656637425880&w=2
> However, Jens pointed out that redesigning rq->end_io() as a full 
> completion handler would be better:
> 
> On Thu, 21 Dec 2006 08:49:47 +0100, Jens Axboe <[EMAIL PROTECTED]>
wrote:
> > Ok, I see what you are getting at. The current ->end_io() is called 
> > when the request has fully completed, you want notification for each

> > chunk potentially completed.
> > 
> > I think a better design here would be to use ->end_io() as the full 
> > completion handler, similar to how bio->bi_end_io() works. A request

> > originating from __make_request() would set something ala:
> .
> > instead of calling the functions manually. That would allow you to 
> > get notification right at the beginning and do what you need, 
> > without adding a special hook for this.
> 
> I thought his comment was reasonable.
> So I modified the patches based on his suggestion.
> 
> 
> WHAT IS CHANGED
> ===
> The change is basically illustlated by the following pseudo code:
> 
> [Before]
>   if (end_that_request_{first/chunk} succeeds) { <-- completes bios
>  
>  end_that_request_last() <-- calls end_io()
>  
>   } else {
>  
>   }
> 
> [After]
>   if (blk_end_request() succeeds) { <-- calls end_io(), completes bios
>  
>   } else {
>  
>   }
> 
> 
> In detail, request completion procedures are changed like below.
> 
> [Before]
>   o 2 steps completion using end_that_request_{first/chunk}
> and end_that_request_last().
>   o Device drivers have ownership of a request until they
> call end_that_request_last().
>   o rq->end_io() is called at the last stage of
> end_that_request_last() for some block layer codes need
> specific request handling when completing it.
> 
> [After]
>   o 1 step completion using blk_end_request().
> (end_that_request_* are no longer used from device drivers.)
>   o Device drivers give over ownership of a request
> when calli

Re: [PATCH 0/7] blk_end_request: full I/O completion handler

2008-02-05 Thread Jens Axboe
On Tue, Feb 05 2008, S, Chandrakala (STSD) wrote:
> Hello,
> 
> We would like to know in which kernel version these patches are
> available.  

They were merged after 2.6.24 was released, so they will show up in the
2.6.25 kernel.

-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[BUG] 2.6.24-mm1 and 2.6.24-git13 kernel panic's while bootup at ide_device_add_all

2008-02-05 Thread Kamalesh Babulal
Hi,

The kernel bootup panics with the 2.6.24-mm1 and 2.6.24-git13 kernel
while defconfig compiled in for x86_64 (Intel(R) Xeon) box

BUG: unable to handle kernel paging request at ffb0
IP: [] init_irq+0x188/0x444
PGD 203067 PUD 204067 PMD 0 
Oops:  [1] SMP 
CPU 2 
Modules linked in:
Pid: 1, comm: swapper Not tainted 2.6.24 #1
RIP: 0010:[]  [] init_irq+0x188/0x444
RSP: 0018:81022f093e00  EFLAGS: 00010287
RAX: ff90 RBX: 808a9100 RCX: 
RDX:  RSI: 81022fc039c0 RDI: 8074dd00
RBP: 81022f093e30 R08: 808af880 R09: 0002
R10: 0001 R11: 810bed60 R12: 808b0400
R13: 808b0410 R14:  R15: 
FS:  () GS:81022f0883c0() knlGS:
CS:  0010 DS: 0018 ES: 0018 CR0: 8005003b
CR2: ffb0 CR3: 00201000 CR4: 06e0
DR0:  DR1:  DR2: 
DR3:  DR6: 0ff0 DR7: 0400
Process swapper (pid: 1, threadinfo 81022f092000, task 81022f0797d0)
Stack:  81022f093e30  808a9100 808a9120
 ffed  81022f093eb0 80414425
 81022f093ec0  8074e920 0246
Call Trace:
 [] ide_device_add_all+0xb27/0xe1b
 [] ide_generic_init+0x3a/0x3e
 [] kernel_init+0x175/0x2e7
 [] child_rip+0xa/0x12
 [] ? acpi_ds_init_one_object+0x0/0x88
 [] ? kernel_init+0x0/0x2e7
 [] ? child_rip+0x0/0x12


Code: 89 03 49 8b 45 18 48 89 18 48 39 1b 75 04 0f 0b eb fe fe 05 51 51 38 00 
fb eb 5b 48 8b 83 28 07 00 00 83 ca ff 48 83 e8 70 74 0e <48> 8b 40 20 48 8b 80 
88 00 00 00 8b 50 04 48 8b 3d 79 ff 2f 00 
RIP  [] init_irq+0x188/0x444
 RSP 
CR2: ffb0
---[ end trace 9010a4c8c4ba608a ]---

(gdb) p init_irq
$1 = {int (ide_hwif_t *)} 0x804134ba 
(gdb) p/x 0x804134ba+0x188
$2 = 0x80413642
(gdb) l *0x80413642
0x80413642 is in init_irq (include/linux/ide.h:1311).
1306print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 2, id, 512, 
0);
1307}
1308
1309static inline int hwif_to_node(ide_hwif_t *hwif)
1310{
1311struct pci_dev *dev = to_pci_dev(hwif->dev);
1312return dev ? pcibus_to_node(dev->bus) : -1;
1313}
1314
1315static inline ide_drive_t *ide_get_paired_drive(ide_drive_t *drive)
(gdb) 

(gdb) p ide_device_add_all
$1 = {int (u8 *, const struct ide_port_info *)} 0x804138fe 

(gdb) p/x 0x804138fe
$2 = 0x804138fe
(gdb) l *0x804138fe
0x804138fe is in ide_device_add_all (drivers/ide/ide-probe.c:1372).
1367hwif->cbl = hwif->cable_detect(hwif);
1368}
1369}
1370
1371int ide_device_add_all(u8 *idx, const struct ide_port_info *d)
1372{
1373ide_hwif_t *hwif, *mate = NULL;
1374int i, rc = 0;
1375
1376for (i = 0; i < MAX_HWIFS; i++) {
(gdb) 

-- 
Thanks & Regards,
Kamalesh Babulal,
Linux Technology Center,
IBM, ISTL.
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html