Re: [PATCH] [media] atmel_isi: allocate memory to store the isi platform data.

[Josh Wu]

Hi, all

Sorry, My mistake here. After checking the code, this ISI bug doesn't 
exist in current mainline code. So I will *cancel* this patch.


Since current mainline will copy this __initdata isi platform data to 
one static structure in function at91_add_device_isi(...). Then pass 
this static structure to the driver.


So the ISI driver has no bug that isi platform became invalid. I meet 
this is because I'm not call the at91_add_device_isi(...) since I try in 
the DT support board.


At last, even no above bug in the code, This isi_platform_data is still 
need to stored in ISI driver. Since if we support DT then we need this 
isi platform data and the function at91_add_device_isi(...) will not to 
be called (it is in device file).


So I think after soc-camera DT support is merged. Then I will send a DT 
support patch for ISI driver which will embed the isi_platform_data into 
atmel_isi.


Thank you all for the replies. That helps a lot even in this small 
patch.  :)


On 8/30/2012 12:02 AM, Guennadi Liakhovetski wrote:

On Wed, 29 Aug 2012, Sylwester Nawrocki wrote:


Hi,

On 08/29/2012 12:11 PM, Josh Wu wrote:

This patch fix the bug: ISI driver's platform data became invalid
when isi platform data's attribution is __initdata.

If the isi platform data is passed as __initdata. Then we need store
it in driver allocated memory. otherwise when we use it out of the
probe() function, then the isi platform data is invalid.

Signed-off-by: Josh Wu josh...@atmel.com
---
  drivers/media/platform/soc_camera/atmel-isi.c |   12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/soc_camera/atmel-isi.c 
b/drivers/media/platform/soc_camera/atmel-isi.c
index ec3f6a0..dc0fdec 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -926,6 +926,7 @@ static int __devexit atmel_isi_remove(struct 
platform_device *pdev)
clk_put(isi-mck);
clk_unprepare(isi-pclk);
clk_put(isi-pclk);
+   kfree(isi-pdata);
kfree(isi);
  
  	return 0;

@@ -968,8 +969,15 @@ static int __devinit atmel_isi_probe(struct 
platform_device *pdev)
goto err_alloc_isi;
}
  
+	isi-pdata = kzalloc(sizeof(struct isi_platform_data), GFP_KERNEL);

+   if (!isi-pdata) {
+   ret = -ENOMEM;
+   dev_err(pdev-dev, Can't allocate isi platform data!\n);
+   goto err_alloc_isi_pdata;
+   }
+   memcpy(isi-pdata, pdata, sizeof(struct isi_platform_data));
+

Why not just embed struct isi_platform_data in struct atmel_isi and drop this
another kzalloc() ?
Then you could simply do isi-pdata = *pdata.

Also, is this going to work when this driver is build and as a module
and its loading is deferred past system booting ? At that time the driver's
platform data may be well discarded.

Right, it will be gone, I think.


You may wan't to duplicate it on the
running boards in board code with kmemdup() or something.

How about removing __initdata from board code?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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


Best Regards,
Josh Wu
--
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

[PATCH v2] media: v4l2-ctrls: add control for dpcm predictor

[Prabhakar Lad]

From: Lad, Prabhakar prabhakar@ti.com

add V4L2_CID_DPCM_PREDICTOR control of type menu, which
determines the dpcm predictor. The predictor can be either
simple or advanced.

Signed-off-by: Lad, Prabhakar prabhakar@ti.com
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Cc: Sakari Ailus sakari.ai...@iki.fi
Cc: Hans Verkuil hans.verk...@cisco.com
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: Sylwester Nawrocki s.nawro...@samsung.com
Cc: Hans de Goede hdego...@redhat.com
Cc: Kyungmin Park kyungmin.p...@samsung.com
---
This patches has one checkpatch warning for line over
80 characters altough it can be avoided I have kept it
for consistency.

Changes for v2: 
1: Added documentaion in controls.xml pointed by Sylwester.
2: Chnaged V4L2_DPCM_PREDICTOR_ADVANCE to V4L2_DPCM_PREDICTOR_ADVANCED
   pointed by Sakari.

 Documentation/DocBook/media/v4l/controls.xml |   25 -
 drivers/media/v4l2-core/v4l2-ctrls.c |9 +
 include/linux/videodev2.h|5 +
 3 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/controls.xml 
b/Documentation/DocBook/media/v4l/controls.xml
index 93b9c68..84746d0 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -4267,7 +4267,30 @@ interface and may change in the future./para
pixels / second.
/entry
  /row
- rowentry/entry/row
+ row
+   entry 
spanname=idconstantV4L2_CID_DPCM_PREDICTOR/constant/entry
+   entrymenu/entry
+ /row
+ row id=v4l2-dpcm-predictor
+   entry spanname=descr DPCM Predictor: depicts what type of 
prediction
+   is used simple or advanced.
+   /entry
+ /row
+ row
+   entrytbl spanname=descr cols=2
+ tbody valign=top
+   row
+entryconstantV4L2_DPCM_PREDICTOR_SIMPLE/constant/entry
+ entryPredictor type is simple/entry
+   /row
+   row
+ 
entryconstantV4L2_DPCM_PREDICTOR_ADVANCED/constant/entry
+ entryPredictor type is advanced/entry
+   /row
+ /tbody
+   /entrytbl
+ /row
+   rowentry/entry/row
/tbody
   /tgroup
   /table
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index b6a2ee7..2d7bc15 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -425,6 +425,11 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
Gray,
NULL,
};
+   static const char * const dpcm_predictor[] = {
+   Simple Predictor,
+   Advanced Predictor,
+   NULL,
+   };
 
