Re: [PATCH 7/8] ide: add struct ide_port_info instances to legacy host drivers

2008-02-11 Thread Russell King 
On Sat, Feb 02, 2008 at 12:35:30AM +0100, Bartlomiej Zolnierkiewicz wrote:
   Index: b/drivers/ide/arm/icside.c
   ===
   --- a/drivers/ide/arm/icside.c
   +++ b/drivers/ide/arm/icside.c
   @@ -459,11 +456,19 @@ icside_register_v5(struct icside_state *

 idx[0] = hwif-index;

   - ide_device_add(idx);
   + ide_device_add(idx, NULL);

 return 0;
}

   +static const struct ide_port_info icside_v6_port_info __initdata = {
   + .host_flags = IDE_HFLAG_SERIALIZE |
   +   IDE_HFLAG_NO_DMA | /* no SFF-style DMA */
   +   IDE_HFLAG_NO_AUTOTUNE,
   + .mwdma_mask = ATA_MWDMA2,
   + .swdma_mask = ATA_SWDMA2,
   +};
   +
  
  Interesting... this driver's support for SWDMA0 is broken since the 
  cycle 
  should be 960 ns long, not 480, and SWDMA2 is underclocked using the same 
  cycle as SWDMA1, 480 ns...
 
 Added Russell to Cc:.

Underclocking the DMA is not a problem - if you look at the timing
diagrams and associated tables in the ATA specifications, you'll
find that they specify the *minimum* cycle timings.

However, you're correct that SWDMA0 is not able to be supported.

-- 
Russell King
 Linux kernel2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-
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 7/8] ide: add struct ide_port_info instances to legacy host drivers

2008-02-11 Thread Atsushi Nemoto 
On Mon, 11 Feb 2008 00:16:07 +0100, Bartlomiej Zolnierkiewicz [EMAIL 
PROTECTED] wrote:
  It is too late for -git but not too late for 2.6.25-rc1. ;-)
  
  Could you make a patch please?
 
 I went ahead and made a patch (sorry, I needed it for git pull request :).
 
 From: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
 Subject: [PATCH] ide: ide_init_port() bugfix

Thanks!  Looks sane for me :)

---
Atsushi Nemoto
-
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 7/8] ide: add struct ide_port_info instances to legacy host drivers

2008-02-10 Thread Bartlomiej Zolnierkiewicz 
On Sunday 10 February 2008, Atsushi Nemoto wrote:
 On Sun, 06 Jan 2008 18:03:10 +0100, Bartlomiej Zolnierkiewicz [EMAIL 
 PROTECTED] wrote:
  +   /* reset DMA masks only for SFF-style DMA controllers */
  +   if ((d-host_flags  IDE_HFLAG_NO_DMA) == 0  hwif-dma_base == 0)
  +   hwif-swdma_mask = hwif-mwdma_mask = hwif-ultra_mask = 0;
 
 It might be too late, but host_flags  IDE_HFLAGS_NO_DMA seems
 wrong for me.

It is too late for -git but not too late for 2.6.25-rc1. ;-)

Could you make a patch please?

Thanks,
Bart
-
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 7/8] ide: add struct ide_port_info instances to legacy host drivers

2008-02-10 Thread Atsushi Nemoto 
On Sun, 06 Jan 2008 18:03:10 +0100, Bartlomiej Zolnierkiewicz [EMAIL 
PROTECTED] wrote:
 + /* reset DMA masks only for SFF-style DMA controllers */
 + if ((d-host_flags  IDE_HFLAG_NO_DMA) == 0  hwif-dma_base == 0)
 + hwif-swdma_mask = hwif-mwdma_mask = hwif-ultra_mask = 0;

It might be too late, but host_flags  IDE_HFLAGS_NO_DMA seems
wrong for me.

---
Atsushi Nemoto
-
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 7/8] ide: add struct ide_port_info instances to legacy host drivers

