cron job: media_tree daily build: ERRORS

2016-10-15 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Sun Oct 16 05:00:15 CEST 2016
media-tree git hash:11a1e0ed7908f04c896e69d0eb65e478c12f8519
media_build git hash:   ecfc9bfca3012b0c6e19967ce90f621f71a6da94
v4l-utils git hash: 79186f9d3d9d3b6bee4a611bd92435d11807
gcc version:i686-linux-gcc (GCC) 6.2.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.7.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: WARNINGS
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: OK
linux-3.13.11-i686: WARNINGS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9-rc1-i686: ERRORS
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: OK
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: OK
linux-4.9-rc1-x86_64: ERRORS
apps: WARNINGS
spec-git: OK
smatch: ERRORS
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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


Re: [PATCH v2 02/31] cinergyT2-core: don't do DMA on stack

2016-10-15 Thread Johannes Stezenbach
On Tue, Oct 11, 2016 at 07:09:17AM -0300, Mauro Carvalho Chehab wrote:
> --- a/drivers/media/usb/dvb-usb/cinergyT2-core.c
> +++ b/drivers/media/usb/dvb-usb/cinergyT2-core.c
> @@ -41,6 +41,8 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
>  
>  struct cinergyt2_state {
>   u8 rc_counter;
> + unsigned char data[64];
> + struct mutex data_mutex;
>  };

Sometimes my thinking is slow but it just occured to me
that this creates a potential issue with cache line sharing.
On an architecture which manages cache coherence in software
(ARM, MIPS etc.) a write to e.g. rc_counter in this example
would dirty the cache line, and a later writeback from the
cache could overwrite parts of data[] which was received via DMA.
In contrast, if the DMA buffer is allocated seperately via
kmalloc it is guaranteed to be safe wrt cache line sharing.
(see bottom of Documentation/DMA-API-HOWTO.txt).

But of course DMA on stack also had the same issue
and no one ever noticed so it's apparently not critical...


Johannes
--
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: musb: isoc pkt loss with pwc

2016-10-15 Thread Matwey V. Kornilov
2016-09-12 22:38 GMT+03:00 Matwey V. Kornilov :
> 2016-09-12 21:57 GMT+03:00 Bin Liu :
>> Hi,
>>
>> On Mon, Sep 12, 2016 at 11:52:46AM +0300, Matwey V. Kornilov wrote:
>>> 2016-09-12 6:28 GMT+03:00 Bin Liu :
>>> > Hi,
>>> >
>>> > On Tue, Aug 30, 2016 at 11:44:33PM +0300, Matwey V. Kornilov wrote:
>>> >> 2016-08-30 21:30 GMT+03:00 Bin Liu :
>>> >> > Hi,
>>> >> >
>>> >> > On Sun, Aug 28, 2016 at 01:13:55PM +0300, Matwey V. Kornilov wrote:
>>> >> >> Hello Bin,
>>> >> >>
>>> >> >> I would like to start new thread on my issue. Let me recall where the 
>>> >> >> issue is:
>>> >> >> There is 100% frame lost in pwc webcam driver due to lots of
>>> >> >> zero-length packages coming from musb driver.
>>> >> >
>>> >> > What is the video resolution and fps?
>>> >>
>>> >> 640x480 YUV420 10 frames per second.
>>> >> pwc uses proprietary compression during device-host transmission, but
>>> >> I don't know how effective it is.
>>> >
>>> > The data rate for VGA YUV420 @10fps is 640x480*1.5*10 = 4.6MB/s, which
>>> > is much higher than full-speed 12Mbps.  So the video data on the bus is
>>> > compressed, not YUV420, I believe.
>>> >
>>> >>
>>> >> >
>>> >> >> The issue is present in all kernels (including 4.8) starting from the 
>>> >> >> commit:
>>> >> >>
>>> >> >> f551e13529833e052f75ec628a8af7b034af20f9 ("Revert "usb: musb:
>>> >> >> musb_host: Enable HCD_BH flag to handle urb return in bottom half"")
>>> >> >
>>> >> > What is the behavior without this commit?
>>> >>
>>> >> Without this commit all frames are being received correctly. Single
>>> >
>>> > Which means without this commit your camera has been working without
>>> > issues, and this is a regression with this commit, right?
>>> >
>>>
>>> Right
>>
>> Okay, thanks for confirming.
>>
>> But we cannot just simply add this flag, as it breaks many other use
>> cases. I will continue work on this to find a solution which works on
>> all use cases.
>>
>
> Ok, thank you.
>

