Re: [PATCH 4/5] arm: dts: sun8i: a83t: Add support for the ir interface

2017-12-17 Thread Maxime Ripard
On Sun, Dec 17, 2017 at 11:45:46PM +0100, Philipp Rossak wrote:
> The ir interface is like on the H3 located at 0x01f02000 and is exactly
> the same. This patch adds support for the ir interface on the A83T.
> 
> Signed-off-by: Philipp Rossak 
> ---
>  arch/arm/boot/dts/sun8i-a83t.dtsi | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi 
> b/arch/arm/boot/dts/sun8i-a83t.dtsi
> index 954c2393325f..9e7ed3b9a6b8 100644
> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> @@ -503,6 +503,16 @@
>   #reset-cells = <1>;
>   };
>  
> + ir: ir@01f02000 {
> + compatible = "allwinner,sun5i-a13-ir";
> + clocks = <_ccu CLK_APB0_IR>, <_ccu CLK_IR>;
> + clock-names = "apb", "ir";
> + resets = <_ccu RST_APB0_IR>;
> + interrupts = ;
> + reg = <0x01f02000 0x40>;

The size should be the size of the whole memory block, not just the
registers you need.

Maxime

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


signature.asc
Description: PGP signature


Re: [PATCH 2/5] media: dt: bindings: Update binding documentation for sunxi IR controller

2017-12-17 Thread Maxime Ripard
Hi,

On Sun, Dec 17, 2017 at 11:45:44PM +0100, Philipp Rossak wrote:
> This patch updates documentation for Device-Tree bindings for sunxi IR
> controller and adds the new optional property for the base clock
> frequency.
> 
> Signed-off-by: Philipp Rossak 
> ---
>  Documentation/devicetree/bindings/media/sunxi-ir.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt 
> b/Documentation/devicetree/bindings/media/sunxi-ir.txt
> index 91648c569b1e..9f45bab07d6e 100644
> --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt
> +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt
> @@ -11,6 +11,7 @@ Required properties:
>  Optional properties:
>  - linux,rc-map-name: see rc.txt file in the same directory.
>  - resets : phandle + reset specifier pair
> +- clock-frequency  : overrides the default base clock frequency (8 MHz)

You're at least missing the unit one needs to use, but something like:
IR Receiver clock frequency, in Hertz. Defaults to 8MHz if missing.

Maxime

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


signature.asc
Description: PGP signature


Re: [PATCH] media: v4l: xilinx: Use SPDX-License-Identifier

2017-12-17 Thread Michal Simek
Hi guys,

On 15.12.2017 10:27, Mauro Carvalho Chehab wrote:
> Em Fri, 15 Dec 2017 10:55:26 +0530
> Dhaval Shah  escreveu:
> 
>> Hi Laurent/Mauro/Greg,
>>
>> On Fri, Dec 15, 2017 at 3:32 AM, Laurent Pinchart
>>  wrote:
>>> Hi Mauro,
>>>
>>> On Thursday, 14 December 2017 23:50:03 EET Mauro Carvalho Chehab wrote:
 Em Thu, 14 Dec 2017 21:57:06 +0100 Greg KH escreveu:
> On Thu, Dec 14, 2017 at 10:44:16PM +0200, Laurent Pinchart wrote:
>> On Thursday, 14 December 2017 22:08:51 EET Greg KH wrote:
>>> On Thu, Dec 14, 2017 at 09:05:27PM +0200, Laurent Pinchart wrote:
 On Thursday, 14 December 2017 20:54:39 EET Joe Perches wrote:
> On Thu, 2017-12-14 at 20:37 +0200, Laurent Pinchart wrote:
>> On Thursday, 14 December 2017 20:32:20 EET Joe Perches wrote:
>>> On Thu, 2017-12-14 at 20:28 +0200, Laurent Pinchart wrote:
 On Thursday, 14 December 2017 19:05:27 EET Mauro Carvalho Chehab
 wrote:
> Em Fri,  8 Dec 2017 18:05:37 +0530 Dhaval Shah escreveu:
>> SPDX-License-Identifier is used for the Xilinx Video IP and
>> related drivers.
>>
>> Signed-off-by: Dhaval Shah 
>
> Hi Dhaval,
>
> You're not listed as one of the Xilinx driver maintainers. I'm
> afraid that, without their explicit acks, sent to the ML, I
> can't accept a patch touching at the driver's license tags.

 The patch doesn't change the license, I don't see why it would
 cause any issue. Greg isn't listed as the maintainer or copyright
 holder of any of the 10k+ files to which he added an SPDX license
 header in the last kernel release.
>>>
>>> Adding a comment line that describes an implicit or
>>> explicit license is different than removing the license
>>> text itself.
>>
>> The SPDX license header is meant to be equivalent to the license
>> text.
>
> I understand that.
> At a minimum, removing BSD license text is undesirable
>
> as that license states:
>  ** Redistributions of source code must retain the above copyright
>  *  notice, this list of conditions and the following disclaimer.
>
> etc...

 But this patch only removes the following text:

 - * 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.

 and replaces it by the corresponding SPDX header.

>> The only reason why the large SPDX patch didn't touch the whole
>> kernel in one go was that it was easier to split in in multiple
>> chunks.
>
> Not really, it was scripted.

 But still manually reviewed as far as I know.

>> This is no different than not including the full GPL license in
>> every header file but only pointing to it through its name and
>> reference, as every kernel source file does.
>
> Not every kernel source file had a license text
> or a reference to another license file.

 Correct, but the files touched by this patch do.

 This issue is in no way specific to linux-media and should be
 decided upon at the top level, not on a per-subsystem basis. Greg,
 could you comment on this ?
>>>
>>> Comment on what exactly?  I don't understand the problem here, care to
>>> summarize it?
>>
>> In a nutshell (if I understand it correctly), Dhaval Shah submitted
>> https:// patchwork.kernel.org/patch/10102451/ which replaces
>>
>> +// SPDX-License-Identifier: GPL-2.0
>> [...]
>> - *
>> - * 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.
>>
>> in all .c and .h files of the Xilinx V4L2 driver
>> (drivers/media/platform/
>> xilinx). I have reviewed the patch and acked it. Mauro then rejected it,
>> stating that he can't accept a change to license text without an
>> explicit ack from the official driver's maintainers. My position is
>> that such a change doesn't change the license and thus doesn't need to
>> track all copyright holders, and can be merged without an explicit ack
>> from the respective maintainers.
>
> Yes, I agree with you, no license is being changed here, and no
> copyright is either.
>
> BUT, I know that most major companies are reviewing this process right
> now.  We have gotten 

Re: [PATCH v3 3/5] media: dt-bindings: ov5640: add support of DVP parallel interface

2017-12-17 Thread Sakari Ailus
Hi Hugues,

Hugues FRUCHET wrote:
> Hi Sakari,
> 
> On 12/07/2017 02:59 PM, Sakari Ailus wrote:
>> Hi Hugues,
>>
>> On Thu, Dec 07, 2017 at 01:40:51PM +0100, Hugues Fruchet wrote:
>>> Add bindings for OV5640 DVP parallel interface support.
>>>
>>> Signed-off-by: Hugues Fruchet 
>>> ---
>>>   .../devicetree/bindings/media/i2c/ov5640.txt   | 27 
>>> --
>>>   1 file changed, 25 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt 
>>> b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>>> index 540b36c..04e2a91 100644
>>> --- a/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>>> +++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>>> @@ -1,4 +1,4 @@
>>> -* Omnivision OV5640 MIPI CSI-2 sensor
>>> +* Omnivision OV5640 MIPI CSI-2 / parallel sensor
>>>   
>>>   Required Properties:
>>>   - compatible: should be "ovti,ov5640"
>>> @@ -18,7 +18,11 @@ The device node must contain one 'port' child node for 
>>> its digital output
>>>   video port, in accordance with the video interface bindings defined in
>>>   Documentation/devicetree/bindings/media/video-interfaces.txt.
>>>   
>>> -Example:
>>> +Parallel or CSI mode is selected according to optional endpoint properties.
>>> +Without properties (or bus properties), parallel mode is selected.
>>> +Specifying any CSI properties such as lanes will enable CSI mode.
>>
>> These bindings never documented what which endpoint properties were needed.
> 
> Ok I will add a section related to endpoint properties for both CSI and 
> parallel.
> 
>>
>> Beyond that, the sensor supports two CSI-2 lanes. You should explicitly
>> specify that, in other words, you'll need "data-lanes" property. Could you
>> add that?
> Ok I will add it to required endpoint property in case of CSI mode.
> I will change commit header to reflect changes on parallel but also CSI 
> documentation.
> 
>>
>> Long time ago when the video-interfaces.txt and the V4L2 OF framework were
>> written, the bus type selection was made implicit and only later on
>> explicit. This is still reflected in how the bus type gets set between
>> CSI-2 D-PHY, parallel and Bt.656.
>>
> I'm a little bit confused, must I explicitly add as required property 
> "bus-type=0" (autodetect) for both cases ? Or must I require 
> "bus-type=1" for CSI and "bus-type=3" for parallel ?

Yes, the confusion will stay for some time to come in some way. :-)

What I wanted to say that the original decision to make this implicit
from the bindings wasn't great --- we have here the device that does
both parallel and CSI-2 D-PHY.

But due to data-lanes, you can rely on implicit bus type selection
working. So no bus-type is needed.

> 
> 
> Talking bindings, I feel that it could be of great help to document also
> the polarity of control signals (hsync/vsync/pclk), they are currently 
> set by ov5640 init sequence and not configurable.
> Moreover, should some checks be added in probe sequence to verify that
> the defined control signals polarity are aligned with default ones from
> init sequence ?

Yes, that's a very good idea. It should have been there all along.

> 
> 
> Here is a proposal:
> 
> "
> The device node must contain one 'port' child node for its digital 
> output video port with a single 'endpoint' subnode, in accordance
> with the video interface bindings defined in
> Documentation/devicetree/bindings/media/video-interfaces.txt.
> 
> OV5640 can be connected to a MIPI CSI bus or a parallel bus endpoint:

CSI-2, please.

> 
> Endpoint node required properties for CSI connection are:
> - remote-endpoint: a phandle to the bus receiver's endpoint node.
> - bus-type: should be set to <1> (MIPI CSI-2 C-PHY)

You can omit bus-type. Or is this really C-PHY?? We don't actually have
any other devices that support C-PHY yet AFAIK.

> - clock-lanes: should be set to <0> (clock lane on hardware lane 0)

But clock-lanes isn't relevant for C-PHY. So you have D-PHY, I presume.

> - data-lanes: should be set to <1 2> (two CSI-2 lanes supported)

This should document what the hardware can do, not what the driver
supports. So <1> or <1 2> it should be.

> 
> Endpoint node required properties for parallel connection are:
> - remote-endpoint: a phandle to the bus receiver's endpoint node.
> - bus-type: should be set to <3> (parallel CCP2)

CCP2 is Compact Camera Port 2, an older serial bus (between CSI and CSI-2).

(I actually wonder if we could fix the bus-type property by giving
separate entries for parallel and CSI-2 D-PHY; I'll still need to check
whether it's used somewhere. I think not. Not relevant for this patchset
though.)

> - bus-width: should be set to <8> for 8 bits parallel bus
>   or <10> for 10 bits parallel bus
> - data-shift: should be set to <2> for 8 bits parallel bus
>(lines 9:2 are used) or <0> for 10 bits parallel bus

s/should/shall/ for both.

> - hsync-active: should be set to <0> 

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

2017-12-17 Thread Sakari Ailus
Hi Wenyou,

Wenyou Yang wrote:
...
> +static int ov7740_start_streaming(struct ov7740 *ov7740)
> +{
> + int ret;
> +
> + if (ov7740->fmt) {
> + ret = regmap_multi_reg_write(ov7740->regmap,
> +  ov7740->fmt->regs,
> +  ov7740->fmt->reg_num);
> + if (ret)
> + return ret;
> + }
> +
> + if (ov7740->frmsize) {
> + ret = regmap_multi_reg_write(ov7740->regmap,
> +  ov7740->frmsize->regs,
> +  ov7740->frmsize->reg_num);
> + if (ret)
> + return ret;
> + }
> +
> + return __v4l2_ctrl_handler_setup(ov7740->subdev.ctrl_handler);

I believe you're still setting the controls after starting streaming.

-- 
Sakari Ailus
sakari.ai...@iki.fi



cron job: media_tree daily build: ERRORS

2017-12-17 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:   Mon Dec 18 05:00:17 CET 2017
media-tree git hash:45267fed3e55845c5b4b279162b273040ae4f587
media_build git hash:   7199b00cdae29c6f914a89ad996fcb9a133e9deb
v4l-utils git hash: 14521966d76e0dfa9230dbb90d53faddae195e00
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: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.2.37-i686: WARNINGS
linux-3.3.8-i686: WARNINGS
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: WARNINGS
linux-3.12.67-i686: WARNINGS
linux-3.13.11-i686: 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: WARNINGS
linux-4.4.22-i686: OK
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: OK
linux-4.7.5-i686: OK
linux-4.8-i686: OK
linux-4.9.26-i686: OK
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12.1-i686: OK
linux-4.13-i686: OK
linux-4.14-i686: OK
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: WARNINGS
linux-3.12.67-x86_64: WARNINGS
linux-3.13.11-x86_64: 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: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9.26-x86_64: WARNINGS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: OK
linux-4.14-x86_64: OK
apps: OK
spec-git: OK
smatch: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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


Re: [PATCH 5/5] arm: dts: sun8i: a83t: bananapi-m3: Enable IR controller

2017-12-17 Thread Chen-Yu Tsai
On Mon, Dec 18, 2017 at 6:45 AM, Philipp Rossak  wrote:
> The Bananapi M3 has an onboard IR receiver.
> This enables the onboard IR receiver subnode.
> Other than the other IR receivers this one needs a base clock frequency

Unlike the other...

> of 300 Hz (3 MHz), to be able to work.
>
> Signed-off-by: Philipp Rossak 
> ---
>  arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts 
> b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
> index 6550bf0e594b..2bf25ca64133 100644
> --- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
> +++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
> @@ -100,6 +100,13 @@
> status = "okay";
>  };
>
> + {

See my other reply about the name.

Otherwise,

Acked-by: Chen-Yu Tsai 

> +   pinctrl-names = "default";
> +   pinctrl-0 = <_pins_a>;
> +   clock-frequency = <300>;
> +   status = "okay";
> +};
> +
>   {
> rgmii_phy: ethernet-phy@1 {
> compatible = "ethernet-phy-ieee802.3-c22";
> --
> 2.11.0
>


Re: [PATCH 3/5] arm: dts: sun8i: a83t: Add the ir pin for the A83T

2017-12-17 Thread Chen-Yu Tsai
On Mon, Dec 18, 2017 at 6:45 AM, Philipp Rossak  wrote:
> The CIR Pin of the A83T is located at PL12.
>
> Signed-off-by: Philipp Rossak 
> ---
>  arch/arm/boot/dts/sun8i-a83t.dtsi | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi 
> b/arch/arm/boot/dts/sun8i-a83t.dtsi
> index a384b766f3dc..954c2393325f 100644
> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> @@ -521,6 +521,11 @@
> drive-strength = <20>;
> bias-pull-up;
> };
> +
> +   ir_pins_a: ir@0 {

ir_pins: ir-pins

And it really should be cir, to distinguish it from IrDA.

ChenYu

> +   pins = "PL12";
> +   function = "s_cir_rx";
> +   };
> };
>
> r_rsb: rsb@1f03400 {
> --
> 2.11.0
>


Re: [PATCH 1/5] media: rc: update sunxi-ir driver to get base clock frequency from devicetree

2017-12-17 Thread Andi Shyti
Hi Philipp,

just a couple of small nitpicks.

> + u32 b_clk_freq;

[...]

> + /* Base clock frequency (optional) */
> + if (of_property_read_u32(dn, "clock-frequency", _clk_freq)) {
> + b_clk_freq = SUNXI_IR_BASE_CLK;
> + }
> +

how about you intialize 'b_clk_freq' to 'SUNXI_IR_BASE_CLK' and
just call 'of_property_read_u32' without if statement.
'b_clk_freq' value should not be changed if "clock-frequency"
is not present in the DTS.

This might avoid misinterpretation from static analyzers that
might think that 'b_clk_freq' is used uninitialized.

> + dev_info(dev, "set base clock frequency to %d Hz.\n", b_clk_freq);

Please use dev_dbg().

Andi


[trivial PATCH] treewide: Align function definition open/close braces

2017-12-17 Thread Joe Perches
Some functions definitions have either the initial open brace and/or
the closing brace outside of column 1.

Move those braces to column 1.

This allows various function analyzers like gnu complexity to work
properly for these modified functions.

Miscellanea:

o Remove extra trailing ; and blank line from xfs_agf_verify

Signed-off-by: Joe Perches 
---
git diff -w shows no difference other than the above 'Miscellanea'

(this is against -next, but it applies against Linus' tree
 with a couple offsets)

 arch/x86/include/asm/atomic64_32.h   |  2 +-
 drivers/acpi/custom_method.c |  2 +-
 drivers/acpi/fan.c   |  2 +-
 drivers/gpu/drm/amd/display/dc/core/dc.c |  2 +-
 drivers/media/i2c/msp3400-kthreads.c |  2 +-
 drivers/message/fusion/mptsas.c  |  2 +-
 drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c |  2 +-
 drivers/net/wireless/ath/ath9k/xmit.c|  2 +-
 drivers/platform/x86/eeepc-laptop.c  |  2 +-
 drivers/rtc/rtc-ab-b5ze-s3.c |  2 +-
 drivers/scsi/dpt_i2o.c   |  2 +-
 drivers/scsi/sym53c8xx_2/sym_glue.c  |  2 +-
 fs/locks.c   |  2 +-
 fs/ocfs2/stack_user.c|  2 +-
 fs/xfs/libxfs/xfs_alloc.c|  5 ++---
 fs/xfs/xfs_export.c  |  2 +-
 kernel/audit.c   |  6 +++---
 kernel/trace/trace_printk.c  |  4 ++--
 lib/raid6/sse2.c | 14 +++---
 sound/soc/fsl/fsl_dma.c  |  2 +-
 20 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/arch/x86/include/asm/atomic64_32.h 
b/arch/x86/include/asm/atomic64_32.h
index 97c46b8169b7..d4d4883080fa 100644
--- a/arch/x86/include/asm/atomic64_32.h
+++ b/arch/x86/include/asm/atomic64_32.h
@@ -122,7 +122,7 @@ static inline long long atomic64_read(const atomic64_t *v)
long long r;
alternative_atomic64(read, "=" (r), "c" (v) : "memory");
return r;
- }
+}
 
 /**
  * atomic64_add_return - add and return
diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index c68e72414a67..e967c1173ba3 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -94,7 +94,7 @@ static void __exit acpi_custom_method_exit(void)
 {
if (cm_dentry)
debugfs_remove(cm_dentry);
- }
+}
 
 module_init(acpi_custom_method_init);
 module_exit(acpi_custom_method_exit);
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index 6cf4988206f2..3563103590c6 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -219,7 +219,7 @@ fan_set_cur_state(struct thermal_cooling_device *cdev, 
unsigned long state)
return fan_set_state_acpi4(device, state);
else
return fan_set_state(device, state);
- }
+}
 
 static const struct thermal_cooling_device_ops fan_cooling_ops = {
.get_max_state = fan_get_max_state,
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index d1488d5ee028..1e0d1e7c5324 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -461,7 +461,7 @@ static void disable_dangling_plane(struct dc *dc, struct 
dc_state *context)
  
**/
 
 struct dc *dc_create(const struct dc_init_data *init_params)