2008-02-10 Thread Bartlomiej Zolnierkiewicz 
On Sunday 10 February 2008, Bartlomiej Zolnierkiewicz wrote:
 On Sunday 10 February 2008, Atsushi Nemoto wrote:
  On Sun, 06 Jan 2008 18:03:10 +0100, Bartlomiej Zolnierkiewicz [EMAIL 
  PROTECTED] wrote:
   + /* reset DMA masks only for SFF-style DMA controllers */
   + if ((d-host_flags  IDE_HFLAG_NO_DMA) == 0  hwif-dma_base == 0)
   + hwif-swdma_mask = hwif-mwdma_mask = hwif-ultra_mask = 0;
  
  It might be too late, but host_flags  IDE_HFLAGS_NO_DMA seems
  wrong for me.
 
 It is too late for -git but not too late for 2.6.25-rc1. ;-)
 
 Could you make a patch please?

I went ahead and made a patch (sorry, I needed it for git pull request :).

From: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
Subject: [PATCH] ide: ide_init_port() bugfix

On Sunday 10 February 2008, Atsushi Nemoto wrote:
 On Sun, 06 Jan 2008 18:03:10 +0100, Bartlomiej Zolnierkiewicz [EMAIL 
 PROTECTED] wrote:
  +   /* reset DMA masks only for SFF-style DMA controllers */
  +   if ((d-host_flags  IDE_HFLAG_NO_DMA) == 0  hwif-dma_base == 0)
  +   hwif-swdma_mask = hwif-mwdma_mask = hwif-ultra_mask = 0;
 
 It might be too late, but host_flags  IDE_HFLAGS_NO_DMA seems
 wrong for me.

Fix regression caused by commmit c413b9b94d9a8e7548cc4b2e04b7df0439ce76fd
(ide: add struct ide_port_info instances to legacy host drivers).

Reported-by: Atsushi Nemoto [EMAIL PROTECTED]
Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
---
 drivers/ide/ide-probe.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/drivers/ide/ide-probe.c
===
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -1355,7 +1355,7 @@ static void ide_init_port(ide_hwif_t *hw
hwif-ultra_mask = d-udma_mask;
 
/* reset DMA masks only for SFF-style DMA controllers */
-   if ((d-host_flags  IDE_HFLAG_NO_DMA) == 0  hwif-dma_base == 0)
+   if ((d-host_flags  IDE_HFLAG_NO_DMA) == 0  hwif-dma_base == 0)
hwif-swdma_mask = hwif-mwdma_mask = hwif-ultra_mask = 0;
 
if (d-host_flags  IDE_HFLAG_RQSIZE_256)

-
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 7/8] ide: add struct ide_port_info instances to legacy host drivers

2008-02-01 Thread Bartlomiej Zolnierkiewicz 

Hi,

On Monday 28 January 2008, Sergei Shtylyov wrote:
 Hello.
 
 Bartlomiej Zolnierkiewicz wrote:
 
  * Remove 'struct pci_dev *dev' argument from ide_hwif_setup_dma().
 
  * Un-static ide_hwif_setup_dma() and add CONFIG_BLK_DEV_IDEDMA_PCI=n 
  version.
 
  * Add 'const struct ide_port_info *d' argument to ide_device_add[_all]().
 
  * Factor out generic ports init from ide_pci_setup_ports() to 
  ide_init_port(),
move it to ide-probe.c and call it in in ide_device_add_all() instead of
ide_pci_setup_ports().
 
  * Move -mate setup to ide_device_add_all() from ide_port_init().
 
  * Add IDE_HFLAG_NO_AUTOTUNE host flag for host drivers that don't enable
-autotune currently.
 
  * Setup hwif-chipset in ide_init_port() but iff pi-chipset is set
(to not override setup done by ide_hwif_configure()).
 
  * Add ETRAX host handling to ide_device_add_all().
 
  * cmd640.c: set IDE_HFLAG_ABUSE_* also for CONFIG_BLK_DEV_CMD640_ENHANCED=n.
 
  * pmac.c: make pmac_ide_setup_dma() return an error value and move DMA masks
setup to pmac_ide_setup_device().
 
  * Add 'struct ide_port_info' instances to legacy host drivers, pass them to
