[PATCH 1/6] staging: comedi: addi_apci_1564: remove eeprom support code

[PATCH 1/6] staging: comedi: addi_apci_1564: remove eeprom support code

[Chase Southwood]

Reading the eeprom on this board is not necessary.  All information
required is in the boardinfo.

Remove the eeprom support code which is not really useful here.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 drivers/staging/comedi/drivers/addi_apci_1564.c | 46 +------------------------
 1 file changed, 1 insertion(+), 45 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 5f6d3b5..df8833b 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -7,7 +7,6 @@
 
 #include "addi-data/addi_common.h"
 
-#include "addi-data/addi_eeprom.c"
 #include "addi-data/hwdrv_apci1564.c"
 
 static const struct addi_board apci1564_boardtypes[] = {
@@ -33,23 +32,6 @@ static const struct addi_board apci1564_boardtypes[] = {
 	},
 };
 
-static int i_ADDIDATA_InsnReadEeprom(struct comedi_device *dev,
-				     struct comedi_subdevice *s,
-				     struct comedi_insn *insn,
-				     unsigned int *data)
-{
-	const struct addi_board *this_board = comedi_board(dev);
-	struct addi_private *devpriv = dev->private;
-	unsigned short w_Address = CR_CHAN(insn->chanspec);
-	unsigned short w_Data;
-
-	w_Data = addi_eeprom_readw(devpriv->i_IobaseAmcc,
-		this_board->pc_EepromChip, 2 * w_Address);
-	data[0] = w_Data;
-
-	return insn->n;
-}
-
 static irqreturn_t v_ADDI_Interrupt(int irq, void *d)
 {
 	struct comedi_device *dev = d;
@@ -75,7 +57,6 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 	struct addi_private *devpriv;
 	struct comedi_subdevice *s;
 	int ret, n_subdevices;
-	unsigned int dw_Dummy;
 
 	dev->board_name = this_board->pc_DriverName;
 
@@ -120,23 +101,6 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 			dev->irq = pcidev->irq;
 	}
 
-	/*  Read eepeom and fill addi_board Structure */
-
-	if (this_board->i_PCIEeprom) {
-		if (!(strcmp(this_board->pc_EepromChip, "S5920"))) {
-			/*  Set 3 wait stait */
-			if (!(strcmp(dev->board_name, "apci035")))
-				outl(0x80808082, devpriv->i_IobaseAmcc + 0x60);
-			else
-				outl(0x83838383, devpriv->i_IobaseAmcc + 0x60);
-
-			/*  Enable the interrupt for the controller */
-			dw_Dummy = inl(devpriv->i_IobaseAmcc + 0x38);
-			outl(dw_Dummy | 0x2000, devpriv->i_IobaseAmcc + 0x38);
-		}
-		addi_eeprom_read_info(dev, pci_resource_start(pcidev, 0));
-	}
-
 	n_subdevices = 7;
 	ret = comedi_alloc_subdevices(dev, n_subdevices);
 	if (ret)
@@ -212,15 +176,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 
 	/* EEPROM */
 	s = &dev->subdevices[6];
-	if (this_board->i_PCIEeprom) {
-		s->type = COMEDI_SUBD_MEMORY;
-		s->subdev_flags = SDF_READABLE | SDF_INTERNAL;
-		s->n_chan = 256;
-		s->maxdata = 0xffff;
-		s->insn_read = i_ADDIDATA_InsnReadEeprom;
-	} else {
-		s->type = COMEDI_SUBD_UNUSED;
-	}
+	s->type = COMEDI_SUBD_UNUSED;
 
 	i_ADDI_Reset(dev);
 	return 0;
-- 
1.9.0

[PATCH 2/6] staging: comedi: addi_apci_1564: remove unnecessary include

[Chase Southwood]

This include is no longer needed.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 drivers/staging/comedi/drivers/addi_apci_1564.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index df8833b..c84e17c 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -3,7 +3,6 @@
 
 #include "../comedidev.h"
 #include "comedi_fc.h"
-#include "amcc_s5933.h"
 
 #include "addi-data/addi_common.h"
 
-- 
1.9.0

[PATCH 3/6] staging: comedi: addi_apci_1032: board has 32 digital inputs

[Chase Southwood]

This board always has 32 digital inputs. Remove the test when
initializing the subdevice.