- {
+{
struct dc *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
unsigned int full_pipe_count;
 
diff --git a/drivers/media/i2c/msp3400-kthreads.c 
b/drivers/media/i2c/msp3400-kthreads.c
index 4dd01e9f553b..dc6cb8d475b3 100644
--- a/drivers/media/i2c/msp3400-kthreads.c
+++ b/drivers/media/i2c/msp3400-kthreads.c
@@ -885,7 +885,7 @@ static int msp34xxg_modus(struct i2c_client *client)
 }
 
 static void msp34xxg_set_source(struct i2c_client *client, u16 reg, int in)
- {
+{
struct msp_state *state = to_state(i2c_get_clientdata(client));
int source, matrix;
 
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 345f6035599e..69a62d23514b 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -2968,7 +2968,7 @@ mptsas_exp_repmanufacture_info(MPT_ADAPTER *ioc,
mutex_unlock(>sas_mgmt.mutex);
 out:
return ret;
- }
+}
 
 static void
 mptsas_parse_device_info(struct sas_identify *identify,
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c 
b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
index 3dd973475125..0ea141ece19e 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
@@ -603,7 +603,7 @@ static struct uni_table_desc 

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

2017-12-17 Thread Sakari Ailus
Hi Jacopo,

On Sun, Dec 17, 2017 at 05:42:54PM +0100, jacopo mondi wrote:
> Hi Sakari,
> 
> On Fri, Dec 15, 2017 at 06:17:04PM +0200, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Wed, Dec 13, 2017 at 07:26:20PM +0100, Jacopo Mondi wrote:
> > > 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.
> >
> > ACPI doesn't have such at the moment. I think printing the full path would
> > still be better. There isn't that much more to print after all.
> 
> So you suggest to just use the full node name for OF. What about ACPI?
> 
> From your other reply I got that I can print the single node name for
> "device ACPI nodes" but not for "non-device ACPI nodes". Should I build
> the full device name in drivers/acpi/properties.c for ACPI devices
> like I'm doing here for fwnodes?

What I think would be nice was that ACPI would receive similar way to print
node names (as well as other information) as OF has, through printk.

I don't demand that to get the patchset in though, I'm fine if this is
limited to OF right now. It's debug info, after all that I've at least
personally been fine without.

> 
> >
> > > ---
> > >
> > >  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
> >
> > I'm not sure I'd add a Kconfig option. This is adding a fairly simple
> > function only to the kernel.
> 
> So I will use a symbol defined in the module to enable/disable debug
> (maybe the "DEBUG" symbol itself?)

Dynamic debug can be enabled via the user space interface or the kernel
command line, I think that should be enough.

> 
> >
> > > + 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
> >
> > Do you need this?
> 
> No dev_dbg() otherwise, isn't it?

Yes. With dynamic 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 

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

2017-12-17 Thread Sakari Ailus
Hi Laurent,

On Sun, Dec 17, 2017 at 07:03:17PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> Thank you for the patch.
> 
> On Wednesday, 13 December 2017 20:26:19 EET Jacopo Mondi wrote:
> > 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.
> 
> Aren't you piling yet another hack on top of an already dirty framework ? How 
> about fixing the root cause of the issue and ensuring that subdevs are 
> properly initialized when the notifier is registered ?

The problem domain is quite complex --- there are multiple drivers working
with multiple objects each here, and things can happen in a different order
--- which needs to be supported but is sometimes missed in design.

In this case the problem is that the sub-device is only registered after
the related notifier is. If you did that the other way around, the V4L2
async framework could well find that everything is done and proceed to call
the complete callback, just before the async sub-device notifier is
registered.

Perhaps this is, once again, a sign that we should really ditch the
complete callback. I'd hope we could find consensus on that. It's a lot of
trouble to support this and I feel it's an entirely arfiticial construct
that does not really solve a problem it's intended to.

> 
> > 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
> 
> [snip]
> 
> > @@ -548,6 +551,20 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
> > sd->fwnode = dev_fwnode(sd->dev);
> > }
> > 
> > +   /*
> > +* If the subdevice has an unregisterd notifier, it's now time
> > +* to register it.
> > +*/
> > +   subdev_notifier = sd->subdev_notifier;
> > +   if (subdev_notifier && !subdev_notifier->registered) {
> > +   ret = __v4l2_async_notifier_register(subdev_notifier);
> > +   if (ret) {
> > +   sd->fwnode = NULL;
> > +   subdev_notifier->owner = NULL;
> > +   return ret;
> > +   }
> > +   }
> 
> This is the part I like the least in this patch set. The 
> v4l2_subdev::subdev_notifier field should really disappear, there's no reason 
> to limit subdevs to a single notifier. Implicit registration of notifiers is 
> a 
> dirty hack in my opinion.
> 
> > mutex_lock(_lock);
> > 
> > INIT_LIST_HEAD(>async_list);
> 
> [snip]
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

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


Re: [PATCH 1/5] media: rc: update sunxi-ir driver to get base clock frequency from devicetree

2017-12-17 Thread Sean Young
On Sun, Dec 17, 2017 at 11:45:43PM +0100, Philipp Rossak wrote:
> This patch updates the sunxi-ir driver to set the base clock frequency from
> devicetree.
> 
> This is neccessary since there are different ir recievers on the
> market, that operate with different frequencys. So this value could be

s/neccessary/necessary/
s/recievers/receivers/
s/frequencys/frequencies/

> set if the attached ir receiver needs an other base clock frequency,
> than the default 8 MHz.

s/other/different/

> 
> Signed-off-by: Philipp Rossak 
> ---
>  drivers/media/rc/sunxi-cir.c | 20 
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> index 97f367b446c4..9bbe55a76860 100644
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c
> @@ -72,12 +72,8 @@
>  /* CIR_REG register idle threshold */
>  #define REG_CIR_ITHR(val)(((val) << 8) & (GENMASK(15, 8)))
>  
> -/* Required frequency for IR0 or IR1 clock in CIR mode */
> +/* Required frequency for IR0 or IR1 clock in CIR mode (default) */
>  #define SUNXI_IR_BASE_CLK 800
> -/* Frequency after IR internal divider  */
> -#define SUNXI_IR_CLK  (SUNXI_IR_BASE_CLK / 64)
> -/* Sample period in ns */
> -#define SUNXI_IR_SAMPLE   (10ul / SUNXI_IR_CLK)
>  /* Noise threshold in samples  */
>  #define SUNXI_IR_RXNOISE  1
>  /* Idle Threshold in samples */
> @@ -122,7 +118,7 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
>   /* for each bit in fifo */
>   dt = readb(ir->base + SUNXI_IR_RXFIFO_REG);
>   rawir.pulse = (dt & 0x80) != 0;
> - rawir.duration = ((dt & 0x7f) + 1) * SUNXI_IR_SAMPLE;
> + rawir.duration = ((dt & 0x7f) + 1) * 
> ir->rc->rx_resolution;

Line over 80 characters.

>   ir_raw_event_store_with_filter(ir->rc, );
>   }
>   }
> @@ -148,6 +144,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>   struct device_node *dn = dev->of_node;
>   struct resource *res;
>   struct sunxi_ir *ir;
> + u32 b_clk_freq;
>  
>   ir = devm_kzalloc(dev, sizeof(struct sunxi_ir), GFP_KERNEL);
>   if (!ir)
> @@ -172,6 +169,11 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>   return PTR_ERR(ir->clk);
>   }
>  
> + /* Base clock frequency (optional) */
> + if (of_property_read_u32(dn, "clock-frequency", _clk_freq)) {
> + b_clk_freq = SUNXI_IR_BASE_CLK;
> + }

No braces here please; please use ./scripts/checkpatch.pl to find issues
like this.

> +
>   /* Reset (optional) */
>   ir->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
>   if (IS_ERR(ir->rst))
> @@ -180,11 +182,12 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>   if (ret)
>   return ret;
>  
> - ret = clk_set_rate(ir->clk, SUNXI_IR_BASE_CLK);
> + ret = clk_set_rate(ir->clk, b_clk_freq);
>   if (ret) {
>   dev_err(dev, "set ir base clock failed!\n");
>   goto exit_reset_assert;
>   }
> + dev_info(dev, "set base clock frequency to %d Hz.\n", b_clk_freq);
>  
>   if (clk_prepare_enable(ir->apb_clk)) {
>   dev_err(dev, "try to enable apb_ir_clk failed\n");
> @@ -225,7 +228,8 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>   ir->rc->map_name = ir->map_name ?: RC_MAP_EMPTY;
>   ir->rc->dev.parent = dev;
>   ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
> - ir->rc->rx_resolution = SUNXI_IR_SAMPLE;
> + /* Frequency after IR internal divider with sample period in ns */
> + ir->rc->rx_resolution = (10ul / (b_clk_freq / 64));
>   ir->rc->timeout = MS_TO_NS(SUNXI_IR_TIMEOUT);
>   ir->rc->driver_name = SUNXI_IR_DEV;
>  
> -- 
> 2.11.0


[PATCH 2/5] media: dt: bindings: Update binding documentation for sunxi IR controller

2017-12-17 Thread Philipp Rossak
This patch updates documentation for Device-Tree bindings for sunxi IR
controller and adds the new optional property for the base clock
frequency.

Signed-off-by: Philipp Rossak 
---
 Documentation/devicetree/bindings/media/sunxi-ir.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt 
b/Documentation/devicetree/bindings/media/sunxi-ir.txt
index 91648c569b1e..9f45bab07d6e 100644
--- a/Documentation/devicetree/bindings/media/sunxi-ir.txt
+++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt
@@ -11,6 +11,7 @@ Required properties:
 Optional properties:
 - linux,rc-map-name: see rc.txt file in the same directory.
 - resets : phandle + reset specifier pair
+- clock-frequency  : overrides the default base clock frequency (8 MHz)
 
 Example:
 
@@ -18,6 +19,7 @@ ir0: ir@1c21800 {
compatible = "allwinner,sun4i-a10-ir";
clocks = <_gates 6>, <_clk>;
clock-names = "apb", "ir";
+   clock-frequency = <300>;
resets = <_rst 1>;
interrupts = <0 5 1>;
reg = <0x01C21800 0x40>;
-- 
2.11.0



[PATCH 1/5] media: rc: update sunxi-ir driver to get base clock frequency from devicetree

2017-12-17 Thread Philipp Rossak
This patch updates the sunxi-ir driver to set the base clock frequency from
devicetree.

This is neccessary since there are different ir recievers on the
market, that operate with different frequencys. So this value could be
set if the attached ir receiver needs an other base clock frequency,
than the default 8 MHz.

Signed-off-by: Philipp Rossak 
---
 drivers/media/rc/sunxi-cir.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 97f367b446c4..9bbe55a76860 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -72,12 +72,8 @@
 /* CIR_REG register idle threshold */
 #define REG_CIR_ITHR(val)(((val) << 8) & (GENMASK(15, 8)))
 
-/* Required frequency for IR0 or IR1 clock in CIR mode */
+/* Required frequency for IR0 or IR1 clock in CIR mode (default) */
 #define SUNXI_IR_BASE_CLK 800
-/* Frequency after IR internal divider  */
-#define SUNXI_IR_CLK  (SUNXI_IR_BASE_CLK / 64)
-/* Sample period in ns */
-#define SUNXI_IR_SAMPLE   (10ul / SUNXI_IR_CLK)
 /* Noise threshold in samples  */
 #define SUNXI_IR_RXNOISE  1
 /* Idle Threshold in samples */
@@ -122,7 +118,7 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
/* for each bit in fifo */
dt = readb(ir->base + SUNXI_IR_RXFIFO_REG);
rawir.pulse = (dt & 0x80) != 0;
-   rawir.duration = ((dt & 0x7f) + 1) * SUNXI_IR_SAMPLE;
+   rawir.duration = ((dt & 0x7f) + 1) * 
ir->rc->rx_resolution;
ir_raw_event_store_with_filter(ir->rc, );
}
}
@@ -148,6 +144,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
struct device_node *dn = dev->of_node;
struct resource *res;
struct sunxi_ir *ir;
+   u32 b_clk_freq;
 
ir = devm_kzalloc(dev, sizeof(struct sunxi_ir), GFP_KERNEL);
if (!ir)
@@ -172,6 +169,11 @@ static int sunxi_ir_probe(struct platform_device *pdev)
return PTR_ERR(ir->clk);
}
 
+   /* Base clock frequency (optional) */
+   if (of_property_read_u32(dn, "clock-frequency", _clk_freq)) {
+   b_clk_freq = SUNXI_IR_BASE_CLK;
+   }
+
/* Reset (optional) */
ir->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
if (IS_ERR(ir->rst))
@@ -180,11 +182,12 @@ static int sunxi_ir_probe(struct platform_device *pdev)
if (ret)
return ret;
 
-   ret = clk_set_rate(ir->clk, SUNXI_IR_BASE_CLK);
+   ret = clk_set_rate(ir->clk, b_clk_freq);
if (ret) {
dev_err(dev, "set ir base clock failed!\n");
goto exit_reset_assert;
}
+   dev_info(dev, "set base clock frequency to %d Hz.\n", b_clk_freq);
 
if (clk_prepare_enable(ir->apb_clk)) {
dev_err(dev, "try to enable apb_ir_clk failed\n");
@@ -225,7 +228,8 @@ static int sunxi_ir_probe(struct platform_device *pdev)
ir->rc->map_name = ir->map_name ?: RC_MAP_EMPTY;
ir->rc->dev.parent = dev;
ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
-   ir->rc->rx_resolution = SUNXI_IR_SAMPLE;
+   /* Frequency after IR internal divider with sample period in ns */
+   ir->rc->rx_resolution = (10ul / (b_clk_freq / 64));
ir->rc->timeout = MS_TO_NS(SUNXI_IR_TIMEOUT);
ir->rc->driver_name = SUNXI_IR_DEV;
 
-- 
2.11.0



[PATCH 3/5] arm: dts: sun8i: a83t: Add the ir pin for the A83T

2017-12-17 Thread Philipp Rossak
The CIR Pin of the A83T is located at PL12.

Signed-off-by: Philipp Rossak 
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi 
b/arch/arm/boot/dts/sun8i-a83t.dtsi
index a384b766f3dc..954c2393325f 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -521,6 +521,11 @@
drive-strength = <20>;
bias-pull-up;
};
+
+   ir_pins_a: ir@0 {
+   pins = "PL12";
+   function = "s_cir_rx";
+   };
};
 
r_rsb: rsb@1f03400 {
-- 
2.11.0



[PATCH 4/5] arm: dts: sun8i: a83t: Add support for the ir interface

2017-12-17 Thread Philipp Rossak
The ir interface is like on the H3 located at 0x01f02000 and is exactly
the same. This patch adds support for the ir interface on the A83T.

Signed-off-by: Philipp Rossak 
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi 
b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 954c2393325f..9e7ed3b9a6b8 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -503,6 +503,16 @@
#reset-cells = <1>;
};
 
