cron job: media_tree daily build: WARNINGS

2017-09-20 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:   Thu Sep 21 05:00:15 CEST 2017
media-tree git hash:1efdf1776e2253b77413c997bed862410e4b6aaf
media_build git hash:   19087750b61fc0c5528e798c47ff845f9234
v4l-utils git hash: 9ee29df352dad950784f0f6f4a1bb96c0aefacc4
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.12.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
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: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.2.37-i686: WARNINGS
linux-3.3.8-i686: WARNINGS
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: WARNINGS
linux-3.12.67-i686: WARNINGS
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.26-i686: OK
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12.1-i686: OK
linux-4.13-i686: OK
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: WARNINGS
linux-3.12.67-x86_64: WARNINGS
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: WARNINGS
linux-4.9.26-x86_64: WARNINGS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: OK
apps: OK
spec-git: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html


Re: [PATCH] media: dvb_ca_en50221: sanity check slot number from userspace

2017-09-20 Thread Dan Carpenter
Looks good.

Reviewed-by: Dan Carpenter 

regards,
dan carpenter



[PATCH v2 17/31] media/i2c/tc358743: Initialize timer

2017-09-20 Thread Kees Cook
This converts to use setup_timer() to set callback and data, though it
doesn't look like this would have worked with timer checking enabled
since no init_timer() was ever called before.

Cc: Mats Randgaard 
Cc: Mauro Carvalho Chehab 
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook 
---
 drivers/media/i2c/tc358743.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index e6f5c363ccab..07ad6a3ff1ec 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1964,8 +1964,8 @@ static int tc358743_probe(struct i2c_client *client,
} else {
INIT_WORK(>work_i2c_poll,
  tc358743_work_i2c_poll);
-   state->timer.data = (unsigned long)state;
-   state->timer.function = tc358743_irq_poll_timer;
+   setup_timer(>timer, tc358743_irq_poll_timer,
+   (unsigned long)state);
state->timer.expires = jiffies +
   msecs_to_jiffies(POLL_INTERVAL_MS);
add_timer(>timer);
-- 
2.7.4



Re: [PATCH 2/5] [media] s2255drv: Adjust 13 checks for null pointers

2017-09-20 Thread Dan Carpenter
On Wed, Sep 20, 2017 at 06:58:56PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Wed, 20 Sep 2017 16:46:19 +0200
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 

You've been told several times that this stuff doesn't work.  Try
applying this patch with `git am` and you'll see why.

regards,
dan carpenter



[PATCH] media: dvb_ca_en50221: sanity check slot number from userspace

2017-09-20 Thread Colin King
From: Colin Ian King 

Currently a user can pass in an unsanitized slot number which
will lead to and out of range index into ca->slot_info. Fix this
by checking that the slot number is no more than the allowed
maximum number of slots. Seems that this bug has been in the driver
forever.

Detected by CoverityScan, CID#139381 ("Untrusted pointer read")

Signed-off-by: Colin Ian King 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 95b3723282f4..e3a92b529dba 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1474,6 +1474,9 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
return -EFAULT;
buf += 2;
count -= 2;
+
+   if (slot >= ca->slot_count)
+   return -EINVAL;
sl = >slot_info[slot];
 
/* check if the slot is actually running */
-- 
2.14.1



Re: [PATCH 01/25] media: dvb_frontend: better document the -EPERM condition

2017-09-20 Thread Shuah Khan
On 09/20/2017 01:11 PM, Mauro Carvalho Chehab wrote:
> Two readonly ioctls can't be allowed if the frontend device
> is opened in read only mode. Explain why.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/dvb-core/dvb_frontend.c | 20 +---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/dvb-core/dvb_frontend.c 
> b/drivers/media/dvb-core/dvb_frontend.c
> index 01bd19fd4c57..7dda5acea3f2 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -1940,9 +1940,23 @@ static int dvb_frontend_ioctl(struct file *file, 
> unsigned int cmd, void *parg)
>   return -ENODEV;
>   }
>  
> - if ((file->f_flags & O_ACCMODE) == O_RDONLY &&
> - (_IOC_DIR(cmd) != _IOC_READ || cmd == FE_GET_EVENT ||
> -  cmd == FE_DISEQC_RECV_SLAVE_REPLY)) {
> + /*
> +  * If the frontend is opened in read-only mode, only the ioctls
> +  * that don't interfere at the tune logic should be accepted.

Nit:

with the tune logic

> +  * That allows an external application to monitor the DVB QoS and
> +  * statistics parameters.
> +  *> +* That matches all _IOR() ioctls, except for two special cases:
> +  *   - FE_GET_EVENT is part of the tuning logic on a DVB application;
> +  *   - FE_DISEQC_RECV_SLAVE_REPLY is part of DiSEqC 2.0
> +  * setup
> +  * So, those two ioctls should also return -EPERM, as otherwise
> +  * reading from them would interfere with a DVB tune application
> +  */
> + if ((file->f_flags & O_ACCMODE) == O_RDONLY
> + && (_IOC_DIR(cmd) != _IOC_READ
> + || cmd == FE_GET_EVENT
> + || cmd == FE_DISEQC_RECV_SLAVE_REPLY)) {
>   up(>sem);
>   return -EPERM;
>   }
> 

Same comment from your previous series. I started looking at
the old series and now the latest. Didn't realize the series
has been revised. :(

Looks good to me

Reviewed by: Shuah Khan 

thanks,
-- Shuah



Re: usb/media/pvrusb2: warning in pvr2_send_request_ex/usb_submit_urb

2017-09-20 Thread Andrey Konovalov
On Wed, Sep 20, 2017 at 9:33 PM, Mike Isely  wrote:

Hi Mike!

>
> What you have here is way beyond just feeding random crap in via the
> syscall interface.  To cause this you have to fake the presence of a
> pvrusb2 compatible *hardware* USB device and then lie about its endpoint
> configuration.  Is that really a concern here?  Are we now saying that
> any kernel driver which talks via USB must now also specifically verify
> the exact expected USB endpoint configuration?  Where does that end?
> How about the vendor-specific RPC protocol that the hardware actually
> implements over the bulk endpoint?  It's likely that the pvrusb2 driver
> may be making assumptions about the expected responses over that
> protocol.

The main assumption here is that an attacker has physical access to a
USB port on a machine. In such case a 100$ Facedancer21 board [1] or a
5$ Raspberry Pi Zero [2] in device mode can be used to emulate
arbitrary USB devices and exploit bugs in the kernel (to execute
arbitrary code or to leak data). USB device descriptors during
enumeration phase and all subsequently received from the device
packets (including vendor-specific protocols) should be considered
untrusted input and checked accordingly.

>
> Please realize that I'm not dismissing this.  I can see some merit in
> this.  But I'm just a bit surprised that now we're going this far.  Is
> this really the intention?  You're talking about code
> (pvrusb2_send_request_ex()) that hasn't changed in about 10 years.
> With this level of paranoia there's got to be a pretty target-rich
> environment over the set of kernel-supported USB devices.

Yes, the intention is to fuzz Linux kernel USB drivers (and USB core
code) by connecting random malformed USB devices and by sending
garbage during subsequent communication.

The fact that the code hasn't changed doesn't mean that it's not buggy :)

>
> To take this another step, wouldn't that same level of paranoia be a
> concern for any externally connected PCI-Express device?  Because that's
> another external way into the computer that involves very non-trivial
> and very hardware-centric protocols.  Thunderbolt devices would be an
> example of this.

At this point being able to connect a PCI-Express device usually leads
to being able to do a DMA attack. But sure, exploitable bugs in
PCE-Express device drivers would be a viable attack vector for systems
with proper IOMMU support. Same goes for any other hot-pluggable
externally accessible port/protocol.

>
>   -Mike

[1] https://int3.cc/products/facedancer21

[2] https://www.raspberrypi.org/products/raspberry-pi-zero/

Thanks!

>
>
> On Wed, 20 Sep 2017, Andrey Konovalov wrote:
>
>> Hi!
>>
>> I've got the following report while fuzzing the kernel with syzkaller.
>>
>> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).
>>
>> There seems to be no check on endpoint type before submitting bulk urb
>> in pvr2_send_request_ex().
>>
>> usb 1-1: New USB device found, idVendor=2040, idProduct=7500
>> usb 1-1: New USB device strings: Mfr=0, Product=255, SerialNumber=0
>> usb 1-1: Product: a
>> gadgetfs: configuration #6
>> pvrusb2: Hardware description: WinTV HVR-1950 Model 750xx
>> usb 1-1: BOGUS urb xfer, pipe 3 != type 1
>> [ cut here ]
>> WARNING: CPU: 1 PID: 2713 at drivers/usb/core/urb.c:449
>> usb_submit_urb+0xf8a/0x11d0
>> Modules linked in:
>> CPU: 1 PID: 2713 Comm: pvrusb2-context Not tainted
>> 4.14.0-rc1-42251-gebb2c2437d80 #210
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> task: 88006b7a18c0 task.stack: 880069978000
>> RIP: 0010:usb_submit_urb+0xf8a/0x11d0 drivers/usb/core/urb.c:448
>> RSP: 0018:88006997f990 EFLAGS: 00010286
>> RAX: 0029 RBX: 880063661900 RCX: 
>> RDX: 0029 RSI: 86876d60 RDI: ed000d32ff24
>> RBP: 88006997fa90 R08: 11000d32fdca R09: 
>> R10:  R11:  R12: 11000d32ff39
>> R13: 0001 R14: 0003 R15: 880068bbed68
>> FS:  () GS:88006c60() knlGS:
>> CS:  0010 DS:  ES:  CR0: 80050033
>> CR2: 01032000 CR3: 6a0ff000 CR4: 06f0
>> Call Trace:
>>  pvr2_send_request_ex+0xa57/0x1d80 
>> drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3645
>>  pvr2_hdw_check_firmware drivers/media/usb/pvrusb2/pvrusb2-hdw.c:1812
>>  pvr2_hdw_setup_low drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2107
>>  pvr2_hdw_setup drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2250
>>  pvr2_hdw_initialize+0x548/0x3c10 
>> drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2327
>>  pvr2_context_check drivers/media/usb/pvrusb2/pvrusb2-context.c:118
>>  pvr2_context_thread_func+0x361/0x8c0
>> drivers/media/usb/pvrusb2/pvrusb2-context.c:167
>>  kthread+0x3a1/0x470 kernel/kthread.c:231
>>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
>> Code: 48 8b 85 30 ff ff ff 48 8d b8 98 

Re: [PATCH 2/6] media: dvb_frontend: cleanup ioctl handling logic

2017-09-20 Thread Mauro Carvalho Chehab
Em Wed, 20 Sep 2017 14:58:12 -0600
Shuah Khan  escreveu:

> > +   c->state = DTV_UNDEFINED;> +err = dvb_frontend_handle_ioctl(file, 
> > cmd, parg);  
> 
> With this change, c->state value gets changed unconditionally before
> calling the ioctl.
> 
> dvb_frontend_ioctl_properties() has logic for c->state == DTV_TUNE.
> Is it safe to set change c->state here? I think it should be set
> only when cmd is != FE_SET_PROPERTY or FE_GET_PROPERTY??

It doesn't mind. c->state is used just for debugging purposes. One of the
patches I made got rid of it.



Thanks,
Mauro


Re: [PATCH 5/6] media: dvb_frontend: better document the -EPERM condition

2017-09-20 Thread Shuah Khan
On 09/19/2017 07:42 AM, Mauro Carvalho Chehab wrote:
> Two readonly ioctls can't be allowed if the frontend device
> is opened in read only mode. Explain why.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/dvb-core/dvb_frontend.c | 20 +---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/dvb-core/dvb_frontend.c 
> b/drivers/media/dvb-core/dvb_frontend.c
> index d0a17d67ab1b..db3f8c597a24 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -1940,9 +1940,23 @@ static int dvb_frontend_ioctl(struct file *file, 
> unsigned int cmd, void *parg)
>   return -ENODEV;
>   }
>  
> - if ((file->f_flags & O_ACCMODE) == O_RDONLY &&
> - (_IOC_DIR(cmd) != _IOC_READ || cmd == FE_GET_EVENT ||
> -  cmd == FE_DISEQC_RECV_SLAVE_REPLY)) {
> + /*
> +  * If the frontend is opened in read-only mode, only the ioctls
> +  * that don't interfere at the tune logic should be accepted.

Nit:

with the tune logic

> +  * That allows an external application to monitor the DVB QoS and
> +  * statistics parameters.
> +  *
> +  * That matches all _IOR() ioctls, except for two special cases:
> +  *   - FE_GET_EVENT is part of the tuning logic on a DVB application;
> +  *   - FE_DISEQC_RECV_SLAVE_REPLY is part of DiSEqC 2.0
> +  * setup
> +  * So, those two ioctls should also return -EPERM, as otherwise
> +  * reading from them would interfere with a DVB tune application
> +  */
> + if ((file->f_flags & O_ACCMODE) == O_RDONLY
> + && (_IOC_DIR(cmd) != _IOC_READ
> + || cmd == FE_GET_EVENT
> + || cmd == FE_DISEQC_RECV_SLAVE_REPLY)) {
>   up(>sem);
>   return -EPERM;
>   }
> 

Looks good to me

Reviewed by: Shuah Khan 

thanks,
-- Shuah


Re: [PATCH 3/6] media: dvb_frontend: get rid of proprierty cache's state

2017-09-20 Thread Shuah Khan
On 09/19/2017 07:42 AM, Mauro Carvalho Chehab wrote:
> In the past, I guess the idea was to use state in order to
> allow an autofush logic. However, in the current code, it is
> used only for debug messages, on a poor man's solution, as
> there's already a debug message to indicate when the properties
> got flushed.
> 
> So, just get rid of it for good.

Okay now PATCH 2/3 makes sense. Looks good to me.

Reviewed-by: Shuah Khan 

> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/dvb-core/dvb_frontend.c | 20 ++--
>  drivers/media/dvb-core/dvb_frontend.h |  5 -
>  2 files changed, 6 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/dvb-core/dvb_frontend.c 
> b/drivers/media/dvb-core/dvb_frontend.c
> index cbe697a094d2..d0a17d67ab1b 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -951,8 +951,6 @@ static int dvb_frontend_clear_cache(struct dvb_frontend 
> *fe)
>   memset(c, 0, offsetof(struct dtv_frontend_properties, strength));
>   c->delivery_system = delsys;
>  
> - c->state = DTV_CLEAR;
> -
>   dev_dbg(fe->dvb->device, "%s: Clearing cache for delivery system %d\n",
>   __func__, c->delivery_system);
>  
> @@ -1775,13 +1773,13 @@ static int dtv_property_process_set(struct 
> dvb_frontend *fe,
>   dvb_frontend_clear_cache(fe);
>   break;
>   case DTV_TUNE:
> - /* interpret the cache of data, build either a traditional 
> frontend
> -  * tunerequest so we can pass validation in the FE_SET_FRONTEND
> -  * ioctl.
> + /*
> +  * Use the cached Digital TV properties to tune the
> +  * frontend
>*/
> - c->state = tvp->cmd;
> - dev_dbg(fe->dvb->device, "%s: Finalised property cache\n",
> - __func__);
> + dev_dbg(fe->dvb->device,
> + "%s: Setting the frontend from property cache\n",
> + __func__);
>  
>   r = dtv_set_frontend(fe);
>   break;
> @@ -1930,7 +1928,6 @@ static int dvb_frontend_ioctl(struct file *file, 
> unsigned int cmd, void *parg)
>  {
>   struct dvb_device *dvbdev = file->private_data;
>   struct dvb_frontend *fe = dvbdev->priv;
> - struct dtv_frontend_properties *c = >dtv_property_cache;
>   struct dvb_frontend_private *fepriv = fe->frontend_priv;
>   int err;
>  
> @@ -1950,7 +1947,6 @@ static int dvb_frontend_ioctl(struct file *file, 
> unsigned int cmd, void *parg)
>   return -EPERM;
>   }
>  
> - c->state = DTV_UNDEFINED;
>   err = dvb_frontend_handle_ioctl(file, cmd, parg);
>  
>   up(>sem);
> @@ -2134,10 +2130,6 @@ static int dvb_frontend_handle_ioctl(struct file *file,
>   }
>   (tvp + i)->result = err;
>   }
> -
> - if (c->state == DTV_TUNE)
> - dev_dbg(fe->dvb->device, "%s: Property cache is full, 
> tuning\n", __func__);
> -
>   kfree(tvp);
>   break;
>   }
> diff --git a/drivers/media/dvb-core/dvb_frontend.h 
> b/drivers/media/dvb-core/dvb_frontend.h
> index 852b91ba49d2..1bdeb10f0d78 100644
> --- a/drivers/media/dvb-core/dvb_frontend.h
> +++ b/drivers/media/dvb-core/dvb_frontend.h
> @@ -620,11 +620,6 @@ struct dtv_frontend_properties {
>   struct dtv_fe_stats post_bit_count;
>   struct dtv_fe_stats block_error;
>   struct dtv_fe_stats block_count;
> -
> - /* private: */
> - /* Cache State */
> - u32 state;
> -
>  };
>  
>  #define DVB_FE_NO_EXIT  0
> 

thanks,
-- Shuah


Re: [PATCH 2/6] media: dvb_frontend: cleanup ioctl handling logic

2017-09-20 Thread Shuah Khan
H Mauro,

On 09/19/2017 07:42 AM, Mauro Carvalho Chehab wrote:
> Currently, there are two handlers for ioctls:
>  - dvb_frontend_ioctl_properties()
>  - dvb_frontend_ioctl_legacy()
> 
> Despite their names, both handles non-legacy DVB ioctls.
> 
> Besides that, there's no reason why to not handle all ioctls
> on a single handler function.
> 
> So, merge them into a single function (dvb_frontend_handle_ioctl)
> and reorganize the ioctl's to indicate what's the current DVB
> API and what's deprecated.
> 
> Despite the big diff, the handling logic for each ioctl is the
> same as before.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/dvb-core/dvb_frontend.c | 402 
> +-
>  1 file changed, 195 insertions(+), 207 deletions(-)
> 
> diff --git a/drivers/media/dvb-core/dvb_frontend.c 
> b/drivers/media/dvb-core/dvb_frontend.c
> index 725cb1c8a088..cbe697a094d2 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -1315,10 +1315,8 @@ static int dtv_get_frontend(struct dvb_frontend *fe,
>   return 0;
>  }
>  
> -static int dvb_frontend_ioctl_legacy(struct file *file,
> - unsigned int cmd, void *parg);
> -static int dvb_frontend_ioctl_properties(struct file *file,
> - unsigned int cmd, void *parg);
> +static int dvb_frontend_handle_ioctl(struct file *file,
> +  unsigned int cmd, void *parg);
>  
>  static int dtv_property_process_get(struct dvb_frontend *fe,
>   const struct dtv_frontend_properties *c,
> @@ -1816,12 +1814,12 @@ static int dtv_property_process_set(struct 
> dvb_frontend *fe,
>   break;
>   case DTV_VOLTAGE:
>   c->voltage = tvp->u.data;
> - r = dvb_frontend_ioctl_legacy(file, FE_SET_VOLTAGE,
> + r = dvb_frontend_handle_ioctl(file, FE_SET_VOLTAGE,
>   (void *)c->voltage);
>   break;
>   case DTV_TONE:
>   c->sectone = tvp->u.data;
> - r = dvb_frontend_ioctl_legacy(file, FE_SET_TONE,
> + r = dvb_frontend_handle_ioctl(file, FE_SET_TONE,
>   (void *)c->sectone);
>   break;
>   case DTV_CODE_RATE_HP:
> @@ -1928,14 +1926,13 @@ static int dtv_property_process_set(struct 
> dvb_frontend *fe,
>   return r;
>  }
>  
> -static int dvb_frontend_ioctl(struct file *file,
> - unsigned int cmd, void *parg)
> +static int dvb_frontend_ioctl(struct file *file, unsigned int cmd, void 
> *parg)
>  {
>   struct dvb_device *dvbdev = file->private_data;
>   struct dvb_frontend *fe = dvbdev->priv;
>   struct dtv_frontend_properties *c = >dtv_property_cache;
>   struct dvb_frontend_private *fepriv = fe->frontend_priv;
> - int err = -EOPNOTSUPP;
> + int err;
>  
>   dev_dbg(fe->dvb->device, "%s: (%d)\n", __func__, _IOC_NR(cmd));
>   if (down_interruptible(>sem))
> @@ -1953,121 +1950,13 @@ static int dvb_frontend_ioctl(struct file *file,
>   return -EPERM;
>   }
>  
> - if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY))
> - err = dvb_frontend_ioctl_properties(file, cmd, parg);
> - else {
> - c->state = DTV_UNDEFINED;
> - err = dvb_frontend_ioctl_legacy(file, cmd, parg);
> - }
> + c->state = DTV_UNDEFINED;> +err = dvb_frontend_handle_ioctl(file, 
> cmd, parg);

With this change, c->state value gets changed unconditionally before
calling the ioctl.

dvb_frontend_ioctl_properties() has logic for c->state == DTV_TUNE.
Is it safe to set change c->state here? I think it should be set
only when cmd is != FE_SET_PROPERTY or FE_GET_PROPERTY??

>  
>   up(>sem);
>   return err;
>  }
>  
> -static int dvb_frontend_ioctl_properties(struct file *file,
> - unsigned int cmd, void *parg)
> -{
> - struct dvb_device *dvbdev = file->private_data;
> - struct dvb_frontend *fe = dvbdev->priv;
> - struct dvb_frontend_private *fepriv = fe->frontend_priv;
> - struct dtv_frontend_properties *c = >dtv_property_cache;
> - int err, i;
> -
> - dev_dbg(fe->dvb->device, "%s:\n", __func__);
> -
> - switch(cmd) {
> - case FE_SET_PROPERTY: {
> - struct dtv_properties *tvps = parg;
> - struct dtv_property *tvp = NULL;
> -
> - dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
> - __func__, tvps->num);
> - dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
> - __func__, tvps->props);
> -
> - /*
> -  * Put an arbitrary limit on the number of messages that can
> -  * be sent at once
> -  */
> - if (!tvps->num || (tvps->num > DTV_IOCTL_MAX_MSGS))
> - return -EINVAL;
> -
> - tvp = memdup_user(tvps->props, 

[PATCH v2] [media] staging: atomisp: use clock framework for camera clocks

2017-09-20 Thread Pierre-Louis Bossart
The Atom ISP driver initializes and configures PMC clocks which are
already handled by the clock framework.

Remove all legacy vlv2_platform_clock stuff and move to the clk API to
avoid conflicts, e.g. with audio machine drivers enabling the MCLK for
external codecs

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Tested-by: Carlo Caione 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Pierre-Louis Bossart 
---
v2: added Andy's Reviewed-by and Fixes tag

 drivers/staging/media/atomisp/Kconfig  |   1 +
 drivers/staging/media/atomisp/platform/Makefile|   1 -
 .../staging/media/atomisp/platform/clock/Makefile  |   6 -
 .../platform/clock/platform_vlv2_plat_clk.c|  40 
 .../platform/clock/platform_vlv2_plat_clk.h|  27 ---
 .../media/atomisp/platform/clock/vlv2_plat_clock.c | 247 -
 .../platform/intel-mid/atomisp_gmin_platform.c |  63 +-
 7 files changed, 52 insertions(+), 333 deletions(-)
 delete mode 100644 drivers/staging/media/atomisp/platform/clock/Makefile
 delete mode 100644 
drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.c
 delete mode 100644 
drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.h
 delete mode 100644 
drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c

diff --git a/drivers/staging/media/atomisp/Kconfig 
b/drivers/staging/media/atomisp/Kconfig
index 8eb13c3ba29c..7cdebea35ccf 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,7 @@
 menuconfig INTEL_ATOMISP
 bool "Enable support to Intel MIPI camera drivers"
 depends on X86 && EFI && MEDIA_CONTROLLER && PCI && ACPI
+   select COMMON_CLK
 help
   Enable support for the Intel ISP2 camera interfaces and MIPI
   sensor drivers.
diff --git a/drivers/staging/media/atomisp/platform/Makefile 
b/drivers/staging/media/atomisp/platform/Makefile
index df157630bda9..0e3b7e1c81c6 100644
--- a/drivers/staging/media/atomisp/platform/Makefile
+++ b/drivers/staging/media/atomisp/platform/Makefile
@@ -2,5 +2,4 @@
 # Makefile for camera drivers.
 #
 
-obj-$(CONFIG_INTEL_ATOMISP) += clock/
 obj-$(CONFIG_INTEL_ATOMISP) += intel-mid/
diff --git a/drivers/staging/media/atomisp/platform/clock/Makefile 
b/drivers/staging/media/atomisp/platform/clock/Makefile
deleted file mode 100644
index 82fbe8b6968a..
--- a/drivers/staging/media/atomisp/platform/clock/Makefile
+++ /dev/null
@@ -1,6 +0,0 @@
-#
-# Makefile for clock devices.
-#
-
-obj-$(CONFIG_INTEL_ATOMISP)+= vlv2_plat_clock.o
-obj-$(CONFIG_INTEL_ATOMISP) += platform_vlv2_plat_clk.o
diff --git 
a/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.c 
b/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.c
deleted file mode 100644
index 0aae9b0283bb..
--- a/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.c
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * platform_vlv2_plat_clk.c - VLV2 platform clock driver
- * Copyright (C) 2013 Intel Corporation
- *
- * Author: Asutosh Pathak 
- * Author: Chandra Sekhar Anagani 
- * Author: Sergio Aguirre 
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 2 of the License.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * General Public License for more details.
- *
- */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-
-static int __init vlv2_plat_clk_init(void)
-{
-   struct platform_device *pdev;
-
-   pdev = platform_device_register_simple("vlv2_plat_clk", -1, NULL, 0);
-   if (IS_ERR(pdev)) {
-   pr_err("platform_vlv2_plat_clk:register failed: %ld\n",
-   PTR_ERR(pdev));
-   return PTR_ERR(pdev);
-   }
-
-   return 0;
-}
-
-device_initcall(vlv2_plat_clk_init);
diff --git 
a/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.h 
b/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.h
deleted file mode 100644
index b730ab0e8223..
--- a/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/*
- * platform_vlv2_plat_clk.h: platform clock driver library header file
- * Copyright (C) 2013 Intel Corporation
- *
- * Author: Asutosh Pathak 
- * Author: Chandra Sekhar Anagani 
- * Author: Sergio Aguirre 
- *
- * This program is free 

Re: [RESEND PATCH v2 4/6] dt: bindings: as3645a: Improve label documentation, DT example

2017-09-20 Thread Rob Herring
On Tue, Sep 19, 2017 at 11:01:02PM +0200, Jacek Anaszewski wrote:
> Hi Pavel,
> 
> On 09/18/2017 10:54 PM, Pavel Machek wrote:
> > On Mon 2017-09-18 17:49:23, Sakari Ailus wrote:
> >> Hi Pavel,
> >>
> >> On Mon, Sep 18, 2017 at 12:56:55PM +0200, Pavel Machek wrote:
> >>> Hi!
> >>>
>  Specify the exact label used if the label property is omitted in DT, as
>  well as use label in the example that conforms to LED device naming.
> 
>  @@ -69,11 +73,11 @@ Example
>   flash-max-microamp = <32>;
>   led-max-microamp = <6>;
>   ams,input-max-microamp = <175>;
>  -label = "as3645a:flash";
>  +label = "as3645a:white:flash";
>   };
>   indicator@1 {
>   reg = <0x1>;
>   led-max-microamp = <1>;
>  -label = "as3645a:indicator";
>  +label = "as3645a:red:indicator";
>   };
>   };
> >>>
> >>> Ok, but userspace still has no chance to determine if this is flash
> >>> from main camera or flash for front camera; todays smartphones have
> >>> flashes on both cameras.
> >>>
> >>> So.. Can I suggset as3645a:white:main_camera_flash or main_flash or
> >>> ?
> >>
> >> If there's just a single one in the device, could you use that?
> >>
> >> Even if we name this so for N9 (and N900), the application still would only
> >> work with the two devices.
> > 
> > Well, I'd plan to name it on other devices, too.
> > 
> >> My suggestion would be to look for a flash LED, and perhaps the maximum
> >> current as well. That should generally work better than assumptions on the
> >> label.
> > 
> > If you just look for flash LED, you don't know if it is front one or
> > back one. Its true that if you have just one flash it is usually on
> > the back camera, but you can't know if maybe driver is not available
> > for the main flash.
> > 
> > Lets get this right, please "main_camera_flash" is 12 bytes more than
> > "flash", and it saves application logic.. more than 12 bytes, I'm sure. 
> 
> What you are trying to introduce is yet another level of LED class
> device naming standard, one level below devicename:colour:function.
> It seems you want also to come up with the set of standarized LED
> function names. This would certainly have to be covered for consistency.

I really dislike how this naming convention is used for label. label is 
supposed to be the phyically identifiable name. Having the devicename 
defeats that. Perhaps color, too. We'd be better off with a color 
property. It seems we're overloading the naming with too many things. 
Now we're adding device association.

I do want to see standard names though. On 96boards for example, there 
are defined LEDs and locations. The function on some are defined (e.g. 
WiFi/BT) and somewhat undefined on others (user{1-4}). I'd like to see 
the same label across all boards.

Rob


Re: [PATCH 2/2] dt: bindings: media: Document data lane numbering without lane reordering

2017-09-20 Thread Rob Herring
On Mon, Sep 18, 2017 at 11:25:05AM +0300, Sakari Ailus wrote:
> Most devices do not support lane reordering and in many cases the
> documentation of the data-lanes property is incomplete for such devices.
> Document that in case the lane reordering isn't supported, monotonically
> incremented values from 0 or 1 shall be used.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)

Acked-by: Rob Herring 



Re: [PATCH 1/2] dt: bindings: media: Document port and endpoint numbering

2017-09-20 Thread Rob Herring
On Mon, Sep 18, 2017 at 11:25:04AM +0300, Sakari Ailus wrote:
> A lot of devices do not need and do not document port or endpoint
> numbering at all, e.g. in case where there's just a single port and a
> single endpoint. Whereas this is just common sense, document it to make it
> explicit.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 12 
> 
>  1 file changed, 12 insertions(+)

This is fine, but bindings should still be explicit. Otherwise, I'm 
wondering if it's a single port/endpoint or they just forgot to document 
it. And I shouldn't have to look at the example to infer that.

Acked-by: Rob Herring 


Re: [PATCH 1/6] media: dvb_frontend: cleanup dvb_frontend_ioctl_properties()

2017-09-20 Thread Mauro Carvalho Chehab
Em Wed, 20 Sep 2017 14:11:39 -0600
Shuah Khan  escreveu:

> On 09/19/2017 07:42 AM, Mauro Carvalho Chehab wrote:
> > Use a switch() on this function, just like on other ioctl
> > handlers and handle parameters inside each part of the
> > switch.
> > 
> > That makes it easier to integrate with the already existing
> > ioctl handler function.
> > 
> > Signed-off-by: Mauro Carvalho Chehab   
> 
> The change looks good. Couple of comments below:
> 
> > ---
> >  drivers/media/dvb-core/dvb_frontend.c | 83 
> > +--
> >  1 file changed, 51 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/media/dvb-core/dvb_frontend.c 
> > b/drivers/media/dvb-core/dvb_frontend.c
> > index 8abe4f541a36..725cb1c8a088 100644
> > --- a/drivers/media/dvb-core/dvb_frontend.c
> > +++ b/drivers/media/dvb-core/dvb_frontend.c
> > @@ -1971,21 +1971,25 @@ static int dvb_frontend_ioctl_properties(struct 
> > file *file,
> > struct dvb_frontend *fe = dvbdev->priv;
> > struct dvb_frontend_private *fepriv = fe->frontend_priv;
> > struct dtv_frontend_properties *c = >dtv_property_cache;
> > -   int err = 0;
> > -
> > -   struct dtv_properties *tvps = parg;
> > -   struct dtv_property *tvp = NULL;
> > -   int i;
> > +   int err, i;
> >  
> > dev_dbg(fe->dvb->device, "%s:\n", __func__);
> >  
> > -   if (cmd == FE_SET_PROPERTY) {
> > -   dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, 
> > tvps->num);
> > -   dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", 
> > __func__, tvps->props);
> > +   switch(cmd) {
> > +   case FE_SET_PROPERTY: {
> > +   struct dtv_properties *tvps = parg;
> > +   struct dtv_property *tvp = NULL;
> >  
> > -   /* Put an arbitrary limit on the number of messages that can
> > -* be sent at once */
> > -   if ((tvps->num == 0) || (tvps->num > DTV_IOCTL_MAX_MSGS))
> > +   dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
> > +   __func__, tvps->num);
> > +   dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
> > +   __func__, tvps->props);
> > +
> > +   /*
> > +* Put an arbitrary limit on the number of messages that can
> > +* be sent at once
> > +*/
> > +   if (!tvps->num || (tvps->num > DTV_IOCTL_MAX_MSGS))
> > return -EINVAL;
> >  
> > tvp = memdup_user(tvps->props, tvps->num * sizeof(*tvp));
> > @@ -1994,23 +1998,34 @@ static int dvb_frontend_ioctl_properties(struct 
> > file *file,
> >  
> > for (i = 0; i < tvps->num; i++) {
> > err = dtv_property_process_set(fe, tvp + i, file);
> > -   if (err < 0)
> > -   goto out;
> > +   if (err < 0) {
> > +   kfree(tvp);
> > +   return err;
> > +   }
> > (tvp + i)->result = err;
> > }
> >  
> > if (c->state == DTV_TUNE)
> > dev_dbg(fe->dvb->device, "%s: Property cache is full, 
> > tuning\n", __func__);
> >  
> > -   } else if (cmd == FE_GET_PROPERTY) {
> > +   kfree(tvp);
> > +   break;
> > +   }
> > +   case FE_GET_PROPERTY: {
> > +   struct dtv_properties *tvps = parg;
> > +   struct dtv_property *tvp = NULL;
> > struct dtv_frontend_properties getp = fe->dtv_property_cache;
> >  
> > -   dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, 
> > tvps->num);
> > -   dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", 
> > __func__, tvps->props);
> > +   dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
> > +   __func__, tvps->num);
> > +   dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
> > +   __func__, tvps->props);
> >  
> > -   /* Put an arbitrary limit on the number of messages that can
> > -* be sent at once */
> > -   if ((tvps->num == 0) || (tvps->num > DTV_IOCTL_MAX_MSGS))
> > +   /*
> > +* Put an arbitrary limit on the number of messages that can
> > +* be sent at once
> > +*/
> > +   if (!tvps->num || (tvps->num > DTV_IOCTL_MAX_MSGS))
> > return -EINVAL;
> >  
> > tvp = memdup_user(tvps->props, tvps->num * sizeof(*tvp));
> > @@ -2025,28 +2040,32 @@ static int dvb_frontend_ioctl_properties(struct 
> > file *file,
> >  */
> > if (fepriv->state != FESTATE_IDLE) {
> > err = dtv_get_frontend(fe, , NULL);
> > -   if (err < 0)
> > -   goto out;
> > +   if (err < 0) {
> > +   kfree(tvp);
> > +   return err;
> > +   }  
> 
> Could avoid 

Re: [PATCH 1/6] media: dvb_frontend: cleanup dvb_frontend_ioctl_properties()

2017-09-20 Thread Shuah Khan
On 09/19/2017 07:42 AM, Mauro Carvalho Chehab wrote:
> Use a switch() on this function, just like on other ioctl
> handlers and handle parameters inside each part of the
> switch.
> 
> That makes it easier to integrate with the already existing
> ioctl handler function.
> 
> Signed-off-by: Mauro Carvalho Chehab 

The change looks good. Couple of comments below:

> ---
>  drivers/media/dvb-core/dvb_frontend.c | 83 
> +--
>  1 file changed, 51 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/media/dvb-core/dvb_frontend.c 
> b/drivers/media/dvb-core/dvb_frontend.c
> index 8abe4f541a36..725cb1c8a088 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -1971,21 +1971,25 @@ static int dvb_frontend_ioctl_properties(struct file 
> *file,
>   struct dvb_frontend *fe = dvbdev->priv;
>   struct dvb_frontend_private *fepriv = fe->frontend_priv;
>   struct dtv_frontend_properties *c = >dtv_property_cache;
> - int err = 0;
> -
> - struct dtv_properties *tvps = parg;
> - struct dtv_property *tvp = NULL;
> - int i;
> + int err, i;
>  
>   dev_dbg(fe->dvb->device, "%s:\n", __func__);
>  
> - if (cmd == FE_SET_PROPERTY) {
> - dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, 
> tvps->num);
> - dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", 
> __func__, tvps->props);
> + switch(cmd) {
> + case FE_SET_PROPERTY: {
> + struct dtv_properties *tvps = parg;
> + struct dtv_property *tvp = NULL;
>  
> - /* Put an arbitrary limit on the number of messages that can
> -  * be sent at once */
> - if ((tvps->num == 0) || (tvps->num > DTV_IOCTL_MAX_MSGS))
> + dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
> + __func__, tvps->num);
> + dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
> + __func__, tvps->props);
> +
> + /*
> +  * Put an arbitrary limit on the number of messages that can
> +  * be sent at once
> +  */
> + if (!tvps->num || (tvps->num > DTV_IOCTL_MAX_MSGS))
>   return -EINVAL;
>  
>   tvp = memdup_user(tvps->props, tvps->num * sizeof(*tvp));
> @@ -1994,23 +1998,34 @@ static int dvb_frontend_ioctl_properties(struct file 
> *file,
>  
>   for (i = 0; i < tvps->num; i++) {
>   err = dtv_property_process_set(fe, tvp + i, file);
> - if (err < 0)
> - goto out;
> + if (err < 0) {
> + kfree(tvp);
> + return err;
> + }
>   (tvp + i)->result = err;
>   }
>  
>   if (c->state == DTV_TUNE)
>   dev_dbg(fe->dvb->device, "%s: Property cache is full, 
> tuning\n", __func__);
>  
> - } else if (cmd == FE_GET_PROPERTY) {
> + kfree(tvp);
> + break;
> + }
> + case FE_GET_PROPERTY: {
> + struct dtv_properties *tvps = parg;
> + struct dtv_property *tvp = NULL;
>   struct dtv_frontend_properties getp = fe->dtv_property_cache;
>  
> - dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, 
> tvps->num);
> - dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", 
> __func__, tvps->props);
> + dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
> + __func__, tvps->num);
> + dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
> + __func__, tvps->props);
>  
> - /* Put an arbitrary limit on the number of messages that can
> -  * be sent at once */
> - if ((tvps->num == 0) || (tvps->num > DTV_IOCTL_MAX_MSGS))
> + /*
> +  * Put an arbitrary limit on the number of messages that can
> +  * be sent at once
> +  */
> + if (!tvps->num || (tvps->num > DTV_IOCTL_MAX_MSGS))
>   return -EINVAL;
>  
>   tvp = memdup_user(tvps->props, tvps->num * sizeof(*tvp));
> @@ -2025,28 +2040,32 @@ static int dvb_frontend_ioctl_properties(struct file 
> *file,
>*/
>   if (fepriv->state != FESTATE_IDLE) {
>   err = dtv_get_frontend(fe, , NULL);
> - if (err < 0)
> - goto out;
> + if (err < 0) {
> + kfree(tvp);
> + return err;
> + }

Could avoid duplicate code keeping out logic perhaps? Is there a reason
for removing this?

>   }
>   for (i = 0; i < tvps->num; i++) {
>   err = 

[RESEND RFC PATCH 7/7] ARM: sun8i: h3: Enable HDMI output on H3 boards

2017-09-20 Thread Jernej Skrabec
Enable HDMI output on all boards which include HDMI connector.

Signed-off-by: Jernej Skrabec 
---
 arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts | 33 +
 arch/arm/boot/dts/sun8i-h3-beelink-x2.dts   | 33 +
 arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts| 33 +
 arch/arm/boot/dts/sun8i-h3-orangepi-2.dts   | 33 +
 arch/arm/boot/dts/sun8i-h3-orangepi-lite.dts| 33 +
 arch/arm/boot/dts/sun8i-h3-orangepi-one.dts | 33 +
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts  | 33 +
 7 files changed, 231 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts 
b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
index d756ff825116..52a0f954df1c 100644
--- a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
@@ -61,6 +61,17 @@
stdout-path = "serial0:115200n8";
};
 
+   connector {
+   compatible = "hdmi-connector";
+   type = "a";
+
+   port {
+   hdmi_con_in: endpoint {
+   remote-endpoint = <_out_con>;
+   };
+   };
+   };
+
leds {
compatible = "gpio-leds";
pinctrl-names = "default";
@@ -103,6 +114,10 @@
};
 };
 
+ {
+   status = "okay";
+};
+
  {
status = "okay";
 };
@@ -126,6 +141,16 @@
status = "okay";
 };
 