ide_device_add() calls and then remove open-coded ports initialization.
 
  Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
 
  Index: b/drivers/ide/arm/icside.c
  ===
  --- a/drivers/ide/arm/icside.c
  +++ b/drivers/ide/arm/icside.c
  @@ -459,11 +456,19 @@ icside_register_v5(struct icside_state *
   
  idx[0] = hwif-index;
   
  -   ide_device_add(idx);
  +   ide_device_add(idx, NULL);
   
  return 0;
   }
   
  +static const struct ide_port_info icside_v6_port_info __initdata = {
  +   .host_flags = IDE_HFLAG_SERIALIZE |
  + IDE_HFLAG_NO_DMA | /* no SFF-style DMA */
  + IDE_HFLAG_NO_AUTOTUNE,
  +   .mwdma_mask = ATA_MWDMA2,
  +   .swdma_mask = ATA_SWDMA2,
  +};
  +
 
 Interesting... this driver's support for SWDMA0 is broken since the cycle 
 should be 960 ns long, not 480, and SWDMA2 is underclocked using the same 
 cycle as SWDMA1, 480 ns...

Added Russell to Cc:.

  Index: b/drivers/ide/cris/ide-cris.c
  ===
  --- a/drivers/ide/cris/ide-cris.c
  +++ b/drivers/ide/cris/ide-cris.c
  @@ -753,6 +753,15 @@ static void cris_set_dma_mode(ide_drive_
  cris_ide_set_speed(TYPE_DMA, 0, strobe, hold);
   }
   
  +static const struct ide_port_info cris_port_info __initdata = {
  +   .chipset= ide_etrax100,
  +   .host_flags = IDE_HFLAG_NO_ATAPI_DMA |
  + IDE_HFLAG_NO_DMA, /* no SFF-style DMA */
  +   .pio_mask   = ATA_PIO4,
  +   .udma_mask  = cris_ultra_mask,
 
 Hm, I wonder which value it will assume, 0x07 or 0?  Not sure even after 
 looking at the source... :-)