+   ir: ir@01f02000 {
+   compatible = "allwinner,sun5i-a13-ir";
+   clocks = <_ccu CLK_APB0_IR>, <_ccu CLK_IR>;
+   clock-names = "apb", "ir";
+   resets = <_ccu RST_APB0_IR>;
+   interrupts = ;
+   reg = <0x01f02000 0x40>;
+   status = "disabled";
+   };
+
r_pio: pinctrl@1f02c00 {
compatible = "allwinner,sun8i-a83t-r-pinctrl";
reg = <0x01f02c00 0x400>;
-- 
2.11.0



[PATCH 0/5] arm: sunxi: IR support for A83T

2017-12-17 Thread Philipp Rossak
This patch series adds support for the sunxi A83T ir module and enhances the 
sunxi-ir driver.
Right now the base clock frequency for the ir driver is a hard coded define and 
is set to 8 MHz.
This works for the most common ir receivers. On the Sinovoip Bananapi M3 the ir 
receiver needs,
a 3 MHz base clock frequency to work without problems with this driver.

This patch series adds support for an optinal property that makes it able to 
override the default
base clock frequency and enables the ir interface on the a83t and the Bananapi 
M3.

changes since rfc:
* The property is now optinal. If the property is not available in the dtb the 
driver 
  uses the default base clock frequency.
* the driver prints out the the selected base clock frequency.
* changed devicetree property from base-clk-frequency to clock-frequency

Regards,
Philipp

rfc: https://www.mail-archive.com/linux-media@vger.kernel.org/msg123359.html 

Philipp Rossak (5):
  media: rc: update sunxi-ir driver to get base clock frequency from
devicetree
  media: dt: bindings: Update binding documentation for sunxi IR
controller
  arm: dts: sun8i: a83t: Add the ir pin for the A83T
  arm: dts: sun8i: a83t: Add support for the ir interface
  arm: dts: sun8i: a83t: bananapi-m3: Enable IR controller

 Documentation/devicetree/bindings/media/sunxi-ir.txt |  2 ++
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts |  7 +++
 arch/arm/boot/dts/sun8i-a83t.dtsi| 15 +++
 drivers/media/rc/sunxi-cir.c | 20 
 4 files changed, 36 insertions(+), 8 deletions(-)

-- 
2.11.0



[PATCH 5/5] arm: dts: sun8i: a83t: bananapi-m3: Enable IR controller

2017-12-17 Thread Philipp Rossak
The Bananapi M3 has an onboard IR receiver.
This enables the onboard IR receiver subnode.
Other than the other IR receivers this one needs a base clock frequency
of 300 Hz (3 MHz), to be able to work.

Signed-off-by: Philipp Rossak 
---
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts 
b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
index 6550bf0e594b..2bf25ca64133 100644
--- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
@@ -100,6 +100,13 @@
status = "okay";
 };
 
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins_a>;
+   clock-frequency = <300>;
+   status = "okay";
+};
+
  {
rgmii_phy: ethernet-phy@1 {
compatible = "ethernet-phy-ieee802.3-c22";
-- 
2.11.0



Re: Support For Charity

2017-12-17 Thread M. M. Fridman
I Mikhail Fridman. has selected you specially as one of my beneficiaries
for my Charitable Donation, Just as I have declared on May 23, 2016 to give
my fortune as charity.

Check the link below for confirmation:

http://www.ibtimes.co.uk/russias-second-wealthiest-man-mikhail-fridman-plans-leaving-14-2bn-fortune-charity-1561604

Reply as soon as possible with further directives.

Best Regards,
Mikhail Fridman.


Re: [PATCH v4 0/6] [media] Add analog mode support for Medion MD95700

2017-12-17 Thread Philippe Ombredanne
On Sun, Dec 17, 2017 at 7:46 PM, Maciej S. Szmigiero
 wrote:
> This series adds support for analog part of Medion 95700 in the cxusb
> driver.



> Changes from v3:
> Add SPDX tag to a newly added "cxusb-analog.c" file.

Thank you.

-- 
Cordially
Philippe Ombredanne


[PATCH v4 4/6] tuner-simple: allow setting mono radio mode

2017-12-17 Thread Maciej S. Szmigiero
For some types of tuners (Philips FMD1216ME(X) MK3 currently) we know that
letting TDA9887 output port 1 remain high (inactive) will switch FM radio
to mono mode.
Let's make use of this functionality - nothing changes for the default
stereo radio mode.

Tested on a Medion 95700 board which has a FMD1216ME tuner.

Signed-off-by: Maciej S. Szmigiero 
---
 drivers/media/tuners/tuner-simple.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/tuners/tuner-simple.c 
b/drivers/media/tuners/tuner-simple.c
index cf44d3657f55..01ab94681d2d 100644
--- a/drivers/media/tuners/tuner-simple.c
+++ b/drivers/media/tuners/tuner-simple.c
@@ -670,6 +670,7 @@ static int simple_set_radio_freq(struct dvb_frontend *fe,
int rc, j;
struct tuner_params *t_params;
unsigned int freq = params->frequency;
+   bool mono = params->audmode == V4L2_TUNER_MODE_MONO;
 
tun = priv->tun;
 
@@ -736,8 +737,8 @@ static int simple_set_radio_freq(struct dvb_frontend *fe,
config |= TDA9887_PORT2_ACTIVE;
if (t_params->intercarrier_mode)
config |= TDA9887_INTERCARRIER;
-/* if (t_params->port1_set_for_fm_mono)
-   config &= ~TDA9887_PORT1_ACTIVE;*/
+   if (t_params->port1_set_for_fm_mono && mono)
+   config &= ~TDA9887_PORT1_ACTIVE;
if (t_params->fm_gain_normal)
config |= TDA9887_GAIN_NORMAL;
if (t_params->radio_if == 2)


[PATCH v4 5/6] [media] cxusb: implement Medion MD95700 digital / analog coexistence

2017-12-17 Thread Maciej S. Szmigiero
This patch prepares cxusb driver for supporting the analog part of
Medion 95700 (previously only the digital - DVB - mode was supported).

Specifically, it adds support for:
* switching the device between analog and digital modes of operation,
* enforcing that only one mode is active at the same time due to hardware
limitations.

Actual implementation of the analog mode will be provided by the next
commit.

Signed-off-by: Maciej S. Szmigiero 
---
 drivers/media/usb/dvb-usb/cxusb.c| 452 +++
 drivers/media/usb/dvb-usb/cxusb.h|  48 
 drivers/media/usb/dvb-usb/dvb-usb-dvb.c  |  20 +-
 drivers/media/usb/dvb-usb/dvb-usb-init.c |  13 +
 drivers/media/usb/dvb-usb/dvb-usb.h  |   8 +
 5 files changed, 487 insertions(+), 54 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/cxusb.c 
b/drivers/media/usb/dvb-usb/cxusb.c
index 7109fc7ab74d..ea55b31cf681 100644
--- a/drivers/media/usb/dvb-usb/cxusb.c
+++ b/drivers/media/usb/dvb-usb/cxusb.c
@@ -16,6 +16,7 @@
  * Copyright (C) 2005 Patrick Boettcher (patrick.boettc...@posteo.de)
  * Copyright (C) 2006 Michael Krufky (mkru...@linuxtv.org)
  * Copyright (C) 2006, 2007 Chris Pascoe (c.pas...@itee.uq.edu.au)
+ * Copyright (C) 2011, 2017 Maciej S. Szmigiero (m...@maciej.szmigiero.name)
  *
  *   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
@@ -24,9 +25,12 @@
  * see Documentation/dvb/README.dvb-usb for more information
  */
 #include 
-#include 
-#include 
+#include 
+#include 
 #include 
+#include 
+#include 
+#include 
 
 #include "cxusb.h"
 
@@ -47,17 +51,46 @@
 #include "si2157.h"
 
 /* debug */
-static int dvb_usb_cxusb_debug;
+int dvb_usb_cxusb_debug;
 module_param_named(debug, dvb_usb_cxusb_debug, int, 0644);
-MODULE_PARM_DESC(debug, "set debugging level (1=rc (or-able))." 
DVB_USB_DEBUG_STATUS);
+MODULE_PARM_DESC(debug, "set debugging level (see cxusb.h)."
+DVB_USB_DEBUG_STATUS);
 
 DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
 
-#define deb_info(args...)   dprintk(dvb_usb_cxusb_debug, 0x03, args)
-#define deb_i2c(args...)dprintk(dvb_usb_cxusb_debug, 0x02, args)
+#define deb_info(args...)   dprintk(dvb_usb_cxusb_debug, CXUSB_DBG_MISC, args)
+#define deb_i2c(args...)dprintk(dvb_usb_cxusb_debug, CXUSB_DBG_I2C, args)
 
-static int cxusb_ctrl_msg(struct dvb_usb_device *d,
- u8 cmd, const u8 *wbuf, int wlen, u8 *rbuf, int rlen)
+enum cxusb_table_index {
+   MEDION_MD95700,
+   DVICO_BLUEBIRD_LG064F_COLD,
+   DVICO_BLUEBIRD_LG064F_WARM,
+   DVICO_BLUEBIRD_DUAL_1_COLD,
+   DVICO_BLUEBIRD_DUAL_1_WARM,
+   DVICO_BLUEBIRD_LGZ201_COLD,
+   DVICO_BLUEBIRD_LGZ201_WARM,
+   DVICO_BLUEBIRD_TH7579_COLD,
+   DVICO_BLUEBIRD_TH7579_WARM,
+   DIGITALNOW_BLUEBIRD_DUAL_1_COLD,
+   DIGITALNOW_BLUEBIRD_DUAL_1_WARM,
+   DVICO_BLUEBIRD_DUAL_2_COLD,
+   DVICO_BLUEBIRD_DUAL_2_WARM,
+   DVICO_BLUEBIRD_DUAL_4,
+   DVICO_BLUEBIRD_DVB_T_NANO_2,
+   DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM,
+   AVERMEDIA_VOLAR_A868R,
+   DVICO_BLUEBIRD_DUAL_4_REV_2,
+   CONEXANT_D680_DMB,
+   MYGICA_D689,
+   MYGICA_T230,
+   MYGICA_T230C,
+   NR__cxusb_table_index
+};
+
+static struct usb_device_id cxusb_table[];
+
+int cxusb_ctrl_msg(struct dvb_usb_device *d,
+  u8 cmd, const u8 *wbuf, int wlen, u8 *rbuf, int rlen)
 {
struct cxusb_state *st = d->priv;
int ret;
@@ -89,7 +122,8 @@ static void cxusb_gpio_tuner(struct dvb_usb_device *d, int 
onoff)
struct cxusb_state *st = d->priv;
u8 o[2], i;
 
-   if (st->gpio_write_state[GPIO_TUNER] == onoff)
+   if (st->gpio_write_state[GPIO_TUNER] == onoff &&
+   !st->gpio_write_refresh[GPIO_TUNER])
return;
 
o[0] = GPIO_TUNER;
@@ -100,6 +134,7 @@ static void cxusb_gpio_tuner(struct dvb_usb_device *d, int 
onoff)
deb_info("gpio_write failed.\n");
 
st->gpio_write_state[GPIO_TUNER] = onoff;
+   st->gpio_write_refresh[GPIO_TUNER] = false;
 }
 
 static int cxusb_bluebird_gpio_rw(struct dvb_usb_device *d, u8 changemask,
@@ -259,7 +294,7 @@ static int cxusb_i2c_xfer(struct i2c_adapter *adap, struct 
i2c_msg msg[],
 
 static u32 cxusb_i2c_func(struct i2c_adapter *adapter)
 {
-   return I2C_FUNC_I2C;
+   return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
 }
 
 static struct i2c_algorithm cxusb_i2c_algo = {
@@ -267,15 +302,48 @@ static struct i2c_algorithm cxusb_i2c_algo = {
.functionality = cxusb_i2c_func,
 };
 
-static int cxusb_power_ctrl(struct dvb_usb_device *d, int onoff)
+static int _cxusb_power_ctrl(struct dvb_usb_device *d, int onoff)
 {
u8 b = 0;
+
+   deb_info("setting power %s\n", onoff ? "ON" : "OFF");
+
if (onoff)
return cxusb_ctrl_msg(d, CMD_POWER_ON, , 1, NULL, 0);
else

[PATCH v4 6/6] [media] cxusb: add analog mode support for Medion MD95700

2017-12-17 Thread Maciej S. Szmigiero
This patch adds support for analog part of Medion 95700 in the cxusb
driver.

What works:
* Video capture at various sizes with sequential fields,
* Input switching (TV Tuner, Composite, S-Video),
* TV and radio tuning,
* Video standard switching and auto detection,
* Radio mode switching (stereo / mono),
* Unplugging while capturing,
* DVB / analog coexistence,
* Raw BT.656 stream support.

What does not work yet:
* Audio,
* VBI,
* Picture controls.

Signed-off-by: Maciej S. Szmigiero 
---
 drivers/media/usb/dvb-usb/Kconfig|8 +-
 drivers/media/usb/dvb-usb/Makefile   |2 +-
 drivers/media/usb/dvb-usb/cxusb-analog.c | 1923 ++
 drivers/media/usb/dvb-usb/cxusb.c|3 +-
 drivers/media/usb/dvb-usb/cxusb.h|  114 +-
 5 files changed, 2031 insertions(+), 19 deletions(-)
 create mode 100644 drivers/media/usb/dvb-usb/cxusb-analog.c

diff --git a/drivers/media/usb/dvb-usb/Kconfig 
b/drivers/media/usb/dvb-usb/Kconfig
index 2651ae277347..85ac8b3c0e5f 100644
--- a/drivers/media/usb/dvb-usb/Kconfig
+++ b/drivers/media/usb/dvb-usb/Kconfig
@@ -120,7 +120,9 @@ config DVB_USB_UMT_010
 
 config DVB_USB_CXUSB
tristate "Conexant USB2.0 hybrid reference design support"
-   depends on DVB_USB
+   depends on DVB_USB && VIDEO_V4L2
+   select VIDEO_CX25840
+   select VIDEOBUF2_VMALLOC
select DVB_PLL if MEDIA_SUBDRV_AUTOSELECT
select DVB_CX22702 if MEDIA_SUBDRV_AUTOSELECT
select DVB_LGDT330X if MEDIA_SUBDRV_AUTOSELECT
@@ -138,8 +140,8 @@ config DVB_USB_CXUSB
select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
help
  Say Y here to support the Conexant USB2.0 hybrid reference design.
- Currently, only DVB and ATSC modes are supported, analog mode
- shall be added in the future. Devices that require this module:
+ DVB and ATSC modes are supported, with basic analog mode support
+ for Medion MD95700. Devices that require this module:
 
  Medion MD95700 hybrid USB2.0 device.
  DViCO FusionHDTV (Bluebird) USB2.0 devices
diff --git a/drivers/media/usb/dvb-usb/Makefile 
b/drivers/media/usb/dvb-usb/Makefile
index 16de1e4f36a4..61e32aacfef6 100644
--- a/drivers/media/usb/dvb-usb/Makefile
+++ b/drivers/media/usb/dvb-usb/Makefile
@@ -41,7 +41,7 @@ obj-$(CONFIG_DVB_USB_M920X) += dvb-usb-m920x.o
 dvb-usb-digitv-objs := digitv.o
 obj-$(CONFIG_DVB_USB_DIGITV) += dvb-usb-digitv.o
 
-dvb-usb-cxusb-objs := cxusb.o
+dvb-usb-cxusb-objs := cxusb.o cxusb-analog.o
 obj-$(CONFIG_DVB_USB_CXUSB) += dvb-usb-cxusb.o
 
 dvb-usb-ttusb2-objs := ttusb2.o
diff --git a/drivers/media/usb/dvb-usb/cxusb-analog.c 
b/drivers/media/usb/dvb-usb/cxusb-analog.c
new file mode 100644
index ..de345cb42e23
--- /dev/null
+++ b/drivers/media/usb/dvb-usb/cxusb-analog.c
@@ -0,0 +1,1923 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* DVB USB compliant linux driver for Conexant USB reference design -
+ * (analog part).
+ *
+ * Copyright (C) 2011, 2017 Maciej S. Szmigiero (m...@maciej.szmigiero.name)
+ *
+ * TODO:
+ *  * audio support,
+ *  * finish radio support (requires audio of course),
+ *  * VBI support,
+ *  * controls support
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "cxusb.h"
+
+static int cxusb_medion_v_queue_setup(struct vb2_queue *q,
+ unsigned int *num_buffers,
+ unsigned int *num_planes,
+ unsigned int sizes[],
+ struct device *alloc_devs[])
+{
+   struct dvb_usb_device *dvbdev = vb2_get_drv_priv(q);
+   struct cxusb_medion_dev *cxdev = dvbdev->priv;
+   unsigned int size = cxdev->raw_mode ?
+   CXUSB_VIDEO_MAX_FRAME_SIZE :
+   cxdev->width * cxdev->height * 2;
+
+   if (*num_planes > 0) {
+   if (*num_planes != 1)
+   return -EINVAL;
+
+   if (sizes[0] < size)
+   return -EINVAL;
+   } else {
+   *num_planes = 1;
+   sizes[0] = size;
+   }
+
+   if (q->num_buffers + *num_buffers < 6)
+   *num_buffers = 6 - q->num_buffers;
+
+   return 0;
+}
+
+static int cxusb_medion_v_buf_init(struct vb2_buffer *vb)
+{
+   struct dvb_usb_device *dvbdev = vb2_get_drv_priv(vb->vb2_queue);
+   struct cxusb_medion_dev *cxdev = dvbdev->priv;
+
+   cxusb_vprintk(dvbdev, OPS, "buffer init\n");
+
+   if (cxdev->raw_mode) {
+   if (vb2_plane_size(vb, 0) < CXUSB_VIDEO_MAX_FRAME_SIZE)
+   return -ENOMEM;
+   } else {
+   if (vb2_plane_size(vb, 0) < cxdev->width * cxdev->height * 2)
+   return -ENOMEM;
+   }
+
+   cxusb_vprintk(dvbdev, OPS, "buffer OK\n");
+
+   return 0;
+}

[PATCH v4 3/6] cx25840: add pin to pad mapping and output format configuration

2017-12-17 Thread Maciej S. Szmigiero
This commit adds pin to pad mapping and output format configuration support
in CX2584x-series chips to cx25840 driver.

This functionality is then used to allow disabling ivtv-specific hacks
(called a "generic mode"), so cx25840 driver can be used for other devices
not needing them without risking compatibility problems.

Signed-off-by: Maciej S. Szmigiero 
---
 drivers/media/i2c/cx25840/cx25840-core.c | 396 ++-
 drivers/media/i2c/cx25840/cx25840-core.h |  13 +
 drivers/media/i2c/cx25840/cx25840-vbi.c  |   3 +
 include/media/drv-intf/cx25840.h |  74 +-
 4 files changed, 484 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/cx25840/cx25840-core.c 
b/drivers/media/i2c/cx25840/cx25840-core.c
index 940c8b171249..7027f73201bf 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.c
+++ b/drivers/media/i2c/cx25840/cx25840-core.c
@@ -21,6 +21,9 @@
  * CX23888 DIF support for the HVR1850
  * Copyright (C) 2011 Steven Toth 
  *
+ * CX2584x pin to pad mapping and output format configuration support are
+ * Copyright (C) 2011 Maciej S. Szmigiero 
+ *
  * 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
@@ -316,6 +319,260 @@ static int cx23885_s_io_pin_config(struct v4l2_subdev 
*sd, size_t n,
return 0;
 }
 
+static u8 cx25840_function_to_pad(struct i2c_client *client, u8 function)
+{
+   switch (function) {
+   case CX25840_PAD_ACTIVE:
+   return 1;
+
+   case CX25840_PAD_VACTIVE:
+   return 2;
+
+   case CX25840_PAD_CBFLAG:
+   return 3;
+
+   case CX25840_PAD_VID_DATA_EXT0:
+   return 4;
+
+   case CX25840_PAD_VID_DATA_EXT1:
+   return 5;
+
+   case CX25840_PAD_GPO0:
+   return 6;
+
+   case CX25840_PAD_GPO1:
+   return 7;
+
+   case CX25840_PAD_GPO2:
+   return 8;
+
+   case CX25840_PAD_GPO3:
+   return 9;
+
+   case CX25840_PAD_IRQ_N:
+   return 10;
+
+   case CX25840_PAD_AC_SYNC:
+   return 11;
+
+   case CX25840_PAD_AC_SDOUT:
+   return 12;
+
+   case CX25840_PAD_PLL_CLK:
+   return 13;
+
+   case CX25840_PAD_VRESET:
+   return 14;
+
+   default:
+   if (function != CX25840_PAD_DEFAULT)
+   v4l_err(client,
+   "invalid function %u, assuming default\n",
+   (unsigned int)function);
+   return 0;
+   }
+}
+
+static void cx25840_set_invert(u8 *pinctrl3, u8 *voutctrl4, u8 function,
+  u8 pin, bool invert)
+{
+   switch (function) {
+   case CX25840_PAD_IRQ_N:
+   if (invert)
+   *pinctrl3 &= ~2;
+   else
+   *pinctrl3 |= 2;
+   break;
+
+   case CX25840_PAD_ACTIVE:
+   if (invert)
+   *voutctrl4 |= BIT(2);
+   else
+   *voutctrl4 &= ~BIT(2);
+   break;
+
+   case CX25840_PAD_VACTIVE:
+   if (invert)
+   *voutctrl4 |= BIT(5);
+   else
+   *voutctrl4 &= ~BIT(5);
+   break;
+
+   case CX25840_PAD_CBFLAG:
+   if (invert)
+   *voutctrl4 |= BIT(4);
+   else
+   *voutctrl4 &= ~BIT(4);
+   break;
+
+   case CX25840_PAD_VRESET:
+   if (invert)
+   *voutctrl4 |= BIT(0);
+   else
+   *voutctrl4 &= ~BIT(0);
+   break;
+   }
+
+   if (function != CX25840_PAD_DEFAULT)
+   return;
+
+   switch (pin) {
+   case CX25840_PIN_DVALID_PRGM0:
+   if (invert)
+   *voutctrl4 |= BIT(6);
+   else
+   *voutctrl4 &= ~BIT(6);
+   break;
+
+   case CX25840_PIN_HRESET_PRGM2:
+   if (invert)
+   *voutctrl4 |= BIT(1);
+   else
+   *voutctrl4 &= ~BIT(1);
+   break;
+   }
+}
+
+static int cx25840_s_io_pin_config(struct v4l2_subdev *sd, size_t n,
+  struct v4l2_subdev_io_pin_config *p)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(sd);
+   unsigned int i;
+   u8 pinctrl[6], pinconf[10], voutctrl4;
+
+   for (i = 0; i < 6; i++)
+   pinctrl[i] = cx25840_read(client, 0x114 + i);
+
+   for (i = 0; i < 10; i++)
+   pinconf[i] = cx25840_read(client, 0x11c + i);
+
+   voutctrl4 = cx25840_read(client, 0x407);
+
+   for (i = 0; i < n; i++) {
+  

[PATCH v4 2/6] cx25840: add kernel-doc description of struct cx25840_state

2017-12-17 Thread Maciej S. Szmigiero
This commit describes a device instance private data of the driver
(struct cx25840_state) in a kernel-doc style comment.

Signed-off-by: Maciej S. Szmigiero 
---
 drivers/media/i2c/cx25840/cx25840-core.h | 33 ++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/cx25840/cx25840-core.h 
b/drivers/media/i2c/cx25840/cx25840-core.h
index 55432ed42714..877b930e5b1f 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.h
+++ b/drivers/media/i2c/cx25840/cx25840-core.h
@@ -45,6 +45,35 @@ enum cx25840_media_pads {
CX25840_NUM_PADS
 };
 
+/**
+ * struct cx25840_state - a device instance private data
+ * @c: i2c_client struct representing this device
+ * @sd:our V4L2 sub-device
+ * @hdl:   our V4L2 control handler
+ * @volume:audio volume V4L2 control (non-cx2583x devices only)
+ * @mute:  audio mute V4L2 control (non-cx2583x devices only)
+ * @pvr150_workaround: whether we enable workaround for Hauppauge PVR150
+ * hardware bug (audio dropping out)
+ * @radio: set if we are currently in the radio mode, otherwise
+ * the current mode is non-radio (that is, video)
+ * @std:   currently set video standard
+ * @vid_input: currently set video input
+ * @aud_input: currently set audio input
+ * @audclk_freq:   currently set audio sample rate
+ * @audmode:   currently set audio mode (when in non-radio mode)
+ * @vbi_line_offset:   vbi line number offset
+ * @id:exact device model
+ * @rev:   raw device id read from the chip
+ * @is_initialized:whether we have already loaded firmware into the chip
+ * and initialized it
+ * @vbi_regs_offset:   offset of vbi regs
+ * @fw_wait:   wait queue to wake an initalization function up when
+ * firmware loading (on a separate workqueue) finishes
+ * @fw_work:   a work that actually loads the firmware on a separate
+ * workqueue
+ * @ir_state:  a pointer to chip IR controller private data
+ * @pads:  array of supported chip pads (currently only a stub)
+ */
 struct cx25840_state {
struct i2c_client *c;
struct v4l2_subdev sd;
@@ -66,8 +95,8 @@ struct cx25840_state {
u32 rev;
int is_initialized;
unsigned vbi_regs_offset;
-   wait_queue_head_t fw_wait;/* wake up when the fw load is finished */
-   struct work_struct fw_work;   /* work entry for fw load */
+   wait_queue_head_t fw_wait;
+   struct work_struct fw_work;
struct cx25840_ir_state *ir_state;
 #if defined(CONFIG_MEDIA_CONTROLLER)
struct media_padpads[CX25840_NUM_PADS];


[PATCH v4 0/6] [media] Add analog mode support for Medion MD95700

2017-12-17 Thread Maciej S. Szmigiero
This series adds support for analog part of Medion 95700 in the cxusb
driver.

What works:
* Video capture at various sizes with sequential fields,
* Input switching (TV Tuner, Composite, S-Video),
* TV and radio tuning,
* Video standard switching and auto detection,
* Radio mode switching (stereo / mono),
* Unplugging while capturing,
* DVB / analog coexistence,
* Raw BT.656 stream support.

What does not work yet:
* Audio,
* VBI,
* Picture controls.

This series (as a one patch) was submitted for inclusion few years ago,
then waited few months in a patch queue.
Unfortunately, by the time it was supposed to be merged there
were enough changes in media that it was no longer mergable.

I thought at that time that I will be able to rebase and retest it soon
but unfortunately up till now I was never able to find enough time to do
so.
Also, with the passing of time the implementation diverged more and
more from the current kernel code, necessitating even more reworking.

That last iteration can be found here:
https://patchwork.linuxtv.org/patch/8048/

Since that version there had been the following changes:
* Adaptation to changes in V4L2 / DVB core,

* Radio device was added, with a possibility to tune to a FM radio
station and switch between stereo and mono modes (tested by taping
audio signal directly at tuner output pin),

* DVB / analog coexistence was improved - resolved a few cases where
DVB core would switch off power or reset the tuner when the device
was still being used but in the analog mode,

* Fixed issues reported by v4l2-compliance,

* Switching to raw BT.656 mode is now done by a custom streaming
parameter set via VIDIOC_S_PARM ioctl instead of using a
V4L2_BUF_TYPE_PRIVATE buffer (which was removed from V4L2),

* General small code cleanups (like using BIT() or ARRAY_SIZE() macros
instead of open coding them, code formatting improvements, etc.).

Changes from v1:
* Only support configuration of cx25840 pins that the cxusb driver is
actually using so there is no need for an ugly CX25840_PIN() macro,

* Split cxusb changes into two patches: first one implementing
digital / analog coexistence in this driver, second one adding the
actual implementation of the analog mode,

* Fix a warning reported by kbuild test robot.

Changes from v2:
* Split out ivtv cx25840 platform data zero-initialization to a separate
commit,

* Add kernel-doc description of struct cx25840_state,

* Make sure that all variables used in CX25840_VCONFIG_OPTION() and
CX25840_VCONFIG_SET_BIT() macros are their explicit parameters,

* Split out some code from cxusb_medion_copy_field() and
cxusb_medion_v_complete_work() functions to separate ones to increase
their readability,

* Generate masks using GENMASK() and BIT() macros in cx25840.h and
cxusb.h.

Changes from v3:
Add SPDX tag to a newly added "cxusb-analog.c" file.

Maciej S. Szmigiero (6):
  ivtv: zero-initialize cx25840 platform data
  cx25840: add kernel-doc description of struct cx25840_state
  cx25840: add pin to pad mapping and output format configuration
  tuner-simple: allow setting mono radio mode
  [media] cxusb: implement Medion MD95700 digital / analog coexistence
  [media] cxusb: add analog mode support for Medion MD95700

 drivers/media/i2c/cx25840/cx25840-core.c |  396 +-
 drivers/media/i2c/cx25840/cx25840-core.h |   46 +-
 drivers/media/i2c/cx25840/cx25840-vbi.c  |3 +
 drivers/media/pci/ivtv/ivtv-i2c.c|1 +
 drivers/media/tuners/tuner-simple.c  |5 +-
 drivers/media/usb/dvb-usb/Kconfig|8 +-
 drivers/media/usb/dvb-usb/Makefile   |2 +-
 drivers/media/usb/dvb-usb/cxusb-analog.c | 1923 ++
 drivers/media/usb/dvb-usb/cxusb.c|  455 ++-
 drivers/media/usb/dvb-usb/cxusb.h|  136 +++
 drivers/media/usb/dvb-usb/dvb-usb-dvb.c  |   20 +-
 drivers/media/usb/dvb-usb/dvb-usb-init.c |   13 +
 drivers/media/usb/dvb-usb/dvb-usb.h  |8 +
 include/media/drv-intf/cx25840.h |   74 +-
 14 files changed, 3024 insertions(+), 66 deletions(-)
 create mode 100644 drivers/media/usb/dvb-usb/cxusb-analog.c


[PATCH v4 1/6] ivtv: zero-initialize cx25840 platform data

2017-12-17 Thread Maciej S. Szmigiero
We need to zero-initialize cx25840 platform data structure to make sure
that its future members do not contain random stack garbage.

Signed-off-by: Maciej S. Szmigiero 
---
 drivers/media/pci/ivtv/ivtv-i2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/pci/ivtv/ivtv-i2c.c 
b/drivers/media/pci/ivtv/ivtv-i2c.c
index 66696e6ee587..b755337ec938 100644
--- a/drivers/media/pci/ivtv/ivtv-i2c.c
+++ b/drivers/media/pci/ivtv/ivtv-i2c.c
@@ -293,6 +293,7 @@ int ivtv_i2c_register(struct ivtv *itv, unsigned idx)
.platform_data = ,
};
 
+   memset(, 0, sizeof(pdata));
pdata.pvr150_workaround = itv->pvr150_workaround;
sd = v4l2_i2c_new_subdev_board(>v4l2_dev, adap,
_info, NULL);


Re: [PATCH v3 6/6] [media] cxusb: add analog mode support for Medion MD95700

2017-12-17 Thread Maciej S. Szmigiero
On 17.12.2017 19:24, Philippe Ombredanne wrote:
> Maciej,
> 
> On Sun, Dec 17, 2017 at 6:37 PM, Maciej S. Szmigiero
>  wrote:
>> This patch adds support for analog part of Medion 95700 in the cxusb
>> driver.
> 
> 
>> --- /dev/null
>> +++ b/drivers/media/usb/dvb-usb/cxusb-analog.c
>> @@ -0,0 +1,1927 @@
>> +/* DVB USB compliant linux driver for Conexant USB reference design -
>> + * (analog part).
>> + *
>> + * Copyright (C) 2011, 2017 Maciej S. Szmigiero (m...@maciej.szmigiero.name)
>> + *
>> + * TODO:
>> + *  * audio support,
>> + *  * finish radio support (requires audio of course),
>> + *  * VBI support,
>> + *  * controls support
>> + *
>> + * 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.
>> + */
> 
> Would you mind using the new SPDX tags here. See Thomas patches [1]. Thanks!
> 
> [1] https://lkml.org/lkml/2017/12/4/934
> 

Will add this tag in a moment, thanks for pointing this out.

Best regards,
Maciej Szmigiero


Re: [PATCH v3 6/6] [media] cxusb: add analog mode support for Medion MD95700

2017-12-17 Thread Philippe Ombredanne
Maciej,

On Sun, Dec 17, 2017 at 6:37 PM, Maciej S. Szmigiero
 wrote:
> This patch adds support for analog part of Medion 95700 in the cxusb
> driver.


> --- /dev/null
> +++ b/drivers/media/usb/dvb-usb/cxusb-analog.c
> @@ -0,0 +1,1927 @@
> +/* DVB USB compliant linux driver for Conexant USB reference design -
> + * (analog part).
> + *
> + * Copyright (C) 2011, 2017 Maciej S. Szmigiero (m...@maciej.szmigiero.name)
> + *
> + * TODO:
> + *  * audio support,
> + *  * finish radio support (requires audio of course),
> + *  * VBI support,
> + *  * controls support
> + *
> + * 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.
> + */

Would you mind using the new SPDX tags here. See Thomas patches [1]. Thanks!

[1] https://lkml.org/lkml/2017/12/4/934
-- 
Cordially
Philippe Ombredanne


Re: [PATCH v5 0/4] NVIDIA Tegra video decoder driver

2017-12-17 Thread Dmitry Osipenko
On 12.12.2017 03:26, Dmitry Osipenko wrote:
> VDE driver provides accelerated video decoding to NVIDIA Tegra SoC's,
> it is a result of reverse-engineering efforts. Driver has been tested on
> Toshiba AC100 and Acer A500, it should work on any Tegra20 device.
> 
> In userspace this driver is utilized by libvdpau-tegra [0] that implements
> VDPAU interface, so any video player that supports VDPAU can provide
> accelerated video decoding on Tegra20 on Linux.
> 
> [0] https://github.com/grate-driver/libvdpau-tegra

Thierry, driver has been approved by media maintainers and should appear in 4.16
(it is already in -next). Please schedule the DT patches for 4.16, thanks.


[PATCH v3 6/6] [media] cxusb: add analog mode support for Medion MD95700

2017-12-17 Thread Maciej S. Szmigiero
This patch adds support for analog part of Medion 95700 in the cxusb
driver.

What works:
* Video capture at various sizes with sequential fields,
* Input switching (TV Tuner, Composite, S-Video),
* TV and radio tuning,
* Video standard switching and auto detection,
* Radio mode switching (stereo / mono),
* Unplugging while capturing,
* DVB / analog coexistence,
* Raw BT.656 stream support.

What does not work yet:
* Audio,
* VBI,
* Picture controls.

Signed-off-by: Maciej S. Szmigiero 
---
 drivers/media/usb/dvb-usb/Kconfig|8 +-
 drivers/media/usb/dvb-usb/Makefile   |2 +-
 drivers/media/usb/dvb-usb/cxusb-analog.c | 1927 ++
 drivers/media/usb/dvb-usb/cxusb.c|3 +-
 drivers/media/usb/dvb-usb/cxusb.h|  114 +-
 5 files changed, 2035 insertions(+), 19 deletions(-)
 create mode 100644 drivers/media/usb/dvb-usb/cxusb-analog.c

diff --git a/drivers/media/usb/dvb-usb/Kconfig 
b/drivers/media/usb/dvb-usb/Kconfig
index 2651ae277347..85ac8b3c0e5f 100644
--- a/drivers/media/usb/dvb-usb/Kconfig
+++ b/drivers/media/usb/dvb-usb/Kconfig
@@ -120,7 +120,9 @@ config DVB_USB_UMT_010
 
 config DVB_USB_CXUSB
tristate "Conexant USB2.0 hybrid reference design support"
-   depends on DVB_USB
+   depends on DVB_USB && VIDEO_V4L2
+   select VIDEO_CX25840
+   select VIDEOBUF2_VMALLOC
select DVB_PLL if MEDIA_SUBDRV_AUTOSELECT
select DVB_CX22702 if MEDIA_SUBDRV_AUTOSELECT
select DVB_LGDT330X if MEDIA_SUBDRV_AUTOSELECT
@@ -138,8 +140,8 @@ config DVB_USB_CXUSB
select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
help
  Say Y here to support the Conexant USB2.0 hybrid reference design.
- Currently, only DVB and ATSC modes are supported, analog mode
- shall be added in the future. Devices that require this module:
+ DVB and ATSC modes are supported, with basic analog mode support
+ for Medion MD95700. Devices that require this module:
 
  Medion MD95700 hybrid USB2.0 device.
  DViCO FusionHDTV (Bluebird) USB2.0 devices
diff --git a/drivers/media/usb/dvb-usb/Makefile 
b/drivers/media/usb/dvb-usb/Makefile
index 16de1e4f36a4..61e32aacfef6 100644
--- a/drivers/media/usb/dvb-usb/Makefile
+++ b/drivers/media/usb/dvb-usb/Makefile
@@ -41,7 +41,7 @@ obj-$(CONFIG_DVB_USB_M920X) += dvb-usb-m920x.o
 dvb-usb-digitv-objs := digitv.o
 obj-$(CONFIG_DVB_USB_DIGITV) += dvb-usb-digitv.o
 
-dvb-usb-cxusb-objs := cxusb.o
+dvb-usb-cxusb-objs := cxusb.o cxusb-analog.o
 obj-$(CONFIG_DVB_USB_CXUSB) += dvb-usb-cxusb.o
 
 dvb-usb-ttusb2-objs := ttusb2.o
diff --git a/drivers/media/usb/dvb-usb/cxusb-analog.c 
b/drivers/media/usb/dvb-usb/cxusb-analog.c
new file mode 100644
index ..8fc290615514
--- /dev/null
+++ b/drivers/media/usb/dvb-usb/cxusb-analog.c
@@ -0,0 +1,1927 @@
+/* DVB USB compliant linux driver for Conexant USB reference design -
+ * (analog part).
+ *
+ * Copyright (C) 2011, 2017 Maciej S. Szmigiero (m...@maciej.szmigiero.name)
+ *
+ * TODO:
+ *  * audio support,
+ *  * finish radio support (requires audio of course),
+ *  * VBI support,
+ *  * controls support
+ *
+ * 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 "cxusb.h"
+
+static int cxusb_medion_v_queue_setup(struct vb2_queue *q,
+ unsigned int *num_buffers,
+ unsigned int *num_planes,
+ unsigned int sizes[],
+ struct device *alloc_devs[])
+{
+   struct dvb_usb_device *dvbdev = vb2_get_drv_priv(q);
+   struct cxusb_medion_dev *cxdev = dvbdev->priv;
+   unsigned int size = cxdev->raw_mode ?
+   CXUSB_VIDEO_MAX_FRAME_SIZE :
+   cxdev->width * cxdev->height * 2;
+
+   if (*num_planes > 0) {
+   if (*num_planes != 1)
+   return -EINVAL;
+
+   if (sizes[0] < size)
+   return -EINVAL;
+   } else {
+   *num_planes = 1;
+   sizes[0] = size;
+   }
+
+   if (q->num_buffers + *num_buffers < 6)
+   *num_buffers = 6 - q->num_buffers;
+
+   return 0;
+}
+
+static int cxusb_medion_v_buf_init(struct vb2_buffer *vb)
+{
+   struct dvb_usb_device *dvbdev = vb2_get_drv_priv(vb->vb2_queue);
+   struct cxusb_medion_dev *cxdev = dvbdev->priv;
+
+   cxusb_vprintk(dvbdev, OPS, "buffer init\n");
+
+   if (cxdev->raw_mode) {
+   if (vb2_plane_size(vb, 0) < CXUSB_VIDEO_MAX_FRAME_SIZE)
+   return 

[PATCH v3 4/6] tuner-simple: allow setting mono radio mode

2017-12-17 Thread Maciej S. Szmigiero
For some types of tuners (Philips FMD1216ME(X) MK3 currently) we know that
letting TDA9887 output port 1 remain high (inactive) will switch FM radio
to mono mode.
Let's make use of this functionality - nothing changes for the default
stereo radio mode.

Tested on a Medion 95700 board which has a FMD1216ME tuner.

Signed-off-by: Maciej S. Szmigiero 
---
 drivers/media/tuners/tuner-simple.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/tuners/tuner-simple.c 
b/drivers/media/tuners/tuner-simple.c
index cf44d3657f55..01ab94681d2d 100644
--- a/drivers/media/tuners/tuner-simple.c
+++ b/drivers/media/tuners/tuner-simple.c
@@ -670,6 +670,7 @@ static int simple_set_radio_freq(struct dvb_frontend *fe,
int rc, j;
struct tuner_params *t_params;
unsigned int freq = params->frequency;
+   bool mono = params->audmode == V4L2_TUNER_MODE_MONO;
 
tun = priv->tun;
 
@@ -736,8 +737,8 @@ static int simple_set_radio_freq(struct dvb_frontend *fe,
config |= TDA9887_PORT2_ACTIVE;
if (t_params->intercarrier_mode)
config |= TDA9887_INTERCARRIER;
-/* if (t_params->port1_set_for_fm_mono)
-   config &= ~TDA9887_PORT1_ACTIVE;*/
+   if (t_params->port1_set_for_fm_mono && mono)
+   config &= ~TDA9887_PORT1_ACTIVE;
if (t_params->fm_gain_normal)
config |= TDA9887_GAIN_NORMAL;
if (t_params->radio_if == 2)


[PATCH v3 5/6] [media] cxusb: implement Medion MD95700 digital / analog coexistence

2017-12-17 Thread Maciej S. Szmigiero
This patch prepares cxusb driver for supporting the analog part of
Medion 95700 (previously only the digital - DVB - mode was supported).

Specifically, it adds support for:
* switching the device between analog and digital modes of operation,
* enforcing that only one mode is active at the same time due to hardware
limitations.

Actual implementation of the analog mode will be provided by the next
commit.

Signed-off-by: Maciej S. Szmigiero 
---
 drivers/media/usb/dvb-usb/cxusb.c| 452 +++
 drivers/media/usb/dvb-usb/cxusb.h|  48 
 drivers/media/usb/dvb-usb/dvb-usb-dvb.c  |  20 +-
 drivers/media/usb/dvb-usb/dvb-usb-init.c |  13 +
 drivers/media/usb/dvb-usb/dvb-usb.h  |   8 +
 5 files changed, 487 insertions(+), 54 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/cxusb.c 
b/drivers/media/usb/dvb-usb/cxusb.c
index 7109fc7ab74d..ea55b31cf681 100644
--- a/drivers/media/usb/dvb-usb/cxusb.c
+++ b/drivers/media/usb/dvb-usb/cxusb.c
@@ -16,6 +16,7 @@
  * Copyright (C) 2005 Patrick Boettcher (patrick.boettc...@posteo.de)
  * Copyright (C) 2006 Michael Krufky (mkru...@linuxtv.org)
  * Copyright (C) 2006, 2007 Chris Pascoe (c.pas...@itee.uq.edu.au)
+ * Copyright (C) 2011, 2017 Maciej S. Szmigiero (m...@maciej.szmigiero.name)
  *
  *   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
@@ -24,9 +25,12 @@
  * see Documentation/dvb/README.dvb-usb for more information
  */
 #include 
-#include 
-#include 
+#include 
+#include 
 #include 
+#include 
+#include 
+#include 
 
 #include "cxusb.h"
 
@@ -47,17 +51,46 @@
 #include "si2157.h"
 
 /* debug */
-static int dvb_usb_cxusb_debug;
+int dvb_usb_cxusb_debug;
 module_param_named(debug, dvb_usb_cxusb_debug, int, 0644);
-MODULE_PARM_DESC(debug, "set debugging level (1=rc (or-able))." 
DVB_USB_DEBUG_STATUS);
+MODULE_PARM_DESC(debug, "set debugging level (see cxusb.h)."
+DVB_USB_DEBUG_STATUS);
 
 DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
 
-#define deb_info(args...)   dprintk(dvb_usb_cxusb_debug, 0x03, args)
-#define deb_i2c(args...)dprintk(dvb_usb_cxusb_debug, 0x02, args)
+#define deb_info(args...)   dprintk(dvb_usb_cxusb_debug, CXUSB_DBG_MISC, args)
+#define deb_i2c(args...)dprintk(dvb_usb_cxusb_debug, CXUSB_DBG_I2C, args)
 
-static int cxusb_ctrl_msg(struct dvb_usb_device *d,
- u8 cmd, const u8 *wbuf, int wlen, u8 *rbuf, int rlen)
+enum cxusb_table_index {
+   MEDION_MD95700,
+   DVICO_BLUEBIRD_LG064F_COLD,
+   DVICO_BLUEBIRD_LG064F_WARM,
+   DVICO_BLUEBIRD_DUAL_1_COLD,
+   DVICO_BLUEBIRD_DUAL_1_WARM,
+   DVICO_BLUEBIRD_LGZ201_COLD,
+   DVICO_BLUEBIRD_LGZ201_WARM,
+   DVICO_BLUEBIRD_TH7579_COLD,
+   DVICO_BLUEBIRD_TH7579_WARM,
+   DIGITALNOW_BLUEBIRD_DUAL_1_COLD,
+   DIGITALNOW_BLUEBIRD_DUAL_1_WARM,
+   DVICO_BLUEBIRD_DUAL_2_COLD,
+   DVICO_BLUEBIRD_DUAL_2_WARM,
+   DVICO_BLUEBIRD_DUAL_4,
+   DVICO_BLUEBIRD_DVB_T_NANO_2,
+   DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM,
+   AVERMEDIA_VOLAR_A868R,
+   DVICO_BLUEBIRD_DUAL_4_REV_2,
+   CONEXANT_D680_DMB,
+   MYGICA_D689,
+   MYGICA_T230,
+   MYGICA_T230C,
+   NR__cxusb_table_index
+};
+
+static struct usb_device_id cxusb_table[];
+
+int cxusb_ctrl_msg(struct dvb_usb_device *d,
+  u8 cmd, const u8 *wbuf, int wlen, u8 *rbuf, int rlen)
 {
struct cxusb_state *st = d->priv;
int ret;
@@ -89,7 +122,8 @@ static void cxusb_gpio_tuner(struct dvb_usb_device *d, int 
onoff)
struct cxusb_state *st = d->priv;
u8 o[2], i;
 
-   if (st->gpio_write_state[GPIO_TUNER] == onoff)
+   if (st->gpio_write_state[GPIO_TUNER] == onoff &&
+   !st->gpio_write_refresh[GPIO_TUNER])
return;
 
o[0] = GPIO_TUNER;
@@ -100,6 +134,7 @@ static void cxusb_gpio_tuner(struct dvb_usb_device *d, int 
onoff)
deb_info("gpio_write failed.\n");
 
st->gpio_write_state[GPIO_TUNER] = onoff;
+   st->gpio_write_refresh[GPIO_TUNER] = false;
 }
 
 static int cxusb_bluebird_gpio_rw(struct dvb_usb_device *d, u8 changemask,
@@ -259,7 +294,7 @@ static int cxusb_i2c_xfer(struct i2c_adapter *adap, struct 
i2c_msg msg[],
 
 static u32 cxusb_i2c_func(struct i2c_adapter *adapter)
 {
-   return I2C_FUNC_I2C;
+   return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
 }
 
 static struct i2c_algorithm cxusb_i2c_algo = {
@@ -267,15 +302,48 @@ static struct i2c_algorithm cxusb_i2c_algo = {
.functionality = cxusb_i2c_func,
 };
 
-static int cxusb_power_ctrl(struct dvb_usb_device *d, int onoff)
+static int _cxusb_power_ctrl(struct dvb_usb_device *d, int onoff)
 {
u8 b = 0;
+
+   deb_info("setting power %s\n", onoff ? "ON" : "OFF");
+
if (onoff)
return cxusb_ctrl_msg(d, CMD_POWER_ON, , 1, NULL, 0);
else

[PATCH v3 3/6] cx25840: add pin to pad mapping and output format configuration

2017-12-17 Thread Maciej S. Szmigiero
This commit adds pin to pad mapping and output format configuration support
in CX2584x-series chips to cx25840 driver.

This functionality is then used to allow disabling ivtv-specific hacks
(called a "generic mode"), so cx25840 driver can be used for other devices
not needing them without risking compatibility problems.

Signed-off-by: Maciej S. Szmigiero 
---
 drivers/media/i2c/cx25840/cx25840-core.c | 396 ++-
 drivers/media/i2c/cx25840/cx25840-core.h |  13 +
 drivers/media/i2c/cx25840/cx25840-vbi.c  |   3 +
 include/media/drv-intf/cx25840.h |  74 +-
 4 files changed, 484 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/cx25840/cx25840-core.c 
b/drivers/media/i2c/cx25840/cx25840-core.c
index 940c8b171249..7027f73201bf 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.c
+++ b/drivers/media/i2c/cx25840/cx25840-core.c
@@ -21,6 +21,9 @@
  * CX23888 DIF support for the HVR1850
  * Copyright (C) 2011 Steven Toth 
  *
+ * CX2584x pin to pad mapping and output format configuration support are
+ * Copyright (C) 2011 Maciej S. Szmigiero 
+ *
  * 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
@@ -316,6 +319,260 @@ static int cx23885_s_io_pin_config(struct v4l2_subdev 
*sd, size_t n,
return 0;
 }
 
+static u8 cx25840_function_to_pad(struct i2c_client *client, u8 function)
+{
+   switch (function) {
+   case CX25840_PAD_ACTIVE:
+   return 1;
+
+   case CX25840_PAD_VACTIVE:
+   return 2;
+
+   case CX25840_PAD_CBFLAG:
+   return 3;
+
+   case CX25840_PAD_VID_DATA_EXT0:
+   return 4;
+
+   case CX25840_PAD_VID_DATA_EXT1:
+   return 5;
+
+   case CX25840_PAD_GPO0:
+   return 6;
+
+   case CX25840_PAD_GPO1:
+   return 7;
+
+   case CX25840_PAD_GPO2:
+   return 8;
+
+   case CX25840_PAD_GPO3:
+   return 9;
+
+   case CX25840_PAD_IRQ_N:
+   return 10;
+
+   case CX25840_PAD_AC_SYNC:
+   return 11;
+
+   case CX25840_PAD_AC_SDOUT:
+   return 12;
+
+   case CX25840_PAD_PLL_CLK:
+   return 13;
+
+   case CX25840_PAD_VRESET:
+   return 14;
+
+   default:
+   if (function != CX25840_PAD_DEFAULT)
+   v4l_err(client,
+   "invalid function %u, assuming default\n",
+   (unsigned int)function);
+   return 0;
+   }
+}
+
+static void cx25840_set_invert(u8 *pinctrl3, u8 *voutctrl4, u8 function,
+  u8 pin, bool invert)
+{
+   switch (function) {
+   case CX25840_PAD_IRQ_N:
+   if (invert)
+   *pinctrl3 &= ~2;
+   else
+   *pinctrl3 |= 2;
+   break;
+
+   case CX25840_PAD_ACTIVE:
+   if (invert)
+   *voutctrl4 |= BIT(2);
+   else
+   *voutctrl4 &= ~BIT(2);
+   break;
+
+   case CX25840_PAD_VACTIVE:
+   if (invert)
+   *voutctrl4 |= BIT(5);
+   else
+   *voutctrl4 &= ~BIT(5);
+   break;
+
+   case CX25840_PAD_CBFLAG:
+   if (invert)
+   *voutctrl4 |= BIT(4);
+   else
+   *voutctrl4 &= ~BIT(4);
+   break;
+
+   case CX25840_PAD_VRESET:
+   if (invert)
+   *voutctrl4 |= BIT(0);
+   else
+   *voutctrl4 &= ~BIT(0);
+   break;
+   }
+
+   if (function != CX25840_PAD_DEFAULT)
+   return;
+
+   switch (pin) {
+   case CX25840_PIN_DVALID_PRGM0:
+   if (invert)
+   *voutctrl4 |= BIT(6);
+   else
+   *voutctrl4 &= ~BIT(6);
+   break;
+
+   case CX25840_PIN_HRESET_PRGM2:
+   if (invert)
+   *voutctrl4 |= BIT(1);
+   else
+   *voutctrl4 &= ~BIT(1);
+   break;
+   }
+}
+
+static int cx25840_s_io_pin_config(struct v4l2_subdev *sd, size_t n,
+  struct v4l2_subdev_io_pin_config *p)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(sd);
+   unsigned int i;
+   u8 pinctrl[6], pinconf[10], voutctrl4;
+
+   for (i = 0; i < 6; i++)
+   pinctrl[i] = cx25840_read(client, 0x114 + i);
+
+   for (i = 0; i < 10; i++)
+   pinconf[i] = cx25840_read(client, 0x11c + i);
+
+   voutctrl4 = cx25840_read(client, 0x407);
+
+   for (i = 0; i < n; i++) {
+  

[PATCH v3 1/6] ivtv: zero-initialize cx25840 platform data

2017-12-17 Thread Maciej S. Szmigiero
We need to zero-initialize cx25840 platform data structure to make sure
that its future members do not contain random stack garbage.

Signed-off-by: Maciej S. Szmigiero 
---
 drivers/media/pci/ivtv/ivtv-i2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/pci/ivtv/ivtv-i2c.c 
b/drivers/media/pci/ivtv/ivtv-i2c.c
index 66696e6ee587..b755337ec938 100644
--- a/drivers/media/pci/ivtv/ivtv-i2c.c
+++ b/drivers/media/pci/ivtv/ivtv-i2c.c
@@ -293,6 +293,7 @@ int ivtv_i2c_register(struct ivtv *itv, unsigned idx)
.platform_data = ,
};
 
+   memset(, 0, sizeof(pdata));
pdata.pvr150_workaround = itv->pvr150_workaround;
sd = v4l2_i2c_new_subdev_board(>v4l2_dev, adap,
_info, NULL);


[PATCH v3 2/6] cx25840: add kernel-doc description of struct cx25840_state

2017-12-17 Thread Maciej S. Szmigiero
This commit describes a device instance private data of the driver
(struct cx25840_state) in a kernel-doc style comment.

Signed-off-by: Maciej S. Szmigiero 
---
 drivers/media/i2c/cx25840/cx25840-core.h | 33 ++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/cx25840/cx25840-core.h 
b/drivers/media/i2c/cx25840/cx25840-core.h
index 55432ed42714..877b930e5b1f 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.h
+++ b/drivers/media/i2c/cx25840/cx25840-core.h
@@ -45,6 +45,35 @@ enum cx25840_media_pads {
CX25840_NUM_PADS
 };
 
+/**
+ * struct cx25840_state - a device instance private data
+ * @c: i2c_client struct representing this device
+ * @sd:our V4L2 sub-device
+ * @hdl:   our V4L2 control handler
+ * @volume:audio volume V4L2 control (non-cx2583x devices only)
+ * @mute:  audio mute V4L2 control (non-cx2583x devices only)
+ * @pvr150_workaround: whether we enable workaround for Hauppauge PVR150
+ * hardware bug (audio dropping out)
+ * @radio: set if we are currently in the radio mode, otherwise
+ * the current mode is non-radio (that is, video)
+ * @std:   currently set video standard
+ * @vid_input: currently set video input
+ * @aud_input: currently set audio input
+ * @audclk_freq:   currently set audio sample rate
+ * @audmode:   currently set audio mode (when in non-radio mode)
+ * @vbi_line_offset:   vbi line number offset
+ * @id:exact device model
+ * @rev:   raw device id read from the chip
+ * @is_initialized:whether we have already loaded firmware into the chip
+ * and initialized it
+ * @vbi_regs_offset:   offset of vbi regs
+ * @fw_wait:   wait queue to wake an initalization function up when
+ * firmware loading (on a separate workqueue) finishes
+ * @fw_work:   a work that actually loads the firmware on a separate
+ * workqueue
+ * @ir_state:  a pointer to chip IR controller private data
+ * @pads:  array of supported chip pads (currently only a stub)
+ */
 struct cx25840_state {
struct i2c_client *c;
struct v4l2_subdev sd;
@@ -66,8 +95,8 @@ struct cx25840_state {
u32 rev;
int is_initialized;
unsigned vbi_regs_offset;
-   wait_queue_head_t fw_wait;/* wake up when the fw load is finished */
-   struct work_struct fw_work;   /* work entry for fw load */
+   wait_queue_head_t fw_wait;
+   struct work_struct fw_work;
struct cx25840_ir_state *ir_state;
 #if defined(CONFIG_MEDIA_CONTROLLER)
struct media_padpads[CX25840_NUM_PADS];


[PATCH v3 0/6] [media] Add analog mode support for Medion MD95700

2017-12-17 Thread Maciej S. Szmigiero
This series adds support for analog part of Medion 95700 in the cxusb
driver.

What works:
* Video capture at various sizes with sequential fields,
* Input switching (TV Tuner, Composite, S-Video),
* TV and radio tuning,
* Video standard switching and auto detection,
* Radio mode switching (stereo / mono),
* Unplugging while capturing,
* DVB / analog coexistence,
* Raw BT.656 stream support.

What does not work yet:
* Audio,
* VBI,
* Picture controls.

This series (as a one patch) was submitted for inclusion few years ago,
then waited few months in a patch queue.
Unfortunately, by the time it was supposed to be merged there
were enough changes in media that it was no longer mergable.

I thought at that time that I will be able to rebase and retest it soon
but unfortunately up till now I was never able to find enough time to do
so.
Also, with the passing of time the implementation diverged more and
more from the current kernel code, necessitating even more reworking.

That last iteration can be found here:
https://patchwork.linuxtv.org/patch/8048/

Since that version there had been the following changes:
* Adaptation to changes in V4L2 / DVB core,

* Radio device was added, with a possibility to tune to a FM radio
station and switch between stereo and mono modes (tested by taping
audio signal directly at tuner output pin),

* DVB / analog coexistence was improved - resolved a few cases where
DVB core would switch off power or reset the tuner when the device
was still being used but in the analog mode,

* Fixed issues reported by v4l2-compliance,

* Switching to raw BT.656 mode is now done by a custom streaming
parameter set via VIDIOC_S_PARM ioctl instead of using a
V4L2_BUF_TYPE_PRIVATE buffer (which was removed from V4L2),

* General small code cleanups (like using BIT() or ARRAY_SIZE() macros
instead of open coding them, code formatting improvements, etc.).

Changes from v1:
* Only support configuration of cx25840 pins that the cxusb driver is
actually using so there is no need for an ugly CX25840_PIN() macro,

* Split cxusb changes into two patches: first one implementing
digital / analog coexistence in this driver, second one adding the
actual implementation of the analog mode,

* Fix a warning reported by kbuild test robot.

Changes from v2:
* Split out ivtv cx25840 platform data zero-initialization to a separate
commit,

* Add kernel-doc description of struct cx25840_state,

* Make sure that all variables used in CX25840_VCONFIG_OPTION() and
CX25840_VCONFIG_SET_BIT() macros are their explicit parameters,

* Split out some code from cxusb_medion_copy_field() and
cxusb_medion_v_complete_work() functions to separate ones to increase
their readability,

* Generate masks using GENMASK() and BIT() macros in cx25840.h and
cxusb.h.

Maciej S. Szmigiero (6):
  ivtv: zero-initialize cx25840 platform data
  cx25840: add kernel-doc description of struct cx25840_state
  cx25840: add pin to pad mapping and output format configuration
  tuner-simple: allow setting mono radio mode
  [media] cxusb: implement Medion MD95700 digital / analog coexistence
  [media] cxusb: add analog mode support for Medion MD95700

 drivers/media/i2c/cx25840/cx25840-core.c |  396 +-
 drivers/media/i2c/cx25840/cx25840-core.h |   46 +-
 drivers/media/i2c/cx25840/cx25840-vbi.c  |3 +
 drivers/media/pci/ivtv/ivtv-i2c.c|1 +
 drivers/media/tuners/tuner-simple.c  |5 +-
 drivers/media/usb/dvb-usb/Kconfig|8 +-
 drivers/media/usb/dvb-usb/Makefile   |2 +-
 drivers/media/usb/dvb-usb/cxusb-analog.c | 1927 ++
 drivers/media/usb/dvb-usb/cxusb.c|  455 ++-
 drivers/media/usb/dvb-usb/cxusb.h|  136 +++
 drivers/media/usb/dvb-usb/dvb-usb-dvb.c  |   20 +-
 drivers/media/usb/dvb-usb/dvb-usb-init.c |   13 +
 drivers/media/usb/dvb-usb/dvb-usb.h  |8 +
 include/media/drv-intf/cx25840.h |   74 +-
 14 files changed, 3028 insertions(+), 66 deletions(-)
 create mode 100644 drivers/media/usb/dvb-usb/cxusb-analog.c


Re: [PATCH v2 1/6] cx25840: add pin to pad mapping and output format configuration

2017-12-17 Thread Maciej S. Szmigiero
On 11.12.2017 16:27, Mauro Carvalho Chehab wrote:
> Em Tue, 10 Oct 2017 23:34:45 +0200
> "Maciej S. Szmigiero"  escreveu:
> 
>> This commit adds pin to pad mapping and output format configuration support
>> in CX2584x-series chips to cx25840 driver.
>>
>> This functionality is then used to allow disabling ivtv-specific hacks
>> (called a "generic mode"), so cx25840 driver can be used for other devices
>> not needing them without risking compatibility problems.
>>
>> Signed-off-by: Maciej S. Szmigiero 
>> ---
>>  drivers/media/i2c/cx25840/cx25840-core.c | 394 
>> ++-
>>  drivers/media/i2c/cx25840/cx25840-core.h |  11 +
>>  drivers/media/i2c/cx25840/cx25840-vbi.c  |   3 +
>>  drivers/media/pci/ivtv/ivtv-i2c.c|   1 +
>>  include/media/drv-intf/cx25840.h |  74 +-
>>  5 files changed, 481 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/i2c/cx25840/cx25840-core.c 
>> b/drivers/media/i2c/cx25840/cx25840-core.c
>> index f38bf819d805..a1efc975852c 100644
>> --- a/drivers/media/i2c/cx25840/cx25840-core.c
>> +++ b/drivers/media/i2c/cx25840/cx25840-core.c
(..)
>> @@ -1630,6 +1907,117 @@ static void log_audio_status(struct i2c_client 
>> *client)
>>  }
>>  }
>>  
>> +#define CX25840_VCONFIG_OPTION(option_mask) \
>> +do {\
>> +if (config_in & (option_mask)) {\
>> +state->vid_config &= ~(option_mask);\
>> +state->vid_config |= config_in & (option_mask); \
> 
> Don't do that: the "config_in" var is not at the macro's parameter.
> It only works if this macro is called at cx25840_vconfig() function.
> The same applies to state. That makes harder for anyone reviewing the
> code to understand it. Also, makes harder to use it on any other place.
> 
> If you want to use a macro, please add all required vars to the macro
> parameters.
> 
>> +}   \
>> +} while (0)
>> +
>> +#define CX25840_VCONFIG_SET_BIT(optionmask, reg, bit, oneval)   
>> \
>> +do {\
>> +if (state->vid_config & (optionmask)) { \
>> +if ((state->vid_config & (optionmask)) ==   \
>> +(oneval))   \
>> +voutctrl[reg] |= BIT(bit);  \
>> +else\
>> +voutctrl[reg] &= ~BIT(bit); \
>> +}   \
>> +} while (0)
> 
> Same applies here: state and voutctrl aren't at macro's parameters.
> 
>> +
>> +int cx25840_vconfig(struct i2c_client *client, u32 config_in)
>> +{
>> +struct cx25840_state *state = to_state(i2c_get_clientdata(client));
>> +u8 voutctrl[3];
>> +unsigned int i;
>> +
>> +/* apply incoming options to the current state */
>> +CX25840_VCONFIG_OPTION(CX25840_VCONFIG_FMT_MASK);
>> +CX25840_VCONFIG_OPTION(CX25840_VCONFIG_RES_MASK);
>> +CX25840_VCONFIG_OPTION(CX25840_VCONFIG_VBIRAW_MASK);
>> +CX25840_VCONFIG_OPTION(CX25840_VCONFIG_ANCDATA_MASK);
>> +CX25840_VCONFIG_OPTION(CX25840_VCONFIG_TASKBIT_MASK);
>> +CX25840_VCONFIG_OPTION(CX25840_VCONFIG_ACTIVE_MASK);
>> +CX25840_VCONFIG_OPTION(CX25840_VCONFIG_VALID_MASK);
>> +CX25840_VCONFIG_OPTION(CX25840_VCONFIG_HRESETW_MASK);
>> +CX25840_VCONFIG_OPTION(CX25840_VCONFIG_CLKGATE_MASK);
>> +CX25840_VCONFIG_OPTION(CX25840_VCONFIG_DCMODE_MASK);
>> +CX25840_VCONFIG_OPTION(CX25840_VCONFIG_IDID0S_MASK);
>> +CX25840_VCONFIG_OPTION(CX25840_VCONFIG_VIPCLAMP_MASK);
> 
> The entire logic here sounds complex, without need. Wouldn't be
> better/clearer if you rewrite it as:
> 
>   u32 option_mask = CX25840_VCONFIG_FMT_MASK
>  | CX25840_VCONFIG_RES_MASK
> ...
>  | CX25840_VCONFIG_VIPCLAMP_MASK;
> 
>   state->vid_config &= ~option_mask;
>   state->vid_config |= config_in & option_mask;
> 
> 

Unfortunately, this would zero the current configuration in
state->vid_config for every possible parameter, whereas the macros above
only touch these parameters that are provided to a cx25840_vconfig()
invocation, leaving the rest as-is.

(All other your comments were implemented in a respin).

> 
> Thanks,
> Mauro
> 

Thanks,
Maciej


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

2017-12-17 Thread Laurent Pinchart
Hello,

On Friday, 15 December 2017 18:17:04 EET Sakari Ailus wrote:
> On Wed, Dec 13, 2017 at 07:26:20PM +0100, Jacopo Mondi wrote:
> > 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.
> 
> ACPI doesn't have such at the moment. I think printing the full path would
> still be better. There isn't that much more to print after all.
> 
> > ---
> > 
> >  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
> 
> I'm not sure I'd add a Kconfig option. This is adding a fairly simple
> function only to the kernel.
> 
> > +   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
> 
> Do you need this?

No this isn't needed. Debugging can be enabled through dynamic debug without 
requiring the Kconfig option. A Kconfig option might be useful to avoid 
compiling the debug code in kernels that have dynamic debug enabled, but those 
are large already and the amount of debug code here is limited, so I don't 
think it's worth it.

> > +#endif
> > +
> > 
> >  #include 
> >  #include 
> >  #include 

[snip]

-- 
Regards,

Laurent Pinchart



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

2017-12-17 Thread Laurent Pinchart
Hi Jacopo,

Thank you for the patch.

On Wednesday, 13 December 2017 20:26:19 EET Jacopo Mondi wrote:
> 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.

Aren't you piling yet another hack on top of an already dirty framework ? How 
about fixing the root cause of the issue and ensuring that subdevs are 
properly initialized when the notifier is registered ?

> 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

[snip]

> @@ -548,6 +551,20 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
>   sd->fwnode = dev_fwnode(sd->dev);
>   }
> 
> + /*
> +  * If the subdevice has an unregisterd notifier, it's now time
> +  * to register it.
> +  */
> + subdev_notifier = sd->subdev_notifier;
> + if (subdev_notifier && !subdev_notifier->registered) {
> + ret = __v4l2_async_notifier_register(subdev_notifier);
> + if (ret) {
> + sd->fwnode = NULL;
> + subdev_notifier->owner = NULL;
> + return ret;
> + }
> + }

This is the part I like the least in this patch set. The 
v4l2_subdev::subdev_notifier field should really disappear, there's no reason 
to limit subdevs to a single notifier. Implicit registration of notifiers is a 
dirty hack in my opinion.

>   mutex_lock(_lock);
> 
>   INIT_LIST_HEAD(>async_list);

[snip]

-- 
Regards,

Laurent Pinchart



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

2017-12-17 Thread Laurent Pinchart
Hello,

On Friday, 15 December 2017 16:38:16 EET Sakari Ailus wrote:
> Hi Jacopo,
> 
> On Wed, Dec 13, 2017 at 07:26:18PM +0100, Jacopo Mondi wrote:
> > 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 
> 
> You'll have struct device either through the v4l2_device or v4l2_subdev. Do
> you need an additional field for this?

I agree with this comment. If there's a reason to add a new field, its life 
time constraints should be documented. The fwnodes are refcounted and you're 
not increasing the refcount here, you should explain why you don't need to.

-- 
Regards,

Laurent Pinchart



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

2017-12-17 Thread Laurent Pinchart
Hi Jacopo,

Thank you for the patch.

On Wednesday, 13 December 2017 20:26:17 EET Jacopo Mondi wrote:
> 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));

You're returning a name here, but it's not the ACPI fwnode name, it's the name 
of the corresponding device. This isn't consistent with the OF implementation. 
As Sakari pointed out, it also won't work for non-device ACPI nodes.

> +}
> +
>  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

Could you please document the life time constraints of the return pointer ?

> + */
> +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));

If you're returning the full name shouldn't the operation be called 
*get_full_name() ?

> +}
> +
>  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,
> 

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

2017-12-17 Thread Laurent Pinchart
Hi Sakari,

Thank you for the patch.

On Wednesday, 13 December 2017 20:26:16 EET Jacopo Mondi wrote:
> 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(-)

[snip]

> 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);

The node's full name is used as the subdev name, have you verified that this 
change won't break the driver ?

> 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++)
> - if (fmd->sensor[i].asd.match.fwnode.fwnode ==
> - of_fwnode_handle(subdev->dev->of_node))
> + for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++) {
> + struct fwnode_handle *fwnode =
> + fwnode_graph_get_port_parent(
> + of_fwnode_handle(subdev->dev->of_node));

Isn't fwnode_graph_get_port_parent() supposed to be called on an endpoint node 
? subdev->dev->of_node is the device's node.

> + if (fmd->sensor[i].asd.match.fwnode.fwnode == fwnode)
>   si = >sensor[i];
> 
> + fwnode_handle_put(fwnode);
> + }
> +
>   if (si == NULL)
>   return -EINVAL;
> 

[snip]

> diff --git a/drivers/media/platform/ti-vpe/cal.c
> b/drivers/media/platform/ti-vpe/cal.c index 8b586c8..9b29706 100644
> --- a/drivers/media/platform/ti-vpe/cal.c
> +++ b/drivers/media/platform/ti-vpe/cal.c
> @@ -1699,7 +1699,7 @@ static int of_cal_create_instance(struct cal_ctx *ctx,
> int inst) goto cleanup_exit;
>   }
> 
> - sensor_node = of_graph_get_remote_port_parent(ep_node);
> + sensor_node = of_graph_get_remote_endpoint(ep_node);
>   if (!sensor_node) {
>   ctx_dbg(3, ctx, "can't get remote parent\n");
>   goto cleanup_exit;

sensor_node->name is used in a debug message that will become a bit less 
useful as a result.

> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c
> b/drivers/media/platform/xilinx/xilinx-vipp.c index d881cf0..17d4ac0 100644
> --- a/drivers/media/platform/xilinx/xilinx-vipp.c
> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c
> @@ -82,6 

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

2017-12-17 Thread jacopo mondi
Hi Sakari,

On Fri, Dec 15, 2017 at 06:17:04PM +0200, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Wed, Dec 13, 2017 at 07:26:20PM +0100, Jacopo Mondi wrote:
> > 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.
>
> ACPI doesn't have such at the moment. I think printing the full path would
> still be better. There isn't that much more to print after all.

So you suggest to just use the full node name for OF. What about ACPI?

>From your other reply I got that I can print the single node name for
"device ACPI nodes" but not for "non-device ACPI nodes". Should I build
the full device name in drivers/acpi/properties.c for ACPI devices
like I'm doing here for fwnodes?

>
> > ---
> >
> >  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
>
> I'm not sure I'd add a Kconfig option. This is adding a fairly simple
> function only to the kernel.

So I will use a symbol defined in the module to enable/disable debug
(maybe the "DEBUG" symbol itself?)

>
> > +   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
>
> Do you need this?

No dev_dbg() otherwise, isn't it?

>
> > +#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];
>
> That's a bit too much to allocate from the stack I think.

For an full name do you think 128 is enough? 256 maybe?

Thanks
  j


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

2017-12-17 Thread jacopo mondi
Hi Sakari,

On Fri, Dec 15, 2017 at 05:20:40PM +0200, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Wed, Dec 13, 2017 at 07:26:19PM +0100, Jacopo Mondi wrote:
> > 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.
>
> Let me rephrase to make sure I understand the problem correctly ---
>
> A sub-device notifier is registered but the notifier's sub-device is not
> registered yet, and finding a match for this notifier leads, to, well
> problems. Is that the reason for this patch?

Almost :)
I never encountered problems registering the sub-notifier, but instead
identifying it when trying to debug what's happening in v4l2-async.

I had a lot of "(null)" notifiers, and that makes debug particularly
painful, as in example I had unexpected matches between a subdev and a
"(null)" notifier and it's pretty hard find out what's going wrong.

So I post-poned registratration (and consequentially matching with the
available subdevices) to a time where I know it can be identified.

>
> I think there could be simpler solutions to address this.
>
> I wonder if we could simply check for sub-device notifier's v4l2_dev field,
> and fail in matching if it's not set. v4l2_device_register_subdev() would
> still need to proceed with calling v4l2_async_notifier_try_all_subdevs()
> and v4l2_async_notifier_try_complete() if that was the case.
>
> What do you think?

v4l2_dev is only set in root notifiers, we maybe can check for a valid
"struct device *" and refuse notifiers without an initialized one in
"__v4l2_async_notifier_register()".

And you're actually right here, when later the subdevice owning the just
refused sub-notifier gets registered (and eventually matched) its
sub-notifier will be processed anyhow, and this makes hunk  "@@ -548,6
+551,20 @@" of my patch unnecessary.

I will test and simplify it. Thanks

>
> >
> > 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);
> > +   

Re: [PATCH 0/8] ddbridge improvements and cleanups

2017-12-17 Thread Daniel Scheller
On Sun, 17 Dec 2017 16:40:41 +0100
Daniel Scheller  wrote:

> I verified this by simply removing tda18212.ko with this DD setup:

Sorry, I forgot to outline this: I also tested by removing stv0367.ko
and cxd2841er.ko of course, which resulted in partially working
hardware, either the cxd-based hardware or the stv-based one didn't
work, while everything else did. Unload worked cleanly, without leaving
any side effects.

All this worked without these patches before of course, but as they
touch the attach logic and make failure cleanup resources instantly
now, this required retesting.

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


[PATCH 6/8] [media] ddbridge: fix deinit order in case of failure in ddb_init()

2017-12-17 Thread Daniel Scheller
From: Daniel Scheller 

In ddb_init(), the deinitialization sequence isn't correct when handling
errors, and could even lead to a memleak depending on where things failed.
Fix the deinit order.

Signed-off-by: Daniel Scheller 
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c 
b/drivers/media/pci/ddbridge/ddbridge-core.c
index 07f3e37a0fca..548b7047ca09 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -3273,7 +3273,7 @@ int ddb_init(struct ddb *dev)
ddb_init_boards(dev);
 
if (ddb_i2c_init(dev) < 0)
-   goto fail;
+   goto fail1;
ddb_ports_init(dev);
if (ddb_buffers_alloc(dev) < 0) {
dev_info(dev->dev, "Could not allocate buffer memory\n");
@@ -3291,14 +3291,14 @@ int ddb_init(struct ddb *dev)
return 0;
 
 fail3:
-   ddb_ports_detach(dev);
dev_err(dev->dev, "fail3\n");
-   ddb_ports_release(dev);
+   ddb_ports_detach(dev);
+   ddb_buffers_free(dev);
 fail2:
dev_err(dev->dev, "fail2\n");
-   ddb_buffers_free(dev);
+   ddb_ports_release(dev);
ddb_i2c_release(dev);
-fail:
+fail1:
dev_err(dev->dev, "fail1\n");
return -1;
 }
-- 
2.13.6



[PATCH 5/8] [media] ddbridge: completely tear down input resources on failure

2017-12-17 Thread Daniel Scheller
From: Daniel Scheller 

In dvb_input_attach(), whenever a demod driver fails to initialise, or if
frontend registration fails, perform a full input/frontend teardown using
dvb_input_detach() (which can safely be done since the current init state
is tracked in the 'attached' struct member). Claimed resources thus are
freed which aren't needed when an input or a port is not functional.

While at it, in ddb_ports_detach(), detach the secondary input first. Also
increase the kernlog severity of TDA18212 errors and tuner failures in
general.

Signed-off-by: Daniel Scheller 
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 46 ++
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c 
b/drivers/media/pci/ddbridge/ddbridge-core.c
index c2f028152388..07f3e37a0fca 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -1032,7 +1032,7 @@ static int tuner_attach_tda18212(struct ddb_input *input, 
u32 porttype)
 
return 0;
 err:
-   dev_notice(dev, "TDA18212 tuner not found. Device is not fully 
operational.\n");
+   dev_err(dev, "TDA18212 tuner not found. Device is not fully 
operational.\n");
return -ENODEV;
 }
 
@@ -1425,7 +1425,7 @@ static int dvb_input_attach(struct ddb_input *input)
dvb->dmxdev.demux = >dmx;
ret = dvb_dmxdev_init(>dmxdev, adap);
if (ret < 0)
-   return ret;
+   goto err_detach;
dvb->attached = 0x11;
 
dvb->mem_frontend.source = DMX_MEMORY_FE;
@@ -1434,12 +1434,12 @@ static int dvb_input_attach(struct ddb_input *input)
dvb->demux.dmx.add_frontend(>demux.dmx, >hw_frontend);
ret = dvbdemux->dmx.connect_frontend(>dmx, >hw_frontend);
if (ret < 0)
-   return ret;
+   goto err_detach;
dvb->attached = 0x12;
 
ret = dvb_net_init(adap, >dvbnet, dvb->dmxdev.demux);
if (ret < 0)
-   return ret;
+   goto err_detach;
dvb->attached = 0x20;
 
dvb->fe = NULL;
@@ -1447,47 +1447,47 @@ static int dvb_input_attach(struct ddb_input *input)
switch (port->type) {
case DDB_TUNER_MXL5XX:
if (ddb_fe_attach_mxl5xx(input) < 0)
-   return -ENODEV;
+   goto err_detach;
break;
case DDB_TUNER_DVBS_ST:
if (demod_attach_stv0900(input, 0) < 0)
-   return -ENODEV;
+   goto err_detach;
if (tuner_attach_stv6110(input, 0) < 0)
goto err_tuner;
break;
case DDB_TUNER_DVBS_ST_AA:
if (demod_attach_stv0900(input, 1) < 0)
-   return -ENODEV;
+   goto err_detach;
if (tuner_attach_stv6110(input, 1) < 0)
goto err_tuner;
break;
case DDB_TUNER_DVBS_STV0910:
if (demod_attach_stv0910(input, 0) < 0)
-   return -ENODEV;
+   goto err_detach;
if (tuner_attach_stv6111(input, 0) < 0)
goto err_tuner;
break;
case DDB_TUNER_DVBS_STV0910_PR:
if (demod_attach_stv0910(input, 1) < 0)
-   return -ENODEV;
+   goto err_detach;
if (tuner_attach_stv6111(input, 1) < 0)
goto err_tuner;
break;
case DDB_TUNER_DVBS_STV0910_P:
if (demod_attach_stv0910(input, 0) < 0)
-   return -ENODEV;
+   goto err_detach;
if (tuner_attach_stv6111(input, 1) < 0)
goto err_tuner;
break;
case DDB_TUNER_DVBCT_TR:
if (demod_attach_drxk(input) < 0)
-   return -ENODEV;
+   goto err_detach;
if (tuner_attach_tda18271(input) < 0)
goto err_tuner;
break;
case DDB_TUNER_DVBCT_ST:
if (demod_attach_stv0367(input) < 0)
-   return -ENODEV;
+   goto err_detach;
if (tuner_attach_tda18212(input, port->type) < 0)
goto err_tuner;
break;
@@ -1507,7 +1507,7 @@ static int dvb_input_attach(struct ddb_input *input)
else
par = 1;
if (demod_attach_cxd28xx(input, par, osc24) < 0)
-   return -ENODEV;
+   goto err_detach;
if (tuner_attach_tda18212(input, port->type) < 0)
goto err_tuner;
break;
@@ -1518,7 +1518,7 @@ static int dvb_input_attach(struct ddb_input *input)
case 

[PATCH 3/8] [media] ddbridge: deduplicate calls to dvb_ca_en50221_init()

2017-12-17 Thread Daniel Scheller
From: Daniel Scheller 

All CI types do dvb_ca_en50221_init() with the same arguments. Move this
call after the switch-case to remove the repetition in every case block.

Cc: Ralph Metzler 
Signed-off-by: Daniel Scheller 
---
 drivers/media/pci/ddbridge/ddbridge-ci.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-ci.c 
b/drivers/media/pci/ddbridge/ddbridge-ci.c
index a4fd747763a0..8dfbc3bbd86d 100644
--- a/drivers/media/pci/ddbridge/ddbridge-ci.c
+++ b/drivers/media/pci/ddbridge/ddbridge-ci.c
@@ -327,8 +327,6 @@ int ddb_ci_attach(struct ddb_port *port, u32 bitrate)
port->en = cxd2099_attach(_cfg, port, >i2c->adap);
if (!port->en)
return -ENODEV;
-   dvb_ca_en50221_init(port->dvb[0].adap,
-   port->en, 0, 1);
break;
 
case DDB_CI_EXTERNAL_XO2:
@@ -336,15 +334,15 @@ int ddb_ci_attach(struct ddb_port *port, u32 bitrate)
ci_xo2_attach(port);
if (!port->en)
return -ENODEV;
-   dvb_ca_en50221_init(port->dvb[0].adap, port->en, 0, 1);
break;
 
case DDB_CI_INTERNAL:
ci_attach(port);
if (!port->en)
return -ENODEV;
-   dvb_ca_en50221_init(port->dvb[0].adap, port->en, 0, 1);
break;
}
+
+   dvb_ca_en50221_init(port->dvb[0].adap, port->en, 0, 1);
return 0;
 }
-- 
2.13.6



[PATCH 7/8] [media] ddbridge: detach first input if the second one failed to init

2017-12-17 Thread Daniel Scheller
From: Daniel Scheller 

In ddb_ports_attach(), if the second input of a dual tuner failed to
initialise, the first one can be detached (and resources be freed) as
this will be counted as the whole port having failed to initialise,
thus the first one won't be used anyway.

Signed-off-by: Daniel Scheller 
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c 
b/drivers/media/pci/ddbridge/ddbridge-core.c
index 548b7047ca09..940371067346 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -1935,8 +1935,10 @@ static int ddb_port_attach(struct ddb_port *port)
if (ret < 0)
break;
ret = dvb_input_attach(port->input[1]);
-   if (ret < 0)
+   if (ret < 0) {
+   dvb_input_detach(port->input[0]);
break;
+   }
port->input[0]->redi = port->input[0];
port->input[1]->redi = port->input[1];
break;
-- 
2.13.6



[PATCH 4/8] [media] ddbridge: move CI detach code to ddbridge-ci.c

2017-12-17 Thread Daniel Scheller
From: Daniel Scheller 

Move the CI teardown code to ddbridge-ci.c where everything else related
to CI hardware lives.

Cc: Ralph Metzler 
Signed-off-by: Daniel Scheller 
---
 drivers/media/pci/ddbridge/ddbridge-ci.c   | 11 +++
 drivers/media/pci/ddbridge/ddbridge-ci.h   |  1 +
 drivers/media/pci/ddbridge/ddbridge-core.c |  8 +---
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-ci.c 
b/drivers/media/pci/ddbridge/ddbridge-ci.c
index 8dfbc3bbd86d..5828111487b0 100644
--- a/drivers/media/pci/ddbridge/ddbridge-ci.c
+++ b/drivers/media/pci/ddbridge/ddbridge-ci.c
@@ -346,3 +346,14 @@ int ddb_ci_attach(struct ddb_port *port, u32 bitrate)
dvb_ca_en50221_init(port->dvb[0].adap, port->en, 0, 1);
return 0;
 }
+
+void ddb_ci_detach(struct ddb_port *port)
+{
+   if (port->dvb[0].dev)
+   dvb_unregister_device(port->dvb[0].dev);
+   if (port->en) {
+   dvb_ca_en50221_release(port->en);
+   kfree(port->en->data);
+   port->en = NULL;
+   }
+}
diff --git a/drivers/media/pci/ddbridge/ddbridge-ci.h 
b/drivers/media/pci/ddbridge/ddbridge-ci.h
index 3a5d7ffab7b7..35a39182dd83 100644
--- a/drivers/media/pci/ddbridge/ddbridge-ci.h
+++ b/drivers/media/pci/ddbridge/ddbridge-ci.h
@@ -26,5 +26,6 @@
 
/**/
 
 int ddb_ci_attach(struct ddb_port *port, u32 bitrate);
+void ddb_ci_detach(struct ddb_port *port);
 
 #endif /* __DDBRIDGE_CI_H__ */
diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c 
b/drivers/media/pci/ddbridge/ddbridge-core.c
index a81125d492ff..c2f028152388 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -1986,13 +1986,7 @@ void ddb_ports_detach(struct ddb *dev)
break;
case DDB_PORT_CI:
case DDB_PORT_LOOP:
-   if (port->dvb[0].dev)
-   dvb_unregister_device(port->dvb[0].dev);
-   if (port->en) {
-   dvb_ca_en50221_release(port->en);
-   kfree(port->en->data);
-   port->en = NULL;
-   }
+   ddb_ci_detach(port);
break;
}
}
-- 
2.13.6



