linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] lirc_bt829 fixes, second try
@ 2013-09-05  2:30 Ben Hutchings
  2013-09-05  2:31 ` [PATCH v2 1/4] [media] lirc_bt829: Fix physical address type Ben Hutchings
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Ben Hutchings @ 2013-09-05  2:30 UTC (permalink / raw)
  To: Jarod Wilson, Mauro Carvalho Chehab; +Cc: linux-media, devel

[-- Attachment #1: Type: text/plain, Size: 513 bytes --]

A less ambitious set of fixes for the lirc_bt829 driver.

Ben.

Ben Hutchings (4):
  [media] lirc_bt829: Fix physical address type
  [media] lirc_bt829: Fix iomap and PCI device leaks
  [media] lirc_bt829: Enable and disable device
  [media] lirc_bt829: Note in TODO why it can't be a normal PCI driver
    yet

 drivers/staging/media/lirc/TODO         |  5 +++++
 drivers/staging/media/lirc/lirc_bt829.c | 33 +++++++++++++++++++++++++++------
 2 files changed, 32 insertions(+), 6 deletions(-)



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/4] [media] lirc_bt829: Fix physical address type
  2013-09-05  2:30 [PATCH v2 0/4] lirc_bt829 fixes, second try Ben Hutchings
@ 2013-09-05  2:31 ` Ben Hutchings
  2013-09-05  3:05   ` Fabio Estevam
  2013-09-05  2:31 ` [PATCH v2 2/4] [media] lirc_bt829: Fix iomap and PCI device leaks Ben Hutchings
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2013-09-05  2:31 UTC (permalink / raw)
  To: Jarod Wilson, Mauro Carvalho Chehab; +Cc: linux-media, devel, Fabio Estevam

[-- Attachment #1: Type: text/plain, Size: 1049 bytes --]

Use phys_addr_t and log format %pa.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/staging/media/lirc/lirc_bt829.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_bt829.c b/drivers/staging/media/lirc/lirc_bt829.c
index fa31ee7..9c7be55 100644
--- a/drivers/staging/media/lirc/lirc_bt829.c
+++ b/drivers/staging/media/lirc/lirc_bt829.c
@@ -63,7 +63,7 @@ static bool debug;
 	} while (0)
 
 static int atir_minor;
-static unsigned long pci_addr_phys;
+static phys_addr_t pci_addr_phys;
 static unsigned char *pci_addr_lin;
 
 static struct lirc_driver atir_driver;
@@ -78,8 +78,7 @@ static struct pci_dev *do_pci_probe(void)
 		pci_addr_phys = 0;
 		if (my_dev->resource[0].flags & IORESOURCE_MEM) {
 			pci_addr_phys = my_dev->resource[0].start;
-			pr_info("memory at 0x%08X\n",
-			       (unsigned int)pci_addr_phys);
+			pr_info("memory at %pa\n", &pci_addr_phys);
 		}
 		if (pci_addr_phys == 0) {
 			pr_err("no memory resource ?\n");



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/4] [media] lirc_bt829: Fix iomap and PCI device leaks
  2013-09-05  2:30 [PATCH v2 0/4] lirc_bt829 fixes, second try Ben Hutchings
  2013-09-05  2:31 ` [PATCH v2 1/4] [media] lirc_bt829: Fix physical address type Ben Hutchings
@ 2013-09-05  2:31 ` Ben Hutchings
  2013-09-05  2:32 ` [PATCH v2 3/4] [media] lirc_bt829: Enable and disable device Ben Hutchings
  2013-09-05  2:32 ` [PATCH v2 4/4] [media] lirc_bt829: Note in TODO why it can't be a normal PCI driver yet Ben Hutchings
  3 siblings, 0 replies; 6+ messages in thread
From: Ben Hutchings @ 2013-09-05  2:31 UTC (permalink / raw)
  To: Jarod Wilson, Mauro Carvalho Chehab; +Cc: linux-media, devel