Excuse me. Any news?

>> Regards,
>> -Bin.
>>
>
>
>
> --
> With best regards,
> Matwey V. Kornilov.
> Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
> 119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
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 04/18] [media] RedRat3: One function call less in redrat3_transmit_ir() after error detection

2016-10-15 Thread SF Markus Elfring
>> diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
>> index 7ae2ced..71e901d 100644
>> --- a/drivers/media/rc/redrat3.c
>> +++ b/drivers/media/rc/redrat3.c
>> @@ -723,10 +723,10 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, 
>> unsigned *txbuf,
>>  {
>>  struct redrat3_dev *rr3 = rcdev->priv;
>>  struct device *dev = rr3->dev;
>> -struct redrat3_irdata *irdata = NULL;
>> +struct redrat3_irdata *irdata;
>>  int ret, ret_len;
>>  int lencheck, cur_sample_len, pipe;
>> -int *sample_lens = NULL;
>> +int *sample_lens;
>>  u8 curlencheck;
>>  unsigned i, sendbuf_len;
>>  
>> @@ -747,7 +747,7 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, 
>> unsigned *txbuf,
>>  irdata = kzalloc(sizeof(*irdata), GFP_KERNEL);
>>  if (!irdata) {
>>  ret = -ENOMEM;
>> -goto out;
>> +goto free_sample;
>>  }
>>  
>>  /* rr3 will disable rc detector on transmit */
>> @@ -776,7 +776,7 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, 
>> unsigned *txbuf,
>>  curlencheck++;
>>  } else {
>>  ret = -EINVAL;
>> -goto out;
>> +goto reset_member;
>>  }
>>  }
>>  irdata->sigdata[i] = lencheck;
>> @@ -811,14 +811,12 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, 
>> unsigned *txbuf,
>>  dev_err(dev, "Error: control msg send failed, rc %d\n", ret);
>>  else
>>  ret = count;
>> -
>> -out:
>> -kfree(irdata);
>> -kfree(sample_lens);
>> -
>> +reset_member:
>>  rr3->transmitting = false;
>>  /* rr3 re-enables rc detector because it was enabled before */
>> -
>> +kfree(irdata);
>> +free_sample:
>> +kfree(sample_lens);
> 
> In this error path, rr3->transmitting is not set to false

Can it be that this reset is not needed because it should have still got this 
value already
in the software refactoring I proposed here?


> so now the driver will never allow you transmit again.

I have got an other impression.


> Also this patch does not apply against latest.

Do you want that I rebase my update suggestion for this software module on a 
published commit
that is more recent than 2016-09-22 (d6ae162bd13998a6511e5efbc7c19ab542ba1555 
for example)?

Regards,
Markus
--
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 4/5] [media] winbond-cir: One variable and its check less in wbcir_shutdown() after error detection

2016-10-15 Thread SF Markus Elfring
>> +/* Set CEIR_EN */
>> +wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x01, 0x01);
>> +set_irqmask:
>>  /*
>>   * ACPI will set the HW disable bit for SP3 which means that the
>>   * output signals are left in an undefined state which may cause
>> @@ -876,6 +858,14 @@ wbcir_shutdown(struct pnp_dev *device)
>>   */
>>  wbcir_set_irqmask(data, WBCIR_IRQ_NONE);
>>  disable_irq(data->irq);
>> +return;
>> +clear_bits:
>> +/* Clear BUFF_EN, Clear END_EN, Clear MATCH_EN */
>> +wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x00, 0x07);
>> +
>> +/* Clear CEIR_EN */
>> +wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x00, 0x01);
>> +goto set_irqmask;
> 
> I'm not convinced that adding a goto which goes backwards is making this
> code any more readible, just so that a local variable can be dropped.