[PATCH 2/8] [media] ddbridge: fix resources cleanup for CI hardware

2017-12-17 Thread Daniel Scheller
From: Daniel Scheller 

Do kfree() on port->en->data instead of port->en. port->en only holds a
ptr to a struct dvb_ca_en50221, which is a member either of a memalloc'ed
struct ddb_ci (DuoFlex CI, Octopus CI Duo) or a struct cxd (CXD2099AR
based Single Flex, allocated by the cxd2099 driver). port->en.data
though holds the ptr to the allocated memory, which must rather be
kfree()'d. Change this accordingly.

Cc: Ralph Metzler 
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 eda004398316..a81125d492ff 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -1990,7 +1990,7 @@ void ddb_ports_detach(struct ddb *dev)
dvb_unregister_device(port->dvb[0].dev);
if (port->en) {
dvb_ca_en50221_release(port->en);
-   kfree(port->en);
+   kfree(port->en->data);
port->en = NULL;
}
break;
-- 
2.13.6



[PATCH 1/8] [media] ddbridge: unregister I2C tuner client before detaching fe's

2017-12-17 Thread Daniel Scheller
From: Daniel Scheller 

Currently, rmmod ddbridge on a KASAN enabled kernel yields this report
for hardware that utilises the tda18212 tuner driver:

  [   50.355229] 
