Re: [PATCH] [media] vb2: move dma-buf unmap from __vb2_dqbuf() to vb2_buffer_done()

[Luis de Bethencourt]

On 20/07/16 19:22, Javier Martinez Canillas wrote:
> Currently the dma-buf is unmapped when the buffer is dequeued by userspace
> but it's not used anymore after the driver finished processing the buffer.
> 
> So instead of doing the dma-buf unmapping in __vb2_dqbuf(), it can be made
> in vb2_buffer_done() after the driver notified that buf processing is done.
> 
> Decoupling the buffer dequeue from the dma-buf unmapping has also the side
> effect of making possible to add dma-buf fence support in the future since
> the buffer could be dequeued even before the driver has finished using it.
> 
> Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
> 
> ---
> Hello,
> 
> I've tested this patch doing DMA buffer sharing between a
> vivid input and output device with both v4l2-ctl and gst:
> 
> $ v4l2-ctl -d0 -e1 --stream-dmabuf --stream-out-mmap
> $ v4l2-ctl -d0 -e1 --stream-mmap --stream-out-dmabuf
> $ gst-launch-1.0 v4l2src device=/dev/video0 io-mode=dmabuf ! v4l2sink 
> device=/dev/video1 io-mode=dmabuf-import
> 
> And I didn't find any issues but more testing will be appreciated.
> 
> Best regards,
> Javier
> 

Hello all,

Tested this using the same GStreamer pipeline as Javier mentions above.
It works nicely.

Thanks,
Luis

Tested-by: Luis de Bethencourt <lui...@osg.samsung.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: [PATCH] [media] vb2: map dmabuf for planes on driver queue instead of vidioc_qbuf

[Luis de Bethencourt]

On 15/07/16 17:26, Javier Martinez Canillas wrote:
> The buffer planes' dma-buf are currently mapped when buffers are queued
> from userspace but it's more appropriate to do the mapping when buffers
> are queued in the driver since that's when the actual DMA operation are
> going to happen.
> 
> Suggested-by: Nicolas Dufresne <nicolas.dufre...@collabora.com>
> Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
> 
> ---
> 
> Hello,
> 
> A side effect of this change is that if the dmabuf map fails for some
> reasons (i.e: a driver using the DMA contig memory allocator but CMA
> not being enabled), the fail will no longer happen on VIDIOC_QBUF but
> later (i.e: in VIDIOC_STREAMON).
> 
> I don't know if that's an issue though but I think is worth mentioning.
> 
> Best regards,
> Javier
> 

Just run this path on the ODROID using GStreamer and the vivid driver.
It worked nicely.

Tested-by: Luis de Bethencourt <lui...@osg.samsung.com>

Thanks Javier,
Luis

--
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] media: s5p-mfc fix invalid memory access from s5p_mfc_release()

[Luis de Bethencourt]

On 08/07/16 23:29, Shuah Khan wrote:
> If s5p_mfc_release() runs after s5p_mfc_remove(), the former will access
> invalid s5p_mfc_dev pointer saved in the s5p_mfc_ctx and runs into kernel
> paging request errors.
> 
> Clear ctx dev pointer in s5p_mfc_remove() and change s5p_mfc_release() to
> avoid work that requires ctx->dev.
> 
> odroid kernel: Unable to handle kernel paging request at virtual address
> f17c1104
> odroid kernel: pgd = ebca4000
> odroid kernel: [f17c1104] *pgd=6e23d811, *pte=, *ppte=
> odroid kernel: Internal error: Oops: 807 [#1] PREEMPT SMP ARM
> odroid kernel: Modules linked in: cpufreq_userspace cpufreq_powersave
> cpufreq_conservative s5p_mfc s5p_jpeg v4l2_mem2mem
> videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_core
> v4l2_common videodev media
> odroid kernel: Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> odroid kernel: task: c2241400 ti: e7018000 task.ti: e7018000
> odroid kernel: PC is at s5p_mfc_reset+0x40/0x28c [s5p_mfc]
> odroid kernel: LR is at s5p_mfc_reset+0x34/0x28c [s5p_mfc]
> odroid kernel: pc : []lr : [] psr: 60010013
> odroid kernel: [] (s5p_mfc_reset [s5p_mfc]) from []
> (s5p_mfc_deinit_hw+0x14/0x3c [s5p_mfc])
> odroid kernel: [] (s5p_mfc_deinit_hw [s5p_mfc]) from []
> (s5p_mfc_release+0xac/0x1c4 [s5p_mfc])
> odroid kernel: [] (s5p_mfc_release [s5p_mfc]) from []
> (v4l2_release+0x38/0x74 [videodev])
> odroid kernel: [] (v4l2_release [videodev]) from []
> (__fput+0x80/0x1c8)
> odroid kernel: [] (__fput) from []
> (task_work_run+0x94/0xc8)
> odroid kernel: [] (task_work_run) from []
> (do_work_pending+0x7c/0xa4)
> odroid kernel: [] (do_work_pending) from []
> (slow_work_pending+0xc/0x20)
> odroid kernel: Code: eb3edacc e5953078 e3a06000 e2833c11 (e5836004)
> 
> Signed-off-by: Shuah Khan <shua...@osg.samsung.com>
> ---

Thanks Shuah.

I've been following this while playing with an ODROID XU4 to fix some issues
in v4l2 usage in GStreamer. So I offered Shuah to test this for her.

Looks good :)

Tested-by: Luis de Bethencourt <lui...@osg.samsung.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: [PATCH 11/19] cx25821-alsa: shutup a Gcc 6.1 warning

[Luis de Bethencourt]

On 24/06/16 16:31, Mauro Carvalho Chehab wrote:
> The PCI device ID table is only used if compiled with modules
> support. When compiled with modules disabled, this is now
> producing this bogus warning:
> 
> drivers/media/pci/cx25821/cx25821-alsa.c:696:35: warning: 
> 'cx25821_audio_pci_tbl' defined but not used [-Wunused-const-variable=]
>  static const struct pci_device_id cx25821_audio_pci_tbl[] = {
>^
> 
> Fix it by annotating that the function may not be used.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/pci/cx25821/cx25821-alsa.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/pci/cx25821/cx25821-alsa.c 
> b/drivers/media/pci/cx25821/cx25821-alsa.c
> index b602eba2b601..df189b16af12 100644
> --- a/drivers/media/pci/cx25821/cx25821-alsa.c
> +++ b/drivers/media/pci/cx25821/cx25821-alsa.c
> @@ -693,7 +693,7 @@ static int snd_cx25821_pcm(struct cx25821_audio_dev 
> *chip, int device,
>   * Only boards with eeprom and byte 1 at eeprom=1 have it
>   */
>  
> -static const struct pci_device_id cx25821_audio_pci_tbl[] = {
> +static const struct pci_device_id __maybe_unused cx25821_audio_pci_tbl[] = {
>   {0x14f1, 0x0920, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
>   {0,}
>  };
> 

In which branch is this happening? I can't seem to be able to reproduce it.

Thanks,
Luis
--
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: [RESEND] fence: add missing descriptions for fence

[Luis de Bethencourt]

On 11/04/16 21:09, Gustavo Padovan wrote:
> Hi Luis,
> 
> 2016-04-11 Luis de Bethencourt <lui...@osg.samsung.com>:
> 
>> The members child_list and active_list were added to the fence struct
>> without descriptions for the Documentation. Adding these.
>>
>> Fixes: b55b54b5db33 ("staging/android: remove struct sync_pt")
>> Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
>> Reviewed-by: Javier Martinez Canillas <jav...@osg.samsung.com>
>> ---
>> Hi,
>>
>> Just resending this patch since it hasn't had any reviews in since
>> March 21st.
>>
>> Thanks,
>> Luis
>>
>>  include/linux/fence.h | 2 ++
>>  1 file changed, 2 insertions(+)
> 
> Reviewed-by: Gustavo Padovan <gustavo.pado...@collabora.co.uk>
> 
>   Gustavo
> 

Thank you Gustavo.

Nice seeing you around here :)

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

[RESEND] fence: add missing descriptions for fence

[Luis de Bethencourt]

The members child_list and active_list were added to the fence struct
without descriptions for the Documentation. Adding these.

Fixes: b55b54b5db33 ("staging/android: remove struct sync_pt")
Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
Reviewed-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---
Hi,

Just resending this patch since it hasn't had any reviews in since
March 21st.

Thanks,
Luis

 include/linux/fence.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/fence.h b/include/linux/fence.h
index 2b17698..2056e9f 100644
--- a/include/linux/fence.h
+++ b/include/linux/fence.h
@@ -49,6 +49,8 @@ struct fence_cb;
  * @timestamp: Timestamp when the fence was signaled.
  * @status: Optional, only valid if < 0, must be set before calling
  * fence_signal, indicates that the fence has completed with an error.
+ * @child_list: list of children fences
+ * @active_list: list of active fences
  *
  * the flags member must be manipulated and read using the appropriate
  * atomic ops (bit_*), so taking the spinlock will not be needed most
-- 
2.6.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 v2] fence: add missing descriptions for fence

[Luis de Bethencourt]

The members child_list and active_list were added to the fence struct
without descriptions for the Documentation. Adding these.

Fixes: b55b54b5db33 ("staging/android: remove struct sync_pt")
Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
Reviewed-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---
Hi,

This second version fixes the commit message as suggested by Javier Martinez.
https://lkml.org/lkml/2016/3/19/135

Thanks,
Luis

 include/linux/fence.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/fence.h b/include/linux/fence.h
index bb52201..ba5b678 100644
--- a/include/linux/fence.h
+++ b/include/linux/fence.h
@@ -49,6 +49,8 @@ struct fence_cb;
  * @timestamp: Timestamp when the fence was signaled.
  * @status: Optional, only valid if < 0, must be set before calling
  * fence_signal, indicates that the fence has completed with an error.
+ * @child_list: list of children fences
+ * @active_list: list of active fences
  *
  * the flags member must be manipulated and read using the appropriate
  * atomic ops (bit_*), so taking the spinlock will not be needed most
-- 
2.6.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] fence: add missing descriptions for fence

[Luis de Bethencourt]

On 19/03/16 23:55, Javier Martinez Canillas wrote:
> Hello Luis,
> 
> On Sat, Mar 19, 2016 at 4:50 PM, Luis de Bethencourt
> <lui...@osg.samsung.com> wrote:
>> Commit b55b54b5db33 ("staging/android: remove struct sync_pt")
>> added the members child_list and active_list to the fence struct, but
>> didn't add descriptions for these. Adding the descriptions.
>>
> 
> Patches whose commit message mentions a specific commit that
> introduced and issue, usually also have a "Fixes:" tag before the
> S-o-B. For example this patch should have:
> 
> Fixes: b55b54b5db33 ("staging/android: remove struct sync_pt")
>> Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
>> ---
>> Hi,
>>
>> Noticed this missing descriptions when running make htmldocs.
>>
>> Got the following warnings:
>> .//include/linux/fence.h:84: warning: No description found for parameter 
>> 'child_list'
>> .//include/linux/fence.h:84: warning: No description found for parameter 
>> 'active_list'
>>
>> Thanks :)
>> Luis
>>
> 
> Patch looks good to me.
> 
> Reviewed-by: Javier Martinez Canillas <jav...@osg.samsung.com>
> 
> Best regards,
> Javier
> 

Hi Javier,

I didn't knew that, but thanks for saying so I can learn and use it in the
future.

I used the 'Commit b55b54b5db33 ("staging/android: remove struct sync_pt")'
format because that is what checkpatch recommended. But after re-reading
Documentation/SubmittingPatches it all makes sense and the process is clear
in my head.

Thanks!
Luis
--
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] fence: add missing descriptions for fence

[Luis de Bethencourt]

Commit b55b54b5db33 ("staging/android: remove struct sync_pt")
added the members child_list and active_list to the fence struct, but
didn't add descriptions for these. Adding the descriptions.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
Hi,

Noticed this missing descriptions when running make htmldocs.

Got the following warnings:
.//include/linux/fence.h:84: warning: No description found for parameter 
'child_list'
.//include/linux/fence.h:84: warning: No description found for parameter 
'active_list'

Thanks :)
Luis

 include/linux/fence.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/fence.h b/include/linux/fence.h
index 2b17698..2056e9f 100644
--- a/include/linux/fence.h
+++ b/include/linux/fence.h
@@ -49,6 +49,8 @@ struct fence_cb;
  * @timestamp: Timestamp when the fence was signaled.
  * @status: Optional, only valid if < 0, must be set before calling
  * fence_signal, indicates that the fence has completed with an error.
+ * @child_list: list of children fences
+ * @active_list: list of active fences
  *
  * the flags member must be manipulated and read using the appropriate
  * atomic ops (bit_*), so taking the spinlock will not be needed most
-- 
2.5.1

--
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: Failed to build on 4.2.6

[Luis de Bethencourt]

Greg Kroah-Hartman writes:

> On Mon, Dec 07, 2015 at 10:25:19AM -0500, Steven Rostedt wrote:
>> Hi,
>>
>> The attached config doesn't build on 4.2.6, but changing it to the
>> following:
>>
>>  VIDEO_V4L2_SUBDEV_API n -> y
>> +V4L2_FLASH_LED_CLASS n
>>
>> does build.
>
> Did this work on older kernels (4.2.5?  .4?  older?)
>
> thanks,
>
> greg k-h

Hi all,

The problem was:

drivers/media/i2c/adv7604.c: In function ‘adv76xx_get_format’:
drivers/media/i2c/adv7604.c:1861:3: error: implicit declaration of function 
‘v4l2_subdev_get_try_format’ [-Werror=implicit-function-declaration]
   fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);

As Randy mentioned, this if fixed by commit
fc88dd16a0e430f57458e6bd9b62a631c6ea53a1

I backported it locally to test this and build worked fine.

Luis
--
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] go7007: fix returned errno codes in do_special()

[Luis de Bethencourt]

