Re: staging driver s626 clashes with philips SAA7146 media/dvb based cards

From: Herton Ronaldo Krzesinski
Date: Wed Jun 17 2009 - 20:37:55 EST


Em Qua 17 Jun 2009, às 21:21:51, Frank Mori Hess escreveu:
> On Wednesday 17 June 2009, you wrote:
> > >
> > > This patch looks buggy. It's changing the logic beyond just checking
> > > for subvendor/subdevice ids.
> >
> > That's the intention here, so that it avoids someone adding a new pci id
> > without specifying either subvendor or subdevice id for 0x1131:0x7146
> > boards, but yes there will be a problem if boards with vendor:id not
> > equal to 0x1131:0x7146 appear in future, as you will be obliged to add
> > subvendor:subdevice id even if not needed.
>
> Your patch breaks configuration of the board unless the bus and slot are
> explicitly specified. Just make a minimal patch that replaces
> pci_get_device with pci_get_subsys and fixes the problem that was
> reported.

Hmm that's not what the patch does, it doesn't break configuration, keeps
the same logic as before (I was wrong in my last email replying to myself),
check it, if it->options[0] and it->options[1] isn't specified, the pdev is
valid so the for loop exits (see !pdev check).

>
> > If not wanted and it gone too far, I can revert to use the same logic as
> > pci_match_id, or just simplify this in case it's unlikely more s626
> > boards appear.
> >
> > The current situation is ugly, comedi subsystem could have a better way
> > to deal with hotplug and probe of devices, without you having to
> > reimplement what pci subsystem functions already does.
>
> Agreed, it's currently in a limbo between trying to support auto probing of
> devices and supporting comedi's old way of configuring hardware. But I
> don't anticipate you are going to refactor the comedi core and all the
> drivers, so just make a minimal patch that doesn't change any logic it
> doesn't need to.
>
>
--
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/