Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1303331 > unrolled thread

[PATCH 6/6] [ALSA] portman2x4 - use new parport device model

Started bySudip Mukherjee <sudipm.mukherjee@gmail.com>
First post2016-01-07 08:20 +0100
Last post2016-01-07 12:50 +0100
Articles 7 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 6/6] [ALSA] portman2x4 - use new parport device model Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-01-07 08:20 +0100
    Re: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model Takashi Iwai <tiwai@suse.de> - 2016-01-07 11:30 +0100
      Re: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport  device model Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-01-07 11:50 +0100
        Re: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model Takashi Iwai <tiwai@suse.de> - 2016-01-07 12:00 +0100
          Re: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport  device model Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-01-07 12:10 +0100
            Re: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport  device model Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-01-07 12:20 +0100
              Re: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model Takashi Iwai <tiwai@suse.de> - 2016-01-07 12:50 +0100

#1303331 — [PATCH 6/6] [ALSA] portman2x4 - use new parport device model

FromSudip Mukherjee <sudipm.mukherjee@gmail.com>
Date2016-01-07 08:20 +0100
Subject[PATCH 6/6] [ALSA] portman2x4 - use new parport device model
Message-ID<qOd1w-4IS-19@gated-at.bofh.it>
Modify portman driver to use the new parallel port device model.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 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 */
+
+	pardev = parport_register_dev_model(p,		   /* port */
+					    DRIVER_NAME,   /* name */
+					    &portman_cb,   /* callbacks */
+					    device_count); /* device number */
 	if (!pardev) {
 		snd_printd("Cannot register pardevice\n");
 		err = -EIO;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1303464 — Re: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model

FromTakashi Iwai <tiwai@suse.de>
Date2016-01-07 11:30 +0100
SubjectRe: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model
Message-ID<qOfZo-6MT-13@gated-at.bofh.it>
In reply to#1303331
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?

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.


Takashi


>  	if (!pardev) {
>  		snd_printd("Cannot register pardevice\n");
>  		err = -EIO;
> -- 
> 1.9.1
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303472 — Re: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model

FromSudip Mukherjee <sudipm.mukherjee@gmail.com>
Date2016-01-07 11:50 +0100
SubjectRe: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model
Message-ID<qOgiK-6Tx-11@gated-at.bofh.it>
In reply to#1303464
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.

Regards
Sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303480 — Re: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model

FromTakashi Iwai <tiwai@suse.de>
Date2016-01-07 12:00 +0100
SubjectRe: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model
Message-ID<qOgsq-6Yo-21@gated-at.bofh.it>
In reply to#1303472
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()).


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303484 — Re: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model

FromSudip Mukherjee <sudipm.mukherjee@gmail.com>
Date2016-01-07 12:10 +0100
SubjectRe: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model
Message-ID<qOgC6-7hc-21@gated-at.bofh.it>
In reply to#1303480
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303489 — Re: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model

FromSudip Mukherjee <sudipm.mukherjee@gmail.com>
Date2016-01-07 12:20 +0100
SubjectRe: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model
Message-ID<qOgLM-7kJ-13@gated-at.bofh.it>
In reply to#1303484
On Thu, Jan 07, 2016 at 04:31:27PM +0530, Sudip Mukherjee wrote:
> 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:
> > > > > 
<snip>
> > > > 
> > > > > +
> > > > > +	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()).

did you mean something like this: (on top of my patch)

diff --git a/sound/drivers/portman2x4.c b/sound/drivers/portman2x4.c
index 88b25ca..d749786 100644
--- a/sound/drivers/portman2x4.c
+++ b/sound/drivers/portman2x4.c
@@ -688,14 +688,8 @@ static void snd_portman_attach(struct parport *p)
 
 	/* Since we dont get the return value of probe
 	 * We need to check if device probing succeeded or not */
-	if (!platform_get_drvdata(device)) {
+	if (!platform_get_drvdata(device))
 		platform_device_unregister(device);
-		return;
-	}
-
-	/* register device in global table */
-	platform_devices[device_count] = device;
-	device_count++;
 }
 
 static void snd_portman_detach(struct parport *p)