switch (id) {
case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
@@ -502,6 +507,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
return mpeg4_profile;
case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
return jpeg_chroma_subsampling;
+   case V4L2_CID_DPCM_PREDICTOR:
+   return dpcm_predictor;
 
default:
return NULL;
@@ -732,6 +739,7 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_IMAGE_PROC_CLASS: return Image Processing 
Controls;
case V4L2_CID_LINK_FREQ:return Link Frequency;
case V4L2_CID_PIXEL_RATE:   return Pixel Rate;
+   case V4L2_CID_DPCM_PREDICTOR:   return DPCM Predictor;
 
default:
return NULL;
@@ -832,6 +840,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum 
v4l2_ctrl_type *type,
case V4L2_CID_ISO_SENSITIVITY_AUTO:
case V4L2_CID_EXPOSURE_METERING:
case V4L2_CID_SCENE_MODE:
+   case V4L2_CID_DPCM_PREDICTOR:
*type = V4L2_CTRL_TYPE_MENU;
break;
case V4L2_CID_LINK_FREQ:
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 6d6dfa7..ca9fb78 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -2000,6 +2000,11 @@ enum v4l2_jpeg_chroma_subsampling {
 
 #define V4L2_CID_LINK_FREQ (V4L2_CID_IMAGE_PROC_CLASS_BASE 
+ 1)
 #define V4L2_CID_PIXEL_RATE(V4L2_CID_IMAGE_PROC_CLASS_BASE 
+ 2)
+#define V4L2_CID_DPCM_PREDICTOR
(V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
+enum v4l2_dpcm_predictor {
+   V4L2_DPCM_PREDICTOR_SIMPLE  = 0,
+   V4L2_DPCM_PREDICTOR_ADVANCED= 1,
+};
 
 /*
  * T U N I N G
-- 
1.7.0.4

--
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 RFC 0/4] V4L2: Vendor specific media bus formats/ frame size control

[Nicolas THERY]

Hello,

Thanks for your reply.  It's good to know your proposal is simply on the
back-burner.

Best regards,
Nicolas

On 2012-08-29 20:41, sakari.ai...@iki.fi wrote:
 Hi Nicolas,

 On Mon, Aug 27, 2012 at 05:48:43PM +0200, Nicolas THERY wrote:
 Hello,

 On 2012-08-23 11:51, Sylwester Nawrocki wrote:
 This patch series introduces new image source class control - 
 V4L2_CID_FRAMESIZE
 and vendor or device specific media bus format section.

 There was already a discussion WRT handling interleaved image data [1].
 I'm not terribly happy with those vendor specific media bus formats but I
 couldn't find better solution that would comply with the V4L2 API concepts
 and would work reliably.
 What about Sakari's Frame format descriptors RFC[1] that would allow to
 describe arbitrary pixel code combinations and provide required information
 (virtual channel and data type) to the CSI receiver driver for configuring 
 the
 hardware?
 I we'll need to continue that work as well, unfortunately I've had higher
 priority things to do. Still, getting that right is complex and will take
 time. The V4L2 pixel format for this sensor will likely be a
 hardware-specific one for quite a while: this sensor in question sends
 several frames in different formats of a single image at once which doesn't
 match to V4L2's pixel format configuration that assumes a single format.

 Kind regards,

--
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 RFC 0/4] V4L2: Vendor specific media bus formats/ frame size control

[Nicolas THERY]

Hello,

Thanks for your reply.  I overlooked this sensor packages multiple streams in a
single DT.  It seems indeed that Sakari's RFC would not help.

Best regards,

On 2012-08-29 23:51, Sylwester Nawrocki wrote:
 Hi Nicolas,
 
 On 08/27/2012 05:48 PM, Nicolas THERY wrote:
 Hello,

 On 2012-08-23 11:51, Sylwester Nawrocki wrote:
 This patch series introduces new image source class control - 
 V4L2_CID_FRAMESIZE
 and vendor or device specific media bus format section.

 There was already a discussion WRT handling interleaved image data [1].
 I'm not terribly happy with those vendor specific media bus formats but I
 couldn't find better solution that would comply with the V4L2 API concepts
 and would work reliably.

 What about Sakari's Frame format descriptors RFC[1] that would allow to
 describe arbitrary pixel code combinations and provide required information
 (virtual channel and data type) to the CSI receiver driver for configuring 
 the
 hardware?
 
 Thanks for reminding about this. The Frame format descriptors would not
 necessarily solve the main problem which I tried to address in this RFC.
 
 The sensor in question uses single MIPI-CSI data type frame as a container
 for multiple data planes, e.g. JPEG compressed stream interleaved with YUV
 image data, some optional padding and a specific metadata describing the
 interleaved image data. There is no MIPI-CSI2 virtual channel or data type 
 interleaving. Everything is transferred on single VC and single DT.
 
 Such a frames need sensor specific S/W algorithm do extract each component.
 
 So it didn't look like the frame descriptors would be helpful here, since
 all this needs to be mapped to a single fourcc. Not sure if defining a
 binary blob fourcc and retrieving frame format information by some other
 means would have been a way to go.
 
 I also had some patches adopting design from Sakari's RFC, for the case where
 in addition to the above frame format there was captured a copy of meta-data,
 (as in the frame footer) send on separate DT (Embedded Data). And this was
 mapped to 2-planar V4L2 pixel format. Even then I used a sensor specific
 media bus code.
 
 In the end of the day I switched to a single-planar format as it had all 
 what's needed to decode the data. And the were some H/W limitations on using
 additional DT. 
 
 The frame format descriptors might be worth to work on, but this doesn't 
 look like a solution to my problem and it is going to take some time to get 
 it right, as Sakari pointed out.
 
 --
 
 Regards,
 Sylwester
 --
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

[PATCH 1/2] [media] ds3000: Remove useless 'locking'

[Rémi Cardona]

Since b9bf2eafaad9c1ef02fb3db38c74568be601a43a, the function
ds3000_firmware_ondemand() is called only once during init. This
locking scheme may have been useful when the firmware was loaded at
each tune.

Furthermore, it looks like this 'lock' was put in to prevent concurrent
access (and not recursion as the comments suggest). However, this open-
coded mechanism is anything but race-free and should have used a proper
mutex.

Signed-off-by: Rémi Cardona remi.card...@smartjog.com
---
 drivers/media/dvb/frontends/ds3000.c |9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/media/dvb/frontends/ds3000.c 
b/drivers/media/dvb/frontends/ds3000.c
index 4c8ac26..066870a 100644
--- a/drivers/media/dvb/frontends/ds3000.c
+++ b/drivers/media/dvb/frontends/ds3000.c
@@ -233,7 +233,6 @@ struct ds3000_state {
struct i2c_adapter *i2c;
const struct ds3000_config *config;
struct dvb_frontend frontend;
-   u8 skip_fw_load;
/* previous uncorrected block counter for DVB-S2 */
u16 prevUCBS2;
 };
@@ -395,8 +394,6 @@ static int ds3000_firmware_ondemand(struct dvb_frontend *fe)
if (ds3000_readreg(state, 0xb2) = 0)
return ret;
 
-   if (state-skip_fw_load)
-   return 0;
/* Load firmware */
/* request the firmware, this will block until someone uploads it */
printk(KERN_INFO %s: Waiting for firmware upload (%s)...\n, __func__,
@@ -410,9 +407,6 @@ static int ds3000_firmware_ondemand(struct dvb_frontend *fe)
return ret;
}
 
-   /* Make sure we don't recurse back through here during loading */
-   state-skip_fw_load = 1;
-
ret = ds3000_load_firmware(fe, fw);
if (ret)
printk(%s: Writing firmware to device failed\n, __func__);
@@ -422,9 +416,6 @@ static int ds3000_firmware_ondemand(struct dvb_frontend *fe)
dprintk(%s: Firmware upload %s\n, __func__,
ret == 0 ? complete : failed);
 
-   /* Ensure firmware is always loaded if required */
-   state-skip_fw_load = 0;
-
return ret;
 }
 
-- 
1.7.10.4

--
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

[PATCH RFC 0/2] ds3000 firmware loading improvements

[Rémi Cardona]

Hi all,

Please consider these 2 patches as an RFC, especially the first one.

The first patch is something I've found while trying to wrap my head
around the driver and I could find no legitimate reason to keep this
lock since commit b9bf2ea, as dvb_frontend_init() is now only called in
a single thread within dvb_frontend_thread().

Here's a little context for the second patch: we (SmartJog) have found
a few TeVii S470/471 cards where the ds3000 front-end *sometimes*
reports that it already has a loaded firmware (the 0xb2 register) yet
the card does not really work. That's why I've added the printk() in
the second patch to display when such a case happens.

We thus have an extra patch that adds a module parameter to force the
firmware loading, regardless of the 0xb2 register's state. This patch
allows us to use cards that would otherwise fail. Would such a patch be
of interest?