This is tricky since it depends on the CRIS version
(0x07 on CRIS V32 and 0x00 on CRIS V10).

  +   .mwdma_mask = ATA_MWDMA2,
  +};
  +
   static int __init init_e100_ide(void)
   {
  hw_regs_t hw;
  Index: b/drivers/ide/ide-probe.c
  ===
  --- a/drivers/ide/ide-probe.c
  +++ b/drivers/ide/ide-probe.c
  @@ -1289,12 +1289,86 @@ static void hwif_register_devices(ide_hw
  }
   }
   
  -int ide_device_add_all(u8 *idx)
  +static void ide_init_port(ide_hwif_t *hwif, unsigned int port,
  + const struct ide_port_info *d)
   {
  -   ide_hwif_t *hwif;
  +   if (d-chipset != ide_etrax100)
  +   hwif-channel = port;
 
Hm, what's so special about ide_etrax100?

The driver supports *4* channels and it didn't set -channel at all.

Since in the meantime ide-cris got marked as BROKEN I guess that a patch
removing above code would be OK.

  +int ide_device_add_all(u8 idx[MAX_HWIFS], const struct ide_port_info *d)
 
 Function prototype doesn't match with one from linux/ide.h which has 
 the 
 first argument as a pointer...

I fixed it locally, thanks.

  +{
  +   ide_hwif_t *hwif, *mate = NULL;
  int i, rc = 0;
   
  for (i = 0; i  MAX_HWIFS; i++) {
  +   if (d == NULL || idx[i] == 0xff) {
 
 Why check for (d == NULL) every time and not do it once and break from 
 the 
 loop or even do it before the loop?

To save on some code lines and resulting code size
(ide_device_add_all is not performance critical).

Bart
-
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 7/8] ide: add struct ide_port_info instances to legacy host drivers

2008-01-28 Thread Sergei Shtylyov 

Hello.

Bartlomiej Zolnierkiewicz wrote:


* Remove 'struct pci_dev *dev' argument from ide_hwif_setup_dma().



* Un-static ide_hwif_setup_dma() and add CONFIG_BLK_DEV_IDEDMA_PCI=n version.



* Add 'const struct ide_port_info *d' argument to ide_device_add[_all]().



* Factor out generic ports init from ide_pci_setup_ports() to ide_init_port(),
  move it to ide-probe.c and call it in in ide_device_add_all() instead of
  ide_pci_setup_ports().



* Move -mate setup to ide_device_add_all() from ide_port_init().



* Add IDE_HFLAG_NO_AUTOTUNE host flag for host drivers that don't enable
  -autotune currently.



* Setup hwif-chipset in ide_init_port() but iff pi-chipset is set
  (to not override setup done by ide_hwif_configure()).



* Add ETRAX host handling to ide_device_add_all().



* cmd640.c: set IDE_HFLAG_ABUSE_* also for CONFIG_BLK_DEV_CMD640_ENHANCED=n.



* pmac.c: make pmac_ide_setup_dma() return an error value and move DMA masks
  setup to pmac_ide_setup_device().



* Add 'struct ide_port_info' instances to legacy host drivers, pass them to
  ide_device_add() calls and then remove open-coded ports initialization.



Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]



Index: b/drivers/ide/arm/icside.c
===
--- a/drivers/ide/arm/icside.c
+++ b/drivers/ide/arm/icside.c
@@ -459,11 +456,19 @@ icside_register_v5(struct icside_state *
 
 	idx[0] = hwif-index;
 
-	ide_device_add(idx);

+   ide_device_add(idx, NULL);
 
 	return 0;

 }
 
+static const struct ide_port_info icside_v6_port_info __initdata = {

+   .host_flags = IDE_HFLAG_SERIALIZE |
+ IDE_HFLAG_NO_DMA | /* no SFF-style DMA */
+ IDE_HFLAG_NO_AUTOTUNE,
+   .mwdma_mask = ATA_MWDMA2,
+   .swdma_mask = ATA_SWDMA2,
+};
+


   Interesting... this driver's support for SWDMA0 is broken since the cycle 
should be 960 ns long, not 480, and SWDMA2 is underclocked using the same 
cycle as SWDMA1, 480 ns...



Index: b/drivers/ide/cris/ide-cris.c
===
--- a/drivers/ide/cris/ide-cris.c
+++ b/drivers/ide/cris/ide-cris.c
@@ -753,6 +753,15 @@ static void cris_set_dma_mode(ide_drive_
cris_ide_set_speed(TYPE_DMA, 0, strobe, hold);
 }
 
+static const struct ide_port_info cris_port_info __initdata = {

+   .chipset= ide_etrax100,
+   .host_flags = IDE_HFLAG_NO_ATAPI_DMA |
+ IDE_HFLAG_NO_DMA, /* no SFF-style DMA */
+   .pio_mask   = ATA_PIO4,
+   .udma_mask  = cris_ultra_mask,


   Hm, I wonder which value it will assume, 0x07 or 0?  Not sure even after 
looking at the source... :-)



+   .mwdma_mask = ATA_MWDMA2,
+};
+
 static int __init init_e100_ide(void)
 {
hw_regs_t hw;
Index: b/drivers/ide/ide-probe.c
===
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -1289,12 +1289,86 @@ static void hwif_register_devices(ide_hw
}
 }
 
-int ide_device_add_all(u8 *idx)

+static void ide_init_port(ide_hwif_t *hwif, unsigned int port,
+ const struct ide_port_info *d)
 {
-   ide_hwif_t *hwif;
+   if (d-chipset != ide_etrax100)
+   hwif-channel = port;


  Hm, what's so special about ide_etrax100?


+int ide_device_add_all(u8 idx[MAX_HWIFS], const struct ide_port_info *d)


   Function prototype doesn't match with one from linux/ide.h which has the 
first argument as a pointer...



+{
+   ide_hwif_t *hwif, *mate = NULL;
int i, rc = 0;
 
 	for (i = 0; i  MAX_HWIFS; i++) {

+   if (d == NULL || idx[i] == 0xff) {


   Why check for (d == NULL) every time and not do it once and break from the 
loop or even do it before the loop?



+   mate = NULL;
+   continue;
+   }
+
+   hwif = ide_hwifs[idx[i]];
+
+   if (d-chipset != ide_etrax100  (i  1)  mate) {
+   hwif-mate = mate;
+   mate-mate = hwif;
+   }
+
+   mate = (i  1) ? NULL : hwif;
+
+   ide_init_port(hwif, i  1, d);
+   }
+
+   for (i = 0; i  MAX_HWIFS; i++) {
if (idx[i] == 0xff)
continue;


MBR, 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