[PATCH] media: marvel-ccic: mmp: select VIDEOBUF2_VMALLOC/DMA_CONTIG

[Arnd Bergmann]

Testing randconfig builds after the return of the mmp ccic driver shows
a link error in some configurations:

drivers/media/platform/marvell-ccic/mcam-core.o: In function `mccic_register':
mcam-core.c:(.text+0x2e48): undefined reference to `vb2_dma_contig_memops'

A closer look at the mcam-core.c file reveals that we need to select
both VIDEOBUF2_DMA_CONTIG and VIDEOBUF2_VMALLOC, as already do for
VIDEO_CAFE_CCIC.

Fixes: 0a9c643c8faa ("media: marvel-ccic: re-enable mmp-driver build")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/platform/marvell-ccic/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/marvell-ccic/Kconfig 
b/drivers/media/platform/marvell-ccic/Kconfig
index 21dacef7c2fc..13cc6f2159d3 100644
--- a/drivers/media/platform/marvell-ccic/Kconfig
+++ b/drivers/media/platform/marvell-ccic/Kconfig
@@ -18,6 +18,8 @@ config VIDEO_MMP_CAMERA
depends on ARCH_MMP || COMPILE_TEST
select VIDEO_OV7670
select I2C_GPIO
+   select VIDEOBUF2_VMALLOC
+   select VIDEOBUF2_DMA_CONTIG
select VIDEOBUF2_DMA_SG
---help---
  This is a Video4Linux2 driver for the integrated camera
-- 
2.9.0

[PATCH 5/5] media: omap2: fix compile-testing with FB_OMAP2=m

[Arnd Bergmann]

Compile-testing with FB_OMAP2=m results in a link error:

drivers/media/platform/omap/omap_vout.o: In function `vidioc_streamoff':
omap_vout.c:(.text+0x1028): undefined reference to `omap_dispc_unregister_isr'
drivers/media/platform/omap/omap_vout.o: In function `omap_vout_release':
omap_vout.c:(.text+0x1330): undefined reference to `omap_dispc_unregister_isr'
drivers/media/platform/omap/omap_vout.o: In function `vidioc_streamon':
omap_vout.c:(.text+0x2dd4): undefined reference to `omap_dispc_register_isr'
drivers/media/platform/omap/omap_vout.o: In function `omap_vout_remove':

In order to enable compile-testing but still keep the correct dependency,
this changes the Kconfig logic so we only allow CONFIG_COMPILE_TEST
building when FB_OMAP is completely disabled, or have use the old
dependency on FB_OMAP to ensure VIDEO_OMAP2_VOUT is also a loadable
module when FB_OMAP2 is.

Fixes: d8555fd2f452 ("media: omap2: allow building it with COMPILE_TEST && 
DRM_OMAP")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/platform/omap/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/omap/Kconfig 
b/drivers/media/platform/omap/Kconfig
index a414bcbb9b08..d827b6c285a6 100644
--- a/drivers/media/platform/omap/Kconfig
+++ b/drivers/media/platform/omap/Kconfig
@@ -6,7 +6,7 @@ config VIDEO_OMAP2_VOUT_VRFB
 config VIDEO_OMAP2_VOUT
tristate "OMAP2/OMAP3 V4L2-Display driver"
depends on MMU
-   depends on FB_OMAP2 || COMPILE_TEST
+   depends on FB_OMAP2 || (COMPILE_TEST && FB_OMAP2=n)
depends on ARCH_OMAP2 || ARCH_OMAP3 || COMPILE_TEST
select VIDEOBUF_GEN
select VIDEOBUF_DMA_CONTIG
-- 
2.9.0

[PATCH 4/5] media: marvel-ccic: allow ccic and mmp drivers to coexist

[Arnd Bergmann]

Randconfig builds fail when one of the two is a built-in driver and
the other one is a loadable module:

drivers/media/platform/marvell-ccic/mcam-core.o: In function `mccic_register':
mcam-core.c:(.text+0x2594): undefined reference to `__this_module'
drivers/media/platform/marvell-ccic/mcam-core.o:(.rodata+0x50): undefined 
reference to `__this_module'

The problem is that mcam-core.c can not be built both ways at the smae
time. However, we can make kbuild take care of that by making the core
driver a separate module, which can be either built-in or loadable
as needed.
Making it a separate module requires exporting a few symbols and
adding the module license from the header.

Fixes: 0a9c643c8faa ("media: marvel-ccic: re-enable mmp-driver build")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/platform/marvell-ccic/Makefile| 9 -
 drivers/media/platform/marvell-ccic/mcam-core.c | 9 -
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/Makefile 
b/drivers/media/platform/marvell-ccic/Makefile
index 05a792c579a2..b3a4d0cdccb8 100644
--- a/drivers/media/platform/marvell-ccic/Makefile
+++ b/drivers/media/platform/marvell-ccic/Makefile
@@ -1,6 +1,5 @@
-obj-$(CONFIG_VIDEO_CAFE_CCIC) += cafe_ccic.o
-cafe_ccic-y := cafe-driver.o mcam-core.o
-
-obj-$(CONFIG_VIDEO_MMP_CAMERA) += mmp_camera.o
-mmp_camera-y := mmp-driver.o mcam-core.o
+obj-$(CONFIG_VIDEO_CAFE_CCIC) += cafe_ccic.o mcam-core.o
+cafe_ccic-y := cafe-driver.o
 
+obj-$(CONFIG_VIDEO_MMP_CAMERA) += mmp_camera.o mcam-core.o
+mmp_camera-y := mmp-driver.o
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c 
b/drivers/media/platform/marvell-ccic/mcam-core.c
index 80670142..dfdbd4354b74 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1720,6 +1720,7 @@ int mccic_irq(struct mcam_camera *cam, unsigned int irqs)
}
return handled;
 }
+EXPORT_SYMBOL_GPL(mccic_irq);
 
 /* -- */
 /*
@@ -1830,7 +1831,7 @@ int mccic_register(struct mcam_camera *cam)
v4l2_device_unregister(>v4l2_dev);
return ret;
 }
-
+EXPORT_SYMBOL_GPL(mccic_register);
 
 void mccic_shutdown(struct mcam_camera *cam)
 {
@@ -1850,6 +1851,7 @@ void mccic_shutdown(struct mcam_camera *cam)
v4l2_ctrl_handler_free(>ctrl_handler);
v4l2_device_unregister(>v4l2_dev);
 }
+EXPORT_SYMBOL_GPL(mccic_shutdown);
 
 /*
  * Power management
@@ -1868,6 +1870,7 @@ void mccic_suspend(struct mcam_camera *cam)
}
mutex_unlock(>s_mutex);
 }
+EXPORT_SYMBOL_GPL(mccic_suspend);
 
 int mccic_resume(struct mcam_camera *cam)
 {
@@ -1898,4 +1901,8 @@ int mccic_resume(struct mcam_camera *cam)
}
return ret;
 }
+EXPORT_SYMBOL_GPL(mccic_resume);
 #endif /* CONFIG_PM */
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Jonathan Corbet <cor...@lwn.net>");
-- 
2.9.0

[PATCH 3/5] media: cx231xx: fix RC_CORE dependency

[Arnd Bergmann]

With CONFIG_RC_CORE=m and VIDEO_CX231XX=y, we get a link failure:

drivers/media/usb/cx231xx/cx231xx-input.o: In function `cx231xx_ir_init':
cx231xx-input.c:(.text+0xd4): undefined reference to `rc_allocate_device'

This narrows down the dependency so that only valid configurations
are allowed.

Fixes: 84545d2a1436 ("media: cx231xx: Remove RC_CORE dependency")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/usb/cx231xx/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/cx231xx/Kconfig 
b/drivers/media/usb/cx231xx/Kconfig
index 0f13192634c7..9e5b3e7c3ef5 100644
--- a/drivers/media/usb/cx231xx/Kconfig
+++ b/drivers/media/usb/cx231xx/Kconfig
@@ -15,7 +15,7 @@ config VIDEO_CX231XX
 
 config VIDEO_CX231XX_RC
bool "Conexant cx231xx Remote Controller additional support"
-   depends on RC_CORE
+   depends on RC_CORE=y || RC_CORE=VIDEO_CX231XX
depends on VIDEO_CX231XX
default y
---help---
-- 
2.9.0

[PATCH 2/5] media: v4l: cadence: include linux/slab.h

[Arnd Bergmann]

I ran into a randconfig build error with the new driver:

drivers/media/platform/cadence/cdns-csi2tx.c: In function 'csi2tx_probe':
drivers/media/platform/cadence/cdns-csi2tx.c:477:11: error: implicit 
declaration of function 'kzalloc'; did you mean 'd_alloc'? 
[-Werror=implicit-function-declaration]

kzalloc() is declared in linux/slab.h, so let's include this to make it
build in all configurations.

Fixes: 84b477e6d4bc ("media: v4l: cadence: Add Cadence MIPI-CSI2 TX driver")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/platform/cadence/cdns-csi2rx.c | 1 +
 drivers/media/platform/cadence/cdns-csi2tx.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c 
b/drivers/media/platform/cadence/cdns-csi2rx.c
index a0f02916006b..43e43c7b3e98 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
diff --git a/drivers/media/platform/cadence/cdns-csi2tx.c 
b/drivers/media/platform/cadence/cdns-csi2tx.c
index dfa1d88d955b..40d0de690ff4 100644
--- a/drivers/media/platform/cadence/cdns-csi2tx.c
+++ b/drivers/media/platform/cadence/cdns-csi2tx.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
-- 
2.9.0

[PATCH 1/5] media: omap3isp: fix warning for !CONFIG_PM

[Arnd Bergmann]

The final version of the COMPILE_TEST patch for this driver missed
one warning about suspend/resume functions that can now appear
on platforms that don't always set CONFIG_PM:

drivers/media/platform/omap3isp/isp.c:1008:13: error: 'isp_resume_modules' 
defined but not used [-Werror=unused-function]
 static void isp_resume_modules(struct isp_device *isp)
 ^~
drivers/media/platform/omap3isp/isp.c:974:12: error: 'isp_suspend_modules' 
defined but not used [-Werror=unused-function]
 static int isp_suspend_modules(struct isp_device *isp)

This marks the respective functions as __maybe_unused as an easy
workaround.

Fixes: 243131134be4 ("media: omap3isp: Allow it to build with COMPILE_TEST")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/platform/omap3isp/isp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index f22cf351e3ee..a658c12eead1 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -971,7 +971,7 @@ static void isp_resume_module_pipeline(struct media_entity 
*me)
  * Returns 0 if suspend left in idle state all the submodules properly,
  * or returns 1 if a general Reset is required to suspend the submodules.
  */