+ {
+   status = "okay";
+};
+
+_out {
+   hdmi_out_con: endpoint {
+   remote-endpoint = <_con_in>;
+   };
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
@@ -139,6 +164,10 @@
};
 };
 
+ {
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>, <_cd_pin>;
@@ -212,6 +241,10 @@
status = "okay";
 };
 
+ {
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
diff --git a/arch/arm/boot/dts/sun8i-h3-beelink-x2.dts 
b/arch/arm/boot/dts/sun8i-h3-beelink-x2.dts
index 546837ccd8af..8a4ec474f183 100644
--- a/arch/arm/boot/dts/sun8i-h3-beelink-x2.dts
+++ b/arch/arm/boot/dts/sun8i-h3-beelink-x2.dts
@@ -61,6 +61,17 @@
stdout-path = "serial0:115200n8";
};
 
+   connector {
+   compatible = "hdmi-connector";
+   type = "a";
+
+   port {
+   hdmi_con_in: endpoint {
+   remote-endpoint = <_out_con>;
+   };
+   };
+   };
+
leds {
compatible = "gpio-leds";
 
@@ -100,6 +111,10 @@
};
 };
 
+ {
+   status = "okay";
+};
+
  {
status = "okay";
 };
@@ -115,12 +130,26 @@
status = "okay";
 };
 
+ {
+   status = "okay";
+};
+
+_out {
+   hdmi_out_con: endpoint {
+   remote-endpoint = <_con_in>;
+   };
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
status = "okay";
 };
 
+ {
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>, <_cd_pin>;
@@ -177,6 +206,10 @@
status = "okay";
 };
 
+ {
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
diff --git a/arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts 
b/arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts
index ec63d104b404..e6b551bb43e7 100644
--- a/arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts
+++ b/arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts
@@ -45,6 +45,21 @@
 / {
model = "FriendlyArm NanoPi M1";
compatible = "friendlyarm,nanopi-m1", "allwinner,sun8i-h3";
+
+   connector {
+   compatible = "hdmi-connector";
+   type = "a";
+
+   port {
+   hdmi_con_in: endpoint {
+   remote-endpoint = <_out_con>;
+   };
+   };
+   };
+};
+
+ {
+   status = "okay";
 };
 
  {
@@ -55,6 +70,20 @@
status = "okay";
 };
 
+ {
+   status = "okay";
+};
+
+_out {
+   hdmi_out_con: endpoint {
+   remote-endpoint = <_con_in>;
+   };
+};
+
+ {
+   status = "okay";
+};
+
  {
status = "okay";
 };
@@ -62,3 +91,7 @@
  {
status = "okay";
 };
+
+ {
+   status = "okay";
+};
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts 
b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
index 17cdeae19c6f..586181c4ce8b 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
@@ -62,6 +62,17 @@
stdout-path = "serial0:115200n8";
};
 
+   connector {
+   compatible = "hdmi-connector";
+   type = "a";
+
+   port {
+   hdmi_con_in: endpoint {
+ 

[RESEND RFC PATCH 4/7] dt-bindings: Document Allwinner DWC HDMI TX node

2017-09-20 Thread Jernej Skrabec
Add documentation about Allwinner DWC HDMI TX node, found in H3 SoC.

Signed-off-by: Jernej Skrabec 
---
 .../bindings/display/sunxi/sun4i-drm.txt   | 158 -
 1 file changed, 157 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt 
b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
index 92512953943e..cb6aee5c486f 100644
--- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
+++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
@@ -60,6 +60,40 @@ Required properties:
 first port should be the input endpoint. The second should be the
 output, usually to an HDMI connector.
 
+DWC HDMI TX Encoder
+-
+
+The HDMI transmitter is a Synopsys DesignWare HDMI 1.4 TX controller IP
+with Allwinner's own PHY IP. It supports audio and video outputs and CEC.
+
+These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
+Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt with the
+following device-specific properties.
+
+Required properties:
+
+  - compatible: value must be one of:
+* "allwinner,sun8i-h3-dw-hdmi"
+  - reg: two pairs of base address and size of memory-mapped region, first
+for controller and second for PHY
+registers.
+  - reg-io-width: See dw_hdmi.txt. Shall be 1.
+  - interrupts: HDMI interrupt number
+  - clocks: phandles to the clocks feeding the HDMI encoder
+* iahb: the HDMI interface clock
+* isfr: the HDMI module clock
+* ddc: the HDMI ddc clock
+  - clock-names: the clock names mentioned above
+  - resets: phandles to the reset controllers driving the encoder
+* hdmi: the reset line for the HDMI
+* ddc: the reset line for the DDC
+  - reset-names: the reset names mentioned above
+
+  - ports: A ports node with endpoint definitions as defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt. The
+first port should be the input endpoint. The second should be the
+output, usually to an HDMI connector.
+
 TV Encoder
 --
 
@@ -255,7 +289,7 @@ Required properties:
   - allwinner,pipelines: list of phandle to the display engine
 frontends (DE 1.0) or mixers (DE 2.0) available.
 
-Example:
+Example 1:
 
 panel: panel {
compatible = "olimex,lcd-olinuxino-43-ts";
@@ -455,3 +489,125 @@ display-engine {
compatible = "allwinner,sun5i-a13-display-engine";
allwinner,pipelines = <>;
 };
+
+Example 2:
+
+connector {
+   compatible = "hdmi-connector";
+   type = "a";
+
+   port {
+   hdmi_con_in: endpoint {
+   remote-endpoint = <_out_con>;
+   };
+   };
+};
+
+de: display-engine {
+   compatible = "allwinner,sun8i-h3-display-engine";
+   allwinner,pipelines = <>;
+};
+
+hdmi: hdmi@1ee {
+   compatible = "allwinner,h3-dw-hdmi";
+   reg = <0x01ee 0x1>,
+ <0x01ef 0x1>;
+   reg-io-width = <1>;
+   interrupts = ;
+   clocks = < CLK_BUS_HDMI>, < CLK_HDMI>,
+< CLK_HDMI_DDC>;
+   clock-names = "iahb", "isfr", "ddc";
+   resets = < RST_BUS_HDMI0>, < RST_BUS_HDMI1>;
+   reset-names = "hdmi", "ddc";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   hdmi_in: port@0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0>;
+
+   hdmi_in_tcon0: endpoint@0 {
+   reg = <0>;
+   remote-endpoint = <_out_hdmi>;
+   };
+   };
+
+   hdmi_out: port@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <1>;
+
+   hdmi_out_con: endpoint {
+   remote-endpoint = <_con_in>;
+   };
+   };
+   };
+};
+
+mixer0: mixer@110 {
+   compatible = "allwinner,sun8i-h3-de2-mixer0";
+   reg = <0x0110 0x10>;
+   clocks = <_clocks CLK_BUS_MIXER0>,
+<_clocks CLK_MIXER0>;
+   clock-names = "bus",
+ "mod";
+   resets = <_clocks RST_MIXER0>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   mixer0_out: port@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <1>;
+
+   mixer0_out_tcon0: endpoint@0 {
+   reg = <0>;
+   remote-endpoint = <_in_mixer0>;
+   };
+   };
+   };
+};
+
+tcon0: lcd-controller@1c0c000 {
+   compatible = "allwinner,sun8i-h3-tcon";
+   reg = <0x01c0c000 0x1000>;
+   interrupts = ;
+   

[RESEND RFC PATCH 3/7] clk: sunxi: Add CLK_SET_RATE_PARENT flag for H3 HDMI clock

2017-09-20 Thread Jernej Skrabec
When setting the HDMI clock of H3, the PLL_VIDEO clock needs to be set.

Add CLK_SET_RATE_PARENT flag for H3 HDMI clock.

Signed-off-by: Jernej Skrabec 
Signed-off-by: Icenowy Zheng 
---
 drivers/clk/sunxi-ng/ccu-sun8i-h3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-h3.c 
b/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
index 7a222ff1ad0a..36224ba93f9d 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
@@ -474,7 +474,7 @@ static SUNXI_CCU_GATE(avs_clk,  "avs",  
"osc24M",
 
 static const char * const hdmi_parents[] = { "pll-video" };
 static SUNXI_CCU_M_WITH_MUX_GATE(hdmi_clk, "hdmi", hdmi_parents,
-0x150, 0, 4, 24, 2, BIT(31), 0);
+0x150, 0, 4, 24, 2, BIT(31), 
CLK_SET_RATE_PARENT);
 
 static SUNXI_CCU_GATE(hdmi_ddc_clk,"hdmi-ddc", "osc24M",
  0x154, BIT(31), 0);
-- 
2.14.1



[RESEND RFC PATCH 5/7] drm: sun4i: Add a glue for the DesignWare HDMI controller in H3

2017-09-20 Thread Jernej Skrabec
Allwinner H3 features DesignWare HDMI Transmitter paired with custom
PHY.

Add a glue driver for it.

For now, only video and CEC are supported. Audio will be supported at
a later time.

Signed-off-by: Jernej Skrabec 
---
 drivers/gpu/drm/sun4i/Kconfig |   9 +
 drivers/gpu/drm/sun4i/Makefile|   1 +
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 500 ++
 3 files changed, 510 insertions(+)
 create mode 100644 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c

diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
index 06f05302ee75..589502ffe31a 100644
--- a/drivers/gpu/drm/sun4i/Kconfig
+++ b/drivers/gpu/drm/sun4i/Kconfig
@@ -40,6 +40,15 @@ config DRM_SUN4I_BACKEND
  do some alpha blending and feed graphics to TCON. If M is
  selected the module will be called sun4i-backend.
 
+config DRM_SUN8I_DW_HDMI
+   tristate "Support for Allwinner version of DesignWare HDMI"
+   depends on DRM_SUN4I
+   select DRM_DW_HDMI
+   help
+ Choose this option if you have an Allwinner SoC with the
+ DesignWare HDMI controller with custom HDMI PHY. If M is
+ selected the module will be called sun8i_dw_hdmi.
+
 config DRM_SUN8I_MIXER
tristate "Support for Allwinner Display Engine 2.0 Mixer"
default MACH_SUN8I
diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
index 43c753cafc88..9c56173bf140 100644
--- a/drivers/gpu/drm/sun4i/Makefile
+++ b/drivers/gpu/drm/sun4i/Makefile
@@ -22,3 +22,4 @@ obj-$(CONFIG_DRM_SUN4I)   += sun4i_tv.o
 obj-$(CONFIG_DRM_SUN4I_BACKEND)+= sun4i-backend.o
 obj-$(CONFIG_DRM_SUN4I_HDMI)   += sun4i-drm-hdmi.o
 obj-$(CONFIG_DRM_SUN8I_MIXER)  += sun8i-mixer.o
+obj-$(CONFIG_DRM_SUN8I_DW_HDMI)+= sun8i_dw_hdmi.o
diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c 
b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
new file mode 100644
index ..65db3e10e311
--- /dev/null
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
@@ -0,0 +1,500 @@
+/*
+ * Copyright (c) 2017, Jernej Skrabec 
+ *
+ * Based on hdmi_bsp_sun8iw7.c which is:
+ * Copyright (c) 2016 Allwinnertech Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "sun4i_crtc.h"
+#include "sun4i_tcon.h"
+
+#define SUN8I_HDMI_PHY_REG_POL 0x
+
+#define SUN8I_HDMI_PHY_REG_READ_EN 0x0010
+#define SUN8I_HDMI_PHY_REG_READ_EN_MAGIC   0x54524545
+
+#define SUN8I_HDMI_PHY_REG_UNSCRAMBLE  0x0014
+#define SUN8I_HDMI_PHY_REG_UNSCRAMBLE_MAGIC0x42494E47
+
+#define SUN8I_HDMI_PHY_REG_CTRL0x0020
+#define SUN8I_HDMI_PHY_REG_UNK10x0024
+#define SUN8I_HDMI_PHY_REG_UNK20x0028
+#define SUN8I_HDMI_PHY_REG_PLL 0x002c
+#define SUN8I_HDMI_PHY_REG_CLK 0x0030
+#define SUN8I_HDMI_PHY_REG_UNK30x0034
+
+#define SUN8I_HDMI_PHY_REG_STATUS  0x0038
+#define SUN8I_HDMI_PHY_REG_STATUS_READYBIT(7)
+#define SUN8I_HDMI_PHY_REG_STATUS_HPD  BIT(19)
+
+#define SUN8I_HDMI_PHY_REG_CEC 0x003c
+
+#define to_sun8i_dw_hdmi(x)container_of(x, struct sun8i_dw_hdmi, x)
+#define set_bits(p, v) writel(readl(p) | (v), p)
+
+struct sun8i_dw_hdmi {
+   struct clk *clk_ahb;
+   struct clk *clk_ddc;
+   struct clk *clk_sfr;
+   struct device *dev;
+   struct drm_encoder encoder;
+   void __iomem *phy_base;
+   struct dw_hdmi_plat_data plat_data;
+   struct reset_control *rst_ddc;
+   struct reset_control *rst_hdmi;
+};
+
+static u32 sun8i_dw_hdmi_get_divider(int clk_khz)
+{
+   /*
+* Due to missing documentation of HDMI PHY, we know correct
+* settings only for following four PHY dividers. Select one
+* based on pixel clock.
+*/
+   if (clk_khz <= 27000)
+   return 11;
+   else if (clk_khz <= 74250)
+   return 4;
+   else if (clk_khz <= 148500)
+   return 2;
+   else
+   return 1;
+}
+
+static void sun8i_dw_hdmi_encoder_disable(struct drm_encoder *encoder)
+{
+   struct sun4i_crtc *crtc = drm_crtc_to_sun4i_crtc(encoder->crtc);
+   struct sun4i_tcon *tcon = crtc->tcon;
+
+   DRM_DEBUG_DRIVER("Disabling HDMI Output\n");
+
+   sun4i_tcon_channel_disable(tcon, 1);
+}
+
+static void sun8i_dw_hdmi_encoder_enable(struct drm_encoder *encoder)
+{
+   struct sun4i_crtc *crtc = drm_crtc_to_sun4i_crtc(encoder->crtc);
+   struct sun4i_tcon *tcon = crtc->tcon;
+
+   DRM_DEBUG_DRIVER("Enabling HDMI Output\n");
+
+   

[RESEND RFC PATCH 2/7] drm: bridge: Enable workaround in dw_hdmi for v1.32a

2017-09-20 Thread Jernej Skrabec
Allwinner SoCs have dw hdmi controller v1.32a which exhibits same
magenta line issue as i.MX6Q and i.MX6DL. Enable workaround for it.

Allwinner never released any kind of dw hdmi or errata documentation,
so it is not clear how many iterations need to be executed. One
iteration seems to be enough.

Signed-off-by: Jernej Skrabec 
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 09cb5a3e4c71..72969240a9d4 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1631,9 +1631,10 @@ static void dw_hdmi_clear_overflow(struct dw_hdmi *hdmi)
 * then write one of the FC registers several times.
 *
 * The number of iterations matters and depends on the HDMI TX revision
-* (and possibly on the platform). So far only i.MX6Q (v1.30a) and
-* i.MX6DL (v1.31a) have been identified as needing the workaround, with
-* 4 and 1 iterations respectively.
+* (and possibly on the platform). So far i.MX6Q (v1.30a), i.MX6DL
+* (v1.31a) and multiple Allwinner SoCs (v1.32a) have been identified
+* as needing the workaround, with 4 iterations for v1.30a and 1
+* iteration for others.
 */
 
switch (hdmi->version) {
@@ -1641,6 +1642,7 @@ static void dw_hdmi_clear_overflow(struct dw_hdmi *hdmi)
count = 4;
break;
case 0x131a:
+   case 0x132a:
count = 1;
break;
default:
-- 
2.14.1



[RESEND RFC PATCH 0/7] sun8i H3 HDMI glue driver for DW HDMI

2017-09-20 Thread Jernej Skrabec
[added media mailing list due to CEC question]

This patch series adds a HDMI glue driver for Allwinner H3 SoC. For now, only
video and CEC functionality is supported. Audio needs more tweaks.

Series is based on the H3 DE2 patch series available on mailing list:
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-August/522697.html
(ignore patches marked with [NOT FOR REVIEW NOW] tag)

Patch 1 adds support for polling plug detection since custom PHY used here
doesn't support HPD interrupt.

Patch 2 enables overflow workaround for v1.32a. This HDMI controller exhibits
same issues as HDMI controller used in iMX6 SoCs.

Patch 3 adds CLK_SET_RATE_PARENT to hdmi clock.

Patch 4 adds dt bindings documentation.

Patch 5 adds actual H3 HDMI glue driver.

Patch 6 and 7 add HDMI node to DT and enable it where needed.

Allwinner used DW HDMI controller in a non standard way:
- register offsets obfuscation layer, which can fortunately be turned off
- register read lock, which has to be disabled by magic number
- custom PHY, which have to be initialized before DW HDMI controller
- non standard clocks
- no HPD interrupt

Because of that, I have two questions:
- Since HPD have to be polled, is it enough just to enable poll mode? I'm
  mainly concerned about invalidating CEC address here.
- PHY has to be initialized before DW HDMI controller to disable offset
  obfuscation and read lock among other things. This means that all clocks have
  to be enabled in glue driver. This poses a problem, since when using
  component model, dw-hdmi bridge uses drvdata for it's own private data and
  prevents glue layer to pass a pointer to unbind function, where clocks should
  be disabled. I noticed same issue in meson DW HDMI glue driver, where clocks
  are also not disabled when unbind callback is called. I noticed that when H3
  SoC is shutdown, HDMI output is still enabled and lastest image is shown on
  monitor until it is unplugged from power supply. Is there any simple solution
  to this?

Chen-Yu,
TL Lim was unable to obtain any answer from Allwinner about HDMI clocks. I think
it is safe to assume that divider in HDMI clock doesn't have any effect.

Branch based on linux-next from 1. September with integrated patches is
available here:
https://github.com/jernejsk/linux-1/tree/h3_hdmi_rfc

Some additonal info about H3 HDMI:
https://linux-sunxi.org/DWC_HDMI_Controller

Thanks to Jens Kuske, who figured out that it is actually DW HDMI controller
and mapped scrambled register offsets to original ones.

Icenowy Zheng (1):
  ARM: sun8i: h3: Add DesignWare HDMI controller node

Jernej Skrabec (6):
  drm: bridge: Enable polling hpd event in dw_hdmi
  drm: bridge: Enable workaround in dw_hdmi for v1.32a
  clk: sunxi: Add CLK_SET_RATE_PARENT flag for H3 HDMI clock
  dt-bindings: Document Allwinner DWC HDMI TX node
  drm: sun4i: Add a glue for the DesignWare HDMI controller in H3
  ARM: sun8i: h3: Enable HDMI output on H3 boards

 .../bindings/display/sunxi/sun4i-drm.txt   | 158 ++-
 arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts|  33 ++
 arch/arm/boot/dts/sun8i-h3-beelink-x2.dts  |  33 ++
 arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts   |  33 ++
 arch/arm/boot/dts/sun8i-h3-orangepi-2.dts  |  33 ++
 arch/arm/boot/dts/sun8i-h3-orangepi-lite.dts   |  33 ++
 arch/arm/boot/dts/sun8i-h3-orangepi-one.dts|  33 ++
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts |  33 ++
 arch/arm/boot/dts/sun8i-h3.dtsi|   5 +
 arch/arm/boot/dts/sunxi-h3-h5.dtsi |  36 ++
 drivers/clk/sunxi-ng/ccu-sun8i-h3.c|   2 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c  |  14 +-
 drivers/gpu/drm/sun4i/Kconfig  |   9 +
 drivers/gpu/drm/sun4i/Makefile |   1 +
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c  | 500 +
 15 files changed, 950 insertions(+), 6 deletions(-)
 create mode 100644 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c

-- 
2.14.1



[RESEND RFC PATCH 1/7] drm: bridge: Enable polling hpd event in dw_hdmi

2017-09-20 Thread Jernej Skrabec
Some custom phys don't support hpd interrupts. Add support for polling
such events.

Signed-off-by: Jernej Skrabec 
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214fa464..09cb5a3e4c71 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1954,7 +1954,11 @@ static int dw_hdmi_bridge_attach(struct drm_bridge 
*bridge)
struct drm_connector *connector = >connector;
 
connector->interlace_allowed = 1;
-   connector->polled = DRM_CONNECTOR_POLL_HPD;
+   if (hdmi->phy.ops->setup_hpd)
+   connector->polled = DRM_CONNECTOR_POLL_HPD;
+   else
+   connector->polled = DRM_CONNECTOR_POLL_CONNECT |
+   DRM_CONNECTOR_POLL_DISCONNECT;
 
drm_connector_helper_add(connector, _hdmi_connector_helper_funcs);
 
-- 
2.14.1



[RESEND RFC PATCH 6/7] ARM: sun8i: h3: Add DesignWare HDMI controller node

2017-09-20 Thread Jernej Skrabec
From: Icenowy Zheng 

The H3 SoC has a DesignWare HDMI controller with some Allwinner-specific
glue and custom PHY.

Since H3 and H5 have same HDMI controller, add related device node in
shared dtsi file.

Signed-off-by: Icenowy Zheng 
Signed-off-by: Jernej Skrabec 
---
 arch/arm/boot/dts/sun8i-h3.dtsi|  5 +
 arch/arm/boot/dts/sunxi-h3-h5.dtsi | 36 
 2 files changed, 41 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 75ad7b65a7fc..b01f5ac60059 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -197,6 +197,11 @@
#address-cells = <1>;
#size-cells = <0>;
reg = <1>;
+
+   tcon0_out_hdmi: endpoint@1 {
+   reg = <1>;
+   remote-endpoint = 
<_in_tcon0>;
+   };
};
};
};
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi 
b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index d38282b9e5d4..28f4df82300e 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -592,6 +592,42 @@
interrupts = ;
};
 