==
  [   50.355271] BUG: KASAN: use-after-free in tda18212_remove+0x5c/0xb0 
[tda18212]
  [   50.355290] Write of size 288 at addr 8800c235cf18 by task rmmod/285

  [   50.355316] CPU: 1 PID: 285 Comm: rmmod Not tainted 
4.15.0-rc1-13744-g352a86ad536f #11
  [   50.355318] Hardware name: Gigabyte Technology Co., Ltd. P35-DS3/P35-DS3, 
BIOS F3 06/11/2007
  [   50.355319] Call Trace:
  [   50.355326]  dump_stack+0x46/0x61
  [   50.355332]  print_address_description+0x79/0x270
  [   50.355336]  ? tda18212_remove+0x5c/0xb0 [tda18212]
  [   50.355339]  kasan_report+0x229/0x340
  [   50.355342]  memset+0x1f/0x40
  [   50.355345]  tda18212_remove+0x5c/0xb0 [tda18212]
  [   50.355350]  i2c_device_remove+0x97/0xe0
  [   50.355355]  device_release_driver_internal+0x267/0x510
  [   50.355358]  bus_remove_device+0x296/0x470
  [   50.355360]  device_del+0x35c/0x890
  [   50.355363]  ? __device_links_no_driver+0x1c0/0x1c0
  [   50.355367]  ? cxd2841er_get_algo+0x10/0x10 [cxd2841er]
  [   50.355371]  ? cxd2841er_get_algo+0x10/0x10 [cxd2841er]
  [   50.355374]  ? __module_text_address+0xe/0x140
  [   50.355377]  device_unregister+0x9/0x20
  [   50.355382]  dvb_input_detach.isra.24+0x286/0x480 [ddbridge]
  [   50.355388]  ddb_ports_detach+0x15f/0x4f0 [ddbridge]
  [   50.355393]  ddb_remove+0x3c/0xb0 [ddbridge]
  [   50.355397]  pci_device_remove+0x93/0x1d0
  [   50.355400]  device_release_driver_internal+0x267/0x510
  [   50.355403]  driver_detach+0xb9/0x1b0
  [   50.355406]  bus_remove_driver+0xd0/0x1f0
  [   50.355410]  pci_unregister_driver+0x25/0x210
  [   50.355415]  module_exit_ddbridge+0xc/0x45 [ddbridge]
  [   50.355418]  SyS_delete_module+0x314/0x440
  [   50.355420]  ? free_module+0x5b0/0x5b0
  [   50.355423]  ? exit_to_usermode_loop+0xa9/0xc0
  [   50.355425]  ? free_module+0x5b0/0x5b0
  [   50.355428]  do_syscall_64+0x179/0x4c0
  [   50.355432]  ? do_page_fault+0x1b/0x60
  [   50.355435]  entry_SYSCALL64_slow_path+0x25/0x25
  [   50.355438] RIP: 0033:0x7fe65d08ade7
  [   50.355439] RSP: 002b:7fff5a6a09a8 EFLAGS: 0202 ORIG_RAX: 