-static int isp_suspend_modules(struct isp_device *isp)
+static int __maybe_unused isp_suspend_modules(struct isp_device *isp)
 {
unsigned long timeout;
 
@@ -1005,7 +1005,7 @@ static int isp_suspend_modules(struct isp_device *isp)
  * isp_resume_modules - Resume ISP submodules.
  * @isp: OMAP3 ISP device
  */
-static void isp_resume_modules(struct isp_device *isp)
+static void __maybe_unused isp_resume_modules(struct isp_device *isp)
 {
omap3isp_stat_resume(>isp_aewb);
omap3isp_stat_resume(>isp_af);
-- 
2.9.0

Re: [PATCH] [RESEND] [media] omap3isp: support 64-bit version of omap3isp_stat_data

[Arnd Bergmann]

On Mon, May 7, 2018 at 5:33 PM, Sakari Ailus <sakari.ai...@iki.fi> wrote:
> On Mon, May 07, 2018 at 04:36:45PM -0400, Arnd Bergmann wrote:
>> On Mon, May 7, 2018 at 9:19 AM, Sakari Ailus <sakari.ai...@iki.fi> wrote:
>> > On Mon, May 07, 2018 at 04:17:32PM +0300, Laurent Pinchart wrote:
>> >> On Thursday, 26 April 2018 00:30:10 EEST Arnd Bergmann wrote:
>> >> > +int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
>> >> > +   struct omap3isp_stat_data_time32 
>> >> > *data)
>> >> > +{
>> >> > +   struct omap3isp_stat_data data64;
>> >> > +   int ret;
>> >> > +
>> >> > +   ret = omap3isp_stat_request_statistics(stat, );
>> >> > +
>> >> > +   data->ts.tv_sec = data64.ts.tv_sec;
>> >> > +   data->ts.tv_usec = data64.ts.tv_usec;
>> >> > +   memcpy(>buf, , sizeof(*data) - sizeof(data->ts));
>> >> > +
>> >> > +   return ret;
>> >>
>> >> We could return immediately after omap3isp_stat_request_statistics() if 
>> >> the
>> >> function fails, but that's no big deal, the error path is clearly a cold 
>> >> path.
>>
>> I looked at it again and briefly thought that it would leak kernel stack
>> data in my version and changing it would be required to avoid that,
>> but I do see now that the absence of the INFO_FL_ALWAYS_COPY
>> flag makes it safe after all.
>>
>> I agree that returning early here would be nicer here, I'll leave it up to
>> Sakari to fold in that change if he likes.
>
> I agree with the change; actually the data64 struct will be left untouched
> if there's an error so changing this doesn't seem to make a difference.
> Private IOCTLs have always_copy == false, so the argument struct isn't
> copied back to the kernel.
>
> The diff is here. Let me know if something went wrong...
>
> diff --git a/drivers/media/platform/omap3isp/ispstat.c 
> b/drivers/media/platform/omap3isp/ispstat.c
> index dfee8eaf2226..47353fee26c3 100644
> --- a/drivers/media/platform/omap3isp/ispstat.c
> +++ b/drivers/media/platform/omap3isp/ispstat.c
> @@ -519,12 +519,14 @@ int omap3isp_stat_request_statistics_time32(struct 
> ispstat *stat,
> int ret;
>
> ret = omap3isp_stat_request_statistics(stat, );
> +   if (ret)
> +   return ret;
>
> data->ts.tv_sec = data64.ts.tv_sec;
> data->ts.tv_usec = data64.ts.tv_usec;
> memcpy(>buf, , sizeof(*data) - sizeof(data->ts));
>
> -   return ret;
> +   return 0;
>  }

Yes, that's exactly what I had in mind. Thanks for fixing it up!

  Arnd

Re: [PATCH] media: zoran: move to dma-mapping interface

[Arnd Bergmann]

On Mon, May 7, 2018 at 5:05 AM, Hans Verkuil <hverk...@xs4all.nl> wrote:
> On 25/04/18 19:22, Mauro Carvalho Chehab wrote:
>> Em Wed, 25 Apr 2018 17:58:25 +0200
>> Arnd Bergmann <a...@arndb.de> escreveu:
>>
>>> On Wed, Apr 25, 2018 at 5:26 PM, Christoph Hellwig <h...@infradead.org> 
>>> wrote:
>>>> On Wed, Apr 25, 2018 at 01:15:18PM +0200, Arnd Bergmann wrote:
>>>>> That thought had occurred to me as well. I removed the oldest ISDN
>>>>> drivers already some years ago, and the OSS sound drivers
>>>>> got removed as well, and comedi got converted to the dma-mapping
>>>>> interfaces, so there isn't much left at all now. This is what we
>>>>> have as of v4.17-rc1:
>>>>
>>>> Yes, I've been looking at various grotty old bits to purge.  Usually
>>>> I've been looking for some non-tree wide patches and CCed the last
>>>> active people to see if they care.  In a few cases people do, but
>>>> most often no one does.
>>>
>>> Let's start with this one (zoran) then, as Mauro is keen on having
>>> all media drivers compile-testable on x86-64 and arm.
>>>
>>> Trent Piepho and Hans Verkuil both worked on this driver in the
>>> 2008/2009 timeframe and those were the last commits from anyone
>>> who appears to have tested their patches on actual hardware.
>>
>> Zoran is a driver for old hardware. I don't doubt that are people
>> out there still using it, but who knows?
>>
>> I have a few those boards packed somewhere. I haven't work with PCI
>> hardware for a while. If needed, I can try to seek for them and
>> do some tests. I need first to unpack a machine with PCI slots...
>> the NUCs I generally use for development don't have any :-)
>>
>> Anyway, except for virt_to_bus() and related stuff, I think that this
>> driver is in good shape, as Hans did a lot of work in the past to
>> make it to use the current media framework.
>>
>>>
>>> Trent, Hans: do you have reason to believe that there might still
>>> be users out there?
>
> I have no way of knowing this. However, I think they are easily replaced
> by much cheaper USB alternatives today.
>
> I did some work on the zoran driver several years ago, but it doesn't use
> the vb2 framework (and not even the older vb1 framework!) so I'm sure there
> are all sorts of bugs in that driver.
>
> Personally I would be fine with moving this driver to staging and removing
> it by, say, the end of the year.
>
> Nobody is going to work on it and I think it is time to retire it.

Based on the link to the old discussed that Bernhard provided, it
seems that the driver was already in a barely usable state 5 years
ago (the remaining users decided to use an even older 2.6 kernel instead),
and nobody has worked on fixing it since, so moving it to staging or
immediately removing it would both seem appropriate.

Arnd

Re: [PATCH] [RESEND] [media] omap3isp: support 64-bit version of omap3isp_stat_data

[Arnd Bergmann]

On Mon, May 7, 2018 at 9:19 AM, Sakari Ailus <sakari.ai...@iki.fi> wrote:
> On Mon, May 07, 2018 at 04:17:32PM +0300, Laurent Pinchart wrote:
>> On Thursday, 26 April 2018 00:30:10 EEST Arnd Bergmann wrote:
>> > +int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
>> > +   struct omap3isp_stat_data_time32 *data)
>> > +{
>> > +   struct omap3isp_stat_data data64;
>> > +   int ret;
>> > +
>> > +   ret = omap3isp_stat_request_statistics(stat, );
>> > +
>> > +   data->ts.tv_sec = data64.ts.tv_sec;
>> > +   data->ts.tv_usec = data64.ts.tv_usec;
>> > +   memcpy(>buf, , sizeof(*data) - sizeof(data->ts));
>> > +
>> > +   return ret;
>>
>> We could return immediately after omap3isp_stat_request_statistics() if the
>> function fails, but that's no big deal, the error path is clearly a cold 
>> path.

I looked at it again and briefly thought that it would leak kernel stack
data in my version and changing it would be required to avoid that,
but I do see now that the absence of the INFO_FL_ALWAYS_COPY
flag makes it safe after all.

I agree that returning early here would be nicer here, I'll leave it up to
Sakari to fold in that change if he likes.

>> > @@ -165,7 +167,14 @@ struct omap3isp_h3a_aewb_config {
>> >   * @config_counter: Number of the configuration associated with the data.
>> >   */
>> >  struct omap3isp_stat_data {
>> > +#ifdef __KERNEL__
>> > +   struct {
>> > +   __s64   tv_sec;
>> > +   __s64   tv_usec;
>> > +   } ts;
>>
>> I share Sakari's comment about this method implying a little-endian system,
>> but as the SoCs that integrate this device are all little-endian, that's not 
>> a
>> problem in practice.

To clarify: the version I have here does *not* imply a little-endian system,
it is supposed to work on both little-endian and big-endian builds, and
endianess is not a property of the SoC either -- you should be able to
build a big-endian kernel and run it on OMAP3 (aside from bugs in other
drivers). Using 'long' here instead of __s64 would however make this
interface broken on big-endian builds since the glibc definition of timeval
does include extra padding on big-endian machines to make the structure
compatible between 32-bit and 64-bit ABIs.

>> Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
>>
>> If you agree with the small comment about header ordering, let's get this
>> patch finally merged.
>
> I can make the change locally in my tree, no need to resend.
>
> Thanks.

Thanks a lot!

Re: [PATCH] [RESEND] [media] omap3isp: support 64-bit version of omap3isp_stat_data

[Arnd Bergmann]

On Thu, May 3, 2018 at 8:56 AM, Sakari Ailus <sakari.ai...@iki.fi> wrote:
> On Wed, Apr 25, 2018 at 11:30:10PM +0200, Arnd Bergmann wrote:
>> @@ -165,7 +167,14 @@ struct omap3isp_h3a_aewb_config {
>>   * @config_counter: Number of the configuration associated with the data.
>>   */
>>  struct omap3isp_stat_data {
>> +#ifdef __KERNEL__
>> + struct {
>> + __s64   tv_sec;
>> + __s64   tv_usec;
>
> Any particular reason for __s64 here instead of e.g. long or __s32? Kernel
> appears to use long in the timespec64 definition.

The user space 'timeval' definition is 16 bytes wide, with the layout
designed to be compatible between 32-bit and 64-bit, so it has to be like
this to match what user spaces sees with the old header files and a new
libc.

We don't yet know what the exact definition of timeval will be in all
libc implementations, but if they have a 32-bit tv_user field, it needs
padding next to it so the lower 32 bits are in the same place as they
would be using that 64-bit field I used.

 Arnd

[PATCH] [RESEND] [media] omap3isp: support 64-bit version of omap3isp_stat_data

[Arnd Bergmann]

C libraries with 64-bit time_t use an incompatible format for
struct omap3isp_stat_data. This changes the kernel code to
support either version, by moving over the normal handling
to the 64-bit variant, and adding compatiblity code to handle
the old binary format with the existing ioctl command code.

Fortunately, the command code includes the size of the structure,
so the difference gets handled automatically. In the process of
eliminating the references to 'struct timeval' from the kernel,
I also change the way the timestamp is generated internally,
basically by open-coding the v4l2_get_timestamp() call.

Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Sakari Ailus <sakari.ai...@iki.fi>
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
I submitted this one in November and asked again in January,
still waiting for a review so it can be applied.
---
 drivers/media/platform/omap3isp/isph3a_aewb.c |  2 ++
 drivers/media/platform/omap3isp/isph3a_af.c   |  2 ++
 drivers/media/platform/omap3isp/isphist.c |  2 ++
 drivers/media/platform/omap3isp/ispstat.c | 21 +++--
 drivers/media/platform/omap3isp/ispstat.h |  4 +++-
 include/uapi/linux/omap3isp.h | 22 ++
 6 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isph3a_aewb.c 
b/drivers/media/platform/omap3isp/isph3a_aewb.c
index d44626f20ac6..3c82dea4d375 100644
--- a/drivers/media/platform/omap3isp/isph3a_aewb.c
+++ b/drivers/media/platform/omap3isp/isph3a_aewb.c
@@ -250,6 +250,8 @@ static long h3a_aewb_ioctl(struct v4l2_subdev *sd, unsigned 
int cmd, void *arg)
return omap3isp_stat_config(stat, arg);
case VIDIOC_OMAP3ISP_STAT_REQ:
return omap3isp_stat_request_statistics(stat, arg);
+   case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
+   return omap3isp_stat_request_statistics_time32(stat, arg);
case VIDIOC_OMAP3ISP_STAT_EN: {
unsigned long *en = arg;
return omap3isp_stat_enable(stat, !!*en);
diff --git a/drivers/media/platform/omap3isp/isph3a_af.c 
b/drivers/media/platform/omap3isp/isph3a_af.c
index 99bd6cc21d86..4da25c84f0c6 100644
--- a/drivers/media/platform/omap3isp/isph3a_af.c
+++ b/drivers/media/platform/omap3isp/isph3a_af.c
@@ -314,6 +314,8 @@ static long h3a_af_ioctl(struct v4l2_subdev *sd, unsigned 
int cmd, void *arg)
return omap3isp_stat_config(stat, arg);
case VIDIOC_OMAP3ISP_STAT_REQ:
return omap3isp_stat_request_statistics(stat, arg);
+   case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
+   return omap3isp_stat_request_statistics_time32(stat, arg);
case VIDIOC_OMAP3ISP_STAT_EN: {
int *en = arg;
return omap3isp_stat_enable(stat, !!*en);
diff --git a/drivers/media/platform/omap3isp/isphist.c 
b/drivers/media/platform/omap3isp/isphist.c
index a4ed5d140d48..d4be3d0e06f9 100644
--- a/drivers/media/platform/omap3isp/isphist.c
+++ b/drivers/media/platform/omap3isp/isphist.c
@@ -435,6 +435,8 @@ static long hist_ioctl(struct v4l2_subdev *sd, unsigned int 
cmd, void *arg)
return omap3isp_stat_config(stat, arg);
case VIDIOC_OMAP3ISP_STAT_REQ:
return omap3isp_stat_request_statistics(stat, arg);
+   case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
+   return omap3isp_stat_request_statistics_time32(stat, arg);
case VIDIOC_OMAP3ISP_STAT_EN: {
int *en = arg;
return omap3isp_stat_enable(stat, !!*en);
diff --git a/drivers/media/platform/omap3isp/ispstat.c 
b/drivers/media/platform/omap3isp/ispstat.c
index 47cbc7e3d825..5967dfd0a9f7 100644
--- a/drivers/media/platform/omap3isp/ispstat.c
+++ b/drivers/media/platform/omap3isp/ispstat.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "isp.h"
 
@@ -237,7 +238,7 @@ static int isp_stat_buf_queue(struct ispstat *stat)
if (!stat->active_buf)
return STAT_NO_BUF;
 
-   v4l2_get_timestamp(>active_buf->ts);
+   ktime_get_ts64(>active_buf->ts);
 
stat->active_buf->buf_size = stat->buf_size;
if (isp_stat_buf_check_magic(stat, stat->active_buf)) {
@@ -500,7 +501,8 @@ int omap3isp_stat_request_statistics(struct ispstat *stat,
return PTR_ERR(buf);
}
 
-   data->ts = buf->ts;
+   data->ts.tv_sec = buf->ts.tv_sec;
+   data->ts.tv_usec = buf->ts.tv_nsec / NSEC_PER_USEC;
data->config_counter = buf->config_counter;
data->frame_number = buf->frame_number;
data->buf_size = buf->buf_size;
@@ -512,6 +514,21 @@ int omap3isp_stat_request_statistics(struct ispstat *stat,
return 0;
 }
 
+int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
+   struct omap3isp_stat_dat

Re: [PATCH] media: zoran: move to dma-mapping interface

[Arnd Bergmann]

On Wed, Apr 25, 2018 at 5:26 PM, Christoph Hellwig <h...@infradead.org> wrote:
> On Wed, Apr 25, 2018 at 01:15:18PM +0200, Arnd Bergmann wrote:
>> That thought had occurred to me as well. I removed the oldest ISDN
>> drivers already some years ago, and the OSS sound drivers
>> got removed as well, and comedi got converted to the dma-mapping
>> interfaces, so there isn't much left at all now. This is what we
>> have as of v4.17-rc1:
>
> Yes, I've been looking at various grotty old bits to purge.  Usually
> I've been looking for some non-tree wide patches and CCed the last
> active people to see if they care.  In a few cases people do, but
> most often no one does.

Let's start with this one (zoran) then, as Mauro is keen on having
all media drivers compile-testable on x86-64 and arm.

Trent Piepho and Hans Verkuil both worked on this driver in the
2008/2009 timeframe and those were the last commits from anyone
who appears to have tested their patches on actual hardware.

Trent, Hans: do you have reason to believe that there might still
be users out there?

   Arnd

Re: [PATCH] media: zoran: move to dma-mapping interface

[Arnd Bergmann]

On Wed, Apr 25, 2018 at 9:21 AM, Christoph Hellwig <h...@infradead.org> wrote:
> On Wed, Apr 25, 2018 at 09:08:13AM +0200, Arnd Bergmann wrote:
>> > That probably also means it can use dma_mmap_coherent instead of the
>> > handcrafted remap_pfn_range loop and the PageReserved abuse.
>>
>> I'd rather not touch that code. How about adding a comment about
>> the fact that it should use dma_mmap_coherent()?
>
> Maybe the real question is if there is anyone that actually cares
> for this driver, or if we are better off just removing it?
>
> Same is true for various other virt_to_bus using drivers, e.g. the
> grotty atm drivers.

That thought had occurred to me as well. I removed the oldest ISDN
drivers already some years ago, and the OSS sound drivers
got removed as well, and comedi got converted to the dma-mapping
interfaces, so there isn't much left at all now. This is what we
have as of v4.17-rc1:

$ git grep -wl '\<bus_to_virt\|virt_to_bus\>' drivers/
drivers/atm/ambassador.c
drivers/atm/firestream.c
drivers/atm/horizon.c
drivers/atm/zatm.c
drivers/block/swim3.c # power mac specific
drivers/gpu/drm/mga/mga_dma.c # commented out
drivers/infiniband/hw/nes/nes_verbs.c # commented out
drivers/isdn/hisax/netjet.c
drivers/net/appletalk/ltpc.c
drivers/net/ethernet/amd/au1000_eth.c # mips specific
drivers/net/ethernet/amd/ni65.c # only in comment
drivers/net/ethernet/apple/bmac.c # power mac specific
drivers/net/ethernet/apple/mace.c # power mac specific
drivers/net/ethernet/dec/tulip/de4x5.c  # trivially fixable
drivers/net/ethernet/i825xx/82596.c # m68k specific
drivers/net/ethernet/i825xx/lasi_82596.c # parisc specific
drivers/net/ethernet/i825xx/lib82596.c # only in comment
drivers/net/ethernet/sgi/ioc3-eth.c # mips specific
drivers/net/wan/cosa.c
drivers/net/wan/lmc/lmc_main.c
drivers/net/wan/z85230.c
drivers/scsi/3w-.c # only in comment
drivers/scsi/BusLogic.c
drivers/scsi/a2091.c # m68k specific
drivers/scsi/a3000.c # m68k specific
drivers/scsi/dc395x.c # only in comment
drivers/scsi/dpt_i2o.c
drivers/scsi/gvp11.c # m68k specific
drivers/scsi/mvme147.c # m68k specific
drivers/scsi/qla1280.c # comment only
drivers/staging/netlogic/xlr_net.c # mips specific
drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c # ppc32 specific
drivers/vme/bridges/vme_ca91cx42.c

My feeling is that we want to keep most of the arch specific
ones, in particular removing the m68k drivers would break
a whole class of machines.

For the ones that don't have a comment on them, they
probably won't be missed much, but it's hard to know for
sure. What we do know is that they never worked on
x86_64, and most of them are for ISA cards.

Arnd

Re: [PATCH] media: zoran: move to dma-mapping interface

[Arnd Bergmann]

On Wed, Apr 25, 2018 at 8:15 AM, Christoph Hellwig <h...@infradead.org> wrote:
> On Tue, Apr 24, 2018 at 10:40:45PM +0200, Arnd Bergmann wrote:
>> @@ -221,6 +222,7 @@ struct zoran_fh {
>>
>>   struct zoran_overlay_settings overlay_settings;
>>   u32 *overlay_mask;  /* overlay mask */
>> + dma_addr_t overlay_mask_dma;
>>   enum zoran_lock_activity overlay_active;/* feature currently in use? */
>>
>>   struct zoran_buffer_col buffers;/* buffers' info */
>> @@ -307,6 +309,7 @@ struct zoran {
>>
>>   struct zoran_overlay_settings overlay_settings;
>>   u32 *overlay_mask;  /* overlay mask */
>> + dma_addr_t overlay_mask_dma;
>>   enum zoran_lock_activity overlay_active;/* feature currently 
>> in use? */
>>
>>   wait_queue_head_t v4l_capq;
>> @@ -346,6 +349,7 @@ struct zoran {
>>
>>   /* zr36057's code buffer table */
>>   __le32 *stat_com;   /* stat_com[i] is indexed by 
>> dma_head/tail & BUZ_MASK_STAT_COM */
>> + dma_addr_t stat_com_dma;
>>
>>   /* (value & BUZ_MASK_FRAME) corresponds to index in pend[] queue */
>>   int jpg_pend[BUZ_MAX_FRAME];
>> diff --git a/drivers/media/pci/zoran/zoran_card.c 
>> b/drivers/media/pci/zoran/zoran_card.c
>> index a6b9ebd20263..dabd8bf77472 100644
>> --- a/drivers/media/pci/zoran/zoran_card.c
>> +++ b/drivers/media/pci/zoran/zoran_card.c
>> @@ -890,6 +890,7 @@ zoran_open_init_params (struct zoran *zr)
>>   /* User must explicitly set a window */
>>   zr->overlay_settings.is_set = 0;
>>   zr->overlay_mask = NULL;
>> + zr->overlay_mask_dma = 0;
>
> I don't think this zeroing is required, and given that 0 is a valid
> dma address on some platforms is also is rather confusing.:w

The driver does this everywhere, which I also noticed as unnecessary,
but it felt better to be consistent with the rest of the driver here than
to follow common practice.

There are some explicit 'if (zr->overlay_mask)' checks in the driver
that require overlay_mask to be initialized. I could set it to
DMA_ERROR_CODE, but you removed that ;-)

It's easy enough to remove the re-zeroing of the number though
if you still think that's better.

>>   mask_line_size = (BUZ_MAX_WIDTH + 31) / 32;
>> - reg = virt_to_bus(zr->overlay_mask);
>> + reg = zr->overlay_mask_dma;
>>   btwrite(reg, ZR36057_MMTR);
>> - reg = virt_to_bus(zr->overlay_mask + mask_line_size);
>> + reg = zr->overlay_mask_dma + mask_line_size;
>
> I think this would be easier to read if the reg assignments got
> removed, e.g.
>
> btwrite(zr->overlay_mask_dma, ZR36057_MMTR);
> btwrite(zr->overlay_mask_dma + mask_line_size, ZR36057_MMBR);
>
> Same in a few other places.

Right, good idea.

>> + virt_tab = (void *)get_zeroed_page(GFP_KERNEL);
>> + if (!mem || !virt_tab) {
>>   dprintk(1,
>>   KERN_ERR
>>   "%s: %s - get_zeroed_page (frag_tab) failed 
>> for buffer %d\n",
>>   ZR_DEVNAME(zr), __func__, i);
>> + kfree(mem);
>> + kfree(virt_tab);
>>   jpg_fbuffer_free(fh);
>>   return -ENOBUFS;
>>   }
>>   fh->buffers.buffer[i].jpg.frag_tab = (__le32 *)mem;
>> - fh->buffers.buffer[i].jpg.frag_tab_bus = virt_to_bus(mem);
>> + fh->buffers.buffer[i].jpg.frag_virt_tab = virt_tab;
>
> From a quick look it seems like frag_tab should be a dma_alloc_coherent
> allocation, or you would need a lot of cache sync operations.

Do you mean the table or the buffers it points to? In the code you
quote, we initialize the table with static data before mapping it,
so I would expect either interface to work fine here.

For the actual buffers, I tried to retain the current behavior and
regular kernel memory, in particular because I wasn't too sure
about changing the mmap() function without understanding what
it really does. ;-)

It looks like the buffers are never accessed from the kernel but
only from hardware and user space, so we don't care about whether
we get a mapping that is coherent between the kernel and hardware,
but we probably should ensure that the page table attributes are the
same in kernel and user space (at least powerpc gets a checkstop
for unmatched cacheability flags).

> That probably also means it can use dma_mmap_coherent instead of the
> handcrafted remap_pfn_range loop and the PageReserved abuse.

I'd rather not touch that code. How about adding a comment about
the fact that it should use dma_mmap_coherent()?

 Arnd

[PATCH] media: zoran: move to dma-mapping interface

[Arnd Bergmann]

No drivers should use virt_to_bus() any more. This converts
one of the few remaining ones to the DMA mapping interface.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/pci/zoran/Kconfig|  2 +-
 drivers/media/pci/zoran/zoran.h| 10 +--
 drivers/media/pci/zoran/zoran_card.c   | 10 +--
 drivers/media/pci/zoran/zoran_device.c | 16 +-
 drivers/media/pci/zoran/zoran_driver.c | 54 +-
 5 files changed, 63 insertions(+), 29 deletions(-)

diff --git a/drivers/media/pci/zoran/Kconfig b/drivers/media/pci/zoran/Kconfig
index 39ec35bd21a5..26f40e124a32 100644
--- a/drivers/media/pci/zoran/Kconfig
+++ b/drivers/media/pci/zoran/Kconfig
@@ -1,6 +1,6 @@
 config VIDEO_ZORAN
tristate "Zoran ZR36057/36067 Video For Linux"
-   depends on PCI && I2C_ALGOBIT && VIDEO_V4L2 && VIRT_TO_BUS
+   depends on PCI && I2C_ALGOBIT && VIDEO_V4L2
depends on !ALPHA
help
  Say Y for support for MJPEG capture cards based on the Zoran
diff --git a/drivers/media/pci/zoran/zoran.h b/drivers/media/pci/zoran/zoran.h
index 9bb3c21aa275..9ff3a9acb60a 100644
--- a/drivers/media/pci/zoran/zoran.h
+++ b/drivers/media/pci/zoran/zoran.h
@@ -183,13 +183,14 @@ struct zoran_buffer {
struct zoran_sync bs;   /* DONE: info to return to application 
*/
union {
struct {
-   __le32 *frag_tab;   /* addresses of frag table */
-   u32 frag_tab_bus;   /* same value cached to save 
time in ISR */
+   __le32 *frag_tab;   /* DMA addresses of frag table 
*/
+   void **frag_virt_tab;   /* virtual addresses of frag 
table */
+   dma_addr_t frag_tab_dma;/* same value cached to save 
time in ISR */
} jpg;
struct {
char *fbuffer;  /* virtual address of frame 
buffer */
unsigned long fbuffer_phys;/* physical address of frame 
buffer */
-   unsigned long fbuffer_bus;/* bus address of frame 
buffer */
+   dma_addr_t fbuffer_dma;/* bus address of frame buffer */
} v4l;
};
 };
@@ -221,6 +222,7 @@ struct zoran_fh {
 
struct zoran_overlay_settings overlay_settings;
u32 *overlay_mask;  /* overlay mask */
+   dma_addr_t overlay_mask_dma;
enum zoran_lock_activity overlay_active;/* feature currently in use? */
 
struct zoran_buffer_col buffers;/* buffers' info */
@@ -307,6 +309,7 @@ struct zoran {
 
struct zoran_overlay_settings overlay_settings;
u32 *overlay_mask;  /* overlay mask */
+   dma_addr_t overlay_mask_dma;
enum zoran_lock_activity overlay_active;/* feature currently in 
use? */
 
wait_queue_head_t v4l_capq;
@@ -346,6 +349,7 @@ struct zoran {
 
/* zr36057's code buffer table */
__le32 *stat_com;   /* stat_com[i] is indexed by 
dma_head/tail & BUZ_MASK_STAT_COM */
+   dma_addr_t stat_com_dma;
 
/* (value & BUZ_MASK_FRAME) corresponds to index in pend[] queue */
int jpg_pend[BUZ_MAX_FRAME];
diff --git a/drivers/media/pci/zoran/zoran_card.c 
b/drivers/media/pci/zoran/zoran_card.c
index a6b9ebd20263..dabd8bf77472 100644
--- a/drivers/media/pci/zoran/zoran_card.c
+++ b/drivers/media/pci/zoran/zoran_card.c
@@ -890,6 +890,7 @@ zoran_open_init_params (struct zoran *zr)
/* User must explicitly set a window */
zr->overlay_settings.is_set = 0;
zr->overlay_mask = NULL;
+   zr->overlay_mask_dma = 0;
zr->overlay_active = ZORAN_FREE;
 
zr->v4l_memgrab_active = 0;
@@ -1028,7 +1029,8 @@ static int zr36057_init (struct zoran *zr)
 
/* allocate memory *before* doing anything to the hardware
 * in case allocation fails */
-   zr->stat_com = kzalloc(BUZ_NUM_STAT_COM * 4, GFP_KERNEL);
+   zr->stat_com = dma_alloc_coherent(>pci_dev->dev,
+   BUZ_NUM_STAT_COM * 4, >stat_com_dma, 
GFP_KERNEL);
zr->video_dev = video_device_alloc();
if (!zr->stat_com || !zr->video_dev) {
dprintk(1,
@@ -1072,7 +1074,8 @@ static int zr36057_init (struct zoran *zr)
return 0;
 
 exit_free:
-   kfree(zr->stat_com);
+   dma_free_coherent(>pci_dev->dev, BUZ_NUM_STAT_COM * 4,
+ zr->stat_com, zr->stat_com_dma);
kfree(zr->video_dev);
return err;
 }
@@ -1107,7 +1110,8 @@ static void zoran_remove(struct pci_dev *pdev)
btwrite(0, ZR36057_SPGPPCR);
free_irq(zr->pci_dev->irq, zr);
/* unmap and free memory */
-   kfree(zr->stat_com);
+   dma_free_coherent(>pci_dev->dev, BUZ_NUM_STAT_COM * 4,
+ z

Re: [PATCH 1/7] asm-generic, media: allow COMPILE_TEST with virt_to_bus

[Arnd Bergmann]

On Fri, Apr 20, 2018 at 7:42 PM, Mauro Carvalho Chehab
 wrote:
> The virt_to_bus/bus_to_virt macros are arch-specific. Some
> archs don't support it. Yet, as it is interesting to allow
> doing compilation tests on non-ia32/ia64 archs, provide a
> fallback for such archs.
>
> While here, enable COMPILE_TEST for two media drivers that
> depends on it.
>
> Signed-off-by: Mauro Carvalho Chehab 

I'd prefer not to do this: virt_to_bus() is deprecated for good reasons,
and I'd rather see the drivers fixed to use dma-mapping.h correctly.

One problem with your patch is that not all architectures include
asm-generic/io.h, so it likely breaks allmodconfig builds on architectures
that don't use that file and don't provide virt_to_bus() either.

  Arnd

Re: [PATCH 0/8] arm: renesas: Change platform dependency to ARCH_RENESAS

[Arnd Bergmann]

On Fri, Apr 20, 2018 at 3:28 PM, Geert Uytterhoeven
<geert+rene...@glider.be> wrote:
> Hi all,
>
> Commit 9b5ba0df4ea4f940 ("ARM: shmobile: Introduce ARCH_RENESAS")
> started the conversion from ARCH_SHMOBILE to ARCH_RENESAS for Renesas
> ARM SoCs.  This patch series completes the conversion, by:
>   1. Updating dependencies for drivers that weren't converted yet,
>   2. Removing the ARCH_SHMOBILE Kconfig symbols on ARM and ARM64.
>
> The first 6 patches can be applied independently by subsystem
> maintainers.
> The last two patches depend on the first 6 patches, and are thus marked
> RFC.

This all looks fine to me.

Acked-by: Arnd Bergmann <a...@arndb.de>

  Arnd

Re: [PATCH 02/16] media: omap3isp: allow it to build with COMPILE_TEST

[Arnd Bergmann]

On Sat, Apr 7, 2018 at 3:14 PM, Mauro Carvalho Chehab
<mche...@s-opensource.com> wrote:
> Em Sat, 07 Apr 2018 14:56:59 +0300
> Laurent Pinchart <laurent.pinch...@ideasonboard.com> escreveu:
>> On Thursday, 5 April 2018 22:44:44 EEST Mauro Carvalho Chehab wrote:
>> > Em Thu, 05 Apr 2018 21:30:27 +0300 Laurent Pinchart escreveu:

>> > > If you want to make drivers compile for all architectures, the APIs they
>> > > use must be defined in linux/, not in asm/. They can be stubbed there
>> > > when not implemented in a particular architecture, but not in the driver.
>> >
>> > In this specific case, the same approach taken here is already needed
>> > by the Renesas VMSA-compatible IPMMU driver, with, btw, is inside
>> > drivers/iommu:
>> >
>> > drivers/iommu/ipmmu-vmsa.c
>>
>> The reason there is different, the driver is shared by ARM32 and ARM64
>> platforms. Furthermore, there's an effort (or at least there was) to move 
>> away
>> from those APIs in the driver, but I think it has stalled.
>
> Anyway, the approach sticks at the driver. As this was accepted even
> inside drivers/iommu, I fail to see why not doing the same on media,
> specially since it really helps us to find bugs at omap3isp driver.
>
> Even without pushing upstream (where Coverity would analyze it),
> we got lots of problems by simply letting omap3isp to use the
> usual tools we use for all other drivers.
>
>> > Also, this API is used only by 3 drivers [1]:
>> >
>> > drivers/iommu/ipmmu-vmsa.c
>> > drivers/iommu/mtk_iommu_v1.c
>> > drivers/media/platform/omap3isp/isp.c
>> >
>> > [1] as blamed by
>> > git grep -l arm_iommu_create_mapping
>>
>> The exynos driver also uses it.
>>
>> > That hardly seems to be an arch-specific iommu solution, but, instead, some
>> > hack used by only three drivers or some legacy iommu binding.
>>
>> It's more complex than that. There are multiple IOMMU-related APIs on ARM, so
>> more recent than others, with different feature sets. While I agree that
>> drivers should move away from arm_iommu_create_mapping(), doing so requires
>> coordination between the IOMMU driver and the bus master driver (for instance
>> the omap3isp driver). It's not a trivial matter, but I'd love if someone
>> submitted patches :-)
>
> If someone steps up to do that, it would be really helpful, but we
> should not trust that this will happen. OMAP3 is an old hardware,
> and not many developers are working on improving its support.

Considering its age, I still see a lot of changes on the arch/arm side of
it, so I wouldn't give up the hope yet.

>> > The omap3isp is, btw, the only driver outside drivers/iommu that needs it.
>> >
>> > So, it sounds that other driver uses some other approach, but hardly
>> > it would be worth to change this driver to use something else.
>> >
>> > So, better to stick with the same solution the Renesas driver used.
>>
>> I'm not responsible for that solution and I didn't think it was a good one at
>> the time it was introduced, but in any case it is not meant at all to support
>> COMPILE_TEST. I still don't think the omap3isp driver should declare stubs 
>> for
>> these functions for the purpose of supporting compile-testing on x86.
>
> Well, there is another alternative. We could do add this to its Makefile:
>
> ifndef CONFIG_ARCH_ARM
> ccflags-y += -I./arch/arm/include/
> endif
>
> And add those stubs to arch/arm/include/asm/dma-iommu.h,
> in order to be used when CONFIG_IOMMU_DMA isn't defined:
>
> #define arm_iommu_create_mapping(...)   NULL
> #define arm_iommu_attach_device(...)-ENODEV
> #define arm_iommu_release_mapping(...)  do {} while (0)
> #define arm_iommu_detach_device(...)do {} while (0)
>
> If done right, such solution could also be used to remove
> the #ifdef inside drivers/iommu/ipmmu-vmsa.c.
>
> Yet, I think that the approach I proposed before is better,
> but maybe arm maintainers may have a different opinion.
>
> Arnd,
>
> What do you think?

I think including a foreign architecture header is worse than your
earlier patch, I'd rather see a local hack in the driver.

I haven't tried it, but how about something simpler like what
I have below.

  Arnd

(in case it works and you want to pick it up with a proper
changelog):

Signed-off-by: Arnd Bergmann <a...@arndb.de>

diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index 8eb000e3d8fd..625f2e407929 100644
--- a/drivers/media/platform/omap3isp/isp.c

Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST

[Arnd Bergmann]

On Fri, Apr 6, 2018 at 4:26 PM, Mauro Carvalho Chehab
<mche...@s-opensource.com> wrote:
> Em Fri, 6 Apr 2018 16:16:46 +0200
> Arnd Bergmann <a...@arndb.de> escreveu:
>
>> On Fri, Apr 6, 2018 at 4:15 PM, Mauro Carvalho Chehab
>> <mche...@s-opensource.com> wrote:
>> > Em Fri, 6 Apr 2018 11:51:16 +0200
>> > Arnd Bergmann <a...@arndb.de> escreveu:
>> >
>> >> On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab
>> >> <mche...@s-opensource.com> wrote:
>> >>
>> >> > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST
>> >> >
>> >> > There aren't many things that would be needed to allow it
>> >> > to build with compile test.
>> >> >
>> >> > Add the needed bits.
>> >> >
>> >> > Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
>> >>
>> >> Reviewed-by: Arnd Bergmann <a...@arndb.de>
>> >
>> > Actually, in order to avoid warnings with smatch, the COMPILE_TEST
>> > macros should be declared as:
>> >
>> > +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v)
>> > +#define in_be32(a) ioread32be((void __iomem *)a)
>>
>> I would just add the correct annotations, I think they've always been 
>> missing.
>> 2 patches coming in a few minutes.
>
> I corrected the annotations too. Now, it gives the same results
> building for both arm and x86.
>
> If you want to double check, the full tree is at:
>
> https://git.linuxtv.org/mchehab/experimental.git/log/?h=compile_test

The __iomem annotations look good, my other patch is still needed to
get a clean build with "make C=1" but doesn't apply cleanly on top of your
version. I assume you'll just fix it up accordingly.

  Arnd

[PATCH 1/2] media: platform: fsl-viu: add __iomem annotations

[Arnd Bergmann]

This avoids countless sparse warnings like

   drivers/media/platform/fsl-viu.c:1081:25: sparse: incorrect type in argument 
2 (different address spaces)
   drivers/media/platform/fsl-viu.c:1082:25: sparse: incorrect type in argument 
2 (different address spaces)

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/platform/fsl-viu.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c
index 200c47c69a75..cc85620267f1 100644
--- a/drivers/media/platform/fsl-viu.c
+++ b/drivers/media/platform/fsl-viu.c
@@ -128,7 +128,7 @@ struct viu_dev {
int dma_done;
 
/* Hardware register area */
-   struct viu_reg  *vr;
+   struct viu_reg __iomem  *vr;
 
/* Interrupt vector */
int irq;
@@ -244,7 +244,7 @@ struct viu_fmt *format_by_fourcc(int fourcc)
 
 void viu_start_dma(struct viu_dev *dev)
 {
-   struct viu_reg *vr = dev->vr;
+   struct viu_reg __iomem *vr = dev->vr;
 
dev->field = 0;
 
@@ -255,7 +255,7 @@ void viu_start_dma(struct viu_dev *dev)
 
 void viu_stop_dma(struct viu_dev *dev)
 {
-   struct viu_reg *vr = dev->vr;
+   struct viu_reg __iomem *vr = dev->vr;
int cnt = 100;
u32 status_cfg;
 
@@ -395,7 +395,7 @@ static void free_buffer(struct videobuf_queue *vq, struct 
viu_buf *buf)
 
 inline int buffer_activate(struct viu_dev *dev, struct viu_buf *buf)
 {
-   struct viu_reg *vr = dev->vr;
+   struct viu_reg __iomem *vr = dev->vr;
int bpp;
 
/* setup the DMA base address */
@@ -703,9 +703,9 @@ static int verify_preview(struct viu_dev *dev, struct 
v4l2_window *win)
return 0;
 }
 
-inline void viu_activate_overlay(struct viu_reg *viu_reg)
+inline void viu_activate_overlay(struct viu_reg __iomem *viu_reg)
 {
-   struct viu_reg *vr = viu_reg;
+   struct viu_reg __iomem *vr = viu_reg;
 
out_be32(>field_base_addr, reg_val.field_base_addr);
out_be32(>dma_inc, reg_val.dma_inc);
@@ -985,9 +985,9 @@ inline void viu_activate_next_buf(struct viu_dev *dev,
}
 }
 
-inline void viu_default_settings(struct viu_reg *viu_reg)
+inline void viu_default_settings(struct viu_reg __iomem *viu_reg)
 {
-   struct viu_reg *vr = viu_reg;
+   struct viu_reg __iomem *vr = viu_reg;
 
out_be32(>luminance, 0x9512A254);
out_be32(>chroma_r, 0x0331);
@@ -1001,7 +1001,7 @@ inline void viu_default_settings(struct viu_reg *viu_reg)
 
 static void viu_overlay_intr(struct viu_dev *dev, u32 status)
 {
-   struct viu_reg *vr = dev->vr;
+   struct viu_reg __iomem *vr = dev->vr;
 
if (status & INT_DMA_END_STATUS)
dev->dma_done = 1;
@@ -1032,7 +1032,7 @@ static void viu_overlay_intr(struct viu_dev *dev, u32 
status)
 static void viu_capture_intr(struct viu_dev *dev, u32 status)
 {
struct viu_dmaqueue *vidq = >vidq;
-   struct viu_reg *vr = dev->vr;
+   struct viu_reg __iomem *vr = dev->vr;
struct viu_buf *buf;
int field_num;
int need_two;
@@ -1104,7 +1104,7 @@ static void viu_capture_intr(struct viu_dev *dev, u32 
status)
 static irqreturn_t viu_intr(int irq, void *dev_id)
 {
struct viu_dev *dev  = (struct viu_dev *)dev_id;
-   struct viu_reg *vr = dev->vr;
+   struct viu_reg __iomem *vr = dev->vr;
u32 status;
u32 error;
 
@@ -1169,7 +1169,7 @@ static int viu_open(struct file *file)
struct video_device *vdev = video_devdata(file);
struct viu_dev *dev = video_get_drvdata(vdev);
struct viu_fh *fh;
-   struct viu_reg *vr;
+   struct viu_reg __iomem *vr;
int minor = vdev->minor;
u32 status_cfg;
 
@@ -1305,7 +1305,7 @@ static int viu_release(struct file *file)
return 0;
 }
 
-void viu_reset(struct viu_reg *reg)
+void viu_reset(struct viu_reg __iomem *reg)
 {
out_be32(>status_cfg, 0);
out_be32(>luminance, 0x9512a254);
-- 
2.9.0

[PATCH 2/2] media: platform: fsl-viu: mark local functions 'static'

[Arnd Bergmann]

Both sparse and gcc (with 'make V=1') warn about non-static symbols that
have not been declared:

drivers/media/platform/fsl-viu.c:235:16: warning: symbol 'format_by_fourcc' was 
not declared. Should it be static?
drivers/media/platform/fsl-viu.c:248:6: warning: symbol 'viu_start_dma' was not 
declared. Should it be static?
drivers/media/platform/fsl-viu.c:259:6: warning: symbol 'viu_stop_dma' was not 
declared. Should it be static?
drivers/media/platform/fsl-viu.c:808:5: warning: symbol 'vidioc_g_fbuf' was not 
declared. Should it be static?
drivers/media/platform/fsl-viu.c:819:5: warning: symbol 'vidioc_s_fbuf' was not 
declared. Should it be static?
drivers/media/platform/fsl-viu.c:1311:6: warning: symbol 'viu_reset' was not 
declared. Should it be static?

In this driver, all instance should indeed be static. This leads to better
optimized code, and avoids potential namespace conflicts.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/platform/fsl-viu.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c
index cc85620267f1..38c9be51f01f 100644
--- a/drivers/media/platform/fsl-viu.c
+++ b/drivers/media/platform/fsl-viu.c
@@ -229,7 +229,7 @@ enum status_config {
 
 static irqreturn_t viu_intr(int irq, void *dev_id);
 
-struct viu_fmt *format_by_fourcc(int fourcc)
+static struct viu_fmt *format_by_fourcc(int fourcc)
 {
int i;
 
@@ -242,7 +242,7 @@ struct viu_fmt *format_by_fourcc(int fourcc)
return NULL;
 }
 
-void viu_start_dma(struct viu_dev *dev)
+static void viu_start_dma(struct viu_dev *dev)
 {
struct viu_reg __iomem *vr = dev->vr;
 
@@ -253,7 +253,7 @@ void viu_start_dma(struct viu_dev *dev)
out_be32(>status_cfg, INT_FIELD_EN);
 }
 
-void viu_stop_dma(struct viu_dev *dev)
+static void viu_stop_dma(struct viu_dev *dev)
 {
struct viu_reg __iomem *vr = dev->vr;
int cnt = 100;
@@ -802,7 +802,7 @@ static int vidioc_overlay(struct file *file, void *priv, 
unsigned int on)
return 0;
 }
 
-int vidioc_g_fbuf(struct file *file, void *priv, struct v4l2_framebuffer *arg)
+static int vidioc_g_fbuf(struct file *file, void *priv, struct 
v4l2_framebuffer *arg)
 {
struct viu_fh  *fh = priv;
struct viu_dev *dev = fh->dev;
@@ -813,7 +813,7 @@ int vidioc_g_fbuf(struct file *file, void *priv, struct 
v4l2_framebuffer *arg)
return 0;
 }
 
-int vidioc_s_fbuf(struct file *file, void *priv, const struct v4l2_framebuffer 
*arg)
+static int vidioc_s_fbuf(struct file *file, void *priv, const struct 
v4l2_framebuffer *arg)
 {
struct viu_fh  *fh = priv;
struct viu_dev *dev = fh->dev;
@@ -1305,7 +1305,7 @@ static int viu_release(struct file *file)
return 0;
 }
 
-void viu_reset(struct viu_reg __iomem *reg)
+static void viu_reset(struct viu_reg __iomem *reg)
 {
out_be32(>status_cfg, 0);
out_be32(>luminance, 0x9512a254);
-- 
2.9.0

Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST

[Arnd Bergmann]

On Fri, Apr 6, 2018 at 4:15 PM, Mauro Carvalho Chehab
<mche...@s-opensource.com> wrote:
> Em Fri, 6 Apr 2018 11:51:16 +0200
> Arnd Bergmann <a...@arndb.de> escreveu:
>
>> On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab
>> <mche...@s-opensource.com> wrote:
>>
>> > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST
>> >
>> > There aren't many things that would be needed to allow it
>> > to build with compile test.
>> >
>> > Add the needed bits.
>> >
>> > Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
>>
>> Reviewed-by: Arnd Bergmann <a...@arndb.de>
>
> Actually, in order to avoid warnings with smatch, the COMPILE_TEST
> macros should be declared as:
>
> +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v)
> +#define in_be32(a) ioread32be((void __iomem *)a)

I would just add the correct annotations, I think they've always been missing.
2 patches coming in a few minutes.

  Arnd

Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST

[Arnd Bergmann]

On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab
<mche...@s-opensource.com> wrote:

> [PATCH] media: fsl-viu: allow building it with COMPILE_TEST
>
> There aren't many things that would be needed to allow it
> to build with compile test.
>
> Add the needed bits.
>
> Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>

Reviewed-by: Arnd Bergmann <a...@arndb.de>

Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST

[Arnd Bergmann]

On Thu, Apr 5, 2018 at 7:54 PM, Mauro Carvalho Chehab
 wrote:
> There aren't many things that would be needed to allow it
> to build with compile test.

> +/* Allow building this driver with COMPILE_TEST */
> +#ifndef CONFIG_PPC_MPC512x
> +#define NO_IRQ   0

The NO_IRQ usage here really needs to die. The portable way to do this
is the simpler

diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c
index 200c47c69a75..707bda89b4f7 100644
--- a/drivers/media/platform/fsl-viu.c
+++ b/drivers/media/platform/fsl-viu.c
@@ -1407,7 +1407,7 @@ static int viu_of_probe(struct platform_device *op)
}

viu_irq = irq_of_parse_and_map(op->dev.of_node, 0);
-   if (viu_irq == NO_IRQ) {
+   if (!viu_irq) {
dev_err(>dev, "Error while mapping the irq\n");
return -EINVAL;
}

> +#define out_be32(v, a) writel(a, v)
> +#define in_be32(a) readl(a)

This does get it to compile, but looks confusing because it mixes up the
endianess. I'd suggest doing it like

#ifndef CONFIG_PPC
#define out_be32(v, a) iowrite32be(a, v)
#define in_be32(a) ioread32be(a)
#endif

  Arnd

Re: [PATCH 00/15] ARM: pxa: switch to DMA slave maps

[Arnd Bergmann]

On Thu, Apr 5, 2018 at 8:29 AM, Ulf Hansson  wrote:
> On 4 April 2018 at 21:56, Boris Brezillon  wrote:
>> On Wed, 04 Apr 2018 21:49:26 +0200
>> Robert Jarzmik  wrote:
>>
>>> Ulf Hansson  writes:
>>>
>>> > On 2 April 2018 at 16:26, Robert Jarzmik  wrote:
>>> >> Hi,
>>> >>
>>> >> This serie is aimed at removing the dmaengine slave compat use, and 
>>> >> transfer
>>> >> knowledge of the DMA requestors into architecture code.
>>> >> As this looks like a patch bomb, each maintainer expressing for his tree 
>>> >> either
>>> >> an Ack or "I want to take through my tree" will be spared in the next 
>>> >> iterations
>>> >> of this serie.
>>> >
>>> > Perhaps an option is to send this hole series as PR for 3.17 rc1, that
>>> > would removed some churns and make this faster/easier? Well, if you
>>> > receive the needed acks of course.
>>> For 3.17-rc1 it looks a bit optimistic with the review time ... If I have 
>>> all
>>
>> Especially since 3.17-rc1 has been released more than 3 years ago :-),
>> but I guess you meant 4.17-rc1.
>
> Yeah, I realize that I was a bit lost in time yesterday. Even more
> people have been having fun about it (me too). :-)

I occasionally still type 2.6.17 when I mean 4.17.

   Arnd

Re: [PATCH 02/15] ARM: pxa: add dma slave map

[Arnd Bergmann]

On Tue, Apr 3, 2018 at 5:18 PM, Robert Jarzmik <robert.jarz...@free.fr> wrote:
> Arnd Bergmann <a...@arndb.de> writes:
>
>>> +   { "smc911x.0", "rx", PDMA_FILTER_PARAM(LOWEST, -1) },
>>> +   { "smc911x.0", "tx", PDMA_FILTER_PARAM(LOWEST, -1) },
>>> +   { "smc91x.0", "data", PDMA_FILTER_PARAM(LOWEST, -1) },
>>
>> This one is interesting, as you are dealing with an off-chip device,
>> and the channel number is '-'1. How does this even work? Does it
>> mean
>
> This relies on pxa_dma, in which the "-1" for a requestor line means "no
> requestor" or said in another way "always requesting". As a consequence, as 
> soon
> as the DMA descriptors are queued, the transfer begins, and it is supposed
> implicitely that the FIFO output availability is at least as quick as the 
> system
> bus and the DMA size is perfectly fit for the FIFO available bytes.
>
> This is what has been the underlying of DMA transfers of smc91x(x) on the PXA
> platforms, where the smc91x(s) are directly wired on the system bus (the same
> bus having DRAM, SRAM, IO-mapped devices).

Ok, I looked at the driver in more detail now and found the scary parts.
So it's using the async DMA interface to do synchronous DMA in
interrupt context in order to transfer the rx data faster than an readsl()
would, correct?

It still feels odd to me that there is an entry in the slave map for
a device that does not have a request line. However, it also seems
that the entire code in those two drivers that deals with DMA is specific
to PXA anyway, so maybe it can be done differently: instead of
calling dma_request_slave_channel_compat() or dma_request_chan()
with a fake request line, how about calling dma_request_channel()
with an NULL filter function and data, and have the driver handle
the empty data case the same way as the rq=-1 case today?

>>> +   /* PXA25x specific map */
>>> +   { "pxa25x-ssp.0", "rx", PDMA_FILTER_PARAM(LOWEST, 13) },
>>> +   { "pxa25x-ssp.0", "tx", PDMA_FILTER_PARAM(LOWEST, 14) },
>>> +   { "pxa25x-nssp.1", "rx", PDMA_FILTER_PARAM(LOWEST, 15) },
>>> +   { "pxa25x-nssp.1", "tx", PDMA_FILTER_PARAM(LOWEST, 16) },
>>> +   { "pxa25x-nssp.2", "rx", PDMA_FILTER_PARAM(LOWEST, 23) },
>>> +   { "pxa25x-nssp.2", "tx", PDMA_FILTER_PARAM(LOWEST, 24) },
>>> +   { "pxa-pcm-audio", "nssp2_rx", PDMA_FILTER_PARAM(LOWEST, 15) },
>>> +   { "pxa-pcm-audio", "nssp2_tx", PDMA_FILTER_PARAM(LOWEST, 16) },
>>> +   { "pxa-pcm-audio", "nssp3_rx", PDMA_FILTER_PARAM(LOWEST, 23) },
>>> +   { "pxa-pcm-audio", "nssp3_tx", PDMA_FILTER_PARAM(LOWEST, 24) },
>>> +
>>> +   /* PXA27x specific map */
>>> +   { "pxa-pcm-audio", "ssp3_rx", PDMA_FILTER_PARAM(LOWEST, 66) },
>>> +   { "pxa-pcm-audio", "ssp3_tx", PDMA_FILTER_PARAM(LOWEST, 67) },
>>> +   { "pxa27x-camera.0", "CI_Y", PDMA_FILTER_PARAM(HIGHEST, 68) },
>>> +   { "pxa27x-camera.0", "CI_U", PDMA_FILTER_PARAM(HIGHEST, 69) },
>>> +   { "pxa27x-camera.0", "CI_V", PDMA_FILTER_PARAM(HIGHEST, 70) },
>>> +
>>> +   /* PXA3xx specific map */
>>> +   { "pxa-pcm-audio", "ssp4_rx", PDMA_FILTER_PARAM(LOWEST, 2) },
>>> +   { "pxa-pcm-audio", "ssp4_tx", PDMA_FILTER_PARAM(LOWEST, 3) },
>>> +   { "pxa2xx-mci.1", "rx", PDMA_FILTER_PARAM(LOWEST, 93) },
>>> +   { "pxa2xx-mci.1", "tx", PDMA_FILTER_PARAM(LOWEST, 94) },
>>> +   { "pxa3xx-nand", "data", PDMA_FILTER_PARAM(LOWEST, 97) },
>>> +   { "pxa2xx-mci.2", "rx", PDMA_FILTER_PARAM(LOWEST, 100) },
>>> +   { "pxa2xx-mci.2", "tx", PDMA_FILTER_PARAM(LOWEST, 101) },
>>> +};
>>
>> Since more than half the entries in here are chip specific, maybe it would be
>> better to split that table into three and have a copy for each one in
>> arch/arm/mach-pxa/pxa{25x.27x.3xx}.c?
> Mmmh, today the split is :
>  - 16 common entries
>  - 10 pxa25x specific entries
>  - 5 pxa27x specific entries
>  - 7 pxa3xx specific entries
>  => total of 38 lines
>
> After the split we'll have :
>  - 26 pxa25x specific entries
>  - 21 pxa27x specific entries
>  - 23 pxa3xx specific entries
>  => total of 70 lines
>
> That doubles the number of lines, not counting the declarations, and amending 
> of
> pxa2xx_set_dmac_info().
>
> If you think it's worth it, what is the driving benefit behind ?

It seems a bit cleaner to only register the tables for the dma lines that
are actually present on a given chip.

>> Does that mean it's actually a memory-to-memory transfer with a device being
>> on the external SRAM interface?
> I'm taking this is the follow up to the "-1" question :0

Right.

Arnd

Re: [PATCH 00/15] ARM: pxa: switch to DMA slave maps

[Arnd Bergmann]

On Mon, Apr 2, 2018 at 4:26 PM, Robert Jarzmik  wrote:
> Hi,
>
> This serie is aimed at removing the dmaengine slave compat use, and transfer
> knowledge of the DMA requestors into architecture code.
>
> This was discussed/advised by Arnd a couple of years back, it's almost time.
>
> The serie is divided in 3 phasees :
>  - phase 1 : patch 1/15 and patch 2/15
>=> this is the preparation work
>  - phase 2 : patches 3/15 .. 10/15
>=> this is the switch of all the drivers
>=> this one will require either an Ack of the maintainers or be taken by 
> them
>   once phase 1 is merged
>  - phase 3 : patches 11/15
>=> this is the last part, cleanup and removal of export of the DMA filter
>   function
>
> As this looks like a patch bomb, each maintainer expressing for his tree 
> either
> an Ack or "I want to take through my tree" will be spared in the next 
> iterations
> of this serie.
>
> Several of these changes have been tested on actual hardware, including :
>  - pxamci
>  - pxa_camera
>  - smc*
>  - ASoC and SSP
>
> Happy review.

This looks really great overall, thanks for cleaning this up!

The SSP part is still a bit weird, as I commented, but I'm sure we can
figure something out there.

Arnd

Re: [PATCH 12/15] dmaengine: pxa: make the filter function internal

[Arnd Bergmann]

On Mon, Apr 2, 2018 at 6:35 PM, kbuild test robot  wrote:

>
>drivers/mtd/nand/marvell_nand.c:2621:17: sparse: undefined identifier 
> 'pxad_filter_fn'
>>> drivers/mtd/nand/marvell_nand.c:2621:17: sparse: call with no type!
>In file included from drivers/mtd/nand/marvell_nand.c:21:0:
>drivers/mtd/nand/marvell_nand.c: In function 'marvell_nfc_init_dma':
>drivers/mtd/nand/marvell_nand.c:2621:42: error: 'pxad_filter_fn' 
> undeclared (first use in this function); did you mean 'dma_filter_fn'?
>   dma_request_slave_channel_compat(mask, pxad_filter_fn,
>  ^
>include/linux/dmaengine.h:1408:46: note: in definition of macro 
> 'dma_request_slave_channel_compat'
>  __dma_request_slave_channel_compat(&(mask), x, y, dev, name)
>  ^
>drivers/mtd/nand/marvell_nand.c:2621:42: note: each undeclared identifier 
> is reported only once for each function it appears in
>   dma_request_slave_channel_compat(mask, pxad_filter_fn,
>  ^
>include/linux/dmaengine.h:1408:46: note: in definition of macro 
> 'dma_request_slave_channel_compat'
>  __dma_request_slave_channel_compat(&(mask), x, y, dev, name)

The driver is a replacement for the pxa3xx nand driver, so it now has
to get changed as well.

   Arnd

Re: [PATCH 14/15] ARM: pxa: change SSP devices allocation

[Arnd Bergmann]

On Mon, Apr 2, 2018 at 4:26 PM, Robert Jarzmik  wrote:

>
> +static struct pxa_ssp_info pxa_ssp_infos[] = {
> +   { .dma_chan_rx_name = "ssp1_rx", .dma_chan_tx_name = "ssp1_tx", },
> +   { .dma_chan_rx_name = "ssp1_rx", .dma_chan_tx_name = "ssp1_tx", },
> +   { .dma_chan_rx_name = "ssp2_rx", .dma_chan_tx_name = "ssp2_tx", },
> +   { .dma_chan_rx_name = "ssp2_rx", .dma_chan_tx_name = "ssp2_tx", },
> +   { .dma_chan_rx_name = "ssp3_rx", .dma_chan_tx_name = "ssp3_tx", },
> +   { .dma_chan_rx_name = "ssp3_rx", .dma_chan_tx_name = "ssp3_tx", },
> +   { .dma_chan_rx_name = "ssp4_rx", .dma_chan_tx_name = "ssp4_tx", },
> +   { .dma_chan_rx_name = "ssp4_rx", .dma_chan_tx_name = "ssp4_tx", },
> +};

This part looks odd to me, you're adding an extra level of indirection to
do two stages of lookups in some form of platform data.

Why can't you just always use "rx" and "tx" as the names here?

(also, I don't see why each line is duplicated, but I'm sure there's
an easy answer for that).

   Arnd

Re: [PATCH 02/15] ARM: pxa: add dma slave map

[Arnd Bergmann]

On Mon, Apr 2, 2018 at 4:26 PM, Robert Jarzmik  wrote:
> +
> +static const struct dma_slave_map pxa_slave_map[] = {
> +   /* PXA25x, PXA27x and PXA3xx common entries */
> +   { "pxa-pcm-audio", "ac97_mic_mono", PDMA_FILTER_PARAM(LOWEST, 8) },
> +   { "pxa-pcm-audio", "ac97_aux_mono_in", PDMA_FILTER_PARAM(LOWEST, 9) },
> +   { "pxa-pcm-audio", "ac97_aux_mono_out", PDMA_FILTER_PARAM(LOWEST, 10) 
> },
> +   { "pxa-pcm-audio", "ac97_stereo_in", PDMA_FILTER_PARAM(LOWEST, 11) },
> +   { "pxa-pcm-audio", "ac97_stereo_out", PDMA_FILTER_PARAM(LOWEST, 12) },
> +   { "pxa-pcm-audio", "ssp1_rx", PDMA_FILTER_PARAM(LOWEST, 13) },
> +   { "pxa-pcm-audio", "ssp1_tx", PDMA_FILTER_PARAM(LOWEST, 14) },
> +   { "pxa-pcm-audio", "ssp2_rx", PDMA_FILTER_PARAM(LOWEST, 15) },
> +   { "pxa-pcm-audio", "ssp2_tx", PDMA_FILTER_PARAM(LOWEST, 16) },
> +   { "pxa2xx-ir", "rx", PDMA_FILTER_PARAM(LOWEST, 17) },
> +   { "pxa2xx-ir", "tx", PDMA_FILTER_PARAM(LOWEST, 18) },
> +   { "pxa2xx-mci.0", "rx", PDMA_FILTER_PARAM(LOWEST, 21) },
> +   { "pxa2xx-mci.0", "tx", PDMA_FILTER_PARAM(LOWEST, 22) },


> +   { "smc911x.0", "rx", PDMA_FILTER_PARAM(LOWEST, -1) },
> +   { "smc911x.0", "tx", PDMA_FILTER_PARAM(LOWEST, -1) },
> +   { "smc91x.0", "data", PDMA_FILTER_PARAM(LOWEST, -1) },

This one is interesting, as you are dealing with an off-chip device,
and the channel number is '-'1. How does this even work? Does it
mean

> +   /* PXA25x specific map */
> +   { "pxa25x-ssp.0", "rx", PDMA_FILTER_PARAM(LOWEST, 13) },
> +   { "pxa25x-ssp.0", "tx", PDMA_FILTER_PARAM(LOWEST, 14) },
> +   { "pxa25x-nssp.1", "rx", PDMA_FILTER_PARAM(LOWEST, 15) },
> +   { "pxa25x-nssp.1", "tx", PDMA_FILTER_PARAM(LOWEST, 16) },
> +   { "pxa25x-nssp.2", "rx", PDMA_FILTER_PARAM(LOWEST, 23) },
> +   { "pxa25x-nssp.2", "tx", PDMA_FILTER_PARAM(LOWEST, 24) },
> +   { "pxa-pcm-audio", "nssp2_rx", PDMA_FILTER_PARAM(LOWEST, 15) },
> +   { "pxa-pcm-audio", "nssp2_tx", PDMA_FILTER_PARAM(LOWEST, 16) },
> +   { "pxa-pcm-audio", "nssp3_rx", PDMA_FILTER_PARAM(LOWEST, 23) },
> +   { "pxa-pcm-audio", "nssp3_tx", PDMA_FILTER_PARAM(LOWEST, 24) },
> +
> +   /* PXA27x specific map */
> +   { "pxa-pcm-audio", "ssp3_rx", PDMA_FILTER_PARAM(LOWEST, 66) },
> +   { "pxa-pcm-audio", "ssp3_tx", PDMA_FILTER_PARAM(LOWEST, 67) },
> +   { "pxa27x-camera.0", "CI_Y", PDMA_FILTER_PARAM(HIGHEST, 68) },
> +   { "pxa27x-camera.0", "CI_U", PDMA_FILTER_PARAM(HIGHEST, 69) },
> +   { "pxa27x-camera.0", "CI_V", PDMA_FILTER_PARAM(HIGHEST, 70) },
> +
> +   /* PXA3xx specific map */
> +   { "pxa-pcm-audio", "ssp4_rx", PDMA_FILTER_PARAM(LOWEST, 2) },
> +   { "pxa-pcm-audio", "ssp4_tx", PDMA_FILTER_PARAM(LOWEST, 3) },
> +   { "pxa2xx-mci.1", "rx", PDMA_FILTER_PARAM(LOWEST, 93) },
> +   { "pxa2xx-mci.1", "tx", PDMA_FILTER_PARAM(LOWEST, 94) },
> +   { "pxa3xx-nand", "data", PDMA_FILTER_PARAM(LOWEST, 97) },
> +   { "pxa2xx-mci.2", "rx", PDMA_FILTER_PARAM(LOWEST, 100) },
> +   { "pxa2xx-mci.2", "tx", PDMA_FILTER_PARAM(LOWEST, 101) },
> +};

Since more than half the entries in here are chip specific, maybe it would be
better to split that table into three and have a copy for each one in
arch/arm/mach-pxa/pxa{25x.27x.3xx}.c? Does that mean it's actually
a memory-to-memory transfer with a device being on the external
SRAM interface?

   Arnd

[PATCH 20/47] media: platform: remove blackfin capture driver

[Arnd Bergmann]

The blackfin architecture is getting removed, so the video
capture driver is also obsolete.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/platform/Kconfig |   2 -
 drivers/media/platform/Makefile|   2 -
 drivers/media/platform/blackfin/Kconfig|  16 -
 drivers/media/platform/blackfin/Makefile   |   2 -
 drivers/media/platform/blackfin/bfin_capture.c | 983 -
 drivers/media/platform/blackfin/ppi.c  | 361 -
 include/media/blackfin/bfin_capture.h  |  39 -
 include/media/blackfin/ppi.h   |  94 ---
 8 files changed, 1499 deletions(-)
 delete mode 100644 drivers/media/platform/blackfin/Kconfig
 delete mode 100644 drivers/media/platform/blackfin/Makefile
 delete mode 100644 drivers/media/platform/blackfin/bfin_capture.c
 delete mode 100644 drivers/media/platform/blackfin/ppi.c
 delete mode 100644 include/media/blackfin/bfin_capture.h
 delete mode 100644 include/media/blackfin/ppi.h

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 5d8fd71fc454..2136702c95fc 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -31,8 +31,6 @@ source "drivers/media/platform/davinci/Kconfig"
 
 source "drivers/media/platform/omap/Kconfig"
 
-source "drivers/media/platform/blackfin/Kconfig"
-
 config VIDEO_SH_VOU
tristate "SuperH VOU video output driver"
depends on MEDIA_CAMERA_SUPPORT
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 85e112122f32..2b07f2e2fca6 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -53,8 +53,6 @@ obj-$(CONFIG_VIDEO_TEGRA_HDMI_CEC)+= tegra-cec/
 
 obj-y  += stm32/
 
-obj-y   += blackfin/
-
 obj-y  += davinci/
 
 obj-$(CONFIG_VIDEO_SH_VOU) += sh_vou.o
diff --git a/drivers/media/platform/blackfin/Kconfig 
b/drivers/media/platform/blackfin/Kconfig
deleted file mode 100644
index 68fa90151b8f..
diff --git a/drivers/media/platform/blackfin/Makefile 
b/drivers/media/platform/blackfin/Makefile
deleted file mode 100644
index 30421bc23080..
diff --git a/drivers/media/platform/blackfin/bfin_capture.c 
b/drivers/media/platform/blackfin/bfin_capture.c
deleted file mode 100644
index b7660b1000fd..
diff --git a/drivers/media/platform/blackfin/ppi.c 
b/drivers/media/platform/blackfin/ppi.c
deleted file mode 100644
index d3dc765c1609..
diff --git a/include/media/blackfin/bfin_capture.h 
b/include/media/blackfin/bfin_capture.h
deleted file mode 100644
index a999a3970c69..
diff --git a/include/media/blackfin/ppi.h b/include/media/blackfin/ppi.h
deleted file mode 100644
index 987e49e8f9c9..
-- 
2.9.0

[PATCH 21/47] media: platform: remove m32r specific arv driver

[Arnd Bergmann]

The m32r architecture is getting removed, so this one is no longer needed.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/platform/Kconfig  |  20 -
 drivers/media/platform/Makefile |   2 -
 drivers/media/platform/arv.c| 884 
 3 files changed, 906 deletions(-)
 delete mode 100644 drivers/media/platform/arv.c

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 2136702c95fc..c7a1cf8a1b01 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -52,26 +52,6 @@ config VIDEO_VIU
  Say Y here if you want to enable VIU device on MPC5121e Rev2+.
  In doubt, say N.
 
-config VIDEO_M32R_AR
-   tristate "AR devices"
-   depends on VIDEO_V4L2
-   depends on M32R || COMPILE_TEST
-   ---help---
- This is a video4linux driver for the Renesas AR (Artificial Retina)
- camera module.
-
-config VIDEO_M32R_AR_M64278
-   tristate "AR device with color module M64278(VGA)"
-   depends on PLAT_M32700UT
-   select VIDEO_M32R_AR
-   ---help---
- This is a video4linux driver for the Renesas AR (Artificial
- Retina) with M64278E-800 camera module.
- This module supports VGA(640x480 pixels) resolutions.
-
- To compile this driver as a module, choose M here: the
- module will be called arv.
-
 config VIDEO_MUX
tristate "Video Multiplexer"
select MULTIPLEXER
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 2b07f2e2fca6..932515df4477 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -3,8 +3,6 @@
 # Makefile for the video capture/playback device drivers.
 #
 
-obj-$(CONFIG_VIDEO_M32R_AR_M64278) += arv.o
-
 obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o
 obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/
 obj-$(CONFIG_VIDEO_MMP_CAMERA) += marvell-ccic/
diff --git a/drivers/media/platform/arv.c b/drivers/media/platform/arv.c
deleted file mode 100644
index 1e865fea803c..
-- 
2.9.0

[PATCH 00/47] arch-removal: device drivers

[Arnd Bergmann]

Hi driver maintainers,

I just posted one series with the removal of eight architectures,
see https://lkml.org/lkml/2018/3/14/505 for details, or
https://lwn.net/Articles/748074/ for more background.

These are the device drivers that go along with them. I have already
picked up the drivers for arch/metag/ into my tree, they were reviewed
earlier.

Please let me know if you have any concerns with the patch, or if you
prefer to pick up the patches in your respective trees.  I created
the patches with 'git format-patch -D', so they will not apply without
manually removing those files.

For anything else, I'd keep the removal patches in my asm-generic tree
and will send a pull request for 4.17 along with the actual arch removal.

   Arnd

Arnd Bergmann
  edac: remove tile driver
  net: tile: remove ethernet drivers
  net: adi: remove blackfin ethernet drivers
  net: 8390: remove m32r specific bits
  net: remove cris etrax ethernet driver
  net: smsc: remove m32r specific smc91x configuration
  raid: remove tile specific raid6 implementation
  rtc: remove tile driver
  rtc: remove bfin driver
  char: remove obsolete ds1302 rtc driver
  char: remove tile-srom.c
  char: remove blackfin OTP driver
  pcmcia: remove m32r drivers
  pcmcia: remove blackfin driver
  ASoC: remove blackfin drivers
  video/logo: remove obsolete logo files
  fbdev: remove blackfin drivers
  fbdev: s1d13xxxfb: remove m32r specific hacks
  crypto: remove blackfin CRC driver
  media: platform: remove blackfin capture driver
  media: platform: remove m32r specific arv driver
  cpufreq: remove blackfin driver
  cpufreq: remove cris specific drivers
  gpio: remove etraxfs driver
  pinctrl: remove adi2/blackfin drivers
  ata: remove bf54x driver
  input: keyboard: remove bf54x driver
  input: misc: remove blackfin rotary driver
  mmc: remove bfin_sdh driver
  can: remove bfin_can driver
  watchdog: remove bfin_wdt driver
  mtd: maps: remove bfin-async-flash driver
  mtd: nand: remove bf5xx_nand driver
  spi: remove blackfin related host drivers
  i2c: remove bfin-twi driver
  pwm: remobe pwm-bfin driver
  usb: host: remove tilegx platform glue
  usb: musb: remove blackfin port
  usb: isp1362: remove blackfin arch glue
  serial: remove cris/etrax uart drivers
  serial: remove blackfin drivers
  serial: remove m32r_sio driver
  serial: remove tile uart driver
  tty: remove bfin_jtag_comm and hvc_bfin_jtag drivers
  tty: hvc: remove tile driver
  staging: irda: remove bfin_sir driver
  staging: iio: remove iio-trig-bfin-timer driver

 .../devicetree/bindings/gpio/gpio-etraxfs.txt  |   22 -
 .../bindings/serial/axis,etraxfs-uart.txt  |   22 -
 Documentation/watchdog/watchdog-parameters.txt |5 -
 MAINTAINERS|8 -
 drivers/ata/Kconfig|9 -
 drivers/ata/Makefile   |1 -
 drivers/ata/pata_bf54x.c   | 1703 
 drivers/char/Kconfig   |   48 -
 drivers/char/Makefile  |3 -
 drivers/char/bfin-otp.c|  237 --
 drivers/char/ds1302.c  |  357 --
 drivers/char/tile-srom.c   |  475 ---
 drivers/cpufreq/Makefile   |3 -
 drivers/cpufreq/blackfin-cpufreq.c |  217 -
 drivers/cpufreq/cris-artpec3-cpufreq.c |   93 -
 drivers/cpufreq/cris-etraxfs-cpufreq.c |   92 -
 drivers/crypto/Kconfig |7 -
 drivers/crypto/Makefile|1 -
 drivers/crypto/bfin_crc.c  |  753 
 drivers/crypto/bfin_crc.h  |  124 -
 drivers/edac/Kconfig   |8 -
 drivers/edac/Makefile  |2 -
 drivers/edac/tile_edac.c   |  265 --
 drivers/gpio/Kconfig   |9 -
 drivers/gpio/Makefile  |1 -
 drivers/gpio/gpio-etraxfs.c|  475 ---
 drivers/i2c/busses/Kconfig |   18 -
 drivers/i2c/busses/Makefile|1 -
 drivers/i2c/busses/i2c-bfin-twi.c  |  737 
 drivers/input/keyboard/Kconfig |9 -
 drivers/input/keyboard/Makefile|1 -
 drivers/input/keyboard/bf54x-keys.c|  396 --
 drivers/input/misc/Kconfig |9 -
 drivers/input/misc/Makefile|1 -
 drivers/input/misc/bfin_rotary.c   |  294 --
 drivers/media/platform/Kconfig |   22 -
 drivers/media/platform/Makefile|4 -
 drivers/media/platform/arv.c   |  884 
 drivers/media/platform/blackfin/Kconfig|   16 -
 drivers/media/platform/blackfin/Makefile

[PATCH] media: imx: work around false-positive warning

[Arnd Bergmann]

The IS_ERR()/PTR_ERR() combination confuses gcc to the point that it
cannot prove the upstream_ep variable to be initialized:

drivers/staging/media/imx/imx-media-csi.c: In function 'csi_link_validate':
drivers/staging/media/imx/imx-media-csi.c:1025:20: error: 'upstream_ep' may be 
used uninitialized in this function [-Werror=maybe-uninitialized]
  priv->upstream_ep = upstream_ep;
  ~~^
drivers/staging/media/imx/imx-media-csi.c:1026:24: error: 
'upstream_ep.bus_type' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
  is_csi2 = (upstream_ep.bus_type == V4L2_MBUS_CSI2);
 ~~~^
drivers/staging/media/imx/imx-media-csi.c:127:19: error: 
'upstream_ep.bus.parallel.bus_width' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]

I could come up with no good way to rewrite this function, as a last
resort, this adds an explicit zero-intialization of the structure.

Fixes: 52e17089d185 ("media: imx: Don't initialize vars that won't be used")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/staging/media/imx/imx-media-csi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 5a195f80a24d..887fed0c3ce0 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1004,7 +1004,7 @@ static int csi_link_validate(struct v4l2_subdev *sd,
 struct v4l2_subdev_format *sink_fmt)
 {
struct csi_priv *priv = v4l2_get_subdevdata(sd);
-   struct v4l2_fwnode_endpoint upstream_ep;
+   struct v4l2_fwnode_endpoint upstream_ep = {};
const struct imx_media_pixfmt *incc;
bool is_csi2;
int ret;
-- 
2.9.0

[PATCH] media: ngene: avoid unused variable warning

[Arnd Bergmann]

The newly added pdev variable is only used in an #ifdef, causing a
build warning without CONFIG_PCI_MSI, unless we move the declaration
inside the same #ifdef:

drivers/media/pci/ngene/ngene-core.c: In function 'ngene_start':
drivers/media/pci/ngene/ngene-core.c:1328:17: error: unused variable 'pdev' 
[-Werror=unused-variable]

Fixes: 6795bf626482 ("media: ngene: convert kernellog printing from printk() to 
dev_*() macros")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/pci/ngene/ngene-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index 3b9a1bfaf6c0..25f16833a475 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -1325,7 +1325,6 @@ static int ngene_buffer_config(struct ngene *dev)
 
 static int ngene_start(struct ngene *dev)
 {
-   struct device *pdev = >pci_dev->dev;
int stat;
int i;
 
@@ -1359,6 +1358,7 @@ static int ngene_start(struct ngene *dev)
 #ifdef CONFIG_PCI_MSI
/* enable MSI if kernel and card support it */
if (pci_msi_enabled() && dev->card_info->msi_supported) {
+   struct device *pdev = >pci_dev->dev;
unsigned long flags;
 
ngwritel(0, NGENE_INT_ENABLE);
-- 
2.9.0

[PATCH] media: v4l: omap_vout: vrfb: remove an unused variable

[Arnd Bergmann]

We now get a warning after the 'dmadev' variable is no longer used:

drivers/media/platform/omap/omap_vout_vrfb.c: In function 
'omap_vout_prepare_vrfb':
drivers/media/platform/omap/omap_vout_vrfb.c:239:21: error: unused variable 
'dmadev' [-Werror=unused-variable]

Fixes: 8f0aa38292f2 ("media: v4l: omap_vout: vrfb: Use the wrapper for 
prep_interleaved_dma()")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/platform/omap/omap_vout_vrfb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/omap/omap_vout_vrfb.c 
b/drivers/media/platform/omap/omap_vout_vrfb.c
index 72c0ac2cbf3d..1d8508237220 100644
--- a/drivers/media/platform/omap/omap_vout_vrfb.c
+++ b/drivers/media/platform/omap/omap_vout_vrfb.c
@@ -236,7 +236,6 @@ int omap_vout_prepare_vrfb(struct omap_vout_device *vout,
struct dma_async_tx_descriptor *tx;
enum dma_ctrl_flags flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
struct dma_chan *chan = vout->vrfb_dma_tx.chan;
-   struct dma_device *dmadev = chan->device;
struct dma_interleaved_template *xt = vout->vrfb_dma_tx.xt;
dma_cookie_t cookie;
enum dma_status status;
-- 
2.9.0

[PATCH] media: cxd2880-spi: avoid out-of-bounds access warning

[Arnd Bergmann]

The -Warray-bounds warning in gcc-8 triggers for a newly added file:

drivers/media/spi/cxd2880-spi.c: In function 'cxd2880_write_reg':
drivers/media/spi/cxd2880-spi.c:111:3: error: 'memcpy' forming offset [133, 
258] is out of the bounds [0, 132] of object 'send_data' with type 'u8[132]' 
{aka 'unsigned char[132]'} [-Werror=array-bounds]

The problem appears to be that we have two range checks in this function,
first comparing against BURST_WRITE_MAX (128) and then comparing against
a literal '255'. The logic checking the buffer size looks at the second
one and decides that this might be the actual maximum data length.

This is understandable behavior from the compiler, but the code is actually
safe. Since the first check is already shorter, we can remove the loop
and only leave that. To be on the safe side in case BURST_WRITE_MAX might
be increased, I'm leaving the check against U8_MAX.

Fixes: bd24fcddf6b8 ("media: cxd2880-spi: Add support for CXD2880 SPI 
interface")
Cc: Martin Sebor <mse...@gmail.com>
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/spi/cxd2880-spi.c | 24 +++-
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/media/spi/cxd2880-spi.c b/drivers/media/spi/cxd2880-spi.c
index 4df3bd312f48..011a37c2f272 100644
--- a/drivers/media/spi/cxd2880-spi.c
+++ b/drivers/media/spi/cxd2880-spi.c
@@ -88,7 +88,7 @@ static int cxd2880_write_reg(struct spi_device *spi,
pr_err("invalid arg\n");
return -EINVAL;
}
-   if (size > BURST_WRITE_MAX) {
+   if (size > BURST_WRITE_MAX || size > U8_MAX) {
pr_err("data size > WRITE_MAX\n");
return -EINVAL;
}
@@ -101,24 +101,14 @@ static int cxd2880_write_reg(struct spi_device *spi,
send_data[0] = 0x0e;
write_data_top = data;
 
-   while (size > 0) {
-   send_data[1] = sub_address;
-   if (size > 255)
-   send_data[2] = 255;
-   else
-   send_data[2] = (u8)size;
+   send_data[1] = sub_address;
+   send_data[2] = (u8)size;
 
-   memcpy(_data[3], write_data_top, send_data[2]);
+   memcpy(_data[3], write_data_top, send_data[2]);
 
-   ret = cxd2880_write_spi(spi, send_data, send_data[2] + 3);
-   if (ret) {
-   pr_err("write spi failed %d\n", ret);
-   break;
-   }
-   sub_address += send_data[2];
-   write_data_top += send_data[2];
-   size -= send_data[2];
-   }
+   ret = cxd2880_write_spi(spi, send_data, send_data[2] + 3);
+   if (ret)
+   pr_err("write spi failed %d\n", ret);
 
return ret;
 }
-- 
2.9.0

Re: [PATCH 00/13] Remove metag architecture

[Arnd Bergmann]

On Thu, Feb 22, 2018 at 12:38 AM, James Hogan  wrote:
> These patches remove the metag architecture and tightly dependent
> drivers from the kernel. With the 4.16 kernel the ancient gcc 4.2.4
> based metag toolchain we have been using is hitting compiler bugs, so
> now seems a good time to drop it altogether.
>
> Quoting from patch 1:
>
> The earliest Meta architecture port of Linux I have a record of was an
> import of a Meta port of Linux v2.4.1 in February 2004, which was worked
> on significantly over the next few years by Graham Whaley, Will Newton,
> Matt Fleming, myself and others.
>
> Eventually the port was merged into mainline in v3.9 in March 2013, not
> long after Imagination Technologies bought MIPS Technologies and shifted
> its CPU focus over to the MIPS architecture.
>
> As a result, though the port was maintained for a while, kept on life
> support for a while longer, and useful for testing a few specific
> drivers for which I don't have ready access to the equivalent MIPS
> hardware, it is now essentially dead with no users.
>
> It is also stuck using an out-of-tree toolchain based on GCC 4.2.4 which
> is no longer maintained, now struggles to build modern kernels due to
> toolchain bugs, and doesn't itself build with a modern GCC. The latest
> buildroot port is still using an old uClibc snapshot which is no longer
> served, and the latest uClibc doesn't build with GCC 4.2.4.
>
> So lets call it a day and drop the Meta architecture port from the
> kernel. RIP Meta.

I've pulled it into my asm-generic tree now, which is also part of linux-next,
and followed up with patches removing frv, m32r, score, unicore32
and blackfin. I have not removed the device drivers yet, but I'm working
on that.

   Arnd

[PATCH] media: renesas-ceu: mark PM functions as __maybe_unused

[Arnd Bergmann]

The PM runtime operations are unused when CONFIG_PM is disabled,
leading to a harmless warning:

drivers/media/platform/renesas-ceu.c:1003:12: error: 'ceu_runtime_suspend' 
defined but not used [-Werror=unused-function]
 static int ceu_runtime_suspend(struct device *dev)
^~~
drivers/media/platform/renesas-ceu.c:987:12: error: 'ceu_runtime_resume' 
defined but not used [-Werror=unused-function]
 static int ceu_runtime_resume(struct device *dev)
^~

This adds a __maybe_unused annotation to shut up the warning.

Fixes: 32e5a70dc8f4 ("media: platform: Add Renesas CEU driver")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/platform/renesas-ceu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/renesas-ceu.c 
b/drivers/media/platform/renesas-ceu.c
index 22330c0c2f6a..eccd60a7ebec 100644
--- a/drivers/media/platform/renesas-ceu.c
+++ b/drivers/media/platform/renesas-ceu.c
@@ -984,7 +984,7 @@ static int ceu_init_mbus_fmt(struct ceu_device *ceudev)
 /*
  * ceu_runtime_resume() - soft-reset the interface and turn sensor power on.
  */
-static int ceu_runtime_resume(struct device *dev)
+static int __maybe_unused ceu_runtime_resume(struct device *dev)
 {
struct ceu_device *ceudev = dev_get_drvdata(dev);
struct v4l2_subdev *v4l2_sd = ceudev->sd->v4l2_sd;
@@ -1000,7 +1000,7 @@ static int ceu_runtime_resume(struct device *dev)
  * ceu_runtime_suspend() - disable capture and interrupts and soft-reset.
  *Turn sensor power off.
  */
-static int ceu_runtime_suspend(struct device *dev)
+static int __maybe_unused ceu_runtime_suspend(struct device *dev)
 {
struct ceu_device *ceudev = dev_get_drvdata(dev);
struct v4l2_subdev *v4l2_sd = ceudev->sd->v4l2_sd;
-- 
2.9.0

[PATCH 2/2] media: ov5695: mark PM functions as __maybe_unused

[Arnd Bergmann]

Without CONFIG_PM, we get a harmless build warning:

drivers/media/i2c/ov2685.c:516:12: error: 'ov2685_runtime_suspend' defined but 
not used [-Werror=unused-function]
 static int ov2685_runtime_suspend(struct device *dev)
^~
drivers/media/i2c/ov2685.c:507:12: error: 'ov2685_runtime_resume' defined but 
not used [-Werror=unused-function]
 static int ov2685_runtime_resume(struct device *dev)

This marks the affected functions as __maybe_unused.

Fixes: e3861d9118c8 ("media: ov2685: add support for OV2685 sensor")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/i2c/ov2685.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov2685.c b/drivers/media/i2c/ov2685.c
index 904ac305d499..9ac702e3ae95 100644
--- a/drivers/media/i2c/ov2685.c
+++ b/drivers/media/i2c/ov2685.c
@@ -504,7 +504,7 @@ static int ov2685_open(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh)
 }
 #endif
 
-static int ov2685_runtime_resume(struct device *dev)
+static int __maybe_unused ov2685_runtime_resume(struct device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct v4l2_subdev *sd = i2c_get_clientdata(client);
@@ -513,7 +513,7 @@ static int ov2685_runtime_resume(struct device *dev)
return __ov2685_power_on(ov2685);
 }
 
-static int ov2685_runtime_suspend(struct device *dev)
+static int __maybe_unused ov2685_runtime_suspend(struct device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct v4l2_subdev *sd = i2c_get_clientdata(client);
-- 
2.9.0

[PATCH 1/2] media: ov5695: mark PM functions as __maybe_unused

[Arnd Bergmann]

Without CONFIG_PM, we get a harmless build warning:

drivers/media/i2c/ov5695.c:1033:12: error: 'ov5695_runtime_suspend' defined but 
not used [-Werror=unused-function]
 static int ov5695_runtime_suspend(struct device *dev)
^~
drivers/media/i2c/ov5695.c:1024:12: error: 'ov5695_runtime_resume' defined but 
not used [-Werror=unused-function]
 static int ov5695_runtime_resume(struct device *dev)

This marks the affected functions as __maybe_unused.

Fixes: 8a77009be4be ("media: ov5695: add support for OV5695 sensor")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/i2c/ov5695.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov5695.c b/drivers/media/i2c/ov5695.c
index 2db7d2e535b9..a4985a4715f5 100644
--- a/drivers/media/i2c/ov5695.c
+++ b/drivers/media/i2c/ov5695.c
@@ -1021,7 +1021,7 @@ static void __ov5695_power_off(struct ov5695 *ov5695)
regulator_bulk_disable(OV5695_NUM_SUPPLIES, ov5695->supplies);
 }
 
-static int ov5695_runtime_resume(struct device *dev)
+static int __maybe_unused ov5695_runtime_resume(struct device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct v4l2_subdev *sd = i2c_get_clientdata(client);
@@ -1030,7 +1030,7 @@ static int ov5695_runtime_resume(struct device *dev)
return __ov5695_power_on(ov5695);
 }
 
-static int ov5695_runtime_suspend(struct device *dev)
+static int __maybe_unused ov5695_runtime_suspend(struct device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct v4l2_subdev *sd = i2c_get_clientdata(client);
-- 
2.9.0

[PATCH] media: i2c: TDA1997x: add CONFIG_SND dependency

[Arnd Bergmann]

Without CONFIG_SND, we get a link error:

ERROR: "snd_soc_register_codec" [drivers/media/i2c/tda1997x.ko] undefined!
ERROR: "snd_soc_unregister_codec" [drivers/media/i2c/tda1997x.ko] undefined!
ERROR: "snd_pcm_hw_constraint_minmax" [drivers/media/i2c/tda1997x.ko] undefined!

This adds the same Kconfig dependency that we have in other
media drivers, using 'select SND_PCM' to ensure that we have
can call snd_pcm_hw_constraint_minmax, while depending on
CONFIG_SND_SOC for registering the codec.

Fixes: 9ac0038db9a7 ("media: i2c: Add TDA1997x HDMI receiver driver")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/i2c/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 94e32d75d632..a44e8c36f13c 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -59,6 +59,8 @@ config VIDEO_TDA9840
 config VIDEO_TDA1997X
tristate "NXP TDA1997x HDMI receiver"
depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   depends on SND_SOC
+   select SND_PCM
---help---
  V4L2 subdevice driver for the NXP TDA1997x HDMI receivers.
 
-- 
2.9.0

Re: [PATCH 00/13] Remove metag architecture

[Arnd Bergmann]

On Fri, Feb 23, 2018 at 12:02 PM, James Hogan <jho...@kernel.org> wrote:
> On Fri, Feb 23, 2018 at 11:26:58AM +0100, Arnd Bergmann wrote:
>> On Thu, Feb 22, 2018 at 12:38 AM, James Hogan <jho...@kernel.org> wrote:
>> > So lets call it a day and drop the Meta architecture port from the
>> > kernel. RIP Meta.
>>
>> Since I brought up the architecture removal independently, I could
>> pick this up into a git tree that also has the removal of some of the
>> other architectures.
>>
>> I see your tree is part of linux-next, so you could also just put it
>> in there and send a pull request at the merge window if you prefer.
>>
>> The only real reason I see for a shared git tree would be to avoid
>> conflicts when we touch the same Kconfig files or #ifdefs in driver,
>> but Meta only appears in
>>
>> config FRAME_POINTER
>> bool "Compile the kernel with frame pointers"
>> depends on DEBUG_KERNEL && \
>> (CRIS || M68K || FRV || UML || \
>>  SUPERH || BLACKFIN || MN10300 || METAG) || \
>> ARCH_WANT_FRAME_POINTERS
>>
>> and
>>
>> include/trace/events/mmflags.h:#elif defined(CONFIG_PARISC) ||
>> defined(CONFIG_METAG) || defined(CONFIG_IA64)
>>
>> so there is little risk.
>
> I'm happy to put v2 in linux-next now (only patch 4 has changed, I just
> sent an updated version), and send you a pull request early next week so
> you can take it from there. The patches can't be directly applied with
> git-am anyway thanks to the -D option to make them more concise.
>
> Sound okay?

Yes, sounds good, thanks!

   Arnd

Re: [PATCH 00/13] Remove metag architecture

[Arnd Bergmann]

On Thu, Feb 22, 2018 at 12:38 AM, James Hogan  wrote:
> These patches remove the metag architecture and tightly dependent
> drivers from the kernel. With the 4.16 kernel the ancient gcc 4.2.4
> based metag toolchain we have been using is hitting compiler bugs, so
> now seems a good time to drop it altogether.
>
> Quoting from patch 1:
>
> The earliest Meta architecture port of Linux I have a record of was an
> import of a Meta port of Linux v2.4.1 in February 2004, which was worked
> on significantly over the next few years by Graham Whaley, Will Newton,
> Matt Fleming, myself and others.
>
> Eventually the port was merged into mainline in v3.9 in March 2013, not
> long after Imagination Technologies bought MIPS Technologies and shifted
> its CPU focus over to the MIPS architecture.
>
> As a result, though the port was maintained for a while, kept on life
> support for a while longer, and useful for testing a few specific
> drivers for which I don't have ready access to the equivalent MIPS
> hardware, it is now essentially dead with no users.
>
> It is also stuck using an out-of-tree toolchain based on GCC 4.2.4 which
> is no longer maintained, now struggles to build modern kernels due to
> toolchain bugs, and doesn't itself build with a modern GCC. The latest
> buildroot port is still using an old uClibc snapshot which is no longer
> served, and the latest uClibc doesn't build with GCC 4.2.4.
>
> So lets call it a day and drop the Meta architecture port from the
> kernel. RIP Meta.

Since I brought up the architecture removal independently, I could
pick this up into a git tree that also has the removal of some of the
other architectures.

I see your tree is part of linux-next, so you could also just put it
in there and send a pull request at the merge window if you prefer.

The only real reason I see for a shared git tree would be to avoid
conflicts when we touch the same Kconfig files or #ifdefs in driver,
but Meta only appears in

config FRAME_POINTER
bool "Compile the kernel with frame pointers"
depends on DEBUG_KERNEL && \
(CRIS || M68K || FRV || UML || \
 SUPERH || BLACKFIN || MN10300 || METAG) || \
ARCH_WANT_FRAME_POINTERS

and

include/trace/events/mmflags.h:#elif defined(CONFIG_PARISC) ||
defined(CONFIG_METAG) || defined(CONFIG_IA64)

so there is little risk.

  Arnd

Re: stable-rc build: 3 warnings 0 failures (stable-rc/v4.14.20-119-g1b1ab1d)

[Arnd Bergmann]

On Tue, Feb 20, 2018 at 1:47 PM, Olof's autobuilder  wrote:

> Warnings:
>
> arm64.allmodconfig:
> drivers/media/tuners/r820t.c:1334:1: warning: the frame size of 2896 bytes is 
> larger than 2048 bytes [-Wframe-larger-than=]

Hi Greg,

please add

16c3ada89cff ("media: r820t: fix r820t_write_reg for KASAN")

This is an old bug, but hasn't shown up before as the stack warning
limit was turned off
in allmodconfig kernels. The fix is also on the backport lists I sent
for 4.9 and 4.4.

Arnd

[PATCH] staging: media: atomisp: remove pointless string copy

[Arnd Bergmann]

gcc-8 points out that a string is copied to itself here:

In file included from 
drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/platform_support.h:25,
 from 
drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/memory_access/memory_access.h:48,
 from 
drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c:16:
In function 'strncpy',
inlined from 'ia_css_debug_pipe_graph_dump_stage' at 
drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/string_support.h:158:2:
include/linux/string.h:253:9: error: '__builtin_strncpy' source argument is the 
same as destination [-Werror=restrict]
  return __builtin_strncpy(p, q, size);
 ^

This removes the bogus code, leaving the behavior otherwise
unchanged.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 .../atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c
index f22d73b56bc6..60395904f89a 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c
@@ -2858,13 +2858,7 @@ ia_css_debug_pipe_graph_dump_stage(
if (l && enable_info[l-1] == ',')
enable_info[--l] = '\0';
 
-   if (l <= ENABLE_LINE_MAX_LENGTH) {
-   /* It fits on one line, copy string and init */
-   /* other helper strings with empty string */
-   strcpy_s(enable_info,
-   sizeof(enable_info),
-   ei);
-   } else {
+   if (l > ENABLE_LINE_MAX_LENGTH) {
/* Too big for one line, find last comma */
p = ENABLE_LINE_MAX_LENGTH;
while (ei[p] != ',')
-- 
2.9.0

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

[Arnd Bergmann]

On Thu, Feb 1, 2018 at 4:29 PM, Maxime Ripard
<maxime.rip...@free-electrons.com> 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: [linux-sunxi] Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

[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

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

[Arnd Bergmann]

On Wed, Jan 31, 2018 at 8:29 AM, Maxime Ripard
<maxime.rip...@free-electrons.com> 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
>> > > <maxime.rip...@free-electrons.com> 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
>> > > >> <linus.wall...@linaro.org> wrote:
>> > > >> > On Mon, Jan 29, 2018 at 9:25 AM, Maxime Ripard
>> > > >> > <maxime.rip...@free-electrons.com> 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.

> 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 

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

[Arnd Bergmann]

On Tue, Jan 30, 2018 at 8:54 AM, Maxime Ripard
<maxime.rip...@free-electrons.com> 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
>> <linus.wall...@linaro.org> wrote:
>> > On Mon, Jan 29, 2018 at 9:25 AM, Maxime Ripard
>> > <maxime.rip...@free-electrons.com> 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.

   Arnd

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

[Arnd Bergmann]

On Mon, Jan 29, 2018 at 10:49 PM, Randy Dunlap  wrote:
> On 01/29/2018 01:21 AM, Yong Deng wrote:
>> Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
>> interface and CSI1 is used for parallel interface. This is not
>> documented in datasheet but by test and guess.
>>
>> This patch implement a v4l2 framework driver for it.
>>
>> Currently, the driver only support the parallel interface. MIPI-CSI2,
>> ISP's support are not included in this patch.
>>
>> Tested-by: Maxime Ripard 
>> Signed-off-by: Yong Deng 
>> ---
>
>
> A previous version (I think v6) had a build error with the use of
> PHYS_OFFSET, so Kconfig was modified to depend on ARM and ARCH_SUNXI
> (one of which seems to be overkill).  As is here, the COMPILE_TEST piece is
> meaningless for all arches except ARM.  If you care enough for COMPILE_TEST
> (and I would), then you could make COMPILE_TEST useful on any arch by
> removing the "depends on ARM" (the ARCH_SUNXI takes care of that) and by
> having an alternate value for PHYS_OFFSET, like so:
>
> +#if defined(CONFIG_COMPILE_TEST) && !defined(PHYS_OFFSET)
> +#define PHYS_OFFSET0
> +#endif
>
> With those 2 changes, the driver builds for me on x86_64.

I think the PHYS_OFFSET really has to get removed from the driver, it's
wrong on ARM as well.

  Arnd

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

[Arnd Bergmann]

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:
>> However, in DT systems, that
>> field is filled only with the parent's node dma-ranges property. In
>> our case, and since the DT parenthood is based on the "control" bus,
>> and not the "data" bus, our parent node would be the AXI bus, and not
>> the memory bus that enforce those constraints.
>>
>> And other devices doing DMA through regular DMA accesses won't have
>> that mapping, so we definitely shouldn't enforce it for all the
>> devices there, but only the one connected to the separate memory bus.
>>
>> tl; dr: the DT is not really an option to store that info.
>>
>> I suggested setting dma_pfn_offset at probe, but Arnd didn't seem too
>> fond of that approach either at the time.
>>
>> So, well, I guess we could do better. We just have no idea how :)
>
> Usually of Arnd cannot suggest something smart, neither can I :(
>
> If some aspect of the memory layout of the system *really* doesn't
> fit into DT because of the assumptions that DT is doing about
> how systems work, we have a problem.
>
> Is the problem that DT's model assumes that devices have one and
> only one data port to the system memory, and that is how it
> populates the Linux devices?
>
> I guess, if nothing else works, I would use auxdata in the board file.
> It is our definitive last resort when DT doesn't hold.
>
> I.e. I would go into struct of_dev_auxdata
> (from ) and add a
> dma_pfn_offset field that gets set into the device's dma_pfn_offset
> in drivers/of/platform.c overriding anything else and use that to hammer
> it down in the boardfile.
>
> But I bet some DT people are going to have other ideas.

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.

Another local hack that we can do here is to have two separate
device nodes and let the device driver bind to one device and then
look up the other one through a phandle to look up a platform_device
for it, which it can then use with the DMA mapping interface.

  Arnd

Re: [PATCH] staging: imx-media-vdic: fix inconsistent IS_ERR and PTR_ERR

[Arnd Bergmann]

On Wed, Jan 24, 2018 at 1:43 AM, Gustavo A. R. Silva
<gust...@embeddedor.com> wrote:
> Fix inconsistent IS_ERR and PTR_ERR in vdic_get_ipu_resources.
> The proper pointer to be passed as argument is ch.
>
> This issue was detected with the help of Coccinelle.
>
> Fixes: 0b2e9e7947e7 ("media: staging/imx: remove confusing IS_ERR_OR_NULL 
> usage")
> Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>

good catch!

Acked-by: Arnd Bergmann <a...@arndb.de>

Re: [PATCH] [media] s3c-camif: array underflow in __camif_subdev_try_format()

[Arnd Bergmann]

On Wed, Jan 24, 2018 at 9:13 AM, Dan Carpenter  wrote:
> On Mon, Jan 22, 2018 at 09:50:04PM +0100, Sylwester Nawrocki wrote:
>> On 01/22/2018 11:37 AM, Dan Carpenter wrote:

> I happened to be looking at the same bugs but using Smatch.  Did you get
> these two bugs as well?
>
> drivers/scsi/sym53c8xx_2/sym_hipd.c:549 sym_getsync() error: iterator 
> underflow 'div_10M' (-1)-255
> drivers/media/i2c/sr030pc30.c:522 try_fmt() error: iterator underflow 
> 'sr030pc30_formats' (-1)-4

I don't recall seeing those two, here is a list of array-bounds
warnings I had in the past
months, mostly while building with gcc-7.2:

/git/arm-soc/arch/arm/mach-vexpress/spc.c:431:38: warning: array
subscript is below array bounds [-Warray-bounds]
/git/arm-soc/arch/x86/include/asm/string_32.h:70:16: error: array
subscript is above array bounds [-Werror=array-bounds]
/git/arm-soc/arch/x86/include/asm/uaccess_64.h:143:20: error: array
subscript is above array bounds [-Werror=array-bounds]
/git/arm-soc/drivers/cpufreq/arm_big_little.c:201:24: warning: array
subscript is above array bounds [-Warray-bounds]
/git/arm-soc/drivers/cpufreq/arm_big_little.c:325:16: warning: array
subscript is above array bounds [-Warray-bounds]
/git/arm-soc/drivers/cpufreq/arm_big_little.c:440:39: warning: array
subscript is above array bounds [-Warray-bounds]
/git/arm-soc/drivers/dma/sh/rcar-dmac.c:876:29: error: array subscript
is above array bounds [-Werror=array-bounds]
/git/arm-soc/drivers/isdn/hardware/eicon/message.c:11302:54: error:
array subscript is above array bounds [-Werror=array-bounds]
/git/arm-soc/drivers/net/ethernet/intel/igb/igb_ptp.c:367: error:
array subscript is below array bounds
/git/arm-soc/drivers/net/ethernet/intel/igb/igb_ptp.c:455: error:
array subscript is below array bounds
/git/arm-soc/drivers/scsi/qla2xxx/qla_gs.c:1398:7: error: array
subscript is above array bounds [-Werror=array-bounds]
/git/arm-soc/drivers/scsi/qla2xxx/qla_gs.c:2279:7: error: array
subscript is above array bounds [-Werror=array-bounds]
/git/arm-soc/drivers/scsi/sym53c416.c:565:58: error: array subscript
is above array bounds [-Werror=array-bounds]
/git/arm-soc/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c:492:14:
error: array subscript -1 is below array bounds of 'struct
ptlrpcd_ctl[]' [-Werror=array-bounds]
/git/arm-soc/fs/f2fs/segment.c:3044:5: error: array subscript is below
array bounds [-Werror=array-bounds]
/git/arm-soc/include/linux/compiler.h:253:20: error: array subscript
is above array bounds [-Werror=array-bounds]
/git/arm-soc/include/linux/dynamic_debug.h:86:20: warning: array
subscript is above array bounds [-Warray-bounds]
/git/arm-soc/include/sound/pcm.h:919:9: error: array subscript is
above array bounds [-Werror=array-bounds]
/git/arm-soc/kernel/bpf/verifier.c:4320:29: error: array subscript is
above array bounds [-Werror=array-bounds]
/git/arm-soc/kernel/rcu/tree.c:3332:13: warning: array subscript is
above array bounds [-Warray-bounds]
/git/arm-soc/net/ipv4/tcp_output.c:2129:40: error: array subscript is
below array bounds [-Werror=array-bounds]
/git/arm-soc/net/ipv4/tcp_output.c:2207:40: error: array subscript is
below array bounds [-Werror=array-bounds]
/git/arm-soc/net/rxrpc/ar-connection.c:589:16: warning: array
subscript is above array bounds [-Warray-bounds]
/git/arm-soc/sound/soc/sh/rcar/cmd.c:88:14: error: array subscript is
below array bounds [-Werror=array-bounds]
/git/arm-soc/sound/soc/sh/rcar/cmd.c:88: error: array subscript is
below array bounds

I've also opened two gcc bugs for warnings that appeared to be issued in error:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83312
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81601

I haven't built with gcc-8 in a while, should try that again now that
PR83312 has been
marked 'fixed'.

   Arnd

[PATCH] media: i2c: ov7740: use gpio/consumer.h instead of gpio.h

[Arnd Bergmann]

When built on a platform without gpiolib support, we run into
a couple of compile errors in ov7740, including:

drivers/media/i2c/ov7740.c: In function 'ov7740_set_power':
drivers/media/i2c/ov7740.c:307:4: error: implicit declaration of function 
'gpiod_direction_output'; did you mean 'gpio_direction_output'? 
[-Werror=implicit-function-declaration]
gpiod_direction_output(ov7740->pwdn_gpio, 0);
drivers/media/i2c/ov7740.c:914:4: error: 'GPIOD_OUT_HIGH' undeclared (first use 
in this function); did you mean 'GPIOF_INIT_HIGH'?

Changing it to use the correct header file solves the problem.

Fixes: 39c5c4471b8d ("media: i2c: Add the ov7740 image sensor driver")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/i2c/ov7740.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
index 0308ba437bbb..0db107196d7a 100644
--- a/drivers/media/i2c/ov7740.c
+++ b/drivers/media/i2c/ov7740.c
@@ -3,7 +3,7 @@
 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
-- 
2.9.0

[PATCH] [v4] media: s3c-camif: fix out-of-bounds array access

[Arnd Bergmann]

While experimenting with older compiler versions, I ran
into a warning that no longer shows up on gcc-4.8 or newer:

drivers/media/platform/s3c-camif/camif-capture.c: In function 
'__camif_subdev_try_format':
drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array 
subscript is below array bounds

This is an off-by-one bug, leading to an access before the start of the
array, while newer compilers silently assume this undefined behavior
cannot happen and leave the loop at index 0 if no other entry matches.

As Sylvester explains, we actually need to ensure that the
value is within the range, so this reworks the loop to be
easier to parse correctly, and an additional check to fall
back on the first format value for any unexpected input.

I found an existing gcc bug for it and added a reduced version
of the function there.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series 
camera interface")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
v4: simplify a bit
v3: fix newly introduced off-by-one bug.
v2: rework logic rather than removing it.
---
 drivers/media/platform/s3c-camif/camif-capture.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
b/drivers/media/platform/s3c-camif/camif-capture.c
index 437395a61065..9ab8e7ee2e1e 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -1256,16 +1256,17 @@ static void __camif_subdev_try_format(struct camif_dev 
*camif,
 {
const struct s3c_camif_variant *variant = camif->variant;
const struct vp_pix_limits *pix_lim;
-   int i = ARRAY_SIZE(camif_mbus_formats);
+   unsigned int i;
 
/* FIXME: constraints against codec or preview path ? */
pix_lim = >vp_pix_limits[VP_CODEC];
 
-   while (i-- >= 0)
+   for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
if (camif_mbus_formats[i] == mf->code)
break;
 
-   mf->code = camif_mbus_formats[i];
+   if (i == ARRAY_SIZE(camif_mbus_formats))
+   mf->code = camif_mbus_formats[0];
 
if (pad == CAMIF_SD_PAD_SINK) {
v4l_bound_align_image(>width, 8, CAMIF_MAX_PIX_WIDTH,
-- 
2.9.0

Re: [PATCH] [v3] media: s3c-camif: fix out-of-bounds array access

[Arnd Bergmann]

On Tue, Jan 16, 2018 at 9:17 PM, Laurent Pinchart
<laurent.pinch...@ideasonboard.com> wrote:
> Hi Arnd,
>
> Thank you for the patch.
>
> On Tuesday, 16 January 2018 18:47:24 EET Arnd Bergmann wrote:
>> While experimenting with older compiler versions, I ran
>> into a warning that no longer shows up on gcc-4.8 or newer:
>>
>> drivers/media/platform/s3c-camif/camif-capture.c: In function
>> '__camif_subdev_try_format':
>> drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array
>> subscript is below array bounds
>>
>> This is an off-by-one bug, leading to an access before the start of the
>> array, while newer compilers silently assume this undefined behavior
>> cannot happen and leave the loop at index 0 if no other entry matches.
>>
>> As Sylvester explains, we actually need to ensure that the
>> value is within the range, so this reworks the loop to be
>> easier to parse correctly, and an additional check to fall
>> back on the first format value for any unexpected input.
>>
>> I found an existing gcc bug for it and added a reduced version
>> of the function there.
>>
>> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
>> Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series
>> camera interface") Signed-off-by: Arnd Bergmann <a...@arndb.de>
>> ---
>> v3: fix newly introduced off-by-one bug.
>> v2: rework logic rather than removing it.
>> ---
>>  drivers/media/platform/s3c-camif/camif-capture.c | 9 ++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c
>> b/drivers/media/platform/s3c-camif/camif-capture.c index
>> 437395a61065..f51b92e94a32 100644
>> --- a/drivers/media/platform/s3c-camif/camif-capture.c
>> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
>> @@ -1256,16 +1256,19 @@ static void __camif_subdev_try_format(struct
>> camif_dev *camif, {
>>   const struct s3c_camif_variant *variant = camif->variant;
>>   const struct vp_pix_limits *pix_lim;
>> - int i = ARRAY_SIZE(camif_mbus_formats);
>> + int i;
>>
>>   /* FIXME: constraints against codec or preview path ? */
>>   pix_lim = >vp_pix_limits[VP_CODEC];
>>
>> - while (i-- >= 0)
>> + for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
>>   if (camif_mbus_formats[i] == mf->code)
>>   break;
>>
>> - mf->code = camif_mbus_formats[i];
>> + if (i == ARRAY_SIZE(camif_mbus_formats))
>> + mf->code = camif_mbus_formats[0];
>> + else
>> + mf->code = camif_mbus_formats[i];
>
> I might be missing something very obvious, but isn't mf->code already ==
> camif_mbus_formats[i] in the else branch ?

Ah, that must be what I was thinking back when I first
discussed it with Sylvester in
https://patchwork.kernel.org/patch/9950041/

Unfortunately, I hadn't given it as much thought today when
I tried to reconstruct the result to send a new version

> How about simply

> unsigned int i;
>
> /* FIXME: constraints against codec or preview path ? */
> pix_lim = >vp_pix_limits[VP_CODEC];
>
> for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
> if (camif_mbus_formats[i] == mf->code)
> break;
>
> if (i == ARRAY_SIZE(camif_mbus_formats))
> mf->code = camif_mbus_formats[0];

Yes, makes sense. I'll send a v4.

  Arnd

Re: [PATCH 5/8] [media] omap3isp: support 64-bit version of omap3isp_stat_data

[Arnd Bergmann]

On Tue, Dec 5, 2017 at 1:41 AM, Laurent Pinchart
 wrote:
> Hi Arnd,
>
> Thank you for the patch.
>
> I'll try to review this without too much delay. In the meantime, I'm CC'ing
> Sakari Ailus who might be faster than me :-)

Hi Laurent and Sakari,

I stumbled over this while cleaning out my backlog of patches. Any chance one
of you can have a look at this one? It's not needed for the 4.16 merge window,
but we do need it at some point and I would like to not see it again the next
time I clean out my old patches ;-)

  Arnd

Re: [PATCH] [v2] media: s3c-camif: fix out-of-bounds array access

[Arnd Bergmann]

On Tue, Jan 16, 2018 at 4:44 PM, Sakari Ailus
 wrote:

>>   if (camif_mbus_formats[i] == mf->code)
>>   break;
>>
>> + if (i == ARRAY_SIZE(camif_mbus_formats))
>> + mf->code = camif_mbus_formats[0];
>> +
>
> Either else here so that the line below is executed only if the condition
> is false, or assign i = 0 above. Otherwise you'll end up with a different
> off-by-one bug. :-)

Oops. Sent v3 now, thanks for the review.

  Arnd

[PATCH] [v3] media: s3c-camif: fix out-of-bounds array access

[Arnd Bergmann]

While experimenting with older compiler versions, I ran
into a warning that no longer shows up on gcc-4.8 or newer:

drivers/media/platform/s3c-camif/camif-capture.c: In function 
'__camif_subdev_try_format':
drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array 
subscript is below array bounds

This is an off-by-one bug, leading to an access before the start of the
array, while newer compilers silently assume this undefined behavior
cannot happen and leave the loop at index 0 if no other entry matches.

As Sylvester explains, we actually need to ensure that the
value is within the range, so this reworks the loop to be
easier to parse correctly, and an additional check to fall
back on the first format value for any unexpected input.

I found an existing gcc bug for it and added a reduced version
of the function there.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series 
camera interface")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
v3: fix newly introduced off-by-one bug.
v2: rework logic rather than removing it.
---
 drivers/media/platform/s3c-camif/camif-capture.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
b/drivers/media/platform/s3c-camif/camif-capture.c
index 437395a61065..f51b92e94a32 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -1256,16 +1256,19 @@ static void __camif_subdev_try_format(struct camif_dev 
*camif,
 {
const struct s3c_camif_variant *variant = camif->variant;
const struct vp_pix_limits *pix_lim;
-   int i = ARRAY_SIZE(camif_mbus_formats);
+   int i;
 
/* FIXME: constraints against codec or preview path ? */
pix_lim = >vp_pix_limits[VP_CODEC];
 
-   while (i-- >= 0)
+   for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
if (camif_mbus_formats[i] == mf->code)
break;
 
-   mf->code = camif_mbus_formats[i];
+   if (i == ARRAY_SIZE(camif_mbus_formats))
+   mf->code = camif_mbus_formats[0];
+   else
+   mf->code = camif_mbus_formats[i];
 
if (pad == CAMIF_SD_PAD_SINK) {
v4l_bound_align_image(>width, 8, CAMIF_MAX_PIX_WIDTH,
-- 
2.9.0

[PATCH] [v2] media: s3c-camif: fix out-of-bounds array access

[Arnd Bergmann]

While experimenting with older compiler versions, I ran
into a warning that no longer shows up on gcc-4.8 or newer:

drivers/media/platform/s3c-camif/camif-capture.c: In function 
'__camif_subdev_try_format':
drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array 
subscript is below array bounds

This is an off-by-one bug, leading to an access before the start of the
array, while newer compilers silently assume this undefined behavior
cannot happen and leave the loop at index 0 if no other entry matches.

As Sylvester explains, we actually need to ensure that the
value is within the range, so this reworks the loop to be
easier to parse correctly, and an additional check to fall
back on the first format value for any unexpected input.

I found an existing gcc bug for it and added a reduced version
of the function there.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series 
camera interface")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
v2: rework logic rather than removing it.
---
 drivers/media/platform/s3c-camif/camif-capture.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
b/drivers/media/platform/s3c-camif/camif-capture.c
index 437395a61065..002609be1400 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -1256,15 +1256,18 @@ static void __camif_subdev_try_format(struct camif_dev 
*camif,
 {
const struct s3c_camif_variant *variant = camif->variant;
const struct vp_pix_limits *pix_lim;
-   int i = ARRAY_SIZE(camif_mbus_formats);
+   int i;
 
/* FIXME: constraints against codec or preview path ? */
pix_lim = >vp_pix_limits[VP_CODEC];
 
-   while (i-- >= 0)
+   for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
if (camif_mbus_formats[i] == mf->code)
break;
 
+   if (i == ARRAY_SIZE(camif_mbus_formats))
+   mf->code = camif_mbus_formats[0];
+
mf->code = camif_mbus_formats[i];
 
if (pad == CAMIF_SD_PAD_SINK) {
-- 
2.9.0

Re: [PATCH] media: uvcvideo: Fixed ktime_t to ns conversion

[Arnd Bergmann]

On Sun, Jan 14, 2018 at 11:21 AM, Jasmin J. <jas...@anw.at> wrote:
> From: Jasmin Jessich <jas...@anw.at>
>
> Commit 828ee8c71950 ("media: uvcvideo: Use ktime_t for timestamps")
> changed to use ktime_t for timestamps. Older Kernels use a struct for
> ktime_t, which requires the conversion function ktime_to_ns to be used on
> some places. With this patch it will compile now also for older Kernel
> versions.
>
> Signed-off-by: Jasmin Jessich <jas...@anw.at>

Looks good to me,

Acked-by: Arnd Bergmann <a...@arndb.de>

Re: [PATCH v2 1/1] media: entity: Add a nop variant of media_entity_cleanup

[Arnd Bergmann]

On Thu, Jan 11, 2018 at 11:19 AM, Sakari Ailus
<sakari.ai...@linux.intel.com> wrote:
> Add nop variant of media_entity_cleanup. This allows calling
> media_entity_cleanup whether or not Media controller is enabled,
> simplifying driver code.
>
> Also drop #ifdefs on a few drivers around media_entity_cleanup().
>
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>

Thanks for addressing this,

Reviewed-by: Arnd Bergmann <a...@arndb.de>

Re: [PATCH 1/1] media: entity: Add a nop variant of media_entity_cleanupr

[Arnd Bergmann]

On Tue, Jan 9, 2018 at 11:31 PM, Sakari Ailus
 wrote:

>> depends on VIDEO_V4L2_SUBDEV_API
>> ---help---
>>   This is a driver for the DW9714 camera lens voice coil.
>> @@ -636,7 +636,6 @@ config VIDEO_OV5670
>> tristate "OmniVision OV5670 sensor support"
>> depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
>> depends on MEDIA_CAMERA_SUPPORT
>> -   depends on MEDIA_CONTROLLER
>
> ov5670 does depend on MC at least right now. I guess it might not take much
> to make it optional. But it's more than this patch. :-)
>
>> select V4L2_FWNODE
>> ---help---
>>   This is a Video4Linux2 sensor-level driver for the OmniVision
>> @@ -667,7 +666,7 @@ config VIDEO_OV7670
>>
>>  config VIDEO_OV7740
>> tristate "OmniVision OV7740 sensor support"
>> -   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
>> +   depends on I2C && VIDEO_V4L2
>
> Hmm. In here the ov7740 driver doesn't seem to depend on MC.

Right, this was on top of the earlier patch I sent that you rejected ;-)

>> depends on MEDIA_CAMERA_SUPPORT
>> ---help---
>>   This is a Video4Linux2 sensor-level driver for the OmniVision
>> @@ -815,7 +814,7 @@ comment "Flash devices"
>>
>>  config VIDEO_ADP1653
>> tristate "ADP1653 flash support"
>> -   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
>> +   depends on I2C && VIDEO_V4L2
>> depends on MEDIA_CAMERA_SUPPORT
>> ---help---
>>   This is a driver for the ADP1653 flash controller. It is used for
>> @@ -823,7 +822,7 @@ config VIDEO_ADP1653
>>
>>  config VIDEO_LM3560
>> tristate "LM3560 dual flash driver support"
>> -   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
>> +   depends on I2C && VIDEO_V4L2
>> depends on MEDIA_CAMERA_SUPPORT
>> select REGMAP_I2C
>> ---help---

Those two also failed to build

>> @@ -832,7 +831,7 @@ config VIDEO_LM3560
>>
>>  config VIDEO_LM3646
>> tristate "LM3646 dual flash driver support"
>> -   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
>> +   depends on I2C && VIDEO_V4L2
>> depends on MEDIA_CAMERA_SUPPORT
>> select REGMAP_I2C
>> ---help---
>
> These also call media_entity_pads_init() unconditionally.
>
> How was this tested? :-)

Not before I sent it, what I meant is that I'd give it a try, blindly applying
the patch to my randconfig build tree to see what breaks.

The result after a day worth of randconfig builds is this one, which
basically matches what you concluded already:

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 03cf3a1a1e06..5d465221fbfa 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -310,14 +310,14 @@ config VIDEO_ML86V7667

 config VIDEO_AD5820
tristate "AD5820 lens voice coil support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
---help---
  This is a driver for the AD5820 camera lens voice coil.
  It is used for example in Nokia N900 (RX-51).

 config VIDEO_DW9714
tristate "DW9714 lens voice coil support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
depends on VIDEO_V4L2_SUBDEV_API
---help---
  This is a driver for the DW9714 camera lens voice coil.
@@ -636,7 +636,6 @@ config VIDEO_OV5670
tristate "OmniVision OV5670 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
depends on MEDIA_CAMERA_SUPPORT
-   depends on MEDIA_CONTROLLER
select V4L2_FWNODE
---help---
  This is a Video4Linux2 sensor-level driver for the OmniVision
@@ -667,7 +666,7 @@ config VIDEO_OV7670

 config VIDEO_OV7740
tristate "OmniVision OV7740 sensor support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
depends on MEDIA_CAMERA_SUPPORT
---help---
  This is a Video4Linux2 sensor-level driver for the OmniVision
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index d7a669058b5e..3f34a1126bd1 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -636,6 +636,11 @@ int media_entity_pads_init(struct media_entity
*entity, u16 num_pads,
  */
 static inline void media_entity_cleanup(struct media_entity *entity) {};

+#ifndef CONFIG_MEDIA_CONTROLLER
+#define media_entity_pads_init(e, n, p) 0
+#define media_entity_cleanup(e) do { } while (0)
+#endif
+
 /**
  * media_create_pad_link() - creates a link between two entities.
  *

I'll just drop that patch then from my build tree.

 Arnd

Re: [PATCH 1/1] media: entity: Add a nop variant of media_entity_cleanupr

[Arnd Bergmann]

On Tue, Jan 9, 2018 at 2:58 PM, Sakari Ailus
 wrote:
> Add nop variant of media_entity_cleanup. This allows calling
> media_entity_cleanup whether or not Media controller is enabled,
> simplifying driver code.
>
> Also drop #ifdefs on a few drivers around media_entity_cleanup() and drop
> the extra semicolon from media_entity_cleanup prototype.
>
> Signed-off-by: Sakari Ailus 
> ---
> Hi Arnd,
>
> I thought about doing something similar with media_entity_pads_init which is
> equally commonly used in drivers that support MC/non-MC cases. The trouble
> with that is that the drivers set up the struct first before calling
> media_entity_pads_init, requiring the #ifdefs in any case. So the benefit
> would be questionable at least. So just media_entity_cleanup this time.

Looks good overall, just two thoughts:

> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index d7a669058b5e..a732af1dbba0 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -634,7 +634,11 @@ int media_entity_pads_init(struct media_entity *entity, 
> u16 num_pads,
>   * This function must be called during the cleanup phase after unregistering
>   * the entity (currently, it does nothing).
>   */
> -static inline void media_entity_cleanup(struct media_entity *entity) {};
> +#if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)
> +static inline void media_entity_cleanup(struct media_entity *entity) {}
> +#else
> +#define media_entity_cleanup(entity) do { } while (false)
> +#endif

This might cause a harmless warning about an unused variable in case
we have a driver that does:

 void f(struct i2c_client *client)
{
struct v4l2_subdev *sd = to_subdev(client);
media_entity_cleanup(sd);
}

None of the drivers you changed would have an unused variable after
your change (otherwise they would already have it before your change),
so it's probably fine.

and second, I'm trying the patch below on top of yours now, will
see how far that gets us ;-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 03cf3a1a1e06..6421da7cb58a 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -310,14 +310,14 @@ config VIDEO_ML86V7667

 config VIDEO_AD5820
tristate "AD5820 lens voice coil support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
---help---
  This is a driver for the AD5820 camera lens voice coil.
  It is used for example in Nokia N900 (RX-51).

 config VIDEO_DW9714
tristate "DW9714 lens voice coil support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
depends on VIDEO_V4L2_SUBDEV_API
---help---
  This is a driver for the DW9714 camera lens voice coil.
@@ -636,7 +636,6 @@ config VIDEO_OV5670
tristate "OmniVision OV5670 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
depends on MEDIA_CAMERA_SUPPORT
-   depends on MEDIA_CONTROLLER
select V4L2_FWNODE
---help---
  This is a Video4Linux2 sensor-level driver for the OmniVision
@@ -667,7 +666,7 @@ config VIDEO_OV7670

 config VIDEO_OV7740
tristate "OmniVision OV7740 sensor support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
depends on MEDIA_CAMERA_SUPPORT
---help---
  This is a Video4Linux2 sensor-level driver for the OmniVision
@@ -815,7 +814,7 @@ comment "Flash devices"

 config VIDEO_ADP1653
tristate "ADP1653 flash support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
depends on MEDIA_CAMERA_SUPPORT
---help---
  This is a driver for the ADP1653 flash controller. It is used for
@@ -823,7 +822,7 @@ config VIDEO_ADP1653

 config VIDEO_LM3560
tristate "LM3560 dual flash driver support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
depends on MEDIA_CAMERA_SUPPORT
select REGMAP_I2C
---help---
@@ -832,7 +831,7 @@ config VIDEO_LM3560

 config VIDEO_LM3646
tristate "LM3646 dual flash driver support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
depends on MEDIA_CAMERA_SUPPORT
select REGMAP_I2C
---help---


   Arnd

Re: [PATCH] media: i2c: ov7740: add media-controller dependency

[Arnd Bergmann]

On Tue, Jan 9, 2018 at 11:13 AM, Sakari Ailus
<sakari.ai...@linux.intel.com> wrote:
> Hi Arnd,
>
> On Mon, Jan 08, 2018 at 01:52:28PM +0100, Arnd Bergmann wrote:
>> Without CONFIG_MEDIA_CONTROLLER, the new driver fails to build:
>>
>> drivers/perf/arm_dsu_pmu.c: In function 'dsu_pmu_probe_pmu':
>> drivers/perf/arm_dsu_pmu.c:661:2: error: implicit declaration of function 
>> 'bitmap_from_u32array'; did you mean 'bitmap_from_arr32'? 
>> [-Werror=implicit-function-declaration]
>
> There seems to be a build error with this driver if Media controller is
> disabled, but this is not the error message

Oops, bad copy-paste, sorry.

> nor adding a dependency to
> Media controller is a sound fix for it.

> drivers/media/i2c/ov7740.c: In function ‘ov7740_probe’:
> drivers/media/i2c/ov7740.c:1139:38: error: ‘struct v4l2_subdev’ has no member 
> named ‘entity’
>   media_entity_cleanup(>subdev.entity);
>
> I think it'd be worth adding nop variants for functions that are commonly
> used by drivers that can be built with or without the Media controller.
>
> I'll send a patch.

Fair enough.

In this case, the problem is not the lack of the wrapper, as we already
have a

static inline void media_entity_cleanup(struct media_entity *entity) {};

helper, but the fact that struct v4l2_subdev contains a struct media_entity
as an optional member that is disabled here.  Since the argument
to media_entity_cleanup is generally this member, we could have
a wrapper around it that takes a v4l2_subdev pointer, like

static inline void media_subdev_entity_cleanup(struct v4l2_subdev* sd)
{
#ifdef CONFIG_MEDIA_CONTROLLER
  media_entity_cleanup(sd->entity);
#endif
}

which seems nicer than making the relatively large media_entity structure
visible unconditionally, or hiding its members instead.

You may also want to revisit the other drivers that have dependencies
on MEDIA_CONTROLLER then:

drivers/media/Kconfig:  depends on MEDIA_CONTROLLER && DVB_CORE
drivers/media/Kconfig:  depends on VIDEO_DEV && MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:  depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:  depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:  depends on MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:  depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:  depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:  depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
drivers/media/i2c/Kconfig:  depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
drivers/media/pci/intel/ipu3/Kconfig:   depends on MEDIA_CONTROLLER
drivers/media/platform/Kconfig: depends on VIDEO_V4L2 && OF &&
VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER
drivers/media/platform/rcar-vin/Kconfig:depends on VIDEO_V4L2
&& VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA && MEDIA_CONTROLLER
drivers/staging/media/atomisp/Kconfig:  depends on X86 && EFI &&
MEDIA_CONTROLLER && PCI && ACPI
drivers/staging/media/imx/Kconfig:  depends on MEDIA_CONTROLLER &&
VIDEO_V4L2 && ARCH_MXC && IMX_IPUV3_CORE

When you come up with a patch, I can throw it in my randconfig build
test machine
to see if that causes any unexpected build failures.

Thanks,

   Arnd

[PATCH] media: intel-ipu3: cio2: mark more PM functions as __maybe_unused

[Arnd Bergmann]

My earlier patch missed two functions, these must be __maybe_unused
as well:

drivers/media/pci/intel/ipu3/ipu3-cio2.c:1867:12: error: 'cio2_runtime_resume' 
defined but not used [-Werror=unused-function]
drivers/media/pci/intel/ipu3/ipu3-cio2.c:1849:12: error: 'cio2_runtime_suspend' 
defined but not used [-Werror=unused-function]

Fixes: 2086dd35705f ("media: intel-ipu3: cio2: mark PM functions as 
__maybe_unused")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
If my previous patch is not in a stable branch yet, folding the new
fixup into that would be ideal.
---
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c 
b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 9db752a7f363..724cd8d9d573 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -1850,7 +1850,7 @@ static void cio2_pci_remove(struct pci_dev *pci_dev)
mutex_destroy(>lock);
 }
 
-static int cio2_runtime_suspend(struct device *dev)
+static int __maybe_unused cio2_runtime_suspend(struct device *dev)
 {
struct pci_dev *pci_dev = to_pci_dev(dev);
struct cio2_device *cio2 = pci_get_drvdata(pci_dev);
@@ -1868,7 +1868,7 @@ static int cio2_runtime_suspend(struct device *dev)
return 0;
 }
 
-static int cio2_runtime_resume(struct device *dev)
+static int __maybe_unused cio2_runtime_resume(struct device *dev)
 {
struct pci_dev *pci_dev = to_pci_dev(dev);
struct cio2_device *cio2 = pci_get_drvdata(pci_dev);
-- 
2.9.0

[PATCH] media: i2c: ov7740: add media-controller dependency

[Arnd Bergmann]

Without CONFIG_MEDIA_CONTROLLER, the new driver fails to build:

drivers/perf/arm_dsu_pmu.c: In function 'dsu_pmu_probe_pmu':
drivers/perf/arm_dsu_pmu.c:661:2: error: implicit declaration of function 
'bitmap_from_u32array'; did you mean 'bitmap_from_arr32'? 
[-Werror=implicit-function-declaration]

This adds a dependency similar to what we have for other drivers
like this.

Fixes: 39c5c4471b8d ("media: i2c: Add the ov7740 image sensor driver")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/i2c/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd296841..03cf3a1a1e06 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -667,7 +667,7 @@ config VIDEO_OV7670
 
 config VIDEO_OV7740
tristate "OmniVision OV7740 sensor support"
-   depends on I2C && VIDEO_V4L2
+   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
depends on MEDIA_CAMERA_SUPPORT
---help---
  This is a Video4Linux2 sensor-level driver for the OmniVision
-- 
2.9.0

[PATCH] media: cobalt: select CONFIG_SND_PCM

[Arnd Bergmann]

The cobalt sound driver has a dependency on ALSA, but not
on the PCM helper code, so this can lead to an extremely
rare link error in randconfig builds:

ERROR: "snd_pcm_period_elapsed" [drivers/media/pci/cobalt/cobalt.ko] undefined!
ERROR: "_snd_pcm_stream_lock_irqsave" [drivers/media/pci/cobalt/cobalt.ko] 
undefined!
ERROR: "snd_pcm_hw_constraint_integer" [drivers/media/pci/cobalt/cobalt.ko] 
undefined!
ERROR: "snd_pcm_set_ops" [drivers/media/pci/cobalt/cobalt.ko] undefined!
ERROR: "snd_pcm_stream_unlock_irqrestore" [drivers/media/pci/cobalt/cobalt.ko] 
undefined!
ERROR: "snd_pcm_lib_ioctl" [drivers/media/pci/cobalt/cobalt.ko] undefined!
ERROR: "snd_pcm_new" [drivers/media/pci/cobalt/cobalt.ko] undefined!

The other audio drivers select 'SND_PCM' for this, so let's
do the same.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/pci/cobalt/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/pci/cobalt/Kconfig b/drivers/media/pci/cobalt/Kconfig
index 70343829a125..aa35cbc0a904 100644
--- a/drivers/media/pci/cobalt/Kconfig
+++ b/drivers/media/pci/cobalt/Kconfig
@@ -6,6 +6,7 @@ config VIDEO_COBALT
depends on SND
depends on MTD
select I2C_ALGOBIT
+   select SND_PCM
select VIDEO_ADV7604
select VIDEO_ADV7511
select VIDEO_ADV7842
-- 
2.9.0

[PATCH] media: staging: tegra-vde: select DMA_SHARED_BUFFER

[Arnd Bergmann]

Without CONFIG_DMA_SHARED_BUFFER we run into a link error for the
dma_buf_* APIs:

ERROR: "dma_buf_map_attachment" [drivers/staging/media/tegra-vde/tegra-vde.ko] 
undefined!
ERROR: "dma_buf_attach" [drivers/staging/media/tegra-vde/tegra-vde.ko] 
undefined!
ERROR: "dma_buf_get" [drivers/staging/media/tegra-vde/tegra-vde.ko] undefined!
ERROR: "dma_buf_put" [drivers/staging/media/tegra-vde/tegra-vde.ko] undefined!
ERROR: "dma_buf_detach" [drivers/staging/media/tegra-vde/tegra-vde.ko] 
undefined!
ERROR: "dma_buf_unmap_attachment" 
[drivers/staging/media/tegra-vde/tegra-vde.ko] undefined!

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/staging/media/tegra-vde/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/tegra-vde/Kconfig 
b/drivers/staging/media/tegra-vde/Kconfig
index ec3ebdaa..5c4914674468 100644
--- a/drivers/staging/media/tegra-vde/Kconfig
+++ b/drivers/staging/media/tegra-vde/Kconfig
@@ -1,6 +1,7 @@
 config TEGRA_VDE
tristate "NVIDIA Tegra Video Decoder Engine driver"
depends on ARCH_TEGRA || COMPILE_TEST
+   select DMA_SHARED_BUFFER
select SRAM
help
Say Y here to enable support for the NVIDIA Tegra video decoder
-- 
2.9.0

[PATCH] [v2] media: au0828: fix VIDEO_V4L2 dependency

[Arnd Bergmann]

After the move of videobuf2 into the common directory, selecting the
au0828 driver with CONFIG_V4L2 disabled started causing a link failure,
as we now attempt to build videobuf2 but it still requires v4l2:

ERROR: "v4l2_event_pending" [drivers/media/common/videobuf/videobuf2-v4l2.ko] 
undefined!
ERROR: "v4l2_fh_release" [drivers/media/common/videobuf/videobuf2-v4l2.ko] 
undefined!
ERROR: "video_devdata" [drivers/media/common/videobuf/videobuf2-v4l2.ko] 
undefined!
ERROR: "__tracepoint_vb2_buf_done" 
[drivers/media/common/videobuf/videobuf2-core.ko] undefined!
ERROR: "__tracepoint_vb2_dqbuf" 
[drivers/media/common/videobuf/videobuf2-core.ko] undefined!
ERROR: "v4l_vb2q_enable_media_source" 
[drivers/media/common/videobuf/videobuf2-core.ko] undefined!

We want to be able to build the core au0828 support without V4L2,
so this makes the 'select' conditional on V4L2, and refines the
dependencies in VIDEO_AU0828_V4L2 so it can only be enabled in
the exact conditions that have VIDEOBUF2_VMALLOC reachable.

Fixes: 03fbdb2fc2b8 ("media: move videobuf2 to drivers/media/common")
Fixes: 05439b1a3693 ("[media] media: au0828 - convert to use videobuf2")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/usb/au0828/Kconfig | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/au0828/Kconfig b/drivers/media/usb/au0828/Kconfig
index 70521e0b4c53..18630b033d5b 100644
--- a/drivers/media/usb/au0828/Kconfig
+++ b/drivers/media/usb/au0828/Kconfig
@@ -4,7 +4,7 @@ config VIDEO_AU0828
depends on I2C && INPUT && DVB_CORE && USB
select I2C_ALGOBIT
select VIDEO_TVEEPROM
-   select VIDEOBUF2_VMALLOC
+   select VIDEOBUF2_VMALLOC if VIDEO_V4L2
select DVB_AU8522_DTV if MEDIA_SUBDRV_AUTOSELECT
select MEDIA_TUNER_XC5000 if MEDIA_SUBDRV_AUTOSELECT
select MEDIA_TUNER_MXL5007T if MEDIA_SUBDRV_AUTOSELECT
@@ -18,7 +18,8 @@ config VIDEO_AU0828
 
 config VIDEO_AU0828_V4L2
bool "Auvitek AU0828 v4l2 analog video support"
-   depends on VIDEO_AU0828 && VIDEO_V4L2
+   depends on VIDEO_AU0828
+   depends on VIDEO_V4L2=y || VIDEO_V4L2=VIDEO_AU0828
select DVB_AU8522_V4L if MEDIA_SUBDRV_AUTOSELECT
select VIDEO_TUNER
default y
-- 
2.9.0

Re: [PATCH 3/3] media: au0828: add VIDEO_V4L2 dependency

[Arnd Bergmann]

On Thu, Jan 4, 2018 at 11:31 AM, Arnd Bergmann <a...@arndb.de> wrote:
> After the move of videobuf2 into the common directory, selecting the
> au0828 driver with CONFIG_V4L2 disabled started causing a link failure,
> as we now attempt to build videobuf2 but it still requires v4l2:
>
> ERROR: "v4l2_event_pending" [drivers/media/common/videobuf/videobuf2-v4l2.ko] 
> undefined!
> ERROR: "v4l2_fh_release" [drivers/media/common/videobuf/videobuf2-v4l2.ko] 
> undefined!
> ERROR: "video_devdata" [drivers/media/common/videobuf/videobuf2-v4l2.ko] 
> undefined!
> ERROR: "__tracepoint_vb2_buf_done" 
> [drivers/media/common/videobuf/videobuf2-core.ko] undefined!
> ERROR: "__tracepoint_vb2_dqbuf" 
> [drivers/media/common/videobuf/videobuf2-core.ko] undefined!
> ERROR: "v4l_vb2q_enable_media_source" 
> [drivers/media/common/videobuf/videobuf2-core.ko] undefined!
>
> This adds the same dependency in au0828 that the other users of videobuf2
> have.
>
> Fixes: 03fbdb2fc2b8 ("media: move videobuf2 to drivers/media/common")
> Fixes: 05439b1a3693 ("[media] media: au0828 - convert to use videobuf2")
> Signed-off-by: Arnd Bergmann <a...@arndb.de>

On a further look, this prevents us from building the RC driver without the V4L2
driver, which was the reason this driver is split. I'll send a v2, but
it needs more
randconfig testing than this version.

Arnd

[PATCH 2/3] media: dvb: fix DVB_MMAP dependency

[Arnd Bergmann]

Enabling CONFIG_DVB_MMAP without CONFIG_VIDEOBUF2_VMALLOC results
in a link error:

drivers/media/dvb-core/dvb_vb2.o: In function `_stop_streaming':
dvb_vb2.c:(.text+0x894): undefined reference to `vb2_buffer_done'
drivers/media/dvb-core/dvb_vb2.o: In function `dvb_vb2_init':
dvb_vb2.c:(.text+0xbec): undefined reference to `vb2_vmalloc_memops'
dvb_vb2.c:(.text+0xc4c): undefined reference to `vb2_core_queue_init'
drivers/media/dvb-core/dvb_vb2.o: In function `dvb_vb2_release':
dvb_vb2.c:(.text+0xe14): undefined reference to `vb2_core_queue_release'
drivers/media/dvb-core/dvb_vb2.o: In function `dvb_vb2_stream_on':
dvb_vb2.c:(.text+0xeb8): undefined reference to `vb2_core_streamon'
drivers/media/dvb-core/dvb_vb2.o: In function `dvb_vb2_stream_off':
dvb_vb2.c:(.text+0xfe8): undefined reference to `vb2_core_streamoff'
drivers/media/dvb-core/dvb_vb2.o: In function `dvb_vb2_fill_buffer':
dvb_vb2.c:(.text+0x13ec): undefined reference to `vb2_plane_vaddr'
dvb_vb2.c:(.text+0x149c): undefined reference to `vb2_buffer_done'

This adds a 'select' statement for it, plus a dependency that
ensures that videobuf2 in turn works, as it in turn depends on
VIDEO_V4L2 to link, and that must not be a module if videobuf2
is built-in.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/Kconfig b/drivers/media/Kconfig
index 3f69b948d102..d1be86ebfd9a 100644
--- a/drivers/media/Kconfig
+++ b/drivers/media/Kconfig
@@ -147,6 +147,8 @@ config DVB_CORE
 config DVB_MMAP
bool "Enable DVB memory-mapped API (EXPERIMENTAL)"
depends on DVB_CORE
+   depends on VIDEO_V4L2=y || VIDEO_V4L2=DVB_CORE
+   select VIDEOBUF2_VMALLOC
default n
help
  This option enables DVB experimental memory-mapped API, with
-- 
2.9.0

[PATCH 3/3] media: au0828: add VIDEO_V4L2 dependency

[Arnd Bergmann]

After the move of videobuf2 into the common directory, selecting the
au0828 driver with CONFIG_V4L2 disabled started causing a link failure,
as we now attempt to build videobuf2 but it still requires v4l2:

ERROR: "v4l2_event_pending" [drivers/media/common/videobuf/videobuf2-v4l2.ko] 
undefined!
ERROR: "v4l2_fh_release" [drivers/media/common/videobuf/videobuf2-v4l2.ko] 
undefined!
ERROR: "video_devdata" [drivers/media/common/videobuf/videobuf2-v4l2.ko] 
undefined!
ERROR: "__tracepoint_vb2_buf_done" 
[drivers/media/common/videobuf/videobuf2-core.ko] undefined!
ERROR: "__tracepoint_vb2_dqbuf" 
[drivers/media/common/videobuf/videobuf2-core.ko] undefined!
ERROR: "v4l_vb2q_enable_media_source" 
[drivers/media/common/videobuf/videobuf2-core.ko] undefined!

This adds the same dependency in au0828 that the other users of videobuf2
have.

Fixes: 03fbdb2fc2b8 ("media: move videobuf2 to drivers/media/common")
Fixes: 05439b1a3693 ("[media] media: au0828 - convert to use videobuf2")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/usb/au0828/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/au0828/Kconfig b/drivers/media/usb/au0828/Kconfig
index 70521e0b4c53..bfaa806633df 100644
--- a/drivers/media/usb/au0828/Kconfig
+++ b/drivers/media/usb/au0828/Kconfig
@@ -1,7 +1,7 @@
 
 config VIDEO_AU0828
tristate "Auvitek AU0828 support"
-   depends on I2C && INPUT && DVB_CORE && USB
+   depends on I2C && INPUT && DVB_CORE && USB && VIDEO_V4L2
select I2C_ALGOBIT
select VIDEO_TVEEPROM
select VIDEOBUF2_VMALLOC
-- 
2.9.0

[PATCH 1/3] media: dvb: fix DVB_MMAP symbol name

[Arnd Bergmann]

CONFIG_DVB_MMAP was misspelled either as CONFIG_DVB_MMSP
or DVB_MMAP, so it had no effect at all. This fixes that,
to make it possible to build it again.

Fixes: 4021053ed52d ("media: dvb-core: make DVB mmap API optional")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/dvb-core/Makefile |  2 +-
 drivers/media/dvb-core/dmxdev.c | 30 +++---
 include/media/dvb_vb2.h |  2 +-
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/media/dvb-core/Makefile b/drivers/media/dvb-core/Makefile
index 3756ccf83384..045e2392c668 100644
--- a/drivers/media/dvb-core/Makefile
+++ b/drivers/media/dvb-core/Makefile
@@ -4,7 +4,7 @@
 #
 
 dvb-net-$(CONFIG_DVB_NET) := dvb_net.o
-dvb-vb2-$(CONFIG_DVB_MMSP) := dvb_vb2.o
+dvb-vb2-$(CONFIG_DVB_MMAP) := dvb_vb2.o
 
 dvb-core-objs := dvbdev.o dmxdev.o dvb_demux.o \
 dvb_ca_en50221.o dvb_frontend.o\
diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index bc198f84b9cd..41e0259fc1c7 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -128,7 +128,7 @@ static int dvb_dvr_open(struct inode *inode, struct file 
*file)
struct dvb_device *dvbdev = file->private_data;
struct dmxdev *dmxdev = dvbdev->priv;
struct dmx_frontend *front;
-#ifndef DVB_MMAP
+#ifndef CONFIG_DVB_MMAP
bool need_ringbuffer = false;
 #else
const bool need_ringbuffer = true;
@@ -144,7 +144,7 @@ static int dvb_dvr_open(struct inode *inode, struct file 
*file)
return -ENODEV;
}
 
-#ifndef DVB_MMAP
+#ifndef CONFIG_DVB_MMAP
if ((file->f_flags & O_ACCMODE) == O_RDONLY)
need_ringbuffer = true;
 #else
@@ -200,7 +200,7 @@ static int dvb_dvr_release(struct inode *inode, struct file 
*file)
 {
struct dvb_device *dvbdev = file->private_data;
struct dmxdev *dmxdev = dvbdev->priv;
-#ifndef DVB_MMAP
+#ifndef CONFIG_DVB_MMAP
bool need_ringbuffer = false;
 #else
const bool need_ringbuffer = true;
@@ -213,7 +213,7 @@ static int dvb_dvr_release(struct inode *inode, struct file 
*file)
dmxdev->demux->connect_frontend(dmxdev->demux,
dmxdev->dvr_orig_fe);
}
-#ifndef DVB_MMAP
+#ifndef CONFIG_DVB_MMAP
if ((file->f_flags & O_ACCMODE) == O_RDONLY)
need_ringbuffer = true;
 #endif
@@ -426,7 +426,7 @@ static int dvb_dmxdev_ts_callback(const u8 *buffer1, size_t 
buffer1_len,
 {
struct dmxdev_filter *dmxdevfilter = feed->priv;
struct dvb_ringbuffer *buffer;
-#ifdef DVB_MMAP
+#ifdef CONFIG_DVB_MMAP
struct dvb_vb2_ctx *ctx;
 #endif
int ret;
@@ -440,12 +440,12 @@ static int dvb_dmxdev_ts_callback(const u8 *buffer1, 
size_t buffer1_len,
if (dmxdevfilter->params.pes.output == DMX_OUT_TAP ||
dmxdevfilter->params.pes.output == DMX_OUT_TSDEMUX_TAP) {
buffer = >buffer;
-#ifdef DVB_MMAP
+#ifdef CONFIG_DVB_MMAP
ctx = >vb2_ctx;
 #endif
} else {
buffer = >dev->dvr_buffer;
-#ifdef DVB_MMAP
+#ifdef CONFIG_DVB_MMAP
ctx = >dev->dvr_vb2_ctx;
 #endif
}
@@ -,7 +,7 @@ static int dvb_demux_do_ioctl(struct file *file,
mutex_unlock(>mutex);
break;
 
-#ifdef DVB_MMAP
+#ifdef CONFIG_DVB_MMAP
case DMX_REQBUFS:
if (mutex_lock_interruptible(>mutex)) {
mutex_unlock(>mutex);
@@ -1199,7 +1199,7 @@ static __poll_t dvb_demux_poll(struct file *file, 
poll_table *wait)
return mask;
 }
 
-#ifdef DVB_MMAP
+#ifdef CONFIG_DVB_MMAP
 static int dvb_demux_mmap(struct file *file, struct vm_area_struct *vma)
 {
struct dmxdev_filter *dmxdevfilter = file->private_data;
@@ -1249,7 +1249,7 @@ static const struct file_operations dvb_demux_fops = {
.release = dvb_demux_release,
.poll = dvb_demux_poll,
.llseek = default_llseek,
-#ifdef DVB_MMAP
+#ifdef CONFIG_DVB_MMAP
.mmap = dvb_demux_mmap,
 #endif
 };
@@ -1280,7 +1280,7 @@ static int dvb_dvr_do_ioctl(struct file *file,
ret = dvb_dvr_set_buffer_size(dmxdev, arg);
break;
 
-#ifdef DVB_MMAP
+#ifdef CONFIG_DVB_MMAP
case DMX_REQBUFS:
ret = dvb_vb2_reqbufs(>dvr_vb2_ctx, parg);
break;
@@ -1322,7 +1322,7 @@ static __poll_t dvb_dvr_poll(struct file *file, 
poll_table *wait)
struct dvb_device *dvbdev = file->private_data;
struct dmxdev *dmxdev = dvbdev->priv;
__poll_t mask = 0;
-#ifndef DVB_MMAP
+#ifndef CONFIG_DVB_MMAP
bool need_ringbuffer = false;
 #else
const bool need_ringbuffer = true;
@@ -1337,7 +1337,7 @@ static __poll_t dvb_dvr_poll(struct file *file, 
poll_table *wait)
 
poll_wait(f

Re: [PATCH] media: don't drop front-end reference count for ->detach

[Arnd Bergmann]

On Wed, Jan 3, 2018 at 11:23 AM, Mauro Carvalho Chehab
<mche...@kernel.org> wrote:
> Em Tue,  2 Jan 2018 10:48:54 +0100
> Arnd Bergmann <a...@arndb.de> escreveu:

>> @@ -2965,7 +2968,6 @@ void dvb_frontend_detach(struct dvb_frontend* fe)
>>   dvb_frontend_invoke_release(fe, fe->ops.release_sec);
>>   dvb_frontend_invoke_release(fe, fe->ops.tuner_ops.release);
>>   dvb_frontend_invoke_release(fe, fe->ops.analog_ops.release);
>> - dvb_frontend_invoke_release(fe, fe->ops.detach);
>>   dvb_frontend_put(fe);
>
> Hmm... stb0899 is not the only driver using detach:
>
> drivers/media/dvb-frontends/stb0899_drv.c:  .detach   
>   = stb0899_detach,
> drivers/media/pci/saa7146/hexium_gemini.c:  .detach = hexium_detach,
> drivers/media/pci/saa7146/hexium_orion.c:   .detach = hexium_detach,
> drivers/media/pci/saa7146/mxb.c:.detach = mxb_detach,
> drivers/media/pci/ttpci/av7110.c:   .detach = av7110_detach,
> drivers/media/pci/ttpci/budget-av.c:.detach = budget_av_detach,
> drivers/media/pci/ttpci/budget-ci.c:.detach = budget_ci_detach,
> drivers/media/pci/ttpci/budget-patch.c: .detach = budget_patch_detach,
> drivers/media/pci/ttpci/budget.c:   .detach = budget_detach,

I'm pretty sure I checked this before and found stb0899_detach to be the
only implementation of dvb_frontend_ops:detach.

The other eight you quoted are all setting the member in struct
saa7146_extension, not struct dvb_frontend_ops, so they are
not called using the same method.

> Unfortunately, I don't have any device that would be affected by
> this change, but it sounds risky to not call this code anymore:
>
> #ifdef CONFIG_MEDIA_ATTACH
> dvb_detach(release);
> #endif
>
> for .detach ops, as it has the potential of preventing unbind on
> those drivers.

The problem that Wolfgang reported originally was specifically that
this dvb_detach() call was one more than there should be,
leading to a negative reference count. As far as I can tell, this
is true for every user of the 'detach' callback.

>> diff --git a/drivers/media/usb/dvb-usb/pctv452e.c 
>> b/drivers/media/usb/dvb-usb/pctv452e.c
>> index 0af74383083d..ae793dac4964 100644
>> --- a/drivers/media/usb/dvb-usb/pctv452e.c
>> +++ b/drivers/media/usb/dvb-usb/pctv452e.c
>> @@ -913,14 +913,6 @@ static int pctv452e_frontend_attach(struct 
>> dvb_usb_adapter *a)
>>   >dev->i2c_adap);
>>   if (!a->fe_adap[0].fe)
>>   return -ENODEV;
>> -
>> - /*
>> -  * dvb_frontend will call dvb_detach for both stb0899_detach
>> -  * and stb0899_release but we only do dvb_attach(stb0899_attach).
>> -  * Increment the module refcount instead.
>> -  */
>> - symbol_get(stb0899_attach);
>
>
> IMHO, the safest fix would be, instead, to do:
>
> #ifdef CONFIG_MEDIA_ATTACH
> symbol_get(stb0899_attach);
> #endif
>
> Btw, we have some code similar to that on other drivers
> with either symbol_get() or symbol_put().

This would work if we do it for every user of a driver that
has a detach callback in dvb_frontend_ops, but I still think
my suggested patch is correct and less error-prone as a
workaround here.

> Yeah, I agree that this sucks. The right fix here is to use i2c high level
> interfaces, binding it via i2c bus, instead of using the symbol_get()
> based dvb_attach() macro.
>
> We're (very) slowing doing such changes along the media subsystem.

Good to know there is a proper solution already. I had some ideas
for how to change it, but the i2c bus method is clearly better than
whatever I had in mind there.

   Arnd

[PATCH] media: intel-ipu3: cio2: fix building with large PAGE_SIZE

[Arnd Bergmann]

The driver apparently assumes that the device uses the same page size
as the CPU, but also assumes that this is 4096 bytes. On architectures
with a larger page size like 65536 bytes, we get a warning about an
integer overflow:

drivers/media/pci/intel/ipu3/ipu3-cio2.c: In function 
'cio2_fbpt_entry_init_dummy':
arch/arm64/include/asm/page-def.h:28:20: error: large integer implicitly 
truncated to unsigned type [-Werror=overflow]
 #define PAGE_SIZE  (_AC(1, UL) << PAGE_SHIFT)
^
drivers/media/pci/intel/ipu3/ipu3-cio2.h:404:26: note: in expansion of macro 
'PAGE_SIZE'
 #define CIO2_PAGE_SIZE   PAGE_SIZE
  ^
drivers/media/pci/intel/ipu3/ipu3-cio2.c:172:3: note: in expansion of macro 
'CIO2_PAGE_SIZE'
   CIO2_PAGE_SIZE / sizeof(u32) * CIO2_MAX_LOPS;

Obviously this won't work, but the driver is also unlikely to ever be
used on such an architecture, so the easiest workaround is to define
the CIO2_PAGE_SIZE macro to the size that the hardware actually uses.

Fixes: c2a6a07afe4a ("media: intel-ipu3: cio2: add new MIPI-CSI2 driver")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/pci/intel/ipu3/ipu3-cio2.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.h 
b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
index 1a559990920f..78a5799f08e7 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.h
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
@@ -401,7 +401,7 @@ struct cio2_device {
 sizeof(struct cio2_fbpt_entry))
 
 #define CIO2_FBPT_SUBENTRY_UNIT4
-#define CIO2_PAGE_SIZE PAGE_SIZE
+#define CIO2_PAGE_SIZE 4096
 
 /* cio2 fbpt first_entry ctrl status */
 #define CIO2_FBPT_CTRL_VALID   BIT(0)
-- 
2.9.0

[PATCH] media: intel-ipu3: cio2: mark PM functions as __maybe_unused

[Arnd Bergmann]

When CONFIG_PM is disabled, we get harmless warnings about the
suspend/resume callbacks being unused:

drivers/media/pci/intel/ipu3/ipu3-cio2.c:1993:12: error: 'cio2_resume' defined 
but not used [-Werror=unused-function]
drivers/media/pci/intel/ipu3/ipu3-cio2.c:1967:12: error: 'cio2_suspend' defined 
but not used [-Werror=unused-function]

This marks them as __maybe_unused to shut up the warnings.

Fixes: c2a6a07afe4a ("media: intel-ipu3: cio2: add new MIPI-CSI2 driver")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c 
b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 941caa987dab..c2f1447eec0b 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -1964,7 +1964,7 @@ static void cio2_fbpt_rearrange(struct cio2_device *cio2, 
struct cio2_queue *q)
cio2_fbpt_entry_enable(cio2, q->fbpt + i * CIO2_MAX_LOPS);
 }
 
-static int cio2_suspend(struct device *dev)
+static int __maybe_unused cio2_suspend(struct device *dev)
 {
struct pci_dev *pci_dev = to_pci_dev(dev);
struct cio2_device *cio2 = pci_get_drvdata(pci_dev);
@@ -1990,7 +1990,7 @@ static int cio2_suspend(struct device *dev)
return 0;
 }
 
-static int cio2_resume(struct device *dev)
+static int __maybe_unused cio2_resume(struct device *dev)
 {
struct pci_dev *pci_dev = to_pci_dev(dev);
struct cio2_device *cio2 = pci_get_drvdata(pci_dev);
-- 
2.9.0

[PATCH] media: don't drop front-end reference count for ->detach

[Arnd Bergmann]

A bugfix introduce a link failure in configurations without CONFIG_MODULES:

In file included from drivers/media/usb/dvb-usb/pctv452e.c:20:0:
drivers/media/usb/dvb-usb/pctv452e.c: In function 'pctv452e_frontend_attach':
drivers/media/dvb-frontends/stb0899_drv.h:151:36: error: weak declaration of 
'stb0899_attach' being applied to a already existing, static definition

The problem is that the !IS_REACHABLE() declaration of stb0899_attach()
is a 'static inline' definition that clashes with the weak definition.

I further observed that the bugfix was only done for one of the five users
of stb0899_attach(), the other four still have the problem.  This reverts
the bugfix and instead addresses the problem by not dropping the reference
count when calling '->detach()', instead we call this function directly
in dvb_frontend_put() before dropping the kref on the front-end.

Cc: Max Kellermann <max.kellerm...@gmail.com>
Cc: Wolfgang Rohdewald <wolfg...@rohdewald.de>
Fixes: f686c14364ad ("[media] stb0899: move code to "detach" callback")
Fixes: 6cdeaed3b142 ("media: dvb_usb_pctv452e: module refcount changes were 
unbalanced")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/dvb-core/dvb_frontend.c | 4 +++-
 drivers/media/usb/dvb-usb/pctv452e.c  | 8 
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index 87fc1bcae5ae..fe10b6f4d3e0 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -164,6 +164,9 @@ static void dvb_frontend_free(struct kref *ref)
 
 static void dvb_frontend_put(struct dvb_frontend *fe)
 {
+   /* call detach before dropping the reference count */
+   if (fe->ops.detach)
+   fe->ops.detach(fe);
/*
 * Check if the frontend was registered, as otherwise
 * kref was not initialized yet.
@@ -2965,7 +2968,6 @@ void dvb_frontend_detach(struct dvb_frontend* fe)
dvb_frontend_invoke_release(fe, fe->ops.release_sec);
dvb_frontend_invoke_release(fe, fe->ops.tuner_ops.release);
dvb_frontend_invoke_release(fe, fe->ops.analog_ops.release);
-   dvb_frontend_invoke_release(fe, fe->ops.detach);
dvb_frontend_put(fe);
 }
 EXPORT_SYMBOL(dvb_frontend_detach);
diff --git a/drivers/media/usb/dvb-usb/pctv452e.c 
b/drivers/media/usb/dvb-usb/pctv452e.c
index 0af74383083d..ae793dac4964 100644
--- a/drivers/media/usb/dvb-usb/pctv452e.c
+++ b/drivers/media/usb/dvb-usb/pctv452e.c
@@ -913,14 +913,6 @@ static int pctv452e_frontend_attach(struct dvb_usb_adapter 
*a)
>dev->i2c_adap);
if (!a->fe_adap[0].fe)
return -ENODEV;
-
-   /*
-* dvb_frontend will call dvb_detach for both stb0899_detach
-* and stb0899_release but we only do dvb_attach(stb0899_attach).
-* Increment the module refcount instead.
-*/
-   symbol_get(stb0899_attach);
-
if ((dvb_attach(lnbp22_attach, a->fe_adap[0].fe,
>dev->i2c_adap)) == NULL)
err("Cannot attach lnbp22\n");
-- 
2.9.0

Re: [PATCH] media: dvb_usb_pctv452e: module refcount changes were unbalanced

[Arnd Bergmann]

On Tue, Nov 28, 2017 at 12:33 PM, Wolfgang Rohdewald
 wrote:

> @@ -913,6 +913,14 @@ static int pctv452e_frontend_attach(struct 
> dvb_usb_adapter *a)
> >dev->i2c_adap);
> if (!a->fe_adap[0].fe)
> return -ENODEV;
> +
> +   /*
> +* dvb_frontend will call dvb_detach for both stb0899_detach
> +* and stb0899_release but we only do dvb_attach(stb0899_attach).
> +* Increment the module refcount instead.
> +*/
> +   symbol_get(stb0899_attach);
> +
> if ((dvb_attach(lnbp22_attach, a->fe_adap[0].fe,
> >dev->i2c_adap)) == NULL)
> err("Cannot attach lnbp22\n");

This caused a build error in today's linux-next:

In file included from drivers/media/usb/dvb-usb/pctv452e.c:20:0:
drivers/media/usb/dvb-usb/pctv452e.c: In function 'pctv452e_frontend_attach':
drivers/media/dvb-frontends/stb0899_drv.h:151:36: error: weak
declaration of 'stb0899_attach' being applied to a already existing,
static definition
 static inline struct dvb_frontend *stb0899_attach(struct
stb0899_config *config,

I don't really understand where the 'weak' declaration came from, but this seems
to be related to resolving a symbol for a function that was declared
'static inline'.

The random configuration that caused this included:

CONFIG_DVB_USB_PCTV452E=y
# CONFIG_DVB_STB0899 is not set
# CONFIG_MODULES is not set

   Arnd

Re: [PATCH] em28xx: split up em28xx_dvb_init to reduce stack size

[Arnd Bergmann]

On Thu, Dec 14, 2017 at 6:10 PM, Mauro Carvalho Chehab
<mche...@kernel.org> wrote:
> Em Mon, 11 Dec 2017 13:05:02 +0100
> Arnd Bergmann <a...@arndb.de> escreveu:
>
>> With CONFIG_KASAN, the init function uses a large amount of kernel stack:
>>
>> drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init.part.4':
>> drivers/media/usb/em28xx/em28xx-dvb.c:2061:1: error: the frame size of 3232 
>> bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
>>
>> Using gcc-7 with -fsanitize-address-use-after-scope makes this even worse:
>>
>> drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init':
>> drivers/media/usb/em28xx/em28xx-dvb.c:2069:1: error: the frame size of 4280 
>> bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
>>
>> By splitting out each part of the switch/case statement that has its own 
>> local
>> variables into a separate function, no single one of them uses more than 500 
>> bytes,
>> and with a noinline_for_stack annotation we can ensure that they are not 
>> merged
>> back together.
>
> The right fix here is really to find a way to simplify the new method
> of binding I2C devices by using some ancillary routines.
>
> I'll keep this patch on my queue for now, but my plan is to try to solve
> this issue instead of applying it, maybe on the next weeks (as the
> volume of patches reduce due to end of year vacations and Seasons).

That's ok, thanks. We have a workaround in linux-mm that partially solves this
problem to the point where the stack size goes down to 1600 bytes with KASAN,
that by itself would be sufficient to let us enable CONFIG_FRAME_WARN
again for 64-bit platforms with the default 2048 byte warning limit. I reposted
that patch mostly since I want to lower the frame sizes further so we can
reduce the warning limit to 1280 bytes for 64-bit architectures in the future.
There around 10 patches needed for that, and they mostly seem to address
actual issues, so I'd like them to get addressed eventually and set the limit
low enough that the warnings we get on 64-bit are more useful again.

However, could you please revisit two other patches:

https://patchwork.linuxtv.org/patch/45716/ dvb-frontends: fix i2c
access helpers for KASAN
https://patchwork.linuxtv.org/patch/45709/ r820t: fix r820t_write_reg for KASAN

These are currently the ones I'm most interested in getting merged
into v4.15 and LTS kernels for the stack size reduction, since they
are blocking the patch that enables CONFIG_FRAME_WARN for
allmodconfig.

Thanks,

   Arnd

Re: [PATCH] tuners: tda8290: reduce stack usage with kasan

[Arnd Bergmann]

On Tue, Dec 12, 2017 at 1:45 PM, Mauro Carvalho Chehab
 wrote:
> Em Tue, 12 Dec 2017 03:42:32 -0800
> Joe Perches  escreveu:
>
>> > I actually thought about marking them 'const' here before sending
>> > (without noticing the changelog text) and then ran into what must
>> > have led me to drop the 'const' originally: tuner_i2c_xfer_send()
>> > takes a non-const pointer. This can be fixed but it requires
>> > an ugly cast:
>>
>> Casting away const is always a horrible hack.
>>
>> Until it could be changed, my preference would
>> be to update the changelog and perhaps add to
>> the changelog the reason why it can not be const
>> as detailed below.
>>
>> ie: xfer_send and xfer_xend_recv both take a
>> non-const unsigned char *

Ok.

> Perhaps, on a separate changeset, we could change I2C routines to
> accept const unsigned char pointers. This is unrelated to tda8290
> KASAN fixes. So, it should go via I2C tree, and, once accepted
> there, we can change V4L2 drivers (and other drivers) accordingly.

I don't see how that would work unfortunately. i2c_msg contains
a pointer to the data, and that is used for both input and output,
including arrays like

struct i2c_msg msgs[] = {
{
.addr = dvo->slave_addr,
.flags = 0,
.len = 1,
.buf = ,
},
{
.addr = dvo->slave_addr,
.flags = I2C_M_RD,
.len = 1,
.buf = val,
}
};

that have one constant output pointer and one non-constant
input pointer. We could add an anonymous union for 'buf'
to make that two separate pointers, but that's barely any
better than the cast, and it would break the named initializers
in the example above, at least on older compilers. Adding
a second pointer to i2c_msg would add a bit of bloat and
also require tree-wide changes or ugly hacks.

   Arnd

Re: [PATCH] tuners: tda8290: reduce stack usage with kasan

[Arnd Bergmann]

On Mon, Dec 11, 2017 at 10:17 PM, Michael Ira Krufky
<mkru...@linuxtv.org> wrote:
> On Mon, Dec 11, 2017 at 2:34 PM, Joe Perches <j...@perches.com> wrote:
>> On Mon, 2017-12-11 at 13:06 +0100, Arnd Bergmann wrote:
>>> With CONFIG_KASAN enabled, we get a relatively large stack frame in one 
>>> function
>>>
>>> drivers/media/tuners/tda8290.c: In function 'tda8290_set_params':
>>> drivers/media/tuners/tda8290.c:310:1: warning: the frame size of 1520 bytes 
>>> is larger than 1024 bytes [-Wframe-larger-than=]
>>>
>>> With CONFIG_KASAN_EXTRA this goes up to
>>>
>>> drivers/media/tuners/tda8290.c: In function 'tda8290_set_params':
>>> drivers/media/tuners/tda8290.c:310:1: error: the frame size of 3200 bytes 
>>> is larger than 3072 bytes [-Werror=frame-larger-than=]
>>>
>>> We can significantly reduce this by marking local arrays as 'static const', 
>>> and
>>> this should result in better compiled code for everyone.
>> []
>>> diff --git a/drivers/media/tuners/tda8290.c b/drivers/media/tuners/tda8290.c
>> []
>>> @@ -63,8 +63,8 @@ static int tda8290_i2c_bridge(struct dvb_frontend *fe, 
>>> int close)
>>>  {
>>>   struct tda8290_priv *priv = fe->analog_demod_priv;
>>>
>>> - unsigned char  enable[2] = { 0x21, 0xC0 };
>>> - unsigned char disable[2] = { 0x21, 0x00 };
>>> + static unsigned char  enable[2] = { 0x21, 0xC0 };
>>> + static unsigned char disable[2] = { 0x21, 0x00 };
>>
>> Doesn't match commit message.
>>
>> static const or just static?
>>
>>> @@ -84,9 +84,9 @@ static int tda8295_i2c_bridge(struct dvb_frontend *fe, 
>>> int close)
>>>  {
>>>   struct tda8290_priv *priv = fe->analog_demod_priv;
>>>
>>> - unsigned char  enable[2] = { 0x45, 0xc1 };
>>> - unsigned char disable[2] = { 0x46, 0x00 };
>>> - unsigned char buf[3] = { 0x45, 0x01, 0x00 };
>>> + static unsigned char  enable[2] = { 0x45, 0xc1 };
>>> + static unsigned char disable[2] = { 0x46, 0x00 };
>>
>> etc.
>>
>>
>
>
> Joe is correct - they can be CONSTified. My bad -- a lot of the code I
> wrote many years ago has this problem -- I wasn't so stack-conscious
> back then.
>
> The bytes in `enable` / `disable` don't get changed, but they may be
> copied to another byte array that does get changed.  If would be best
> to make these `static const`

Right. This was an older patch of mine that I picked up again
after running into a warning that I had been ignoring for a while,
and I didn't double-check the message.

I actually thought about marking them 'const' here before sending
(without noticing the changelog text) and then ran into what must
have led me to drop the 'const' originally: tuner_i2c_xfer_send()
takes a non-const pointer. This can be fixed but it requires
an ugly cast:

diff --git a/drivers/media/tuners/tuner-i2c.h b/drivers/media/tuners/tuner-i2c.h
index bda67a5a76f2..809466eec780 100644
--- a/drivers/media/tuners/tuner-i2c.h
+++ b/drivers/media/tuners/tuner-i2c.h
@@ -34,10 +34,10 @@ struct tuner_i2c_props {
 };

 static inline int tuner_i2c_xfer_send(struct tuner_i2c_props *props,
- unsigned char *buf, int len)
+ const unsigned char *buf, int len)
 {
struct i2c_msg msg = { .addr = props->addr, .flags = 0,
-  .buf = buf, .len = len };
+  .buf = (unsigned char *)buf, .len = len };
int ret = i2c_transfer(props->adap, , 1);

return (ret == 1) ? len : ret;
@@ -54,11 +54,11 @@ static inline int tuner_i2c_xfer_recv(struct
tuner_i2c_props *props,
 }

 static inline int tuner_i2c_xfer_send_recv(struct tuner_i2c_props *props,
-  unsigned char *obuf, int olen,
+  const unsigned char *obuf, int olen,
   unsigned char *ibuf, int ilen)
 {
struct i2c_msg msg[2] = { { .addr = props->addr, .flags = 0,
-   .buf = obuf, .len = olen },
+   .buf = (unsigned char *)obuf, .len = olen },
  { .addr = props->addr, .flags = I2C_M_RD,
.buf = ibuf, .len = ilen } };
int ret = i2c_transfer(props->adap, msg, 2);

Should I submit it as a two-patch series with that added in, or update
the changelog to not mention 'const' instead?

   Arnd

Re: [PATCH 1/2] usb: gadget: restore tristate-choice for legacy gadgets

[Arnd Bergmann]

On Mon, Dec 11, 2017 at 5:09 PM, Bart Van Assche <bart.vanass...@wdc.com> wrote:
> On Mon, 2017-12-11 at 12:30 +0100, Arnd Bergmann wrote:
>> One patch that was meant as a cleanup apparently did more than it intended,
>> allowing all combinations of legacy gadget drivers to be built into the
>> kernel, and leaving an empty 'choice' statement behind:
>>
>> drivers/usb/gadget/Kconfig:487:warning: choice default symbol 'USB_ETH' is 
>> not contained in the choice
>>
>> The description of commit 7a9618a22aad ("usb: gadget: allow to enable legacy
>> drivers without USB_ETH") was a bit cryptic, as it did not change the
>> behavior of USB_ETH other than allowing it to be built into the kernel
>> alongside other legacy gadgets, which is not a valid configuration.
>>
>> As Felipe explained in the description for commit bc49d1d17dcf ("usb:
>> gadget: don't couple configfs to legacy gadgets"), the configfs based
>> gadgets can be freely configured as loadable modules or built-in
>> drivers, but the legacy gadgets can only be modules if there is more
>> than one of them, so we require the 'choice' statement here.
>>
>> This leaves the added USB_GADGET_LEGACY menuconfig symbol in place,
>> but then restores the 'choice' below it, so we can enforce the
>> single-legacy-gadget rule as before.
>
> Hello Arnd,
>
> A discussion is ongoing about whether or not commit 7a9618a22aad should be 
> reverted.
> Please drop this patch until a conclusion has been reached.

Ok. I'll use a revert of 7a9618a22aad in my local test tree then.
Reverting that is probably good, I thought about suggesting that
instead, but couldn't tell whether you had a bigger plan behind that
commit.

  Arnd

[PATCH] tuners: tda8290: reduce stack usage with kasan

[Arnd Bergmann]

With CONFIG_KASAN enabled, we get a relatively large stack frame in one function

drivers/media/tuners/tda8290.c: In function 'tda8290_set_params':
drivers/media/tuners/tda8290.c:310:1: warning: the frame size of 1520 bytes is 
larger than 1024 bytes [-Wframe-larger-than=]

With CONFIG_KASAN_EXTRA this goes up to

drivers/media/tuners/tda8290.c: In function 'tda8290_set_params':
drivers/media/tuners/tda8290.c:310:1: error: the frame size of 3200 bytes is 
larger than 3072 bytes [-Werror=frame-larger-than=]

We can significantly reduce this by marking local arrays as 'static const', and
this should result in better compiled code for everyone.

I have another patch for the same symptom to patch tuner_i2c_xfer_*, and we
actually want both of them.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/tuners/tda8290.c | 76 ++
 1 file changed, 39 insertions(+), 37 deletions(-)

diff --git a/drivers/media/tuners/tda8290.c b/drivers/media/tuners/tda8290.c
index a59c567c55d6..19854221b72d 100644
--- a/drivers/media/tuners/tda8290.c
+++ b/drivers/media/tuners/tda8290.c
@@ -63,8 +63,8 @@ static int tda8290_i2c_bridge(struct dvb_frontend *fe, int 
close)
 {
struct tda8290_priv *priv = fe->analog_demod_priv;
 
-   unsigned char  enable[2] = { 0x21, 0xC0 };
-   unsigned char disable[2] = { 0x21, 0x00 };
+   static unsigned char  enable[2] = { 0x21, 0xC0 };
+   static unsigned char disable[2] = { 0x21, 0x00 };
unsigned char *msg;
 
if (close) {
@@ -84,9 +84,9 @@ static int tda8295_i2c_bridge(struct dvb_frontend *fe, int 
close)
 {
struct tda8290_priv *priv = fe->analog_demod_priv;
 
-   unsigned char  enable[2] = { 0x45, 0xc1 };
-   unsigned char disable[2] = { 0x46, 0x00 };
-   unsigned char buf[3] = { 0x45, 0x01, 0x00 };
+   static unsigned char  enable[2] = { 0x45, 0xc1 };
+   static unsigned char disable[2] = { 0x46, 0x00 };
+   static unsigned char buf[3] = { 0x45, 0x01, 0x00 };
unsigned char *msg;
 
if (close) {
@@ -178,24 +178,24 @@ static void tda8290_set_params(struct dvb_frontend *fe,
 {
struct tda8290_priv *priv = fe->analog_demod_priv;
 
-   unsigned char soft_reset[]  = { 0x00, 0x00 };
+   static unsigned char soft_reset[]  = { 0x00, 0x00 };
unsigned char easy_mode[]   = { 0x01, priv->tda8290_easy_mode };
-   unsigned char expert_mode[] = { 0x01, 0x80 };
-   unsigned char agc_out_on[]  = { 0x02, 0x00 };
-   unsigned char gainset_off[] = { 0x28, 0x14 };
-   unsigned char if_agc_spd[]  = { 0x0f, 0x88 };
-   unsigned char adc_head_6[]  = { 0x05, 0x04 };
-   unsigned char adc_head_9[]  = { 0x05, 0x02 };
-   unsigned char adc_head_12[] = { 0x05, 0x01 };
-   unsigned char pll_bw_nom[]  = { 0x0d, 0x47 };
-   unsigned char pll_bw_low[]  = { 0x0d, 0x27 };
-   unsigned char gainset_2[]   = { 0x28, 0x64 };
-   unsigned char agc_rst_on[]  = { 0x0e, 0x0b };
-   unsigned char agc_rst_off[] = { 0x0e, 0x09 };
-   unsigned char if_agc_set[]  = { 0x0f, 0x81 };
-   unsigned char addr_adc_sat  = 0x1a;
-   unsigned char addr_agc_stat = 0x1d;
-   unsigned char addr_pll_stat = 0x1b;
+   static unsigned char expert_mode[] = { 0x01, 0x80 };
+   static unsigned char agc_out_on[]  = { 0x02, 0x00 };
+   static unsigned char gainset_off[] = { 0x28, 0x14 };
+   static unsigned char if_agc_spd[]  = { 0x0f, 0x88 };
+   static unsigned char adc_head_6[]  = { 0x05, 0x04 };
+   static unsigned char adc_head_9[]  = { 0x05, 0x02 };
+   static unsigned char adc_head_12[] = { 0x05, 0x01 };
+   static unsigned char pll_bw_nom[]  = { 0x0d, 0x47 };
+   static unsigned char pll_bw_low[]  = { 0x0d, 0x27 };
+   static unsigned char gainset_2[]   = { 0x28, 0x64 };
+   static unsigned char agc_rst_on[]  = { 0x0e, 0x0b };
+   static unsigned char agc_rst_off[] = { 0x0e, 0x09 };
+   static unsigned char if_agc_set[]  = { 0x0f, 0x81 };
+   static unsigned char addr_adc_sat  = 0x1a;
+   static unsigned char addr_agc_stat = 0x1d;
+   static unsigned char addr_pll_stat = 0x1b;
unsigned char adc_sat, agc_stat,
  pll_stat;
int i;
@@ -468,9 +468,9 @@ static void tda8290_standby(struct dvb_frontend *fe)
 {
struct tda8290_priv *priv = fe->analog_demod_priv;
 
-   unsigned char cb1[] = { 0x30, 0xD0 };
-   unsigned char tda8290_standby[] = { 0x00, 0x02 };
-   unsigned char tda8290_agc_tri[] = { 0x02, 0x20 };
+   static unsigned char cb1[] = { 0x30, 0xD0 };
+   static unsigned char tda8290_standby[] = { 0x00, 0x02 };
+   static unsigned char tda8290_agc_tri[] = { 0x02, 0x20 };
struct i2c_msg msg = {.addr = priv->tda827x_addr, .flags=0, .buf=cb1, 
.len = 2};
 
if (fe->ops.analog_ops.i2c_gate_ctrl)
@@ -495,9 +495,9 @@ static void tda8290_init_if(struct dvb_frontend *fe)
 {

[PATCH] em28xx: split up em28xx_dvb_init to reduce stack size

[Arnd Bergmann]

With CONFIG_KASAN, the init function uses a large amount of kernel stack:

drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init.part.4':
drivers/media/usb/em28xx/em28xx-dvb.c:2061:1: error: the frame size of 3232 
bytes is larger than 2048 bytes [-Werror=frame-larger-than=]

Using gcc-7 with -fsanitize-address-use-after-scope makes this even worse:

drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init':
drivers/media/usb/em28xx/em28xx-dvb.c:2069:1: error: the frame size of 4280 
bytes is larger than 3072 bytes [-Werror=frame-larger-than=]

By splitting out each part of the switch/case statement that has its own local
variables into a separate function, no single one of them uses more than 500 
bytes,
and with a noinline_for_stack annotation we can ensure that they are not merged
back together.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/usb/em28xx/em28xx-dvb.c | 947 ++
 1 file changed, 508 insertions(+), 439 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c 
b/drivers/media/usb/em28xx/em28xx-dvb.c
index 9950a740e04e..6665885dbaa7 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -934,7 +934,7 @@ static struct lgdt3306a_config 
hauppauge_01595_lgdt3306a_config = {
 
 /* -- */
 
-static int em28xx_attach_xc3028(u8 addr, struct em28xx *dev)
+static noinline_for_stack int em28xx_attach_xc3028(u8 addr, struct em28xx *dev)
 {
struct dvb_frontend *fe;
struct xc2028_config cfg;
@@ -1126,6 +1126,492 @@ static void em28xx_unregister_dvb(struct em28xx_dvb 
*dvb)
dvb_unregister_adapter(>adapter);
 }
 
+static noinline_for_stack int em28174_dvb_init_pctv_460e(struct em28xx *dev)
+{
+   struct em28xx_dvb *dvb = dev->dvb;
+   struct i2c_client *client;
+   struct i2c_board_info board_info;
+   struct tda10071_platform_data tda10071_pdata = {};
+   struct a8293_platform_data a8293_pdata = {};
+   int result;
+
+   /* attach demod + tuner combo */
+   tda10071_pdata.clk = 40444000, /* 40.444 MHz */
+   tda10071_pdata.i2c_wr_max = 64,
+   tda10071_pdata.ts_mode = TDA10071_TS_SERIAL,
+   tda10071_pdata.pll_multiplier = 20,
+   tda10071_pdata.tuner_i2c_addr = 0x14,
+   memset(_info, 0, sizeof(board_info));
+   strlcpy(board_info.type, "tda10071_cx24118", I2C_NAME_SIZE);
+   board_info.addr = 0x55;
+   board_info.platform_data = _pdata;
+   request_module("tda10071");
+   client = i2c_new_device(>i2c_adap[dev->def_i2c_bus], _info);
+   if (client == NULL || client->dev.driver == NULL) {
+   result = -ENODEV;
+   goto out_free;
+   }
+   if (!try_module_get(client->dev.driver->owner)) {
+   i2c_unregister_device(client);
+   result = -ENODEV;
+   goto out_free;
+   }
+   dvb->fe[0] = tda10071_pdata.get_dvb_frontend(client);
+   dvb->i2c_client_demod = client;
+
+   /* attach SEC */
+   a8293_pdata.dvb_frontend = dvb->fe[0];
+   memset(_info, 0, sizeof(board_info));
+   strlcpy(board_info.type, "a8293", I2C_NAME_SIZE);
+   board_info.addr = 0x08;
+   board_info.platform_data = _pdata;
+   request_module("a8293");
+   client = i2c_new_device(>i2c_adap[dev->def_i2c_bus], _info);
+   if (client == NULL || client->dev.driver == NULL) {
+   module_put(dvb->i2c_client_demod->dev.driver->owner);
+   i2c_unregister_device(dvb->i2c_client_demod);
+   result = -ENODEV;
+   goto out_free;
+   }
+   if (!try_module_get(client->dev.driver->owner)) {
+   i2c_unregister_device(client);
+   module_put(dvb->i2c_client_demod->dev.driver->owner);
+   i2c_unregister_device(dvb->i2c_client_demod);
+   result = -ENODEV;
+   goto out_free;
+   }
+   dvb->i2c_client_sec = client;
+   result = 0;
+out_free:
+   return result;
+}
+
+static noinline_for_stack int em28178_dvb_init_pctv_461e(struct em28xx *dev)
+{
+   struct em28xx_dvb *dvb = dev->dvb;
+   struct i2c_client *client;
+   struct i2c_adapter *i2c_adapter;
+   struct i2c_board_info board_info;
+   struct m88ds3103_platform_data m88ds3103_pdata = {};
+   struct ts2020_config ts2020_config = {};
+   struct a8293_platform_data a8293_pdata = {};
+   int result;
+
+   /* attach demod */
+   m88ds3103_pdata.clk = 2700;
+   m88ds3103_pdata.i2c_wr_max = 33;
+   m88ds3103_pdata.ts_mode = M88DS3103_TS_PARALLEL;
+   m88ds3103_pdata.ts_clk = 16000;
+   m88ds3103_pdata.ts_clk_pol = 1;
+   m88ds3103_pdata.agc = 0x99;
+   memset(_info, 0, sizeof(board_info));
+   strlcp

[PATCH 2/2] usb: gadget: webcam: fix V4L2 Kconfig dependency

[Arnd Bergmann]

Configuring the USB_G_WEBCAM driver as built-in leads to a link
error when CONFIG_VIDEO_V4L2 is a loadable module:

drivers/usb/gadget/function/f_uvc.o: In function `uvc_function_setup':
f_uvc.c:(.text+0xfe): undefined reference to `v4l2_event_queue'
drivers/usb/gadget/function/f_uvc.o: In function `uvc_function_ep0_complete':
f_uvc.c:(.text+0x188): undefined reference to `v4l2_event_queue'

This changes the Kconfig dependency to disallow that configuration,
and force it to be a module in that case as well.

This is apparently a rather old bug, but very hard to trigger
even in thousands of randconfig builds.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/usb/gadget/legacy/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/legacy/Kconfig 
b/drivers/usb/gadget/legacy/Kconfig
index 2d80a9d1d5d9..fbd974965399 100644
--- a/drivers/usb/gadget/legacy/Kconfig
+++ b/drivers/usb/gadget/legacy/Kconfig
@@ -513,7 +513,7 @@ endif
 # or video class gadget drivers), or specific hardware, here.
 config USB_G_WEBCAM
tristate "USB Webcam Gadget"
-   depends on VIDEO_DEV
+   depends on VIDEO_V4L2
select USB_LIBCOMPOSITE
select VIDEOBUF2_VMALLOC
select USB_F_UVC
-- 
2.9.0

[PATCH 1/2] usb: gadget: restore tristate-choice for legacy gadgets

[Arnd Bergmann]

One patch that was meant as a cleanup apparently did more than it intended,
allowing all combinations of legacy gadget drivers to be built into the
kernel, and leaving an empty 'choice' statement behind:

drivers/usb/gadget/Kconfig:487:warning: choice default symbol 'USB_ETH' is not 
contained in the choice

The description of commit 7a9618a22aad ("usb: gadget: allow to enable legacy
drivers without USB_ETH") was a bit cryptic, as it did not change the
behavior of USB_ETH other than allowing it to be built into the kernel
alongside other legacy gadgets, which is not a valid configuration.

As Felipe explained in the description for commit bc49d1d17dcf ("usb:
gadget: don't couple configfs to legacy gadgets"), the configfs based
gadgets can be freely configured as loadable modules or built-in
drivers, but the legacy gadgets can only be modules if there is more
than one of them, so we require the 'choice' statement here.

This leaves the added USB_GADGET_LEGACY menuconfig symbol in place,
but then restores the 'choice' below it, so we can enforce the
single-legacy-gadget rule as before.

Fixes: 7a9618a22aad ("usb: gadget: allow to enable legacy drivers without 
USB_ETH")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/usb/gadget/Kconfig| 28 
 drivers/usb/gadget/legacy/Kconfig | 28 
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 0a19a76645ad..eab61f552c19 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -482,34 +482,6 @@ config USB_CONFIGFS_F_TCM
  Both protocols can work on USB2.0 and USB3.0.
  UAS utilizes the USB 3.0 feature called streams support.
 
-choice
-   tristate "USB Gadget precomposed configurations"
-   default USB_ETH
-   optional
-   help
- A Linux "Gadget Driver" talks to the USB Peripheral Controller
- driver through the abstract "gadget" API.  Some other operating
- systems call these "client" drivers, of which "class drivers"
- are a subset (implementing a USB device class specification).
- A gadget driver implements one or more USB functions using
- the peripheral hardware.
-
- Gadget drivers are hardware-neutral, or "platform independent",
- except that they sometimes must understand quirks or limitations
- of the particular controllers they work with.  For example, when
- a controller doesn't support alternate configurations or provide
- enough of the right types of endpoints, the gadget driver might
- not be able work with that controller, or might need to implement
- a less common variant of a device class protocol.
-
- The available choices each represent a single precomposed USB
- gadget configuration. In the device model, each option contains
- both the device instantiation as a child for a USB gadget
- controller, and the relevant drivers for each function declared
- by the device.
-
-endchoice
-
 source "drivers/usb/gadget/legacy/Kconfig"
 
 endif # USB_GADGET
diff --git a/drivers/usb/gadget/legacy/Kconfig 
b/drivers/usb/gadget/legacy/Kconfig
index 9570bbeced4f..2d80a9d1d5d9 100644
--- a/drivers/usb/gadget/legacy/Kconfig
+++ b/drivers/usb/gadget/legacy/Kconfig
@@ -21,6 +21,32 @@ menuconfig USB_GADGET_LEGACY
 
 if USB_GADGET_LEGACY
 
+choice
+   tristate "USB Gadget precomposed configurations"
+   default USB_ETH
+   optional
+   help
+ A Linux "Gadget Driver" talks to the USB Peripheral Controller
+ driver through the abstract "gadget" API.  Some other operating
+ systems call these "client" drivers, of which "class drivers"
+ are a subset (implementing a USB device class specification).
+ A gadget driver implements one or more USB functions using
+ the peripheral hardware.
+
+ Gadget drivers are hardware-neutral, or "platform independent",
+ except that they sometimes must understand quirks or limitations
+ of the particular controllers they work with.  For example, when
+ a controller doesn't support alternate configurations or provide
+ enough of the right types of endpoints, the gadget driver might
+ not be able work with that controller, or might need to implement
+ a less common variant of a device class protocol.
+
+ The available choices each represent a single precomposed USB
+ gadget configuration. In the device model, each option contains
+ both the device instantiation as a child for a USB gadget
+ controller, and the relevant drivers for each function declared
+ by the device.
+
 config USB_ZER

[PATCH] string.h: work around for increased stack usage

[Arnd Bergmann]

The hardened strlen() function causes rather large stack usage in at
least one file in the kernel, in particular when CONFIG_KASAN is enabled:

drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init':
drivers/media/usb/em28xx/em28xx-dvb.c:2062:1: error: the frame size of 3256 
bytes is larger than 204 bytes [-Werror=frame-larger-than=]

Analyzing this problem led to the discovery that gcc fails to merge the
stack slots for the i2c_board_info[] structures after we strlcpy() into
them, due to the 'noreturn' attribute on the source string length check.

I reported this as a gcc bug, but it is unlikely to get fixed for gcc-8,
since it is relatively easy to work around, and it gets triggered rarely.
An earlier workaround I did added an empty inline assembly statement
before the call to fortify_panic(), which works surprisingly well,
but is really ugly and unintuitive.

This is a new approach to the same problem, this time addressing it by
not calling the 'extern __real_strnlen()' function for string constants
where __builtin_strlen() is a compile-time constant and therefore known
to be safe. We do this by checking if the last character in the string
is a compile-time constant '\0'. If it is, we can assume that
strlen() of the string is also constant. As a side-effect, this should
also improve the object code output for any other call of strlen()
on a string constant.

Cc: sta...@vger.kernel.org
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365
Link: https://patchwork.kernel.org/patch/9980413/
Link: https://patchwork.kernel.org/patch/9974047/
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
v3: don't use an asm barrier but use a constant string change.

Aside from two other patches for drivers/media that I sent last week,
this should fix all stack frames above 2KB, once all three are merged,
I'll send the patch to re-enable the warning.
---
 include/linux/string.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 410ecf17de3c..e5cc3f27f6e0 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -259,7 +259,8 @@ __FORTIFY_INLINE __kernel_size_t strlen(const char *p)
 {
__kernel_size_t ret;
size_t p_size = __builtin_object_size(p, 0);
-   if (p_size == (size_t)-1)
+   if (p_size == (size_t)-1 ||
+   (__builtin_constant_p(p[p_size - 1]) && p[p_size - 1] == '\0'))
return __builtin_strlen(p);
ret = strnlen(p, p_size);
if (p_size <= ret)
-- 
2.9.0

Re: [PATCH 2/8] [media] uvc_video: use ktime_t for timestamps

[Arnd Bergmann]

On Tue, Dec 5, 2017 at 1:58 AM, Laurent Pinchart
<laurent.pinch...@ideasonboard.com> wrote:
> Hi Arnd,
>
> On Tuesday, 5 December 2017 02:37:27 EET Laurent Pinchart wrote:
>> On Monday, 27 November 2017 15:19:54 EET Arnd Bergmann wrote:
>> > uvc_video_get_ts() returns a 'struct timespec', but all its users
>> > really want a nanoseconds variable anyway.
>> >
>> > Changing the deprecated ktime_get_ts/ktime_get_real_ts to ktime_get
>> > and ktime_get_real simplifies the code noticeably, while keeping
>> > the resulting numbers unchanged.
>> >
>> > Signed-off-by: Arnd Bergmann <a...@arndb.de>
>> > ---
>> >
>> >  drivers/media/usb/uvc/uvc_video.c | 37 -
>> >  drivers/media/usb/uvc/uvcvideo.h  |  2 +-
>> >  2 files changed, 13 insertions(+), 26 deletions(-)
>
> [snip]
>
>> > -   struct timespec ts;
>> > +   u64 timestamp;
>
> [snip]
>
>> > uvc_trace(UVC_TRACE_CLOCK, "%s: SOF %u.%06llu y %llu ts %llu "
>> >   "buf ts %llu (x1 %u/%u/%u x2 %u/%u/%u y1 %u y2 %u)\n",
>> >   stream->dev->name,
>> >   sof >> 16, div_u64(((u64)sof & 0x) * 100LLU, 65536),
>> > - y, timespec_to_ns(), vbuf->vb2_buf.timestamp,
>> > + y, timestamp, vbuf->vb2_buf.timestamp,
>> >   x1, first->host_sof, first->dev_sof,
>> >   x2, last->host_sof, last->dev_sof, y1, y2);
>
> As you've done lots of work moving code away from timespec I figured out I
> would ask, what is the preferred way to print a ktime in secs.nsecs format ?
> Should I use ktime_to_timespec and print ts.tv_sec and ts.tv_nsec, or is there
> a better way ?

We had patches under discussion to add a special printk format string that
would pretty-print a date, but that never got merged. Usually there is a
tradeoff between runtime to convert the nanoseconds into a different format
and how useful you want it to be. ktime_to_timespec() can be a bit slow on
some architectures, since it has to do a 64-bit division, but then again
the sprintf logic also needs to do that. If the output isn't on a time-critical
path, you can use time64_to_tm and print it in years/months/days/hours/
minutes/seconds, but depending on where it gets printed, that may not
be easier to interpret than the seconds/nanoseconds or pure
nanoseconds.

   Arnd

[PATCH v2] dvb-frontends: fix i2c access helpers for KASAN

[Arnd Bergmann]

A typical code fragment was copied across many dvb-frontend drivers and
causes large stack frames when built with with CONFIG_KASAN on gcc-5/6/7:

drivers/media/dvb-frontends/cxd2841er.c:3225:1: error: the frame size of 3992 
bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
drivers/media/dvb-frontends/cxd2841er.c:3404:1: error: the frame size of 3136 
bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
drivers/media/dvb-frontends/stv0367.c:3143:1: error: the frame size of 4016 
bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
drivers/media/dvb-frontends/stv090x.c:3430:1: error: the frame size of 5312 
bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
drivers/media/dvb-frontends/stv090x.c:4248:1: error: the frame size of 4872 
bytes is larger than 3072 bytes [-Werror=frame-larger-than=]

gcc-8 now solves this by consolidating the stack slots for the argument
variables, but on older compilers we can get the same behavior by taking
the pointer of a local variable rather than the inline function argument.

Cc: sta...@vger.kernel.org
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
v2: add a comment for each instance of the problem, linking
to the gcc bugzilla (I left out the http:// so it would fit
in one line)
---
 drivers/media/dvb-frontends/ascot2e.c | 4 +++-
 drivers/media/dvb-frontends/cxd2841er.c   | 4 +++-
 drivers/media/dvb-frontends/helene.c  | 4 +++-
 drivers/media/dvb-frontends/horus3a.c | 4 +++-
 drivers/media/dvb-frontends/itd1000.c | 5 +++--
 drivers/media/dvb-frontends/mt312.c   | 5 -
 drivers/media/dvb-frontends/stb0899_drv.c | 3 ++-
 drivers/media/dvb-frontends/stb6100.c | 6 --
 drivers/media/dvb-frontends/stv0367.c | 4 +++-
 drivers/media/dvb-frontends/stv090x.c | 4 +++-
 drivers/media/dvb-frontends/stv6110x.c| 4 +++-
 drivers/media/dvb-frontends/zl10039.c | 4 +++-
 12 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/drivers/media/dvb-frontends/ascot2e.c 
b/drivers/media/dvb-frontends/ascot2e.c
index 0ee0df53b91b..79d5d89bc95e 100644
--- a/drivers/media/dvb-frontends/ascot2e.c
+++ b/drivers/media/dvb-frontends/ascot2e.c
@@ -155,7 +155,9 @@ static int ascot2e_write_regs(struct ascot2e_priv *priv,
 
 static int ascot2e_write_reg(struct ascot2e_priv *priv, u8 reg, u8 val)
 {
-   return ascot2e_write_regs(priv, reg, , 1);
+   u8 tmp = val; /* see gcc.gnu.org/bugzilla/show_bug.cgi?id=81715 */
+
+   return ascot2e_write_regs(priv, reg, , 1);
 }
 
 static int ascot2e_read_regs(struct ascot2e_priv *priv,
diff --git a/drivers/media/dvb-frontends/cxd2841er.c 
b/drivers/media/dvb-frontends/cxd2841er.c
index 48ee9bc00c06..ccbd84fd6428 100644
--- a/drivers/media/dvb-frontends/cxd2841er.c
+++ b/drivers/media/dvb-frontends/cxd2841er.c
@@ -257,7 +257,9 @@ static int cxd2841er_write_regs(struct cxd2841er_priv *priv,
 static int cxd2841er_write_reg(struct cxd2841er_priv *priv,
   u8 addr, u8 reg, u8 val)
 {
-   return cxd2841er_write_regs(priv, addr, reg, , 1);
+   u8 tmp = val; /* see gcc.gnu.org/bugzilla/show_bug.cgi?id=81715 */
+
+   return cxd2841er_write_regs(priv, addr, reg, , 1);
 }
 
 static int cxd2841er_read_regs(struct cxd2841er_priv *priv,
diff --git a/drivers/media/dvb-frontends/helene.c 
b/drivers/media/dvb-frontends/helene.c
index 4bf5a551ba40..2ab8d83e5576 100644
--- a/drivers/media/dvb-frontends/helene.c
+++ b/drivers/media/dvb-frontends/helene.c
@@ -331,7 +331,9 @@ static int helene_write_regs(struct helene_priv *priv,
 
 static int helene_write_reg(struct helene_priv *priv, u8 reg, u8 val)
 {
-   return helene_write_regs(priv, reg, , 1);
+   u8 tmp = val; /* see gcc.gnu.org/bugzilla/show_bug.cgi?id=81715 */
+
+   return helene_write_regs(priv, reg, , 1);
 }
 
 static int helene_read_regs(struct helene_priv *priv,
diff --git a/drivers/media/dvb-frontends/horus3a.c 
b/drivers/media/dvb-frontends/horus3a.c
index 68d759c4c52e..5c8b405f2ddc 100644
--- a/drivers/media/dvb-frontends/horus3a.c
+++ b/drivers/media/dvb-frontends/horus3a.c
@@ -89,7 +89,9 @@ static int horus3a_write_regs(struct horus3a_priv *priv,
 
 static int horus3a_write_reg(struct horus3a_priv *priv, u8 reg, u8 val)
 {
-   return horus3a_write_regs(priv, reg, , 1);
+   u8 tmp = val; /* see gcc.gnu.org/bugzilla/show_bug.cgi?id=81715 */
+
+   return horus3a_write_regs(priv, reg, , 1);
 }
 
 static int horus3a_enter_power_save(struct horus3a_priv *priv)
diff --git a/drivers/media/dvb-frontends/itd1000.c 
b/drivers/media/dvb-frontends/itd1000.c
index 5bb1e73a10b4..ce7c443d3eac 100644
--- a/drivers/media/dvb-frontends/itd1000.c
+++ b/drivers/media/dvb-frontends/itd1000.c
@@ -95,8 +95,9 @@ static int itd1000_read_reg(struct itd1000_state *state, u8 
reg)
 
 static inline int itd1000_write_reg(struct itd1000_state *state, u8 r, u8 v)
 {
-   int ret = itd1000_write_regs(state, r, , 1);
-  

Re: [PATCH, RESEND 1/2] dvb-frontends: fix i2c access helpers for KASAN

[Arnd Bergmann]

On Thu, Nov 30, 2017 at 1:49 PM, Mauro Carvalho Chehab
<mche...@kernel.org> wrote:
>> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715
>> Signed-off-by: Arnd Bergmann <a...@arndb.de>
>> ---
>> I'm undecided here whether there should be a comment pointing
>> to PR81715 for each file that the bogus local variable workaround
>> to prevent it from being cleaned up again. It's probably not
>> necessary since anything that causes actual problems would also
>> trigger a build warning.

>
> This kind of sucks, and it is completely unexpected... why val is
> so special that it would require this kind of hack?

It's explained in the gcc bug report: basically gcc always skipped
one optimization on inline function arguments that it does on
normal variables. Without KASAN and asan-stack, we didn't
notice because the impact was fairly small, but I ended up finally
getting to the bottom of it in September, and it finally got fixed.

I had an older version of the patch that was much more invasive
before we understood what exactly is happening, see
https://lkml.org/lkml/2017/3/2/484

> Also, there's always a risk of someone see it and decide to
> simplify the code, returning it to the previous state.
>
> So, if we're willing to do something like that, IMHO, we should have
> some macro that would document it, and fall back to the direct
> code if the compiler is not gcc 5, 6 or 7.

Older compilers are also affected and will produce better code
with my change, the difference is just smaller without asan-stack
(added ion gcc-5) is disabled, since that increases the stack
space used by each variable to (IIRC) 32 bytes.

The fixed gcc-8 produces identical code with and without my
change.

I don't think that a macro would help here at all, but if you
prefer, I could add a link to that gcc bug in each function that
has the problem.

 Arnd

[PATCH, RESEND 1/2] dvb-frontends: fix i2c access helpers for KASAN

[Arnd Bergmann]

A typical code fragment was copied across many dvb-frontend drivers and
causes large stack frames when built with with CONFIG_KASAN on gcc-5/6/7:

drivers/media/dvb-frontends/cxd2841er.c:3225:1: error: the frame size of 3992 
bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
drivers/media/dvb-frontends/cxd2841er.c:3404:1: error: the frame size of 3136 
bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
drivers/media/dvb-frontends/stv0367.c:3143:1: error: the frame size of 4016 
bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
drivers/media/dvb-frontends/stv090x.c:3430:1: error: the frame size of 5312 
bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
drivers/media/dvb-frontends/stv090x.c:4248:1: error: the frame size of 4872 
bytes is larger than 3072 bytes [-Werror=frame-larger-than=]

gcc-8 now solves this by consolidating the stack slots for the argument
variables, but on older compilers we can get the same behavior by taking
the pointer of a local variable rather than the inline function argument.

Cc: sta...@vger.kernel.org
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
I'm undecided here whether there should be a comment pointing
to PR81715 for each file that the bogus local variable workaround
to prevent it from being cleaned up again. It's probably not
necessary since anything that causes actual problems would also
trigger a build warning.
---
 drivers/media/dvb-frontends/ascot2e.c | 4 +++-
 drivers/media/dvb-frontends/cxd2841er.c   | 4 +++-
 drivers/media/dvb-frontends/helene.c  | 4 +++-
 drivers/media/dvb-frontends/horus3a.c | 4 +++-
 drivers/media/dvb-frontends/itd1000.c | 5 +++--
 drivers/media/dvb-frontends/mt312.c   | 4 +++-
 drivers/media/dvb-frontends/stb0899_drv.c | 3 ++-
 drivers/media/dvb-frontends/stb6100.c | 6 --
 drivers/media/dvb-frontends/stv0367.c | 4 +++-
 drivers/media/dvb-frontends/stv090x.c | 4 +++-
 drivers/media/dvb-frontends/stv6110x.c| 4 +++-
 drivers/media/dvb-frontends/zl10039.c | 4 +++-
 12 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/drivers/media/dvb-frontends/ascot2e.c 
b/drivers/media/dvb-frontends/ascot2e.c
index 0ee0df53b91b..1219272ca3f0 100644
--- a/drivers/media/dvb-frontends/ascot2e.c
+++ b/drivers/media/dvb-frontends/ascot2e.c
@@ -155,7 +155,9 @@ static int ascot2e_write_regs(struct ascot2e_priv *priv,
 
 static int ascot2e_write_reg(struct ascot2e_priv *priv, u8 reg, u8 val)
 {
-   return ascot2e_write_regs(priv, reg, , 1);
+   u8 tmp = val;
+
+   return ascot2e_write_regs(priv, reg, , 1);
 }
 
 static int ascot2e_read_regs(struct ascot2e_priv *priv,
diff --git a/drivers/media/dvb-frontends/cxd2841er.c 
b/drivers/media/dvb-frontends/cxd2841er.c
index 48ee9bc00c06..b7574deff5c6 100644
--- a/drivers/media/dvb-frontends/cxd2841er.c
+++ b/drivers/media/dvb-frontends/cxd2841er.c
@@ -257,7 +257,9 @@ static int cxd2841er_write_regs(struct cxd2841er_priv *priv,
 static int cxd2841er_write_reg(struct cxd2841er_priv *priv,
   u8 addr, u8 reg, u8 val)
 {
-   return cxd2841er_write_regs(priv, addr, reg, , 1);
+   u8 tmp = val;
+
+   return cxd2841er_write_regs(priv, addr, reg, , 1);
 }
 
 static int cxd2841er_read_regs(struct cxd2841er_priv *priv,
diff --git a/drivers/media/dvb-frontends/helene.c 
b/drivers/media/dvb-frontends/helene.c
index 4bf5a551ba40..6e93f2d1575b 100644
--- a/drivers/media/dvb-frontends/helene.c
+++ b/drivers/media/dvb-frontends/helene.c
@@ -331,7 +331,9 @@ static int helene_write_regs(struct helene_priv *priv,
 
 static int helene_write_reg(struct helene_priv *priv, u8 reg, u8 val)
 {
-   return helene_write_regs(priv, reg, , 1);
+   u8 tmp = val;
+
+   return helene_write_regs(priv, reg, , 1);
 }
 
 static int helene_read_regs(struct helene_priv *priv,
diff --git a/drivers/media/dvb-frontends/horus3a.c 
b/drivers/media/dvb-frontends/horus3a.c
index 68d759c4c52e..fa9e2d373073 100644
--- a/drivers/media/dvb-frontends/horus3a.c
+++ b/drivers/media/dvb-frontends/horus3a.c
@@ -89,7 +89,9 @@ static int horus3a_write_regs(struct horus3a_priv *priv,
 
 static int horus3a_write_reg(struct horus3a_priv *priv, u8 reg, u8 val)
 {
-   return horus3a_write_regs(priv, reg, , 1);
+   u8 tmp = val;
+
+   return horus3a_write_regs(priv, reg, , 1);
 }
 
 static int horus3a_enter_power_save(struct horus3a_priv *priv)
diff --git a/drivers/media/dvb-frontends/itd1000.c 
b/drivers/media/dvb-frontends/itd1000.c
index 5bb1e73a10b4..1ac5177162f6 100644
--- a/drivers/media/dvb-frontends/itd1000.c
+++ b/drivers/media/dvb-frontends/itd1000.c
@@ -95,8 +95,9 @@ static int itd1000_read_reg(struct itd1000_state *state, u8 
reg)
 
 static inline int itd1000_write_reg(struct itd1000_state *state, u8 r, u8 v)
 {
-   int ret = itd1000_write_regs(state, r, , 1);
-   state->shadow[r] = v;
+   u8 tmp = v;
+ 

[PATCH, RESEND 2/2] r820t: fix r820t_write_reg for KASAN

[Arnd Bergmann]

With CONFIG_KASAN, we get an overly long stack frame due to inlining
the register access functions:

drivers/media/tuners/r820t.c: In function 'generic_set_freq.isra.7':
drivers/media/tuners/r820t.c:1334:1: error: the frame size of 2880 bytes is 
larger than 2048 bytes [-Werror=frame-larger-than=]

This is caused by a gcc bug that has now been fixed in gcc-8.
To work around the problem, we can pass the register data
through a local variable that older gcc versions can optimize
out as well.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/tuners/r820t.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
index ba80376a3b86..d097eb04a0e9 100644
--- a/drivers/media/tuners/r820t.c
+++ b/drivers/media/tuners/r820t.c
@@ -396,9 +396,11 @@ static int r820t_write(struct r820t_priv *priv, u8 reg, 
const u8 *val,
return 0;
 }
 
-static int r820t_write_reg(struct r820t_priv *priv, u8 reg, u8 val)
+static inline int r820t_write_reg(struct r820t_priv *priv, u8 reg, u8 val)
 {
-   return r820t_write(priv, reg, , 1);
+   u8 tmp = val; /* work around GCC PR81715 with asan-stack=1 */
+
+   return r820t_write(priv, reg, , 1);
 }
 
 static int r820t_read_cache_reg(struct r820t_priv *priv, int reg)
@@ -411,17 +413,18 @@ static int r820t_read_cache_reg(struct r820t_priv *priv, 
int reg)
return -EINVAL;
 }
 
-static int r820t_write_reg_mask(struct r820t_priv *priv, u8 reg, u8 val,
+static inline int r820t_write_reg_mask(struct r820t_priv *priv, u8 reg, u8 val,
u8 bit_mask)
 {
+   u8 tmp = val;
int rc = r820t_read_cache_reg(priv, reg);
 
if (rc < 0)
return rc;
 
-   val = (rc & ~bit_mask) | (val & bit_mask);
+   tmp = (rc & ~bit_mask) | (tmp & bit_mask);
 
-   return r820t_write(priv, reg, , 1);
+   return r820t_write(priv, reg, , 1);
 }
 
 static int r820t_read(struct r820t_priv *priv, u8 reg, u8 *val, int len)
-- 
2.9.0

[PATCH] media: ov13858: select V4L2_FWNODE

[Arnd Bergmann]

v4l2_async_register_subdev_sensor_common() is only provided when
CONFIG_V4L2_FWNODE is enabled, otherwise we get a link failure:

drivers/media/i2c/ov13858.o: In function `ov13858_probe':
ov13858.c:(.text+0xf74): undefined reference to 
`v4l2_async_register_subdev_sensor_common'

This adds a Kconfig 'select' statement like all the other users of
this interface have.

Fixes: 2e8a9fbb7950 ("media: ov13858: Add support for flash and lens devices")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
This is the same patch I submitted for et8ek8 earlier. Both
are needed for 4.15.
---
 drivers/media/i2c/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 3c6d6428f525..cb5d7ff82915 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -676,6 +676,7 @@ config VIDEO_OV13858
tristate "OmniVision OV13858 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
---help---
  This is a Video4Linux2 sensor-level driver for the OmniVision
  OV13858 camera.
-- 
2.9.0

[PATCH v2] [media] vivid: use ktime_t for timestamp calculation

[Arnd Bergmann]

timespec is generally deprecated because of the y2038 overflow.
In vivid, the usage is fine, since we are dealing with monotonic
timestamps, but we can also simplify the code by going to ktime_t.

Using ktime_divns() should be roughly as efficient as the old code,
since the constant 64-bit division gets turned into a multiplication
on modern platforms, and we save multiple 32-bit divisions that can be
expensive e.g. on ARMv7.

Tested-by: Hans Verkuil <hans.verk...@cisco.com>
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
v2: fix a small mistake in the use_alternates computation
---
 drivers/media/platform/vivid/vivid-core.c |  2 +-
 drivers/media/platform/vivid/vivid-core.h |  2 +-
 drivers/media/platform/vivid/vivid-radio-rx.c | 11 +--
 drivers/media/platform/vivid/vivid-radio-tx.c |  8 +++-
 drivers/media/platform/vivid/vivid-rds-gen.h  |  1 +
 5 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/vivid/vivid-core.c 
b/drivers/media/platform/vivid/vivid-core.c
index 5f316a5e38db..a091cfd93164 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -995,7 +995,7 @@ static int vivid_create_instance(struct platform_device 
*pdev, int inst)
 
dev->edid_max_blocks = dev->edid_blocks = 2;
memcpy(dev->edid, vivid_hdmi_edid, sizeof(vivid_hdmi_edid));
-   ktime_get_ts(>radio_rds_init_ts);
+   dev->radio_rds_init_time = ktime_get();
 
/* create all controls */
ret = vivid_create_controls(dev, ccs_cap == -1, ccs_out == -1, 
no_error_inj,
diff --git a/drivers/media/platform/vivid/vivid-core.h 
b/drivers/media/platform/vivid/vivid-core.h
index 5cdf95bdc4d1..d8bff4dcefb7 100644
--- a/drivers/media/platform/vivid/vivid-core.h
+++ b/drivers/media/platform/vivid/vivid-core.h
@@ -510,7 +510,7 @@ struct vivid_dev {
 
/* Shared between radio receiver and transmitter */
boolradio_rds_loop;
-   struct timespec radio_rds_init_ts;
+   ktime_t radio_rds_init_time;
 
/* CEC */
struct cec_adapter  *cec_rx_adap;
diff --git a/drivers/media/platform/vivid/vivid-radio-rx.c 
b/drivers/media/platform/vivid/vivid-radio-rx.c
index 47c36c26096b..35fbff490535 100644
--- a/drivers/media/platform/vivid/vivid-radio-rx.c
+++ b/drivers/media/platform/vivid/vivid-radio-rx.c
@@ -38,9 +38,9 @@ ssize_t vivid_radio_rx_read(struct file *file, char __user 
*buf,
 size_t size, loff_t *offset)
 {
struct vivid_dev *dev = video_drvdata(file);
-   struct timespec ts;
struct v4l2_rds_data *data = dev->rds_gen.data;
bool use_alternates;
+   ktime_t timestamp;
unsigned blk;
int perc;
int i;
@@ -64,17 +64,16 @@ ssize_t vivid_radio_rx_read(struct file *file, char __user 
*buf,
}
 
 retry:
-   ktime_get_ts();
-   use_alternates = ts.tv_sec % 10 >= 5;
+   timestamp = ktime_sub(ktime_get(), dev->radio_rds_init_time);
+   blk = ktime_divns(timestamp, VIVID_RDS_NSEC_PER_BLK);
+   use_alternates = (blk % VIVID_RDS_GEN_BLOCKS) & 1;
+
if (dev->radio_rx_rds_last_block == 0 ||
dev->radio_rx_rds_use_alternates != use_alternates) {
dev->radio_rx_rds_use_alternates = use_alternates;
/* Re-init the RDS generator */
vivid_radio_rds_init(dev);
}
-   ts = timespec_sub(ts, dev->radio_rds_init_ts);
-   blk = ts.tv_sec * 100 + ts.tv_nsec / 1000;
-   blk = (blk * VIVID_RDS_GEN_BLOCKS) / 500;
if (blk >= dev->radio_rx_rds_last_block + VIVID_RDS_GEN_BLOCKS)
dev->radio_rx_rds_last_block = blk - VIVID_RDS_GEN_BLOCKS + 1;
 
diff --git a/drivers/media/platform/vivid/vivid-radio-tx.c 
b/drivers/media/platform/vivid/vivid-radio-tx.c
index 0e8025b7b4dd..897b56195ca7 100644
--- a/drivers/media/platform/vivid/vivid-radio-tx.c
+++ b/drivers/media/platform/vivid/vivid-radio-tx.c
@@ -37,7 +37,7 @@ ssize_t vivid_radio_tx_write(struct file *file, const char 
__user *buf,
 {
struct vivid_dev *dev = video_drvdata(file);
struct v4l2_rds_data *data = dev->rds_gen.data;
-   struct timespec ts;
+   ktime_t timestamp;
unsigned blk;
int i;
 
@@ -58,10 +58,8 @@ ssize_t vivid_radio_tx_write(struct file *file, const char 
__user *buf,
dev->radio_tx_rds_owner = file->private_data;
 
 retry:
-   ktime_get_ts();
-   ts = timespec_sub(ts, dev->radio_rds_init_ts);
-   blk = ts.tv_sec * 100 + ts.tv_nsec / 1000;
-   blk = (blk * VIVID_RDS_GEN_BLOCKS) / 500;
+   timestamp = ktime_sub(ktime_get(), dev->radio_rds_init_time);
+   blk = ktime_divns(timestamp, VIVID_RDS_NSEC_PER_BLK);
if (blk - VIVID_RDS_GEN_BLOCKS >= dev->radio_tx_rds_last_block)
dev->r

Re: [PATCH 6/8] [media] vivid: use ktime_t for timestamp calculation

[Arnd Bergmann]

On Mon, Nov 27, 2017 at 4:14 PM, Hans Verkuil  wrote:

>> - ktime_get_ts();
>> - use_alternates = ts.tv_sec % 10 >= 5;
>> + timestamp = ktime_sub(ktime_get(), dev->radio_rds_init_time);
>> + blk = ktime_divns(timestamp, VIVID_RDS_NSEC_PER_BLK);
>> + use_alternates = blk % VIVID_RDS_GEN_BLOCKS;
>> +
>
> Almost right. This last line should be:
>
> use_alternates = (blk / VIVID_RDS_GEN_BLOCKS) & 1;
>
> With that in place it works and you can add my:
>
> Tested-by: Hans Verkuil 

Makes sense. Sending a fixed version now, thanks a lot for testing!

Arnd

[PATCH v2] [media] staging: atomisp: convert timestamps to ktime_t

[Arnd Bergmann]

timespec overflows in 2038 on 32-bit architectures, and the
getnstimeofday() suffers from possible time jumps, so the
timestamps here are better done using ktime_get(), which has
neither of those problems.

In case of ov2680, we don't seem to use the timestamp at
all, so I just remove it.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
v2: use min_t() as suggested by Andy Shevchenko
---
 drivers/staging/media/atomisp/i2c/ov2680.h|  1 -
 .../staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c | 19 ---
 drivers/staging/media/atomisp/i2c/ov5693/ov5693.h |  2 +-
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/ov2680.h 
b/drivers/staging/media/atomisp/i2c/ov2680.h
index bf4897347df7..03f75dd80f87 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.h
+++ b/drivers/staging/media/atomisp/i2c/ov2680.h
@@ -174,7 +174,6 @@ struct ov2680_format {
struct mutex input_lock;
struct v4l2_ctrl_handler ctrl_handler;
struct camera_sensor_platform_data *platform_data;
-   struct timespec timestamp_t_focus_abs;
int vt_pix_clk_freq_mhz;
int fmt_idx;
int run_mode;
diff --git a/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c 
b/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c
index 3e7c3851280f..9fa25bb8f1ee 100644
--- a/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c
+++ b/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c
@@ -973,7 +973,7 @@ static int ov5693_t_focus_abs(struct v4l2_subdev *sd, s32 
value)
if (ret == 0) {
dev->number_of_steps = value - dev->focus;
dev->focus = value;
-   getnstimeofday(&(dev->timestamp_t_focus_abs));
+   dev->timestamp_t_focus_abs = ktime_get();
} else
dev_err(>dev,
"%s: i2c failed. ret %d\n", __func__, ret);
@@ -993,16 +993,13 @@ static int ov5693_q_focus_status(struct v4l2_subdev *sd, 
s32 *value)
 {
u32 status = 0;
struct ov5693_device *dev = to_ov5693_sensor(sd);
-   struct timespec temptime;
-   const struct timespec timedelay = {
-   0,
-   min((u32)abs(dev->number_of_steps) * DELAY_PER_STEP_NS,
-   (u32)DELAY_MAX_PER_STEP_NS),
-   };
-
-   getnstimeofday();
-   temptime = timespec_sub(temptime, (dev->timestamp_t_focus_abs));
-   if (timespec_compare(, ) <= 0) {
+   ktime_t temptime;
+   ktime_t timedelay = ns_to_ktime(min_t(u32,
+   abs(dev->number_of_steps) * DELAY_PER_STEP_NS,
+   DELAY_MAX_PER_STEP_NS));
+
+   temptime = ktime_sub(ktime_get(), (dev->timestamp_t_focus_abs));
+   if (ktime_compare(temptime, timedelay) <= 0) {
status |= ATOMISP_FOCUS_STATUS_MOVING;
status |= ATOMISP_FOCUS_HP_IN_PROGRESS;
} else {
diff --git a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h 
b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h
index 2ea63807c56d..68cfcb4a6c3c 100644
--- a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h
+++ b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h
@@ -221,7 +221,7 @@ struct ov5693_device {
struct v4l2_ctrl_handler ctrl_handler;
 
struct camera_sensor_platform_data *platform_data;
-   struct timespec timestamp_t_focus_abs;
+   ktime_t timestamp_t_focus_abs;
int vt_pix_clk_freq_mhz;
int fmt_idx;
int run_mode;
-- 
2.9.0

Re: [Y2038] [PATCH 7/8] [media] staging: atomisp: convert timestamps to ktime_t

[Arnd Bergmann]

On Mon, Nov 27, 2017 at 4:05 PM, Andy Shevchenko
<andriy.shevche...@linux.intel.com> wrote:
> On Mon, 2017-11-27 at 14:19 +0100, Arnd Bergmann wrote:
>> timespec overflows in 2038 on 32-bit architectures, and the
>> getnstimeofday() suffers from possible time jumps, so the
>> timestamps here are better done using ktime_get(), which has
>> neither of those problems.
>>
>> In case of ov2680, we don't seem to use the timestamp at
>> all, so I just remove it.
>>
>
>> + ktime_t timedelay = ns_to_ktime(
>>   min((u32)abs(dev->number_of_steps) *
>> DELAY_PER_STEP_NS,
>> - (u32)DELAY_MAX_PER_STEP_NS),
>> - };
>> + (u32)DELAY_MAX_PER_STEP_NS));
>
> Since you are touching this, it might make sense to convert to
>
> min_t(u32, ...)
>
> ...and locate lines something like:
>
> ktime_t timeday = ns_to_ktime(min_t(u32,
>  param1,
>  param2));
>
> From my pov will make readability better.

Yes, good idea. Re-sending the patch now. Thanks for taking a look,

  Arnd

[PATCH 8/8] [media] staging: imx: use ktime_t for timestamps

[Arnd Bergmann]

The imx media driver passes around monotonic timestamps in the deprecated
'timespec' format. This is not a problem for the driver, as they won't
overflow, but moving to either timespec64 or ktime_t is preferred.

I'm picking ktime_t for simplicity here. frame_interval_monitor() is
the main function that changes, as it tries to compare a time interval
in microseconds. The algorithm slightly changes here, to avoid 64-bit
division. The code previously assumed that the error was at most 32-bit
worth of microseconds here, so I'm making the same assumption but add
an explicit test for it.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/staging/media/imx/imx-media-csi.c |  8 ++--
 drivers/staging/media/imx/imx-media-fim.c | 30 +-
 drivers/staging/media/imx/imx-media.h |  2 +-
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index bb1d6dafca83..26994b429cf2 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -207,13 +207,9 @@ static irqreturn_t csi_idmac_eof_interrupt(int irq, void 
*dev_id)
goto unlock;
}
 
-   if (priv->fim) {
-   struct timespec cur_ts;
-
-   ktime_get_ts(_ts);
+   if (priv->fim)
/* call frame interval monitor */
-   imx_media_fim_eof_monitor(priv->fim, _ts);
-   }
+   imx_media_fim_eof_monitor(priv->fim, ktime_get());
 
csi_vb2_buf_done(priv);
 
diff --git a/drivers/staging/media/imx/imx-media-fim.c 
b/drivers/staging/media/imx/imx-media-fim.c
index 47275ef803f3..6df189135db8 100644
--- a/drivers/staging/media/imx/imx-media-fim.c
+++ b/drivers/staging/media/imx/imx-media-fim.c
@@ -66,7 +66,7 @@ struct imx_media_fim {
int   icap_flags;
 
int   counter;
-   struct timespec   last_ts;
+   ktime_t   last_ts;
unsigned long sum;   /* usec */
unsigned long nominal;   /* usec */
 
@@ -147,22 +147,26 @@ static void send_fim_event(struct imx_media_fim *fim, 
unsigned long error)
  * (presumably random) interrupt latency.
  */
 static void frame_interval_monitor(struct imx_media_fim *fim,
-  struct timespec *ts)
+  ktime_t timestamp)
 {
-   unsigned long interval, error, error_avg;
+   long long interval, error;
+   unsigned long error_avg;
bool send_event = false;
-   struct timespec diff;
 
if (!fim->enabled || ++fim->counter <= 0)
goto out_update_ts;
 
-   diff = timespec_sub(*ts, fim->last_ts);
-   interval = diff.tv_sec * 1000 * 1000 + diff.tv_nsec / 1000;
-   error = abs(interval - fim->nominal);
+   /* max error is less than l00µs, so use 32-bit division or fail */
+   interval = ktime_to_ns(ktime_sub(timestamp, fim->last_ts));
+   error = abs(interval - NSEC_PER_USEC * (u64)fim->nominal);
+   if (error > U32_MAX)
+   error = U32_MAX;
+   else
+   error = abs((u32)error / NSEC_PER_USEC);
 
if (fim->tolerance_max && error >= fim->tolerance_max) {
dev_dbg(fim->sd->dev,
-   "FIM: %lu ignored, out of tolerance bounds\n",
+   "FIM: %llu ignored, out of tolerance bounds\n",
error);
fim->counter--;
goto out_update_ts;
@@ -184,7 +188,7 @@ static void frame_interval_monitor(struct imx_media_fim 
*fim,
}
 
 out_update_ts:
-   fim->last_ts = *ts;
+   fim->last_ts = timestamp;
if (send_event)
send_fim_event(fim, error_avg);
 }
@@ -195,14 +199,14 @@ static void frame_interval_monitor(struct imx_media_fim 
*fim,
  * to interrupt latency.
  */
 static void fim_input_capture_handler(int channel, void *dev_id,
- struct timespec *ts)
+ ktime_t timestamp)
 {
struct imx_media_fim *fim = dev_id;
unsigned long flags;
 
spin_lock_irqsave(>lock, flags);
 
-   frame_interval_monitor(fim, ts);
+   frame_interval_monitor(fim, timestamp);
 
if (!completion_done(>icap_first_event))
complete(>icap_first_event);
@@ -405,14 +409,14 @@ static int init_fim_controls(struct imx_media_fim *fim)
  * the frame_interval_monitor() is called by the input capture event
  * callback handler in that case.
  */
-void imx_media_fim_eof_monitor(struct imx_media_fim *fim, struct timespec *ts)
+void imx_media_fim_eof_monitor(struct imx_media_fim *fim, ktime_t timestamp)
 {
unsigned long flags;
 
spin_lock_irqsave(>lock, flags);
 
if (!icap_enabled(fi