+   hdmi: hdmi@1ee {
+   compatible = "allwinner,sun8i-h3-dw-hdmi";
+   reg = <0x01ee 0x1>,
+ <0x01ef 0x1>;
+   reg-io-width = <1>;
+   interrupts = ;
+   clocks = < CLK_BUS_HDMI>, < CLK_HDMI>,
+< CLK_HDMI_DDC>;
+   clock-names = "iahb", "isfr", "ddc";
+   resets = < RST_BUS_HDMI0>, < RST_BUS_HDMI1>;
+   reset-names = "hdmi", "ddc";
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   hdmi_in: port@0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0>;
+
+   hdmi_in_tcon0: endpoint@0 {
+   reg = <0>;
+   remote-endpoint = 
<_out_hdmi>;
+   };
+   };
+
+   hdmi_out: port@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <1>;
+   };
+   };
+   };
+
rtc: rtc@01f0 {
compatible = "allwinner,sun6i-a31-rtc";
reg = <0x01f0 0x54>;
-- 
2.14.1



Re: [RFC PATCH v5 3/6] i2c: add docs to clarify DMA handling

2017-09-20 Thread Mauro Carvalho Chehab
Em Wed, 20 Sep 2017 20:59:53 +0200
Wolfram Sang  escreveu:

> Signed-off-by: Wolfram Sang 

Documentation looks OK on my eyes. So:

Reviewed-by: Mauro Carvalho Chehab 

> ---
>  Documentation/i2c/DMA-considerations | 58 
> 
>  1 file changed, 58 insertions(+)
>  create mode 100644 Documentation/i2c/DMA-considerations
> 
> diff --git a/Documentation/i2c/DMA-considerations 
> b/Documentation/i2c/DMA-considerations
> new file mode 100644
> index 00..5a63355c6a9b6f
> --- /dev/null
> +++ b/Documentation/i2c/DMA-considerations
> @@ -0,0 +1,58 @@
> +=
> +Linux I2C and DMA
> +=
> +
> +Given that I2C is a low-speed bus where largely small messages are 
> transferred,
> +it is not considered a prime user of DMA access. At this time of writing, 
> only
> +10% of I2C bus master drivers have DMA support implemented. And the vast
> +majority of transactions are so small that setting up DMA for it will likely
> +add more overhead than a plain PIO transfer.
> +
> +Therefore, it is *not* mandatory that the buffer of an I2C message is DMA 
> safe.
> +It does not seem reasonable to apply additional burdens when the feature is 
> so
> +rarely used. However, it is recommended to use a DMA-safe buffer if your
> +message size is likely applicable for DMA. Most drivers have this threshold
> +around 8 bytes (as of today, this is mostly an educated guess, however). For
> +any message of 16 byte or larger, it is probably a really good idea. Please
> +note that other subsystems you use might add requirements. E.g., if your
> +I2C bus master driver is using USB as a bridge, then you need to have DMA
> +safe buffers always, because USB requires it.
> +
> +For clients, if you use a DMA safe buffer in i2c_msg, set the I2C_M_DMA_SAFE
> +flag with it. Then, the I2C core and drivers know they can safely operate DMA
> +on it. Note that using this flag is optional. I2C host drivers which are not
> +updated to use this flag will work like before. And like before, they risk
> +using an unsafe DMA buffer. To improve this situation, using I2C_M_DMA_SAFE 
> in
> +more and more clients and host drivers is the planned way forward. Note also
> +that setting this flag makes only sense in kernel space. User space data is
> +copied into kernel space anyhow. The I2C core makes sure the destination
> +buffers in kernel space are always DMA capable.
> +
> +FIXME: Need to implement i2c_master_{send|receive}_dma and proper buffers 
> for i2c_smbus_xfer_emulated.
> +
> +Drivers wishing to implement safe DMA can use helper functions from the I2C
> +core. One gives you a DMA-safe buffer for a given i2c_msg as long as a 
> certain
> +threshold is met::
> +
> + dma_buf = i2c_get_dma_safe_msg_buf(msg, threshold_in_byte);
> +
> +If a buffer is returned, it is either msg->buf for the I2C_M_DMA_SAFE case 
> or a
> +bounce buffer. But you don't need to care about that detail, just use the
> +returned buffer. If NULL is returned, the threshold was not met or a bounce
> +buffer could not be allocated. Fall back to PIO in that case.
> +
> +In any case, a buffer obtained from above needs to be released. It ensures 
> data
> +is copied back to the message and a potentially used bounce buffer is freed::
> +
> + i2c_release_dma_safe_msg_buf(msg, dma_buf);
> +
> +The bounce buffer handling from the core is generic and simple. It will 
> always
> +allocate a new bounce buffer. If you want a more sophisticated handling (e.g.
> +reusing pre-allocated buffers), you are free to implement your own.
> +
> +Please also check the in-kernel documentation for details. The i2c-sh_mobile
> +driver can be used as a reference example how to use the above helpers.
> +
> +Final note: If you plan to use DMA with I2C (or with anything else, actually)
> +make sure you have CONFIG_DMA_API_DEBUG enabled during development. It can 
> help
> +you find various issues which can be complex to debug otherwise.



Thanks,
Mauro


Re: usb/media/pvrusb2: warning in pvr2_send_request_ex/usb_submit_urb

2017-09-20 Thread Mike Isely

What you have here is way beyond just feeding random crap in via the 
syscall interface.  To cause this you have to fake the presence of a 
pvrusb2 compatible *hardware* USB device and then lie about its endpoint 
configuration.  Is that really a concern here?  Are we now saying that 
any kernel driver which talks via USB must now also specifically verify 
the exact expected USB endpoint configuration?  Where does that end?  
How about the vendor-specific RPC protocol that the hardware actually 
implements over the bulk endpoint?  It's likely that the pvrusb2 driver 
may be making assumptions about the expected responses over that 
protocol.

Please realize that I'm not dismissing this.  I can see some merit in 
this.  But I'm just a bit surprised that now we're going this far.  Is 
this really the intention?  You're talking about code 
(pvrusb2_send_request_ex()) that hasn't changed in about 10 years.  
With this level of paranoia there's got to be a pretty target-rich 
environment over the set of kernel-supported USB devices.

To take this another step, wouldn't that same level of paranoia be a 
concern for any externally connected PCI-Express device?  Because that's 
another external way into the computer that involves very non-trivial 
and very hardware-centric protocols.  Thunderbolt devices would be an 
example of this.

  -Mike


On Wed, 20 Sep 2017, Andrey Konovalov wrote:

> Hi!
> 
> I've got the following report while fuzzing the kernel with syzkaller.
> 
> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).
> 
> There seems to be no check on endpoint type before submitting bulk urb
> in pvr2_send_request_ex().
> 
> usb 1-1: New USB device found, idVendor=2040, idProduct=7500
> usb 1-1: New USB device strings: Mfr=0, Product=255, SerialNumber=0
> usb 1-1: Product: a
> gadgetfs: configuration #6
> pvrusb2: Hardware description: WinTV HVR-1950 Model 750xx
> usb 1-1: BOGUS urb xfer, pipe 3 != type 1
> [ cut here ]
> WARNING: CPU: 1 PID: 2713 at drivers/usb/core/urb.c:449
> usb_submit_urb+0xf8a/0x11d0
> Modules linked in:
> CPU: 1 PID: 2713 Comm: pvrusb2-context Not tainted
> 4.14.0-rc1-42251-gebb2c2437d80 #210
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> task: 88006b7a18c0 task.stack: 880069978000
> RIP: 0010:usb_submit_urb+0xf8a/0x11d0 drivers/usb/core/urb.c:448
> RSP: 0018:88006997f990 EFLAGS: 00010286
> RAX: 0029 RBX: 880063661900 RCX: 
> RDX: 0029 RSI: 86876d60 RDI: ed000d32ff24
> RBP: 88006997fa90 R08: 11000d32fdca R09: 
> R10:  R11:  R12: 11000d32ff39
> R13: 0001 R14: 0003 R15: 880068bbed68
> FS:  () GS:88006c60() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 01032000 CR3: 6a0ff000 CR4: 06f0
> Call Trace:
>  pvr2_send_request_ex+0xa57/0x1d80 
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3645
>  pvr2_hdw_check_firmware drivers/media/usb/pvrusb2/pvrusb2-hdw.c:1812
>  pvr2_hdw_setup_low drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2107
>  pvr2_hdw_setup drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2250
>  pvr2_hdw_initialize+0x548/0x3c10 drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2327
>  pvr2_context_check drivers/media/usb/pvrusb2/pvrusb2-context.c:118
>  pvr2_context_thread_func+0x361/0x8c0
> drivers/media/usb/pvrusb2/pvrusb2-context.c:167
>  kthread+0x3a1/0x470 kernel/kthread.c:231
>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
> Code: 48 8b 85 30 ff ff ff 48 8d b8 98 00 00 00 e8 ee 82 89 fe 45 89
> e8 44 89 f1 4c 89 fa 48 89 c6 48 c7 c7 40 c0 ea 86 e8 30 1b dc fc <0f>
> ff e9 9b f7 ff ff e8 aa 95 25 fd e9 80 f7 ff ff e8 50 74 f3
> ---[ end trace 6919030503719da6 ]---
> 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8


Re: [PATCH] [media] staging: atomisp: use clock framework for camera clocks

2017-09-20 Thread Andy Shevchenko
On Wed, 2017-09-20 at 12:01 -0500, Pierre-Louis Bossart wrote:
> 
> On 09/20/2017 04:12 AM, Andy Shevchenko wrote:
> > On Tue, 2017-09-19 at 15:45 -0500, Pierre-Louis Bossart wrote:
> > > The Atom ISP driver initializes and configures PMC clocks which
> > > are
> > > already handled by the clock framework.
> > > 
> > > Remove all legacy vlv2_platform_clock stuff and move to the clk
> > > API to
> > > avoid conflicts, e.g. with audio machine drivers enabling the MCLK
> > > for
> > > external codecs
> > > 
> > 
> > I think it might have a Fixes: tag as well (though I dunno which
> > commit
> > could be considered as anchor).
> 
> The initial integration of the atomisp driver already had this
> problem, 
> i'll add a reference to
> 'a49d25364dfb9 ("staging/atomisp: Add support for the Intel IPU v2")'

...which seems to be the best choice (you can check how many new commits
use that one as an origin for Fixes: tag).

> > 
> > (I doubt Git is so clever to remove files based on information out
> > of
> > the diff, can you check it and if needed to resend without -D
> > implied?)
> 
> Gee, I thought -C -M -D were the standard options to checkpatch,
> never 
> realized it'd prevent patches from applying. Thanks for the tip.

-C -M — yes for sure.

Last time I checked patches, generated with help of -D, do not remove
the files when you do git am. So, I don't know if it still the case.
Safe option is to use -C -M for public (+ -D locally only to see less
noise).

-- 
Andy Shevchenko 
Intel Finland Oy


usb/media/pvrusb2: warning in pvr2_send_request_ex/usb_submit_urb

2017-09-20 Thread Andrey Konovalov
Hi!

I've got the following report while fuzzing the kernel with syzkaller.

On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).

There seems to be no check on endpoint type before submitting bulk urb
in pvr2_send_request_ex().

usb 1-1: New USB device found, idVendor=2040, idProduct=7500
usb 1-1: New USB device strings: Mfr=0, Product=255, SerialNumber=0
usb 1-1: Product: a
gadgetfs: configuration #6
pvrusb2: Hardware description: WinTV HVR-1950 Model 750xx
usb 1-1: BOGUS urb xfer, pipe 3 != type 1
[ cut here ]
WARNING: CPU: 1 PID: 2713 at drivers/usb/core/urb.c:449
usb_submit_urb+0xf8a/0x11d0
Modules linked in:
CPU: 1 PID: 2713 Comm: pvrusb2-context Not tainted
4.14.0-rc1-42251-gebb2c2437d80 #210
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: 88006b7a18c0 task.stack: 880069978000
RIP: 0010:usb_submit_urb+0xf8a/0x11d0 drivers/usb/core/urb.c:448
RSP: 0018:88006997f990 EFLAGS: 00010286
RAX: 0029 RBX: 880063661900 RCX: 
RDX: 0029 RSI: 86876d60 RDI: ed000d32ff24
RBP: 88006997fa90 R08: 11000d32fdca R09: 
R10:  R11:  R12: 11000d32ff39
R13: 0001 R14: 0003 R15: 880068bbed68
FS:  () GS:88006c60() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 01032000 CR3: 6a0ff000 CR4: 06f0
Call Trace:
 pvr2_send_request_ex+0xa57/0x1d80 drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3645
 pvr2_hdw_check_firmware drivers/media/usb/pvrusb2/pvrusb2-hdw.c:1812
 pvr2_hdw_setup_low drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2107
 pvr2_hdw_setup drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2250
 pvr2_hdw_initialize+0x548/0x3c10 drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2327
 pvr2_context_check drivers/media/usb/pvrusb2/pvrusb2-context.c:118
 pvr2_context_thread_func+0x361/0x8c0
drivers/media/usb/pvrusb2/pvrusb2-context.c:167
 kthread+0x3a1/0x470 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
Code: 48 8b 85 30 ff ff ff 48 8d b8 98 00 00 00 e8 ee 82 89 fe 45 89
e8 44 89 f1 4c 89 fa 48 89 c6 48 c7 c7 40 c0 ea 86 e8 30 1b dc fc <0f>
ff e9 9b f7 ff ff e8 aa 95 25 fd e9 80 f7 ff ff e8 50 74 f3
---[ end trace 6919030503719da6 ]---


[PATCH 3/3] [media] dvb-ttusb-budget: Adjust eight checks for null pointers