@@ -768,7 +762,7 @@ static int snd_portman_probe(struct platform_device *pdev)
 	pardev = parport_register_dev_model(p,		   /* port */
 					    DRIVER_NAME,   /* name */
 					    &portman_cb,   /* callbacks */
-					    device_count); /* device number */
+					    pdev->id);	   /* device number */
 	if (!pardev) {
 		snd_printd("Cannot register pardevice\n");
 		err = -EIO;
@@ -812,6 +806,10 @@ static int snd_portman_probe(struct platform_device *pdev)
 		goto __err;
 	}
 
+	/* register device in global table */
+	platform_devices[device_count] = device;
+	device_count++;
+
 	snd_printk(KERN_INFO "Portman 2x4 on 0x%lx\n", p->base);
 	return 0;
 

regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303503 — Re: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model

FromTakashi Iwai <tiwai@suse.de>
Date2016-01-07 12:50 +0100
SubjectRe: [alsa-devel] [PATCH 6/6] [ALSA] portman2x4 - use new parport device model
Message-ID<qOheO-7wQ-33@gated-at.bofh.it>
In reply to#1303489
On Thu, 07 Jan 2016 12:19:36 +0100,
Sudip Mukherjee wrote:
> 
> On Thu, Jan 07, 2016 at 04:31:27PM +0530, Sudip Mukherjee wrote:
> > 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:
> > > > > > 
> <snip>
> > > > > 
> > > > > > +
> > > > > > +	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()).
> 
> did you mean something like this: (on top of my patch)
> 
> diff --git a/sound/drivers/portman2x4.c b/sound/drivers/portman2x4.c
> index 88b25ca..d749786 100644
> --- a/sound/drivers/portman2x4.c
> +++ b/sound/drivers/portman2x4.c
> @@ -688,14 +688,8 @@ static void snd_portman_attach(struct parport *p)
>  
>  	/* Since we dont get the return value of probe
>  	 * We need to check if device probing succeeded or not */
> -	if (!platform_get_drvdata(device)) {
> +	if (!platform_get_drvdata(device))
>  		platform_device_unregister(device);
> -		return;
> -	}
> -
> -	/* register device in global table */
> -	platform_devices[device_count] = device;
> -	device_count++;
>  }
>  
>  static void snd_portman_detach(struct parport *p)
> @@ -768,7 +762,7 @@ static int snd_portman_probe(struct platform_device *pdev)
>  	pardev = parport_register_dev_model(p,		   /* port */
>  					    DRIVER_NAME,   /* name */
>  					    &portman_cb,   /* callbacks */
> -					    device_count); /* device number */
> +					    pdev->id);	   /* device number */
>  	if (!pardev) {
>  		snd_printd("Cannot register pardevice\n");
>  		err = -EIO;
> @@ -812,6 +806,10 @@ static int snd_portman_probe(struct platform_device *pdev)
>  		goto __err;
>  	}
>  
> +	/* register device in global table */
> +	platform_devices[device_count] = device;
> +	device_count++;
> +
>  	snd_printk(KERN_INFO "Portman 2x4 on 0x%lx\n", p->base);
>  	return 0;

Hmm, this doesn't look better either.
I think the necessary change is just the access of device_count in
partport_register_dev_model().  This can be replaced with pdev->id, so
that you don't touch device_count at all there.

In anyway, these patch series (also for mst) are too late for 4.5.
These are neither serious bug fix nor function improvement, and the
drivers you're modifying are for the very minor devices.
So, please resubmit after the next merge window is closed.

While we're at it, some comments about other patches in the series:
I see no big merit to split several whitespace and blank line patches.
These are basically all whitespace fixes, so smash as a single patch.
The most important point is that this won't change any code context at
all but just a matter of white spaces.  (And write it clearly in the
change log -- so that reader can ignore this e.g. while bisecting.)

The NULL check is a matter of taste and isn't worth to fix at all.

The assignment in if is good to fix in general, so this can be a
separate patch indeed.


thanks,

Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web