Would a patch that completely removes the register check (and thus
always loads the firmware at init time) be a preferred alternative?

Thanks for any pointers/reviews.

Rémi Cardona

-- 
SmartJog | T: +33 1 5868 6229
27 Blvd Hippolyte Marquès, 94200 Ivry-sur-Seine, France
www.smartjog.com | a TDF Group company

--
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

[PATCH 2/2] [media] ds3000: properly report firmware loading issues

[Rémi Cardona]

ds3000_readreg() returns negative values in case of i2c failures. The
old code would simply return 0 when failing to read the 0xb2 register,
misleading ds3000_initfe() into believing that the firmware had been
correctly loaded.

Signed-off-by: Rémi Cardona remi.card...@smartjog.com
---
 drivers/media/dvb/frontends/ds3000.c |8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb/frontends/ds3000.c 
b/drivers/media/dvb/frontends/ds3000.c
index 066870a..4c774c4 100644
--- a/drivers/media/dvb/frontends/ds3000.c
+++ b/drivers/media/dvb/frontends/ds3000.c
@@ -391,8 +391,14 @@ static int ds3000_firmware_ondemand(struct dvb_frontend 
*fe)
 
dprintk(%s()\n, __func__);
 
-   if (ds3000_readreg(state, 0xb2) = 0)
+   ret = ds3000_readreg(state, 0xb2);
+   if (ret == 0) {
+   printk(KERN_INFO %s: Firmware already uploaded, skipping\n,
+   __func__);
return ret;
+   } else if (ret  0) {
+   return ret;
+   }
 
/* Load firmware */
/* request the firmware, this will block until someone uploads it */
-- 
1.7.10.4

--
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 2/2] [media] ds3000: properly report firmware loading issues

[Antti Palosaari]

On 08/30/2012 12:36 PM, Rémi Cardona wrote:

ds3000_readreg() returns negative values in case of i2c failures. The
old code would simply return 0 when failing to read the 0xb2 register,
misleading ds3000_initfe() into believing that the firmware had been
correctly loaded.

Signed-off-by: Rémi Cardona remi.card...@smartjog.com
---
  drivers/media/dvb/frontends/ds3000.c |8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb/frontends/ds3000.c 
b/drivers/media/dvb/frontends/ds3000.c
index 066870a..4c774c4 100644
--- a/drivers/media/dvb/frontends/ds3000.c
+++ b/drivers/media/dvb/frontends/ds3000.c
@@ -391,8 +391,14 @@ static int ds3000_firmware_ondemand(struct dvb_frontend 
*fe)

dprintk(%s()\n, __func__);

-   if (ds3000_readreg(state, 0xb2) = 0)
+   ret = ds3000_readreg(state, 0xb2);
+   if (ret == 0) {
+   printk(KERN_INFO %s: Firmware already uploaded, skipping\n,
+   __func__);
return ret;
+   } else if (ret  0) {
+   return ret;
+   }

/* Load firmware */
/* request the firmware, this will block until someone uploads it */



As I understand firmware downloading failure is coming from the fact 
that register read fails = fails to detect if firmware is already 
running or not.


Original behavior to expect firmware is loaded and running when register 
read fails is very stupid and your fix seems much better.


So first priority should be try fix that issue with register read. Is it 
coming from the USB stack (eg. error 110 timeout) or some other error 
coming from the fact chip answers wrong?


Do you see other register I/O failing too?

Does adding few usec sleep help?


regards
Antti


--
http://palosaari.fi/
--
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: [RFC v4] V4L DT bindings

[Nicolas THERY]

Hello,

I've got a couple of questions regarding lane swapping and
polarity inversion.

On 2012-08-25 01:27, Guennadi Liakhovetski wrote:
 Hi all
 
 After an initial RFC [1] and taking into consideration an even earlier 
 patch-set [2], Sylwester and I have spent some time discussing V4L DT 
 bindings, below is a result of those discussions.
 
 We have chosen to try to design a DT example, documentation and 
 implementation should follow. I'll try to bring together just several most 
 important points, that might not be immediately obvious from the example.
 
 1. Sylwester has initially designed his patches around a concept of a 
 central video node, that contains (references to) all video devices on 
 the system. This might make finding all relevant components easier and 
 should make power management more readily available. In the below example 
 such a node is missing. For now we decided not to require one, but systems 
 may choose to use them. Support for them might be added to the V4L DT 
 subsystem later.
 
 2. The below example contains the following 4 components:
(a) an SoC bridge (CEU node ceu0@0xfe91), note, that the bridge 
is also providing a master clock mclk: master_clock to sensors
(b) a CSI-2 interface csi2: csi2@0xffc9, that can be used with 
the above bridge
(c) an I2C parallel camera sensor ov772x_1: ov772x@0x21
(d) an I2C serial (MIPI CSI-2) camera sensor imx074: imx074@0x1a
 
 3. Linking of various components follows the V4L2 MC concept: each video 
 node can contain xxx: videolink@x child nodes. These nodes specify the 
 opposite end of the link and a local pad configuration. This is required, 
 because two linked pads might require different configuration. E.g., if 
 the board contains an inverter in the camera vertical sync line, 
 respective pads have to be configured with opposite vsync polarities.
 
 4. In the below example the following links are defined:
(a) ov772x_1_1: videolink@1 is a child of the CEU node, it links the 
CEU with the ov772x sensor.
(b) csi2_0: videolink@0 is also child of the CEU node, it links the 
CEU with the CSI-2 module. Note, that this link might not be 
necessary, since this is an SoC internal connection and drivers 
will know themselves how to configure it
(c) ceu0_1: videolink@0 is a chile of the OV772x node
(d) csi2_0_1: videolink@0 is a child of the IMX074 camera sensor node
(e) imx074_1: videolink@1 is a child of the CSI-2 node
(f) ceu0: videolink@0 is a child of the CSI-2 node - also might not 
be required
 
 5. Remote node references in videolinks are unidirectional. I.e., 
 videolink nodes on downstream devices (e.g., the bridge) reference 
 phandles of upstream nodes (e.g., sensors), but not the other way round. 
 This should be sufficient for the proposed probing method:
(a) external subdevices like sensors are children on their respective 
busses (e.g., I2C) and can be probed before the bridge. In this 
case probing can fail, because the master clock is not supplied 
yet. Therefore the sensor driver will have to request deferred 
probing.
(b) the bridge device is probed, the driver instantiates the clock, 
before returning the driver registers a notifier (in this case on 
the I2C bus)
(c) sensor .probe() is tried again, this time the clock is available, 
so, this time probing succeeds
(d) the bridge driver notifier is called, it scans videolink child 
nodes, finds a match, gets a link to the subdevice
 
 6. In the below example we are using the reg property to enumerate 
 videolink child nodes. Doubts have been expressed previous in thread [1] 
 about validity of such use. If it's proven, that reg shouldn't be used 
 in this case, a new property shall be introduced.
 
 7. Concerning data lines. We have chosen to use bus-width and 
 data-shift for parallel busses and new properties data-lanes and 
 clock-lanes to describe pin assignment on MIPI CSI-2 devices and 
 additionally a bus-width property per videolink child of CSI-2 
 controllers to specify how many data lanes are actually used for this 
 link.
 
 Any comments welcome.
 
 It's been a pleasure working on this together with Sylwester, it is a pity 
 he won't be coming to the KS this time, hopefully, we'll continue this 
 cooperation during upcoming discussion and implementation phases.
 
 [1] 
 http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/50755
 [2] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/11143
 
 Thanks
 Guennadi
 ---
 Guennadi Liakhovetski, Ph.D.
 Freelance Open-Source Software Developer
 http://www.open-technology.de/
 
 // Describe an imaginary configuration: a CEU serving either a parallel ov7725
 // sensor, or a serial imx074 sensor, connected over a CSI-2 SoC interface
 //
 // Any vendor-specific properties here are only provided as examples. The
 // 