The driver is using -1 instead of the -ENOMEM defined macro to specify
that a buffer allocation failed. Since the error number is propagated,
the caller will get a -EPERM which is the wrong error condition.

Also, the smatch tool complains with the following warnings:

gen_mjpeghdr_to_package() warn: returning -1 instead of -ENOMEM is sloppy
gen_mpeg1hdr_to_package() warn: returning -1 instead of -ENOMEM is sloppy
gen_mpeg4hdr_to_package() warn: returning -1 instead of -ENOMEM is sloppy

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---
 drivers/media/usb/go7007/go7007-fw.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/go7007/go7007-fw.c 
b/drivers/media/usb/go7007/go7007-fw.c
index 5f4c9b9..60bf5f0 100644
--- a/drivers/media/usb/go7007/go7007-fw.c
+++ b/drivers/media/usb/go7007/go7007-fw.c
@@ -379,7 +379,7 @@ static int gen_mjpeghdr_to_package(struct go7007 *go, 
__le16 *code, int space)
 
buf = kzalloc(4096, GFP_KERNEL);
if (buf == NULL)
-   return -1;
+   return -ENOMEM;
 
for (i = 1; i < 32; ++i) {
mjpeg_frame_header(go, buf + size, i);
@@ -646,7 +646,7 @@ static int gen_mpeg1hdr_to_package(struct go7007 *go,
 
buf = kzalloc(5120, GFP_KERNEL);
if (buf == NULL)
-   return -1;
+   return -ENOMEM;
 
framelen[0] = mpeg1_frame_header(go, buf, 0, 1, PFRAME);
if (go->interlace_coding)
@@ -832,7 +832,7 @@ static int gen_mpeg4hdr_to_package(struct go7007 *go,
 
buf = kzalloc(5120, GFP_KERNEL);
if (buf == NULL)
-   return -1;
+   return -ENOMEM;
 
framelen[0] = mpeg4_frame_header(go, buf, 0, PFRAME);
i = 368;
-- 
2.5.1

--
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] cx25821, cx88, tm6000: use SNDRV_DEFAULT_ENABLE_PNP

[Luis de Bethencourt]

Instead of manually initializing the bool array enable, use the
SNDRV_DEFAULT_ENABLE_PNP macro. As most drivers do.

Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com>
---

Hi,

I don't see any good reason to use = 1 instead of = SNDRV_DEFAULT_ENABLE_PNP.
Semantically it is the same, and I don't find a purpose to explicitely set the
first element of the array separately.

The proposed patch makes these drivers consistent with the rest and more
readable. In a related note, maybe the dummy driver in sound/drivers/dummy.c
should be updated as well.

I can split this below fix into a patch per driver if that's better.

Thanks,
Luis

 drivers/media/pci/cx25821/cx25821-alsa.c | 2 +-
 drivers/media/pci/cx88/cx88-alsa.c   | 2 +-
 drivers/media/usb/tm6000/tm6000-alsa.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/cx25821/cx25821-alsa.c 
b/drivers/media/pci/cx25821/cx25821-alsa.c
index 24f964b..b602eba 100644
--- a/drivers/media/pci/cx25821/cx25821-alsa.c
+++ b/drivers/media/pci/cx25821/cx25821-alsa.c
@@ -102,7 +102,7 @@ struct cx25821_audio_dev {
 
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; /* Index 0-MAX */
 static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;  /* ID for this card */
-static bool enable[SNDRV_CARDS] = { 1, [1 ... (SNDRV_CARDS - 1)] = 1 };
+static bool enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE_PNP;
 
 module_param_array(enable, bool, NULL, 0444);
 MODULE_PARM_DESC(enable, "Enable cx25821 soundcard. default enabled.");
diff --git a/drivers/media/pci/cx88/cx88-alsa.c 
b/drivers/media/pci/cx88/cx88-alsa.c
index 7f8dc60..57ddf8a 100644
--- a/drivers/media/pci/cx88/cx88-alsa.c
+++ b/drivers/media/pci/cx88/cx88-alsa.c
@@ -101,7 +101,7 @@ typedef struct cx88_audio_dev snd_cx88_card_t;
 
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; /* Index 0-MAX */
 static const char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;/* ID for this 
card */
-static bool enable[SNDRV_CARDS] = {1, [1 ... (SNDRV_CARDS - 1)] = 1};
+static bool enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE_PNP;
 
 module_param_array(enable, bool, NULL, 0444);
 MODULE_PARM_DESC(enable, "Enable cx88x soundcard. default enabled.");
diff --git a/drivers/media/usb/tm6000/tm6000-alsa.c 
b/drivers/media/usb/tm6000/tm6000-alsa.c
index 74e5697..e21c7aa 100644
--- a/drivers/media/usb/tm6000/tm6000-alsa.c
+++ b/drivers/media/usb/tm6000/tm6000-alsa.c
@@ -42,7 +42,7 @@
 
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; /* Index 0-MAX */
 
-static bool enable[SNDRV_CARDS] = {1, [1 ... (SNDRV_CARDS - 1)] = 1};
+static bool enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE_PNP;
 
 module_param_array(enable, bool, NULL, 0444);
 MODULE_PARM_DESC(enable, "Enable tm6000x soundcard. default enabled.");
-- 
2.4.6

--
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] media: cxd2099: move pre-init values out of init()

[Luis de Bethencourt]

On Wed, Apr 08, 2015 at 08:09:03AM -0300, Mauro Carvalho Chehab wrote:
 Em Sun, 8 Feb 2015 20:55:36 +
 luisbg l...@debethencourt.com escreveu:
 
  Improve code readability by moving out all pre-init values from the init
  function.
  
  Signed-off-by: Luis de Bethencourt luis...@samsung.com
  ---
   drivers/staging/media/cxd2099/cxd2099.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/staging/media/cxd2099/cxd2099.c 
  b/drivers/staging/media/cxd2099/cxd2099.c
  index 657ea48..bafe36f 100644
  --- a/drivers/staging/media/cxd2099/cxd2099.c
  +++ b/drivers/staging/media/cxd2099/cxd2099.c
  @@ -300,7 +300,6 @@ static int init(struct cxd *ci)
  int status;
   
  mutex_lock(ci-lock);
  -   ci-mode = -1;
  do {
  status = write_reg(ci, 0x00, 0x00);
  if (status  0)
  @@ -420,7 +419,6 @@ static int init(struct cxd *ci)
  status = write_regm(ci, 0x09, 0x08, 0x08);
  if (status  0)
  break;
  -   ci-cammode = -1;
  cam_mode(ci, 0);
  } while (0);
  mutex_unlock(ci-lock);
  @@ -711,6 +709,8 @@ struct dvb_ca_en50221 *cxd2099_attach(struct 
  cxd2099_cfg *cfg,
   
  ci-en = en_templ;
  ci-en.data = ci;
  +   ci-mode = -1;
  +   ci-cammode = -1;
 
 This actually changes the logic, as, cammode is == -1 only if the
 do {} while loop succeeds.
 
 Also, calling cam_mode(ci, 0) will change cammode to 0. Btw, for
 it to work, ci-mode should be initialized earlier.
 
 So, this patch looks very wrong on my eyes, except if you found
 a real bug on it.
 
 Have you tested it on a real device? What bug does it fix?
 
 Regards,
 Mauro
 

Apologies. You are right and this patch is completely wrong.

I submitted this while reading the code of a few drivers to learn from them.
I should've retracted it before you spent time reviewing it. Sorry for that.

Thanks,
Luis

  init(ci);
  dev_info(i2c-dev, Attached CXD2099AR at %02x\n, ci-cfg.adr);
  return ci-en;
--
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: Opening firmware source code (vhdl)

[Luis de Bethencourt]

On Mon, Feb 16, 2015 at 11:04:47AM -0500, Abylay Ospan wrote:
 Hello,
 
 We're fully opening firmware sources for our new card - NetUP Dual
 Universal DVB CI. License is GPLv3. Sources is VHDL for Altera FPGA
 EP4CGX22CF19C8
 and can be compiled with Altera Quartus II (free edition). Hope this
 will help for enthusiasts and developers to deeply understand hardware
 part of DVB card.
 
 Source code:
 https://github.com/aospan/NetUP_Dual_Universal_CI-fpga
 
 Here is a description for building and uploading fw into DVB card:
 http://linuxtv.org/wiki/index.php/FPGA_fw_for_NetUP_Dual_Universal_CI
 
 Feel free to contact me for any questions or comments.
 
 -- 
 Abylay Ospan,
 NetUP Inc.
 http://www.netup.tv
 --

Thanks for open sourcing the firmware of your new card!

I am sure the owners of this hardware will appreciate this.

Luis
--
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: Recent commit introduces compiler Error in some platforms

[Luis de Bethencourt]

On Mon, Feb 16, 2015 at 01:36:43PM +0100, Hans Verkuil wrote:
 On 02/16/2015 01:18 PM, Luis de Bethencourt wrote:
  Hi all,
  
  As can be seen in Han's build log:
  http://hverkuil.home.xs4all.nl/logs/Saturday.log
  
  The recent commit bc0c5aa35ac88342831933ca7758ead62d9bae2b introduces a
  compiler error in some platforms.
  
  /home/hans/work/build/media_build/v4l/ir-hix5hd2.c: In function 
  'hix5hd2_ir_config':
  /home/hans/work/build/media_build/v4l/ir-hix5hd2.c:95:2: error: implicit 
  declaration of function 'writel_relaxed' 
  [-Werror=implicit-function-declaration]
writel_relaxed(0x01, priv-base + IR_ENABLE);
^
  
  Better than reverting, what would be a good solution for this problem?
  I am happy to implment it once I know what is the right direction.
  
  From what I see that commit mentions that the function is now available from
  include/asm-generic/io.h, but this isn't included.
 
 I've just fixed the media_build repository to handle this. Do a git pull and
 it should compile again (only tested against kernel 3.18).
 
 Regards,
 
   Hans
 

Great! Nice to know this is already fixed.

Thanks,
Luis

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

Recent commit introduces compiler Error in some platforms

[Luis de Bethencourt]

Hi all,

As can be seen in Han's build log:
http://hverkuil.home.xs4all.nl/logs/Saturday.log

The recent commit bc0c5aa35ac88342831933ca7758ead62d9bae2b introduces a
compiler error in some platforms.

/home/hans/work/build/media_build/v4l/ir-hix5hd2.c: In function 
'hix5hd2_ir_config':
/home/hans/work/build/media_build/v4l/ir-hix5hd2.c:95:2: error: implicit 
declaration of function 'writel_relaxed' [-Werror=implicit-function-declaration]
  writel_relaxed(0x01, priv-base + IR_ENABLE);
  ^

Better than reverting, what would be a good solution for this problem?
I am happy to implment it once I know what is the right direction.

From what I see that commit mentions that the function is now available from
include/asm-generic/io.h, but this isn't included.

Thanks,
Luis
--
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] rtl2832: remove compiler warning

[Luis de Bethencourt]

On Wed, Feb 11, 2015 at 11:08:51AM +, Luis de Bethencourt wrote:
 Cleaning up the following compiler warning:
 rtl2832.c:703:12: warning: 'tmp' may be used uninitialized in this function
 
 Even though it could never happen since if rtl2832_rd_demod_reg () doesn't set
 tmp, this line would never run because we go to err. It is still nice to avoid
 compiler warnings.
 
 Signed-off-by: Luis de Bethencourt luis...@samsung.com
 ---
  drivers/media/dvb-frontends/rtl2832.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/media/dvb-frontends/rtl2832.c 
 b/drivers/media/dvb-frontends/rtl2832.c
 index 5d2d8f4..20fa245 100644
 --- a/drivers/media/dvb-frontends/rtl2832.c
 +++ b/drivers/media/dvb-frontends/rtl2832.c
 @@ -685,7 +685,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, 
 fe_status_t *status)
   struct rtl2832_dev *dev = fe-demodulator_priv;
   struct i2c_client *client = dev-client;
   int ret;
 - u32 tmp;
 + u32 uninitialized_var(tmp);
  
   dev_dbg(client-dev, \n);
  
 -- 
 2.1.3
 

Hi Mauro,

The warning is still happening. Just a reminder in case you want to apply this
patch.

Thanks,
Luis
--
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] media: Pinnacle 73e infrared control stopped working since kernel 3.17

[Luis de Bethencourt]

On Thu, Feb 12, 2015 at 08:15:10AM +0100, David Cimbůrek wrote:
 I'll try to describe my thoughts.
 
 The changed structure dib0700_rc_response is used in
 dib0700_core.c:dib0700_rc_urb_completion(struct urb *purb) function:
 
 struct dib0700_rc_response *poll_reply;
 ...
 poll_reply = purb-transfer_buffer;
 
 dib0700_rc_urb_completion() is then used in
 dib0700_core.c:dib0700_rc_setup() in macros usb_fill_bulk_urb and
 usb_fill_int_urb. These macros are defined in header file usb.h. Here
 I have found in macro description this:
 
  * @transfer_buffer: pointer to the transfer buffer
 
 I suppose that it means that the struct dib0700_rc_response is being
 filled from this transfer buffer. Therefore I suppose that the order
 of structure members IS important.
 
 Of course it's only my guess but my patch is really working for me :-)
 

Hi David,

I am away from the keyboard most of today. Busy with a training.

I will follow the code you describe as soon as I can in a few hours and report
back.

