* [PATCH 1/3] [IDE] Add helper __ide_setup_pci_device()
2007-10-24 23:48 [PATCH] sound/oss/pss: set_io_base() always returns success, mark it void Jeff Garzik
@ 2007-10-24 23:48 ` Jeff Garzik
2007-10-25 19:33 ` Bartlomiej Zolnierkiewicz
2007-10-24 23:48 ` [PATCH 2/3] drivers/ide/pci/sc1200.c: remove pointless hwif lookup loop Jeff Garzik
` (6 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Jeff Garzik @ 2007-10-24 23:48 UTC (permalink / raw)
To: LKML, linux-ide; +Cc: akpm
Like ide_setup_pci_device(), except with additional array argument
'u8 *idx' that permits low-level driver to become aware of its assigned
hwifs.
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
drivers/ide/setup-pci.c | 17 ++++++++++++-----
include/linux/ide.h | 1 +
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
index 02d14bf..4960b9f 100644
--- a/drivers/ide/setup-pci.c
+++ b/drivers/ide/setup-pci.c
@@ -666,12 +666,10 @@ out:
return ret;
}
-int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
+int __ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d,
+ u8 *idx)
{
- u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
- int ret;
-
- ret = do_ide_setup_pci_device(dev, d, &idx[0], 1);
+ int ret = do_ide_setup_pci_device(dev, d, idx, 1);
if (ret >= 0)
ide_device_add(idx);
@@ -679,6 +677,15 @@ int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
return ret;
}
+EXPORT_SYMBOL_GPL(__ide_setup_pci_device);
+
+int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
+{
+ u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
+
+ return __ide_setup_pci_device(dev, d, &idx[0]);
+}
+
EXPORT_SYMBOL_GPL(ide_setup_pci_device);
int ide_setup_pci_devices(struct pci_dev *dev1, struct pci_dev *dev2,
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 4ed4777..3404fb9 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1244,6 +1244,7 @@ struct ide_port_info {
u8 udma_mask;
};
+int __ide_setup_pci_device(struct pci_dev *, const struct ide_port_info *, u8 *);
int ide_setup_pci_device(struct pci_dev *, const struct ide_port_info *);
int ide_setup_pci_devices(struct pci_dev *, struct pci_dev *, const struct ide_port_info *);
--
1.5.2.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 1/3] [IDE] Add helper __ide_setup_pci_device()
2007-10-24 23:48 ` [PATCH 1/3] [IDE] Add helper __ide_setup_pci_device() Jeff Garzik
@ 2007-10-25 19:33 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 23+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-10-25 19:33 UTC (permalink / raw)
To: Jeff Garzik; +Cc: LKML, linux-ide, akpm
[ The last few days were extra busy with pushing previously queued
IDE bits upstream but I'm back to reviewing the new stuff. ]
Thanks for splitting your patch on smaller chunks.
On Thursday 25 October 2007, Jeff Garzik wrote:
> Like ide_setup_pci_device(), except with additional array argument
> 'u8 *idx' that permits low-level driver to become aware of its assigned
> hwifs.
I would prefer to avoid exporting this information to host drivers
if possible (they shouldn't need to know about higher-layer decisions,
plus this change opens the door for various "creative" abuses).
In case of sc1200 host driver fixes (patches #2-3/3) it should be
possible to remove the need for the below patch and at the same time
simplify sc1200 code further (more details in review of patch #2/2).
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
> drivers/ide/setup-pci.c | 17 ++++++++++++-----
> include/linux/ide.h | 1 +
> 2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
> index 02d14bf..4960b9f 100644
> --- a/drivers/ide/setup-pci.c
> +++ b/drivers/ide/setup-pci.c
> @@ -666,12 +666,10 @@ out:
> return ret;
> }
>
> -int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
> +int __ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d,
> + u8 *idx)
> {
> - u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
> - int ret;
> -
> - ret = do_ide_setup_pci_device(dev, d, &idx[0], 1);
> + int ret = do_ide_setup_pci_device(dev, d, idx, 1);
>
> if (ret >= 0)
> ide_device_add(idx);
> @@ -679,6 +677,15 @@ int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
> return ret;
> }
>
> +EXPORT_SYMBOL_GPL(__ide_setup_pci_device);
> +
> +int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
> +{
> + u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
> +
> + return __ide_setup_pci_device(dev, d, &idx[0]);
> +}
> +
> EXPORT_SYMBOL_GPL(ide_setup_pci_device);
>
> int ide_setup_pci_devices(struct pci_dev *dev1, struct pci_dev *dev2,
> diff --git a/include/linux/ide.h b/include/linux/ide.h
> index 4ed4777..3404fb9 100644
> --- a/include/linux/ide.h
> +++ b/include/linux/ide.h
> @@ -1244,6 +1244,7 @@ struct ide_port_info {
> u8 udma_mask;
> };
>
> +int __ide_setup_pci_device(struct pci_dev *, const struct ide_port_info *, u8 *);
> int ide_setup_pci_device(struct pci_dev *, const struct ide_port_info *);
> int ide_setup_pci_devices(struct pci_dev *, struct pci_dev *, const struct ide_port_info *);
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/3] drivers/ide/pci/sc1200.c: remove pointless hwif lookup loop
2007-10-24 23:48 [PATCH] sound/oss/pss: set_io_base() always returns success, mark it void Jeff Garzik
2007-10-24 23:48 ` [PATCH 1/3] [IDE] Add helper __ide_setup_pci_device() Jeff Garzik
@ 2007-10-24 23:48 ` Jeff Garzik
2007-10-25 20:01 ` Bartlomiej Zolnierkiewicz
2007-10-24 23:48 ` [PATCH 3/3] drivers/ide/pci/sc1200.c: fix suspend/resume buglets and warnings Jeff Garzik
` (5 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Jeff Garzik @ 2007-10-24 23:48 UTC (permalink / raw)
To: LKML, linux-ide; +Cc: akpm
Store our hwif indices at probe time, in order to eliminate a needless
and ugly loop across all hwifs, searching for our pci device.
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
drivers/ide/pci/sc1200.c | 76 +++++++++++++++++++++++++++++++---------------
1 files changed, 51 insertions(+), 25 deletions(-)
diff --git a/drivers/ide/pci/sc1200.c b/drivers/ide/pci/sc1200.c
index d2c8b55..17e58d6 100644
--- a/drivers/ide/pci/sc1200.c
+++ b/drivers/ide/pci/sc1200.c
@@ -41,6 +41,8 @@
#define PCI_CLK_66 0x02
#define PCI_CLK_33A 0x03
+#define SC1200_IFS 4
+
static unsigned short sc1200_get_pci_clock (void)
{
unsigned char chip_id, silicon_revision;
@@ -261,31 +263,32 @@ static void sc1200_set_pio_mode(ide_drive_t *drive, const u8 pio)
}
#ifdef CONFIG_PM
-static ide_hwif_t *lookup_pci_dev (ide_hwif_t *prev, struct pci_dev *dev)
-{
- int h;
-
- for (h = 0; h < MAX_HWIFS; h++) {
- ide_hwif_t *hwif = &ide_hwifs[h];
- if (prev) {
- if (hwif == prev)
- prev = NULL; // found previous, now look for next match
- } else {
- if (hwif && hwif->pci_dev == dev)
- return hwif; // found next match
- }
- }
- return NULL; // not found
-}
-
typedef struct sc1200_saved_state_s {
__u32 regs[4];
} sc1200_saved_state_t;
+static unsigned int pack_hwif_idx(u8 *idx)
+{
+ return (((unsigned int) idx[0]) << 0) |
+ (((unsigned int) idx[1]) << 8) |
+ (((unsigned int) idx[2]) << 16) |
+ (((unsigned int) idx[3]) << 24);
+}
+
+static ide_hwif_t *sc1200_hwif(struct pci_dev *pdev, unsigned int iface)
+{
+ unsigned int packed_hwifs, idx;
+
+ packed_hwifs = (unsigned long) pci_get_drvdata(pdev);
+ idx = (packed_hwifs >> (iface * 8)) & 0xff;
+
+ return (idx == 0xff) ? NULL : &ide_hwifs[idx];
+}
static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
{
- ide_hwif_t *hwif = NULL;
+ ide_hwif_t *hwif;
+ int i;
printk("SC1200: suspend(%u)\n", state.event);
@@ -295,9 +298,14 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
//
// Loop over all interfaces that are part of this PCI device:
//
- while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
+ for (i = 0; i < SC1200_IFS; i++) {
sc1200_saved_state_t *ss;
unsigned int basereg, r;
+
+ hwif = sc1200_hwif(dev, i);
+ if (!hwif)
+ continue;
+
//
// allocate a permanent save area, if not already allocated
//
@@ -310,7 +318,7 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
}
ss = (sc1200_saved_state_t *)hwif->config_data;
//
- // Save timing registers: this may be unnecessary if
+ // Save timing registers: this may be unnecessary if
// BIOS also does it
//
basereg = hwif->channel ? 0x50 : 0x40;
@@ -320,7 +328,7 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
}
}
- /* You don't need to iterate over disks -- sysfs should have done that for you already */
+ /* You don't need to iterate over disks -- sysfs should have done that for you already */
pci_disable_device(dev);
pci_set_power_state(dev, pci_choose_state(dev, state));
@@ -330,7 +338,8 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
static int sc1200_resume (struct pci_dev *dev)
{
- ide_hwif_t *hwif = NULL;
+ ide_hwif_t *hwif;
+ int i;
pci_set_power_state(dev, PCI_D0); // bring chip back from sleep state
dev->current_state = PM_EVENT_ON;
@@ -338,9 +347,15 @@ static int sc1200_resume (struct pci_dev *dev)
//
// loop over all interfaces that are part of this pci device:
//
- while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
+ for (i = 0; i < SC1200_IFS; i++) {
unsigned int basereg, r;
- sc1200_saved_state_t *ss = (sc1200_saved_state_t *)hwif->config_data;
+ sc1200_saved_state_t *ss;
+
+ hwif = sc1200_hwif(dev, i);
+ if (!hwif)
+ continue;
+
+ ss = (sc1200_saved_state_t *)hwif->config_data;
//
// Restore timing registers: this may be unnecessary if BIOS also does it
@@ -386,7 +401,18 @@ static const struct ide_port_info sc1200_chipset __devinitdata = {
static int __devinit sc1200_init_one(struct pci_dev *dev, const struct pci_device_id *id)
{
- return ide_setup_pci_device(dev, &sc1200_chipset);
+ unsigned int packed_hwifs;
+ int rc;
+ u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
+
+ rc = __ide_setup_pci_device(dev, &sc1200_chipset, &idx[0]);
+ if (rc)
+ return rc;
+
+ packed_hwifs = pack_hwif_idx(&idx[0]);
+
+ pci_set_drvdata(dev, (void *)(unsigned long) packed_hwifs);
+ return 0;
}
static const struct pci_device_id sc1200_pci_tbl[] = {
--
1.5.2.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 2/3] drivers/ide/pci/sc1200.c: remove pointless hwif lookup loop
2007-10-24 23:48 ` [PATCH 2/3] drivers/ide/pci/sc1200.c: remove pointless hwif lookup loop Jeff Garzik
@ 2007-10-25 20:01 ` Bartlomiej Zolnierkiewicz
2007-10-26 1:25 ` Jeff Garzik
0 siblings, 1 reply; 23+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-10-25 20:01 UTC (permalink / raw)
To: Jeff Garzik; +Cc: LKML, linux-ide, akpm
On Thursday 25 October 2007, Jeff Garzik wrote:
> Store our hwif indices at probe time, in order to eliminate a needless
> and ugly loop across all hwifs, searching for our pci device.
It seems that we can simplify it even further and remove knowledge about
hwifs altogether from suspend/resume methods.
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
> drivers/ide/pci/sc1200.c | 76 +++++++++++++++++++++++++++++++---------------
> 1 files changed, 51 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/ide/pci/sc1200.c b/drivers/ide/pci/sc1200.c
> index d2c8b55..17e58d6 100644
> --- a/drivers/ide/pci/sc1200.c
> +++ b/drivers/ide/pci/sc1200.c
> @@ -41,6 +41,8 @@
> #define PCI_CLK_66 0x02
> #define PCI_CLK_33A 0x03
>
> +#define SC1200_IFS 4
> +
> static unsigned short sc1200_get_pci_clock (void)
> {
> unsigned char chip_id, silicon_revision;
> @@ -261,31 +263,32 @@ static void sc1200_set_pio_mode(ide_drive_t *drive, const u8 pio)
> }
>
> #ifdef CONFIG_PM
> -static ide_hwif_t *lookup_pci_dev (ide_hwif_t *prev, struct pci_dev *dev)
> -{
> - int h;
> -
> - for (h = 0; h < MAX_HWIFS; h++) {
> - ide_hwif_t *hwif = &ide_hwifs[h];
> - if (prev) {
> - if (hwif == prev)
> - prev = NULL; // found previous, now look for next match
> - } else {
> - if (hwif && hwif->pci_dev == dev)
> - return hwif; // found next match
> - }
> - }
> - return NULL; // not found
> -}
> -
> typedef struct sc1200_saved_state_s {
> __u32 regs[4];
> } sc1200_saved_state_t;
>
> +static unsigned int pack_hwif_idx(u8 *idx)
> +{
> + return (((unsigned int) idx[0]) << 0) |
> + (((unsigned int) idx[1]) << 8) |
> + (((unsigned int) idx[2]) << 16) |
> + (((unsigned int) idx[3]) << 24);
> +}
> +
> +static ide_hwif_t *sc1200_hwif(struct pci_dev *pdev, unsigned int iface)
> +{
> + unsigned int packed_hwifs, idx;
> +
> + packed_hwifs = (unsigned long) pci_get_drvdata(pdev);
> + idx = (packed_hwifs >> (iface * 8)) & 0xff;
> +
> + return (idx == 0xff) ? NULL : &ide_hwifs[idx];
> +}
>
> static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
> {
> - ide_hwif_t *hwif = NULL;
> + ide_hwif_t *hwif;
> + int i;
>
> printk("SC1200: suspend(%u)\n", state.event);
>
> @@ -295,9 +298,14 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
> //
> // Loop over all interfaces that are part of this PCI device:
> //
> - while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
> + for (i = 0; i < SC1200_IFS; i++) {
> sc1200_saved_state_t *ss;
> unsigned int basereg, r;
> +
> + hwif = sc1200_hwif(dev, i);
> + if (!hwif)
> + continue;
> +
> //
> // allocate a permanent save area, if not already allocated
> //
> @@ -310,7 +318,7 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
> }
> ss = (sc1200_saved_state_t *)hwif->config_data;
> //
> - // Save timing registers: this may be unnecessary if
> + // Save timing registers: this may be unnecessary if
> // BIOS also does it
> //
> basereg = hwif->channel ? 0x50 : 0x40;
Please take a close look at the line above and the next three lines:
for (r = 0; r < 4; ++r) {
pci_read_config_dword (hwif->pci_dev, basereg + (r<<2), &ss->regs[r]);
}
It is highly obfuscated but the sc1200_suspend() reads 16 bytes from
the offset 0x40 (for the primary port) and puts them in the corresponding
struct sc1200_saved_state_s buffer, then it reads another 16 bytes from the
offset 0x50 (for the secondary port) and puts it in the another buffer.
In summary sc1200_suspend() reads 32 continuous bytes from offset 0x40
and the exactly reverse operation happens in sc1200_resume().
Given that and the fact that struct sc1200_save_state_s buffers are used
_only_ by sc1200_{suspend,resume}() we may safely convert the code to use
one buffer for both ports (the whole PCI device). We just need to bump
the size of struct sc1200_saved_state_s (from 4 to 8 double-words) and use
pci_{get,set}_drvdata() instead of hwif->config_data. Then we can remove
looping over interfaces completely from sc1200_{suspend,resume}()! :)
> @@ -320,7 +328,7 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
> }
> }
>
> - /* You don't need to iterate over disks -- sysfs should have done that for you already */
> + /* You don't need to iterate over disks -- sysfs should have done that for you already */
>
> pci_disable_device(dev);
> pci_set_power_state(dev, pci_choose_state(dev, state));
> @@ -330,7 +338,8 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
>
> static int sc1200_resume (struct pci_dev *dev)
> {
> - ide_hwif_t *hwif = NULL;
> + ide_hwif_t *hwif;
> + int i;
>
> pci_set_power_state(dev, PCI_D0); // bring chip back from sleep state
> dev->current_state = PM_EVENT_ON;
> @@ -338,9 +347,15 @@ static int sc1200_resume (struct pci_dev *dev)
> //
> // loop over all interfaces that are part of this pci device:
> //
> - while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
> + for (i = 0; i < SC1200_IFS; i++) {
> unsigned int basereg, r;
> - sc1200_saved_state_t *ss = (sc1200_saved_state_t *)hwif->config_data;
> + sc1200_saved_state_t *ss;
> +
> + hwif = sc1200_hwif(dev, i);
> + if (!hwif)
> + continue;
> +
> + ss = (sc1200_saved_state_t *)hwif->config_data;
>
> //
> // Restore timing registers: this may be unnecessary if BIOS also does it
> @@ -386,7 +401,18 @@ static const struct ide_port_info sc1200_chipset __devinitdata = {
>
> static int __devinit sc1200_init_one(struct pci_dev *dev, const struct pci_device_id *id)
> {
> - return ide_setup_pci_device(dev, &sc1200_chipset);
> + unsigned int packed_hwifs;
> + int rc;
> + u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
> +
> + rc = __ide_setup_pci_device(dev, &sc1200_chipset, &idx[0]);
> + if (rc)
> + return rc;
> +
> + packed_hwifs = pack_hwif_idx(&idx[0]);
> +
> + pci_set_drvdata(dev, (void *)(unsigned long) packed_hwifs);
> + return 0;
> }
>
> static const struct pci_device_id sc1200_pci_tbl[] = {
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/3] drivers/ide/pci/sc1200.c: remove pointless hwif lookup loop
2007-10-25 20:01 ` Bartlomiej Zolnierkiewicz
@ 2007-10-26 1:25 ` Jeff Garzik
2007-10-31 21:53 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 23+ messages in thread
From: Jeff Garzik @ 2007-10-26 1:25 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: LKML, linux-ide, akpm
Bartlomiej Zolnierkiewicz wrote:
> On Thursday 25 October 2007, Jeff Garzik wrote:
>> Store our hwif indices at probe time, in order to eliminate a needless
>> and ugly loop across all hwifs, searching for our pci device.
>
> It seems that we can simplify it even further and remove knowledge about
> hwifs altogether from suspend/resume methods.
>
>> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
>> ---
>> drivers/ide/pci/sc1200.c | 76 +++++++++++++++++++++++++++++++---------------
>> 1 files changed, 51 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/ide/pci/sc1200.c b/drivers/ide/pci/sc1200.c
>> index d2c8b55..17e58d6 100644
>> --- a/drivers/ide/pci/sc1200.c
>> +++ b/drivers/ide/pci/sc1200.c
>> @@ -41,6 +41,8 @@
>> #define PCI_CLK_66 0x02
>> #define PCI_CLK_33A 0x03
>>
>> +#define SC1200_IFS 4
>> +
>> static unsigned short sc1200_get_pci_clock (void)
>> {
>> unsigned char chip_id, silicon_revision;
>> @@ -261,31 +263,32 @@ static void sc1200_set_pio_mode(ide_drive_t *drive, const u8 pio)
>> }
>>
>> #ifdef CONFIG_PM
>> -static ide_hwif_t *lookup_pci_dev (ide_hwif_t *prev, struct pci_dev *dev)
>> -{
>> - int h;
>> -
>> - for (h = 0; h < MAX_HWIFS; h++) {
>> - ide_hwif_t *hwif = &ide_hwifs[h];
>> - if (prev) {
>> - if (hwif == prev)
>> - prev = NULL; // found previous, now look for next match
>> - } else {
>> - if (hwif && hwif->pci_dev == dev)
>> - return hwif; // found next match
>> - }
>> - }
>> - return NULL; // not found
>> -}
>> -
>> typedef struct sc1200_saved_state_s {
>> __u32 regs[4];
>> } sc1200_saved_state_t;
>>
>> +static unsigned int pack_hwif_idx(u8 *idx)
>> +{
>> + return (((unsigned int) idx[0]) << 0) |
>> + (((unsigned int) idx[1]) << 8) |
>> + (((unsigned int) idx[2]) << 16) |
>> + (((unsigned int) idx[3]) << 24);
>> +}
>> +
>> +static ide_hwif_t *sc1200_hwif(struct pci_dev *pdev, unsigned int iface)
>> +{
>> + unsigned int packed_hwifs, idx;
>> +
>> + packed_hwifs = (unsigned long) pci_get_drvdata(pdev);
>> + idx = (packed_hwifs >> (iface * 8)) & 0xff;
>> +
>> + return (idx == 0xff) ? NULL : &ide_hwifs[idx];
>> +}
>>
>> static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
>> {
>> - ide_hwif_t *hwif = NULL;
>> + ide_hwif_t *hwif;
>> + int i;
>>
>> printk("SC1200: suspend(%u)\n", state.event);
>>
>> @@ -295,9 +298,14 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
>> //
>> // Loop over all interfaces that are part of this PCI device:
>> //
>> - while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
>> + for (i = 0; i < SC1200_IFS; i++) {
>> sc1200_saved_state_t *ss;
>> unsigned int basereg, r;
>> +
>> + hwif = sc1200_hwif(dev, i);
>> + if (!hwif)
>> + continue;
>> +
>> //
>> // allocate a permanent save area, if not already allocated
>> //
>> @@ -310,7 +318,7 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
>> }
>> ss = (sc1200_saved_state_t *)hwif->config_data;
>> //
>> - // Save timing registers: this may be unnecessary if
>> + // Save timing registers: this may be unnecessary if
>> // BIOS also does it
>> //
>> basereg = hwif->channel ? 0x50 : 0x40;
>
> Please take a close look at the line above and the next three lines:
>
> for (r = 0; r < 4; ++r) {
> pci_read_config_dword (hwif->pci_dev, basereg + (r<<2), &ss->regs[r]);
> }
>
> It is highly obfuscated but the sc1200_suspend() reads 16 bytes from
> the offset 0x40 (for the primary port) and puts them in the corresponding
> struct sc1200_saved_state_s buffer, then it reads another 16 bytes from the
> offset 0x50 (for the secondary port) and puts it in the another buffer.
>
> In summary sc1200_suspend() reads 32 continuous bytes from offset 0x40
> and the exactly reverse operation happens in sc1200_resume().
>
> Given that and the fact that struct sc1200_save_state_s buffers are used
> _only_ by sc1200_{suspend,resume}() we may safely convert the code to use
> one buffer for both ports (the whole PCI device). We just need to bump
> the size of struct sc1200_saved_state_s (from 4 to 8 double-words) and use
> pci_{get,set}_drvdata() instead of hwif->config_data. Then we can remove
> looping over interfaces completely from sc1200_{suspend,resume}()! :)
May I assume you'll handle that task? :) It sounds like you have a far
better idea than mine, and my main goal -- fixing bugs and warning -- is
accomplished anyway with the merging of patch #3.
Jeff
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/3] drivers/ide/pci/sc1200.c: remove pointless hwif lookup loop
2007-10-26 1:25 ` Jeff Garzik
@ 2007-10-31 21:53 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 23+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-10-31 21:53 UTC (permalink / raw)
To: Jeff Garzik; +Cc: LKML, linux-ide, akpm
On Friday 26 October 2007, Jeff Garzik wrote:
> Bartlomiej Zolnierkiewicz wrote:
> > On Thursday 25 October 2007, Jeff Garzik wrote:
> >> Store our hwif indices at probe time, in order to eliminate a needless
> >> and ugly loop across all hwifs, searching for our pci device.
> >
> > It seems that we can simplify it even further and remove knowledge about
> > hwifs altogether from suspend/resume methods.
> >
> >> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> >> ---
> >> drivers/ide/pci/sc1200.c | 76 +++++++++++++++++++++++++++++++---------------
> >> 1 files changed, 51 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/drivers/ide/pci/sc1200.c b/drivers/ide/pci/sc1200.c
> >> index d2c8b55..17e58d6 100644
> >> --- a/drivers/ide/pci/sc1200.c
> >> +++ b/drivers/ide/pci/sc1200.c
> >> @@ -41,6 +41,8 @@
> >> #define PCI_CLK_66 0x02
> >> #define PCI_CLK_33A 0x03
> >>
> >> +#define SC1200_IFS 4
> >> +
> >> static unsigned short sc1200_get_pci_clock (void)
> >> {
> >> unsigned char chip_id, silicon_revision;
> >> @@ -261,31 +263,32 @@ static void sc1200_set_pio_mode(ide_drive_t *drive, const u8 pio)
> >> }
> >>
> >> #ifdef CONFIG_PM
> >> -static ide_hwif_t *lookup_pci_dev (ide_hwif_t *prev, struct pci_dev *dev)
> >> -{
> >> - int h;
> >> -
> >> - for (h = 0; h < MAX_HWIFS; h++) {
> >> - ide_hwif_t *hwif = &ide_hwifs[h];
> >> - if (prev) {
> >> - if (hwif == prev)
> >> - prev = NULL; // found previous, now look for next match
> >> - } else {
> >> - if (hwif && hwif->pci_dev == dev)
> >> - return hwif; // found next match
> >> - }
> >> - }
> >> - return NULL; // not found
> >> -}
> >> -
> >> typedef struct sc1200_saved_state_s {
> >> __u32 regs[4];
> >> } sc1200_saved_state_t;
> >>
> >> +static unsigned int pack_hwif_idx(u8 *idx)
> >> +{
> >> + return (((unsigned int) idx[0]) << 0) |
> >> + (((unsigned int) idx[1]) << 8) |
> >> + (((unsigned int) idx[2]) << 16) |
> >> + (((unsigned int) idx[3]) << 24);
> >> +}
> >> +
> >> +static ide_hwif_t *sc1200_hwif(struct pci_dev *pdev, unsigned int iface)
> >> +{
> >> + unsigned int packed_hwifs, idx;
> >> +
> >> + packed_hwifs = (unsigned long) pci_get_drvdata(pdev);
> >> + idx = (packed_hwifs >> (iface * 8)) & 0xff;
> >> +
> >> + return (idx == 0xff) ? NULL : &ide_hwifs[idx];
> >> +}
> >>
> >> static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
> >> {
> >> - ide_hwif_t *hwif = NULL;
> >> + ide_hwif_t *hwif;
> >> + int i;
> >>
> >> printk("SC1200: suspend(%u)\n", state.event);
> >>
> >> @@ -295,9 +298,14 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
> >> //
> >> // Loop over all interfaces that are part of this PCI device:
> >> //
> >> - while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
> >> + for (i = 0; i < SC1200_IFS; i++) {
> >> sc1200_saved_state_t *ss;
> >> unsigned int basereg, r;
> >> +
> >> + hwif = sc1200_hwif(dev, i);
> >> + if (!hwif)
> >> + continue;
> >> +
> >> //
> >> // allocate a permanent save area, if not already allocated
> >> //
> >> @@ -310,7 +318,7 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
> >> }
> >> ss = (sc1200_saved_state_t *)hwif->config_data;
> >> //
> >> - // Save timing registers: this may be unnecessary if
> >> + // Save timing registers: this may be unnecessary if
> >> // BIOS also does it
> >> //
> >> basereg = hwif->channel ? 0x50 : 0x40;
> >
> > Please take a close look at the line above and the next three lines:
> >
> > for (r = 0; r < 4; ++r) {
> > pci_read_config_dword (hwif->pci_dev, basereg + (r<<2), &ss->regs[r]);
> > }
> >
> > It is highly obfuscated but the sc1200_suspend() reads 16 bytes from
> > the offset 0x40 (for the primary port) and puts them in the corresponding
> > struct sc1200_saved_state_s buffer, then it reads another 16 bytes from the
> > offset 0x50 (for the secondary port) and puts it in the another buffer.
> >
> > In summary sc1200_suspend() reads 32 continuous bytes from offset 0x40
> > and the exactly reverse operation happens in sc1200_resume().
> >
> > Given that and the fact that struct sc1200_save_state_s buffers are used
> > _only_ by sc1200_{suspend,resume}() we may safely convert the code to use
> > one buffer for both ports (the whole PCI device). We just need to bump
> > the size of struct sc1200_saved_state_s (from 4 to 8 double-words) and use
> > pci_{get,set}_drvdata() instead of hwif->config_data. Then we can remove
> > looping over interfaces completely from sc1200_{suspend,resume}()! :)
>
> May I assume you'll handle that task? :) It sounds like you have a far
> better idea than mine, and my main goal -- fixing bugs and warning -- is
> accomplished anyway with the merging of patch #3.
Uh, OK...
[PATCH] sc1200: remove pointless hwif lookup loop
Save PCI regs values for both IDE ports in one buffer, in order to eliminate
a needless and ugly loop across all hwifs, searching for our PCI device.
Partially based on the previous patch by Jeff Garzik.
Cc: Jeff Garzik <jeff@garzik.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/pci/sc1200.c | 106 ++++++++++++++++-------------------------------
1 file changed, 37 insertions(+), 69 deletions(-)
Index: b/drivers/ide/pci/sc1200.c
===================================================================
--- a/drivers/ide/pci/sc1200.c
+++ b/drivers/ide/pci/sc1200.c
@@ -261,66 +261,39 @@ static void sc1200_set_pio_mode(ide_driv
}
#ifdef CONFIG_PM
-static ide_hwif_t *lookup_pci_dev (ide_hwif_t *prev, struct pci_dev *dev)
-{
- int h;
-
- for (h = 0; h < MAX_HWIFS; h++) {
- ide_hwif_t *hwif = &ide_hwifs[h];
- if (prev) {
- if (hwif == prev)
- prev = NULL; // found previous, now look for next match
- } else {
- if (hwif && hwif->pci_dev == dev)
- return hwif; // found next match
- }
- }
- return NULL; // not found
-}
-
-typedef struct sc1200_saved_state_s {
- __u32 regs[4];
-} sc1200_saved_state_t;
-
+struct sc1200_saved_state {
+ u32 regs[8];
+};
static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
{
- ide_hwif_t *hwif = NULL;
-
printk("SC1200: suspend(%u)\n", state.event);
+ /*
+ * we only save state when going from full power to less
+ */
if (state.event == PM_EVENT_ON) {
- // we only save state when going from full power to less
+ struct sc1200_saved_state *ss;
+ unsigned int r;
- //
- // Loop over all interfaces that are part of this PCI device:
- //
- while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
- sc1200_saved_state_t *ss;
- unsigned int basereg, r;
- //
- // allocate a permanent save area, if not already allocated
- //
- ss = (sc1200_saved_state_t *)hwif->config_data;
- if (ss == NULL) {
- ss = kmalloc(sizeof(sc1200_saved_state_t), GFP_KERNEL);
- if (ss == NULL)
- return -ENOMEM;
- hwif->config_data = (unsigned long)ss;
- }
- ss = (sc1200_saved_state_t *)hwif->config_data;
- //
- // Save timing registers: this may be unnecessary if
- // BIOS also does it
- //
- basereg = hwif->channel ? 0x50 : 0x40;
- for (r = 0; r < 4; ++r) {
- pci_read_config_dword (hwif->pci_dev, basereg + (r<<2), &ss->regs[r]);
- }
+ /*
+ * allocate a permanent save area, if not already allocated
+ */
+ ss = (struct sc1200_saved_state *)pci_get_drvdata(dev);
+ if (ss == NULL) {
+ ss = kmalloc(sizeof(*ss), GFP_KERNEL);
+ if (ss == NULL)
+ return -ENOMEM;
+ pci_set_drvdata(dev, ss);
}
- }
- /* You don't need to iterate over disks -- sysfs should have done that for you already */
+ /*
+ * save timing registers
+ * (this may be unnecessary if BIOS also does it)
+ */
+ for (r = 0; r < 8; r++)
+ pci_read_config_dword(dev, 0x40 + r * 4, &ss->regs[r]);
+ }
pci_disable_device(dev);
pci_set_power_state(dev, pci_choose_state(dev, state));
@@ -329,30 +302,25 @@ static int sc1200_suspend (struct pci_de
static int sc1200_resume (struct pci_dev *dev)
{
- ide_hwif_t *hwif = NULL;
- int i;
+ struct sc1200_saved_state *ss;
+ unsigned int r;
+ int i;
i = pci_enable_device(dev);
if (i)
return i;
- //
- // loop over all interfaces that are part of this pci device:
- //
- while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
- unsigned int basereg, r;
- sc1200_saved_state_t *ss = (sc1200_saved_state_t *)hwif->config_data;
-
- //
- // Restore timing registers: this may be unnecessary if BIOS also does it
- //
- basereg = hwif->channel ? 0x50 : 0x40;
- if (ss != NULL) {
- for (r = 0; r < 4; ++r) {
- pci_write_config_dword(hwif->pci_dev, basereg + (r<<2), ss->regs[r]);
- }
- }
+ ss = (struct sc1200_saved_state *)pci_get_drvdata(dev);
+
+ /*
+ * restore timing registers
+ * (this may be unnecessary if BIOS also does it)
+ */
+ if (ss) {
+ for (r = 0; r < 8; r++)
+ pci_write_config_dword(dev, 0x40 + r * 4, ss->regs[r]);
}
+
return 0;
}
#endif
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/3] drivers/ide/pci/sc1200.c: fix suspend/resume buglets and warnings
2007-10-24 23:48 [PATCH] sound/oss/pss: set_io_base() always returns success, mark it void Jeff Garzik
2007-10-24 23:48 ` [PATCH 1/3] [IDE] Add helper __ide_setup_pci_device() Jeff Garzik
2007-10-24 23:48 ` [PATCH 2/3] drivers/ide/pci/sc1200.c: remove pointless hwif lookup loop Jeff Garzik
@ 2007-10-24 23:48 ` Jeff Garzik
2007-10-25 20:10 ` Bartlomiej Zolnierkiewicz
2007-10-24 23:48 ` [PATCH 1/4] [SCSI] ips: remove ips_ha members that duplicate struct pci_dev members Jeff Garzik
` (4 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Jeff Garzik @ 2007-10-24 23:48 UTC (permalink / raw)
To: LKML, linux-ide; +Cc: akpm
* We shouldn't bother with dev->current_state, the PCI API functions we
call manage this for us (and do a far better job at it too).
* Remove pci_set_power_state(dev, PCI_D0) call in resume, as
pci_enable_device() does the same thing.
* Check pci_enable_device() return value. If it failed, fail
the entire resume and avoid programming timings into the [potentially
dead/asleep] chip.
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
drivers/ide/pci/sc1200.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/ide/pci/sc1200.c b/drivers/ide/pci/sc1200.c
index 17e58d6..10c79a5 100644
--- a/drivers/ide/pci/sc1200.c
+++ b/drivers/ide/pci/sc1200.c
@@ -332,7 +332,6 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
pci_disable_device(dev);
pci_set_power_state(dev, pci_choose_state(dev, state));
- dev->current_state = state.event;
return 0;
}
@@ -341,9 +340,10 @@ static int sc1200_resume (struct pci_dev *dev)
ide_hwif_t *hwif;
int i;
- pci_set_power_state(dev, PCI_D0); // bring chip back from sleep state
- dev->current_state = PM_EVENT_ON;
- pci_enable_device(dev);
+ i = pci_enable_device(dev);
+ if (i)
+ return i;
+
//
// loop over all interfaces that are part of this pci device:
//
--
1.5.2.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 3/3] drivers/ide/pci/sc1200.c: fix suspend/resume buglets and warnings
2007-10-24 23:48 ` [PATCH 3/3] drivers/ide/pci/sc1200.c: fix suspend/resume buglets and warnings Jeff Garzik
@ 2007-10-25 20:10 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 23+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-10-25 20:10 UTC (permalink / raw)
To: Jeff Garzik; +Cc: LKML, linux-ide, akpm
On Thursday 25 October 2007, Jeff Garzik wrote:
> * We shouldn't bother with dev->current_state, the PCI API functions we
> call manage this for us (and do a far better job at it too).
>
> * Remove pci_set_power_state(dev, PCI_D0) call in resume, as
> pci_enable_device() does the same thing.
>
> * Check pci_enable_device() return value. If it failed, fail
> the entire resume and avoid programming timings into the [potentially
> dead/asleep] chip.
>
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
applied, thanks
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/4] [SCSI] ips: remove ips_ha members that duplicate struct pci_dev members
2007-10-24 23:48 [PATCH] sound/oss/pss: set_io_base() always returns success, mark it void Jeff Garzik
` (2 preceding siblings ...)
2007-10-24 23:48 ` [PATCH 3/3] drivers/ide/pci/sc1200.c: fix suspend/resume buglets and warnings Jeff Garzik
@ 2007-10-24 23:48 ` Jeff Garzik
2007-10-25 4:27 ` Andrew Morton
2007-10-25 14:32 ` Salyzyn, Mark
2007-10-24 23:48 ` [PATCH 2/4] [SCSI] ips: trim trailing whitespace Jeff Garzik
` (3 subsequent siblings)
7 siblings, 2 replies; 23+ messages in thread
From: Jeff Garzik @ 2007-10-24 23:48 UTC (permalink / raw)
To: LKML, linux-scsi; +Cc: akpm
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
drivers/scsi/ips.c | 178 ++++++++++++++++++++++++----------------------------
drivers/scsi/ips.h | 20 +++----
2 files changed, 91 insertions(+), 107 deletions(-)
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index 5c5a9b2..595a91a 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -707,7 +707,7 @@ ips_release(struct Scsi_Host *sh)
release_region(ha->io_addr, ha->io_len);
/* free IRQ */
- free_irq(ha->irq, ha);
+ free_irq(ha->pcidev->irq, ha);
scsi_host_put(sh);
@@ -1637,7 +1637,7 @@ ips_make_passthru(ips_ha_t *ha, struct scsi_cmnd *SC, ips_scb_t *scb, int intr)
return (IPS_FAILURE);
}
- if (ha->device_id == IPS_DEVICEID_COPPERHEAD &&
+ if (ha->pcidev->device == IPS_DEVICEID_COPPERHEAD &&
pt->CoppCP.cmd.flashfw.op_code ==
IPS_CMD_RW_BIOSFW) {
ret = ips_flash_copperhead(ha, pt, scb);
@@ -2021,7 +2021,7 @@ ips_cleanup_passthru(ips_ha_t * ha, ips_scb_t * scb)
pt->ExtendedStatus = scb->extended_status;
pt->AdapterType = ha->ad_type;
- if (ha->device_id == IPS_DEVICEID_COPPERHEAD &&
+ if (ha->pcidev->device == IPS_DEVICEID_COPPERHEAD &&
(scb->cmd.flashfw.op_code == IPS_CMD_DOWNLOAD ||
scb->cmd.flashfw.op_code == IPS_CMD_RW_BIOSFW))
ips_free_flash_copperhead(ha);
@@ -2075,7 +2075,7 @@ ips_host_info(ips_ha_t * ha, char *ptr, off_t offset, int len)
ha->mem_ptr);
}
- copy_info(&info, "\tIRQ number : %d\n", ha->irq);
+ copy_info(&info, "\tIRQ number : %d\n", ha->pcidev->irq);
/* For the Next 3 lines Check for Binary 0 at the end and don't include it if it's there. */
/* That keeps everything happy for "text" operations on the proc file. */
@@ -2232,31 +2232,31 @@ ips_identify_controller(ips_ha_t * ha)
{
METHOD_TRACE("ips_identify_controller", 1);
- switch (ha->device_id) {
+ switch (ha->pcidev->device) {
case IPS_DEVICEID_COPPERHEAD:
- if (ha->revision_id <= IPS_REVID_SERVERAID) {
+ if (ha->pcidev->revision <= IPS_REVID_SERVERAID) {
ha->ad_type = IPS_ADTYPE_SERVERAID;
- } else if (ha->revision_id == IPS_REVID_SERVERAID2) {
+ } else if (ha->pcidev->revision == IPS_REVID_SERVERAID2) {
ha->ad_type = IPS_ADTYPE_SERVERAID2;
- } else if (ha->revision_id == IPS_REVID_NAVAJO) {
+ } else if (ha->pcidev->revision == IPS_REVID_NAVAJO) {
ha->ad_type = IPS_ADTYPE_NAVAJO;
- } else if ((ha->revision_id == IPS_REVID_SERVERAID2)
+ } else if ((ha->pcidev->revision == IPS_REVID_SERVERAID2)
&& (ha->slot_num == 0)) {
ha->ad_type = IPS_ADTYPE_KIOWA;
- } else if ((ha->revision_id >= IPS_REVID_CLARINETP1) &&
- (ha->revision_id <= IPS_REVID_CLARINETP3)) {
+ } else if ((ha->pcidev->revision >= IPS_REVID_CLARINETP1) &&
+ (ha->pcidev->revision <= IPS_REVID_CLARINETP3)) {
if (ha->enq->ucMaxPhysicalDevices == 15)
ha->ad_type = IPS_ADTYPE_SERVERAID3L;
else
ha->ad_type = IPS_ADTYPE_SERVERAID3;
- } else if ((ha->revision_id >= IPS_REVID_TROMBONE32) &&
- (ha->revision_id <= IPS_REVID_TROMBONE64)) {
+ } else if ((ha->pcidev->revision >= IPS_REVID_TROMBONE32) &&
+ (ha->pcidev->revision <= IPS_REVID_TROMBONE64)) {
ha->ad_type = IPS_ADTYPE_SERVERAID4H;
}
break;
case IPS_DEVICEID_MORPHEUS:
- switch (ha->subdevice_id) {
+ switch (ha->pcidev->subsystem_device) {
case IPS_SUBDEVICEID_4L:
ha->ad_type = IPS_ADTYPE_SERVERAID4L;
break;
@@ -2285,7 +2285,7 @@ ips_identify_controller(ips_ha_t * ha)
break;
case IPS_DEVICEID_MARCO:
- switch (ha->subdevice_id) {
+ switch (ha->pcidev->subsystem_device) {
case IPS_SUBDEVICEID_6M:
ha->ad_type = IPS_ADTYPE_SERVERAID6M;
break;
@@ -2332,20 +2332,20 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
strncpy(ha->bios_version, " ?", 8);
- if (ha->device_id == IPS_DEVICEID_COPPERHEAD) {
+ if (ha->pcidev->device == IPS_DEVICEID_COPPERHEAD) {
if (IPS_USE_MEMIO(ha)) {
/* Memory Mapped I/O */
/* test 1st byte */
writel(0, ha->mem_ptr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
if (readb(ha->mem_ptr + IPS_REG_FLDP) != 0x55)
return;
writel(1, ha->mem_ptr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
if (readb(ha->mem_ptr + IPS_REG_FLDP) != 0xAA)
@@ -2353,20 +2353,20 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
/* Get Major version */
writel(0x1FF, ha->mem_ptr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
major = readb(ha->mem_ptr + IPS_REG_FLDP);
/* Get Minor version */
writel(0x1FE, ha->mem_ptr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
minor = readb(ha->mem_ptr + IPS_REG_FLDP);
/* Get SubMinor version */
writel(0x1FD, ha->mem_ptr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
subminor = readb(ha->mem_ptr + IPS_REG_FLDP);
@@ -2375,14 +2375,14 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
/* test 1st byte */
outl(0, ha->io_addr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
if (inb(ha->io_addr + IPS_REG_FLDP) != 0x55)
return;
outl(cpu_to_le32(1), ha->io_addr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
if (inb(ha->io_addr + IPS_REG_FLDP) != 0xAA)
@@ -2390,21 +2390,21 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
/* Get Major version */
outl(cpu_to_le32(0x1FF), ha->io_addr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
major = inb(ha->io_addr + IPS_REG_FLDP);
/* Get Minor version */
outl(cpu_to_le32(0x1FE), ha->io_addr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
minor = inb(ha->io_addr + IPS_REG_FLDP);
/* Get SubMinor version */
outl(cpu_to_le32(0x1FD), ha->io_addr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
subminor = inb(ha->io_addr + IPS_REG_FLDP);
@@ -4903,7 +4903,7 @@ ips_init_copperhead(ips_ha_t * ha)
/* Enable busmastering */
outb(IPS_BIT_EBM, ha->io_addr + IPS_REG_SCPR);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
/* fix for anaconda64 */
outl(0, ha->io_addr + IPS_REG_NDAE);
@@ -4997,7 +4997,7 @@ ips_init_copperhead_memio(ips_ha_t * ha)
/* Enable busmastering */
writeb(IPS_BIT_EBM, ha->mem_ptr + IPS_REG_SCPR);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
/* fix for anaconda64 */
writel(0, ha->mem_ptr + IPS_REG_NDAE);
@@ -5142,7 +5142,7 @@ ips_reset_copperhead(ips_ha_t * ha)
METHOD_TRACE("ips_reset_copperhead", 1);
DEBUG_VAR(1, "(%s%d) ips_reset_copperhead: io addr: %x, irq: %d",
- ips_name, ha->host_num, ha->io_addr, ha->irq);
+ ips_name, ha->host_num, ha->io_addr, ha->pcidev->irq);
reset_counter = 0;
@@ -5187,7 +5187,7 @@ ips_reset_copperhead_memio(ips_ha_t * ha)
METHOD_TRACE("ips_reset_copperhead_memio", 1);
DEBUG_VAR(1, "(%s%d) ips_reset_copperhead_memio: mem addr: %x, irq: %d",
- ips_name, ha->host_num, ha->mem_addr, ha->irq);
+ ips_name, ha->host_num, ha->mem_addr, ha->pcidev->irq);
reset_counter = 0;
@@ -5233,7 +5233,7 @@ ips_reset_morpheus(ips_ha_t * ha)
METHOD_TRACE("ips_reset_morpheus", 1);
DEBUG_VAR(1, "(%s%d) ips_reset_morpheus: mem addr: %x, irq: %d",
- ips_name, ha->host_num, ha->mem_addr, ha->irq);
+ ips_name, ha->host_num, ha->mem_addr, ha->pcidev->irq);
reset_counter = 0;
@@ -6196,32 +6196,32 @@ ips_erase_bios(ips_ha_t * ha)
/* Clear the status register */
outl(0, ha->io_addr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
outb(0x50, ha->io_addr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
/* Erase Setup */
outb(0x20, ha->io_addr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
/* Erase Confirm */
outb(0xD0, ha->io_addr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
/* Erase Status */
outb(0x70, ha->io_addr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
timeout = 80000; /* 80 seconds */
while (timeout > 0) {
- if (ha->revision_id == IPS_REVID_TROMBONE64) {
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64) {
outl(0, ha->io_addr + IPS_REG_FLAP);
udelay(25); /* 25 us */
}
@@ -6241,13 +6241,13 @@ ips_erase_bios(ips_ha_t * ha)
/* try to suspend the erase */
outb(0xB0, ha->io_addr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
/* wait for 10 seconds */
timeout = 10000;
while (timeout > 0) {
- if (ha->revision_id == IPS_REVID_TROMBONE64) {
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64) {
outl(0, ha->io_addr + IPS_REG_FLAP);
udelay(25); /* 25 us */
}
@@ -6277,12 +6277,12 @@ ips_erase_bios(ips_ha_t * ha)
/* Otherwise, we were successful */
/* clear status */
outb(0x50, ha->io_addr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
/* enable reads */
outb(0xFF, ha->io_addr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
return (0);
@@ -6308,32 +6308,32 @@ ips_erase_bios_memio(ips_ha_t * ha)
/* Clear the status register */
writel(0, ha->mem_ptr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
writeb(0x50, ha->mem_ptr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
/* Erase Setup */
writeb(0x20, ha->mem_ptr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
/* Erase Confirm */
writeb(0xD0, ha->mem_ptr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
/* Erase Status */
writeb(0x70, ha->mem_ptr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
timeout = 80000; /* 80 seconds */
while (timeout > 0) {
- if (ha->revision_id == IPS_REVID_TROMBONE64) {
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64) {
writel(0, ha->mem_ptr + IPS_REG_FLAP);
udelay(25); /* 25 us */
}
@@ -6353,13 +6353,13 @@ ips_erase_bios_memio(ips_ha_t * ha)
/* try to suspend the erase */
writeb(0xB0, ha->mem_ptr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
/* wait for 10 seconds */
timeout = 10000;
while (timeout > 0) {
- if (ha->revision_id == IPS_REVID_TROMBONE64) {
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64) {
writel(0, ha->mem_ptr + IPS_REG_FLAP);
udelay(25); /* 25 us */
}
@@ -6389,12 +6389,12 @@ ips_erase_bios_memio(ips_ha_t * ha)
/* Otherwise, we were successful */
/* clear status */
writeb(0x50, ha->mem_ptr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
/* enable reads */
writeb(0xFF, ha->mem_ptr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
return (0);
@@ -6423,21 +6423,21 @@ ips_program_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
for (i = 0; i < buffersize; i++) {
/* write a byte */
outl(cpu_to_le32(i + offset), ha->io_addr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
outb(0x40, ha->io_addr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
outb(buffer[i], ha->io_addr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
/* wait up to one second */
timeout = 1000;
while (timeout > 0) {
- if (ha->revision_id == IPS_REVID_TROMBONE64) {
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64) {
outl(0, ha->io_addr + IPS_REG_FLAP);
udelay(25); /* 25 us */
}
@@ -6454,11 +6454,11 @@ ips_program_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
if (timeout == 0) {
/* timeout error */
outl(0, ha->io_addr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
outb(0xFF, ha->io_addr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
return (1);
@@ -6468,11 +6468,11 @@ ips_program_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
if (status & 0x18) {
/* programming error */
outl(0, ha->io_addr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
outb(0xFF, ha->io_addr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
return (1);
@@ -6481,11 +6481,11 @@ ips_program_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
/* Enable reading */
outl(0, ha->io_addr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
outb(0xFF, ha->io_addr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
return (0);
@@ -6514,21 +6514,21 @@ ips_program_bios_memio(ips_ha_t * ha, char *buffer, uint32_t buffersize,
for (i = 0; i < buffersize; i++) {
/* write a byte */
writel(i + offset, ha->mem_ptr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
writeb(0x40, ha->mem_ptr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
writeb(buffer[i], ha->mem_ptr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
/* wait up to one second */
timeout = 1000;
while (timeout > 0) {
- if (ha->revision_id == IPS_REVID_TROMBONE64) {
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64) {
writel(0, ha->mem_ptr + IPS_REG_FLAP);
udelay(25); /* 25 us */
}
@@ -6545,11 +6545,11 @@ ips_program_bios_memio(ips_ha_t * ha, char *buffer, uint32_t buffersize,
if (timeout == 0) {
/* timeout error */
writel(0, ha->mem_ptr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
writeb(0xFF, ha->mem_ptr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
return (1);
@@ -6559,11 +6559,11 @@ ips_program_bios_memio(ips_ha_t * ha, char *buffer, uint32_t buffersize,
if (status & 0x18) {
/* programming error */
writel(0, ha->mem_ptr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
writeb(0xFF, ha->mem_ptr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
return (1);
@@ -6572,11 +6572,11 @@ ips_program_bios_memio(ips_ha_t * ha, char *buffer, uint32_t buffersize,
/* Enable reading */
writel(0, ha->mem_ptr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
writeb(0xFF, ha->mem_ptr + IPS_REG_FLDP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
return (0);
@@ -6601,14 +6601,14 @@ ips_verify_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
/* test 1st byte */
outl(0, ha->io_addr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
if (inb(ha->io_addr + IPS_REG_FLDP) != 0x55)
return (1);
outl(cpu_to_le32(1), ha->io_addr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
if (inb(ha->io_addr + IPS_REG_FLDP) != 0xAA)
return (1);
@@ -6617,7 +6617,7 @@ ips_verify_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
for (i = 2; i < buffersize; i++) {
outl(cpu_to_le32(i + offset), ha->io_addr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
checksum = (uint8_t) checksum + inb(ha->io_addr + IPS_REG_FLDP);
@@ -6650,14 +6650,14 @@ ips_verify_bios_memio(ips_ha_t * ha, char *buffer, uint32_t buffersize,
/* test 1st byte */
writel(0, ha->mem_ptr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
if (readb(ha->mem_ptr + IPS_REG_FLDP) != 0x55)
return (1);
writel(1, ha->mem_ptr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
if (readb(ha->mem_ptr + IPS_REG_FLDP) != 0xAA)
return (1);
@@ -6666,7 +6666,7 @@ ips_verify_bios_memio(ips_ha_t * ha, char *buffer, uint32_t buffersize,
for (i = 2; i < buffersize; i++) {
writel(i + offset, ha->mem_ptr + IPS_REG_FLAP);
- if (ha->revision_id == IPS_REVID_TROMBONE64)
+ if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
udelay(25); /* 25 us */
checksum =
@@ -6837,9 +6837,9 @@ ips_register_scsi(int index)
}
ha = IPS_HA(sh);
memcpy(ha, oldha, sizeof (ips_ha_t));
- free_irq(oldha->irq, oldha);
+ free_irq(oldha->pcidev->irq, oldha);
/* Install the interrupt handler with the new ha */
- if (request_irq(ha->irq, do_ipsintr, IRQF_SHARED, ips_name, ha)) {
+ if (request_irq(ha->pcidev->irq, do_ipsintr, IRQF_SHARED, ips_name, ha)) {
IPS_PRINTK(KERN_WARNING, ha->pcidev,
"Unable to install interrupt handler\n");
scsi_host_put(sh);
@@ -6851,10 +6851,7 @@ ips_register_scsi(int index)
ips_ha[index] = ha;
/* Store away needed values for later use */
- sh->io_port = ha->io_addr;
- sh->n_io_port = ha->io_addr ? 255 : 0;
sh->unique_id = (ha->io_addr) ? ha->io_addr : ha->mem_addr;
- sh->irq = ha->irq;
sh->sg_tablesize = sh->hostt->sg_tablesize;
sh->can_queue = sh->hostt->can_queue;
sh->cmd_per_lun = sh->hostt->cmd_per_lun;
@@ -6992,8 +6989,6 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
uint32_t mem_len;
uint8_t bus;
uint8_t func;
- uint8_t irq;
- uint16_t subdevice_id;
int j;
int index;
dma_addr_t dma_address;
@@ -7014,7 +7009,6 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
return -1;
/* stuff that we get in dev */
- irq = pci_dev->irq;
bus = pci_dev->bus->number;
func = pci_dev->devfn;
@@ -7068,8 +7062,6 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
}
}
- subdevice_id = pci_dev->subsystem_device;
-
/* found a controller */
ha = kzalloc(sizeof (ips_ha_t), GFP_KERNEL);
if (ha == NULL) {
@@ -7084,7 +7076,6 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
ha->active = 1;
/* Store info in HA structure */
- ha->irq = irq;
ha->io_addr = io_addr;
ha->io_len = io_len;
ha->mem_addr = mem_addr;
@@ -7092,10 +7083,7 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
ha->mem_ptr = mem_ptr;
ha->ioremap_ptr = ioremap_ptr;
ha->host_num = (uint32_t) index;
- ha->revision_id = pci_dev->revision;
ha->slot_num = PCI_SLOT(pci_dev->devfn);
- ha->device_id = pci_dev->device;
- ha->subdevice_id = subdevice_id;
ha->pcidev = pci_dev;
/*
@@ -7240,7 +7228,7 @@ ips_init_phase2(int index)
}
/* Install the interrupt handler */
- if (request_irq(ha->irq, do_ipsintr, IRQF_SHARED, ips_name, ha)) {
+ if (request_irq(ha->pcidev->irq, do_ipsintr, IRQF_SHARED, ips_name, ha)) {
IPS_PRINTK(KERN_WARNING, ha->pcidev,
"Unable to install interrupt handler\n");
return ips_abort_init(ha, index);
@@ -7253,14 +7241,14 @@ ips_init_phase2(int index)
if (!ips_allocatescbs(ha)) {
IPS_PRINTK(KERN_WARNING, ha->pcidev,
"Unable to allocate a CCB\n");
- free_irq(ha->irq, ha);
+ free_irq(ha->pcidev->irq, ha);
return ips_abort_init(ha, index);
}
if (!ips_hainit(ha)) {
IPS_PRINTK(KERN_WARNING, ha->pcidev,
"Unable to initialize controller\n");
- free_irq(ha->irq, ha);
+ free_irq(ha->pcidev->irq, ha);
return ips_abort_init(ha, index);
}
/* Free the temporary SCB */
@@ -7270,7 +7258,7 @@ ips_init_phase2(int index)
if (!ips_allocatescbs(ha)) {
IPS_PRINTK(KERN_WARNING, ha->pcidev,
"Unable to allocate CCBs\n");
- free_irq(ha->irq, ha);
+ free_irq(ha->pcidev->irq, ha);
return ips_abort_init(ha, index);
}
diff --git a/drivers/scsi/ips.h b/drivers/scsi/ips.h
index 3bcbd9f..fb4a036 100644
--- a/drivers/scsi/ips.h
+++ b/drivers/scsi/ips.h
@@ -60,14 +60,14 @@
*/
#define IPS_HA(x) ((ips_ha_t *) x->hostdata)
#define IPS_COMMAND_ID(ha, scb) (int) (scb - ha->scbs)
- #define IPS_IS_TROMBONE(ha) (((ha->device_id == IPS_DEVICEID_COPPERHEAD) && \
- (ha->revision_id >= IPS_REVID_TROMBONE32) && \
- (ha->revision_id <= IPS_REVID_TROMBONE64)) ? 1 : 0)
- #define IPS_IS_CLARINET(ha) (((ha->device_id == IPS_DEVICEID_COPPERHEAD) && \
- (ha->revision_id >= IPS_REVID_CLARINETP1) && \
- (ha->revision_id <= IPS_REVID_CLARINETP3)) ? 1 : 0)
- #define IPS_IS_MORPHEUS(ha) (ha->device_id == IPS_DEVICEID_MORPHEUS)
- #define IPS_IS_MARCO(ha) (ha->device_id == IPS_DEVICEID_MARCO)
+ #define IPS_IS_TROMBONE(ha) (((ha->pcidev->device == IPS_DEVICEID_COPPERHEAD) && \
+ (ha->pcidev->revision >= IPS_REVID_TROMBONE32) && \
+ (ha->pcidev->revision <= IPS_REVID_TROMBONE64)) ? 1 : 0)
+ #define IPS_IS_CLARINET(ha) (((ha->pcidev->device == IPS_DEVICEID_COPPERHEAD) && \
+ (ha->pcidev->revision >= IPS_REVID_CLARINETP1) && \
+ (ha->pcidev->revision <= IPS_REVID_CLARINETP3)) ? 1 : 0)
+ #define IPS_IS_MORPHEUS(ha) (ha->pcidev->device == IPS_DEVICEID_MORPHEUS)
+ #define IPS_IS_MARCO(ha) (ha->pcidev->device == IPS_DEVICEID_MARCO)
#define IPS_USE_I2O_DELIVER(ha) ((IPS_IS_MORPHEUS(ha) || \
(IPS_IS_TROMBONE(ha) && \
(ips_force_i2o))) ? 1 : 0)
@@ -1034,7 +1034,6 @@ typedef struct ips_ha {
uint8_t ha_id[IPS_MAX_CHANNELS+1];
uint32_t dcdb_active[IPS_MAX_CHANNELS];
uint32_t io_addr; /* Base I/O address */
- uint8_t irq; /* IRQ for adapter */
uint8_t ntargets; /* Number of targets */
uint8_t nbus; /* Number of buses */
uint8_t nlun; /* Number of Luns */
@@ -1066,10 +1065,7 @@ typedef struct ips_ha {
int ioctl_reset; /* IOCTL Requested Reset Flag */
uint16_t reset_count; /* number of resets */
time_t last_ffdc; /* last time we sent ffdc info*/
- uint8_t revision_id; /* Revision level */
- uint16_t device_id; /* PCI device ID */
uint8_t slot_num; /* PCI Slot Number */
- uint16_t subdevice_id; /* Subsystem device ID */
int ioctl_len; /* size of ioctl buffer */
dma_addr_t ioctl_busaddr; /* dma address of ioctl buffer*/
uint8_t bios_version[8]; /* BIOS Revision */
--
1.5.2.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 1/4] [SCSI] ips: remove ips_ha members that duplicate struct pci_dev members
2007-10-24 23:48 ` [PATCH 1/4] [SCSI] ips: remove ips_ha members that duplicate struct pci_dev members Jeff Garzik
@ 2007-10-25 4:27 ` Andrew Morton
2007-10-25 5:09 ` Jeff Garzik
2007-10-25 14:32 ` Salyzyn, Mark
1 sibling, 1 reply; 23+ messages in thread
From: Andrew Morton @ 2007-10-25 4:27 UTC (permalink / raw)
To: Jeff Garzik; +Cc: LKML, linux-scsi
On Wed, 24 Oct 2007 19:48:26 -0400 (EDT) Jeff Garzik <jeff@garzik.org> wrote:
> drivers/scsi/ips.c | 178 ++++++++++++++++++++++++----------------------------
this driver seems a bit of a basket case :(
What's going on here?
scb->dcdb.cmd_attribute =
ips_command_direction[scb->scsi_cmd->cmnd[0]];
/* Allow a WRITE BUFFER Command to Have no Data */
/* This is Used by Tape Flash Utilites */
if ((scb->scsi_cmd->cmnd[0] == WRITE_BUFFER) && (scb->data_len == 0))
scb->dcdb.cmd_attribute = 0;
if (!(scb->dcdb.cmd_attribute & 0x3))
scb->dcdb.transfer_length = 0;
if (scb->data_len >= IPS_MAX_XFER) {
I hope that's just busted indentation and not a missing {} block.
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 1/4] [SCSI] ips: remove ips_ha members that duplicate struct pci_dev members
2007-10-25 4:27 ` Andrew Morton
@ 2007-10-25 5:09 ` Jeff Garzik
0 siblings, 0 replies; 23+ messages in thread
From: Jeff Garzik @ 2007-10-25 5:09 UTC (permalink / raw)
To: Andrew Morton; +Cc: LKML, linux-scsi
Andrew Morton wrote:
> On Wed, 24 Oct 2007 19:48:26 -0400 (EDT) Jeff Garzik <jeff@garzik.org> wrote:
>
>> drivers/scsi/ips.c | 178 ++++++++++++++++++++++++----------------------------
>
> this driver seems a bit of a basket case :(
>
>
> What's going on here?
>
> scb->dcdb.cmd_attribute =
> ips_command_direction[scb->scsi_cmd->cmnd[0]];
>
> /* Allow a WRITE BUFFER Command to Have no Data */
> /* This is Used by Tape Flash Utilites */
> if ((scb->scsi_cmd->cmnd[0] == WRITE_BUFFER) && (scb->data_len == 0))
> scb->dcdb.cmd_attribute = 0;
>
> if (!(scb->dcdb.cmd_attribute & 0x3))
> scb->dcdb.transfer_length = 0;
>
> if (scb->data_len >= IPS_MAX_XFER) {
>
> I hope that's just busted indentation and not a missing {} block.
The driver is one of the grotty drivers people are afraid to touch,
therefore it bitrots even faster, in a vicious cycle. You don't have to
look hard at all to find checkpatch pukeage, very real bugs, and
Pythonesque silliness.
Not having hardware I went only for changes that are provable and
obvious, really...
Jeff
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 1/4] [SCSI] ips: remove ips_ha members that duplicate struct pci_dev members
2007-10-24 23:48 ` [PATCH 1/4] [SCSI] ips: remove ips_ha members that duplicate struct pci_dev members Jeff Garzik
2007-10-25 4:27 ` Andrew Morton
@ 2007-10-25 14:32 ` Salyzyn, Mark
2007-10-25 22:42 ` Jeff Garzik
1 sibling, 1 reply; 23+ messages in thread
From: Salyzyn, Mark @ 2007-10-25 14:32 UTC (permalink / raw)
To: Jeff Garzik, LKML, linux-scsi; +Cc: akpm
ACK. Inspected; Mechanical, precise and no introduction of bugs.
Sincerely -- Mark Salyzyn
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org
> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Jeff Garzik
> Sent: Wednesday, October 24, 2007 7:48 PM
> To: LKML; linux-scsi@vger.kernel.org
> Cc: akpm@linux-foundation.org
> Subject: [PATCH 1/4] [SCSI] ips: remove ips_ha members that
> duplicate struct pci_dev members
>
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
> drivers/scsi/ips.c | 178
> ++++++++++++++++++++++++----------------------------
> drivers/scsi/ips.h | 20 +++----
> 2 files changed, 91 insertions(+), 107 deletions(-)
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/4] [SCSI] ips: trim trailing whitespace
2007-10-24 23:48 [PATCH] sound/oss/pss: set_io_base() always returns success, mark it void Jeff Garzik
` (3 preceding siblings ...)
2007-10-24 23:48 ` [PATCH 1/4] [SCSI] ips: remove ips_ha members that duplicate struct pci_dev members Jeff Garzik
@ 2007-10-24 23:48 ` Jeff Garzik
2007-10-25 14:33 ` Salyzyn, Mark
2007-10-24 23:48 ` [PATCH 3/4] [SCSI] ips: PCI API cleanups Jeff Garzik
` (2 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Jeff Garzik @ 2007-10-24 23:48 UTC (permalink / raw)
To: LKML, linux-scsi; +Cc: akpm
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
drivers/scsi/ips.c | 44 ++++++++++++++++++++++----------------------
drivers/scsi/ips.h | 12 ++++++------
2 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index 595a91a..c9f3e1f 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -389,17 +389,17 @@ static struct pci_device_id ips_pci_table[] = {
MODULE_DEVICE_TABLE( pci, ips_pci_table );
static char ips_hot_plug_name[] = "ips";
-
+
static int __devinit ips_insert_device(struct pci_dev *pci_dev, const struct pci_device_id *ent);
static void __devexit ips_remove_device(struct pci_dev *pci_dev);
-
+
static struct pci_driver ips_pci_driver = {
.name = ips_hot_plug_name,
.id_table = ips_pci_table,
.probe = ips_insert_device,
.remove = __devexit_p(ips_remove_device),
};
-
+
/*
* Necessary forward function protoypes
@@ -587,7 +587,7 @@ static void
ips_setup_funclist(ips_ha_t * ha)
{
- /*
+ /*
* Setup Functions
*/
if (IPS_IS_MORPHEUS(ha) || IPS_IS_MARCO(ha)) {
@@ -2081,7 +2081,7 @@ ips_host_info(ips_ha_t * ha, char *ptr, off_t offset, int len)
/* That keeps everything happy for "text" operations on the proc file. */
if (le32_to_cpu(ha->nvram->signature) == IPS_NVRAM_P5_SIG) {
- if (ha->nvram->bios_low[3] == 0) {
+ if (ha->nvram->bios_low[3] == 0) {
copy_info(&info,
"\tBIOS Version : %c%c%c%c%c%c%c\n",
ha->nvram->bios_high[0], ha->nvram->bios_high[1],
@@ -2782,8 +2782,8 @@ ips_next(ips_ha_t * ha, int intr)
/* Allow a WRITE BUFFER Command to Have no Data */
/* This is Used by Tape Flash Utilites */
- if ((scb->scsi_cmd->cmnd[0] == WRITE_BUFFER) && (scb->data_len == 0))
- scb->dcdb.cmd_attribute = 0;
+ if ((scb->scsi_cmd->cmnd[0] == WRITE_BUFFER) && (scb->data_len == 0))
+ scb->dcdb.cmd_attribute = 0;
if (!(scb->dcdb.cmd_attribute & 0x3))
scb->dcdb.transfer_length = 0;
@@ -3404,7 +3404,7 @@ ips_map_status(ips_ha_t * ha, ips_scb_t * scb, ips_stat_t * sp)
/* Restrict access to physical DASD */
if (scb->scsi_cmd->cmnd[0] == INQUIRY) {
- ips_scmd_buf_read(scb->scsi_cmd,
+ ips_scmd_buf_read(scb->scsi_cmd,
&inquiryData, sizeof (inquiryData));
if ((inquiryData.DeviceType & 0x1f) == TYPE_DISK) {
errcode = DID_TIME_OUT;
@@ -4090,10 +4090,10 @@ ips_chkstatus(ips_ha_t * ha, IPS_STATUS * pstatus)
scb->scsi_cmd->result = errcode << 16;
} else { /* bus == 0 */
/* restrict access to physical drives */
- if (scb->scsi_cmd->cmnd[0] == INQUIRY) {
- ips_scmd_buf_read(scb->scsi_cmd,
+ if (scb->scsi_cmd->cmnd[0] == INQUIRY) {
+ ips_scmd_buf_read(scb->scsi_cmd,
&inquiryData, sizeof (inquiryData));
- if ((inquiryData.DeviceType & 0x1f) == TYPE_DISK)
+ if ((inquiryData.DeviceType & 0x1f) == TYPE_DISK)
scb->scsi_cmd->result = DID_TIME_OUT << 16;
}
} /* else */
@@ -4661,8 +4661,8 @@ ips_isinit_morpheus(ips_ha_t * ha)
uint32_t bits;
METHOD_TRACE("ips_is_init_morpheus", 1);
-
- if (ips_isintr_morpheus(ha))
+
+ if (ips_isintr_morpheus(ha))
ips_flush_and_reset(ha);
post = readl(ha->mem_ptr + IPS_REG_I960_MSG0);
@@ -4686,7 +4686,7 @@ ips_isinit_morpheus(ips_ha_t * ha)
/* state ( was trying to INIT and an interrupt was already pending ) ... */
/* */
/****************************************************************************/
-static void
+static void
ips_flush_and_reset(ips_ha_t *ha)
{
ips_scb_t *scb;
@@ -4718,9 +4718,9 @@ ips_flush_and_reset(ips_ha_t *ha)
if (ret == IPS_SUCCESS) {
time = 60 * IPS_ONE_SEC; /* Max Wait time is 60 seconds */
done = 0;
-
+
while ((time > 0) && (!done)) {
- done = ips_poll_for_flush_complete(ha);
+ done = ips_poll_for_flush_complete(ha);
/* This may look evil, but it's only done during extremely rare start-up conditions ! */
udelay(1000);
time--;
@@ -4749,17 +4749,17 @@ static int
ips_poll_for_flush_complete(ips_ha_t * ha)
{
IPS_STATUS cstatus;
-
+
while (TRUE) {
cstatus.value = (*ha->func.statupd) (ha);
if (cstatus.value == 0xffffffff) /* If No Interrupt to process */
break;
-
+
/* Success is when we see the Flush Command ID */
- if (cstatus.fields.command_id == IPS_MAX_CMDS )
+ if (cstatus.fields.command_id == IPS_MAX_CMDS )
return 1;
- }
+ }
return 0;
}
@@ -5920,7 +5920,7 @@ ips_read_config(ips_ha_t * ha, int intr)
return (0);
}
-
+
memcpy(ha->conf, ha->ioctl_data, sizeof(*ha->conf));
return (1);
}
@@ -5959,7 +5959,7 @@ ips_readwrite_page5(ips_ha_t * ha, int write, int intr)
scb->cmd.nvram.buffer_addr = ha->ioctl_busaddr;
if (write)
memcpy(ha->ioctl_data, ha->nvram, sizeof(*ha->nvram));
-
+
/* issue the command */
if (((ret =
ips_send_wait(ha, scb, ips_cmd_timeout, intr)) == IPS_FAILURE)
diff --git a/drivers/scsi/ips.h b/drivers/scsi/ips.h
index fb4a036..e0657b6 100644
--- a/drivers/scsi/ips.h
+++ b/drivers/scsi/ips.h
@@ -92,7 +92,7 @@
#ifndef min
#define min(x,y) ((x) < (y) ? x : y)
#endif
-
+
#ifndef __iomem /* For clean compiles in earlier kernels without __iomem annotations */
#define __iomem
#endif
@@ -171,7 +171,7 @@
#define IPS_CMD_DOWNLOAD 0x20
#define IPS_CMD_RW_BIOSFW 0x22
#define IPS_CMD_GET_VERSION_INFO 0xC6
- #define IPS_CMD_RESET_CHANNEL 0x1A
+ #define IPS_CMD_RESET_CHANNEL 0x1A
/*
* Adapter Equates
@@ -458,7 +458,7 @@ typedef struct {
uint32_t reserved3;
uint32_t buffer_addr;
uint32_t reserved4;
-} IPS_IOCTL_CMD, *PIPS_IOCTL_CMD;
+} IPS_IOCTL_CMD, *PIPS_IOCTL_CMD;
typedef struct {
uint8_t op_code;
@@ -552,7 +552,7 @@ typedef struct {
uint32_t cccr;
} IPS_NVRAM_CMD, *PIPS_NVRAM_CMD;
-typedef struct
+typedef struct
{
uint8_t op_code;
uint8_t command_id;
@@ -650,7 +650,7 @@ typedef struct {
uint8_t device_address;
uint8_t cmd_attribute;
uint8_t cdb_length;
- uint8_t reserved_for_LUN;
+ uint8_t reserved_for_LUN;
uint32_t transfer_length;
uint32_t buffer_pointer;
uint16_t sg_count;
@@ -790,7 +790,7 @@ typedef struct {
/* SubSystem Parameter[4] */
#define IPS_GET_VERSION_SUPPORT 0x00018000 /* Mask for Versioning Support */
-typedef struct
+typedef struct
{
uint32_t revision;
uint8_t bootBlkVersion[32];
--
1.5.2.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* RE: [PATCH 2/4] [SCSI] ips: trim trailing whitespace
2007-10-24 23:48 ` [PATCH 2/4] [SCSI] ips: trim trailing whitespace Jeff Garzik
@ 2007-10-25 14:33 ` Salyzyn, Mark
0 siblings, 0 replies; 23+ messages in thread
From: Salyzyn, Mark @ 2007-10-25 14:33 UTC (permalink / raw)
To: Jeff Garzik, LKML, linux-scsi; +Cc: akpm
ACK. Inspected; trivial, clean and no sign of any code changes.
Sincerely -- Mark Salyzyn
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org
> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Jeff Garzik
> Sent: Wednesday, October 24, 2007 7:48 PM
> To: LKML; linux-scsi@vger.kernel.org
> Cc: akpm@linux-foundation.org
> Subject: [PATCH 2/4] [SCSI] ips: trim trailing whitespace
>
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
> drivers/scsi/ips.c | 44
> ++++++++++++++++++++++----------------------
> drivers/scsi/ips.h | 12 ++++++------
> 2 files changed, 28 insertions(+), 28 deletions(-)
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/4] [SCSI] ips: PCI API cleanups
2007-10-24 23:48 [PATCH] sound/oss/pss: set_io_base() always returns success, mark it void Jeff Garzik
` (4 preceding siblings ...)
2007-10-24 23:48 ` [PATCH 2/4] [SCSI] ips: trim trailing whitespace Jeff Garzik
@ 2007-10-24 23:48 ` Jeff Garzik
2007-10-25 6:54 ` Rolf Eike Beer
2007-10-25 14:37 ` Salyzyn, Mark
2007-10-24 23:48 ` [PATCH 4/4] [SCSI] ips: handle scsi_add_host() failure, and other err cleanups Jeff Garzik
2007-10-24 23:48 ` [PATCH] [libata] fix 'if(' and similar areas that lack whitespace Jeff Garzik
7 siblings, 2 replies; 23+ messages in thread
From: Jeff Garzik @ 2007-10-24 23:48 UTC (permalink / raw)
To: LKML, linux-scsi; +Cc: akpm
* pass Scsi_Host to ips_remove_device() via pci_set_drvdata(),
allowing us to eliminate the ips_ha[] search loop and call
ips_release() directly.
* call pci_{request,release}_regions() and eliminate individual
request/release_[mem_]region() calls
* call pci_disable_device(), paired with pci_enable_device()
* s/0/NULL/ in a few places
* check ioremap() return value
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
drivers/scsi/ips.c | 72 ++++++++++++++++++++++-----------------------------
1 files changed, 31 insertions(+), 41 deletions(-)
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index c9f3e1f..fb90b6c 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -702,10 +702,6 @@ ips_release(struct Scsi_Host *sh)
/* free extra memory */
ips_free(ha);
- /* Free I/O Region */
- if (ha->io_addr)
- release_region(ha->io_addr, ha->io_len);
-
/* free IRQ */
free_irq(ha->pcidev->irq, ha);
@@ -4393,8 +4389,6 @@ ips_free(ips_ha_t * ha)
ha->mem_ptr = NULL;
}
- if (ha->mem_addr)
- release_mem_region(ha->mem_addr, ha->mem_len);
ha->mem_addr = 0;
}
@@ -6879,20 +6873,14 @@ ips_register_scsi(int index)
static void __devexit
ips_remove_device(struct pci_dev *pci_dev)
{
- int i;
- struct Scsi_Host *sh;
- ips_ha_t *ha;
+ struct Scsi_Host *sh = pci_get_drvdata(pci_dev);
- for (i = 0; i < IPS_MAX_ADAPTERS; i++) {
- ha = ips_ha[i];
- if (ha) {
- if ((pci_dev->bus->number == ha->pcidev->bus->number) &&
- (pci_dev->devfn == ha->pcidev->devfn)) {
- sh = ips_sh[i];
- ips_release(sh);
- }
- }
- }
+ pci_set_drvdata(pci_dev, NULL);
+
+ ips_release(sh);
+
+ pci_release_regions(pci_dev);
+ pci_disable_device(pci_dev);
}
/****************************************************************************/
@@ -6946,12 +6934,17 @@ module_exit(ips_module_exit);
static int __devinit
ips_insert_device(struct pci_dev *pci_dev, const struct pci_device_id *ent)
{
- int uninitialized_var(index);
+ int index = -1;
int rc;
METHOD_TRACE("ips_insert_device", 1);
- if (pci_enable_device(pci_dev))
- return -1;
+ rc = pci_enable_device(pci_dev);
+ if (rc)
+ return rc;
+
+ rc = pci_request_regions(pci_dev, "ips");
+ if (rc)
+ goto err_out;
rc = ips_init_phase1(pci_dev, &index);
if (rc == SUCCESS)
@@ -6967,6 +6960,19 @@ ips_insert_device(struct pci_dev *pci_dev, const struct pci_device_id *ent)
ips_num_controllers++;
ips_next_controller = ips_num_controllers;
+
+ if (rc < 0) {
+ rc = -ENODEV;
+ goto err_out_regions;
+ }
+
+ pci_set_drvdata(pci_dev, ips_sh[index]);
+ return 0;
+
+err_out_regions:
+ pci_release_regions(pci_dev);
+err_out:
+ pci_disable_device(pci_dev);
return rc;
}
@@ -6999,7 +7005,7 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
METHOD_TRACE("ips_init_phase1", 1);
index = IPS_MAX_ADAPTERS;
for (j = 0; j < IPS_MAX_ADAPTERS; j++) {
- if (ips_ha[j] == 0) {
+ if (ips_ha[j] == NULL) {
index = j;
break;
}
@@ -7036,32 +7042,17 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
uint32_t base;
uint32_t offs;
- if (!request_mem_region(mem_addr, mem_len, "ips")) {
- IPS_PRINTK(KERN_WARNING, pci_dev,
- "Couldn't allocate IO Memory space %x len %d.\n",
- mem_addr, mem_len);
- return -1;
- }
-
base = mem_addr & PAGE_MASK;
offs = mem_addr - base;
ioremap_ptr = ioremap(base, PAGE_SIZE);
+ if (!ioremap_ptr)
+ return -1;
mem_ptr = ioremap_ptr + offs;
} else {
ioremap_ptr = NULL;
mem_ptr = NULL;
}
- /* setup I/O mapped area (if applicable) */
- if (io_addr) {
- if (!request_region(io_addr, io_len, "ips")) {
- IPS_PRINTK(KERN_WARNING, pci_dev,
- "Couldn't allocate IO space %x len %d.\n",
- io_addr, io_len);
- return -1;
- }
- }
-
/* found a controller */
ha = kzalloc(sizeof (ips_ha_t), GFP_KERNEL);
if (ha == NULL) {
@@ -7070,7 +7061,6 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
return -1;
}
-
ips_sh[index] = NULL;
ips_ha[index] = ha;
ha->active = 1;
--
1.5.2.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 3/4] [SCSI] ips: PCI API cleanups
2007-10-24 23:48 ` [PATCH 3/4] [SCSI] ips: PCI API cleanups Jeff Garzik
@ 2007-10-25 6:54 ` Rolf Eike Beer
2007-10-25 14:37 ` Salyzyn, Mark
1 sibling, 0 replies; 23+ messages in thread
From: Rolf Eike Beer @ 2007-10-25 6:54 UTC (permalink / raw)
To: Jeff Garzik; +Cc: LKML, linux-scsi, akpm
[-- Attachment #1: Type: text/plain, Size: 1373 bytes --]
Jeff Garzik wrote:
> * pass Scsi_Host to ips_remove_device() via pci_set_drvdata(),
> allowing us to eliminate the ips_ha[] search loop and call
> ips_release() directly.
>
> * call pci_{request,release}_regions() and eliminate individual
> request/release_[mem_]region() calls
>
> * call pci_disable_device(), paired with pci_enable_device()
>
> * s/0/NULL/ in a few places
>
> * check ioremap() return value
> @@ -7036,32 +7042,17 @@ ips_init_phase1(struct pci_dev *pci_dev, int
> *indexPtr) uint32_t base;
> uint32_t offs;
>
> - if (!request_mem_region(mem_addr, mem_len, "ips")) {
> - IPS_PRINTK(KERN_WARNING, pci_dev,
> - "Couldn't allocate IO Memory space %x len %d.\n",
> - mem_addr, mem_len);
> - return -1;
> - }
> -
> base = mem_addr & PAGE_MASK;
> offs = mem_addr - base;
> ioremap_ptr = ioremap(base, PAGE_SIZE);
This looks odd. What are we actually doing here?
It seems that we want to map that PCI BAR. Since we're playing with PAGE_MASK
I assume the BAR always has a length <PAGE_SIZE, else we would get page
aligned memory anyway. If that's true something like
mem_ptr = pci_iomap(pci_dev, bar, 0)
should do the trick. Later we would do
pci_iounmap(pci_dev, mem_ptr)
This whole ioremap_ptr stuff can go away at all. Am I right? Then I'll cook up
a patch soon.
Eike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 194 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* RE: [PATCH 3/4] [SCSI] ips: PCI API cleanups
2007-10-24 23:48 ` [PATCH 3/4] [SCSI] ips: PCI API cleanups Jeff Garzik
2007-10-25 6:54 ` Rolf Eike Beer
@ 2007-10-25 14:37 ` Salyzyn, Mark
1 sibling, 0 replies; 23+ messages in thread
From: Salyzyn, Mark @ 2007-10-25 14:37 UTC (permalink / raw)
To: Jeff Garzik, LKML, linux-scsi; +Cc: akpm
ACK. Inspected only. Looks ok.
Sincerely -- Mark Salyzyn
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org
> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Jeff Garzik
> Sent: Wednesday, October 24, 2007 7:49 PM
> To: LKML; linux-scsi@vger.kernel.org
> Cc: akpm@linux-foundation.org
> Subject: [PATCH 3/4] [SCSI] ips: PCI API cleanups
>
> * pass Scsi_Host to ips_remove_device() via pci_set_drvdata(),
> allowing us to eliminate the ips_ha[] search loop and call
> ips_release() directly.
>
> * call pci_{request,release}_regions() and eliminate individual
> request/release_[mem_]region() calls
>
> * call pci_disable_device(), paired with pci_enable_device()
>
> * s/0/NULL/ in a few places
>
> * check ioremap() return value
>
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
> drivers/scsi/ips.c | 72
> ++++++++++++++++++++++-----------------------------
> 1 files changed, 31 insertions(+), 41 deletions(-)
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/4] [SCSI] ips: handle scsi_add_host() failure, and other err cleanups
2007-10-24 23:48 [PATCH] sound/oss/pss: set_io_base() always returns success, mark it void Jeff Garzik
` (5 preceding siblings ...)
2007-10-24 23:48 ` [PATCH 3/4] [SCSI] ips: PCI API cleanups Jeff Garzik
@ 2007-10-24 23:48 ` Jeff Garzik
2007-10-25 14:39 ` Salyzyn, Mark
2007-10-24 23:48 ` [PATCH] [libata] fix 'if(' and similar areas that lack whitespace Jeff Garzik
7 siblings, 1 reply; 23+ messages in thread
From: Jeff Garzik @ 2007-10-24 23:48 UTC (permalink / raw)
To: LKML, linux-scsi; +Cc: akpm
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
drivers/scsi/ips.c | 18 +++++++++++++-----
1 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index fb90b6c..b8e2f5a 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -6836,13 +6836,10 @@ ips_register_scsi(int index)
if (request_irq(ha->pcidev->irq, do_ipsintr, IRQF_SHARED, ips_name, ha)) {
IPS_PRINTK(KERN_WARNING, ha->pcidev,
"Unable to install interrupt handler\n");
- scsi_host_put(sh);
- return -1;
+ goto err_out_sh;
}
kfree(oldha);
- ips_sh[index] = sh;
- ips_ha[index] = ha;
/* Store away needed values for later use */
sh->unique_id = (ha->io_addr) ? ha->io_addr : ha->mem_addr;
@@ -6858,10 +6855,21 @@ ips_register_scsi(int index)
sh->max_channel = ha->nbus - 1;
sh->can_queue = ha->max_cmds - 1;
- scsi_add_host(sh, NULL);
+ if (scsi_add_host(sh, &ha->pcidev->dev))
+ goto err_out;
+
+ ips_sh[index] = sh;
+ ips_ha[index] = ha;
+
scsi_scan_host(sh);
return 0;
+
+err_out:
+ free_irq(ha->pcidev->irq, ha);
+err_out_sh:
+ scsi_host_put(sh);
+ return -1;
}
/*---------------------------------------------------------------------------*/
--
1.5.2.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* RE: [PATCH 4/4] [SCSI] ips: handle scsi_add_host() failure, and other err cleanups
2007-10-24 23:48 ` [PATCH 4/4] [SCSI] ips: handle scsi_add_host() failure, and other err cleanups Jeff Garzik
@ 2007-10-25 14:39 ` Salyzyn, Mark
0 siblings, 0 replies; 23+ messages in thread
From: Salyzyn, Mark @ 2007-10-25 14:39 UTC (permalink / raw)
To: Jeff Garzik, LKML, linux-scsi; +Cc: akpm
ACK. Inspected. cleanup with zero risk.
Sincerely -- Mark Salyzyn
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org
> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Jeff Garzik
> Sent: Wednesday, October 24, 2007 7:49 PM
> To: LKML; linux-scsi@vger.kernel.org
> Cc: akpm@linux-foundation.org
> Subject: [PATCH 4/4] [SCSI] ips: handle scsi_add_host()
> failure, and other err cleanups
>
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
> drivers/scsi/ips.c | 18 +++++++++++++-----
> 1 files changed, 13 insertions(+), 5 deletions(-)
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] [libata] fix 'if(' and similar areas that lack whitespace
2007-10-24 23:48 [PATCH] sound/oss/pss: set_io_base() always returns success, mark it void Jeff Garzik
` (6 preceding siblings ...)
2007-10-24 23:48 ` [PATCH 4/4] [SCSI] ips: handle scsi_add_host() failure, and other err cleanups Jeff Garzik
@ 2007-10-24 23:48 ` Jeff Garzik
2007-10-25 15:35 ` Richard Knutsson
7 siblings, 1 reply; 23+ messages in thread
From: Jeff Garzik @ 2007-10-24 23:48 UTC (permalink / raw)
To: LKML; +Cc: akpm
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
drivers/ata/pata_acpi.c | 4 +-
drivers/ata/pata_optidma.c | 2 +-
drivers/ata/pata_pdc2027x.c | 2 +-
drivers/ata/pata_pdc202xx_old.c | 4 +-
drivers/ata/pata_via.c | 2 +-
drivers/ata/pata_winbond.c | 2 +-
drivers/ata/sata_nv.c | 46 +++++++++++++++++++-------------------
drivers/ata/sata_sx4.c | 4 +-
8 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/drivers/ata/pata_acpi.c b/drivers/ata/pata_acpi.c
index 0f6f7bc..e4542ab 100644
--- a/drivers/ata/pata_acpi.c
+++ b/drivers/ata/pata_acpi.c
@@ -181,7 +181,7 @@ static void pacpi_set_piomode(struct ata_port *ap, struct ata_device *adev)
int unit = adev->devno;
struct pata_acpi *acpi = ap->private_data;
- if(!(acpi->gtm.flags & 0x10))
+ if (!(acpi->gtm.flags & 0x10))
unit = 0;
/* Now stuff the nS values into the structure */
@@ -202,7 +202,7 @@ static void pacpi_set_dmamode(struct ata_port *ap, struct ata_device *adev)
int unit = adev->devno;
struct pata_acpi *acpi = ap->private_data;
- if(!(acpi->gtm.flags & 0x10))
+ if (!(acpi->gtm.flags & 0x10))
unit = 0;
/* Now stuff the nS values into the structure */
diff --git a/drivers/ata/pata_optidma.c b/drivers/ata/pata_optidma.c
index 6b07b5b..f9b485a 100644
--- a/drivers/ata/pata_optidma.c
+++ b/drivers/ata/pata_optidma.c
@@ -449,7 +449,7 @@ static int optiplus_with_udma(struct pci_dev *pdev)
/* Find function 1 */
dev1 = pci_get_device(0x1045, 0xC701, NULL);
- if(dev1 == NULL)
+ if (dev1 == NULL)
return 0;
/* Rev must be >= 0x10 */
diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
index 3d3f155..2622577 100644
--- a/drivers/ata/pata_pdc2027x.c
+++ b/drivers/ata/pata_pdc2027x.c
@@ -348,7 +348,7 @@ static unsigned long pdc2027x_mode_filter(struct ata_device *adev, unsigned long
ata_id_c_string(pair->id, model_num, ATA_ID_PROD,
ATA_ID_PROD_LEN + 1);
/* If the master is a maxtor in UDMA6 then the slave should not use UDMA 6 */
- if(strstr(model_num, "Maxtor") == 0 && pair->dma_mode == XFER_UDMA_6)
+ if (strstr(model_num, "Maxtor") == 0 && pair->dma_mode == XFER_UDMA_6)
mask &= ~ (1 << (6 + ATA_SHIFT_UDMA));
return ata_pci_default_filter(adev, mask);
diff --git a/drivers/ata/pata_pdc202xx_old.c b/drivers/ata/pata_pdc202xx_old.c
index 65d9516..bc7c2d5 100644
--- a/drivers/ata/pata_pdc202xx_old.c
+++ b/drivers/ata/pata_pdc202xx_old.c
@@ -351,9 +351,9 @@ static int pdc202xx_init_one(struct pci_dev *dev, const struct pci_device_id *id
struct pci_dev *bridge = dev->bus->self;
/* Don't grab anything behind a Promise I2O RAID */
if (bridge && bridge->vendor == PCI_VENDOR_ID_INTEL) {
- if( bridge->device == PCI_DEVICE_ID_INTEL_I960)
+ if (bridge->device == PCI_DEVICE_ID_INTEL_I960)
return -ENODEV;
- if( bridge->device == PCI_DEVICE_ID_INTEL_I960RM)
+ if (bridge->device == PCI_DEVICE_ID_INTEL_I960RM)
return -ENODEV;
}
}
diff --git a/drivers/ata/pata_via.c b/drivers/ata/pata_via.c
index ea7a9a6..a4175fb 100644
--- a/drivers/ata/pata_via.c
+++ b/drivers/ata/pata_via.c
@@ -176,7 +176,7 @@ static int via_cable_detect(struct ata_port *ap) {
if ((config->flags & VIA_UDMA) < VIA_UDMA_66)
return ATA_CBL_PATA40;
/* UDMA 66 chips have only drive side logic */
- else if((config->flags & VIA_UDMA) < VIA_UDMA_100)
+ else if ((config->flags & VIA_UDMA) < VIA_UDMA_100)
return ATA_CBL_PATA_UNK;
/* UDMA 100 or later */
pci_read_config_dword(pdev, 0x50, &ata66);
diff --git a/drivers/ata/pata_winbond.c b/drivers/ata/pata_winbond.c
index 549cbbe..311cdb3 100644
--- a/drivers/ata/pata_winbond.c
+++ b/drivers/ata/pata_winbond.c
@@ -279,7 +279,7 @@ static __init int winbond_init(void)
if (request_region(port, 2, "pata_winbond")) {
ret = winbond_init_one(port);
- if(ret <= 0)
+ if (ret <= 0)
release_region(port, 2);
else ct+= ret;
}
diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
index 2e0279f..9210bb2 100644
--- a/drivers/ata/sata_nv.c
+++ b/drivers/ata/sata_nv.c
@@ -1012,7 +1012,7 @@ static irqreturn_t nv_adma_interrupt(int irq, void *dev_instance)
u32 check_commands;
int pos, error = 0;
- if(ata_tag_valid(ap->link.active_tag))
+ if (ata_tag_valid(ap->link.active_tag))
check_commands = 1 << ap->link.active_tag;
else
check_commands = ap->link.sactive;
@@ -1028,7 +1028,7 @@ static irqreturn_t nv_adma_interrupt(int irq, void *dev_instance)
}
}
- if(notifier_clears[0] || notifier_clears[1]) {
+ if (notifier_clears[0] || notifier_clears[1]) {
/* Note: Both notifier clear registers must be written
if either is set, even if one is zero, according to NVIDIA. */
struct nv_adma_port_priv *pp = host->ports[0]->private_data;
@@ -1119,7 +1119,7 @@ static void nv_adma_post_internal_cmd(struct ata_queued_cmd *qc)
{
struct nv_adma_port_priv *pp = qc->ap->private_data;
- if(pp->flags & NV_ADMA_PORT_REGISTER_MODE)
+ if (pp->flags & NV_ADMA_PORT_REGISTER_MODE)
ata_bmdma_post_internal_cmd(qc);
}
@@ -1194,10 +1194,10 @@ static int nv_adma_port_start(struct ata_port *ap)
tmp = readw(mmio + NV_ADMA_CTL);
writew(tmp | NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
- readw( mmio + NV_ADMA_CTL ); /* flush posted write */
+ readw(mmio + NV_ADMA_CTL ); /* flush posted write */
udelay(1);
writew(tmp & ~NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
- readw( mmio + NV_ADMA_CTL ); /* flush posted write */
+ readw(mmio + NV_ADMA_CTL ); /* flush posted write */
return 0;
}
@@ -1255,10 +1255,10 @@ static int nv_adma_port_resume(struct ata_port *ap)
tmp = readw(mmio + NV_ADMA_CTL);
writew(tmp | NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
- readw( mmio + NV_ADMA_CTL ); /* flush posted write */
+ readw(mmio + NV_ADMA_CTL ); /* flush posted write */
udelay(1);
writew(tmp & ~NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
- readw( mmio + NV_ADMA_CTL ); /* flush posted write */
+ readw(mmio + NV_ADMA_CTL ); /* flush posted write */
return 0;
}
@@ -1359,12 +1359,12 @@ static int nv_adma_use_reg_mode(struct ata_queued_cmd *qc)
/* ADMA engine can only be used for non-ATAPI DMA commands,
or interrupt-driven no-data commands, where a result taskfile
is not required. */
- if((pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE) ||
+ if ((pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE) ||
(qc->tf.flags & ATA_TFLAG_POLLING) ||
(qc->flags & ATA_QCFLAG_RESULT_TF))
return 1;
- if((qc->flags & ATA_QCFLAG_DMAMAP) ||
+ if ((qc->flags & ATA_QCFLAG_DMAMAP) ||
(qc->tf.protocol == ATA_PROT_NODATA))
return 0;
@@ -1401,7 +1401,7 @@ static void nv_adma_qc_prep(struct ata_queued_cmd *qc)
nv_adma_tf_to_cpb(&qc->tf, cpb->tf);
- if(qc->flags & ATA_QCFLAG_DMAMAP) {
+ if (qc->flags & ATA_QCFLAG_DMAMAP) {
nv_adma_fill_sg(qc, cpb);
ctl_flags |= NV_CPB_CTL_APRD_VALID;
} else
@@ -1435,7 +1435,7 @@ static unsigned int nv_adma_qc_issue(struct ata_queued_cmd *qc)
and (number of cpbs to append -1) in top 8 bits */
wmb();
- if(curr_ncq != pp->last_issue_ncq) {
+ if (curr_ncq != pp->last_issue_ncq) {
/* Seems to need some delay before switching between NCQ and non-NCQ
commands, else we get command timeouts and such. */
udelay(20);
@@ -1641,12 +1641,12 @@ static void nv_error_handler(struct ata_port *ap)
static void nv_adma_error_handler(struct ata_port *ap)
{
struct nv_adma_port_priv *pp = ap->private_data;
- if(!(pp->flags & NV_ADMA_PORT_REGISTER_MODE)) {
+ if (!(pp->flags & NV_ADMA_PORT_REGISTER_MODE)) {
void __iomem *mmio = pp->ctl_block;
int i;
u16 tmp;
- if(ata_tag_valid(ap->link.active_tag) || ap->link.sactive) {
+ if (ata_tag_valid(ap->link.active_tag) || ap->link.sactive) {
u32 notifier = readl(mmio + NV_ADMA_NOTIFIER);
u32 notifier_error = readl(mmio + NV_ADMA_NOTIFIER_ERROR);
u32 gen_ctl = readl(pp->gen_block + NV_ADMA_GEN_CTL);
@@ -1660,9 +1660,9 @@ static void nv_adma_error_handler(struct ata_port *ap)
notifier, notifier_error, gen_ctl, status,
cpb_count, next_cpb_idx);
- for( i=0;i<NV_ADMA_MAX_CPBS;i++) {
+ for (i = 0; i < NV_ADMA_MAX_CPBS; i++) {
struct nv_adma_cpb *cpb = &pp->cpb[i];
- if( (ata_tag_valid(ap->link.active_tag) && i == ap->link.active_tag) ||
+ if ((ata_tag_valid(ap->link.active_tag) && i == ap->link.active_tag) ||
ap->link.sactive & (1 << i) )
ata_port_printk(ap, KERN_ERR,
"CPB %d: ctl_flags 0x%x, resp_flags 0x%x\n",
@@ -1674,7 +1674,7 @@ static void nv_adma_error_handler(struct ata_port *ap)
nv_adma_register_mode(ap);
/* Mark all of the CPBs as invalid to prevent them from being executed */
- for( i=0;i<NV_ADMA_MAX_CPBS;i++)
+ for (i = 0; i < NV_ADMA_MAX_CPBS; i++)
pp->cpb[i].ctl_flags &= ~NV_CPB_CTL_CPB_VALID;
/* clear CPB fetch count */
@@ -1683,10 +1683,10 @@ static void nv_adma_error_handler(struct ata_port *ap)
/* Reset channel */
tmp = readw(mmio + NV_ADMA_CTL);
writew(tmp | NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
- readw( mmio + NV_ADMA_CTL ); /* flush posted write */
+ readw(mmio + NV_ADMA_CTL); /* flush posted write */
udelay(1);
writew(tmp & ~NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
- readw( mmio + NV_ADMA_CTL ); /* flush posted write */
+ readw(mmio + NV_ADMA_CTL); /* flush posted write */
}
ata_bmdma_drive_eh(ap, ata_std_prereset, ata_std_softreset,
@@ -2440,32 +2440,32 @@ static int nv_pci_device_resume(struct pci_dev *pdev)
int rc;
rc = ata_pci_device_do_resume(pdev);
- if(rc)
+ if (rc)
return rc;
if (pdev->dev.power.power_state.event == PM_EVENT_SUSPEND) {
- if(hpriv->type >= CK804) {
+ if (hpriv->type >= CK804) {
u8 regval;
pci_read_config_byte(pdev, NV_MCP_SATA_CFG_20, ®val);
regval |= NV_MCP_SATA_CFG_20_SATA_SPACE_EN;
pci_write_config_byte(pdev, NV_MCP_SATA_CFG_20, regval);
}
- if(hpriv->type == ADMA) {
+ if (hpriv->type == ADMA) {
u32 tmp32;
struct nv_adma_port_priv *pp;
/* enable/disable ADMA on the ports appropriately */
pci_read_config_dword(pdev, NV_MCP_SATA_CFG_20, &tmp32);
pp = host->ports[0]->private_data;
- if(pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE)
+ if (pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE)
tmp32 &= ~(NV_MCP_SATA_CFG_20_PORT0_EN |
NV_MCP_SATA_CFG_20_PORT0_PWB_EN);
else
tmp32 |= (NV_MCP_SATA_CFG_20_PORT0_EN |
NV_MCP_SATA_CFG_20_PORT0_PWB_EN);
pp = host->ports[1]->private_data;
- if(pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE)
+ if (pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE)
tmp32 &= ~(NV_MCP_SATA_CFG_20_PORT1_EN |
NV_MCP_SATA_CFG_20_PORT1_PWB_EN);
else
diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c
index b6026bc..16b3b8a 100644
--- a/drivers/ata/sata_sx4.c
+++ b/drivers/ata/sata_sx4.c
@@ -1091,7 +1091,7 @@ static int pdc20621_detect_dimm(struct ata_host *host)
return 0;
if (pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, 9, &data)) {
- if(data <= 0x75)
+ if (data <= 0x75)
return 133;
} else
return 0;
@@ -1254,7 +1254,7 @@ static unsigned int pdc20621_dimm_init(struct ata_host *host)
If SX4 is on PCI-X bus, after 3 seconds, the timer counter
register should be >= (0xffffffff - 3x10^8).
*/
- if(tcount >= PCI_X_TCOUNT) {
+ if (tcount >= PCI_X_TCOUNT) {
ticks = (time_period - tcount);
VPRINTK("Num counters 0x%x (%d)\n", ticks, ticks);
--
1.5.2.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH] [libata] fix 'if(' and similar areas that lack whitespace
2007-10-24 23:48 ` [PATCH] [libata] fix 'if(' and similar areas that lack whitespace Jeff Garzik
@ 2007-10-25 15:35 ` Richard Knutsson
0 siblings, 0 replies; 23+ messages in thread
From: Richard Knutsson @ 2007-10-25 15:35 UTC (permalink / raw)
To: Jeff Garzik; +Cc: LKML, akpm
Jeff Garzik wrote:
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
> drivers/ata/pata_acpi.c | 4 +-
> drivers/ata/pata_optidma.c | 2 +-
> drivers/ata/pata_pdc2027x.c | 2 +-
> drivers/ata/pata_pdc202xx_old.c | 4 +-
> drivers/ata/pata_via.c | 2 +-
> drivers/ata/pata_winbond.c | 2 +-
> drivers/ata/sata_nv.c | 46 +++++++++++++++++++-------------------
> drivers/ata/sata_sx4.c | 4 +-
> 8 files changed, 33 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/ata/pata_acpi.c b/drivers/ata/pata_acpi.c
> index 0f6f7bc..e4542ab 100644
> --- a/drivers/ata/pata_acpi.c
> +++ b/drivers/ata/pata_acpi.c
> @@ -181,7 +181,7 @@ static void pacpi_set_piomode(struct ata_port *ap, struct ata_device *adev)
> int unit = adev->devno;
> struct pata_acpi *acpi = ap->private_data;
>
> - if(!(acpi->gtm.flags & 0x10))
> + if (!(acpi->gtm.flags & 0x10))
> unit = 0;
>
> /* Now stuff the nS values into the structure */
> @@ -202,7 +202,7 @@ static void pacpi_set_dmamode(struct ata_port *ap, struct ata_device *adev)
> int unit = adev->devno;
> struct pata_acpi *acpi = ap->private_data;
>
> - if(!(acpi->gtm.flags & 0x10))
> + if (!(acpi->gtm.flags & 0x10))
> unit = 0;
>
> /* Now stuff the nS values into the structure */
> diff --git a/drivers/ata/pata_optidma.c b/drivers/ata/pata_optidma.c
> index 6b07b5b..f9b485a 100644
> --- a/drivers/ata/pata_optidma.c
> +++ b/drivers/ata/pata_optidma.c
> @@ -449,7 +449,7 @@ static int optiplus_with_udma(struct pci_dev *pdev)
>
> /* Find function 1 */
> dev1 = pci_get_device(0x1045, 0xC701, NULL);
> - if(dev1 == NULL)
> + if (dev1 == NULL)
>
Maybe "if (!dev1)" instead?
> return 0;
>
> /* Rev must be >= 0x10 */
>
>
<snip>
> diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
> index 2e0279f..9210bb2 100644
> --- a/drivers/ata/sata_nv.c
> +++ b/drivers/ata/sata_nv.c
> @@ -1012,7 +1012,7 @@ static irqreturn_t nv_adma_interrupt(int irq, void *dev_instance)
> u32 check_commands;
> int pos, error = 0;
>
> - if(ata_tag_valid(ap->link.active_tag))
> + if (ata_tag_valid(ap->link.active_tag))
> check_commands = 1 << ap->link.active_tag;
> else
> check_commands = ap->link.sactive;
> @@ -1028,7 +1028,7 @@ static irqreturn_t nv_adma_interrupt(int irq, void *dev_instance)
> }
> }
>
> - if(notifier_clears[0] || notifier_clears[1]) {
> + if (notifier_clears[0] || notifier_clears[1]) {
> /* Note: Both notifier clear registers must be written
> if either is set, even if one is zero, according to NVIDIA. */
> struct nv_adma_port_priv *pp = host->ports[0]->private_data;
> @@ -1119,7 +1119,7 @@ static void nv_adma_post_internal_cmd(struct ata_queued_cmd *qc)
> {
> struct nv_adma_port_priv *pp = qc->ap->private_data;
>
> - if(pp->flags & NV_ADMA_PORT_REGISTER_MODE)
> + if (pp->flags & NV_ADMA_PORT_REGISTER_MODE)
> ata_bmdma_post_internal_cmd(qc);
> }
>
> @@ -1194,10 +1194,10 @@ static int nv_adma_port_start(struct ata_port *ap)
>
> tmp = readw(mmio + NV_ADMA_CTL);
> writew(tmp | NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
> - readw( mmio + NV_ADMA_CTL ); /* flush posted write */
> + readw(mmio + NV_ADMA_CTL ); /* flush posted write */
>
Missed the space before closing parentheses...
> udelay(1);
> writew(tmp & ~NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
> - readw( mmio + NV_ADMA_CTL ); /* flush posted write */
> + readw(mmio + NV_ADMA_CTL ); /* flush posted write */
>
and here
>
> return 0;
> }
> @@ -1255,10 +1255,10 @@ static int nv_adma_port_resume(struct ata_port *ap)
>
> tmp = readw(mmio + NV_ADMA_CTL);
> writew(tmp | NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
> - readw( mmio + NV_ADMA_CTL ); /* flush posted write */
> + readw(mmio + NV_ADMA_CTL ); /* flush posted write */
>
and here
> udelay(1);
> writew(tmp & ~NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
> - readw( mmio + NV_ADMA_CTL ); /* flush posted write */
> + readw(mmio + NV_ADMA_CTL ); /* flush posted write */
>
and here
>
> return 0;
> }
>
The rest looks good.
cu
Richard Knutsson
^ permalink raw reply [flat|nested] 23+ messages in thread