00b0
  [   50.355443] RAX: ffda RBX:  RCX: 
7fe65d08ade7
  [   50.355445] RDX: 000a RSI: 0800 RDI: 
00f4e268
  [   50.355447] RBP: 00f4e200 R08:  R09: 
1999
  [   50.355449] R10: 0891 R11: 0202 R12: 
7fff5a6a14ef
  [   50.355451] R13:  R14: 00f4e200 R15: 
00f4d010

  [   50.355462] Allocated by task 164:
  [   50.355477]  cxd2841er_attach+0xc3/0x7f0 [cxd2841er]
  [   50.355482]  demod_attach_cxd28xx+0x14c/0x3f0 [ddbridge]
  [   50.355486]  dvb_input_attach+0x671/0x1e20 [ddbridge]
  [   50.355490]  ddb_ports_attach+0x3d7/0xbf0 [ddbridge]
  [   50.355495]  ddb_init+0x4b3/0xa30 [ddbridge]
  [   50.355499]  ddb_probe+0xa51/0xfe0 [ddbridge]
  [   50.355501]  pci_device_probe+0x279/0x480
  [   50.355504]  driver_probe_device+0x46f/0x7a0
  [   50.355506]  __driver_attach+0x133/0x170
  [   50.355509]  bus_for_each_dev+0x10a/0x190
  [   50.355511]  bus_add_driver+0x2a3/0x5a0
  [   50.355513]  driver_register+0x182/0x3a0
  [   50.355516]  arc4_set_key+0x8f/0x2a0 [arc4]
  [   50.355518]  do_one_initcall+0x77/0x1d0
  [   50.355521]  do_init_module+0x1c2/0x548
  [   50.355523]  load_module+0x5e61/0x8df0
  [   50.355525]  SyS_finit_module+0x142/0x150
  [   50.355527]  do_syscall_64+0x179/0x4c0
  [   50.355529]  return_from_SYSCALL_64+0x0/0x65

  [   50.355539] Freed by task 285:
  [   50.31]  kfree+0x6c/0xa0
  [   50.38]  __dvb_frontend_free+0x81/0xb0 [dvb_core]
  [   50.355562]  dvb_input_detach.isra.24+0x17c/0x480 [ddbridge]
  [   50.355566]  ddb_ports_detach+0x15f/0x4f0 [ddbridge]
  [   50.355570]  ddb_remove+0x3c/0xb0 [ddbridge]
  [   50.355573]  pci_device_remove+0x93/0x1d0
  [   50.355576]  device_release_driver_internal+0x267/0x510
  [   50.355578]  driver_detach+0xb9/0x1b0
  [   50.355580]  bus_remove_driver+0xd0/0x1f0
  [   50.355583]  pci_unregister_driver+0x25/0x210
  [   50.355587]  module_exit_ddbridge+0xc/0x45 [ddbridge]
  [   50.355590]  SyS_delete_module+0x314/0x440
  [   50.355592]  do_syscall_64+0x179/0x4c0
  [   50.355594]  return_from_SYSCALL_64+0x0/0x65

  [   50.355604] The buggy address belongs to the object at 8800c235cd80
  which belongs to the cache kmalloc-2048 of size 2048
  [   50.355630] The buggy address is located 408 bytes inside of
  2048-byte region [8800c235cd80, 8800c235d580)
  [   50.355652] The buggy address belongs to the page:
  [   50.355666] page:ea0002a7bc20 count:1 mapcount:0 