Thanks,
Luis

 
 
 2015-02-12 1:10 GMT+01:00 Luis de Bethencourt l...@debethencourt.com:
  On Tue, Feb 10, 2015 at 11:38:11AM +0100, David Cimbůrek wrote:
  Please include this patch to kernel! It takes too much time for such a
  simple fix!
 
 
  The patch is simple but why it fixes the issue isn't that simple. Could you
  explain why the order of the variables inside the structure is breaking 
  things?
 
  All the uses of the variables inside the structure that I can see are by 
  name.
  Not by memory offsets.
 
  Thanks,
  Luis
 
 
  2015-01-07 13:51 GMT+01:00 David Cimbůrek david.cimbu...@gmail.com:
   No one is interested? I'd like to get this patch to kernel to fix the
   issue. Can someone here do it please?
  
  
   2014-12-20 14:36 GMT+01:00 David Cimbůrek david.cimbu...@gmail.com:
   Hi,
  
   with kernel 3.17 remote control for Pinnacle 73e (ID 2304:0237
   Pinnacle Systems, Inc. PCTV 73e [DiBcom DiB7000PC]) does not work
   anymore.
  
   I checked the changes and found out the problem in commit
   af3a4a9bbeb00df3e42e77240b4cdac5479812f9.
  
   In dib0700_core.c in struct dib0700_rc_response the following union:
  
   union {
   u16 system16;
   struct {
   u8 not_system;
   u8 system;
   };
   };
  
   has been replaced by simple variables:
  
   u8 system;
   u8 not_system;
  
   But these variables are in reverse order! When I switch the order
   back, the remote works fine again! Here is the patch:
  
  
   --- a/drivers/media/usb/dvb-usr/dib0700_core.c2014-12-20
   14:27:15.0 +0100
   +++ b/drivers/media/usb/dvb-usr/dib0700_core.c2014-12-20
   14:27:36.0 +0100
   @@ -658,8 +658,8 @@
struct dib0700_rc_response {
u8 report_id;
u8 data_state;
   -u8 system;
u8 not_system;
   +u8 system;
u8 data;
u8 not_data;
};
  
  
   Regards,
   David
  --
  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
 --
 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
--
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] media: Pinnacle 73e infrared control stopped working since kernel 3.17

[Luis de Bethencourt]

On Thu, Feb 12, 2015 at 06:34:40PM +0100, David Cimbůrek wrote:
 2015-02-12 12:50 GMT+01:00 Mauro Carvalho Chehab mche...@osg.samsung.com:
  Em Wed, 11 Feb 2015 17:41:01 +0100
  David Cimbůrek david.cimbu...@gmail.com escreveu:
 
  Please don't top post. I reordered the messages below in order to get some
  sanity.
 
 
  2015-02-11 15:40 GMT+01:00 David Härdeman da...@hardeman.nu:
   Can you generate some scancodes before and after commit
   af3a4a9bbeb00df3e42e77240b4cdac5479812f9?
 
  Let me know what exactly do you want me to do (which commands, which
  traces etc.). I'm not very familiar with the Linux media stuff...
 
  As root, you should run:
 
  # ir-keytable -r
 
  This will print the scancodes and their key associations.
 
  Also, on what architecture are you testing?
 
  Regards,
  Mauro
 
 Output of the ir-keytable -r is available here:
 http://pastebin.com/eEDu1Bmn. It is the same before and after the
 patch.
 
 Architecture is x86_64.



From the top-posted thread. Merging it here to not confuse people.

 I'll try to describe my thoughts.
 
 The changed structure dib0700_rc_response is used in
 dib0700_core.c:dib0700_rc_urb_completion(struct urb *purb) function:
 
 struct dib0700_rc_response *poll_reply;
 ...
 poll_reply = purb-transfer_buffer;
 
 dib0700_rc_urb_completion() is then used in
 dib0700_core.c:dib0700_rc_setup() in macros usb_fill_bulk_urb and
 usb_fill_int_urb. These macros are defined in header file usb.h. Here
 I have found in macro description this:
 
  * @transfer_buffer: pointer to the transfer buffer
 
 I suppose that it means that the struct dib0700_rc_response is being
 filled from this transfer buffer. Therefore I suppose that the order
 of structure members IS important.
 
 Of course it's only my guess but my patch is really working for me :-)

Hi,

I looked at this again and I still don't see why the order is important.
Plus the code looks like it does what it should be doing when using
RC_SCANCODE_NEC, RC_SCANCODE_NEC32, RC_SCANCODE_NECX and RC_SCANCODE_RC5.

Unfortunately I can't review this if I am not sure about it, and I don't
have the device to be able to properly test your patch.

Hopefully your print of the scancodes helps.

Luis
--
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] dib0700: remove unused macros

[Luis de Bethencourt]

Remove unused macros RC_REPEAT_DELAY and RC_REPEAT_DELAY_V1_20

Signed-off-by: Luis de Bethencourt luis...@samsung.com
---
 drivers/media/usb/dvb-usb/dib0700_core.c| 3 ---
 drivers/media/usb/dvb-usb/dib0700_devices.c | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c 
b/drivers/media/usb/dvb-usb/dib0700_core.c
index 50856db..2b40393 100644
--- a/drivers/media/usb/dvb-usb/dib0700_core.c
+++ b/drivers/media/usb/dvb-usb/dib0700_core.c
@@ -651,9 +651,6 @@ out:
return ret;
 }
 
-/* Number of keypresses to ignore before start repeating */
-#define RC_REPEAT_DELAY_V1_20 10
-
 /* This is the structure of the RC response packet starting in firmware 1.20 */
 struct dib0700_rc_response {
u8 report_id;
diff --git a/drivers/media/usb/dvb-usb/dib0700_devices.c 
b/drivers/media/usb/dvb-usb/dib0700_devices.c
index e1757b8..d7d55a2 100644
--- a/drivers/media/usb/dvb-usb/dib0700_devices.c
+++ b/drivers/media/usb/dvb-usb/dib0700_devices.c
@@ -510,9 +510,6 @@ static int stk7700ph_tuner_attach(struct dvb_usb_adapter 
*adap)
 
 static u8 rc_request[] = { REQUEST_POLL_RC, 0 };
 
-/* Number of keypresses to ignore before start repeating */
-#define RC_REPEAT_DELAY 6
-
 /*
  * This function is used only when firmware is  1.20 version. Newer
  * firmwares use bulk mode, with functions implemented at dib0700_core,
-- 
2.1.0

--
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] [media] si2165: Fix possible leak in si2165_upload_firmware()

[Luis de Bethencourt]

On Thu, Feb 12, 2015 at 01:42:45AM +0200, Antti Palosaari wrote:
 On 02/12/2015 01:38 AM, Luis de Bethencourt wrote:
 On Wed, Feb 11, 2015 at 10:45:01PM +0100, Matthias Schwarzott wrote:
 On 11.02.2015 21:58, Christian Engelmayer wrote:
 In case of an error function si2165_upload_firmware() releases the already
 requested firmware in the exit path. However, there is one deviation where
 the function directly returns. Use the correct cleanup so that the firmware
 memory gets freed correctly. Detected by Coverity CID 1269120.
 
 Signed-off-by: Christian Engelmayer cenge...@gmx.at
 ---
 Compile tested only. Applies against linux-next.
 ---
   drivers/media/dvb-frontends/si2165.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/media/dvb-frontends/si2165.c 
 b/drivers/media/dvb-frontends/si2165.c
 index 98ddb49ad52b..4cc5d10ed0d4 100644
 --- a/drivers/media/dvb-frontends/si2165.c
 +++ b/drivers/media/dvb-frontends/si2165.c
 @@ -505,7 +505,7 @@ static int si2165_upload_firmware(struct si2165_state 
 *state)
/* reset crc */
ret = si2165_writereg8(state, 0x0379, 0x01);
if (ret)
 -  return ret;
 +  goto error;
 
ret = si2165_upload_firmware_block(state, data, len,
   offset, block_count);
 
 Good catch.
 
 Signed-off-by: Matthias Schwarzott z...@gentoo.org
 
 
 Good catch indeed.
 
 Can I sign off? Not sure what the rules are.
 
 Signed-off-by: Luis de Bethencourt luis...@samsung.com
 
 
 You cannot sign it unless patch is going through hands. Probably you want
 review it. Check documentation SubmittingPatches.
 
 https://www.kernel.org/doc/Documentation/SubmittingPatches
 
 regards
 Antti
 
 -- 
 http://palosaari.fi/
 --

Hi Antti,

That was an interesting read. Now I know how these tags work :)
Thanks for the pointing it out to me.

So I meant Reviewed-by:

Luis
--
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] media: Pinnacle 73e infrared control stopped working since kernel 3.17

[Luis de Bethencourt]

On Tue, Feb 10, 2015 at 11:38:11AM +0100, David Cimbůrek wrote:
 Please include this patch to kernel! It takes too much time for such a
 simple fix!
 

The patch is simple but why it fixes the issue isn't that simple. Could you
explain why the order of the variables inside the structure is breaking things?

All the uses of the variables inside the structure that I can see are by name.
Not by memory offsets.

Thanks,
Luis

 
 2015-01-07 13:51 GMT+01:00 David Cimbůrek david.cimbu...@gmail.com:
  No one is interested? I'd like to get this patch to kernel to fix the
  issue. Can someone here do it please?
 
 
  2014-12-20 14:36 GMT+01:00 David Cimbůrek david.cimbu...@gmail.com:
  Hi,
 
  with kernel 3.17 remote control for Pinnacle 73e (ID 2304:0237
  Pinnacle Systems, Inc. PCTV 73e [DiBcom DiB7000PC]) does not work
  anymore.
 
  I checked the changes and found out the problem in commit
  af3a4a9bbeb00df3e42e77240b4cdac5479812f9.
 
  In dib0700_core.c in struct dib0700_rc_response the following union:
 
  union {
  u16 system16;
  struct {
  u8 not_system;
  u8 system;
  };
  };
 
  has been replaced by simple variables:
 
  u8 system;
  u8 not_system;
 
  But these variables are in reverse order! When I switch the order
  back, the remote works fine again! Here is the patch:
 
 
  --- a/drivers/media/usb/dvb-usr/dib0700_core.c2014-12-20
  14:27:15.0 +0100
  +++ b/drivers/media/usb/dvb-usr/dib0700_core.c2014-12-20
  14:27:36.0 +0100
  @@ -658,8 +658,8 @@
   struct dib0700_rc_response {
   u8 report_id;
   u8 data_state;
  -u8 system;
   u8 not_system;
  +u8 system;
   u8 data;
   u8 not_data;
   };
 
 
  Regards,
  David
 --
 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
--
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] [media] si2165: Fix possible leak in si2165_upload_firmware()

[Luis de Bethencourt]

On Wed, Feb 11, 2015 at 10:45:01PM +0100, Matthias Schwarzott wrote:
 On 11.02.2015 21:58, Christian Engelmayer wrote:
  In case of an error function si2165_upload_firmware() releases the already
  requested firmware in the exit path. However, there is one deviation where
  the function directly returns. Use the correct cleanup so that the firmware
  memory gets freed correctly. Detected by Coverity CID 1269120.
  
  Signed-off-by: Christian Engelmayer cenge...@gmx.at
  ---
  Compile tested only. Applies against linux-next.
  ---
   drivers/media/dvb-frontends/si2165.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/drivers/media/dvb-frontends/si2165.c 
  b/drivers/media/dvb-frontends/si2165.c
  index 98ddb49ad52b..4cc5d10ed0d4 100644
  --- a/drivers/media/dvb-frontends/si2165.c
  +++ b/drivers/media/dvb-frontends/si2165.c
  @@ -505,7 +505,7 @@ static int si2165_upload_firmware(struct si2165_state 
  *state)
  /* reset crc */
  ret = si2165_writereg8(state, 0x0379, 0x01);
  if (ret)
  -   return ret;
  +   goto error;
   
  ret = si2165_upload_firmware_block(state, data, len,
 offset, block_count);
  
 Good catch.
 
 Signed-off-by: Matthias Schwarzott z...@gentoo.org
 

Good catch indeed.

Can I sign off? Not sure what the rules are.

Signed-off-by: Luis de Bethencourt luis...@samsung.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

[PATCH v2] rtl2832: remove compiler warning

[Luis de Bethencourt]

Cleaning up the following compiler warning:
rtl2832.c:703:12: warning: 'tmp' may be used uninitialized in this function

Even though it could never happen since if rtl2832_rd_demod_reg () doesn't set
tmp, this line would never run because we go to err. It is still nice to avoid
compiler warnings.

Signed-off-by: Luis de Bethencourt luis...@samsung.com
---
 drivers/media/dvb-frontends/rtl2832.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/rtl2832.c 
b/drivers/media/dvb-frontends/rtl2832.c
index 5d2d8f4..20fa245 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -685,7 +685,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, 
fe_status_t *status)
struct rtl2832_dev *dev = fe-demodulator_priv;
struct i2c_client *client = dev-client;
int ret;
-   u32 tmp;
+   u32 uninitialized_var(tmp);
 
dev_dbg(client-dev, \n);
 
-- 
2.1.3

--
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] rtl2832: remove compiler warning

[Luis de Bethencourt]

On Tue, Feb 10, 2015 at 09:35:52PM -0200, Mauro Carvalho Chehab wrote:
 Em Tue, 10 Feb 2015 12:57:24 +0200
 Antti Palosaari cr...@iki.fi escreveu:
 
  On 02/09/2015 12:44 AM, Luis de Bethencourt wrote:
   Cleaning the following compiler warning:
   rtl2832.c:703:12: warning: 'tmp' may be used uninitialized in this 
   function
  
   Even though it could never happen since if rtl2832_rd_demod_reg () 
   doesn't set
   tmp, this line would never run because we go to err. It is still nice to 
   avoid
   compiler warnings.
  
   Signed-off-by: Luis de Bethencourt luis...@samsung.com
   ---
 drivers/media/dvb-frontends/rtl2832.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  
   diff --git a/drivers/media/dvb-frontends/rtl2832.c 
   b/drivers/media/dvb-frontends/rtl2832.c
   index 5d2d8f4..ad36d1c 100644
   --- a/drivers/media/dvb-frontends/rtl2832.c
   +++ b/drivers/media/dvb-frontends/rtl2832.c
   @@ -685,7 +685,7 @@ static int rtl2832_read_status(struct dvb_frontend 
   *fe, fe_status_t *status)
 struct rtl2832_dev *dev = fe-demodulator_priv;
 struct i2c_client *client = dev-client;
 int ret;
   - u32 tmp;
   + u32 tmp = 0;
  
 dev_dbg(client-dev, \n);
  
  I looked the code and I cannot see how it could used as uninitialized. 
  Dunno how it could be fixed properly.
  
  Also, I think idiom to say compiler that variable could be uninitialized 
  is to store its own value. But I am fine with zero initialization too.
  
  u32 tmp = tmp;
 
 Actually, the right way is to declare it as:
 
   u32 uninitialized_var(tmp)
 
 The syntax to suppress compiler warnings depends on the compiler:
 
 include/linux/compiler-clang.h:#define uninitialized_var(x) x = *((x))
 include/linux/compiler-gcc.h:#define uninitialized_var(x) x = x
 
 Also, using uninitialized_var() better documents it.
 
 Regards,
 Mauro

