[PATCH] via-camera: Fix OLPC serial check

2011-03-03 Thread Daniel Drake
The code that checks the OLPC serial port is never built at the moment,
because CONFIG_OLPC_XO_1_5 doesn't exist and probably won't be added.

Fix it so that it gets compiled in, only executes on OLPC laptops, and
move the check into the probe routine.

The compiler is smart enough to eliminate this code when CONFIG_OLPC=n
(due to machine_is_olpc() always returning false).

Signed-off-by: Daniel Drake 
---
 drivers/media/video/via-camera.c |   83 +-
 1 files changed, 37 insertions(+), 46 deletions(-)

diff --git a/drivers/media/video/via-camera.c b/drivers/media/video/via-camera.c
index 2f973cd..4f19edc 100644
--- a/drivers/media/video/via-camera.c
+++ b/drivers/media/video/via-camera.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "via-camera.h"
 
@@ -38,14 +39,12 @@ MODULE_PARM_DESC(flip_image,
"If set, the sensor will be instructed to flip the image "
"vertically.");
 
-#ifdef CONFIG_OLPC_XO_1_5
 static int override_serial;
 module_param(override_serial, bool, 0444);
 MODULE_PARM_DESC(override_serial,
"The camera driver will normally refuse to load if "
"the XO 1.5 serial port is enabled.  Set this option "
-   "to force the issue.");
-#endif
+   "to force-enable the camera.");
 
 /*
  * Basic window sizes.
@@ -1261,6 +1260,37 @@ static struct video_device viacam_v4l_template = {
.release= video_device_release_empty, /* Check this */
 };
 
+/*
+ * The OLPC folks put the serial port on the same pin as
+ * the camera. They also get grumpy if we break the
+ * serial port and keep them from using it.  So we have
+ * to check the serial enable bit and not step on it.
+ */
+#define VIACAM_SERIAL_DEVFN 0x88
+#define VIACAM_SERIAL_CREG 0x46
+#define VIACAM_SERIAL_BIT 0x40
+
+static __devinit bool viacam_serial_is_enabled(void)
+{
+   struct pci_bus *pbus = pci_find_bus(0, 0);
+   u8 cbyte;
+
+   pci_bus_read_config_byte(pbus, VIACAM_SERIAL_DEVFN,
+   VIACAM_SERIAL_CREG, &cbyte);
+   if ((cbyte & VIACAM_SERIAL_BIT) == 0)
+   return false; /* Not enabled */
+   if (override_serial == 0) {
+   printk(KERN_NOTICE "Via camera: serial port is enabled, " \
+   "refusing to load.\n");
+   printk(KERN_NOTICE "Specify override_serial=1 to force " \
+   "module loading.\n");
+   return true;
+   }
+   printk(KERN_NOTICE "Via camera: overriding serial port\n");
+   pci_bus_write_config_byte(pbus, VIACAM_SERIAL_DEVFN,
+   VIACAM_SERIAL_CREG, cbyte & ~VIACAM_SERIAL_BIT);
+   return false;
+}
 
 static __devinit int viacam_probe(struct platform_device *pdev)
 {
@@ -1292,6 +1322,10 @@ static __devinit int viacam_probe(struct platform_device 
*pdev)
printk(KERN_ERR "viacam: No I/O memory, so no pictures\n");
return -ENOMEM;
}
+
+   if (machine_is_olpc() && viacam_serial_is_enabled())
+   return -EBUSY;
+
/*
 * Basic structure initialization.
 */
@@ -1395,7 +1429,6 @@ static __devexit int viacam_remove(struct platform_device 
*pdev)
return 0;
 }
 
