* [PATCH] libata: remove ata_sff_data_xfer_noirq()
@ 2018-05-04 15:06 Sebastian Andrzej Siewior
2018-05-07 15:49 ` Tejun Heo
0 siblings, 1 reply; 4+ messages in thread
From: Sebastian Andrzej Siewior @ 2018-05-04 15:06 UTC (permalink / raw)
To: linux-ide
Cc: tglx, Tejun Heo, Bartlomiej Zolnierkiewicz, linux-doc,
Jonathan Corbet, Sebastian Andrzej Siewior
ata_sff_data_xfer_noirq() is invoked via the ->sff_data_xfer hook. The
latter is invoked by ata_pio_sector(), atapi_send_cdb() and
__atapi_pio_bytes() which in turn is invoked by ata_sff_hsm_move().
The latter function requires that the "ap->lock" lock is held which
needs to be taken with disabled interrupts.
There is no need have to have ata_sff_data_xfer_noirq() which invokes
ata_sff_data_xfer32() with disabled interrupts because at this point the
interrupts are already disabled.
Remove the function and its references to it and replace all callers
with ata_sff_data_xfer32().
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
Documentation/driver-api/libata.rst | 3 +--
drivers/ata/libata-sff.c | 30 -----------------------------
drivers/ata/pata_cmd640.c | 2 +-
drivers/ata/pata_icside.c | 2 +-
drivers/ata/pata_imx.c | 2 +-
drivers/ata/pata_legacy.c | 6 +++---
drivers/ata/pata_palmld.c | 2 +-
drivers/ata/pata_pcmcia.c | 2 +-
drivers/ata/pata_platform.c | 2 +-
drivers/ata/pata_via.c | 2 +-
include/linux/libata.h | 2 --
11 files changed, 11 insertions(+), 44 deletions(-)
diff --git a/Documentation/driver-api/libata.rst b/Documentation/driver-api/libata.rst
index 4adc056f7635..70e180e6b93d 100644
--- a/Documentation/driver-api/libata.rst
+++ b/Documentation/driver-api/libata.rst
@@ -118,8 +118,7 @@ PIO data read/write
All bmdma-style drivers must implement this hook. This is the low-level
operation that actually copies the data bytes during a PIO data
transfer. Typically the driver will choose one of
-:c:func:`ata_sff_data_xfer_noirq`, :c:func:`ata_sff_data_xfer`, or
-:c:func:`ata_sff_data_xfer32`.
+:c:func:`ata_sff_data_xfer`, or :c:func:`ata_sff_data_xfer32`.
ATA command execute
~~~~~~~~~~~~~~~~~~~
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index cc2f2e35f4c2..c5ea0fc635e5 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -657,36 +657,6 @@ unsigned int ata_sff_data_xfer32(struct ata_queued_cmd *qc, unsigned char *buf,
}
EXPORT_SYMBOL_GPL(ata_sff_data_xfer32);
-/**
- * ata_sff_data_xfer_noirq - Transfer data by PIO
- * @qc: queued command
- * @buf: data buffer
- * @buflen: buffer length
- * @rw: read/write
- *
- * Transfer data from/to the device data register by PIO. Do the
- * transfer with interrupts disabled.
- *
- * LOCKING:
- * Inherited from caller.
- *
- * RETURNS:
- * Bytes consumed.
- */
-unsigned int ata_sff_data_xfer_noirq(struct ata_queued_cmd *qc, unsigned char *buf,
- unsigned int buflen, int rw)
-{
- unsigned long flags;
- unsigned int consumed;
-
- local_irq_save(flags);
- consumed = ata_sff_data_xfer32(qc, buf, buflen, rw);
- local_irq_restore(flags);
-
- return consumed;
-}
-EXPORT_SYMBOL_GPL(ata_sff_data_xfer_noirq);
-
/**
* ata_pio_sector - Transfer a sector of data.
* @qc: Command on going
diff --git a/drivers/ata/pata_cmd640.c b/drivers/ata/pata_cmd640.c
index c47caa807fa9..e3532eda7b05 100644
--- a/drivers/ata/pata_cmd640.c
+++ b/drivers/ata/pata_cmd640.c
@@ -178,7 +178,7 @@ static struct scsi_host_template cmd640_sht = {
static struct ata_port_operations cmd640_port_ops = {
.inherits = &ata_sff_port_ops,
/* In theory xfer_noirq is not needed once we kill the prefetcher */
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer32,
.sff_irq_check = cmd640_sff_irq_check,
.qc_issue = cmd640_qc_issue,
.cable_detect = ata_cable_40wire,
diff --git a/drivers/ata/pata_icside.c b/drivers/ata/pata_icside.c
index 188f2f2eb21f..c272f2cbb47c 100644
--- a/drivers/ata/pata_icside.c
+++ b/drivers/ata/pata_icside.c
@@ -324,7 +324,7 @@ static struct ata_port_operations pata_icside_port_ops = {
.inherits = &ata_bmdma_port_ops,
/* no need to build any PRD tables for DMA */
.qc_prep = ata_noop_qc_prep,
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer32,
.bmdma_setup = pata_icside_bmdma_setup,
.bmdma_start = pata_icside_bmdma_start,
.bmdma_stop = pata_icside_bmdma_stop,
diff --git a/drivers/ata/pata_imx.c b/drivers/ata/pata_imx.c
index d4caa23f5a88..108101325efd 100644
--- a/drivers/ata/pata_imx.c
+++ b/drivers/ata/pata_imx.c
@@ -102,7 +102,7 @@ static struct scsi_host_template pata_imx_sht = {
static struct ata_port_operations pata_imx_port_ops = {
.inherits = &ata_sff_port_ops,
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer32,
.cable_detect = ata_cable_unknown,
.set_piomode = pata_imx_set_piomode,
};
diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
index 53828b6c3044..8ea4b8431fc8 100644
--- a/drivers/ata/pata_legacy.c
+++ b/drivers/ata/pata_legacy.c
@@ -246,12 +246,12 @@ static const struct ata_port_operations legacy_base_port_ops = {
static struct ata_port_operations simple_port_ops = {
.inherits = &legacy_base_port_ops,
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer32,
};
static struct ata_port_operations legacy_port_ops = {
.inherits = &legacy_base_port_ops,
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer32,
.set_mode = legacy_set_mode,
};
@@ -341,7 +341,7 @@ static unsigned int pdc_data_xfer_vlb(struct ata_queued_cmd *qc,
}
local_irq_restore(flags);
} else
- buflen = ata_sff_data_xfer_noirq(qc, buf, buflen, rw);
+ buflen = ata_sff_data_xfer32(qc, buf, buflen, rw);
return buflen;
}
diff --git a/drivers/ata/pata_palmld.c b/drivers/ata/pata_palmld.c
index 8c0d7d736b7a..d071ab6864a8 100644
--- a/drivers/ata/pata_palmld.c
+++ b/drivers/ata/pata_palmld.c
@@ -44,7 +44,7 @@ static struct scsi_host_template palmld_sht = {
static struct ata_port_operations palmld_port_ops = {
.inherits = &ata_sff_port_ops,
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer32,
.cable_detect = ata_cable_40wire,
};
diff --git a/drivers/ata/pata_pcmcia.c b/drivers/ata/pata_pcmcia.c
index a541eacc5e95..9b0e6c72e3f9 100644
--- a/drivers/ata/pata_pcmcia.c
+++ b/drivers/ata/pata_pcmcia.c
@@ -151,7 +151,7 @@ static struct scsi_host_template pcmcia_sht = {
static struct ata_port_operations pcmcia_port_ops = {
.inherits = &ata_sff_port_ops,
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer32,
.cable_detect = ata_cable_40wire,
.set_mode = pcmcia_set_mode,
};
diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c
index c503ded87bb8..d6f8f5406442 100644
--- a/drivers/ata/pata_platform.c
+++ b/drivers/ata/pata_platform.c
@@ -49,7 +49,7 @@ static struct scsi_host_template pata_platform_sht = {
static struct ata_port_operations pata_platform_port_ops = {
.inherits = &ata_sff_port_ops,
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer32,
.cable_detect = ata_cable_unknown,
.set_mode = pata_platform_set_mode,
};
diff --git a/drivers/ata/pata_via.c b/drivers/ata/pata_via.c
index 1ca6bcab369f..fd19f1ce83aa 100644
--- a/drivers/ata/pata_via.c
+++ b/drivers/ata/pata_via.c
@@ -471,7 +471,7 @@ static struct ata_port_operations via_port_ops = {
static struct ata_port_operations via_port_ops_noirq = {
.inherits = &via_port_ops,
- .sff_data_xfer = ata_sff_data_xfer_noirq,
+ .sff_data_xfer = ata_sff_data_xfer32,
};
/**
diff --git a/include/linux/libata.h b/include/linux/libata.h
index ed9826b21c5e..1435e0e68823 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1831,8 +1831,6 @@ extern unsigned int ata_sff_data_xfer(struct ata_queued_cmd *qc,
unsigned char *buf, unsigned int buflen, int rw);
extern unsigned int ata_sff_data_xfer32(struct ata_queued_cmd *qc,
unsigned char *buf, unsigned int buflen, int rw);
-extern unsigned int ata_sff_data_xfer_noirq(struct ata_queued_cmd *qc,
- unsigned char *buf, unsigned int buflen, int rw);
extern void ata_sff_irq_on(struct ata_port *ap);
extern void ata_sff_irq_clear(struct ata_port *ap);
extern int ata_sff_hsm_move(struct ata_port *ap, struct ata_queued_cmd *qc,
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] libata: remove ata_sff_data_xfer_noirq()
2018-05-04 15:06 [PATCH] libata: remove ata_sff_data_xfer_noirq() Sebastian Andrzej Siewior
@ 2018-05-07 15:49 ` Tejun Heo
2018-05-07 15:52 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 4+ messages in thread
From: Tejun Heo @ 2018-05-07 15:49 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: linux-ide, tglx, Bartlomiej Zolnierkiewicz, linux-doc, Jonathan Corbet
Hello, Sebastian.
On Fri, May 04, 2018 at 05:06:20PM +0200, Sebastian Andrzej Siewior wrote:
> ata_sff_data_xfer_noirq() is invoked via the ->sff_data_xfer hook. The
> latter is invoked by ata_pio_sector(), atapi_send_cdb() and
> __atapi_pio_bytes() which in turn is invoked by ata_sff_hsm_move().
> The latter function requires that the "ap->lock" lock is held which
> needs to be taken with disabled interrupts.
>
> There is no need have to have ata_sff_data_xfer_noirq() which invokes
> ata_sff_data_xfer32() with disabled interrupts because at this point the
> interrupts are already disabled.
> Remove the function and its references to it and replace all callers
> with ata_sff_data_xfer32().
Can you please add irq disabled assert to ata_sff_data_xfer*()?
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libata: remove ata_sff_data_xfer_noirq()
2018-05-07 15:49 ` Tejun Heo
@ 2018-05-07 15:52 ` Sebastian Andrzej Siewior
2018-05-24 13:58 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 4+ messages in thread
From: Sebastian Andrzej Siewior @ 2018-05-07 15:52 UTC (permalink / raw)
To: Tejun Heo
Cc: linux-ide, tglx, Bartlomiej Zolnierkiewicz, linux-doc, Jonathan Corbet
On 2018-05-07 08:49:08 [-0700], Tejun Heo wrote:
> Hello, Sebastian.
>
> On Fri, May 04, 2018 at 05:06:20PM +0200, Sebastian Andrzej Siewior wrote:
> > ata_sff_data_xfer_noirq() is invoked via the ->sff_data_xfer hook. The
> > latter is invoked by ata_pio_sector(), atapi_send_cdb() and
> > __atapi_pio_bytes() which in turn is invoked by ata_sff_hsm_move().
> > The latter function requires that the "ap->lock" lock is held which
> > needs to be taken with disabled interrupts.
> >
> > There is no need have to have ata_sff_data_xfer_noirq() which invokes
> > ata_sff_data_xfer32() with disabled interrupts because at this point the
> > interrupts are already disabled.
> > Remove the function and its references to it and replace all callers
> > with ata_sff_data_xfer32().
>
> Can you please add irq disabled assert to ata_sff_data_xfer*()?
Why irq-disabled assert? Can we use lockdep_assert_held() instead?
> Thanks.
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libata: remove ata_sff_data_xfer_noirq()
2018-05-07 15:52 ` Sebastian Andrzej Siewior
@ 2018-05-24 13:58 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 4+ messages in thread
From: Sebastian Andrzej Siewior @ 2018-05-24 13:58 UTC (permalink / raw)
To: Tejun Heo
Cc: linux-ide, tglx, Bartlomiej Zolnierkiewicz, linux-doc, Jonathan Corbet
On 2018-05-07 17:52:16 [+0200], To Tejun Heo wrote:
> On 2018-05-07 08:49:08 [-0700], Tejun Heo wrote:
> > Hello, Sebastian.
Hi Tejun,
> > On Fri, May 04, 2018 at 05:06:20PM +0200, Sebastian Andrzej Siewior wrote:
> > > ata_sff_data_xfer_noirq() is invoked via the ->sff_data_xfer hook. The
> > > latter is invoked by ata_pio_sector(), atapi_send_cdb() and
> > > __atapi_pio_bytes() which in turn is invoked by ata_sff_hsm_move().
> > > The latter function requires that the "ap->lock" lock is held which
> > > needs to be taken with disabled interrupts.
> > >
> > > There is no need have to have ata_sff_data_xfer_noirq() which invokes
> > > ata_sff_data_xfer32() with disabled interrupts because at this point the
> > > interrupts are already disabled.
> > > Remove the function and its references to it and replace all callers
> > > with ata_sff_data_xfer32().
> >
> > Can you please add irq disabled assert to ata_sff_data_xfer*()?
>
> Why irq-disabled assert? Can we use lockdep_assert_held() instead?
That irq-disabled assert won't work on RT as expected that is why I
intend to remove the local_irq_save() (which is not needed). If we could
avoid the irq-disabled assert or use a lock instead, then it wouldn't
trigger another error on RT.
> > Thanks.
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-05-24 13:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-04 15:06 [PATCH] libata: remove ata_sff_data_xfer_noirq() Sebastian Andrzej Siewior
2018-05-07 15:49 ` Tejun Heo
2018-05-07 15:52 ` Sebastian Andrzej Siewior
2018-05-24 13:58 ` Sebastian Andrzej Siewior
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.