Thanks for your feedback.

Is such a "backward jump" usual and finally required when you would like
to move a bit of common error handling code to the end without using extra
local variables and a few statements should still be performed after it?

Regards,
Markus
--
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 00/57] don't break long lines on strings

2016-10-15 Thread Mauro Carvalho Chehab
Em Sat, 15 Oct 2016 15:46:14 +0200
Stefan Richter  escreveu:

> On Oct 14 Mauro Carvalho Chehab wrote:
> > There are lots of drivers on media that breaks long line strings in order to
> > fit into 80 columns. This was an usual practice to make checkpatch happy.  
> 
> This was practice even before checkpatch existed.

True, but before checkpatch, we didn't care much to enforce breaking
long string lines, letting up to the patch author to decide. As far
as I remember, on media subsystem, only a handful drivers were actually
breaking long strings (or even respecting the 80 cols limit). 

After checkpatch, we started to enforce such practice, until some discussions 
on LKML arguing that breaking long strings actually make more harm than good,
as it makes harder to use grep to identify what part of the Kernel produced
a certain log message.

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


Re: [PATCH 03/57] [media] firewire: don't break long lines

2016-10-15 Thread Stefan Richter
On Oct 15 Takashi Sakamoto wrote:
> On Oct 15 2016 05:19, Mauro Carvalho Chehab wrote:
> > Due to the 80-cols checkpatch warnings, several strings
> > were broken into multiple lines. This is not considered
> > a good practice anymore, as it makes harder to grep for
> > strings at the source code. So, join those continuation
> > lines.
> > 
> > Signed-off-by: Mauro Carvalho Chehab   
> 
> I prefer this patch because of the same reason in patch comment.
> 
> Reviewed-by: Takashi Sakamoto 

Acked-by: Stefan Richter 

> > ---
> >  drivers/media/firewire/firedtv-avc.c | 5 +++--
> >  drivers/media/firewire/firedtv-rc.c  | 5 +++--
> >  2 files changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/firewire/firedtv-avc.c 
> > b/drivers/media/firewire/firedtv-avc.c
> > index 251a556112a9..e04235ea23fb 100644
> > --- a/drivers/media/firewire/firedtv-avc.c
> > +++ b/drivers/media/firewire/firedtv-avc.c
> > @@ -1181,8 +1181,9 @@ int avc_ca_pmt(struct firedtv *fdtv, char *msg, int 
> > length)
> > if (es_info_length > 0) {
> > pmt_cmd_id = msg[read_pos++];
> > if (pmt_cmd_id != 1 && pmt_cmd_id != 4)
> > -   dev_err(fdtv->device, "invalid pmt_cmd_id %d "
> > -   "at stream level\n", pmt_cmd_id);
> > +   dev_err(fdtv->device,
> > +   "invalid pmt_cmd_id %d at stream 
> > level\n",
> > +   pmt_cmd_id);
> >  
> > if (es_info_length > sizeof(c->operand) - 4 -
> >  write_pos) {
> > diff --git a/drivers/media/firewire/firedtv-rc.c 
> > b/drivers/media/firewire/firedtv-rc.c
> > index f82d4a93feb3..babfb9cee20e 100644
> > --- a/drivers/media/firewire/firedtv-rc.c
> > +++ b/drivers/media/firewire/firedtv-rc.c
> > @@ -184,8 +184,9 @@ void fdtv_handle_rc(struct firedtv *fdtv, unsigned int 
> > code)
> > else if (code >= 0x4540 && code <= 0x4542)
> > code = oldtable[code - 0x4521];
> > else {
> > -   printk(KERN_DEBUG "firedtv: invalid key code 0x%04x "
> > -  "from remote control\n", code);
> > +   printk(KERN_DEBUG
> > +  "firedtv: invalid key code 0x%04x from remote control\n",
> > +  code);
> > return;
> > }  
> 
> 
> Regards
> 
> Takashi Sakamoto
> --
> 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



-- 
Stefan Richter
-==- =-=- -
http://arcgraph.de/sr/
--
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 00/57] don't break long lines on strings

2016-10-15 Thread Stefan Richter
On Oct 14 Mauro Carvalho Chehab wrote:
> There are lots of drivers on media that breaks long line strings in order to
> fit into 80 columns. This was an usual practice to make checkpatch happy.

This was practice even before checkpatch existed.
-- 
Stefan Richter
-==- =-=- -
http://arcgraph.de/sr/
--
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 04/18] [media] RedRat3: One function call less in redrat3_transmit_ir() after error detection

