The icside driver can be configured without DMA support, but it
doesn't compile in this case, because DMA operations are referenced.
drivers/ide/icside.c:523: error: ‘icside_v6_port_ops’ undeclared
drivers/ide/icside.c:522: error: ‘icside_dma_init’ undeclared
Signed-off-by: Christian Dietrich <[email protected]>
---
drivers/ide/icside.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
I attach the configuration, that exposes this compilation error. This patch was
*NOT* tested on real hardware. I don't if the fix really does solve the
problem, but at least it is a hint that there is a problem.
diff --git a/drivers/ide/icside.c b/drivers/ide/icside.c
index 8716066..62c59c1 100644
--- a/drivers/ide/icside.c
+++ b/drivers/ide/icside.c
@@ -271,7 +271,7 @@ static void icside_set_dma_mode(ide_hwif_t *hwif, ide_drive_t *drive)
ide_set_drivedata(drive, (void *)cycle_time);
- printk("%s: %s selected (peak %dMB/s)\n", drive->name,
+ printk("%s: %s selected (peak %ldMB/s)\n", drive->name,
ide_xfer_verbose(xfer_mode),
2000 / (unsigned long)ide_get_drivedata(drive));
}
@@ -456,7 +456,9 @@ err_free:
static const struct ide_port_info icside_v6_port_info __initdata = {
.init_dma = icside_dma_off_init,
.port_ops = &icside_v6_no_dma_port_ops,
+#ifdef CONFIG_BLK_DEV_IDEDMA_ICS
.dma_ops = &icside_v6_dma_ops,
+#endif
.host_flags = IDE_HFLAG_SERIALIZE | IDE_HFLAG_MMIO,
.mwdma_mask = ATA_MWDMA2,
.swdma_mask = ATA_SWDMA2,
@@ -518,10 +520,12 @@ icside_register_v6(struct icside_state *state, struct expansion_card *ec)
ecard_set_drvdata(ec, state);
+#ifdef CONFIG_BLK_DEV_IDEDMA_ICS
if (ec->dma != NO_DMA && !request_dma(ec->dma, DRV_NAME)) {
d.init_dma = icside_dma_init;
d.port_ops = &icside_v6_port_ops;
} else
+#endif
d.dma_ops = NULL;
ret = ide_host_register(host, &d, hws);
--
1.7.5.4
Hello.
On 29-05-2012 14:30, Christian Dietrich wrote:
> The icside driver can be configured without DMA support, but it
> doesn't compile in this case, because DMA operations are referenced.
> drivers/ide/icside.c:523: error: ‘icside_v6_port_ops’ undeclared
> drivers/ide/icside.c:522: error: ‘icside_dma_init’ undeclared
> Signed-off-by: Christian Dietrich<[email protected]>
> ---
> drivers/ide/icside.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
> I attach the configuration, that exposes this compilation error.
Apparently, you forgot to do it. :-)
> This patch was
> *NOT* tested on real hardware. I don't if the fix really does solve the
> problem, but at least it is a hint that there is a problem.
> diff --git a/drivers/ide/icside.c b/drivers/ide/icside.c
> index 8716066..62c59c1 100644
> --- a/drivers/ide/icside.c
> +++ b/drivers/ide/icside.c
> @@ -271,7 +271,7 @@ static void icside_set_dma_mode(ide_hwif_t *hwif, ide_drive_t *drive)
>
> ide_set_drivedata(drive, (void *)cycle_time);
>
> - printk("%s: %s selected (peak %dMB/s)\n", drive->name,
> + printk("%s: %s selected (peak %ldMB/s)\n", drive->name,
Unrelated change, should go into its own patch.
> ide_xfer_verbose(xfer_mode),
> 2000 / (unsigned long)ide_get_drivedata(drive));
> }
> @@ -456,7 +456,9 @@ err_free:
> static const struct ide_port_info icside_v6_port_info __initdata = {
> .init_dma = icside_dma_off_init,
> .port_ops =&icside_v6_no_dma_port_ops,
> +#ifdef CONFIG_BLK_DEV_IDEDMA_ICS
You forgot to remove the useless '#define icside_v6_dma_ops NULL' above.
Also, you could set .dma_ops inside the 'if' below (and get rid of 'else'
then), removing this line.
> .dma_ops =&icside_v6_dma_ops,
> +#endif
> .host_flags = IDE_HFLAG_SERIALIZE | IDE_HFLAG_MMIO,
> .mwdma_mask = ATA_MWDMA2,
> .swdma_mask = ATA_SWDMA2,
> @@ -518,10 +520,12 @@ icside_register_v6(struct icside_state *state, struct expansion_card *ec)
>
> ecard_set_drvdata(ec, state);
>
> +#ifdef CONFIG_BLK_DEV_IDEDMA_ICS
> if (ec->dma != NO_DMA && !request_dma(ec->dma, DRV_NAME)) {
> d.init_dma = icside_dma_init;
> d.port_ops = &icside_v6_port_ops;
> } else
> +#endif
> d.dma_ops = NULL;
>
> ret = ide_host_register(host,&d, hws);
MBR, Sergei
Peak datarate is never bigger than an integer, and can therefore, as
suggested in the printk format string, casted to an integer.
---
drivers/ide/icside.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/ide/icside.c b/drivers/ide/icside.c
index 83e5100..755ef6b 100644
--- a/drivers/ide/icside.c
+++ b/drivers/ide/icside.c
@@ -271,9 +271,9 @@ static void icside_set_dma_mode(ide_hwif_t *hwif, ide_drive_t *drive)
ide_set_drivedata(drive, (void *)cycle_time);
- printk("%s: %s selected (peak %dMB/s)\n", drive->name,
- ide_xfer_verbose(xfer_mode),
- 2000 / (unsigned long)ide_get_drivedata(drive));
+ printk(KERN_INFO "%s: %s selected (peak %dMB/s)\n", drive->name,
+ ide_xfer_verbose(xfer_mode),
+ (int) (2000 / (unsigned long)ide_get_drivedata(drive)));
}
static const struct ide_port_ops icside_v6_port_ops = {
--
1.7.5.4
Please submit these patches with proper signoffs et al.
On Thu, 2012-05-31 at 12:08 +0200, Christian Dietrich wrote:
> Peak datarate is never bigger than an integer, and can therefore, as
> suggested in the printk format string, casted to an integer.
> ---
> drivers/ide/icside.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ide/icside.c b/drivers/ide/icside.c
> index 83e5100..755ef6b 100644
> --- a/drivers/ide/icside.c
> +++ b/drivers/ide/icside.c
> @@ -271,9 +271,9 @@ static void icside_set_dma_mode(ide_hwif_t *hwif, ide_drive_t *drive)
>
> ide_set_drivedata(drive, (void *)cycle_time);
>
> - printk("%s: %s selected (peak %dMB/s)\n", drive->name,
> - ide_xfer_verbose(xfer_mode),
> - 2000 / (unsigned long)ide_get_drivedata(drive));
> + printk(KERN_INFO "%s: %s selected (peak %dMB/s)\n", drive->name,
> + ide_xfer_verbose(xfer_mode),
> + (int) (2000 / (unsigned long)ide_get_drivedata(drive)));
Why do an unsigned long int divide then cast
the result at all? Why not just
do a divide without the cast?
Using ide_get_drivedata is rather odd too as it would be
more readable as without the set/get and just use:
printk(KERN_IFNO "%s: %s selected (peak %luMB/s)\n",
drive->name, ide_xfer_verbose(xfer_mode),
2000UL / cycle_time);
ide_set_drivedata(drive, (void *)cycle_time);
without the additional get.
Also, cycle_time _could_ be used uninitialized
if somehow an xfer_mode switch/case isn't reached.
The icside driver can be configured without DMA support, but it
doesn't compile then, because DMA operations are referenced.
drivers/ide/icside.c:523: error: 'icside_v6_port_ops' undeclared
drivers/ide/icside.c:522: error: 'icside_dma_init' undeclared
Signed-off-by: Christian Dietrich<[email protected]>
---
drivers/ide/icside.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/ide/icside.c b/drivers/ide/icside.c
index 8716066..83e5100 100644
--- a/drivers/ide/icside.c
+++ b/drivers/ide/icside.c
@@ -375,8 +375,6 @@ static const struct ide_dma_ops icside_v6_dma_ops = {
.dma_test_irq = icside_dma_test_irq,
.dma_lost_irq = ide_dma_lost_irq,
};
-#else
-#define icside_v6_dma_ops NULL
#endif
static int icside_dma_off_init(ide_hwif_t *hwif, const struct ide_port_info *d)
@@ -456,7 +454,6 @@ err_free:
static const struct ide_port_info icside_v6_port_info __initdata = {
.init_dma = icside_dma_off_init,
.port_ops = &icside_v6_no_dma_port_ops,
- .dma_ops = &icside_v6_dma_ops,
.host_flags = IDE_HFLAG_SERIALIZE | IDE_HFLAG_MMIO,
.mwdma_mask = ATA_MWDMA2,
.swdma_mask = ATA_SWDMA2,
@@ -518,11 +515,13 @@ icside_register_v6(struct icside_state *state, struct expansion_card *ec)
ecard_set_drvdata(ec, state);
+#ifdef CONFIG_BLK_DEV_IDEDMA_ICS
if (ec->dma != NO_DMA && !request_dma(ec->dma, DRV_NAME)) {
d.init_dma = icside_dma_init;
d.port_ops = &icside_v6_port_ops;
- } else
- d.dma_ops = NULL;
+ d.dma_ops = &icside_v6_dma_ops;
+ }
+#endif
ret = ide_host_register(host, &d, hws);
if (ret)
--
1.7.5.4
Use correct format string parameter for the peak datarate, and prevent
uninitialized use of cycle_time.
Signed-off-by: Christian Dietrich <[email protected]>
---
drivers/ide/icside.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/ide/icside.c b/drivers/ide/icside.c
index 83e5100..bcb507b 100644
--- a/drivers/ide/icside.c
+++ b/drivers/ide/icside.c
@@ -236,7 +236,7 @@ static const struct ide_port_ops icside_v6_no_dma_port_ops = {
*/
static void icside_set_dma_mode(ide_hwif_t *hwif, ide_drive_t *drive)
{
- unsigned long cycle_time;
+ unsigned long cycle_time = 0;
int use_dma_info = 0;
const u8 xfer_mode = drive->dma_mode;
@@ -271,9 +271,9 @@ static void icside_set_dma_mode(ide_hwif_t *hwif, ide_drive_t *drive)
ide_set_drivedata(drive, (void *)cycle_time);
- printk("%s: %s selected (peak %dMB/s)\n", drive->name,
- ide_xfer_verbose(xfer_mode),
- 2000 / (unsigned long)ide_get_drivedata(drive));
+ printk(KERN_INFO "%s: %s selected (peak %luMB/s)\n",
+ drive->name, ide_xfer_verbose(xfer_mode),
+ 2000 / (cycle_time ? cycle_time : (unsigned long) -1));
}
static const struct ide_port_ops icside_v6_port_ops = {
--
1.7.5.4
From: Christian Dietrich <[email protected]>
Date: Tue, 05 Jun 2012 14:23:20 +0200
> The icside driver can be configured without DMA support, but it
> doesn't compile then, because DMA operations are referenced.
>
> drivers/ide/icside.c:523: error: 'icside_v6_port_ops' undeclared
> drivers/ide/icside.c:522: error: 'icside_dma_init' undeclared
>
> Signed-off-by: Christian Dietrich<[email protected]>
Applied.
From: Christian Dietrich <[email protected]>
Date: Tue, 05 Jun 2012 14:28:59 +0200
> Use correct format string parameter for the peak datarate, and prevent
> uninitialized use of cycle_time.
>
> Signed-off-by: Christian Dietrich <[email protected]>
Applied.