cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Jun 27 05:00:15 CEST 2018 media-tree git hash:f2809d20b9250c675fca8268a0f6274277cca7ff media_build git hash: 26d102795c91f8593a4f74f96b955f9a8b81dbc3 v4l-utils git hash: 5c197a3bbe7358670765d09f67ae2f05e89a61d1 gcc version:i686-linux-gcc (GCC) 8.1.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.16.0-1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK Check COMPILE_TEST: OK linux-2.6.36.4-i686: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-i686: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-i686: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-i686: OK linux-2.6.39.4-x86_64: OK linux-3.0.101-i686: OK linux-3.0.101-x86_64: OK linux-3.1.10-i686: OK linux-3.1.10-x86_64: OK linux-3.2.101-i686: OK linux-3.2.101-x86_64: OK linux-3.3.8-i686: OK linux-3.3.8-x86_64: OK linux-3.4.113-i686: OK linux-3.4.113-x86_64: OK linux-3.5.7-i686: OK linux-3.5.7-x86_64: OK linux-3.6.11-i686: OK linux-3.6.11-x86_64: OK linux-3.7.10-i686: OK linux-3.7.10-x86_64: OK linux-3.8.13-i686: OK linux-3.8.13-x86_64: OK linux-3.9.11-i686: OK linux-3.9.11-x86_64: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.56-i686: OK linux-3.16.56-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.102-i686: OK linux-3.18.102-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.51-i686: OK linux-4.1.51-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.109-i686: OK linux-4.4.109-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.91-i686: OK linux-4.9.91-x86_64: OK linux-4.10.17-i686: OK linux-4.10.17-x86_64: OK linux-4.11.12-i686: OK linux-4.11.12-x86_64: OK linux-4.12.14-i686: OK linux-4.12.14-x86_64: OK linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.42-i686: OK linux-4.14.42-x86_64: OK linux-4.15.14-i686: OK linux-4.15.14-x86_64: OK linux-4.16.8-i686: OK linux-4.16.8-x86_64: OK linux-4.17.2-i686: OK linux-4.17.2-x86_64: OK linux-4.18-rc1-i686: ERRORS linux-4.18-rc1-x86_64: ERRORS apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH v4] media: add imx319 camera sensor driver
On 06/26/2018 11:15 PM, Sakari Ailus wrote:
> Hi Bingbu,
>
> On Thu, May 31, 2018 at 06:19:24PM +0800, bingbu@intel.com wrote:
>> From: Bingbu Cao
>>
>> Add a v4l2 sub-device driver for the Sony imx319 image sensor.
>> This is a camera sensor using the i2c bus for control and the
>> csi-2 bus for data.
>>
>> This driver supports following features:
>> - manual exposure and analog/digital gain control support
>> - vblank/hblank control support
>> - 4 test patterns control support
>> - vflip/hflip control support (will impact the output bayer order)
>> - support following resolutions:
>> - 3264x2448, 3280x2464 @ 30fps
>> - 1936x1096, 1920x1080 @ 60fps
>> - 1640x1232, 1640x922, 1296x736, 1280x720 @ 120fps
>> - support 4 bayer orders output (via change v/hflip)
>> - SRGGB10(default), SGRBG10, SGBRG10, SBGGR10
>>
>> Signed-off-by: Bingbu Cao
>> Signed-off-by: Tianshu Qiu
> Could you obtain the CSI-2 bus speed as well as the external clock
> frequency from the firmware? See e.g.
> drivers/media/i2c/smiapp/smiapp-core.c and
> v4l2_fwnode_endpoint_alloc_parse() there. You could use the clock-frequency
> property for the clock.
Ack.
> ...
>
>> +static void imx319_free_controls(struct imx319 *imx319)
>> +{
>> +v4l2_ctrl_handler_free(imx319->sd.ctrl_handler);
>> +}
> Please use v4l2_ctrl_handler_free() directly instead, and remove this
> function.
>
> Both apply to the imx355 driver as well.
Ack.
>
Goodday
I am Ms.Smadar Barber-Tsadik, I am the Chief Executive Officer with FIRST INTERNATIONAL BANK OF ISRAEL LTD (FIBI). I am getting in touch with you regarding an extremely important and urgent matter. If you would oblige me the opportunity, I shall provide you with details upon your response, please you are to contact me on my private email: [ mailto:barbertsa...@hotmail.com | barbertsa...@hotmail.com ] Faithfully, Smadar Barber-Tsadik
Re: V4L2_CID_USER_MAX217X_BASE == V4L2_CID_USER_IMX_BASE
Hello Helmut,
On 06/22/2018 12:51 AM, Helmut Grohne wrote:
Hi,
I found it strange that the macros V4L2_CID_USER_MAX217X_BASE and
V4L2_CID_USER_IMX_BASE have equal value even though each of them state
that they reserved a range. Those reservations look conflicting to me.
Yes, they conflict.
The macro V4L2_CID_USER_MAX217X_BASE came first,
No, imx came first. e1302912 ("media: Add i.MX media core driver")
is dated June 10, 2017. 8d67ae25 ("media: v4l2-ctrls: Reserve controls for
MAX217X") is dated two days later.
and
V4L2_CID_USER_IMX_BASE was introduced in e130291212df ("media: Add i.MX
media core driver") with the conflicting assignment (not a merge error).
The authors of that patch mostly make up the recipient list.
There were 8 revisions of the imx-media driver posted. In all of
those postings, V4L2_CID_USER_MAX217X_BASE did not exist yet.
So it looks like 8d67ae25 was merged at the same time as e1302912
but the conflict went unnoticed.
Steve
Re: [PATCH v3 05/13] media: v4l2-fwnode: Add a convenience function for registering subdevs with notifiers
On 06/26/2018 12:45 AM, Sakari Ailus wrote:
On Tue, May 08, 2018 at 08:55:04PM -0700, Steve Longerbeam wrote:
On 05/08/2018 03:28 AM, Sakari Ailus wrote:
Hi Steve,
Again, sorry about the delay. This thread got buried in my inbox. :-(
Please see my reply below.
On Mon, Apr 23, 2018 at 11:00:22AM -0700, Steve Longerbeam wrote:
On 04/23/2018 12:14 AM, Sakari Ailus wrote:
Hi Steve,
On Tue, Mar 20, 2018 at 05:37:21PM -0700, Steve Longerbeam wrote:
Adds v4l2_async_register_fwnode_subdev(), which is a convenience function
for parsing a sub-device's fwnode port endpoints for connected remote
sub-devices, registering a sub-device notifier, and then registering
the sub-device itself.
Signed-off-by: Steve Longerbeam
---
Changes since v2:
- fix error-out path in v4l2_async_register_fwnode_subdev() that forgot
to put device.
Changes since v1:
- add #include to v4l2-fwnode.h for
'struct v4l2_subdev' declaration.
---
drivers/media/v4l2-core/v4l2-fwnode.c | 101
++
include/media/v4l2-fwnode.h | 43 +++
2 files changed, 144 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
b/drivers/media/v4l2-core/v4l2-fwnode.c
index 99198b9..d42024d 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -880,6 +880,107 @@ int v4l2_async_register_subdev_sensor_common(struct
v4l2_subdev *sd)
}
EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor_common);
+int v4l2_async_register_fwnode_subdev(
+ struct v4l2_subdev *sd, size_t asd_struct_size,
+ unsigned int *ports, unsigned int num_ports,
+ int (*parse_endpoint)(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd))
+{
+ struct v4l2_async_notifier *notifier;
+ struct device *dev = sd->dev;
+ struct fwnode_handle *fwnode;
+ unsigned int subdev_port;
+ bool is_port;
+ int ret;
+
+ if (WARN_ON(!dev))
+ return -ENODEV;
+
+ fwnode = dev_fwnode(dev);
+ if (!fwnode_device_is_available(fwnode))
+ return -ENODEV;
+
+ is_port = (is_of_node(fwnode) &&
+ of_node_cmp(to_of_node(fwnode)->name, "port") == 0);
What's the intent of this and the code below? You may not parse the graph
data structure here, it should be done in the actual firmware
implementation instead.
The i.MX6 CSI sub-device registers itself from a port fwnode, so
the intent of the is_port code below is to support the i.MX6 CSI.
I can remove the is_port checks, but it means
v4l2_async_register_fwnode_subdev() won't be usable by the CSI
sub-device.
This won't scale.
The vast majority of sub-devices register themselves as
port parent nodes. So for now at least, I think
v4l2_async_register_fwnode_subdev() could be useful to many
platforms.
Instead, I think we'd need to separate registering
sub-devices (through async sub-devices) and binding them with the driver
that registered the notifier. Or at least change how that process works: a
single sub-device can well be bound to multiple notifiers,
Ok, that is certainly not the case now, a sub-device can only
be bound to a single notifier.
or multiple
times to the same notifier while it may be registered only once.
Anyway, this is a future generalization if I understand you
correctly. Not something to deal with here.
Also, sub-devices generally do not match ports.
Yes that's generally true, sub-devices generally match to port parent
nodes. But I do know of one other sub-device that buck that trend.
The ADV748x CSI-2 output sub-devices match against endpoint nodes.
Endpoints, yes, but not ports.
Well, the imx CSI registers from a port node.
I looked at the imx driver and it appears to be parsing DT without much
help from the frameworks; graph or V4L2 fwnode. Is there a reason to do so,
other than it's a staging driver? :-)
That's the whole point of this patchset. It gets rid of the code in imx
that discovers subdevices by recursively walking the graph, replacing
with the subdev notifier framework.
Do you happen to have any DT source (or bindings) for this?
Documentation/devicetree/bindings/media/imx.txt.
For example DT source, it's all in the imx6 reference boards,
see imx6qdl-sabresd.dtsi for example.
It'd help to understand what is the aim here.
The aim of what? Of this specific commit? I'll reprint it here:
Adds v4l2_async_register_fwnode_subdev(), which is a convenience function
for parsing a sub-device's fwnode port endpoints for connected remote
sub-devices, registering a sub-device notifier, and then registering
the sub-device itself.
Steve
Re: [PATCH v3 03/13] media: v4l2: async: Add v4l2_async_notifier_add_subdev
On 06/26/2018 12:12 AM, Sakari Ailus wrote:
On Wed, May 09, 2018 at 04:06:32PM -0700, Steve Longerbeam wrote:
On 05/08/2018 03:12 AM, Sakari Ailus wrote:
On Fri, Apr 20, 2018 at 10:12:33AM -0700, Steve Longerbeam wrote:
Hi Sakari,
On 04/20/2018 05:24 AM, Sakari Ailus wrote:
Hi Steve,
Thanks for the patchset.
On Tue, Mar 20, 2018 at 05:37:19PM -0700, Steve Longerbeam wrote:
v4l2_async_notifier_add_subdev() adds an asd to the notifier. It checks
that the asd's match_type is valid and that no other equivalent asd's
have already been added to this notifier's asd list, or to other
registered notifier's waiting or done lists, and increments num_subdevs.
v4l2_async_notifier_add_subdev() does not make use of the notifier subdevs
array, otherwise it would have to re-allocate the array every time the
function was called. In place of the subdevs array, the function adds
the asd to a new master asd_list. The function will return error with a
WARN() if it is ever called with the subdevs array allocated.
In v4l2_async_notifier_has_async_subdev(), __v4l2_async_notifier_register(),
and v4l2_async_notifier_cleanup(), alternatively operate on the subdevs
array or a non-empty notifier->asd_list.
I do agree with the approach, but this patch leaves the remaining users of
the subdevs array in the notifier intact. Could you rework them to use the
v4l2_async_notifier_add_subdev() as well?
There seem to be just a few of them --- exynos4-is and rcar_drif.
I count more than a few :)
% cd drivers/media && grep -l -r --include "*.[ch]"
'notifier[\.\-]>*subdevs[ ]*='
v4l2-core/v4l2-async.c
platform/pxa_camera.c
platform/ti-vpe/cal.c
platform/exynos4-is/media-dev.c
platform/qcom/camss-8x16/camss.c
platform/soc_camera/soc_camera.c
platform/atmel/atmel-isi.c
platform/atmel/atmel-isc.c
platform/stm32/stm32-dcmi.c
platform/davinci/vpif_capture.c
platform/davinci/vpif_display.c
platform/renesas-ceu.c
platform/am437x/am437x-vpfe.c
platform/xilinx/xilinx-vipp.c
platform/rcar_drif.c
So not including v4l2-core, the list is:
pxa_camera
ti-vpe
exynos4-is
qcom
soc_camera
atmel
stm32
davinci
renesas-ceu
am437x
xilinx
rcar_drif
Given such a large list of the users of the notifier->subdevs array,
I think this should be done is two steps: submit this patchset first,
that keeps the backward compatibility, and then a subsequent patchset
that converts all drivers to use v4l2_async_notifier_add_subdev(), at
which point the subdevs array can be removed from struct
v4l2_async_notifier.
Or, do you still think this should be done all at once? I could add a
large patch to this patchset that does the conversion and removes
the subdevs array.
Sorry for the delay. I grepped for this, too, but I guess I ended up using
an expression that missed most of the users. :-(
I think it'd be very good to perform that conversion --- the code in the
framework would be quite a bit cleaner and easier to maintain whereas the
per-driver conversions seem pretty simple, such as this on in
drivers/media/platform/atmel/atmel-isi.c :
/* Register the subdevices notifier. */
subdevs = devm_kzalloc(isi->dev, sizeof(*subdevs), GFP_KERNEL);
if (!subdevs) {
of_node_put(isi->entity.node);
return -ENOMEM;
}
subdevs[0] = >entity.asd;
isi->notifier.subdevs = subdevs;
isi->notifier.num_subdevs = 1;
isi->notifier.ops = _graph_notify_ops;
Yes, the conversions are fairly straightforward. I've completed that work,
but it was a very manual conversion, every platform is different and needed
careful study.
Although I was careful about getting the error-out paths correct, there
could
be mistakes there, which would result in memory leaks. And obviously I can't
re-test all these platforms. So this patch is very high-risk. More eyes are
needed on it, ideally the maintainer(s) of each affected platform.
I just submitted v4 of this series, but did not include this large un-tested
patch in v4 for those reasons.
Instead, this patch, and follow-up patches that strips support for subdevs
array altogether from v4l2-async.c, and updates rst docs, are available at
my
media-tree mirror on github:
g...@github.com:slongerbeam/mediatree.git
in the branch 'remove-subdevs-array'. The branch is based off this series
(branch 'imx-subdev-notifiers.6').
Would you be able to post these to the list? I'd really like this being
done as part of the related patchset, rather than leaving the mess in the
framework.
Backward compatibility can look messy.
I can include the patch that converts all platforms at once. But as I
said it is completely untested.
So I'm curious, what is the policy in V4L2 community regarding
merging untested patches? Do we go ahead and merge and then
fixup errors as they are discovered, or should the patch get basic
validation by everyone who has access to the affected hardware
first?
Steve
Re: [PATCH] media: em28xx: fix a regression with HVR-950
On Tue, Jun 26, 2018 at 4:09 PM, Brad Love wrote: > Hi Mauro, > > It turns out this patch breaks DualHD multiple tuner capability. When > alt mode is set in start_streaming it immediately kills the other tuners > stream. Essentially both tuners cannot be used together when this is > applied. I unfortunately don't have a HVR-950 to try and fix the > original regression better. Can you please take another look at this? > Until this is sorted, DualHD are effectively broken. Yeah, because the Empia device uses the same USB Interface for both endpoints, you need to have a reference count and only change the alternate when the first endpoint is put into use and then only reset the alternate back to zero when neither endpoint is in use. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com
Re: [PATCH] media: em28xx: fix a regression with HVR-950
Hi Mauro,
It turns out this patch breaks DualHD multiple tuner capability. When
alt mode is set in start_streaming it immediately kills the other tuners
stream. Essentially both tuners cannot be used together when this is
applied. I unfortunately don't have a HVR-950 to try and fix the
original regression better. Can you please take another look at this?
Until this is sorted, DualHD are effectively broken.
Cheers,
Brad
On 2018-03-09 06:21, Mauro Carvalho Chehab wrote:
> Changeset be7fd3c3a8c5 ("media: em28xx: Hauppauge DualHD second tuner
> functionality") removed the logic with sets the alternate for the DVB
> device. Without setting the right alternate, the device won't be
> able to submit URBs, and userspace fails with -EMSGSIZE:
>
> ERROR DMX_SET_PES_FILTER failed (PID = 0x2000): 90 Message too long
>
> Tested with Hauppauge HVR-950 model A1C0.
>
> Cc: Brad Love
> Fixes: be7fd3c3a8c5 ("media: em28xx: Hauppauge DualHD second tuner
> functionality")
> Signed-off-by: Mauro Carvalho Chehab
> ---
> drivers/media/usb/em28xx/em28xx-dvb.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c
> b/drivers/media/usb/em28xx/em28xx-dvb.c
> index a54cb8dc52c9..2ce7de1c7cce 100644
> --- a/drivers/media/usb/em28xx/em28xx-dvb.c
> +++ b/drivers/media/usb/em28xx/em28xx-dvb.c
> @@ -199,6 +199,7 @@ static int em28xx_start_streaming(struct em28xx_dvb *dvb)
> int rc;
> struct em28xx_i2c_bus *i2c_bus = dvb->adapter.priv;
> struct em28xx *dev = i2c_bus->dev;
> + struct usb_device *udev = interface_to_usbdev(dev->intf);
> int dvb_max_packet_size, packet_multiplier, dvb_alt;
>
> if (dev->dvb_xfer_bulk) {
> @@ -217,6 +218,7 @@ static int em28xx_start_streaming(struct em28xx_dvb *dvb)
> dvb_alt = dev->dvb_alt_isoc;
> }
>
> + usb_set_interface(udev, dev->ifnum, dvb_alt);
> rc = em28xx_set_mode(dev, EM28XX_DIGITAL_MODE);
> if (rc < 0)
> return rc;
Re: [1/3] media: rc: drivers should produce alternate pulse and space timing events
On Tue, 2018-06-26 at 17:33 +0100, Sean Young wrote:
> On Tue, Jun 26, 2018 at 04:39:51PM +0200, Jerome Brunet wrote:
> > On Tue, 2018-06-26 at 15:37 +0100, Sean Young wrote:
> > > On Tue, Jun 19, 2018 at 04:09:20PM +0200, Jerome Brunet wrote:
> > > > On Tue, 2018-06-19 at 13:57 +0100, Sean Young wrote:
> > > > > On Tue, Jun 19, 2018 at 02:08:12PM +0200, Jerome Brunet wrote:
> > > > > > On Sat, 2018-05-12 at 11:55 +0100, Sean Young wrote:
> > > > > > > Report an error if this is not the case or any problem with the
> > > > > > > generated
> > > > > > > raw events.
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > Since the inclusion of this patch, every 3 to 15 seconds, I get the
> > > > > > following
> > > > > > message:
> > > > > >
> > > > > > "rc rc0: two consecutive events of type space"
> > > > > >
> > > > > > on the console of amlogic s400 platform
> > > > > > (arch/arm64/boot/dts/amlogic/meson-axg-
> > > > > > s400.dts). I don't know much about ir protocol and surely there is
> > > > > > something
> > > > > > worth investigating in the related driver, but ...
> > > > > >
> > > > > > >
> > > > > > > Signed-off-by: Sean Young
> > > > > > > ---
> > > > > > > drivers/media/rc/rc-ir-raw.c | 19 +++
> > > > > > > 1 file changed, 15 insertions(+), 4 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/media/rc/rc-ir-raw.c
> > > > > > > b/drivers/media/rc/rc-ir-raw.c
> > > > > > > index 2e50104ae138..49c56da9bc67 100644
> > > > > > > --- a/drivers/media/rc/rc-ir-raw.c
> > > > > > > +++ b/drivers/media/rc/rc-ir-raw.c
> > > > > > > @@ -22,16 +22,27 @@ static int ir_raw_event_thread(void *data)
> > > > > > > {
> > > > > > > struct ir_raw_event ev;
> > > > > > > struct ir_raw_handler *handler;
> > > > > > > - struct ir_raw_event_ctrl *raw = (struct ir_raw_event_ctrl
> > > > > > > *)data;
> > > > > > > + struct ir_raw_event_ctrl *raw = data;
> > > > > > > + struct rc_dev *dev = raw->dev;
> > > > > > >
> > > > > > > while (1) {
> > > > > > > mutex_lock(_raw_handler_lock);
> > > > > > > while (kfifo_out(>kfifo, , 1)) {
> > > > > > > + if (is_timing_event(ev)) {
> > > > > > > + if (ev.duration == 0)
> > > > > > > + dev_err(>dev, "nonsensical
> > > > > > > timing event of duration 0");
> > > > > > > + if (is_timing_event(raw->prev_ev) &&
> > > > > > > + !is_transition(, >prev_ev))
> > > > > > > + dev_err(>dev, "two
> > > > > > > consecutive events of type %s",
> > > > > > > + TO_STR(ev.pulse));
> > > > > > > + if (raw->prev_ev.reset && ev.pulse == 0)
> > > > > > > + dev_err(>dev, "timing
> > > > > > > event after reset should be pulse");
> > > > > > > + }
> > > > > >
> > > > > > ... considering that we continue the processing as if nothing
> > > > > > happened, is it
> > > > > > really an error ?
> > > > > >
> > > > > > Could we consider something less invasive ? like dev_dbg() or
> > > > > > dev_warn_once() ?
> > > > >
> > > > > Maybe it should be dev_warn(). The fact that it is not
> > > > > dev_warn_once() means
> > > > > that we now know this occurs regularly.
> > > >
> > > > It seems weird to report this over and over again.
> > > >
> > > > >
> > > > > Would you mind testing the following patch please?
> > > > >
> > > > > Thanks
> > > > >
> > > > > Sean
> > > > >
> > > > > From 6a44fbe4738d230b9cf378777e7e9a93e5fda726 Mon Sep 17 00:00:00 2001
> > > > > From: Sean Young
> > > > > Date: Tue, 19 Jun 2018 13:50:36 +0100
> > > > > Subject: [PATCH] media: rc: meson: rc rc0: two consecutive events of
> > > > > type
> > > > > space
> > > > >
> > > > > The meson generates one edge per interrupt. The duration is encoded
> > > > > in 12
> > > > > bits of 10 microseconds, so it can only encoding a maximum of 40
> > > > > milliseconds. As a result, it can produce multiple space events.
> > > > >
> > > > > Signed-off-by: Sean Young
> > > > > ---
> > > > > drivers/media/rc/meson-ir.c | 3 ++-
> > > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
> > > > > index f449b35d25e7..9747426719b2 100644
> > > > > --- a/drivers/media/rc/meson-ir.c
> > > > > +++ b/drivers/media/rc/meson-ir.c
> > > > > @@ -97,7 +97,8 @@ static irqreturn_t meson_ir_irq(int irqno, void
> > > > > *dev_id)
> > > > > status = readl_relaxed(ir->reg + IR_DEC_STATUS);
> > > > > rawir.pulse = !!(status & STATUS_IR_DEC_IN);
> > > > >
> > > > > - ir_raw_event_store_with_timeout(ir->rc, );
> > > > > + if (ir_raw_event_store_with_filter(ir->rc, ))
> > > > > + ir_raw_event_handle(ir->rc);
> > > > >
> > > > > spin_unlock(>lock);
> > > > >
> > > >
> > > > Solve the problem
Re: [1/3] media: rc: drivers should produce alternate pulse and space timing events
On Tue, Jun 26, 2018 at 04:39:51PM +0200, Jerome Brunet wrote:
> On Tue, 2018-06-26 at 15:37 +0100, Sean Young wrote:
> > On Tue, Jun 19, 2018 at 04:09:20PM +0200, Jerome Brunet wrote:
> > > On Tue, 2018-06-19 at 13:57 +0100, Sean Young wrote:
> > > > On Tue, Jun 19, 2018 at 02:08:12PM +0200, Jerome Brunet wrote:
> > > > > On Sat, 2018-05-12 at 11:55 +0100, Sean Young wrote:
> > > > > > Report an error if this is not the case or any problem with the
> > > > > > generated
> > > > > > raw events.
> > > > >
> > > > > Hi,
> > > > >
> > > > > Since the inclusion of this patch, every 3 to 15 seconds, I get the
> > > > > following
> > > > > message:
> > > > >
> > > > > "rc rc0: two consecutive events of type space"
> > > > >
> > > > > on the console of amlogic s400 platform
> > > > > (arch/arm64/boot/dts/amlogic/meson-axg-
> > > > > s400.dts). I don't know much about ir protocol and surely there is
> > > > > something
> > > > > worth investigating in the related driver, but ...
> > > > >
> > > > > >
> > > > > > Signed-off-by: Sean Young
> > > > > > ---
> > > > > > drivers/media/rc/rc-ir-raw.c | 19 +++
> > > > > > 1 file changed, 15 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/media/rc/rc-ir-raw.c
> > > > > > b/drivers/media/rc/rc-ir-raw.c
> > > > > > index 2e50104ae138..49c56da9bc67 100644
> > > > > > --- a/drivers/media/rc/rc-ir-raw.c
> > > > > > +++ b/drivers/media/rc/rc-ir-raw.c
> > > > > > @@ -22,16 +22,27 @@ static int ir_raw_event_thread(void *data)
> > > > > > {
> > > > > > struct ir_raw_event ev;
> > > > > > struct ir_raw_handler *handler;
> > > > > > - struct ir_raw_event_ctrl *raw = (struct ir_raw_event_ctrl
> > > > > > *)data;
> > > > > > + struct ir_raw_event_ctrl *raw = data;
> > > > > > + struct rc_dev *dev = raw->dev;
> > > > > >
> > > > > > while (1) {
> > > > > > mutex_lock(_raw_handler_lock);
> > > > > > while (kfifo_out(>kfifo, , 1)) {
> > > > > > + if (is_timing_event(ev)) {
> > > > > > + if (ev.duration == 0)
> > > > > > + dev_err(>dev, "nonsensical
> > > > > > timing event of duration 0");
> > > > > > + if (is_timing_event(raw->prev_ev) &&
> > > > > > + !is_transition(, >prev_ev))
> > > > > > + dev_err(>dev, "two
> > > > > > consecutive events of type %s",
> > > > > > + TO_STR(ev.pulse));
> > > > > > + if (raw->prev_ev.reset && ev.pulse == 0)
> > > > > > + dev_err(>dev, "timing
> > > > > > event after reset should be pulse");
> > > > > > + }
> > > > >
> > > > > ... considering that we continue the processing as if nothing
> > > > > happened, is it
> > > > > really an error ?
> > > > >
> > > > > Could we consider something less invasive ? like dev_dbg() or
> > > > > dev_warn_once() ?
> > > >
> > > > Maybe it should be dev_warn(). The fact that it is not dev_warn_once()
> > > > means
> > > > that we now know this occurs regularly.
> > >
> > > It seems weird to report this over and over again.
> > >
> > > >
> > > > Would you mind testing the following patch please?
> > > >
> > > > Thanks
> > > >
> > > > Sean
> > > >
> > > > From 6a44fbe4738d230b9cf378777e7e9a93e5fda726 Mon Sep 17 00:00:00 2001
> > > > From: Sean Young
> > > > Date: Tue, 19 Jun 2018 13:50:36 +0100
> > > > Subject: [PATCH] media: rc: meson: rc rc0: two consecutive events of
> > > > type
> > > > space
> > > >
> > > > The meson generates one edge per interrupt. The duration is encoded in
> > > > 12
> > > > bits of 10 microseconds, so it can only encoding a maximum of 40
> > > > milliseconds. As a result, it can produce multiple space events.
> > > >
> > > > Signed-off-by: Sean Young
> > > > ---
> > > > drivers/media/rc/meson-ir.c | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
> > > > index f449b35d25e7..9747426719b2 100644
> > > > --- a/drivers/media/rc/meson-ir.c
> > > > +++ b/drivers/media/rc/meson-ir.c
> > > > @@ -97,7 +97,8 @@ static irqreturn_t meson_ir_irq(int irqno, void
> > > > *dev_id)
> > > > status = readl_relaxed(ir->reg + IR_DEC_STATUS);
> > > > rawir.pulse = !!(status & STATUS_IR_DEC_IN);
> > > >
> > > > - ir_raw_event_store_with_timeout(ir->rc, );
> > > > + if (ir_raw_event_store_with_filter(ir->rc, ))
> > > > + ir_raw_event_handle(ir->rc);
> > > >
> > > > spin_unlock(>lock);
> > > >
> > >
> > > Solve the problem on meson. Thx
> > > Feel free to add this submitting the patch
> >
> > Actually this patch is broken. ir_raw_event_store_with_timeout() generates
> > IR timeouts, but ir_raw_event_store_with_filter() does not.
> >
> > So this will
Re: [PATCH v4] media: add imx319 camera sensor driver
Hi Bingbu,
On Thu, May 31, 2018 at 06:19:24PM +0800, bingbu@intel.com wrote:
> From: Bingbu Cao
>
> Add a v4l2 sub-device driver for the Sony imx319 image sensor.
> This is a camera sensor using the i2c bus for control and the
> csi-2 bus for data.
>
> This driver supports following features:
> - manual exposure and analog/digital gain control support
> - vblank/hblank control support
> - 4 test patterns control support
> - vflip/hflip control support (will impact the output bayer order)
> - support following resolutions:
> - 3264x2448, 3280x2464 @ 30fps
> - 1936x1096, 1920x1080 @ 60fps
> - 1640x1232, 1640x922, 1296x736, 1280x720 @ 120fps
> - support 4 bayer orders output (via change v/hflip)
> - SRGGB10(default), SGRBG10, SGBRG10, SBGGR10
>
> Signed-off-by: Bingbu Cao
> Signed-off-by: Tianshu Qiu
Could you obtain the CSI-2 bus speed as well as the external clock
frequency from the firmware? See e.g.
drivers/media/i2c/smiapp/smiapp-core.c and
v4l2_fwnode_endpoint_alloc_parse() there. You could use the clock-frequency
property for the clock.
...
> +static void imx319_free_controls(struct imx319 *imx319)
> +{
> + v4l2_ctrl_handler_free(imx319->sd.ctrl_handler);
> +}
Please use v4l2_ctrl_handler_free() directly instead, and remove this
function.
Both apply to the imx355 driver as well.
--
Kind regards,
Sakari Ailus
sakari.ai...@linux.intel.com
Re: [1/3] media: rc: drivers should produce alternate pulse and space timing events
On Tue, 2018-06-26 at 15:37 +0100, Sean Young wrote:
> On Tue, Jun 19, 2018 at 04:09:20PM +0200, Jerome Brunet wrote:
> > On Tue, 2018-06-19 at 13:57 +0100, Sean Young wrote:
> > > On Tue, Jun 19, 2018 at 02:08:12PM +0200, Jerome Brunet wrote:
> > > > On Sat, 2018-05-12 at 11:55 +0100, Sean Young wrote:
> > > > > Report an error if this is not the case or any problem with the
> > > > > generated
> > > > > raw events.
> > > >
> > > > Hi,
> > > >
> > > > Since the inclusion of this patch, every 3 to 15 seconds, I get the
> > > > following
> > > > message:
> > > >
> > > > "rc rc0: two consecutive events of type space"
> > > >
> > > > on the console of amlogic s400 platform
> > > > (arch/arm64/boot/dts/amlogic/meson-axg-
> > > > s400.dts). I don't know much about ir protocol and surely there is
> > > > something
> > > > worth investigating in the related driver, but ...
> > > >
> > > > >
> > > > > Signed-off-by: Sean Young
> > > > > ---
> > > > > drivers/media/rc/rc-ir-raw.c | 19 +++
> > > > > 1 file changed, 15 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/media/rc/rc-ir-raw.c
> > > > > b/drivers/media/rc/rc-ir-raw.c
> > > > > index 2e50104ae138..49c56da9bc67 100644
> > > > > --- a/drivers/media/rc/rc-ir-raw.c
> > > > > +++ b/drivers/media/rc/rc-ir-raw.c
> > > > > @@ -22,16 +22,27 @@ static int ir_raw_event_thread(void *data)
> > > > > {
> > > > > struct ir_raw_event ev;
> > > > > struct ir_raw_handler *handler;
> > > > > - struct ir_raw_event_ctrl *raw = (struct ir_raw_event_ctrl
> > > > > *)data;
> > > > > + struct ir_raw_event_ctrl *raw = data;
> > > > > + struct rc_dev *dev = raw->dev;
> > > > >
> > > > > while (1) {
> > > > > mutex_lock(_raw_handler_lock);
> > > > > while (kfifo_out(>kfifo, , 1)) {
> > > > > + if (is_timing_event(ev)) {
> > > > > + if (ev.duration == 0)
> > > > > + dev_err(>dev, "nonsensical
> > > > > timing event of duration 0");
> > > > > + if (is_timing_event(raw->prev_ev) &&
> > > > > + !is_transition(, >prev_ev))
> > > > > + dev_err(>dev, "two
> > > > > consecutive events of type %s",
> > > > > + TO_STR(ev.pulse));
> > > > > + if (raw->prev_ev.reset && ev.pulse == 0)
> > > > > + dev_err(>dev, "timing
> > > > > event after reset should be pulse");
> > > > > + }
> > > >
> > > > ... considering that we continue the processing as if nothing happened,
> > > > is it
> > > > really an error ?
> > > >
> > > > Could we consider something less invasive ? like dev_dbg() or
> > > > dev_warn_once() ?
> > >
> > > Maybe it should be dev_warn(). The fact that it is not dev_warn_once()
> > > means
> > > that we now know this occurs regularly.
> >
> > It seems weird to report this over and over again.
> >
> > >
> > > Would you mind testing the following patch please?
> > >
> > > Thanks
> > >
> > > Sean
> > >
> > > From 6a44fbe4738d230b9cf378777e7e9a93e5fda726 Mon Sep 17 00:00:00 2001
> > > From: Sean Young
> > > Date: Tue, 19 Jun 2018 13:50:36 +0100
> > > Subject: [PATCH] media: rc: meson: rc rc0: two consecutive events of type
> > > space
> > >
> > > The meson generates one edge per interrupt. The duration is encoded in 12
> > > bits of 10 microseconds, so it can only encoding a maximum of 40
> > > milliseconds. As a result, it can produce multiple space events.
> > >
> > > Signed-off-by: Sean Young
> > > ---
> > > drivers/media/rc/meson-ir.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
> > > index f449b35d25e7..9747426719b2 100644
> > > --- a/drivers/media/rc/meson-ir.c
> > > +++ b/drivers/media/rc/meson-ir.c
> > > @@ -97,7 +97,8 @@ static irqreturn_t meson_ir_irq(int irqno, void *dev_id)
> > > status = readl_relaxed(ir->reg + IR_DEC_STATUS);
> > > rawir.pulse = !!(status & STATUS_IR_DEC_IN);
> > >
> > > - ir_raw_event_store_with_timeout(ir->rc, );
> > > + if (ir_raw_event_store_with_filter(ir->rc, ))
> > > + ir_raw_event_handle(ir->rc);
> > >
> > > spin_unlock(>lock);
> > >
> >
> > Solve the problem on meson. Thx
> > Feel free to add this submitting the patch
>
> Actually this patch is broken. ir_raw_event_store_with_timeout() generates
> IR timeouts, but ir_raw_event_store_with_filter() does not.
>
> So this will need some rethinking. I think we need a
> ir_raw_event_store_with_timeout_with_filter() but that is getting silly now.
>
> Maybe filter should be a boolean property of an rc device and happen
> transparently. I'll write something like that when I get some time.
>
would you still object to use dev_warn_once() in the
Re: [1/3] media: rc: drivers should produce alternate pulse and space timing events
On Tue, Jun 19, 2018 at 04:09:20PM +0200, Jerome Brunet wrote:
> On Tue, 2018-06-19 at 13:57 +0100, Sean Young wrote:
> > On Tue, Jun 19, 2018 at 02:08:12PM +0200, Jerome Brunet wrote:
> > > On Sat, 2018-05-12 at 11:55 +0100, Sean Young wrote:
> > > > Report an error if this is not the case or any problem with the
> > > > generated
> > > > raw events.
> > >
> > > Hi,
> > >
> > > Since the inclusion of this patch, every 3 to 15 seconds, I get the
> > > following
> > > message:
> > >
> > > "rc rc0: two consecutive events of type space"
> > >
> > > on the console of amlogic s400 platform
> > > (arch/arm64/boot/dts/amlogic/meson-axg-
> > > s400.dts). I don't know much about ir protocol and surely there is
> > > something
> > > worth investigating in the related driver, but ...
> > >
> > > >
> > > > Signed-off-by: Sean Young
> > > > ---
> > > > drivers/media/rc/rc-ir-raw.c | 19 +++
> > > > 1 file changed, 15 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
> > > > index 2e50104ae138..49c56da9bc67 100644
> > > > --- a/drivers/media/rc/rc-ir-raw.c
> > > > +++ b/drivers/media/rc/rc-ir-raw.c
> > > > @@ -22,16 +22,27 @@ static int ir_raw_event_thread(void *data)
> > > > {
> > > > struct ir_raw_event ev;
> > > > struct ir_raw_handler *handler;
> > > > - struct ir_raw_event_ctrl *raw = (struct ir_raw_event_ctrl
> > > > *)data;
> > > > + struct ir_raw_event_ctrl *raw = data;
> > > > + struct rc_dev *dev = raw->dev;
> > > >
> > > > while (1) {
> > > > mutex_lock(_raw_handler_lock);
> > > > while (kfifo_out(>kfifo, , 1)) {
> > > > + if (is_timing_event(ev)) {
> > > > + if (ev.duration == 0)
> > > > + dev_err(>dev, "nonsensical
> > > > timing event of duration 0");
> > > > + if (is_timing_event(raw->prev_ev) &&
> > > > + !is_transition(, >prev_ev))
> > > > + dev_err(>dev, "two
> > > > consecutive events of type %s",
> > > > + TO_STR(ev.pulse));
> > > > + if (raw->prev_ev.reset && ev.pulse == 0)
> > > > + dev_err(>dev, "timing
> > > > event after reset should be pulse");
> > > > + }
> > >
> > > ... considering that we continue the processing as if nothing happened,
> > > is it
> > > really an error ?
> > >
> > > Could we consider something less invasive ? like dev_dbg() or
> > > dev_warn_once() ?
> >
> > Maybe it should be dev_warn(). The fact that it is not dev_warn_once() means
> > that we now know this occurs regularly.
>
> It seems weird to report this over and over again.
>
> >
> > Would you mind testing the following patch please?
> >
> > Thanks
> >
> > Sean
> >
> > From 6a44fbe4738d230b9cf378777e7e9a93e5fda726 Mon Sep 17 00:00:00 2001
> > From: Sean Young
> > Date: Tue, 19 Jun 2018 13:50:36 +0100
> > Subject: [PATCH] media: rc: meson: rc rc0: two consecutive events of type
> > space
> >
> > The meson generates one edge per interrupt. The duration is encoded in 12
> > bits of 10 microseconds, so it can only encoding a maximum of 40
> > milliseconds. As a result, it can produce multiple space events.
> >
> > Signed-off-by: Sean Young
> > ---
> > drivers/media/rc/meson-ir.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
> > index f449b35d25e7..9747426719b2 100644
> > --- a/drivers/media/rc/meson-ir.c
> > +++ b/drivers/media/rc/meson-ir.c
> > @@ -97,7 +97,8 @@ static irqreturn_t meson_ir_irq(int irqno, void *dev_id)
> > status = readl_relaxed(ir->reg + IR_DEC_STATUS);
> > rawir.pulse = !!(status & STATUS_IR_DEC_IN);
> >
> > - ir_raw_event_store_with_timeout(ir->rc, );
> > + if (ir_raw_event_store_with_filter(ir->rc, ))
> > + ir_raw_event_handle(ir->rc);
> >
> > spin_unlock(>lock);
> >
>
> Solve the problem on meson. Thx
> Feel free to add this submitting the patch
Actually this patch is broken. ir_raw_event_store_with_timeout() generates
IR timeouts, but ir_raw_event_store_with_filter() does not.
So this will need some rethinking. I think we need a
ir_raw_event_store_with_timeout_with_filter() but that is getting silly now.
Maybe filter should be a boolean property of an rc device and happen
transparently. I'll write something like that when I get some time.
Sean
[GIT PULL v2 for 4.19] Sensor, lens and ISP driver patches
Hi Mauro,
Here's the usual bunch of sensor, lens and ISP driver patches. In
particular, there are new drivers for ov2680 sensor and dw9807 VCM lens
controller part.
Since v1, I took out Arnd's Cadence patches since they need to go to the
fixes branch instead (see:
https://patchwork.linuxtv.org/patch/50464/>), as well as the ov2680
driver for addressing compile issues.
There are also a few additional patches for making the
v4l2_find_nearest_size sparse-friendly and ov5640 improvements.
Please pull.
The following changes since commit f2809d20b9250c675fca8268a0f6274277cca7ff:
media: omap2: fix compile-testing with FB_OMAP2=m (2018-06-05 09:56:56 -0400)
are available in the git repository at:
ssh://linuxtv.org/git/sailus/media_tree.git for-4.19-1.1
for you to fetch changes up to eba3bcf150fb59461e9ebe1c16c233680e1334bf:
media: ov5640: fix frame interval enumeration (2018-06-26 17:07:02 +0300)
Akinobu Mita (14):
media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING
media: ov772x: add checks for register read errors
media: ov772x: add media controller support
media: ov772x: use generic names for reset and powerdown gpios
media: ov772x: omit consumer ID when getting clock reference
media: ov772x: support device tree probing
media: ov772x: handle nested s_power() calls
media: ov772x: reconstruct s_frame_interval()
media: ov772x: use v4l2_ctrl to get current control value
media: ov772x: avoid accessing registers under power saving mode
media: ov772x: make set_fmt() and s_frame_interval() return -EBUSY while
streaming
media: ov772x: create subdevice device node
media: s3c-camif: ignore -ENOIOCTLCMD from v4l2_subdev_call for s_power
media: soc_camera: ov772x: correct setting of banding filter
Alan Chiang (2):
media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil
media: dw9807: Add dw9807 vcm driver
Arnd Bergmann (1):
media: omap3isp: fix warning for !CONFIG_PM
Hugues Fruchet (1):
media: ov5640: fix frame interval enumeration
Javier Martinez Canillas (1):
media: omap3isp: zero-initialize the isp cam_xclk{a,b} initial data
Philipp Puschmann (1):
media: ov5640: adjust xclk_max
Sakari Ailus (1):
v4l-common: Make v4l2_find_nearest_size more sparse-friendly
Yong Zhi (1):
MAINTAINERS: Update entry for Intel IPU3 cio2 driver
.../bindings/media/i2c/dongwoon,dw9807.txt | 9 +
MAINTAINERS| 10 +
arch/sh/boards/mach-migor/setup.c | 7 +-
drivers/media/i2c/Kconfig | 10 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/dw9807.c | 329 +++
drivers/media/i2c/ov5640.c | 36 +--
drivers/media/i2c/ov772x.c | 353 +++--
drivers/media/i2c/soc_camera/ov772x.c | 2 +-
drivers/media/platform/omap3isp/isp.c | 6 +-
drivers/media/platform/s3c-camif/camif-capture.c | 2 +
include/media/v4l2-common.h| 2 +-
12 files changed, 647 insertions(+), 120 deletions(-)
create mode 100644
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
create mode 100644 drivers/media/i2c/dw9807.c
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Re: [PATCH v6 2/2] media: ov2680: Add Omnivision OV2680 sensor driver
On Wed, May 09, 2018 at 03:31:59PM +0100, Rui Miguel Silva wrote:
...
> +static int ov2680_init_cfg(struct v4l2_subdev *sd,
> +struct v4l2_subdev_pad_config *cfg)
> +{
> + struct ov2680_dev *sensor = to_ov2680_dev(sd);
> + struct v4l2_mbus_framefmt *mf;
> +
> + mf = v4l2_subdev_get_try_format(sd, cfg, 0);
> +
> + *mf = sensor->fmt;
The init_cfg callback is intended for initialising the format configuration
(as well as compose and crop where relevant) to the device default values.
The implementation in e.g. drivers/media/i2c/ov7251.c seems abour right,
for instance.
I think this would be relevant in addressing the compile issues without
subdev uAPI, too.
I've postponed the two patches, feel free to send either delta or v7 of
this.
--
Sakari Ailus
sakari.ai...@linux.intel.com
PROCUREMENT DEPARTMENT
Good day. Kindly find attached PO 28057 for your reference and action. Your quick response is highly appreciated. We wish to get your quotations on confirmation of as attached. Awaits your urgent responds on confirmation of the above THANKS & REGARDS ? Alma Patubo PROCUREMENT DEPARTMENT Office # 204, 2nd Floor,Building No P3 Ferij Bin Omran, AL Jazeera Al-Arabiya St. P.O. Box 14156 Doha, State of Head Office Tel : +974 4406 2762 - 600 Fax No : +974 4487 9886
[GIT PULL v2 for 4.19] Add "rotation" property for sensors, use it
Hi Mauro, This pull request adds the "rotation" property already used for display panels for sensors. Support for the property is added to the smiapp and ov5640 drivers. Since v1, the bindings have been split of from the driver patches and there are improvements in binding documentation. Please pull. The following changes since commit f2809d20b9250c675fca8268a0f6274277cca7ff: media: omap2: fix compile-testing with FB_OMAP2=m (2018-06-05 09:56:56 -0400) are available in the git repository at: ssh://linuxtv.org/git/sailus/media_tree.git v4l2-rotation for you to fetch changes up to 1a2b68f7db247da7b46cf06d5a8f57a6de8fd74b: media: ov5640: add support of module orientation (2018-06-21 13:46:45 +0300) Hugues Fruchet (3): media: ov5640: add HFLIP/VFLIP controls support dt-bindings: ov5640: Add "rotation" property media: ov5640: add support of module orientation Sakari Ailus (3): dt-bindings: media: Define "rotation" property for sensors dt-bindings: smia: Add "rotation" property smiapp: Support the "rotation" property .../devicetree/bindings/media/i2c/nokia,smia.txt | 3 + .../devicetree/bindings/media/i2c/ov5640.txt | 5 + .../devicetree/bindings/media/video-interfaces.txt | 4 + drivers/media/i2c/ov5640.c | 127 ++--- drivers/media/i2c/smiapp/smiapp-core.c | 16 +++ 5 files changed, 137 insertions(+), 18 deletions(-) -- Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [PATCH 0/4] media: sun6i: Add support for the H3 CSI controller
On Tue, Jun 26, 2018 at 02:42:11PM +0530, Jagan Teki wrote: > On Tue, Jun 26, 2018 at 1:53 PM, Maxime Ripard > wrote: > > Hi, > > > > On Tue, Jun 26, 2018 at 11:41:56AM +0530, Jagan Teki wrote: > >> On Mon, Mar 5, 2018 at 3:34 PM, Maxime Ripard > >> wrote: > >> > The H3 and H5 have a CSI controller based on the one previously found > >> > in the A31, that is currently supported by the sun6i-csi driver. > >> > > >> > Add the compatibles to the device tree bindings and to the driver to > >> > make it work properly. > >> > > >> > This obviously depends on the serie "Initial Allwinner V3s CSI > >> > Support" by Yong Deng. > >> > > >> > Let me know what you think, > >> > Maxime > >> > > >> > Maxime Ripard (2): > >> > dt-bindings: media: sun6i: Add A31 and H3 compatibles > >> > media: sun6i: Add A31 compatible > >> > > >> > Mylène Josserand (2): > >> > ARM: dts: sun8i: Add the H3/H5 CSI controller > >> > [DO NOT MERGE] ARM: dts: sun8i: Add CAM500B camera module to the Nano > >> > Pi M1+ > >> > >> Just trying to understand what interface has been tested with npi-m1+, > >> is it DVP (parallel) interface? > > > > Yes > > > >> I've Bananapi 5MP[1] and trying to test on top, and look like its > >> MIPI CSI2. I guess Yong patch[2] doesn't support CSI2 yet, am I > >> correct? > > > > This is what Yong's cover letter is saying yes :) > > Thanks, and this ov5640 tested on -next or any specific code? please > provide if you have different code base. It should work with the current ov5640 driver. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH 0/4] media: sun6i: Add support for the H3 CSI controller
On Tue, Jun 26, 2018 at 1:53 PM, Maxime Ripard wrote: > Hi, > > On Tue, Jun 26, 2018 at 11:41:56AM +0530, Jagan Teki wrote: >> On Mon, Mar 5, 2018 at 3:34 PM, Maxime Ripard >> wrote: >> > The H3 and H5 have a CSI controller based on the one previously found >> > in the A31, that is currently supported by the sun6i-csi driver. >> > >> > Add the compatibles to the device tree bindings and to the driver to >> > make it work properly. >> > >> > This obviously depends on the serie "Initial Allwinner V3s CSI >> > Support" by Yong Deng. >> > >> > Let me know what you think, >> > Maxime >> > >> > Maxime Ripard (2): >> > dt-bindings: media: sun6i: Add A31 and H3 compatibles >> > media: sun6i: Add A31 compatible >> > >> > Mylène Josserand (2): >> > ARM: dts: sun8i: Add the H3/H5 CSI controller >> > [DO NOT MERGE] ARM: dts: sun8i: Add CAM500B camera module to the Nano >> > Pi M1+ >> >> Just trying to understand what interface has been tested with npi-m1+, >> is it DVP (parallel) interface? > > Yes > >> I've Bananapi 5MP[1] and trying to test on top, and look like its >> MIPI CSI2. I guess Yong patch[2] doesn't support CSI2 yet, am I >> correct? > > This is what Yong's cover letter is saying yes :) Thanks, and this ov5640 tested on -next or any specific code? please provide if you have different code base. Jagan.
Re: [PATCH 0/4] media: sun6i: Add support for the H3 CSI controller
Hi, On Tue, Jun 26, 2018 at 11:41:56AM +0530, Jagan Teki wrote: > On Mon, Mar 5, 2018 at 3:34 PM, Maxime Ripard > wrote: > > The H3 and H5 have a CSI controller based on the one previously found > > in the A31, that is currently supported by the sun6i-csi driver. > > > > Add the compatibles to the device tree bindings and to the driver to > > make it work properly. > > > > This obviously depends on the serie "Initial Allwinner V3s CSI > > Support" by Yong Deng. > > > > Let me know what you think, > > Maxime > > > > Maxime Ripard (2): > > dt-bindings: media: sun6i: Add A31 and H3 compatibles > > media: sun6i: Add A31 compatible > > > > Mylène Josserand (2): > > ARM: dts: sun8i: Add the H3/H5 CSI controller > > [DO NOT MERGE] ARM: dts: sun8i: Add CAM500B camera module to the Nano > > Pi M1+ > > Just trying to understand what interface has been tested with npi-m1+, > is it DVP (parallel) interface? Yes > I've Bananapi 5MP[1] and trying to test on top, and look like its > MIPI CSI2. I guess Yong patch[2] doesn't support CSI2 yet, am I > correct? This is what Yong's cover letter is saying yes :) The name Allwinner chose for this IP is very unfortunate... Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v3 05/13] media: v4l2-fwnode: Add a convenience function for registering subdevs with notifiers
On Tue, May 08, 2018 at 08:55:04PM -0700, Steve Longerbeam wrote:
>
>
> On 05/08/2018 03:28 AM, Sakari Ailus wrote:
> > Hi Steve,
> >
> > Again, sorry about the delay. This thread got buried in my inbox. :-(
> > Please see my reply below.
> >
> > On Mon, Apr 23, 2018 at 11:00:22AM -0700, Steve Longerbeam wrote:
> > >
> > > On 04/23/2018 12:14 AM, Sakari Ailus wrote:
> > > > Hi Steve,
> > > >
> > > > On Tue, Mar 20, 2018 at 05:37:21PM -0700, Steve Longerbeam wrote:
> > > > > Adds v4l2_async_register_fwnode_subdev(), which is a convenience
> > > > > function
> > > > > for parsing a sub-device's fwnode port endpoints for connected remote
> > > > > sub-devices, registering a sub-device notifier, and then registering
> > > > > the sub-device itself.
> > > > >
> > > > > Signed-off-by: Steve Longerbeam
> > > > > ---
> > > > > Changes since v2:
> > > > > - fix error-out path in v4l2_async_register_fwnode_subdev() that
> > > > > forgot
> > > > > to put device.
> > > > > Changes since v1:
> > > > > - add #include to v4l2-fwnode.h for
> > > > > 'struct v4l2_subdev' declaration.
> > > > > ---
> > > > >drivers/media/v4l2-core/v4l2-fwnode.c | 101
> > > > > ++
> > > > >include/media/v4l2-fwnode.h | 43 +++
> > > > >2 files changed, 144 insertions(+)
> > > > >
> > > > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> > > > > b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > > > index 99198b9..d42024d 100644
> > > > > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > > > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > > > @@ -880,6 +880,107 @@ int
> > > > > v4l2_async_register_subdev_sensor_common(struct v4l2_subdev *sd)
> > > > >}
> > > > >EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor_common);
> > > > > +int v4l2_async_register_fwnode_subdev(
> > > > > + struct v4l2_subdev *sd, size_t asd_struct_size,
> > > > > + unsigned int *ports, unsigned int num_ports,
> > > > > + int (*parse_endpoint)(struct device *dev,
> > > > > + struct v4l2_fwnode_endpoint *vep,
> > > > > + struct v4l2_async_subdev *asd))
> > > > > +{
> > > > > + struct v4l2_async_notifier *notifier;
> > > > > + struct device *dev = sd->dev;
> > > > > + struct fwnode_handle *fwnode;
> > > > > + unsigned int subdev_port;
> > > > > + bool is_port;
> > > > > + int ret;
> > > > > +
> > > > > + if (WARN_ON(!dev))
> > > > > + return -ENODEV;
> > > > > +
> > > > > + fwnode = dev_fwnode(dev);
> > > > > + if (!fwnode_device_is_available(fwnode))
> > > > > + return -ENODEV;
> > > > > +
> > > > > + is_port = (is_of_node(fwnode) &&
> > > > > +of_node_cmp(to_of_node(fwnode)->name, "port") == 0);
> > > > What's the intent of this and the code below? You may not parse the
> > > > graph
> > > > data structure here, it should be done in the actual firmware
> > > > implementation instead.
> > > The i.MX6 CSI sub-device registers itself from a port fwnode, so
> > > the intent of the is_port code below is to support the i.MX6 CSI.
> > >
> > > I can remove the is_port checks, but it means
> > > v4l2_async_register_fwnode_subdev() won't be usable by the CSI
> > > sub-device.
> > This won't scale.
>
> The vast majority of sub-devices register themselves as
> port parent nodes. So for now at least, I think
> v4l2_async_register_fwnode_subdev() could be useful to many
> platforms.
>
> > Instead, I think we'd need to separate registering
> > sub-devices (through async sub-devices) and binding them with the driver
> > that registered the notifier. Or at least change how that process works: a
> > single sub-device can well be bound to multiple notifiers,
>
> Ok, that is certainly not the case now, a sub-device can only
> be bound to a single notifier.
>
> > or multiple
> > times to the same notifier while it may be registered only once.
>
> Anyway, this is a future generalization if I understand you
> correctly. Not something to deal with here.
>
> >
> > > > Also, sub-devices generally do not match ports.
> > > Yes that's generally true, sub-devices generally match to port parent
> > > nodes. But I do know of one other sub-device that buck that trend.
> > > The ADV748x CSI-2 output sub-devices match against endpoint nodes.
> > Endpoints, yes, but not ports.
>
> Well, the imx CSI registers from a port node.
I looked at the imx driver and it appears to be parsing DT without much
help from the frameworks; graph or V4L2 fwnode. Is there a reason to do so,
other than it's a staging driver? :-)
Do you happen to have any DT source (or bindings) for this? It'd help to
understand what is the aim here.
>
> >
> > > >How sub-devices generally
> > > > correspond to fwnodes is up to the device.
> > > What do you think of adding a v4l2_async_register_port_fwnode_subdev(),
> > > and a v4l2_async_register_endpoint_fwnode_subdev() to
Re: [PATCH v3 03/13] media: v4l2: async: Add v4l2_async_notifier_add_subdev
On Wed, May 09, 2018 at 04:06:32PM -0700, Steve Longerbeam wrote:
>
>
> On 05/08/2018 03:12 AM, Sakari Ailus wrote:
> > On Fri, Apr 20, 2018 at 10:12:33AM -0700, Steve Longerbeam wrote:
> > > Hi Sakari,
> > >
> > >
> > > On 04/20/2018 05:24 AM, Sakari Ailus wrote:
> > > > Hi Steve,
> > > >
> > > > Thanks for the patchset.
> > > >
> > > > On Tue, Mar 20, 2018 at 05:37:19PM -0700, Steve Longerbeam wrote:
> > > > > v4l2_async_notifier_add_subdev() adds an asd to the notifier. It
> > > > > checks
> > > > > that the asd's match_type is valid and that no other equivalent asd's
> > > > > have already been added to this notifier's asd list, or to other
> > > > > registered notifier's waiting or done lists, and increments
> > > > > num_subdevs.
> > > > >
> > > > > v4l2_async_notifier_add_subdev() does not make use of the notifier
> > > > > subdevs
> > > > > array, otherwise it would have to re-allocate the array every time the
> > > > > function was called. In place of the subdevs array, the function adds
> > > > > the asd to a new master asd_list. The function will return error with
> > > > > a
> > > > > WARN() if it is ever called with the subdevs array allocated.
> > > > >
> > > > > In v4l2_async_notifier_has_async_subdev(),
> > > > > __v4l2_async_notifier_register(),
> > > > > and v4l2_async_notifier_cleanup(), alternatively operate on the
> > > > > subdevs
> > > > > array or a non-empty notifier->asd_list.
> > > > I do agree with the approach, but this patch leaves the remaining users
> > > > of
> > > > the subdevs array in the notifier intact. Could you rework them to use
> > > > the
> > > > v4l2_async_notifier_add_subdev() as well?
> > > >
> > > > There seem to be just a few of them --- exynos4-is and rcar_drif.
> > > I count more than a few :)
> > >
> > > % cd drivers/media && grep -l -r --include "*.[ch]"
> > > 'notifier[\.\-]>*subdevs[ ]*='
> > > v4l2-core/v4l2-async.c
> > > platform/pxa_camera.c
> > > platform/ti-vpe/cal.c
> > > platform/exynos4-is/media-dev.c
> > > platform/qcom/camss-8x16/camss.c
> > > platform/soc_camera/soc_camera.c
> > > platform/atmel/atmel-isi.c
> > > platform/atmel/atmel-isc.c
> > > platform/stm32/stm32-dcmi.c
> > > platform/davinci/vpif_capture.c
> > > platform/davinci/vpif_display.c
> > > platform/renesas-ceu.c
> > > platform/am437x/am437x-vpfe.c
> > > platform/xilinx/xilinx-vipp.c
> > > platform/rcar_drif.c
> > >
> > >
> > > So not including v4l2-core, the list is:
> > >
> > > pxa_camera
> > > ti-vpe
> > > exynos4-is
> > > qcom
> > > soc_camera
> > > atmel
> > > stm32
> > > davinci
> > > renesas-ceu
> > > am437x
> > > xilinx
> > > rcar_drif
> > >
> > >
> > > Given such a large list of the users of the notifier->subdevs array,
> > > I think this should be done is two steps: submit this patchset first,
> > > that keeps the backward compatibility, and then a subsequent patchset
> > > that converts all drivers to use v4l2_async_notifier_add_subdev(), at
> > > which point the subdevs array can be removed from struct
> > > v4l2_async_notifier.
> > >
> > > Or, do you still think this should be done all at once? I could add a
> > > large patch to this patchset that does the conversion and removes
> > > the subdevs array.
> > Sorry for the delay. I grepped for this, too, but I guess I ended up using
> > an expression that missed most of the users. :-(
> >
> > I think it'd be very good to perform that conversion --- the code in the
> > framework would be quite a bit cleaner and easier to maintain whereas the
> > per-driver conversions seem pretty simple, such as this on in
> > drivers/media/platform/atmel/atmel-isi.c :
> >
> > /* Register the subdevices notifier. */
> > subdevs = devm_kzalloc(isi->dev, sizeof(*subdevs), GFP_KERNEL);
> > if (!subdevs) {
> > of_node_put(isi->entity.node);
> > return -ENOMEM;
> > }
> >
> > subdevs[0] = >entity.asd;
> >
> > isi->notifier.subdevs = subdevs;
> > isi->notifier.num_subdevs = 1;
> > isi->notifier.ops = _graph_notify_ops;
>
>
> Yes, the conversions are fairly straightforward. I've completed that work,
> but it was a very manual conversion, every platform is different and needed
> careful study.
>
> Although I was careful about getting the error-out paths correct, there
> could
> be mistakes there, which would result in memory leaks. And obviously I can't
> re-test all these platforms. So this patch is very high-risk. More eyes are
> needed on it, ideally the maintainer(s) of each affected platform.
>
> I just submitted v4 of this series, but did not include this large un-tested
> patch in v4 for those reasons.
>
> Instead, this patch, and follow-up patches that strips support for subdevs
> array altogether from v4l2-async.c, and updates rst docs, are available at
> my
> media-tree mirror on github:
>
> g...@github.com:slongerbeam/mediatree.git
>
> in the branch 'remove-subdevs-array'. The branch is based off this series
> (branch
Re: [PATCH 0/4] media: sun6i: Add support for the H3 CSI controller
On Mon, Mar 5, 2018 at 3:34 PM, Maxime Ripard wrote: > Hi, > > The H3 and H5 have a CSI controller based on the one previously found > in the A31, that is currently supported by the sun6i-csi driver. > > Add the compatibles to the device tree bindings and to the driver to > make it work properly. > > This obviously depends on the serie "Initial Allwinner V3s CSI > Support" by Yong Deng. > > Let me know what you think, > Maxime > > Maxime Ripard (2): > dt-bindings: media: sun6i: Add A31 and H3 compatibles > media: sun6i: Add A31 compatible > > Mylène Josserand (2): > ARM: dts: sun8i: Add the H3/H5 CSI controller > [DO NOT MERGE] ARM: dts: sun8i: Add CAM500B camera module to the Nano > Pi M1+ Just trying to understand what interface has been tested with npi-m1+, is it DVP (parallel) interface? I've Bananapi 5MP[1] and trying to test on top, and look like its MIPI CSI2. I guess Yong patch[2] doesn't support CSI2 yet, am I correct? [1] https://www.amazon.in/Generic-V3-0-BPI-M3-camera-chipset/dp/B0727N5CD1 [2] https://patchwork.kernel.org/patch/10380067/ Jagan. -- Jagan Teki Senior Linux Kernel Engineer | Amarula Solutions U-Boot, Linux | Upstream Maintainer Hyderabad, India.