Hi Mauro,

That is a way more elegant solution. Great!
I will check out that compiler-clang header file, it's interesting.

I just sent a revised patch using this :)

Thanks,
Luis
--
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] rtl2832: remove compiler warning

[Luis de Bethencourt]

On Tue, Feb 10, 2015 at 12:57:24PM +0200, Antti Palosaari wrote:
 On 02/09/2015 12:44 AM, Luis de Bethencourt wrote:
 Cleaning the following compiler warning:
 rtl2832.c:703:12: warning: 'tmp' may be used uninitialized in this function
 
 Even though it could never happen since if rtl2832_rd_demod_reg () doesn't 
 set
 tmp, this line would never run because we go to err. It is still nice to 
 avoid
 compiler warnings.
 
 Signed-off-by: Luis de Bethencourt luis...@samsung.com
 ---
   drivers/media/dvb-frontends/rtl2832.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/media/dvb-frontends/rtl2832.c 
 b/drivers/media/dvb-frontends/rtl2832.c
 index 5d2d8f4..ad36d1c 100644
 --- a/drivers/media/dvb-frontends/rtl2832.c
 +++ b/drivers/media/dvb-frontends/rtl2832.c
 @@ -685,7 +685,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, 
 fe_status_t *status)
  struct rtl2832_dev *dev = fe-demodulator_priv;
  struct i2c_client *client = dev-client;
  int ret;
 -u32 tmp;
 +u32 tmp = 0;
 
  dev_dbg(client-dev, \n);
 
 I looked the code and I cannot see how it could used as uninitialized. Dunno
 how it could be fixed properly.

Hi Antti,

I agree. If rtl2832_rd_demod_reg() in line 696 doesn't set tmp it is because
it has errored out. Which means rtl2832_read_status() itself will goto err
before the check for 'if (tmp == 11)' line that generates the warning.

I mentioned this in my commit message :)

 
 Also, I think idiom to say compiler that variable could be uninitialized is
 to store its own value. But I am fine with zero initialization too.
 
 u32 tmp = tmp;
 
 regards
 Antti
 
 -- 
 http://palosaari.fi/

If you prefer I use the 'u32 tmp = tmp;' I am happy to change my patch.
You say you are fine with zero initialization too, but I prefer offering
whatever you think is best.

Thanks for taking the time to look at this,
Luis
--
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: divide error: 0000 in the gspca_topro

[Luis de Bethencourt]

On Mon, Feb 09, 2015 at 01:56:56PM -0200, Mauro Carvalho Chehab wrote:
 Em Mon, 09 Feb 2015 10:23:48 +
 Luis de Bethencourt l...@debethencourt.com escreveu:
 
  On Sun, Feb 08, 2015 at 06:07:45PM -0800, Linus Torvalds wrote:
   I got this, and it certainly seems relevant,.
   
   It would seem that that whole 'quality' thing needs some range
   checking, it should presumably be in the range [1..100] in order to
   avoid negative 'sc' values or the divide-by-zero.
   
   Hans, Mauro?
   
 Linus
  
  Hello Linus,
  
  The case of quality being set to 0 is correctly handled in
  drivers/media/usb/gspca/jpeg.h [0], so I have sent a patch to do the same
  in topro.c.
 
 Patch looks good to me.
 
 I'll double check if some other driver has the same bad handling for
 quality set and give a couple days for Hans to take a look.
 
 If he's fine with this approach, I'll add it on a separate pull request.
 
 Regards,
 Mauro
 

Hi Mauro,

Thanks for taking the time to look at this.

After sending the patch I searched around for any similar cases, only
finding coda/coda-jpeg.c [0], but in this case the quality is clipped to 5 if
it is  5.

I might have missed some other case though. Just letting you know to help you
save some time.

Cheers,
Luis

[0] 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/coda/coda-jpeg.c#n216

  
  Thanks,
  Luis
  
  [0] 
  https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/usb/gspca/jpeg.h#n157
  
   
   -- Forwarded message --
   From: Peter Kovář peter.ko...@reflexion.tv
   Date: Sun, Feb 8, 2015 at 2:18 PM
   Subject: divide error:  in the gspca_topro
   To: Linus Torvalds torva...@linux-foundation.org
   
   
   Hi++ Linus!
   
   There is a trivial bug in the gspca_topro webcam driver.
   
   /* set the JPEG quality for sensor soi763a */
   static void jpeg_set_qual(u8 *jpeg_hdr,
 int quality)
   {
   int i, sc;
   
   if (quality  50)
   sc = 5000 / quality;
   else
   sc = 200 - quality * 2;
   
   
   
   Crash can be reproduced by setting JPEG quality to zero in the guvcview
   application.
   
   Cheers,
   
   Peter Kovář
   50 65 74 65 72 20 4B 6F 76 C3 A1 C5 99
   --
   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
--
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: Fwd: divide error: 0000 in the gspca_topro

[Luis de Bethencourt]

On Sun, Feb 08, 2015 at 06:07:45PM -0800, Linus Torvalds wrote:
 I got this, and it certainly seems relevant,.
 
 It would seem that that whole 'quality' thing needs some range
 checking, it should presumably be in the range [1..100] in order to
 avoid negative 'sc' values or the divide-by-zero.
 
 Hans, Mauro?
 
   Linus

Hello Linus,

The case of quality being set to 0 is correctly handled in
drivers/media/usb/gspca/jpeg.h [0], so I have sent a patch to do the same
in topro.c.

Thanks,
Luis

[0] 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/usb/gspca/jpeg.h#n157

 
 -- Forwarded message --
 From: Peter Kovář peter.ko...@reflexion.tv
 Date: Sun, Feb 8, 2015 at 2:18 PM
 Subject: divide error:  in the gspca_topro
 To: Linus Torvalds torva...@linux-foundation.org
 
 
 Hi++ Linus!
 
 There is a trivial bug in the gspca_topro webcam driver.
 
 /* set the JPEG quality for sensor soi763a */
 static void jpeg_set_qual(u8 *jpeg_hdr,
   int quality)
 {
 int i, sc;
 
 if (quality  50)
 sc = 5000 / quality;
 else
 sc = 200 - quality * 2;
 
 
 
 Crash can be reproduced by setting JPEG quality to zero in the guvcview
 application.
 
 Cheers,
 
 Peter Kovář
 50 65 74 65 72 20 4B 6F 76 C3 A1 C5 99
 --
 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
--
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] gpsca: remove the risk of a division by zero

[Luis de Bethencourt]

As reported by Peter Kovar, there's a potential risk of a division by zero on
calls to jpeg_set_qual() when quality is zero.

As quality can't be 0 or lower than that, add an extra clause to cover this
special case.

Signed-off-by: Luis de Bethencourt luis...@samsung.com
---
 drivers/media/usb/gspca/topro.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/gspca/topro.c b/drivers/media/usb/gspca/topro.c
index 5fcd1ee..c70ff40 100644
--- a/drivers/media/usb/gspca/topro.c
+++ b/drivers/media/usb/gspca/topro.c
@@ -969,7 +969,9 @@ static void jpeg_set_qual(u8 *jpeg_hdr,
 {
int i, sc;
 
-   if (quality  50)
+   if (quality = 0)
+   sc = 5000;
+   else if (quality  50)
sc = 5000 / quality;
else
sc = 200 - quality * 2;
-- 
2.1.3

--
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] gpsca: remove the risk of a division by zero

[Luis de Bethencourt]

On Mon, Feb 09, 2015 at 10:16:25AM +, Luis de Bethencourt wrote:
 As reported by Peter Kovar, there's a potential risk of a division by zero on
 calls to jpeg_set_qual() when quality is zero.
 
 As quality can't be 0 or lower than that, add an extra clause to cover this
 special case.
 
 Signed-off-by: Luis de Bethencourt luis...@samsung.com
 ---
  drivers/media/usb/gspca/topro.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/media/usb/gspca/topro.c b/drivers/media/usb/gspca/topro.c
 index 5fcd1ee..c70ff40 100644
 --- a/drivers/media/usb/gspca/topro.c
 +++ b/drivers/media/usb/gspca/topro.c
 @@ -969,7 +969,9 @@ static void jpeg_set_qual(u8 *jpeg_hdr,
  {
   int i, sc;
  
 - if (quality  50)
 + if (quality = 0)
 + sc = 5000;
 + else if (quality  50)
   sc = 5000 / quality;
   else
   sc = 200 - quality * 2;
 -- 
 2.1.3
 

Reported here:
http://www.mail-archive.com/linux-media@vger.kernel.org/msg84989.html

Thanks :)
Luis
--
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: bcm2048: remove unused return of function

[Luis de Bethencourt]

Integer return of bcm2048_parse_rds_rt () is never used, changing the return
type to void.

Signed-off-by: Luis de Bethencourt luis...@samsung.com
---
 drivers/staging/media/bcm2048/radio-bcm2048.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index 5382506..7f3d528 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -1579,7 +1579,7 @@ static void bcm2048_parse_rt_match_d(struct 
bcm2048_device *bdev, int i,
bcm2048_parse_rds_rt_block(bdev, i, index+2, crc);
 }
 
-static int bcm2048_parse_rds_rt(struct bcm2048_device *bdev)
+static void bcm2048_parse_rds_rt(struct bcm2048_device *bdev)
 {
int i, index = 0, crc, match_b = 0, match_c = 0, match_d = 0;
 
@@ -1615,8 +1615,6 @@ static int bcm2048_parse_rds_rt(struct bcm2048_device 
*bdev)
match_b = 1;
}
}
-
-   return 0;
 }
 
 static void bcm2048_parse_rds_ps_block(struct bcm2048_device *bdev, int i,
-- 
2.1.0

--
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] rtl2832: remove compiler warning

[Luis de Bethencourt]

Cleaning the following compiler warning:
rtl2832.c:703:12: warning: 'tmp' may be used uninitialized in this function

Even though it could never happen since if rtl2832_rd_demod_reg () doesn't set
tmp, this line would never run because we go to err. It is still nice to avoid
compiler warnings.

Signed-off-by: Luis de Bethencourt luis...@samsung.com
---
 drivers/media/dvb-frontends/rtl2832.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/rtl2832.c 
b/drivers/media/dvb-frontends/rtl2832.c
index 5d2d8f4..ad36d1c 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -685,7 +685,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, 
fe_status_t *status)
struct rtl2832_dev *dev = fe-demodulator_priv;
struct i2c_client *client = dev-client;
int ret;
-   u32 tmp;
+   u32 tmp = 0;
 
dev_dbg(client-dev, \n);
 
-- 
2.1.0

--
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] rtl2832: remove compiler warning

[Luis de Bethencourt]

On Sun, Feb 08, 2015 at 10:44:22PM +, Luis de Bethencourt wrote:
 Cleaning the following compiler warning:
 rtl2832.c:703:12: warning: 'tmp' may be used uninitialized in this function
 
 Even though it could never happen since if rtl2832_rd_demod_reg () doesn't set
 tmp, this line would never run because we go to err. It is still nice to avoid
 compiler warnings.
 
 Signed-off-by: Luis de Bethencourt luis...@samsung.com
 ---
  drivers/media/dvb-frontends/rtl2832.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/media/dvb-frontends/rtl2832.c 
 b/drivers/media/dvb-frontends/rtl2832.c
 index 5d2d8f4..ad36d1c 100644
 --- a/drivers/media/dvb-frontends/rtl2832.c
 +++ b/drivers/media/dvb-frontends/rtl2832.c
 @@ -685,7 +685,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, 
 fe_status_t *status)
   struct rtl2832_dev *dev = fe-demodulator_priv;
   struct i2c_client *client = dev-client;
   int ret;
 - u32 tmp;
 + u32 tmp = 0;
  
   dev_dbg(client-dev, \n);
  
 -- 
 2.1.0
 

Hello all :)

This warning can be seen in:
http://hverkuil.home.xs4all.nl/logs/Saturday.log

Thank you Hans for the daily build and logs.

Luis
--
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] DocBook: grammatical correction on DVB Overview

[Luis de Bethencourt]

---
 Documentation/DocBook/media/dvb/intro.xml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/media/dvb/intro.xml 
b/Documentation/DocBook/media/dvb/intro.xml
index 2048b53..e98bfa9 100644
--- a/Documentation/DocBook/media/dvb/intro.xml
+++ b/Documentation/DocBook/media/dvb/intro.xml
@@ -85,8 +85,8 @@ real time and re-inserted into the TS./para
  paraDemultiplexer which filters the incoming DVB stream/para
 
 paraThe demultiplexer splits the TS into its components like audio and
-video streams. Besides usually several of such audio and video streams
-it also contains data streams with information about the programs
+video streams. As well as several of such audio and video streams, it
+usually also contains data streams with information about the programs
 offered in this or other streams of the same provider./para
 
 /listitem
-- 
2.1.3

--
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] DocBook: grammatical correction on DVB Overview

[Luis de Bethencourt]

On Fri, Feb 06, 2015 at 01:23:15PM -0500, Jonathan Corbet wrote:
 On Fri, 6 Feb 2015 18:17:52 +
 Luis de Bethencourt l...@debethencourt.com wrote:
 
  -video streams. Besides usually several of such audio and video streams
  -it also contains data streams with information about the programs
  +video streams. As well as several of such audio and video streams, it
  +usually also contains data streams with information about the programs
 
 Not sure if I see this as an improvement or not; you've changed the
 meaning of the sentence a bit.  It also lacks changelog and signoff...
 
 Thanks,
 
 jon