Also, since this board is the only one supported by this driver,
remove the boardinfo about the digital inputs and just use the
data directly in the subdevice init.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 drivers/staging/comedi/drivers/addi_apci_1564.c | 26 +++++++++----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index c84e17c..fe42f9d 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -20,8 +20,6 @@ static const struct addi_board apci1564_boardtypes[] = {
 		.i_Timer		= 1,
 		.interrupt		= apci1564_interrupt,
 		.reset			= apci1564_reset,
-		.di_config		= apci1564_di_config,
-		.di_bits		= apci1564_di_insn_bits,
 		.do_config		= apci1564_do_config,
 		.do_bits		= apci1564_do_insn_bits,
 		.do_read		= apci1564_do_read,
@@ -115,21 +113,15 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 
 	/*  Allocate and Initialise DI Subdevice Structures */
 	s = &dev->subdevices[2];
-	if (devpriv->s_EeParameters.i_NbrDiChannel) {
-		s->type = COMEDI_SUBD_DI;
-		s->subdev_flags = SDF_READABLE | SDF_GROUND | SDF_COMMON;
-		s->n_chan = devpriv->s_EeParameters.i_NbrDiChannel;
-		s->maxdata = 1;
-		s->len_chanlist =
-			devpriv->s_EeParameters.i_NbrDiChannel;
-		s->range_table = &range_digital;
-		s->insn_config = this_board->di_config;
-		s->insn_read = this_board->di_read;
-		s->insn_write = this_board->di_write;
-		s->insn_bits = this_board->di_bits;
-	} else {
-		s->type = COMEDI_SUBD_UNUSED;
-	}
+	s->type = COMEDI_SUBD_DI;
+	s->subdev_flags = SDF_READABLE;
+	s->n_chan = 32;
+	s->maxdata = 1;
+	s->len_chanlist = 32;
+	s->range_table = &range_digital;
+	s->insn_config = apci1564_di_config;
+	s->insn_bits = apci1564_di_insn_bits;
+
 	/*  Allocate and Initialise DO Subdevice Structures */
 	s = &dev->subdevices[3];
 	if (devpriv->s_EeParameters.i_NbrDoChannel) {
-- 
1.9.0

Re: [PATCH 3/6] staging: comedi: addi_apci_1032: board has 32 digital inputs

[Ian Abbott]

On 28/04/14 19:11, Hartley Sweeten wrote:
> On Saturday, April 26, 2014 6:37 PM, Chase Southwood wrote:
>> This board always has 32 digital inputs. Remove the test when
>> initializing the subdevice.
>>
>> Also, since this board is the only one supported by this driver,
>> remove the boardinfo about the digital inputs and just use the
>> data directly in the subdevice init.
>>
>> Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
>> Cc: Ian Abbott <abbotti@mev.co.uk>
>> Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
>> ---
>>   drivers/staging/comedi/drivers/addi_apci_1564.c | 26 +++++++++----------------
>>   1 file changed, 9 insertions(+), 17 deletions(-)
>
> Looks good.

Apart from the subject not matching the patch!

(I would have replied to the original, but I seem to have mislaid it.)

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

[PATCH 3/6 v2] staging: comedi: addi_apci_1564: board has 32 digital inputs

[Chase Southwood]

This board always has 32 digital inputs. Remove the test when
initializing the subdevice.

Also, since this board is the only one supported by this driver,
remove the boardinfo about the digital inputs and just use the
data directly in the subdevice init.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
2: Changed incorrect title.

 drivers/staging/comedi/drivers/addi_apci_1564.c | 26 +++++++++----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index c84e17c..fe42f9d 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -20,8 +20,6 @@ static const struct addi_board apci1564_boardtypes[] = {
 		.i_Timer		= 1,
 		.interrupt		= apci1564_interrupt,
 		.reset			= apci1564_reset,
-		.di_config		= apci1564_di_config,
-		.di_bits		= apci1564_di_insn_bits,
 		.do_config		= apci1564_do_config,
 		.do_bits		= apci1564_do_insn_bits,
 		.do_read		= apci1564_do_read,
@@ -115,21 +113,15 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 
 	/*  Allocate and Initialise DI Subdevice Structures */
 	s = &dev->subdevices[2];
-	if (devpriv->s_EeParameters.i_NbrDiChannel) {
-		s->type = COMEDI_SUBD_DI;
-		s->subdev_flags = SDF_READABLE | SDF_GROUND | SDF_COMMON;
-		s->n_chan = devpriv->s_EeParameters.i_NbrDiChannel;
-		s->maxdata = 1;
-		s->len_chanlist =
-			devpriv->s_EeParameters.i_NbrDiChannel;
-		s->range_table = &range_digital;
-		s->insn_config = this_board->di_config;
-		s->insn_read = this_board->di_read;
-		s->insn_write = this_board->di_write;
-		s->insn_bits = this_board->di_bits;
-	} else {
-		s->type = COMEDI_SUBD_UNUSED;
-	}
+	s->type = COMEDI_SUBD_DI;
+	s->subdev_flags = SDF_READABLE;
+	s->n_chan = 32;
+	s->maxdata = 1;
+	s->len_chanlist = 32;
+	s->range_table = &range_digital;
+	s->insn_config = apci1564_di_config;
+	s->insn_bits = apci1564_di_insn_bits;
+
 	/*  Allocate and Initialise DO Subdevice Structures */
 	s = &dev->subdevices[3];
 	if (devpriv->s_EeParameters.i_NbrDoChannel) {
-- 
1.9.0

Re: [PATCH 3/6 v2] staging: comedi: addi_apci_1564: board has 32 digital inputs

[Ian Abbott]

On 2014-04-29 09:34, Chase Southwood wrote:
> This board always has 32 digital inputs. Remove the test when
> initializing the subdevice.
>
> Also, since this board is the only one supported by this driver,
> remove the boardinfo about the digital inputs and just use the
> data directly in the subdevice init.
>
> Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
> Cc: Ian Abbott <abbotti@mev.co.uk>
> Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
> ---
> 2: Changed incorrect title.
>
>   drivers/staging/comedi/drivers/addi_apci_1564.c | 26 +++++++++----------------
>   1 file changed, 9 insertions(+), 17 deletions(-)

Looks good.

Reviewed-by: Ian Abbott <abbotti@mev.co.uk>

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

[PATCH 4/6] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars

[Chase Southwood]

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.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@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

Re: [PATCH 4/6] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars

[Ian Abbott]

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.southwood@gmail.com>
> Cc: Ian Abbott <abbotti@mev.co.uk>
> Cc: H Hartley Sweeten <hsweeten@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: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

Re: [PATCH 4/6] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars

[Ian Abbott]

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.southwood@gmail.com>
>> Cc: Ian Abbott <abbotti@mev.co.uk>
>> Cc: H Hartley Sweeten <hsweeten@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: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

Re: [PATCH 4/6] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars

[Chase Southwood]

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
<HartleyS@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.southwood@gmail.com>
>> Cc: Ian Abbott <abbotti@mev.co.uk>
>> Cc: H Hartley Sweeten <hsweeten@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
>

[PATCH 4/6 v2] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars

[Chase Southwood]

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.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
2: Bad PCI bar numbers corrected.

 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, 1);
+	devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 0);
 
 	/* Initialize parameters that can be overridden in EEPROM */
 	devpriv->s_EeParameters.i_NbrAiChannel = this_board->i_NbrAiChannel;
-- 
1.9.0

Re: [PATCH 4/6 v2] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars

[Ian Abbott]

On 2014-04-29 09:35, 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.

The description needs fixing as it's back to using PCI bar 0 and 1. 
Also, there's a typo: doon't -> don't. :)

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

[PATCH 4/6 v3] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars

[Chase Southwood]

This driver only uses PCI bar 0 (devpriv->i_IobaseAmcc), and PCI bar 1
(dev->iobase), don't bother reading the unused PCI bars.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
2: Bad PCI bar numbers corrected.

3: Fixed silly typos in the changelog

 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, 1);
+	devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 0);
 
 	/* Initialize parameters that can be overridden in EEPROM */
 	devpriv->s_EeParameters.i_NbrAiChannel = this_board->i_NbrAiChannel;
