Re: [PATCH v2] staging: comedi: resolve section mismatch in s626

[Greg KH]

On Sun, Mar 18, 2012 at 08:09:25PM -0700, Gerard Snitselaar wrote:
> s626_attach is referencing s626_pci_table which is annotated
> __devinitconst. Put pci vendor/device ids in the s626_board struct and
> use s626_boards instead similar to what is done in gsc_hpdi.
> 
> v2: I had moved the PCI id defines to s626.h earlier, but later
> decided to leave them in s626.c and forgot to move them up above where
> they are being used in s626_boards.
> 
> Signed-off-by: Gerard Snitselaar <dev@xxxxxxxxxxxxxx>
> ---
>  drivers/staging/comedi/drivers/s626.c |   27 +++++++++++++++++++--------
>  1 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers/s626.c b/drivers/staging/comedi/drivers/s626.c
> index 23fc64b..e531f1c 100644
> --- a/drivers/staging/comedi/drivers/s626.c
> +++ b/drivers/staging/comedi/drivers/s626.c
> @@ -83,8 +83,17 @@ MODULE_AUTHOR("Gianluca Palli <gpalli@xxxxxxxxxxxxx>");
>  MODULE_DESCRIPTION("Sensoray 626 Comedi driver module");
>  MODULE_LICENSE("GPL");
>  
> +#define PCI_VENDOR_ID_S626 0x1131
> +#define PCI_DEVICE_ID_S626 0x7146
> +#define PCI_SUBVENDOR_ID_S626 0x6000
> +#define PCI_SUBDEVICE_ID_S626 0x0272
> +
>  struct s626_board {
>         const char *name;
> +       int vendor_id;
> +       int device_id;
> +       int subvendor_id;
> +       int subdevice_id;
>         int ai_chans;
>         int ai_bits;
>         int ao_chans;
> @@ -97,6 +106,10 @@ struct s626_board {
>  static const struct s626_board s626_boards[] = {
>         {
>          .name = "s626",
> +        .vendor_id = PCI_VENDOR_ID_S626,
> +        .device_id = PCI_DEVICE_ID_S626,
> +        .subvendor_id = PCI_SUBVENDOR_ID_S626,
> +        .subdevice_id = PCI_SUBDEVICE_ID_S626,
>          .ai_chans = S626_ADC_CHANNELS,
>          .ai_bits = 14,
>          .ao_chans = S626_DAC_CHANNELS,
> @@ -108,8 +121,6 @@ static const struct s626_board s626_boards[] = {
>  };
>  
>  #define thisboard ((const struct s626_board *)dev->board_ptr)
> -#define PCI_VENDOR_ID_S626 0x1131
> -#define PCI_DEVICE_ID_S626 0x7146
>  
>  /*
>   * For devices with vendor:device id == 0x1131:0x7146 you must specify
> @@ -117,7 +128,7 @@ static const struct s626_board s626_boards[] = {
>   * Philips SAA7146 media/dvb based cards.
>   */
>  static DEFINE_PCI_DEVICE_TABLE(s626_pci_table) = {
> -       {PCI_VENDOR_ID_S626, PCI_DEVICE_ID_S626, 0x6000, 0x0272, 0, 0, 0},
> +       {PCI_VENDOR_ID_S626, PCI_DEVICE_ID_S626, PCI_SUBVENDOR_ID_S626, PCI_SUBDEVICE_ID_S626, 0, 0, 0},
>         {0}
>  };
>  
> @@ -554,17 +565,17 @@ static int s626_attach(struct comedi_device *dev, struct comedi_devconfig *it)
>         resource_size_t resourceStart;
>         dma_addr_t appdma;
>         struct comedi_subdevice *s;
> -       const struct pci_device_id *ids;
>         struct pci_dev *pdev = NULL;
>  
>         if (alloc_private(dev, sizeof(struct s626_private)) < 0)
>                 return -ENOMEM;
>  
> -       for (i = 0; i < (ARRAY_SIZE(s626_pci_table) - 1) && !pdev; i++) {
> -               ids = &s626_pci_table[i];
> +       for (i = 0; i < ARRAY_SIZE(s626_boards) && !pdev; i++) {
>                 do {
> -                       pdev = pci_get_subsys(ids->vendor, ids->device,
> -                                             ids->subvendor, ids->subdevice,
> +                       pdev = pci_get_subsys(s626_boards[i].vendor_id,
> +                                             s626_boards[i].device_id,
> +                                             s626_boards[i].subvendor_id,
> +                                             s626_boards[i].subdevice_id,
>                                               pdev);

Ick, why is this loop even needed?  We are only here if the pci device
is present in the system so this shouldn't be needed at all, right?

Or is this a bit more complex than I am making it out to be?

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/