[-- Attachment #1: Type: text/plain, Size: 1687 bytes --]

We must call iounmap() and pci_dev_put() when removed and on
the probe failure path.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/staging/media/lirc/lirc_bt829.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_bt829.c b/drivers/staging/media/lirc/lirc_bt829.c
index 9c7be55..a61d233 100644
--- a/drivers/staging/media/lirc/lirc_bt829.c
+++ b/drivers/staging/media/lirc/lirc_bt829.c
@@ -82,6 +82,7 @@ static struct pci_dev *do_pci_probe(void)
 		}
 		if (pci_addr_phys == 0) {
 			pr_err("no memory resource ?\n");
+			pci_dev_put(my_dev);
 			return NULL;
 		}
 	} else {
@@ -119,13 +120,16 @@ static void atir_set_use_dec(void *data)
 int init_module(void)
 {
 	struct pci_dev *pdev;
+	int rc;
 
 	pdev = do_pci_probe();
 	if (pdev == NULL)
 		return -ENODEV;
 
-	if (!atir_init_start())
-		return -ENODEV;
+	if (!atir_init_start()) {
+		rc = -ENODEV;
+		goto err_put_dev;
+	}
 
 	strcpy(atir_driver.name, "ATIR");
 	atir_driver.minor       = -1;
@@ -141,17 +145,28 @@ int init_module(void)
 	atir_minor = lirc_register_driver(&atir_driver);
 	if (atir_minor < 0) {
 		pr_err("failed to register driver!\n");
-		return atir_minor;
+		rc = atir_minor;
+		goto err_unmap;
 	}
 	dprintk("driver is registered on minor %d\n", atir_minor);
 
 	return 0;
+
+err_unmap:
+	iounmap(pci_addr_lin);
+err_put_dev:
+	pci_dev_put(pdev);
+	return rc;
 }
 

 void cleanup_module(void)
 {
+	struct pci_dev *pdev = to_pci_dev(atir_driver.dev);
+
 	lirc_unregister_driver(atir_minor);
+	iounmap(pci_addr_lin);
+	pci_dev_put(pdev);
 }
 




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 3/4] [media] lirc_bt829: Enable and disable device
  2013-09-05  2:30 [PATCH v2 0/4] lirc_bt829 fixes, second try Ben Hutchings
  2013-09-05  2:31 ` [PATCH v2 1/4] [media] lirc_bt829: Fix physical address type Ben Hutchings
  2013-09-05  2:31 ` [PATCH v2 2/4] [media] lirc_bt829: Fix iomap and PCI device leaks Ben Hutchings
@ 2013-09-05  2:32 ` Ben Hutchings
  2013-09-05  2:32 ` [PATCH v2 4/4] [media] lirc_bt829: Note in TODO why it can't be a normal PCI driver yet Ben Hutchings
  3 siblings, 0 replies; 6+ messages in thread
From: Ben Hutchings @ 2013-09-05  2:32 UTC (permalink / raw)
  To: Jarod Wilson, Mauro Carvalho Chehab; +Cc: linux-media, devel

[-- Attachment #1: Type: text/plain, Size: 1097 bytes --]

We must not assume that the PCI device is already enabled.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/staging/media/lirc/lirc_bt829.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/lirc/lirc_bt829.c b/drivers/staging/media/lirc/lirc_bt829.c
index a61d233..623f10e 100644
--- a/drivers/staging/media/lirc/lirc_bt829.c
+++ b/drivers/staging/media/lirc/lirc_bt829.c
@@ -126,9 +126,13 @@ int init_module(void)
 	if (pdev == NULL)
 		return -ENODEV;
 
+	rc = pci_enable_device(pdev);
+	if (rc)
+		goto err_put_dev;
+
 	if (!atir_init_start()) {
 		rc = -ENODEV;
-		goto err_put_dev;
+		goto err_disable;
 	}
 
 	strcpy(atir_driver.name, "ATIR");
@@ -154,6 +158,8 @@ int init_module(void)
 
 err_unmap:
 	iounmap(pci_addr_lin);
+err_disable:
+	pci_disable_device(pdev);
 err_put_dev:
 	pci_dev_put(pdev);
 	return rc;
@@ -166,6 +172,7 @@ void cleanup_module(void)
 
 	lirc_unregister_driver(atir_minor);
 	iounmap(pci_addr_lin);
+	pci_disable_device(pdev);
 	pci_dev_put(pdev);
 }
 



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 4/4] [media] lirc_bt829: Note in TODO why it can't be a normal PCI driver yet
  2013-09-05  2:30 [PATCH v2 0/4] lirc_bt829 fixes, second try Ben Hutchings
                   ` (2 preceding siblings ...)
  2013-09-05  2:32 ` [PATCH v2 3/4] [media] lirc_bt829: Enable and disable device Ben Hutchings
