Re: [PATCH 4/6] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars
Hartley and Ian, Yes, I had a feeling I should have gone with what was present, but my desire to follow the documentation I wasn't too familiar with won out :P I'll spin a new revision of this patchset ASAP with this changed back and all of my bad titles (oof...) fixed as well and I'll make sure to be more careful in the future. Thanks your all of the assistance and patience, Chase On Mon, Apr 28, 2014 at 1:09 PM, Hartley Sweeten wrote: > On Saturday, April 26, 2014 6:37 PM, Chase Southwood wrote: >> This driver only uses PCI bar 1 (devpriv->i_IobaseAmcc), and PCI bar 2 >> (dev->iobase) doon't bother reading the unused PCI bars. >> >> Signed-off-by: Chase Southwood >> Cc: Ian Abbott >> Cc: H Hartley Sweeten >> --- >> >> Hartley, >> >> As far as I can tell from reading the I/O Mapping you sent me, these bar >> numbers are correct, but it seems a bit odd, so please offer correction >> if I am interpreting the document incorrectly. >> >> Thanks, >> Chase >> >> drivers/staging/comedi/drivers/addi_apci_1564.c | 11 ++- >> 1 file changed, 2 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c >> b/drivers/staging/comedi/drivers/addi_apci_1564.c >> index fe42f9d..7e42d47 100644 >> --- a/drivers/staging/comedi/drivers/addi_apci_1564.c >> +++ b/drivers/staging/comedi/drivers/addi_apci_1564.c >> @@ -65,15 +65,8 @@ static int apci1564_auto_attach(struct comedi_device *dev, >> if (ret) >> return ret; >> >> - if (this_board->i_IorangeBase1) >> - dev->iobase = pci_resource_start(pcidev, 1); >> - else >> - dev->iobase = pci_resource_start(pcidev, 0); >> - >> - devpriv->iobase = dev->iobase; >> - devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 0); >> - devpriv->i_IobaseAddon = pci_resource_start(pcidev, 2); >> - devpriv->i_IobaseReserved = pci_resource_start(pcidev, 3); >> + dev->iobase = pci_resource_start(pcidev, 2); >> + devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 1); >> >> /* Initialize parameters that can be overridden in EEPROM */ >> devpriv->s_EeParameters.i_NbrAiChannel = this_board->i_NbrAiChannel; > > The document I sent you from ADDI-DATA does show that the I/O map revision can > be determined by reading the "Header EEPROM" at BAR1 + 0x00. This seems to > verify Ian's comment about earlier versions of the board having a different > mapping. > > As Ian already mentioned. It's probably best to leave the PCI bar resource > mapping > as it currently is. If anyone reports an issue it can be addressed later. > > Regards, > Hartley > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/6] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars
Hartley and Ian, Yes, I had a feeling I should have gone with what was present, but my desire to follow the documentation I wasn't too familiar with won out :P I'll spin a new revision of this patchset ASAP with this changed back and all of my bad titles (oof...) fixed as well and I'll make sure to be more careful in the future. Thanks your all of the assistance and patience, Chase On Mon, Apr 28, 2014 at 1:09 PM, Hartley Sweeten hartl...@visionengravers.com wrote: On Saturday, April 26, 2014 6:37 PM, Chase Southwood wrote: This driver only uses PCI bar 1 (devpriv-i_IobaseAmcc), and PCI bar 2 (dev-iobase) doon't bother reading the unused PCI bars. Signed-off-by: Chase Southwood chase.southw...@gmail.com Cc: Ian Abbott abbo...@mev.co.uk Cc: H Hartley Sweeten hswee...@visionengravers.com --- Hartley, As far as I can tell from reading the I/O Mapping you sent me, these bar numbers are correct, but it seems a bit odd, so please offer correction if I am interpreting the document incorrectly. Thanks, Chase drivers/staging/comedi/drivers/addi_apci_1564.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c index fe42f9d..7e42d47 100644 --- a/drivers/staging/comedi/drivers/addi_apci_1564.c +++ b/drivers/staging/comedi/drivers/addi_apci_1564.c @@ -65,15 +65,8 @@ static int apci1564_auto_attach(struct comedi_device *dev, if (ret) return ret; - if (this_board-i_IorangeBase1) - dev-iobase = pci_resource_start(pcidev, 1); - else - dev-iobase = pci_resource_start(pcidev, 0); - - devpriv-iobase = dev-iobase; - devpriv-i_IobaseAmcc = pci_resource_start(pcidev, 0); - devpriv-i_IobaseAddon = pci_resource_start(pcidev, 2); - devpriv-i_IobaseReserved = pci_resource_start(pcidev, 3); + dev-iobase = pci_resource_start(pcidev, 2); + devpriv-i_IobaseAmcc = pci_resource_start(pcidev, 1); /* Initialize parameters that can be overridden in EEPROM */ devpriv-s_EeParameters.i_NbrAiChannel = this_board-i_NbrAiChannel; The document I sent you from ADDI-DATA does show that the I/O map revision can be determined by reading the Header EEPROM at BAR1 + 0x00. This seems to verify Ian's comment about earlier versions of the board having a different mapping. As Ian already mentioned. It's probably best to leave the PCI bar resource mapping as it currently is. If anyone reports an issue it can be addressed later. Regards, Hartley -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/6] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars
On 2014-04-28 11:19, Ian Abbott wrote: On 2014-04-27 02:37, Chase Southwood wrote: This driver only uses PCI bar 1 (devpriv->i_IobaseAmcc), and PCI bar 2 (dev->iobase) doon't bother reading the unused PCI bars. Signed-off-by: Chase Southwood Cc: Ian Abbott Cc: H Hartley Sweeten --- Hartley, As far as I can tell from reading the I/O Mapping you sent me, these bar numbers are correct, but it seems a bit odd, so please offer correction if I am interpreting the document incorrectly. Thanks, Chase drivers/staging/comedi/drivers/addi_apci_1564.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c index fe42f9d..7e42d47 100644 --- a/drivers/staging/comedi/drivers/addi_apci_1564.c +++ b/drivers/staging/comedi/drivers/addi_apci_1564.c @@ -65,15 +65,8 @@ static int apci1564_auto_attach(struct comedi_device *dev, if (ret) return ret; -if (this_board->i_IorangeBase1) -dev->iobase = pci_resource_start(pcidev, 1); -else -dev->iobase = pci_resource_start(pcidev, 0); - -devpriv->iobase = dev->iobase; -devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 0); -devpriv->i_IobaseAddon = pci_resource_start(pcidev, 2); -devpriv->i_IobaseReserved = pci_resource_start(pcidev, 3); +dev->iobase = pci_resource_start(pcidev, 2); +devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 1); I'm sure the original resources are correct, so since this_board->i_IorangeBase1 is non-zero: dev->iobase = pci_resource_start(pcidev, 1); devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 0); Maybe this depends on the revision of the board, though. The documents Chase saw were for APCI1564 revision D, but the PCI BAR assignments might have been different for earlier revisions of the board. All the driver code I've ever seen for this board (including one of ADDI-DATA's own Linux drivers, which no longer seems to be available for download) has used PCI BARs 0 and 1 with the same functions as the comedi driver. Most "off-the-shelf" PCI interface chips reserve at least PCI BAR 0 for their own registers. The clearest image I've found of this board seems to have an Altera ACEX FPGA with built-in PCI interface, so it might not have that restriction on BAR 0. http://pcqt.com/DAQ/images/apci-1564-400.jpg I think it's best to stick with the existing BAR assignments until proved otherwise. I suspect the member name 'i_IobaseAmcc' is just a hangover from the distant past and could just be changed to 'iobasemain' or something, since most of the code seems to use it. -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/6] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars
On 2014-04-27 02:37, Chase Southwood wrote: This driver only uses PCI bar 1 (devpriv->i_IobaseAmcc), and PCI bar 2 (dev->iobase) doon't bother reading the unused PCI bars. Signed-off-by: Chase Southwood Cc: Ian Abbott Cc: H Hartley Sweeten --- Hartley, As far as I can tell from reading the I/O Mapping you sent me, these bar numbers are correct, but it seems a bit odd, so please offer correction if I am interpreting the document incorrectly. Thanks, Chase drivers/staging/comedi/drivers/addi_apci_1564.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c index fe42f9d..7e42d47 100644 --- a/drivers/staging/comedi/drivers/addi_apci_1564.c +++ b/drivers/staging/comedi/drivers/addi_apci_1564.c @@ -65,15 +65,8 @@ static int apci1564_auto_attach(struct comedi_device *dev, if (ret) return ret; - if (this_board->i_IorangeBase1) - dev->iobase = pci_resource_start(pcidev, 1); - else - dev->iobase = pci_resource_start(pcidev, 0); - - devpriv->iobase = dev->iobase; - devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 0); - devpriv->i_IobaseAddon = pci_resource_start(pcidev, 2); - devpriv->i_IobaseReserved = pci_resource_start(pcidev, 3); + dev->iobase = pci_resource_start(pcidev, 2); + devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 1); I'm sure the original resources are correct, so since this_board->i_IorangeBase1 is non-zero: dev->iobase = pci_resource_start(pcidev, 1); devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 0); I suspect the member name 'i_IobaseAmcc' is just a hangover from the distant past and could just be changed to 'iobasemain' or something, since most of the code seems to use it. -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/6] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars
On 2014-04-27 02:37, Chase Southwood wrote: This driver only uses PCI bar 1 (devpriv-i_IobaseAmcc), and PCI bar 2 (dev-iobase) doon't bother reading the unused PCI bars. Signed-off-by: Chase Southwood chase.southw...@gmail.com Cc: Ian Abbott abbo...@mev.co.uk Cc: H Hartley Sweeten hswee...@visionengravers.com --- Hartley, As far as I can tell from reading the I/O Mapping you sent me, these bar numbers are correct, but it seems a bit odd, so please offer correction if I am interpreting the document incorrectly. Thanks, Chase drivers/staging/comedi/drivers/addi_apci_1564.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c index fe42f9d..7e42d47 100644 --- a/drivers/staging/comedi/drivers/addi_apci_1564.c +++ b/drivers/staging/comedi/drivers/addi_apci_1564.c @@ -65,15 +65,8 @@ static int apci1564_auto_attach(struct comedi_device *dev, if (ret) return ret; - if (this_board-i_IorangeBase1) - dev-iobase = pci_resource_start(pcidev, 1); - else - dev-iobase = pci_resource_start(pcidev, 0); - - devpriv-iobase = dev-iobase; - devpriv-i_IobaseAmcc = pci_resource_start(pcidev, 0); - devpriv-i_IobaseAddon = pci_resource_start(pcidev, 2); - devpriv-i_IobaseReserved = pci_resource_start(pcidev, 3); + dev-iobase = pci_resource_start(pcidev, 2); + devpriv-i_IobaseAmcc = pci_resource_start(pcidev, 1); I'm sure the original resources are correct, so since this_board-i_IorangeBase1 is non-zero: dev-iobase = pci_resource_start(pcidev, 1); devpriv-i_IobaseAmcc = pci_resource_start(pcidev, 0); I suspect the member name 'i_IobaseAmcc' is just a hangover from the distant past and could just be changed to 'iobasemain' or something, since most of the code seems to use it. -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/6] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars
On 2014-04-28 11:19, Ian Abbott wrote: On 2014-04-27 02:37, Chase Southwood wrote: This driver only uses PCI bar 1 (devpriv-i_IobaseAmcc), and PCI bar 2 (dev-iobase) doon't bother reading the unused PCI bars. Signed-off-by: Chase Southwood chase.southw...@gmail.com Cc: Ian Abbott abbo...@mev.co.uk Cc: H Hartley Sweeten hswee...@visionengravers.com --- Hartley, As far as I can tell from reading the I/O Mapping you sent me, these bar numbers are correct, but it seems a bit odd, so please offer correction if I am interpreting the document incorrectly. Thanks, Chase drivers/staging/comedi/drivers/addi_apci_1564.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c index fe42f9d..7e42d47 100644 --- a/drivers/staging/comedi/drivers/addi_apci_1564.c +++ b/drivers/staging/comedi/drivers/addi_apci_1564.c @@ -65,15 +65,8 @@ static int apci1564_auto_attach(struct comedi_device *dev, if (ret) return ret; -if (this_board-i_IorangeBase1) -dev-iobase = pci_resource_start(pcidev, 1); -else -dev-iobase = pci_resource_start(pcidev, 0); - -devpriv-iobase = dev-iobase; -devpriv-i_IobaseAmcc = pci_resource_start(pcidev, 0); -devpriv-i_IobaseAddon = pci_resource_start(pcidev, 2); -devpriv-i_IobaseReserved = pci_resource_start(pcidev, 3); +dev-iobase = pci_resource_start(pcidev, 2); +devpriv-i_IobaseAmcc = pci_resource_start(pcidev, 1); I'm sure the original resources are correct, so since this_board-i_IorangeBase1 is non-zero: dev-iobase = pci_resource_start(pcidev, 1); devpriv-i_IobaseAmcc = pci_resource_start(pcidev, 0); Maybe this depends on the revision of the board, though. The documents Chase saw were for APCI1564 revision D, but the PCI BAR assignments might have been different for earlier revisions of the board. All the driver code I've ever seen for this board (including one of ADDI-DATA's own Linux drivers, which no longer seems to be available for download) has used PCI BARs 0 and 1 with the same functions as the comedi driver. Most off-the-shelf PCI interface chips reserve at least PCI BAR 0 for their own registers. The clearest image I've found of this board seems to have an Altera ACEX FPGA with built-in PCI interface, so it might not have that restriction on BAR 0. http://pcqt.com/DAQ/images/apci-1564-400.jpg I think it's best to stick with the existing BAR assignments until proved otherwise. I suspect the member name 'i_IobaseAmcc' is just a hangover from the distant past and could just be changed to 'iobasemain' or something, since most of the code seems to use it. -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/6] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars
This driver only uses PCI bar 1 (devpriv->i_IobaseAmcc), and PCI bar 2 (dev->iobase) doon't bother reading the unused PCI bars. Signed-off-by: Chase Southwood Cc: Ian Abbott Cc: H Hartley Sweeten --- Hartley, As far as I can tell from reading the I/O Mapping you sent me, these bar numbers are correct, but it seems a bit odd, so please offer correction if I am interpreting the document incorrectly. Thanks, Chase drivers/staging/comedi/drivers/addi_apci_1564.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c index fe42f9d..7e42d47 100644 --- a/drivers/staging/comedi/drivers/addi_apci_1564.c +++ b/drivers/staging/comedi/drivers/addi_apci_1564.c @@ -65,15 +65,8 @@ static int apci1564_auto_attach(struct comedi_device *dev, if (ret) return ret; - if (this_board->i_IorangeBase1) - dev->iobase = pci_resource_start(pcidev, 1); - else - dev->iobase = pci_resource_start(pcidev, 0); - - devpriv->iobase = dev->iobase; - devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 0); - devpriv->i_IobaseAddon = pci_resource_start(pcidev, 2); - devpriv->i_IobaseReserved = pci_resource_start(pcidev, 3); + dev->iobase = pci_resource_start(pcidev, 2); + devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 1); /* Initialize parameters that can be overridden in EEPROM */ devpriv->s_EeParameters.i_NbrAiChannel = this_board->i_NbrAiChannel; -- 1.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/6] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars
This driver only uses PCI bar 1 (devpriv-i_IobaseAmcc), and PCI bar 2 (dev-iobase) doon't bother reading the unused PCI bars. Signed-off-by: Chase Southwood chase.southw...@gmail.com Cc: Ian Abbott abbo...@mev.co.uk Cc: H Hartley Sweeten hswee...@visionengravers.com --- Hartley, As far as I can tell from reading the I/O Mapping you sent me, these bar numbers are correct, but it seems a bit odd, so please offer correction if I am interpreting the document incorrectly. Thanks, Chase drivers/staging/comedi/drivers/addi_apci_1564.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c index fe42f9d..7e42d47 100644 --- a/drivers/staging/comedi/drivers/addi_apci_1564.c +++ b/drivers/staging/comedi/drivers/addi_apci_1564.c @@ -65,15 +65,8 @@ static int apci1564_auto_attach(struct comedi_device *dev, if (ret) return ret; - if (this_board-i_IorangeBase1) - dev-iobase = pci_resource_start(pcidev, 1); - else - dev-iobase = pci_resource_start(pcidev, 0); - - devpriv-iobase = dev-iobase; - devpriv-i_IobaseAmcc = pci_resource_start(pcidev, 0); - devpriv-i_IobaseAddon = pci_resource_start(pcidev, 2); - devpriv-i_IobaseReserved = pci_resource_start(pcidev, 3); + dev-iobase = pci_resource_start(pcidev, 2); + devpriv-i_IobaseAmcc = pci_resource_start(pcidev, 1); /* Initialize parameters that can be overridden in EEPROM */ devpriv-s_EeParameters.i_NbrAiChannel = this_board-i_NbrAiChannel; -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