Hi Jon,

The original sentence is hard to read, I had to go a few times before I think
I understood what it meant. Which might be different from your interpretation.

How about?:
Besides several of such audio and video stream, it usually also contains data
streams with information about the programs [...]

If this isn't worth it and it is just nitpicking, feel free to drop.

Thanks for taking the time to look into this,
Luis
--
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] dvb-usb: fix spaces after commas

[Luis de Bethencourt]

Fixing a few checkpatch errors of type: space required after that ','

Signed-off-by: Luis de Bethencourt luis...@samsung.com
---
 drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dvb-usb-dvb.c 
b/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
index 719413b..bd901e1 100644
--- a/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
+++ b/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
@@ -84,14 +84,15 @@ static int dvb_usb_ctrl_feed(struct dvb_demux_feed 
*dvbdmxfeed, int onoff)
 
 static int dvb_usb_start_feed(struct dvb_demux_feed *dvbdmxfeed)
 {
-   deb_ts(start pid: 0x%04x, feedtype: %d\n, 
dvbdmxfeed-pid,dvbdmxfeed-type);
-   return dvb_usb_ctrl_feed(dvbdmxfeed,1);
+   deb_ts(start pid: 0x%04x, feedtype: %d\n, dvbdmxfeed-pid,
+  dvbdmxfeed-type);
+   return dvb_usb_ctrl_feed(dvbdmxfeed, 1);
 }
 
 static int dvb_usb_stop_feed(struct dvb_demux_feed *dvbdmxfeed)
 {
deb_ts(stop pid: 0x%04x, feedtype: %d\n, dvbdmxfeed-pid, 
dvbdmxfeed-type);
-   return dvb_usb_ctrl_feed(dvbdmxfeed,0);
+   return dvb_usb_ctrl_feed(dvbdmxfeed, 0);
 }
 
 int dvb_usb_adapter_dvb_init(struct dvb_usb_adapter *adap, short *adapter_nums)
@@ -108,8 +109,8 @@ int dvb_usb_adapter_dvb_init(struct dvb_usb_adapter *adap, 
short *adapter_nums)
adap-dvb_adap.priv = adap;
 
if (adap-dev-props.read_mac_address) {
-   if 
(adap-dev-props.read_mac_address(adap-dev,adap-dvb_adap.proposed_mac) == 0)
-   info(MAC address: %pM,adap-dvb_adap.proposed_mac);
+   if (adap-dev-props.read_mac_address(adap-dev, 
adap-dvb_adap.proposed_mac) == 0)
+   info(MAC address: %pM, adap-dvb_adap.proposed_mac);
else
err(MAC address reading failed.);
}
@@ -128,7 +129,7 @@ int dvb_usb_adapter_dvb_init(struct dvb_usb_adapter *adap, 
short *adapter_nums)
adap-demux.stop_feed= dvb_usb_stop_feed;
adap-demux.write_to_decoder = NULL;
if ((ret = dvb_dmx_init(adap-demux))  0) {
-   err(dvb_dmx_init failed: error %d,ret);
+   err(dvb_dmx_init failed: error %d, ret);
goto err_dmx;
}
 
@@ -136,13 +137,13 @@ int dvb_usb_adapter_dvb_init(struct dvb_usb_adapter 
*adap, short *adapter_nums)
adap-dmxdev.demux   = adap-demux.dmx;
adap-dmxdev.capabilities= 0;
if ((ret = dvb_dmxdev_init(adap-dmxdev, adap-dvb_adap))  0) {
-   err(dvb_dmxdev_init failed: error %d,ret);
+   err(dvb_dmxdev_init failed: error %d, ret);
goto err_dmx_dev;
}
 
if ((ret = dvb_net_init(adap-dvb_adap, adap-dvb_net,
adap-demux.dmx))  0) {
-   err(dvb_net_init failed: error %d,ret);
+   err(dvb_net_init failed: error %d, ret);
goto err_net_init;
}
 
-- 
2.1.3

--
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] dvb-usb: fix spaces after commas

[Luis de Bethencourt]

Signed-off-by: Luis de Bethencourt luis...@samsung.com
---
 drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dvb-usb-dvb.c 
b/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
index 719413b..c901d15 100644
--- a/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
+++ b/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
@@ -84,14 +84,14 @@ static int dvb_usb_ctrl_feed(struct dvb_demux_feed 
*dvbdmxfeed, int onoff)
 
 static int dvb_usb_start_feed(struct dvb_demux_feed *dvbdmxfeed)
 {
-   deb_ts(start pid: 0x%04x, feedtype: %d\n, 
dvbdmxfeed-pid,dvbdmxfeed-type);
-   return dvb_usb_ctrl_feed(dvbdmxfeed,1);
+   deb_ts(start pid: 0x%04x, feedtype: %d\n, dvbdmxfeed-pid, 
dvbdmxfeed-type);
+   return dvb_usb_ctrl_feed(dvbdmxfeed, 1);
 }
 
 static int dvb_usb_stop_feed(struct dvb_demux_feed *dvbdmxfeed)
 {
deb_ts(stop pid: 0x%04x, feedtype: %d\n, dvbdmxfeed-pid, 
dvbdmxfeed-type);
-   return dvb_usb_ctrl_feed(dvbdmxfeed,0);
+   return dvb_usb_ctrl_feed(dvbdmxfeed, 0);
 }
 
 int dvb_usb_adapter_dvb_init(struct dvb_usb_adapter *adap, short *adapter_nums)
@@ -108,8 +108,8 @@ int dvb_usb_adapter_dvb_init(struct dvb_usb_adapter *adap, 
short *adapter_nums)
adap-dvb_adap.priv = adap;
 
if (adap-dev-props.read_mac_address) {
-   if 
(adap-dev-props.read_mac_address(adap-dev,adap-dvb_adap.proposed_mac) == 0)
-   info(MAC address: %pM,adap-dvb_adap.proposed_mac);
+   if (adap-dev-props.read_mac_address(adap-dev, 
adap-dvb_adap.proposed_mac) == 0)
+   info(MAC address: %pM, adap-dvb_adap.proposed_mac);
else
err(MAC address reading failed.);
}
@@ -128,7 +128,7 @@ int dvb_usb_adapter_dvb_init(struct dvb_usb_adapter *adap, 
short *adapter_nums)
adap-demux.stop_feed= dvb_usb_stop_feed;
adap-demux.write_to_decoder = NULL;
if ((ret = dvb_dmx_init(adap-demux))  0) {
-   err(dvb_dmx_init failed: error %d,ret);
+   err(dvb_dmx_init failed: error %d, ret);
goto err_dmx;
}
 
@@ -136,13 +136,13 @@ int dvb_usb_adapter_dvb_init(struct dvb_usb_adapter 
*adap, short *adapter_nums)
adap-dmxdev.demux   = adap-demux.dmx;
adap-dmxdev.capabilities= 0;
if ((ret = dvb_dmxdev_init(adap-dmxdev, adap-dvb_adap))  0) {
-   err(dvb_dmxdev_init failed: error %d,ret);
+   err(dvb_dmxdev_init failed: error %d, ret);
goto err_dmx_dev;
}
 
if ((ret = dvb_net_init(adap-dvb_adap, adap-dvb_net,
adap-demux.dmx))  0) {
-   err(dvb_net_init failed: error %d,ret);
+   err(dvb_net_init failed: error %d, ret);
goto err_net_init;
}
 
-- 
2.1.3

--
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 v4 2/2] staging: media: lirc: lirc_zilog.c: keep consistency in dev functions

[Luis de Bethencourt]

The previous patch switched some dev functions to move the string to a second
line. Doing this for all similar functions because it makes the driver easier
to read if all similar lines use the same criteria.

Signed-off-by: Luis de Bethencourt l...@debethencourt.com
---
 drivers/staging/media/lirc/lirc_zilog.c | 57 ++---
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index 8814a7e..27464da 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -369,7 +369,7 @@ static int add_to_buf(struct IR *ir)
ret = i2c_master_send(rx-c, sendbuf, 1);
if (ret != 1) {
dev_err(ir-l.dev, i2c_master_send failed with %d\n,
-  ret);
+   ret);
if (failures = 3) {
mutex_unlock(ir-ir_lock);
dev_err(ir-l.dev,
@@ -412,8 +412,9 @@ static int add_to_buf(struct IR *ir)
rx-b[0] = keybuf[3];
rx-b[1] = keybuf[4];
rx-b[2] = keybuf[5];
-   dev_dbg(ir-l.dev, key (0x%02x/0x%02x)\n,
-  rx-b[0], rx-b[1]);
+   dev_dbg(ir-l.dev,
+   key (0x%02x/0x%02x)\n,
+   rx-b[0], rx-b[1]);
}
 
/* key pressed ? */
@@ -657,8 +658,8 @@ static int send_data_block(struct IR_tx *tx, unsigned char 
*data_block)
dev_dbg(tx-ir-l.dev, %*ph, 5, buf);
ret = i2c_master_send(tx-c, buf, tosend + 1);
if (ret != tosend + 1) {
-   dev_err(tx-ir-l.dev, i2c_master_send failed with 
%d\n,
-  ret);
+   dev_err(tx-ir-l.dev,
+   i2c_master_send failed with %d\n, ret);
return ret  0 ? ret : -EFAULT;
}
i += tosend;
@@ -711,7 +712,7 @@ static int send_boot_data(struct IR_tx *tx)
}
if ((buf[0] != 0x80)  (buf[0] != 0xa0)) {
dev_err(tx-ir-l.dev, unexpected IR TX init response: %02x\n,
-  buf[0]);
+   buf[0]);
return 0;
}
dev_notice(tx-ir-l.dev,
@@ -763,8 +764,9 @@ static int fw_load(struct IR_tx *tx)
/* Request codeset data file */
ret = request_firmware(fw_entry, haup-ir-blaster.bin, tx-ir-l.dev);
if (ret != 0) {
-   dev_err(tx-ir-l.dev, firmware haup-ir-blaster.bin not 
available (%d)\n,
-   ret);
+   dev_err(tx-ir-l.dev,
+   firmware haup-ir-blaster.bin not available (%d)\n,
+   ret);
ret = ret  0 ? ret : -EFAULT;
goto out;
}
@@ -814,7 +816,7 @@ static int fw_load(struct IR_tx *tx)
goto corrupt;
 
dev_dbg(tx-ir-l.dev, %u IR blaster codesets loaded\n,
-  tx_data-num_code_sets);
+   tx_data-num_code_sets);
 
tx_data-code_sets = vmalloc(
tx_data-num_code_sets * sizeof(char *));
@@ -944,8 +946,9 @@ static ssize_t read(struct file *filep, char __user 
*outbuf, size_t n,
unsigned char buf[MAX_XFER_SIZE];
 
if (rbuf-chunk_size  sizeof(buf)) {
-   dev_err(ir-l.dev, chunk_size is too big 
(%d)!\n,
-   rbuf-chunk_size);
+   dev_err(ir-l.dev,
+   chunk_size is too big (%d)!\n,
+   rbuf-chunk_size);
ret = -EINVAL;
break;
}
@@ -968,8 +971,8 @@ static ssize_t read(struct file *filep, char __user 
*outbuf, size_t n,
put_ir_rx(rx, false);
set_current_state(TASK_RUNNING);
 
-   dev_dbg(ir-l.dev, read result = %d (%s)\n,
-  ret, ret ? Error : OK);
+   dev_dbg(ir-l.dev, read result = %d (%s)\n, ret,
+   ret ? Error : OK);
 
return ret ? ret : written;
 }
@@ -1081,7 +1084,7 @@ static int send_code(struct IR_tx *tx, unsigned int code, 
unsigned int key)
}
if (buf[0] != 0x80) {
dev_err(tx-ir-l.dev, unexpected IR TX response #2: %02x\n,
-  buf[0]);
+   buf[0]);
return -EFAULT;
}
 
@@ -1233,7 +1236,7 @@ static unsigned int poll(struct file *filep, poll_table 
*wait)
ret = lirc_buffer_empty(rbuf) ? 0 : (POLLIN|POLLRDNORM);
 
dev_dbg(ir

Re: [PATCH v4 1/2] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

[Luis de Bethencourt]

On Wed, Dec 10, 2014 at 03:39:09PM -0800, Joe Perches wrote:
 On Wed, 2014-12-10 at 22:33 +, Luis de Bethencourt wrote:
  checkpatch makes an exception to the 80-colum rule for quotes strings, and
  Documentation/CodingStyle recommends not splitting quotes strings across 
  lines
  because it breaks the ability to grep for the string. Fixing these.
 []
  diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
  b/drivers/staging/media/lirc/lirc_zilog.c
 []
  @@ -794,9 +796,9 @@ static int fw_load(struct IR_tx *tx)
  if (!read_uint8(data, tx_data-endp, version))
  goto corrupt;
  if (version != 1) {
  -   dev_err(tx-ir-l.dev, unsupported code set file version (%u, 
  expected
  -   1) -- please upgrade to a newer driver,
  -   version);
  +   dev_err(tx-ir-l.dev,
  +   unsupported code set file version (%u, expected 1) -- 
  please upgrade to a newer driver,
  +   version);
 
 Unrelated but this one should have a '\n' termination
 at the end of the format.
 

I can add that change, no problem. As part of this patch or a third one?

Thanks for reviewing,
Luis
--
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 1/2] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

[Luis de Bethencourt]

On Wed, Dec 10, 2014 at 04:01:04PM -0800, Joe Perches wrote:
 On Wed, 2014-12-10 at 23:57 +, Luis de Bethencourt wrote:
  On Wed, Dec 10, 2014 at 03:39:09PM -0800, Joe Perches wrote:
   On Wed, 2014-12-10 at 22:33 +, Luis de Bethencourt wrote:
diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
 []
+   dev_err(tx-ir-l.dev,
+   unsupported code set file version (%u, 
expected 1) -- please upgrade to a newer driver,
+   version);
   
   Unrelated but this one should have a '\n' termination
   at the end of the format.
  I can add that change, no problem. As part of this patch or a third one?
 
 Up to you.
 
 

I like keeping my patches divided in functional units. Resubmitting these
patches as v5 with your suggestion as the third one, since it needs to be
applied on top of the other 2.

Thanks Joe!

Luis
--
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 v5 1/3] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