Re: [PATCH 2/2] [media] ds3000: properly report firmware loading issues

[Rémi Cardona]

Hi Antti,

On 08/30/2012 03:39 PM, Antti Palosaari wrote:
 As I understand firmware downloading failure is coming from the fact
 that register read fails = fails to detect if firmware is already
 running or not.

Well we actually see 2 cases:

 - the register read failure (when ds3000_readreg() returns negative
values). This case is fairly rare, and no changes we've done to the
driver allowed us to make those cards work.

 - the register read returning 0. Looking at the current code, it looks
like the 0xb2 register is supposed to mean that a firmware is loaded.
This case is fairly common: we've had many cards randomly saying that a
firmware was loaded when none had been. Often, a simple reboot will do
the trick. But sometimes, forcing the firmware upload (ie, bypassing the
0xb2 register check) allows the stubborn cards to function properly.

 Original behavior to expect firmware is loaded and running when register
 read fails is very stupid and your fix seems much better.

Well, this patch should not really change the behavior. It just
propagates register read errors to ds3000_initfe(). It'll just fail earlier.

 So first priority should be try fix that issue with register read. Is it
 coming from the USB stack (eg. error 110 timeout) or some other error
 coming from the fact chip answers wrong?

The cards we're using are PCIe (and not the ones with an embedded USB
controller).

 Do you see other register I/O failing too?

I'll see if I can get you an answer for that, since the cards are
shipped with the appliance we send to our customers. Remote debugging is
somewhat tricky.

 Does adding few usec sleep help?

I'm not quite sure where to add those sleeps. In the register
reading/writing functions? 10us? 100us?

Many thanks

Rémi
--
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 2/2] [media] ds3000: properly report firmware loading issues

[Antti Palosaari]

On 08/30/2012 06:21 PM, Rémi Cardona wrote:

Hi Antti,

On 08/30/2012 03:39 PM, Antti Palosaari wrote:

As I understand firmware downloading failure is coming from the fact
that register read fails = fails to detect if firmware is already
running or not.


Well we actually see 2 cases:

  - the register read failure (when ds3000_readreg() returns negative
values). This case is fairly rare, and no changes we've done to the
driver allowed us to make those cards work.


hmm, looks like ds3000_readreg() logic is still a little bit broken. It 
checks count of sent messages and compares it to 2. But if I2C-adapter 
sends only 1 message or 3 (which should not be possible) function return 
that count instead of -EREMOTEIO. OK, quite rare situation, but one 
point more to fail if I2C-adapter has also bug.


But that happens for return value 0 too. Could it be the issue? 
I2C-adapter returns 0 for some reason? Bug in I2C-adapter with bug in 
ds3000_readreg() implementation?



  - the register read returning 0. Looking at the current code, it looks
like the 0xb2 register is supposed to mean that a firmware is loaded.
This case is fairly common: we've had many cards randomly saying that a
firmware was loaded when none had been. Often, a simple reboot will do
the trick. But sometimes, forcing the firmware upload (ie, bypassing the
0xb2 register check) allows the stubborn cards to function properly.


Original behavior to expect firmware is loaded and running when register
read fails is very stupid and your fix seems much better.


Well, this patch should not really change the behavior. It just
propagates register read errors to ds3000_initfe(). It'll just fail earlier.


So first priority should be try fix that issue with register read. Is it
coming from the USB stack (eg. error 110 timeout) or some other error
coming from the fact chip answers wrong?


The cards we're using are PCIe (and not the ones with an embedded USB
controller).


The idea of my question was to ask where those errors are coming from (I 
spoke mistakenly about USB because I usually play with USB devices).


You basically see two different kind of errors, 1) bus communication 
fails, eg. usb timeouts. 2) chips returns error status. Later cases the 
error could come from the this could come from the firmware if chip uses 
firmware or from the silicon. It could be from the I2C-adapter firmware.



Do you see other register I/O failing too?


I'll see if I can get you an answer for that, since the cards are
shipped with the appliance we send to our customers. Remote debugging is
somewhat tricky.


Does adding few usec sleep help?


I'm not quite sure where to add those sleeps. In the register
reading/writing functions? 10us? 100us?


Add sleep after the each operation. Good place to add sleep is 
I2C-adapter. I2C-adapters usually supports two different operations, 
write and read + write using repeated START condition. Former us used 
typically for register write and later for register read.


500us is good choice. If it is only that one register read which causes 
problems, how about repeating it?




Many thanks

Rémi




--
http://palosaari.fi/
--
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

[PATCHv3 3/9] ir-rx51: Trivial fixes

[Timo Kokkonen]

-Fix typo

-Change pwm_timer_num type to match type in platform data

-Remove extra parenthesis

-Replace magic constant with proper bit defintions

-Remove duplicate exit pointer

Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
---
 drivers/media/rc/Kconfig   |  2 +-
 drivers/media/rc/ir-rx51.c | 10 ++
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 093982b..4a68014 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -278,7 +278,7 @@ config IR_RX51
   Say Y or M here if you want to enable support for the IR
   transmitter diode built in the Nokia N900 (RX51) device.
 
-  The driver uses omap DM timers for gereating the carrier
+  The driver uses omap DM timers for generating the carrier
   wave and pulses.
 
 config RC_LOOPBACK
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index e2db94e..125d4c3 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -59,7 +59,7 @@ struct lirc_rx51 {
int wbuf[WBUF_LEN];
int wbuf_index;
unsigned long   device_is_open;
-   unsigned intpwm_timer_num;
+   int pwm_timer_num;
 };
 
 static void lirc_rx51_on(struct lirc_rx51 *lirc_rx51)
@@ -138,11 +138,14 @@ static irqreturn_t lirc_rx51_interrupt_handler(int irq, 
void *ptr)
if (!retval)
return IRQ_NONE;
 
-   if ((retval  ~OMAP_TIMER_INT_MATCH))
+   if (retval  ~OMAP_TIMER_INT_MATCH)
dev_err_ratelimited(lirc_rx51-dev,
: Unexpected interrupt source: %x\n, retval);
 