-
 static struct platform_driver viacam_driver = {
.driver = {
.name = "viafb-camera",
@@ -1404,50 +1437,8 @@ static struct platform_driver viacam_driver = {
.remove = viacam_remove,
 };
 
-
-#ifdef CONFIG_OLPC_XO_1_5
-/*
- * The OLPC folks put the serial port on the same pin as
- * the camera. They also get grumpy if we break the
- * serial port and keep them from using it.  So we have
- * to check the serial enable bit and not step on it.
- */
-#define VIACAM_SERIAL_DEVFN 0x88
-#define VIACAM_SERIAL_CREG 0x46
-#define VIACAM_SERIAL_BIT 0x40
-
-static __devinit int viacam_check_serial_port(void)
-{
-   struct pci_bus *pbus = pci_find_bus(0, 0);
-   u8 cbyte;
-
-   pci_bus_read_config_byte(pbus, VIACAM_SERIAL_DEVFN,
-   VIACAM_SERIAL_CREG, &cbyte);
-   if ((cbyte & VIACAM_SERIAL_BIT) == 0)
-   return 0; /* Not enabled */
-   if (override_serial == 0) {
-   printk(KERN_NOTICE "Via camera: serial port is enabled, " \
-   "refusing to load.\n");
-   printk(KERN_NOTICE "Specify override_serial=1 to force " \
-   "module loading.\n");
-   return -EBUSY;
-   }
-   printk(KERN_NOTICE "Via camera: overriding serial port\n");
-   pci_bus_write_config_byte(pbus, VIACAM_SERIAL_DEVFN,
-   VIACAM_SERIAL_CREG, cbyte & ~VIACAM_SERIAL_BIT);
-   return 0;
-}
-#endif
-
-
-
-
 static int viacam_init(void)
 {
-#ifdef CONFIG_OLPC_XO_1_5
-   if (viacam_check_serial_port())
-   return -EBUSY;
-#endif
return platform_driver_register(&viacam_driver);

Re: [PATCH] via-camera: Fix OLPC serial check

2011-03-10 Thread Jonathan Corbet
[Trying to bash my inbox into reasonable shape, sorry for the slow
response...]

On Thu,  3 Mar 2011 19:03:31 + (GMT)
Daniel Drake  wrote:

> The code that checks the OLPC serial port is never built at the moment,
> because CONFIG_OLPC_XO_1_5 doesn't exist and probably won't be added.
> 
> Fix it so that it gets compiled in, only executes on OLPC laptops, and
> move the check into the probe routine.
> 
> The compiler is smart enough to eliminate this code when CONFIG_OLPC=n
> (due to machine_is_olpc() always returning false).

Getting rid of the nonexistent config option is clearly the right thing to
do.  I only wonder about moving the check to viacam_probe().  The nice
thing about having things fail in viacam_init() is that, if the camera is
not usable, the module will not load at all.  By the time you get to
viacam_probe(), the module is there but not will be useful for anything.

Did the check need to move for some reason?  If so, a one-of-these-days
nice feature might be to allow changing override_serial at run time.

Regardless, the behavior change only affects OLPC folks using the serial
line, so I'm OK with it.

Acked-by: Jonathan Corbet 

Thanks,

jon

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] via-camera: Fix OLPC serial check

2011-03-10 Thread Daniel Drake
On 10 March 2011 16:24, Jonathan Corbet  wrote:
> Did the check need to move for some reason?  If so, a one-of-these-days
> nice feature might be to allow changing override_serial at run time.

Yes. Otherwise the check would execute if that driver is loaded on
XO-1. The check either had to be modified or moved into a place where
we know we're on the right hardware. I chose the latter. You can still
unload the module and reload with a different setting if needed.

I haven't heard of anyone using the module param now that the firmware
sets the bit correctly and new serial adapters have been widely
distributed, making the selection between camera and serial fully
automatic.

Thanks for the review. Mauro, please merge.

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] via-camera: Fix OLPC serial check

2011-03-21 Thread Mauro Carvalho Chehab
Em 10-03-2011 13:24, Jonathan Corbet escreveu:
> [Trying to bash my inbox into reasonable shape, sorry for the slow
> response...]
> 
> On Thu,  3 Mar 2011 19:03:31 + (GMT)
> Daniel Drake  wrote:
> 
>> The code that checks the OLPC serial port is never built at the moment,
>> because CONFIG_OLPC_XO_1_5 doesn't exist and probably won't be added.
>>
>> Fix it so that it gets compiled in, only executes on OLPC laptops, and
>> move the check into the probe routine.
>>
>> The compiler is smart enough to eliminate this code when CONFIG_OLPC=n
>> (due to machine_is_olpc() always returning false).
> 
> Getting rid of the nonexistent config option is clearly the right thing to
> do.  I only wonder about moving the check to viacam_probe().  The nice
> thing about having things fail in viacam_init() is that, if the camera is
> not usable, the module will not load at all.  By the time you get to
> viacam_probe(), the module is there but not will be useful for anything.
> 
> Did the check need to move for some reason?  If so, a one-of-these-days
> nice feature might be to allow changing override_serial at run time.
> 
> Regardless, the behavior change only affects OLPC folks using the serial
> line, so I'm OK with it.
> 
>   Acked-by: Jonathan Corbet 

Weird, patchwork didn't catch the acked-by:
https://patchwork.kernel.org/patch/607461/

Perhaps it only does the right thing if the ack is not indented?

As reference, I'm enclosing what patchwork provides when clicking at the
mbox download hyperlink.

Anyway, manually added.

Thanks!
Mauro

---

>From patchwork Thu Mar  3 19:03:31 2011
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: via-camera: Fix OLPC serial check
Date: Thu, 03 Mar 2011 19:03:31 -
From: Daniel Drake 
X-Patchwork-Id: 607461
Message-Id: <20110303190331.e8ed79d4...@zog.reactivated.net>
To: mche...@infradead.org
Cc: linux-media@vger.kernel.org

The code that checks the OLPC serial port is never built at the moment,
because CONFIG_OLPC_XO_1_5 doesn't exist and probably won't be added.

Fix it so that it gets compiled in, only executes on OLPC laptops, and
move the check into the probe routine.

The compiler is smart enough to eliminate this code when CONFIG_OLPC=n
(due to machine_is_olpc() always returning false).

Signed-off-by: Daniel Drake 

---
drivers/media/video/via-camera.c |   83 +-
 1 files changed, 37 insertions(+), 46 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html