[Luis de Bethencourt]

checkpatch makes an exception to the 80-colum rule for quotes strings, and
Documentation/CodingStyle recommends not splitting quotes strings across lines
because it breaks the ability to grep for the string. Fixing these.

WARNING: quoted string split across lines

Signed-off-by: Luis de Bethencourt l...@debethencourt.com
---
 drivers/staging/media/lirc/lirc_zilog.c | 61 ++---
 1 file changed, 34 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index dca806a..8814a7e 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -372,14 +372,14 @@ static int add_to_buf(struct IR *ir)
   ret);
if (failures = 3) {
mutex_unlock(ir-ir_lock);
-   dev_err(ir-l.dev, unable to read from the IR 
chip 
-   after 3 resets, giving up\n);
+   dev_err(ir-l.dev,
+   unable to read from the IR chip after 
3 resets, giving up\n);
break;
}
 
/* Looks like the chip crashed, reset it */
-   dev_err(ir-l.dev, polling the IR receiver chip 
failed, 
-   trying reset\n);
+   dev_err(ir-l.dev,
+   polling the IR receiver chip failed, trying 
reset\n);
 
set_current_state(TASK_UNINTERRUPTIBLE);
if (kthread_should_stop()) {
@@ -405,8 +405,9 @@ static int add_to_buf(struct IR *ir)
ret = i2c_master_recv(rx-c, keybuf, sizeof(keybuf));
mutex_unlock(ir-ir_lock);
if (ret != sizeof(keybuf)) {
-   dev_err(ir-l.dev, i2c_master_recv failed with %d -- 
-   keeping last read buffer\n, ret);
+   dev_err(ir-l.dev,
+   i2c_master_recv failed with %d -- keeping last 
read buffer\n,
+   ret);
} else {
rx-b[0] = keybuf[3];
rx-b[1] = keybuf[4];
@@ -713,8 +714,9 @@ static int send_boot_data(struct IR_tx *tx)
   buf[0]);
return 0;
}
-   dev_notice(tx-ir-l.dev, Zilog/Hauppauge IR blaster firmware version 
-%d.%d.%d loaded\n, buf[1], buf[2], buf[3]);
+   dev_notice(tx-ir-l.dev,
+  Zilog/Hauppauge IR blaster firmware version %d.%d.%d 
loaded\n,
+  buf[1], buf[2], buf[3]);
 
return 0;
 }
@@ -794,9 +796,9 @@ static int fw_load(struct IR_tx *tx)
if (!read_uint8(data, tx_data-endp, version))
goto corrupt;
if (version != 1) {
-   dev_err(tx-ir-l.dev, unsupported code set file version (%u, 
expected
-   1) -- please upgrade to a newer driver,
-   version);
+   dev_err(tx-ir-l.dev,
+   unsupported code set file version (%u, expected 1) -- 
please upgrade to a newer driver,
+   version);
fw_unload_locked();
ret = -EFAULT;
goto out;
@@ -983,8 +985,9 @@ static int send_code(struct IR_tx *tx, unsigned int code, 
unsigned int key)
ret = get_key_data(data_block, code, key);
 
if (ret == -EPROTO) {
-   dev_err(tx-ir-l.dev, failed to get data for code %u, key %u 
-- check 
-   lircd.conf entries\n, code, key);
+   dev_err(tx-ir-l.dev,
+   failed to get data for code %u, key %u -- check 
lircd.conf entries\n,
+   code, key);
return ret;
} else if (ret != 0)
return ret;
@@ -1059,12 +1062,14 @@ static int send_code(struct IR_tx *tx, unsigned int 
code, unsigned int key)
ret = i2c_master_send(tx-c, buf, 1);
if (ret == 1)
break;
-   dev_dbg(tx-ir-l.dev, NAK expected: i2c_master_send 
-   failed with %d (try %d)\n, ret, i+1);
+   dev_dbg(tx-ir-l.dev,
+   NAK expected: i2c_master_send failed with %d (try 
%d)\n,
+   ret, i+1);
}
if (ret != 1) {
-   dev_err(tx-ir-l.dev, IR TX chip never got ready: last 
i2c_master_send 
-   failed with %d\n, ret);
+   dev_err(tx-ir-l.dev,
+   IR TX chip never got ready: last i2c_master_send 
failed with %d\n,
+   ret);
return ret  0 ? ret : -EFAULT;
}
 
@@ -1167,12 +1172,12 @@ static ssize_t write

[PATCH v5 2/3] staging: media: lirc: lirc_zilog.c: keep consistency in dev functions

[Luis de Bethencourt]

The previous patch switched some dev functions to move the string to a second
line. Doing this for all similar functions because it makes the driver easier
to read if all similar lines use the same criteria.

Signed-off-by: Luis de Bethencourt l...@debethencourt.com
---
 drivers/staging/media/lirc/lirc_zilog.c | 57 ++---
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index 8814a7e..27464da 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -369,7 +369,7 @@ static int add_to_buf(struct IR *ir)
ret = i2c_master_send(rx-c, sendbuf, 1);
if (ret != 1) {
dev_err(ir-l.dev, i2c_master_send failed with %d\n,
-  ret);
+   ret);
if (failures = 3) {
mutex_unlock(ir-ir_lock);
dev_err(ir-l.dev,
@@ -412,8 +412,9 @@ static int add_to_buf(struct IR *ir)
rx-b[0] = keybuf[3];
rx-b[1] = keybuf[4];
rx-b[2] = keybuf[5];
-   dev_dbg(ir-l.dev, key (0x%02x/0x%02x)\n,
-  rx-b[0], rx-b[1]);
+   dev_dbg(ir-l.dev,
+   key (0x%02x/0x%02x)\n,
+   rx-b[0], rx-b[1]);
}
 
/* key pressed ? */
@@ -657,8 +658,8 @@ static int send_data_block(struct IR_tx *tx, unsigned char 
*data_block)
dev_dbg(tx-ir-l.dev, %*ph, 5, buf);
ret = i2c_master_send(tx-c, buf, tosend + 1);
if (ret != tosend + 1) {
-   dev_err(tx-ir-l.dev, i2c_master_send failed with 
%d\n,
-  ret);
+   dev_err(tx-ir-l.dev,
+   i2c_master_send failed with %d\n, ret);
return ret  0 ? ret : -EFAULT;
}
i += tosend;
@@ -711,7 +712,7 @@ static int send_boot_data(struct IR_tx *tx)
}
if ((buf[0] != 0x80)  (buf[0] != 0xa0)) {
dev_err(tx-ir-l.dev, unexpected IR TX init response: %02x\n,
-  buf[0]);
+   buf[0]);
return 0;
}
dev_notice(tx-ir-l.dev,
@@ -763,8 +764,9 @@ static int fw_load(struct IR_tx *tx)
/* Request codeset data file */
ret = request_firmware(fw_entry, haup-ir-blaster.bin, tx-ir-l.dev);
if (ret != 0) {
-   dev_err(tx-ir-l.dev, firmware haup-ir-blaster.bin not 
available (%d)\n,
-   ret);
+   dev_err(tx-ir-l.dev,
+   firmware haup-ir-blaster.bin not available (%d)\n,
+   ret);
ret = ret  0 ? ret : -EFAULT;
goto out;
}
@@ -814,7 +816,7 @@ static int fw_load(struct IR_tx *tx)
goto corrupt;
 
dev_dbg(tx-ir-l.dev, %u IR blaster codesets loaded\n,
-  tx_data-num_code_sets);
+   tx_data-num_code_sets);
 
tx_data-code_sets = vmalloc(
tx_data-num_code_sets * sizeof(char *));
@@ -944,8 +946,9 @@ static ssize_t read(struct file *filep, char __user 
*outbuf, size_t n,
unsigned char buf[MAX_XFER_SIZE];
 
if (rbuf-chunk_size  sizeof(buf)) {
-   dev_err(ir-l.dev, chunk_size is too big 
(%d)!\n,
-   rbuf-chunk_size);
+   dev_err(ir-l.dev,
+   chunk_size is too big (%d)!\n,
+   rbuf-chunk_size);
ret = -EINVAL;
break;
}
@@ -968,8 +971,8 @@ static ssize_t read(struct file *filep, char __user 
*outbuf, size_t n,
put_ir_rx(rx, false);
set_current_state(TASK_RUNNING);
 
-   dev_dbg(ir-l.dev, read result = %d (%s)\n,
-  ret, ret ? Error : OK);
+   dev_dbg(ir-l.dev, read result = %d (%s)\n, ret,
+   ret ? Error : OK);
 
return ret ? ret : written;
 }
@@ -1081,7 +1084,7 @@ static int send_code(struct IR_tx *tx, unsigned int code, 
unsigned int key)
}
if (buf[0] != 0x80) {
dev_err(tx-ir-l.dev, unexpected IR TX response #2: %02x\n,
-  buf[0]);
+   buf[0]);
return -EFAULT;
}
 
@@ -1233,7 +1236,7 @@ static unsigned int poll(struct file *filep, poll_table 
*wait)
ret = lirc_buffer_empty(rbuf) ? 0 : (POLLIN|POLLRDNORM);
 
dev_dbg(ir

[PATCH v5 3/3] staging: media: lirc: lirc_zilog.c: missing newline in dev_err()

[Luis de Bethencourt]

Missing newline character at the end of string passed to dev_err()

Signed-off-by: Luis de Bethencourt l...@debethencourt.com
---
 drivers/staging/media/lirc/lirc_zilog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index 27464da..7def690 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -799,7 +799,7 @@ static int fw_load(struct IR_tx *tx)
goto corrupt;
if (version != 1) {
dev_err(tx-ir-l.dev,
-   unsupported code set file version (%u, expected 1) -- 
please upgrade to a newer driver,
+   unsupported code set file version (%u, expected 1) -- 
please upgrade to a newer driver\n,
version);
fw_unload_locked();
ret = -EFAULT;
-- 
2.1.3

--
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 v3 1/2] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

[Luis de Bethencourt]

checkpatch makes an exception to the 80-column rule for quotes strings, and
Documentation/CodingStyle recommends not splitting quotes strings across lines
because it breaks the ability to grep for the string. Fixing these.

WARNING: quoted string split across lines

Signed-off-by: Luis de Bethencourt l...@debethencourt.com
---
 drivers/staging/media/lirc/lirc_zilog.c | 61 ++---
 1 file changed, 34 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index dca806a..8814a7e 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -372,14 +372,14 @@ static int add_to_buf(struct IR *ir)
   ret);
if (failures = 3) {
mutex_unlock(ir-ir_lock);
-   dev_err(ir-l.dev, unable to read from the IR 
chip 
-   after 3 resets, giving up\n);
+   dev_err(ir-l.dev,
+   unable to read from the IR chip after 
3 resets, giving up\n);
break;
}
 
/* Looks like the chip crashed, reset it */
-   dev_err(ir-l.dev, polling the IR receiver chip 
failed, 
-   trying reset\n);
+   dev_err(ir-l.dev,
+   polling the IR receiver chip failed, trying 
reset\n);
 
set_current_state(TASK_UNINTERRUPTIBLE);
if (kthread_should_stop()) {
@@ -405,8 +405,9 @@ static int add_to_buf(struct IR *ir)
ret = i2c_master_recv(rx-c, keybuf, sizeof(keybuf));
mutex_unlock(ir-ir_lock);
if (ret != sizeof(keybuf)) {
-   dev_err(ir-l.dev, i2c_master_recv failed with %d -- 
-   keeping last read buffer\n, ret);
+   dev_err(ir-l.dev,
+   i2c_master_recv failed with %d -- keeping last 
read buffer\n,
+   ret);
} else {
rx-b[0] = keybuf[3];
rx-b[1] = keybuf[4];
@@ -713,8 +714,9 @@ static int send_boot_data(struct IR_tx *tx)
   buf[0]);
return 0;
}
-   dev_notice(tx-ir-l.dev, Zilog/Hauppauge IR blaster firmware version 
-%d.%d.%d loaded\n, buf[1], buf[2], buf[3]);
+   dev_notice(tx-ir-l.dev,
+  Zilog/Hauppauge IR blaster firmware version %d.%d.%d 
loaded\n,
+  buf[1], buf[2], buf[3]);
 
return 0;
 }
@@ -794,9 +796,9 @@ static int fw_load(struct IR_tx *tx)
if (!read_uint8(data, tx_data-endp, version))
goto corrupt;
if (version != 1) {
-   dev_err(tx-ir-l.dev, unsupported code set file version (%u, 
expected
-   1) -- please upgrade to a newer driver,
-   version);
+   dev_err(tx-ir-l.dev,
+   unsupported code set file version (%u, expected 1) -- 
please upgrade to a newer driver,
+   version);
fw_unload_locked();
ret = -EFAULT;
goto out;
@@ -983,8 +985,9 @@ static int send_code(struct IR_tx *tx, unsigned int code, 
unsigned int key)
ret = get_key_data(data_block, code, key);
 
if (ret == -EPROTO) {
-   dev_err(tx-ir-l.dev, failed to get data for code %u, key %u 
-- check 
-   lircd.conf entries\n, code, key);
+   dev_err(tx-ir-l.dev,
+   failed to get data for code %u, key %u -- check 
lircd.conf entries\n,
+   code, key);
return ret;
} else if (ret != 0)
return ret;
@@ -1059,12 +1062,14 @@ static int send_code(struct IR_tx *tx, unsigned int 
code, unsigned int key)
ret = i2c_master_send(tx-c, buf, 1);
if (ret == 1)
break;
-   dev_dbg(tx-ir-l.dev, NAK expected: i2c_master_send 
-   failed with %d (try %d)\n, ret, i+1);
+   dev_dbg(tx-ir-l.dev,
+   NAK expected: i2c_master_send failed with %d (try 
%d)\n,
+   ret, i+1);
}
if (ret != 1) {
-   dev_err(tx-ir-l.dev, IR TX chip never got ready: last 
i2c_master_send 
-   failed with %d\n, ret);
+   dev_err(tx-ir-l.dev,
+   IR TX chip never got ready: last i2c_master_send 
failed with %d\n,
+   ret);
return ret  0 ? ret : -EFAULT;
}
 
