[GIT PULL FOR v3.8] Regression fix: cx18/ivtv: remove __init from a non-init function.

2013-02-08 Thread Hans Verkuil
Mauro,

Please fast-track this for 3.8. Yesterday I discovered that commits made earlier
for 3.8 kill ivtv and cx18 (as in: unable to boot, instant crash) since a
function was made __init that was actually called *after* initialization.

We are already at rc6 and this *must* make it for 3.8. Without this patch
anyone with a cx18/ivtv will crash immediately as soon as they upgrade to 3.8.

Regards,

Hans

The following changes since commit 248ac368ce4b3cd36515122d888403909d7a2500:

  [media] s5p-fimc: Fix fimc-lite entities deregistration (2013-02-06 09:42:19 
-0200)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git ivtv

for you to fetch changes up to ddf276062e68607323fca363b99bdf426dddad9b:

  cx18/ivtv: fix regression: remove __init from a non-init function. 
(2013-02-08 09:30:11 +0100)


Hans Verkuil (1):
  cx18/ivtv: fix regression: remove __init from a non-init function.

 drivers/media/pci/cx18/cx18-alsa-main.c |2 +-
 drivers/media/pci/ivtv/ivtv-alsa-main.c |2 +-
 2 files changed, 2 insertions(+), 2 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


Re: [RFC PATCH 1/8] stk-webcam: various fixes.

2013-02-08 Thread Hans Verkuil
On Thu February 7 2013 23:39:58 Arvydas Sidorenko wrote:
 On Wed, Feb 6, 2013 at 8:32 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 
  I've improved v4l2-compliance a bit, but I've also pushed a fix (I hope) to
  my git branch.
 
  It's great if you can test this!
 

Thanks for the testing! I've pushed some more improvements to my git branch.
Hopefully the compliance tests are now running OK. Please check the dmesg
output as well.

In addition I've added an 'upside down' message to the kernel log that tells
me whether the driver is aware that your sensor is upside down or not.

Which laptop do you have? Asus G1?

Regards,

Hans
--
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 v2 0/5] Common Display Framework

2013-02-08 Thread Tomi Valkeinen
On 2013-02-04 12:05, Marcus Lorentzon wrote:

 As discussed at FOSDEM I will give DSI bus with full feature set a
 try.

Please do, but as a reminder I want to raise the issues I see with a DSI
bus:

- A device can be a child of only one bus. So if DSI is used only for
video, the device is a child of, say, i2c bus, and thus there's no DSI
bus. How to configure and use DSI in this case?

- If DSI is used for both control and video, we have two separate APIs
for the bus. What I mean here is that for the video-only case above, we
need a video-only-API for DSI. This API should contain all necessary
methods to configure DSI. But we need similar methods for the control
API also.

So, I hope you come up with some solution for this, but as I see it,
it's easily the most simple and clear option to have one video_source
style entity for the DSI bus itself, which is used for both control and
video.

 Tomi




signature.asc
Description: OpenPGP digital signature


Re: [RFC v2 0/5] Common Display Framework

2013-02-08 Thread Marcus Lorentzon

On 02/08/2013 11:51 AM, Tomi Valkeinen wrote:

On 2013-02-04 12:05, Marcus Lorentzon wrote:


As discussed at FOSDEM I will give DSI bus with full feature set a
try.

Please do, but as a reminder I want to raise the issues I see with a DSI
bus:

- A device can be a child of only one bus. So if DSI is used only for
video, the device is a child of, say, i2c bus, and thus there's no DSI
bus. How to configure and use DSI in this case?

- If DSI is used for both control and video, we have two separate APIs
for the bus. What I mean here is that for the video-only case above, we
need a video-only-API for DSI. This API should contain all necessary
methods to configure DSI. But we need similar methods for the control
API also.

So, I hope you come up with some solution for this, but as I see it,
it's easily the most simple and clear option to have one video_source
style entity for the DSI bus itself, which is used for both control and
video.


Thanks, it is not that I'm totally against the video source stuff. And I 
share your concerns, none of the solutions are perfect. It just doesn't 
feel right to create this dummy source device without investigating 
the DSI bus route. But I will try to write up some history/problem 
description and ask Greg KH for guidance. If he has a strong opinion 
either way, there is not much more to discuss ;)


/BR
/Marcus

--
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] crystalhd git.linuxtv.org kernel driver: Fix PM suspend broken by emergency patches

2013-02-08 Thread thomas schorpp

Fix PM suspend() broken by emergency patches, thanks to Philip Langdale for 
pointing out.

...But PM resume() didn't work anyway with the original code, always err 
invalid args.

Recommended workaround user space PM handling for ACPI S3:

/etc/pm/config.d/00suspend_modules:2:SUSPEND_MODULES=dvb_ttpci crystalhd

/etc/pm/sleep.d/92_crystalhd
#!/bin/sh

#SERVICES=crystalhd

#for service in $SERVICES; do
#services_reverse=$service $services_reverse
#done

case $1 in
hibernate|suspend)
;;
thaw|resume)
echo 1  /sys/devices/pci:00/:00:1c.1/:02:00.0/reset
#echo 1  /sys/devices/pci:00/:00:1c.1/:02:00.0/rescan
esac

and after malfunction with FW Command T/O, etc, crystalhd PCI-E device needs 
the reset up to 3x and module reload.

--

Patch attached.

crystalhd git.linuxtv.org kernel driver: Fix PM suspend broken by emergency 
patches

Signed-off-by: Thomas Schorpp thomas.scho...@gmail.com

y
tom


diff --git a/driver/linux/crystalhd_cmds.c b/driver/linux/crystalhd_cmds.c
index cecd710..cb6e65d 100644
--- a/driver/linux/crystalhd_cmds.c
+++ b/driver/linux/crystalhd_cmds.c
@@ -32,6 +32,11 @@ static struct crystalhd_user *bc_cproc_get_uid(struct crystalhd_cmd *ctx)
 	struct crystalhd_user *user = NULL;
 	int i;
 
