cron job: media_tree daily build: ERRORS

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

Results of the daily build of media_tree:

date:   Thu Dec 14 05:00:15 CET 2017
media-tree git hash:b32a2b42f76cdfd06b4b58a1ddf987ba329ae34e
media_build git hash:   f5a5e5e470d834f9843fee7a7c2ce3e4be610ca7
v4l-utils git hash: 06177db29466bebcceb43d6aaffd859bfd35e42a
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0-3911-g6f737e1f
smatch version: v0.5.0-3911-g6f737e1f
host hardware:  x86_64
host os:4.13.0-164

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

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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


[PATCH] pvrusb2: correctly return V4L2_PIX_FMT_MPEG in enum_fmt

2017-12-13 Thread Hans Verkuil
The pvrusb2 code appears to have a some old workaround code for xawtv that 
causes a
WARN() due to an unrecognized pixelformat 0 in v4l2_ioctl.c.

Since all other MPEG drivers fill this in correctly, it is a safe assumption 
that
this particular problem no longer exists.

While I'm at it, clean up the code a bit.

Signed-off-by: Hans Verkuil 
---
I'll try to give this a spin in the morning with xawtv and my ivtv card (that 
also
uses V4L2_PIX_FMT_MPEG), just to make sure xawtv no longer breaks if it sees it.

Oleksandr, are you able to test this as well on your pvrusb2?

Regards,

Hans
---
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c 
b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
index 4320bda9352d..cc90be364a30 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
@@ -78,18 +78,6 @@ static int vbi_nr[PVR_NUM] = {[0 ... PVR_NUM-1] = -1};
 module_param_array(vbi_nr, int, NULL, 0444);
 MODULE_PARM_DESC(vbi_nr, "Offset for device's vbi dev minor");

-static struct v4l2_fmtdesc pvr_fmtdesc [] = {
-   {
-   .index  = 0,
-   .type   = V4L2_BUF_TYPE_VIDEO_CAPTURE,
-   .flags  = V4L2_FMT_FLAG_COMPRESSED,
-   .description= "MPEG1/2",
-   // This should really be V4L2_PIX_FMT_MPEG, but xawtv
-   // breaks when I do that.
-   .pixelformat= 0, // V4L2_PIX_FMT_MPEG,
-   }
-};
-
 #define PVR_FORMAT_PIX  0
 #define PVR_FORMAT_VBI  1