@@ -1167,12 +1172,12 @@ static ssize_t write

[PATCH v3 2/2] staging: media: lirc: lirc_zilog.c: keep consistency in dev functions

[Luis de Bethencourt]

The previous patch switched some dev functions to move the string to a second
line. Doing this for all similar functions because it makes the driver easier
to read if all similar lines use the same criteria.

Signed-off-by: Luis de Bethencourt l...@debethencourt.com
---
 drivers/staging/media/lirc/lirc_zilog.c | 155 +---
 1 file changed, 102 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index 8814a7e..af46827 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -322,7 +322,8 @@ static int add_to_buf(struct IR *ir)
struct IR_tx *tx;
 
if (lirc_buffer_full(rbuf)) {
-   dev_dbg(ir-l.dev, buffer overflow\n);
+   dev_dbg(ir-l.dev,
+   buffer overflow\n);
return -EOVERFLOW;
}
 
@@ -368,8 +369,9 @@ static int add_to_buf(struct IR *ir)
 */
ret = i2c_master_send(rx-c, sendbuf, 1);
if (ret != 1) {
-   dev_err(ir-l.dev, i2c_master_send failed with %d\n,
-  ret);
+   dev_err(ir-l.dev,
+   i2c_master_send failed with %d\n,
+   ret);
if (failures = 3) {
mutex_unlock(ir-ir_lock);
dev_err(ir-l.dev,
@@ -412,8 +414,9 @@ static int add_to_buf(struct IR *ir)
rx-b[0] = keybuf[3];
rx-b[1] = keybuf[4];
rx-b[2] = keybuf[5];
-   dev_dbg(ir-l.dev, key (0x%02x/0x%02x)\n,
-  rx-b[0], rx-b[1]);
+   dev_dbg(ir-l.dev,
+   key (0x%02x/0x%02x)\n,
+   rx-b[0], rx-b[1]);
}
 
/* key pressed ? */
@@ -464,7 +467,8 @@ static int lirc_thread(void *arg)
struct IR *ir = arg;
struct lirc_buffer *rbuf = ir-l.rbuf;
 
-   dev_dbg(ir-l.dev, poll thread started\n);
+   dev_dbg(ir-l.dev,
+   poll thread started\n);
 
while (!kthread_should_stop()) {
set_current_state(TASK_INTERRUPTIBLE);
@@ -492,7 +496,8 @@ static int lirc_thread(void *arg)
wake_up_interruptible(rbuf-wait_poll);
}
 
-   dev_dbg(ir-l.dev, poll thread ended\n);
+   dev_dbg(ir-l.dev,
+   poll thread ended\n);
return 0;
 }
 
@@ -654,11 +659,14 @@ static int send_data_block(struct IR_tx *tx, unsigned 
char *data_block)
buf[0] = (unsigned char)(i + 1);
for (j = 0; j  tosend; ++j)
buf[1 + j] = data_block[i + j];
-   dev_dbg(tx-ir-l.dev, %*ph, 5, buf);
+   dev_dbg(tx-ir-l.dev,
+   %*ph,
+   5, buf);
ret = i2c_master_send(tx-c, buf, tosend + 1);
if (ret != tosend + 1) {
-   dev_err(tx-ir-l.dev, i2c_master_send failed with 
%d\n,
-  ret);
+   dev_err(tx-ir-l.dev,
+   i2c_master_send failed with %d\n,
+   ret);
return ret  0 ? ret : -EFAULT;
}
i += tosend;
@@ -682,7 +690,9 @@ static int send_boot_data(struct IR_tx *tx)
buf[1] = 0x20;
ret = i2c_master_send(tx-c, buf, 2);
if (ret != 2) {
-   dev_err(tx-ir-l.dev, i2c_master_send failed with %d\n, ret);
+   dev_err(tx-ir-l.dev,
+   i2c_master_send failed with %d\n,
+   ret);
return ret  0 ? ret : -EFAULT;
}
 
@@ -699,19 +709,24 @@ static int send_boot_data(struct IR_tx *tx)
}
 
if (ret != 1) {
-   dev_err(tx-ir-l.dev, i2c_master_send failed with %d\n, ret);
+   dev_err(tx-ir-l.dev,
+   i2c_master_send failed with %d\n,
+   ret);
return ret  0 ? ret : -EFAULT;
}
 
/* Here comes the firmware version... (hopefully) */
ret = i2c_master_recv(tx-c, buf, 4);
if (ret != 4) {
-   dev_err(tx-ir-l.dev, i2c_master_recv failed with %d\n, ret);
+   dev_err(tx-ir-l.dev,
+   i2c_master_recv failed with %d\n,
+   ret);
return 0;
}
if ((buf[0] != 0x80)  (buf[0] != 0xa0)) {
-   dev_err(tx-ir-l.dev, unexpected IR TX init response: %02x\n,
-  buf[0]);
+   dev_err(tx-ir-l.dev,
+   unexpected IR TX init response: %02x\n,
+   buf[0

Re: [PATCH v2] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

[Luis de Bethencourt]

On Thu, Dec 04, 2014 at 01:16:11PM -0200, Mauro Carvalho Chehab wrote:
 Hi Luis,
 
 Em Tue, 25 Nov 2014 20:36:29 +
 Luis de Bethencourt l...@debethencourt.com escreveu:
 
  checkpatch makes an exception to the 80-colum rule for quotes strings, and
  Documentation/CodingStyle recommends not splitting quotes strings across 
  lines
  because it breaks the ability to grep for the string. Fixing these.
  
  WARNING: quoted string split across lines
  
  Signed-off-by: Luis de Bethencourt l...@debethencourt.com
  ---
  Changes in v2:
  - As pointed out by Joe Perches I missed a space when joining a set of 
  strings
  
  Thanks for the review Joe
   drivers/staging/media/lirc/lirc_zilog.c | 39 
  ++---
   1 file changed, 17 insertions(+), 22 deletions(-)
  
  diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
  b/drivers/staging/media/lirc/lirc_zilog.c
  index dca806a..a35d6f2 100644
  --- a/drivers/staging/media/lirc/lirc_zilog.c
  +++ b/drivers/staging/media/lirc/lirc_zilog.c
  @@ -372,14 +372,12 @@ static int add_to_buf(struct IR *ir)
 ret);
  if (failures = 3) {
  mutex_unlock(ir-ir_lock);
  -   dev_err(ir-l.dev, unable to read from the IR 
  chip 
  -   after 3 resets, giving up\n);
  +   dev_err(ir-l.dev, unable to read from the IR 
  chip after 3 resets, giving up\n);
 
 This patch didn't apply on my tree, as what I have there is a driver
 custom macro. Probably I'm missing some patch (or it got merged via some
 other tree):

Hi Mauro,

The patch is against Greg's staging-testing branch. If you want it against any
of your branches let me know and I will fix it.

 
 +
 zilog_error(unable to read from the IR chip 
 after 3 resets, giving up\n);
 +===
 +   dev_err(ir-l.dev, unable to read from the 
 IR chip after 3 resets, giving up\n);
 +
 
 One general note about this patch: those strings are already big, so
 the best is to put them at the beginning of the second line, aligned with
 the parenthesis, e. g:
   dev_err(ir-l.dev,
   unable to read from the IR chip after 
 3 resets, giving up\n);
 
 Ok, on some, the second line will still violate the 80-cols max, but
 on others it may actually fit. So, better to do the same thing along
 the entire driver, as it makes easier to read it if all similar lines
 use the same criteria.

I just sent two patches. The first one changes the original patch to follow the
format you suggest here. The second one changes the rest of the driver to be
consisent with this.

Thanks a lot for the review :)
Luis

 
 Regards,
 Mauro.
 
  break;
  }
   
  /* Looks like the chip crashed, reset it */
  -   dev_err(ir-l.dev, polling the IR receiver chip 
  failed, 
  -   trying reset\n);
  +   dev_err(ir-l.dev, polling the IR receiver chip 
  failed, trying reset\n);
   
  set_current_state(TASK_UNINTERRUPTIBLE);
  if (kthread_should_stop()) {
  @@ -405,8 +403,8 @@ static int add_to_buf(struct IR *ir)
  ret = i2c_master_recv(rx-c, keybuf, sizeof(keybuf));
  mutex_unlock(ir-ir_lock);
  if (ret != sizeof(keybuf)) {
  -   dev_err(ir-l.dev, i2c_master_recv failed with %d -- 
  -   keeping last read buffer\n, ret);
  +   dev_err(ir-l.dev, i2c_master_recv failed with %d -- 
  keeping last read buffer\n,
  +   ret);
  } else {
  rx-b[0] = keybuf[3];
  rx-b[1] = keybuf[4];
  @@ -713,8 +711,8 @@ static int send_boot_data(struct IR_tx *tx)
 buf[0]);
  return 0;
  }
  -   dev_notice(tx-ir-l.dev, Zilog/Hauppauge IR blaster firmware version 
  -%d.%d.%d loaded\n, buf[1], buf[2], buf[3]);
  +   dev_notice(tx-ir-l.dev, Zilog/Hauppauge IR blaster firmware version 
  %d.%d.%d loaded\n,
  +buf[1], buf[2], buf[3]);
   
  return 0;
   }
  @@ -794,8 +792,7 @@ static int fw_load(struct IR_tx *tx)
  if (!read_uint8(data, tx_data-endp, version))
  goto corrupt;
  if (version != 1) {
  -   dev_err(tx-ir-l.dev, unsupported code set file version (%u, 
  expected
  -   1) -- please upgrade to a newer driver,
  +   dev_err(tx-ir-l.dev, unsupported code set file version (%u, 
  expected 1) -- please upgrade to a newer driver,
  version);
  fw_unload_locked();
  ret = -EFAULT;
  @@ -983,8 +980,8 @@ static int

Re: [PATCH] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

[Luis de Bethencourt]

On Wed, Nov 26, 2014 at 08:05:55AM -0800, Joe Perches wrote:
 On Wed, 2014-11-26 at 15:42 +, Luis de Bethencourt wrote:
  On 26 November 2014 at 01:49, Joe Perches j...@perches.com wrote:
 []
   There is a script I posted a while back that
   groups various checkpatch types together and
   makes it a bit easier to do cleanup style
   patches.
  
   https://lkml.org/lkml/2014/7/11/794
  That is useful! I just run it on staging/octeon/ and it wrote two patches.
  Will submit them in a minute.
 
 Please make sure and write better commit messages
 than the script produces.
 

Will do :)

   Using checkpatch to get familiar with kernel
   development is fine and all, but fixing actual
   defects and submitting new code is way more
   useful.
 []
  I agree. I was just using checkpatch to learn about the development process.
  How to create patches, submit patches, follow review, and such. Better to
  do it
  with small changes like this first.
 
 That's a good way to start.
 
  Which makes me wonder. Is my patch accepted? Will it be merged? I can do the
  proposed logging macro additions in a few days. Not sure yet how the final
  step of the process when patches get accepted and merged works.
 
 You will generally get an email from a maintainer
 when patches are accepted/rejected or you get
 feedback asking for various changes.
 
 Greg KH does that for drivers/staging but not for
 drivers/staging/media.  Mauro Carvalho Chehab does.
 
 These emails are not immediate.  It can take 2 or 3
 weeks for a response.  Sometimes longer, sometimes
 shorter, sometimes no response ever comes.


I understand. Busy people.
 
 After a month or so, if you get no response, maybe
 the maintainer never saw it.  You should maybe
 expand the cc: list for the email.
 
 When the patch is more than a trivial style cleanup,
 Andrew Morton generally picks up orphan patches.
 
 For some subsystems, there are tracking mechanisms
 like patchwork:
 
 For instance, netdev (net/ and drivers/net/) uses:
 http://patchwork.ozlabs.org/project/netdev/list/
 and David Miller, the primary networking maintainer
 is very prompt about updating it.
 
 There's this list of patchwork entries, but maintainer
 activity of these lists vary:
 
 https://patchwork.kernel.org/
 

Very interesting.

I will follow the process through and learn on the way.

Thanks Joe!
--
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] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

[Luis de Bethencourt]

checkpatch makes an exception to the 80-colum rule for quotes strings, and
Documentation/CodingStyle recommends not splitting quotes strings across lines
because it breaks the ability to grep for the string. Fixing these.

WARNING: quoted string split across lines

Signed-off-by: Luis de Bethencourt l...@debethencourt.com
---
 drivers/staging/media/lirc/lirc_zilog.c | 39 ++---
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index dca806a..07675f1 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -372,14 +372,12 @@ static int add_to_buf(struct IR *ir)
   ret);
if (failures = 3) {
mutex_unlock(ir-ir_lock);
-   dev_err(ir-l.dev, unable to read from the IR 
chip 
-   after 3 resets, giving up\n);
+   dev_err(ir-l.dev, unable to read from the IR 
chip after 3 resets, giving up\n);
break;
}
 
/* Looks like the chip crashed, reset it */
-   dev_err(ir-l.dev, polling the IR receiver chip 
failed, 
-   trying reset\n);
+   dev_err(ir-l.dev, polling the IR receiver chip 
failed, trying reset\n);
 
