On Fri, 05 Feb 2016 17:50:51 +0100,
Sudip Mukherjee wrote:
>
> On Friday 05 February 2016 05:25 PM, Takashi Iwai wrote:
> > On Fri, 05 Feb 2016 07:17:06 +0100,
> > Sudip Mukherjee wrote:
> >>
> >> On Thu, Feb 04, 2016 at 05:51:07PM +0100, Takashi Iwai wrote:
> >>> On Thu, 04 Feb 2016 17:38:23 +0100,
> >>> Sudip Mukherjee wrote:
> >>>>
> >>>> Modify portman driver to use the new parallel port device model.
> >>>> The advantage of using the device model is that the device gets binded
> >>>> to the hardware, we get the feature of hotplug, we can bind/unbind
> >>>> the driver at runtime.
> >>>> The only change is in the way the driver gets registered with the
> >>>> parallel port subsystem and so as a result there is no user visible
> >>>> change or any chance of regression.
> >>>>
> >>>> Signed-off-by: Sudip Mukherjee <sudip@xxxxxxxxxxxxxxx>
> >>>> ---
> >>>>
> >>>> v3: changed commit message
> >>>> v2:
> >>>> 1. pardev_cb is initialized while declaring, thus removing the use of
> >>>> memset.
> >>>> 2. used pdev->id.
> >>>> 3. v1 did not have the parport probe callback, but
> >>>> we will need the probe callback for binding as the name of the driver
> >>>> and the name of the device is different.
> >>>> 4. in v1 I missed modifying snd_portman_probe_port().
> >>>>
> >>>> sound/drivers/portman2x4.c | 53 ++++++++++++++++++++++++++++++++++------------
> >>>> 1 file changed, 39 insertions(+), 14 deletions(-)
> >>>>
> >>>> diff --git a/sound/drivers/portman2x4.c b/sound/drivers/portman2x4.c
> >>>> index 172685d..a22f56c 100644
> >>>> --- a/sound/drivers/portman2x4.c
> >>>> +++ b/sound/drivers/portman2x4.c
> >>>> @@ -650,10 +650,21 @@ static int snd_portman_probe_port(struct parport *p)
> >>>> {
> >>>> struct pardevice *pardev;
> >>>> int res;
> >>>> -
> >>>> - pardev = parport_register_device(p, DRIVER_NAME,
> >>>> - NULL, NULL, NULL,
> >>>> - 0, NULL);
> >>>> + struct pardev_cb pdev_cb = {
> >>>> + .preempt = NULL,
> >>>> + .wakeup = NULL,
> >>>> + .private = NULL,
> >>>> + .irq_func = NULL,
> >>>> + .flags = 0,
> >>>> + };
> >>>> +
> >>>> + /*
> >>>> + * Specify the device number as SNDRV_CARDS + 1 so that the
> >>>> + * device id alloted to this temporary device will never clash
> >>>> + * with an actual device already registered.
> >>>> + */
> >>>> + pardev = parport_register_dev_model(p, DRIVER_NAME, &pdev_cb,
> >>>> + SNDRV_CARDS + 1);
> >>>
> >>> Hmm, doesn't this result in a device name like "xxx.33" ?
> >>
> >> yes, it will. But this is a temoporary device just to check if the
> >> sound card is connected to that particular parallel port or not. After
> >> checking this device is immediately unregistered. My idea here was to
> >> have a device number which will never clash with another device number.
> >> And we can never have a device like "xxx.33", so no conflict. :)
> >
> > Ah, this is the temporary one. If so, does it make sense to convert
> > this to dev_model one? This means that the device will be notified to
> > udev even though this is a temporary one to be removed immediately.
>
> But since we are registering a device it should ideally follow the
> dev_model.
We shouldn't advertise the device that shouldn't be handled by the
user-space. The device you're trying to register there is the one
that lives only shortly just for probing the address.
> > It's what we'd want to avoid. The function serves just as probing the
> > availability of the given port, not really registering anything
> > there.
>
> To my understanding, it is probing for the availability of the port and
> it is also calling portman_probe() which is initializing hardware
> handshake lines to midi box and checking if the portman card is
> connected to that parallel port or not.
>
> >
> > That is, we need to change the registration flow itself if we really
> > want to move dev_model for the whole.
>
> Any hint, how to register then?
> Without probing (reading and writing to that port) I can not know if
> that port is having the card and to use the port I need to register a
> device with that port.
Just returning the error at probe of the parport device itself instead
of doing the probe twice? The current way is racy in anyway.
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
