Re: [PATCH 4/5] arm: dts: sun8i: a83t: Add support for the ir interface
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
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
Hi guys, On 15.12.2017 10:27, Mauro Carvalho Chehab wrote: > Em Fri, 15 Dec 2017 10:55:26 +0530 > Dhaval Shahescreveu: > >> 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
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
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
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
On Mon, Dec 18, 2017 at 6:45 AM, Philipp Rossakwrote: > 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
On Mon, Dec 18, 2017 at 6:45 AM, Philipp Rossakwrote: > 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
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
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
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
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
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
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
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
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
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
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
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
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
On Sun, Dec 17, 2017 at 7:46 PM, Maciej S. Szmigierowrote: > 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
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
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
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
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
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
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
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
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
Maciej, On Sun, Dec 17, 2017 at 6:37 PM, Maciej S. Szmigierowrote: > 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
On Sun, 17 Dec 2017 16:40:41 +0100 Daniel Schellerwrote: > 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()
From: Daniel SchellerIn 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
From: Daniel SchellerIn 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()
From: Daniel SchellerAll 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
From: Daniel SchellerIn 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
From: Daniel SchellerMove 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
From: Daniel SchellerDo 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
From: Daniel SchellerCurrently, 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
From: Daniel SchellerAs 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
From: Daniel SchellerThis 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
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 MetzlerWe 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
Em Sun, 17 Dec 2017 12:06:37 + Sean Youngescreveu: > 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
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
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
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