mapping:8800c235c500 index:0x0 compound_mapcount: 0
  [   50.355688] flags: 

[PATCH 8/8] [media] ddbridge: improve ddb_ports_attach() failure handling

2017-12-17 Thread Daniel Scheller
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 don't 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.

If all ports failed to initialise where connected hardware was detected on
at first, return -ENODEV though to cause this PCI device to fail and free
all allocated resources. In any case, leave a kernel log warning (or
error, even) if things went wrong.

Signed-off-by: Daniel Scheller 
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 26 +++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c 
b/drivers/media/pci/ddbridge/ddbridge-core.c
index 940371067346..c7d923e0e21a 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -1964,7 +1964,7 @@ static int ddb_port_attach(struct ddb_port *port)
 
 int ddb_ports_attach(struct ddb *dev)
 {
-   int i, ret = 0;
+   int i, numports, err_ports = 0, ret = 0;
struct ddb_port *port;
 
if (dev->port_num) {
@@ -1974,11 +1974,31 @@ int ddb_ports_attach(struct ddb *dev)
return ret;
}
}
+
+   numports = dev->port_num;
+
for (i = 0; i < dev->port_num; i++) {
port = >port[i];
-   ret = ddb_port_attach(port);
+   if (port->class != DDB_PORT_NONE) {
+   ret = ddb_port_attach(port);
+   if (ret)
+   err_ports++;
+   } else {
+   numports--;
+   }
}
-   return ret;
+
+   if (err_ports) {
+   if (err_ports == numports) {
+   dev_err(dev->dev, "All connected ports failed to 
initialise!\n");
+   return -ENODEV;
+   }
+
+   dev_warn(dev->dev, "%d of %d connected ports failed to 
initialise!\n",
+err_ports, numports);
+   }
+
+   return 0;
 }
 
 void ddb_ports_detach(struct ddb *dev)
