Re: [BK PATCHES] 2.6.x libata fixes (mostly)
Roland Dreier schrieb: > Prakash> If I am not totally mistaken this is not gcc4 friendly > Prakash> code. (lvalue thing...) > > Actually you misread the code slightly. It's a little subtle, but > code like > > *(__le32 *)prd = cpu_to_le32(len); > > is not using a cast as an lvalue. It's dereferencing a cast and as > such is totally correct, idiomatic and clean C. OK, thanks for clearing that up. Obviously my C knowledge needs to be improved... -- Prakash Punnoor formerly known as Prakash K. Cheemplavam signature.asc Description: OpenPGP digital signature
Re: [BK PATCHES] 2.6.x libata fixes (mostly)
Prakash> If I am not totally mistaken this is not gcc4 friendly Prakash> code. (lvalue thing...) Actually you misread the code slightly. It's a little subtle, but code like *(__le32 *)prd = cpu_to_le32(len); is not using a cast as an lvalue. It's dereferencing a cast and as such is totally correct, idiomatic and clean C. - Roland - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [BK PATCHES] 2.6.x libata fixes (mostly)
Alexey Dobriyan schrieb: > On Wednesday 23 February 2005 21:57, Jeff Garzik wrote: >>+ addr = sg_dma_address(sg); >>+ *(u64 *)prd = cpu_to_le64(addr); > > > *(__le64 *) prd > > >>+ prd += sizeof(u64); > > >>+ len = sg_dma_len(sg); >>+ *(u32 *)prd = cpu_to_le32(len); > > > *(__le32 *) prd If I am not totally mistaken this is not gcc4 friendly code. (lvalue thing...) Wouldn't it be better to prevent double patching? -- Prakash Punnoor formerly known as Prakash K. Cheemplavam signature.asc Description: OpenPGP digital signature
Re: [BK PATCHES] 2.6.x libata fixes (mostly)
Thanks. All this stuff was minor, so I'll wait until 2.6.11 release to update. I forwarded the qstor stuff to maintainer Mark Lord. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [BK PATCHES] 2.6.x libata fixes (mostly)
On Wednesday 23 February 2005 23:45, Alexey Dobriyan wrote:
> > +static void qs_ata_setup_port(struct ata_ioports *port, unsigned long base)
> > +{
> > + port->cmd_addr =
>
> > + port->error_addr=
>
> > + port->status_addr =
>
> > + port->altstatus_addr=
>
> Oo-oops...
Too much snipping and time to sleep for me, sorry. :-(
Alexey
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [BK PATCHES] 2.6.x libata fixes (mostly)
On Wednesday 23 February 2005 21:57, Jeff Garzik wrote:
> This BK push includes additional hardware support, but that's only
> because it's (a) obviously low impact and (b) it was in the queue.
> --- a/drivers/scsi/ahci.c
> +++ b/drivers/scsi/ahci.c
> +static u8 ahci_check_err(struct ata_port *ap)
> +{
> + void *mmio = (void *) ap->ioaddr.cmd_addr;
void __iomem *
> + return (readl(mmio + PORT_TFDATA) >> 8) & 0xFF;
> --- a/drivers/scsi/libata-core.c
> +++ b/drivers/scsi/libata-core.c
> + * ata_qc_free - free unused ata_queued_cmd
> + * @qc: Command to complete
"Command to free"?
--- /dev/null
+++ b/drivers/scsi/sata_qstor.c
> + u8 *prd = pp->pkt + QS_CPB_BYTES;
> + for (nelem = 0; nelem < qc->n_elem; nelem++,sg++) {
> + u64 addr;
> + u32 len;
> + addr = sg_dma_address(sg);
> + *(u64 *)prd = cpu_to_le64(addr);
*(__le64 *) prd
> + prd += sizeof(u64);
> + len = sg_dma_len(sg);
> + *(u32 *)prd = cpu_to_le32(len);
*(__le32 *) prd
> + prd += sizeof(u64);
Should this be "prd += sizeof(u32)"? Looks suspicious.
> +static void qs_qc_prep(struct ata_queued_cmd *qc)
> +{
> + *(u32 *)([ 4]) = cpu_to_le32(qc->nsect * ATA_SECT_SIZE);
> + *(u32 *)([ 8]) = cpu_to_le32(qc->n_elem);
> + *(u64 *)([16]) = cpu_to_le64(addr);
__le* again...
> +static void qs_ata_setup_port(struct ata_ioports *port, unsigned long base)
> +{
> + port->cmd_addr =
> + port->error_addr=
> + port->status_addr =
> + port->altstatus_addr=
Oo-oops...
> +static int qs_set_dma_masks(struct pci_dev *pdev, void __iomem *mmio_base)
> +{
> + if (have_64bit_bus &&
> + !pci_set_dma_mask(pdev, 0xULL)) {
> + rc = pci_set_consistent_dma_mask(pdev, 0xULL);
> + if (rc) {
> + rc = pci_set_consistent_dma_mask(pdev, 0xULL);
We already have DMA_{32,64}BIT_MASK.
> + } else {
> + rc = pci_set_dma_mask(pdev, 0xULL);
> + rc = pci_set_consistent_dma_mask(pdev, 0xULL);
Alexey
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[BK PATCHES] 2.6.x libata fixes (mostly)
This BK push includes additional hardware support, but that's only
because it's (a) obviously low impact and (b) it was in the queue.
Far more important are:
1) API additions, to fix a severe bug: advanced drivers such as AHCI
were directly bitbanging --non-existent-- PCI IDE registers, causing an
oops, because of function calls buried deep within libata.
Solution: add the necessary hooks that should have existed all along,
for this functionality.
2) Fix stomping on active devices, if pci_request_regions() fails. It
is rather more urgent for libata than other situations, since we must
deal with PCI devices that secretly export ISA device regions (legacy
IDE 0x1f0, 0x170).
3) Fix command queue leak, if SCSI->ATA translation fails.
Please do a
bk pull bk://gkernel.bkbits.net/libata-2.6
This will update the following files:
drivers/scsi/Kconfig|8
drivers/scsi/Makefile |1
drivers/scsi/ahci.c | 18 +
drivers/scsi/ata_piix.c |4
drivers/scsi/libata-core.c | 115 ++-
drivers/scsi/libata-scsi.c |1
drivers/scsi/libata.h |1
drivers/scsi/sata_nv.c | 10
drivers/scsi/sata_promise.c |8
drivers/scsi/sata_qstor.c | 700
drivers/scsi/sata_sil.c | 10
drivers/scsi/sata_sis.c | 10
drivers/scsi/sata_svw.c | 10
drivers/scsi/sata_sx4.c |8
drivers/scsi/sata_uli.c | 10
drivers/scsi/sata_via.c | 213 +
drivers/scsi/sata_vsc.c | 10
include/linux/libata.h | 64
18 files changed, 1058 insertions(+), 143 deletions(-)
through these ChangeSets:
<[EMAIL PROTECTED]> (05/02/23 1.2043)
[libata] Add missing hooks, to avoid oops in advanced SATA drivers
Advanced SATA drivers should not (and cannot) use the basic
PCI IDE hooks for checking the Status and Error registers, as these
registers are either in non-standard locations, or simply don't
exist.
In the error handling path, libata was unconditionally calling some
PCI IDE hardware bitbanging functions, which would cause an oops
in the AHCI driver and any other advanced libata driver.
<[EMAIL PROTECTED]> (05/02/18 1.2040)
[PATCH] libata: fix command queue leak when xlat_func fails
ata_scsi_translate allocates from the libata command queue by calling
ata_scsi_qc_new. If xlat_func returns non-zero, control jumps to
err_out which fails to free the allocated command. Fix is to add a
new API to free unused commands.
Signed-off-by: John W. Linville <[EMAIL PROTECTED]>
Signed-off-by: Jeff Garzik <[EMAIL PROTECTED]>
<[EMAIL PROTECTED]> (05/02/17 1.2035.16.1)
[libata] add ->bmdma_{stop,status} hooks
The timeout/error handling path was assuming that the hardware in
question was PCI IDE BMDMA-like, which is incorrect in a few cases.
Turn direct function calls into two new hooks.
<[EMAIL PROTECTED]> (05/02/17 1.2038)
[PATCH] sata_qstor: new basic driver for Pacific Digital
This is a new basic libata SATA driver
for the Pacific Digital QStor SATA/RAID card.
It features ordinary per-drive SATA with single-request DMA
for R/W commands, and PIO for everything else.
It currently does not implement any of the hardware RAID support,
nor hot-plug, nor the tagged/native command-queuing features.
On the other hand, it is small and simple as a result.
Signed-off-by: Mark Lord <[EMAIL PROTECTED]>
Signed-off-by: Jeff Garzik <[EMAIL PROTECTED]>
<[EMAIL PROTECTED]> (05/02/13 1.2037)
[libata] do not call pci_disable_device() for certain errors
If PCI request regions fails, then someone else is using the
hardware we wish to use. For that one case, calling pci_disable_device()
is rather rude.
<[EMAIL PROTECTED]> (04/11/16 1.1938.367.2)
[libata sata_via] add support for VT6421 SATA
<[EMAIL PROTECTED]> (04/11/16 1.1938.367.1)
[libata sata_via] minor cleanups
Preparation for addition of VT6421 support. Mostly moving bits
of code into discrete functions.
diff -Nru a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
--- a/drivers/scsi/Kconfig 2005-02-23 14:50:15 -05:00
+++ b/drivers/scsi/Kconfig 2005-02-23 14:50:15 -05:00
@@ -457,6 +457,14 @@
If unsure, say N.
+config SCSI_SATA_QSTOR
+ tristate "Pacific Digital SATA QStor support"
+ depends on SCSI_SATA && PCI
+ help
+ This option enables support for Pacific Digital Serial ATA QStor.
+
+ If unsure, say N.
+
config SCSI_SATA_SX4
tristate "Promise SATA SX4 support"
depends on SCSI_SATA && PCI && EXPERIMENTAL
diff -Nru a/drivers/scsi/Makefile b/drivers/scsi/Makefile
--- a/drivers/scsi/Makefile 2005-02-23 14:50:15 -05:00
+++ b/drivers/scsi/Makefile 2005-02-23 14:50:15 -05:00
@@ -125,6 +125,7 @@
obj-$(CONFIG_SCSI_SATA_SVW)+= libata.o sata_svw.o
obj-$(CONFIG_SCSI_ATA_PIIX)+= libata.o
[BK PATCHES] 2.6.x libata fixes (mostly)
This BK push includes additional hardware support, but that's only
because it's (a) obviously low impact and (b) it was in the queue.
Far more important are:
1) API additions, to fix a severe bug: advanced drivers such as AHCI
were directly bitbanging --non-existent-- PCI IDE registers, causing an
oops, because of function calls buried deep within libata.
Solution: add the necessary hooks that should have existed all along,
for this functionality.
2) Fix stomping on active devices, if pci_request_regions() fails. It
is rather more urgent for libata than other situations, since we must
deal with PCI devices that secretly export ISA device regions (legacy
IDE 0x1f0, 0x170).
3) Fix command queue leak, if SCSI-ATA translation fails.
Please do a
bk pull bk://gkernel.bkbits.net/libata-2.6
This will update the following files:
drivers/scsi/Kconfig|8
drivers/scsi/Makefile |1
drivers/scsi/ahci.c | 18 +
drivers/scsi/ata_piix.c |4
drivers/scsi/libata-core.c | 115 ++-
drivers/scsi/libata-scsi.c |1
drivers/scsi/libata.h |1
drivers/scsi/sata_nv.c | 10
drivers/scsi/sata_promise.c |8
drivers/scsi/sata_qstor.c | 700
drivers/scsi/sata_sil.c | 10
drivers/scsi/sata_sis.c | 10
drivers/scsi/sata_svw.c | 10
drivers/scsi/sata_sx4.c |8
drivers/scsi/sata_uli.c | 10
drivers/scsi/sata_via.c | 213 +
drivers/scsi/sata_vsc.c | 10
include/linux/libata.h | 64
18 files changed, 1058 insertions(+), 143 deletions(-)
through these ChangeSets:
[EMAIL PROTECTED] (05/02/23 1.2043)
[libata] Add missing hooks, to avoid oops in advanced SATA drivers
Advanced SATA drivers should not (and cannot) use the basic
PCI IDE hooks for checking the Status and Error registers, as these
registers are either in non-standard locations, or simply don't
exist.
In the error handling path, libata was unconditionally calling some
PCI IDE hardware bitbanging functions, which would cause an oops
in the AHCI driver and any other advanced libata driver.
[EMAIL PROTECTED] (05/02/18 1.2040)
[PATCH] libata: fix command queue leak when xlat_func fails
ata_scsi_translate allocates from the libata command queue by calling
ata_scsi_qc_new. If xlat_func returns non-zero, control jumps to
err_out which fails to free the allocated command. Fix is to add a
new API to free unused commands.
Signed-off-by: John W. Linville [EMAIL PROTECTED]
Signed-off-by: Jeff Garzik [EMAIL PROTECTED]
[EMAIL PROTECTED] (05/02/17 1.2035.16.1)
[libata] add -bmdma_{stop,status} hooks
The timeout/error handling path was assuming that the hardware in
question was PCI IDE BMDMA-like, which is incorrect in a few cases.
Turn direct function calls into two new hooks.
[EMAIL PROTECTED] (05/02/17 1.2038)
[PATCH] sata_qstor: new basic driver for Pacific Digital
This is a new basic libata SATA driver
for the Pacific Digital QStor SATA/RAID card.
It features ordinary per-drive SATA with single-request DMA
for R/W commands, and PIO for everything else.
It currently does not implement any of the hardware RAID support,
nor hot-plug, nor the tagged/native command-queuing features.
On the other hand, it is small and simple as a result.
Signed-off-by: Mark Lord [EMAIL PROTECTED]
Signed-off-by: Jeff Garzik [EMAIL PROTECTED]
[EMAIL PROTECTED] (05/02/13 1.2037)
[libata] do not call pci_disable_device() for certain errors
If PCI request regions fails, then someone else is using the
hardware we wish to use. For that one case, calling pci_disable_device()
is rather rude.
[EMAIL PROTECTED] (04/11/16 1.1938.367.2)
[libata sata_via] add support for VT6421 SATA
[EMAIL PROTECTED] (04/11/16 1.1938.367.1)
[libata sata_via] minor cleanups
Preparation for addition of VT6421 support. Mostly moving bits
of code into discrete functions.
diff -Nru a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
--- a/drivers/scsi/Kconfig 2005-02-23 14:50:15 -05:00
+++ b/drivers/scsi/Kconfig 2005-02-23 14:50:15 -05:00
@@ -457,6 +457,14 @@
If unsure, say N.
+config SCSI_SATA_QSTOR
+ tristate Pacific Digital SATA QStor support
+ depends on SCSI_SATA PCI
+ help
+ This option enables support for Pacific Digital Serial ATA QStor.
+
+ If unsure, say N.
+
config SCSI_SATA_SX4
tristate Promise SATA SX4 support
depends on SCSI_SATA PCI EXPERIMENTAL
diff -Nru a/drivers/scsi/Makefile b/drivers/scsi/Makefile
--- a/drivers/scsi/Makefile 2005-02-23 14:50:15 -05:00
+++ b/drivers/scsi/Makefile 2005-02-23 14:50:15 -05:00
@@ -125,6 +125,7 @@
obj-$(CONFIG_SCSI_SATA_SVW)+= libata.o sata_svw.o
obj-$(CONFIG_SCSI_ATA_PIIX)+= libata.o ata_piix.o
Re: [BK PATCHES] 2.6.x libata fixes (mostly)
On Wednesday 23 February 2005 21:57, Jeff Garzik wrote:
This BK push includes additional hardware support, but that's only
because it's (a) obviously low impact and (b) it was in the queue.
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
+static u8 ahci_check_err(struct ata_port *ap)
+{
+ void *mmio = (void *) ap-ioaddr.cmd_addr;
void __iomem *
+ return (readl(mmio + PORT_TFDATA) 8) 0xFF;
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
+ * ata_qc_free - free unused ata_queued_cmd
+ * @qc: Command to complete
Command to free?
--- /dev/null
+++ b/drivers/scsi/sata_qstor.c
+ u8 *prd = pp-pkt + QS_CPB_BYTES;
+ for (nelem = 0; nelem qc-n_elem; nelem++,sg++) {
+ u64 addr;
+ u32 len;
+ addr = sg_dma_address(sg);
+ *(u64 *)prd = cpu_to_le64(addr);
*(__le64 *) prd
+ prd += sizeof(u64);
+ len = sg_dma_len(sg);
+ *(u32 *)prd = cpu_to_le32(len);
*(__le32 *) prd
+ prd += sizeof(u64);
Should this be prd += sizeof(u32)? Looks suspicious.
+static void qs_qc_prep(struct ata_queued_cmd *qc)
+{
+ *(u32 *)(buf[ 4]) = cpu_to_le32(qc-nsect * ATA_SECT_SIZE);
+ *(u32 *)(buf[ 8]) = cpu_to_le32(qc-n_elem);
+ *(u64 *)(buf[16]) = cpu_to_le64(addr);
__le* again...
+static void qs_ata_setup_port(struct ata_ioports *port, unsigned long base)
+{
+ port-cmd_addr =
+ port-error_addr=
+ port-status_addr =
+ port-altstatus_addr=
Oo-oops...
+static int qs_set_dma_masks(struct pci_dev *pdev, void __iomem *mmio_base)
+{
+ if (have_64bit_bus
+ !pci_set_dma_mask(pdev, 0xULL)) {
+ rc = pci_set_consistent_dma_mask(pdev, 0xULL);
+ if (rc) {
+ rc = pci_set_consistent_dma_mask(pdev, 0xULL);
We already have DMA_{32,64}BIT_MASK.
+ } else {
+ rc = pci_set_dma_mask(pdev, 0xULL);
+ rc = pci_set_consistent_dma_mask(pdev, 0xULL);
Alexey
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [BK PATCHES] 2.6.x libata fixes (mostly)
On Wednesday 23 February 2005 23:45, Alexey Dobriyan wrote:
+static void qs_ata_setup_port(struct ata_ioports *port, unsigned long base)
+{
+ port-cmd_addr =
+ port-error_addr=
+ port-status_addr =
+ port-altstatus_addr=
Oo-oops...
Too much snipping and time to sleep for me, sorry. :-(
Alexey
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [BK PATCHES] 2.6.x libata fixes (mostly)
Thanks. All this stuff was minor, so I'll wait until 2.6.11 release to update. I forwarded the qstor stuff to maintainer Mark Lord. Jeff - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [BK PATCHES] 2.6.x libata fixes (mostly)
Alexey Dobriyan schrieb: On Wednesday 23 February 2005 21:57, Jeff Garzik wrote: + addr = sg_dma_address(sg); + *(u64 *)prd = cpu_to_le64(addr); *(__le64 *) prd + prd += sizeof(u64); + len = sg_dma_len(sg); + *(u32 *)prd = cpu_to_le32(len); *(__le32 *) prd If I am not totally mistaken this is not gcc4 friendly code. (lvalue thing...) Wouldn't it be better to prevent double patching? -- Prakash Punnoor formerly known as Prakash K. Cheemplavam signature.asc Description: OpenPGP digital signature
Re: [BK PATCHES] 2.6.x libata fixes (mostly)
Prakash If I am not totally mistaken this is not gcc4 friendly Prakash code. (lvalue thing...) Actually you misread the code slightly. It's a little subtle, but code like *(__le32 *)prd = cpu_to_le32(len); is not using a cast as an lvalue. It's dereferencing a cast and as such is totally correct, idiomatic and clean C. - Roland - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [BK PATCHES] 2.6.x libata fixes (mostly)
Roland Dreier schrieb: Prakash If I am not totally mistaken this is not gcc4 friendly Prakash code. (lvalue thing...) Actually you misread the code slightly. It's a little subtle, but code like *(__le32 *)prd = cpu_to_le32(len); is not using a cast as an lvalue. It's dereferencing a cast and as such is totally correct, idiomatic and clean C. OK, thanks for clearing that up. Obviously my C knowledge needs to be improved... -- Prakash Punnoor formerly known as Prakash K. Cheemplavam signature.asc Description: OpenPGP digital signature
