lkml.org 
[lkml]   [2016]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model
On Thu, Jan 07, 2016 at 11:50:15AM +0100, Takashi Iwai wrote:
> On Thu, 07 Jan 2016 11:44:34 +0100,
> Sudip Mukherjee wrote:
> >
> > On Thu, Jan 07, 2016 at 11:26:44AM +0100, Takashi Iwai wrote:
> > > On Thu, 07 Jan 2016 08:15:51 +0100,
> > > Sudip Mukherjee wrote:
> > > >
> > > > Modify portman driver to use the new parallel port device model.
> > > >
> > > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > >
> > > Did you actually test this?
> >
> > No. :(
> > I donot have the hardware. But since the only change is in the way it
> > registers with the parport so it should not break.
> > I was preparing v2 for this and the other one. I missed seeing some more
> > points.
> > >
> > > Also about the changes:
> > >
> > > > ---
> > > > sound/drivers/portman2x4.c | 24 ++++++++++++++----------
> > > > 1 file changed, 14 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/sound/drivers/portman2x4.c b/sound/drivers/portman2x4.c
> > > > index 5fcde7d..88b25ca 100644
> > > > --- a/sound/drivers/portman2x4.c
> > > > +++ b/sound/drivers/portman2x4.c
> > > > @@ -704,9 +704,10 @@ static void snd_portman_detach(struct parport *p)
> > > > }
> > > >
> > > > static struct parport_driver portman_parport_driver = {
> > > > - .name = "portman2x4",
> > > > - .attach = snd_portman_attach,
> > > > - .detach = snd_portman_detach
> > > > + .name = "portman2x4",
> > > > + .match_port = snd_portman_attach,
> > > > + .detach = snd_portman_detach,
> > > > + .devmodel = true,
> > > > };
> > > >
> > > > /*********************************************************************
> > > > @@ -734,6 +735,7 @@ static int snd_portman_probe(struct platform_device *pdev)
> > > > struct snd_card *card = NULL;
> > > > struct portman *pm = NULL;
> > > > int err;
> > > > + struct pardev_cb portman_cb;
> > > >
> > > > p = platform_get_drvdata(pdev);
> > > > platform_set_drvdata(pdev, NULL);
> > > > @@ -758,13 +760,15 @@ static int snd_portman_probe(struct platform_device *pdev)
> > > > sprintf(card->longname, "%s at 0x%lx, irq %i",
> > > > card->shortname, p->base, p->irq);
> > > >
> > > > - pardev = parport_register_device(p, /* port */
> > > > - DRIVER_NAME, /* name */
> > > > - NULL, /* preempt */
> > > > - NULL, /* wakeup */
> > > > - snd_portman_interrupt, /* ISR */
> > > > - PARPORT_DEV_EXCL, /* flags */
> > > > - (void *)card); /* private */
> > > > + memset(&portman_cb, 0, sizeof(portman_cb));
> > > > + portman_cb.private = card; /* private */
> > > > + portman_cb.irq_func = snd_portman_interrupt; /* ISR */
> > > > + portman_cb.flags = PARPORT_DEV_EXCL; /* flags */
> > >
> > > You can put them initializers except for private. Then the explicit
> > > memset can be omitted.
> > >
> > > > +
> > > > + pardev = parport_register_dev_model(p, /* port */
> > > > + DRIVER_NAME, /* name */
> > > > + &portman_cb, /* callbacks */
> > > > + device_count); /* device number */
> > >
> > > Does device_count really work similarly for
> > > parport_register_dev_model()? I supposed the argument being the
> > > device id number while you're passing the number of devices to
> > > create.
> >
> > This device_count is actually used for the device name in
> > /sys/bus/parport/devices. Something like DRIVER_NAME.device_count.
>
> Well, but device_count is incremented in snd_portman_attach(). The
> management of device_count should be moved around the caller side, if
> we use this as the id (and use the assigned id instead of device_count
> in snd_portman_attach()).

But, snd_portman_attach() finally decides if the probe/attach was a
success or not. And it will save the device in
platform_devices[device_count] and then it will increment device_count
to prepare it for the next device. Ofcourse, we can do it in
snd_portman_probe() but isn't snd_portman_attach() the caller here?

If you want I can move the count to snd_portman_probe() but since I do
not have the hardware I tried to have the minimum possible change.

regards
sudip


\
 
 \ /
  Last update: 2016-01-07 12:41    [W:0.332 / U:0.360 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site