Re: [PATCH v3 3/3] portman2x4 - use new parport device model

[Sudip Mukherjee]

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.

> 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.

Regards
Sudip