Messages in this thread | | | Date | Fri, 05 Feb 2016 22:34:42 +0530 | From | Sudip Mukherjee <> | Subject | Re: [PATCH v3 3/3] portman2x4 - use new parport device model |
| |
On Friday 05 February 2016 10:31 PM, Takashi Iwai wrote: > 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@vectorindia.org> >>>>>> --- >>>>>> >>>>>> 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.
Ohhhk.. so we only register once and if we find the card - we continue, we donot find the card then we unregister the device and return error code. Great. I will send you a patch for your review.
Regards Sudip
| |