date:20180201

Re: [RFC PATCH 1/9] media: add request API core and UAPI

2018-02-01 Thread Tomasz Figa

On Fri, Feb 2, 2018 at 4:33 PM, Sakari Ailus
 wrote:
>> >> +/**
>> >> + * struct media_request_queue - queue of requests
>> >> + *
>> >> + * @mdev:media_device that manages this queue
>> >> + * @ops: implementation of the queue
>> >> + * @mutex:   protects requests, active_request, req_id, and all members 
>> >> of
>> >> + *   struct media_request
>> >> + * @active_request: request being currently run by this queue
>> >> + * @requests:list of requests (not in any particular order) that 
>> >> this
>> >> + *   queue owns.
>> >> + * @req_id:  counter used to identify requests for debugging purposes
>> >> + */
>> >> +struct media_request_queue {
>> >> + struct media_device *mdev;
>> >> + const struct media_request_queue_ops *ops;
>> >> +
>> >> + struct mutex mutex;
>> >
>> > Any particular reason for using a mutex? The request queue lock will need
>> > to be acquired from interrupts, too, so this should be changed to a
>> > spinlock.
>>
>> Will it be acquired from interrupts? In any case it should be possible
>> to change this to a spinlock.
>
> Using mutexes will effectively make this impossible, and I don't think we
> can safely say there's not going to be a need for that. So spinlocks,
> please.
>

IMHO whether a mutex or spinlock is the right thing depends on what
kind of critical section it is used for. If it only protects data (and
according to the comment, this one seems to do so), spinlock might
actually have better properties, e.g. not introducing the need to
reschedule, if another CPU is accessing the data at the moment. It
might also depend on how heavy the data accesses are, though. We
shouldn't need to spin for too long time.

Best regards,
Tomasz

Re: [RFC PATCH 1/9] media: add request API core and UAPI

2018-02-01 Thread Sakari Ailus

Hi Alexandre,

On Tue, Jan 30, 2018 at 01:23:05PM +0900, Alexandre Courbot wrote:
> Hi Sakari, thanks for the review!
> 
> The version you reviewed is not the latest one, but I suppose most of
> your comments still apply.
> 
> On Fri, Jan 26, 2018 at 5:39 PM, Sakari Ailus  wrote:
> > Hi Alexandre,
> >
> > I remember it was discussed that the work after the V4L2 jobs API would
> > continue from the existing request API patches. I see that at least the
> > rather important support for events is missing in this version. Why was it
> > left out?
> 
> Request completion is signaled by polling on the request FD, so we
> don't need to rely on V4L2 events to signal this anymore. If we want
> to signal different kinds of events on requests we could implement a
> more sophisticated event system on top of that, but for our current
> needs polling is sufficient.

Right. This works for now indeed. We will need to revisit this when
requests are moved to the media device in the future.

> 
> What other kind of event besides completion could we want to deliver
> to user-space from a request?
> 
> >
> > I also see that variable size IOCTL argument support is no longer included.
> 
> Do we need this for the request API?

Technically there's no strict need for that now. However when the requests
are moved to the media device (i.e. other device nodes are not needed
anymore), then this is a must.

It was proposed and AFAIR agreed on as well that new media device
IOCTLs would not use reserved fields any longer but rely on variable size
IOCTL arguments instead. This is in line with your request argument struct
having no reserved fields and I don't think we should add them there.

> 
> >
> > On Fri, Dec 15, 2017 at 04:56:17PM +0900, Alexandre Courbot wrote:
> >> The request API provides a way to group buffers and device parameters
> >> into units of work to be queued and executed. This patch introduces the
> >> UAPI and core framework.
> >>
> >> This patch is based on the previous work by Laurent Pinchart. The core
> >> has changed considerably, but the UAPI is mostly untouched.
> >>
> >> Signed-off-by: Alexandre Courbot 
> >> ---
> >>  drivers/media/Makefile   |   3 +-
> >>  drivers/media/media-device.c |   6 +
> >>  drivers/media/media-request.c| 390 
> >> +++
> >>  drivers/media/v4l2-core/v4l2-ioctl.c |   2 +-
> >>  include/media/media-device.h |   3 +
> >>  include/media/media-entity.h |   6 +
> >>  include/media/media-request.h| 269 
> >>  include/uapi/linux/media.h   |  11 +
> >>  8 files changed, 688 insertions(+), 2 deletions(-)
> >>  create mode 100644 drivers/media/media-request.c
> >>  create mode 100644 include/media/media-request.h
> >>
> >> diff --git a/drivers/media/Makefile b/drivers/media/Makefile
> >> index 594b462ddf0e..985d35ec6b29 100644
> >> --- a/drivers/media/Makefile
> >> +++ b/drivers/media/Makefile
> >> @@ -3,7 +3,8 @@
> >>  # Makefile for the kernel multimedia device drivers.
> >>  #
> >>
> >> -media-objs   := media-device.o media-devnode.o media-entity.o
> >> +media-objs   := media-device.o media-devnode.o media-entity.o \
> >> +media-request.o
> >>
> >>  #
> >>  # I2C drivers should come before other drivers, otherwise they'll fail
> >> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> >> index e79f72b8b858..045cec7d2de9 100644
> >> --- a/drivers/media/media-device.c
> >> +++ b/drivers/media/media-device.c
> >> @@ -32,6 +32,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>
> >>  #ifdef CONFIG_MEDIA_CONTROLLER
> >>
> >> @@ -407,6 +408,7 @@ static const struct media_ioctl_info ioctl_info[] = {
> >>   MEDIA_IOC(ENUM_LINKS, media_device_enum_links, 
> >> MEDIA_IOC_FL_GRAPH_MUTEX),
> >>   MEDIA_IOC(SETUP_LINK, media_device_setup_link, 
> >> MEDIA_IOC_FL_GRAPH_MUTEX),
> >>   MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, 
> >> MEDIA_IOC_FL_GRAPH_MUTEX),
> >> + MEDIA_IOC(REQUEST_CMD, media_device_request_cmd, 0),
> >>  };
> >>
> >>  static long media_device_ioctl(struct file *filp, unsigned int cmd,
> >> @@ -688,6 +690,10 @@ EXPORT_SYMBOL_GPL(media_device_init);
> >>
> >>  void media_device_cleanup(struct media_device *mdev)
> >>  {
> >> + if (mdev->req_queue) {
> >> + mdev->req_queue->ops->release(mdev->req_queue);
> >> + mdev->req_queue = NULL;
> >> + }
> >>   ida_destroy(>entity_internal_idx);
> >>   mdev->entity_internal_idx_max = 0;
> >>   media_graph_walk_cleanup(>pm_count_walk);
> >> diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c
> >> new file mode 100644
> >> index ..15dc65ddfe41
> >> --- /dev/null
> >> +++ b/drivers/media/media-request.c
> >> @@ -0,0 +1,390 @@
> >> +/*
> >> + * Request and request queue base management
> >> + *
> >> + * Copyright (C) 2017, The Chromium OS

Re: [RFCv2 05/17] media: Document the media request API

2018-02-01 Thread Alexandre Courbot

Hi Randy,

On Fri, Feb 2, 2018 at 3:14 AM, Randy Dunlap  wrote:
> On 01/31/2018 02:24 AM, Alexandre Courbot wrote:
>> From: Laurent Pinchart 
>>
>> The media request API is made of a new ioctl to implement request
>> management. Document it.
>>
>> Signed-off-by: Laurent Pinchart 
>> [acour...@chromium.org: adapt for newest API]
>> Signed-off-by: Alexandre Courbot 
>> ---
>>  Documentation/media/uapi/mediactl/media-funcs.rst  |   1 +
>>  .../media/uapi/mediactl/media-ioc-request-cmd.rst  | 141 
>> +
>>  2 files changed, 142 insertions(+)
>>  create mode 100644 
>> Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst
>>
>> diff --git a/Documentation/media/uapi/mediactl/media-funcs.rst 
>> b/Documentation/media/uapi/mediactl/media-funcs.rst
>> index 076856501cdb..e3a45d82ffcb 100644
>> --- a/Documentation/media/uapi/mediactl/media-funcs.rst
>> +++ b/Documentation/media/uapi/mediactl/media-funcs.rst
>> @@ -15,4 +15,5 @@ Function Reference
>>  media-ioc-g-topology
>>  media-ioc-enum-entities
>>  media-ioc-enum-links
>> +media-ioc-request-cmd
>>  media-ioc-setup-link
>> diff --git a/Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst 
>> b/Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst
>> new file mode 100644
>> index ..723b422afcce
>> --- /dev/null
>> +++ b/Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst
>> @@ -0,0 +1,141 @@
>> +.. -*- coding: utf-8; mode: rst -*-
>> +
>> +.. _media_ioc_request_cmd:
>> +
>> +***
>> +ioctl MEDIA_IOC_REQUEST_CMD
>> +***
>> +
>> +Name
>> +
>> +
>> +MEDIA_IOC_REQUEST_CMD - Manage media device requests
>> +
>> +
>> +Synopsis
>> +
>> +
>> +.. c:function:: int ioctl( int fd, MEDIA_IOC_REQUEST_CMD, struct 
>> media_request_cmd *argp )
>> +:name: MEDIA_IOC_REQUEST_CMD
>> +
>> +
>> +Arguments
>> +=
>> +
>> +``fd``
>> +File descriptor returned by :ref:`open() `.
>> +
>> +``argp``
>> +
>> +
>> +Description
>> +===
>> +
>> +The MEDIA_IOC_REQUEST_CMD ioctl allow applications to manage media device
>
>allows
>
>> +requests. A request is an object that can group media device configuration
>> +parameters, including subsystem-specific parameters, in order to apply all 
>> the
>> +parameters atomically. Applications are responsible for allocating and
>> +deleting requests, filling them with configuration parameters submitting 
>> them.
>
> and 
> submitting them.
>
>> +
>> +Request operations are performed by calling the MEDIA_IOC_REQUEST_CMD ioctl
>> +with a pointer to a struct :c:type:`media_request_cmd` with the cmd field 
>> set
>> +to the appropriate command. :ref:`media-request-command` lists the commands
>> +supported by the ioctl.
>> +
>> +The struct :c:type:`media_request_cmd` request field contains the file
>> +descriptorof the request on which the command operates. For the
>
>descriptor of
>
>> +``MEDIA_REQ_CMD_ALLOC`` command the field is set to zero by applications and
>> +filled by the driver. For all other commands the field is set by 
>> applications
>> +and left untouched by the driver.
>> +
>> +To allocate a new request applications use the ``MEDIA_REQ_CMD_ALLOC``
>> +command. The driver will allocate a new request and return its FD in the
>> +request field. After allocation, the request is "empty", which means that it
>> +does not hold any state of its own, and that the hardware's state will not 
>> be
>> +affected by it unless it is passed as argument to V4L2 or media controller
>> +commands.
>> +
>> +Requests are reference-counted. A newly allocated request is referenced
>> +by the returned file descriptor, and can be later referenced by
>> +subsystem-specific operations. Requests will thus be automatically deleted
>> +when they're not longer used after the returned file descriptor is closed.
>
>  no longer
>
>> +
>> +If a request isn't needed applications can delete it by calling ``close()``
>> +on it. The driver will drop the file handle reference. The request will not
>> +be usable through the MEDIA_IOC_REQUEST_CMD ioctl anymore, but will only be
>> +deleted when the last reference is released. If no other reference exists 
>> when
>> +``close()`` is invoked the request will be deleted immediately.
>> +
>> +After creating a request applications should fill it with configuration
>> +parameters. This is performed through subsystem-specific request APIs 
>> outside
>> +the scope of the media controller API. See the appropriate subsystem APIs 
>> for
>> +more information, including how they interact with the MEDIA_IOC_REQUEST_CMD
>> +ioctl.
>> +
>> +Once a request contains all the desired configuration parameters it can be
>> +submitted using the

cron job: media_tree daily build: ERRORS

2018-02-01 Thread Hans Verkuil

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

Results of the daily build of media_tree:

date:   Fri Feb  2 05:00:25 CET 2018
media-tree git hash:273caa260035c03d89ad63d72d8cd3d9e5c5e3f1
media_build git hash:   d17383327f00d45e6c07161876fb4f3d9d9358e1
v4l-utils git hash: aa287ca25393cf8bcb05d2679ccbe340a798d344
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0-3911-g6f737e1f
smatch version: v0.5.0-3911-g6f737e1f
host hardware:  x86_64
host os:4.14.0-364

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: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.8-i686: ERRORS
linux-3.9.2-i686: ERRORS
linux-3.10.1-i686: ERRORS
linux-3.11.1-i686: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.16.7-i686: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.18.7-i686: ERRORS
linux-3.19-i686: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.1.33-i686: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.4.22-i686: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.8-i686: ERRORS
linux-4.9.26-i686: ERRORS
linux-4.10.14-i686: WARNINGS
linux-4.11-i686: WARNINGS
linux-4.12.1-i686: WARNINGS
linux-4.13-i686: WARNINGS
linux-4.14-i686: WARNINGS
linux-4.15-i686: WARNINGS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-x86_64: ERRORS
linux-3.10.1-x86_64: ERRORS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.7-x86_64: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.7-x86_64: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.33-x86_64: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.22-x86_64: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9.26-x86_64: ERRORS
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: WARNINGS
linux-4.14-x86_64: WARNINGS
linux-4.15-x86_64: WARNINGS
apps: WARNINGS
spec-git: OK
smatch: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

[PATCH v2 2/9] em28xx: Bulk transfer implementation fix

2018-02-01 Thread Brad Love

Set appropriate bulk/ISOC transfer multiplier on capture start.
This sets ISOC transfer to USB endpoint configuration
This sets bulk transfer to 48128 bytes (188 * 256)

The bulk multiplier is maximum allowed according to Empia.

Signed-off-by: Brad Love 
---
Changes since v1
- use ISOC endpoint configuration instead of constant
- removed TS2 from comment

  drivers/media/usb/em28xx/em28xx-core.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/media/usb/em28xx/em28xx-core.c 
b/drivers/media/usb/em28xx/em28xx-core.c
index ef38e56..6727d14 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -638,6 +638,19 @@ int em28xx_capture_start(struct em28xx *dev, int start)
dev->chip_id == CHIP_ID_EM28174 ||
dev->chip_id == CHIP_ID_EM28178) {
/* The Transport Stream Enable Register moved in em2874 */
+   if (dev->dvb_xfer_bulk) {
+   /* Max Tx Size = 188 * 256 = 48128 - LCM(188,512) * 2 */
+   em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
+   EM2874_R5D_TS1_PKT_SIZE :
+   EM2874_R5E_TS2_PKT_SIZE,
+   0xFF);
+   } else {
+   /* ISOC Maximum Transfer Size = 188 * 5 */
+   em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
+   EM2874_R5D_TS1_PKT_SIZE :
+   EM2874_R5E_TS2_PKT_SIZE,
+   dev->dvb_max_pkt_size_isoc / 188);
+   }
if (dev->ts == PRIMARY_TS)
rc = em28xx_write_reg_bits(dev,
EM2874_R5F_TS_ENABLE,
-- 
2.7.4

Good Morning !

2018-02-01 Thread Anny



Good Morning , 



I have a project i would like to bring to you and i want you to help me discuss 
it. please let me know if you will be available for this project. 



Thanks

Re: Regression in VB2 alloc prepare/finish balancing with em28xx/au0828

2018-02-01 Thread Hans Verkuil

On 02/01/2018 07:19 PM, Devin Heitmueller wrote:
> Hi Sakari, Hans,
> 
> Do either of you have any thoughts on whether I'm actually leaking any
> resources, or whether this is just a warning that doesn't have any
> practical implication since I'm tearing down the videobuf2 queue?
> 
> I don't really care about the embedded use case - do you see any
> reason where at least for my local tree I cannot simply revert this
> patch until a real solution is found?

Drivers that use videobuf2-vmalloc are not affected by this since that
doesn't implement the prepare/finish mem_ops.

dma-contig and dma-sg are affected and syncing to/from device/cpu will
be unbalanced, which could lead to corrupt frames. Although I think that
given the situation that triggers this it is unlikely to be a real issue.

In any case, em28xx/au0828 are OK.

Regards,

Hans

> 
> Cheers,
> 
> Devin
> 
> On Mon, Jan 29, 2018 at 8:44 PM, Devin Heitmueller
>  wrote:
>> Hello Sakari,
>>
>> Thanks for taking the time to investigate.  See comments inline.
>>
>> On Sun, Jan 28, 2018 at 5:23 PM, Sakari Ailus
>>  wrote:
>>> Hi Devin,
>>>
>>> On Sun, Jan 28, 2018 at 09:12:44AM -0500, Devin Heitmueller wrote:
 Hello all,

 I recently updated to the latest kernel, and I am seeing the following
 dumped to dmesg with both au0828 and em28xx based devices whenever I
 exit tvtime (my kernel is compiled with CONFIG_VIDEO_ADV_DEBUG=y by
 default):
>>>
>>> Thanks for reporting this. Would you be able to provide the full dmesg,
>>> with VB2 debug parameter set to 2?
>>
>> Output can be found at https://pastebin.com/nXS7MTJH
>>
>>> I can't immediately see how you'd get this, well, without triggering a
>>> kernel warning or two. The code is pretty complex though.
>>
>> If this is something I screwed up when I did the VB2 port for em28xx
>> several years ago, point me in the right direction and I'll see what I
>> can do.  However given we're seeing it with multiple drivers, this
>> feels like some subtle issue inside videobuf2.
>>
>> Devin
>>
>> --
>> Devin J. Heitmueller - Kernel Labs
>> http://www.kernellabs.com
> 
> 
>

Re: Regression in VB2 alloc prepare/finish balancing with em28xx/au0828

2018-02-01 Thread Devin Heitmueller

Hi Sakari, Hans,

Do either of you have any thoughts on whether I'm actually leaking any
resources, or whether this is just a warning that doesn't have any
practical implication since I'm tearing down the videobuf2 queue?

I don't really care about the embedded use case - do you see any
reason where at least for my local tree I cannot simply revert this
patch until a real solution is found?

Cheers,

Devin

On Mon, Jan 29, 2018 at 8:44 PM, Devin Heitmueller
 wrote:
> Hello Sakari,
>
> Thanks for taking the time to investigate.  See comments inline.
>
> On Sun, Jan 28, 2018 at 5:23 PM, Sakari Ailus
>  wrote:
>> Hi Devin,
>>
>> On Sun, Jan 28, 2018 at 09:12:44AM -0500, Devin Heitmueller wrote:
>>> Hello all,
>>>
>>> I recently updated to the latest kernel, and I am seeing the following
>>> dumped to dmesg with both au0828 and em28xx based devices whenever I
>>> exit tvtime (my kernel is compiled with CONFIG_VIDEO_ADV_DEBUG=y by
>>> default):
>>
>> Thanks for reporting this. Would you be able to provide the full dmesg,
>> with VB2 debug parameter set to 2?
>
> Output can be found at https://pastebin.com/nXS7MTJH
>
>> I can't immediately see how you'd get this, well, without triggering a
>> kernel warning or two. The code is pretty complex though.
>
> If this is something I screwed up when I did the VB2 port for em28xx
> several years ago, point me in the right direction and I'll see what I
> can do.  However given we're seeing it with multiple drivers, this
> feels like some subtle issue inside videobuf2.
>
> Devin
>
> --
> Devin J. Heitmueller - Kernel Labs
> http://www.kernellabs.com



-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [RFCv2 05/17] media: Document the media request API

2018-02-01 Thread Randy Dunlap

On 01/31/2018 02:24 AM, Alexandre Courbot wrote:
> From: Laurent Pinchart 
> 
> The media request API is made of a new ioctl to implement request
> management. Document it.
> 
> Signed-off-by: Laurent Pinchart 
> [acour...@chromium.org: adapt for newest API]
> Signed-off-by: Alexandre Courbot 
> ---
>  Documentation/media/uapi/mediactl/media-funcs.rst  |   1 +
>  .../media/uapi/mediactl/media-ioc-request-cmd.rst  | 141 
> +
>  2 files changed, 142 insertions(+)
>  create mode 100644 
> Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst
> 
> diff --git a/Documentation/media/uapi/mediactl/media-funcs.rst 
> b/Documentation/media/uapi/mediactl/media-funcs.rst
> index 076856501cdb..e3a45d82ffcb 100644
> --- a/Documentation/media/uapi/mediactl/media-funcs.rst
> +++ b/Documentation/media/uapi/mediactl/media-funcs.rst
> @@ -15,4 +15,5 @@ Function Reference
>  media-ioc-g-topology
>  media-ioc-enum-entities
>  media-ioc-enum-links
> +media-ioc-request-cmd
>  media-ioc-setup-link
> diff --git a/Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst 
> b/Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst
> new file mode 100644
> index ..723b422afcce
> --- /dev/null
> +++ b/Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst
> @@ -0,0 +1,141 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _media_ioc_request_cmd:
> +
> +***
> +ioctl MEDIA_IOC_REQUEST_CMD
> +***
> +
> +Name
> +
> +
> +MEDIA_IOC_REQUEST_CMD - Manage media device requests
> +
> +
> +Synopsis
> +
> +
> +.. c:function:: int ioctl( int fd, MEDIA_IOC_REQUEST_CMD, struct 
> media_request_cmd *argp )
> +:name: MEDIA_IOC_REQUEST_CMD
> +
> +
> +Arguments
> +=
> +
> +``fd``
> +File descriptor returned by :ref:`open() `.
> +
> +``argp``
> +
> +
> +Description
> +===
> +
> +The MEDIA_IOC_REQUEST_CMD ioctl allow applications to manage media device

   allows

> +requests. A request is an object that can group media device configuration
> +parameters, including subsystem-specific parameters, in order to apply all 
> the
> +parameters atomically. Applications are responsible for allocating and
> +deleting requests, filling them with configuration parameters submitting 
> them.

and submitting 
them.

> +
> +Request operations are performed by calling the MEDIA_IOC_REQUEST_CMD ioctl
> +with a pointer to a struct :c:type:`media_request_cmd` with the cmd field set
> +to the appropriate command. :ref:`media-request-command` lists the commands
> +supported by the ioctl.
> +
> +The struct :c:type:`media_request_cmd` request field contains the file
> +descriptorof the request on which the command operates. For the

   descriptor of

> +``MEDIA_REQ_CMD_ALLOC`` command the field is set to zero by applications and
> +filled by the driver. For all other commands the field is set by applications
> +and left untouched by the driver.
> +
> +To allocate a new request applications use the ``MEDIA_REQ_CMD_ALLOC``
> +command. The driver will allocate a new request and return its FD in the
> +request field. After allocation, the request is "empty", which means that it
> +does not hold any state of its own, and that the hardware's state will not be
> +affected by it unless it is passed as argument to V4L2 or media controller
> +commands.
> +
> +Requests are reference-counted. A newly allocated request is referenced
> +by the returned file descriptor, and can be later referenced by
> +subsystem-specific operations. Requests will thus be automatically deleted
> +when they're not longer used after the returned file descriptor is closed.

 no longer

> +
> +If a request isn't needed applications can delete it by calling ``close()``
> +on it. The driver will drop the file handle reference. The request will not
> +be usable through the MEDIA_IOC_REQUEST_CMD ioctl anymore, but will only be
> +deleted when the last reference is released. If no other reference exists 
> when
> +``close()`` is invoked the request will be deleted immediately.
> +
> +After creating a request applications should fill it with configuration
> +parameters. This is performed through subsystem-specific request APIs outside
> +the scope of the media controller API. See the appropriate subsystem APIs for
> +more information, including how they interact with the MEDIA_IOC_REQUEST_CMD
> +ioctl.
> +
> +Once a request contains all the desired configuration parameters it can be
> +submitted using the ``MEDIA_REQ_CMD_SUBMIT`` command. This will let the
> +buffers queued for the request be passed to their respective drivers, which
> +will then apply the request's parameters before processing them.
> +
> +Once a request has been queued

RE: [PATCH v5 4/5] media: ov5640: add support of DVP parallel interface

2018-02-01 Thread Fabrizio Castro

Hello Hugues,

> Subject: [PATCH v5 4/5] media: ov5640: add support of DVP parallel interface
>
> Add support of DVP parallel mode in addition of
> existing MIPI CSI mode. The choice between two modes
> and configuration is made through device tree.
>
> Signed-off-by: Hugues Fruchet 
> ---
>  drivers/media/i2c/ov5640.c | 148 
> +++--
>  1 file changed, 130 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 9f031f3..a44b680 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -34,13 +34,19 @@
>
>  #define OV5640_DEFAULT_SLAVE_ID 0x3c
>
> +#define OV5640_REG_SYS_CTRL00x3008
>  #define OV5640_REG_CHIP_ID0x300a
> +#define OV5640_REG_IO_MIPI_CTRL000x300e
> +#define OV5640_REG_PAD_OUTPUT_ENABLE010x3017
> +#define OV5640_REG_PAD_OUTPUT_ENABLE020x3018
>  #define OV5640_REG_PAD_OUTPUT000x3019
> +#define OV5640_REG_SYSTEM_CONTROL10x302e
>  #define OV5640_REG_SC_PLL_CTRL00x3034
>  #define OV5640_REG_SC_PLL_CTRL10x3035
>  #define OV5640_REG_SC_PLL_CTRL20x3036
>  #define OV5640_REG_SC_PLL_CTRL30x3037
>  #define OV5640_REG_SLAVE_ID0x3100
> +#define OV5640_REG_SCCB_SYS_CTRL10x3103
>  #define OV5640_REG_SYS_ROOT_DIVIDER0x3108
>  #define OV5640_REG_AWB_R_GAIN0x3400
>  #define OV5640_REG_AWB_G_GAIN0x3402
> @@ -70,6 +76,7 @@
>  #define OV5640_REG_HZ5060_CTRL010x3c01
>  #define OV5640_REG_SIGMADELTA_CTRL0C0x3c0c
>  #define OV5640_REG_FRAME_CTRL010x4202
> +#define OV5640_REG_POLARITY_CTRL000x4740
>  #define OV5640_REG_MIPI_CTRL000x4800
>  #define OV5640_REG_DEBUG_MODE0x4814
>  #define OV5640_REG_PRE_ISP_TEST_SET10x503d
> @@ -982,7 +989,111 @@ static int ov5640_get_gain(struct ov5640_dev *sensor)
>  return gain & 0x3ff;
>  }
>
> -static int ov5640_set_stream(struct ov5640_dev *sensor, bool on)
> +static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
> +{
> +int ret;
> +unsigned int flags = sensor->ep.bus.parallel.flags;
> +u8 pclk_pol = 0;
> +u8 hsync_pol = 0;
> +u8 vsync_pol = 0;
> +
> +/*
> + * Note about parallel port configuration.
> + *
> + * When configured in parallel mode, the OV5640 will
> + * output 10 bits data on DVP data lines [9:0].
> + * If only 8 bits data are wanted, the 8 bits data lines
> + * of the camera interface must be physically connected
> + * on the DVP data lines [9:2].
> + *
> + * Control lines polarity can be configured through
> + * devicetree endpoint control lines properties.
> + * If no endpoint control lines properties are set,
> + * polarity will be as below:
> + * - VSYNC:active high
> + * - HREF:active low
> + * - PCLK:active low
> + */
> +
> +if (on) {
> +/*
> + * reset MIPI PCLK/SERCLK divider
> + *
> + * SC PLL CONTRL1 0
> + * - [3..0]:MIPI PCLK/SERCLK divider
> + */
> +ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL1, 0x0f, 0);
> +if (ret)
> +return ret;
> +
> +/*
> + * configure parallel port control lines polarity
> + *
> + * POLARITY CTRL0
> + * - [5]:PCLK polarity (0: active low, 1: active high)
> + * - [1]:HREF polarity (0: active low, 1: active high)
> + * - [0]:VSYNC polarity (mismatch here between
> + *datasheet and hardware, 0 is active high
> + *and 1 is active low...)

I know that yourself and Maxime have both confirmed that VSYNC polarity is 
inverted, but I am looking at HSYNC and VSYNC with a logic analyser and I am 
dumping the values written to OV5640_REG_POLARITY_CTRL00 and to me it looks 
like that HSYNC is active HIGH when hsync_pol == 0, and VSYNC is active HIGH 
when vsync_pol == 1.
Between the SoC and the sensor I have a voltage translator, 2.8V input -> 3.3V 
output, I am measuring the signals at the translator outputs.
Register 0x302A (chip revision register) on my sensor contains 0xb0.

Could you please double check this? How is it possible that this works 
differently on my sensor?

Thanks,
Fabrizio

> + */
> +if (flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
> +pclk_pol = 1;
> +if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> +hsync_pol = 1;
> +if (flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
> +vsync_pol = 1;
> +
> +ret = ov5640_write_reg(sensor,
> +   OV5640_REG_POLARITY_CTRL00,
> +   (pclk_pol << 5) |
> +   (hsync_pol << 1) |
> +   vsync_pol);
> +
> +if (ret)
> +return ret;
> +}
> +
> +/*
> + * powerdown MIPI TX/RX PHY & disable MIPI
> + *
> + * MIPI CONTROL 00
> + * 4: PWDN PHY TX
> + * 3: PWDN PHY RX
> + * 2: MIPI enable
> + */
> +ret = ov5640_write_reg(sensor,
> +   OV5640_REG_IO_MIPI_CTRL00, on ? 0x18 : 0);
> +if (ret)
> +return ret;
> +
> +/*
> + * enable VSYNC/HREF/PCLK DVP control lines
> + * & D[9:6] DVP data lines
> + *
> + * PAD OUTPUT ENABLE 01
> + * - 6:VSYNC output enable
> + * - 5:HREF output enable
> + * - 4:PCLK output enable
> + * - [3:0]:D[9:6] output enable
> + */
> +ret = ov5640_write_reg(sensor,
> +   OV5640_REG_PAD_OUTPUT_ENABLE01,
> +   on ? 0x7f : 0);
> +if (ret)
> +return ret;
> +
> +/*
> + * enable D[5:0] DVP data lines
> + *
> + * PAD OUTPUT ENABLE 02
> +

Re: [PATCH] media: ov5640: fix virtual_channel parameter permissions

2018-02-01 Thread jacopo mondi

Hi Hugues,

On Thu, Feb 01, 2018 at 02:17:34PM +0100, Hugues Fruchet wrote:
> Fix module_param(virtual_channel) permissions.
> This problem was detected by checkpatch:
> $ scripts/checkpatch.pl -f drivers/media/i2c/ov5640.c
> ERROR: Use 4 digit octal (0777) not decimal permissions
> #131: FILE: drivers/media/i2c/ov5640.c:131:
> +module_param(virtual_channel, int, 0);
>
> Also explicitly set initial value to 0 for default value
> and add an error trace in case of virtual_channel not in
> the valid range of values.
>
> Signed-off-by: Hugues Fruchet 
> ---
>  drivers/media/i2c/ov5640.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 696a28b..906f202 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -127,8 +127,8 @@ struct ov5640_pixfmt {
>   * FIXME: remove this when a subdev API becomes available
>   * to set the MIPI CSI-2 virtual channel.
>   */
> -static unsigned int virtual_channel;
> -module_param(virtual_channel, int, 0);
> +static unsigned int virtual_channel = 0;
> +module_param(virtual_channel, int, 0444);

Parameter type is unsigned int, please use uint here.
As also checkpatch reports, it is not necessary to initialize static
variables to 0.

>  MODULE_PARM_DESC(virtual_channel,
>"MIPI CSI-2 virtual channel (0..3), default 0");
>
> @@ -1358,11 +1358,15 @@ static int ov5640_binning_on(struct ov5640_dev 
> *sensor)
>
>  static int ov5640_set_virtual_channel(struct ov5640_dev *sensor)
>  {
> + struct i2c_client *client = sensor->i2c_client;
>   u8 temp, channel = virtual_channel;
>   int ret;
>
> - if (channel > 3)
> + if (channel > 3) {
> + dev_err(>dev, "%s: wrong virtual_channel parameter 
> value, expected (0..3), got %d\n",

I understand you don't want to break error messages to 80 columns to
ease grep for errors, but you can break the line after "client->dev"
to make this less painful for the eyes.

Thanks
   j

> + __func__, channel);
>   return -EINVAL;
> + }
>
>   ret = ov5640_read_reg(sensor, OV5640_REG_DEBUG_MODE, );
>   if (ret)
> --
> 1.9.1
>

Respond for details

2018-02-01 Thread Mr. Allen



Dear sir/ madam


Do you need a loan to pay off bills ? To pay off your mortgage quickly ? To set 
up a new business or to Re- finance your existing business ? I can help you 
secure a private loan at 3% interest rate please respond for more details



Thanks


Allen

Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

2018-02-01 Thread Robin Murphy

On 31/01/18 09:37, Arnd Bergmann wrote:

On Wed, Jan 31, 2018 at 8:29 AM, Maxime Ripard
wrote:

Hi Thierry,

On Tue, Jan 30, 2018 at 11:01:50AM +0100, Thierry Reding wrote:

On Tue, Jan 30, 2018 at 10:59:16AM +0100, Thierry Reding wrote:

On Tue, Jan 30, 2018 at 10:24:48AM +0100, Arnd Bergmann wrote:

On Tue, Jan 30, 2018 at 8:54 AM, Maxime Ripard
wrote:

On Mon, Jan 29, 2018 at 03:34:02PM +0100, Arnd Bergmann wrote:

On Mon, Jan 29, 2018 at 10:25 AM, Linus Walleij
wrote:

On Mon, Jan 29, 2018 at 9:25 AM, Maxime Ripard
wrote:

On Sat, Jan 27, 2018 at 05:14:26PM +0100, Linus Walleij wrote:

At one point we had discussed adding a 'dma-masters' property that
lists all the buses on which a device can be a dma master, and
the respective properties of those masters (iommu, coherency,
offset, ...).

IIRC at the time we decided that we could live without that complexity,
but perhaps we cannot.

Are you talking about this ?
https://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/dma/dma.txt#L41

It doesn't seem to be related to that issue to me. And in our
particular cases, all the devices are DMA masters, the RAM is just
mapped to another address.

No, that's not the one I was thinking of. The idea at the time was much
more generic, and not limited to dma engines. I don't recall the details,
but I think that Thierry was either involved or made the proposal at the
time.

Yeah, I vaguely remember discussing something like this before. A quick
search through my inbox yielded these two threads, mostly related to
IOMMU but I think there were some mentions about dma-ranges and so on as
well. I'll have to dig deeper into those threads to refresh my memories,
but I won't get around to it until later today.

If someone wants to read up on this in the meantime, here are the links:

https://lkml.org/lkml/2014/4/27/346
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/257200.html

From a quick glance the issue of dma-ranges was something that we hand-
waved at the time.

Also found this, which seems to be relevant as well:

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/252715.html

Adding Dave.

Thanks for the pointers, I started to read through it.

I guess we have to come up with two solutions here: a short term one
to address the users we already have in the kernel properly, and a
long term one where we could use that discussion as a starting point.

For the short term one, could we just set the device dma_pfn_offset to
PHYS_OFFSET at probe time, and use our dma_addr_t directly later on,
or would this also cause some issues?

That would certainly be an improvement over the current version,
it keeps the hack more localized. That's fine with me. Note that both
PHYS_OFFSET and dma_pfn_offset have architecture specific
meanings and they could in theory change, so ideally we'd do that
fixup somewhere in arch/arm/mach-sunxi/ at boot time before the
driver gets probed, but this wouldn't work on arm64 if we need it
there too.

dma_pfn_offset certainly *shouldn't* be architecture-specific; it's just
historically not been used consistently everywhere. That should now be
addressed by Christoph's big dma_direct_ops cleanup (now merged) which
fills in the places it was being missed in generic code. From quickly
skimming this thread, it sounds like that is really should be sufficient
for this particular hardware - if all its DMA goes through an
interconnect which makes RAM appear at 0, then you give it "dma-ranges =
<0 PHYS_OFFSET size>;" and things should 'just work' provided the DMA
API is otherwise being used correctly.

If different devices have differing DMA master address maps (such that
you can't just place a single dma-ranges on your "soc" node) then the
trick is to wrap them in intermediate "simple-bus" nodes with a
straight-though "ranges;" and the appropriate "dma-ranges = ...". Yes,
it's a bit of a bodge, but then pretending AXI/APB interconnects are
buses in the sense that DT assumes is already a bit of a bodge.

For the long term plan, from what I read from the discussion, it's
mostly centered around IOMMU indeed, and we don't have that. What we
would actually need is to break the assumption that the DMA "parent"
bus is the DT node's parent bus.

And I guess this could be done quite easily by adding a dma-parent
property with a phandle to the bus controller, that would have a
dma-ranges property of its own with the proper mapping described
there. It should be simple enough to support, but is there anything
that could prevent something like that to work properly (such as
preventing further IOMMU-related developments that were described in
those mail threads).

I've thought about it a little bit now. A dma-parent property would nicely
solve two problems:

- a device on a memory mapped

Re: [linux-sunxi] Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

2018-02-01 Thread Maxime Ripard

On Thu, Feb 01, 2018 at 11:34:43AM +, Liviu Dudau wrote:
> On Thu, Feb 01, 2018 at 10:20:28AM +0100, Arnd Bergmann wrote:
> > On Thu, Feb 1, 2018 at 9:32 AM, Maxime Ripard
> >  wrote:
> > > On Wed, Jan 31, 2018 at 02:47:53PM +, Liviu Dudau wrote:
> > >> On Wed, Jan 31, 2018 at 08:42:12AM +0100, Maxime Ripard wrote:
> > >> > On Wed, Jan 31, 2018 at 03:08:08AM +, Liviu Dudau wrote:
> > >> > > On Fri, Jan 26, 2018 at 11:00:41AM +0800, Yong wrote:
> > >>
> > >> Yeah, sorry, my threading of the discussion was broken and I've seen
> > >> the rest of the thread after I have replied. My bad!
> > >>
> > >> >
> > >> > In our case, the bus where the device is attached will not do the
> > >> > address translations, and shouldn't.
> > >>
> > >> In my view, the bus is already doing address translation at physical
> > >> level, AFAIU it remaps the memory to zero.
> > >
> > > Not really. It uses a separate bus with a different mapping for the
> > > DMA accesses (and only the DMA accesses). The AXI (or AHB, I'm not
> > > sure, but, well, the "registers" bus) doesn't remap anything in
> > > itself, and we only describe this one usually in our DTs.
> 
> I was actually thinking about the DMA bus (AXI bus, most likely), not the
> "registers" bus (yes, usually APB or AHB). The DMA bus is the one that does
> the implicit remapping for the addresses it uses, if I understood you 
> correctly.
> 
> > Exactly, the DT model fundamentally assumes that each a device is
> > connected to exactly one bus, so we make up a device *tree* rather
> > than a non-directed mesh topology that you might see in modern
> > SoCs.
> 
> I think you are right, but we also have the registers property for a device 
> node
> and that can be used for describing the "registers" bus. Now, it is possible
> that some driver code gets confused between accessing the device registers
> (which in Arm world happens through an APB or AHB bus) and the device doing 
> system
> read/writes which usually happends through an AXI (or for very old systems, 
> AHB) bus.
> 
> For the sake of making sure we are talking about the same thing and in hope
> that Maxime or Yong can give a more detailed picture of this device

Keep in mind that this part is heavily under-documented to us, and
this is mostly information collected through testing and reading
through the various vendor trees.

As far as I know, a few devices (Display Engine, hardware codec, the
CSI driver that spawned this discussion) are connected to the memory
through a proprietary bus that does the remapping. The registers part
is connected to an AHB bus.

> I'll re-iterate what a lot of devices in the Arm world look like
> nowadays:
> 
> - they have a bus for accessing the "registers" of the device, for controlling
>   the behaviour of that device. Inside the SoC, that happens through the
>   APB bus and it has a separate clock. The CPU has a view of those registers
>   through some mapping in the address space that has been backed by the 
> hardware
>   engineers at design time and in DT we express that through the "registers" 
> property,
>   plus the "apb_clk" for most of the bindings. In DT world we express the 
> mapping
>   vis-a-vis the parent bus by using the "ranges" property.

We do have that.

> - they have a high speed bus for doing data transfers. Inside the SoC that
>   happens through an AXI or more modern CCI interconnect bus. The CPU does not
>   have a direct view on those transfers, but by using IOMMUs, SMMUs or simple
>   bus mastering capabilities it can gain knowledge of the state of the 
> transfers
>   and/or influence the target memory.

Part of that bus controller allows to probe the bus bandwidth and / or
set limits for the various controllers connected to it, so it seems to
match that description.

>   In the DT world, we use the "dma-ranges" property like you say to
>   express the translations that happen on that bus.

Right, except that the way dma-ranges is parsed at the moment is that
it will look on the parent node for dma-ranges. In this case, the
parent in the DT will be our AHB bus, and not all the devices
connected on that AHB bus that do DMA go through that DMA bus with the
different mapping. So setting it there isn't an option.

> Maxime/Yong: does your device have more than one AXI bus for doing
> transfers?

Not as far as I know.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

[PATCH 2/2] media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil

2018-02-01 Thread Andy Yeh

From: Alan Chiang 

Dongwoon DW9807 is a voice coil lens driver.

Also add a vendor prefix for Dongwoon for one did not exist previously.

Signed-off-by: Andy Yeh 
---
 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 +
 1 file changed, 9 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt 
b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
new file mode 100644
index 000..0a1a860
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
@@ -0,0 +1,9 @@
+Dongwoon Anatech DW9807 voice coil lens driver
+
+DW9807 is a 10-bit DAC with current sink capability. It is intended for
+controlling voice coil lenses.
+
+Mandatory properties:
+
+- compatible: "dongwoon,dw9807"
+- reg: I2C slave address
-- 
2.7.4

[PATCH 0/2] DW9807 DT binding and driver patches

2018-02-01 Thread Andy Yeh

From: Alan Chiang 

Hi Sakari and Tomasz,

The two patches are the DT binding and driver for DW9807 VCM controller.

Alan Chiang (2):
  media: dw9807: Add dw9807 vcm driver
  media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil

 .../bindings/media/i2c/dongwoon,dw9807.txt |   9 +
 MAINTAINERS|   7 +
 drivers/media/i2c/Kconfig  |  10 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/dw9807.c | 320 +
 5 files changed, 347 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
 create mode 100644 drivers/media/i2c/dw9807.c

-- 
2.7.4

[PATCH v5 1/2] media: dw9807: Add dw9807 vcm driver

2018-02-01 Thread Andy Yeh

From: Alan Chiang 

DW9807 is a 10 bit DAC from Dongwoon, designed for linear
control of voice coil motor.

This driver creates a V4L2 subdevice and
provides control to set the desired focus.

Signed-off-by: Andy Yeh 
---
since v1:
- changed author.
since v2:
- addressed outstanding comments.
- enabled sequential write to update 2 registers in a single transaction.
since v3:
- addressed comments for v3.
- Remove redundant codes and declar some variables as constant variable.
- separate DT binding to another patch
since v4:
- Remove unnecessary typecast
- Put the temporary and loop variables in the end of the declaration

 MAINTAINERS|   7 +
 drivers/media/i2c/Kconfig  |  10 ++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/dw9807.c | 320 +
 4 files changed, 338 insertions(+)
 create mode 100644 drivers/media/i2c/dw9807.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 845fc25..a339bb5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4385,6 +4385,13 @@ T:   git git://linuxtv.org/media_tree.git
 S: Maintained
 F: drivers/media/i2c/dw9714.c
 
+DONGWOON DW9807 LENS VOICE COIL DRIVER
+M: Sakari Ailus 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/dw9807.c
+
 DOUBLETALK DRIVER
 M: "James R. Van Zandt" 
 L: blinux-l...@redhat.com
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cb5d7ff..fd01842 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -325,6 +325,16 @@ config VIDEO_DW9714
  capability. This is designed for linear control of
  voice coil motors, controlled via I2C serial interface.
 
+config VIDEO_DW9807
+   tristate "DW9807 lens voice coil support"
+   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on VIDEO_V4L2_SUBDEV_API
+   ---help---
+ This is a driver for the DW9807 camera lens voice coil.
+ DW9807 is a 10 bit DAC with 100mA output current sink
+ capability. This is designed for linear control of
+ voice coil motors, controlled via I2C serial interface.
+
 config VIDEO_SAA7110
tristate "Philips SAA7110 video decoder"
depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 548a9ef..1b62639 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
 obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
 obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
 obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
+obj-$(CONFIG_VIDEO_DW9807)  += dw9807.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
 obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o
diff --git a/drivers/media/i2c/dw9807.c b/drivers/media/i2c/dw9807.c
new file mode 100644
index 000..95626e9
--- /dev/null
+++ b/drivers/media/i2c/dw9807.c
@@ -0,0 +1,320 @@
+// Copyright (C) 2018 Intel Corporation
+// SPDX-License-Identifier: GPL-2.0
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DW9807_NAME"dw9807"
+#define DW9807_MAX_FOCUS_POS   1023
+/*
+ * This sets the minimum granularity for the focus positions.
+ * A value of 1 gives maximum accuracy for a desired focus position
+ */
+#define DW9807_FOCUS_STEPS 1
+/*
+ * This acts as the minimum granularity of lens movement.
+ * Keep this value power of 2, so the control steps can be
+ * uniformly adjusted for gradual lens movement, with desired
+ * number of control steps.
+ */
+#define DW9807_CTRL_STEPS  16
+#define DW9807_CTRL_DELAY_US   1000
+
+#define DW9807_CTL_ADDR0x02
+/*
+ * DW9807 separates two registers to control the VCM position.
+ * One for MSB value, another is LSB value.
+ */
+#define DW9807_MSB_ADDR0x03
+#define DW9807_LSB_ADDR0x04
+#define DW9807_STATUS_ADDR 0x05
+#define DW9807_MODE_ADDR   0x06
+#define DW9807_RESONANCE_ADDR  0x07
+
+#define MAX_RETRY  10
+
+struct dw9807_device {
+   struct v4l2_ctrl_handler ctrls_vcm;
+   struct v4l2_subdev sd;
+   u16 current_val;
+};
+
+static inline struct dw9807_device *sd_to_dw9807_vcm(struct v4l2_subdev 
*subdev)
+{
+   return container_of(subdev, struct dw9807_device, sd);
+}
+
+static int dw9807_i2c_check(struct i2c_client *client)
+{
+   const char status_addr = DW9807_STATUS_ADDR;
+   char status_result = 0x1;
+   int ret;
+
+   ret = i2c_master_send(client, (const char *)_addr, 
sizeof(status_addr));
+   if (ret != sizeof(status_addr)) {
+   dev_err(>dev, "I2C write STATUS address fail ret = 
%d\n",
+   ret);
+   return -EIO;
+   }
+
+   ret = i2c_master_recv(client, (char *)_result, 
sizeof(status_result));

Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

2018-02-01 Thread Arnd Bergmann

On Thu, Feb 1, 2018 at 4:29 PM, Maxime Ripard
 wrote:
> On Wed, Jan 31, 2018 at 10:37:37AM +0100, Arnd Bergmann wrote:
>> On Wed, Jan 31, 2018 at 8:29 AM, Maxime Ripard
>
>> I can think of a couple of other problems that may or may not be
>> relevant in the future that would require a more complex solution:
>>
>> - a device that is a bus master on more than one bus, e.g. a
>>   DMA engine that can copy between the CPU address space and
>>   another memory controller that is not visible to the CPU
>>
>> - a device that is connected to main memory both through an IOMMU
>>   and directly through its parent bus, and the device itself is in
>>   control over which of the two it uses (usually the IOMMU would
>>   contol whether a device is bypassing translation)
>>
>> - a device that has a single DMA address space with some form
>>   of non-linear mapping to one or more parent buses. Some of these
>>   can be expressed using the parent's dma-ranges properties, but
>>   our code currently only looks at the first entry in dma-ranges.
>
> As far as I know, we're in neither of these cases.

The point here was more about the general question of where we are
heading with the complexity of finding the right DMA settings. It's
already too complicated for anyone to fully understand what is
going on with DMA masks, offset, coherency etc when we look
at the existing DT bindings. Adding more complexity makes it
worse, so if anyone else is in need of a solution for the issues
above, we should try to accommodate their needs at the same time
to avoid adding more complexity now and again later on if we
can come up with a way that works for everyone now.

 Arnd

Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

2018-02-01 Thread Maxime Ripard

On Wed, Jan 31, 2018 at 10:37:37AM +0100, Arnd Bergmann wrote:
> On Wed, Jan 31, 2018 at 8:29 AM, Maxime Ripard
>  wrote:
> > Hi Thierry,
> >
> > On Tue, Jan 30, 2018 at 11:01:50AM +0100, Thierry Reding wrote:
> >> On Tue, Jan 30, 2018 at 10:59:16AM +0100, Thierry Reding wrote:
> >> > On Tue, Jan 30, 2018 at 10:24:48AM +0100, Arnd Bergmann wrote:
> >> > > On Tue, Jan 30, 2018 at 8:54 AM, Maxime Ripard
> >> > >  wrote:
> >> > > > On Mon, Jan 29, 2018 at 03:34:02PM +0100, Arnd Bergmann wrote:
> >> > > >> On Mon, Jan 29, 2018 at 10:25 AM, Linus Walleij
> >> > > >>  wrote:
> >> > > >> > On Mon, Jan 29, 2018 at 9:25 AM, Maxime Ripard
> >> > > >> >  wrote:
> >> > > >> >> On Sat, Jan 27, 2018 at 05:14:26PM +0100, Linus Walleij wrote:
> >> > >
> >> > > >>
> >> > > >> At one point we had discussed adding a 'dma-masters' property that
> >> > > >> lists all the buses on which a device can be a dma master, and
> >> > > >> the respective properties of those masters (iommu, coherency,
> >> > > >> offset, ...).
> >> > > >>
> >> > > >> IIRC at the time we decided that we could live without that 
> >> > > >> complexity,
> >> > > >> but perhaps we cannot.
> >> > > >
> >> > > > Are you talking about this ?
> >> > > > https://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/dma/dma.txt#L41
> >> > > >
> >> > > > It doesn't seem to be related to that issue to me. And in our
> >> > > > particular cases, all the devices are DMA masters, the RAM is just
> >> > > > mapped to another address.
> >> > >
> >> > > No, that's not the one I was thinking of. The idea at the time was much
> >> > > more generic, and not limited to dma engines. I don't recall the 
> >> > > details,
> >> > > but I think that Thierry was either involved or made the proposal at 
> >> > > the
> >> > > time.
> >> >
> >> > Yeah, I vaguely remember discussing something like this before. A quick
> >> > search through my inbox yielded these two threads, mostly related to
> >> > IOMMU but I think there were some mentions about dma-ranges and so on as
> >> > well. I'll have to dig deeper into those threads to refresh my memories,
> >> > but I won't get around to it until later today.
> >> >
> >> > If someone wants to read up on this in the meantime, here are the links:
> >> >
> >> > https://lkml.org/lkml/2014/4/27/346
> >> > 
> >> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/257200.html
> >> >
> >> > From a quick glance the issue of dma-ranges was something that we hand-
> >> > waved at the time.
> >>
> >> Also found this, which seems to be relevant as well:
> >>
> >>   
> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/252715.html
> >>
> >> Adding Dave.
> >
> > Thanks for the pointers, I started to read through it.
> >
> > I guess we have to come up with two solutions here: a short term one
> > to address the users we already have in the kernel properly, and a
> > long term one where we could use that discussion as a starting point.
> >
> > For the short term one, could we just set the device dma_pfn_offset to
> > PHYS_OFFSET at probe time, and use our dma_addr_t directly later on,
> > or would this also cause some issues?
> 
> That would certainly be an improvement over the current version,
> it keeps the hack more localized. That's fine with me.

Ok, we'll do that in that driver and convert the existing drivers
then.

> Note that both PHYS_OFFSET and dma_pfn_offset have architecture
> specific meanings and they could in theory change, so ideally we'd
> do that fixup somewhere in arch/arm/mach-sunxi/ at boot time before
> the driver gets probed, but this wouldn't work on arm64 if we need
> it there too.

Unfortunately, we do :/

> > For the long term plan, from what I read from the discussion, it's
> > mostly centered around IOMMU indeed, and we don't have that. What we
> > would actually need is to break the assumption that the DMA "parent"
> > bus is the DT node's parent bus.
> >
> > And I guess this could be done quite easily by adding a dma-parent
> > property with a phandle to the bus controller, that would have a
> > dma-ranges property of its own with the proper mapping described
> > there. It should be simple enough to support, but is there anything
> > that could prevent something like that to work properly (such as
> > preventing further IOMMU-related developments that were described in
> > those mail threads).
> 
> I've thought about it a little bit now. A dma-parent property would nicely
> solve two problems:
> 
> - a device on a memory mapped control bus that is a bus master on
>   a different bus. This is the case we are talking about here AFAICT
> 
> - a device that is on a different kind of bus (i2c, spi, usb, ...) but also
>   happens to be a dma master on another bus. I suspect we have
>   some of these today and they work by accident because

[PATCH] media: ov5640: fix virtual_channel parameter permissions

2018-02-01 Thread Hugues Fruchet

Fix module_param(virtual_channel) permissions.
This problem was detected by checkpatch:
$ scripts/checkpatch.pl -f drivers/media/i2c/ov5640.c
ERROR: Use 4 digit octal (0777) not decimal permissions
#131: FILE: drivers/media/i2c/ov5640.c:131:
+module_param(virtual_channel, int, 0);

Also explicitly set initial value to 0 for default value
and add an error trace in case of virtual_channel not in
the valid range of values.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/i2c/ov5640.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 696a28b..906f202 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -127,8 +127,8 @@ struct ov5640_pixfmt {
  * FIXME: remove this when a subdev API becomes available
  * to set the MIPI CSI-2 virtual channel.
  */
-static unsigned int virtual_channel;
-module_param(virtual_channel, int, 0);
+static unsigned int virtual_channel = 0;
+module_param(virtual_channel, int, 0444);
 MODULE_PARM_DESC(virtual_channel,
 "MIPI CSI-2 virtual channel (0..3), default 0");
 
@@ -1358,11 +1358,15 @@ static int ov5640_binning_on(struct ov5640_dev *sensor)
 
 static int ov5640_set_virtual_channel(struct ov5640_dev *sensor)
 {
+   struct i2c_client *client = sensor->i2c_client;
u8 temp, channel = virtual_channel;
int ret;
 
-   if (channel > 3)
+   if (channel > 3) {
+   dev_err(>dev, "%s: wrong virtual_channel parameter 
value, expected (0..3), got %d\n",
+   __func__, channel);
return -EINVAL;
+   }
 
ret = ov5640_read_reg(sensor, OV5640_REG_DEBUG_MODE, );
if (ret)
-- 
1.9.1

Re: [linux-sunxi] Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

2018-02-01 Thread Liviu Dudau

On Thu, Feb 01, 2018 at 10:20:28AM +0100, Arnd Bergmann wrote:
> On Thu, Feb 1, 2018 at 9:32 AM, Maxime Ripard
>  wrote:
> > On Wed, Jan 31, 2018 at 02:47:53PM +, Liviu Dudau wrote:
> >> On Wed, Jan 31, 2018 at 08:42:12AM +0100, Maxime Ripard wrote:
> >> > On Wed, Jan 31, 2018 at 03:08:08AM +, Liviu Dudau wrote:
> >> > > On Fri, Jan 26, 2018 at 11:00:41AM +0800, Yong wrote:
> >>
> >> Yeah, sorry, my threading of the discussion was broken and I've seen
> >> the rest of the thread after I have replied. My bad!
> >>
> >> >
> >> > In our case, the bus where the device is attached will not do the
> >> > address translations, and shouldn't.
> >>
> >> In my view, the bus is already doing address translation at physical
> >> level, AFAIU it remaps the memory to zero.
> >
> > Not really. It uses a separate bus with a different mapping for the
> > DMA accesses (and only the DMA accesses). The AXI (or AHB, I'm not
> > sure, but, well, the "registers" bus) doesn't remap anything in
> > itself, and we only describe this one usually in our DTs.

I was actually thinking about the DMA bus (AXI bus, most likely), not the
"registers" bus (yes, usually APB or AHB). The DMA bus is the one that does
the implicit remapping for the addresses it uses, if I understood you correctly.

> 
> Exactly, the DT model fundamentally assumes that each a device is
> connected to exactly one bus, so we make up a device *tree* rather
> than a non-directed mesh topology that you might see in modern
> SoCs.

I think you are right, but we also have the registers property for a device node
and that can be used for describing the "registers" bus. Now, it is possible
that some driver code gets confused between accessing the device registers
(which in Arm world happens through an APB or AHB bus) and the device doing 
system
read/writes which usually happends through an AXI (or for very old systems, 
AHB) bus.

For the sake of making sure we are talking about the same thing and in hope
that Maxime or Yong can give a more detailed picture of this device, I'll
re-iterate what a lot of devices in the Arm world look like nowadays:

- they have a bus for accessing the "registers" of the device, for controlling
  the behaviour of that device. Inside the SoC, that happens through the
  APB bus and it has a separate clock. The CPU has a view of those registers
  through some mapping in the address space that has been backed by the hardware
  engineers at design time and in DT we express that through the "registers" 
property,
  plus the "apb_clk" for most of the bindings. In DT world we express the 
mapping
  vis-a-vis the parent bus by using the "ranges" property.

- they have a high speed bus for doing data transfers. Inside the SoC that
  happens through an AXI or more modern CCI interconnect bus. The CPU does not
  have a direct view on those transfers, but by using IOMMUs, SMMUs or simple
  bus mastering capabilities it can gain knowledge of the state of the transfers
  and/or influence the target memory. In the DT world, we use the "dma-ranges"
  property like you say to express the translations that happen on that bus.

Maxime/Yong: does your device have more than one AXI bus for doing transfers?


> 
> The "dma-ranges" property was introduced when this first started
> falling apart and we got devices that had a different translation
> in DMA direction compared to control direction (i.e. the "ranges"
> property), but that still assumed that every device on a given bus
> had the same view of DMA space.
> 
> With just "dma-ranges", we could easy deal with a topology where
> each DMA master on an AXI bus sees main memory at address
> zero but the CPU sees the same memory at a high address while
> seeing the MMIO ranges at a low address.
> 
> What we cannot represent is multiple masters on the same
> AXI bus that use a different translation. Making up arbitrary
> intermediate buses would get this to work, but would likely
> cause other problems in the future when we do something
> else that relies on having a correct representation of the
> hierarchy of all the AXI/AHB/APB buses in the system, such
> as doing per-bus bandwidth allocation for child devices or
> anything else that requires configuring the bus bridge itself.

Agree we cannot express multiple masters on the same AXI bus having
different translations. Maybe we need to try to make things look more like
PCI where the child busses can have their own view of the world as long as
they don't try to communicate upwards to their parents or sideways to sibling
busses?


> 
> >> What you (we?) need is a simple bus driver that registers the
> >> correct virt_to_bus()/bus_to_virt() hooks for the device that do
> >> this translation at the DMA API level as well.
> >
> > Like I said, this only impact DMA transfers, and not the registers
> > accesses. We have other devices sitting on the same bus that do not
> > perform DMA accesses through that special memory bus

Kredit? so schnell wie möglich?

2018-02-01 Thread Martin Kelly

Kredit? so schnell wie möglich? unkompliziert und seriös ? Bei uns genau 
richtig. Wir arbeiten europaweit. Wir vermitteln Kredite und Darlehen zu fairen 
Konditionen. Durch unsere seriöse, kompetente und ehrliche Kreditberatung haben 
wir über Jahre eine starke Position auf dem Markt.

Re: [linux-sunxi] Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

2018-02-01 Thread Arnd Bergmann

On Thu, Feb 1, 2018 at 9:32 AM, Maxime Ripard
 wrote:
> On Wed, Jan 31, 2018 at 02:47:53PM +, Liviu Dudau wrote:
>> On Wed, Jan 31, 2018 at 08:42:12AM +0100, Maxime Ripard wrote:
>> > On Wed, Jan 31, 2018 at 03:08:08AM +, Liviu Dudau wrote:
>> > > On Fri, Jan 26, 2018 at 11:00:41AM +0800, Yong wrote:
>>
>> Yeah, sorry, my threading of the discussion was broken and I've seen
>> the rest of the thread after I have replied. My bad!
>>
>> >
>> > In our case, the bus where the device is attached will not do the
>> > address translations, and shouldn't.
>>
>> In my view, the bus is already doing address translation at physical
>> level, AFAIU it remaps the memory to zero.
>
> Not really. It uses a separate bus with a different mapping for the
> DMA accesses (and only the DMA accesses). The AXI (or AHB, I'm not
> sure, but, well, the "registers" bus) doesn't remap anything in
> itself, and we only describe this one usually in our DTs.

Exactly, the DT model fundamentally assumes that each a device is
connected to exactly one bus, so we make up a device *tree* rather
than a non-directed mesh topology that you might see in modern
SoCs.

The "dma-ranges" property was introduced when this first started
falling apart and we got devices that had a different translation
in DMA direction compared to control direction (i.e. the "ranges"
property), but that still assumed that every device on a given bus
had the same view of DMA space.

With just "dma-ranges", we could easy deal with a topology where
each DMA master on an AXI bus sees main memory at address
zero but the CPU sees the same memory at a high address while
seeing the MMIO ranges at a low address.

What we cannot represent is multiple masters on the same
AXI bus that use a different translation. Making up arbitrary
intermediate buses would get this to work, but would likely
cause other problems in the future when we do something
else that relies on having a correct representation of the
hierarchy of all the AXI/AHB/APB buses in the system, such
as doing per-bus bandwidth allocation for child devices or
anything else that requires configuring the bus bridge itself.

>> What you (we?) need is a simple bus driver that registers the
>> correct virt_to_bus()/bus_to_virt() hooks for the device that do
>> this translation at the DMA API level as well.
>
> Like I said, this only impact DMA transfers, and not the registers
> accesses. We have other devices sitting on the same bus that do not
> perform DMA accesses through that special memory bus and will not have
> that mapping changed.

virt_to_bus()/bus_to_virt() don't actually exist on modern platforms any
more, but when they did, they were only about dma access, not
mmio access, so they would correspond to what we do with
'dma-ranges' rather than 'ranges'.

Arnd

[ragnatech:media-tree] BUILD SUCCESS 273caa260035c03d89ad63d72d8cd3d9e5c5e3f1

2018-02-01 Thread kbuild test robot

tree/branch: git://git.ragnatech.se/linux  media-tree
branch HEAD: 273caa260035c03d89ad63d72d8cd3d9e5c5e3f1  media: 
v4l2-compat-ioctl32.c: make ctrl_is_pointer work for subdevs

elapsed time: 283m

configs tested: 131

The following configs have been built successfully.
More configs may be tested in the coming days.

x86_64 acpi-redef
x86_64   allyesdebian
x86_64nfsroot
i386   randconfig-x019-201804
i386   randconfig-x010-201804
i386   randconfig-x015-201804
i386   randconfig-x014-201804
i386   randconfig-x012-201804
i386   randconfig-x011-201804
i386   randconfig-x017-201804
i386   randconfig-x016-201804
i386   randconfig-x018-201804
i386   randconfig-x013-201804
i386 randconfig-n0-201804
x86_64 randconfig-x008-201804
x86_64 randconfig-x000-201804
x86_64 randconfig-x005-201804
x86_64 randconfig-x002-201804
x86_64 randconfig-x007-201804
x86_64 randconfig-x009-201804
x86_64 randconfig-x006-201804
x86_64 randconfig-x001-201804
x86_64 randconfig-x003-201804
x86_64 randconfig-x004-201804
i386 randconfig-i0-201804
i386 randconfig-i1-201804
alpha   defconfig
pariscallnoconfig
parisc b180_defconfig
pariscc3000_defconfig
parisc  defconfig
arm   allnoconfig
arm at91_dt_defconfig
arm   efm32_defconfig
arm  exynos_defconfig
armmulti_v5_defconfig
armmulti_v7_defconfig
armshmobile_defconfig
arm   sunxi_defconfig
arm64 allnoconfig
arm64   defconfig
blackfinBF526-EZBRD_defconfig
blackfinBF533-EZKIT_defconfig
blackfinBF561-EZKIT-SMP_defconfig
blackfin  TCM-BF537_defconfig
cris etrax-100lx_v2_defconfig
powerpc  katmai_defconfig
sparc64  allyesconfig
x86_64 randconfig-g0-02011358
frv defconfig
mn10300 asb2364_defconfig
openriscor1ksim_defconfig
tile tilegx_defconfig
um i386_defconfig
um   x86_64_defconfig
i386 allmodconfig
i386 randconfig-a0-201804
i386 randconfig-a1-201804
x86_64 randconfig-s0-02011538
x86_64 randconfig-s1-02011538
x86_64 randconfig-s2-02011538
x86_64 randconfig-x010-201804
x86_64 randconfig-x011-201804
x86_64 randconfig-x012-201804
x86_64 randconfig-x013-201804
x86_64 randconfig-x014-201804
x86_64 randconfig-x015-201804
x86_64 randconfig-x016-201804
x86_64 randconfig-x017-201804
x86_64 randconfig-x018-201804
x86_64 randconfig-x019-201804
i386 alldefconfig
i386  allnoconfig
i386defconfig
m68k   m5475evb_defconfig
m68k  multi_defconfig
m68k   sun3_defconfig
i386 randconfig-s0-201804
i386 randconfig-s1-201804
x86_64 randconfig-s3-02011549
x86_64 randconfig-s4-02011549
x86_64 randconfig-s5-02011549
arm  colibri_pxa270_defconfig
mips   jazz_defconfig
shdreamcast_defconfig
x86_64 randconfig-u0-02011532
powerpc   allnoconfig
powerpc defconfig
powerpc   ppc64_defconfig
s390default_defconfig
microblaze  mmu_defconfig
microblazenommu_defconfig
sparc   defconfig
sparc64   allnoconfig
sparc64 defconfig
c6xevmc6678_defconfig
h8300h8300h-sim_defconfig
m32r   m32104ut_defconfig
m32r mappi3.smp_defconfig
m32r opsput_defconfig
m32r   usrv_defconfig
nios2

[PATCH v2] media: ov5640: various typo & style fixes

2018-02-01 Thread Hugues Fruchet

Various typo & style fixes either detected by code
review or checkpatch.

Signed-off-by: Hugues Fruchet 
---
version 2:
   As per Sakari's review comment:
  - Drop module_param() permission update, will be pushed into a new commit
  - Drop changes related to checkpatch "CHECK: Lines should not end with a '('
style result is worse than before

 drivers/media/i2c/ov5640.c | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 6f18460..696a28b 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -14,14 +14,14 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
-#include 
-#include 
 #include 
 #include 
 #include 
@@ -139,7 +139,7 @@ struct ov5640_pixfmt {
 
 /* regulator supplies */
 static const char * const ov5640_supply_name[] = {
-   "DOVDD", /* Digital I/O (1.8V) suppply */
+   "DOVDD", /* Digital I/O (1.8V) supply */
"DVDD",  /* Digital Core (1.5V) supply */
"AVDD",  /* Analog (2.8V) supply */
 };
@@ -245,7 +245,6 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
v4l2_ctrl *ctrl)
  */
 
 static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
-
{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
{0x3103, 0x03, 0, 0}, {0x3017, 0x00, 0, 0}, {0x3018, 0x00, 0, 0},
{0x3034, 0x18, 0, 0}, {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0},
@@ -334,7 +333,6 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
v4l2_ctrl *ctrl)
 };
 
 static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
-
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
@@ -377,7 +375,6 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
v4l2_ctrl *ctrl)
 };
 
 static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
-
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
@@ -484,6 +481,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
v4l2_ctrl *ctrl)
{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
+
 static const struct reg_value ov5640_setting_15fps_QCIF_176_144[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
@@ -886,7 +884,7 @@ static int ov5640_read_reg16(struct ov5640_dev *sensor, u16 
reg, u16 *val)
ret = ov5640_read_reg(sensor, reg, );
if (ret)
return ret;
-   ret = ov5640_read_reg(sensor, reg+1, );
+   ret = ov5640_read_reg(sensor, reg + 1, );
if (ret)
return ret;
 
@@ -947,7 +945,7 @@ static int ov5640_load_regs(struct ov5640_dev *sensor,
break;
 
if (delay_ms)
-   usleep_range(1000*delay_ms, 1000*delay_ms+100);
+   usleep_range(1000 * delay_ms, 1000 * delay_ms + 100);
}
 
return ret;
@@ -1289,7 +1287,6 @@ static int ov5640_set_bandingfilter(struct ov5640_dev 
*sensor)
return ret;
prev_vts = ret;
 
-
/* calculate banding filter */
/* 60Hz */
band_step60 = sensor->prev_sysclk * 100 / sensor->prev_hts * 100 / 120;
@@ -1405,8 +1402,8 @@ static int ov5640_set_virtual_channel(struct ov5640_dev 
*sensor)
  * sensor changes between scaling and subsampling, go through
  * exposure calculation
  */
-static int ov5640_set_mode_exposure_calc(
-   struct ov5640_dev *sensor, const struct ov5640_mode_info *mode)
+static int ov5640_set_mode_exposure_calc(struct ov5640_dev *sensor,
+const struct ov5640_mode_info *mode)
 {
u32 prev_shutter, prev_gain16;
u32 cap_shutter, cap_gain16;
@@ -1416,7 +1413,7 @@ static int ov5640_set_mode_exposure_calc(
u8 average;
int ret;
 
-   if (mode->reg_data == NULL)
+   if (!mode->reg_data)
return -EINVAL;
 
/* read preview shutter */
@@ -1570,7 +1567,7 @@ static int ov5640_set_mode_direct(struct ov5640_dev 
*sensor,
 {
int ret;
 
-   if (mode->reg_data == NULL)
+   if (!mode->reg_data)
return -EINVAL;
 
/* Write capture setting */
@@ -2117,7 +2114,8 @@ static int ov5640_set_ctrl_gain(struct ov5640_dev 
*sensor, int auto_gain)
 
if (ctrls->auto_gain->is_new) {
ret = ov5640_mod_reg(sensor, OV5640_REG_AEC_PK_MANUAL,
-BIT(1), ctrls->auto_gain->val ? 0

Re: [linux-sunxi] Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

2018-02-01 Thread Maxime Ripard

On Wed, Jan 31, 2018 at 02:47:53PM +, Liviu Dudau wrote:
> On Wed, Jan 31, 2018 at 08:42:12AM +0100, Maxime Ripard wrote:
> > On Wed, Jan 31, 2018 at 03:08:08AM +, Liviu Dudau wrote:
> > > On Fri, Jan 26, 2018 at 11:00:41AM +0800, Yong wrote:
> > > > Hi Maxime,
> > > > 
> > > > On Fri, 26 Jan 2018 09:46:58 +0800
> > > > Yong  wrote:
> > > > 
> > > > > Hi Maxime,
> > > > > 
> > > > > Do you have any experience in solving this problem?
> > > > > It seems the PHYS_OFFSET maybe undeclared when the ARCH is not arm.
> > > > 
> > > > Got it.
> > > > Should I add 'depends on ARM' in Kconfig?
> > > 
> > > No, I don't think you should do that, you should fix the code.
> > > 
> > > The dma_addr_t addr that you've got is ideally coming from 
> > > dma_alloc_coherent(),
> > > in which case the addr is already "suitable" for use by the device 
> > > (because the
> > > bus where the device is attached to does all the address translations).
> > 
> > Like we're discussing in that other part of the thread with Thierry
> > and Arnd, things are slightly more complicated than that :)
> 
> Yeah, sorry, my threading of the discussion was broken and I've seen
> the rest of the thread after I have replied. My bad!
> 
> > 
> > In our case, the bus where the device is attached will not do the
> > address translations, and shouldn't.
> 
> In my view, the bus is already doing address translation at physical
> level, AFAIU it remaps the memory to zero.

Not really. It uses a separate bus with a different mapping for the
DMA accesses (and only the DMA accesses). The AXI (or AHB, I'm not
sure, but, well, the "registers" bus) doesn't remap anything in
itself, and we only describe this one usually in our DTs.

> What you (we?) need is a simple bus driver that registers the
> correct virt_to_bus()/bus_to_virt() hooks for the device that do
> this translation at the DMA API level as well.

Like I said, this only impact DMA transfers, and not the registers
accesses. We have other devices sitting on the same bus that do not
perform DMA accesses through that special memory bus and will not have
that mapping changed.

> > > If you apply PHYS_OFFSET forcefully to it you might get unexpected
> > > results.
> > 
> > Out of curiosity, what would be these unexpected results?
> 
> If in the future (or a parallel world setup) the device is sitting
> behind an IOMMU, the addr value might well be smaller than
> PHYS_OFFSET and you will under-wrap, possibly starting to hit kernel
> physical addresses (or anything sitting at the top of the physical
> memory).
> 
> From my time playing with IOMMUs and PCI domains, I've learned to
> treat the dma_addr_t as a cookie value and never try to do
> arithmetics with it.

I've never worked with PCI or IOMMU, so I tend to overlook them, but
yeah, you're right :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [RFC PATCH 1/9] media: add request API core and UAPI

Re: [RFC PATCH 1/9] media: add request API core and UAPI

Re: [RFCv2 05/17] media: Document the media request API

cron job: media_tree daily build: ERRORS

[PATCH v2 2/9] em28xx: Bulk transfer implementation fix

Good Morning !

Re: Regression in VB2 alloc prepare/finish balancing with em28xx/au0828

Re: Regression in VB2 alloc prepare/finish balancing with em28xx/au0828

Re: [RFCv2 05/17] media: Document the media request API

RE: [PATCH v5 4/5] media: ov5640: add support of DVP parallel interface

Re: [PATCH] media: ov5640: fix virtual_channel parameter permissions

Respond for details

Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

Re: [linux-sunxi] Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

[PATCH 2/2] media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil

[PATCH 0/2] DW9807 DT binding and driver patches

[PATCH v5 1/2] media: dw9807: Add dw9807 vcm driver

Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

[PATCH] media: ov5640: fix virtual_channel parameter permissions

Re: [linux-sunxi] Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

Kredit? so schnell wie möglich?

Re: [linux-sunxi] Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

[ragnatech:media-tree] BUILD SUCCESS 273caa260035c03d89ad63d72d8cd3d9e5c5e3f1

[PATCH v2] media: ov5640: various typo & style fixes

Re: [linux-sunxi] Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

26 matches

Site Navigation

Mail list logo

Footer information