2016-10-15 Thread Sean Young
On Thu, Oct 13, 2016 at 06:24:46PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Thu, 13 Oct 2016 10:50:24 +0200
> 
> The kfree() function was called in one case by the
> redrat3_transmit_ir() function during error handling
> even if the passed variable contained a null pointer.
> 
> * Adjust jump targets according to the Linux coding style convention.
> 
> * Move the resetting for the data structure member "transmitting"
>   at the end.
> 
> * Delete initialisations for the variables "irdata" and "sample_lens"
>   at the beginning which became unnecessary with this refactoring.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/media/rc/redrat3.c | 18 --
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
> index 7ae2ced..71e901d 100644
> --- a/drivers/media/rc/redrat3.c
> +++ b/drivers/media/rc/redrat3.c
> @@ -723,10 +723,10 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, 
> unsigned *txbuf,
>  {
>   struct redrat3_dev *rr3 = rcdev->priv;
>   struct device *dev = rr3->dev;
> - struct redrat3_irdata *irdata = NULL;
> + struct redrat3_irdata *irdata;
>   int ret, ret_len;
>   int lencheck, cur_sample_len, pipe;
> - int *sample_lens = NULL;
> + int *sample_lens;
>   u8 curlencheck;
>   unsigned i, sendbuf_len;
>  
> @@ -747,7 +747,7 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, 
> unsigned *txbuf,
>   irdata = kzalloc(sizeof(*irdata), GFP_KERNEL);
>   if (!irdata) {
>   ret = -ENOMEM;
> - goto out;
> + goto free_sample;
>   }
>  
>   /* rr3 will disable rc detector on transmit */
> @@ -776,7 +776,7 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, 
> unsigned *txbuf,
>   curlencheck++;
>   } else {
>   ret = -EINVAL;
> - goto out;
> + goto reset_member;
>   }
>   }
>   irdata->sigdata[i] = lencheck;
> @@ -811,14 +811,12 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, 
> unsigned *txbuf,
>   dev_err(dev, "Error: control msg send failed, rc %d\n", ret);
>   else
>   ret = count;
> -
> -out:
> - kfree(irdata);
> - kfree(sample_lens);
> -
> +reset_member:
>   rr3->transmitting = false;
>   /* rr3 re-enables rc detector because it was enabled before */
> -
> + kfree(irdata);
> +free_sample:
> + kfree(sample_lens);

In this error path, rr3->transmitting is not set to false so now the driver
will never allow you transmit again.

Also this patch does not apply against latest.

Sean

>   return ret;
>  }
>  
> -- 
> 2.10.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
--
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 4/5] [media] winbond-cir: One variable and its check less in wbcir_shutdown() after error detection

