[PATCH 01/30] staging: comedi: add pci_dev pointer to comedi_device

[PATCH 01/30] staging: comedi: add pci_dev pointer to comedi_device

[H Hartley Sweeten]

The pci_dev pointer in the private driver data is used by every
comedi pci driver. Some of them only have the need for the
private data because of this pointer.

Introduce the pci_dev pointer in the comedi_device struct so it
can be used instead of needing it in the private data.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/staging/comedi/comedidev.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h
index de8c99c..55f2373 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -212,6 +212,8 @@ struct comedi_device {
 	 * DMA_NONE */
 	struct device *hw_dev;
 
+	struct pci_dev *pcidev;
+
 	const char *board_name;
 	const void *board_ptr;
 	int attached;
-- 
1.7.11

Re: [PATCH 01/30] staging: comedi: add pci_dev pointer to comedi_device

[Greg KH]

On Wed, Jul 11, 2012 at 02:49:14PM -0700, H Hartley Sweeten wrote:
> The pci_dev pointer in the private driver data is used by every
> comedi pci driver. Some of them only have the need for the
> private data because of this pointer.
> 
> Introduce the pci_dev pointer in the comedi_device struct so it
> can be used instead of needing it in the private data.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Ian Abbott <abbotti@mev.co.uk>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/staging/comedi/comedidev.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h
> index de8c99c..55f2373 100644
> --- a/drivers/staging/comedi/comedidev.h
> +++ b/drivers/staging/comedi/comedidev.h
> @@ -212,6 +212,8 @@ struct comedi_device {
>  	 * DMA_NONE */
>  	struct device *hw_dev;
>  
> +	struct pci_dev *pcidev;

No, the field above this, hw_dev, should be used instead here, as that's
what it is there for, right?

Care to rework this series with that change instead?

thanks,

greg k-h

RE: [PATCH 01/30] staging: comedi: add pci_dev pointer to comedi_device

[H Hartley Sweeten]

On Monday, July 16, 2012 6:52 PM, Greg KH wrote:
> On Wed, Jul 11, 2012 at 02:49:14PM -0700, H Hartley Sweeten wrote:
>> The pci_dev pointer in the private driver data is used by every
>> comedi pci driver. Some of them only have the need for the
>> private data because of this pointer.
>> 
>> Introduce the pci_dev pointer in the comedi_device struct so it
>> can be used instead of needing it in the private data.
>> 
>> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
>> Cc: Ian Abbott <abbotti@mev.co.uk>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> ---
>>  drivers/staging/comedi/comedidev.h | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h
>> index de8c99c..55f2373 100644
>> --- a/drivers/staging/comedi/comedidev.h
>> +++ b/drivers/staging/comedi/comedidev.h
>> @@ -212,6 +212,8 @@ struct comedi_device {
>>  	 * DMA_NONE */
>>  	struct device *hw_dev;
>>  
>> +	struct pci_dev *pcidev;
>
> No, the field above this, hw_dev, should be used instead here, as that's
> what it is there for, right?

The hw_dev pointer is currently only used for something dealing with dma.
I have not dug into it yet to see what exactly it's used for.  The comment
says:

	/* hw_dev is passed to dma_alloc_coherent when allocating async buffers
	 * for subdevices that have async_dma_dir set to something other than
	 * DMA_NONE */

> Care to rework this series with that change instead?

It could probably be used with some sort of container_of but I'm not sure.

Regards,
Hartley

Re: [PATCH 01/30] staging: comedi: add pci_dev pointer to comedi_device

[Greg KH]

On Mon, Jul 16, 2012 at 08:55:47PM -0500, H Hartley Sweeten wrote:
> On Monday, July 16, 2012 6:52 PM, Greg KH wrote:
> > On Wed, Jul 11, 2012 at 02:49:14PM -0700, H Hartley Sweeten wrote:
> >> The pci_dev pointer in the private driver data is used by every
> >> comedi pci driver. Some of them only have the need for the
> >> private data because of this pointer.
> >> 
> >> Introduce the pci_dev pointer in the comedi_device struct so it
> >> can be used instead of needing it in the private data.
> >> 
> >> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> >> Cc: Ian Abbott <abbotti@mev.co.uk>
> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> ---
> >>  drivers/staging/comedi/comedidev.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >> 
> >> diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h
> >> index de8c99c..55f2373 100644
> >> --- a/drivers/staging/comedi/comedidev.h
> >> +++ b/drivers/staging/comedi/comedidev.h
> >> @@ -212,6 +212,8 @@ struct comedi_device {
> >>  	 * DMA_NONE */
> >>  	struct device *hw_dev;
> >>  
> >> +	struct pci_dev *pcidev;
> >
> > No, the field above this, hw_dev, should be used instead here, as that's
> > what it is there for, right?
> 
> The hw_dev pointer is currently only used for something dealing with dma.
> I have not dug into it yet to see what exactly it's used for.  The comment
> says:
> 
> 	/* hw_dev is passed to dma_alloc_coherent when allocating async buffers
> 	 * for subdevices that have async_dma_dir set to something other than
> 	 * DMA_NONE */

Which is exactly what the pci device should be used for, it knows this
information :)

> > Care to rework this series with that change instead?
> 
> It could probably be used with some sort of container_of but I'm not sure.

Yes it can.

To set the field:

	hw_dev = &pci_dev->dev;

to get it back:
	pci_dev = container_of(hw_dev, struct pci_device, struct device);

I think.  That's off the top of my head, please try it out first.

And use a macro for the container_of stuff, that makes it easier to
understand.

thanks,

greg k-h

RE: [PATCH 01/30] staging: comedi: add pci_dev pointer to comedi_device

[H Hartley Sweeten]

On Monday, July 16, 2012 7:01 PM, Greg KH wrote:
> On Mon, Jul 16, 2012 at 08:55:47PM -0500, H Hartley Sweeten wrote:
>> On Monday, July 16, 2012 6:52 PM, Greg KH wrote:
>>> No, the field above this, hw_dev, should be used instead here, as that's
>>> what it is there for, right?
>>
>> The hw_dev pointer is currently only used for something dealing with dma.
>> I have not dug into it yet to see what exactly it's used for.  The comment
>> says:
>> 
>> 	/* hw_dev is passed to dma_alloc_coherent when allocating async buffers
>> 	 * for subdevices that have async_dma_dir set to something other than
>> 	 * DMA_NONE */
>
> Which is exactly what the pci device should be used for, it knows this
> information :)
>
>>> Care to rework this series with that change instead?
>> 
>> It could probably be used with some sort of container_of but I'm not sure.
>
> Yes it can.
>
> To set the field:
>
> 	hw_dev = &pci_dev->dev;
>
> to get it back:
>	pci_dev = container_of(hw_dev, struct pci_device, struct device);
>
> I think.  That's off the top of my head, please try it out first.
>
> And use a macro for the container_of stuff, that makes it easier to
> understand.

Greg,

I'm posting a patch to the adl_pci6208 driver right now. It compiles fine
but could you look it over and see if it looks right.

Thanks,
Hartley