-- 
2.13.6



[PATCH 0/8] ddbridge improvements and cleanups

2017-12-17 Thread Daniel Scheller
From: Daniel Scheller 

This series improves on a few things in ddbridge:

* Fix up a KASAN report which pops up with all TDA18212-equipped hardware
  by changing the order of all frontend driver teardown. This was
  originally thought to be a problem of the tda18212 driver, see
  https://patchwork.linuxtv.org/patch/45992/ (though other drivers with
  that problem may remain).
* Fix up CI resources cleanup handling, and further cosmetics and code
  move, all CI related
* Frontend cleanup improvements when handling errors (ie. when on one
  port the device initialisation fails). Whenever a tuner module fails
  now, everything should be cleaned up properly (and early) now, while
  all other (working) tuners are being usable. Proper errors are printed
  to the kernel log about this.

Mauro, I'm pretty sure you like this overall approach better, compared
to https://patchwork.linuxtv.org/patch/45810/ :-) In fact, I picked up
your idea of counting ports and act accordingly. Partial hardware setup
now starts up all working parts properly, while releasing resources
early when the nonworking parts are handled. If no ports could be started
at all, the driver instance will fail gracefully and report this to
upper layers.

I verified this by simply removing tda18212.ko with this DD setup:

* CineCTv6 bridge card (stv0367+tda18212 soldered on it, handled as
  port 0), one DuoFlex C2T2 connected to port 1 (cxd2841er+tda18212)
* Octopus CI Duo, one DuoFlex C2T2I (cxd2841er+tda18212) connected to
  port 1, one SingleCI module (cxd2099) connected to port 2

Upon modprobe ddbridge, the CTv6 will completely fail due to the tuner
driver not initialising (it's not there, actually). The OctoCIDUO
will fail on the C2T2I Flex, but starts up the CI hardware, registers
it's en50221 device nodes and things work fine with it. Unload cleans
up everything, no leaked usecounts, no KASAN complaints. Putting back
tda18212.ko, modprobe ddbridge - registers everything. Unload cleans
up everything properly aswell.

Not entirely sure, but patch 1 might be something for stable (ie. 4.14).

Daniel Scheller (8):
  [media] ddbridge: unregister I2C tuner client before detaching fe's
  [media] ddbridge: fix resources cleanup for CI hardware
  [media] ddbridge: deduplicate calls to dvb_ca_en50221_init()
  [media] ddbridge: move CI detach code to ddbridge-ci.c
  [media] ddbridge: completely tear down input resources on failure
  [media] ddbridge: fix deinit order in case of failure in ddb_init()
  [media] ddbridge: detach first input if the second one failed to init
  [media] ddbridge: improve ddb_ports_attach() failure handling

 drivers/media/pci/ddbridge/ddbridge-ci.c   |  17 +++--
 drivers/media/pci/ddbridge/ddbridge-ci.h   |   1 +
 drivers/media/pci/ddbridge/ddbridge-core.c | 108 ++---
 3 files changed, 81 insertions(+), 45 deletions(-)

-- 
2.13.6



[PATCH 0/2] Support Physical Layer Scrambling

2017-12-17 Thread Ralph Metzler

Athanasios Oikonomou writes:
 > A new property DTV_SCRAMBLING_SEQUENCE_INDEX introduced to control
 > the gold sequence that several demods support.
 > 
 > Also the DVB API was increased in order userspace to be aware of the
 > changes.
 > 
 > The stv090x driver was changed to make use of the new property.
 > 
 > Those commits based on discussion previously made on the mailling list.
 > https://www.mail-archive.com/linux-media@vger.kernel.org/msg122600.html
 > 
 > I would like to thanks Ralph Metzler (r...@metzlerbros.de) for the
 > great help and ideas he provide me in order create those patches.
 > 
 > Athanasios Oikonomou (2):
 >   media: dvb_frontend: add physical layer scrambling support
 >   media: stv090x: add physical layer scrambling support
 > 
 >  .../media/uapi/dvb/fe_property_parameters.rst  | 18 
 > ++
 >  .../uapi/dvb/frontend-property-satellite-systems.rst   |  2 ++
 >  drivers/media/dvb-core/dvb_frontend.c  | 12 
 >  drivers/media/dvb-core/dvb_frontend.h  |  5 +
 >  drivers/media/dvb-frontends/stv090x.c  | 16 
 >  include/uapi/linux/dvb/frontend.h  |  5 -
 >  include/uapi/linux/dvb/version.h   |  2 +-
 >  7 files changed, 58 insertions(+), 2 deletions(-)
 > 
 > -- 
 > 2.1.4

Acked-by: Ralph Metzler 


We had some thoughts about having a:

#define NO_SCRAMBLING_CODE (~0U)

But DVB-S2 is always scrambling (with default index 0) and other delivery 
systems can ignore this
property. Or do you think it is needed?


One could add a define for AUTO or AUTO_S2X for the standard 7 indices to be 
tested
in DVB-S2X. But either dvb_frontend.c or the demod driver would have to support 
this in software.
I don't think there is a demod which supports this in hardware yet?


Regards,
Ralph


Re: dvb usb issues since kernel 4.9

2017-12-17 Thread Mauro Carvalho Chehab
Em Sun, 17 Dec 2017 12:06:37 +
Sean Young  escreveu:

> Hi Josef,

Em Sun, 17 Dec 2017 11:19:38 +0100
"Josef Griebichler"  escreveu:

> > Hello Mr. Caumont,
> >  
> > since switch to kernel 4.9 there are several users which have issues with 
> > their usb dvb cards.
> > Some get artifacts when watching livetv, I'm getting discontinuity errors 
> > in tvheadend when recording.
> > I'm using latest test build of LibreElec with kernel 4.14.6 but the issues 
> > are still there.
> > There's an librelec forum thread for this topic
> > https://forum.libreelec.tv/thread/4235-dvb-issue-since-le-switched-to-kernel-4-9-x/
> > and also an open kernel bug
> > https://bugzilla.kernel.org/show_bug.cgi?id=197835
> >  
> > This is my dmesg http://sprunge.us/WRIE
> > and tvh service log http://sprunge.us/bEiE
> >  
> > I saw in kernel changelog that you made an improvement/change for dvb und 
> > usb (commit 9a11204d2b26324636ff54f8d28095ed5dd17e91)
> >  
> > Is there anything that can be done to improve our situation or are we 
> > forced to stay with kernel 4.8?
> >  
> > Thanks for support!
> >  
> > Josef
> 
> Between kernel v4.8 and v4.9 there are many changes, and it is unlikely that
> commit 9a11204d2b26324636ff54f8d28095ed5dd17e91 is responsible for this.

Let me add linux-media@vger.kernel.org and linux-...@vger.kernel.org ML.

Josef, Please be sure that your e-mailer won't be sending e-mails with
HTML tags on it, otherwise the ML server will automatically drop.

> What would be really helpful is if you could find out which commit did
> cause a regression. This can be done by bisecting the kernel. There are
> various guides to this:
> 
>   https://wiki.ubuntu.com/Kernel/KernelBisection
> or
>   https://wiki.archlinux.org/index.php/Bisecting_bugs
> 
> Once the commit has been identified we can work together to narrow it down
> to the exact change, and then work together on a fix.

Yeah, we need more data in order to start tracking it. I suspect,
however, that a simple git bisect may not work in this case, due to the
USB changes that forbids DMA on stack that was added to Kernel 4.9, if
the card Josef is using was affected by such change.

Probably, he'll need to disable CONFIG_VMAP_STACK in the middle
of bisect (e. g. when the patch that implements it is added), 
or to cherry-pick any needed DMA fixup patch on the top of Kernel
4.8 before starting bisect.

It is also worth mentioning what's the USB host controller that
are used, and what's the media driver, as this could be an issue
there.

That's said, from the bug report, it seems that this is
happening on RPi3. Could you please test it also on a PC? That
will help to identify if the bug is at RPi's host driver or
on media drivers.

With regards to RPi3, there are actually two different drivers
for it: one used on Raspbian Kernel, and another one upstream.
They're completely different ones.

What driver are you using?

Thanks,
Mauro


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

2017-12-17 Thread Kieran Bingham
On 17/12/17 13:10, Kieran Bingham wrote:
> Hi Jacopo,
> 
> Thank you for the patch,
> 
> This seems like a good thing to do at a glance here, but I'll leave contextual
> judgement like that to Sakari.

Oh - I hit send and *then* my mail client wakes up and tells me Sakari reviewed
two days ago.

--
Kieran


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

2017-12-17 Thread Kieran Bingham
Hi Jacopo,

Thank you for the patch,

This seems like a good thing to do at a glance here, but I'll leave contextual
judgement like that to Sakari.

A few minor grammatical nits here and a question on locking.

On 13/12/17 18:26, Jacopo Mondi wrote:
> 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

uninitialized

> '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

postponing is not hyphenated.

> 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

Upscaled? Is this the right word here ? perhaps 'processed'?

"the complete() call chain cannot be process before the subdevice to which the
notifiers belong has been registered"

> 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 

Re: more build failures

2017-12-17 Thread Vincent McIntyre
On Sat, Dec 16, 2017 at 09:30:46AM -0200, Mauro Carvalho Chehab wrote:
> 
> Just pushed two patches to media build, in order to address those
> issues. Here, it is now compiling fine with Kernel 4.4.59.
> 

Yep, working again. Thank you for taking the time to sort this out.

Regards
Vince