-   omap_dm_timer_write_status(lirc_rx51-pulse_timer, 7);
+   omap_dm_timer_write_status(lirc_rx51-pulse_timer,
+   OMAP_TIMER_INT_MATCH|
+   OMAP_TIMER_INT_OVERFLOW |
+   OMAP_TIMER_INT_CAPTURE);
if (lirc_rx51-wbuf_index  0) {
dev_err_ratelimited(lirc_rx51-dev,
: BUG wbuf_index has value of %i\n,
@@ -489,7 +492,6 @@ struct platform_driver lirc_rx51_platform_driver = {
.remove = __exit_p(lirc_rx51_remove),
.suspend= lirc_rx51_suspend,
.resume = lirc_rx51_resume,
-   .remove = __exit_p(lirc_rx51_remove),
.driver = {
.name   = DRIVER_NAME,
.owner  = THIS_MODULE,
-- 
1.7.12

--
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

[PATCHv3 1/9] ir-rx51: Adjust dependencies

[Timo Kokkonen]

Although this kind of IR diode circuitry is known to exist only in
N900 hardware, nothing prevents making similar circuitry on any OMAP
based board. The MACH_NOKIA_RX51 dependency is thus not something we
want to be there.

Also, this should depend on LIRC as it is a LIRC driver.

Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
---
 drivers/media/rc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index ffef8b4..093982b 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -273,7 +273,7 @@ config IR_IGUANA
 
 config IR_RX51
tristate Nokia N900 IR transmitter diode
-   depends on MACH_NOKIA_RX51  OMAP_DM_TIMER
+   depends on OMAP_DM_TIMER  LIRC
---help---
   Say Y or M here if you want to enable support for the IR
   transmitter diode built in the Nokia N900 (RX51) device.
-- 
1.7.12

--
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

[PATCHv3 2/9] ir-rx51: Handle signals properly

[Timo Kokkonen]

The lirc-dev expects the ir-code to be transmitted when the write call
returns back to the user space. We should not leave TX ongoing no
matter what is the reason we return to the user space. Easiest
solution for that is to simply remove interruptible sleeps.

The first wait_event_interruptible is thus replaced with return -EBUSY
in case there is still ongoing transfer. This should suffice as the
concept of sending multiple codes in parallel does not make sense.

The second wait_event_interruptible call is replaced with
wait_even_timeout with a fixed and safe timeout that should prevent
the process from getting stuck in kernel for too long.

Also, from now on we will force the TX to stop before we return from
write call. If the TX happened to time out for some reason, we should
not leave the HW transmitting anything.

Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
---
 drivers/media/rc/ir-rx51.c | 39 ---
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 9487dd3..e2db94e 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51 *lirc_rx51)
  OMAP_TIMER_TRIGGER_NONE);
 }
 
+static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
+{
+   if (lirc_rx51-wbuf_index  0)
+   return;
+
+   lirc_rx51_off(lirc_rx51);
+   lirc_rx51-wbuf_index = -1;
+   omap_dm_timer_stop(lirc_rx51-pwm_timer);
+   omap_dm_timer_stop(lirc_rx51-pulse_timer);
+   omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer, 0);
+   wake_up(lirc_rx51-wqueue);
+}
+
 static int init_timing_params(struct lirc_rx51 *lirc_rx51)
 {
u32 load, match;
@@ -160,13 +173,7 @@ static irqreturn_t lirc_rx51_interrupt_handler(int irq, 
void *ptr)
 
return IRQ_HANDLED;
 end:
-   /* Stop TX here */
-   lirc_rx51_off(lirc_rx51);
-   lirc_rx51-wbuf_index = -1;
-   omap_dm_timer_stop(lirc_rx51-pwm_timer);
-   omap_dm_timer_stop(lirc_rx51-pulse_timer);
-   omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer, 0);
-   wake_up_interruptible(lirc_rx51-wqueue);
+   lirc_rx51_stop_tx(lirc_rx51);
 
return IRQ_HANDLED;
 }