-- 
1.9.0

Re: [PATCH 4/6 v3] staging: comedi: addi_apci_1564: simplify the PCI bar reading and don't read the unused bars

[Ian Abbott]

On 2014-04-30 08:57, Chase Southwood wrote:
> This driver only uses PCI bar 0 (devpriv->i_IobaseAmcc), and PCI bar 1
> (dev->iobase), don't bother reading the unused PCI bars.
>
> Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
> Cc: Ian Abbott <abbotti@mev.co.uk>
> Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
> ---
> 2: Bad PCI bar numbers corrected.
>
> 3: Fixed silly typos in the changelog
>
>   drivers/staging/comedi/drivers/addi_apci_1564.c | 11 ++---------
>   1 file changed, 2 insertions(+), 9 deletions(-)

Okay now!

Reviewed-by: Ian Abbott <abbotti@mev.co.uk>

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

[PATCH 5/6] staging: comedi: addi_apci_2032: remove unnecessary info from boardinfo

[Chase Southwood]

The i_IorangeBase1, i_PCIEeprom, and pc_EepromChip data in the boardinfo
was only needed to work out the usage of the PCI bars.  Now that that is
squared away, this info is no longer needed and can be removed.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 drivers/staging/comedi/drivers/addi_apci_1564.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 7e42d47..d5be8d3 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -11,9 +11,6 @@
 static const struct addi_board apci1564_boardtypes[] = {
 	{
 		.pc_DriverName		= "apci1564",
-		.i_IorangeBase1		= APCI1564_ADDRESS_RANGE,
-		.i_PCIEeprom		= ADDIDATA_EEPROM,
-		.pc_EepromChip		= ADDIDATA_93C76,
 		.i_NbrDiChannel		= 32,
 		.i_NbrDoChannel		= 32,
 		.i_DoMaxdata		= 0xffffffff,
-- 
1.9.0

[PATCH 5/6 v2] staging: comedi: addi_apci_1564: remove unnecessary info from boardinfo

[Chase Southwood]

The i_IorangeBase1, i_PCIEeprom, and pc_EepromChip data in the boardinfo
was only needed to work out the usage of the PCI bars.  Now that that is
squared away, this info is no longer needed and can be removed.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
2: Incorrect patch title fixed.

 drivers/staging/comedi/drivers/addi_apci_1564.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 7e42d47..d5be8d3 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -11,9 +11,6 @@
 static const struct addi_board apci1564_boardtypes[] = {
 	{
 		.pc_DriverName		= "apci1564",
-		.i_IorangeBase1		= APCI1564_ADDRESS_RANGE,
-		.i_PCIEeprom		= ADDIDATA_EEPROM,
-		.pc_EepromChip		= ADDIDATA_93C76,
 		.i_NbrDiChannel		= 32,
 		.i_NbrDoChannel		= 32,
 		.i_DoMaxdata		= 0xffffffff,
-- 
1.9.0

Re: [PATCH 5/6 v2] staging: comedi: addi_apci_1564: remove unnecessary info from boardinfo

[Ian Abbott]

On 2014-04-29 09:37, Chase Southwood wrote:
> The i_IorangeBase1, i_PCIEeprom, and pc_EepromChip data in the boardinfo
> was only needed to work out the usage of the PCI bars.  Now that that is
> squared away, this info is no longer needed and can be removed.
>
> Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
> Cc: Ian Abbott <abbotti@mev.co.uk>
> Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
> ---
> 2: Incorrect patch title fixed.
>
>   drivers/staging/comedi/drivers/addi_apci_1564.c | 3 ---
>   1 file changed, 3 deletions(-)

Reviewed-by: Ian Abbott <abbotti@mev.co.uk>

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

[PATCH 6/6] staging: comedi: addi_apci_2032: remove use of devpriv->s_EeParameters

[Chase Southwood]

This driver no longer reads the eeprom to find the board specific data,
all the necessary data is in the boardinfo. Use the boardinfo directly
instead of passing through devpriv->s_EeParameters.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
Ian and Hartley,

The auto_attach() function is starting to look much better now.  My next patchset
will be geared towards only allocating subdevices which are actually used.

Thanks,
Chase

 drivers/staging/comedi/drivers/addi_apci_1564.c | 27 +++++--------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index d5be8d3..b34ae34 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -65,22 +65,6 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 	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;
-	devpriv->s_EeParameters.i_NbrAoChannel = this_board->i_NbrAoChannel;
-	devpriv->s_EeParameters.i_AiMaxdata = this_board->i_AiMaxdata;
-	devpriv->s_EeParameters.i_AoMaxdata = this_board->i_AoMaxdata;
-	devpriv->s_EeParameters.i_NbrDiChannel = this_board->i_NbrDiChannel;
-	devpriv->s_EeParameters.i_NbrDoChannel = this_board->i_NbrDoChannel;
-	devpriv->s_EeParameters.i_DoMaxdata = this_board->i_DoMaxdata;
-	devpriv->s_EeParameters.i_Timer = this_board->i_Timer;
-	devpriv->s_EeParameters.ui_MinAcquisitiontimeNs =
-		this_board->ui_MinAcquisitiontimeNs;
-	devpriv->s_EeParameters.ui_MinDelaytimeNs =
-		this_board->ui_MinDelaytimeNs;
-
-	/* ## */
-
 	if (pcidev->irq > 0) {
 		ret = request_irq(pcidev->irq, v_ADDI_Interrupt, IRQF_SHARED,
 				  dev->board_name, dev);
@@ -114,14 +98,13 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 
 	/*  Allocate and Initialise DO Subdevice Structures */
 	s = &dev->subdevices[3];
-	if (devpriv->s_EeParameters.i_NbrDoChannel) {
+	if (this_board->i_NbrDoChannel) {
 		s->type = COMEDI_SUBD_DO;
 		s->subdev_flags =
 			SDF_READABLE | SDF_WRITEABLE | SDF_GROUND | SDF_COMMON;
-		s->n_chan = devpriv->s_EeParameters.i_NbrDoChannel;
-		s->maxdata = devpriv->s_EeParameters.i_DoMaxdata;
-		s->len_chanlist =
-			devpriv->s_EeParameters.i_NbrDoChannel;
+		s->n_chan = this_board->i_NbrDoChannel;
+		s->maxdata = this_board->i_DoMaxdata;
+		s->len_chanlist = this_board->i_NbrDoChannel;
 		s->range_table = &range_digital;
 
 		/* insn_config - for digital output memory */
@@ -135,7 +118,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 
 	/*  Allocate and Initialise Timer Subdevice Structures */
 	s = &dev->subdevices[4];
-	if (devpriv->s_EeParameters.i_Timer) {
+	if (this_board->i_Timer) {
 		s->type = COMEDI_SUBD_TIMER;
 		s->subdev_flags = SDF_WRITEABLE | SDF_GROUND | SDF_COMMON;
 		s->n_chan = 1;
-- 
1.9.0

Re: [PATCH 6/6] staging: comedi: addi_apci_2032: remove use of devpriv->s_EeParameters

[Chase Southwood]

Hartley,

Ah geez...my brain got all tangled looking at old git commits for
other addi-data drivers.  I apologize for this silly mistake.  I'll
fix that title and send a new version of the patchset tomorrow.

Thanks,
Chase

On Mon, Apr 28, 2014 at 12:59 PM, Hartley Sweeten
<HartleyS@visionengravers.com> wrote:
> On Saturday, April 26, 2014 6:39 PM, Chase Southwood wrote:
>> This driver no longer reads the eeprom to find the board specific data,
>> all the necessary data is in the boardinfo. Use the boardinfo directly
>> instead of passing through devpriv->s_EeParameters.
>>
>> Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
>> Cc: Ian Abbott <abbotti@mev.co.uk>
>> Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
>
> Chase,
>
> The subject does not match the patch.
>
> Regards,
> Hartley

[PATCH 6/6 v2] staging: comedi: addi_apci_1564: remove use of devpriv->s_EeParameters

[Chase Southwood]

This driver no longer reads the eeprom to find the board specific data,
all the necessary data is in the boardinfo. Use the boardinfo directly
instead of passing through devpriv->s_EeParameters.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
2: Incorrect patch title fixed.

Ian and Hartley,

The auto_attach() function is starting to look much better now.  My next patchset
will be geared towards only allocating subdevices which are actually used.

Thanks,
Chase

 drivers/staging/comedi/drivers/addi_apci_1564.c | 27 +++++--------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index d5be8d3..b34ae34 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -65,22 +65,6 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 	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;
-	devpriv->s_EeParameters.i_NbrAoChannel = this_board->i_NbrAoChannel;
-	devpriv->s_EeParameters.i_AiMaxdata = this_board->i_AiMaxdata;
-	devpriv->s_EeParameters.i_AoMaxdata = this_board->i_AoMaxdata;
-	devpriv->s_EeParameters.i_NbrDiChannel = this_board->i_NbrDiChannel;
-	devpriv->s_EeParameters.i_NbrDoChannel = this_board->i_NbrDoChannel;
-	devpriv->s_EeParameters.i_DoMaxdata = this_board->i_DoMaxdata;
-	devpriv->s_EeParameters.i_Timer = this_board->i_Timer;
-	devpriv->s_EeParameters.ui_MinAcquisitiontimeNs =
-		this_board->ui_MinAcquisitiontimeNs;
-	devpriv->s_EeParameters.ui_MinDelaytimeNs =
-		this_board->ui_MinDelaytimeNs;
-
-	/* ## */
-
 	if (pcidev->irq > 0) {
 		ret = request_irq(pcidev->irq, v_ADDI_Interrupt, IRQF_SHARED,
 				  dev->board_name, dev);
@@ -114,14 +98,13 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 
 	/*  Allocate and Initialise DO Subdevice Structures */
 	s = &dev->subdevices[3];
-	if (devpriv->s_EeParameters.i_NbrDoChannel) {
+	if (this_board->i_NbrDoChannel) {
 		s->type = COMEDI_SUBD_DO;
 		s->subdev_flags =
 			SDF_READABLE | SDF_WRITEABLE | SDF_GROUND | SDF_COMMON;
-		s->n_chan = devpriv->s_EeParameters.i_NbrDoChannel;
-		s->maxdata = devpriv->s_EeParameters.i_DoMaxdata;
-		s->len_chanlist =
-			devpriv->s_EeParameters.i_NbrDoChannel;
+		s->n_chan = this_board->i_NbrDoChannel;
+		s->maxdata = this_board->i_DoMaxdata;
+		s->len_chanlist = this_board->i_NbrDoChannel;
 		s->range_table = &range_digital;
 
 		/* insn_config - for digital output memory */
@@ -135,7 +118,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 
 	/*  Allocate and Initialise Timer Subdevice Structures */
 	s = &dev->subdevices[4];
-	if (devpriv->s_EeParameters.i_Timer) {
+	if (this_board->i_Timer) {
 		s->type = COMEDI_SUBD_TIMER;
 		s->subdev_flags = SDF_WRITEABLE | SDF_GROUND | SDF_COMMON;
 		s->n_chan = 1;
-- 
1.9.0

Re: [PATCH 6/6 v2] staging: comedi: addi_apci_1564: remove use of devpriv->s_EeParameters

[Ian Abbott]

On 2014-04-29 09:38, Chase Southwood wrote:
> This driver no longer reads the eeprom to find the board specific data,
> all the necessary data is in the boardinfo. Use the boardinfo directly
> instead of passing through devpriv->s_EeParameters.
>
> Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
> Cc: Ian Abbott <abbotti@mev.co.uk>
> Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
> ---
> 2: Incorrect patch title fixed.
>
> Ian and Hartley,
>
> The auto_attach() function is starting to look much better now.  My next patchset
> will be geared towards only allocating subdevices which are actually used.

Great!  All looking good except the description of PATCH 4/6 v2.

Revie

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

RE: [PATCH 6/6 v2] staging: comedi: addi_apci_1564: remove use of devpriv->s_EeParameters

[Hartley Sweeten]

On, Tuesday, April 29, 2014 1:38 AM, Chase Southwood wrote: 
> This driver no longer reads the eeprom to find the board specific data,
> all the necessary data is in the boardinfo. Use the boardinfo directly
> instead of passing through devpriv->s_EeParameters.
>
> Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
> Cc: Ian Abbott <abbotti@mev.co.uk>
> Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
> ---
> 2: Incorrect patch title fixed.
>
> Ian and Hartley,
>
> The auto_attach() function is starting to look much better now.  My next patchset
> will be geared towards only allocating subdevices which are actually used.

Other than Ian's comment on patch 4/6 everything looks good to me.

For the series:
Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>

BTW, for a patch series you should include a cover letter (PATCH 00/xx).

Regards,
Hartley

Re: [PATCH 6/6 v2] staging: comedi: addi_apci_1564: remove use of devpriv->s_EeParameters

[Chase Southwood]

Ian and Hartley,

Thanks so much, I greatly appreciate the review.  I'll fix the
changelog for patch 4 and send once more (as I assume that's easier
for Greg).  Also, I should know better about the cover letter as
well...I was once told not to send them for strictly cleanup patchsets
(as Greg can't do anything with them and cleanups should be obvious)
but I've gotten in the habit of not doing a cover letter for any
patchsets.  I will send cover letters (and be more careful about my
other mistakes) in the future.

Thanks,
Chase

On Tue, Apr 29, 2014 at 3:33 PM, Hartley Sweeten
<HartleyS@visionengravers.com> wrote:
> On, Tuesday, April 29, 2014 1:38 AM, Chase Southwood wrote:
>> This driver no longer reads the eeprom to find the board specific data,
>> all the necessary data is in the boardinfo. Use the boardinfo directly
>> instead of passing through devpriv->s_EeParameters.
>>
>> Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
>> Cc: Ian Abbott <abbotti@mev.co.uk>
>> Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
>> ---
>> 2: Incorrect patch title fixed.
>>
>> Ian and Hartley,
>>
>> The auto_attach() function is starting to look much better now.  My next patchset
>> will be geared towards only allocating subdevices which are actually used.
>
> Other than Ian's comment on patch 4/6 everything looks good to me.
>
> For the series:
> Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>
>
> BTW, for a patch series you should include a cover letter (PATCH 00/xx).
>
> Regards,
> Hartley

RE: [PATCH 6/6 v2] staging: comedi: addi_apci_1564: remove use of devpriv->s_EeParameters

[Hartley Sweeten]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1133 bytes --]

On Wednesday, April 30, 2014 12:52 AM, Chase Southwood wrote:
> Thanks so much, I greatly appreciate the review.  I'll fix the
> changelog for patch 4 and send once more (as I assume that's easier
> for Greg).  Also, I should know better about the cover letter as
> well...I was once told not to send them for strictly cleanup patchsets
> (as Greg can't do anything with them and cleanups should be obvious)
> but I've gotten in the habit of not doing a cover letter for any
> patchsets.  I will send cover letters (and be more careful about my
> other mistakes) in the future.

Chase,

Good job on the cleanup you have done so far.

The cover letter does not get committed as part of the patch set. But
when you are submitting a series it give a convenient  place to add
the sign off tag for a series. Also, if you would have done a cover letter
for this series you would have probably spotted the subject line issues
in patches 5 and 6.

Keep up the good work!

Hartley

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 6/6 v2] staging: comedi: addi_apci_1564: remove use of devpriv->s_EeParameters

[Chase Southwood]

Hartley,

Yes, you raise very good points.  In any case, I have added cover
letters to my submitting checklist so hopefully everything will be a
lot easier for everyone next go round.

Thanks,
Chase

On Wed, Apr 30, 2014 at 11:58 AM, Hartley Sweeten
<HartleyS@visionengravers.com> wrote:
> On Wednesday, April 30, 2014 12:52 AM, Chase Southwood wrote:
>> Thanks so much, I greatly appreciate the review.  I'll fix the
>> changelog for patch 4 and send once more (as I assume that's easier
>> for Greg).  Also, I should know better about the cover letter as
>> well...I was once told not to send them for strictly cleanup patchsets
>> (as Greg can't do anything with them and cleanups should be obvious)
>> but I've gotten in the habit of not doing a cover letter for any
>> patchsets.  I will send cover letters (and be more careful about my
>> other mistakes) in the future.
>
> Chase,
>
> Good job on the cleanup you have done so far.
>
> The cover letter does not get committed as part of the patch set. But
> when you are submitting a series it give a convenient  place to add
> the sign off tag for a series. Also, if you would have done a cover letter
> for this series you would have probably spotted the subject line issues
> in patches 5 and 6.
>
> Keep up the good work!
>
> Hartley
>

[PATCH 6/6 v3] staging: comedi: addi_apci_1564: remove use of devpriv->s_EeParameters

[Chase Southwood]

This driver no longer reads the eeprom to find the board specific data,
all the necessary data is in the boardinfo. Use the boardinfo directly
instead of passing through devpriv->s_EeParameters.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
2: Incorrect patch title fixed.

3: I apparently failed to redo this patch properly when I changed PATCH 4/6 and
it no longer applied.  Just noticed this, and verified that the whole series
applies now.

 drivers/staging/comedi/drivers/addi_apci_1564.c | 27 +++++--------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index d5be8d3..b34ae34 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -65,22 +65,6 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 	dev->iobase = pci_resource_start(pcidev, 1);
 	devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 0);
 
-	/* Initialize parameters that can be overridden in EEPROM */
-	devpriv->s_EeParameters.i_NbrAiChannel = this_board->i_NbrAiChannel;
-	devpriv->s_EeParameters.i_NbrAoChannel = this_board->i_NbrAoChannel;
-	devpriv->s_EeParameters.i_AiMaxdata = this_board->i_AiMaxdata;
-	devpriv->s_EeParameters.i_AoMaxdata = this_board->i_AoMaxdata;
-	devpriv->s_EeParameters.i_NbrDiChannel = this_board->i_NbrDiChannel;
-	devpriv->s_EeParameters.i_NbrDoChannel = this_board->i_NbrDoChannel;
-	devpriv->s_EeParameters.i_DoMaxdata = this_board->i_DoMaxdata;
-	devpriv->s_EeParameters.i_Timer = this_board->i_Timer;
-	devpriv->s_EeParameters.ui_MinAcquisitiontimeNs =
-		this_board->ui_MinAcquisitiontimeNs;
-	devpriv->s_EeParameters.ui_MinDelaytimeNs =
-		this_board->ui_MinDelaytimeNs;
-
-	/* ## */
-
 	if (pcidev->irq > 0) {
 		ret = request_irq(pcidev->irq, v_ADDI_Interrupt, IRQF_SHARED,
 				  dev->board_name, dev);
@@ -114,14 +98,13 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 
 	/*  Allocate and Initialise DO Subdevice Structures */
 	s = &dev->subdevices[3];
-	if (devpriv->s_EeParameters.i_NbrDoChannel) {
+	if (this_board->i_NbrDoChannel) {
 		s->type = COMEDI_SUBD_DO;
 		s->subdev_flags =
 			SDF_READABLE | SDF_WRITEABLE | SDF_GROUND | SDF_COMMON;
-		s->n_chan = devpriv->s_EeParameters.i_NbrDoChannel;
-		s->maxdata = devpriv->s_EeParameters.i_DoMaxdata;
-		s->len_chanlist =
-			devpriv->s_EeParameters.i_NbrDoChannel;
+		s->n_chan = this_board->i_NbrDoChannel;
+		s->maxdata = this_board->i_DoMaxdata;
+		s->len_chanlist = this_board->i_NbrDoChannel;
 		s->range_table = &range_digital;
 
 		/* insn_config - for digital output memory */
@@ -135,7 +118,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 
 	/*  Allocate and Initialise Timer Subdevice Structures */
 	s = &dev->subdevices[4];
-	if (devpriv->s_EeParameters.i_Timer) {
+	if (this_board->i_Timer) {
 		s->type = COMEDI_SUBD_TIMER;
 		s->subdev_flags = SDF_WRITEABLE | SDF_GROUND | SDF_COMMON;
 		s->n_chan = 1;
-- 
1.9.0

Re: [PATCH 1/6] staging: comedi: addi_apci_1564: remove eeprom support code

[Dan Carpenter]

Nice, Chase, you've become an expert on comedi, it seems.

I can't say how happy comedi patches make me these days.  Ian, you and
Hartley are doing a fantastic job.

regards,
dan carpenter

Re: [PATCH 1/6] staging: comedi: addi_apci_1564: remove eeprom support code

[Greg KH]

On Mon, Apr 28, 2014 at 12:40:05PM +0300, Dan Carpenter wrote:
> Nice, Chase, you've become an expert on comedi, it seems.
> 
> I can't say how happy comedi patches make me these days.  Ian, you and
> Hartley are doing a fantastic job.

Same here, it's great to see all of this cleanup happening.  Maybe
someday it can move out of staging...  :)

thanks,

greg k-h

Re: [PATCH 1/6] staging: comedi: addi_apci_1564: remove eeprom support code

[Chase Southwood]

On Sat, May 3, 2014 at 6:52 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Apr 28, 2014 at 12:40:05PM +0300, Dan Carpenter wrote:
>> Nice, Chase, you've become an expert on comedi, it seems.
>>
>> I can't say how happy comedi patches make me these days.  Ian, you and
>> Hartley are doing a fantastic job.
>
> Same here, it's great to see all of this cleanup happening.  Maybe
> someday it can move out of staging...  :)

Thanks everyone! :) Yeah, I suppose it would be nice to see comedi
graduate, eh?  AFAICT, it looks like it's been hanging around in
staging a while...

Thanks,
Chase