+	if (!ctx) {
+		dev_err(chddev(), %s: Invalid Arg\n, __func__);
+		return user;
+	}
+
 	for (i = 0; i  BC_LINK_MAX_OPENS; i++) {
 		if (!ctx-user[i].in_use) {
 			user = ctx-user[i];
@@ -46,6 +51,11 @@ int bc_get_userhandle_count(struct crystalhd_cmd *ctx)
 {
 	int i, count = 0;
 
+	if (!ctx) {
+		dev_err(chddev(), %s: Invalid Arg\n, __func__);
+		return BC_STS_INV_ARG;
+	}
+
 	for (i = 0; i  BC_LINK_MAX_OPENS; i++) {
 		if (ctx-user[i].in_use)
 			count++;
@@ -154,7 +164,7 @@ static BC_STATUS bc_cproc_get_hwtype(struct crystalhd_cmd *ctx, crystalhd_ioctl_
 static BC_STATUS bc_cproc_reg_rd(struct crystalhd_cmd *ctx,
  crystalhd_ioctl_data *idata)
 {
-	if (!ctx || !idata)
+	if (!ctx || !ctx-hw_ctx || !idata)
 		return BC_STS_INV_ARG;
 	idata-udata.u.regAcc.Value = ctx-hw_ctx-pfnReadDevRegister(ctx-adp,
 	idata-udata.u.regAcc.Offset);
@@ -164,7 +174,7 @@ static BC_STATUS bc_cproc_reg_rd(struct crystalhd_cmd *ctx,
 static BC_STATUS bc_cproc_reg_wr(struct crystalhd_cmd *ctx,
  crystalhd_ioctl_data *idata)
 {
-	if (!ctx || !idata)
+	if (!ctx || !ctx-hw_ctx || !idata)
 		return BC_STS_INV_ARG;
 
 	ctx-hw_ctx-pfnWriteDevRegister(ctx-adp, idata-udata.u.regAcc.Offset,
@@ -176,7 +186,7 @@ static BC_STATUS bc_cproc_reg_wr(struct crystalhd_cmd *ctx,
 static BC_STATUS bc_cproc_link_reg_rd(struct crystalhd_cmd *ctx,
   crystalhd_ioctl_data *idata)
 {
-	if (!ctx || !idata)
+	if (!ctx || !ctx-hw_ctx || !idata)
 		return BC_STS_INV_ARG;
 
 	idata-udata.u.regAcc.Value = ctx-hw_ctx-pfnReadFPGARegister(ctx-adp,
@@ -187,7 +197,7 @@ static BC_STATUS bc_cproc_link_reg_rd(struct crystalhd_cmd *ctx,
 static BC_STATUS bc_cproc_link_reg_wr(struct crystalhd_cmd *ctx,
   crystalhd_ioctl_data *idata)
 {
-	if (!ctx || !idata)
+	if (!ctx || !ctx-hw_ctx || !idata)
 		return BC_STS_INV_ARG;
 
 	ctx-hw_ctx-pfnWriteFPGARegister(ctx-adp, idata-udata.u.regAcc.Offset,
@@ -201,7 +211,7 @@ static BC_STATUS bc_cproc_mem_rd(struct crystalhd_cmd *ctx,
 {
 	BC_STATUS sts = BC_STS_SUCCESS;
 
-	if (!ctx || !idata || !idata-add_cdata)
+	if (!ctx || !ctx-hw_ctx || !idata || !idata-add_cdata)
 		return BC_STS_INV_ARG;
 
 	if (idata-udata.u.devMem.NumDwords  (idata-add_cdata_sz / 4)) {
@@ -220,7 +230,7 @@ static BC_STATUS bc_cproc_mem_wr(struct crystalhd_cmd *ctx,
 {
 	BC_STATUS sts = BC_STS_SUCCESS;
 
-	if (!ctx || !idata || !idata-add_cdata)
+	if (!ctx || !ctx-hw_ctx || !idata || !idata-add_cdata)
 		return BC_STS_INV_ARG;
 
 	if (idata-udata.u.devMem.NumDwords  (idata-add_cdata_sz / 4)) {
@@ -307,7 +317,7 @@ static BC_STATUS bc_cproc_download_fw(struct crystalhd_cmd *ctx,
 
 	dev_dbg(chddev(), Downloading FW\n);
 
-	if (!ctx || !idata || !idata-add_cdata || !idata-add_cdata_sz) {
+	if (!ctx || !ctx-hw_ctx || !idata || !idata-add_cdata || !idata-add_cdata_sz) {
 		dev_err(chddev(), %s: Invalid Arg\n, __func__);
 		return BC_STS_INV_ARG;
 	}
@@ -350,7 +360,7 @@ static BC_STATUS bc_cproc_do_fw_cmd(struct crystalhd_cmd *ctx, crystalhd_ioctl_d
 	BC_STATUS sts;
 	uint32_t *cmd;
 
-	if (!(ctx-state  BC_LINK_INIT)) {
+	if ( !ctx || !idata || !(ctx-state  BC_LINK_INIT) || !ctx-hw_ctx) {
 		dev_dbg(dev, Link invalid state do fw cmd %x \n, ctx-state);
 		return BC_STS_ERR_USAGE;
 	}
@@ -395,7 +405,7 @@ static void bc_proc_in_completion(struct crystalhd_dio_req *dio_hnd,
 		return;
 	}
 	if (sts == BC_STS_IO_USER_ABORT || sts == BC_STS_PWR_MGMT)
-		 return;
+		return;
 
 	dio_hnd-uinfo.comp_sts = sts;
 	dio_hnd-uinfo.ev_sts = 1;
@@ -407,6 +417,9 @@ static BC_STATUS bc_cproc_codein_sleep(struct crystalhd_cmd *ctx)
 	wait_queue_head_t sleep_ev;
 	int rc = 0;
 
+	if (!ctx)
+		return BC_STS_INV_ARG;
+
 	if (ctx-state  BC_LINK_SUSPEND)
 		return BC_STS_PWR_MGMT;
 

Re: terratec h5 rev. 3?

2013-02-08 Thread Mauro Carvalho Chehab
Em Mon, 28 Jan 2013 13:02:43 +0100
Roland Scheidegger rscheidegger_li...@hispeed.ch escreveu:

 From: Roland Scheidegger rscheidegger_li...@hispeed.ch
 To: linux-media@vger.kernel.org
 Subject: Re: terratec h5 rev. 3?
 Date: Mon, 28 Jan 2013 13:02:43 +0100
 Sender: linux-media-ow...@vger.kernel.org
 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130105 
 Thunderbird/17.0.2
 
 Am 21.12.2012 06:38, schrieb linux-media-ow...@vger.kernel.org:
  Hi,
  
  I've recently got a terratec h5 for dvb-c and thought it would be
  supported but it looks like it's a newer revision not recognized by em28xx.
  After using the new_id hack it gets recognized and using various htc
  cards (notably h5 or cinergy htc stick, cards 79 and 82 respectively) it
  seems to _nearly_ work but not quite (I was using h5 firmware for the
  older version). Tuning, channel scan works however tv (or dvb radio)
  does not, since it appears the error rate is going through the roof
  (with some imagination it is possible to see some parts of the picture
  sometimes and hear some audio pieces).  
 
 Ok I have received a replacement now and I can confirm this stick in
 fact just works like the same as the older versions (I guess maybe the
 first one had bad solder point?). I can't judge signal sensitivity or
 anything like that (the snr values are rather humorous) but it's
 definitely good enough now with no reception issues. The IR receiver is
 another matter and I was unsuccesful in making it work for now (I guess
 noone got it working with the old versions neither as it lacks the ir
 entries).
 So could this card be added? I've added the trivial patch.

Could you please add your Signed-of-by: to the patch?

Thanks,
Mauro
 
 Roland
 
 
 
 [h5rev3.diff  text/x-patch (549 bytes)] 

-- 

Cheers,
Mauro
--
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 RESEND] media: rc: gpio-ir-recv: add support for device tree parsing

2013-02-08 Thread Mauro Carvalho Chehab
Em Wed, 06 Feb 2013 18:18:22 +0100
Sebastian Hesselbarth sebastian.hesselba...@gmail.com escreveu:

 On 02/06/2013 02:48 PM, Sylwester Nawrocki wrote:
  On 02/06/2013 09:03 AM, Sebastian Hesselbarth wrote:
  This patch adds device tree parsing for gpio_ir_recv platform_data and
  the mandatory binding documentation. It basically follows what we already
  have for e.g. gpio_keys. All required device tree properties are OS
  independent but optional properties allow linux specific support for rc
  protocols and maps.
 
  There was a similar patch sent by Matus Ujhelyi but that discussion
  died after the first reviews.
 
  Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com
  ---
  ...
  diff --git a/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt 
  b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
  new file mode 100644
  index 000..937760c
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
  @@ -0,0 +1,20 @@
  +Device-Tree bindings for GPIO IR receiver
  +
  +Required properties:
  +  - compatible = gpio-ir-receiver;
  +  - gpios: OF device-tree gpio specification.
  +
  +Optional properties:
  +  - linux,allowed-rc-protocols: Linux specific u64 bitmask of allowed
  +  rc protocols.
 
  You likely need to specify in these bindings documentation which bit
  corresponds to which RC protocol.
 
  I'm not very familiar with the RC internals, but why it has to be
  specified statically in the device tree, when decoding seems to be
  mostly software defined ? I might be missing something though..
 
 Sylwester,
 
 I am not familiar with RC internals either. Maybe somebody with more
 insight in media/rc can clarify the specific needs for the rc subsystem.
 I was just transferring the DT support approach taken by gpio_keys to
 gpio_ir_recv as I will be using it on mach-dove/cubox soon.

The allowed rc protocol field are there for devices with hardware IR
support, where only a limited set of remote protocols can be decoded.

For software decoders RC_BIT_ALL is the proper setup. Users of course
can change it via sysfs at runtime, or a software decoder may be
disabled at compilation time by not selecting its CONFIG_* var.

  Couldn't this be configured at run time, with all protocols allowed
  as the default ?
 
 Actually, this is how the internal rc code works. If there is nothing
 defined for allowed_protocols it assumes that all protocols are supported.
 That is why above node properties are optional.
 
 About the binding documentation of allowed_protocols, rc_map, or the
 default behavior of current linux code, I don't think they will stay
 in-sync for long. 

Why not? The rc_map name is used either by Kernelspace or by Userspace,
in order to provide the IR keycode name that matches a given keytable.

There's no plans to change it, even in the long term.

Regards,
Mauro.

-- 

Cheers,
Mauro
--
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 RESEND] media: rc: gpio-ir-recv: add support for device tree parsing

2013-02-08 Thread Sebastian Hesselbarth
On Fri, Feb 8, 2013 at 6:57 PM, Mauro Carvalho Chehab
mche...@redhat.com wrote:
 Em Wed, 06 Feb 2013 18:18:22 +0100
 Sebastian Hesselbarth sebastian.hesselba...@gmail.com escreveu:
 On 02/06/2013 02:48 PM, Sylwester Nawrocki wrote:
  On 02/06/2013 09:03 AM, Sebastian Hesselbarth wrote:
  This patch adds device tree parsing for gpio_ir_recv platform_data and
  the mandatory binding documentation. It basically follows what we already
  have for e.g. gpio_keys. All required device tree properties are OS
  independent but optional properties allow linux specific support for rc
  protocols and maps.
 
  There was a similar patch sent by Matus Ujhelyi but that discussion
  died after the first reviews.
 
  Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com
  ---
  ...
  diff --git a/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt 
  b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
  new file mode 100644
  index 000..937760c
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
  @@ -0,0 +1,20 @@
  +Device-Tree bindings for GPIO IR receiver
  +
  +Required properties:
  +  - compatible = gpio-ir-receiver;
  +  - gpios: OF device-tree gpio specification.
  +
  +Optional properties:
  +  - linux,allowed-rc-protocols: Linux specific u64 bitmask of allowed
  +  rc protocols.
 
  You likely need to specify in these bindings documentation which bit
  corresponds to which RC protocol.
 
  I'm not very familiar with the RC internals, but why it has to be
  specified statically in the device tree, when decoding seems to be
  mostly software defined ? I might be missing something though..

 Sylwester,

 I am not familiar with RC internals either. Maybe somebody with more
 insight in media/rc can clarify the specific needs for the rc subsystem.
 I was just transferring the DT support approach taken by gpio_keys to
 gpio_ir_recv as I will be using it on mach-dove/cubox soon.

 The allowed rc protocol field are there for devices with hardware IR
 support, where only a limited set of remote protocols can be decoded.

 For software decoders RC_BIT_ALL is the proper setup. Users of course
 can change it via sysfs at runtime, or a software decoder may be
 disabled at compilation time by not selecting its CONFIG_* var.

Mauro,

thanks for the clarification! So for v2 of the patch, you all agree on removing
linux,allowed-rc-protocols from device node properties?

  Couldn't this be configured at run time, with all protocols allowed
  as the default ?

 Actually, this is how the internal rc code works. If there is nothing
 defined for allowed_protocols it assumes that all protocols are supported.
 That is why above node properties are optional.

 About the binding documentation of allowed_protocols, rc_map, or the
 default behavior of current linux code, I don't think they will stay
 in-sync for long.

 Why not? The rc_map name is used either by Kernelspace or by Userspace,
 in order to provide the IR keycode name that matches a given keytable.

 There's no plans to change it, even in the long term.

Actually, I wasn't referring to changing names or bitmasks but updating
the binding documentation with new allowed protocols or supported map
names.

For linux,rc-map-name property it should be enough to just write that it
relates to linux rc subsystem rc_map name - how to actually
set it to a useful name is documented in rc subsystem. And if the
property is not set at all, DT parsing in gpio_ir_recv assumes the
subsystem (or gpio_ir_recv platform) default, IIRC rc-none.

I'll respin a v2 without allowed-protocols property soon.

Sebastian
--
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] [media] tveeprom: Fix lots of bad whitespace

2013-02-08 Thread Mauro Carvalho Chehab
While running checkpatch.pl after the last patch, I noticed
lots of those:
WARNING: please, no space before tabs
#151: FILE: drivers/media/common/tveeprom.c:99:
+^I{ TUNER_ABSENT,^I^INone },$

(together with other checkpatch.pl errors/warnings)

While I won't be fixing everything, as I have already an
script to fix the above, let's do it, in order to clean it
a little bit.

While here, also drop cmacs-specific format text at the end.

Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
---
 drivers/media/common/tveeprom.c | 290 
 1 file changed, 142 insertions(+), 148 deletions(-)

diff --git a/drivers/media/common/tveeprom.c b/drivers/media/common/tveeprom.c
index 3b6cf03..cc1e172 100644
--- a/drivers/media/common/tveeprom.c
+++ b/drivers/media/common/tveeprom.c
@@ -96,170 +96,170 @@ static struct HAUPPAUGE_TUNER
 hauppauge_tuner[] =
 {
/* 0-9 */
-   { TUNER_ABSENT, None },
-   { TUNER_ABSENT, External },
-   { TUNER_ABSENT, Unspecified },
-   { TUNER_PHILIPS_PAL,Philips FI1216 },
-   { TUNER_PHILIPS_SECAM,  Philips FI1216MF },
-   { TUNER_PHILIPS_NTSC,   Philips FI1236 },
-   { TUNER_PHILIPS_PAL_I,  Philips FI1246 },
+   { TUNER_ABSENT, None },
+   { TUNER_ABSENT, External },
+   { TUNER_ABSENT, Unspecified },
+   { TUNER_PHILIPS_PAL,Philips FI1216 },
+   { TUNER_PHILIPS_SECAM,  Philips FI1216MF },
+   { TUNER_PHILIPS_NTSC,   Philips FI1236 },
+   { TUNER_PHILIPS_PAL_I,  Philips FI1246 },
{ TUNER_PHILIPS_PAL_DK, Philips FI1256 },
-   { TUNER_PHILIPS_PAL,Philips FI1216 MK2 },
-   { TUNER_PHILIPS_SECAM,  Philips FI1216MF MK2 },
+   { TUNER_PHILIPS_PAL,Philips FI1216 MK2 },
+   { TUNER_PHILIPS_SECAM,  Philips FI1216MF MK2 },
/* 10-19 */
-   { TUNER_PHILIPS_NTSC,   Philips FI1236 MK2 },
-   { TUNER_PHILIPS_PAL_I,  Philips FI1246 MK2 },
+   { TUNER_PHILIPS_NTSC,   Philips FI1236 MK2 },
+   { TUNER_PHILIPS_PAL_I,  Philips FI1246 MK2 },
{ TUNER_PHILIPS_PAL_DK, Philips FI1256 MK2 },
-   { TUNER_TEMIC_NTSC, Temic 4032FY5 },
-   { TUNER_TEMIC_PAL,  Temic 4002FH5 },
-   { TUNER_TEMIC_PAL_I,Temic 4062FY5 },
-   { TUNER_PHILIPS_PAL,Philips FR1216 MK2 },
-   { TUNER_PHILIPS_SECAM,  Philips FR1216MF MK2 },
-   { TUNER_PHILIPS_NTSC,   Philips FR1236 MK2 },
-   { TUNER_PHILIPS_PAL_I,  Philips FR1246 MK2 },
+   { TUNER_TEMIC_NTSC, Temic 4032FY5 },
+   { TUNER_TEMIC_PAL,  Temic 4002FH5 },
+   { TUNER_TEMIC_PAL_I,Temic 4062FY5 },
+   { TUNER_PHILIPS_PAL,Philips FR1216 MK2 },
+   { TUNER_PHILIPS_SECAM,  Philips FR1216MF MK2 },
+   { TUNER_PHILIPS_NTSC,   Philips FR1236 MK2 },
+   { TUNER_PHILIPS_PAL_I,  Philips FR1246 MK2 },
/* 20-29 */
{ TUNER_PHILIPS_PAL_DK, Philips FR1256 MK2 },
-   { TUNER_PHILIPS_PAL,Philips FM1216 },
-   { TUNER_PHILIPS_SECAM,  Philips FM1216MF },
-   { TUNER_PHILIPS_NTSC,   Philips FM1236 },
-   { TUNER_PHILIPS_PAL_I,  Philips FM1246 },
+   { TUNER_PHILIPS_PAL,Philips FM1216 },
+   { TUNER_PHILIPS_SECAM,  Philips FM1216MF },
+   { TUNER_PHILIPS_NTSC,   Philips FM1236 },
+   { TUNER_PHILIPS_PAL_I,  Philips FM1246 },
{ TUNER_PHILIPS_PAL_DK, Philips FM1256 },
-   { TUNER_TEMIC_4036FY5_NTSC, Temic 4036FY5 },
-   { TUNER_ABSENT, Samsung TCPN9082D },
-   { TUNER_ABSENT, Samsung TCPM9092P },
-   { TUNER_TEMIC_4006FH5_PAL,  Temic 4006FH5 },
+   { TUNER_TEMIC_4036FY5_NTSC, Temic 4036FY5 },
+   { TUNER_ABSENT, Samsung TCPN9082D },
+   { TUNER_ABSENT, Samsung TCPM9092P },
+   { TUNER_TEMIC_4006FH5_PAL,  Temic 4006FH5 },
/* 30-39 */
-   { TUNER_ABSENT, Samsung TCPN9085D },
-   { TUNER_ABSENT, Samsung TCPB9085P },
-   { TUNER_ABSENT, Samsung TCPL9091P },
-   { TUNER_TEMIC_4039FR5_NTSC, Temic 4039FR5 },
-   { TUNER_PHILIPS_FQ1216ME,   Philips FQ1216 ME },
-   { TUNER_TEMIC_4066FY5_PAL_I,Temic 4066FY5 },
-   { TUNER_PHILIPS_NTSC,   Philips TD1536 },
-   { TUNER_PHILIPS_NTSC,   Philips TD1536D },
-   { TUNER_PHILIPS_NTSC,   Philips FMR1236 }, /* mono radio */
-   { TUNER_ABSENT, Philips FI1256MP },
+   { TUNER_ABSENT, Samsung TCPN9085D },
+   { 

Re: [PATCH RESEND] media: rc: gpio-ir-recv: add support for device tree parsing

2013-02-08 Thread Sylwester Nawrocki
On 02/06/2013 06:18 PM, Sebastian Hesselbarth wrote:
 +Optional properties:
 +- linux,allowed-rc-protocols: Linux specific u64 bitmask of allowed
 +rc protocols.

 You likely need to specify in these bindings documentation which bit
 corresponds to which RC protocol.

 I'm not very familiar with the RC internals, but why it has to be
 specified statically in the device tree, when decoding seems to be
 mostly software defined ? I might be missing something though..
 
 Sylwester,
 
 I am not familiar with RC internals either. Maybe somebody with more
 insight in media/rc can clarify the specific needs for the rc subsystem.
 I was just transferring the DT support approach taken by gpio_keys to
 gpio_ir_recv as I will be using it on mach-dove/cubox soon.
 
 Couldn't this be configured at run time, with all protocols allowed
 as the default ?
 
 Actually, this is how the internal rc code works. If there is nothing
 defined for allowed_protocols it assumes that all protocols are supported.
 That is why above node properties are optional.
 
 About the binding documentation of allowed_protocols, rc_map, or the
 default behavior of current linux code, I don't think they will stay
 in-sync for long. I'd rather completely remove those os-specific properties
 from DT, but that hits the above statement about the needs of media/rc
 subsystem.

I think the bindings could define the bitmask, independently of the
RC code. I suppose it is correct to have a list of protocols defined
this way. Since, as Mauro pointed out, it is needed for some hardware
devices that support only selected protocols.
Then you could probably drop the 'linux,' prefix, but I'm not 100% sure
about it.

 Actually, I am not assigning the parsed gpio_ir_recv_platform_data to
 pdev-dev.platform_data but pdata ptr instead. Either I don't see the
 difference in pointer assignments between your code and mine or you
 were mislead from struct gpio_ir_recv_platform_data above.

Sorry, you're right. I think I was just trying to be to quick with this
review.. Your code is correct, however I would probably avoid the ERR_PTR()
pattern as much as possible.

 Anyway, I agree to test for pdev-dev.of_node and call gpio_ir_recv_parse_dt
 if set.

--
Regards,
Sylwester




-- 
Sylwester Nawrocki
실베스터 나브로츠키
Samsung Poland RD Center
--
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 RESEND] media: rc: gpio-ir-recv: add support for device tree parsing

2013-02-08 Thread Mauro Carvalho Chehab
Em Fri, 8 Feb 2013 19:12:31 +0100
Sebastian Hesselbarth sebastian.hesselba...@gmail.com escreveu:

 On Fri, Feb 8, 2013 at 6:57 PM, Mauro Carvalho Chehab
 mche...@redhat.com wrote:
  Em Wed, 06 Feb 2013 18:18:22 +0100
  Sebastian Hesselbarth sebastian.hesselba...@gmail.com escreveu:
  On 02/06/2013 02:48 PM, Sylwester Nawrocki wrote:
   On 02/06/2013 09:03 AM, Sebastian Hesselbarth wrote:
   This patch adds device tree parsing for gpio_ir_recv platform_data and
   the mandatory binding documentation. It basically follows what we 
   already
   have for e.g. gpio_keys. All required device tree properties are OS
   independent but optional properties allow linux specific support for rc
   protocols and maps.
  
   There was a similar patch sent by Matus Ujhelyi but that discussion
   died after the first reviews.
  
   Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com
   ---
   ...
   diff --git 
   a/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt 
   b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
   new file mode 100644
   index 000..937760c
   --- /dev/null
   +++ b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
   @@ -0,0 +1,20 @@
   +Device-Tree bindings for GPIO IR receiver
   +
   +Required properties:
   +  - compatible = gpio-ir-receiver;
   +  - gpios: OF device-tree gpio specification.
   +
   +Optional properties:
   +  - linux,allowed-rc-protocols: Linux specific u64 bitmask of allowed
   +  rc protocols.
  
   You likely need to specify in these bindings documentation which bit
   corresponds to which RC protocol.
  
   I'm not very familiar with the RC internals, but why it has to be
   specified statically in the device tree, when decoding seems to be
   mostly software defined ? I might be missing something though..
 
  Sylwester,
 
  I am not familiar with RC internals either. Maybe somebody with more
  insight in media/rc can clarify the specific needs for the rc subsystem.
  I was just transferring the DT support approach taken by gpio_keys to
  gpio_ir_recv as I will be using it on mach-dove/cubox soon.
 
  The allowed rc protocol field are there for devices with hardware IR
  support, where only a limited set of remote protocols can be decoded.
 
  For software decoders RC_BIT_ALL is the proper setup. Users of course
  can change it via sysfs at runtime, or a software decoder may be
  disabled at compilation time by not selecting its CONFIG_* var.
 
 Mauro,
 
 thanks for the clarification! So for v2 of the patch, you all agree on 
 removing
 linux,allowed-rc-protocols from device node properties?

Yes.
 
   Couldn't this be configured at run time, with all protocols allowed
   as the default ?
 
  Actually, this is how the internal rc code works. If there is nothing
  defined for allowed_protocols it assumes that all protocols are supported.
  That is why above node properties are optional.
 
  About the binding documentation of allowed_protocols, rc_map, or the
  default behavior of current linux code, I don't think they will stay
  in-sync for long.
 
  Why not? The rc_map name is used either by Kernelspace or by Userspace,
  in order to provide the IR keycode name that matches a given keytable.
 
  There's no plans to change it, even in the long term.
 
 Actually, I wasn't referring to changing names or bitmasks but updating
 the binding documentation with new allowed protocols or supported map
 names.
 
 For linux,rc-map-name property it should be enough to just write that it
 relates to linux rc subsystem rc_map name - how to actually
 set it to a useful name is documented in rc subsystem.

It should be one of the names that are there at include/media/rc-map.h.

 And if the
 property is not set at all, DT parsing in gpio_ir_recv assumes the
 subsystem (or gpio_ir_recv platform) default, IIRC rc-none.

The right default should be rc-empty, but please use the macro RC_MAP_EMPTY
instead.

 I'll respin a v2 without allowed-protocols property soon.
 
 Sebastian


-- 

Cheers,
Mauro
--
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: [git:v4l-dvb/for_v3.9] [media] [PATH, 1/2] mxl5007 move reset to attach

2013-02-08 Thread Michael Krufky
Mauro,

This isn't ready for merge yet.  Please revert it.  This needs more
work as I explained on the mailing list.

-Mike Krufky

On Fri, Feb 8, 2013 at 12:37 PM, Mauro Carvalho Chehab
mche...@redhat.com wrote:
 This is an automatic generated email to let you know that the following patch 
 were queued at the
 http://git.linuxtv.org/media_tree.git tree:

 Subject: [media] [PATH,1/2] mxl5007 move reset to attach
 Author:  Jose Alberto Reguero jaregu...@telefonica.net
 Date:Sun Feb 3 18:30:38 2013 -0300

 This patch move the soft reset to the attach function because with dual
 tuners, when one tuner do reset, the other one is perturbed, and the
 stream has errors.

 Signed-off-by: Jose Alberto Reguero jaregu...@telefonica.net
 Reviewed-by: Antti Palosaari cr...@iki.fi
 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com

  drivers/media/tuners/mxl5007t.c |   17 +
  1 files changed, 13 insertions(+), 4 deletions(-)

 ---

 http://git.linuxtv.org/media_tree.git?a=commitdiff;h=0a3237704dec476be3cdfbe8fc9df9cc65b14442

 diff --git a/drivers/media/tuners/mxl5007t.c b/drivers/media/tuners/mxl5007t.c
 index 69e453e..eb61304 100644
 --- a/drivers/media/tuners/mxl5007t.c
 +++ b/drivers/media/tuners/mxl5007t.c
 @@ -531,10 +531,6 @@ static int mxl5007t_tuner_init(struct mxl5007t_state 
 *state,
 struct reg_pair_t *init_regs;
 int ret;

 -   ret = mxl5007t_soft_reset(state);
 -   if (mxl_fail(ret))
 -   goto fail;
 -
 /* calculate initialization reg array */
 init_regs = mxl5007t_calc_init_regs(state, mode);

 @@ -900,7 +896,20 @@ struct dvb_frontend *mxl5007t_attach(struct dvb_frontend 
 *fe,
 /* existing tuner instance */
 break;
 }
 +
 +   if (fe-ops.i2c_gate_ctrl)
 +   fe-ops.i2c_gate_ctrl(fe, 1);
 +
 +   ret = mxl5007t_soft_reset(state);
 +
 +   if (fe-ops.i2c_gate_ctrl)
 +   fe-ops.i2c_gate_ctrl(fe, 0);
 +
 +   if (mxl_fail(ret))
 +   goto fail;
 +
 fe-tuner_priv = state;
 +
 mutex_unlock(mxl5007t_list_mutex);

 memcpy(fe-ops.tuner_ops, mxl5007t_tuner_ops,

 ___
 linuxtv-commits mailing list
 linuxtv-comm...@linuxtv.org
 http://www.linuxtv.org/cgi-bin/mailman/listinfo/linuxtv-commits
--
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: [git:v4l-dvb/for_v3.9] [media] [PATH, 1/2] mxl5007 move reset to attach

2013-02-08 Thread Antti Palosaari

Could you explain what is wrong with that patch?

Antti

On 02/08/2013 09:23 PM, Michael Krufky wrote:

Mauro,

This isn't ready for merge yet.  Please revert it.  This needs more
work as I explained on the mailing list.

-Mike Krufky

On Fri, Feb 8, 2013 at 12:37 PM, Mauro Carvalho Chehab
mche...@redhat.com wrote:

This is an automatic generated email to let you know that the following patch 
were queued at the
http://git.linuxtv.org/media_tree.git tree:

Subject: [media] [PATH,1/2] mxl5007 move reset to attach
Author:  Jose Alberto Reguero jaregu...@telefonica.net
Date:Sun Feb 3 18:30:38 2013 -0300

This patch move the soft reset to the attach function because with dual
tuners, when one tuner do reset, the other one is perturbed, and the
stream has errors.

Signed-off-by: Jose Alberto Reguero jaregu...@telefonica.net
Reviewed-by: Antti Palosaari cr...@iki.fi
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com

  drivers/media/tuners/mxl5007t.c |   17 +
  1 files changed, 13 insertions(+), 4 deletions(-)

---

http://git.linuxtv.org/media_tree.git?a=commitdiff;h=0a3237704dec476be3cdfbe8fc9df9cc65b14442

diff --git a/drivers/media/tuners/mxl5007t.c b/drivers/media/tuners/mxl5007t.c
index 69e453e..eb61304 100644
--- a/drivers/media/tuners/mxl5007t.c
+++ b/drivers/media/tuners/mxl5007t.c
@@ -531,10 +531,6 @@ static int mxl5007t_tuner_init(struct mxl5007t_state 
*state,
 struct reg_pair_t *init_regs;
 int ret;

-   ret = mxl5007t_soft_reset(state);
-   if (mxl_fail(ret))
-   goto fail;
-
 /* calculate initialization reg array */
 init_regs = mxl5007t_calc_init_regs(state, mode);

@@ -900,7 +896,20 @@ struct dvb_frontend *mxl5007t_attach(struct dvb_frontend 
*fe,
 /* existing tuner instance */
 break;
 }
+
+   if (fe-ops.i2c_gate_ctrl)
+   fe-ops.i2c_gate_ctrl(fe, 1);
+
+   ret = mxl5007t_soft_reset(state);
+
+   if (fe-ops.i2c_gate_ctrl)
+   fe-ops.i2c_gate_ctrl(fe, 0);
+
+   if (mxl_fail(ret))
+   goto fail;
+
 fe-tuner_priv = state;
+
 mutex_unlock(mxl5007t_list_mutex);

 memcpy(fe-ops.tuner_ops, mxl5007t_tuner_ops,

___
linuxtv-commits mailing list
linuxtv-comm...@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linuxtv-commits



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


cron job: media_tree daily build: ERRORS

2013-02-08 Thread 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:   Fri Feb  8 19:00:25 CET 2013
git branch: for_v3.9
git hash:   47ebe3f93250fce6f4eaa16309ae5eee9d4099b3
gcc version:i686-linux-gcc (GCC) 4.7.2
host hardware:  x86_64
host os:3.8.03-marune

linux-git-arm-davinci: WARNINGS
linux-git-arm-exynos: ERRORS
linux-git-arm-omap: WARNINGS
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: WARNINGS
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.31.14-i686: ERRORS
linux-2.6.32.27-i686: ERRORS
linux-2.6.33.7-i686: ERRORS
linux-2.6.34.7-i686: ERRORS
linux-2.6.35.9-i686: ERRORS
linux-2.6.36.4-i686: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.2.37-i686: WARNINGS
linux-3.3.8-i686: WARNINGS
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-rc4-i686: OK
linux-2.6.31.14-x86_64: ERRORS
linux-2.6.32.27-x86_64: ERRORS
linux-2.6.33.7-x86_64: ERRORS
linux-2.6.34.7-x86_64: ERRORS
linux-2.6.35.9-x86_64: ERRORS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-rc4-x86_64: WARNINGS
apps: WARNINGS
spec-git: OK
sparse: ERRORS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API 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 v2] media: rc: gpio-ir-recv: add support for device tree parsing

2013-02-08 Thread Sylwester Nawrocki

On 02/08/2013 09:38 PM, Sebastian Hesselbarth wrote:

This patch adds device tree parsing for gpio_ir_recv platform_data and
the mandatory binding documentation. It basically follows what we already
have for e.g. gpio_keys. All required device tree properties are OS
independent but an optional property allow linux specific support for rc
maps.

There was a similar patch sent by Matus Ujhelyi but that discussion
died after the first reviews.

Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com
---
Changelog

v1-v2:
- get rid of ptr returned by _get_devtree_pdata()
- check for of_node instead for NULL pdata
- remove unneccessary double check for gpios property
- remove unneccessary #ifdef CONFIG_OF around match table

Cc: Grant Likelygrant.lik...@secretlab.ca
Cc: Rob Herringrob.herr...@calxeda.com
Cc: Rob Landleyr...@landley.net
Cc: Mauro Carvalho Chehabmche...@redhat.com
Cc: Sebastian Hesselbarthsebastian.hesselba...@gmail.com
Cc: Benoit Thebaudeaubenoit.thebaud...@advansee.com
Cc: David Hardemanda...@hardeman.nu
Cc: Trilok Sonits...@codeaurora.org
Cc: Sylwester Nawrockis.nawro...@samsung.com
Cc: Matus Ujhelyiujhely...@gmail.com
Cc: devicetree-disc...@lists.ozlabs.org
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
---
  .../devicetree/bindings/media/gpio-ir-receiver.txt |   16 ++
  drivers/media/rc/gpio-ir-recv.c|   57 
  2 files changed, 73 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/media/gpio-ir-receiver.txt

diff --git a/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt 
b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
new file mode 100644
index 000..8589f30
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
@@ -0,0 +1,16 @@
+Device-Tree bindings for GPIO IR receiver
+
+Required properties:
+   - compatible = gpio-ir-receiver;
+   - gpios: OF device-tree gpio specification.


Perhaps:
- compatible: should be gpio-ir-receiver;
- gpios: specifies GPIO used for IR signal reception.
?

+
+Optional properties:
+   - linux,rc-map-name: Linux specific remote control map name.
+
+Example node:
+
+   ir: ir-receiver {
+   compatible = gpio-ir-receiver;
+   gpios =gpio0 19 1;
+   linux,rc-map-name = rc-rc6-mce;
+   };
diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 4f71a7d..3c62006 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -16,6 +16,7 @@
  #includelinux/interrupt.h
  #includelinux/gpio.h
  #includelinux/slab.h
+#includelinux/of_gpio.h
  #includelinux/platform_device.h
  #includelinux/irq.h
  #includemedia/rc-core.h
@@ -30,6 +31,50 @@ struct gpio_rc_dev {
bool active_low;
  };

+#ifdef CONFIG_OF
+/*
+ * Translate OpenFirmware node properties into platform_data
+ */
+static int gpio_ir_recv_get_devtree_pdata(struct device *dev,
+ struct gpio_ir_recv_platform_data *pdata)
+{
+   struct device_node *np = dev-of_node;
+   enum of_gpio_flags flags;
+   int gpio;
+
+   gpio = of_get_gpio_flags(np, 0,flags);
+   if (gpio  0) {
+   if (gpio != -EPROBE_DEFER)
+   dev_err(dev, Failed to get gpio flags, error: %d\n,
+   gpio);


dev_err(dev, Failed to get gpio flags (%d)\n,   
gpio);
?

+   return gpio;
+   }
+
+   pdata-gpio_nr = gpio;
+   pdata-active_low = (flags  OF_GPIO_ACTIVE_LOW) ? true : false;


This could be simplified to:

pdata-active_low = (flags  OF_GPIO_ACTIVE_LOW);


+   /* probe() takes care of map_name == NULL or allowed_protos == 0 */
+   pdata-map_name = of_get_property(np, linux,rc-map-name, NULL);
+   pdata-allowed_protos = 0;
+
+   return 0;
+}
+
+static struct of_device_id gpio_ir_recv_of_match[] = {
+   { .compatible = gpio-ir-receiver, },
+   { },
+};
+MODULE_DEVICE_TABLE(of, gpio_ir_recv_of_match);
+
+#else /* !CONFIG_OF */
+
+static inline struct gpio_ir_recv_platform_data *
+gpio_ir_recv_get_devtree_pdata(struct device *dev)


Please check how it compiles with CONFIG_OF disabled ;)

You could also make it:

#define gpio_ir_recv_get_devtree_pdata  (-ENOSYS)


+{
+   return ERR_PTR(-ENODEV);
+}
+
+#endif
+
  static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id)
  {
struct gpio_rc_dev *gpio_dev = dev_id;
@@ -66,6 +111,17 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
pdev-dev.platform_data;
int rc;

+   if (pdev-dev.of_node) {
+   struct gpio_ir_recv_platform_data *dtpdata =


I think you could use pdata here instead, as previously. But I'm fine with
as it is now as well.


+   devm_kzalloc(pdev-dev, sizeof(*dtpdata), GFP_KERNEL);
+

Re: [PATCH v2] media: rc: gpio-ir-recv: add support for device tree parsing

2013-02-08 Thread Sebastian Hesselbarth

On 02/08/2013 10:26 PM, Sylwester Nawrocki wrote:

On 02/08/2013 09:38 PM, Sebastian Hesselbarth wrote:

This patch adds device tree parsing for gpio_ir_recv platform_data and
the mandatory binding documentation. It basically follows what we already
have for e.g. gpio_keys. All required device tree properties are OS
independent but an optional property allow linux specific support for rc
maps.

There was a similar patch sent by Matus Ujhelyi but that discussion
died after the first reviews.

Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com
---
Changelog

v1-v2:
- get rid of ptr returned by _get_devtree_pdata()
- check for of_node instead for NULL pdata
- remove unneccessary double check for gpios property
- remove unneccessary #ifdef CONFIG_OF around match table

Cc: Grant Likelygrant.lik...@secretlab.ca
Cc: Rob Herringrob.herr...@calxeda.com
Cc: Rob Landleyr...@landley.net
Cc: Mauro Carvalho Chehabmche...@redhat.com
Cc: Sebastian Hesselbarthsebastian.hesselba...@gmail.com
Cc: Benoit Thebaudeaubenoit.thebaud...@advansee.com
Cc: David Hardemanda...@hardeman.nu
Cc: Trilok Sonits...@codeaurora.org
Cc: Sylwester Nawrockis.nawro...@samsung.com
Cc: Matus Ujhelyiujhely...@gmail.com
Cc: devicetree-disc...@lists.ozlabs.org
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
---
.../devicetree/bindings/media/gpio-ir-receiver.txt | 16 ++
drivers/media/rc/gpio-ir-recv.c | 57 
2 files changed, 73 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/gpio-ir-receiver.txt

diff --git a/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
new file mode 100644
index 000..8589f30
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
@@ -0,0 +1,16 @@
+Device-Tree bindings for GPIO IR receiver
+
+Required properties:
+ - compatible = gpio-ir-receiver;
+ - gpios: OF device-tree gpio specification.


Perhaps:
- compatible: should be gpio-ir-receiver;
- gpios: specifies GPIO used for IR signal reception.
?


Ok, i'll change that.


+
+Optional properties:
+ - linux,rc-map-name: Linux specific remote control map name.
+
+Example node:
+
+ ir: ir-receiver {
+ compatible = gpio-ir-receiver;
+ gpios =gpio0 19 1;
+ linux,rc-map-name = rc-rc6-mce;
+ };
diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 4f71a7d..3c62006 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -16,6 +16,7 @@
#includelinux/interrupt.h
#includelinux/gpio.h
#includelinux/slab.h
+#includelinux/of_gpio.h
#includelinux/platform_device.h
#includelinux/irq.h
#includemedia/rc-core.h
@@ -30,6 +31,50 @@ struct gpio_rc_dev {
bool active_low;
};

+#ifdef CONFIG_OF
+/*
+ * Translate OpenFirmware node properties into platform_data
+ */
+static int gpio_ir_recv_get_devtree_pdata(struct device *dev,
+ struct gpio_ir_recv_platform_data *pdata)
+{
+ struct device_node *np = dev-of_node;
+ enum of_gpio_flags flags;
+ int gpio;
+
+ gpio = of_get_gpio_flags(np, 0,flags);
+ if (gpio 0) {
+ if (gpio != -EPROBE_DEFER)
+ dev_err(dev, Failed to get gpio flags, error: %d\n,
+ gpio);


dev_err(dev, Failed to get gpio flags (%d)\n, gpio);
?


Ack.


+ return gpio;
+ }
+
+ pdata-gpio_nr = gpio;
+ pdata-active_low = (flags OF_GPIO_ACTIVE_LOW) ? true : false;


This could be simplified to:

pdata-active_low = (flags  OF_GPIO_ACTIVE_LOW);


Ack.


+ /* probe() takes care of map_name == NULL or allowed_protos == 0 */
+ pdata-map_name = of_get_property(np, linux,rc-map-name, NULL);
+ pdata-allowed_protos = 0;
+
+ return 0;
+}
+
+static struct of_device_id gpio_ir_recv_of_match[] = {
+ { .compatible = gpio-ir-receiver, },
+ { },
+};
+MODULE_DEVICE_TABLE(of, gpio_ir_recv_of_match);
+
+#else /* !CONFIG_OF */
+
+static inline struct gpio_ir_recv_platform_data *
+gpio_ir_recv_get_devtree_pdata(struct device *dev)


Please check how it compiles with CONFIG_OF disabled ;)


Grrrml ;) I'll fix that of course...


You could also make it:

#define gpio_ir_recv_get_devtree_pdata (-ENOSYS)


Hmm, does that also play with parameter passing of the
CONFIG_OF gpio_ir_recv_get_devtree_pdata() ?


+{
+ return ERR_PTR(-ENODEV);
+}
+
+#endif
+
static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id)
{
struct gpio_rc_dev *gpio_dev = dev_id;
@@ -66,6 +111,17 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
pdev-dev.platform_data;
int rc;

+ if (pdev-dev.of_node) {
+ struct gpio_ir_recv_platform_data *dtpdata =


I think you could use pdata here instead, as previously. But I'm fine with
as it is now as well.


Yeah, but pdata is const and I will change it within _get_devtree_pdata().
I could cast the const away when passing it to gpio_ir_recv_get_devtree_pdata()
but it is almost the same amount of code.. and it is cleaner this way.




+ devm_kzalloc(pdev-dev, sizeof(*dtpdata), GFP_KERNEL);
+ if (!dtpdata)
+ return -ENOMEM;
+ rc 

Re: [PATCH v2] media: rc: gpio-ir-recv: add support for device tree parsing

2013-02-08 Thread Sylwester Nawrocki

On 02/08/2013 10:36 PM, Sebastian Hesselbarth wrote:

You could also make it:

#define gpio_ir_recv_get_devtree_pdata (-ENOSYS)


Hmm, does that also play with parameter passing of the
CONFIG_OF gpio_ir_recv_get_devtree_pdata() ?


Oops, should have been:

#define gpio_ir_recv_get_devtree_pdata(dev, pd) (-ENOSYS)


#define gpio_ir_recv_get_devtree_pdata (-ENOSYS)

+{
+ return ERR_PTR(-ENODEV);
+}
+
+#endif
+
static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id)
{
struct gpio_rc_dev *gpio_dev = dev_id;
@@ -66,6 +111,17 @@ static int gpio_ir_recv_probe(struct
platform_device *pdev)
pdev-dev.platform_data;
int rc;

+ if (pdev-dev.of_node) {
+ struct gpio_ir_recv_platform_data *dtpdata =


I think you could use pdata here instead, as previously. But I'm fine
with
as it is now as well.


Yeah, but pdata is const and I will change it within _get_devtree_pdata().
I could cast the const away when passing it to
gpio_ir_recv_get_devtree_pdata()
but it is almost the same amount of code.. and it is cleaner this way.


True, let's leave it intact then.

S.
--
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 v4 01/10] s5p-csis: Add device tree support

2013-02-08 Thread Sylwester Nawrocki

On 02/07/2013 12:36 AM, Stephen Warren wrote:

On 02/01/2013 12:09 PM, Sylwester Nawrocki wrote:

s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC
device. This patch support for binding the driver to the MIPI-CSIS
devices instantiated from device tree and for parsing all SoC and
board specific properties.



diff --git a/Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt

b/Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt


+Optional properties:
+
+- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
+   value when this property is not specified is 166 MHz;


Shouldn't this be a clocks property, so that the driver can call
clk_get(), clk_prepare_enable(), clk_get_rate(), etc. on it?


Hi Stephen,

Thanks for your review!

I also use clocks and clock-names property, but didn't specify
it here, because the Exynos SoCs clocks driver is not in the mainline yet.

There are two clocks the driver needs to be aware of. One of them needs
to have a specific parent clock set and the frequency set properly,
so when it is put into a data pipeline with other IPs there is no overflows
of their input/output FIFOs.

devfreq may change those frequencies if enabled, however its another topic.
I wanted to ensure the device has initially correct local clock frequency
set, with which it can operate.

--

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


Re: [PATCH v4 02/10] s5p-fimc: Add device tree support for FIMC devices

2013-02-08 Thread Sylwester Nawrocki

On 02/07/2013 12:40 AM, Stephen Warren wrote:

diff --git a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt 
b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt



+Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC)
+--
+
+The Exynos Camera subsystem comprises of multiple sub-devices that are
+represented by separate platform devices. Some of the IPs come in different


platform devices is a rather Linux-centric term, and DT bindings
should be OS-agnostic. Perhaps use device tree nodes here?


Indeed, thank you for the suggestion, I'll change it.


+variants across the SoC revisions (FIMC) and some remain mostly unchanged
+(MIPI CSIS, FIMC-LITE).
+
+All those sub-subdevices are defined as parent nodes of the common device


s/parent nodes/child node/ I think?


Yeah, 'parent nodes' doesn't really make sense. Thanks for catching it.


+For every fimc node a numbered alias should be present in the aliases node.
+Aliases are of the form fimcn, wheren  is an integer (0...N) specifying
+the IP's instance index.


Why? Isn't it up to the DT author whether they care if each fimc node is
assigned a specific identification v.s. whether identification is
assigned automatically?


There are at least three different kinds of IPs that come in multiple
instances in an SoC. To activate data links between them each instance
needs to be clearly identified. There are also differences between
instances of same device. Hence it's important these aliases don't have
random values.

Some more details about the SoC can be found at [1]. The aliases are
also already used in the Exynos5 GScaler bindings [2] in a similar way.


+Optional properties
+
+ - clock-frequency - maximum FIMC local clock (LCLK) frequency


Again, I'd expect a clocks property here instead.


Please see my comment to patch 01/10. Analogously, I needed this clock
frequency to ensure reliable video data pipeline operation.

[1] http://tinyurl.com/anw9udm
[2] http://www.spinics.net/lists/arm-kernel/msg200036.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 v4 02/10] s5p-fimc: Add device tree support for FIMC devices

2013-02-08 Thread Stephen Warren
On 02/08/2013 04:16 PM, Sylwester Nawrocki wrote:
 On 02/07/2013 12:40 AM, Stephen Warren wrote:
 diff --git
 a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt
 b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt

 +Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC)
 +--
...
 +For every fimc node a numbered alias should be present in the
 aliases node.
 +Aliases are of the form fimcn, wheren  is an integer (0...N)
 specifying
 +the IP's instance index.

 Why? Isn't it up to the DT author whether they care if each fimc node is
 assigned a specific identification v.s. whether identification is
 assigned automatically?
 
 There are at least three different kinds of IPs that come in multiple
 instances in an SoC. To activate data links between them each instance
 needs to be clearly identified. There are also differences between
 instances of same device. Hence it's important these aliases don't have
 random values.
 
 Some more details about the SoC can be found at [1]. The aliases are
 also already used in the Exynos5 GScaler bindings [2] in a similar way.

Hmmm. I'd expect explicit DT properties to represent the
instance-specific configuration, or even different compatible values.
Relying on the alias ID seems rather indirect; what if in e.g.
Exynos6/... the mapping from instance/alias ID to feature set changes.
With explicit DT properties, that'd just be a .dts change, whereas by
requiring alias IDs now, you'd need a driver change to support this.
--
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 v4 05/10] s5p-fimc: Add device tree based sensors registration

2013-02-08 Thread Sylwester Nawrocki

On 02/07/2013 12:42 AM, Stephen Warren wrote:

On 02/01/2013 12:09 PM, Sylwester Nawrocki wrote:

The sensor (I2C and/or SPI client) devices are instantiated by their
corresponding control bus drivers. Since the I2C client's master clock
is often provided by a video bus receiver (host interface) or other
than I2C/SPI controller device, the drivers of those client devices
are not accessing hardware in their driver's probe() callback. Instead,
after enabling clock, the host driver calls back into a sub-device
when it wants to activate them. This pattern is used by some in-tree
drivers and this patch also uses it for DT case. This patch is intended
as a first step for adding device tree support to the S5P/Exynos SoC
camera drivers. The second one is adding support for asynchronous
sub-devices registration and clock control from sub-device driver
level. The bindings shall not change when asynchronous probing support
is added.



diff --git a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt 
b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt



+The sensor device nodes should be added as their control bus controller


I think as should be to?


Yes, something is wrong here. Hopefully this is more correct:

The sensor device nodes should be added to their control bus controller
(e.g. I2C0) nodes and linked to a port node in the csis or parallel-ports


+(e.g. I2C0) child nodes and linked to a port node in the csis or parallel-ports
+node, using common the common video interfaces bindings, i.e. port/endpoint

   
And this needs correction too.


+node pairs. The implementation of this binding requires clock-frequency
+property to be present in the sensor device nodes.

--
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 v4 01/10] s5p-csis: Add device tree support

2013-02-08 Thread Stephen Warren
On 02/08/2013 03:29 PM, Sylwester Nawrocki wrote:
 On 02/07/2013 12:36 AM, Stephen Warren wrote:
 On 02/01/2013 12:09 PM, Sylwester Nawrocki wrote:
 s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC
 device. This patch support for binding the driver to the MIPI-CSIS
 devices instantiated from device tree and for parsing all SoC and
 board specific properties.

 diff --git
 a/Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt
 b/Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt

 +Optional properties:
 +
 +- clock-frequency : The IP's main (system bus) clock frequency in
 Hz, default
 +value when this property is not specified is 166 MHz;

 Shouldn't this be a clocks property, so that the driver can call
 clk_get(), clk_prepare_enable(), clk_get_rate(), etc. on it?
 
 Hi Stephen,
 
 Thanks for your review!
 
 I also use clocks and clock-names property, but didn't specify
 it here, because the Exynos SoCs clocks driver is not in the mainline yet.

I'm a bit sympathetic to this, since I've been trying to push Tegra
towards the common clock framework and describing clock connectivity in
DT, before adding new drivers that need clocks, specifically to avoid
this kind of situation.

The problem here is that if this gets merged now, then the clock driver
comes along later, you'll have to change this binding definition to
account for it, and DT bindings aren't supposed to be changed...

Do you have any clock driver at all upstream yet? Or, could you add a
dummy clock driver to satisfy the driver's clkl_get() needs? If you do,
you can always set up some AUXDATA so that clk_get() works for your
driver right now, and then remove that once the complete clock driver is
in place with full device tree support. That's how we've ended up
handling this for Tegra drivers.

Anyway, this is all mainly food-for-thought rather than an objection to
the patch; I'd leave that to Grant/Rob if applicable.
--
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 v4 06/10] s5p-fimc: Use pinctrl API for camera ports configuration

2013-02-08 Thread Sylwester Nawrocki

On 02/07/2013 12:44 AM, Stephen Warren wrote:

On 02/01/2013 12:09 PM, Sylwester Nawrocki wrote:

Before the camera ports can be used the pinmux needs to be configured
properly. This patch adds a function to set the camera ports pinctrl
to a default state within the media driver's probe().
The camera port(s) are then configured for the video bus operation.



diff --git a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt 
b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt



+- pinctrl-names: pinctrl names for camera port pinmux control, at least
+default needs to be specified.
+- pinctrl-0...N   : pinctrl properties corresponding to pinctrl-names
+


A reference to the binding document describing the pin control bindings
would be appropriate here. Given that reference, I'm not sure if
spelling out the property names makes sense since it feels a little like
duplication; an alternative might be simply:

The pinctrl bindings defined in ../../../pinctrl/pinctrl-bindings.txt
must be used to define a pinctrl state named default.


OK, I will add a reference to the pinctrl bindings instead.


However, this isn't a big deal; it's fine either way.


--

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


Re: [PATCH v2] media: rc: gpio-ir-recv: add support for device tree parsing

2013-02-08 Thread Mauro Carvalho Chehab
Em Fri,  8 Feb 2013 21:38:07 +0100
Sebastian Hesselbarth sebastian.hesselba...@gmail.com escreveu:

 This patch adds device tree parsing for gpio_ir_recv platform_data and
 the mandatory binding documentation. It basically follows what we already
 have for e.g. gpio_keys. All required device tree properties are OS
 independent but an optional property allow linux specific support for rc
 maps.
 
 There was a similar patch sent by Matus Ujhelyi but that discussion
 died after the first reviews.
 
 Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
 ---
 Changelog
 
 v1-v2:
 - get rid of ptr returned by _get_devtree_pdata()
 - check for of_node instead for NULL pdata
 - remove unneccessary double check for gpios property
 - remove unneccessary #ifdef CONFIG_OF around match table
 
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: Rob Herring rob.herr...@calxeda.com
 Cc: Rob Landley r...@landley.net
 Cc: Mauro Carvalho Chehab mche...@redhat.com
 Cc: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
 Cc: Benoit Thebaudeau benoit.thebaud...@advansee.com
 Cc: David Hardeman da...@hardeman.nu
 Cc: Trilok Soni ts...@codeaurora.org
 Cc: Sylwester Nawrocki s.nawro...@samsung.com
 Cc: Matus Ujhelyi ujhely...@gmail.com
 Cc: devicetree-disc...@lists.ozlabs.org
 Cc: linux-...@vger.kernel.org
 Cc: linux-ker...@vger.kernel.org
 Cc: linux-media@vger.kernel.org
 ---
  .../devicetree/bindings/media/gpio-ir-receiver.txt |   16 ++
  drivers/media/rc/gpio-ir-recv.c|   57 
 
  2 files changed, 73 insertions(+)
  create mode 100644 
 Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
 
 diff --git a/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt 
 b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
 new file mode 100644
 index 000..8589f30
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
 @@ -0,0 +1,16 @@
 +Device-Tree bindings for GPIO IR receiver
 +
 +Required properties:
 + - compatible = gpio-ir-receiver;
 + - gpios: OF device-tree gpio specification.
 +
 +Optional properties:
 + - linux,rc-map-name: Linux specific remote control map name.
 +
 +Example node:
 +
 + ir: ir-receiver {
 + compatible = gpio-ir-receiver;
 + gpios = gpio0 19 1;
 + linux,rc-map-name = rc-rc6-mce;

Please change this to:
linux,rc-map-name = RC_MAP_RC6_MCE;

(as defined at include/media/rc-map.h).

The idea of having those strings defined at the same header file is to:

- make sure that the same keyboard is spelled at the same way on
all places;

- avoid people to duplicate IR keytables, using different names;

- help userspace to get the right table. In the future, the
plan is to remove all keytables from kernelspace, keeping there just the
name of the keytable. The existing userspace code (ir-keytables, part
of v4l-utils) use the keytable name to dynamically load the right table
in runtime and to switch the IR core to only handle the protocol that
it is associated with the loaded keytable.

Regards,
Mauro
--
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 v4 02/10] s5p-fimc: Add device tree support for FIMC devices

2013-02-08 Thread Sylwester Nawrocki

On 02/09/2013 12:21 AM, Stephen Warren wrote:

On 02/08/2013 04:16 PM, Sylwester Nawrocki wrote:

On 02/07/2013 12:40 AM, Stephen Warren wrote:

diff --git
a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt
b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt



+Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC)
+--

...

+For every fimc node a numbered alias should be present in the
aliases node.
+Aliases are of the form fimcn, wheren   is an integer (0...N)
specifying
+the IP's instance index.


Why? Isn't it up to the DT author whether they care if each fimc node is
assigned a specific identification v.s. whether identification is
assigned automatically?


There are at least three different kinds of IPs that come in multiple
instances in an SoC. To activate data links between them each instance
needs to be clearly identified. There are also differences between
instances of same device. Hence it's important these aliases don't have
random values.

Some more details about the SoC can be found at [1]. The aliases are
also already used in the Exynos5 GScaler bindings [2] in a similar way.


Hmmm. I'd expect explicit DT properties to represent the
instance-specific configuration, or even different compatible values.
Relying on the alias ID seems rather indirect; what if in e.g.
Exynos6/... the mapping from instance/alias ID to feature set changes.
With explicit DT properties, that'd just be a .dts change, whereas by
requiring alias IDs now, you'd need a driver change to support this.


In the initial version of this patch series I used cell-index property,
but then Grant pointed out in some other mail thread it should be
avoided. Hence I used the node aliases.

Different compatible values might not work, when for example there
are 3 IPs out of 4 of one type and the fourth one of another type.
It wouldn't even by really different types, just quirks/little
differences between them, e.g. no data path routed to one of other IPs.

Then to connect e.g. MIPI-CSIS.0 to FIMC.2 at run time an index of the
MIPI-CSIS needs to be written to the FIMC.2 data input control register.
Even though MIPI-CSIS.N are same in terms of hardware structure they still
need to be distinguished as separate instances.

--
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 v4 01/10] s5p-csis: Add device tree support

2013-02-08 Thread Sylwester Nawrocki

On 02/09/2013 12:27 AM, Stephen Warren wrote:

On 02/08/2013 03:29 PM, Sylwester Nawrocki wrote:

On 02/07/2013 12:36 AM, Stephen Warren wrote:

On 02/01/2013 12:09 PM, Sylwester Nawrocki wrote:

s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC
device. This patch support for binding the driver to the MIPI-CSIS
devices instantiated from device tree and for parsing all SoC and
board specific properties.



diff --git
a/Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt

b/Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt


+Optional properties:
+
+- clock-frequency : The IP's main (system bus) clock frequency in
Hz, default
+value when this property is not specified is 166 MHz;


Shouldn't this be a clocks property, so that the driver can call
clk_get(), clk_prepare_enable(), clk_get_rate(), etc. on it?


Hi Stephen,

Thanks for your review!

I also use clocks and clock-names property, but didn't specify
it here, because the Exynos SoCs clocks driver is not in the mainline yet.


I'm a bit sympathetic to this, since I've been trying to push Tegra
towards the common clock framework and describing clock connectivity in
DT, before adding new drivers that need clocks, specifically to avoid
this kind of situation.

The problem here is that if this gets merged now, then the clock driver
comes along later, you'll have to change this binding definition to
account for it, and DT bindings aren't supposed to be changed...


Yes, I wasn't quite sure if this is a really serious problem or not. There
is already quite a few drivers for the Exynos SoC IPs that have DT support
and their bindings will need to be changed when the new clocks driver
gets upstreamed...


Do you have any clock driver at all upstream yet? Or, could you add a
dummy clock driver to satisfy the driver's clkl_get() needs? If you do,
you can always set up some AUXDATA so that clk_get() works for your
driver right now, and then remove that once the complete clock driver is
in place with full device tree support. That's how we've ended up
handling this for Tegra drivers.


Yes, there is the clocks support upstream, only not using the common clock
API. And I used indeed AUXDATA in v3 of these patches.

The Exynos common clock API based driver was supposed to be merged in v3.9,
but it seems it won't happen. These patches also won't make it to 3.9.
Then hopefully both appear in 3.10 together.

I will add the clock properties to relevant nodes, assuming the new clocks
driver is available.


Anyway, this is all mainly food-for-thought rather than an objection to
the patch; I'd leave that to Grant/Rob if applicable.


:-) OK, thanks for the suggestions.

--
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 v4 02/10] s5p-fimc: Add device tree support for FIMC devices

2013-02-08 Thread Stephen Warren
On 02/08/2013 05:05 PM, Sylwester Nawrocki wrote:
 On 02/09/2013 12:21 AM, Stephen Warren wrote:
 On 02/08/2013 04:16 PM, Sylwester Nawrocki wrote:
 On 02/07/2013 12:40 AM, Stephen Warren wrote:
 diff --git
 a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt
 b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt

 +Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC)
 +--
 ...
 +For every fimc node a numbered alias should be present in the
 aliases node.
 +Aliases are of the form fimcn, wheren   is an integer (0...N)
 specifying
 +the IP's instance index.

 Why? Isn't it up to the DT author whether they care if each fimc
 node is
 assigned a specific identification v.s. whether identification is
 assigned automatically?

 There are at least three different kinds of IPs that come in multiple
 instances in an SoC. To activate data links between them each instance
 needs to be clearly identified. There are also differences between
 instances of same device. Hence it's important these aliases don't have
 random values.

 Some more details about the SoC can be found at [1]. The aliases are
 also already used in the Exynos5 GScaler bindings [2] in a similar way.

 Hmmm. I'd expect explicit DT properties to represent the
 instance-specific configuration, or even different compatible values.
 Relying on the alias ID seems rather indirect; what if in e.g.
 Exynos6/... the mapping from instance/alias ID to feature set changes.
 With explicit DT properties, that'd just be a .dts change, whereas by
 requiring alias IDs now, you'd need a driver change to support this.
 
 In the initial version of this patch series I used cell-index property,
 but then Grant pointed out in some other mail thread it should be
 avoided. Hence I used the node aliases.

To me, using cell-index is semantically equivalent to using the alias ID.

 Different compatible values might not work, when for example there
 are 3 IPs out of 4 of one type and the fourth one of another type.
 It wouldn't even by really different types, just quirks/little
 differences between them, e.g. no data path routed to one of other IPs.

I was thinking of using feature-/quirk-oriented properties. For example,
if there's a port on 3 of the 4 devices to connect to some other IP
block, simply include a boolean property to indicate whether that port
is present. It would be in 3 of the nodes but not the 4th.

 Then to connect e.g. MIPI-CSIS.0 to FIMC.2 at run time an index of the
 MIPI-CSIS needs to be written to the FIMC.2 data input control register.
 Even though MIPI-CSIS.N are same in terms of hardware structure they still
 need to be distinguished as separate instances.

Oh, so you're using the alias ID as the value to write into the FIMC.2
register for that. I'm not 100% familiar with aliases, but they seem
like a more user-oriented naming thing to me, whereas values for hooking
up intra-SoC routing are an unrelated namespace semantically, even if
the values happen to line up right now. Perhaps rather than a Boolean
property I mentioned above, use a custom property to indicate the ID
that the FIMC.2 object knows the MIPI-CSIS.0 object as? While this seems
similar to using cell-index, my *guess* is that Grant's objection to
using cell-index was more based on re-using cell-index for something
other than its intended purpose than pushing you to use an alias ID
rather than a property.

After all, what happens in some later SoC where you have two different
types of module that feed into the common module, such that type A
sources have IDs 0..3 in the common module, and type B sources have IDs
4..7 in the common module - you wouldn't want to require alias ISs 4..7
for the type B DT nodes.
--
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: terratec h5 rev. 3?

2013-02-08 Thread Roland Scheidegger
Am 08.02.2013 16:17, schrieb Mauro Carvalho Chehab:
 Em Mon, 28 Jan 2013 13:02:43 +0100
 Roland Scheidegger rscheidegger_li...@hispeed.ch escreveu:
 
 From: Roland Scheidegger rscheidegger_li...@hispeed.ch
 To: linux-media@vger.kernel.org
 Subject: Re: terratec h5 rev. 3?
 Date: Mon, 28 Jan 2013 13:02:43 +0100
 Sender: linux-media-ow...@vger.kernel.org
 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130105 
 Thunderbird/17.0.2

 Am 21.12.2012 06:38, schrieb linux-media-ow...@vger.kernel.org:
 Hi,

 I've recently got a terratec h5 for dvb-c and thought it would be
 supported but it looks like it's a newer revision not recognized by em28xx.
 After using the new_id hack it gets recognized and using various htc
 cards (notably h5 or cinergy htc stick, cards 79 and 82 respectively) it
 seems to _nearly_ work but not quite (I was using h5 firmware for the
 older version). Tuning, channel scan works however tv (or dvb radio)
 does not, since it appears the error rate is going through the roof
 (with some imagination it is possible to see some parts of the picture
 sometimes and hear some audio pieces).  

 Ok I have received a replacement now and I can confirm this stick in
 fact just works like the same as the older versions (I guess maybe the
 first one had bad solder point?). I can't judge signal sensitivity or
 anything like that (the snr values are rather humorous) but it's
 definitely good enough now with no reception issues. The IR receiver is
 another matter and I was unsuccesful in making it work for now (I guess
 noone got it working with the old versions neither as it lacks the ir
 entries).
 So could this card be added? I've added the trivial patch.
 
 Could you please add your Signed-of-by: to the patch?

Sure. Here's this masterpiece of code again :-).

Roland


From 03b68ff4f13736cf59140e090bf1609accd8dcb0 Mon Sep 17 00:00:00 2001
From: Roland Scheidegger rscheidegger_li...@hispeed.ch
Date: Sat, 9 Feb 2013 01:08:55 +0100
Subject: [PATCH] [media] em28xx: add usb id for terratec h5 rev. 3
Cc: Linux Media Mailing List linux-media@vger.kernel.org

Seems to work just the same as older revisions.

Signed-off-by: Roland Scheidegger rscheidegger_li...@hispeed.ch
---
 drivers/media/usb/em28xx/em28xx-cards.c |2 ++
 1 Datei geändert, 2 Zeilen hinzugefügt(+)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 0a5aa62..5b24594 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2080,6 +2080,8 @@ struct usb_device_id em28xx_id_table[] = {
 			.driver_info = EM2884_BOARD_TERRATEC_H5 },
 	{ USB_DEVICE(0x0ccd, 0x10ad),	/* H5 Rev. 2 */
 			.driver_info = EM2884_BOARD_TERRATEC_H5 },
+	{ USB_DEVICE(0x0ccd, 0x10b6),	/* H5 Rev. 3 */
+			.driver_info = EM2884_BOARD_TERRATEC_H5 },			
 	{ USB_DEVICE(0x0ccd, 0x0084),
 			.driver_info = EM2860_BOARD_TERRATEC_AV350 },
 	{ USB_DEVICE(0x0ccd, 0x0096),
-- 
1.7.10.4



Re: [GIT PULL FOR v3.8] Regression fix: cx18/ivtv: remove __init from a non-init function.

2013-02-08 Thread Mauro Carvalho Chehab
Em Fri, 8 Feb 2013 09:40:27 +0100
Hans Verkuil hverk...@xs4all.nl escreveu:

 Mauro,
 
 Please fast-track this for 3.8. Yesterday I discovered that commits made 
 earlier
 for 3.8 kill ivtv and cx18 (as in: unable to boot, instant crash) since a
 function was made __init that was actually called *after* initialization.
 
 We are already at rc6 and this *must* make it for 3.8. Without this patch
 anyone with a cx18/ivtv will crash immediately as soon as they upgrade to 3.8.
 
 Regards,
 
   Hans
 
 The following changes since commit 248ac368ce4b3cd36515122d888403909d7a2500:
 
   [media] s5p-fimc: Fix fimc-lite entities deregistration (2013-02-06 
 09:42:19 -0200)
 
 are available in the git repository at:
 
   git://linuxtv.org/hverkuil/media_tree.git ivtv
 
 for you to fetch changes up to ddf276062e68607323fca363b99bdf426dddad9b:
 
   cx18/ivtv: fix regression: remove __init from a non-init function. 
 (2013-02-08 09:30:11 +0100)
 
 
 Hans Verkuil (1):
   cx18/ivtv: fix regression: remove __init from a non-init function.

Hmm... the patch seems to be broken/incomplete:


WARNING: drivers/media/pci/cx18/cx18-alsa.o(.text+0x449): Section mismatch in 
reference from the function cx18_alsa_load() to the function 
.init.text:snd_cx18_pcm_create()
The function cx18_alsa_load() references
the function __init snd_cx18_pcm_create().
This is often because cx18_alsa_load lacks a __init 
annotation or the annotation of snd_cx18_pcm_create is wrong.

WARNING: drivers/media/pci/cx18/built-in.o(.text+0x1be69): Section mismatch in 
reference from the function cx18_alsa_load() to the function 
.init.text:snd_cx18_pcm_create()
The function cx18_alsa_load() references
the function __init snd_cx18_pcm_create().
This is often because cx18_alsa_load lacks a __init 
annotation or the annotation of snd_cx18_pcm_create is wrong.

WARNING: drivers/media/pci/ivtv/ivtv-alsa.o(.text+0x454): Section mismatch in 
reference from the function ivtv_alsa_load() to the function 
.init.text:snd_ivtv_pcm_create()
The function ivtv_alsa_load() references
the function __init snd_ivtv_pcm_create().
This is often because ivtv_alsa_load lacks a __init 
annotation or the annotation of snd_ivtv_pcm_create is wrong.

WARNING: drivers/media/pci/ivtv/built-in.o(.text+0x20790): Section mismatch in 
reference from the function ivtv_alsa_load() to the function 
.init.text:snd_ivtv_pcm_create()
The function ivtv_alsa_load() references
the function __init snd_ivtv_pcm_create().
This is often because ivtv_alsa_load lacks a __init 
annotation or the annotation of snd_ivtv_pcm_create is wrong.

WARNING: drivers/media/pci/built-in.o(.text+0x6b958): Section mismatch in 
reference from the function ivtv_alsa_load() to the function 
.init.text:snd_ivtv_pcm_create()
The function ivtv_alsa_load() references
the function __init snd_ivtv_pcm_create().
This is often because ivtv_alsa_load lacks a __init 
annotation or the annotation of snd_ivtv_pcm_create is wrong.

WARNING: drivers/media/pci/built-in.o(.text+0x9fc21): Section mismatch in 
reference from the function cx18_alsa_load() to the function 
.init.text:snd_cx18_pcm_create()
The function cx18_alsa_load() references
the function __init snd_cx18_pcm_create().
This is often because cx18_alsa_load lacks a __init 
annotation or the annotation of snd_cx18_pcm_create is wrong.

WARNING: drivers/media/built-in.o(.text+0x289f48): Section mismatch in 
reference from the function ivtv_alsa_load() to the function 
.init.text:snd_ivtv_pcm_create()
The function ivtv_alsa_load() references
the function __init snd_ivtv_pcm_create().
This is often because ivtv_alsa_load lacks a __init 
annotation or the annotation of snd_ivtv_pcm_create is wrong.

WARNING: drivers/media/built-in.o(.text+0x2be211): Section mismatch in 
reference from the function cx18_alsa_load() to the function 
.init.text:snd_cx18_pcm_create()
The function cx18_alsa_load() references
the function __init snd_cx18_pcm_create().
This is often because cx18_alsa_load lacks a __init 
annotation or the annotation of snd_cx18_pcm_create is wrong.
-- 

Cheers,
Mauro
--
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: rc: gpio-ir-recv: add support for device tree parsing

2013-02-08 Thread Sebastian Hesselbarth

On 02/09/2013 01:03 AM, Mauro Carvalho Chehab wrote:

Em Fri,  8 Feb 2013 21:38:07 +0100
Sebastian Hesselbarthsebastian.hesselba...@gmail.com  escreveu:


This patch adds device tree parsing for gpio_ir_recv platform_data and
the mandatory binding documentation. It basically follows what we already
have for e.g. gpio_keys. All required device tree properties are OS
independent but an optional property allow linux specific support for rc
maps.

There was a similar patch sent by Matus Ujhelyi but that discussion
died after the first reviews.

Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com
---
Changelog

v1-v2:
- get rid of ptr returned by _get_devtree_pdata()
- check for of_node instead for NULL pdata
- remove unneccessary double check for gpios property
- remove unneccessary #ifdef CONFIG_OF around match table

Cc: Grant Likelygrant.lik...@secretlab.ca
Cc: Rob Herringrob.herr...@calxeda.com
Cc: Rob Landleyr...@landley.net
Cc: Mauro Carvalho Chehabmche...@redhat.com
Cc: Sebastian Hesselbarthsebastian.hesselba...@gmail.com
Cc: Benoit Thebaudeaubenoit.thebaud...@advansee.com
Cc: David Hardemanda...@hardeman.nu
Cc: Trilok Sonits...@codeaurora.org
Cc: Sylwester Nawrockis.nawro...@samsung.com
Cc: Matus Ujhelyiujhely...@gmail.com
Cc: devicetree-disc...@lists.ozlabs.org
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
---
  .../devicetree/bindings/media/gpio-ir-receiver.txt |   16 ++
  drivers/media/rc/gpio-ir-recv.c|   57 
  2 files changed, 73 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/media/gpio-ir-receiver.txt

diff --git a/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt 
b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
new file mode 100644
index 000..8589f30
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt
@@ -0,0 +1,16 @@
+Device-Tree bindings for GPIO IR receiver
+
+Required properties:
+   - compatible = gpio-ir-receiver;
+   - gpios: OF device-tree gpio specification.
+
+Optional properties:
+   - linux,rc-map-name: Linux specific remote control map name.
+
+Example node:
+
+   ir: ir-receiver {
+   compatible = gpio-ir-receiver;
+   gpios =gpio0 19 1;
+   linux,rc-map-name = rc-rc6-mce;


Please change this to:
linux,rc-map-name = RC_MAP_RC6_MCE;

(as defined at include/media/rc-map.h).


Mauro,

this is not possible in device tree bindings. Device tree properties
can only carry numeric or string types (and some other stuff) but no
OS specific enumerations. So using strings is the only option here.


The idea of having those strings defined at the same header file is to:


Unfortunately, device tree blobs don't know about linux header files.

That leaves two options:
- allow the user to supply a string of the map in his device tree description
  and risk that there may be a broken map name
- remove linux,rc-map-name from DT binding and let the user configure in
  from user space (which is propably best choice anyway)

I tried both, DT supplied map name and ir-keytable from userspace
both work fine.

Sebastian


- make sure that the same keyboard is spelled at the same way on
all places;

- avoid people to duplicate IR keytables, using different names;

- help userspace to get the right table. In the future, the
plan is to remove all keytables from kernelspace, keeping there just the
name of the keytable. The existing userspace code (ir-keytables, part
of v4l-utils) use the keytable name to dynamically load the right table
in runtime and to switch the IR core to only handle the protocol that
it is associated with the loaded keytable.




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