@ 2013-09-05  2:32 ` Ben Hutchings
  3 siblings, 0 replies; 6+ messages in thread
From: Ben Hutchings @ 2013-09-05  2:32 UTC (permalink / raw)
  To: Jarod Wilson, Mauro Carvalho Chehab; +Cc: linux-media, devel

[-- Attachment #1: Type: text/plain, Size: 829 bytes --]

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/staging/media/lirc/TODO | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/staging/media/lirc/TODO b/drivers/staging/media/lirc/TODO
index b6cb593..cbea5d8 100644
--- a/drivers/staging/media/lirc/TODO
+++ b/drivers/staging/media/lirc/TODO
@@ -2,6 +2,11 @@
   (see drivers/media/IR/mceusb.c vs. lirc_mceusb.c in lirc cvs for an
   example of a previously completed port).
 
+- lirc_bt829 uses registers on a Mach64 VT, which has a separate kernel
+  framebuffer driver (atyfb) and userland X driver (mach64).  It can't
+  simply be converted to a normal PCI driver, but ideally it should be
+  coordinated with the other drivers.
+
 Please send patches to:
 Jarod Wilson <jarod@wilsonet.com>
 Greg Kroah-Hartman <greg@kroah.com>


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/4] [media] lirc_bt829: Fix physical address type
  2013-09-05  2:31 ` [PATCH v2 1/4] [media] lirc_bt829: Fix physical address type Ben Hutchings
@ 2013-09-05  3:05   ` Fabio Estevam
  0 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2013-09-05  3:05 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Jarod Wilson, Mauro Carvalho Chehab, linux-media, devel

On Wed, Sep 4, 2013 at 11:31 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> Use phys_addr_t and log format %pa.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
>  drivers/staging/media/lirc/lirc_bt829.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/media/lirc/lirc_bt829.c b/drivers/staging/media/lirc/lirc_bt829.c
> index fa31ee7..9c7be55 100644
> --- a/drivers/staging/media/lirc/lirc_bt829.c
> +++ b/drivers/staging/media/lirc/lirc_bt829.c
> @@ -63,7 +63,7 @@ static bool debug;
>         } while (0)
>
>  static int atir_minor;
> -static unsigned long pci_addr_phys;
> +static phys_addr_t pci_addr_phys;
>  static unsigned char *pci_addr_lin;
>
>  static struct lirc_driver atir_driver;
> @@ -78,8 +78,7 @@ static struct pci_dev *do_pci_probe(void)
>                 pci_addr_phys = 0;
>                 if (my_dev->resource[0].flags & IORESOURCE_MEM) {
>                         pci_addr_phys = my_dev->resource[0].start;
> -                       pr_info("memory at 0x%08X\n",
> -                              (unsigned int)pci_addr_phys);
> +                       pr_info("memory at %pa\n", &pci_addr_phys);

Looks much better :-)

Reviewed-by: Fabio Estevam <fabio.estevam@freescale.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-09-05  3:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-05  2:30 [PATCH v2 0/4] lirc_bt829 fixes, second try Ben Hutchings
2013-09-05  2:31 ` [PATCH v2 1/4] [media] lirc_bt829: Fix physical address type Ben Hutchings
2013-09-05  3:05   ` Fabio Estevam
2013-09-05  2:31 ` [PATCH v2 2/4] [media] lirc_bt829: Fix iomap and PCI device leaks Ben Hutchings
2013-09-05  2:32 ` [PATCH v2 3/4] [media] lirc_bt829: Enable and disable device Ben Hutchings
2013-09-05  2:32 ` [PATCH v2 4/4] [media] lirc_bt829: Note in TODO why it can't be a normal PCI driver yet Ben Hutchings

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).