@@ -99,17 +87,11 @@ static struct v4l2_format pvr_format [] = {
.fmt= {
.pix= {
.width  = 720,
-   .height = 576,
-   // This should really be V4L2_PIX_FMT_MPEG,
-   // but xawtv breaks when I do that.
-   .pixelformat= 0, // V4L2_PIX_FMT_MPEG,
+   .height = 576,
+   .pixelformat= V4L2_PIX_FMT_MPEG,
.field  = V4L2_FIELD_INTERLACED,
-   .bytesperline   = 0,  // doesn't make sense
- // here
-   //FIXME : Don't know what to put here...
-   .sizeimage  = (32*1024),
-   .colorspace = 0, // doesn't make sense here
-   .priv   = 0
+   /* FIXME : Don't know what to put here... */
+   .sizeimage  = 32 * 1024,
}
}
},
@@ -407,11 +389,11 @@ static int pvr2_g_frequency(struct file *file, void 
*priv, struct v4l2_frequency

 static int pvr2_enum_fmt_vid_cap(struct file *file, void *priv, struct 
v4l2_fmtdesc *fd)
 {
-   /* Only one format is supported : mpeg.*/
-   if (fd->index != 0)
+   /* Only one format is supported: MPEG. */
+   if (fd->index)
return -EINVAL;

-   memcpy(fd, pvr_fmtdesc, sizeof(struct v4l2_fmtdesc));
+   fd->pixelformat = V4L2_PIX_FMT_MPEG;
return 0;
 }



Re: [PATCH v4 3/5] media: i2c: Add TDA1997x HDMI receiver driver

2017-12-13 Thread Tim Harvey
On Mon, Dec 4, 2017 at 9:30 AM, Tim Harvey  wrote:
> On Mon, Dec 4, 2017 at 4:50 AM, Hans Verkuil  wrote:
>> Hi Tim,
>>
>> Found a few more small issues. After that's fixed and you have the Ack for 
>> the
>> bindings this can be merged I think.
>
> Hans,
>
> Thanks. Can you weigh in on the bindings? Rob was hoping for some
> discussion on making some generic bus format types for video and I'm
> not familiar with the other video encoders/decoders enough to know if
> there is enough commonality.
>
>>
>> On 11/29/2017 10:19 PM, Tim Harvey wrote:
> 
>>> +
>>> +/* parse an infoframe and do some sanity checks on it */
>>> +static unsigned int
>>> +tda1997x_parse_infoframe(struct tda1997x_state *state, u16 addr)
>>> +{
>>> + struct v4l2_subdev *sd = >sd;
>>> + union hdmi_infoframe frame;
>>> + u8 buffer[40];
>>> + u8 reg;
>>> + int len, err;
>>> +
>>> + /* read data */
>>> + len = io_readn(sd, addr, sizeof(buffer), buffer);
>>> + err = hdmi_infoframe_unpack(, buffer);
>>> + if (err) {
>>> + v4l_err(state->client,
>>> + "failed parsing %d byte infoframe: 0x%04x/0x%02x\n",
>>> + len, addr, buffer[0]);
>>> + return err;
>>> + }
>>> + hdmi_infoframe_log(KERN_INFO, >client->dev, );
>>> + switch (frame.any.type) {
>>> + /* Audio InfoFrame: see HDMI spec 8.2.2 */
>>> + case HDMI_INFOFRAME_TYPE_AUDIO:
>>> + /* sample rate */
>>> + switch (frame.audio.sample_frequency) {
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_32000:
>>> + state->audio_samplerate = 32000;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_44100:
>>> + state->audio_samplerate = 44100;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_48000:
>>> + state->audio_samplerate = 48000;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_88200:
>>> + state->audio_samplerate = 88200;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_96000:
>>> + state->audio_samplerate = 96000;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_176400:
>>> + state->audio_samplerate = 176400;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_192000:
>>> + state->audio_samplerate = 192000;
>>> + break;
>>> + default:
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM:
>>> + break;
>>> + }
>>> +
>>> + /* sample size */
>>> + switch (frame.audio.sample_size) {
>>> + case HDMI_AUDIO_SAMPLE_SIZE_16:
>>> + state->audio_samplesize = 16;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_SIZE_20:
>>> + state->audio_samplesize = 20;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_SIZE_24:
>>> + state->audio_samplesize = 24;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_SIZE_STREAM:
>>> + default:
>>> + break;
>>> + }
>>> +
>>> + /* Channel Count */
>>> + state->audio_channels = frame.audio.channels;
>>> + if (frame.audio.channel_allocation &&
>>> + frame.audio.channel_allocation != state->audio_ch_alloc) {
>>> + /* use the channel assignment from the infoframe */
>>> + state->audio_ch_alloc = 
>>> frame.audio.channel_allocation;
>>> + tda1997x_configure_audout(sd, state->audio_ch_alloc);
>>> + /* reset the audio FIFO */
>>> + tda1997x_hdmi_info_reset(sd, RESET_AUDIO, false);
>>> + }
>>> + break;
>>> +
>>> + /* Auxiliary Video information (AVI) InfoFrame: see HDMI spec 8.2.1 */
>>> + case HDMI_INFOFRAME_TYPE_AVI:
>>> + state->colorspace = frame.avi.colorspace;
>>> + state->colorimetry = frame.avi.colorimetry;
>>> + state->content = frame.avi.content_type;
>>> + /* Quantization Range */
>>> + switch (state->rgb_quantization_range) {
>>> + case V4L2_DV_RGB_RANGE_AUTO:
>>> + state->range = frame.avi.quantization_range;
>>> + break;
>>> + case V4L2_DV_RGB_RANGE_LIMITED:
>>> + state->range = HDMI_QUANTIZATION_RANGE_LIMITED;
>>> + break;
>>> + case V4L2_DV_RGB_RANGE_FULL:
>>> + state->range = HDMI_QUANTIZATION_RANGE_FULL;
>>> + break;
>>> + }
>>> + if (state->range == 

Re: [PATCH 3/5] media: i2c: Add TDA1997x HDMI receiver driver

2017-12-13 Thread Tim Harvey
On Tue, Dec 12, 2017 at 4:18 AM, Hans Verkuil  wrote:
> Hi Tim,
>
> Sorry for the delay, I needed to find some time to think about this.
>
> On 11/16/17 05:30, Rob Herring wrote:
>> On Wed, Nov 15, 2017 at 10:31:14AM -0800, Tim Harvey wrote:
>>> On Wed, Nov 15, 2017 at 7:52 AM, Rob Herring  wrote:
 On Thu, Nov 09, 2017 at 10:45:34AM -0800, Tim Harvey wrote:
> Add support for the TDA1997x HDMI receivers.
>
> Cc: Hans Verkuil 
> Signed-off-by: Tim Harvey 
> ---
> v3:
>  - use V4L2_DV_BT_FRAME_WIDTH/HEIGHT macros
>  - fixed missing break
>  - use only hdmi_infoframe_log for infoframe logging
>  - simplify tda1997x_s_stream error handling
>  - add delayed work proc to handle hotplug enable/disable
>  - fix set_edid (disable HPD before writing, enable after)
>  - remove enabling edid by default
>  - initialize timings
>  - take quant range into account in colorspace conversion
>  - remove vendor/product tracking (we provide this in log_status via 
> infoframes)
>  - add v4l_controls
>  - add more detail to log_status
>  - calculate vhref generator timings
>  - timing detection fixes (rounding errors, hswidth errors)
>  - rename configure_input/configure_conv functions
>
> v2:
>  - implement dv timings enum/cap
>  - remove deprecated g_mbus_config op
>  - fix dv_query_timings
>  - add EDID get/set handling
>  - remove max-pixel-rate support
>  - add audio codec DAI support
>  - change audio bindings
> ---
>  drivers/media/i2c/Kconfig|9 +
>  drivers/media/i2c/Makefile   |1 +
>  drivers/media/i2c/tda1997x.c | 3485 
> ++
>  include/dt-bindings/media/tda1997x.h |   78 +

 This belongs with the binding documentation patch.

>>>
>>> Rob,
>>>
>>> Thanks - missed that. I will move it for v4.
>>>
>>> Regarding your previous comment to the v2 series:
 The rest of the binding looks fine, but I have some reservations about
 this. I think this should be common probably. There's been a few
 bindings for display recently that deal with the interface format. Maybe
 some vendor property is needed here to map a standard interface format
 back to pin configuration.
>>>
>>> I take it this is not an 'Ack' for the bindings?
>>>
>>> Which did you feel should be made common? I admit I was surprised
>>> there wasn't a common binding for audio bus format (i2s|spdif) but if
>>> you were referring to the video data that would probably be much more
>>> complicated.
>>
>> The video data. Either you have to try to come up with some way to map
>> color components to signals/pins (and even cycles) or you just enumerate
>> the formats and keep adding to them when new ones appear. There's h/w
>> that allows the former, but in the end you have to interoperate, so
>> enumerating the formats is probably enough.
>>
>>> I was hoping one of the media/driver maintainers would respond to your
>>> comment with thoughts as I'm not familiar with a very wide variety of
>>> receivers.
>>
>> I am hoping, too.
>
> I don't think it is right to store this in the DT. How you map the output pins
> is a driver thing. So when you are requested to enumerate the mediabus formats
> (include/uapi/linux/media-bus-format.h) you support, you do so based on the
> capabilities of the hardware, and when a format is requested you program the
> hardware accordingly.
>
> The device tree should describe the physical characteristics like the number
> of pins that are hooked up (i.e. are there 24, 30 or 36 pins connected).
>
> These vidout-portcfg mappings do not appear to describe physical properties
> but really register settings.

Hans,

They are register settings that define which bits on the internal data
bus are mapped to which pins on the package. Internally these parts
have a 36bit video data bus but externally the tda19971 only has 24
pins and even then perhaps only 8 are hooked up if using BT656 and the
registers also define 'which 8' as it could have been connected to the
upper or lower part of the bus. So while the bindings from
video-interfaces.txt provide bus-width and details of the sync
signals, additional hardware-specific interconnect details such as how
the video bits are shifted/mixed on the output bus are needed here.
This is why I feel vidout-portcfg should be a dt property vs something
like a module param.

Regards,

Tim


Re: Kernel Oopses from v4l_enum_fmt

2017-12-13 Thread Oleksandr Ostrenko

On 13.12.17 23:04, Nicolas Dufresne wrote:

Le mercredi 13 décembre 2017 à 22:33 +0100, Oleksandr Ostrenko a
écrit :

Dear all,

There is an issue in v4l_enum_fmt leading to kernel panic under
certain
circumstance. It happens while I try to capture video from my TV
tuner.

When I connect this USB TV tuner (WinTV HVR-1900) it gets recognized
just fine. However, whenever I try to capture a video from the
device,
it hangs the terminal and I end up with a lot of "Unknown
pixelformat
0x" errors from v4l_enum_fmt in dmesg that eventually lead
to
kernel panic on a machine with Linux Mint. On another machine with
openSUSE it does not hang but just keeps producing the error message
below until I stop the video acquisition. I have already tried
several
kernel versions (4.4, 4.8, 4.14) and two different distributions
(Mint,
openSUSE) but to no avail.

Can somebody give me a hint on debugging this issue?
Below are sample outputs of lsusb and dmesg.

Thanks,
Oleksandr

lsusb

Bus 001 Device 002: ID 8087:8001 Intel Corp.
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 003 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 002 Device 002: ID 8087:0a2a Intel Corp.
Bus 002 Device 005: ID 2040:7300 Hauppauge
Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

Relevant dmesg

[  515.920080] usb 2-3: new high-speed USB device number 4 using
xhci_hcd
[  516.072041] usb 2-3: New USB device found, idVendor=2040,
idProduct=7300
[  516.072045] usb 2-3: New USB device strings: Mfr=1, Product=2,
SerialNumber=3
[  516.072047] usb 2-3: Product: WinTV
[  516.072049] usb 2-3: Manufacturer: Hauppauge
[  516.072051] usb 2-3: SerialNumber: 7300-00-F04BADA0
[  516.072474] pvrusb2: Hardware description: WinTV HVR-1900 Model
73xxx
[  517.089290] pvrusb2: Device microcontroller firmware (re)loaded;
it
should now reset and reconnect.
[  517.121228] usb 2-3: USB disconnect, device number 4
[  517.121436] pvrusb2: Device being rendered inoperable
[  518.908091] usb 2-3: new high-speed USB device number 5 using
xhci_hcd
[  519.065592] usb 2-3: New USB device found, idVendor=2040,
idProduct=7300
[  519.065597] usb 2-3: New USB device strings: Mfr=1, Product=2,
SerialNumber=3
[  519.065600] usb 2-3: Product: WinTV
[  519.065602] usb 2-3: Manufacturer: Hauppauge
[  519.065605] usb 2-3: SerialNumber: 7300-00-F04BADA0
[  519.066862] pvrusb2: Hardware description: WinTV HVR-1900 Model
73xxx
[  519.098815] pvrusb2: Binding ir_rx_z8f0811_haup to i2c address
0x71.
[  519.098872] pvrusb2: Binding ir_tx_z8f0811_haup to i2c address
0x70.
[  519.131651] cx25840 6-0044: cx25843-24 found @ 0x88 (pvrusb2_a)
[  519.133234] lirc_dev: IR Remote Control driver registered, major
241
[  519.134192] lirc_zilog: module is from the staging directory, the
quality is unknown, you have been warned.
[  519.134194] lirc_zilog: module is from the staging directory, the
quality is unknown, you have been warned.
[  519.134564] Zilog/Hauppauge IR driver initializing
[  519.135628] probing IR Rx on pvrusb2_a (i2c-6)
[  519.135674] probe of IR Rx on pvrusb2_a (i2c-6) done. Waiting on
IR Tx.
[  519.135678] i2c i2c-6: probe of IR Rx on pvrusb2_a (i2c-6) done
[  519.135706] probing IR Tx on pvrusb2_a (i2c-6)
[  519.135728] i2c i2c-6: Direct firmware load for haup-ir-
blaster.bin
failed with error -2
[  519.135730] i2c i2c-6: firmware haup-ir-blaster.bin not available
(-2)
[  519.135799] i2c i2c-6: lirc_dev: driver lirc_zilog registered at
minor = 0
[  519.135800] i2c i2c-6: IR unit on pvrusb2_a (i2c-6) registered as
lirc0 and ready
[  519.135802] i2c i2c-6: probe of IR Tx on pvrusb2_a (i2c-6) done
[  519.135826] initialization complete
[  519.140759] pvrusb2: Attached sub-driver cx25840
[  519.147644] tuner: 6-0042: Tuner -1 found with type(s) Radio TV.
[  519.147667] pvrusb2: Attached sub-driver tuner
[  521.029446] cx25840 6-0044: loaded v4l-cx25840.fw firmware (14264
bytes)
[  521.124582] tveeprom: Hauppauge model 73219, rev D1E9, serial#
4031491488
[  521.124586] tveeprom: MAC address is 00:0d:fe:4b:ad:a0
[  521.124588] tveeprom: tuner model is Philips 18271_8295 (idx 149,
type 54)
[  521.124591] tveeprom: TV standards PAL(B/G) PAL(I) SECAM(L/L')
PAL(D/D1/K) ATSC/DVB Digital (eeprom 0xf4)
[  521.124593] tveeprom: audio processor is CX25843 (idx 37)
[  521.124594] tveeprom: decoder processor is CX25843 (idx 30)
[  521.124596] tveeprom: has radio, has IR receiver, has IR
transmitter
[  521.124606] pvrusb2: Supported video standard(s) reported
available
in hardware: PAL-B/B1/D/D1/G/H/I/K;SECAM-B/D/G/H/K/K
[  521.124617] pvrusb2: Device initialization completed successfully.
[  521.124811] pvrusb2: registered device video0 [mpeg]
[  521.124819] dvbdev: DVB: registering new adapter (pvrusb2-dvb)
[  523.039178] cx25840 6-0044: loaded v4l-cx25840.fw firmware (14264
bytes)
[  523.160593] tda829x 6-0042: setting tuner address to 60
[  523.217717] tda18271 6-0060: creating new instance
[  523.260592] tda18271: TDA18271HD/C1 detected @ 6-0060
[ 

Re: Kernel Oopses from v4l_enum_fmt

2017-12-13 Thread Nicolas Dufresne
Le mercredi 13 décembre 2017 à 22:33 +0100, Oleksandr Ostrenko a
écrit :
> Dear all,
> 
> There is an issue in v4l_enum_fmt leading to kernel panic under
> certain 
> circumstance. It happens while I try to capture video from my TV
> tuner.
> 
> When I connect this USB TV tuner (WinTV HVR-1900) it gets recognized 
> just fine. However, whenever I try to capture a video from the
> device, 
> it hangs the terminal and I end up with a lot of "Unknown
> pixelformat 
> 0x" errors from v4l_enum_fmt in dmesg that eventually lead
> to 
> kernel panic on a machine with Linux Mint. On another machine with 
> openSUSE it does not hang but just keeps producing the error message 
> below until I stop the video acquisition. I have already tried
> several 
> kernel versions (4.4, 4.8, 4.14) and two different distributions
> (Mint, 
> openSUSE) but to no avail.
> 
> Can somebody give me a hint on debugging this issue?
> Below are sample outputs of lsusb and dmesg.
> 
> Thanks,
> Oleksandr
> 
> lsusb
> 
> Bus 001 Device 002: ID 8087:8001 Intel Corp.
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 003 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 002 Device 002: ID 8087:0a2a Intel Corp.
> Bus 002 Device 005: ID 2040:7300 Hauppauge
> Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> 
> Relevant dmesg
> 
> [  515.920080] usb 2-3: new high-speed USB device number 4 using
> xhci_hcd
> [  516.072041] usb 2-3: New USB device found, idVendor=2040,
> idProduct=7300
> [  516.072045] usb 2-3: New USB device strings: Mfr=1, Product=2, 
> SerialNumber=3
> [  516.072047] usb 2-3: Product: WinTV
> [  516.072049] usb 2-3: Manufacturer: Hauppauge
> [  516.072051] usb 2-3: SerialNumber: 7300-00-F04BADA0
> [  516.072474] pvrusb2: Hardware description: WinTV HVR-1900 Model
> 73xxx
> [  517.089290] pvrusb2: Device microcontroller firmware (re)loaded;
> it 
> should now reset and reconnect.
> [  517.121228] usb 2-3: USB disconnect, device number 4
> [  517.121436] pvrusb2: Device being rendered inoperable
> [  518.908091] usb 2-3: new high-speed USB device number 5 using
> xhci_hcd
> [  519.065592] usb 2-3: New USB device found, idVendor=2040,
> idProduct=7300
> [  519.065597] usb 2-3: New USB device strings: Mfr=1, Product=2, 
> SerialNumber=3
> [  519.065600] usb 2-3: Product: WinTV
> [  519.065602] usb 2-3: Manufacturer: Hauppauge
> [  519.065605] usb 2-3: SerialNumber: 7300-00-F04BADA0
> [  519.066862] pvrusb2: Hardware description: WinTV HVR-1900 Model
> 73xxx
> [  519.098815] pvrusb2: Binding ir_rx_z8f0811_haup to i2c address
> 0x71.
> [  519.098872] pvrusb2: Binding ir_tx_z8f0811_haup to i2c address
> 0x70.
> [  519.131651] cx25840 6-0044: cx25843-24 found @ 0x88 (pvrusb2_a)
> [  519.133234] lirc_dev: IR Remote Control driver registered, major
> 241
> [  519.134192] lirc_zilog: module is from the staging directory, the 
> quality is unknown, you have been warned.
> [  519.134194] lirc_zilog: module is from the staging directory, the 
> quality is unknown, you have been warned.
> [  519.134564] Zilog/Hauppauge IR driver initializing
> [  519.135628] probing IR Rx on pvrusb2_a (i2c-6)
> [  519.135674] probe of IR Rx on pvrusb2_a (i2c-6) done. Waiting on
> IR Tx.
> [  519.135678] i2c i2c-6: probe of IR Rx on pvrusb2_a (i2c-6) done
> [  519.135706] probing IR Tx on pvrusb2_a (i2c-6)
> [  519.135728] i2c i2c-6: Direct firmware load for haup-ir-
> blaster.bin 
> failed with error -2
> [  519.135730] i2c i2c-6: firmware haup-ir-blaster.bin not available
> (-2)
> [  519.135799] i2c i2c-6: lirc_dev: driver lirc_zilog registered at 
> minor = 0
> [  519.135800] i2c i2c-6: IR unit on pvrusb2_a (i2c-6) registered as 
> lirc0 and ready
> [  519.135802] i2c i2c-6: probe of IR Tx on pvrusb2_a (i2c-6) done
> [  519.135826] initialization complete
> [  519.140759] pvrusb2: Attached sub-driver cx25840
> [  519.147644] tuner: 6-0042: Tuner -1 found with type(s) Radio TV.
> [  519.147667] pvrusb2: Attached sub-driver tuner
> [  521.029446] cx25840 6-0044: loaded v4l-cx25840.fw firmware (14264
> bytes)
> [  521.124582] tveeprom: Hauppauge model 73219, rev D1E9, serial#
> 4031491488
> [  521.124586] tveeprom: MAC address is 00:0d:fe:4b:ad:a0
> [  521.124588] tveeprom: tuner model is Philips 18271_8295 (idx 149, 
> type 54)
> [  521.124591] tveeprom: TV standards PAL(B/G) PAL(I) SECAM(L/L') 
> PAL(D/D1/K) ATSC/DVB Digital (eeprom 0xf4)
> [  521.124593] tveeprom: audio processor is CX25843 (idx 37)
> [  521.124594] tveeprom: decoder processor is CX25843 (idx 30)
> [  521.124596] tveeprom: has radio, has IR receiver, has IR
> transmitter
> [  521.124606] pvrusb2: Supported video standard(s) reported
> available 
> in hardware: PAL-B/B1/D/D1/G/H/I/K;SECAM-B/D/G/H/K/K
> [  521.124617] pvrusb2: Device initialization completed successfully.
> [  521.124811] pvrusb2: registered device video0 [mpeg]
> [  521.124819] dvbdev: DVB: registering new adapter (pvrusb2-dvb)
> [  523.039178] cx25840 6-0044: loaded 

Kernel Oopses from v4l_enum_fmt

2017-12-13 Thread Oleksandr Ostrenko

Dear all,

There is an issue in v4l_enum_fmt leading to kernel panic under certain 
circumstance. It happens while I try to capture video from my TV tuner.


When I connect this USB TV tuner (WinTV HVR-1900) it gets recognized 
just fine. However, whenever I try to capture a video from the device, 
it hangs the terminal and I end up with a lot of "Unknown pixelformat 
0x" errors from v4l_enum_fmt in dmesg that eventually lead to 
kernel panic on a machine with Linux Mint. On another machine with 
openSUSE it does not hang but just keeps producing the error message 
below until I stop the video acquisition. I have already tried several 
kernel versions (4.4, 4.8, 4.14) and two different distributions (Mint, 
openSUSE) but to no avail.


Can somebody give me a hint on debugging this issue?
Below are sample outputs of lsusb and dmesg.

Thanks,
Oleksandr

lsusb

Bus 001 Device 002: ID 8087:8001 Intel Corp.
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 003 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 002 Device 002: ID 8087:0a2a Intel Corp.
Bus 002 Device 005: ID 2040:7300 Hauppauge
Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

Relevant dmesg

[  515.920080] usb 2-3: new high-speed USB device number 4 using xhci_hcd
[  516.072041] usb 2-3: New USB device found, idVendor=2040, idProduct=7300
[  516.072045] usb 2-3: New USB device strings: Mfr=1, Product=2, 
SerialNumber=3

[  516.072047] usb 2-3: Product: WinTV
[  516.072049] usb 2-3: Manufacturer: Hauppauge
[  516.072051] usb 2-3: SerialNumber: 7300-00-F04BADA0
[  516.072474] pvrusb2: Hardware description: WinTV HVR-1900 Model 73xxx
[  517.089290] pvrusb2: Device microcontroller firmware (re)loaded; it 
should now reset and reconnect.

[  517.121228] usb 2-3: USB disconnect, device number 4
[  517.121436] pvrusb2: Device being rendered inoperable
[  518.908091] usb 2-3: new high-speed USB device number 5 using xhci_hcd
[  519.065592] usb 2-3: New USB device found, idVendor=2040, idProduct=7300
[  519.065597] usb 2-3: New USB device strings: Mfr=1, Product=2, 
SerialNumber=3

[  519.065600] usb 2-3: Product: WinTV
[  519.065602] usb 2-3: Manufacturer: Hauppauge
[  519.065605] usb 2-3: SerialNumber: 7300-00-F04BADA0
[  519.066862] pvrusb2: Hardware description: WinTV HVR-1900 Model 73xxx
[  519.098815] pvrusb2: Binding ir_rx_z8f0811_haup to i2c address 0x71.
[  519.098872] pvrusb2: Binding ir_tx_z8f0811_haup to i2c address 0x70.
[  519.131651] cx25840 6-0044: cx25843-24 found @ 0x88 (pvrusb2_a)
[  519.133234] lirc_dev: IR Remote Control driver registered, major 241
[  519.134192] lirc_zilog: module is from the staging directory, the 
quality is unknown, you have been warned.
[  519.134194] lirc_zilog: module is from the staging directory, the 
quality is unknown, you have been warned.

[  519.134564] Zilog/Hauppauge IR driver initializing
[  519.135628] probing IR Rx on pvrusb2_a (i2c-6)
[  519.135674] probe of IR Rx on pvrusb2_a (i2c-6) done. Waiting on IR Tx.
[  519.135678] i2c i2c-6: probe of IR Rx on pvrusb2_a (i2c-6) done
[  519.135706] probing IR Tx on pvrusb2_a (i2c-6)
[  519.135728] i2c i2c-6: Direct firmware load for haup-ir-blaster.bin 
failed with error -2

[  519.135730] i2c i2c-6: firmware haup-ir-blaster.bin not available (-2)
[  519.135799] i2c i2c-6: lirc_dev: driver lirc_zilog registered at 
minor = 0
[  519.135800] i2c i2c-6: IR unit on pvrusb2_a (i2c-6) registered as 
lirc0 and ready

[  519.135802] i2c i2c-6: probe of IR Tx on pvrusb2_a (i2c-6) done
[  519.135826] initialization complete
[  519.140759] pvrusb2: Attached sub-driver cx25840
[  519.147644] tuner: 6-0042: Tuner -1 found with type(s) Radio TV.
[  519.147667] pvrusb2: Attached sub-driver tuner
[  521.029446] cx25840 6-0044: loaded v4l-cx25840.fw firmware (14264 bytes)
[  521.124582] tveeprom: Hauppauge model 73219, rev D1E9, serial# 4031491488
[  521.124586] tveeprom: MAC address is 00:0d:fe:4b:ad:a0
[  521.124588] tveeprom: tuner model is Philips 18271_8295 (idx 149, 
type 54)
[  521.124591] tveeprom: TV standards PAL(B/G) PAL(I) SECAM(L/L') 
PAL(D/D1/K) ATSC/DVB Digital (eeprom 0xf4)

[  521.124593] tveeprom: audio processor is CX25843 (idx 37)
[  521.124594] tveeprom: decoder processor is CX25843 (idx 30)
[  521.124596] tveeprom: has radio, has IR receiver, has IR transmitter
[  521.124606] pvrusb2: Supported video standard(s) reported available 
in hardware: PAL-B/B1/D/D1/G/H/I/K;SECAM-B/D/G/H/K/K

[  521.124617] pvrusb2: Device initialization completed successfully.
[  521.124811] pvrusb2: registered device video0 [mpeg]
[  521.124819] dvbdev: DVB: registering new adapter (pvrusb2-dvb)
[  523.039178] cx25840 6-0044: loaded v4l-cx25840.fw firmware (14264 bytes)
[  523.160593] tda829x 6-0042: setting tuner address to 60
[  523.217717] tda18271 6-0060: creating new instance
[  523.260592] tda18271: TDA18271HD/C1 detected @ 6-0060
[  523.768592] tda829x 6-0042: type set to tda8295+18271
[  533.360586] cx25840 6-0044: 

Re: [PATCH v10 4/4] [media] platform: Add Synopsys DesignWare HDMI RX Controller Driver

2017-12-13 Thread Hans Verkuil
On 13/12/17 15:00, Jose Abreu wrote:
> Hi Hans,
> 
> On 13-12-2017 10:00, Hans Verkuil wrote:
>> On 12/12/17 17:02, Jose Abreu wrote:
>>>
> +static int dw_hdmi_s_routing(struct v4l2_subdev *sd, u32 input, u32 
> output,
> + u32 config)
> +{
> + struct dw_hdmi_dev *dw_dev = to_dw_dev(sd);
> +
> + if (!has_signal(dw_dev, input))
> + return -EINVAL;
 Why would this be a reason to reject this? There may be no signal now, but 
 a signal
 might appear later.
>>> I would expect s_routing to only be called if there is an input
>>> connected, otherwise we are just wasting resources by trying to
>>> equalize an input that is not present ... I can remove the "if"
>>> as there are other safe guards for this though (for example g_fmt
>>> will return an error) ...
>> No, s_routing is typically called as a result of a VIDIOC_S_INPUT
>> call, and that can come whether or not there is a signal on an
>> input. In fact, initially the first input is always selected anyway,
>> whether or not there is a signal.
>>
>> g_fmt will just return the current configured format, this is unrelated
>> to whether or not there is a signal.
>>
>> The only times the driver checks whether or not there is a signal (and
>> what that is) are:
>>
>> 1) g_input_status
>> 2) query_dv_timings
>> 3) when the irq detects a signal change and sends V4L2_EVENT_SOURCE_CHANGE
> 
> Ok, I will remove the checks then.
> 
>>
> + msleep(50); /* Wait for 1 field */
 How do you know this waits for 1 field? Or is this: "Wait for at least 1 
 field"?
>>> Its wait at least for 1 field. This is over-generous because its
>>> assuming the frame rate is 20fps (which in HDMI does not happen).
>> With custom timings it can happen (i.e. a 15 fps stream). Admittedly, it's 
>> not
>> common, but people sometimes use it.
>>
 I don't know exactly how the IP does this, but it looks fishy to me. If it 
 is
 correct, then it could use a few comments about what is going on here as 
 it is
 not obvious.
>>> The IP updates the values at each field but I need to change this
>>> register to populate all values in the bt struct.
>> How do you know which field (top or bottom) you've captured? How do you know 
>> you
>> didn't miss e.g. the bottom field and instead end up with two top field 
>> measurements?
>>
>> The top and bottom field are almost, but not quite the same. Typically the 
>> vertical
>> backporch of the fields differs by 1 where the second field's backporch is 
>> larger by 1 line.
>>
 And what happens if the framerate is even slower? You know the pixelclock 
 and
 total width+height, so you can calculate the framerate from that.
>>> Hmm, but then I have to consider pixelclk error, msleep error, ...
>> But you have that now as well.
>>
>> An alternative is to measure a single field and deduce the backporch values 
>> from that.
>>
>> At least for all the common HDMI interlaced formats il_vbackporch is an even 
>> value and
>> vbackporch is il_vbackporch - 1.
>>
>> So if you get an even backporch, then you found il_vbackporch, and if it is 
>> odd, then
>> you found vbackporch.
>>
> + bt->vsync = hdmi_readl(dw_dev, HDMI_MD_VOL);
> +
> + hdmi_mask_writel(dw_dev, 0x0, HDMI_MD_VCTRL,
> + HDMI_MD_VCTRL_V_OFFS_LIN_MODE_OFFSET,
> + HDMI_MD_VCTRL_V_OFFS_LIN_MODE_MASK);
> + msleep(50); /* Wait for 1 field */
> + bt->vbackporch = hdmi_readl(dw_dev, HDMI_MD_VOL);
> + bt->vfrontporch = vtot - bt->height - bt->vsync - bt->vbackporch;
> +
> + if (bt->interlaced == V4L2_DV_INTERLACED) {
> + hdmi_mask_writel(dw_dev, 0x1, HDMI_MD_VCTRL,
> + HDMI_MD_VCTRL_V_MODE_OFFSET,
> + HDMI_MD_VCTRL_V_MODE_MASK);
> + msleep(100); /* Wait for 2 fields */
> +
> + vtot = hdmi_readl(dw_dev, HDMI_MD_VTL);
> + hdmi_mask_writel(dw_dev, 0x1, HDMI_MD_VCTRL,
> + HDMI_MD_VCTRL_V_OFFS_LIN_MODE_OFFSET,
> + HDMI_MD_VCTRL_V_OFFS_LIN_MODE_MASK);
> + msleep(50); /* Wait for 1 field */
> + bt->il_vsync = hdmi_readl(dw_dev, HDMI_MD_VOL);
> +
> + hdmi_mask_writel(dw_dev, 0x0, HDMI_MD_VCTRL,
> + HDMI_MD_VCTRL_V_OFFS_LIN_MODE_OFFSET,
> + HDMI_MD_VCTRL_V_OFFS_LIN_MODE_MASK);
> + msleep(50);
> + bt->il_vbackporch = hdmi_readl(dw_dev, HDMI_MD_VOL);
> + bt->il_vfrontporch = vtot - bt->height - bt->il_vsync -
> + bt->il_vbackporch;
> +
> + hdmi_mask_writel(dw_dev, 0x0, HDMI_MD_VCTRL,
> + HDMI_MD_VCTRL_V_MODE_OFFSET,
> + HDMI_MD_VCTRL_V_MODE_MASK);
 Same here, I'm not sure this is correct. What is the output of
 'v4l2-ctl --query-dv-timings' when you feed it a 

Re: [PATCH 2/2] [media] ddbridge: don't break on single/last port attach failure

2017-12-13 Thread Daniel Scheller
On Wed, 13 Dec 2017 17:44:37 -0200
Mauro Carvalho Chehab  wrote:

> Em Wed, 13 Dec 2017 18:40:52 +0100
> Daniel Scheller  escreveu:
> 
> > On Wed, 13 Dec 2017 13:26:02 -0200
> > Mauro Carvalho Chehab  wrote:
> >   
> > > Em Wed,  6 Dec 2017 18:59:15 +0100
> > > Daniel Scheller  escreveu:
> > > 
> > > > From: Daniel Scheller 
> > > > 
> > > > As all error handling improved quite a bit, don't stop attaching
> > > > frontends if one of them failed, since - if other tuner modules
> > > > are connected to the PCIe bridge - other hardware may just
> > > > work, so lets not break on a single port failure, but rather
> > > > initialise as much as possible. Ie. if there are issues with a
> > > > C2T2-equipped PCIe bridge card which has additional DuoFlex
> > > > modules connected and the bridge generally works, the DuoFlex
> > > > tuners can still work fine. Also, this only had an effect
> > > > anyway if the failed device/port was the last one being
> > > > enumerated.
> > > > 
> > > > Signed-off-by: Daniel Scheller 
> > > > ---
> > > >  drivers/media/pci/ddbridge/ddbridge-core.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c
> > > > b/drivers/media/pci/ddbridge/ddbridge-core.c index
> > > > 11c5cae92408..b43c40e0bf73 100644 ---
> > > > a/drivers/media/pci/ddbridge/ddbridge-core.c +++
> > > > b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -1962,7 +1962,7
> > > > @@ int ddb_ports_attach(struct ddb *dev) }
> > > > for (i = 0; i < dev->port_num; i++) {
> > > > port = >port[i];
> > > > -   ret = ddb_port_attach(port);
> > > > +   ddb_port_attach(port);  
> > > 
> > > Nah, ignoring an error doesn't seem right. It should at least
> > > print that attach failed.
> > 
> > This is already the case in ddb_port_attach() (if (ret < 0)
> > dev_err(...)).
> >   
> > > Also, if all attaches fail, probably the best
> > > would be to just detach everything and go to the error handling
> > > code, as there's something serious happening.
> > 
> > Well, will recheck the whole error handling there then when already
> > at it, as single port failures can still leave some
> > half-initialised stuff behind until ddbridge gets unloaded.  
> 
> If this is the case, then you need to fix also the unbind logic,
> to be sure that nothing gets left. The best is to compile your test
> Kernel with KASAN enabled, in order to see if the remove logic is
> OK.

There's nothing wrong regarding memory corruption when this happens,
the state machine in the driver keeps track of this, knows how far a
port got, tears down exactly these resources, and doesn't blindly free
things (use-after-free etc). On unload, everything is correctly removed
from memory, the unbind/teardown logic works fine regarding this. The
only real issue which also other drivers suffered from was improper
un-refcounting but all this was completely fixed with the latest
changes in core:dvb_frontend.c (frontend_free and related friends).

But that KASAN thing is a good hint for some other issue I'm having
with another driver for which I've no idea yet how to track that down,
thanks for that (yet some things to learn and discover).

Best regards,
Daniel Scheller
-- 
https://github.com/herrnst


Re: [PATCH 2/2] [media] ddbridge: don't break on single/last port attach failure

2017-12-13 Thread Mauro Carvalho Chehab
Em Wed, 13 Dec 2017 18:40:52 +0100
Daniel Scheller  escreveu:

> On Wed, 13 Dec 2017 13:26:02 -0200
> Mauro Carvalho Chehab  wrote:
> 
> > Em Wed,  6 Dec 2017 18:59:15 +0100
> > Daniel Scheller  escreveu:
> >   
> > > From: Daniel Scheller 
> > > 
> > > As all error handling improved quite a bit, don't stop attaching
> > > frontends if one of them failed, since - if other tuner modules are
> > > connected to the PCIe bridge - other hardware may just work, so
> > > lets not break on a single port failure, but rather initialise as
> > > much as possible. Ie. if there are issues with a C2T2-equipped PCIe
> > > bridge card which has additional DuoFlex modules connected and the
> > > bridge generally works, the DuoFlex tuners can still work fine.
> > > Also, this only had an effect anyway if the failed device/port was
> > > the last one being enumerated.
> > > 
> > > Signed-off-by: Daniel Scheller 
> > > ---
> > >  drivers/media/pci/ddbridge/ddbridge-core.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c
> > > b/drivers/media/pci/ddbridge/ddbridge-core.c index
> > > 11c5cae92408..b43c40e0bf73 100644 ---
> > > a/drivers/media/pci/ddbridge/ddbridge-core.c +++
> > > b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -1962,7 +1962,7 @@
> > > int ddb_ports_attach(struct ddb *dev) }
> > >   for (i = 0; i < dev->port_num; i++) {
> > >   port = >port[i];
> > > - ret = ddb_port_attach(port);
> > > + ddb_port_attach(port);
> > 
> > Nah, ignoring an error doesn't seem right. It should at least print
> > that attach failed.  
> 
> This is already the case in ddb_port_attach() (if (ret < 0)
> dev_err(...)).
> 
> > Also, if all attaches fail, probably the best
> > would be to just detach everything and go to the error handling code,
> > as there's something serious happening.  
> 
> Well, will recheck the whole error handling there then when already at
> it, as single port failures can still leave some half-initialised stuff
> behind until ddbridge gets unloaded.

If this is the case, then you need to fix also the unbind logic,
to be sure that nothing gets left. The best is to compile your test
Kernel with KASAN enabled, in order to see if the remove logic is
OK.

> 
> Thanks for your review, comments and your proposal!
> 
> Best regards,
> Daniel Scheller



Thanks,
Mauro


Re: [PATCH v4 00/12] [dt-bindings] [media] Add document file and driver for Sony CXD2880 DVB-T2/T tuner + demodulator

2017-12-13 Thread Mauro Carvalho Chehab
Em Fri, 13 Oct 2017 14:46:35 +0900
 escreveu:

> From: Yasunari Takiguchi 
> 
> Hi,
> 
> This is the patch series (version 4) of Sony CXD2880 DVB-T2/T tuner + 
> demodulator driver.The driver supports DVB-API and interfaces through 
> SPI.
> 
> We have tested the driver on Raspberry Pi 3 and got picture and sound 
> from a media player.
> 
> The change history of this patch series is as below.

Finally had time to review this patch series. Sorry for taking so long.
4Q is usually very busy.

I answered each patch with comments. There ones I didn't comment
(patches 1, 4 and 8-12) is because I didn't see anything wrong,
except for the notes I did on other patches, mainly:
- they lack SPDX license text;
- the error codes need to review.

Additionally, on patches 8-11, I found weird to not found any
64 bits division, but I noticed some code there that it seemed
to actually be doing such division using some algorithm to make
them work on 32 bits machine. If so, please replace them by
do_div64(), as it makes clearer about what's been doing, and it
provides better performance when compiled on 64 bit Kernels (as
they become just a DIV asm operation).

Regards,
Mauro

Thanks,
Mauro


Re: [PATCH v4 07/12] [media] cxd2880: Add top level of the driver

2017-12-13 Thread Mauro Carvalho Chehab
Em Fri, 13 Oct 2017 15:09:34 +0900
 escreveu:

> From: Yasunari Takiguchi 
> 
> This provides the main dvb frontend operation functions
> for the Sony CXD2880 DVB-T2/T tuner + demodulator driver.
> 
> Signed-off-by: Yasunari Takiguchi 
> Signed-off-by: Masayuki Yamamoto 
> Signed-off-by: Hideki Nozawa 
> Signed-off-by: Kota Yonezawa 
> Signed-off-by: Toshihiko Matsumoto 
> Signed-off-by: Satoshi Watanabe 
> ---
> 
> [Change list]
> Changes in V4
>drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
>   -modified typo "inavlid" to "invalid" at pr_err
>   -removed unnecessary initialization at variable declaration
>   -removed unnecessary brace {}
>   -changed to use cxd2880_dvbt_tune and cxd2880_dvbt2_tune 
>instead of cxd2880_integ_dvbt_tune and cxd2880_integ_dvbt2_tune
>(because we changed it so that demodulator does not 
>  wait for locking the signal.) 
> 
> Changes in V3
>drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
>   -adjusted indent spaces
>   -modified debugging code
>   -removed unnecessary cast
>   -modified return code
>   -modified coding style of if() 
>   -modified about measurement period of PER/BER.
>   -changed hexadecimal code to lower case. 
> 
>  drivers/media/dvb-frontends/cxd2880/cxd2880_top.c | 2019 
> +
>  1 file changed, 2019 insertions(+)
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
> 
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c 
> b/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
> new file mode 100644
> index ..c82aaf0c1597
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
> @@ -0,0 +1,2019 @@
> +/*
> + * cxd2880_top.c
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see .
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__

Same comments as on other patches: use SPDX and dev_foo() for printing 
messages.

> +
> +#include 
> +
> +#include "dvb_frontend.h"
> +#include "dvb_math.h"
> +
> +#include "cxd2880.h"
> +#include "cxd2880_tnrdmd_mon.h"
> +#include "cxd2880_tnrdmd_dvbt2_mon.h"
> +#include "cxd2880_tnrdmd_dvbt_mon.h"
> +#include "cxd2880_integ.h"
> +#include "cxd2880_tnrdmd_dvbt2.h"
> +#include "cxd2880_tnrdmd_dvbt.h"
> +#include "cxd2880_devio_spi.h"
> +#include "cxd2880_spi_device.h"
> +#include "cxd2880_tnrdmd_driver_version.h"
> +
> +struct cxd2880_priv {
> + struct cxd2880_tnrdmd tnrdmd;
> + struct spi_device *spi;
> + struct cxd2880_io regio;
> + struct cxd2880_spi_device spi_device;
> + struct cxd2880_spi cxd2880_spi;
> + struct cxd2880_dvbt_tune_param dvbt_tune_param;
> + struct cxd2880_dvbt2_tune_param dvbt2_tune_param;
> + struct mutex *spi_mutex; /* For SPI access exclusive control */
> + unsigned long pre_ber_update;
> + unsigned long pre_ber_interval;
> + unsigned long post_ber_update;
> + unsigned long post_ber_interval;
> + unsigned long ucblock_update;
> + unsigned long ucblock_interval;
> + enum fe_status s;
> +};
> +
> +static int cxd2880_pre_bit_err_t(
> + struct cxd2880_tnrdmd *tnrdmd, u32 *pre_bit_err,
> + u32 *pre_bit_count)
> +{
> + u8 rdata[2];
> + int ret;
> +
> + if ((!tnrdmd) || (!pre_bit_err) || (!pre_bit_count))
> + return -EINVAL;
> +
> + if (tnrdmd->diver_mode == CXD2880_TNRDMD_DIVERMODE_SUB)
> + return -EINVAL;
> +
> + if (tnrdmd->state != CXD2880_TNRDMD_STATE_ACTIVE)
> + return -EPERM;
> +
> + if 

Re: [PATCH v4 06/12] [media] cxd2880: Add integration layer for the driver

2017-12-13 Thread Mauro Carvalho Chehab
Em Fri, 13 Oct 2017 15:08:34 +0900
 escreveu:

> From: Yasunari Takiguchi 
> 
> These functions monitor the driver and watch for task completion.
> This is part of the Sony CXD2880 DVB-T2/T tuner + demodulator driver.

If I understand well, the goal here is to have thread that would
be waking up from time to time, right? Just use the infrastructure
that the Kernel has for it, like a kthread, or timer_setup() & friends.

Take a look at include/linux/timer.h, and just use what's already
defined.


> 
> Signed-off-by: Yasunari Takiguchi 
> Signed-off-by: Masayuki Yamamoto 
> Signed-off-by: Hideki Nozawa 
> Signed-off-by: Kota Yonezawa 
> Signed-off-by: Toshihiko Matsumoto 
> Signed-off-by: Satoshi Watanabe 
> ---
> 
> [Change list]
> Changes in V4   
>drivers/media/dvb-frontends/cxd2880/cxd2880_integ.c
>   -removed unnecessary initialization at variable declaration
> 
> Changes in V3
>drivers/media/dvb-frontends/cxd2880/cxd2880_integ.c
>   -changed cxd2880_atomic_read to atomic_read
>   -changed cxd2880_atomic_set to atomic_set
>   -modified return code
>   -modified coding style of if() 
>drivers/media/dvb-frontends/cxd2880/cxd2880_integ.h
>   -modified return code
> 
>  .../media/dvb-frontends/cxd2880/cxd2880_integ.c| 98 
> ++
>  .../media/dvb-frontends/cxd2880/cxd2880_integ.h| 44 ++
>  2 files changed, 142 insertions(+)
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_integ.c
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_integ.h
> 
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_integ.c 
> b/drivers/media/dvb-frontends/cxd2880/cxd2880_integ.c
> new file mode 100644
> index ..7264fc355d6b
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_integ.c
> @@ -0,0 +1,98 @@
> +/*
> + * cxd2880_integ.c
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * integration layer common functions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see .
> + */
> +
> +#include "cxd2880_tnrdmd.h"
> +#include "cxd2880_tnrdmd_mon.h"
> +#include "cxd2880_integ.h"
> +
> +int cxd2880_integ_init(struct cxd2880_tnrdmd *tnr_dmd)
> +{
> + int ret;
> + struct cxd2880_stopwatch timer;
> + unsigned int elapsed_time = 0;
> + u8 cpu_task_completed = 0;
> +
> + if (!tnr_dmd)
> + return -EINVAL;
> +
> + ret = cxd2880_tnrdmd_init1(tnr_dmd);
> + if (ret)
> + return ret;
> +
> + ret = cxd2880_stopwatch_start();
> + if (ret)
> + return ret;
> +
> + while (1) {
> + ret = cxd2880_stopwatch_elapsed(, _time);
> + if (ret)
> + return ret;
> +
> + ret =
> + cxd2880_tnrdmd_check_internal_cpu_status(tnr_dmd,
> +  _task_completed);
> + if (ret)
> + return ret;
> +
> + if (cpu_task_completed)
> + break;
> +
> + if (elapsed_time > CXD2880_TNRDMD_WAIT_INIT_TIMEOUT)
> + return -ETIME;
> + ret =
> + cxd2880_stopwatch_sleep(,
> + CXD2880_TNRDMD_WAIT_INIT_INTVL);
> + if (ret)
> + return ret;
> + }
> +
> + ret = cxd2880_tnrdmd_init2(tnr_dmd);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +int cxd2880_integ_cancel(struct cxd2880_tnrdmd *tnr_dmd)
> +{
> + if (!tnr_dmd)
> + return -EINVAL;
> +
> + atomic_set(_dmd->cancel, 1);

Re: [PATCH v4 05/12] [media] cxd2880: Add tuner part of the driver

2017-12-13 Thread Mauro Carvalho Chehab
Em Fri, 13 Oct 2017 15:07:25 +0900
 escreveu:

> From: Yasunari Takiguchi 
> 
> This part of the driver has the main routines to handle
> the tuner and demodulator functionality.  The tnrdmd_mon.* files
> have monitor functions for the driver.
> This is part of the Sony CXD2880 DVB-T2/T tuner + demodulator driver.
> 
> Signed-off-by: Yasunari Takiguchi 
> Signed-off-by: Masayuki Yamamoto 
> Signed-off-by: Hideki Nozawa 
> Signed-off-by: Kota Yonezawa 
> Signed-off-by: Toshihiko Matsumoto 
> Signed-off-by: Satoshi Watanabe 
> ---
> 
> [Change list]
> Changes in V4
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.c
>   -used over 80 columns limit, it makes fine to read codes
>   -removed unnecessary initialization at variable declaration
>   -modified how to write consecutive registers
>   -removed unnecessary brace {}
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.h
>   -adjusted of indent spaces of macro
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_driver_version.h
>   -updated version information
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.c
>   -removed unnecessary brace {}
> 
> Changes in V3
>drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
>   -removed code relevant to ISDB-T
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.c
>   -removed unnecessary cast
>   -removed code relevant to ISDB-T
>   -changed CXD2880_SLEEP to usleep_range
>   -changed cxd2880_memset to memset 
>   -changed cxd2880_atomic_set to atomic_set
>   -modified return code
>   -modified coding style of if()
>   -changed to use const values at writing a lot of registers 
>with a command. 
>   -changed hexadecimal code to lower case. 
>   -adjusted of indent spaces
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.h
>   -removed code relevant to ISDB-T
>   -changed cxd2880_atomic struct to atomic_t
>   -modified return code
>   -changed hexadecimal code to lower case. 
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_driver_version.h
>   -updated version information
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.c
>   -changed CXD2880_SLEEP to usleep_range
>   -removed unnecessary cast
>   -modified return code
>   -modified coding style of if() 
>   -changed hexadecimal code to lower case. 
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.h
>   -modified return code
> 
> Changes in V2
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_driver_version.h
>   -updated version information
> 
>  drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h  |   46 +
>  .../media/dvb-frontends/cxd2880/cxd2880_tnrdmd.c   | 3687 
> 
>  .../media/dvb-frontends/cxd2880/cxd2880_tnrdmd.h   |  391 +++
>  .../cxd2880/cxd2880_tnrdmd_driver_version.h|   29 +
>  .../dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.c |  218 ++
>  .../dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.h |   52 +
>  6 files changed, 4423 insertions(+)
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.c
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.h
>  create mode 100644 
> drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_driver_version.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.c
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.h
> 
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h 
> b/drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
> new file mode 100644
> index ..2d35d3990060
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
> @@ -0,0 +1,46 @@
> +/*
> + * cxd2880_dtv.h
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * DTV related definitions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF 

[PATCH 5/5] v4l2: async: Add debug output to v4l2-async module

2017-12-13 Thread Jacopo Mondi
The v4l2-async module operations are quite complex to follow, due to the
asynchronous nature of subdevices and notifiers registration and
matching procedures. In order to help with debugging of failed or
erroneous matching between a subdevice and the notifier collected
async_subdevice it gets matched against, introduce a few dev_dbg() calls
in v4l2_async core operations.

Protect the debug operations with a Kconfig defined symbol, to make sure
when debugging is disabled, no additional code or data is added to the
module.

Notifiers are identified by the name of the subdevice or v4l2_dev they are
registered by, while subdevice matching which now happens on endpoints,
need a longer description built walking the fwnode graph backwards
collecting parent nodes names (otherwise we would have had printouts
like: "Matching "endpoint" with "endpoint"" which are not that useful).

Signed-off-by: Jacopo Mondi 

---
For fwnodes backed by OF, I may have used the "%pOF" format modifier to
get the full node name instead of parsing the fwnode graph by myself with
"v4l2_async_fwnode_full_name()". Unfortunately I'm not aware of anything
like "%pOF" for ACPI backed fwnodes. Also, walking the fwnode graph by
myself allows me to reduce the depth, to reduce the debug messages output
length which is anyway long enough to result disturbing on a 80columns
terminal window.
---

 drivers/media/v4l2-core/Kconfig  |  8 
 drivers/media/v4l2-core/v4l2-async.c | 81 
 2 files changed, 89 insertions(+)

diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index a35c336..8331736 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -17,6 +17,14 @@ config VIDEO_ADV_DEBUG
  V4L devices.
  In doubt, say N.

+config VIDEO_V4L2_ASYNC_DEBUG
+   bool "Enable debug functionalities for V4L2 async module"
+   depends on VIDEO_V4L2
+   default n
+   ---help---
+ Say Y here to enable debug output in V4L2 async module.
+ In doubt, say N.
+
 config VIDEO_FIXED_MINOR_RANGES
bool "Enable old-style fixed minor ranges on drivers/video devices"
default n
diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index c13a781..307e1a5 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -8,6 +8,10 @@
  * published by the Free Software Foundation.
  */

+#if defined(CONFIG_VIDEO_V4L2_ASYNC_DEBUG)
+#define DEBUG
+#endif
+
 #include 
 #include 
 #include 
@@ -25,6 +29,52 @@
 #include 
 #include 

+#if defined(CONFIG_VIDEO_V4L2_ASYNC_DEBUG)
+#define V4L2_ASYNC_FWNODE_NAME_LEN 512
+
+static void __v4l2_async_fwnode_full_name(char *name,
+ unsigned int len,
+ unsigned int max_depth,
+ struct fwnode_handle *fwnode)
+{
+   unsigned int buf_len = len < V4L2_ASYNC_FWNODE_NAME_LEN ?
+  len : V4L2_ASYNC_FWNODE_NAME_LEN;
+   char __tmp[V4L2_ASYNC_FWNODE_NAME_LEN];
+   struct fwnode_handle *parent;
+
+   memset(name, 0, buf_len);
+   buf_len -= snprintf(__tmp, buf_len, "%s", fwnode_get_name(fwnode));
+
+   parent = fwnode;
+   while ((parent = fwnode_get_parent(parent)) && buf_len &&
+   --max_depth) {
+   buf_len -= snprintf(name, buf_len, "%s/%s",
+   fwnode_get_name(parent), __tmp);
+   strcpy(__tmp, name);
+   }
+}
+
+static void v4l2_async_fwnode_full_name(char *name,
+   unsigned int len,
+   struct fwnode_handle *fwnode)
+{
+   /*
+* Usually 4 as nesting level is sufficient to identify an
+* endpoint firmware node uniquely.
+*/
+   __v4l2_async_fwnode_full_name(name, len, 4, fwnode);
+}
+
+#else /* CONFIG_VIDEO_V4L2_ASYNC_DEBUG */
+#define V4L2_ASYNC_FWNODE_NAME_LEN 0
+
+static void v4l2_async_fwnode_full_name(char *name,
+   unsigned int len,
+   struct fwnode_handle *fwnode)
+{
+}
+#endif /* CONFIG_VIDEO_V4L2_ASYNC_DEBUG */
+
 static struct device *v4l2_async_notifier_dev(
struct v4l2_async_notifier *notifier)
 {
@@ -54,9 +104,12 @@ static void v4l2_async_notifier_call_unbind(struct 
v4l2_async_notifier *n,

 static int v4l2_async_notifier_call_complete(struct v4l2_async_notifier *n)
 {
+   struct device *dev = v4l2_async_notifier_dev(n);
if (!n->ops || !n->ops->complete)
return 0;

+   dev_dbg(dev, "Complete notifier \"%s\"\n", fwnode_get_name(n->owner));
+
return n->ops->complete(n);
 }

@@ -100,8 +153,17 @@ static struct v4l2_async_subdev *v4l2_async_find_match(
struct v4l2_async_notifier 

[PATCH 3/5] include: v4l2_async: Add 'owner' field to notifier

2017-12-13 Thread Jacopo Mondi
Notifiers can be registered as root notifiers (identified by a 'struct
v4l2_device *') or subdevice notifiers (identified by a 'struct
v4l2_subdev *'). In order to identify a notifier no matter if it is root
or not, add a 'struct fwnode_handle *owner' field, whose name can be
printed out for debug purposes.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/v4l2-core/v4l2-async.c | 2 ++
 include/media/v4l2-async.h   | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index a6bddff..0a1bf1d 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -447,6 +447,7 @@ int v4l2_async_notifier_register(struct v4l2_device 
*v4l2_dev,
return -EINVAL;
 
notifier->v4l2_dev = v4l2_dev;
+   notifier->owner = dev_fwnode(v4l2_dev->dev);
 
ret = __v4l2_async_notifier_register(notifier);
if (ret)
@@ -465,6 +466,7 @@ int v4l2_async_subdev_notifier_register(struct v4l2_subdev 
*sd,
return -EINVAL;
 
notifier->sd = sd;
+   notifier->owner = dev_fwnode(sd->dev);
 
ret = __v4l2_async_notifier_register(notifier);
if (ret)
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 6152434..a15c01d 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -106,6 +106,7 @@ struct v4l2_async_notifier_operations {
  * @v4l2_dev:  v4l2_device of the root notifier, NULL otherwise
  * @sd:sub-device that registered the notifier, NULL otherwise
  * @parent:parent notifier
+ * @owner: reference to notifier fwnode_handle, mostly useful for debug
  * @waiting:   list of struct v4l2_async_subdev, waiting for their drivers
  * @done:  list of struct v4l2_subdev, already probed
  * @list:  member in a global list of notifiers
@@ -118,6 +119,7 @@ struct v4l2_async_notifier {
struct v4l2_device *v4l2_dev;
struct v4l2_subdev *sd;
struct v4l2_async_notifier *parent;
+   struct fwnode_handle *owner;
struct list_head waiting;
struct list_head done;
struct list_head list;
-- 
2.7.4



[PATCH 4/5] v4l2: async: Postpone subdev_notifier registration

2017-12-13 Thread Jacopo Mondi
Currently, subdevice notifiers are tested against all available
subdevices as soon as they get registered. It often happens anyway
that the subdevice they are connected to is not yet initialized, as
it usually gets registered later in drivers' code. This makes debug
of v4l2_async particularly painful, as identifying a notifier with
an unitialized subdevice is tricky as they don't have a valid
'struct device *' or 'struct fwnode_handle *' to be identified with.

In order to make sure that the notifier's subdevices is initialized
when the notifier is tesed against available subdevices post-pone the
actual notifier registration at subdevice registration time.

It is worth noting that post-poning registration of a subdevice notifier
does not impact on the completion of the notifiers chain, as even if a
subdev notifier completes as soon as it gets registered, the complete()
call chain cannot be upscaled as long as the subdevice the notifiers
belongs to is not registered.

Also, it is now safe to access a notifier 'struct device *' as we're now
sure it is properly initialized when the notifier is actually
registered.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/v4l2-core/v4l2-async.c | 65 +++-
 include/media/v4l2-async.h   |  2 ++
 2 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index 0a1bf1d..c13a781 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -25,6 +25,13 @@
 #include 
 #include 

+static struct device *v4l2_async_notifier_dev(
+   struct v4l2_async_notifier *notifier)
+{
+   return notifier->v4l2_dev ? notifier->v4l2_dev->dev :
+   notifier->sd->dev;
+}
+
 static int v4l2_async_notifier_call_bound(struct v4l2_async_notifier *n,
  struct v4l2_subdev *subdev,
  struct v4l2_async_subdev *asd)
@@ -124,19 +131,6 @@ static struct v4l2_async_subdev *v4l2_async_find_match(
return NULL;
 }

-/* Find the sub-device notifier registered by a sub-device driver. */
-static struct v4l2_async_notifier *v4l2_async_find_subdev_notifier(
-   struct v4l2_subdev *sd)
-{
-   struct v4l2_async_notifier *n;
-
-   list_for_each_entry(n, _list, list)
-   if (n->sd == sd)
-   return n;
-
-   return NULL;
-}
-
 /* Get v4l2_device related to the notifier if one can be found. */
 static struct v4l2_device *v4l2_async_notifier_find_v4l2_dev(
struct v4l2_async_notifier *notifier)
@@ -160,7 +154,7 @@ static bool v4l2_async_notifier_can_complete(

list_for_each_entry(sd, >done, async_list) {
struct v4l2_async_notifier *subdev_notifier =
-   v4l2_async_find_subdev_notifier(sd);
+   sd->subdev_notifier;

if (subdev_notifier &&
!v4l2_async_notifier_can_complete(subdev_notifier))
@@ -228,7 +222,7 @@ static int v4l2_async_match_notify(struct 
v4l2_async_notifier *notifier,
/*
 * See if the sub-device has a notifier. If not, return here.
 */
-   subdev_notifier = v4l2_async_find_subdev_notifier(sd);
+   subdev_notifier = sd->subdev_notifier;
if (!subdev_notifier || subdev_notifier->parent)
return 0;

@@ -294,7 +288,7 @@ static void v4l2_async_notifier_unbind_all_subdevs(

list_for_each_entry_safe(sd, tmp, >done, async_list) {
struct v4l2_async_notifier *subdev_notifier =
-   v4l2_async_find_subdev_notifier(sd);
+   sd->subdev_notifier;

if (subdev_notifier)
v4l2_async_notifier_unbind_all_subdevs(subdev_notifier);
@@ -371,8 +365,7 @@ static bool v4l2_async_notifier_fwnode_has_async_subdev(

 static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier)
 {
-   struct device *dev =
-   notifier->v4l2_dev ? notifier->v4l2_dev->dev : NULL;
+   struct device *dev = v4l2_async_notifier_dev(notifier);
struct v4l2_async_subdev *asd;
int ret;
int i;
@@ -383,6 +376,8 @@ static int __v4l2_async_notifier_register(struct 
v4l2_async_notifier *notifier)
INIT_LIST_HEAD(>waiting);
INIT_LIST_HEAD(>done);

+   notifier->owner = dev_fwnode(dev);
+
mutex_lock(_lock);

for (i = 0; i < notifier->num_subdevs; i++) {
@@ -421,6 +416,7 @@ static int __v4l2_async_notifier_register(struct 
v4l2_async_notifier *notifier)

/* Keep also completed notifiers on the list */
list_add(>list, _list);
+   notifier->registered = true;

mutex_unlock(_lock);

@@ -447,7 +443,7 @@ int v4l2_async_notifier_register(struct 

[PATCH 0/5] Add debug output to v4l2-async

2017-12-13 Thread Jacopo Mondi
Hello Sakari,
   while testing rcar-vin setup on top of your RFC (included in the series)
that moves the framework to perform endpoint matching, I realized how hard is
to follow what happens with asynchronous notifiers, sub-notifiers and
sub-devices.

In order to better understand what happens and ease debug of v4l2-async
operations I have introduced some dev_dbg() output, protected by a Kconfig
option.

Before being able to properly identify (sub-)notifiers and subdevices I had to
extend fwnode_* framework to support a new .get_name() operation, and modify
v4l2_async to make sure notifiers always have a valid fwnode_handle field to
be identified with.

I have tested this only with not yet mainlined drivers (rcar-vin, rcar-csi2,
max9286) each of them performing non-trivial endpoint matching. Also I only
tested this on OF based systems (not tested on ACPI).
This considered, I am copying linux-media anyhow for feedbacks.

Thanks
   j

Jacopo Mondi (4):
  device property: Add fwnode_get_name() operation
  include: v4l2_async: Add 'owner' field to notifier
  v4l2: async: Postpone subdev_notifier registration
  v4l2: async: Add debug output to v4l2-async module

Sakari Ailus (1):
  v4l: async: Use endpoint node, not device node, for fwnode match

 drivers/acpi/property.c|   6 +
 drivers/base/property.c|  12 ++
 drivers/media/platform/am437x/am437x-vpfe.c|   2 +-
 drivers/media/platform/atmel/atmel-isc.c   |   2 +-
 drivers/media/platform/atmel/atmel-isi.c   |   2 +-
 drivers/media/platform/davinci/vpif_capture.c  |   2 +-
 drivers/media/platform/exynos4-is/media-dev.c  |  14 ++-
 drivers/media/platform/pxa_camera.c|   2 +-
 drivers/media/platform/qcom/camss-8x16/camss.c |   2 +-
 drivers/media/platform/rcar_drif.c |   2 +-
 drivers/media/platform/stm32/stm32-dcmi.c  |   2 +-
 drivers/media/platform/ti-vpe/cal.c|   2 +-
 drivers/media/platform/xilinx/xilinx-vipp.c|  16 ++-
 drivers/media/v4l2-core/Kconfig|   8 ++
 drivers/media/v4l2-core/v4l2-async.c   | 152 +
 drivers/media/v4l2-core/v4l2-fwnode.c  |   2 +-
 drivers/of/property.c  |   6 +
 include/linux/fwnode.h |   2 +
 include/linux/property.h   |   1 +
 include/media/v4l2-async.h |   4 +
 20 files changed, 200 insertions(+), 41 deletions(-)

--
2.7.4



[PATCH 2/5] device property: Add fwnode_get_name() operation

2017-12-13 Thread Jacopo Mondi
Add operation to retrieve the device name from a fwnode handle.

Signed-off-by: Jacopo Mondi 
---
 drivers/acpi/property.c  |  6 ++
 drivers/base/property.c  | 12 
 drivers/of/property.c|  6 ++
 include/linux/fwnode.h   |  2 ++
 include/linux/property.h |  1 +
 5 files changed, 27 insertions(+)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index e26ea20..1e3971c 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1186,6 +1186,11 @@ acpi_fwnode_property_read_string_array(const struct 
fwnode_handle *fwnode,
   val, nval);
 }
 
+static const char *acpi_fwnode_get_name(const struct fwnode_handle *fwnode)
+{
+   return acpi_dev_name(to_acpi_device_node(fwnode));
+}
+
 static struct fwnode_handle *
 acpi_fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
 const char *childname)
@@ -1281,6 +1286,7 @@ static int acpi_fwnode_graph_parse_endpoint(const struct 
fwnode_handle *fwnode,
acpi_fwnode_property_read_string_array, \
.get_parent = acpi_node_get_parent, \
.get_next_child_node = acpi_get_next_subnode,   \
+   .get_name = acpi_fwnode_get_name,   \
.get_named_child_node = acpi_fwnode_get_named_child_node, \
.get_reference_args = acpi_fwnode_get_reference_args,   \
.graph_get_next_endpoint =  \
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 851b1b6..a87b4a9 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -950,6 +950,18 @@ int device_add_properties(struct device *dev,
 EXPORT_SYMBOL_GPL(device_add_properties);
 
 /**
+ * fwnode_get_name - Return the fwnode_handle name
+ * @fwnode: Firmware node to get name from
+ *
+ * Returns a pointer to the firmware node name
+ */
+const char *fwnode_get_name(const struct fwnode_handle *fwnode)
+{
+   return fwnode_call_ptr_op(fwnode, get_name);
+}
+EXPORT_SYMBOL(fwnode_get_name);
+
+/**
  * fwnode_get_next_parent - Iterate to the node's parent
  * @fwnode: Firmware whose parent is retrieved
  *
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 8ad33a4..6c195a8 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -875,6 +875,11 @@ of_fwnode_property_read_string_array(const struct 
fwnode_handle *fwnode,
of_property_count_strings(node, propname);
 }
 
+static const char *of_fwnode_get_name(const struct fwnode_handle *fwnode)
+{
+   return of_node_full_name(to_of_node(fwnode));
+}
+
 static struct fwnode_handle *
 of_fwnode_get_parent(const struct fwnode_handle *fwnode)
 {
@@ -988,6 +993,7 @@ const struct fwnode_operations of_fwnode_ops = {
.property_present = of_fwnode_property_present,
.property_read_int_array = of_fwnode_property_read_int_array,
.property_read_string_array = of_fwnode_property_read_string_array,
+   .get_name = of_fwnode_get_name,
.get_parent = of_fwnode_get_parent,
.get_next_child_node = of_fwnode_get_next_child_node,
.get_named_child_node = of_fwnode_get_named_child_node,
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 411a84c..5d3a8c6 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -57,6 +57,7 @@ struct fwnode_reference_args {
  *  otherwise.
  * @property_read_string_array: Read an array of string properties. Return zero
  * on success, a negative error code otherwise.
+ * @get_name: Return the fwnode name.
  * @get_parent: Return the parent of an fwnode.
  * @get_next_child_node: Return the next child node in an iteration.
  * @get_named_child_node: Return a child node with a given name.
@@ -81,6 +82,7 @@ struct fwnode_operations {
(*property_read_string_array)(const struct fwnode_handle *fwnode_handle,
  const char *propname, const char **val,
  size_t nval);
+   const char *(*get_name)(const struct fwnode_handle *fwnode);
struct fwnode_handle *(*get_parent)(const struct fwnode_handle *fwnode);
struct fwnode_handle *
(*get_next_child_node)(const struct fwnode_handle *fwnode,
diff --git a/include/linux/property.h b/include/linux/property.h
index f6189a3..0fc464f 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -78,6 +78,7 @@ int fwnode_property_get_reference_args(const struct 
fwnode_handle *fwnode,
   unsigned int nargs, unsigned int index,
   struct fwnode_reference_args *args);
 
+const char *fwnode_get_name(const struct fwnode_handle *fwnode);
 struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode);
 struct fwnode_handle 

[PATCH 1/5] v4l: async: Use endpoint node, not device node, for fwnode match

2017-12-13 Thread Jacopo Mondi
From: Sakari Ailus 

V4L2 async framework can use both device's fwnode and endpoints's fwnode
for matching the async sub-device with the sub-device. In order to proceed
moving towards endpoint matching assign the endpoint to the async
sub-device.

As most async sub-device drivers (and the related hardware) only supports
a single endpoint, use the first endpoint found. This works for all
current drivers --- we only ever supported a single async sub-device per
device to begin with.

For async devices that have no endpoints, continue to use the fwnode
related to the device. This includes e.g. lens devices.

Signed-off-by: Sakari Ailus 
---
 drivers/media/platform/am437x/am437x-vpfe.c|  2 +-
 drivers/media/platform/atmel/atmel-isc.c   |  2 +-
 drivers/media/platform/atmel/atmel-isi.c   |  2 +-
 drivers/media/platform/davinci/vpif_capture.c  |  2 +-
 drivers/media/platform/exynos4-is/media-dev.c  | 14 ++
 drivers/media/platform/pxa_camera.c|  2 +-
 drivers/media/platform/qcom/camss-8x16/camss.c |  2 +-
 drivers/media/platform/rcar_drif.c |  2 +-
 drivers/media/platform/stm32/stm32-dcmi.c  |  2 +-
 drivers/media/platform/ti-vpe/cal.c|  2 +-
 drivers/media/platform/xilinx/xilinx-vipp.c| 16 +---
 drivers/media/v4l2-core/v4l2-async.c   |  8 ++--
 drivers/media/v4l2-core/v4l2-fwnode.c  |  2 +-
 13 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
b/drivers/media/platform/am437x/am437x-vpfe.c
index 0997c64..892d9e9 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2493,7 +2493,7 @@ vpfe_get_pdata(struct platform_device *pdev)
if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
sdinfo->vpfe_param.vdpol = 1;
 
-   rem = of_graph_get_remote_port_parent(endpoint);
+   rem = of_graph_get_remote_endpoint(endpoint);
if (!rem) {
dev_err(>dev, "Remote device at %pOF not found\n",
endpoint);
diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 13f1c1c..c8bb60e 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -2044,7 +2044,7 @@ static int isc_parse_dt(struct device *dev, struct 
isc_device *isc)
if (!epn)
break;
 
-   rem = of_graph_get_remote_port_parent(epn);
+   rem = of_graph_get_remote_endpoint(epn);
if (!rem) {
dev_notice(dev, "Remote device at %pOF not found\n",
   epn);
diff --git a/drivers/media/platform/atmel/atmel-isi.c 
b/drivers/media/platform/atmel/atmel-isi.c
index e900995..eafdf91 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -1119,7 +1119,7 @@ static int isi_graph_parse(struct atmel_isi *isi, struct 
device_node *node)
if (!ep)
return -EINVAL;
 
-   remote = of_graph_get_remote_port_parent(ep);
+   remote = of_graph_get_remote_endpoint(ep);
if (!remote) {
of_node_put(ep);
return -EINVAL;
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index a89367a..e150d75 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1572,7 +1572,7 @@ vpif_capture_get_pdata(struct platform_device *pdev)
if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
chan->vpif_if.vd_pol = 1;
 
-   rem = of_graph_get_remote_port_parent(endpoint);
+   rem = of_graph_get_remote_endpoint(endpoint);
if (!rem) {
dev_dbg(>dev, "Remote device at %pOF not found\n",
endpoint);
diff --git a/drivers/media/platform/exynos4-is/media-dev.c 
b/drivers/media/platform/exynos4-is/media-dev.c
index c15596b..c6b0220 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -409,7 +409,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
 
pd->mux_id = (endpoint.base.port - 1) & 0x1;
 
-   rem = of_graph_get_remote_port_parent(ep);
+   rem = of_graph_get_remote_endpoint(ep);
of_node_put(ep);
if (rem == NULL) {
v4l2_info(>v4l2_dev, "Remote device at %pOF not found\n",
@@ -1360,11 +1360,17 @@ static int subdev_notifier_bound(struct 
v4l2_async_notifier *notifier,
int i;
 
/* Find platform data for this sensor subdev */
-   for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++)
-  

Re: [PATCH v4 03/12] [media] cxd2880: Add common files for the driver

2017-12-13 Thread Mauro Carvalho Chehab
Em Fri, 13 Oct 2017 15:02:59 +0900
 escreveu:

> From: Yasunari Takiguchi 
> 
> These are common files for the driver for the
> Sony CXD2880 DVB-T2/T tuner + demodulator.
> These contains helper functions for the driver.
> 
> Signed-off-by: Yasunari Takiguchi 
> Signed-off-by: Masayuki Yamamoto 
> Signed-off-by: Hideki Nozawa 
> Signed-off-by: Kota Yonezawa 
> Signed-off-by: Toshihiko Matsumoto 
> Signed-off-by: Satoshi Watanabe 
> ---
> 
> [Change list]
> Changes in V4
>drivers/media/dvb-frontends/cxd2880/cxd2880_io.c
>   -removed unnecessary initialization at variable declaration
>   -modified how to write consecutive registers
> 
> Changes in V3
>drivers/media/dvb-frontends/cxd2880/cxd2880.h
>   -no change
>drivers/media/dvb-frontends/cxd2880/cxd2880_common.c
>   -changed MASKUPPER/MASKLOWER with GENMASK 
>drivers/media/dvb-frontends/cxd2880/cxd2880_common.h
>   -removed definition NULL and SONY_SLEEP
>   -changed CXD2880_SLEEP to usleep_range
>   -changed cxd2880_atomic_set to atomic_set
>   -removed cxd2880_atomic struct and cxd2880_atomic_read
>   -changed stop-watch function
>   -modified return code
>drivers/media/dvb-frontends/cxd2880/cxd2880_io.c
>   -removed unnecessary cast
>   -modified return code
>   -changed hexadecimal code to lower case. 
>drivers/media/dvb-frontends/cxd2880/cxd2880_io.h
>   -modified return code 
>drivers/media/dvb-frontends/cxd2880/cxd2880_stopwatch_port.c
>   -changed CXD2880_SLEEP to usleep_range
>   -changed stop-watch function
>   -modified return code
>#drivers/media/dvb-frontends/cxd2880/cxd2880_stdlib.h
>   -cxd2880_stdlib.h file was removed from V3.
> 
>  drivers/media/dvb-frontends/cxd2880/cxd2880.h  | 46 +++
>  .../media/dvb-frontends/cxd2880/cxd2880_common.c   | 38 +
>  .../media/dvb-frontends/cxd2880/cxd2880_common.h   | 50 
>  drivers/media/dvb-frontends/cxd2880/cxd2880_io.c   | 89 
> ++
>  drivers/media/dvb-frontends/cxd2880/cxd2880_io.h   | 71 +
>  .../dvb-frontends/cxd2880/cxd2880_stopwatch_port.c | 60 +++
>  6 files changed, 354 insertions(+)
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_common.c
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_common.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_io.c
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_io.h
>  create mode 100644 
> drivers/media/dvb-frontends/cxd2880/cxd2880_stopwatch_port.c
> 
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880.h 
> b/drivers/media/dvb-frontends/cxd2880/cxd2880.h
> new file mode 100644
> index ..281f9a784eb5
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880.h
> @@ -0,0 +1,46 @@
> +/*
> + * cxd2880.h
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver public definitions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see .
> + */

Same comment made on patch 2 applies to this one and to the entire
series, with regards to SPDX.

> +
> +#ifndef CXD2880_H
> +#define CXD2880_H
> +
> +struct cxd2880_config {
> + struct spi_device *spi;
> + struct mutex *spi_mutex; /* For SPI access exclusive control */
> +};
> +
> +#if IS_REACHABLE(CONFIG_DVB_CXD2880)
> +extern struct dvb_frontend *cxd2880_attach(struct dvb_frontend *fe,
> + struct cxd2880_config *cfg);
> +#else
> +static inline struct dvb_frontend *cxd2880_attach(struct 

Re: [PATCH v4 02/12] [media] cxd2880-spi: Add support for CXD2880 SPI interface

2017-12-13 Thread Mauro Carvalho Chehab
Em Fri, 13 Oct 2017 14:59:28 +0900
 escreveu:

> From: Yasunari Takiguchi 
> 
> This is the SPI adapter part of the driver for the
> Sony CXD2880 DVB-T2/T tuner + demodulator.
> 
> Signed-off-by: Yasunari Takiguchi 
> Signed-off-by: Masayuki Yamamoto 
> Signed-off-by: Hideki Nozawa 
> Signed-off-by: Kota Yonezawa 
> Signed-off-by: Toshihiko Matsumoto 
> Signed-off-by: Satoshi Watanabe 
> ---
> 
> [Change list]
> Changes in V4
>drivers/media/spi/cxd2880-spi.c
>   -removed Camel case
>   -removed unnecessary initialization at variable declaration
>   -removed unnecessary brace {}
> 
> Changes in V3
>drivers/media/spi/cxd2880-spi.c
>   -adjusted of indent spaces
>   -removed unnecessary cast
>   -changed debugging code
>   -changed timeout method
>   -modified coding style of if()
>   -changed hexadecimal code to lower case. 
> 
> Changes in V2
>drivers/media/spi/cxd2880-spi.c
>   -Modified PID filter setting.
> 
>  drivers/media/spi/cxd2880-spi.c | 695 
> 
>  1 file changed, 695 insertions(+)
>  create mode 100644 drivers/media/spi/cxd2880-spi.c
> 
> diff --git a/drivers/media/spi/cxd2880-spi.c b/drivers/media/spi/cxd2880-spi.c
> new file mode 100644
> index ..387cb32f90b8
> --- /dev/null
> +++ b/drivers/media/spi/cxd2880-spi.c
> @@ -0,0 +1,695 @@
> +/*
> + * cxd2880-spi.c
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * SPI adapter
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see .
> + */

A minor issue (that it is not a show-stopper): we're now using SPDX
instead of repeating the copyright notes everywhere. Please look at
those articles for more details:

https://blogs.s-osg.org/linux-kernel-license-practices-revisited-spdx/
https://lwn.net/Articles/739183/

> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__

It would be better to use dev_foo() debug macros instead of
pr_foo() ones.

> +
> +#include 
> +#include 
> +
> +#include "dvb_demux.h"
> +#include "dmxdev.h"
> +#include "dvb_frontend.h"
> +#include "cxd2880.h"
> +
> +#define CXD2880_MAX_FILTER_SIZE 32
> +#define BURST_WRITE_MAX 128
> +#define MAX_TRANS_PACKET 300
> +
> +struct cxd2880_ts_buf_info {
> + u8 read_ready;
> + u8 almost_full;
> + u8 almost_empty;
> + u8 overflow;
> + u8 underflow;

Hmm... those seem to be booleans. The best is to declare them as:

u8 read_ready:1;
u8 almost_full:1;
u8 almost_empty:1;
u8 overflow:1;
u8 underflow:1;

or to use bool, in order to make it clearer.

> + u16 packet_num;
> +};
> +
> +struct cxd2880_pid_config {
> + u8 is_enable;
> + u16 pid;
> +};
> +
> +struct cxd2880_pid_filter_config {
> + u8 is_negative;
> + struct cxd2880_pid_config pid_config[CXD2880_MAX_FILTER_SIZE];
> +};
> +
> +struct cxd2880_dvb_spi {
> + struct dvb_frontend dvb_fe;
> + struct dvb_adapter adapter;
> + struct dvb_demux demux;
> + struct dmxdev dmxdev;
> + struct dmx_frontend dmx_fe;
> + struct task_struct *cxd2880_ts_read_thread;
> + struct spi_device *spi;
> + struct mutex spi_mutex; /* For SPI access exclusive control */
> + int feed_count;
> + int all_pid_feed_count;
> + u8 *ts_buf;
> + struct cxd2880_pid_filter_config filter_config;
> +};
> +
> +DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
> +
> +static int cxd2880_write_spi(struct spi_device *spi, u8 *data, u32 size)
> +{
> + struct spi_message msg;
> + struct spi_transfer tx;
> + int ret;
> +
> + if ((!spi) || (!data)) {
> + 

Re: [PATCH 2/2] [media] ddbridge: don't break on single/last port attach failure

2017-12-13 Thread Daniel Scheller
On Wed, 13 Dec 2017 13:26:02 -0200
Mauro Carvalho Chehab  wrote:

> Em Wed,  6 Dec 2017 18:59:15 +0100
> Daniel Scheller  escreveu:
> 
> > From: Daniel Scheller 
> > 
> > As all error handling improved quite a bit, don't stop attaching
> > frontends if one of them failed, since - if other tuner modules are
> > connected to the PCIe bridge - other hardware may just work, so
> > lets not break on a single port failure, but rather initialise as
> > much as possible. Ie. if there are issues with a C2T2-equipped PCIe
> > bridge card which has additional DuoFlex modules connected and the
> > bridge generally works, the DuoFlex tuners can still work fine.
> > Also, this only had an effect anyway if the failed device/port was
> > the last one being enumerated.
> > 
> > Signed-off-by: Daniel Scheller 
> > ---
> >  drivers/media/pci/ddbridge/ddbridge-core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c
> > b/drivers/media/pci/ddbridge/ddbridge-core.c index
> > 11c5cae92408..b43c40e0bf73 100644 ---
> > a/drivers/media/pci/ddbridge/ddbridge-core.c +++
> > b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -1962,7 +1962,7 @@
> > int ddb_ports_attach(struct ddb *dev) }
> > for (i = 0; i < dev->port_num; i++) {
> > port = >port[i];
> > -   ret = ddb_port_attach(port);
> > +   ddb_port_attach(port);  
> 
> Nah, ignoring an error doesn't seem right. It should at least print
> that attach failed.

This is already the case in ddb_port_attach() (if (ret < 0)
dev_err(...)).

> Also, if all attaches fail, probably the best
> would be to just detach everything and go to the error handling code,
> as there's something serious happening.

Well, will recheck the whole error handling there then when already at
it, as single port failures can still leave some half-initialised stuff
behind until ddbridge gets unloaded.

Thanks for your review, comments and your proposal!

Best regards,
Daniel Scheller
-- 
https://github.com/herrnst


[PATCH] media: ov9650: support VIDIOC_DBG_G/S_REGISTER ioctls

2017-12-13 Thread Akinobu Mita
This adds support VIDIOC_DBG_G/S_REGISTER ioctls.

There are many device control registers contained in the OV9650.  So
this helps debugging the lower level issues by getting and setting the
registers.

Cc: Sylwester Nawrocki 
Cc: Sakari Ailus 
Cc: Mauro Carvalho Chehab 
Signed-off-by: Akinobu Mita 
---
 drivers/media/i2c/ov9650.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 69433e1..c6462cf 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -1374,6 +1374,38 @@ static int ov965x_open(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh)
return 0;
 }
 
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+
+static int ov965x_g_register(struct v4l2_subdev *sd,
+struct v4l2_dbg_register *reg)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(sd);
+   u8 val = 0;
+   int ret;
+
+   if (reg->reg > 0xff)
+   return -EINVAL;
+
+   ret = ov965x_read(client, reg->reg, );
+   reg->val = val;
+   reg->size = 1;
+
+   return ret;
+}
+
+static int ov965x_s_register(struct v4l2_subdev *sd,
+const struct v4l2_dbg_register *reg)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+   if (reg->reg > 0xff || reg->val > 0xff)
+   return -EINVAL;
+
+   return ov965x_write(client, reg->reg, reg->val);
+}
+
+#endif
+
 static const struct v4l2_subdev_pad_ops ov965x_pad_ops = {
.enum_mbus_code = ov965x_enum_mbus_code,
.enum_frame_size = ov965x_enum_frame_sizes,
@@ -1397,6 +1429,10 @@ static const struct v4l2_subdev_core_ops ov965x_core_ops 
= {
.log_status = v4l2_ctrl_subdev_log_status,
.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
.unsubscribe_event = v4l2_event_subdev_unsubscribe,
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+   .g_register = ov965x_g_register,
+   .s_register = ov965x_s_register,
+#endif
 };
 
 static const struct v4l2_subdev_ops ov965x_subdev_ops = {
-- 
2.7.4



Re: [PATCH 1/2 v6] uvcvideo: send a control event when a Control Change interrupt arrives

2017-12-13 Thread Guennadi Liakhovetski
Sorry, forgot to mention a change from the previous version: now 
autoupdate Control Change events are also delivered.

Thanks
Guennadi

On Wed, 13 Dec 2017, Guennadi Liakhovetski wrote:

> UVC defines a method of handling asynchronous controls, which sends a
> USB packet over the interrupt pipe. This patch implements support for
> such packets by sending a control event to the user. Since this can
> involve USB traffic and, therefore, scheduling, this has to be done
> in a work queue.
> 
> Signed-off-by: Guennadi Liakhovetski 
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c   | 166 
> +
>  drivers/media/usb/uvc/uvc_status.c | 111 ++---
>  drivers/media/usb/uvc/uvc_v4l2.c   |   4 +-
>  drivers/media/usb/uvc/uvcvideo.h   |  15 +++-
>  include/uapi/linux/uvcvideo.h  |   2 +
>  5 files changed, 269 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c 
> b/drivers/media/usb/uvc/uvc_ctrl.c
> index 20397ab..2a592c2 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -1222,30 +1223,134 @@ static void uvc_ctrl_send_event(struct uvc_fh 
> *handle,
>  {
>   struct v4l2_subscribed_event *sev;
>   struct v4l2_event ev;
> + bool autoupdate;
>  
>   if (list_empty(>ev_subs))
>   return;
>  
> + if (!handle) {
> + autoupdate = true;
> + sev = list_first_entry(>ev_subs,
> +struct v4l2_subscribed_event, node);
> + handle = container_of(sev->fh, struct uvc_fh, vfh);
> + } else {
> + autoupdate = false;
> + }
> +
>   uvc_ctrl_fill_event(handle->chain, , ctrl, mapping, value, changes);
>  
>   list_for_each_entry(sev, >ev_subs, node) {
>   if (sev->fh && (sev->fh != >vfh ||
>   (sev->flags & V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK) ||
> - (changes & V4L2_EVENT_CTRL_CH_FLAGS)))
> + (changes & V4L2_EVENT_CTRL_CH_FLAGS) || autoupdate))
>   v4l2_event_queue_fh(sev->fh, );
>   }
>  }
>  
> -static void uvc_ctrl_send_slave_event(struct uvc_fh *handle,
> - struct uvc_control *master, u32 slave_id,
> - const struct v4l2_ext_control *xctrls, unsigned int xctrls_count)
> +static void __uvc_ctrl_send_slave_event(struct uvc_fh *handle,
> + struct uvc_control *master, u32 slave_id)
>  {
>   struct uvc_control_mapping *mapping = NULL;
>   struct uvc_control *ctrl = NULL;
>   u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
> - unsigned int i;
>   s32 val = 0;
>  
> + __uvc_find_control(master->entity, slave_id, , , 0);
> + if (ctrl == NULL)
> + return;
> +
> + if (__uvc_ctrl_get(handle->chain, ctrl, mapping, ) == 0)
> + changes |= V4L2_EVENT_CTRL_CH_VALUE;
> +
> + uvc_ctrl_send_event(handle, ctrl, mapping, val, changes);
> +}
> +
> +static void uvc_ctrl_status_event_work(struct work_struct *work)
> +{
> + struct uvc_device *dev = container_of(work, struct uvc_device,
> +   async_ctrl.work);
> + struct uvc_video_chain *chain;
> + struct uvc_ctrl_work *w = >async_ctrl;
> + struct uvc_control_mapping *mapping;
> + struct uvc_control *ctrl;
> + struct uvc_fh *handle;
> + __u8 *data;
> + unsigned int i;
> +
> + spin_lock_irq(>lock);
> + data = w->data;
> + w->data = NULL;
> + chain = w->chain;
> + ctrl = w->ctrl;
> + handle = ctrl->handle;
> + ctrl->handle = NULL;
> + spin_unlock_irq(>lock);
> +
> + if (mutex_lock_interruptible(>ctrl_mutex))
> + goto free;
> +
> + list_for_each_entry(mapping, >info.mappings, list) {
> + s32 value = mapping->get(mapping, UVC_GET_CUR, data);
> +
> + for (i = 0; i < ARRAY_SIZE(mapping->slave_ids); ++i) {
> + if (!mapping->slave_ids[i])
> + break;
> +
> + __uvc_ctrl_send_slave_event(handle, ctrl,
> + mapping->slave_ids[i]);
> + }
> +
> + if (mapping->v4l2_type == V4L2_CTRL_TYPE_MENU) {
> + struct uvc_menu_info *menu = mapping->menu_info;
> + unsigned int i;
> +
> + for (i = 0; i < mapping->menu_count; ++i, ++menu)
> + if (menu->value == value) {
> + value = i;
> + break;
> + }
> + }
> +
> + uvc_ctrl_send_event(handle, ctrl, mapping, value,
> + V4L2_EVENT_CTRL_CH_VALUE);
> + }
> +
> + mutex_unlock(>ctrl_mutex);
> +
> +free:
> + 

[ragnatech:media-tree] BUILD SUCCESS 330dada5957e3ca0c8811b14c45e3ac42c694651

2017-12-13 Thread kbuild test robot
tree/branch: git://git.ragnatech.se/linux  media-tree
branch HEAD: 330dada5957e3ca0c8811b14c45e3ac42c694651  media: dvb_frontend: fix 
return error code

elapsed time: 72m

configs tested: 622

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

pariscc3000_defconfig
parisc b180_defconfig
parisc  defconfig
alpha   defconfig
pariscallnoconfig
s390   gcov_defconfig
tile  allnoconfig
um   x86_64_defconfig
arm cm_x2xx_defconfig
armmmp2_defconfig
powerpc rainier_defconfig
arm  pcm027_defconfig
crisetrax-100lx_defconfig
powerpc sbc834x_defconfig
blackfinBF561-EZKIT-SMP_defconfig
m32rmappi2.opsp_defconfig
m68k   m5275evb_defconfig
x86_64 randconfig-a0-12131849
x86_64 randconfig-a0-12132016
x86_64 randconfig-a0-12132140
arm64   defconfig
powerpc canyonlands_defconfig
powerpc mpc8315_rdb_defconfig
arm orion5x_defconfig
powerpc  acadia_defconfig
shsh7785lcr_defconfig
blackfinDNP5370_defconfig
powerpc mpc5200_defconfig
sh   se7206_defconfig
arm  collie_defconfig
armmagician_defconfig
sh  polaris_defconfig
blackfin  BF561-ACVILON_defconfig
mips db1xxx_defconfig
powerpc  bamboo_defconfig
ia64sim_defconfig
m68kmvme147_defconfig
powerpc  mgcoge_defconfig
powerpcsam440ep_defconfig
shecovec24-romimage_defconfig
arm ezx_defconfig
mips  ath25_defconfig
sh apsh4a3a_defconfig
arm lpc32xx_defconfig
arm   netwinder_defconfig
powerpc  g5_defconfig
i386   randconfig-c0-12131617
i386   randconfig-c0-12131618
i386   randconfig-c0-12131619
i386   randconfig-c0-12131620
i386   randconfig-c0-12131622
i386   randconfig-c0-12131623
i386   randconfig-c0-12131624
i386   randconfig-c0-12131625
i386   randconfig-c0-12131626
i386   randconfig-c0-12131627
i386   randconfig-c0-12131629
i386   randconfig-c0-12131630
i386   randconfig-c0-12131631
i386   randconfig-c0-12131632
i386   randconfig-c0-12131633
i386   randconfig-c0-12131634
i386   randconfig-c0-12131635
i386   randconfig-c0-12131636
i386   randconfig-c0-12131637
i386   randconfig-c0-12131638
i386   randconfig-c0-12131639
i386   randconfig-c0-12131640
i386   randconfig-c0-12131641
i386   randconfig-c0-12131642
i386   randconfig-c0-12131643
i386   randconfig-c0-12131644
i386   randconfig-c0-12131645
i386   randconfig-c0-12131646
i386   randconfig-c0-12131647
i386   randconfig-c0-12131648
i386   randconfig-c0-12131649
i386   randconfig-c0-12131650
i386   randconfig-c0-12131651
i386   randconfig-c0-12131652
i386   randconfig-c0-12131653
i386   randconfig-c0-12131654
i386   randconfig-c0-12131953
i386   randconfig-c0-12132008
i386   randconfig-c0-12132125
i386 randconfig-x010-12131225
i386 randconfig-x011-12131225
i386 randconfig-x012-12131225
i386 randconfig-x013-12131225
i386 randconfig-x014-12131225
i386 randconfig-x015-12131225
i386 randconfig-x016-12131225
i386 randconfig-x017-12131225
i386 randconfig-x018-12131225
i386 randconfig-x019-12131225
x86_64lkp
x86_64   rhel
x86_64   rhel-7.2
microblaze  mmu_defconfig
microblazenommu_defconfig
i386 randconfig-n0-201750
ia64 alldefconfig
ia64 

Re: [PATCH v3 03/12] media: rkisp1: Add user space ABI definitions

2017-12-13 Thread Hans Verkuil
On 06/12/17 12:19, Jacob Chen wrote:
> From: Jeffy Chen 
> 
> Add the header for userspace
> 
> Signed-off-by: Jeffy Chen 
> Signed-off-by: Jacob Chen 
> ---
>  include/uapi/linux/rkisp1-config.h | 785 
> +
>  1 file changed, 785 insertions(+)
>  create mode 100644 include/uapi/linux/rkisp1-config.h
> 
> diff --git a/include/uapi/linux/rkisp1-config.h 
> b/include/uapi/linux/rkisp1-config.h
> new file mode 100644
> index ..82fecbee23a9
> --- /dev/null
> +++ b/include/uapi/linux/rkisp1-config.h
> @@ -0,0 +1,785 @@
> +/*
> + * Rockchip isp1 driver
> + *
> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
> + *
> + * This software is available to you under a choice of one of two
> + * licenses.  You may choose to be licensed under the terms of the GNU
> + * General Public License (GPL) Version 2, available from the file
> + * COPYING in the main directory of this source tree, or the
> + * OpenIB.org BSD license below:
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + *  - Redistributions of source code must retain the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer.
> + *
> + *  - Redistributions in binary form must reproduce the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer in the documentation and/or other materials
> + *provided with the distribution.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.

Please use the new SPDX license identifier.

> + */
> +
> +#ifndef _UAPI_RKISP1_CONFIG_H
> +#define _UAPI_RKISP1_CONFIG_H
> +
> +#include 
> +#include 
> +
> +#define CIFISP_MODULE_DPCC  (1 << 0)
> +#define CIFISP_MODULE_BLS   (1 << 1)
> +#define CIFISP_MODULE_SDG   (1 << 2)
> +#define CIFISP_MODULE_HST   (1 << 3)
> +#define CIFISP_MODULE_LSC   (1 << 4)
> +#define CIFISP_MODULE_AWB_GAIN  (1 << 5)
> +#define CIFISP_MODULE_FLT   (1 << 6)
> +#define CIFISP_MODULE_BDM   (1 << 7)
> +#define CIFISP_MODULE_CTK   (1 << 8)
> +#define CIFISP_MODULE_GOC   (1 << 9)
> +#define CIFISP_MODULE_CPROC (1 << 10)
> +#define CIFISP_MODULE_AFC   (1 << 11)
> +#define CIFISP_MODULE_AWB   (1 << 12)
> +#define CIFISP_MODULE_IE(1 << 13)
> +#define CIFISP_MODULE_AEC   (1 << 14)
> +#define CIFISP_MODULE_WDR   (1 << 15)
> +#define CIFISP_MODULE_DPF   (1 << 16)
> +#define CIFISP_MODULE_DPF_STRENGTH  (1 << 17)
> +
> +#define CIFISP_CTK_COEFF_MAX0x100
> +#define CIFISP_CTK_OFFSET_MAX   0x800
> +
> +#define CIFISP_AE_MEAN_MAX  25
> +#define CIFISP_HIST_BIN_N_MAX   16
> +#define CIFISP_AFM_MAX_WINDOWS  3
> +#define CIFISP_DEGAMMA_CURVE_SIZE   17
> +
> +#define CIFISP_BDM_MAX_TH   0xFF
> +
> +/*
> + * Black level compensation
> + */
> +/* maximum value for horizontal start address */
> +#define CIFISP_BLS_START_H_MAX 0x0FFF
> +/* maximum value for horizontal stop address */
> +#define CIFISP_BLS_STOP_H_MAX  0x0FFF
> +/* maximum value for vertical start address */
> +#define CIFISP_BLS_START_V_MAX 0x0FFF
> +/* maximum value for vertical stop address */
> +#define CIFISP_BLS_STOP_V_MAX  0x0FFF
> +/* maximum is 2^18 = 262144*/
> +#define CIFISP_BLS_SAMPLES_MAX 0x0012
> +/* maximum value for fixed black level */
> +#define CIFISP_BLS_FIX_SUB_MAX 0x0FFF
> +/* minimum value for fixed black level */
> +#define CIFISP_BLS_FIX_SUB_MIN 0xF000
> +/* 13 bit range (signed)*/
> +#define CIFISP_BLS_FIX_MASK0x1FFF
> +
> +/*
> + * Automatic white balance measurments
> + */
> +#define CIFISP_AWB_MAX_GRID1
> +#define CIFISP_AWB_MAX_FRAMES  7
> +
> +/*
> + * Gamma out
> + */
> +/* Maximum number of color samples supported */
> +#define CIFISP_GAMMA_OUT_MAX_SAMPLES   17
> +
> +/*
> + * Lens shade correction
> + */
> +#define CIFISP_LSC_GRAD_TBL_SIZE   8
> +#define CIFISP_LSC_SIZE_TBL_SIZE   8
> +/*
> + * The following matches the tuning process,
> + * not the max 

[PATCH 2/2 v6] uvcvideo: handle control pipe protocol STALLs

2017-12-13 Thread Guennadi Liakhovetski
When a command ends up in a STALL on the control pipe, use the Request
Error Code control to provide a more precise error information to the
user.

Signed-off-by: Guennadi Liakhovetski 
---
 drivers/media/usb/uvc/uvc_video.c | 59 +++
 1 file changed, 53 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c 
b/drivers/media/usb/uvc/uvc_video.c
index 2fc0bf2..cfcc4861 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -34,15 +34,59 @@ static int __uvc_query_ctrl(struct uvc_device *dev, __u8 
query, __u8 unit,
__u8 intfnum, __u8 cs, void *data, __u16 size,
int timeout)
 {
-   __u8 type = USB_TYPE_CLASS | USB_RECIP_INTERFACE;
+   __u8 type = USB_TYPE_CLASS | USB_RECIP_INTERFACE, tmp, error;
unsigned int pipe;
+   int ret;
 
pipe = (query & 0x80) ? usb_rcvctrlpipe(dev->udev, 0)
  : usb_sndctrlpipe(dev->udev, 0);
type |= (query & 0x80) ? USB_DIR_IN : USB_DIR_OUT;
 
-   return usb_control_msg(dev->udev, pipe, query, type, cs << 8,
+   ret = usb_control_msg(dev->udev, pipe, query, type, cs << 8,
unit << 8 | intfnum, data, size, timeout);
+
+   if (ret != -EPIPE)
+   return ret;
+
+   tmp = *(u8 *)data;
+
+   pipe = usb_rcvctrlpipe(dev->udev, 0);
+   type = USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN;
+   ret = usb_control_msg(dev->udev, pipe, UVC_GET_CUR, type,
+ UVC_VC_REQUEST_ERROR_CODE_CONTROL << 8,
+ unit << 8 | intfnum, data, 1, timeout);
+   error = *(u8 *)data;
+   *(u8 *)data = tmp;
+
+   if (ret < 0)
+   return ret;
+
+   if (!ret)
+   return -EINVAL;
+
+   uvc_trace(UVC_TRACE_CONTROL, "Control error %u\n", error);
+
+   switch (error) {
+   case 0:
+   /* Cannot happen - we received a STALL */
+   return -EPIPE;
+   case 1: /* Not ready */
+   return -EAGAIN;
+   case 2: /* Wrong state */
+   return -EILSEQ;
+   case 3: /* Power */
+   return -EREMOTE;
+   case 4: /* Out of range */
+   return -ERANGE;
+   case 5: /* Invalid unit */
+   case 6: /* Invalid control */
+   case 7: /* Invalid Request */
+   case 8: /* Invalid value within range */
+   default: /* reserved or unknown */
+   break;
+   }
+
+   return -EINVAL;
 }
 
 static const char *uvc_query_name(__u8 query)
@@ -80,7 +124,7 @@ int uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 
unit,
uvc_printk(KERN_ERR, "Failed to query (%s) UVC control %u on "
"unit %u: %d (exp. %u).\n", uvc_query_name(query), cs,
unit, ret, size);
-   return -EIO;
+   return ret < 0 ? ret : -EIO;
}
 
return 0;
@@ -203,13 +247,15 @@ static int uvc_get_video_ctrl(struct uvc_streaming 
*stream,
uvc_warn_once(stream->dev, UVC_WARN_PROBE_DEF, "UVC non "
"compliance - GET_DEF(PROBE) not supported. "
"Enabling workaround.\n");
-   ret = -EIO;
+   if (ret >= 0)
+   ret = -EIO;
goto out;
} else if (ret != size) {
uvc_printk(KERN_ERR, "Failed to query (%u) UVC %s control : "
"%d (exp. %u).\n", query, probe ? "probe" : "commit",
ret, size);
-   ret = -EIO;
+   if (ret >= 0)
+   ret = -EIO;
goto out;
}
 
@@ -290,7 +336,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
uvc_printk(KERN_ERR, "Failed to set UVC %s control : "
"%d (exp. %u).\n", probe ? "probe" : "commit",
ret, size);
-   ret = -EIO;
+   if (ret >= 0)
+   ret = -EIO;
}
 
kfree(data);
-- 
1.9.3



[PATCH 1/2 v6] uvcvideo: send a control event when a Control Change interrupt arrives

2017-12-13 Thread Guennadi Liakhovetski
UVC defines a method of handling asynchronous controls, which sends a
USB packet over the interrupt pipe. This patch implements support for
such packets by sending a control event to the user. Since this can
involve USB traffic and, therefore, scheduling, this has to be done
in a work queue.

Signed-off-by: Guennadi Liakhovetski 
---
 drivers/media/usb/uvc/uvc_ctrl.c   | 166 +
 drivers/media/usb/uvc/uvc_status.c | 111 ++---
 drivers/media/usb/uvc/uvc_v4l2.c   |   4 +-
 drivers/media/usb/uvc/uvcvideo.h   |  15 +++-
 include/uapi/linux/uvcvideo.h  |   2 +
 5 files changed, 269 insertions(+), 29 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 20397ab..2a592c2 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -1222,30 +1223,134 @@ static void uvc_ctrl_send_event(struct uvc_fh *handle,
 {
struct v4l2_subscribed_event *sev;
struct v4l2_event ev;
+   bool autoupdate;
 
if (list_empty(>ev_subs))
return;
 
+   if (!handle) {
+   autoupdate = true;
+   sev = list_first_entry(>ev_subs,
+  struct v4l2_subscribed_event, node);
+   handle = container_of(sev->fh, struct uvc_fh, vfh);
+   } else {
+   autoupdate = false;
+   }
+
uvc_ctrl_fill_event(handle->chain, , ctrl, mapping, value, changes);
 
list_for_each_entry(sev, >ev_subs, node) {
if (sev->fh && (sev->fh != >vfh ||
(sev->flags & V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK) ||
-   (changes & V4L2_EVENT_CTRL_CH_FLAGS)))
+   (changes & V4L2_EVENT_CTRL_CH_FLAGS) || autoupdate))
v4l2_event_queue_fh(sev->fh, );
}
 }
 
-static void uvc_ctrl_send_slave_event(struct uvc_fh *handle,
-   struct uvc_control *master, u32 slave_id,
-   const struct v4l2_ext_control *xctrls, unsigned int xctrls_count)
+static void __uvc_ctrl_send_slave_event(struct uvc_fh *handle,
+   struct uvc_control *master, u32 slave_id)
 {
struct uvc_control_mapping *mapping = NULL;
struct uvc_control *ctrl = NULL;
u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
-   unsigned int i;
s32 val = 0;
 
+   __uvc_find_control(master->entity, slave_id, , , 0);
+   if (ctrl == NULL)
+   return;
+
+   if (__uvc_ctrl_get(handle->chain, ctrl, mapping, ) == 0)
+   changes |= V4L2_EVENT_CTRL_CH_VALUE;
+
+   uvc_ctrl_send_event(handle, ctrl, mapping, val, changes);
+}
+
+static void uvc_ctrl_status_event_work(struct work_struct *work)
+{
+   struct uvc_device *dev = container_of(work, struct uvc_device,
+ async_ctrl.work);
+   struct uvc_video_chain *chain;
+   struct uvc_ctrl_work *w = >async_ctrl;
+   struct uvc_control_mapping *mapping;
+   struct uvc_control *ctrl;
+   struct uvc_fh *handle;
+   __u8 *data;
+   unsigned int i;
+
+   spin_lock_irq(>lock);
+   data = w->data;
+   w->data = NULL;
+   chain = w->chain;
+   ctrl = w->ctrl;
+   handle = ctrl->handle;
+   ctrl->handle = NULL;
+   spin_unlock_irq(>lock);
+
+   if (mutex_lock_interruptible(>ctrl_mutex))
+   goto free;
+
+   list_for_each_entry(mapping, >info.mappings, list) {
+   s32 value = mapping->get(mapping, UVC_GET_CUR, data);
+
+   for (i = 0; i < ARRAY_SIZE(mapping->slave_ids); ++i) {
+   if (!mapping->slave_ids[i])
+   break;
+
+   __uvc_ctrl_send_slave_event(handle, ctrl,
+   mapping->slave_ids[i]);
+   }
+
+   if (mapping->v4l2_type == V4L2_CTRL_TYPE_MENU) {
+   struct uvc_menu_info *menu = mapping->menu_info;
+   unsigned int i;
+
+   for (i = 0; i < mapping->menu_count; ++i, ++menu)
+   if (menu->value == value) {
+   value = i;
+   break;
+   }
+   }
+
+   uvc_ctrl_send_event(handle, ctrl, mapping, value,
+   V4L2_EVENT_CTRL_CH_VALUE);
+   }
+
+   mutex_unlock(>ctrl_mutex);
+
+free:
+   kfree(data);
+}
+
+void uvc_ctrl_status_event(struct uvc_video_chain *chain,
+  struct uvc_control *ctrl, __u8 *data, size_t len)
+{
+   struct uvc_device *dev = chain->dev;
+   struct uvc_ctrl_work *w = >async_ctrl;
+
+   if (list_empty(>info.mappings))
+   return;
+

[PATCH 0/2 v6] uvcvideo: asynchronous controls

2017-12-13 Thread Guennadi Liakhovetski
This is an update of the two patches, adding asynchronous control
support to the uvcvideo driver. If a control is sent, while the camera
is still processing an earlier control, it will generate a protocol
STALL condition on the control pipe.

Thanks
Guennadi

Guennadi Liakhovetski (2):
  uvcvideo: send a control event when a Control Change interrupt arrives
  uvcvideo: handle control pipe protocol STALLs

 drivers/media/usb/uvc/uvc_ctrl.c   | 166 +
 drivers/media/usb/uvc/uvc_status.c | 111 ++---
 drivers/media/usb/uvc/uvc_v4l2.c   |   4 +-
 drivers/media/usb/uvc/uvc_video.c  |  59 +++--
 drivers/media/usb/uvc/uvcvideo.h   |  15 +++-
 include/uapi/linux/uvcvideo.h  |   2 +
 6 files changed, 322 insertions(+), 35 deletions(-)

-- 
1.9.3



Re: [PATCH 2/2] [media] ddbridge: don't break on single/last port attach failure

2017-12-13 Thread Mauro Carvalho Chehab
Em Wed,  6 Dec 2017 18:59:15 +0100
Daniel Scheller  escreveu:

> From: Daniel Scheller 
> 
> As all error handling improved quite a bit, don't stop attaching frontends
> if one of them failed, since - if other tuner modules are connected to
> the PCIe bridge - other hardware may just work, so lets not break on a
> single port failure, but rather initialise as much as possible. Ie. if
> there are issues with a C2T2-equipped PCIe bridge card which has
> additional DuoFlex modules connected and the bridge generally works,
> the DuoFlex tuners can still work fine. Also, this only had an effect
> anyway if the failed device/port was the last one being enumerated.
> 
> Signed-off-by: Daniel Scheller 
> ---
>  drivers/media/pci/ddbridge/ddbridge-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c 
> b/drivers/media/pci/ddbridge/ddbridge-core.c
> index 11c5cae92408..b43c40e0bf73 100644
> --- a/drivers/media/pci/ddbridge/ddbridge-core.c
> +++ b/drivers/media/pci/ddbridge/ddbridge-core.c
> @@ -1962,7 +1962,7 @@ int ddb_ports_attach(struct ddb *dev)
>   }
>   for (i = 0; i < dev->port_num; i++) {
>   port = >port[i];
> - ret = ddb_port_attach(port);
> + ddb_port_attach(port);

Nah, ignoring an error doesn't seem right. It should at least print
that attach failed. Also, if all attaches fail, probably the best
would be to just detach everything and go to the error handling code,
as there's something serious happening.

Something like:

for (i = 0; i < dev->port_num; i++) {
port = >port[i];
ret = ddb_port_attach(port);
if (ret) {
dev_warn(port->dev->dev, "attach failed\n");
err_ports++;
}
if (err_ports == dev->port_num)
return -ENODEV;

Thanks,
Mauro


Re: [PATCH 1/2] media: dt-bindings: coda: Add compatible for CodaHx4 on i.MX51

2017-12-13 Thread Philipp Zabel
Hi Baruch,

On Wed, 2017-12-13 at 16:21 +0200, Baruch Siach wrote:
> Hi Philipp,
> 
> On Wed, Dec 13, 2017 at 03:09:17PM +0100, Philipp Zabel wrote:
> > Add a compatible for the CodaHx4 VPU used on i.MX51.
> > 
> > Signed-off-by: Philipp Zabel 
> > ---
> >  Documentation/devicetree/bindings/media/coda.txt | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/coda.txt 
> > b/Documentation/devicetree/bindings/media/coda.txt
> > index 2865d04e40305..660f5ecf2a23b 100644
> > --- a/Documentation/devicetree/bindings/media/coda.txt
> > +++ b/Documentation/devicetree/bindings/media/coda.txt
> > @@ -7,6 +7,7 @@ called VPU (Video Processing Unit).
> >  Required properties:
> >  - compatible : should be "fsl,-src" for i.MX SoCs:
> >(a) "fsl,imx27-vpu" for CodaDx6 present in i.MX27
> > +  (a) "fsl,imx51-vpu" for CodaHx4 present in i.MX51
> 
> Renumbering the strings might be useful.

yes, thank you. I should probably add the generic "cnm,coda"
compatibles as well. I'll send a fixed version.

regards
Philipp


Re: [PATCH 0/3] Add support compat in dvb_frontend.c

2017-12-13 Thread Mauro Carvalho Chehab
Em Sat, 2 Dec 2017 05:50:47 -0200
Mauro Carvalho Chehab  escreveu:

> Hi Jaedon,
> 
> Em Fri,  1 Dec 2017 21:31:27 +0900
> Jaedon Shin  escreveu:
> 
> > This patch series supports compat ioctl for 32-bit user space applications
> > in 64-bit system.
> > 
> > Jaedon Shin (3):
> >   media: dvb_frontend: Add unlocked_ioctl in dvb_frontend.c
> >   media: dvb_frontend: Add compat_ioctl callback
> >   media: dvb_frontend: Add commands implementation for compat ioct  
> 
> Thanks for the series. Yeah, indeed we need something like that.
> 
> Yet, I suspect that you should also move the logic inside
> dvb_frontend_handle_ioctl() with copies from/to userspace.
> 
> We don't want the logic there to be called when a 32-bit userspace
> copy happens, as it should now use the new compat32 code.

Nevermind. After reviewing it carefully, the way you handled is OK.

It seems that only thing you missed were to add an include
to linux/compat.h.

I'll add it and apply this patchset.

Thanks,
Mauro


Re: [PATCH 1/2] media: dt-bindings: coda: Add compatible for CodaHx4 on i.MX51

2017-12-13 Thread Baruch Siach
Hi Philipp,

On Wed, Dec 13, 2017 at 03:09:17PM +0100, Philipp Zabel wrote:
> Add a compatible for the CodaHx4 VPU used on i.MX51.
> 
> Signed-off-by: Philipp Zabel 
> ---
>  Documentation/devicetree/bindings/media/coda.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/coda.txt 
> b/Documentation/devicetree/bindings/media/coda.txt
> index 2865d04e40305..660f5ecf2a23b 100644
> --- a/Documentation/devicetree/bindings/media/coda.txt
> +++ b/Documentation/devicetree/bindings/media/coda.txt
> @@ -7,6 +7,7 @@ called VPU (Video Processing Unit).
>  Required properties:
>  - compatible : should be "fsl,-src" for i.MX SoCs:
>(a) "fsl,imx27-vpu" for CodaDx6 present in i.MX27
> +  (a) "fsl,imx51-vpu" for CodaHx4 present in i.MX51

Renumbering the strings might be useful.

>(b) "fsl,imx53-vpu" for CODA7541 present in i.MX53
>(c) "fsl,imx6q-vpu" for CODA960 present in i.MX6q
>  - reg: should be register base and length as documented in the

baruch

-- 
 http://baruch.siach.name/blog/  ~. .~   Tk Open Systems
=}ooO--U--Ooo{=
   - bar...@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -


[PATCH 1/2] media: dt-bindings: coda: Add compatible for CodaHx4 on i.MX51

2017-12-13 Thread Philipp Zabel
Add a compatible for the CodaHx4 VPU used on i.MX51.

Signed-off-by: Philipp Zabel 
---
 Documentation/devicetree/bindings/media/coda.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/coda.txt 
b/Documentation/devicetree/bindings/media/coda.txt
index 2865d04e40305..660f5ecf2a23b 100644
--- a/Documentation/devicetree/bindings/media/coda.txt
+++ b/Documentation/devicetree/bindings/media/coda.txt
@@ -7,6 +7,7 @@ called VPU (Video Processing Unit).
 Required properties:
 - compatible : should be "fsl,-src" for i.MX SoCs:
   (a) "fsl,imx27-vpu" for CodaDx6 present in i.MX27
+  (a) "fsl,imx51-vpu" for CodaHx4 present in i.MX51
   (b) "fsl,imx53-vpu" for CODA7541 present in i.MX53
   (c) "fsl,imx6q-vpu" for CODA960 present in i.MX6q
 - reg: should be register base and length as documented in the
-- 
2.11.0



[PATCH 2/2] media: coda: Add i.MX51 (CodaHx4) support

2017-12-13 Thread Philipp Zabel
Add support for the CodaHx4 VPU used on i.MX51.

Decoding h.264, MPEG-4, and MPEG-2 video works, as well as encoding
h.264. MPEG-4 encoding is not enabled, it currently produces visual
artifacts.

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda/coda-bit.c| 45 ++-
 drivers/media/platform/coda/coda-common.c | 44 +++---
 drivers/media/platform/coda/coda.h|  1 +
 3 files changed, 74 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c 
b/drivers/media/platform/coda/coda-bit.c
index bfc4ecf6f068b..393d8a1a2a67c 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -68,8 +68,9 @@ static void coda_command_async(struct coda_ctx *ctx, int cmd)
 {
struct coda_dev *dev = ctx->dev;
 
-   if (dev->devtype->product == CODA_960 ||
-   dev->devtype->product == CODA_7541) {
+   if (dev->devtype->product == CODA_HX4 ||
+   dev->devtype->product == CODA_7541 ||
+   dev->devtype->product == CODA_960) {
/* Restore context related registers to CODA */
coda_write(dev, ctx->bit_stream_param,
CODA_REG_BIT_BIT_STREAM_PARAM);
@@ -505,7 +506,8 @@ static int coda_alloc_context_buffers(struct coda_ctx *ctx,
goto err;
}
 
-   if (!ctx->psbuf.vaddr && dev->devtype->product == CODA_7541) {
+   if (!ctx->psbuf.vaddr && (dev->devtype->product == CODA_HX4 ||
+ dev->devtype->product == CODA_7541)) {
ret = coda_alloc_context_buf(ctx, >psbuf,
 CODA7_PS_BUF_SIZE, "psbuf");
if (ret < 0)
@@ -593,6 +595,7 @@ static void coda_setup_iram(struct coda_ctx *ctx)
int dbk_bits;
int bit_bits;
int ip_bits;
+   int me_bits;
 
memset(iram_info, 0, sizeof(*iram_info));
iram_info->next_paddr = dev->iram.paddr;
@@ -602,10 +605,17 @@ static void coda_setup_iram(struct coda_ctx *ctx)
return;
 
switch (dev->devtype->product) {
+   case CODA_HX4:
+   dbk_bits = CODA7_USE_HOST_DBK_ENABLE;
+   bit_bits = CODA7_USE_HOST_BIT_ENABLE;
+   ip_bits = CODA7_USE_HOST_IP_ENABLE;
+   me_bits = CODA7_USE_HOST_ME_ENABLE;
+   break;
case CODA_7541:
dbk_bits = CODA7_USE_HOST_DBK_ENABLE | CODA7_USE_DBK_ENABLE;
bit_bits = CODA7_USE_HOST_BIT_ENABLE | CODA7_USE_BIT_ENABLE;
ip_bits = CODA7_USE_HOST_IP_ENABLE | CODA7_USE_IP_ENABLE;
+   me_bits = CODA7_USE_HOST_ME_ENABLE | CODA7_USE_ME_ENABLE;
break;
case CODA_960:
dbk_bits = CODA9_USE_HOST_DBK_ENABLE | CODA9_USE_DBK_ENABLE;
@@ -625,7 +635,8 @@ static void coda_setup_iram(struct coda_ctx *ctx)
w64 = mb_width * 64;
 
/* Prioritize in case IRAM is too small for everything */
-   if (dev->devtype->product == CODA_7541) {
+   if (dev->devtype->product == CODA_HX4 ||
+   dev->devtype->product == CODA_7541) {
iram_info->search_ram_size = round_up(mb_width * 16 *
  36 + 2048, 1024);
iram_info->search_ram_paddr = coda_iram_alloc(iram_info,
@@ -634,8 +645,7 @@ static void coda_setup_iram(struct coda_ctx *ctx)
pr_err("IRAM is smaller than the search ram 
size\n");
goto out;
}
-   iram_info->axi_sram_use |= CODA7_USE_HOST_ME_ENABLE |
-  CODA7_USE_ME_ENABLE;
+   iram_info->axi_sram_use |= me_bits;
}
 
/* Only H.264BP and H.263P3 are considered */
@@ -687,7 +697,8 @@ static void coda_setup_iram(struct coda_ctx *ctx)
v4l2_dbg(1, coda_debug, >dev->v4l2_dev,
 "IRAM smaller than needed\n");
 
-   if (dev->devtype->product == CODA_7541) {
+   if (dev->devtype->product == CODA_HX4 ||
+   dev->devtype->product == CODA_7541) {
/* TODO - Enabling these causes picture errors on CODA7541 */
if (ctx->inst_type == CODA_INST_DECODER) {
/* fw 1.4.50 */
@@ -705,6 +716,7 @@ static void coda_setup_iram(struct coda_ctx *ctx)
 
 static u32 coda_supported_firmwares[] = {
CODA_FIRMWARE_VERNUM(CODA_DX6, 2, 2, 5),
+   CODA_FIRMWARE_VERNUM(CODA_HX4, 1, 4, 50),
CODA_FIRMWARE_VERNUM(CODA_7541, 1, 4, 50),
CODA_FIRMWARE_VERNUM(CODA_960, 2, 1, 5),
CODA_FIRMWARE_VERNUM(CODA_960, 2, 3, 10),
@@ -889,6 +901,7 @@ static int coda_start_encoding(struct coda_ctx *ctx)
case CODA_960:
 

[BUG] cx88: a possible sleep-in-atomic bug in snd_cx88_switch_put

2017-12-13 Thread Jia-Ju Bai

The driver may sleep under a spinlock.
The function call path is:
snd_cx88_switch_put (acquire the spinlock)
  v4l2_ctrl_find
mutex_lock --> may sleep

I do not find a good way to fix it, so I only report.
This possible bug is found by my static analysis tool (DSAC) and checked 
by my code review.



Thanks,
Jia-Ju Bai


Re: [PATCH v10 4/4] [media] platform: Add Synopsys DesignWare HDMI RX Controller Driver

2017-12-13 Thread Jose Abreu
Hi Hans,

On 13-12-2017 10:00, Hans Verkuil wrote:
> On 12/12/17 17:02, Jose Abreu wrote:
>>
 +static int dw_hdmi_s_routing(struct v4l2_subdev *sd, u32 input, u32 
 output,
 +  u32 config)
 +{
 +  struct dw_hdmi_dev *dw_dev = to_dw_dev(sd);
 +
 +  if (!has_signal(dw_dev, input))
 +  return -EINVAL;
>>> Why would this be a reason to reject this? There may be no signal now, but 
>>> a signal
>>> might appear later.
>> I would expect s_routing to only be called if there is an input
>> connected, otherwise we are just wasting resources by trying to
>> equalize an input that is not present ... I can remove the "if"
>> as there are other safe guards for this though (for example g_fmt
>> will return an error) ...
> No, s_routing is typically called as a result of a VIDIOC_S_INPUT
> call, and that can come whether or not there is a signal on an
> input. In fact, initially the first input is always selected anyway,
> whether or not there is a signal.
>
> g_fmt will just return the current configured format, this is unrelated
> to whether or not there is a signal.
>
> The only times the driver checks whether or not there is a signal (and
> what that is) are:
>
> 1) g_input_status
> 2) query_dv_timings
> 3) when the irq detects a signal change and sends V4L2_EVENT_SOURCE_CHANGE

Ok, I will remove the checks then.

>
 +  msleep(50); /* Wait for 1 field */
>>> How do you know this waits for 1 field? Or is this: "Wait for at least 1 
>>> field"?
>> Its wait at least for 1 field. This is over-generous because its
>> assuming the frame rate is 20fps (which in HDMI does not happen).
> With custom timings it can happen (i.e. a 15 fps stream). Admittedly, it's not
> common, but people sometimes use it.
>
>>> I don't know exactly how the IP does this, but it looks fishy to me. If it 
>>> is
>>> correct, then it could use a few comments about what is going on here as it 
>>> is
>>> not obvious.
>> The IP updates the values at each field but I need to change this
>> register to populate all values in the bt struct.
> How do you know which field (top or bottom) you've captured? How do you know 
> you
> didn't miss e.g. the bottom field and instead end up with two top field 
> measurements?
>
> The top and bottom field are almost, but not quite the same. Typically the 
> vertical
> backporch of the fields differs by 1 where the second field's backporch is 
> larger by 1 line.
>
>>> And what happens if the framerate is even slower? You know the pixelclock 
>>> and
>>> total width+height, so you can calculate the framerate from that.
>> Hmm, but then I have to consider pixelclk error, msleep error, ...
> But you have that now as well.
>
> An alternative is to measure a single field and deduce the backporch values 
> from that.
>
> At least for all the common HDMI interlaced formats il_vbackporch is an even 
> value and
> vbackporch is il_vbackporch - 1.
>
> So if you get an even backporch, then you found il_vbackporch, and if it is 
> odd, then
> you found vbackporch.
>
 +  bt->vsync = hdmi_readl(dw_dev, HDMI_MD_VOL);
 +
 +  hdmi_mask_writel(dw_dev, 0x0, HDMI_MD_VCTRL,
 +  HDMI_MD_VCTRL_V_OFFS_LIN_MODE_OFFSET,
 +  HDMI_MD_VCTRL_V_OFFS_LIN_MODE_MASK);
 +  msleep(50); /* Wait for 1 field */
 +  bt->vbackporch = hdmi_readl(dw_dev, HDMI_MD_VOL);
 +  bt->vfrontporch = vtot - bt->height - bt->vsync - bt->vbackporch;
 +
 +  if (bt->interlaced == V4L2_DV_INTERLACED) {
 +  hdmi_mask_writel(dw_dev, 0x1, HDMI_MD_VCTRL,
 +  HDMI_MD_VCTRL_V_MODE_OFFSET,
 +  HDMI_MD_VCTRL_V_MODE_MASK);
 +  msleep(100); /* Wait for 2 fields */
 +
 +  vtot = hdmi_readl(dw_dev, HDMI_MD_VTL);
 +  hdmi_mask_writel(dw_dev, 0x1, HDMI_MD_VCTRL,
 +  HDMI_MD_VCTRL_V_OFFS_LIN_MODE_OFFSET,
 +  HDMI_MD_VCTRL_V_OFFS_LIN_MODE_MASK);
 +  msleep(50); /* Wait for 1 field */
 +  bt->il_vsync = hdmi_readl(dw_dev, HDMI_MD_VOL);
 +
 +  hdmi_mask_writel(dw_dev, 0x0, HDMI_MD_VCTRL,
 +  HDMI_MD_VCTRL_V_OFFS_LIN_MODE_OFFSET,
 +  HDMI_MD_VCTRL_V_OFFS_LIN_MODE_MASK);
 +  msleep(50);
 +  bt->il_vbackporch = hdmi_readl(dw_dev, HDMI_MD_VOL);
 +  bt->il_vfrontporch = vtot - bt->height - bt->il_vsync -
 +  bt->il_vbackporch;
 +
 +  hdmi_mask_writel(dw_dev, 0x0, HDMI_MD_VCTRL,
 +  HDMI_MD_VCTRL_V_MODE_OFFSET,
 +  HDMI_MD_VCTRL_V_MODE_MASK);
>>> Same here, I'm not sure this is correct. What is the output of
>>> 'v4l2-ctl --query-dv-timings' when you feed it a standard interlaced format?
>> I used v4l2-ctl --log-status with interlaced format and
>> everything seemed correct ...
> Can you show a 

Re: [bug report] media: lirc: improve locking

2017-12-13 Thread Sean Young
On Wed, Dec 13, 2017 at 01:07:31PM +0300, Dan Carpenter wrote:
> Hello Sean Young,
> 
> The patch 131fd7fc3c01: "media: lirc: improve locking" from Nov 4,
> 2017, leads to the following static checker warning:
> 
>   drivers/media/rc/lirc_dev.c:373 ir_lirc_transmit_ir()
>   error: 'txbuf' dereferencing possible ERR_PTR()
> 
> drivers/media/rc/lirc_dev.c
>330  txbuf = memdup_user(buf, n);
>331  if (IS_ERR(txbuf)) {
>332  ret = PTR_ERR(txbuf);
>333  goto out;
> 
> This used to be a direct return...
> 
>334  }
>335  }
>336  
>337  for (i = 0; i < count; i++) {
>338  if (txbuf[i] > IR_MAX_DURATION / 1000 - duration || 
> !txbuf[i]) {
>339  ret = -EINVAL;
>340  goto out;
>341  }
>342  
>343  duration += txbuf[i];
>344  }
>345  
>346  ret = dev->tx_ir(dev, txbuf, count);
>347  if (ret < 0)
>348  goto out;
>349  
>350  if (fh->send_mode == LIRC_MODE_SCANCODE) {
>351  ret = n;
>352  } else {
>353  for (duration = i = 0; i < ret; i++)
>354  duration += txbuf[i];
>355  
>356  ret *= sizeof(unsigned int);
>357  
>358  /*
>359   * The lircd gap calculation expects the write 
> function to
>360   * wait for the actual IR signal to be transmitted 
> before
>361   * returning.
>362   */
>363  towait = ktime_us_delta(ktime_add_us(start, duration),
>364  ktime_get());
>365  if (towait > 0) {
>366  set_current_state(TASK_INTERRUPTIBLE);
>367  schedule_timeout(usecs_to_jiffies(towait));
> ^
> This looks like a long wait?  Are you sure you want to hold the lock
> this whole time?

Good point, there is no need for that.

>368  }
>369  }
>370  
>371  out:
>372  mutex_unlock(>lock);
>373  kfree(txbuf);
>   ^
> Can't pass an error pointer to kfree().

Yep.

Thank you for the report. I'll write patch.

Thanks

Sean


Re: [PATCH/RFC] not use a DiSEqC switch

2017-12-13 Thread Mauro Carvalho Chehab
Em Mon, 27 Nov 2017 17:26:07 -0200
Mauro Carvalho Chehab  escreveu:

> Em Fri, 24 Nov 2017 10:52:04 +0200
> Maksym Veremeyenko  escreveu:
> 
> > Hi,
> > 
> > there is a code in function *dvbsat_diseqc_set_input*:
> > 
> > [...]
> > /* Negative numbers means to not use a DiSEqC switch */
> > if (parms->p.sat_number < 0)
> > return 0;
> > [...]
> > 
> > if it mean /there is no DiSEqC switch/ then LNB's *polarity* and *band* 
> > settings still should be applied - attached patch fixes that behavior.
> > 
> > if it mean /current DVB is a slave/ i.e. it is connected to LOOP OUT of 
> > another DVB, so no need to configure anything, then statement above is 
> > correct and no patches from this email should be applied.  
> 
> No, it actually means that there's no DiSEqC at all; the LNBf
> is a bandstacking one, where different polarities use different
> LO, like on those LNBf:
> 
>   {
>   .desc = {
>   .name = N_("Big Dish - Multipoint LNBf"),
>   .alias = "C-MULT",
>   },
>   .freqrange = {
>   { 3700, 4200, 5150, 0, POLARIZATION_R },
>   { 3700, 4200, 5750, 0, POLARIZATION_L }
>   },
>   }, {
> 
>   .desc = {
>   .name = N_("BrasilSat Amazonas 1/2 - 2 Oscilators"),
>   .alias = "AMAZONAS",
>   },
>   .freqrange = {
>   { 11037, 11360, 9670, 0, POLARIZATION_V },
>   { 11780, 12150, 1, 0, POLARIZATION_H },
>   { 10950, 11280, 1, 0, POLARIZATION_H },
>   },
>   },
> 
> 
> The case where the LNBf accepts DiSEqC commands, but there's no
> switch will work just fine, as the switch control data will be
> silently ignored.
> 
> Ok, removing them could reduce a little bit the tuning time, at
> the expense of making harder for the user, as he would need to
> select between 4 different DiSEqC situations:
> 
>   - no DiSEqC at all;
>   - DiSEqC LNbf, no DiSEqC switch;
>   - DiSEqC LNbf, DiSEqC switch with 2 ports (miniDiSEqC);
>   - DiSEqC LNbf, DiSEqC switch with 4 ports.
> 
> The way the code is, if DiSEqC is selected, it will send both
> mini-DiSEqC (if satellite number < 2) and DiSEqC commands, so, 
> all 3 DiSEqC cases will be covered by just one configuration.

After revising this and doing some tests with the help of
Rafaël, it actually mades sense to apply something like that,
but adding an extra check for SCR/Unicable case.

Just added the patches today. Sorry for the noise.

Thanks,
Mauro


Re: [PATCH] dvb-sat: do the best to tune if DiSEqC is disabled

2017-12-13 Thread Rafaël Carré
On 13/12/2017 13:58, Mauro Carvalho Chehab wrote:
> If sat_number is not filled (e.g. it is -1), the dvb-sat
> disables DiSEqC. However, currently, it also breaks support
> for non-bandstacking LNBf.
> 
> Change the logic to fix it. There is a drawback on this
> approach, though: usually, on bandstacking arrangements,
> only one device needs to feed power to the LNBf. The
> others don't need to send power. With the previous code,
> no power would be sent at all, if sat_number == -1.
> 
> Now, it will always power the LNBf when using it.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  lib/libdvbv5/dvb-sat.c | 48 
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/lib/libdvbv5/dvb-sat.c b/lib/libdvbv5/dvb-sat.c
> index b012318c4195..8c04f66f973b 100644
> --- a/lib/libdvbv5/dvb-sat.c
> +++ b/lib/libdvbv5/dvb-sat.c
> @@ -523,11 +523,8 @@ static int dvbsat_diseqc_set_input(struct 
> dvb_v5_fe_parms_priv *parms,
>   struct diseqc_cmd cmd;
>   const struct dvb_sat_lnb_priv *lnb = (void *)parms->p.lnb;
>  
> - /* Negative numbers means to not use a DiSEqC switch */
> - if (parms->p.sat_number < 0) {
> - /* If not bandstack, warn if DiSEqC is disabled */
> - if (!lnb->freqrange[0].pol)
> - dvb_logwarn(_("DiSEqC disabled. Probably won't tune."));
> + if (sat_number < 0 && t) {
> + dvb_logwarn(_("DiSEqC disabled. Can't tune using 
> SCR/Unicable."));
>   return 0;
>   }
>  
> @@ -546,7 +543,7 @@ static int dvbsat_diseqc_set_input(struct 
> dvb_v5_fe_parms_priv *parms,
>   vol_high = 1;
>   } else {
>   /* Adjust voltage/tone accordingly */
> - if (parms->p.sat_number < 2) {
> + if (sat_number < 2) {
>   vol_high = pol_v ? 0 : 1;
>   tone_on = high_band;
>   }
> @@ -560,28 +557,31 @@ static int dvbsat_diseqc_set_input(struct 
> dvb_v5_fe_parms_priv *parms,
>   if (rc)
>   return rc;
>  
> - usleep(15 * 1000);
> -
> - if (!t)
> - rc = dvbsat_diseqc_write_to_port_group(parms, , high_band,
> - pol_v, sat_number);
> - else
> - rc = dvbsat_scr_odu_channel_change(parms, , high_band,
> - pol_v, sat_number, t);
> + if (sat_number >= 0) {
> + /* DiSEqC is enabled. Send DiSEqC commands */
> + usleep(15 * 1000);
>  
> - if (rc) {
> - dvb_logerr(_("sending diseq failed"));
> - return rc;
> - }
> - usleep((15 + parms->p.diseqc_wait) * 1000);
> + if (!t)
> + rc = dvbsat_diseqc_write_to_port_group(parms, , 
> high_band,
> + pol_v, 
> sat_number);
> + else
> + rc = dvbsat_scr_odu_channel_change(parms, , 
> high_band,
> + pol_v, 
> sat_number, t);
>  
> - /* miniDiSEqC/Toneburst commands are defined only for up to 2 
> sattelites */
> - if (parms->p.sat_number < 2) {
> - rc = dvb_fe_diseqc_burst(>p, parms->p.sat_number);
> - if (rc)
> + if (rc) {
> + dvb_logerr(_("sending diseq failed"));
>   return rc;
> + }
> + usleep((15 + parms->p.diseqc_wait) * 1000);
> +
> + /* miniDiSEqC/Toneburst commands are defined only for up to 2 
> sattelites */
> + if (parms->p.sat_number < 2) {
> + rc = dvb_fe_diseqc_burst(>p, 
> parms->p.sat_number);
> + if (rc)
> + return rc;
> + }
> + usleep(15 * 1000);
>   }
> - usleep(15 * 1000);
>  
>   rc = dvb_fe_sec_tone(>p, tone_on ? SEC_TONE_ON : SEC_TONE_OFF);
>  
>



Tested-by:  Rafaël Carré 


Re: [PATCH v1 07/10] v4l: i2c: Copy ov772x soc_camera sensor driver

2017-12-13 Thread Philippe Ombredanne
Jacopo,

On Wed, Nov 15, 2017 at 11:56 AM, Jacopo Mondi
 wrote:
> Copy the soc_camera based driver in v4l2 sensor driver directory.
> This commit just copies the original file without modifying it.
>
> Signed-off-by: Jacopo Mondi 



> --- /dev/null
> +++ b/drivers/media/i2c/ov772x.c
> @@ -0,0 +1,1124 @@
> +/*
> + * ov772x Camera Driver
> + *
> + * Copyright (C) 2008 Renesas Solutions Corp.
> + * Kuninori Morimoto 
> + *
> + * Based on ov7670 and soc_camera_platform driver,
> + *
> + * Copyright 2006-7 Jonathan Corbet 
> + * Copyright (C) 2008 Magnus Damm
> + * Copyright (C) 2008, Guennadi Liakhovetski 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */

You may want to use the new SPDX ids as documented in Thomas doc
patches instead of the loner legalese?
-- 
Cordially
Philippe Ombredanne


[PATCH] dvb-sat: do the best to tune if DiSEqC is disabled

2017-12-13 Thread Mauro Carvalho Chehab
If sat_number is not filled (e.g. it is -1), the dvb-sat
disables DiSEqC. However, currently, it also breaks support
for non-bandstacking LNBf.

Change the logic to fix it. There is a drawback on this
approach, though: usually, on bandstacking arrangements,
only one device needs to feed power to the LNBf. The
others don't need to send power. With the previous code,
no power would be sent at all, if sat_number == -1.

Now, it will always power the LNBf when using it.

Signed-off-by: Mauro Carvalho Chehab 
---
 lib/libdvbv5/dvb-sat.c | 48 
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/lib/libdvbv5/dvb-sat.c b/lib/libdvbv5/dvb-sat.c
index b012318c4195..8c04f66f973b 100644
--- a/lib/libdvbv5/dvb-sat.c
+++ b/lib/libdvbv5/dvb-sat.c
@@ -523,11 +523,8 @@ static int dvbsat_diseqc_set_input(struct 
dvb_v5_fe_parms_priv *parms,
struct diseqc_cmd cmd;
const struct dvb_sat_lnb_priv *lnb = (void *)parms->p.lnb;
 
-   /* Negative numbers means to not use a DiSEqC switch */
-   if (parms->p.sat_number < 0) {
-   /* If not bandstack, warn if DiSEqC is disabled */
-   if (!lnb->freqrange[0].pol)
-   dvb_logwarn(_("DiSEqC disabled. Probably won't tune."));
+   if (sat_number < 0 && t) {
+   dvb_logwarn(_("DiSEqC disabled. Can't tune using 
SCR/Unicable."));
return 0;
}
 
@@ -546,7 +543,7 @@ static int dvbsat_diseqc_set_input(struct 
dvb_v5_fe_parms_priv *parms,
vol_high = 1;
} else {
/* Adjust voltage/tone accordingly */
-   if (parms->p.sat_number < 2) {
+   if (sat_number < 2) {
vol_high = pol_v ? 0 : 1;
tone_on = high_band;
}
@@ -560,28 +557,31 @@ static int dvbsat_diseqc_set_input(struct 
dvb_v5_fe_parms_priv *parms,
if (rc)
return rc;
 
-   usleep(15 * 1000);
-
-   if (!t)
-   rc = dvbsat_diseqc_write_to_port_group(parms, , high_band,
-   pol_v, sat_number);
-   else
-   rc = dvbsat_scr_odu_channel_change(parms, , high_band,
-   pol_v, sat_number, t);
+   if (sat_number >= 0) {
+   /* DiSEqC is enabled. Send DiSEqC commands */
+   usleep(15 * 1000);
 
-   if (rc) {
-   dvb_logerr(_("sending diseq failed"));
-   return rc;
-   }
-   usleep((15 + parms->p.diseqc_wait) * 1000);
+   if (!t)
+   rc = dvbsat_diseqc_write_to_port_group(parms, , 
high_band,
+   pol_v, 
sat_number);
+   else
+   rc = dvbsat_scr_odu_channel_change(parms, , 
high_band,
+   pol_v, 
sat_number, t);
 
-   /* miniDiSEqC/Toneburst commands are defined only for up to 2 
sattelites */
-   if (parms->p.sat_number < 2) {
-   rc = dvb_fe_diseqc_burst(>p, parms->p.sat_number);
-   if (rc)
+   if (rc) {
+   dvb_logerr(_("sending diseq failed"));
return rc;
+   }
+   usleep((15 + parms->p.diseqc_wait) * 1000);
+
+   /* miniDiSEqC/Toneburst commands are defined only for up to 2 
sattelites */
+   if (parms->p.sat_number < 2) {
+   rc = dvb_fe_diseqc_burst(>p, 
parms->p.sat_number);
+   if (rc)
+   return rc;
+   }
+   usleep(15 * 1000);
}
-   usleep(15 * 1000);
 
rc = dvb_fe_sec_tone(>p, tone_on ? SEC_TONE_ON : SEC_TONE_OFF);
 
-- 
2.14.3



Re: [GIT PULL for v4.15-rc3] media fixes

2017-12-13 Thread Mauro Carvalho Chehab
Em Wed, 13 Dec 2017 13:27:47 +0100
Geert Uytterhoeven  escreveu:

> Hi Mauro,
> 
> On Wed, Dec 13, 2017 at 12:53 PM, Mauro Carvalho Chehab
>  wrote:
> > Em Wed, 13 Dec 2017 10:03:56 +0100
> > Geert Uytterhoeven  escreveu:  
> >> On Mon, Dec 11, 2017 at 12:12 PM, Mauro Carvalho Chehab
> >>  wrote:  
> >> > Without this series, I was getting 809 lines of bogus warnings (see 
> >> > below),
> >> > with was preventing me to see new warnings on my incremental builds
> >> > while applying new patches at the media tree.  
> >>
> >> $ linux-log-diff build.log{.old,}
> >>
> >> (from https://github.com/geertu/linux-scripts)  
> >
> > That's nice!
> >
> > Yet, it is producing some noise. I did a clean build with:
> >
> > $ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y W=1 
> > CHECK='' M=drivers/staging/media | grep -v -e " CC " -e " LD " -e " AR " -e 
> > " CHK " -e " CALL " -e " UPD " -e "scripts/kconfig/conf " -e " CHECK " 
> > >old.log
> > $ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y W=1 
> > CHECK='' M=drivers/media| grep -v -e " CC " -e " LD " -e " AR " -e " CHK " 
> > -e " CALL " -e " UPD " -e "scripts/kconfig/conf " -e " CHECK "  >>old.log
> >
> > and added a new uninitialized "foo" var to a random driver, doing an
> > incremental build with:
> >
> > $ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y W=1 
> > CHECK='' | grep -v -e " CC " -e " LD " -e " AR " -e " CHK " -e " CALL " -e 
> > " UPD " -e "scripts/kconfig/conf " -e " CHECK " M=drivers/staging/media 
> > >new.log
> > $ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y W=1 
> > CHECK='' | grep -v -e " CC " -e " LD " -e " AR " -e " CHK " -e " CALL " -e 
> > " UPD " -e "scripts/kconfig/conf " -e " CHECK " M=drivers/media >new.log
> >
> > Then, I ran the script:
> >
> > $ linux-log-diff old.log new.log
> >
> > *** ERRORS ***
> >
> >
> > *** WARNINGS ***
> >
> > 1 warning regressions:
> >   + drivers/media/dvb-frontends/dibx000_common.c: warning: unused variable 
> > 'foo' [-Wunused-variable]:  => 22:5
> >
> > 3 warning improvements:
> >   - ./arch/x86/include/asm/bitops.h: warning: asm output is not an lvalue: 
> > 430:22 =>
> >   - 
> > drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/mmu_private.h:
> >  warning: function 'mmu_reg_load' with external linkage has definition: 
> > 35:30 =>
> >   - 
> > drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/mmu_private.h:
> >  warning: function 'mmu_reg_store' with external linkage has definition: 
> > 24:26 =>
> >
> > It detected the "foo" var warning, but it outputs 3 warning improvements
> > on files that were not even built the second time.  
> 
> If the file wasn't built, the warning cannot be in the log ;-)
> So yes, it works best for full builds, only flagging warnings that
> (dis)appeared (and ignoring changes due to changed line numbers!).
> 
> If you do lots of incremental builds, you want to append the last incremental
> log to the existing full log before doing a new build, to avoid false 
> positives
> from files that weren't built in the previous run:
> 
> $ cat new.log >> old.log
> $ make ... > new.log
> $ linux-log-diff old.log new.log
> 
> And only new warnings should be reported.

Ok. I'll do some trials with this script, as it sounds promising!

Regards,
Mauro


-- 
Thanks,
Mauro


Re: [GIT PULL for v4.15-rc3] media fixes

2017-12-13 Thread Geert Uytterhoeven
Hi Mauro,

On Wed, Dec 13, 2017 at 12:53 PM, Mauro Carvalho Chehab
 wrote:
> Em Wed, 13 Dec 2017 10:03:56 +0100
> Geert Uytterhoeven  escreveu:
>> On Mon, Dec 11, 2017 at 12:12 PM, Mauro Carvalho Chehab
>>  wrote:
>> > Without this series, I was getting 809 lines of bogus warnings (see below),
>> > with was preventing me to see new warnings on my incremental builds
>> > while applying new patches at the media tree.
>>
>> $ linux-log-diff build.log{.old,}
>>
>> (from https://github.com/geertu/linux-scripts)
>
> That's nice!
>
> Yet, it is producing some noise. I did a clean build with:
>
> $ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y W=1 
> CHECK='' M=drivers/staging/media | grep -v -e " CC " -e " LD " -e " AR " -e " 
> CHK " -e " CALL " -e " UPD " -e "scripts/kconfig/conf " -e " CHECK " >old.log
> $ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y W=1 
> CHECK='' M=drivers/media| grep -v -e " CC " -e " LD " -e " AR " -e " CHK " -e 
> " CALL " -e " UPD " -e "scripts/kconfig/conf " -e " CHECK "  >>old.log
>
> and added a new uninitialized "foo" var to a random driver, doing an
> incremental build with:
>
> $ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y W=1 
> CHECK='' | grep -v -e " CC " -e " LD " -e " AR " -e " CHK " -e " CALL " -e " 
> UPD " -e "scripts/kconfig/conf " -e " CHECK " M=drivers/staging/media >new.log
> $ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y W=1 
> CHECK='' | grep -v -e " CC " -e " LD " -e " AR " -e " CHK " -e " CALL " -e " 
> UPD " -e "scripts/kconfig/conf " -e " CHECK " M=drivers/media >new.log
>
> Then, I ran the script:
>
> $ linux-log-diff old.log new.log
>
> *** ERRORS ***
>
>
> *** WARNINGS ***
>
> 1 warning regressions:
>   + drivers/media/dvb-frontends/dibx000_common.c: warning: unused variable 
> 'foo' [-Wunused-variable]:  => 22:5
>
> 3 warning improvements:
>   - ./arch/x86/include/asm/bitops.h: warning: asm output is not an lvalue: 
> 430:22 =>
>   - 
> drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/mmu_private.h:
>  warning: function 'mmu_reg_load' with external linkage has definition: 35:30 
> =>
>   - 
> drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/mmu_private.h:
>  warning: function 'mmu_reg_store' with external linkage has definition: 
> 24:26 =>
>
> It detected the "foo" var warning, but it outputs 3 warning improvements
> on files that were not even built the second time.

If the file wasn't built, the warning cannot be in the log ;-)
So yes, it works best for full builds, only flagging warnings that
(dis)appeared (and ignoring changes due to changed line numbers!).

If you do lots of incremental builds, you want to append the last incremental
log to the existing full log before doing a new build, to avoid false positives
from files that weren't built in the previous run:

$ cat new.log >> old.log
$ make ... > new.log
$ linux-log-diff old.log new.log

And only new warnings should be reported.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH v2] media: ABS macro parameter parenthesization

2017-12-13 Thread Mauro Carvalho Chehab
Em Fri, 17 Nov 2017 09:55:44 -0500
dgopst...@nyu.edu escreveu:

> From: Dan Gopstein 
> 
> Two definitions of the ABS (absolute value) macro fail to parenthesize
> their parameter properly. This can lead to a bad expansion for
> low-precedence expression arguments. Add parens to protect against
> troublesome arguments.
> 
> For example: ABS(1-2) currently expands to ((1-2) < 0 ? (-1-2) : (1-2))
> which evaluates to -3. But the correct expansion would be
> ((1-2) < 0 ? -(1-2) : (1-2)) which evaluates to 1.
> 
> Signed-off-by: Dan Gopstein 
> ---
> v1->v2:
> * unmangled the patch
> * added example to commit text
> 
>  drivers/media/dvb-frontends/dibx000_common.h | 2 +-
>  drivers/media/dvb-frontends/mb86a16.c| 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/dibx000_common.h 
> b/drivers/media/dvb-frontends/dibx000_common.h
> index 8784af9..ae60f5d 100644
> --- a/drivers/media/dvb-frontends/dibx000_common.h
> +++ b/drivers/media/dvb-frontends/dibx000_common.h
> @@ -223,7 +223,7 @@ struct dvb_frontend_parametersContext {
>  
>  #define FE_CALLBACK_TIME_NEVER 0x
>  
> -#define ABS(x) ((x < 0) ? (-x) : (x))
> +#define ABS(x) (((x) < 0) ? -(x) : (x))
>  
>  #define DATA_BUS_ACCESS_MODE_8BIT 0x01
>  #define DATA_BUS_ACCESS_MODE_16BIT0x02
> diff --git a/drivers/media/dvb-frontends/mb86a16.c 
> b/drivers/media/dvb-frontends/mb86a16.c
> index dfe322e..2d921c7 100644
> --- a/drivers/media/dvb-frontends/mb86a16.c
> +++ b/drivers/media/dvb-frontends/mb86a16.c
> @@ -31,7 +31,7 @@
>  static unsigned int verbose = 5;
>  module_param(verbose, int, 0644);
>  
> -#define ABS(x)   ((x) < 0 ? (-x) : (x))
> +#define ABS(x)   ((x) < 0 ? -(x) : (x))
>  
>  struct mb86a16_state {
>   struct i2c_adapter  *i2c_adap;

Actually, the Kernel has already a macro for that, called abs().

So, the better would be to just remove those macros,
replacing ABS(foo) by abs(foo).

Thanks,
Mauro


Re: [PATCH v1 10/10] media: i2c: tw9910: Remove soc_camera dependencies

2017-12-13 Thread Hans Verkuil
On 15/11/17 11:56, Jacopo Mondi wrote:
> Remove soc_camera framework dependencies from tw9910 sensor driver.
> - Handle clock directly
> - Register async subdevice
> - Add platform specific enable/disable functions
> - Adjust build system
> 
> This commit does not remove the original soc_camera based driver.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/i2c/Kconfig  |  9 ++
>  drivers/media/i2c/Makefile |  1 +
>  drivers/media/i2c/tw9910.c | 80 
> ++
>  include/media/i2c/tw9910.h |  6 
>  4 files changed, 75 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index ff251ce..bbd77ee 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -415,6 +415,15 @@ config VIDEO_TW9906
> To compile this driver as a module, choose M here: the
> module will be called tw9906.
> 
> +config VIDEO_TW9910
> + tristate "Techwell TW9910 video decoder"
> + depends on VIDEO_V4L2 && I2C
> + ---help---
> +   Support for Techwell TW9910 NTSC/PAL/SECAM video decoder.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called tw9910.
> +
>  config VIDEO_VPX3220
>   tristate "vpx3220a, vpx3216b & vpx3214c video decoders"
>   depends on VIDEO_V4L2 && I2C
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index b2459a1..835784a 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_VIDEO_TVP7002) += tvp7002.o
>  obj-$(CONFIG_VIDEO_TW2804) += tw2804.o
>  obj-$(CONFIG_VIDEO_TW9903) += tw9903.o
>  obj-$(CONFIG_VIDEO_TW9906) += tw9906.o
> +obj-$(CONFIG_VIDEO_TW9910) += tw9910.o
>  obj-$(CONFIG_VIDEO_CS3308) += cs3308.o
>  obj-$(CONFIG_VIDEO_CS5345) += cs5345.o
>  obj-$(CONFIG_VIDEO_CS53L32A) += cs53l32a.o
> diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
> index bdb5e0a..f422da2 100644
> --- a/drivers/media/i2c/tw9910.c
> +++ b/drivers/media/i2c/tw9910.c
> @@ -16,6 +16,7 @@
>   * published by the Free Software Foundation.
>   */
> 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -25,9 +26,7 @@
>  #include 
>  #include 
> 
> -#include 
>  #include 
> -#include 
>  #include 
> 
>  #define GET_ID(val)  ((val & 0xF8) >> 3)
> @@ -228,7 +227,7 @@ struct tw9910_scale_ctrl {
> 
>  struct tw9910_priv {
>   struct v4l2_subdev  subdev;
> - struct v4l2_clk *clk;
> + struct clk  *clk;
>   struct tw9910_video_info*info;
>   const struct tw9910_scale_ctrl  *scale;
>   v4l2_std_id norm;
> @@ -582,13 +581,40 @@ static int tw9910_s_register(struct v4l2_subdev *sd,
>  }
>  #endif
> 
> +static int tw9910_power_on(struct tw9910_priv *priv)
> +{
> + int ret;
> +
> + if (priv->info->platform_enable) {
> + ret = priv->info->platform_enable();
> + if (ret)
> + return ret;
> + }
> +
> + if (priv->clk)
> + return clk_enable(priv->clk);
> +
> + return 0;
> +}
> +
> +static int tw9910_power_off(struct tw9910_priv *priv)
> +{
> + if (priv->info->platform_enable)
> + priv->info->platform_disable();
> +
> + if (priv->clk)
> + clk_disable(priv->clk);
> +
> + return 0;
> +}
> +
>  static int tw9910_s_power(struct v4l2_subdev *sd, int on)
>  {
>   struct i2c_client *client = v4l2_get_subdevdata(sd);
> - struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
>   struct tw9910_priv *priv = to_tw9910(client);
> 
> - return soc_camera_set_power(>dev, ssdd, priv->clk, on);
> + return on ? tw9910_power_on(priv) :
> + tw9910_power_off(priv);
>  }
> 
>  static int tw9910_set_frame(struct v4l2_subdev *sd, u32 *width, u32 *height)
> @@ -614,7 +640,7 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32 
> *width, u32 *height)
>* set bus width
>*/
>   val = 0x00;
> - if (SOCAM_DATAWIDTH_16 == priv->info->buswidth)
> + if (priv->info->buswidth == TW9910_DATAWIDTH_16)
>   val = LEN;
> 
>   ret = tw9910_mask_set(client, OPFORM, LEN, val);
> @@ -799,8 +825,8 @@ static int tw9910_video_probe(struct i2c_client *client)
>   /*
>* tw9910 only use 8 or 16 bit bus width
>*/
> - if (SOCAM_DATAWIDTH_16 != priv->info->buswidth &&
> - SOCAM_DATAWIDTH_8  != priv->info->buswidth) {
> + if (priv->info->buswidth != TW9910_DATAWIDTH_16 &&
> + priv->info->buswidth != TW9910_DATAWIDTH_8) {
>   dev_err(>dev, "bus width error\n");
>   return -ENODEV;
>   }
> @@ -859,15 +885,11 @@ static int tw9910_enum_mbus_code(struct v4l2_subdev *sd,
>  static int tw9910_g_mbus_config(struct v4l2_subdev *sd,
>   struct v4l2_mbus_config *cfg)
>  {
> - struct 

Re: [PATCH v1 07/10] v4l: i2c: Copy ov772x soc_camera sensor driver

2017-12-13 Thread Hans Verkuil
On 15/11/17 11:56, Jacopo Mondi wrote:
> Copy the soc_camera based driver in v4l2 sensor driver directory.
> This commit just copies the original file without modifying it.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/i2c/ov772x.c | 1124 
> 
>  1 file changed, 1124 insertions(+)
>  create mode 100644 drivers/media/i2c/ov772x.c
> 
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> new file mode 100644
> index 000..8063835
> --- /dev/null
> +++ b/drivers/media/i2c/ov772x.c
> @@ -0,0 +1,1124 @@
> +/*
> + * ov772x Camera Driver
> + *
> + * Copyright (C) 2008 Renesas Solutions Corp.
> + * Kuninori Morimoto 
> + *
> + * Based on ov7670 and soc_camera_platform driver,
> + *
> + * Copyright 2006-7 Jonathan Corbet 
> + * Copyright (C) 2008 Magnus Damm
> + * Copyright (C) 2008, Guennadi Liakhovetski 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * register offset
> + */
> +#define GAIN0x00 /* AGC - Gain control gain setting */
> +#define BLUE0x01 /* AWB - Blue channel gain setting */
> +#define RED 0x02 /* AWB - Red   channel gain setting */
> +#define GREEN   0x03 /* AWB - Green channel gain setting */
> +#define COM10x04 /* Common control 1 */
> +#define BAVG0x05 /* U/B Average Level */
> +#define GAVG0x06 /* Y/Gb Average Level */
> +#define RAVG0x07 /* V/R Average Level */
> +#define AECH0x08 /* Exposure Value - AEC MSBs */
> +#define COM20x09 /* Common control 2 */
> +#define PID 0x0A /* Product ID Number MSB */
> +#define VER 0x0B /* Product ID Number LSB */
> +#define COM30x0C /* Common control 3 */
> +#define COM40x0D /* Common control 4 */
> +#define COM50x0E /* Common control 5 */
> +#define COM60x0F /* Common control 6 */
> +#define AEC 0x10 /* Exposure Value */
> +#define CLKRC   0x11 /* Internal clock */
> +#define COM70x12 /* Common control 7 */
> +#define COM80x13 /* Common control 8 */
> +#define COM90x14 /* Common control 9 */
> +#define COM10   0x15 /* Common control 10 */
> +#define REG16   0x16 /* Register 16 */
> +#define HSTART  0x17 /* Horizontal sensor size */
> +#define HSIZE   0x18 /* Horizontal frame (HREF column) end high 8-bit */
> +#define VSTART  0x19 /* Vertical frame (row) start high 8-bit */
> +#define VSIZE   0x1A /* Vertical sensor size */
> +#define PSHFT   0x1B /* Data format - pixel delay select */
> +#define MIDH0x1C /* Manufacturer ID byte - high */
> +#define MIDL0x1D /* Manufacturer ID byte - low  */
> +#define LAEC0x1F /* Fine AEC value */
> +#define COM11   0x20 /* Common control 11 */
> +#define BDBASE  0x22 /* Banding filter Minimum AEC value */
> +#define DBSTEP  0x23 /* Banding filter Maximum Setp */
> +#define AEW 0x24 /* AGC/AEC - Stable operating region (upper limit) 
> */
> +#define AEB 0x25 /* AGC/AEC - Stable operating region (lower limit) 
> */
> +#define VPT 0x26 /* AGC/AEC Fast mode operating region */
> +#define REG28   0x28 /* Register 28 */
> +#define HOUTSIZE0x29 /* Horizontal data output size MSBs */
> +#define EXHCH   0x2A /* Dummy pixel insert MSB */
> +#define EXHCL   0x2B /* Dummy pixel insert LSB */
> +#define VOUTSIZE0x2C /* Vertical data output size MSBs */
> +#define ADVFL   0x2D /* LSB of insert dummy lines in Vertical direction 
> */
> +#define ADVFH   0x2E /* MSG of insert dummy lines in Vertical direction 
> */
> +#define YAVE0x2F /* Y/G Channel Average value */
> +#define LUMHTH  0x30 /* Histogram AEC/AGC Luminance high level threshold 
> */
> +#define LUMLTH  0x31 /* Histogram AEC/AGC Luminance low  level threshold 
> */
> +#define HREF0x32 /* Image start and size control */
> +#define DM_LNL  0x33 /* Dummy line low  8 bits */
> +#define DM_LNH  0x34 /* Dummy line high 8 bits */
> +#define ADOFF_B 0x35 /* AD offset compensation value for B  channel */
> +#define ADOFF_R 0x36 /* AD offset compensation value for R  channel */
> +#define ADOFF_GB0x37 /* AD offset compensation value for Gb channel */
> +#define ADOFF_GR0x38 /* AD offset compensation value for Gr channel */
> +#define OFF_B   0x39 /* Analog process B  channel offset value */
> +#define OFF_R   0x3A /* Analog process R  channel offset value */
> +#define OFF_GB  0x3B /* Analog process Gb channel offset value */
> 

Re: [PATCH v1 03/10] v4l: platform: Add Renesas CEU driver

2017-12-13 Thread Hans Verkuil
On 15/11/17 11:55, Jacopo Mondi wrote:
> Add driver for Renesas Capture Engine Unit (CEU).
> 
> The CEU interface supports capturing 'data' (YUV422) and 'images'
> (NV[12|21|16|61]).
> 
> This driver aims to replace the soc_camera based sh_mobile_ceu one.
> 
> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> platform GR-Peach.
> 
> Tested with ov7725 camera sensor on SH4 platform Migo-R.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/platform/Kconfig   |9 +
>  drivers/media/platform/Makefile  |2 +
>  drivers/media/platform/renesas-ceu.c | 1768 
> ++
>  3 files changed, 1779 insertions(+)
>  create mode 100644 drivers/media/platform/renesas-ceu.c
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 3c4f7fa..401caea 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -144,6 +144,15 @@ config VIDEO_STM32_DCMI
> To compile this driver as a module, choose M here: the module
> will be called stm32-dcmi.
> 
> +config VIDEO_RENESAS_CEU
> + tristate "Renesas Capture Engine Unit (CEU) driver"
> + depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
> + depends on ARCH_SHMOBILE || ARCH_R7S72100 || COMPILE_TEST
> + select VIDEOBUF2_DMA_CONTIG
> + select V4L2_FWNODE
> + ---help---
> +   This is a v4l2 driver for the Renesas CEU Interface
> +
>  source "drivers/media/platform/soc_camera/Kconfig"
>  source "drivers/media/platform/exynos4-is/Kconfig"
>  source "drivers/media/platform/am437x/Kconfig"
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 327f80a..0d1f02b 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -27,6 +27,8 @@ obj-$(CONFIG_VIDEO_CODA)+= coda/
> 
>  obj-$(CONFIG_VIDEO_SH_VEU)   += sh_veu.o
> 
> +obj-$(CONFIG_VIDEO_RENESAS_CEU)  += renesas-ceu.o
> +
>  obj-$(CONFIG_VIDEO_MEM2MEM_DEINTERLACE)  += m2m-deinterlace.o
> 
>  obj-$(CONFIG_VIDEO_MUX)  += video-mux.o
> diff --git a/drivers/media/platform/renesas-ceu.c 
> b/drivers/media/platform/renesas-ceu.c
> new file mode 100644
> index 000..aaba3cd
> --- /dev/null
> +++ b/drivers/media/platform/renesas-ceu.c
> @@ -0,0 +1,1768 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * V4L2 Driver for Renesas Capture Engine Unit (CEU) interface
> + *
> + * Copyright (C) 2017 Jacopo Mondi 
> + *
> + * Based on soc-camera driver "soc_camera/sh_mobile_ceu_camera.c"
> + * Copyright (C) 2008 Magnus Damm
> + *
> + * Based on V4L2 Driver for PXA camera host - "pxa_camera.c",
> + * Copyright (C) 2006, Sascha Hauer, Pengutronix
> + * Copyright (C) 2008, Guennadi Liakhovetski 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#define DRIVER_NAME  "renesas-ceu"
> +
> +/* 
> 
> + * CEU registers offsets and masks
> + */
> +#define CEU_CAPSR0x00 /* Capture start register  */
> +#define CEU_CAPCR0x04 /* Capture control register*/
> +#define CEU_CAMCR0x08 /* Capture interface control register  */
> +#define CEU_CAMOR0x10 /* Capture interface offset register   */
> +#define CEU_CAPWR0x14 /* Capture interface width register*/
> +#define CEU_CAIFR0x18 /* Capture interface input format register */
> +#define CEU_CRCNTR   0x28 /* CEU register control register   */
> +#define CEU_CRCMPR   0x2c /* CEU register forcible control register  */
> +#define CEU_CFLCR0x30 /* Capture filter control register */
> +#define CEU_CFSZR0x34 /* Capture filter size clip register   */
> +#define CEU_CDWDR0x38 /* Capture destination width register  */
> +#define CEU_CDAYR0x3c /* Capture data address Y register */
> +#define CEU_CDACR0x40 /* Capture data address C register */
> +#define CEU_CFWCR0x5c /* Firewall operation control register */
> +#define CEU_CDOCR0x64 /* Capture data output control register*/
> +#define CEU_CEIER0x70 /* Capture event interrupt enable register */
> +#define CEU_CETCR0x74 /* Capture event flag clear register   */
> +#define 

Re: [GIT PULL for v4.15-rc3] media fixes

2017-12-13 Thread Mauro Carvalho Chehab
Em Wed, 13 Dec 2017 10:03:56 +0100
Geert Uytterhoeven  escreveu:

> Hi Mauro,
> 
> On Mon, Dec 11, 2017 at 12:12 PM, Mauro Carvalho Chehab
>  wrote:
> > Without this series, I was getting 809 lines of bogus warnings (see below),
> > with was preventing me to see new warnings on my incremental builds
> > while applying new patches at the media tree.  
> 
> $ linux-log-diff build.log{.old,}
> 
> (from https://github.com/geertu/linux-scripts)

That's nice!

Yet, it is producing some noise. I did a clean build with:

$ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y W=1 
CHECK='' M=drivers/staging/media | grep -v -e " CC " -e " LD " -e " AR " -e " 
CHK " -e " CALL " -e " UPD " -e "scripts/kconfig/conf " -e " CHECK " >old.log
$ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y W=1 
CHECK='' M=drivers/media| grep -v -e " CC " -e " LD " -e " AR " -e " CHK " -e " 
CALL " -e " UPD " -e "scripts/kconfig/conf " -e " CHECK "  >>old.log

and added a new uninitialized "foo" var to a random driver, doing an
incremental build with:

$ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y W=1 
CHECK='' | grep -v -e " CC " -e " LD " -e " AR " -e " CHK " -e " CALL " -e " 
UPD " -e "scripts/kconfig/conf " -e " CHECK " M=drivers/staging/media >new.log
$ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y W=1 
CHECK='' | grep -v -e " CC " -e " LD " -e " AR " -e " CHK " -e " CALL " -e " 
UPD " -e "scripts/kconfig/conf " -e " CHECK " M=drivers/media >new.log

Then, I ran the script:

$ linux-log-diff old.log new.log

*** ERRORS ***


*** WARNINGS ***

1 warning regressions:
  + drivers/media/dvb-frontends/dibx000_common.c: warning: unused variable 
'foo' [-Wunused-variable]:  => 22:5

3 warning improvements:
  - ./arch/x86/include/asm/bitops.h: warning: asm output is not an lvalue: 
430:22 => 
  - 
drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/mmu_private.h:
 warning: function 'mmu_reg_load' with external linkage has definition: 35:30 
=> 
  - 
drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/mmu_private.h:
 warning: function 'mmu_reg_store' with external linkage has definition: 24:26 
=> 

It detected the "foo" var warning, but it outputs 3 warning improvements
on files that were not even built the second time.

-- 
Thanks,
Mauro


[PATCH] dvbv5-zap: accept -S 0 option

2017-12-13 Thread Rafaël Carré
Signed-off-by: Rafaël Carré 
---
 utils/dvb/dvbv5-zap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/dvb/dvbv5-zap.c b/utils/dvb/dvbv5-zap.c
index a88500d1..1b6dabd0 100644
--- a/utils/dvb/dvbv5-zap.c
+++ b/utils/dvb/dvbv5-zap.c
@@ -930,7 +930,7 @@ int main(int argc, char **argv)
goto err;
if (lnb >= 0)
parms->lnb = dvb_sat_get_lnb(lnb);
-   if (args.sat_number > 0)
+   if (args.sat_number >= 0)
parms->sat_number = args.sat_number;
parms->diseqc_wait = args.diseqc_wait;
parms->freq_bpf = args.freq_bpf;
-- 
2.14.1



[PATCH] Staging: media: atomisp: made function static

2017-12-13 Thread Sergiy Redko
Fixed sparse warning by making 'dtrace_dot' function static.

Signed-off-by: Sergiy Redko 
---
 .../media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c  | 1 +
 1 file changed, 1 insertion(+)

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 dd1127a21494..f22d73b56bc6 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
@@ -2567,6 +2567,7 @@ ia_css_debug_mode_enable_dma_channel(int dma_id,
return rc;
 }
 
+static
 void dtrace_dot(const char *fmt, ...)
 {
va_list ap;
-- 
2.15.1



[PATCH] pxa_camera: rename the soc_camera_ prefix to pxa_camera_

2017-12-13 Thread Hans Verkuil
Rename soc_camera to pxa_camera as this has no longer anything to do with the 
old
soc_camera driver/framework. It's confusing when grepping on soc_camera.

Signed-off-by: Hans Verkuil 
---
diff --git a/drivers/media/platform/pxa_camera.c 
b/drivers/media/platform/pxa_camera.c
index 295f34ad1080..7c765a00c504 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -647,16 +647,16 @@ static unsigned int pxa_mbus_config_compatible(const 
struct v4l2_mbus_config *cf
 }

 /**
- * struct soc_camera_format_xlate - match between host and sensor formats
+ * struct pxa_camera_format_xlate - match between host and sensor formats
  * @code: code of a sensor provided format
  * @host_fmt: host format after host translation from code
  *
  * Host and sensor translation structure. Used in table of host and sensor
- * formats matchings in soc_camera_device. A host can override the generic list
+ * formats matchings in pxa_camera_device. A host can override the generic list
  * generation by implementing get_formats(), and use it for format checks and
  * format setup.
  */
-struct soc_camera_format_xlate {
+struct pxa_camera_format_xlate {
u32 code;
const struct pxa_mbus_pixelfmt *host_fmt;
 };
@@ -693,8 +693,8 @@ struct pxa_camera_dev {
struct v4l2_async_notifier notifier;
struct vb2_queuevb2_vq;
struct v4l2_subdev  *sensor;
-   struct soc_camera_format_xlate *user_formats;
-   const struct soc_camera_format_xlate *current_fmt;
+   struct pxa_camera_format_xlate *user_formats;
+   const struct pxa_camera_format_xlate *current_fmt;
struct v4l2_pix_format  current_pix;

struct v4l2_async_subdev asd;
@@ -743,8 +743,8 @@ static const char *pxa_cam_driver_description = 
"PXA_Camera";
 /*
  * Format translation functions
  */
-static const struct soc_camera_format_xlate
-*pxa_mbus_xlate_by_fourcc(struct soc_camera_format_xlate *user_formats,
+static const struct pxa_camera_format_xlate
+*pxa_mbus_xlate_by_fourcc(struct pxa_camera_format_xlate *user_formats,
  unsigned int fourcc)
 {
unsigned int i;
@@ -755,17 +755,17 @@ static const struct soc_camera_format_xlate
return NULL;
 }

-static struct soc_camera_format_xlate *pxa_mbus_build_fmts_xlate(
+static struct pxa_camera_format_xlate *pxa_mbus_build_fmts_xlate(
struct v4l2_device *v4l2_dev, struct v4l2_subdev *subdev,
int (*get_formats)(struct v4l2_device *, unsigned int,
-  struct soc_camera_format_xlate *xlate))
+  struct pxa_camera_format_xlate *xlate))
 {
unsigned int i, fmts = 0, raw_fmts = 0;
int ret;
struct v4l2_subdev_mbus_code_enum code = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
};
-   struct soc_camera_format_xlate *user_formats;
+   struct pxa_camera_format_xlate *user_formats;

while (!v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, )) {
raw_fmts++;
@@ -1722,7 +1722,7 @@ static bool pxa_camera_packing_supported(const struct 
pxa_mbus_pixelfmt *fmt)

 static int pxa_camera_get_formats(struct v4l2_device *v4l2_dev,
  unsigned int idx,
- struct soc_camera_format_xlate *xlate)
+ struct pxa_camera_format_xlate *xlate)
 {
struct pxa_camera_dev *pcdev = v4l2_dev_to_pcdev(v4l2_dev);
int formats = 0, ret;
@@ -1794,7 +1794,7 @@ static int pxa_camera_get_formats(struct v4l2_device 
*v4l2_dev,

 static int pxa_camera_build_formats(struct pxa_camera_dev *pcdev)
 {
-   struct soc_camera_format_xlate *xlate;
+   struct pxa_camera_format_xlate *xlate;

xlate = pxa_mbus_build_fmts_xlate(>v4l2_dev, pcdev->sensor,
  pxa_camera_get_formats);
@@ -1883,7 +1883,7 @@ static int pxac_vidioc_try_fmt_vid_cap(struct file *filp, 
void *priv,
  struct v4l2_format *f)
 {
struct pxa_camera_dev *pcdev = video_drvdata(filp);
-   const struct soc_camera_format_xlate *xlate;
+   const struct pxa_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = >fmt.pix;
struct v4l2_subdev_pad_config pad_cfg;
struct v4l2_subdev_format format = {
@@ -1947,7 +1947,7 @@ static int pxac_vidioc_s_fmt_vid_cap(struct file *filp, 
void *priv,
struct v4l2_format *f)
 {
struct pxa_camera_dev *pcdev = video_drvdata(filp);
-   const struct soc_camera_format_xlate *xlate;
+   const struct pxa_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = >fmt.pix;
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,


[bug report] media: lirc: improve locking

2017-12-13 Thread Dan Carpenter
Hello Sean Young,

The patch 131fd7fc3c01: "media: lirc: improve locking" from Nov 4,
2017, leads to the following static checker warning:

drivers/media/rc/lirc_dev.c:373 ir_lirc_transmit_ir()
error: 'txbuf' dereferencing possible ERR_PTR()

drivers/media/rc/lirc_dev.c
   330  txbuf = memdup_user(buf, n);
   331  if (IS_ERR(txbuf)) {
   332  ret = PTR_ERR(txbuf);
   333  goto out;

This used to be a direct return...

   334  }
   335  }
   336  
   337  for (i = 0; i < count; i++) {
   338  if (txbuf[i] > IR_MAX_DURATION / 1000 - duration || 
!txbuf[i]) {
   339  ret = -EINVAL;
   340  goto out;
   341  }
   342  
   343  duration += txbuf[i];
   344  }
   345  
   346  ret = dev->tx_ir(dev, txbuf, count);
   347  if (ret < 0)
   348  goto out;
   349  
   350  if (fh->send_mode == LIRC_MODE_SCANCODE) {
   351  ret = n;
   352  } else {
   353  for (duration = i = 0; i < ret; i++)
   354  duration += txbuf[i];
   355  
   356  ret *= sizeof(unsigned int);
   357  
   358  /*
   359   * The lircd gap calculation expects the write function 
to
   360   * wait for the actual IR signal to be transmitted 
before
   361   * returning.
   362   */
   363  towait = ktime_us_delta(ktime_add_us(start, duration),
   364  ktime_get());
   365  if (towait > 0) {
   366  set_current_state(TASK_INTERRUPTIBLE);
   367  schedule_timeout(usecs_to_jiffies(towait));
^
This looks like a long wait?  Are you sure you want to hold the lock
this whole time?

   368  }
   369  }
   370  
   371  out:
   372  mutex_unlock(>lock);
   373  kfree(txbuf);
  ^
Can't pass an error pointer to kfree().

   374  kfree(raw);

regards,
dan carpenter


Re: [PATCH v10 4/4] [media] platform: Add Synopsys DesignWare HDMI RX Controller Driver

2017-12-13 Thread Philippe Ombredanne
Jose,

On 12/12/17 17:02, Jose Abreu wrote:


>>> Signed-off-by: Jose Abreu 
>>> Cc: Joao Pinto 
>>> Cc: Mauro Carvalho Chehab 
>>> Cc: Hans Verkuil 
>>> Cc: Sylwester Nawrocki 
>>> Cc: Sakari Ailus 
>>> Cc: Philippe Ombredanne 
>>> ---
>>> Changes from v9:
>>> - Use SPDX License ID (Philippe)

For the use of SPDX tags, thanks!

Acked-by: Philippe Ombredanne 


Re: [PATCH v10 4/4] [media] platform: Add Synopsys DesignWare HDMI RX Controller Driver

2017-12-13 Thread Hans Verkuil
On 12/12/17 17:02, Jose Abreu wrote:
> Hi Hans,
> 
> On 12-12-2017 15:47, Hans Verkuil wrote:
>> Hi Jose,
>>
>> Some more comments:
> 
> Thanks for the review!
> 
>>
>> On 11/12/17 18:41, Jose Abreu wrote:
>>> This is an initial submission for the Synopsys DesignWare HDMI RX
>>> Controller Driver. This driver interacts with a phy driver so that
>>> a communication between them is created and a video pipeline is
>>> configured.
>>>
>>> The controller + phy pipeline can then be integrated into a fully
>>> featured system that can be able to receive video up to 4k@60Hz
>>> with deep color 48bit RGB, depending on the platform. Although,
>>> this initial version does not yet handle deep color modes.
>>>
>>> This driver was implemented as a standard V4L2 subdevice and its
>>> main features are:
>>> - Internal state machine that reconfigures phy until the
>>> video is not stable
>>> - JTAG communication with phy
>>> - Inter-module communication with phy driver
>>> - Debug write/read ioctls
>>>
>>> Some notes:
>>> - RX sense controller (cable connection/disconnection) must
>>> be handled by the platform wrapper as this is not integrated
>>> into the controller RTL
>>> - The same goes for EDID ROM's
>>> - ZCAL calibration is needed only in FPGA platforms, in ASIC
>>> this is not needed
>>> - The state machine is not an ideal solution as it creates a
>>> kthread but it is needed because some sources might not be
>>> very stable at sending the video (i.e. we must react
>>> accordingly).
>>>
>>> Signed-off-by: Jose Abreu 
>>> Cc: Joao Pinto 
>>> Cc: Mauro Carvalho Chehab 
>>> Cc: Hans Verkuil 
>>> Cc: Sylwester Nawrocki 
>>> Cc: Sakari Ailus 
>>> Cc: Philippe Ombredanne 
>>> ---
>>> Changes from v9:
>>> - Use SPDX License ID (Philippe)
>>> - Use LOW_DRIVE CEC error (Hans)
>>> - Fill bt->il_* fields (Hans)
>>> - Fix format->field to NONE (Hans)
>>> - Drop a left-over comment (Hans)
>>> - Use CEC_CAP_DEFAULTS (Hans)
>>> Changes from v8:
>>> - Incorporate Sakari's work on ASYNC subdevs
>>> Changes from v6:
>>> - edid-phandle now also looks for parent node (Sylwester)
>>> - Fix kbuild build warnings
>>> Changes from v5:
>>> - Removed HDCP 1.4 support (Hans)
>>> - Removed some CEC debug messages (Hans)
>>> - Add s_dv_timings callback (Hans)
>>> - Add V4L2_CID_DV_RX_POWER_PRESENT ctrl (Hans)
>>> Changes from v4:
>>> - Add flag V4L2_SUBDEV_FL_HAS_DEVNODE (Sylwester)
>>> - Remove some comments and change some messages to dev_dbg (Sylwester)
>>> - Use v4l2_async_subnotifier_register() (Sylwester)
>>> Changes from v3:
>>> - Use v4l2 async API (Sylwester)
>>> - Do not block waiting for phy
>>> - Do not use busy waiting delays (Sylwester)
>>> - Simplify dw_hdmi_power_on (Sylwester)
>>> - Use clock API (Sylwester)
>>> - Use compatible string (Sylwester)
>>> - Minor fixes (Sylwester)
>>> Changes from v2:
>>> - Address review comments from Hans regarding CEC
>>> - Use CEC notifier
>>> - Enable SCDC
>>> Changes from v1:
>>> - Add support for CEC
>>> - Correct typo errors
>>> - Correctly detect interlaced video modes
>>> - Correct VIC parsing
>>> Changes from RFC:
>>> - Add support for HDCP 1.4
>>> - Fixup HDMI_VIC not being parsed (Hans)
>>> - Send source change signal when powering off (Hans)
>>> - Add a "wait stable delay"
>>> - Detect interlaced video modes (Hans)
>>> - Restrain g/s_register from reading/writing to HDCP regs (Hans)
>>> ---
>>>  drivers/media/platform/dwc/Kconfig  |   15 +
>>>  drivers/media/platform/dwc/Makefile |1 +
>>>  drivers/media/platform/dwc/dw-hdmi-rx.c | 1840 
>>> +++
>>>  drivers/media/platform/dwc/dw-hdmi-rx.h |  419 +++
>>>  include/media/dwc/dw-hdmi-rx-pdata.h|   48 +
>>>  5 files changed, 2323 insertions(+)
>>>  create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.c
>>>  create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.h
>>>  create mode 100644 include/media/dwc/dw-hdmi-rx-pdata.h
>>>
>>> diff --git a/drivers/media/platform/dwc/Kconfig 
>>> b/drivers/media/platform/dwc/Kconfig
>>> index 361d38d..3ddccde 100644
>>> --- a/drivers/media/platform/dwc/Kconfig
>>> +++ b/drivers/media/platform/dwc/Kconfig
>>> @@ -6,3 +6,18 @@ config VIDEO_DWC_HDMI_PHY_E405
>>>  
>>>   To compile this driver as a module, choose M here. The module
>>>   will be called dw-hdmi-phy-e405.
>>> +
>>> +config VIDEO_DWC_HDMI_RX
>>> +   tristate "Synopsys Designware HDMI Receiver driver"
>>> +   depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
>>> +   help
>>> + Support for Synopsys Designware HDMI RX controller.
>>> +
>>> + To compile this driver as a module, choose M here. The module
>>> + will be called 

vidioc-g-dv-timings.rst: fix typo (frontporch -> backporch)

2017-12-13 Thread Hans Verkuil
The description of V4L2_DV_FL_HALF_LINE mixed up frontporch with backporch.

It's the backporch that has different sizes for interlaced formats, the 
frontporch
remains constant.

Signed-off-by: Hans Verkuil 
---
diff --git a/Documentation/media/uapi/v4l/   
b/Documentation/media/uapi/v4l/vidioc-g-dv-timings.rst
index 2696380626d4..1a034e825161 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-dv-timings.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-dv-timings.rst
@@ -267,7 +267,7 @@ EBUSY
will also be cleared.
 * - ``V4L2_DV_FL_HALF_LINE``
   - Specific to interlaced formats: if set, then the vertical
-   frontporch of field 1 (aka the odd field) is really one half-line
+   backporch of field 1 (aka the odd field) is really one half-line
longer and the vertical backporch of field 2 (aka the even field)
is really one half-line shorter, so each field has exactly the
same number of half-lines. Whether half-lines can be detected or


Re: [PATCH v8] uvcvideo: Add a metadata device node

2017-12-13 Thread Guennadi Liakhovetski
Hi Laurent,

On Wed, 13 Dec 2017, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Tuesday, 12 December 2017 10:30:39 EET Guennadi Liakhovetski wrote:
> > On Mon, 11 Dec 2017, Laurent Pinchart wrote:
> > > On Monday, 11 December 2017 23:44:09 EET Guennadi Liakhovetski wrote:
> > >> On Mon, 11 Dec 2017, Laurent Pinchart wrote:
> > >>> On Monday, 11 December 2017 22:16:23 EET Laurent Pinchart wrote:
> >  On Wednesday, 6 December 2017 17:15:40 EET Guennadi Liakhovetski wrote:
> > > From: Guennadi Liakhovetski 
> > > 
> > > Some UVC video cameras contain metadata in their payload headers.
> > > This patch extracts that data, adding more clock synchronisation
> > > information, on both bulk and isochronous endpoints and makes it
> > > available to the user space on a separate video node, using the
> > > V4L2_CAP_META_CAPTURE capability and the V4L2_BUF_TYPE_META_CAPTURE
> > > buffer queue type. By default, only the V4L2_META_FMT_UVC pixel
> > > format is available from those nodes. However, cameras can be added to
> > > the device ID table to additionally specify their own metadata format,
> > > in which case that format will also become available from the metadata
> > > node.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski
> > > 
> > > ---
> > > 
> > > v8: addressed comments and integrated changes from Laurent, thanks
> > > again, e.g.:
> > > 
> > > - multiple stylistic changes
> > > - remove the UVC_DEV_FLAG_METADATA_NODE flag / quirk: nodes are now
> > >   created unconditionally
> > > - reuse uvc_ioctl_querycap()
> > > - reuse code in uvc_register_video()
> > > - set an error flag when the metadata buffer overflows
> > > 
> > >  drivers/media/usb/uvc/Makefile   |   2 +-
> > >  drivers/media/usb/uvc/uvc_driver.c   |  15 ++-
> > >  drivers/media/usb/uvc/uvc_isight.c   |   2 +-
> > >  drivers/media/usb/uvc/uvc_metadata.c | 179+++
> > >  drivers/media/usb/uvc/uvc_queue.c|  44 +++--
> > >  drivers/media/usb/uvc/uvc_video.c| 132 +++--
> > >  drivers/media/usb/uvc/uvcvideo.h |  16 +++-
> > >  include/uapi/linux/uvcvideo.h|  26 +
> > >  8 files changed, 394 insertions(+), 22 deletions(-)
> > >  create mode 100644 drivers/media/usb/uvc/uvc_metadata.c
> >  
> >  [snip]
> >  
> > > diff --git a/drivers/media/usb/uvc/uvc_video.c
> > > b/drivers/media/usb/uvc/uvc_video.c index 13f459e..2fc0bf2 100644
> > > --- a/drivers/media/usb/uvc/uvc_video.c
> > > +++ b/drivers/media/usb/uvc/uvc_video.c
> >  
> >  [snip]
> >  
> > > +static void uvc_video_decode_meta(struct uvc_streaming *stream,
> > > +   struct uvc_buffer *meta_buf,
> > > +   const u8 *mem, unsigned int length)
> > > +{
> > > + struct uvc_meta_buf *meta;
> > > + size_t len_std = 2;
> > > + bool has_pts, has_scr;
> > > + unsigned long flags;
> > > + ktime_t time;
> > > + const u8 *scr;
> 
> [snip]
> 
> > > + meta = (struct uvc_meta_buf *)((u8 *)meta_buf->mem +
> > > meta_buf->bytesused);
> > > + local_irq_save(flags);
> > > + time = uvc_video_get_time();
> > > + meta->sof = usb_get_current_frame_number(stream->dev->udev);
> >  
> >  You need a put_unaligned here too. If you're fine with the patch
> >  below there's no need to resubmit, and
> > >>> 
> > >>> One more thing, __put_unaligned_cpu16() and __put_unaligned_cpu64()
> > >>> don't compile on x86_64 with v4.12 (using media_build.git). I propose
> > >>> replacing them with put_unaligned() which compiles and should do the
> > >>> right thing.
> > >> 
> > >> Sure, thanks for catching! Shall I fix and resubmit?
> > > 
> > > If you're fine with
> > > 
> > >   git://linuxtv.org/pinchartl/media.git uvc/next
> > 
> > I was a bit concerned about just using "int" for unaligned writing of a
> > 16-bit value, but looking at definitions, after a couple of macros
> > put_unaligned() currently resolves to one inline functions, which should
> > make that safe. However, at least theoretically, an arch could decide to
> > implement put_unaligned() as a macro, which might turn out to be unsafe
> > for this... Not sure how concerned should we be about such a possibility
> > :-) If you think, that's fine, then I'm ok with using the version from
> > that your branch.
> 
> Why do you think that would be unsafe ? The return type of 
> usb_get_current_frame_number() is int, so introducing an intermediate int 
> variable should at least not make things worse. If put_unaligned() is 
> implemented solely using macros I would still expect them to operate on the 
> type of the destination operand, and cast the source value appropriately.

Well, 

Re: [PATCH v8] uvcvideo: Add a metadata device node

2017-12-13 Thread Laurent Pinchart
Hi Guennadi,

On Tuesday, 12 December 2017 10:30:39 EET Guennadi Liakhovetski wrote:
> On Mon, 11 Dec 2017, Laurent Pinchart wrote:
> > On Monday, 11 December 2017 23:44:09 EET Guennadi Liakhovetski wrote:
> >> On Mon, 11 Dec 2017, Laurent Pinchart wrote:
> >>> On Monday, 11 December 2017 22:16:23 EET Laurent Pinchart wrote:
>  On Wednesday, 6 December 2017 17:15:40 EET Guennadi Liakhovetski wrote:
> > From: Guennadi Liakhovetski 
> > 
> > Some UVC video cameras contain metadata in their payload headers.
> > This patch extracts that data, adding more clock synchronisation
> > information, on both bulk and isochronous endpoints and makes it
> > available to the user space on a separate video node, using the
> > V4L2_CAP_META_CAPTURE capability and the V4L2_BUF_TYPE_META_CAPTURE
> > buffer queue type. By default, only the V4L2_META_FMT_UVC pixel
> > format is available from those nodes. However, cameras can be added to
> > the device ID table to additionally specify their own metadata format,
> > in which case that format will also become available from the metadata
> > node.
> > 
> > Signed-off-by: Guennadi Liakhovetski
> > 
> > ---
> > 
> > v8: addressed comments and integrated changes from Laurent, thanks
> > again, e.g.:
> > 
> > - multiple stylistic changes
> > - remove the UVC_DEV_FLAG_METADATA_NODE flag / quirk: nodes are now
> >   created unconditionally
> > - reuse uvc_ioctl_querycap()
> > - reuse code in uvc_register_video()
> > - set an error flag when the metadata buffer overflows
> > 
> >  drivers/media/usb/uvc/Makefile   |   2 +-
> >  drivers/media/usb/uvc/uvc_driver.c   |  15 ++-
> >  drivers/media/usb/uvc/uvc_isight.c   |   2 +-
> >  drivers/media/usb/uvc/uvc_metadata.c | 179+++
> >  drivers/media/usb/uvc/uvc_queue.c|  44 +++--
> >  drivers/media/usb/uvc/uvc_video.c| 132 +++--
> >  drivers/media/usb/uvc/uvcvideo.h |  16 +++-
> >  include/uapi/linux/uvcvideo.h|  26 +
> >  8 files changed, 394 insertions(+), 22 deletions(-)
> >  create mode 100644 drivers/media/usb/uvc/uvc_metadata.c
>  
>  [snip]
>  
> > diff --git a/drivers/media/usb/uvc/uvc_video.c
> > b/drivers/media/usb/uvc/uvc_video.c index 13f459e..2fc0bf2 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
>  
>  [snip]
>  
> > +static void uvc_video_decode_meta(struct uvc_streaming *stream,
> > + struct uvc_buffer *meta_buf,
> > + const u8 *mem, unsigned int length)
> > +{
> > +   struct uvc_meta_buf *meta;
> > +   size_t len_std = 2;
> > +   bool has_pts, has_scr;
> > +   unsigned long flags;
> > +   ktime_t time;
> > +   const u8 *scr;

[snip]

> > +   meta = (struct uvc_meta_buf *)((u8 *)meta_buf->mem +
> > meta_buf->bytesused);
> > +   local_irq_save(flags);
> > +   time = uvc_video_get_time();
> > +   meta->sof = usb_get_current_frame_number(stream->dev->udev);
>  
>  You need a put_unaligned here too. If you're fine with the patch
>  below there's no need to resubmit, and
> >>> 
> >>> One more thing, __put_unaligned_cpu16() and __put_unaligned_cpu64()
> >>> don't compile on x86_64 with v4.12 (using media_build.git). I propose
> >>> replacing them with put_unaligned() which compiles and should do the
> >>> right thing.
> >> 
> >> Sure, thanks for catching! Shall I fix and resubmit?
> > 
> > If you're fine with
> > 
> > git://linuxtv.org/pinchartl/media.git uvc/next
> 
> I was a bit concerned about just using "int" for unaligned writing of a
> 16-bit value, but looking at definitions, after a couple of macros
> put_unaligned() currently resolves to one inline functions, which should
> make that safe. However, at least theoretically, an arch could decide to
> implement put_unaligned() as a macro, which might turn out to be unsafe
> for this... Not sure how concerned should we be about such a possibility
> :-) If you think, that's fine, then I'm ok with using the version from
> that your branch.

Why do you think that would be unsafe ? The return type of 
usb_get_current_frame_number() is int, so introducing an intermediate int 
variable should at least not make things worse. If put_unaligned() is 
implemented solely using macros I would still expect them to operate on the 
type of the destination operand, and cast the source value appropriately.

[snip]

-- 
Regards,

Laurent Pinchart



Re: [PATCH 0/2] uvcvideo: Refactor code to ease metadata implementation

2017-12-13 Thread Laurent Pinchart
Hi Guennadi,

On Tuesday, 12 December 2017 09:45:11 EET Guennadi Liakhovetski wrote:
> Hi Laurent,
> 
> Thanks for the patches. Please feel free to add either or both of
> 
> Reviewed-by: Guennadi Liakhovetski 
> Tested-by: Guennadi Liakhovetski 
> 
> to both of the patches. Whereas in fact strictly speaking your current
> tree has updated improved versions of the patches, at least of the first
> of them - it now correctly handles the struct video_device::vfl_dir field,
> even thoough I'd still find merging that "if" with the following "switch"
> prettier ;-) So, strictly speaking you'd have to post those updated
> versions, in any case my approval tags refer to versions in your tree with
> commit IDs
> 
> 53464c9f76da054ac3c291d27f348170d2a346c6
> and
> b6c5f10563c4ee8437cd9131bc3d389514456519

Thank you. You're absolutely right, I've reposted the patches in a v2 with 
your tags included.

-- 
Regards,

Laurent Pinchart



[PATCH v2 0/2] uvcvideo: Refactor code to ease metadata implementation

2017-12-13 Thread Laurent Pinchart
Hello

This small patch series refactors the uvc_video_register() function to extract
the code that you need into a new uvc_video_register_device() function. Please
let me know if it can help.

Compared to v1, the first patch has been updated not to rely on the fact that
VFL_DIR_RX equals to zero.

Laurent Pinchart (2):
  uvcvideo: Factor out video device registration to a function
  uvcvideo: Report V4L2 device caps through the video_device structure

 drivers/media/usb/uvc/uvc_driver.c | 79 ++
 drivers/media/usb/uvc/uvc_v4l2.c   |  4 --
 drivers/media/usb/uvc/uvcvideo.h   |  8 
 3 files changed, 62 insertions(+), 29 deletions(-)

-- 
Regards,

Laurent Pinchart



[PATCH v2 2/2] uvcvideo: Report V4L2 device caps through the video_device structure

2017-12-13 Thread Laurent Pinchart
The V4L2 core populates the struct v4l2_capability device_caps field
from the same field in video_device. There's no need to handle that
manually in the driver.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Guennadi Liakhovetski 
Tested-by: Guennadi Liakhovetski 
---
 drivers/media/usb/uvc/uvc_driver.c | 11 +++
 drivers/media/usb/uvc/uvc_v4l2.c   |  4 
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c 
b/drivers/media/usb/uvc/uvc_driver.c
index f6fbfc122871..d63458539e03 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1921,6 +1921,17 @@ int uvc_register_video_device(struct uvc_device *dev,
vdev->vfl_dir = VFL_DIR_TX;
else
vdev->vfl_dir = VFL_DIR_RX;
+
+   switch (type) {
+   case V4L2_BUF_TYPE_VIDEO_CAPTURE:
+   default:
+   vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
+   break;
+   case V4L2_BUF_TYPE_VIDEO_OUTPUT:
+   vdev->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
+   break;
+   }
+
strlcpy(vdev->name, dev->name, sizeof vdev->name);
 
/*
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 3e7e283a44a8..5e0323982577 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -568,10 +568,6 @@ static int uvc_ioctl_querycap(struct file *file, void *fh,
usb_make_path(stream->dev->udev, cap->bus_info, sizeof(cap->bus_info));
cap->capabilities = V4L2_CAP_DEVICE_CAPS | V4L2_CAP_STREAMING
  | chain->caps;
-   if (stream->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
-   cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
-   else
-   cap->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
 
return 0;
 }
-- 
Regards,

Laurent Pinchart



[PATCH v2 1/2] uvcvideo: Factor out video device registration to a function

2017-12-13 Thread Laurent Pinchart
The function will then be used to register the video device for metadata
capture.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Guennadi Liakhovetski 
Tested-by: Guennadi Liakhovetski 
---
 drivers/media/usb/uvc/uvc_driver.c | 68 --
 drivers/media/usb/uvc/uvcvideo.h   |  8 +
 2 files changed, 51 insertions(+), 25 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c 
b/drivers/media/usb/uvc/uvc_driver.c
index 07e427d36516..f6fbfc122871 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -24,6 +24,7 @@
 #include 
 
 #include 
+#include 
 
 #include "uvcvideo.h"
 
@@ -1889,52 +1890,65 @@ static void uvc_unregister_video(struct uvc_device *dev)
kref_put(>ref, uvc_delete);
 }
 
-static int uvc_register_video(struct uvc_device *dev,
-   struct uvc_streaming *stream)
+int uvc_register_video_device(struct uvc_device *dev,
+ struct uvc_streaming *stream,
+ struct video_device *vdev,
+ struct uvc_video_queue *queue,
+ enum v4l2_buf_type type,
+ const struct v4l2_file_operations *fops,
+ const struct v4l2_ioctl_ops *ioctl_ops)
 {
-   struct video_device *vdev = >vdev;
int ret;
 
/* Initialize the video buffers queue. */
-   ret = uvc_queue_init(>queue, stream->type, !uvc_no_drop_param);
+   ret = uvc_queue_init(queue, type, !uvc_no_drop_param);
if (ret)
return ret;
 
-   /* Initialize the streaming interface with default streaming
-* parameters.
-*/
-   ret = uvc_video_init(stream);
-   if (ret < 0) {
-   uvc_printk(KERN_ERR, "Failed to initialize the device "
-   "(%d).\n", ret);
-   return ret;
-   }
-
-   uvc_debugfs_init_stream(stream);
-
/* Register the device with V4L. */
 
-   /* We already hold a reference to dev->udev. The video device will be
+   /*
+* We already hold a reference to dev->udev. The video device will be
 * unregistered before the reference is released, so we don't need to
 * get another one.
 */
vdev->v4l2_dev = >vdev;
-   vdev->fops = _fops;
-   vdev->ioctl_ops = _ioctl_ops;
+   vdev->fops = fops;
+   vdev->ioctl_ops = ioctl_ops;
vdev->release = uvc_release;
vdev->prio = >chain->prio;
-   if (stream->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+   if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
vdev->vfl_dir = VFL_DIR_TX;
+   else
+   vdev->vfl_dir = VFL_DIR_RX;
strlcpy(vdev->name, dev->name, sizeof vdev->name);
 
-   /* Set the driver data before calling video_register_device, otherwise
-* uvc_v4l2_open might race us.
+   /*
+* Set the driver data before calling video_register_device, otherwise
+* the file open() handler might race us.
 */
video_set_drvdata(vdev, stream);
 
ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
if (ret < 0) {
-   uvc_printk(KERN_ERR, "Failed to register video device (%d).\n",
+   uvc_printk(KERN_ERR, "Failed to register %s device (%d).\n",
+  v4l2_type_names[type], ret);
+   return ret;
+   }
+
+   kref_get(>ref);
+   return 0;
+}
+
+static int uvc_register_video(struct uvc_device *dev,
+   struct uvc_streaming *stream)
+{
+   int ret;
+
+   /* Initialize the streaming interface with default parameters. */
+   ret = uvc_video_init(stream);
+   if (ret < 0) {
+   uvc_printk(KERN_ERR, "Failed to initialize the device (%d).\n",
   ret);
return ret;
}
@@ -1944,8 +1958,12 @@ static int uvc_register_video(struct uvc_device *dev,
else
stream->chain->caps |= V4L2_CAP_VIDEO_OUTPUT;
 
-   kref_get(>ref);
-   return 0;
+   uvc_debugfs_init_stream(stream);
+
+   /* Register the device with V4L. */
+   return uvc_register_video_device(dev, stream, >vdev,
+>queue, stream->type,
+_fops, _ioctl_ops);
 }
 
 /*
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index dd9c79a92622..c36fa0991141 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -716,6 +716,14 @@ void uvc_video_clock_update(struct uvc_streaming *stream,
struct vb2_v4l2_buffer *vbuf,
struct uvc_buffer *buf);
 
+int uvc_register_video_device(struct uvc_device *dev,
+ struct uvc_streaming *stream,
+

Re: [GIT PULL for v4.15-rc3] media fixes

2017-12-13 Thread Geert Uytterhoeven
Hi Mauro,

On Mon, Dec 11, 2017 at 12:12 PM, Mauro Carvalho Chehab
 wrote:
> Without this series, I was getting 809 lines of bogus warnings (see below),
> with was preventing me to see new warnings on my incremental builds
> while applying new patches at the media tree.

$ linux-log-diff build.log{.old,}

(from https://github.com/geertu/linux-scripts)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [Patch v6 10/12] [media] v4l2: Add v4l2 control IDs for HEVC encoder

2017-12-13 Thread Smitha T Murthy
On Tue, 2017-12-12 at 10:46 +0100, Sylwester Nawrocki wrote:
> On 12/12/2017 03:34 AM, Smitha T Murthy wrote:
> >> s/Lay/Layer here and below
> >>
> > Ok I will change it.
> 
> While it's fine to make such change for controls up to 
> V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_QP...
> 
> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L1_QP:return "HEVC 
> >>> Hierarchical Lay 1 QP";
> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L2_QP:return "HEVC 
> >>> Hierarchical Lay 2 QP";
> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L3_QP:return "HEVC 
> >>> Hierarchical Lay 3 QP";
> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L4_QP:return "HEVC 
> >>> Hierarchical Lay 4 QP";
> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L5_QP:return "HEVC 
> >>> Hierarchical Lay 5 QP";
> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_QP:return "HEVC 
> >>> Hierarchical Lay 6 QP";
> 
> ...for the controls below we may need to replace "Lay" with "L." 
> to make sure the length of the string don't exceed 31 characters 
> (32 with terminating NULL). The names below seem to be 1 character 
> too long and will be truncated when running VIDIOC_QUERY_CTRL ioctl.
> 
Yes true, also to keep the uniformity I will change for all
V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L1_QP and
V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L0_BR "Lay" with "L".

> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L0_BR:return "HEVC 
> >>> Hierarchical Lay 0 Bit Rate";
> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L1_BR:return "HEVC 
> >>> Hierarchical Lay 1 Bit Rate";
> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L2_BR:return "HEVC 
> >>> Hierarchical Lay 2 Bit Rate";
> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L3_BR:return "HEVC 
> >>> Hierarchical Lay 3 Bit Rate";
> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L4_BR:return "HEVC 
> >>> Hierarchical Lay 4 Bit Rate";
> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L5_BR:return "HEVC 
> >>> Hierarchical Lay 5 Bit Rate";
> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_BR:return "HEVC 
> >>> Hierarchical Lay 6 Bit Rate";
> 
> --
> Regards,
> Sylwester
> 
Thank you for the review.

Regards,
Smitha