set_current_state(TASK_UNINTERRUPTIBLE);
if (kthread_should_stop()) {
@@ -405,8 +403,8 @@ static int add_to_buf(struct IR *ir)
ret = i2c_master_recv(rx-c, keybuf, sizeof(keybuf));
mutex_unlock(ir-ir_lock);
if (ret != sizeof(keybuf)) {
-   dev_err(ir-l.dev, i2c_master_recv failed with %d -- 
-   keeping last read buffer\n, ret);
+   dev_err(ir-l.dev, i2c_master_recv failed with %d -- 
keeping last read buffer\n,
+   ret);
} else {
rx-b[0] = keybuf[3];
rx-b[1] = keybuf[4];
@@ -713,8 +711,8 @@ static int send_boot_data(struct IR_tx *tx)
   buf[0]);
return 0;
}
-   dev_notice(tx-ir-l.dev, Zilog/Hauppauge IR blaster firmware version 
-%d.%d.%d loaded\n, buf[1], buf[2], buf[3]);
+   dev_notice(tx-ir-l.dev, Zilog/Hauppauge IR blaster firmware version 
%d.%d.%d loaded\n,
+buf[1], buf[2], buf[3]);
 
return 0;
 }
@@ -794,8 +792,7 @@ static int fw_load(struct IR_tx *tx)
if (!read_uint8(data, tx_data-endp, version))
goto corrupt;
if (version != 1) {
-   dev_err(tx-ir-l.dev, unsupported code set file version (%u, 
expected
-   1) -- please upgrade to a newer driver,
+   dev_err(tx-ir-l.dev, unsupported code set file version (%u, 
expected1) -- please upgrade to a newer driver,
version);
fw_unload_locked();
ret = -EFAULT;
@@ -983,8 +980,8 @@ static int send_code(struct IR_tx *tx, unsigned int code, 
unsigned int key)
ret = get_key_data(data_block, code, key);
 
if (ret == -EPROTO) {
-   dev_err(tx-ir-l.dev, failed to get data for code %u, key %u 
-- check 
-   lircd.conf entries\n, code, key);
+   dev_err(tx-ir-l.dev, failed to get data for code %u, key %u 
-- check lircd.conf entries\n,
+   code, key);
return ret;
} else if (ret != 0)
return ret;
@@ -1059,8 +1056,8 @@ static int send_code(struct IR_tx *tx, unsigned int code, 
unsigned int key)
ret = i2c_master_send(tx-c, buf, 1);
if (ret == 1)
break;
-   dev_dbg(tx-ir-l.dev, NAK expected: i2c_master_send 
-   failed with %d (try %d)\n, ret, i+1);
+   dev_dbg(tx-ir-l.dev, NAK expected: i2c_master_send failed 
with %d (try %d)\n,
+   ret, i+1);
}
if (ret != 1) {
dev_err(tx-ir-l.dev, IR TX chip never got ready: last 
i2c_master_send 
@@ -1167,12 +1164,10 @@ static ssize_t write(struct file *filep, const char 
__user *buf, size_t n,
 */
if (ret != 0) {
/* Looks like the chip crashed, reset it */
-   dev_err(tx-ir-l.dev, sending to the IR transmitter 
chip 
-   failed, trying reset\n);
+   dev_err(tx-ir-l.dev, sending to the IR transmitter 
chip failed, trying reset\n);
 
if (failures = 3) {
-   dev_err(tx-ir-l.dev

[PATCH v2] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

[Luis de Bethencourt]

checkpatch makes an exception to the 80-colum rule for quotes strings, and
Documentation/CodingStyle recommends not splitting quotes strings across lines
because it breaks the ability to grep for the string. Fixing these.

WARNING: quoted string split across lines

Signed-off-by: Luis de Bethencourt l...@debethencourt.com
---
Changes in v2:
- As pointed out by Joe Perches I missed a space when joining a set of 
strings

Thanks for the review Joe
 drivers/staging/media/lirc/lirc_zilog.c | 39 ++---
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index dca806a..a35d6f2 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -372,14 +372,12 @@ static int add_to_buf(struct IR *ir)
   ret);
if (failures = 3) {
mutex_unlock(ir-ir_lock);
-   dev_err(ir-l.dev, unable to read from the IR 
chip 
-   after 3 resets, giving up\n);
+   dev_err(ir-l.dev, unable to read from the IR 
chip after 3 resets, giving up\n);
break;
}
 
/* Looks like the chip crashed, reset it */
-   dev_err(ir-l.dev, polling the IR receiver chip 
failed, 
-   trying reset\n);
+   dev_err(ir-l.dev, polling the IR receiver chip 
failed, trying reset\n);
 
set_current_state(TASK_UNINTERRUPTIBLE);
if (kthread_should_stop()) {
@@ -405,8 +403,8 @@ static int add_to_buf(struct IR *ir)
ret = i2c_master_recv(rx-c, keybuf, sizeof(keybuf));
mutex_unlock(ir-ir_lock);
if (ret != sizeof(keybuf)) {
-   dev_err(ir-l.dev, i2c_master_recv failed with %d -- 
-   keeping last read buffer\n, ret);
+   dev_err(ir-l.dev, i2c_master_recv failed with %d -- 
keeping last read buffer\n,
+   ret);
} else {
rx-b[0] = keybuf[3];
rx-b[1] = keybuf[4];
@@ -713,8 +711,8 @@ static int send_boot_data(struct IR_tx *tx)
   buf[0]);
return 0;
}
-   dev_notice(tx-ir-l.dev, Zilog/Hauppauge IR blaster firmware version 
-%d.%d.%d loaded\n, buf[1], buf[2], buf[3]);
+   dev_notice(tx-ir-l.dev, Zilog/Hauppauge IR blaster firmware version 
%d.%d.%d loaded\n,
+buf[1], buf[2], buf[3]);
 
return 0;
 }
@@ -794,8 +792,7 @@ static int fw_load(struct IR_tx *tx)
if (!read_uint8(data, tx_data-endp, version))
goto corrupt;
if (version != 1) {
-   dev_err(tx-ir-l.dev, unsupported code set file version (%u, 
expected
-   1) -- please upgrade to a newer driver,
+   dev_err(tx-ir-l.dev, unsupported code set file version (%u, 
expected 1) -- please upgrade to a newer driver,
version);
fw_unload_locked();
ret = -EFAULT;
@@ -983,8 +980,8 @@ static int send_code(struct IR_tx *tx, unsigned int code, 
unsigned int key)
ret = get_key_data(data_block, code, key);
 
if (ret == -EPROTO) {
-   dev_err(tx-ir-l.dev, failed to get data for code %u, key %u 
-- check 
-   lircd.conf entries\n, code, key);
+   dev_err(tx-ir-l.dev, failed to get data for code %u, key %u 
-- check lircd.conf entries\n,
+   code, key);
return ret;
} else if (ret != 0)
return ret;
@@ -1059,8 +1056,8 @@ static int send_code(struct IR_tx *tx, unsigned int code, 
unsigned int key)
ret = i2c_master_send(tx-c, buf, 1);
if (ret == 1)
break;
-   dev_dbg(tx-ir-l.dev, NAK expected: i2c_master_send 
-   failed with %d (try %d)\n, ret, i+1);
+   dev_dbg(tx-ir-l.dev, NAK expected: i2c_master_send failed 
with %d (try %d)\n,
+   ret, i+1);
}
if (ret != 1) {
dev_err(tx-ir-l.dev, IR TX chip never got ready: last 
i2c_master_send 
@@ -1167,12 +1164,10 @@ static ssize_t write(struct file *filep, const char 
__user *buf, size_t n,
 */
if (ret != 0) {
/* Looks like the chip crashed, reset it */
-   dev_err(tx-ir-l.dev, sending to the IR transmitter 
chip 
-   failed, trying reset\n);
+   dev_err(tx-ir-l.dev, sending to the IR

Re: [PATCH] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

[Luis de Bethencourt]

On Tue, Nov 25, 2014 at 12:27:24PM -0800, Joe Perches wrote:
 On Tue, 2014-11-25 at 20:19 +, Luis de Bethencourt wrote:
  checkpatch makes an exception to the 80-colum rule for quotes strings, and
  Documentation/CodingStyle recommends not splitting quotes strings across 
  lines
  because it breaks the ability to grep for the string. Fixing these.
 []
  diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
  b/drivers/staging/media/lirc/lirc_zilog.c
 []
  @@ -794,8 +792,7 @@ static int fw_load(struct IR_tx *tx)
  if (!read_uint8(data, tx_data-endp, version))
  goto corrupt;
  if (version != 1) {
  -   dev_err(tx-ir-l.dev, unsupported code set file version (%u, 
  expected
  -   1) -- please upgrade to a newer driver,
  +   dev_err(tx-ir-l.dev, unsupported code set file version (%u, 
  expected1) -- please upgrade to a newer driver,
  version);
 
 Hello Luis.
 
 Please look at the strings being coalesced before
 submitting patches.
 
 It's a fairly common defect to have either a missing
 space between the coalesced fragments or too many
 spaces.
 
 It's almost certain that there should be a space
 between the expected and 1 here.
 
 

Hello Joe,

Thanks for taking the time to review this. I sent a new
version fixing the missing space. 
--
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] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

[Luis de Bethencourt]

On Tue, Nov 25, 2014 at 01:00:07PM -0800, Joe Perches wrote:
 On Tue, 2014-11-25 at 20:40 +, Luis de Bethencourt wrote:
  On Tue, Nov 25, 2014 at 12:27:24PM -0800, Joe Perches wrote:
   On Tue, 2014-11-25 at 20:19 +, Luis de Bethencourt wrote:
checkpatch makes an exception to the 80-colum rule for quotes strings, 
and
Documentation/CodingStyle recommends not splitting quotes strings 
across lines
because it breaks the ability to grep for the string. Fixing these.
   []
diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
   []
@@ -794,8 +792,7 @@ static int fw_load(struct IR_tx *tx)
if (!read_uint8(data, tx_data-endp, version))
goto corrupt;
if (version != 1) {
-   dev_err(tx-ir-l.dev, unsupported code set file 
version (%u, expected
-   1) -- please upgrade to a newer driver,
+   dev_err(tx-ir-l.dev, unsupported code set file 
version (%u, expected1) -- please upgrade to a newer driver,
version);
   
   Hello Luis.
   
   Please look at the strings being coalesced before
   submitting patches.
   
   It's a fairly common defect to have either a missing
   space between the coalesced fragments or too mano
   spaces.
   
   It's almost certain that there should be a space
   between the expected and 1 here.
   
   
  
  Hello Joe,
  
  Thanks for taking the time to review this. I sent a new
  version fixing the missing space. 
 
 Thanks.
 
 In the future, you might consider being more
 comprehensive with your patches.

Wasn't sure about the scope of the style fixing
patches. I've been reading Kernel Newbies and
this looked like a good way to start
contributing. Good to know more exhaustive
changes are welcome.

 
 This code could be neatened a bit by:
 
 o using another set of logging macros
 o removing the unnecessary ftrace like logging
 o realigning arguments

Great ideas.
Should this have been all included in one patch,
or each as part of a series with the previous
one?
Want to take the opportunity to learn about the
process.

 
 Something like:
 
 ---
  drivers/staging/media/lirc/lirc_zilog.c | 151 
 +++-
  1 file changed, 73 insertions(+), 78 deletions(-)
 
 diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
 b/drivers/staging/media/lirc/lirc_zilog.c
 index bebb9f1..523af12 100644
 --- a/drivers/staging/media/lirc/lirc_zilog.c
 +++ b/drivers/staging/media/lirc/lirc_zilog.c
 @@ -158,6 +158,17 @@ static bool debug;   /* debug output */
  static bool tx_only; /* only handle the IR Tx function */
  static int minor = -1;   /* minor number */
  
 +/* logging macros */
 +#define ir_err(ir, fmt, ...) \
 + dev_err((ir)-l.dev, fmt, ##__VA_ARGS__)
 +#define ir_warn(ir, fmt, ...)\
 + dev_warn((ir)-l.dev, fmt, ##__VA_ARGS__)
 +#define ir_notice(ir, fmt, ...)  \
 + dev_notice((ir)-l.dev, fmt, ##__VA_ARGS__)
 +#define ir_info(ir, fmt, ...)\
 + dev_info((ir)-l.dev, fmt, ##__VA_ARGS__)
 +#define ir_dbg(ir, fmt, ...) \
 + dev_dbg((ir)-l.dev, fmt, ##__VA_ARGS__)
  
  /* struct IR reference counting */
  static struct IR *get_ir_device(struct IR *ir, bool ir_devices_lock_held)
 @@ -322,7 +333,7 @@ static int add_to_buf(struct IR *ir)
   struct IR_tx *tx;
  
   if (lirc_buffer_full(rbuf)) {
 - dev_dbg(ir-l.dev, buffer overflow\n);
 + ir_dbg(ir, buffer overflow\n);
   return -EOVERFLOW;
   }
  
 @@ -368,18 +379,15 @@ static int add_to_buf(struct IR *ir)
*/
   ret = i2c_master_send(rx-c, sendbuf, 1);
   if (ret != 1) {
 - dev_err(ir-l.dev, i2c_master_send failed with %d\n,
 -ret);
 + ir_err(ir, i2c_master_send failed with %d\n, ret);
   if (failures = 3) {
   mutex_unlock(ir-ir_lock);
 - dev_err(ir-l.dev, unable to read from the IR 
 chip 
 - after 3 resets, giving up\n);
 + ir_err(ir, unable to read from the IR chip 
 after 3 resets, giving up\n);
   break;
   }
  
   /* Looks like the chip crashed, reset it */
 - dev_err(ir-l.dev, polling the IR receiver chip 
 failed, 
 - trying reset\n);
 + ir_err(ir, polling the IR receiver chip failed, trying 
 reset\n);
  
   set_current_state(TASK_UNINTERRUPTIBLE);
   if (kthread_should_stop()) {
 @@ -405,14 +413,13 @@ static int add_to_buf(struct IR *ir)
   ret