2017-09-20 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 20 Sep 2017 20:53:13 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written …

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c 
b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
index fef3c8554e91..2e97b1e64249 100644
--- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
@@ -1572,7 +1572,7 @@ static void frontend_init(struct ttusb* ttusb)
case 0x1003: // Hauppauge/TT Nova-USB-S budget (stv0299/ALPS 
BSRU6|BSBE1(tsa5059))
// try the stv0299 based first
ttusb->fe = dvb_attach(stv0299_attach, _stv0299_config, 
>i2c_adap);
-   if (ttusb->fe != NULL) {
+   if (ttusb->fe) {
ttusb->fe->ops.tuner_ops.set_params = 
philips_tsa5059_tuner_set_params;
 
if(ttusb->revision == TTUSB_REV_2_2) { // ALPS BSBE1
@@ -1586,7 +1586,7 @@ static void frontend_init(struct ttusb* ttusb)
 
// Grundig 29504-491
ttusb->fe = dvb_attach(tda8083_attach, 
_novas_grundig_29504_491_config, >i2c_adap);
-   if (ttusb->fe != NULL) {
+   if (ttusb->fe) {
ttusb->fe->ops.tuner_ops.set_params = 
ttusb_novas_grundig_29504_491_tuner_set_params;
ttusb->fe->ops.set_voltage = ttusb_set_voltage;
break;
@@ -1595,13 +1595,13 @@ static void frontend_init(struct ttusb* ttusb)
 
case 0x1004: // Hauppauge/TT DVB-C budget (ves1820/ALPS TDBE2(sp5659))
ttusb->fe = dvb_attach(ves1820_attach, _tdbe2_config, 
>i2c_adap, read_pwm(ttusb));
-   if (ttusb->fe != NULL) {
+   if (ttusb->fe) {
ttusb->fe->ops.tuner_ops.set_params = 
alps_tdbe2_tuner_set_params;
break;
}
 
ttusb->fe = dvb_attach(stv0297_attach, 
_philips_tdm1316l_config, >i2c_adap);
-   if (ttusb->fe != NULL) {
+   if (ttusb->fe) {
ttusb->fe->ops.tuner_ops.set_params = 
dvbc_philips_tdm1316l_tuner_set_params;
break;
}
@@ -1610,14 +1610,14 @@ static void frontend_init(struct ttusb* ttusb)
case 0x1005: // Hauppauge/TT Nova-USB-t budget (tda10046/Philips 
td1316(tda6651tt) OR cx22700/ALPS TDMB7(??))
// try the ALPS TDMB7 first
ttusb->fe = dvb_attach(cx22700_attach, _tdmb7_config, 
>i2c_adap);
-   if (ttusb->fe != NULL) {
+   if (ttusb->fe) {
ttusb->fe->ops.tuner_ops.set_params = 
alps_tdmb7_tuner_set_params;
break;
}
 
// Philips td1316
ttusb->fe = dvb_attach(tda10046_attach, 
_tdm1316l_config, >i2c_adap);
-   if (ttusb->fe != NULL) {
+   if (ttusb->fe) {
ttusb->fe->ops.tuner_ops.init = 
philips_tdm1316l_tuner_init;
ttusb->fe->ops.tuner_ops.set_params = 
philips_tdm1316l_tuner_set_params;
break;
@@ -1625,7 +1625,7 @@ static void frontend_init(struct ttusb* ttusb)
break;
}
 
-   if (ttusb->fe == NULL) {
+   if (!ttusb->fe) {
printk("dvb-ttusb-budget: A frontend driver was not found for 
device [%04x:%04x]\n",
   le16_to_cpu(ttusb->dev->descriptor.idVendor),
   le16_to_cpu(ttusb->dev->descriptor.idProduct));
@@ -1781,7 +1781,7 @@ static void ttusb_disconnect(struct usb_interface *intf)
dvb_net_release(>dvbnet);
dvb_dmxdev_release(>dmxdev);
dvb_dmx_release(>dvb_demux);
-   if (ttusb->fe != NULL) {
+   if (ttusb->fe) {
dvb_unregister_frontend(ttusb->fe);
dvb_frontend_detach(ttusb->fe);
}
-- 
2.14.1



[PATCH 2/3] [media] dvb-ttusb-budget: Improve two size determinations in ttusb_probe()

2017-09-20 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 20 Sep 2017 20:46:11 +0200

* The script "checkpatch.pl" pointed information out like the following.

  ERROR: do not use assignment in if condition

  Thus fix an affected source code place.

* Replace the specification of data structures by variable references
  as the parameter for the operator "sizeof" to make the corresponding size
  determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c 
b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
index 38394c9ecc67..fef3c8554e91 100644
--- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
@@ -1657,7 +1657,8 @@ static int ttusb_probe(struct usb_interface *intf, const 
struct usb_device_id *i
 
if (intf->altsetting->desc.bInterfaceNumber != 1) return -ENODEV;
 
-   if (!(ttusb = kzalloc(sizeof(struct ttusb), GFP_KERNEL)))
+   ttusb = kzalloc(sizeof(*ttusb), GFP_KERNEL);
+   if (!ttusb)
return -ENOMEM;
 
ttusb->dev = udev;
@@ -1692,7 +1693,7 @@ static int ttusb_probe(struct usb_interface *intf, const 
struct usb_device_id *i
ttusb->adapter.priv = ttusb;
 
/* i2c */
-   memset(>i2c_adap, 0, sizeof(struct i2c_adapter));
+   memset(>i2c_adap, 0, sizeof(ttusb->i2c_adap));
strcpy(ttusb->i2c_adap.name, "TTUSB DEC");
 
i2c_set_adapdata(>i2c_adap, ttusb);
-- 
2.14.1



[PATCH 15/25] media: dvb_demux: fix type of dvb_demux_feed.ts_type

2017-09-20 Thread Mauro Carvalho Chehab
Just like pes_type, this field represents an enum. Properly
identify it as such.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_demux.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_demux.h 
b/drivers/media/dvb-core/dvb_demux.h
index d9b30d669bf3..c9e94bc3a2e5 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -95,7 +95,7 @@ struct dvb_demux_feed {
ktime_t timeout;
struct dvb_demux_filter *filter;
 
-   int ts_type;
+   enum ts_filter_type ts_type;
enum dmx_ts_pes pes_type;
 
int cc;
-- 
2.13.5



[PATCH 1/3] [media] dvb-ttusb-budget: Use common error handling code in ttusb_probe()

2017-09-20 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 20 Sep 2017 20:25:24 +0200

Add two jump targets so that a bit of exception handling can be better
reused at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c 
b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
index b842f367249f..38394c9ecc67 100644
--- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
@@ -1675,8 +1675,7 @@ static int ttusb_probe(struct usb_interface *intf, const 
struct usb_device_id *i
if (result < 0) {
dprintk("%s: ttusb_alloc_iso_urbs - failed\n", __func__);
mutex_unlock(>semi2c);
-   kfree(ttusb);
-   return result;
+   goto err_free_usb;
}
 
if (ttusb_init_controller(ttusb))
@@ -1687,11 +1686,9 @@ static int ttusb_probe(struct usb_interface *intf, const 
struct usb_device_id *i
result = dvb_register_adapter(>adapter,
  "Technotrend/Hauppauge Nova-USB",
  THIS_MODULE, >dev, adapter_nr);
-   if (result < 0) {
-   ttusb_free_iso_urbs(ttusb);
-   kfree(ttusb);
-   return result;
-   }
+   if (result < 0)
+   goto err_free_iso_urbs;
+
ttusb->adapter.priv = ttusb;
 
/* i2c */
@@ -1762,7 +1759,9 @@ static int ttusb_probe(struct usb_interface *intf, const 
struct usb_device_id *i
i2c_del_adapter(>i2c_adap);
 err_unregister_adapter:
dvb_unregister_adapter (>adapter);
+err_free_iso_urbs:
ttusb_free_iso_urbs(ttusb);
+err_free_usb:
kfree(ttusb);
return result;
 }
-- 
2.14.1



[PATCH 21/25] scripts: kernel-doc: fix nexted handling

2017-09-20 Thread Mauro Carvalho Chehab
At DVB, we have, at some structs, things like (see
struct dvb_demux_feed, at dvb_demux.h):

union {
struct dmx_ts_feed ts;
struct dmx_section_feed sec;
} feed;

union {
dmx_ts_cb ts;
dmx_section_cb sec;
} cb;

Fix the nested parser to avoid it to eat the first union.

Signed-off-by: Mauro Carvalho Chehab 
---
 scripts/kernel-doc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 9d3eafea58f0..15f934a23d1d 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -2173,7 +2173,7 @@ sub dump_struct($$) {
my $members = $3;
 
# ignore embedded structs or unions
-   $members =~ s/({.*})//g;
+   $members =~ s/({[^\}]*})//g;
$nested = $1;
 
# ignore members marked private:
-- 
2.13.5



[PATCH 09/25] media: dvb_demux.h: add an enum for DMX_TYPE_* and document

2017-09-20 Thread Mauro Carvalho Chehab
kernel-doc allows documenting enums. Also, it makes clearer
about the meaning of each field on structures.

So, convert DMX_TYPE_* to an enum.

While here, get rid of the unused DMX_TYPE_PES.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_demux.h | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_demux.h 
b/drivers/media/dvb-core/dvb_demux.h
index 6f572ca8d339..6bc4b27dbff3 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -26,9 +26,16 @@
 
 #include "demux.h"
 
-#define DMX_TYPE_TS  0
-#define DMX_TYPE_SEC 1
-#define DMX_TYPE_PES 2
+/**
+ * enum dvb_dmx_filter_type - type of demux feed.
+ *
+ * @DMX_TYPE_TS:   feed is in TS mode.
+ * @DMX_TYPE_SEC:  feed is in Section mode.
+ */
+enum dvb_dmx_filter_type {
+   DMX_TYPE_TS,
+   DMX_TYPE_SEC,
+};
 
 #define DMX_STATE_FREE  0
 #define DMX_STATE_ALLOCATED 1
@@ -52,7 +59,7 @@ struct dvb_demux_filter {
struct dvb_demux_feed *feed;
int index;
int state;
-   int type;
+   enum dvb_dmx_filter_type type;
 
u16 hw_handle;
struct timer_list timer;
@@ -73,7 +80,7 @@ struct dvb_demux_feed {
 
struct dvb_demux *demux;
void *priv;
-   int type;
+   enum dvb_dmx_filter_type type;
int state;
u16 pid;
 
-- 
2.13.5



[PATCH 18/25] media: dvb_frontend: get rid of dtv_get_property_dump()

2017-09-20 Thread Mauro Carvalho Chehab
Simplify the get property handling and move it to the existing
code at dtv_property_process_get() directly.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_frontend.c | 43 ++-
 1 file changed, 12 insertions(+), 31 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index b7094c7a405f..607eaf3db052 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1107,36 +1107,6 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] = 
{
_DTV_CMD(DTV_STAT_TOTAL_BLOCK_COUNT, 0, 0),
 };
 
-static void dtv_get_property_dump(struct dvb_frontend *fe,
- struct dtv_property *tvp)
-{
-   int i;
-
-   if (tvp->cmd <= 0 || tvp->cmd > DTV_MAX_COMMAND) {
-   dev_warn(fe->dvb->device, "%s: GET tvp.cmd = 0x%08x undefined\n"
-   , __func__,
-   tvp->cmd);
-   return;
-   }
-
-   dev_dbg(fe->dvb->device, "%s: GET tvp.cmd= 0x%08x (%s)\n", __func__,
-   tvp->cmd,
-   dtv_cmds[tvp->cmd].name);
-
-   if (dtv_cmds[tvp->cmd].buffer) {
-   dev_dbg(fe->dvb->device, "%s: tvp.u.buffer.len = 0x%02x\n",
-   __func__, tvp->u.buffer.len);
-
-   for(i = 0; i < tvp->u.buffer.len; i++)
-   dev_dbg(fe->dvb->device,
-   "%s: tvp.u.buffer.data[0x%02x] = 
0x%02x\n",
-   __func__, i, tvp->u.buffer.data[i]);
-   } else {
-   dev_dbg(fe->dvb->device, "%s: tvp.u.data = 0x%08x\n", __func__,
-   tvp->u.data);
-   }
-}
-
 /* Synchronise the legacy tuning parameters into the cache, so that demodulator
  * drivers can use a single set_frontend tuning function, regardless of whether
  * it's being used for the legacy or new API, reducing code and complexity.
@@ -1529,7 +1499,18 @@ static int dtv_property_process_get(struct dvb_frontend 
*fe,
return -EINVAL;
}
 
-   dtv_get_property_dump(fe, tvp);
+   if (!dtv_cmds[tvp->cmd].buffer)
+   dev_dbg(fe->dvb->device,
+   "%s: GET cmd 0x%08x (%s) = 0x%08x\n",
+   __func__, tvp->cmd, dtv_cmds[tvp->cmd].name,
+   tvp->u.data);
+   else
+   dev_dbg(fe->dvb->device,
+   "%s: GET cmd 0x%08x (%s) len %d: %*ph\n",
+   __func__,
+   tvp->cmd, dtv_cmds[tvp->cmd].name,
+   tvp->u.buffer.len,
+   tvp->u.buffer.len, tvp->u.buffer.data);
 
return 0;
 }
-- 
2.13.5



[PATCH 19/25] media: dvb_demux.h: document structs defined on it

2017-09-20 Thread Mauro Carvalho Chehab
There are three structs defined inside dvb_demux.h. None
of them are currently documented.

Add documentation for them.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_demux.c |  1 +
 drivers/media/dvb-core/dvb_demux.h | 58 --
 2 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_demux.c 
b/drivers/media/dvb-core/dvb_demux.c
index b9360cbc3519..acade7543b82 100644
--- a/drivers/media/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb-core/dvb_demux.c
@@ -367,6 +367,7 @@ static inline void dvb_dmx_swfilter_packet_type(struct 
dvb_demux_feed *feed,
else
feed->cb.ts(buf, 188, NULL, 0, >feed.ts);
}
+   /* Used only on full-featured devices */
if (feed->ts_type & TS_DECODER)
if (feed->demux->write_to_decoder)
feed->demux->write_to_decoder(feed, buf, 188);
diff --git a/drivers/media/dvb-core/dvb_demux.h 
b/drivers/media/dvb-core/dvb_demux.h
index 5b05e6320e33..43b0cab2e932 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -95,15 +95,16 @@ struct dvb_demux_filter {
  * struct dvb_demux_feed - describes a DVB field
  *
  * @feed:  a digital TV feed. It can either be a TS or a section feed:
- *   - if the feed is TS, it contains  dvb_ts_feed;
- *   - if the feed is section, it contains
- *  dmx_section_feed.
+ * if the feed is TS, it contains  dvb_ts_feed @ts;
+ * if the feed is section, it contains
+ *  dmx_section_feed @sec.
  * @cb:digital TV callbacks. depending on the feed type, it 
can be:
- *   - if the feed is TS, it contains a dmx_ts_cb() callback;
- *   - if the feed is section, it contains a dmx_section_cb() 
callback.
+ * if the feed is TS, it contains a dmx_ts_cb() @ts callback;
+ * if the feed is section, it contains a dmx_section_cb() @sec
+ * callback.
  *
  * @demux: pointer to  dvb_demux.
- * @priv:  private data for the filter handling routine.
+ * @priv:  private data that can optionally be used by a DVB driver.
  * @type:  type of the filter, as defined by  dvb_dmx_filter_type.
  * @state: state of the filter as defined by  dvb_dmx_state.
  * @pid:   PID to be filtered.
@@ -118,7 +119,6 @@ struct dvb_demux_filter {
  * @list_head: head for the list of digital TV demux feeds.
  * @index: a unique index for each feed. Can be used as hardware
  * pid filter index.
- *
  */
 struct dvb_demux_feed {
union {
@@ -152,6 +152,44 @@ struct dvb_demux_feed {
unsigned int index;
 };
 
+/**
+ * struct dvb_demux - represents a digital TV demux
+ * @dmx:   embedded  dmx_demux with demux capabilities
+ * and callbacks.
+ * @priv:  private data that can optionally be used by
+ * a DVB driver.
+ * @filternum: maximum amount of DVB filters.
+ * @feednum:   maximum amount of DVB feeds.
+ * @start_feed:callback routine to be called in order to start
+ * a DVB feed.
+ * @stop_feed: callback routine to be called in order to stop
+ * a DVB feed.
+ * @write_to_decoder:  callback routine to be called if the feed is TS and
+ * it is routed to an A/V decoder, when a new TS packet
+ * is received.
+ * Used only on av7110-av.c.
+ * @check_crc32:   callback routine to check CRC. If not initialized,
+ * dvb_demux will use an internal one.
+ * @memcopy:   callback routine to memcopy received data.
+ * If not initialized, dvb_demux will default to memcpy().
+ * @users: counter for the number of demux opened file descriptors.
+ * Currently, it is limited to 10 users.
+ * @filter:pointer to  dvb_demux_filter.
+ * @feed:  pointer to  dvb_demux_feed.
+ * @frontend_list:  list_head with frontends used by the demux.
+ * @pesfilter: array of  dvb_demux_feed with the PES types
+ * that will be filtered.
+ * @pids:  list of filtered program IDs.
+ * @feed_list:  list_head with feeds.
+ * @tsbuf: temporary buffer used internally to store TS packets.
+ * @tsbufp:temporary buffer index used internally.
+ * @mutex: pointer to  mutex used to protect feed set
+ * logic.
+ * @lock:  pointer to _t, used to protect buffer handling.
+ * @cnt_storage:   buffer used for TS/TEI continuity check.
+ * @speed_last_time:   _t used for TS speed check.
+ * @speed_pkts_cnt:packets count used for TS speed 

[PATCH 12/25] media: dvb_demux: mark a boolean field as such

2017-09-20 Thread Mauro Carvalho Chehab
The struct dvb_demux_filter.doneq is a boolean.

Mark it as such, as it helps to understand what it does.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_demux.c | 4 ++--
 drivers/media/dvb-core/dvb_demux.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_demux.c 
b/drivers/media/dvb-core/dvb_demux.c
index 6628f80d184f..68e93362c081 100644
--- a/drivers/media/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb-core/dvb_demux.c
@@ -898,14 +898,14 @@ static void prepare_secfilters(struct dvb_demux_feed 
*dvbdmxfeed)
return;
do {
sf = >filter;
-   doneq = 0;
+   doneq = false;
for (i = 0; i < DVB_DEMUX_MASK_MAX; i++) {
mode = sf->filter_mode[i];
mask = sf->filter_mask[i];
f->maskandmode[i] = mask & mode;
doneq |= f->maskandnotmode[i] = mask & ~mode;
}
-   f->doneq = doneq ? 1 : 0;
+   f->doneq = doneq ? true : false;
} while ((f = f->next));
 }
 
diff --git a/drivers/media/dvb-core/dvb_demux.h 
b/drivers/media/dvb-core/dvb_demux.h
index 045f7fd1a8b1..700887938145 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -64,7 +64,7 @@ struct dvb_demux_filter {
struct dmx_section_filter filter;
u8 maskandmode[DMX_MAX_FILTER_SIZE];
u8 maskandnotmode[DMX_MAX_FILTER_SIZE];
-   int doneq;
+   bool doneq;
 
struct dvb_demux_filter *next;
struct dvb_demux_feed *feed;
-- 
2.13.5



[PATCH 00/25] DVB cleanups and documentation improvements

2017-09-20 Thread Mauro Carvalho Chehab
This patch series comes after a previous patchset with DVB fixes.
both series are there at:

   https://git.linuxtv.org/mchehab/experimental.git/log/?h=dvb-fixes-v3

It is mainly focused on improving the DVB kAPI documentation, making
it (finally!) in sync with the current implementation. It also contains
a patch getting rid of the legacy (non-working) uAPI examples.

While reviewing the code implementation, I noticed some struct fields
that aren't used at all by any DVB driver or core. So, the series gets
rid of them. Others are used only on av7110, and are documented as
such.

After this patch series, both DVB uAPI and kAPI are fully documented
(except for the legacy video/audio/osd uAPI, that doesn't have any
kAPI associated to them).

Granted, some things could be improved at the documentation, but at
least it doesn't carry anymore any big gap or conflict!

Please review and test.

PS.: there is one patch in this series that really belongs to kernel-doc
tree. I sent it already in separate, but, as without it several kernel-doc
markups are ignored, I'm adding it here for consistency.

Mauro Carvalho Chehab (24):
  media: dvb_frontend: better document the -EPERM condition
  media: dvb_frontend: fix return values for FE_SET_PROPERTY
  media: dvbdev: convert DVB device types into an enum
  media: dvbdev: fully document its functions
  media: dvb_frontend.h: improve kernel-doc markups
  media: dtv-core.rst: add chapters and introductory tests for common
parts
  media: dtv-core.rst: split into multiple files
  media: dtv-demux.rst: minor markup improvements
  media: dvb_demux.h: add an enum for DMX_TYPE_* and document
  media: dvb_demux.h: add an enum for DMX_STATE_* and document
  media: dvb_demux.h: get rid of unused timer at struct dvb_demux_filter
  media: dvb_demux: mark a boolean field as such
  media: dvb_demux: dvb_demux_feed.pusi_seen is boolean
  media: dvb_demux.h: get rid of DMX_FEED_ENTRY() macro
  media: dvb_demux: fix type of dvb_demux_feed.ts_type
  media: dvb_demux: document dvb_demux_filter and dvb_demux_feed
  media: dvb_frontend: get rid of dtv_get_property_dump()
  media: dvb_demux.h: document structs defined on it
  media: dvb_demux.h: document functions
  scripts: kernel-doc: fix nexted handling
  media: dmxdev.h: add kernel-doc markups for data types and functions
  media: dtv-demux.rst: parse other demux headers with kernel-doc
  media: dvb-net.rst: document DVB network kAPI interface
  media: dvb uAPI docs: get rid of examples section

Satendra Singh Thakur (1):
  media: dvb_frontend: dtv_property_process_set() cleanups

 Documentation/media/kapi/dtv-ca.rst  |   4 +
 Documentation/media/kapi/dtv-common.rst  |  55 +++
 Documentation/media/kapi/dtv-core.rst| 574 +--
 Documentation/media/kapi/dtv-demux.rst   |  82 
 Documentation/media/kapi/dtv-frontend.rst| 443 +
 Documentation/media/kapi/dtv-net.rst |   4 +
 Documentation/media/uapi/dvb/examples.rst| 378 +--
 Documentation/media/uapi/dvb/fe-get-property.rst |   7 +-
 drivers/media/dvb-core/dmxdev.h  |  90 +++-
 drivers/media/dvb-core/dvb_demux.c   |  17 +-
 drivers/media/dvb-core/dvb_demux.h   | 248 +-
 drivers/media/dvb-core/dvb_frontend.c| 180 +++
 drivers/media/dvb-core/dvb_frontend.h|  94 ++--
 drivers/media/dvb-core/dvb_net.h |  34 +-
 drivers/media/dvb-core/dvbdev.c  |  34 +-
 drivers/media/dvb-core/dvbdev.h  | 137 +-
 drivers/media/pci/ttpci/av7110.c |   2 +-
 drivers/media/pci/ttpci/budget-core.c|   2 +-
 include/uapi/linux/dvb/frontend.h|   2 +-
 scripts/kernel-doc   |   2 +-
 20 files changed, 1248 insertions(+), 1141 deletions(-)
 create mode 100644 Documentation/media/kapi/dtv-ca.rst
 create mode 100644 Documentation/media/kapi/dtv-common.rst
 create mode 100644 Documentation/media/kapi/dtv-demux.rst
 create mode 100644 Documentation/media/kapi/dtv-frontend.rst
 create mode 100644 Documentation/media/kapi/dtv-net.rst

-- 
2.13.5




[PATCH 11/25] media: dvb_demux.h: get rid of unused timer at struct dvb_demux_filter

2017-09-20 Thread Mauro Carvalho Chehab
This field is not used. So, get rid of it.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_demux.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_demux.h 
b/drivers/media/dvb-core/dvb_demux.h
index b24d69b5a20f..045f7fd1a8b1 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -73,7 +73,6 @@ struct dvb_demux_filter {
enum dvb_dmx_filter_type type;
 
u16 hw_handle;
-   struct timer_list timer;
 };
 
 #define DMX_FEED_ENTRY(pos) list_entry(pos, struct dvb_demux_feed, list_head)
-- 
2.13.5



[PATCH 06/25] media: dtv-core.rst: add chapters and introductory tests for common parts

2017-09-20 Thread Mauro Carvalho Chehab
Better document the DVB common parts by adding two sections
and an introductory text for each.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/kapi/dtv-core.rst | 12 
 1 file changed, 12 insertions(+)

diff --git a/Documentation/media/kapi/dtv-core.rst 
b/Documentation/media/kapi/dtv-core.rst
index de9a228aca8a..4cf9cf63bafd 100644
--- a/Documentation/media/kapi/dtv-core.rst
+++ b/Documentation/media/kapi/dtv-core.rst
@@ -29,8 +29,20 @@ I2C bus.
 Digital TV Common functions
 ---
 
+Math functions
+~~
+
+Provide some commonly-used math functions, usually required in order to
+estimate signal strength and signal to noise measurements in dB.
+
 .. kernel-doc:: drivers/media/dvb-core/dvb_math.h
 
+
+DVB devices
+~~~
+
+Those functions are responsible for handling the DVB device nodes.
+
 .. kernel-doc:: drivers/media/dvb-core/dvbdev.h
 
 Digital TV Ring buffer
-- 
2.13.5



[PATCH 23/25] media: dtv-demux.rst: parse other demux headers with kernel-doc

2017-09-20 Thread Mauro Carvalho Chehab
Now that we have kernel-doc markups at dvb_demux.h and dmxdev.h,
parse them.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/kapi/dtv-demux.rst | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/kapi/dtv-demux.rst 
b/Documentation/media/kapi/dtv-demux.rst
index 3ee69266e206..7aa865a2b43f 100644
--- a/Documentation/media/kapi/dtv-demux.rst
+++ b/Documentation/media/kapi/dtv-demux.rst
@@ -66,7 +66,17 @@ function call directly from a hardware interrupt.
 This mechanism is implemented by :c:func:`dmx_ts_cb()` and 
:c:func:`dmx_section_cb()`
 callbacks.
 
-Digital TV Demux functions and types
-
+Digital TV Demux device registration functions and data structures
+~~
+
+.. kernel-doc:: drivers/media/dvb-core/dmxdev.h
+
+High-level Digital TV demux interface
+~
+
+.. kernel-doc:: drivers/media/dvb-core/dvb_demux.h
+
+Driver-internal low-level hardware specific driver demux interface
+~~
 
 .. kernel-doc:: drivers/media/dvb-core/demux.h
-- 
2.13.5



[PATCH 01/25] media: dvb_frontend: better document the -EPERM condition

2017-09-20 Thread Mauro Carvalho Chehab
Two readonly ioctls can't be allowed if the frontend device
is opened in read only mode. Explain why.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_frontend.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index 01bd19fd4c57..7dda5acea3f2 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1940,9 +1940,23 @@ static int dvb_frontend_ioctl(struct file *file, 
unsigned int cmd, void *parg)
return -ENODEV;
}
 
-   if ((file->f_flags & O_ACCMODE) == O_RDONLY &&
-   (_IOC_DIR(cmd) != _IOC_READ || cmd == FE_GET_EVENT ||
-cmd == FE_DISEQC_RECV_SLAVE_REPLY)) {
+   /*
+* If the frontend is opened in read-only mode, only the ioctls
+* that don't interfere at the tune logic should be accepted.
+* That allows an external application to monitor the DVB QoS and
+* statistics parameters.
+*
+* That matches all _IOR() ioctls, except for two special cases:
+*   - FE_GET_EVENT is part of the tuning logic on a DVB application;
+*   - FE_DISEQC_RECV_SLAVE_REPLY is part of DiSEqC 2.0
+* setup
+* So, those two ioctls should also return -EPERM, as otherwise
+* reading from them would interfere with a DVB tune application
+*/
+   if ((file->f_flags & O_ACCMODE) == O_RDONLY
+   && (_IOC_DIR(cmd) != _IOC_READ
+   || cmd == FE_GET_EVENT
+   || cmd == FE_DISEQC_RECV_SLAVE_REPLY)) {
up(>sem);
return -EPERM;
}
-- 
2.13.5



[PATCH 22/25] media: dmxdev.h: add kernel-doc markups for data types and functions

2017-09-20 Thread Mauro Carvalho Chehab
Despite being used by DVB drivers, this header was not documented.

Document it.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dmxdev.h | 90 -
 1 file changed, 88 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-core/dmxdev.h b/drivers/media/dvb-core/dmxdev.h
index 054fd4eb6192..9aa3ce3fc407 100644
--- a/drivers/media/dvb-core/dmxdev.h
+++ b/drivers/media/dvb-core/dmxdev.h
@@ -36,12 +36,33 @@
 #include "demux.h"
 #include "dvb_ringbuffer.h"
 
+/**
+ * enum dmxdev_type - type of demux filter type.
+ *
+ * @DMXDEV_TYPE_NONE:  no filter set.
+ * @DMXDEV_TYPE_SEC:   section filter.
+ * @DMXDEV_TYPE_PES:   Program Elementary Stream (PES) filter.
+ */
 enum dmxdev_type {
DMXDEV_TYPE_NONE,
DMXDEV_TYPE_SEC,
DMXDEV_TYPE_PES,
 };
 
+/**
+ * enum dmxdev_state - state machine for the dmxdev.
+ *
+ * @DMXDEV_STATE_FREE: indicates that the filter is freed.
+ * @DMXDEV_STATE_ALLOCATED:indicates that the filter was allocated
+ * to be used.
+ * @DMXDEV_STATE_SET:  indicates that the filter parameters are set.
+ * @DMXDEV_STATE_GO:   indicates that the filter is running.
+ * @DMXDEV_STATE_DONE: indicates that a packet was already filtered
+ * and the filter is now disabled.
+ * Set only if %DMX_ONESHOT. See
+ * _sct_filter_params.
+ * @DMXDEV_STATE_TIMEDOUT: Indicates a timeout condition.
+ */
 enum dmxdev_state {
DMXDEV_STATE_FREE,
DMXDEV_STATE_ALLOCATED,
@@ -51,12 +72,49 @@ enum dmxdev_state {
DMXDEV_STATE_TIMEDOUT
 };
 
+/**
+ * struct dmxdev_feed - digital TV dmxdev feed
+ *
+ * @pid:   Program ID to be filtered
+ * @ts:pointer to  dmx_ts_feed
+ * @next:   list_head pointing to the next feed.
+ */
+
 struct dmxdev_feed {
u16 pid;
struct dmx_ts_feed *ts;
struct list_head next;
 };
 
+/**
+ * struct dmxdev_filter - digital TV dmxdev filter
+ *
+ * @filter:a dmxdev filter. Currently used only for section filter:
+ * if the filter is Section, it contains a
+ *  dmx_section_filter @sec pointer.
+ * @feed:  a dmxdev feed. Depending on the feed type, it can be:
+ * for TS feed: a  list_head @ts list of TS and PES
+ * feeds;
+ * for section feed: a  dmx_section_feed @sec pointer.
+ * @params:dmxdev filter parameters. Depending on the feed type, it
+ * can be:
+ * for section filter: a  dmx_sct_filter_params @sec
+ * embedded struct;
+ * for a TS filter: a  dmx_pes_filter_params @pes
+ * embedded struct.
+ * @type:  type of the dmxdev filter, as defined by  dmxdev_type.
+ * @state: state of the dmxdev filter, as defined by  dmxdev_state.
+ * @dev:   pointer to  dmxdev.
+ * @buffer:an embedded  dvb_ringbuffer buffer.
+ * @mutex: protects the access to  dmxdev_filter.
+ * @timer:  timer_list embedded timer, used to check for
+ * feed timeouts.
+ * Only for section filter.
+ * @todo:  index for the @secheader.
+ * Only for section filter.
+ * @secheader: buffer cache to parse the section header.
+ * Only for section filter.
+ */
 struct dmxdev_filter {
union {
struct dmx_section_filter *sec;
@@ -86,7 +144,23 @@ struct dmxdev_filter {
u8 secheader[3];
 };
 
-
+/**
+ * struct dmxdev - Describes a digital TV demux device.
+ *
+ * @dvbdev:pointer to  dvb_device associated with
+ * the demux device node.
+ * @dvr_dvbdev:pointer to  dvb_device associated with
+ * the dvr device node.
+ * @filter:pointer to  dmxdev_filter.
+ * @demux: pointer to  dmx_demux.
+ * @filternum: number of filters.
+ * @capabilities:  demux capabilities as defined by  dmx_demux_caps.
+ * @exit:  flag to indicate that the demux is being released.
+ * @dvr_orig_fe:   pointer to  dmx_frontend.
+ * @dvr_buffer:embedded  dvb_ringbuffer for DVB output.
+ * @mutex: protects the usage of this structure.
+ * @lock:  protects access to >filter->data.
+ */
 struct dmxdev {
struct dvb_device *dvbdev;
struct dvb_device *dvr_dvbdev;
@@ -108,8 +182,20 @@ struct dmxdev {
spinlock_t lock;
 };
 
+/**
+ * dvb_dmxdev_init - initializes a digital TV demux and registers both demux
+ * and DVR devices.
+ *
+ * @dmxdev: pointer to  dmxdev.
+ * @adap: pointer to  dvb_adapter.
+ */
+int dvb_dmxdev_init(struct dmxdev *dmxdev, struct dvb_adapter *adap);
 
-int dvb_dmxdev_init(struct dmxdev *dmxdev, struct dvb_adapter *);
+/**
+ * dvb_dmxdev_release - releases a digital TV demux and unregisters it.
+ *
+ * @dmxdev: pointer to  

[PATCH 04/25] media: dvbdev: fully document its functions

2017-09-20 Thread Mauro Carvalho Chehab
There are several functions at the dvbdev that are common to all
digital TV device nodes with aren't documented.

Add documentation for them. No functional changes.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvbdev.h | 86 +
 1 file changed, 78 insertions(+), 8 deletions(-)

diff --git a/drivers/media/dvb-core/dvbdev.h b/drivers/media/dvb-core/dvbdev.h
index 53058da83873..bbc1c20c0529 100644
--- a/drivers/media/dvb-core/dvbdev.h
+++ b/drivers/media/dvb-core/dvbdev.h
@@ -261,7 +261,7 @@ void dvb_unregister_device(struct dvb_device *dvbdev);
  * dvb_create_media_graph - Creates media graph for the Digital TV part of the
  * device.
  *
- * @adap:  pointer to struct dvb_adapter
+ * @adap:  pointer to  dvb_adapter
  * @create_rf_connector:   if true, it creates the RF connector too
  *
  * This function checks all DVB-related functions at the media controller
@@ -274,12 +274,23 @@ void dvb_unregister_device(struct dvb_device *dvbdev);
 __must_check int dvb_create_media_graph(struct dvb_adapter *adap,
bool create_rf_connector);
 
+/**
+ * dvb_register_media_controller - registers a media controller at DVB adapter
+ *
+ * @adap:  pointer to  dvb_adapter
+ * @mdev:  pointer to  media_device
+ */
 static inline void dvb_register_media_controller(struct dvb_adapter *adap,
 struct media_device *mdev)
 {
adap->mdev = mdev;
 }
 
+/**
+ * dvb_get_media_controller - gets the associated media controller
+ *
+ * @adap:  pointer to  dvb_adapter
+ */
 static inline struct media_device
 *dvb_get_media_controller(struct dvb_adapter *adap)
 {
@@ -296,20 +307,71 @@ int dvb_create_media_graph(struct dvb_adapter *adap,
 #define dvb_get_media_controller(a) NULL
 #endif
 
-int dvb_generic_open (struct inode *inode, struct file *file);
-int dvb_generic_release (struct inode *inode, struct file *file);
-long dvb_generic_ioctl (struct file *file,
- unsigned int cmd, unsigned long arg);
+/**
+ * dvb_generic_open - Digital TV open function, used by DVB devices
+ *
+ * @inode: pointer to  inode.
+ * @file: pointer to  file.
+ *
+ * Checks if a DVB devnode is still valid, and if the permissions are
+ * OK and increment negative use count.
+ */
+int dvb_generic_open(struct inode *inode, struct file *file);
 
-/* we don't mess with video_usercopy() any more,
-we simply define out own dvb_usercopy(), which will hopefully become
-generic_usercopy()  someday... */
+/**
+ * dvb_generic_close - Digital TV close function, used by DVB devices
+ *
+ * @inode: pointer to  inode.
+ * @file: pointer to  file.
+ *
+ * Checks if a DVB devnode is still valid, and if the permissions are
+ * OK and decrement negative use count.
+ */
+int dvb_generic_release(struct inode *inode, struct file *file);
 
+/**
+ * dvb_generic_ioctl - Digital TV close function, used by DVB devices
+ *
+ * @file: pointer to  file.
+ * @cmd: Ioctl name.
+ * @arg: Ioctl argument.
+ *
+ * Checks if a DVB devnode and struct dvbdev.kernel_ioctl is still valid.
+ * If so, calls dvb_usercopy().
+ */
+long dvb_generic_ioctl(struct file *file,
+  unsigned int cmd, unsigned long arg);
+
+/**
+ * dvb_usercopy - copies data from/to userspace memory when an ioctl is
+ *  issued.
+ *
+ * @file: Pointer to struct 
+ * @cmd: Ioctl name.
+ * @arg: Ioctl argument.
+ * @func: function that will actually handle the ioctl
+ *
+ * Ancillary function that uses ioctl direction and size to copy from
+ * userspace. Then, it calls @func, and, if needed, data is copied back
+ * to userspace.
+ */
 int dvb_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
 int (*func)(struct file *file, unsigned int cmd, void *arg));
 
 /** generic DVB attach function. */
 #ifdef CONFIG_MEDIA_ATTACH
+
+/**
+ * dvb_attach - attaches a DVB frontend into the DVB core.
+ *
+ * @FUNCTION:  function on a frontend module to be called.
+ * @ARGS...:   @FUNCTION arguments.
+ *
+ * This ancillary function loads a frontend module in runtime and runs
+ * the @FUNCTION function there, with @ARGS.
+ * As it increments symbol usage cont, at unregister, dvb_detach()
+ * should be called.
+ */
 #define dvb_attach(FUNCTION, ARGS...) ({ \
void *__r = NULL; \
typeof() __a = symbol_request(FUNCTION); \
@@ -323,6 +385,14 @@ int dvb_usercopy(struct file *file, unsigned int cmd, 
unsigned long arg,
__r; \
 })
 
+/**
+ * dvb_detach - detaches a DVB frontend loaded via dvb_attach()
+ *
+ * @FUNC:  attach function
+ *
+ * Decrements usage count for a function previously called via dvb_attach().
+ */
+
 #define dvb_detach(FUNC)   symbol_put_addr(FUNC)
 
 #else
-- 
2.13.5



[PATCH 16/25] media: dvb_demux: document dvb_demux_filter and dvb_demux_feed

2017-09-20 Thread Mauro Carvalho Chehab
Document those two structs using kernel-doc markups.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_demux.h | 49 --
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_demux.h 
b/drivers/media/dvb-core/dvb_demux.h
index c9e94bc3a2e5..5b05e6320e33 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -60,6 +60,21 @@ enum dvb_dmx_state {
 
 #define SPEED_PKTS_INTERVAL 5
 
+/**
+ * struct dvb_demux_filter - Describes a DVB demux section filter.
+ *
+ * @filter:Section filter as defined by  dmx_section_filter.
+ * @maskandmode:   logical ``and`` bit mask.
+ * @maskandnotmode:logical ``and not`` bit mask.
+ * @doneq: flag that indicates when a filter is ready.
+ * @next:  pointer to the next section filter.
+ * @feed:   dvb_demux_feed pointer.
+ * @index: index of the used demux filter.
+ * @state: state of the filter as described by  dvb_dmx_state.
+ * @type:  type of the filter as described
+ * by  dvb_dmx_filter_type.
+ */
+
 struct dvb_demux_filter {
struct dmx_section_filter filter;
u8 maskandmode[DMX_MAX_FILTER_SIZE];
@@ -72,9 +87,39 @@ struct dvb_demux_filter {
enum dvb_dmx_state state;
enum dvb_dmx_filter_type type;
 
+   /* private: used only by av7110 */
u16 hw_handle;
 };
 
+/**
+ * struct dvb_demux_feed - describes a DVB field
+ *
+ * @feed:  a digital TV feed. It can either be a TS or a section feed:
+ *   - if the feed is TS, it contains  dvb_ts_feed;
+ *   - if the feed is section, it contains
+ *  dmx_section_feed.
+ * @cb:digital TV callbacks. depending on the feed type, it 
can be:
+ *   - if the feed is TS, it contains a dmx_ts_cb() callback;
+ *   - if the feed is section, it contains a dmx_section_cb() 
callback.
+ *
+ * @demux: pointer to  dvb_demux.
+ * @priv:  private data for the filter handling routine.
+ * @type:  type of the filter, as defined by  dvb_dmx_filter_type.
+ * @state: state of the filter as defined by  dvb_dmx_state.
+ * @pid:   PID to be filtered.
+ * @timeout:   feed timeout.
+ * @filter:pointer to  dvb_demux_filter.
+ * @ts_type:   type of TS, as defined by  ts_filter_type.
+ * @pes_type:  type of PES, as defined by  dmx_ts_pes.
+ * @cc:MPEG-TS packet continuity counter
+ * @pusi_seen: if true, indicates that a discontinuity was detected.
+ * it is used to prevent feeding of garbage from previous section.
+ * @peslen:length of the PES (Packet Elementary Stream).
+ * @list_head: head for the list of digital TV demux feeds.
+ * @index: a unique index for each feed. Can be used as hardware
+ * pid filter index.
+ *
+ */
 struct dvb_demux_feed {
union {
struct dmx_ts_feed ts;
@@ -99,12 +144,12 @@ struct dvb_demux_feed {
enum dmx_ts_pes pes_type;
 
int cc;
-   bool pusi_seen; /* prevents feeding of garbage from previous 
section */
+   bool pusi_seen;
 
u16 peslen;
 
struct list_head list_head;
-   unsigned int index; /* a unique index for each feed (can be used as 
hardware pid filter index) */
+   unsigned int index;
 };
 
 struct dvb_demux {
-- 
2.13.5



[PATCH 17/25] media: dvb_frontend: dtv_property_process_set() cleanups

2017-09-20 Thread Mauro Carvalho Chehab
From: Satendra Singh Thakur 

Since all properties in the func dtv_property_process_set() use
at most 4 bytes arguments, change the code to pass
u32 cmd and u32 data as function arguments, instead of passing a
pointer to the entire struct dtv_property *tvp.

Instead of having a generic dtv_property_dump(), added its own
properties debug logic at dtv_property_process_set().

Signed-off-by: Satendra Singh Thakur 
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_frontend.c | 125 --
 1 file changed, 72 insertions(+), 53 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index bd60a490ce0f..b7094c7a405f 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1107,22 +1107,19 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] 
= {
_DTV_CMD(DTV_STAT_TOTAL_BLOCK_COUNT, 0, 0),
 };
 
-static void dtv_property_dump(struct dvb_frontend *fe,
- bool is_set,
+static void dtv_get_property_dump(struct dvb_frontend *fe,
  struct dtv_property *tvp)
 {
int i;
 
if (tvp->cmd <= 0 || tvp->cmd > DTV_MAX_COMMAND) {
-   dev_warn(fe->dvb->device, "%s: %s tvp.cmd = 0x%08x undefined\n",
-   __func__,
-   is_set ? "SET" : "GET",
+   dev_warn(fe->dvb->device, "%s: GET tvp.cmd = 0x%08x undefined\n"
+   , __func__,
tvp->cmd);
return;
}
 
-   dev_dbg(fe->dvb->device, "%s: %s tvp.cmd= 0x%08x (%s)\n", __func__,
-   is_set ? "SET" : "GET",
+   dev_dbg(fe->dvb->device, "%s: GET tvp.cmd= 0x%08x (%s)\n", __func__,
tvp->cmd,
dtv_cmds[tvp->cmd].name);
 
@@ -1532,7 +1529,7 @@ static int dtv_property_process_get(struct dvb_frontend 
*fe,
return -EINVAL;
}
 
-   dtv_property_dump(fe, false, tvp);
+   dtv_get_property_dump(fe, tvp);
 
return 0;
 }
@@ -1755,16 +1752,36 @@ static int dvbv3_set_delivery_system(struct 
dvb_frontend *fe)
return emulate_delivery_system(fe, delsys);
 }
 
+/**
+ * dtv_property_process_set -  Sets a single DTV property
+ * @fe:Pointer to  dvb_frontend
+ * @file:  Pointer to  file
+ * @cmd:   Digital TV command
+ * @data:  An unsigned 32-bits number
+ *
+ * This routine assigns the property
+ * value to the corresponding member of
+ *  dtv_frontend_properties
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
 static int dtv_property_process_set(struct dvb_frontend *fe,
-   struct dtv_property *tvp,
-   struct file *file)
+   struct file *file,
+   u32 cmd, u32 data)
 {
int r = 0;
struct dtv_frontend_properties *c = >dtv_property_cache;
 
-   dtv_property_dump(fe, true, tvp);
-
-   switch(tvp->cmd) {
+   /** Dump DTV command name and value*/
+   if (!cmd || cmd > DTV_MAX_COMMAND)
+   dev_warn(fe->dvb->device, "%s: SET cmd 0x%08x undefined\n",
+__func__, cmd);
+   else
+   dev_dbg(fe->dvb->device,
+   "%s: SET cmd 0x%08x (%s) to 0x%08x\n",
+   __func__, cmd, dtv_cmds[cmd].name, data);
+   switch (cmd) {
case DTV_CLEAR:
/*
 * Reset a cache of data specific to the frontend here. This 
does
@@ -1784,133 +1801,133 @@ static int dtv_property_process_set(struct 
dvb_frontend *fe,
r = dtv_set_frontend(fe);
break;
case DTV_FREQUENCY:
-   c->frequency = tvp->u.data;
+   c->frequency = data;
break;
case DTV_MODULATION:
-   c->modulation = tvp->u.data;
+   c->modulation = data;
break;
case DTV_BANDWIDTH_HZ:
-   c->bandwidth_hz = tvp->u.data;
+   c->bandwidth_hz = data;
break;
case DTV_INVERSION:
-   c->inversion = tvp->u.data;
+   c->inversion = data;
break;
case DTV_SYMBOL_RATE:
-   c->symbol_rate = tvp->u.data;
+   c->symbol_rate = data;
break;
case DTV_INNER_FEC:
-   c->fec_inner = tvp->u.data;
+   c->fec_inner = data;
break;
case DTV_PILOT:
-   c->pilot = tvp->u.data;
+   c->pilot = data;
break;
case DTV_ROLLOFF:
-   c->rolloff = tvp->u.data;
+   c->rolloff = data;
break;
case 

[PATCH 14/25] media: dvb_demux.h: get rid of DMX_FEED_ENTRY() macro

2017-09-20 Thread Mauro Carvalho Chehab
This isn't used anywere. Get rid of it.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_demux.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_demux.h 
b/drivers/media/dvb-core/dvb_demux.h
index 9db3c2b7c64e..d9b30d669bf3 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -75,8 +75,6 @@ struct dvb_demux_filter {
u16 hw_handle;
 };
 
-#define DMX_FEED_ENTRY(pos) list_entry(pos, struct dvb_demux_feed, list_head)
-
 struct dvb_demux_feed {
union {
struct dmx_ts_feed ts;
-- 
2.13.5



[PATCH 03/25] media: dvbdev: convert DVB device types into an enum

2017-09-20 Thread Mauro Carvalho Chehab
Enums can be documented via kernel-doc. So, convert the
DVB_DEVICE_* macros to an enum.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvbdev.c | 34 +++
 drivers/media/dvb-core/dvbdev.h | 51 -
 2 files changed, 64 insertions(+), 21 deletions(-)

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 41aad0f99d73..7b4cdcfbb02c 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -51,8 +51,15 @@ static LIST_HEAD(dvb_adapter_list);
 static DEFINE_MUTEX(dvbdev_register_lock);
 
 static const char * const dnames[] = {
-   "video", "audio", "sec", "frontend", "demux", "dvr", "ca",
-   "net", "osd"
+   [DVB_DEVICE_VIDEO] ="video",
+   [DVB_DEVICE_AUDIO] ="audio",
+   [DVB_DEVICE_SEC] =  "sec",
+   [DVB_DEVICE_FRONTEND] = "frontend",
+   [DVB_DEVICE_DEMUX] ="demux",
+   [DVB_DEVICE_DVR] =  "dvr",
+   [DVB_DEVICE_CA] =   "ca",
+   [DVB_DEVICE_NET] =  "net",
+   [DVB_DEVICE_OSD] =  "osd"
 };
 
 #ifdef CONFIG_DVB_DYNAMIC_MINORS
@@ -60,7 +67,24 @@ static const char * const dnames[] = {
 #define DVB_MAX_IDSMAX_DVB_MINORS
 #else
 #define DVB_MAX_IDS4
-#define nums2minor(num, type, id)  ((num << 6) | (id << 4) | type)
+
+static int nums2minor(int num, enum dvb_device_type type, int id)
+{
+   int n = (num << 6) | (id << 4);
+
+   switch (type) {
+   case DVB_DEVICE_VIDEO:  return n;
+   case DVB_DEVICE_AUDIO:  return n | 1;
+   case DVB_DEVICE_SEC:return n | 2;
+   case DVB_DEVICE_FRONTEND:   return n | 3;
+   case DVB_DEVICE_DEMUX:  return n | 4;
+   case DVB_DEVICE_DVR:return n | 5;
+   case DVB_DEVICE_CA: return n | 6;
+   case DVB_DEVICE_NET:return n | 7;
+   case DVB_DEVICE_OSD:return n | 8;
+   }
+}
+
 #define MAX_DVB_MINORS (DVB_MAX_ADAPTERS*64)
 #endif
 
@@ -426,8 +450,8 @@ static int dvb_register_media_device(struct dvb_device 
*dvbdev,
 }
 
 int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
-   const struct dvb_device *template, void *priv, int type,
-   int demux_sink_pads)
+   const struct dvb_device *template, void *priv,
+   enum dvb_device_type type, int demux_sink_pads)
 {
struct dvb_device *dvbdev;
struct file_operations *dvbdevfops;
diff --git a/drivers/media/dvb-core/dvbdev.h b/drivers/media/dvb-core/dvbdev.h
index 49189392cf3b..53058da83873 100644
--- a/drivers/media/dvb-core/dvbdev.h
+++ b/drivers/media/dvb-core/dvbdev.h
@@ -35,15 +35,37 @@
 
 #define DVB_UNSET (-1)
 
-#define DVB_DEVICE_VIDEO  0
-#define DVB_DEVICE_AUDIO  1
-#define DVB_DEVICE_SEC2
-#define DVB_DEVICE_FRONTEND   3
-#define DVB_DEVICE_DEMUX  4
-#define DVB_DEVICE_DVR5
-#define DVB_DEVICE_CA 6
-#define DVB_DEVICE_NET7
-#define DVB_DEVICE_OSD8
+/* List of DVB device types */
+
+/**
+ * enum dvb_device_type - type of the Digital TV device
+ *
+ * @DVB_DEVICE_SEC:Digital TV standalone Common Interface (CI)
+ * @DVB_DEVICE_FRONTEND:   Digital TV frontend.
+ * @DVB_DEVICE_DEMUX:  Digital TV demux.
+ * @DVB_DEVICE_DVR:Digital TV digital video record (DVR).
+ * @DVB_DEVICE_CA: Digital TV Conditional Access (CA).
+ * @DVB_DEVICE_NET:Digital TV network.
+ *
+ * @DVB_DEVICE_VIDEO:  Digital TV video decoder.
+ * Deprecated. Used only on av7110-av.
+ * @DVB_DEVICE_AUDIO:  Digital TV audio decoder.
+ * Deprecated. Used only on av7110-av.
+ * @DVB_DEVICE_OSD:Digital TV On Screen Display (OSD).
+ * Deprecated. Used only on av7110.
+ */
+enum dvb_device_type {
+   DVB_DEVICE_SEC,
+   DVB_DEVICE_FRONTEND,
+   DVB_DEVICE_DEMUX,
+   DVB_DEVICE_DVR,
+   DVB_DEVICE_CA,
+   DVB_DEVICE_NET,
+
+   DVB_DEVICE_VIDEO,
+   DVB_DEVICE_AUDIO,
+   DVB_DEVICE_OSD,
+};
 
 #define DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr) \
static short adapter_nr[] = \
@@ -104,8 +126,7 @@ struct dvb_adapter {
  * @list_head: List head with all DVB devices
  * @fops:  pointer to struct file_operations
  * @adapter:   pointer to the adapter that holds this device node
- * @type:  type of the device: DVB_DEVICE_SEC, DVB_DEVICE_FRONTEND,
- * DVB_DEVICE_DEMUX, DVB_DEVICE_DVR, DVB_DEVICE_CA, DVB_DEVICE_NET
+ * @type:  type of the device, as defined by  dvb_device_type.
  * @minor: devnode minor number. Major number is always DVB_MAJOR.
  * @id:device ID number, inside the 

[PATCH 24/25] media: dvb-net.rst: document DVB network kAPI interface

2017-09-20 Thread Mauro Carvalho Chehab
That's the last DVB kAPI that misses documentation.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/kapi/dtv-core.rst |  1 +
 Documentation/media/kapi/dtv-net.rst  |  4 
 drivers/media/dvb-core/dvb_net.h  | 34 --
 3 files changed, 37 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/media/kapi/dtv-net.rst

diff --git a/Documentation/media/kapi/dtv-core.rst 
b/Documentation/media/kapi/dtv-core.rst
index 8ee384f61fa0..bca743dc6b43 100644
--- a/Documentation/media/kapi/dtv-core.rst
+++ b/Documentation/media/kapi/dtv-core.rst
@@ -34,3 +34,4 @@ I2C bus.
 dtv-frontend
 dtv-demux
 dtv-ca
+dtv-net
diff --git a/Documentation/media/kapi/dtv-net.rst 
b/Documentation/media/kapi/dtv-net.rst
new file mode 100644
index ..ced991b73d69
--- /dev/null
+++ b/Documentation/media/kapi/dtv-net.rst
@@ -0,0 +1,4 @@
+Digital TV Network kABI
+---
+
+.. kernel-doc:: drivers/media/dvb-core/dvb_net.h
diff --git a/drivers/media/dvb-core/dvb_net.h b/drivers/media/dvb-core/dvb_net.h
index e9b18aa03e02..1eae8bad7cc1 100644
--- a/drivers/media/dvb-core/dvb_net.h
+++ b/drivers/media/dvb-core/dvb_net.h
@@ -30,6 +30,22 @@
 
 #ifdef CONFIG_DVB_NET
 
+/**
+ * struct dvb_net - describes a DVB network interface
+ *
+ * @dvbdev:pointer to  dvb_device.
+ * @device:array of pointers to  net_device.
+ * @state: array of integers to each net device. A value
+ * different than zero means that the interface is
+ * in usage.
+ * @exit:  flag to indicate when the device is being removed.
+ * @demux: pointer to  dmx_demux.
+ * @ioctl_mutex:   protect access to this struct.
+ *
+ * Currently, the core supports up to %DVB_NET_DEVICES_MAX (10) network
+ * devices.
+ */
+
 struct dvb_net {
struct dvb_device *dvbdev;
struct net_device *device[DVB_NET_DEVICES_MAX];
@@ -39,8 +55,22 @@ struct dvb_net {
struct mutex ioctl_mutex;
 };
 
-void dvb_net_release(struct dvb_net *);
-int  dvb_net_init(struct dvb_adapter *, struct dvb_net *, struct dmx_demux *);
+/**
+ * dvb_net_init - nitializes a digital TV network device and registers it.
+ *
+ * @adap:  pointer to  dvb_adapter.
+ * @dvbnet:pointer to  dvb_net.
+ * @dmxdemux:  pointer to  dmx_demux.
+ */
+int dvb_net_init(struct dvb_adapter *adap, struct dvb_net *dvbnet,
+ struct dmx_demux *dmxdemux);
+
+/**
+ * dvb_net_release - releases a digital TV network device and unregisters it.
+ *
+ * @dvbnet:pointer to  dvb_net.
+ */
+void dvb_net_release(struct dvb_net *dvbnet);
 
 #else
 
-- 
2.13.5



[PATCH 25/25] media: dvb uAPI docs: get rid of examples section

2017-09-20 Thread Mauro Carvalho Chehab
That section is too outdated and got superseded by DVBv5 and
by libdvbv5.

Maybe some day we'll end adding updated examples there, but
while nobody has time or interest of doing that, just mention
that there and get rid of the current examples for good.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/uapi/dvb/examples.rst | 378 +-
 1 file changed, 6 insertions(+), 372 deletions(-)

diff --git a/Documentation/media/uapi/dvb/examples.rst 
b/Documentation/media/uapi/dvb/examples.rst
index e0f627ca2e4d..16dd90fa9e94 100644
--- a/Documentation/media/uapi/dvb/examples.rst
+++ b/Documentation/media/uapi/dvb/examples.rst
@@ -6,377 +6,11 @@
 Examples
 
 
-In this section we would like to present some examples for using the Digital
-TV API.
+In the past, we used to have a set of examples here. However, those
+examples got out of date and doesn't even compile nowadays.
 
-.. note::
+Also, nowadays, the best is to use the libdvbv5 DVB API nowadays,
+with is fully documented.
 
-   This section is out of date, and the code below won't even
-   compile. Please refer to the
-   `libdvbv5 `__ for
-   updated/recommended examples.
-
-
-.. _tuning:
-
-Example: Tuning
-===
-
-We will start with a generic tuning subroutine that uses the frontend
-and SEC, as well as the demux devices. The example is given for QPSK
-tuners, but can easily be adjusted for QAM.
-
-
-.. code-block:: c
-
- #include 
- #include 
- #include 
- #include 
- #include 
- #include 
- #include 
- #include 
-
- #include 
- #include 
- #include 
- #include 
-
- #define DMX "/dev/dvb/adapter0/demux1"
- #define FRONT "/dev/dvb/adapter0/frontend1"
- #define SEC "/dev/dvb/adapter0/sec1"
-
- /* routine for checking if we have a signal and other status information*/
- int FEReadStatus(int fd, fe_status_t *stat)
- {
-int ans;
-
-if ( (ans = ioctl(fd,FE_READ_STATUS,stat) < 0)){
-perror("FE READ STATUS: ");
-return -1;
-}
-
-if (*stat & FE_HAS_POWER)
-printf("FE HAS POWER\\n");
-
-if (*stat & FE_HAS_SIGNAL)
-printf("FE HAS SIGNAL\\n");
-
-if (*stat & FE_SPECTRUM_INV)
-printf("SPEKTRUM INV\\n");
-
-return 0;
- }
-
-
- /* tune qpsk */
- /* freq: frequency of transponder  */
- /* vpid, apid, tpid: PIDs of video, audio and teletext TS packets  */
- /* diseqc:   DiSEqC address of the used LNB*/
- /* pol:  Polarisation  */
- /* srate:Symbol Rate   */
- /* fec.  FEC   */
- /* lnb_lof1: local frequency of lower LNB band */
- /* lnb_lof2: local frequency of upper LNB band */
- /* lnb_slof: switch frequency of LNB   */
-
- int set_qpsk_channel(int freq, int vpid, int apid, int tpid,
-int diseqc, int pol, int srate, int fec, int lnb_lof1,
-int lnb_lof2, int lnb_slof)
- {
-struct secCommand scmd;
-struct secCmdSequence scmds;
-struct dmx_pes_filter_params pesFilterParams;
-FrontendParameters frp;
-struct pollfd pfd[1];
-FrontendEvent event;
-int demux1, demux2, demux3, front;
-
-frequency = (uint32_t) freq;
-symbolrate = (uint32_t) srate;
-
-if((front = open(FRONT,O_RDWR)) < 0){
-perror("FRONTEND DEVICE: ");
-return -1;
-}
-
-if((sec = open(SEC,O_RDWR)) < 0){
-perror("SEC DEVICE: ");
-return -1;
-}
-
-if (demux1 < 0){
-if ((demux1=open(DMX, O_RDWR|O_NONBLOCK))
-< 0){
-perror("DEMUX DEVICE: ");
-return -1;
-}
-}
-
-if (demux2 < 0){
-if ((demux2=open(DMX, O_RDWR|O_NONBLOCK))
-< 0){
-perror("DEMUX DEVICE: ");
-return -1;
-}
-}
-
-if (demux3 < 0){
-if ((demux3=open(DMX, O_RDWR|O_NONBLOCK))
-< 0){
-perror("DEMUX DEVICE: ");
-return -1;
-}
-}
-
-if (freq < lnb_slof) {
-frp.Frequency = (freq - lnb_lof1);
-scmds.continuousTone = SEC_TONE_OFF;
-} else {
-frp.Frequency = (freq - lnb_lof2);
-scmds.continuousTone = SEC_TONE_ON;
-}
-frp.Inversion = INVERSION_AUTO;
-if (pol) scmds.voltage = SEC_VOLTAGE_18;
-else scmds.voltage = SEC_VOLTAGE_13;
-
-scmd.type=0;
-scmd.u.diseqc.addr=0x10;
-scmd.u.diseqc.cmd=0x38;
-

[PATCH 08/25] media: dtv-demux.rst: minor markup improvements

2017-09-20 Thread Mauro Carvalho Chehab
Add a cross-reference to a mentioned structure and split
the kernel-doc stuff on a separate chapter.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/kapi/dtv-demux.rst | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/Documentation/media/kapi/dtv-demux.rst 
b/Documentation/media/kapi/dtv-demux.rst
index 8169c479156e..3ee69266e206 100644
--- a/Documentation/media/kapi/dtv-demux.rst
+++ b/Documentation/media/kapi/dtv-demux.rst
@@ -7,8 +7,8 @@ Digital TV Demux
 The Kernel Digital TV Demux kABI defines a driver-internal interface for
 registering low-level, hardware specific driver to a hardware independent
 demux layer. It is only of interest for Digital TV device driver writers.
-The header file for this kABI is named demux.h and located in
-drivers/media/dvb-core.
+The header file for this kABI is named ``demux.h`` and located in
+``drivers/media/dvb-core``.
 
 The demux kABI should be implemented for each demux in the system. It is
 used to select the TS source of a demux and to manage the demux resources.
@@ -27,7 +27,7 @@ tuning, are devined via the Digital TV Frontend kABI.
 The functions that implement the abstract interface demux should be defined
 static or module private and registered to the Demux core for external
 access. It is not necessary to implement every function in the struct
-_demux. For example, a demux interface might support Section filtering,
+:c:type:`dmx_demux`. For example, a demux interface might support Section 
filtering,
 but not PES filtering. The kABI client is expected to check the value of any
 function pointer before calling the function: the value of ``NULL`` means
 that the function is not available.
@@ -43,8 +43,6 @@ Linux Kernel calls the functions of a network device 
interface from a
 bottom half context. Thus, if a demux kABI function is called from network
 device code, the function must not sleep.
 
-
-
 Demux Callback API
 ~~
 
@@ -68,4 +66,7 @@ function call directly from a hardware interrupt.
 This mechanism is implemented by :c:func:`dmx_ts_cb()` and 
:c:func:`dmx_section_cb()`
 callbacks.
 
+Digital TV Demux functions and types
+
+
 .. kernel-doc:: drivers/media/dvb-core/demux.h
-- 
2.13.5



[PATCH 07/25] media: dtv-core.rst: split into multiple files

2017-09-20 Thread Mauro Carvalho Chehab
Instead of document all kAPI into a single file, split it
on multiple ones. That makes easier to maintain each part.

As a side effect, it will produce multiple html pages, with
is a good idea.

No changes at the text. Just some chapter levels changed.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/kapi/dtv-ca.rst   |   4 +
 Documentation/media/kapi/dtv-common.rst   |  55 +++
 Documentation/media/kapi/dtv-core.rst | 585 +-
 Documentation/media/kapi/dtv-demux.rst|  71 
 Documentation/media/kapi/dtv-frontend.rst | 443 ++
 5 files changed, 579 insertions(+), 579 deletions(-)
 create mode 100644 Documentation/media/kapi/dtv-ca.rst
 create mode 100644 Documentation/media/kapi/dtv-common.rst
 create mode 100644 Documentation/media/kapi/dtv-demux.rst
 create mode 100644 Documentation/media/kapi/dtv-frontend.rst

diff --git a/Documentation/media/kapi/dtv-ca.rst 
b/Documentation/media/kapi/dtv-ca.rst
new file mode 100644
index ..a4dd700189b0
--- /dev/null
+++ b/Documentation/media/kapi/dtv-ca.rst
@@ -0,0 +1,4 @@
+Digital TV Conditional Access kABI
+--
+
+.. kernel-doc:: drivers/media/dvb-core/dvb_ca_en50221.h
diff --git a/Documentation/media/kapi/dtv-common.rst 
b/Documentation/media/kapi/dtv-common.rst
new file mode 100644
index ..40cf1033b5e1
--- /dev/null
+++ b/Documentation/media/kapi/dtv-common.rst
@@ -0,0 +1,55 @@
+Digital TV Common functions
+---
+
+Math functions
+~~
+
+Provide some commonly-used math functions, usually required in order to
+estimate signal strength and signal to noise measurements in dB.
+
+.. kernel-doc:: drivers/media/dvb-core/dvb_math.h
+
+
+DVB devices
+~~~
+
+Those functions are responsible for handling the DVB device nodes.
+
+.. kernel-doc:: drivers/media/dvb-core/dvbdev.h
+
+Digital TV Ring buffer
+~~
+
+Those routines implement ring buffers used to handle digital TV data and
+copy it from/to userspace.
+
+.. note::
+
+  1) For performance reasons read and write routines don't check buffer sizes
+ and/or number of bytes free/available. This has to be done before these
+ routines are called. For example:
+
+   .. code-block:: c
+
+/* write @buflen: bytes */
+free = dvb_ringbuffer_free(rbuf);
+if (free >= buflen)
+count = dvb_ringbuffer_write(rbuf, buffer, buflen);
+else
+/* do something */
+
+/* read min. 1000, max. @bufsize: bytes */
+avail = dvb_ringbuffer_avail(rbuf);
+if (avail >= 1000)
+count = dvb_ringbuffer_read(rbuf, buffer, min(avail, bufsize));
+else
+/* do something */
+
+  2) If there is exactly one reader and one writer, there is no need
+ to lock read or write operations.
+ Two or more readers must be locked against each other.
+ Flushing the buffer counts as a read operation.
+ Resetting the buffer counts as a read and write operation.
+ Two or more writers must be locked against each other.
+
+.. kernel-doc:: drivers/media/dvb-core/dvb_ringbuffer.h
diff --git a/Documentation/media/kapi/dtv-core.rst 
b/Documentation/media/kapi/dtv-core.rst
index 4cf9cf63bafd..8ee384f61fa0 100644
--- a/Documentation/media/kapi/dtv-core.rst
+++ b/Documentation/media/kapi/dtv-core.rst
@@ -26,584 +26,11 @@ I2C bus.
abandoned standard, not used anymore) and ATSC version 3.0 current
proposals. Currently, the DVB subsystem doesn't implement those standards.
 
-Digital TV Common functions

 
-Math functions
-~~
+.. toctree::
+:maxdepth: 1
 
-Provide some commonly-used math functions, usually required in order to
-estimate signal strength and signal to noise measurements in dB.
-
-.. kernel-doc:: drivers/media/dvb-core/dvb_math.h
-
-
-DVB devices
-~~~
-
-Those functions are responsible for handling the DVB device nodes.
-
-.. kernel-doc:: drivers/media/dvb-core/dvbdev.h
-
-Digital TV Ring buffer
---
-
-Those routines implement ring buffers used to handle digital TV data and
-copy it from/to userspace.
-
-.. note::
-
-  1) For performance reasons read and write routines don't check buffer sizes
- and/or number of bytes free/available. This has to be done before these
- routines are called. For example:
-
-   .. code-block:: c
-
-/* write @buflen: bytes */
-free = dvb_ringbuffer_free(rbuf);
-if (free >= buflen)
-count = dvb_ringbuffer_write(rbuf, buffer, buflen);
-else
-/* do something */
-
-/* read min. 1000, max. @bufsize: bytes */
-avail = dvb_ringbuffer_avail(rbuf);
-if (avail >= 1000)
-count = dvb_ringbuffer_read(rbuf, buffer, min(avail, bufsize));
-else
-/* do something */
-
-  2) If there is exactly 

[PATCH 05/25] media: dvb_frontend.h: improve kernel-doc markups

2017-09-20 Thread Mauro Carvalho Chehab
Several minor adjustments at the kernel-doc markups:

- some syntax fixes;
- some cross-references;
- add cross-references for the mentioned ioctls;
- some constants marked as such.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_frontend.h | 94 +--
 1 file changed, 47 insertions(+), 47 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.h 
b/drivers/media/dvb-core/dvb_frontend.h
index d273ed2f72c9..ace0c2fb26c2 100644
--- a/drivers/media/dvb-core/dvb_frontend.h
+++ b/drivers/media/dvb-core/dvb_frontend.h
@@ -180,8 +180,8 @@ enum dvbfe_search {
 /**
  * struct dvb_tuner_ops - Tuner information and callbacks
  *
- * @info:  embedded struct dvb_tuner_info with tuner properties
- * @release:   callback function called when frontend is dettached.
+ * @info:  embedded  dvb_tuner_info with tuner properties
+ * @release:   callback function called when frontend is detached.
  * drivers should free any allocated memory.
  * @init:  callback function used to initialize the tuner device.
  * @sleep: callback function used to put the tuner to sleep.
@@ -191,14 +191,14 @@ enum dvbfe_search {
  * resuming from suspend.
  * @set_params:callback function used to inform the tuner to 
tune
  * into a digital TV channel. The properties to be used
- * are stored at @dvb_frontend.dtv_property_cache;. The
- * tuner demod can change the parameters to reflect the
- * changes needed for the channel to be tuned, and
+ * are stored at  dvb_frontend.dtv_property_cache.
+ * The tuner demod can change the parameters to reflect
+ * the changes needed for the channel to be tuned, and
  * update statistics. This is the recommended way to set
  * the tuner parameters and should be used on newer
  * drivers.
  * @set_analog_params: callback function used to tune into an analog TV
- * channel on hybrid tuners. It passes @analog_parameters;
+ * channel on hybrid tuners. It passes @analog_parameters
  * to the driver.
  * @set_config:callback function used to send some 
tuner-specific
  * parameters.
@@ -207,9 +207,9 @@ enum dvbfe_search {
  * @get_if_frequency:  get the Intermediate Frequency, in Hz. For baseband,
  * should return 0.
  * @get_status:returns the frontend lock status
- * @get_rf_strength:   returns the RF signal strengh. Used mostly to support
+ * @get_rf_strength:   returns the RF signal strength. Used mostly to support
  * analog TV and radio. Digital TV should report, instead,
- * via DVBv5 API (@dvb_frontend.dtv_property_cache;).
+ * via DVBv5 API ( dvb_frontend.dtv_property_cache).
  * @get_afc:   Used only by analog TV core. Reports the frequency
  * drift due to AFC.
  * @calc_regs: callback function used to pass register data settings
@@ -217,7 +217,7 @@ enum dvbfe_search {
  * @set_frequency: Set a new frequency. Shouldn't be used on newer drivers.
  * @set_bandwidth: Set a new frequency. Shouldn't be used on newer drivers.
  *
- * NOTE: frequencies used on get_frequency and set_frequency are in Hz for
+ * NOTE: frequencies used on @get_frequency and @set_frequency are in Hz for
  * terrestrial/cable or kHz for satellite.
  *
  */
@@ -283,14 +283,14 @@ struct analog_demod_info {
  * @set_params:callback function used to inform the demod to 
set the
  * demodulator parameters needed to decode an analog or
  * radio channel. The properties are passed via
- * struct @analog_params;.
+ *  analog_params.
  * @has_signal:returns 0x if has signal, or 0 if it 
doesn't.
  * @get_afc:   Used only by analog TV core. Reports the frequency
  * drift due to AFC.
  * @tuner_status:  callback function that returns tuner status bits, e. g.
- * TUNER_STATUS_LOCKED and TUNER_STATUS_STEREO.
+ * %TUNER_STATUS_LOCKED and %TUNER_STATUS_STEREO.
  * @standby:   set the tuner to standby mode.
- * @release:   callback function called when frontend is dettached.
+ * @release:   callback function called when frontend is detached.
  * drivers should free any allocated memory.
  * @i2c_gate_ctrl: controls the I2C gate. Newer drivers should use I2C
  * mux support instead.
@@ -321,10 +321,10 @@ struct dtv_frontend_properties;
  * struct 

[PATCH 20/25] media: dvb_demux.h: document functions

2017-09-20 Thread Mauro Carvalho Chehab
The functions used on dvb_demux.h are largely used on DVB drivers.
Yet, none of them are documented.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_demux.h | 106 +++--
 1 file changed, 103 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_demux.h 
b/drivers/media/dvb-core/dvb_demux.h
index 43b0cab2e932..15ee2ea23efe 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -232,13 +232,113 @@ struct dvb_demux {
int recording;
 };
 
-int dvb_dmx_init(struct dvb_demux *dvbdemux);
-void dvb_dmx_release(struct dvb_demux *dvbdemux);
-void dvb_dmx_swfilter_packets(struct dvb_demux *dvbdmx, const u8 *buf,
+/**
+ * dvb_dmx_init - initialize a digital TV demux struct.
+ *
+ * @demux:  dvb_demux to be initialized.
+ *
+ * Before being able to register a digital TV demux struct, drivers
+ * should call this routine. On its typical usage, some fields should
+ * be initialized at the driver before calling it.
+ *
+ * A typical usecase is::
+ *
+ * dvb->demux.dmx.capabilities =
+ * DMX_TS_FILTERING | DMX_SECTION_FILTERING |
+ * DMX_MEMORY_BASED_FILTERING;
+ * dvb->demux.priv   = dvb;
+ * dvb->demux.filternum  = 256;
+ * dvb->demux.feednum= 256;
+ * dvb->demux.start_feed = driver_start_feed;
+ * dvb->demux.stop_feed  = driver_stop_feed;
+ * ret = dvb_dmx_init(>demux);
+ * if (ret < 0)
+ * return ret;
+ */
+int dvb_dmx_init(struct dvb_demux *demux);
+
+/**
+ * dvb_dmx_release - releases a digital TV demux internal buffers.
+ *
+ * @demux:  dvb_demux to be released.
+ *
+ * The DVB core internally allocates data at @demux. This routine
+ * releases those data. Please notice that the struct itelf is not
+ * released, as it can be embedded on other structs.
+ */
+void dvb_dmx_release(struct dvb_demux *demux);
+
+/**
+ * dvb_dmx_swfilter_packets - use dvb software filter for a buffer with
+ * multiple MPEG-TS packets with 188 bytes each.
+ *
+ * @demux: pointer to  dvb_demux
+ * @buf: buffer with data to be filtered
+ * @count: number of MPEG-TS packets with size of 188.
+ *
+ * The routine will discard a DVB packet that don't start with 0x47.
+ *
+ * Use this routine if the DVB demux fills MPEG-TS buffers that are
+ * already aligned.
+ *
+ * NOTE: The @buf size should have size equal to ``count * 188``.
+ */
+void dvb_dmx_swfilter_packets(struct dvb_demux *demux, const u8 *buf,
  size_t count);
+
+/**
+ * dvb_dmx_swfilter -  use dvb software filter for a buffer with
+ * multiple MPEG-TS packets with 188 bytes each.
+ *
+ * @demux: pointer to  dvb_demux
+ * @buf: buffer with data to be filtered
+ * @count: number of MPEG-TS packets with size of 188.
+ *
+ * If a DVB packet doesn't start with 0x47, it will seek for the first
+ * byte that starts with 0x47.
+ *
+ * Use this routine if the DVB demux fill buffers that may not start with
+ * a packet start mark (0x47).
+ *
+ * NOTE: The @buf size should have size equal to ``count * 188``.
+ */
 void dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf, size_t count);
+
+/**
+ * dvb_dmx_swfilter_204 -  use dvb software filter for a buffer with
+ * multiple MPEG-TS packets with 204 bytes each.
+ *
+ * @demux: pointer to  dvb_demux
+ * @buf: buffer with data to be filtered
+ * @count: number of MPEG-TS packets with size of 204.
+ *
+ * If a DVB packet doesn't start with 0x47, it will seek for the first
+ * byte that starts with 0x47.
+ *
+ * Use this routine if the DVB demux fill buffers that may not start with
+ * a packet start mark (0x47).
+ *
+ * NOTE: The @buf size should have size equal to ``count * 204``.
+ */
 void dvb_dmx_swfilter_204(struct dvb_demux *demux, const u8 *buf,
  size_t count);
+
+/**
+ * dvb_dmx_swfilter_raw -  make the raw data available to userspace without
+ * filtering
+ *
+ * @demux: pointer to  dvb_demux
+ * @buf: buffer with data
+ * @count: number of packets to be passed. The actual size of each packet
+ * depends on the _demux->feed->cb.ts logic.
+ *
+ * Use it if the driver needs to deliver the raw payload to userspace without
+ * passing through the kernel demux. That is meant to support some
+ * delivery systems that aren't based on MPEG-TS.
+ *
+ * This function relies on _demux->feed->cb.ts to actually handle the
+ * buffer.
+ */
 void dvb_dmx_swfilter_raw(struct dvb_demux *demux, const u8 *buf,
  size_t count);
 
-- 
2.13.5



[PATCH 02/25] media: dvb_frontend: fix return values for FE_SET_PROPERTY

2017-09-20 Thread Mauro Carvalho Chehab
There are several problems with regards to the return of
FE_SET_PROPERTY. The original idea were to return per-property
return codes via tvp->result field, and to return an updated
set of values.

However, that never worked. What's actually implemented is:

- the FE_SET_PROPERTY implementation doesn't call .get_frontend
  callback in order to get the actual parameters after return;

- the tvp->result field is only filled if there's no error.
  So, it is always filled with zero;

- FE_SET_PROPERTY doesn't call memdup_user() nor any other
  copy_to_user() function. So, any changes at the properies
  will be lost;

- FE_SET_PROPERTY is declared as a write-only ioctl (IOW).

While we could fix the above, it could cause regressions.

So, let's just assume what the code really does, updating
the documentation accordingly and removing the logic that
would update the discarded tvp->result.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/uapi/dvb/fe-get-property.rst | 7 +--
 drivers/media/dvb-core/dvb_frontend.c| 2 --
 include/uapi/linux/dvb/frontend.h| 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/Documentation/media/uapi/dvb/fe-get-property.rst 
b/Documentation/media/uapi/dvb/fe-get-property.rst
index 948d2ba84f2c..b69741d9cedf 100644
--- a/Documentation/media/uapi/dvb/fe-get-property.rst
+++ b/Documentation/media/uapi/dvb/fe-get-property.rst
@@ -48,8 +48,11 @@ depends on the delivery system and on the device:
 
-  This call requires read/write access to the device.
 
-   -  At return, the values are updated to reflect the actual parameters
-  used.
+.. note::
+
+   At return, the values aren't updated to reflect the actual
+   parameters used. If the actual parameters are needed, an explicit
+   call to ``FE_GET_PROPERTY`` is needed.
 
 -  ``FE_GET_PROPERTY:``
 
diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index 7dda5acea3f2..bd60a490ce0f 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2142,7 +2142,6 @@ static int dvb_frontend_handle_ioctl(struct file *file,
kfree(tvp);
return err;
}
-   (tvp + i)->result = err;
}
kfree(tvp);
break;
@@ -2187,7 +2186,6 @@ static int dvb_frontend_handle_ioctl(struct file *file,
kfree(tvp);
return err;
}
-   (tvp + i)->result = err;
}
 
if (copy_to_user((void __user *)tvps->props, tvp,
diff --git a/include/uapi/linux/dvb/frontend.h 
b/include/uapi/linux/dvb/frontend.h
index 861cacd5711f..6bc26f35217b 100644
--- a/include/uapi/linux/dvb/frontend.h
+++ b/include/uapi/linux/dvb/frontend.h
@@ -830,7 +830,7 @@ struct dtv_fe_stats {
  * @cmd:   Digital TV command.
  * @reserved:  Not used.
  * @u: Union with the values for the command.
- * @result:Result of the command set (currently unused).
+ * @result:Unused
  *
  * The @u union may have either one of the values below:
  *
-- 
2.13.5



[PATCH 10/25] media: dvb_demux.h: add an enum for DMX_STATE_* and document

2017-09-20 Thread Mauro Carvalho Chehab
kernel-doc allows documenting enums. Also, it makes clearer
about the meaning of each field on structures.

So, convert DMX_STATE_* to an enum.

While here, get rid of the unused DMX_STATE_SET.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_demux.h | 25 ++---
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_demux.h 
b/drivers/media/dvb-core/dvb_demux.h
index 6bc4b27dbff3..b24d69b5a20f 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -37,11 +37,22 @@ enum dvb_dmx_filter_type {
DMX_TYPE_SEC,
 };
 
-#define DMX_STATE_FREE  0
-#define DMX_STATE_ALLOCATED 1
-#define DMX_STATE_SET   2
-#define DMX_STATE_READY 3
-#define DMX_STATE_GO4
+/**
+ * enum dvb_dmx_state - state machine for a demux filter.
+ *
+ * @DMX_STATE_FREE:indicates that the filter is freed.
+ * @DMX_STATE_ALLOCATED:   indicates that the filter was allocated
+ * to be used.
+ * @DMX_STATE_READY:   indicates that the filter is ready
+ * to be used.
+ * @DMX_STATE_GO:  indicates that the filter is running.
+ */
+enum dvb_dmx_state {
+   DMX_STATE_FREE,
+   DMX_STATE_ALLOCATED,
+   DMX_STATE_READY,
+   DMX_STATE_GO,
+};
 
 #define DVB_DEMUX_MASK_MAX 18
 
@@ -58,7 +69,7 @@ struct dvb_demux_filter {
struct dvb_demux_filter *next;
struct dvb_demux_feed *feed;
int index;
-   int state;
+   enum dvb_dmx_state state;
enum dvb_dmx_filter_type type;
 
u16 hw_handle;
@@ -81,7 +92,7 @@ struct dvb_demux_feed {
struct dvb_demux *demux;
void *priv;
enum dvb_dmx_filter_type type;
-   int state;
+   enum dvb_dmx_state state;
u16 pid;
 
ktime_t timeout;
-- 
2.13.5



[PATCH 13/25] media: dvb_demux: dvb_demux_feed.pusi_seen is boolean

2017-09-20 Thread Mauro Carvalho Chehab
Instead of using an integer to represent it, use boolean,
as this better describes what this field really means.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_demux.c| 12 ++--
 drivers/media/dvb-core/dvb_demux.h|  2 +-
 drivers/media/pci/ttpci/av7110.c  |  2 +-
 drivers/media/pci/ttpci/budget-core.c |  2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_demux.c 
b/drivers/media/dvb-core/dvb_demux.c
index 68e93362c081..b9360cbc3519 100644
--- a/drivers/media/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb-core/dvb_demux.c
@@ -223,10 +223,10 @@ static void dvb_dmx_swfilter_section_new(struct 
dvb_demux_feed *feed)
  *  when the second packet arrives.
  *
  * Fix:
- * when demux is started, let feed->pusi_seen = 0 to
+ * when demux is started, let feed->pusi_seen = false to
  * prevent initial feeding of garbage from the end of
  * previous section. When you for the first time see PUSI=1
- * then set feed->pusi_seen = 1
+ * then set feed->pusi_seen = true
  */
 static int dvb_dmx_swfilter_section_copy_dump(struct dvb_demux_feed *feed,
  const u8 *buf, u8 len)
@@ -318,10 +318,10 @@ static int dvb_dmx_swfilter_section_packet(struct 
dvb_demux_feed *feed,
 */
 #endif
/*
-* Discontinuity detected. Reset pusi_seen = 0 to
+* Discontinuity detected. Reset pusi_seen to
 * stop feeding of suspicious data until next PUSI=1 arrives
 */
-   feed->pusi_seen = 0;
+   feed->pusi_seen = false;
dvb_dmx_swfilter_section_new(feed);
}
 
@@ -335,8 +335,8 @@ static int dvb_dmx_swfilter_section_packet(struct 
dvb_demux_feed *feed,
 
dvb_dmx_swfilter_section_copy_dump(feed, before,
   before_len);
-   /* before start of new section, set pusi_seen = 1 */
-   feed->pusi_seen = 1;
+   /* before start of new section, set pusi_seen */
+   feed->pusi_seen = true;
dvb_dmx_swfilter_section_new(feed);
dvb_dmx_swfilter_section_copy_dump(feed, after,
   after_len);
diff --git a/drivers/media/dvb-core/dvb_demux.h 
b/drivers/media/dvb-core/dvb_demux.h
index 700887938145..9db3c2b7c64e 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -101,7 +101,7 @@ struct dvb_demux_feed {
enum dmx_ts_pes pes_type;
 
int cc;
-   int pusi_seen;  /* prevents feeding of garbage from previous 
section */
+   bool pusi_seen; /* prevents feeding of garbage from previous 
section */
 
u16 peslen;
 
diff --git a/drivers/media/pci/ttpci/av7110.c b/drivers/media/pci/ttpci/av7110.c
index f46947d8adf8..f89fb23f6c57 100644
--- a/drivers/media/pci/ttpci/av7110.c
+++ b/drivers/media/pci/ttpci/av7110.c
@@ -1224,7 +1224,7 @@ static int budget_start_feed(struct dvb_demux_feed *feed)
dprintk(2, "av7110: %p\n", budget);
 
spin_lock(>feedlock1);
-   feed->pusi_seen = 0; /* have a clean section start */
+   feed->pusi_seen = false; /* have a clean section start */
status = start_ts_capture(budget);
spin_unlock(>feedlock1);
return status;
diff --git a/drivers/media/pci/ttpci/budget-core.c 
b/drivers/media/pci/ttpci/budget-core.c
index 97499b2af714..b3dc45b91101 100644
--- a/drivers/media/pci/ttpci/budget-core.c
+++ b/drivers/media/pci/ttpci/budget-core.c
@@ -330,7 +330,7 @@ static int budget_start_feed(struct dvb_demux_feed *feed)
return -EINVAL;
 
spin_lock(>feedlock);
-   feed->pusi_seen = 0; /* have a clean section start */
+   feed->pusi_seen = false; /* have a clean section start */
if (budget->feeding++ == 0)
status = start_ts_capture(budget);
spin_unlock(>feedlock);
-- 
2.13.5



[PATCH 0/3] [media] TTUSB DVB Budget: Fine-tuning for three function implementations

2017-09-20 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 20 Sep 2017 21:03:45 +0200

Three update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Use common error handling code in ttusb_probe()
  Improve two size determinations in ttusb_probe()
  Adjust eight checks for null pointers

 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 34 +++
 1 file changed, 17 insertions(+), 17 deletions(-)

-- 
2.14.1



usb/media/smsusb: null-ptr-deref in smsusb_init_device

2017-09-20 Thread Andrey Konovalov
Hi!

I've got the following report while fuzzing the kernel with syzkaller.

On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).

The null-ptr-deref happens on
dev->udev->ep_in[1]->desc.wMaxPacketSize. There seems to be no check
on the number of endpoints.

usb 1-1: New USB device found, idVendor=2040, idProduct=5530
usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
gadgetfs: configuration #4
smsusb:smsusb_probe: board id=8, interface number 0
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault:  [#1] PREEMPT SMP KASAN
Modules linked in:
CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted
4.14.0-rc1-42251-gebb2c2437d80-dirty #208
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: usb_hub_wq hub_event
task: 88006bb26300 task.stack: 88006bba
RIP: 0010:smsusb_init_device+0x2f0/0xd10 drivers/media/usb/siano/smsusb.c:431
RSP: 0018:88006bba6340 EFLAGS: 00010247
RAX: dc00 RBX: 880063e1 RCX: 11003ab8
RDX:  RSI: 880063e10bac RDI: 0004
RBP: 88006bba6478 R08: ed000d774c84 R09: 88006bba63b0
R10: 000e R11: ed000d774c83 R12: 
R13:  R14:  R15: 88006840d500
FS:  () GS:88006c60() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 7fff57742008 CR3: 67444000 CR4: 06f0
Call Trace:
 smsusb_probe+0x4f5/0xdc0 drivers/media/usb/siano/smsusb.c:571
 usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
 really_probe drivers/base/dd.c:413
 driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
 __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
 bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
 __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
 device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
 bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
 device_add+0xd0b/0x1660 drivers/base/core.c:1835
 usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
 generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
 usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
 really_probe drivers/base/dd.c:413
 driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
 __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
 bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
 __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
 device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
 bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
 device_add+0xd0b/0x1660 drivers/base/core.c:1835
 usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
 hub_port_connect drivers/usb/core/hub.c:4903
 hub_port_connect_change drivers/usb/core/hub.c:5009
 port_event drivers/usb/core/hub.c:5115
 hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
 process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
 worker_thread+0x221/0x1850 kernel/workqueue.c:2253
 kthread+0x3a1/0x470 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
Code: 00 0f 85 d1 07 00 00 48 8b 85 f0 fe ff ff 4c 8b a0 a8 05 00 00
48 b8 00 00 00 00 00 fc ff df 49 8d 7c 24 04 48 89 fa 48 c1 ea 03 <0f>
b6 14 02 48 89 f8 83 e0 07 83 c0 01 38 d0 7c 08 84 d2 0f 85
RIP: smsusb_init_device+0x2f0/0xd10 RSP: 88006bba6340
---[ end trace 1e8f3aa7788a0764 ]---


[RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling

2017-09-20 Thread Wolfram Sang
One helper checks if DMA is suitable and optionally creates a bounce
buffer, if not. The other function returns the bounce buffer and makes
sure the data is properly copied back to the message.

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/i2c-core-base.c | 45 +
 include/linux/i2c.h |  3 +++
 2 files changed, 48 insertions(+)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 56e46581b84bdb..925a22794d3ced 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2241,6 +2241,51 @@ void i2c_put_adapter(struct i2c_adapter *adap)
 }
 EXPORT_SYMBOL(i2c_put_adapter);
 
+/**
+ * i2c_get_dma_safe_msg_buf() - get a DMA safe buffer for the given i2c_msg
+ * @msg: the message to be checked
+ * @threshold: the amount of byte from which using DMA makes sense
+ *
+ * Return: NULL if a DMA safe buffer was not obtained. Use msg->buf with PIO.
+ *
+ *Or a valid pointer to be used with DMA. Note that it can either be
+ *msg->buf or a bounce buffer. After use, release it by calling
+ *i2c_release_dma_safe_msg_buf().
+ *
+ * This function must only be called from process context!
+ */
+u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold)
+{
+   if (msg->len < threshold)
+   return NULL;
+
+   if (msg->flags & I2C_M_DMA_SAFE)
+   return msg->buf;
+
+   if (msg->flags & I2C_M_RD)
+   return kzalloc(msg->len, GFP_KERNEL);
+   else
+   return kmemdup(msg->buf, msg->len, GFP_KERNEL);
+}
+EXPORT_SYMBOL_GPL(i2c_get_dma_safe_msg_buf);
+
+/**
+ * i2c_release_dma_safe_msg_buf - release DMA safe buffer and sync with i2c_msg
+ * @msg: the message to be synced with
+ * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be NULL.
+ */
+void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
+{
+   if (!buf || buf == msg->buf)
+   return;
+
+   if (msg->flags & I2C_M_RD)
+   memcpy(msg->buf, buf, msg->len);
+
+   kfree(buf);
+}
+EXPORT_SYMBOL_GPL(i2c_release_dma_safe_msg_buf);
+
 MODULE_AUTHOR("Simon G. Vogl ");
 MODULE_DESCRIPTION("I2C-Bus main module");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index d501d3956f13f0..1e99342f180f45 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -767,6 +767,9 @@ static inline u8 i2c_8bit_addr_from_msg(const struct 
i2c_msg *msg)
return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
 }
 
+u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold);
+void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf);
+
 int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short 
addr);
 /**
  * module_i2c_driver() - Helper macro for registering a modular I2C driver
-- 
2.11.0



[RFC PATCH v5 0/6] i2c: document DMA handling and add helpers for it

2017-09-20 Thread Wolfram Sang
So, after revisiting old mail threads, taking part in a similar discussion on
the USB list, and implementing a not-convincing solution before, here is what I
cooked up to document and ease DMA handling for I2C within Linux. Please have a
look at the documentation introduced in patch 3 for details. And to make it
clear again: The stuff below is opt-in. If host drivers are not updated, they
will continue to work like before.

While the previous versions tried to magically apply bounce buffers when
needed, it became clear that detecting DMA safe buffers is too fragile. This
approach is now opt-in, a DMA_SAFE flag needs to be set on an i2c_msg. The
outcome so far is very convincing IMO. The core additions are simple and easy
to understand (makes me even think of inlining them again?). The driver changes
for the Renesas IP cores became easier to understand, too. While only a tad for
the i2c-sh_mobile driver, the situation became a LOT better for the i2c-rcar
driver. No more DMA disabling for the whole transfer in case of unsafe buffers,
we are back to per-msg handling. And the code fix is now an easy to understand
one line change. Yay!

Of course, we must now whitelist DMA safe buffers. An example for I2C_RDWR case
is in this series. It makes the i2ctransfer utility have DMA_SAFE buffers,
which is nice for testing as i2cdump will (currently) not use DMA_SAFE buffers.
My plan is to add two new calls: i2c_master_{send|receive}_dma_safe which can
be used if DMA_SAFE buffers are provided. So, drivers can simply switch to
them. Also, the buffers used within i2c_smbus_xfer_emulated() need to be
converted to be DMA_SAFE which will cover a huge bunch of use cases. The rest
is then updating drivers which can be done when needed.

As these conversions are not done yet, this patch series has RFC status. But I
already would like to get opinions on this approach, so I'll cc mailing lists
of the heavier I2C users. Please let me know what you think.

All patches have been tested with a Renesas Salvator-X board (r8a7796/M3-W).

The branch can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
renesas/topic/i2c-core-dma-rfc-v5

And big kudos to Renesas Electronics for funding this work, thank you very much!

Regards,

   Wolfram

Changes since v4:
* rebased to v4.14-rc1
* (hopefully) improved the docs to be more clear
* added basic RST syntax to the docs

This is mainly an update to agree on the docs. Missing code is still missing
and will be added in v6.

Changes since v3:
* completely redesigned


Wolfram Sang (6):
  i2c: add a message flag for DMA safe buffers
  i2c: add helpers to ease DMA handling
  i2c: add docs to clarify DMA handling
  i2c: sh_mobile: use helper to decide if DMA is useful
  i2c: rcar: skip DMA if buffer is not safe
  i2c: dev: mark RDWR buffers as DMA_SAFE

 Documentation/i2c/DMA-considerations | 58 
 drivers/i2c/busses/i2c-rcar.c|  2 +-
 drivers/i2c/busses/i2c-sh_mobile.c   |  8 +++--
 drivers/i2c/i2c-core-base.c  | 45 
 drivers/i2c/i2c-dev.c|  2 ++
 include/linux/i2c.h  |  3 ++
 include/uapi/linux/i2c.h |  3 ++
 7 files changed, 118 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/i2c/DMA-considerations

-- 
2.11.0



[RFC PATCH v5 4/6] i2c: sh_mobile: use helper to decide if DMA is useful

2017-09-20 Thread Wolfram Sang
This ensures that we fall back to PIO if the message length is too small
for DMA being useful. Otherwise, we use DMA. A bounce buffer might be
applied by the helper if the original message buffer is not DMA safe.

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/busses/i2c-sh_mobile.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c 
b/drivers/i2c/busses/i2c-sh_mobile.c
index 6f2aaeb7c4fa15..b76399d8a3abd3 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -145,6 +145,7 @@ struct sh_mobile_i2c_data {
struct dma_chan *dma_rx;
struct scatterlist sg;
enum dma_data_direction dma_direction;
+   u8 *dma_buf;
 };
 
 struct sh_mobile_dt_config {
@@ -548,6 +549,8 @@ static void sh_mobile_i2c_dma_callback(void *data)
pd->pos = pd->msg->len;
pd->stop_after_dma = true;
 
+   i2c_release_dma_safe_msg_buf(pd->msg, pd->dma_buf);
+
iic_set_clr(pd, ICIC, 0, ICIC_TDMAE | ICIC_RDMAE);
 }
 
@@ -608,7 +611,7 @@ static void sh_mobile_i2c_xfer_dma(struct 
sh_mobile_i2c_data *pd)
if (IS_ERR(chan))
return;
 
-   dma_addr = dma_map_single(chan->device->dev, pd->msg->buf, 
pd->msg->len, dir);
+   dma_addr = dma_map_single(chan->device->dev, pd->dma_buf, pd->msg->len, 
dir);
if (dma_mapping_error(chan->device->dev, dma_addr)) {
dev_dbg(pd->dev, "dma map failed, using PIO\n");
return;
@@ -665,7 +668,8 @@ static int start_ch(struct sh_mobile_i2c_data *pd, struct 
i2c_msg *usr_msg,
pd->pos = -1;
pd->sr = 0;
 
-   if (pd->msg->len > 8)
+   pd->dma_buf = i2c_get_dma_safe_msg_buf(pd->msg, 8);
+   if (pd->dma_buf)
sh_mobile_i2c_xfer_dma(pd);
 
/* Enable all interrupts to begin with */
-- 
2.11.0



[RFC PATCH v5 3/6] i2c: add docs to clarify DMA handling

2017-09-20 Thread Wolfram Sang
Signed-off-by: Wolfram Sang 
---
 Documentation/i2c/DMA-considerations | 58 
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/i2c/DMA-considerations

diff --git a/Documentation/i2c/DMA-considerations 
b/Documentation/i2c/DMA-considerations
new file mode 100644
index 00..5a63355c6a9b6f
--- /dev/null
+++ b/Documentation/i2c/DMA-considerations
@@ -0,0 +1,58 @@
+=
+Linux I2C and DMA
+=
+
+Given that I2C is a low-speed bus where largely small messages are transferred,
+it is not considered a prime user of DMA access. At this time of writing, only
+10% of I2C bus master drivers have DMA support implemented. And the vast
+majority of transactions are so small that setting up DMA for it will likely
+add more overhead than a plain PIO transfer.
+
+Therefore, it is *not* mandatory that the buffer of an I2C message is DMA safe.
+It does not seem reasonable to apply additional burdens when the feature is so
+rarely used. However, it is recommended to use a DMA-safe buffer if your
+message size is likely applicable for DMA. Most drivers have this threshold
+around 8 bytes (as of today, this is mostly an educated guess, however). For
+any message of 16 byte or larger, it is probably a really good idea. Please
+note that other subsystems you use might add requirements. E.g., if your
+I2C bus master driver is using USB as a bridge, then you need to have DMA
+safe buffers always, because USB requires it.
+
+For clients, if you use a DMA safe buffer in i2c_msg, set the I2C_M_DMA_SAFE
+flag with it. Then, the I2C core and drivers know they can safely operate DMA
+on it. Note that using this flag is optional. I2C host drivers which are not
+updated to use this flag will work like before. And like before, they risk
+using an unsafe DMA buffer. To improve this situation, using I2C_M_DMA_SAFE in
+more and more clients and host drivers is the planned way forward. Note also
+that setting this flag makes only sense in kernel space. User space data is
+copied into kernel space anyhow. The I2C core makes sure the destination
+buffers in kernel space are always DMA capable.
+
+FIXME: Need to implement i2c_master_{send|receive}_dma and proper buffers for 
i2c_smbus_xfer_emulated.
+
+Drivers wishing to implement safe DMA can use helper functions from the I2C
+core. One gives you a DMA-safe buffer for a given i2c_msg as long as a certain
+threshold is met::
+
+   dma_buf = i2c_get_dma_safe_msg_buf(msg, threshold_in_byte);
+
+If a buffer is returned, it is either msg->buf for the I2C_M_DMA_SAFE case or a
+bounce buffer. But you don't need to care about that detail, just use the
+returned buffer. If NULL is returned, the threshold was not met or a bounce
+buffer could not be allocated. Fall back to PIO in that case.
+
+In any case, a buffer obtained from above needs to be released. It ensures data
+is copied back to the message and a potentially used bounce buffer is freed::
+
+   i2c_release_dma_safe_msg_buf(msg, dma_buf);
+
+The bounce buffer handling from the core is generic and simple. It will always
+allocate a new bounce buffer. If you want a more sophisticated handling (e.g.
+reusing pre-allocated buffers), you are free to implement your own.
+
+Please also check the in-kernel documentation for details. The i2c-sh_mobile
+driver can be used as a reference example how to use the above helpers.
+
+Final note: If you plan to use DMA with I2C (or with anything else, actually)
+make sure you have CONFIG_DMA_API_DEBUG enabled during development. It can help
+you find various issues which can be complex to debug otherwise.
-- 
2.11.0



[RFC PATCH v5 1/6] i2c: add a message flag for DMA safe buffers

2017-09-20 Thread Wolfram Sang
I2C has no requirement that the buffer of a message needs to be DMA
safe. In case it is, it can now be flagged, so drivers wishing to
do DMA can use the buffer directly.

Signed-off-by: Wolfram Sang 
---
 include/uapi/linux/i2c.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/i2c.h b/include/uapi/linux/i2c.h
index 009e27bb9abe19..1c683cb319e4b7 100644
--- a/include/uapi/linux/i2c.h
+++ b/include/uapi/linux/i2c.h
@@ -71,6 +71,9 @@ struct i2c_msg {
 #define I2C_M_RD   0x0001  /* read data, from slave to master */
/* I2C_M_RD is guaranteed to be 0x0001! 
*/
 #define I2C_M_TEN  0x0010  /* this is a ten bit chip address */
+#define I2C_M_DMA_SAFE 0x0200  /* the buffer of this message is DMA 
safe */
+   /* makes only sense in kernelspace */
+   /* userspace buffers are copied anyway 
*/
 #define I2C_M_RECV_LEN 0x0400  /* length will be first received byte */
 #define I2C_M_NO_RD_ACK0x0800  /* if 
I2C_FUNC_PROTOCOL_MANGLING */
 #define I2C_M_IGNORE_NAK   0x1000  /* if I2C_FUNC_PROTOCOL_MANGLING */
-- 
2.11.0



[RFC PATCH v5 5/6] i2c: rcar: skip DMA if buffer is not safe

2017-09-20 Thread Wolfram Sang
This HW is prone to races, so it needs to setup new messages in irq
context. That means we can't alloc bounce buffers if a message buffer is
not DMA safe. So, in that case, simply fall back to PIO.

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/busses/i2c-rcar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 15d764afec3b29..8a2ae3e6c561c4 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -359,7 +359,7 @@ static void rcar_i2c_dma(struct rcar_i2c_priv *priv)
int len;
 
/* Do not use DMA if it's not available or for messages < 8 bytes */
-   if (IS_ERR(chan) || msg->len < 8)
+   if (IS_ERR(chan) || msg->len < 8 || !(msg->flags & I2C_M_DMA_SAFE))
return;
 
if (read) {
-- 
2.11.0



[RFC PATCH v5 6/6] i2c: dev: mark RDWR buffers as DMA_SAFE

2017-09-20 Thread Wolfram Sang
Signed-off-by: Wolfram Sang 
---
 drivers/i2c/i2c-dev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index 6f638bbc922db4..bbc7aadb4c899d 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -280,6 +280,8 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client 
*client,
res = PTR_ERR(rdwr_pa[i].buf);
break;
}
+   /* memdup_user allocates with GFP_KERNEL, so DMA is ok */
+   rdwr_pa[i].flags |= I2C_M_DMA_SAFE;
 
/*
 * If the message length is received from the slave (similar
-- 
2.11.0



usb/media/cx231xx: null-ptr-deref in cx231xx_usb_probe

2017-09-20 Thread Andrey Konovalov
Hi!

I've got the following report while fuzzing the kernel with syzkaller.

On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).

The null-ptr-deref happens on assoc_desc->bFirstInterface, where
assoc_desc = udev->actconfig->intf_assoc[0]. There seems to be no
check that the device actually contains an Interface Association
Descriptor.

kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault:  [#1] PREEMPT SMP KASAN
Modules linked in:
CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted 4.14.0-rc1-42251-gebb2c2437d80 #206
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: usb_hub_wq hub_event
task: 88006bb26300 task.stack: 88006bba
RIP: 0010:cx231xx_usb_probe+0x96a/0x32e0
drivers/media/usb/cx231xx/cx231xx-cards.c:1687
RSP: 0018:88006bba63e0 EFLAGS: 00010246
RAX: dc00 RBX:  RCX: f8f8
RDX:  RSI: 86876d60 RDI: 0002
RBP: 88006bba65e8 R08: 0002 R09: 
R10:  R11:  R12: 870a62e0
R13: 88005ba70028 R14: 880062c9aa80 R15: 880062c9b018
FS:  () GS:88006c60() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 7f83211b5518 CR3: 5b9dc000 CR4: 06f0
Call Trace:
 usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
 really_probe drivers/base/dd.c:413
 driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
 __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
 bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
 __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
 device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
 bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
 device_add+0xd0b/0x1660 drivers/base/core.c:1835
 usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
 generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
 usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
 really_probe drivers/base/dd.c:413
 driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
 __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
 bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
 __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
 device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
 bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
 device_add+0xd0b/0x1660 drivers/base/core.c:1835
 usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
 hub_port_connect drivers/usb/core/hub.c:4903
 hub_port_connect_change drivers/usb/core/hub.c:5009
 port_event drivers/usb/core/hub.c:5115
 hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
 process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
 worker_thread+0x221/0x1850 kernel/workqueue.c:2253
 kthread+0x3a1/0x470 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
Code: 18 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 aa 28 00 00 48 b8 00
00 00 00 00 fc ff df 48 8b 5b 18 48 8d 7b 02 48 89 fa 48 c1 ea 03 <0f>
b6 04 02 48 89 fa 83 e2 07 38 d0 7f 08 84 c0 0f 85 50 28 00
RIP: cx231xx_usb_probe+0x96a/0x32e0 RSP: 88006bba63e0


Re: [RFC PATCH v4 3/6] i2c: add docs to clarify DMA handling

2017-09-20 Thread Wolfram Sang

> In order to avoid that, and to place them into a box using monotonic fonts,
> I usually add "::" at the preceding line, e. g.:

Just in time: I added the '::' and will resubmit the new version in a
minute.

Thanks for the pointers!



signature.asc
Description: PGP signature


Re: [RFC PATCH v4 3/6] i2c: add docs to clarify DMA handling

2017-09-20 Thread Mauro Carvalho Chehab
Em Wed, 20 Sep 2017 19:18:40 +0200
Wolfram Sang  escreveu:

> Hi Mauro,
> 
> > > +Linux I2C and DMA
> > > +-  
> > 
> > I would use, instead:
> > 
> > =
> > Linux I2C and DMA
> > =
> > 
> > As this is the way we're starting document titles, after converted to
> > ReST. So, better to have it already using the right format, as one day  
> 
> I did this.
> 
> > There are also a couple of things here that Sphinx would complain.  
> 
> The only complaint I got was
> 
>   WARNING: document isn't included in any toctree
> 
> which makes sense because I renamed it only temporarily to *.rst

Yeah, that is expected.

> > So, it could be worth to rename it to *.rst, while you're writing
> > it, and see what:
> > make htmldocs
> > will complain and how it will look in html.  
> 
> So, no complaints from Sphinx and the HTML output looks good IMO. Was
> there anything specific you had in mind when saying that Sphinx would
> complain?

Perhaps my comments weren't clear enough. Sorry! I didn't actually 
tried to parse it with Sphinx. Just wanted to hint you about that,
as testing the docs with Sphinx could be useful when writing
documentation. 

Usually, things like function declarations produce warnings if they
contain pointers, e. g. something like:

foo(void *bar);

as asterisks mean italics. It would complain about the lack of
an end asterisk.

In order to avoid that, and to place them into a box using monotonic fonts,
I usually add "::" at the preceding line, e. g.:

::

foo(void *bar);

or:

some description::

foo(void *bar)

on all functions (even the ones that don't use asterisks, as the
html output looks nicer.

I double-checked this patch: it doesn't contain anything that would
cause warnings or parse errors. Still, I would prefer to use
**not** instead of *not*, and would add the "::", but that's my
personal taste.

Thanks,
Mauro


pgpSHsBLytuVf.pgp
Description: Assinatura digital OpenPGP


Re: [PATCH] dma-fence: fix dma_fence_get_rcu_safe

2017-09-20 Thread Daniel Vetter
On Mon, Sep 11, 2017 at 01:06:32PM +0200, Christian König wrote:
> Am 11.09.2017 um 12:01 schrieb Chris Wilson:
> > [SNIP]
> > > Yeah, but that is illegal with a fence objects.
> > >
> > > When anybody allocates fences this way it breaks at least
> > > reservation_object_get_fences_rcu(),
> > > reservation_object_wait_timeout_rcu() and
> > > reservation_object_test_signaled_single().
> > Many, many months ago I sent patches to fix them all.
>
> Found those after a bit a searching. Yeah, those patches where proposed more
> than a year ago, but never pushed upstream.
>
> Not sure if we really should go this way. dma_fence objects are shared
> between drivers and since we can't judge if it's the correct fence based on
> a criteria in the object (only the read counter which is outside) all
> drivers need to be correct for this.
>
> I would rather go the way and change dma_fence_release() to wrap
> fence->ops->release into call_rcu() to keep the whole RCU handling outside
> of the individual drivers.

Hm, I entirely dropped the ball on this, I kinda assumed that we managed
to get some agreement on this between i915 and dma_fence. Adding a pile
more people.

Joonas, Tvrtko, I guess we need to fix this one way or the other.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: A patch slipped through the cracks?

2017-09-20 Thread Mauro Carvalho Chehab
Em Wed, 20 Sep 2017 19:17:17 +0200
Lubomir Rintel  escreveu:

> Hi,
> 
> we're trying to get this reasonably trivial patch [1] applied for more
> than a year and four attempts now. (I'm not including it in this
> message so that this message won't be ignored for the same reason the
> submissions were, whatever they are.)
> 
> [1] https://patchwork.linuxtv.org/patch/40862/
> 
> I have no idea what went wrong. There was a suspicion (somewhat
> confirmed by the initial patch submitter) that spam filtering could
> have dropped the first message. Since then the patch did make it to the
> list numerous times and was picked up by patchwork.
> 
> The patchwork's idea about the patch being "Superseded" is wrong -- I
> have no idea why. But someone *please* look into this and apply the
> patch.

Not sure what happened, but I tagged is as new. I'll take a look
on it later, when I start handling patches for Kernel 4.15.

Regards,
Maur

Thanks,
Mauro


Re: [RFC PATCH v4 3/6] i2c: add docs to clarify DMA handling

2017-09-20 Thread Wolfram Sang
Hi Mauro,

> > +Linux I2C and DMA
> > +-
> 
> I would use, instead:
> 
> =
> Linux I2C and DMA
> =
> 
> As this is the way we're starting document titles, after converted to
> ReST. So, better to have it already using the right format, as one day

I did this.

> There are also a couple of things here that Sphinx would complain.

The only complaint I got was

WARNING: document isn't included in any toctree

which makes sense because I renamed it only temporarily to *.rst

> So, it could be worth to rename it to *.rst, while you're writing
> it, and see what:
>   make htmldocs
> will complain and how it will look in html.

So, no complaints from Sphinx and the HTML output looks good IMO. Was
there anything specific you had in mind when saying that Sphinx would
complain?

Regards,

   Wolfram



signature.asc
Description: PGP signature


Re: [media] Siano: Use common error handling code in smsusb_init_device()

2017-09-20 Thread SF Markus Elfring
> If smscore_register_device() succeeds then mdev is freed when we call
> smsusb_term_device(intf);  The call tree is:

Thanks for your constructive information.


How do you think about another implementation detail in this function then?

May the statement “kfree(mdev);” be executed before “smsusb_term_device(intf);”
in one if branch?

Regards,
Markus


A patch slipped through the cracks?

2017-09-20 Thread Lubomir Rintel
Hi,

we're trying to get this reasonably trivial patch [1] applied for more
than a year and four attempts now. (I'm not including it in this
message so that this message won't be ignored for the same reason the
submissions were, whatever they are.)

[1] https://patchwork.linuxtv.org/patch/40862/

I have no idea what went wrong. There was a suspicion (somewhat
confirmed by the initial patch submitter) that spam filtering could
have dropped the first message. Since then the patch did make it to the
list numerous times and was picked up by patchwork.

The patchwork's idea about the patch being "Superseded" is wrong -- I
have no idea why. But someone *please* look into this and apply the
patch.

Thank you
Lubo


[PATCH 5/5] [media] s2255drv: Delete an unnecessary return statement in five functions

2017-09-20 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 20 Sep 2017 17:50:36 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: void function return statements are not generally useful

Thus remove such a statement in the affected functions.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/s2255/s2255drv.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c 
b/drivers/media/usb/s2255/s2255drv.c
index 5a5d5ae833ff..2f0e0fafc4e2 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -471,6 +471,5 @@ static void planar422p_to_yuv_packed(const unsigned char 
*in,
out[i + 3] = (fmt == V4L2_PIX_FMT_YUYV) ? *pCb++ : *pY++;
}
-   return;
 }
 
 static void s2255_reset_dsppower(struct s2255_dev *dev)
@@ -482,5 +481,4 @@ static void s2255_reset_dsppower(struct s2255_dev *dev)
s2255_vendor_req(dev, 0x10, 0x, 0x, NULL, 0, 1);
-   return;
 }
 
 /* kickstarts the firmware loading. from probe
@@ -1586,6 +1584,5 @@ static void s2255_video_device_release(struct 
video_device *vdev)
if (atomic_dec_and_test(>num_channels))
s2255_destroy(dev);
-   return;
 }
 
 static const struct video_device template = {
@@ -1890,5 +1887,4 @@ static void s2255_read_video_callback(struct s2255_dev 
*dev,
dprintk(dev, 50, "callback read video done\n");
-   return;
 }
 
 static long s2255_vendor_req(struct s2255_dev *dev, unsigned char Request,
@@ -2205,5 +2201,4 @@ static void s2255_stop_readpipe(struct s2255_dev *dev)
dprintk(dev, 4, "%s", __func__);
-   return;
 }
 
 static void s2255_fwload_start(struct s2255_dev *dev, int reset)
-- 
2.14.1



[PATCH 4/5] [media] s2255drv: Use common error handling code in read_pipe_completion()

2017-09-20 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 20 Sep 2017 17:45:13 +0200

Add a jump target so that a bit of exception handling can be better
reused at the end of this function.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/s2255/s2255drv.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c 
b/drivers/media/usb/s2255/s2255drv.c
index 29bc73ad7d8a..5a5d5ae833ff 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -2065,11 +2065,9 @@ static void read_pipe_completion(struct urb *purb)
pipe_info = purb->context;
-   if (!pipe_info) {
-   dev_err(>dev->dev, "no context!\n");
-   return;
-   }
+   if (!pipe_info)
+   goto report_failure;
+
dev = pipe_info->dev;
-   if (!dev) {
-   dev_err(>dev->dev, "no context!\n");
-   return;
-   }
+   if (!dev)
+   goto report_failure;
+
status = purb->status;
@@ -2107,6 +2105,9 @@ static void read_pipe_completion(struct urb *purb)
dprintk(dev, 2, "%s :complete state 0\n", __func__);
}
return;
+
+report_failure:
+   dev_err(>dev->dev, "no context!\n");
 }
 
 static int s2255_start_readpipe(struct s2255_dev *dev)
-- 
2.14.1



Re: [PATCH] [media] staging: atomisp: use clock framework for camera clocks

2017-09-20 Thread Pierre-Louis Bossart



On 09/20/2017 04:12 AM, Andy Shevchenko wrote:

On Tue, 2017-09-19 at 15:45 -0500, Pierre-Louis Bossart wrote:

The Atom ISP driver initializes and configures PMC clocks which are
already handled by the clock framework.

Remove all legacy vlv2_platform_clock stuff and move to the clk API to
avoid conflicts, e.g. with audio machine drivers enabling the MCLK for
external codecs


I think it might have a Fixes: tag as well (though I dunno which commit
could be considered as anchor).
The initial integration of the atomisp driver already had this problem, 
i'll add a reference to

'a49d25364dfb9 ("staging/atomisp: Add support for the Intel IPU v2")'


(I doubt Git is so clever to remove files based on information out of
the diff, can you check it and if needed to resend without -D implied?)
Gee, I thought -C -M -D were the standard options to checkpatch, never 
realized it'd prevent patches from applying. Thanks for the tip.




Other than that - nice clean up!

Reviewed-by: Andy Shevchenko 

I'll add your Reviewed-by in the v2. Thanks for the review.




Tested-by: Carlo Caione 
Signed-off-by: Pierre-Louis Bossart 
---
  drivers/staging/media/atomisp/Kconfig  |   1 +
  drivers/staging/media/atomisp/platform/Makefile|   1 -
  .../staging/media/atomisp/platform/clock/Makefile  |   6 -
  .../platform/clock/platform_vlv2_plat_clk.c|  40 
  .../platform/clock/platform_vlv2_plat_clk.h|  27 ---
  .../media/atomisp/platform/clock/vlv2_plat_clock.c | 247 
-
  .../platform/intel-mid/atomisp_gmin_platform.c |  63 +-
  7 files changed, 52 insertions(+), 333 deletions(-)
  delete mode 100644
drivers/staging/media/atomisp/platform/clock/Makefile
  delete mode 100644
drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.c
  delete mode 100644
drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.h
  delete mode 100644
drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c

diff --git a/drivers/staging/media/atomisp/Kconfig
b/drivers/staging/media/atomisp/Kconfig
index 8eb13c3ba29c..7cdebea35ccf 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,7 @@
  menuconfig INTEL_ATOMISP
  bool "Enable support to Intel MIPI camera drivers"
  depends on X86 && EFI && MEDIA_CONTROLLER && PCI && ACPI
+   select COMMON_CLK
  help
Enable support for the Intel ISP2 camera interfaces and
MIPI
sensor drivers.
diff --git a/drivers/staging/media/atomisp/platform/Makefile
b/drivers/staging/media/atomisp/platform/Makefile
index df157630bda9..0e3b7e1c81c6 100644
--- a/drivers/staging/media/atomisp/platform/Makefile
+++ b/drivers/staging/media/atomisp/platform/Makefile
@@ -2,5 +2,4 @@
  # Makefile for camera drivers.
  #
  
-obj-$(CONFIG_INTEL_ATOMISP) += clock/

  obj-$(CONFIG_INTEL_ATOMISP) += intel-mid/
diff --git a/drivers/staging/media/atomisp/platform/clock/Makefile
b/drivers/staging/media/atomisp/platform/clock/Makefile
deleted file mode 100644
index 82fbe8b6968a..
diff --git
a/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.
c
b/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.
c
deleted file mode 100644
index 0aae9b0283bb..
diff --git
a/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.
h
b/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.
h
deleted file mode 100644
index b730ab0e8223..
diff --git
a/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
b/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
deleted file mode 100644
index f96789a31819..
diff --git a/drivers/staging/media/atomisp/platform/intel-
mid/atomisp_gmin_platform.c
b/drivers/staging/media/atomisp/platform/intel-
mid/atomisp_gmin_platform.c
index edaae93af8f9..17b4cfae5abf 100644
--- a/drivers/staging/media/atomisp/platform/intel-
mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-
mid/atomisp_gmin_platform.c
@@ -4,10 +4,10 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
-#include "../../include/linux/vlv2_plat_clock.h"
  #include 
  #include 
  #include 
@@ -17,11 +17,7 @@
  
  #define MAX_SUBDEVS 8
  
-/* Should be defined in vlv2_plat_clock API, isn't: */

-#define VLV2_CLK_PLL_19P2MHZ 1
-#define VLV2_CLK_XTAL_19P2MHZ 0
-#define VLV2_CLK_ON  1
-#define VLV2_CLK_OFF 2
+#define VLV2_CLK_PLL_19P2MHZ 1 /* XTAL on CHT */
  #define ELDO1_SEL_REG 0x19
  #define ELDO1_1P8V0x16
  #define ELDO1_CTRL_SHIFT 0x00
@@ -33,6 +29,7 @@ struct gmin_subdev {
struct v4l2_subdev *subdev;
int clock_num;
int clock_src;
+   struct clk *pmc_clk;
struct gpio_desc *gpio0;
struct gpio_desc *gpio1;
struct regulator *v1p8_reg;
@@ 

[PATCH 3/5] [media] s2255drv: Improve two size determinations in s2255_probe()

2017-09-20 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 20 Sep 2017 16:56:20 +0200

Replace the specification of data structures by variable references
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/s2255/s2255drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c 
b/drivers/media/usb/s2255/s2255drv.c
index aee83bf6fa94..29bc73ad7d8a 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -2237,4 +2237,4 @@ static int s2255_probe(struct usb_interface *interface,
/* allocate memory for our device state and initialize it to zero */
-   dev = kzalloc(sizeof(struct s2255_dev), GFP_KERNEL);
+   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
if (!dev)
return -ENOMEM;
@@ -2247,4 +2247,4 @@ static int s2255_probe(struct usb_interface *interface,
dev->pid = id->idProduct;
-   dev->fw_data = kzalloc(sizeof(struct s2255_fw), GFP_KERNEL);
+   dev->fw_data = kzalloc(sizeof(*dev->fw_data), GFP_KERNEL);
if (!dev->fw_data)
goto errorFWDATA1;
-- 
2.14.1



[PATCH 2/5] [media] s2255drv: Adjust 13 checks for null pointers

2017-09-20 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 20 Sep 2017 16:46:19 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written !…

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/s2255/s2255drv.c | 29 +
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c 
b/drivers/media/usb/s2255/s2255drv.c
index 29285e8cd742..aee83bf6fa94 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -516,6 +516,6 @@ static void s2255_fwchunk_complete(struct urb *urb)
wake_up(>wait_fw);
return;
}
-   if (data->fw_urb == NULL) {
+   if (!data->fw_urb) {
s2255_dev_err(>dev, "disconnected\n");
atomic_set(>fw_state, S2255_FW_FAILED);
@@ -680,5 +680,5 @@ static int buffer_prepare(struct vb2_buffer *vb)
dprintk(vc->dev, 4, "%s\n", __func__);
-   if (vc->fmt == NULL)
+   if (!vc->fmt)
return -EINVAL;
 
if ((w < norm_minw(vc)) ||
@@ -785,6 +785,5 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void 
*priv,
fmt = format_by_fourcc(f->fmt.pix.pixelformat);
-
-   if (fmt == NULL)
+   if (!fmt)
return -EINVAL;
 
field = f->fmt.pix.field;
@@ -853,6 +852,5 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void 
*priv,
fmt = format_by_fourcc(f->fmt.pix.pixelformat);
-
-   if (fmt == NULL)
+   if (!fmt)
return -EINVAL;
 
if (vb2_is_busy(q)) {
@@ -936,6 +934,6 @@ static u32 get_transfer_size(struct s2255_mode *mode)
unsigned int mask_mult;
 
-   if (mode == NULL)
+   if (!mode)
return 0;
 
if (mode->format == FORMAT_NTSC) {
@@ -1390,4 +1388,4 @@ static int vidioc_enum_framesizes(struct file *file, void 
*priv,
fmt = format_by_fourcc(fe->pixel_format);
-   if (fmt == NULL)
+   if (!fmt)
return -EINVAL;
fe->type = V4L2_FRMSIZE_TYPE_DISCRETE;
@@ -1412,5 +1410,5 @@ static int vidioc_enum_frameintervals(struct file *file, 
void *priv,
fmt = format_by_fourcc(fe->pixel_format);
-   if (fmt == NULL)
+   if (!fmt)
return -EINVAL;
 
sizes = is_ntsc ? ntsc_sizes : pal_sizes;
@@ -1834,6 +1832,5 @@ static int save_frame(struct s2255_dev *dev, struct 
s2255_pipeinfo *pipe_info)
psrc = (u8 *)pipe_info->transfer_buffer + offset;
 
-
-   if (frm->lpvbits == NULL) {
+   if (!frm->lpvbits) {
dprintk(dev, 1, "s2255 frame buffer == NULL.%p %p %d %d",
frm, dev, dev->cc, idx);
@@ -1965,6 +1962,6 @@ static int s2255_create_sys_buffers(struct s2255_vc *vc)
vc->buffer.frame[i].lpvbits = vmalloc(reqsize);
vc->buffer.frame[i].size = reqsize;
-   if (vc->buffer.frame[i].lpvbits == NULL) {
+   if (!vc->buffer.frame[i].lpvbits) {
pr_info("out of memory.  using less frames\n");
vc->buffer.dwFrames = i;
break;
@@ -2007,6 +2004,6 @@ static int s2255_board_init(struct s2255_dev *dev)
pipe->transfer_buffer = kzalloc(pipe->max_transfer_size,
GFP_KERNEL);
-   if (pipe->transfer_buffer == NULL) {
+   if (!pipe->transfer_buffer) {
dprintk(dev, 1, "out of memory!\n");
return -ENOMEM;
}
@@ -2068,8 +2065,8 @@ static void read_pipe_completion(struct urb *purb)
pipe_info = purb->context;
-   if (pipe_info == NULL) {
+   if (!pipe_info) {
dev_err(>dev->dev, "no context!\n");
return;
}
dev = pipe_info->dev;
-   if (dev == NULL) {
+   if (!dev) {
dev_err(>dev->dev, "no context!\n");
@@ -2257,5 +2254,5 @@ static int s2255_probe(struct usb_interface *interface,
dev->udev = usb_get_dev(interface_to_usbdev(interface));
-   if (dev->udev == NULL) {
+   if (!dev->udev) {
dev_err(>dev, "null usb device\n");
retval = -ENODEV;
goto errorUDEV;
-- 
2.14.1



[PATCH 1/5] [media] s2255drv: Delete three error messages for a failed memory allocation in s2255_probe()

2017-09-20 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 20 Sep 2017 16:30:13 +0200

Omit extra messages for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/s2255/s2255drv.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c 
b/drivers/media/usb/s2255/s2255drv.c
index b2f239c4ba42..29285e8cd742 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -2242,13 +2242,9 @@ static int s2255_probe(struct usb_interface *interface,
-   if (dev == NULL) {
-   s2255_dev_err(>dev, "out of memory\n");
+   if (!dev)
return -ENOMEM;
-   }
 
dev->cmdbuf = kzalloc(S2255_CMDBUF_SIZE, GFP_KERNEL);
-   if (dev->cmdbuf == NULL) {
-   s2255_dev_err(>dev, "out of memory\n");
+   if (!dev->cmdbuf)
goto errorFWDATA1;
-   }
 
atomic_set(>num_channels, 0);
dev->pid = id->idProduct;
@@ -2303,7 +2299,6 @@ static int s2255_probe(struct usb_interface *interface,
-   if (!dev->fw_data->pfw_data) {
-   dev_err(>dev, "out of memory!\n");
+   if (!dev->fw_data->pfw_data)
goto errorFWDATA2;
-   }
+
/* load the first chunk */
if (request_firmware(>fw_data->fw,
 FIRMWARE_FILE_NAME, >udev->dev)) {
-- 
2.14.1



[PATCH 0/5] [media] s2255drv: Fine-tuning for some function implementations

2017-09-20 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 20 Sep 2017 18:18:28 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (5):
  Delete three error messages for a failed memory allocation in s2255_probe()
  Adjust 13 checks for null pointers
  Improve two size determinations in s2255_probe()
  Use common error handling code in read_pipe_completion()
  Delete an unnecessary return statement in five functions

 drivers/media/usb/s2255/s2255drv.c | 64 --
 1 file changed, 26 insertions(+), 38 deletions(-)

-- 
2.14.1



[PATCH] scripts: kernel-doc: fix nexted handling

2017-09-20 Thread Mauro Carvalho Chehab
At DVB, we have, at some structs, things like (see
struct dvb_demux_feed, at dvb_demux.h):

union {
struct dmx_ts_feed ts;
struct dmx_section_feed sec;
} feed;

union {
dmx_ts_cb ts;
dmx_section_cb sec;
} cb;

Fix the nested parser to avoid it to eat the first union.

Signed-off-by: Mauro Carvalho Chehab 
---

Jon,

While documenting some DVB demux  headers, I noticed the above bug.

 scripts/kernel-doc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 9d3eafea58f0..15f934a23d1d 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -2173,7 +2173,7 @@ sub dump_struct($$) {
my $members = $3;
 
# ignore embedded structs or unions
-   $members =~ s/({.*})//g;
+   $members =~ s/({[^\}]*})//g;
$nested = $1;
 
# ignore members marked private:
-- 
2.13.5




[PATCH v3 3/4] [media] bcm2835-unicam: Driver for CCP2/CSI2 camera interface

2017-09-20 Thread Dave Stevenson
Add driver for the Unicam camera receiver block on
BCM283x processors.

Signed-off-by: Dave Stevenson 
---

Changes from v2.
- Don't store the irq value as it isn't used outside the probe.
- Change PIX_FMT_ALL_ defines to avoid potential clashes with 4CCs.
- Clock now called "lp" and not "lp_clock".
- Minor description changes.

 drivers/media/platform/Kconfig   |1 +
 drivers/media/platform/Makefile  |1 +
 drivers/media/platform/bcm2835/Kconfig   |   14 +
 drivers/media/platform/bcm2835/Makefile  |3 +
 drivers/media/platform/bcm2835/bcm2835-unicam.c  | 2192 ++
 drivers/media/platform/bcm2835/vc4-regs-unicam.h |  264 +++
 6 files changed, 2475 insertions(+)
 create mode 100644 drivers/media/platform/bcm2835/Kconfig
 create mode 100644 drivers/media/platform/bcm2835/Makefile
 create mode 100644 drivers/media/platform/bcm2835/bcm2835-unicam.c
 create mode 100644 drivers/media/platform/bcm2835/vc4-regs-unicam.h

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 7e7cc49..1e5f004 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -150,6 +150,7 @@ source "drivers/media/platform/am437x/Kconfig"
 source "drivers/media/platform/xilinx/Kconfig"
 source "drivers/media/platform/rcar-vin/Kconfig"
 source "drivers/media/platform/atmel/Kconfig"
+source "drivers/media/platform/bcm2835/Kconfig"
 
 config VIDEO_TI_CAL
tristate "TI CAL (Camera Adaptation Layer) driver"
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index c1ef946..045de3f 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -90,3 +90,4 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS)+= 
qcom/camss-8x16/
 obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/
 
 obj-y  += meson/
+obj-y  += bcm2835/
diff --git a/drivers/media/platform/bcm2835/Kconfig 
b/drivers/media/platform/bcm2835/Kconfig
new file mode 100644
index 000..6a74842
--- /dev/null
+++ b/drivers/media/platform/bcm2835/Kconfig
@@ -0,0 +1,14 @@
+# Broadcom VideoCore4 V4L2 camera support
+
+config VIDEO_BCM2835_UNICAM
+   tristate "Broadcom BCM2835 Unicam video capture driver"
+   depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   depends on ARCH_BCM2835 || COMPILE_TEST
+   select VIDEOBUF2_DMA_CONTIG
+   select V4L2_FWNODE
+   ---help---
+ Say Y here to enable V4L2 subdevice for CSI2 receiver.
+ This is a V4L2 subdevice that interfaces directly to the VC4 
peripheral.
+
+  To compile this driver as a module, choose M here. The module
+  will be called bcm2835-unicam.
diff --git a/drivers/media/platform/bcm2835/Makefile 
b/drivers/media/platform/bcm2835/Makefile
new file mode 100644
index 000..a98aba0
--- /dev/null
+++ b/drivers/media/platform/bcm2835/Makefile
@@ -0,0 +1,3 @@
+# Makefile for BCM2835 Unicam driver
+
+obj-$(CONFIG_VIDEO_BCM2835_UNICAM) += bcm2835-unicam.o
diff --git a/drivers/media/platform/bcm2835/bcm2835-unicam.c 
b/drivers/media/platform/bcm2835/bcm2835-unicam.c
new file mode 100644
index 000..93831fb
--- /dev/null
+++ b/drivers/media/platform/bcm2835/bcm2835-unicam.c
@@ -0,0 +1,2192 @@
+/*
+ * BCM2835 Unicam capture Driver
+ *
+ * Copyright (C) 2017 - Raspberry Pi (Trading) Ltd.
+ *
+ * Dave Stevenson 
+ *
+ * Based on TI am437x driver by Benoit Parrot and Lad, Prabhakar and
+ * TI CAL camera interface driver by Benoit Parrot.
+ *
+ *
+ * There are two camera drivers in the kernel for BCM283x - this one
+ * and bcm2835-camera (currently in staging).
+ *
+ * This driver directly controls the Unicam peripheral - there is no
+ * involvement with the VideoCore firmware. Unicam receives CSI-2 or
+ * CCP2 data and writes it into SDRAM. The only processing options are
+ * to repack Bayer data into an alternate format, and applying windowing
+ * (currently not implemented).
+ * It should be possible to connect it to any sensor with a
+ * suitable output interface and V4L2 subdevice driver.
+ *
+ * bcm2835-camera uses the VideoCore firmware to control the sensor,
+ * Unicam, ISP, and all tuner control loops. Fully processed frames are
+ * delivered to the driver by the firmware. It only has sensor drivers
+ * for Omnivision OV5647, and Sony IMX219 sensors.
+ *
+ * The two drivers are mutually exclusive for the same Unicam instance.
+ * The VideoCore firmware checks the device tree configuration during boot.
+ * If it finds device tree nodes called csi0 or csi1 it will block the
+ * firmware from accessing the peripheral, and bcm2835-camera will
+ * not be able to stream data.
+ *
+ *
+ * This program is free software; you may redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the 

[PATCH v3 4/4] MAINTAINERS: Add entry for BCM2835 camera driver

2017-09-20 Thread Dave Stevenson
Signed-off-by: Dave Stevenson 
---

No changes from v2 to v3

 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index eb930eb..b47ddaa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2713,6 +2713,13 @@ S:   Maintained
 N: bcm2835
 F: drivers/staging/vc04_services
 
+BROADCOM BCM2835 CAMERA DRIVER
+M: Dave Stevenson 
+L: linux-media@vger.kernel.org
+S: Maintained
+F: drivers/media/platform/bcm2835/
+F: Documentation/devicetree/bindings/media/bcm2835-unicam.txt
+
 BROADCOM BCM47XX MIPS ARCHITECTURE
 M: Hauke Mehrtens 
 M: Rafał Miłecki 
-- 
2.7.4



[PATCH v3 2/4] [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver

2017-09-20 Thread Dave Stevenson
Document the DT bindings for the CSI2/CCP2 receiver peripheral
(known as Unicam) on BCM283x SoCs.

Signed-off-by: Dave Stevenson 
---

Changes since v2
- Removed all references to Linux drivers.
- Reworded section about disabling the firmware driver.
- Renamed clock from "lp_clock" to "lp" in description and example.
- Referred to video-interfaces.txt and stated requirements on remote-endpoint
  and data-lanes.
- Corrected typo in example from csi to csi1.
- Removed unnecessary #address-cells and #size-cells in example.
- Removed setting of status from the example.

 .../devicetree/bindings/media/bcm2835-unicam.txt   | 85 ++
 1 file changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/bcm2835-unicam.txt

diff --git a/Documentation/devicetree/bindings/media/bcm2835-unicam.txt 
b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
new file mode 100644
index 000..7714fb3
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
@@ -0,0 +1,85 @@
+Broadcom BCM283x Camera Interface (Unicam)
+--
+
+The Unicam block on BCM283x SoCs is the receiver for either
+CSI-2 or CCP2 data from image sensors or similar devices.
+
+The main platform using this SoC is the Raspberry Pi family of boards.
+On the Pi the VideoCore firmware can also control this hardware block,
+and driving it from two different processors will cause issues.
+To avoid this, the firmware checks the device tree configuration
+during boot. If it finds device tree nodes called csi0 or csi1 then
+it will stop the firmware accessing the block, and it can then
+safely be used via the device tree binding.
+
+Required properties:
+===
+- compatible   : must be "brcm,bcm2835-unicam".
+- reg  : physical base address and length of the register sets for the
+ device.
+- interrupts   : should contain the IRQ line for this Unicam instance.
+- clocks   : list of clock specifiers, corresponding to entries in
+ clock-names property.
+- clock-names  : must contain an "lp" entry, matching entries in the
+ clocks property.
+
+Unicam supports a single port node. It should contain one 'port' child node
+with child 'endpoint' node. Please refer to the bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Within the endpoint node the "remote-endpoint" and "data-lanes" properties
+are mandatory.
+Data lane reordering is not supported so the data lanes must be in order,
+starting at 1. The number of data lanes should represent the number of
+usable lanes for the hardware block. That may be limited by either the SoC or
+how the platform presents the interface, and the lower value must be used.
+
+Lane reordering is not supported on the clock lane either, so the optional
+property "clock-lane" will implicitly be <0>.
+Similarly lane inversion is not supported, therefore "lane-polarities" will
+implicitly be <0 0 0 0 0>.
+Neither of these values will be checked.
+
+Example:
+   csi1: csi1@7e801000 {
+   compatible = "brcm,bcm2835-unicam";
+   reg = <0x7e801000 0x800>,
+ <0x7e802004 0x4>;
+   interrupts = <2 7>;
+   clocks = < BCM2835_CLOCK_CAM1>;
+   clock-names = "lp";
+
+   port {
+   csi1_ep: endpoint {
+   remote-endpoint = <_0>;
+   data-lanes = <1 2>;
+   };
+   };
+   };
+
+   i2c0: i2c@7e205000 {
+   tc358743: csi-hdmi-bridge@0f {
+   compatible = "toshiba,tc358743";
+   reg = <0x0f>;
+
+   clocks = <_clk>;
+   clock-names = "refclk";
+
+   tc358743_clk: bridge-clk {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <2700>;
+   };
+
+   port {
+   tc358743_0: endpoint {
+   remote-endpoint = <_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1 2>;
+   clock-noncontinuous;
+   link-frequencies =
+   /bits/ 64 <29700>;
+   };
+   };
+   };
+   };
-- 
2.7.4



[PATCH v3 0/4] BCM283x Camera Receiver driver

2017-09-20 Thread Dave Stevenson
Hi All.

v3 of this patch set.
This addresses the DT issues raised by Rob, and the things raised by
Eric on the driver. More complete change details between v2 and v3
are in the individual patches.

Thanks
  Dave

Dave Stevenson (4):
  [media] v4l2-common: Add helper function for fourcc to string
  [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver
  [media] bcm2835-unicam: Driver for CCP2/CSI2 camera interface
  MAINTAINERS: Add entry for BCM2835 camera driver

 .../devicetree/bindings/media/bcm2835-unicam.txt   |   85 +
 MAINTAINERS|7 +
 drivers/media/platform/Kconfig |1 +
 drivers/media/platform/Makefile|1 +
 drivers/media/platform/bcm2835/Kconfig |   14 +
 drivers/media/platform/bcm2835/Makefile|3 +
 drivers/media/platform/bcm2835/bcm2835-unicam.c| 2192 
 drivers/media/platform/bcm2835/vc4-regs-unicam.h   |  264 +++
 drivers/media/v4l2-core/v4l2-common.c  |   18 +
 include/media/v4l2-common.h|3 +
 10 files changed, 2588 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/bcm2835-unicam.txt
 create mode 100644 drivers/media/platform/bcm2835/Kconfig
 create mode 100644 drivers/media/platform/bcm2835/Makefile
 create mode 100644 drivers/media/platform/bcm2835/bcm2835-unicam.c
 create mode 100644 drivers/media/platform/bcm2835/vc4-regs-unicam.h

-- 
2.7.4



[PATCH v3 1/4] [media] v4l2-common: Add helper function for fourcc to string

2017-09-20 Thread Dave Stevenson
New helper function char *v4l2_fourcc2s(u32 fourcc, char *buf)
that converts a fourcc into a nice printable version.

Signed-off-by: Dave Stevenson 
---

No changes from v2 to v3

 drivers/media/v4l2-core/v4l2-common.c | 18 ++
 include/media/v4l2-common.h   |  3 +++
 2 files changed, 21 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c 
b/drivers/media/v4l2-core/v4l2-common.c
index a5ea1f5..0219895 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -405,3 +405,21 @@ void v4l2_get_timestamp(struct timeval *tv)
tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
 }
 EXPORT_SYMBOL_GPL(v4l2_get_timestamp);
+
+char *v4l2_fourcc2s(u32 fourcc, char *buf)
+{
+   buf[0] = fourcc & 0x7f;
+   buf[1] = (fourcc >> 8) & 0x7f;
+   buf[2] = (fourcc >> 16) & 0x7f;
+   buf[3] = (fourcc >> 24) & 0x7f;
+   if (fourcc & (1 << 31)) {
+   buf[4] = '-';
+   buf[5] = 'B';
+   buf[6] = 'E';
+   buf[7] = '\0';
+   } else {
+   buf[4] = '\0';
+   }
+   return buf;
+}
+EXPORT_SYMBOL_GPL(v4l2_fourcc2s);
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index aac8b7b..5b0fff9 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -264,4 +264,7 @@ const struct v4l2_frmsize_discrete 
*v4l2_find_nearest_format(
 
 void v4l2_get_timestamp(struct timeval *tv);
 
+#define V4L2_FOURCC_MAX_SIZE 8
+char *v4l2_fourcc2s(u32 fourcc, char *buf);
+
 #endif /* V4L2_COMMON_H_ */
-- 
2.7.4



Re: [PATCH v13 11/25] v4l: async: Introduce helpers for calling async ops callbacks

2017-09-20 Thread Sakari Ailus
Hi Laurent,

On Tue, Sep 19, 2017 at 07:27:17PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Tuesday, 19 September 2017 17:50:49 EEST Sakari Ailus wrote:
> > On Tue, Sep 19, 2017 at 03:43:48PM +0300, Laurent Pinchart wrote:
> > > On Tuesday, 19 September 2017 15:13:11 EEST Sakari Ailus wrote:
> > >> On Tue, Sep 19, 2017 at 03:01:14PM +0300, Laurent Pinchart wrote:
> > >>> On Friday, 15 September 2017 17:17:10 EEST Sakari Ailus wrote:
> >  Add three helper functions to call async operations callbacks.
> >  Besides simplifying callbacks, this allows async notifiers to have no
> >  ops set, i.e. it can be left NULL.
> >  
> >  Signed-off-by: Sakari Ailus 
> >  Acked-by: Hans Verkuil 
> >  ---
> >  
> >   drivers/media/v4l2-core/v4l2-async.c | 49 ++
> >   include/media/v4l2-async.h   |  1 +
> >   2 files changed, 37 insertions(+), 13 deletions(-)
> >  
> >  diff --git a/drivers/media/v4l2-core/v4l2-async.c
> >  b/drivers/media/v4l2-core/v4l2-async.c index
> >  7b2125b3d62f..c35d04b9122f
> >  100644
> >  --- a/drivers/media/v4l2-core/v4l2-async.c
> >  +++ b/drivers/media/v4l2-core/v4l2-async.c
> >  @@ -25,6 +25,34 @@
> >  
> >   #include 
> >   #include 
> >  
> >  +static int v4l2_async_notifier_call_bound(struct v4l2_async_notifier
> >  *n,
> >  +struct v4l2_subdev *subdev,
> >  +struct v4l2_async_subdev *asd)
> >  +{
> >  +  if (!n->ops || !n->ops->bound)
> >  +  return 0;
> >  +
> >  +  return n->ops->bound(n, subdev, asd);
> >  +}
> >  +
> >  +static void v4l2_async_notifier_call_unbind(struct
> >  v4l2_async_notifier
> >  *n,
> >  +  struct v4l2_subdev *subdev,
> >  +  struct v4l2_async_subdev 
> >  *asd)
> >  +{
> >  +  if (!n->ops || !n->ops->unbind)
> >  +  return;
> >  +
> >  +  n->ops->unbind(n, subdev, asd);
> >  +}
> >  +
> >  +static int v4l2_async_notifier_call_complete(struct
> >  v4l2_async_notifier
> >  *n)
> >  +{
> >  +  if (!n->ops || !n->ops->complete)
> >  +  return 0;
> >  +
> >  +  return n->ops->complete(n);
> >  +}
> >  +
> > >>> 
> > >>> Wouldn't it be enough to add a single v4l2_async_notifier_call() macro
> > >>> ?
> > >>> 
> > >>> #define v4l2_async_notifier_call(n, op, args...) \
> > >>> 
> > >>> ((n)->ops && (n)->ops->op ? (n)->ops->op(n, ##args) : 0)
> > >> 
> > >> I actually had that in an earlier version but I changed it based on
> > >> review comments from Hans. A single macro isn't enough: some functions
> > >> have int return type. I think the way it is now is nicer.
> > > 
> > > What bothers me there is the overhead of a function call.
> > 
> > Overhead... of a function call?
> > 
> > Do you really mean what you're saying? :-) The functions will be called a
> > relatively small number of times during module loading / device probing.
> 
> That's why I haven't said it's a big deal :-) There's of course no need to 
> optimize that if the tradeoff is large, but if all operations had the same 
> return type a macro could have been useful (although in this very specific 
> case I'm more concerned about code size than about CPU cycles).

Code size in the async framework? Generally calling a function doesn't take
a lot of code, and the kernel is, well, full of function calls. I'm frankly
more concerned about the number of lines of code to maintain and
readability of that code.

> 
> > > By the way, what's the use case for ops being NULL ?
> > 
> > If a driver has no need for any of the callbacks, there's no benefit from
> > having to set ops struct either. This applies to devices that are
> > associated to the sensor, for instance.
> 
> So in that case the subdev notifier is only registered to delay the 
> complete() 
> callback until the flash and lens controllers are available, with the sensor 
> itself having no need to access the flash and lens controllers ?

Essentially yes. In the future we'll need to make use of the association
information to tell which devices are related but this shouldn't be a job
for individual drivers.

-- 
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH v13 06/25] omap3isp: Use generic parser for parsing fwnode endpoints

2017-09-20 Thread Sakari Ailus
Hi Laurent,

On Tue, Sep 19, 2017 at 03:46:18PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Tuesday, 19 September 2017 15:43:26 EEST Sakari Ailus wrote:
> > On Tue, Sep 19, 2017 at 02:40:29PM +0300, Laurent Pinchart wrote:
> > > > @@ -2256,7 +2210,9 @@ static int isp_probe(struct platform_device *pdev)
> > > > 
> > > > if (ret)
> > > > 
> > > > return ret;
> > > > 
> > > > -   ret = isp_fwnodes_parse(>dev, >notifier);
> > > > +   ret = v4l2_async_notifier_parse_fwnode_endpoints(
> > > > +   >dev, >notifier, sizeof(struct 
> > > > isp_async_subdev),
> > > > +   isp_fwnode_parse);
> > > > 
> > > > if (ret < 0)
> > > 
> > > The documentation in patch 05/25 states that v4l2_async_notifier_release()
> > > should be called even if v4l2_async_notifier_parse_fwnode_endpoints()
> > > fails. I don't think that's needed here, so you might want to update the
> > > documentation (and possibly the implementation of the function).
> > 
> > It is. If parsing fails, async sub-devices may have been already set up.
> > This happens e.g. when the parsing fails after the first one has been
> > successfully set up already.
> 
> But for v4l2_async_notifier_parse_fwnode_endpoints() we could clean up 
> internally when an error occurs. Otherwise you need to call 
> v4l2_async_notifier_release() here.

If a driver uses the variant that parses the endpoints by port, how should
that function behave? Release just as many async sub-devices it set up, and
leave the rest for the driver to handle?

The reason I left it as such as to make the responsibility clear: it
belongs to the driver.

I can change that if you really think it makes a difference for better. I'm
just not that certain about it.

-- 
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH] [media] Siano: Use common error handling code in smsusb_init_device()

2017-09-20 Thread Dan Carpenter
On Wed, Sep 20, 2017 at 02:40:28PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Wed, 20 Sep 2017 14:30:55 +0200
> 
> Add a jump target so that a bit of exception handling can be better
> reused at the end of this function.
> 
> This refactoring might fix also an error situation where the
> function "kfree" was not called after a software failure
> was noticed in two cases.
> 

No.  It doesn't fix a leak, it introduces a double free.  If
smscore_register_device() succeeds then mdev is freed when we call
smsusb_term_device(intf);  The call tree is:

smsusb_term_device()
 -> smscore_unregister_device()
-> smscore_notify_clients()
   -> smsdvb_onremove()
  -> smsdvb_unregister_client()
 -> smsdvb_media_device_unregister()
-> kfree(coredev->media_dev);

regards,
dan carpenter



Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.

2017-09-20 Thread Hans Verkuil
On 09/20/17 14:50, Sakari Ailus wrote:
> Hi Hans and others,
> 
> On Wed, Sep 20, 2017 at 01:24:02PM +0200, Hans Verkuil wrote:
>> On 09/20/17 13:00, Dave Stevenson wrote:
>>> On 20 September 2017 at 11:23, Philipp Zabel  wrote:
 Hi,

 On Wed, 2017-09-20 at 10:14 +0100, Dave Stevenson wrote:
> Hi Mauro & Philipp
>
> On 19 September 2017 at 17:49, Mauro Carvalho Chehab
>  wrote:
>> Em Tue, 19 Sep 2017 17:24:45 +0200
>> Philipp Zabel  escreveu:
>>
>>> Hi Dave,
>>>
>>> On Tue, 2017-09-19 at 14:08 +0100, Dave Stevenson wrote:
 The existing fixed value of 16 worked for UYVY 720P60 over
 2 lanes at 594MHz, or UYVY 1080P60 over 4 lanes. (RGB888
 1080P60 needs 6 lanes at 594MHz).
 It doesn't allow for lower resolutions to work as the FIFO
 underflows.

 Using a value of 300 works for all resolutions down to VGA60,
 and the increase in frame delay is <4usecs for 1080P60 UYVY
 (2.55usecs for RGB888).

 Signed-off-by: Dave Stevenson 
>>>
>>> Can we increase this to 320? This would also allow
>>> 720p60 at 594 Mbps / 4 lanes, according to the xls.
>
> Unless I've missed something then the driver would currently request
> only 2 lanes for 720p60 UYVY, and that works with the existing FIFO
> setting of 16. Likewise 720p60 RGB888 requests 3 lanes and also works
> on a FIFO setting of 16.
> How/why were you thinking we need to run all four lanes for 720p60
> without other significant driver mods around lane config?

 The driver currently silently changes the number of active lanes
 depending on required data rate, with no way to communicate it to the
 receiver.
>>>
>>> It is communicated over the subdevice API - tc358743_g_mbus_config
>>> reports back the appropriate number of lanes to the receiver
>>> subdevice.
>>> A suitable v4l2_subdev_has_op(dev->sensor, video, g_mbus_config) call
>>> as you're starting streaming therefore gives you the correct
>>> information. That's what I've just done for the BCM283x Unicam
>>> driver[1], but admittedly I'm not using the media controller API which
>>> i.MX6 is.
>>
>> Shouldn't this information come from the device tree? The g_mbus_config
>> op is close to being deprecated or even removed. There are currently only
>> two obscure V4L2 bridge drivers that call it. It dates from pre-DT times
>> I rather not see it used in a new bridge driver.
>>
>> The problem is that contains data that belongs to the DT (hardware
>> capabilities). Things that can actually change dynamically should be
>> communicated via another op. We don't have that, so that should be created.
> 
> The DT tells how many lanes are connected in hardware, but up to now that's
> also been the number of lanes actually used.
> 
> The g_mbus_config() is there, and I'd like to replace that with the more
> generic frame descriptors, with CSI-2 virtual channel and data type
> support. They're however not quite ready yet nor I've recently worked on
> them.
> 
> I think using g_mbus_config() for the purpose right now is entirely
> acceptable, we can rework that later on when adding support for frame
> descriptors.
> 

I don't like it :-)

Currently g_mbus_config returns (and I quote from v4l2-mediabus.h): "How
many lanes the client can use". I.e. the capabilities of the HW.

If we are going to use this to communicate how many lines are currently
in use, then I would propose that we add a lane mask, i.e. something like
this:

/* Number of lanes in use, 0 == use all available lanes (default) */
#define V4L2_MBUS_CSI2_LANE_MASK(3 << 10)

And add comments along the lines that this is a temporary fix.

I would feel a lot happier (or a lot less unhappy) if we'd do it this way.
Rather than re-interpreting bits that are not quite what they should be.

I'd also add a comment that all other flags must be 0 if the device tree is
used. This to avoid mixing the two.

Regards,

Hans


Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.

2017-09-20 Thread Sakari Ailus
Hi Hans and others,

On Wed, Sep 20, 2017 at 01:24:02PM +0200, Hans Verkuil wrote:
> On 09/20/17 13:00, Dave Stevenson wrote:
> > On 20 September 2017 at 11:23, Philipp Zabel  wrote:
> >> Hi,
> >>
> >> On Wed, 2017-09-20 at 10:14 +0100, Dave Stevenson wrote:
> >>> Hi Mauro & Philipp
> >>>
> >>> On 19 September 2017 at 17:49, Mauro Carvalho Chehab
> >>>  wrote:
>  Em Tue, 19 Sep 2017 17:24:45 +0200
>  Philipp Zabel  escreveu:
> 
> > Hi Dave,
> >
> > On Tue, 2017-09-19 at 14:08 +0100, Dave Stevenson wrote:
> >> The existing fixed value of 16 worked for UYVY 720P60 over
> >> 2 lanes at 594MHz, or UYVY 1080P60 over 4 lanes. (RGB888
> >> 1080P60 needs 6 lanes at 594MHz).
> >> It doesn't allow for lower resolutions to work as the FIFO
> >> underflows.
> >>
> >> Using a value of 300 works for all resolutions down to VGA60,
> >> and the increase in frame delay is <4usecs for 1080P60 UYVY
> >> (2.55usecs for RGB888).
> >>
> >> Signed-off-by: Dave Stevenson 
> >
> > Can we increase this to 320? This would also allow
> > 720p60 at 594 Mbps / 4 lanes, according to the xls.
> >>>
> >>> Unless I've missed something then the driver would currently request
> >>> only 2 lanes for 720p60 UYVY, and that works with the existing FIFO
> >>> setting of 16. Likewise 720p60 RGB888 requests 3 lanes and also works
> >>> on a FIFO setting of 16.
> >>> How/why were you thinking we need to run all four lanes for 720p60
> >>> without other significant driver mods around lane config?
> >>
> >> The driver currently silently changes the number of active lanes
> >> depending on required data rate, with no way to communicate it to the
> >> receiver.
> > 
> > It is communicated over the subdevice API - tc358743_g_mbus_config
> > reports back the appropriate number of lanes to the receiver
> > subdevice.
> > A suitable v4l2_subdev_has_op(dev->sensor, video, g_mbus_config) call
> > as you're starting streaming therefore gives you the correct
> > information. That's what I've just done for the BCM283x Unicam
> > driver[1], but admittedly I'm not using the media controller API which
> > i.MX6 is.
> 
> Shouldn't this information come from the device tree? The g_mbus_config
> op is close to being deprecated or even removed. There are currently only
> two obscure V4L2 bridge drivers that call it. It dates from pre-DT times
> I rather not see it used in a new bridge driver.
> 
> The problem is that contains data that belongs to the DT (hardware
> capabilities). Things that can actually change dynamically should be
> communicated via another op. We don't have that, so that should be created.

The DT tells how many lanes are connected in hardware, but up to now that's
also been the number of lanes actually used.

The g_mbus_config() is there, and I'd like to replace that with the more
generic frame descriptors, with CSI-2 virtual channel and data type
support. They're however not quite ready yet nor I've recently worked on
them.

I think using g_mbus_config() for the purpose right now is entirely
acceptable, we can rework that later on when adding support for frame
descriptors.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi


[PATCH] [media] Siano: Use common error handling code in smsusb_init_device()

2017-09-20 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 20 Sep 2017 14:30:55 +0200

Add a jump target so that a bit of exception handling can be better
reused at the end of this function.

This refactoring might fix also an error situation where the
function "kfree" was not called after a software failure
was noticed in two cases.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/siano/smsusb.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c
index 8c1f926567ec..b8e7b05cf6d0 100644
--- a/drivers/media/usb/siano/smsusb.c
+++ b/drivers/media/usb/siano/smsusb.c
@@ -458,12 +458,10 @@ static int smsusb_init_device(struct usb_interface *intf, 
int board_id)
rc = smscore_register_device(, >coredev, mdev);
if (rc < 0) {
pr_err("smscore_register_device(...) failed, rc %d\n", rc);
-   smsusb_term_device(intf);
 #ifdef CONFIG_MEDIA_CONTROLLER_DVB
media_device_unregister(mdev);
 #endif
-   kfree(mdev);
-   return rc;
+   goto terminate_device;
}
 
smscore_set_board_id(dev->coredev, board_id);
@@ -480,8 +478,7 @@ static int smsusb_init_device(struct usb_interface *intf, 
int board_id)
rc = smsusb_start_streaming(dev);
if (rc < 0) {
pr_err("smsusb_start_streaming(...) failed\n");
-   smsusb_term_device(intf);
-   return rc;
+   goto terminate_device;
}
 
dev->state = SMSUSB_ACTIVE;
@@ -489,13 +486,17 @@ static int smsusb_init_device(struct usb_interface *intf, 
int board_id)
rc = smscore_start_device(dev->coredev);
if (rc < 0) {
pr_err("smscore_start_device(...) failed\n");
-   smsusb_term_device(intf);
-   return rc;
+   goto terminate_device;
}
 
pr_debug("device 0x%p created\n", dev);
 
return rc;
+
+terminate_device:
+   smsusb_term_device(intf);
+   kfree(mdev);
+   return rc;
 }
 
 static int smsusb_probe(struct usb_interface *intf,
-- 
2.14.1



Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.

2017-09-20 Thread Hans Verkuil
On 09/20/17 14:23, Dave Stevenson wrote:
> On 20 September 2017 at 12:24, Hans Verkuil  wrote:
>> On 09/20/17 13:00, Dave Stevenson wrote:
>>> On 20 September 2017 at 11:23, Philipp Zabel  wrote:
 Hi,

 On Wed, 2017-09-20 at 10:14 +0100, Dave Stevenson wrote:
> Hi Mauro & Philipp
>
> On 19 September 2017 at 17:49, Mauro Carvalho Chehab
>  wrote:
>> Em Tue, 19 Sep 2017 17:24:45 +0200
>> Philipp Zabel  escreveu:
>>
>>> Hi Dave,
>>>
>>> On Tue, 2017-09-19 at 14:08 +0100, Dave Stevenson wrote:
 The existing fixed value of 16 worked for UYVY 720P60 over
 2 lanes at 594MHz, or UYVY 1080P60 over 4 lanes. (RGB888
 1080P60 needs 6 lanes at 594MHz).
 It doesn't allow for lower resolutions to work as the FIFO
 underflows.

 Using a value of 300 works for all resolutions down to VGA60,
 and the increase in frame delay is <4usecs for 1080P60 UYVY
 (2.55usecs for RGB888).

 Signed-off-by: Dave Stevenson 
>>>
>>> Can we increase this to 320? This would also allow
>>> 720p60 at 594 Mbps / 4 lanes, according to the xls.
>
> Unless I've missed something then the driver would currently request
> only 2 lanes for 720p60 UYVY, and that works with the existing FIFO
> setting of 16. Likewise 720p60 RGB888 requests 3 lanes and also works
> on a FIFO setting of 16.
> How/why were you thinking we need to run all four lanes for 720p60
> without other significant driver mods around lane config?

 The driver currently silently changes the number of active lanes
 depending on required data rate, with no way to communicate it to the
 receiver.
>>>
>>> It is communicated over the subdevice API - tc358743_g_mbus_config
>>> reports back the appropriate number of lanes to the receiver
>>> subdevice.
>>> A suitable v4l2_subdev_has_op(dev->sensor, video, g_mbus_config) call
>>> as you're starting streaming therefore gives you the correct
>>> information. That's what I've just done for the BCM283x Unicam
>>> driver[1], but admittedly I'm not using the media controller API which
>>> i.MX6 is.
>>
>> Shouldn't this information come from the device tree? The g_mbus_config
>> op is close to being deprecated or even removed. There are currently only
>> two obscure V4L2 bridge drivers that call it. It dates from pre-DT times
>> I rather not see it used in a new bridge driver.
>>
>> The problem is that contains data that belongs to the DT (hardware
>> capabilities). Things that can actually change dynamically should be
>> communicated via another op. We don't have that, so that should be created.
>>
>> I've CC-ed Sakari, he is the specialist for such things.
> 
> You've reminded me that I asked that same question earlier in the
> year, and Sakari had replied -
> http://www.spinics.net/lists/linux-media/msg115550.html
> 
> Is it specifically device tree related? Just because the lanes are
> physically there doesn't necessarily mean they have to be used.

The DT should tell how many lanes are connected.

g/s_mbus_config was really doing the job that the device tree does today,
but it does so badly.

My recommendation is that you don't use it at all. Instead look at the
DT for the number of lanes.

*If* it becomes clear that you need to communicate the actual number of
lanes in use, then we need to make a new op or whatever.

> 
> A quick test with the spreadsheet appears to say that 1080p24 UYVY
> over 4 lanes at 594Mbps needs a FIFO setting >=480 (the max is 511). I
> would anticipate that to be one of the worst situations as we're
> dealing with a FIFO underflow herewhen there is a significantly faster
> CSI rate than HDMI.
> It can't be supported with a 972Mbps link frequency over 4 lanes
> (needs >=667), and 2 lanes needs a FIFO setting >=374.
> 
> I'll see what numbers fall out of the new spreadsheet for all standard
> modes. If there are some modes that can't be supported over 4 lanes
> then there is an absolute requirement for communicating the number of
> lanes to use.
> 
> Seeing as Cisco have kit shipping with this chip and driver, can I ask
> how they are managing the choice over number of lanes in use?

It's using g_mbus_config since it runs on an old pre-DT kernel.

I personally would be perfectly happy with a simple new op to just
communicate the number of lanes in use, but there may be more things
that should be passed on to the bridge driver. Sakari knows this better
than I do.

But g_mbus_config shouldn't be used here. No way you could have known
that, we really need to clarify that in v4l2-subdev.h.

Hmm, it DOES say that for s_mbus_config, but not for g_mbus_config.

Regards,

Hans



Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.

2017-09-20 Thread Philipp Zabel
On Wed, 2017-09-20 at 13:24 +0200, Hans Verkuil wrote:
> On 09/20/17 13:00, Dave Stevenson wrote:
[...]
> > It is communicated over the subdevice API - tc358743_g_mbus_config
> > reports back the appropriate number of lanes to the receiver
> > subdevice.
> > A suitable v4l2_subdev_has_op(dev->sensor, video, g_mbus_config) call
> > as you're starting streaming therefore gives you the correct
> > information. That's what I've just done for the BCM283x Unicam
> > driver[1], but admittedly I'm not using the media controller API which
> > i.MX6 is.
> 
> Shouldn't this information come from the device tree? The g_mbus_config
> op is close to being deprecated or even removed. There are currently only
> two obscure V4L2 bridge drivers that call it. It dates from pre-DT times
> I rather not see it used in a new bridge driver.
> 
> The problem is that contains data that belongs to the DT (hardware
> capabilities). Things that can actually change dynamically should be
> communicated via another op. We don't have that, so that should be
> created.
> 
> I've CC-ed Sakari, he is the specialist for such things.

The total number of MIPI CSI-2 lanes (as well as their order) and the
list of allowed link frequencies are static and come from the device
tree.

But the possible combinations of link frequency and number of active
lanes out of those for a given resolution and format can vary depending
on both transmitter and receiver capabilities.

For example, if the DT specifies 4 lanes and both 148.5 MHz and 297 MHz
link frequencies, the Toshiba chip could send 720p60 YUYV via 4 lanes at
148.5 MHz, via 2 lanes at 297 MHz, or even via 4 lanes at 297 MHz, with
the longer FIFO delay.

> > 
> > [1] http://www.spinics.net/lists/linux-media/msg121813.html, as part
> > of the unicam_start_streaming function.
> > 
> > > The i.MX6 MIPI CSI-2 receiver driver can't cope with that, as it always
> > > activates all four lanes that are configured in the device tree. I can
> > > work around that with the following patch:
> > 
> > It can't cope running at less than 4 lanes, or it can't cope with a
> > change?

The hardware can cope with both, although I don't know if there are
receivers that can not.
In my case this is just about not knowing how many lanes to activate
(see below) and which link frequency to choose (currently fixed to the
first).

[...]
> > > [...] 300 is giving a fair safety margin.
> > > 
> > > It does not work for 720p60 on 4 lanes at 594 Mbit/s, as the spreadsheet
> > > warns, and testing shows.
> > 
> > If it doesn't work with 720p60, then I guess it has no hope with many
> > other resolutions.

That would have to be checked on a case by case basis, unfortunately.
I have a usecase that only supports 1080p50/60 and 720p50/60.

> > It sounds like confirming whether g_mbus_config is a potential
> > solution for i.MX6 (sorry I'm not familiar enough with that code to do
> > my own quick search), but otherwise cranking it up to 320 is
> > reasonable, and I'll see what other numbers fall out of the
> > spreadsheet.

It seems we need a replacement for g_mbus_config that only returns the
dynamic parameters.

regards
Philipp


Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.

2017-09-20 Thread Dave Stevenson
On 20 September 2017 at 12:24, Hans Verkuil  wrote:
> On 09/20/17 13:00, Dave Stevenson wrote:
>> On 20 September 2017 at 11:23, Philipp Zabel  wrote:
>>> Hi,
>>>
>>> On Wed, 2017-09-20 at 10:14 +0100, Dave Stevenson wrote:
 Hi Mauro & Philipp

 On 19 September 2017 at 17:49, Mauro Carvalho Chehab
  wrote:
> Em Tue, 19 Sep 2017 17:24:45 +0200
> Philipp Zabel  escreveu:
>
>> Hi Dave,
>>
>> On Tue, 2017-09-19 at 14:08 +0100, Dave Stevenson wrote:
>>> The existing fixed value of 16 worked for UYVY 720P60 over
>>> 2 lanes at 594MHz, or UYVY 1080P60 over 4 lanes. (RGB888
>>> 1080P60 needs 6 lanes at 594MHz).
>>> It doesn't allow for lower resolutions to work as the FIFO
>>> underflows.
>>>
>>> Using a value of 300 works for all resolutions down to VGA60,
>>> and the increase in frame delay is <4usecs for 1080P60 UYVY
>>> (2.55usecs for RGB888).
>>>
>>> Signed-off-by: Dave Stevenson 
>>
>> Can we increase this to 320? This would also allow
>> 720p60 at 594 Mbps / 4 lanes, according to the xls.

 Unless I've missed something then the driver would currently request
 only 2 lanes for 720p60 UYVY, and that works with the existing FIFO
 setting of 16. Likewise 720p60 RGB888 requests 3 lanes and also works
 on a FIFO setting of 16.
 How/why were you thinking we need to run all four lanes for 720p60
 without other significant driver mods around lane config?
>>>
>>> The driver currently silently changes the number of active lanes
>>> depending on required data rate, with no way to communicate it to the
>>> receiver.
>>
>> It is communicated over the subdevice API - tc358743_g_mbus_config
>> reports back the appropriate number of lanes to the receiver
>> subdevice.
>> A suitable v4l2_subdev_has_op(dev->sensor, video, g_mbus_config) call
>> as you're starting streaming therefore gives you the correct
>> information. That's what I've just done for the BCM283x Unicam
>> driver[1], but admittedly I'm not using the media controller API which
>> i.MX6 is.
>
> Shouldn't this information come from the device tree? The g_mbus_config
> op is close to being deprecated or even removed. There are currently only
> two obscure V4L2 bridge drivers that call it. It dates from pre-DT times
> I rather not see it used in a new bridge driver.
>
> The problem is that contains data that belongs to the DT (hardware
> capabilities). Things that can actually change dynamically should be
> communicated via another op. We don't have that, so that should be created.
>
> I've CC-ed Sakari, he is the specialist for such things.

You've reminded me that I asked that same question earlier in the
year, and Sakari had replied -
http://www.spinics.net/lists/linux-media/msg115550.html

Is it specifically device tree related? Just because the lanes are
physically there doesn't necessarily mean they have to be used.

A quick test with the spreadsheet appears to say that 1080p24 UYVY
over 4 lanes at 594Mbps needs a FIFO setting >=480 (the max is 511). I
would anticipate that to be one of the worst situations as we're
dealing with a FIFO underflow herewhen there is a significantly faster
CSI rate than HDMI.
It can't be supported with a 972Mbps link frequency over 4 lanes
(needs >=667), and 2 lanes needs a FIFO setting >=374.

I'll see what numbers fall out of the new spreadsheet for all standard
modes. If there are some modes that can't be supported over 4 lanes
then there is an absolute requirement for communicating the number of
lanes to use.

Seeing as Cisco have kit shipping with this chip and driver, can I ask
how they are managing the choice over number of lanes in use?


As for this patch it sounds like we need to crank the FIFO setting up
to the maximum of 511, and potentially a second patch that removes
g_mbus_config and only reads DT if that is the desired behaviour.

>>
>> [1] http://www.spinics.net/lists/linux-media/msg121813.html, as part
>> of the unicam_start_streaming function.
>>
>>> The i.MX6 MIPI CSI-2 receiver driver can't cope with that, as it always
>>> activates all four lanes that are configured in the device tree. I can
>>> work around that with the following patch:
>>
>> It can't cope running at less than 4 lanes, or it can't cope with a change?
>>
>>> --8<--
>>> Subject: [PATCH] [media] tc358743: do not dynamically reduce number of lanes
>>>
>>> Dynamic lane number reduction does not work with receivers that
>>> configure a fixed lane number according to the device tree settings.
>>> To allow 720p60 at 594 Mbit/s on 4 lanes, increase the fifo_level
>>> and tclk_trailcnt settings.
>>>
>>> Signed-off-by: Philipp Zabel 
>>> ---
>>>  drivers/media/i2c/tc358743.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git 

Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.

2017-09-20 Thread Hans Verkuil
On 09/20/17 13:00, Dave Stevenson wrote:
> On 20 September 2017 at 11:23, Philipp Zabel  wrote:
>> Hi,
>>
>> On Wed, 2017-09-20 at 10:14 +0100, Dave Stevenson wrote:
>>> Hi Mauro & Philipp
>>>
>>> On 19 September 2017 at 17:49, Mauro Carvalho Chehab
>>>  wrote:
 Em Tue, 19 Sep 2017 17:24:45 +0200
 Philipp Zabel  escreveu:

> Hi Dave,
>
> On Tue, 2017-09-19 at 14:08 +0100, Dave Stevenson wrote:
>> The existing fixed value of 16 worked for UYVY 720P60 over
>> 2 lanes at 594MHz, or UYVY 1080P60 over 4 lanes. (RGB888
>> 1080P60 needs 6 lanes at 594MHz).
>> It doesn't allow for lower resolutions to work as the FIFO
>> underflows.
>>
>> Using a value of 300 works for all resolutions down to VGA60,
>> and the increase in frame delay is <4usecs for 1080P60 UYVY
>> (2.55usecs for RGB888).
>>
>> Signed-off-by: Dave Stevenson 
>
> Can we increase this to 320? This would also allow
> 720p60 at 594 Mbps / 4 lanes, according to the xls.
>>>
>>> Unless I've missed something then the driver would currently request
>>> only 2 lanes for 720p60 UYVY, and that works with the existing FIFO
>>> setting of 16. Likewise 720p60 RGB888 requests 3 lanes and also works
>>> on a FIFO setting of 16.
>>> How/why were you thinking we need to run all four lanes for 720p60
>>> without other significant driver mods around lane config?
>>
>> The driver currently silently changes the number of active lanes
>> depending on required data rate, with no way to communicate it to the
>> receiver.
> 
> It is communicated over the subdevice API - tc358743_g_mbus_config
> reports back the appropriate number of lanes to the receiver
> subdevice.
> A suitable v4l2_subdev_has_op(dev->sensor, video, g_mbus_config) call
> as you're starting streaming therefore gives you the correct
> information. That's what I've just done for the BCM283x Unicam
> driver[1], but admittedly I'm not using the media controller API which
> i.MX6 is.

Shouldn't this information come from the device tree? The g_mbus_config
op is close to being deprecated or even removed. There are currently only
two obscure V4L2 bridge drivers that call it. It dates from pre-DT times
I rather not see it used in a new bridge driver.

The problem is that contains data that belongs to the DT (hardware
capabilities). Things that can actually change dynamically should be
communicated via another op. We don't have that, so that should be created.

I've CC-ed Sakari, he is the specialist for such things.

> 
> [1] http://www.spinics.net/lists/linux-media/msg121813.html, as part
> of the unicam_start_streaming function.
> 
>> The i.MX6 MIPI CSI-2 receiver driver can't cope with that, as it always
>> activates all four lanes that are configured in the device tree. I can
>> work around that with the following patch:
> 
> It can't cope running at less than 4 lanes, or it can't cope with a change?
> 
>> --8<--
>> Subject: [PATCH] [media] tc358743: do not dynamically reduce number of lanes
>>
>> Dynamic lane number reduction does not work with receivers that
>> configure a fixed lane number according to the device tree settings.
>> To allow 720p60 at 594 Mbit/s on 4 lanes, increase the fifo_level
>> and tclk_trailcnt settings.
>>
>> Signed-off-by: Philipp Zabel 
>> ---
>>  drivers/media/i2c/tc358743.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
>> index 64f504542a819..70a9435928cdb 100644
>> --- a/drivers/media/i2c/tc358743.c
>> +++ b/drivers/media/i2c/tc358743.c
>> @@ -683,7 +683,7 @@ static void tc358743_set_csi(struct v4l2_subdev *sd)
>>  {
>> struct tc358743_state *state = to_state(sd);
>> struct tc358743_platform_data *pdata = >pdata;
>> -   unsigned lanes = tc358743_num_csi_lanes_needed(sd);
>> +   unsigned lanes = state->bus.num_data_lanes;
>>
>> v4l2_dbg(3, debug, sd, "%s:\n", __func__);
>>
>> @@ -1906,7 +1906,7 @@ static int tc358743_probe_of(struct tc358743_state 
>> *state)
>> state->pdata.ddc5v_delay = DDC5V_DELAY_100_MS;
>> state->pdata.enable_hdcp = false;
>> /* A FIFO level of 16 should be enough for 2-lane 720p60 at 594 MHz. 
>> */
>> -   state->pdata.fifo_level = 16;
>> +   state->pdata.fifo_level = 320;
>> /*
>>  * The PLL input clock is obtained by dividing refclk by pll_prd.
>>  * It must be between 6 MHz and 40 MHz, lower frequency is better.
>> @@ -1948,7 +1948,7 @@ static int tc358743_probe_of(struct tc358743_state 
>> *state)
>> state->pdata.lptxtimecnt = 0x003;
>> /* tclk-preparecnt: 3, tclk-zerocnt: 20 */
>> state->pdata.tclk_headercnt = 0x1403;
>> -   state->pdata.tclk_trailcnt = 0x00;
>> +   state->pdata.tclk_trailcnt = 0x01;
>> /* 

Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.

2017-09-20 Thread Dave Stevenson
On 20 September 2017 at 11:23, Philipp Zabel  wrote:
> Hi,
>
> On Wed, 2017-09-20 at 10:14 +0100, Dave Stevenson wrote:
>> Hi Mauro & Philipp
>>
>> On 19 September 2017 at 17:49, Mauro Carvalho Chehab
>>  wrote:
>> > Em Tue, 19 Sep 2017 17:24:45 +0200
>> > Philipp Zabel  escreveu:
>> >
>> > > Hi Dave,
>> > >
>> > > On Tue, 2017-09-19 at 14:08 +0100, Dave Stevenson wrote:
>> > > > The existing fixed value of 16 worked for UYVY 720P60 over
>> > > > 2 lanes at 594MHz, or UYVY 1080P60 over 4 lanes. (RGB888
>> > > > 1080P60 needs 6 lanes at 594MHz).
>> > > > It doesn't allow for lower resolutions to work as the FIFO
>> > > > underflows.
>> > > >
>> > > > Using a value of 300 works for all resolutions down to VGA60,
>> > > > and the increase in frame delay is <4usecs for 1080P60 UYVY
>> > > > (2.55usecs for RGB888).
>> > > >
>> > > > Signed-off-by: Dave Stevenson 
>> > >
>> > > Can we increase this to 320? This would also allow
>> > > 720p60 at 594 Mbps / 4 lanes, according to the xls.
>>
>> Unless I've missed something then the driver would currently request
>> only 2 lanes for 720p60 UYVY, and that works with the existing FIFO
>> setting of 16. Likewise 720p60 RGB888 requests 3 lanes and also works
>> on a FIFO setting of 16.
>> How/why were you thinking we need to run all four lanes for 720p60
>> without other significant driver mods around lane config?
>
> The driver currently silently changes the number of active lanes
> depending on required data rate, with no way to communicate it to the
> receiver.

It is communicated over the subdevice API - tc358743_g_mbus_config
reports back the appropriate number of lanes to the receiver
subdevice.
A suitable v4l2_subdev_has_op(dev->sensor, video, g_mbus_config) call
as you're starting streaming therefore gives you the correct
information. That's what I've just done for the BCM283x Unicam
driver[1], but admittedly I'm not using the media controller API which
i.MX6 is.

[1] http://www.spinics.net/lists/linux-media/msg121813.html, as part
of the unicam_start_streaming function.

> The i.MX6 MIPI CSI-2 receiver driver can't cope with that, as it always
> activates all four lanes that are configured in the device tree. I can
> work around that with the following patch:

It can't cope running at less than 4 lanes, or it can't cope with a change?

> --8<--
> Subject: [PATCH] [media] tc358743: do not dynamically reduce number of lanes
>
> Dynamic lane number reduction does not work with receivers that
> configure a fixed lane number according to the device tree settings.
> To allow 720p60 at 594 Mbit/s on 4 lanes, increase the fifo_level
> and tclk_trailcnt settings.
>
> Signed-off-by: Philipp Zabel 
> ---
>  drivers/media/i2c/tc358743.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> index 64f504542a819..70a9435928cdb 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -683,7 +683,7 @@ static void tc358743_set_csi(struct v4l2_subdev *sd)
>  {
> struct tc358743_state *state = to_state(sd);
> struct tc358743_platform_data *pdata = >pdata;
> -   unsigned lanes = tc358743_num_csi_lanes_needed(sd);
> +   unsigned lanes = state->bus.num_data_lanes;
>
> v4l2_dbg(3, debug, sd, "%s:\n", __func__);
>
> @@ -1906,7 +1906,7 @@ static int tc358743_probe_of(struct tc358743_state 
> *state)
> state->pdata.ddc5v_delay = DDC5V_DELAY_100_MS;
> state->pdata.enable_hdcp = false;
> /* A FIFO level of 16 should be enough for 2-lane 720p60 at 594 MHz. 
> */
> -   state->pdata.fifo_level = 16;
> +   state->pdata.fifo_level = 320;
> /*
>  * The PLL input clock is obtained by dividing refclk by pll_prd.
>  * It must be between 6 MHz and 40 MHz, lower frequency is better.
> @@ -1948,7 +1948,7 @@ static int tc358743_probe_of(struct tc358743_state 
> *state)
> state->pdata.lptxtimecnt = 0x003;
> /* tclk-preparecnt: 3, tclk-zerocnt: 20 */
> state->pdata.tclk_headercnt = 0x1403;
> -   state->pdata.tclk_trailcnt = 0x00;
> +   state->pdata.tclk_trailcnt = 0x01;
> /* ths-preparecnt: 3, ths-zerocnt: 1 */
> state->pdata.ths_headercnt = 0x0103;
> state->pdata.twakeup = 0x4882;
> --
> 2.11.0
> -->8--
>
> Just adding the same heuristic as tc358743_num_csi_lanes_needed in the
> imx6-mipi-csi2 driver doesn't work, as the heuristic is specific to the
> Toshiba chip. There are MIPI CSI-2 sensors that only support a fixed
> number of lanes, for example.
>
> I'd need a way to communicate the number of active MIPI CSI-2 lanes
> between transmitting and receiving subdevice driver.
>
>> Once I've got a v3 done on the Unicam driver I'll bash through the
>> standard HDMI modes and check what value 

Re: [PATCH v13 13/25] v4l: async: Allow async notifier register call succeed with no subdevs

2017-09-20 Thread Sakari Ailus
Hi Laurent,

On Tue, Sep 19, 2017 at 08:54:12PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Tuesday, 19 September 2017 18:03:48 EEST Sakari Ailus wrote:
> > On Tue, Sep 19, 2017 at 05:58:32PM +0300, Sakari Ailus wrote:
> > >> This skips adding the notifier to the notifier_list. Won't this result
> > >> in an oops when calling list_del(>list) in
> > >> v4l2_async_notifier_unregister() ?
> > > 
> > > Good point. I'll add initialising the list head to the register function,
> > > with an appropriate comment.
> > 
> > I'll set v4l2_dev NULL instead; no tricks with lists needed.
> 
> Shouldn't the notifier still be added to the notifier_list ?

Would there be any benefit of that?

The notifier's v4l2_dev field is also used to determine whether the
notifier is registered currently. If the notifier is added to the notifier
list, we need to remove it in unregistration as well.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com


  1   2   >