On Wednesday, February 22, 2012 21:33:29 Ondrej Zary wrote: > On Saturday 18 February 2012 17:33:32 Ondrej Zary wrote: > > Hello, > > there are some ISA radio cards with PnP support (e.g. SF16-FMI) but the new > > ISA radio framework has no PnP support. > > > > I got AOpen FX-3D/Pro Radio card which is AD1816 with Gemtek radio - and > > with PnP. But radio-gemtek fails to load because the radio I/O port is not > > enabled (and the driver does not support PnP). > > > > Tried to add PnP support to radio-isa but failed. Splitted non-isa_driver > > related parts from radio_isa_probe() to a separate function and tried to > > create radio_isa_pnp_probe() only to realize that I'm not able to access > > struct radio_isa_driver. > > > > radio_isa_probe() relies on the fact that "driver" (struct isa_driver) is > > the first element of struct radio_isa_driver, so these two structs have the > > same pointer: > > HW radio driver registers the driver by calling: > > isa_register_driver(&gemtek_driver.driver, GEMTEK_MAX); > > radio_isa_probe() in radio-isa.c does: > > struct radio_isa_driver *drv = pdev->platform_data; > > > > So adding struct pnp_driver to struct radio_isa_driver does not seem to be > > possible. > > Adding PnP support to original radio-gemtek (before conversion to ISA radio > framework) is easy. A patch like this (mostly copied from radio-cadet) allows > radio on AOpen FX-3D/Pro Radio card to work. > But how to do this with the new driver? I've been looking at that and I think the best way is to add radio_isa_pnp_probe and radio_isa_pnp_remove functions that can be used with the pnp framework. Both radio_isa_probe and radio_isa_pnp_probe call the same probe() function that gets the radio_isa_driver pointer. The struct radio_isa_driver should probably be modified as follows: struct radio_isa_driver { struct isa_driver *isa_drv; struct pnp_driver *pnp_drv; const struct radio_isa_ops *ops; ... Or perhaps even better: struct radio_isa_driver { union { struct isa_driver *isa_drv; struct pnp_driver *pnp_drv; }; const struct radio_isa_ops *ops; ... radio_isa_querycap uses driver.name, so I suspect struct radio_isa_driver should be extended with a const char *name that is set correctly by either radio_isa_probe or radio_isa_pnp_probe. Another alternative seems to be what radio-sf16fmi.c does. It uses low-level pnp functions but not pnp_driver. This approach should fit easily into the radio-isa framework. Regards, Hans > > > --- a/drivers/media/radio/radio-gemtek.c > +++ b/drivers/media/radio/radio-gemtek.c > @@ -23,6 +23,7 @@ > #include <linux/videodev2.h> /* kernel radio structs */ > #include <linux/mutex.h> > #include <linux/io.h> /* outb, outb_p */ > +#include <linux/pnp.h> > #include <media/v4l2-ioctl.h> > #include <media/v4l2-device.h> > > @@ -329,6 +330,46 @@ static int gemtek_verify(struct gemtek *gt, int port) > return 1; > } > > +#ifdef CONFIG_PNP > + > +static struct pnp_device_id gemtek_pnp_devices[] = { > + /* AOpen FX-3D/Pro Radio */ > + {.id = "ADS7183", .driver_data = 0}, > + {.id = ""} > +}; > + > +MODULE_DEVICE_TABLE(pnp, gemtek_pnp_devices); > + > +static int gemtek_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id) > +{ > + if (!dev) > + return -ENODEV; > + /* only support one device */ > + if (io > 0) > + return -EBUSY; > + > + if (!pnp_port_valid(dev, 0)) > + return -ENODEV; > + > + io = pnp_port_start(dev, 0); > + > + printk(KERN_INFO "radio-gemtek: PnP reports device at %#x\n", io); > + > + return io; > +} > + > +static struct pnp_driver gemtek_pnp_driver = { > + .name = "radio-gemtek", > + .id_table = gemtek_pnp_devices, > + .probe = gemtek_pnp_probe, > + .remove = NULL, > +}; > + > +#else > +static struct pnp_driver gemtek_pnp_driver; > +#endif > + > + > /* > * Automatic probing for card. > */ > @@ -536,8 +577,11 @@ static int __init gemtek_init(void) > mutex_init(>->lock); > > gt->verified = -1; > + if (io < 0) > + pnp_register_driver(&gemtek_pnp_driver); > + else > + gemtek_probe(gt); > gt->io = io; > - gemtek_probe(gt); > if (gt->io) { > if (!request_region(gt->io, 1, "gemtek")) { > v4l2_err(v4l2_dev, "I/O port 0x%x already in use.\n", gt->io); > @@ -608,6 +652,7 @@ static void __exit gemtek_exit(void) > video_unregister_device(>->vdev); > v4l2_device_unregister(>->v4l2_dev); > release_region(gt->io, 1); > + pnp_unregister_driver(&gemtek_pnp_driver); > } > > module_init(gemtek_init); > > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html