2016-10-15 Thread Sean Young
On Fri, Oct 14, 2016 at 01:44:02PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Fri, 14 Oct 2016 12:48:41 +0200
> 
> The local variable "do_wake" was set to "false" after an invalid system
> setting was detected so that a bit of error handling was triggered.
> 
> * Replace these assignments by direct jumps to the source code with the
> desired exception handling.
> 
> * Delete this status variable and a corresponding check which became
>   unnecessary with this refactoring.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/media/rc/winbond-cir.c | 78 
> ++
>  1 file changed, 34 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index 9d05e17..3d286b9 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -699,16 +699,13 @@ wbcir_shutdown(struct pnp_dev *device)
>  {
>   struct device *dev = &device->dev;
>   struct wbcir_data *data = pnp_get_drvdata(device);
> - bool do_wake = true;
>   u8 match[11];
>   u8 mask[11];
>   u8 rc6_csl;
>   int i;
>  
> - if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev)) {
> - do_wake = false;
> - goto finish;
> - }
> + if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev))
> + goto clear_bits;
>  
>   rc6_csl = 0;
>   memset(match, 0, sizeof(match));
> @@ -716,9 +713,8 @@ wbcir_shutdown(struct pnp_dev *device)
>   switch (protocol) {
>   case IR_PROTOCOL_RC5:
>   if (wake_sc > 0xFFF) {
> - do_wake = false;
>   dev_err(dev, "RC5 - Invalid wake scancode\n");
> - break;
> + goto clear_bits;
>   }
>  
>   /* Mask = 13 bits, ex toggle */
> @@ -735,9 +731,8 @@ wbcir_shutdown(struct pnp_dev *device)
>  
>   case IR_PROTOCOL_NEC:
>   if (wake_sc > 0xFF) {
> - do_wake = false;
>   dev_err(dev, "NEC - Invalid wake scancode\n");
> - break;
> + goto clear_bits;
>   }
>  
>   mask[0] = mask[1] = mask[2] = mask[3] = 0xFF;
> @@ -757,9 +752,8 @@ wbcir_shutdown(struct pnp_dev *device)
>  
>   if (wake_rc6mode == 0) {
>   if (wake_sc > 0x) {
> - do_wake = false;
>   dev_err(dev, "RC6 - Invalid wake scancode\n");
> - break;
> + goto clear_bits;
>   }
>  
>   /* Command */
> @@ -813,9 +807,8 @@ wbcir_shutdown(struct pnp_dev *device)
>   } else if (wake_sc <= 0x007F) {
>   rc6_csl = 60;
>   } else {
> - do_wake = false;
>   dev_err(dev, "RC6 - Invalid wake scancode\n");
> - break;
> + goto clear_bits;
>   }
>  
>   /* Header */
> @@ -825,49 +818,38 @@ wbcir_shutdown(struct pnp_dev *device)
>   mask[i++] = 0x0F;
>  
>   } else {
> - do_wake = false;
>   dev_err(dev, "RC6 - Invalid wake mode\n");
> + goto clear_bits;
>   }
>  
>   break;
>  
>   default:
> - do_wake = false;
> - break;
> + goto clear_bits;
>   }
>  
> -finish:
> - if (do_wake) {
> - /* Set compare and compare mask */
> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
> -WBCIR_REGSEL_COMPARE | WBCIR_REG_ADDR0,
> -0x3F);
> - outsb(data->wbase + WBCIR_REG_WCEIR_DATA, match, 11);
> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
> -WBCIR_REGSEL_MASK | WBCIR_REG_ADDR0,
> -0x3F);
> - outsb(data->wbase + WBCIR_REG_WCEIR_DATA, mask, 11);
> -
> - /* RC6 Compare String Len */
> - outb(rc6_csl, data->wbase + WBCIR_REG_WCEIR_CSL);
> -
> - /* Clear status bits NEC_REP, BUFF, MSG_END, MATCH */
> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_STS, 0x17, 0x17);
> + /* Set compare and compare mask */
> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
> +WBCIR_REGSEL_COMPARE | WBCIR_REG_ADDR0,
> +0x3F);
> + outsb(data->wbase + WBCIR_REG_WCEIR_DATA, match, 11);
> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
> +WBCIR_REGSEL_MASK | WBCIR_REG_ADDR0,
> +0x3F);
> + outsb(data->wbase + WBCIR_REG_WCEIR_DATA, mask, 11);
>  
> - /* Clear BUFF_EN,

Re: [RFC 1/5] media: i2c: max2175: Add MAX2175 support

2016-10-15 Thread Geert Uytterhoeven
Hi Ramesh,

On Wed, Oct 12, 2016 at 4:10 PM, Ramesh Shanmugasundaram
 wrote:
> This patch adds driver support for MAX2175 chip. This is Maxim
> Integrated's RF to Bits tuner front end chip designed for software-defined
> radio solutions. This driver exposes the tuner as a sub-device instance
> with standard and custom controls to configure the device.
>
> Signed-off-by: Ramesh Shanmugasundaram < for your 
> patch!amesh.shanmugasunda...@bp.renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/max2175.txt
> @@ -0,0 +1,60 @@
> +Maxim Integrated MAX2175 RF to Bits tuner
> +-
> +
> +The MAX2175 IC is an advanced analog/digital hybrid-radio receiver with
> +RF to BitsĀ® front-end designed for software-defined radio solutions.
> +
> +Required properties:
> +
> +- compatible: "maxim,max2175" for MAX2175 RF-to-bits tuner.
> +- clocks: phandle to the fixed xtal clock.
> +- clock-names: name of the fixed xtal clock.
> +- port: video interface child port node of a tuner that defines the local
> +  and remote endpoints. The remote endpoint is assumed to be an SDR device
> +  that is capable of receiving the digital samples from the tuner.
> +
> +Optional properties:
> +
> +- maxim,slave : empty property indicates this is a slave of another
> +master tuner. This is used to define two tuners in
> +diversity mode (1 master, 1 slave). By default each
> +tuner is an individual master.
> +- maxim,refout-load: load capacitance value (in pF) on reference output
> +drive level. The mapping of these load values to
> +respective bit values are given below.
> +0 - Reference output disabled
> +1 - 10pF load
> +2 - 20pF load
> +3 - 30pF load
> +4 - 40pF load
> +5 - 60pF load
> +6 - 70pF load

For properties involving units, usually the unit is made part of the
property name, e.g. maxim,refout-load-pF = 40.
This avoids confusion, and allows for extension later.

> +/* A tuner device instance under i2c bus */
> +max2175_0: tuner@60 {
> +   #clock-cells = <0>;
> +   compatible = "maxim,max2175";
> +   reg = <0x60>;
> +   clocks = <&maxim_xtal>;
> +   clock-names = "xtal";
> +   maxim,refout-load = <10>;

10 is not listed above. Perhaps you meant 10 pF?

> --- /dev/null
> +++ b/drivers/media/i2c/max2175/max2175.c
> @@ -0,0 +1,1624 @@

> +/* NOTE: Any addition/deletion in the below list should be reflected in
> + * max2175_modetag enum
> + */

You can drop the above comment if you make this explicit using C99
designated initializers, cfr. below.

> +static const struct max2175_rxmode eu_rx_modes[] = { /* Indexed by EU 
> modetag */
> +   /* EU modes */
> +   { MAX2175_BAND_VHF, 18264, 0, { 0, 0, 0, 0 } },

[MAX2175_DAB_1_2] = { MAX2175_BAND_VHF, 18264, 0, { 0, 0, 0, 0 } },

> +};
> +
> +static const struct max2175_rxmode na_rx_modes[] = { /* Indexed by NA 
> modetag */
> +   /* NA modes */
> +   { MAX2175_BAND_FM, 98255520, 1, { 0, 0, 0, 0 } },

[MAX2175_NA_FM_1_0] = { MAX2175_BAND_FM, 98255520, 1, { 0, 0, 0, 0 } },

> +struct max2175_ctx {
> +   struct v4l2_subdev sd;
> +   struct i2c_client *client;
> +   struct device *dev;
> +
> +   /* Cached configuration */
> +   u8 regs[256];
> +   enum max2175_modetag mode;  /* Receive mode tag */
> +   u32 freq;   /* In Hz */
> +   struct max2175_rxmode *rx_modes;
> +
> +   /* Device settings */
> +   bool master;
> +   u32 decim_ratio;
> +   u64 xtal_freq;
> +
> +   /* ROM values */
> +   u8 rom_bbf_bw_am;
> +   u8 rom_bbf_bw_fm;
> +   u8 rom_bbf_bw_dab;
> +
> +   /* Local copy of old settings */
> +   u8 i2s_test;
> +
> +   u8 nbd_gain;
> +   u8 nbd_threshold;
> +   u8 wbd_gain;
> +   u8 wbd_threshold;
> +   u8 bbd_threshold;
> +   u8 bbdclip_threshold;
> +   u8 lt_wbd_threshold;
> +   u8 lt_wbd_gain;
> +
> +   /* Controls */
> +   struct v4l2_ctrl_handler ctrl_hdl;
> +   struct v4l2_ctrl *lna_gain; /* LNA gain value */
> +   struct v4l2_ctrl *if_gain;  /* I/F gain value */
> +   struct v4l2_ctrl *pll_lock; /* PLL lock */
> +   struct v4l2_ctrl *i2s_en;   /* I2S output enable */
> +   struct v4l2_ctrl *i2s_mode; /* I2S mode value */
> +   struct v4l2_ctrl *am_hiz;   /* AM High impledance input */
> +   struct v4l2_ctrl *hsls; /* High-side/Low-side polarity */
> +   struct v4l2_ctrl *rx_mode;  /* Receive mode */
> +
> +   /* Driver private variables */
> +   bool mode_resolved; /* Flag to sanity check settings */
> +};

Sorting the struct members by decreasing

Re: [PATCH 03/57] [media] firewire: don't break long lines

2016-10-15 Thread Takashi Sakamoto
Hi,

On Oct 15 2016 05:19, Mauro Carvalho Chehab wrote:
> Due to the 80-cols checkpatch warnings, several strings
> were broken into multiple lines. This is not considered
> a good practice anymore, as it makes harder to grep for
> strings at the source code. So, join those continuation
> lines.
> 
> Signed-off-by: Mauro Carvalho Chehab 

I prefer this patch because of the same reason in patch comment.

Reviewed-by: Takashi Sakamoto 

> ---
>  drivers/media/firewire/firedtv-avc.c | 5 +++--
>  drivers/media/firewire/firedtv-rc.c  | 5 +++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/firewire/firedtv-avc.c 
> b/drivers/media/firewire/firedtv-avc.c
> index 251a556112a9..e04235ea23fb 100644
> --- a/drivers/media/firewire/firedtv-avc.c
> +++ b/drivers/media/firewire/firedtv-avc.c
> @@ -1181,8 +1181,9 @@ int avc_ca_pmt(struct firedtv *fdtv, char *msg, int 
> length)
>   if (es_info_length > 0) {
>   pmt_cmd_id = msg[read_pos++];
>   if (pmt_cmd_id != 1 && pmt_cmd_id != 4)
> - dev_err(fdtv->device, "invalid pmt_cmd_id %d "
> - "at stream level\n", pmt_cmd_id);
> + dev_err(fdtv->device,
> + "invalid pmt_cmd_id %d at stream 
> level\n",
> + pmt_cmd_id);
>  
>   if (es_info_length > sizeof(c->operand) - 4 -
>write_pos) {
> diff --git a/drivers/media/firewire/firedtv-rc.c 
> b/drivers/media/firewire/firedtv-rc.c
> index f82d4a93feb3..babfb9cee20e 100644
> --- a/drivers/media/firewire/firedtv-rc.c
> +++ b/drivers/media/firewire/firedtv-rc.c
> @@ -184,8 +184,9 @@ void fdtv_handle_rc(struct firedtv *fdtv, unsigned int 
> code)
>   else if (code >= 0x4540 && code <= 0x4542)
>   code = oldtable[code - 0x4521];
>   else {
> - printk(KERN_DEBUG "firedtv: invalid key code 0x%04x "
> -"from remote control\n", code);
> + printk(KERN_DEBUG
> +"firedtv: invalid key code 0x%04x from remote control\n",
> +code);
>   return;
>   }


Regards

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