@@ -246,8 +253,9 @@ static ssize_t lirc_rx51_write(struct file *file, const 
char *buf,
if ((count  WBUF_LEN) || (count % 2 == 0))
return -EINVAL;
 
-   /* Wait any pending transfers to finish */
-   wait_event_interruptible(lirc_rx51-wqueue, lirc_rx51-wbuf_index  0);
+   /* We can have only one transmit at a time */
+   if (lirc_rx51-wbuf_index = 0)
+   return -EBUSY;
 
if (copy_from_user(lirc_rx51-wbuf, buf, n))
return -EFAULT;
@@ -273,9 +281,18 @@ static ssize_t lirc_rx51_write(struct file *file, const 
char *buf,
 
/*
 * Don't return back to the userspace until the transfer has
-* finished
+* finished. However, we wish to not spend any more than 500ms
+* in kernel. No IR code TX should ever take that long.
+*/
+   i = wait_event_timeout(lirc_rx51-wqueue, lirc_rx51-wbuf_index  0,
+   HZ / 2);
+
+   /*
+* Ensure transmitting has really stopped, even if the timers
+* went mad or something else happened that caused it still
+* sending out something.
 */
-   wait_event_interruptible(lirc_rx51-wqueue, lirc_rx51-wbuf_index  0);
+   lirc_rx51_stop_tx(lirc_rx51);
 
/* We can sleep again */
lirc_rx51-pdata-set_max_mpu_wakeup_lat(lirc_rx51-dev, -1);
-- 
1.7.12

--
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

[PATCHv3 7/9] ir-rx51: Convert latency constraints to PM QoS API

[Timo Kokkonen]

Convert the driver from the obsolete omap_pm_set_max_mpu_wakeup_lat
API to the new PM QoS API. This allows the callback to be removed from
the platform data structure.

The latency requirements are also adjusted to prevent the MPU from
going into sleep mode. This is needed as the GP timers have no means
to wake up the MPU once it has gone into sleep. The side effect is
that from now on the driver actually works even if there is no
background load keeping the MPU awake.

Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
Acked-by: Tony Lindgren t...@atomide.com
Acked-by: Jean Pihet j-pi...@ti.com
---
 arch/arm/mach-omap2/board-rx51-peripherals.c |  2 --
 drivers/media/rc/ir-rx51.c   | 17 -
 include/media/ir-rx51.h  |  2 --
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c 
b/arch/arm/mach-omap2/board-rx51-peripherals.c
index ca07264..e0750cb 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -34,7 +34,6 @@
 #include plat/gpmc.h
 #include plat/onenand.h
 #include plat/gpmc-smc91x.h
-#include plat/omap-pm.h
 
 #include mach/board-rx51.h
 
@@ -1227,7 +1226,6 @@ static void __init rx51_init_tsc2005(void)
 
 #if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
 static struct lirc_rx51_platform_data rx51_lirc_data = {
-   .set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
.pwm_timer = 9, /* Use GPT 9 for CIR */
 };
 
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 6e1ffa6..96ed23d 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -25,6 +25,7 @@
 #include linux/platform_device.h
 #include linux/sched.h
 #include linux/wait.h
+#include linux/pm_qos.h
 
 #include plat/dmtimer.h
 #include plat/clock.h
@@ -49,6 +50,7 @@ struct lirc_rx51 {
struct omap_dm_timer *pulse_timer;
struct device*dev;
struct lirc_rx51_platform_data *pdata;
+   struct pm_qos_request   pm_qos_request;
wait_queue_head_t wqueue;
 
unsigned long   fclk_khz;
@@ -268,10 +270,16 @@ static ssize_t lirc_rx51_write(struct file *file, const 
char *buf,
lirc_rx51-wbuf[count] = -1; /* Insert termination mark */
 
/*
-* Adjust latency requirements so the device doesn't go in too
-* deep sleep states
+* If the MPU is going into too deep sleep state while we are
+* transmitting the IR code, timers will not be able to wake
+* up the MPU. Thus, we need to set a strict enough latency
+* requirement in order to ensure the interrupts come though
+* properly. The 10us latency requirement is low enough to
+* keep MPU from sleeping and thus ensures the interrupts are
+* getting through properly.
 */
-   lirc_rx51-pdata-set_max_mpu_wakeup_lat(lirc_rx51-dev, 50);
+   pm_qos_add_request(lirc_rx51-pm_qos_request,
+   PM_QOS_CPU_DMA_LATENCY, 10);
 
lirc_rx51_on(lirc_rx51);
lirc_rx51-wbuf_index = 1;
@@ -292,8 +300,7 @@ static ssize_t lirc_rx51_write(struct file *file, const 
char *buf,
 */
lirc_rx51_stop_tx(lirc_rx51);
 
-   /* We can sleep again */
-   lirc_rx51-pdata-set_max_mpu_wakeup_lat(lirc_rx51-dev, -1);
+   pm_qos_remove_request(lirc_rx51-pm_qos_request);
 
return n;
 }
diff --git a/include/media/ir-rx51.h b/include/media/ir-rx51.h
index 104aa89..57523f2 100644
--- a/include/media/ir-rx51.h
+++ b/include/media/ir-rx51.h
@@ -3,8 +3,6 @@
 
 struct lirc_rx51_platform_data {
int pwm_timer;
-
-   int(*set_max_mpu_wakeup_lat)(struct device *dev, long t);
 };
 
 #endif
-- 
1.7.12

--
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

[PATCHv3 8/9] ir-rx51: Remove useless variable from struct lirc_rx51

[Timo Kokkonen]

As clearly visible from the patch, this variable has no useful purpose
what so ever. Thus, it can be removed altogether without any side
effects.

Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
---
 drivers/media/rc/ir-rx51.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 96ed23d..edb1562 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -57,7 +57,6 @@ struct lirc_rx51 {
unsigned intfreq;   /* carrier frequency */
unsigned intduty_cycle; /* carrier duty cycle */
unsigned intirq_num;
-   unsigned intmatch;
int wbuf[WBUF_LEN];
int wbuf_index;
unsigned long   device_is_open;
@@ -102,8 +101,6 @@ static int init_timing_params(struct lirc_rx51 *lirc_rx51)
omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer, 0);
omap_dm_timer_start(lirc_rx51-pulse_timer);
 
-   lirc_rx51-match = 0;
-
return 0;
 }
 
@@ -113,11 +110,7 @@ static int pulse_timer_set_timeout(struct lirc_rx51 
*lirc_rx51, int usec)
 
BUG_ON(usec  0);
 
-   if (lirc_rx51-match == 0)
-   counter = omap_dm_timer_read_counter(lirc_rx51-pulse_timer);
-   else
-   counter = lirc_rx51-match;
-
+   counter = omap_dm_timer_read_counter(lirc_rx51-pulse_timer);
counter += (u32)(lirc_rx51-fclk_khz * usec / (1000));
omap_dm_timer_set_match(lirc_rx51-pulse_timer, 1, counter);
omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer,
-- 
1.7.12

--
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

[PATCHv3 9/9] ir-rx51: Add missing quote mark in Kconfig text

[Timo Kokkonen]

This trivial fix cures the following warning message:

drivers/media/rc/Kconfig:275:warning: multi-line strings not supported

Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
---
 drivers/media/rc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 4a68014..1300655 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -272,7 +272,7 @@ config IR_IGUANA
   be called iguanair.
 
 config IR_RX51
-   tristate Nokia N900 IR transmitter diode
+   tristate Nokia N900 IR transmitter diode
depends on OMAP_DM_TIMER  LIRC
---help---
   Say Y or M here if you want to enable support for the IR
-- 
1.7.12

--
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

[PATCHv3 6/9] ir-rx51: Replace module_{init,exit} macros with module_platform_driver

[Timo Kokkonen]

No reason to avoid using the existing helpers.

Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
---
 drivers/media/rc/ir-rx51.c | 12 +---
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 16b3c1f..6e1ffa6 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -495,17 +495,7 @@ struct platform_driver lirc_rx51_platform_driver = {
},
 };
 
-static int __init lirc_rx51_init(void)
-{
-   return platform_driver_register(lirc_rx51_platform_driver);
-}
-module_init(lirc_rx51_init);
-
-static void __exit lirc_rx51_exit(void)
-{
-   platform_driver_unregister(lirc_rx51_platform_driver);
-}
-module_exit(lirc_rx51_exit);
+module_platform_driver(lirc_rx51_platform_driver);
 
 MODULE_DESCRIPTION(LIRC TX driver for Nokia RX51);
 MODULE_AUTHOR(Nokia Corporation);
-- 
1.7.12

--
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 v2] media: v4l2-ctrls: add control for dpcm predictor

[Sakari Ailus]

Hi Hans and Prabhakar,

Hans Verkuil wrote:

Hi Prabhakar!

I've got some documentation review comments below...

On Thu August 30 2012 00:58:16 Prabhakar Lad wrote:

From: Lad, Prabhakar prabhakar@ti.com

add V4L2_CID_DPCM_PREDICTOR control of type menu, which
determines the dpcm predictor. The predictor can be either
simple or advanced.

Signed-off-by: Lad, Prabhakar prabhakar@ti.com
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Cc: Sakari Ailus sakari.ai...@iki.fi
Cc: Hans Verkuil hans.verk...@cisco.com
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: Sylwester Nawrocki s.nawro...@samsung.com
Cc: Hans de Goede hdego...@redhat.com
Cc: Kyungmin Park kyungmin.p...@samsung.com
---
This patches has one checkpatch warning for line over
80 characters altough it can be avoided I have kept it
for consistency.

Changes for v2:
1: Added documentaion in controls.xml pointed by Sylwester.
2: Chnaged V4L2_DPCM_PREDICTOR_ADVANCE to V4L2_DPCM_PREDICTOR_ADVANCED
pointed by Sakari.

  Documentation/DocBook/media/v4l/controls.xml |   25 -
  drivers/media/v4l2-core/v4l2-ctrls.c |9 +
  include/linux/videodev2.h|5 +
  3 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/controls.xml 
b/Documentation/DocBook/media/v4l/controls.xml
index 93b9c68..84746d0 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -4267,7 +4267,30 @@ interface and may change in the future./para
pixels / second.
/entry
  /row
- rowentry/entry/row
+ row
+   entry 
spanname=idconstantV4L2_CID_DPCM_PREDICTOR/constant/entry
+   entrymenu/entry
+ /row
+ row id=v4l2-dpcm-predictor
+   entry spanname=descr DPCM Predictor: depicts what type of 
prediction
+   is used simple or advanced.


This is not useful information. It basically just rephrases the name of the
define without actually telling me anything.

I would expect to see here at least the following:

- what the DPCM abbreviation stands for
- a link or bibliography reference to the relevant standard (if there is any)


There's a Wikipedia article:

URL:http://en.wikipedia.org/wiki/Differential_pulse-code_modulation

It's the same DPCM encoding as in many of the compressed raw bayer formats.


- a high-level explanation of what this do and what the difference is between
  simple and advanced.


That's somewhat subjective and hardware dependent. If I understand this 
correctly, the advanced should differ only quality-wise from simple 
option. Why one would use simple instead of advanced then? Perhaps 
mostly for testing purposes; it might be that the advanced predictor 
could have issues in certain cases where the simple would not. However 
this isn't the only piece of hardware where I see that this is 
configurable, and the simple one was even the default.



If this is part of a video compression standard, then this control would 
probably
belong to the MPEG control class as well.


It's not --- DPCM compression is typically used on the bus between the 
sensor and the receiver to compress the data as the bus is often a 
limiting factor in the transfer rate. DPCM compression can be used to 
squeeze the data from 10 to 8 bits without much loss in quality or 
dynamic range.


Cheers,

--
Sakari Ailus
sakari.ai...@iki.fi

--
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

[PATCHv3 5/9] ir-rx51: Move platform data checking into probe function

[Timo Kokkonen]

This driver is useless without proper platform data. If data is not
available, we should not register the driver at all. Once this check
is done, the BUG_ON check during device open is no longer needed.

Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
---
 drivers/media/rc/ir-rx51.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index f22e5e4..16b3c1f 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -378,7 +378,6 @@ static long lirc_rx51_ioctl(struct file *filep,
 static int lirc_rx51_open(struct inode *inode, struct file *file)
 {
struct lirc_rx51 *lirc_rx51 = lirc_get_pdata(file);
-   BUG_ON(!lirc_rx51);
 
file-private_data = lirc_rx51;
 
@@ -458,6 +457,9 @@ static int lirc_rx51_resume(struct platform_device *dev)
 
 static int __devinit lirc_rx51_probe(struct platform_device *dev)
 {
+   if (!dev-dev.platform_data)
+   return -ENODEV;
+
lirc_rx51_driver.features = LIRC_RX51_DRIVER_FEATURES;
lirc_rx51.pdata = dev-dev.platform_data;
lirc_rx51.pwm_timer_num = lirc_rx51.pdata-pwm_timer;
-- 
1.7.12

--
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

[PATCHv3 0/9] Fixes in response to review comments

[Timo Kokkonen]

These patches fix most of the issues pointed out in the patch review
by Sean Young and Sakari Ailus.

The most noticeable change after these patch set is that the IR
transmission no longer times out even if the timers are not waking up
the MPU as it should be. Now that Jean Pihet kindly instructed me how
to use the PM QoS API for setting the latency constraints, the driver
will now work without any background load. Someone might consider such
restriction a blocker bug, that is fixed on this patch set.

Changes since v2:

- The 10us PM QoS latency requrement is documented in the code

- A missing quote mark is added into the Kconfig text

Changes since v1:

- Replace wake_up_interruptible with wake_up, as the driver is having
  non-interruptible sleeps

- Instead of just removing the set_max_mpu_wakeup_lat calls, replace
  them with QoS API calls

Timo Kokkonen (9):
  ir-rx51: Adjust dependencies
  ir-rx51: Handle signals properly
  ir-rx51: Trivial fixes
  ir-rx51: Clean up timer initialization code
  ir-rx51: Move platform data checking into probe function
  ir-rx51: Replace module_{init,exit} macros with
module_platform_driver
  ir-rx51: Convert latency constraints to PM QoS API
  ir-rx51: Remove useless variable from struct lirc_rx51
  ir-rx51: Add missing quote mark in Kconfig text

 arch/arm/mach-omap2/board-rx51-peripherals.c |   2 -
 drivers/media/rc/Kconfig |   6 +-
 drivers/media/rc/ir-rx51.c   | 102 ++-
 include/media/ir-rx51.h  |   2 -
 4 files changed, 57 insertions(+), 55 deletions(-)

-- 
1.7.12

--
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

[PATCHv3 4/9] ir-rx51: Clean up timer initialization code

[Timo Kokkonen]

Remove a redundant macro definition. This is unneeded and becomes more
readable once the actual timer code is refactored a little.

Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
---
 drivers/media/rc/ir-rx51.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 125d4c3..f22e5e4 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -105,11 +105,9 @@ static int init_timing_params(struct lirc_rx51 *lirc_rx51)
return 0;
 }
 
-#define tics_after(a, b) ((long)(b) - (long)(a)  0)
-
 static int pulse_timer_set_timeout(struct lirc_rx51 *lirc_rx51, int usec)
 {
-   int counter;
+   int counter, counter_now;
 
BUG_ON(usec  0);
 
@@ -122,11 +120,8 @@ static int pulse_timer_set_timeout(struct lirc_rx51 
*lirc_rx51, int usec)
omap_dm_timer_set_match(lirc_rx51-pulse_timer, 1, counter);
omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer,
 OMAP_TIMER_INT_MATCH);
-   if (tics_after(omap_dm_timer_read_counter(lirc_rx51-pulse_timer),
-  counter)) {
-   return 1;
-   }
-   return 0;
+   counter_now = omap_dm_timer_read_counter(lirc_rx51-pulse_timer);
+   return (counter - counter_now)  0;
 }
 
 static irqreturn_t lirc_rx51_interrupt_handler(int irq, void *ptr)
-- 
1.7.12

--
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

cron job: media_tree daily build: WARNINGS

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:Thu Aug 30 19:00:21 CEST 2012
git hash:79e8c7bebb467bbc3f2514d75bba669a3f354324
gcc version:  i686-linux-gcc (GCC) 4.7.1
host hardware:x86_64
host os:  3.4.07-marune

linux-git-arm-eabi-davinci: WARNINGS
linux-git-arm-eabi-exynos: OK
linux-git-arm-eabi-omap: WARNINGS
linux-git-i686: WARNINGS
linux-git-m32r: WARNINGS
linux-git-mips: WARNINGS
linux-git-powerpc64: WARNINGS
linux-git-x86_64: WARNINGS
linux-2.6.31.12-x86_64: WARNINGS
linux-2.6.32.6-x86_64: WARNINGS
linux-2.6.33-x86_64: WARNINGS
linux-2.6.34-x86_64: WARNINGS
linux-2.6.35.3-x86_64: WARNINGS
linux-2.6.36-x86_64: WARNINGS
linux-2.6.37-x86_64: WARNINGS
linux-2.6.38.2-x86_64: WARNINGS
linux-2.6.39.1-x86_64: WARNINGS
linux-3.0-x86_64: WARNINGS
linux-3.1-x86_64: WARNINGS
linux-3.2.1-x86_64: WARNINGS
linux-3.3-x86_64: WARNINGS
linux-3.4-x86_64: WARNINGS
linux-3.5-x86_64: WARNINGS
linux-3.6-rc2-x86_64: WARNINGS
linux-2.6.31.12-i686: WARNINGS
linux-2.6.32.6-i686: WARNINGS
linux-2.6.33-i686: WARNINGS
linux-2.6.34-i686: WARNINGS
linux-2.6.35.3-i686: WARNINGS
linux-2.6.36-i686: WARNINGS
linux-2.6.37-i686: WARNINGS
linux-2.6.38.2-i686: WARNINGS
linux-2.6.39.1-i686: WARNINGS
linux-3.0-i686: WARNINGS
linux-3.1-i686: WARNINGS
linux-3.2.1-i686: WARNINGS
linux-3.3-i686: WARNINGS
linux-3.4-i686: WARNINGS
linux-3.5-i686: WARNINGS
linux-3.6-rc2-i686: WARNINGS
apps: WARNINGS
spec-git: OK
sparse: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Thursday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2

The V4L-DVB specification from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html
--
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 0/8] rc-core: patches for 3.7

[Jarod Wilson]

On Sat, Aug 25, 2012 at 11:46:47PM +0200, David Härdeman wrote:
 This is two minor winbond-cir fixes as well as the first six patches
 from my previous patchbomb.
 
 The latter have been modified so that backwards compatibility is retained
 as much as possible (the format of the sysfs files do not change for
 example).

I've read through the set, and it all seems to make sense to me, but I
haven't actually tried it out with any of the hardware I've got. I assume
its been tested on various other hardware though.

Side note: my life has been turned a wee bit upside down, been busy
dealing with some fairly big changes, and that's still ongoing, thus the
relative lack of repsonsiveness on, well, anything, lately.


-- 
Jarod Wilson
ja...@redhat.com

--
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: [RFC v4] V4L DT bindings

[Sylwester Nawrocki]

On 08/30/2012 05:19 PM, Nicolas THERY wrote:
  i2c0: i2c@0xfff2 {
  ...
  ov772x_1: ov772x@0x21 {
  compatible = omnivision,ov772x;
  reg =0x21;
  vddio-supply =regulator1;
  vddcore-supply =regulator2;
  bus-width =10;

  clock-frequency =2000;
  clocks =mclk 0;
  clock-names = mclk

  #address-cells =1;
  #size-cells =0;
  ...
  ceu0_1: videolink@0 {
  reg =0;   /* link configuration 
 to local pad #0 */
  bus-width =8;
  hsync-active =1;
  hsync-active =0;  /* who came up with an 
 inverter here?... */
  pclk-sample =1;
  };
  };

  imx074: imx074@0x1a {
  compatible = sony,imx074;
  reg =0x1a;
  vddio-supply =regulator1;
  vddcore-supply =regulator2;
  clock-lanes =0;
  data-lanes =1,2;

  clock-frequency =3000;/* shared clock with 
 ov772x_1 */
  clocks =mclk 0;
  clock-names = mclk

  #address-cells =1;
  #size-cells =0;
  ...
  csi2_0_1: videolink@0 {
  reg =0;   /* link configuration 
 to local pad #0 */
  bus-width =2; /* 2 lanes, fixed roles, also 
 described above */
  };
  };
  ...
  };

  csi2: csi2@0xffc9 {
  compatible = renesas,sh-mobile-csi2;
  reg =0xffc9 0x1000;
  interrupts =0x17a0;
  #address-cells =1;
  #size-cells =0;

  /* Ok to have them global? */

I'm not sure, maybe it's better to move it under videolink@1 node,
to keep it together with 'bus-width' property ?

  clock-lanes =0;
  data-lanes =2,1;
 
 In imx074@0x1a above, the data-lanes property is1,2.  Is it
 reversed here to show that lanes are swapped between the sensor and the
 CSI rx?  If not, how to express lane swapping?

Yes, this indicates lanes remapping at the receiver.

Probably we could make it a single value with length determined by
'bus-width', since we're going to use 'bus-width' for CSI buses as well, 
(optionally) in addition to 'clock-lanes' and 'data-lanes' ?

  ...
  imx074_1: videolink@1 {
  reg =1;
  client =imx074 0;
  bus-width =2;

  csi2-ecc;
  csi2-crc;

  renesas,csi2-phy =0;
  };
  ceu0: videolink@0 {
  reg =0;
  immutable;
  };
  };
 
 How to express that the positive and negative signals of a given
 clock/data lane are inversed?  Is it somehow with the hsync-active
 property?

Hmm, I don't think this is covered in this RFC. hsync-active is mostly
intended for the parallel buses. We need to come up with new properties
to handle CSI data/clock lane polarity swapping. There was a short
discussion about that already:
http://www.mail-archive.com/linux-media@vger.kernel.org/msg41724.html

 Actually there may be two positive/negative inversion cases to consider:
 
 - the positive/negative signals are inversed both in low-power and
high-speed modes (e.g. physical lines between sensor module and SoC
are swapped on the PCB);
 
 - the positive/negative signals are inversed in high-speed mode only
(the sensor and CSI rx use opposite polarities in high-speed mode).

Then is this positive/negative LVDS lines swapping separately configurable
in hardware for low-power and high-speed mode ? What is an advantage of it ?

One possible solution would be to have a one to two elements array property,
e.g.

lanes-polarity = 0 0 0 0 0, 1 1 1 1 1;

where the first entry would indicate lanes polarity for high speed mode and
the second one for low power mode. For receivers/transmitters that don't
allow to configure the polarities separately for different bus states there
could be just one entry. The width of each element could be determined by 
value of the 'bus-width' property + 1.

Would it make sense ?

--

Regards,
Sylwester
--
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

V4L DT @ plumbers (was Re: [RFC v4] V4L DT bindings)

[Guennadi Liakhovetski]

Hi all

Do we still want to try to organise a short discussion of this while at 
plumbers? Maybe tomorrow during or around lunch? Or any other time for 
that matter?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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: V4L DT @ plumbers (was Re: [RFC v4] V4L DT bindings)

[Laurent Pinchart]

Hi Guennadi,

On Thursday 30 August 2012 22:58:17 Guennadi Liakhovetski wrote:
 Hi all
 
 Do we still want to try to organise a short discussion of this while at
 plumbers? Maybe tomorrow during or around lunch? Or any other time for
 that matter?

I'm certainly interested. I have a meeting tomorrow at a still unknown type, 
apart from that I'm free.

-- 
Regards,

Laurent Pinchart

--
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: V4L DT @ plumbers (was Re: [RFC v4] V4L DT bindings)

[Guennadi Liakhovetski]

On Fri, 31 Aug 2012, Laurent Pinchart wrote:

 Hi Guennadi,
 
 On Thursday 30 August 2012 22:58:17 Guennadi Liakhovetski wrote:
  Hi all
  
  Do we still want to try to organise a short discussion of this while at
  plumbers? Maybe tomorrow during or around lunch? Or any other time for
  that matter?
 
 I'm certainly interested. I have a meeting tomorrow at a still unknown type, 
 apart from that I'm free.

How about beginning tomorrow at 11:55 (Nautilus 3  4 are still free 
according to the schedule) and then continuing after the lunch for another 
hour if needed (first talk in the afternoon tomorrow is at 14:10).

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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: V4L DT @ plumbers (was Re: [RFC v4] V4L DT bindings)

[Hans Verkuil]

Fine by me!

  Hans

Guennadi Liakhovetski g.liakhovet...@gmx.de wrote:

On Fri, 31 Aug 2012, Laurent Pinchart wrote:

 Hi Guennadi,
 
 On Thursday 30 August 2012 22:58:17 Guennadi Liakhovetski wrote:
  Hi all
  
  Do we still want to try to organise a short discussion of this while at
  plumbers? Maybe tomorrow during or around lunch? Or any other time for
  that matter?
 
 I'm certainly interested. I have a meeting tomorrow at a still unknown type, 
 apart from that I'm free.

How about beginning tomorrow at 11:55 (Nautilus 3  4 are still free 
according to the schedule) and then continuing after the lunch for another 
hour if needed (first talk in the afternoon tomorrow is at 14:10).

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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