Re: [PATCH v2 1/6] [media] v4l: add blend modes controls

[Laurent Pinchart]

Hi Jacob,

Thank you for the patch.

On Saturday 15 Jul 2017 14:58:35 Jacob Chen wrote:
> At peresent, we don't have a control for Compositing and Blend.
> All drivers are just doing copies while actually many hardwares
> supports more functions.
> 
> So Adding V4L2 controls for Compositing and Blend, used for for
> composting streams.
> 
> The values are based on porter duff operations.
> Defined in below links.
> https://developer.xamarin.com/api/type/Android.Graphics.PorterDuff+Mode/

According to http://ssp.impulsetrain.com/porterduff.html,

"Despite being referred to as alpha blending and despite alpha often being 
used to model opacity, in concept Porter/Duff is not a way to blend the source 
and destination shapes. It is way to overlay, combine and trim them as if they 
were pieces of cardboard."

It then goes on defining blend modes that are a better match for the 
definition of blend. You might thus want to rename this control.

> Signed-off-by: Jacob Chen 
> Suggested-by: Nicolas Dufresne 
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 19 +++
>  include/uapi/linux/v4l2-controls.h   | 18 +-

Controls need to be documented in Documentation/media/uapi/v4l/control.rst or 
Documentation/media/uapi/v4l/extended-controls.rst.

>  2 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c index b9e08e3..8a235fd 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -216,6 +216,21 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>   "Private Packet, IVTV Format",
>   NULL
>   };
> + static const char * const blend_modes[] = {
> + "Source",
> + "Source Top",
> + "Source In",
> + "Source Out",
> + "Source Over",
> + "Destination",
> + "Destination Top",
> + "Destination In",
> + "Destination Out",
> + "Destination Over",
> + "Add",
> + "Clear",
> + NULL
> + };
>   static const char * const camera_power_line_frequency[] = {
>   "Disabled",
>   "50 Hz",
> @@ -522,6 +537,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>   return camera_exposure_metering;
>   case V4L2_CID_AUTO_FOCUS_RANGE:
>   return camera_auto_focus_range;
> + case V4L2_CID_BLEND:
> + return blend_modes;
>   case V4L2_CID_COLORFX:
>   return colorfx;
>   case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE:
> @@ -655,6 +672,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>   case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:   return "Min Number of Output
> Buffers"; case V4L2_CID_ALPHA_COMPONENT:  return "Alpha 
Component";
>   case V4L2_CID_COLORFX_CBCR: return "Color Effects, CbCr";
> + case V4L2_CID_BLEND:return "Compositing and Blend 
Modes";
> 
>   /* Codec controls */
>   /* The MPEG controls are applicable to all codec controls
> @@ -1033,6 +1051,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> v4l2_ctrl_type *type, case V4L2_CID_MPEG_STREAM_VBI_FMT:
>   case V4L2_CID_EXPOSURE_AUTO:
>   case V4L2_CID_AUTO_FOCUS_RANGE:
> + case V4L2_CID_BLEND:
>   case V4L2_CID_COLORFX:
>   case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE:
>   case V4L2_CID_TUNE_PREEMPHASIS:
> diff --git a/include/uapi/linux/v4l2-controls.h
> b/include/uapi/linux/v4l2-controls.h index 0d2e1e0..019fdca 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -140,8 +140,24 @@ enum v4l2_colorfx {
>  #define V4L2_CID_ALPHA_COMPONENT (V4L2_CID_BASE+41)
>  #define V4L2_CID_COLORFX_CBCR(V4L2_CID_BASE+42)
> 
> +#define V4L2_CID_BLEND   (V4L2_CID_BASE+43)

The image processing class (V4L2_CTRL_CLASS_IMAGE_PROC) would seem like a 
better fit than the base class for this control.

> +enum v4l2_blend_mode {
> + V4L2_BLEND_SRC  = 0,
> + V4L2_BLEND_SRCATOP  = 1,
> + V4L2_BLEND_SRCIN= 2,
> + V4L2_BLEND_SRCOUT   = 3,
> + V4L2_BLEND_SRCOVER  = 4,
> + V4L2_BLEND_DST  = 5,
> + V4L2_BLEND_DSTATOP  = 6,
> + V4L2_BLEND_DSTIN= 7,
> + V4L2_BLEND_DSTOUT   = 8,
> + V4L2_BLEND_DSTOVER  = 9,
> + V4L2_BLEND_ADD  = 10,
> + V4L2_BLEND_CLEAR= 11,
> +};
> + 
>  /* last CID + 1 */
> -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+43)
> +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+44)
> 
>  /* USER-class 

Re: [PATCH 1/3] [media] uvcvideo: variable size controls

[Laurent Pinchart]

Hi Philipp,

Thank you for the patch.

On Friday 14 Jul 2017 22:14:22 Philipp Zabel wrote:
> Some USB webcam controllers have extension unit controls that report
> different lengths via GET_LEN, depending on internal state.

If I ever need to hire a hardware designer, I'll make sure to reject any 
candidate who thinks that creativity is an asset :-(

If the size changes, could the flags change as well ? Should you issue a 
GET_INFO too ?

> Add a flag to mark these controls as variable length and issue GET_LEN
> before GET/SET_CUR transfers to verify the current length.

What happens if the internal state changes between the GET_LEN and the 
GET/SET_CUR ?

> Signed-off-by: Philipp Zabel 
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 26 +-
>  include/uapi/linux/uvcvideo.h|  2 ++
>  2 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c
> b/drivers/media/usb/uvc/uvc_ctrl.c index c2ee6e39fd0c..ce69e2c6937d 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1597,7 +1597,7 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device
> *dev, struct usb_device_id id;
>   u8 entity;
>   u8 selector;
> - u8 flags;
> + u16 flags;
>   };
> 
>   static const struct uvc_ctrl_fixup fixups[] = {
> @@ -1799,6 +1799,30 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
>   goto done;
>   }
> 
> + if ((ctrl->info.flags & UVC_CTRL_FLAG_VARIABLE_LEN) && reqflags) {
> + data = kmalloc(2, GFP_KERNEL);
> + /* Check if the control length has changed */
> + ret = uvc_query_ctrl(chain->dev, UVC_GET_LEN, xqry->unit,
> +  chain->dev->intfnum, xqry->selector, 
data,
> +  2);
> + size = le16_to_cpup((__le16 *)data);
> + kfree(data);

Now data is not NULL.

> + if (ret < 0) {
> + uvc_trace(UVC_TRACE_CONTROL,
> +   "GET_LEN failed on control %pUl/%u (%d).\n",
> +   entity->extension.guidExtensionCode,
> +   xqry->selector, ret);
> + goto done;

And the kfree(data) at the done label will cause a double free.

> + }
> + if (ctrl->info.size != size) {
> + uvc_trace(UVC_TRACE_CONTROL,
> +   "XU control %pUl/%u queried: len %u -> 
%u\n",
> +   entity->extension.guidExtensionCode,
> +   xqry->selector, ctrl->info.size, size);
> + ctrl->info.size = size;
> + }
> + }

How about moving this code (or part of it at least) to a function that could 
be shared with uvc_ctrl_fill_xu_info() ?

>   if (size != xqry->size) {
>   ret = -ENOBUFS;
>   goto done;
> diff --git a/include/uapi/linux/uvcvideo.h b/include/uapi/linux/uvcvideo.h
> index 3b081862b9e8..0f0d63e79045 100644
> --- a/include/uapi/linux/uvcvideo.h
> +++ b/include/uapi/linux/uvcvideo.h
> @@ -27,6 +27,8 @@
>  #define UVC_CTRL_FLAG_RESTORE(1 << 6)
>  /* Control can be updated by the camera. */
>  #define UVC_CTRL_FLAG_AUTO_UPDATE(1 << 7)
> +/* Control can change LEN */
> +#define UVC_CTRL_FLAG_VARIABLE_LEN   (1 << 8)
> 
>  #define UVC_CTRL_FLAG_GET_RANGE \
>   (UVC_CTRL_FLAG_GET_CUR | UVC_CTRL_FLAG_GET_MIN | \

-- 
Regards,

Laurent Pinchart

Re: rc-core: how to use hid (hardware) decoder?

[Sean Young]

Hello,

On Fri, Jul 14, 2017 at 04:14:05AM +0300, Antti Palosaari wrote:
> Moikka!
> Some remote controller receivers uses HID interface. I looked rc-core
> implementation, but failed to find how it could be used for hid. I need
> somehow get scancodes and keycodes out from rc-core and write those to
> hardware which then generate hid events. Also, I am not sure if kernel
> keycodes are same than HID codes, but if not then those should be translated
> somehow.
>
> There is rc_g_keycode_from_table() function, which could be used to dump
> current scancode:keycode mapping, but calling it in a loop millions of times
> is not surely correctly :]

Possibly you could use rc_map_get() to get the entire array. However you
would be limited to rc keymaps which are compiled into the kernel.

It be good to expose an interface to userspace which allows you to read and
write the mapping from IR protocol + scancode to hid usage codes; you could
then have an ir-keytable-like tool to change the keymap.


Sean

Re: rc-core: how to use hid (hardware) decoder?

[Antti Palosaari]

On 07/15/2017 12:05 PM, Sean Young wrote:

Hello,

On Fri, Jul 14, 2017 at 04:14:05AM +0300, Antti Palosaari wrote:

Moikka!
Some remote controller receivers uses HID interface. I looked rc-core
implementation, but failed to find how it could be used for hid. I need
somehow get scancodes and keycodes out from rc-core and write those to
hardware which then generate hid events. Also, I am not sure if kernel
keycodes are same than HID codes, but if not then those should be translated
somehow.

There is rc_g_keycode_from_table() function, which could be used to dump
current scancode:keycode mapping, but calling it in a loop millions of times
is not surely correctly :]


Possibly you could use rc_map_get() to get the entire array. However you
would be limited to rc keymaps which are compiled into the kernel.

It be good to expose an interface to userspace which allows you to read and
write the mapping from IR protocol + scancode to hid usage codes; you could
then have an ir-keytable-like tool to change the keymap.


That was just the plan. I added callback to ir_update_mapping() in order 
to get info and new rc_map every-time when it was changed. Then driver 
configures hid table accordingly.


I ran some issues:
* HID has very limited set of keys used for remote controllers compared 
to linux. So mapping from Linux remote controller to HID went hard.


* NEC 16/24/32 mess. rc_map used by rc-core was typed as NEC16, even 
there was NEC24 scancodes. So more self-made heuristics as hw wants NEC32.


So I given it up. I can configure that remote both polling mode via 
rc-core and HID. rc-core gives much more flexibility, mainly due to 
limited keymap of HID (hw supports only HID page 7, keyboard).


regards
Antti

--
http://palosaari.fi/

Re: [PATCH 3/3] [media] uvcvideo: skip non-extension unit controls on Oculus Rift Sensors

[Laurent Pinchart]

Hi Philipp,

Thank you for the patch.

On Friday 14 Jul 2017 22:14:24 Philipp Zabel wrote:
> The Oculus Rift Sensors (DK2 and CV1) allow to configure their sensor chips
> directly via I2C commands using extension unit controls. The processing and
> camera unit controls do not function at all.

Do the processing and camera units they report controls that don't work when 
exercised ? Who in a sane state of mind could have designed such a terrible 
product ?

If I understand you correctly, this device requires userspace code that knows 
how to program the sensor (and possibly other chips). If that's the case, is 
there an open-source implementation of that code publicly available ?

> Signed-off-by: Philipp Zabel 
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c
> b/drivers/media/usb/uvc/uvc_ctrl.c index 86cb16a2e7f4..573e1f8735bf 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2165,6 +2165,10 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
>  {
>   struct uvc_entity *entity;
>   unsigned int i;
> + const struct usb_device_id xu_only[] = {
> + { USB_DEVICE(0x2833, 0x0201) },
> + { USB_DEVICE(0x2833, 0x0211) },
> + };
> 
>   /* Walk the entities list and instantiate controls */
>   list_for_each_entry(entity, >entities, list) {
> @@ -2172,6 +2176,16 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
>   unsigned int bControlSize = 0, ncontrols;
>   __u8 *bmControls = NULL;
> 
> + /* Oculus Sensors only handle extension unit controls */
> + if (UVC_ENTITY_TYPE(entity) != UVC_VC_EXTENSION_UNIT) {
> + for (i = 0; i < ARRAY_SIZE(xu_only); i++) {
> + if (usb_match_one_id(dev->intf, _only[i]))
> + break;
> + }
> + if (i != ARRAY_SIZE(xu_only))
> + continue;
> + }
> +
>   if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT) {
>   bmControls = entity->extension.bmControls;
>   bControlSize = entity->extension.bControlSize;

-- 
Regards,

Laurent Pinchart

[PATCH v2 6/6] dt-bindings: Document the Rockchip RGA bindings

[Jacob Chen]

Add DT bindings documentation for Rockchip RGA

Signed-off-by: Yakir Yang 
Signed-off-by: Jacob Chen 
---
 .../devicetree/bindings/media/rockchip-rga.txt | 35 ++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/rockchip-rga.txt

diff --git a/Documentation/devicetree/bindings/media/rockchip-rga.txt 
b/Documentation/devicetree/bindings/media/rockchip-rga.txt
new file mode 100644
index 000..966eba0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/rockchip-rga.txt
@@ -0,0 +1,35 @@
+device-tree bindings for rockchip 2D raster graphic acceleration controller 
(RGA)
+
+RGA is a separate 2D raster graphic acceleration unit. It accelerates 2D
+graphics operations, such as point/line drawing, image scaling, rotation,
+BitBLT, alpha blending and image blur/sharpness.
+
+Required properties:
+- compatible: value should be one of the following
+   "rockchip,rk3228-rga";
+   "rockchip,rk3288-rga";
+   "rockchip,rk3399-rga";
+
+- interrupts: RGA interrupt number.
+
+- clocks: phandle to RGA sclk/hclk/aclk clocks
+
+- clock-names: should be "aclk" "hclk" and "sclk"
+
+- resets: Must contain an entry for each entry in reset-names.
+  See ../reset/reset.txt for details.
+- reset-names: should be "core" "axi" and "ahb"
+
+Example:
+SoC specific DT entry:
+   rga: rga@ff68 {
+   compatible = "rockchip,rk3399-rga";
+   reg = <0xff68 0x1>;
+   interrupts = ;
+   interrupt-names = "rga";
+   clocks = < ACLK_RGA>, < HCLK_RGA>, < SCLK_RGA_CORE>;
+   clock-names = "aclk", "hclk", "sclk";
+
+   resets = < SRST_RGA_CORE>, < SRST_A_RGA>, < 
SRST_H_RGA>;
+   reset-names = "core, "axi", "ahb";
+   };
-- 
2.7.4

[PATCH v2 5/6] ARM: dts: rockchip: enable RGA for rk3288 devices

[Jacob Chen]

Signed-off-by: Jacob Chen 
---
 arch/arm/boot/dts/rk3288-evb.dtsi | 4 
 arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi | 4 
 arch/arm/boot/dts/rk3288-firefly.dtsi | 4 
 arch/arm/boot/dts/rk3288-miqi.dts | 4 
 arch/arm/boot/dts/rk3288-popmetal.dts | 4 
 arch/arm/boot/dts/rk3288-tinker.dts   | 4 
 6 files changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-evb.dtsi 
b/arch/arm/boot/dts/rk3288-evb.dtsi
index 4905760..ec12162 100644
--- a/arch/arm/boot/dts/rk3288-evb.dtsi
+++ b/arch/arm/boot/dts/rk3288-evb.dtsi
@@ -379,6 +379,10 @@
};
 };
 
+ {
+   status = "okay";
+};
+
  {
status = "okay";
 };
diff --git a/arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi 
b/arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi
index 8134966..fffa92e2 100644
--- a/arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi
+++ b/arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi
@@ -283,6 +283,10 @@
};
 };
 
+ {
+   status = "okay";
+};
+
  {
rockchip,hw-tshut-mode = <0>;
rockchip,hw-tshut-polarity = <0>;
diff --git a/arch/arm/boot/dts/rk3288-firefly.dtsi 
b/arch/arm/boot/dts/rk3288-firefly.dtsi
index f520589..74a6ce5 100644
--- a/arch/arm/boot/dts/rk3288-firefly.dtsi
+++ b/arch/arm/boot/dts/rk3288-firefly.dtsi
@@ -500,6 +500,10 @@
};
 };
 
+ {
+   status = "okay";
+};
+
  {
vref-supply = <_18>;
status = "okay";
diff --git a/arch/arm/boot/dts/rk3288-miqi.dts 
b/arch/arm/boot/dts/rk3288-miqi.dts
index 21326f3..dc5e6bd 100644
--- a/arch/arm/boot/dts/rk3288-miqi.dts
+++ b/arch/arm/boot/dts/rk3288-miqi.dts
@@ -401,6 +401,10 @@
};
 };
 
+ {
+   status = "okay";
+};
+
  {
vref-supply = <_18>;
status = "okay";
diff --git a/arch/arm/boot/dts/rk3288-popmetal.dts 
b/arch/arm/boot/dts/rk3288-popmetal.dts
index aa1f9ec..362e5aa 100644
--- a/arch/arm/boot/dts/rk3288-popmetal.dts
+++ b/arch/arm/boot/dts/rk3288-popmetal.dts
@@ -490,6 +490,10 @@
};
 };
 
+ {
+   status = "okay";
+};
+
  {
rockchip,hw-tshut-mode = <0>;
rockchip,hw-tshut-polarity = <0>;
diff --git a/arch/arm/boot/dts/rk3288-tinker.dts 
b/arch/arm/boot/dts/rk3288-tinker.dts
index 525b0e5..1a8c149 100644
--- a/arch/arm/boot/dts/rk3288-tinker.dts
+++ b/arch/arm/boot/dts/rk3288-tinker.dts
@@ -460,6 +460,10 @@
status = "okay";
 };
 
+ {
+   status = "okay";
+};
+
  {
vref-supply = <_ldo1>;
status ="okay";
-- 
2.7.4

Re: [PATCH v2 5/6] ARM: dts: rockchip: enable RGA for rk3288 devices

[Laurent Pinchart]

Hi Jacob,

Thank you for the patch.

On Saturday 15 Jul 2017 14:58:39 Jacob Chen wrote:
> Signed-off-by: Jacob Chen 
> ---
>  arch/arm/boot/dts/rk3288-evb.dtsi | 4 
>  arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi | 4 
>  arch/arm/boot/dts/rk3288-firefly.dtsi | 4 
>  arch/arm/boot/dts/rk3288-miqi.dts | 4 
>  arch/arm/boot/dts/rk3288-popmetal.dts | 4 
>  arch/arm/boot/dts/rk3288-tinker.dts   | 4 

Some boards are missing from this list (Fennec, Phycore, ...) What criteria 
have you used to decide on which ones to enable the RGA ? That should be 
explained in the commit message.

>  6 files changed, 24 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/rk3288-evb.dtsi
> b/arch/arm/boot/dts/rk3288-evb.dtsi index 4905760..ec12162 100644
> --- a/arch/arm/boot/dts/rk3288-evb.dtsi
> +++ b/arch/arm/boot/dts/rk3288-evb.dtsi
> @@ -379,6 +379,10 @@
>   };
>  };
> 
> + {
> + status = "okay";
> +};
> +
>   {
>   status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi
> b/arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi index 8134966..fffa92e2
> 100644
> --- a/arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi
> +++ b/arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi
> @@ -283,6 +283,10 @@
>   };
>  };
> 
> + {
> + status = "okay";
> +};
> +
>   {
>   rockchip,hw-tshut-mode = <0>;
>   rockchip,hw-tshut-polarity = <0>;
> diff --git a/arch/arm/boot/dts/rk3288-firefly.dtsi
> b/arch/arm/boot/dts/rk3288-firefly.dtsi index f520589..74a6ce5 100644
> --- a/arch/arm/boot/dts/rk3288-firefly.dtsi
> +++ b/arch/arm/boot/dts/rk3288-firefly.dtsi
> @@ -500,6 +500,10 @@
>   };
>  };
> 
> + {
> + status = "okay";
> +};
> +
>   {
>   vref-supply = <_18>;
>   status = "okay";
> diff --git a/arch/arm/boot/dts/rk3288-miqi.dts
> b/arch/arm/boot/dts/rk3288-miqi.dts index 21326f3..dc5e6bd 100644
> --- a/arch/arm/boot/dts/rk3288-miqi.dts
> +++ b/arch/arm/boot/dts/rk3288-miqi.dts
> @@ -401,6 +401,10 @@
>   };
>  };
> 
> + {
> + status = "okay";
> +};
> +
>   {
>   vref-supply = <_18>;
>   status = "okay";
> diff --git a/arch/arm/boot/dts/rk3288-popmetal.dts
> b/arch/arm/boot/dts/rk3288-popmetal.dts index aa1f9ec..362e5aa 100644
> --- a/arch/arm/boot/dts/rk3288-popmetal.dts
> +++ b/arch/arm/boot/dts/rk3288-popmetal.dts
> @@ -490,6 +490,10 @@
>   };
>  };
> 
> + {
> + status = "okay";
> +};
> +
>   {
>   rockchip,hw-tshut-mode = <0>;
>   rockchip,hw-tshut-polarity = <0>;
> diff --git a/arch/arm/boot/dts/rk3288-tinker.dts
> b/arch/arm/boot/dts/rk3288-tinker.dts index 525b0e5..1a8c149 100644
> --- a/arch/arm/boot/dts/rk3288-tinker.dts
> +++ b/arch/arm/boot/dts/rk3288-tinker.dts
> @@ -460,6 +460,10 @@
>   status = "okay";
>  };
> 
> + {
> + status = "okay";
> +};
> +
>   {
>   vref-supply = <_ldo1>;
>   status ="okay";

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/6] dt-bindings: Document the Rockchip RGA bindings

[Laurent Pinchart]

Hi Jacob,

Thank you for the patch.

On Saturday 15 Jul 2017 14:58:40 Jacob Chen wrote:
> Add DT bindings documentation for Rockchip RGA
> 
> Signed-off-by: Yakir Yang 
> Signed-off-by: Jacob Chen 
> ---
>  .../devicetree/bindings/media/rockchip-rga.txt | 35 +++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/rockchip-rga.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/rockchip-rga.txt
> b/Documentation/devicetree/bindings/media/rockchip-rga.txt new file mode
> 100644
> index 000..966eba0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/rockchip-rga.txt
> @@ -0,0 +1,35 @@
> +device-tree bindings for rockchip 2D raster graphic acceleration controller
> (RGA)
> +
> +RGA is a separate 2D raster graphic acceleration unit. It accelerates 2D

"Separate" from what ? Do you mean "standalone" ?

> +graphics operations, such as point/line drawing, image scaling, rotation,
> +BitBLT, alpha blending and image blur/sharpness.
> +
> +Required properties:
> +- compatible: value should be one of the following
> + "rockchip,rk3228-rga";
> + "rockchip,rk3288-rga";
> + "rockchip,rk3399-rga";

The driver in patch 2/6 has match entry for rk3328, which is missing from this 
list.

As the implementation of the driver doesn't seem to discriminate between the 
four SoCs, wouldn't it make sense to create a generic compatible string on 
which the driver would match ? You can have both the generic and SoC-specific 
compatible strings in DT if there are differences between the IP core in those 
SoCs that might need to be handled later by the driver.

> +- interrupts: RGA interrupt number.

This is an "interrupt specifier", not just an "interrupt number" (as you can 
see in the example below there are three numbers)

> +
> +- clocks: phandle to RGA sclk/hclk/aclk clocks
> +
> +- clock-names: should be "aclk" "hclk" and "sclk"

Nitpicking, there should be a comma after "aclk".

> +
> +- resets: Must contain an entry for each entry in reset-names.
> +  See ../reset/reset.txt for details.
> +- reset-names: should be "core" "axi" and "ahb"

And a comma after "core".

> +
> +Example:
> +SoC specific DT entry:

s/SoC specific/SoC-specific/

> + rga: rga@ff68 {
> + compatible = "rockchip,rk3399-rga";
> + reg = <0xff68 0x1>;
> + interrupts = ;
> + interrupt-names = "rga";

The interrupt-names property is not described above. Do you really need it ?

> + clocks = < ACLK_RGA>, < HCLK_RGA>, < 
SCLK_RGA_CORE>;
> + clock-names = "aclk", "hclk", "sclk";
> +
> + resets = < SRST_RGA_CORE>, < SRST_A_RGA>, < 
SRST_H_RGA>;
> + reset-names = "core, "axi", "ahb";
> + };

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/6] [media] rockchip/rga: v4l2 m2m support

[Laurent Pinchart]

Hi Jacob,

Thank you for the patch.

On Saturday 15 Jul 2017 14:58:36 Jacob Chen wrote:
> Rockchip RGA is a separate 2D raster graphic acceleration unit. It
> accelerates 2D graphics operations, such as point/line drawing, image
> scaling, rotation, BitBLT, alpha blending and image blur/sharpness.
> 
> The drvier is mostly based on s5p-g2d v4l2 m2m driver.
> And supports various operations from the rendering pipeline.
>  - copy
>  - fast solid color fill
>  - rotation
>  - flip
>  - alpha blending

I notice that you don't support the drawing operations. How do you plan to 
support them later through the V4L2 M2M API ? I hate stating the obvious, but 
wouldn't the DRM API be better fit for a graphic accelerator ?

Additionally, V4L2 M2M has one source and one destination. How do you 
implement alpha blending in that case, which by definition requires at least 
two sources ?

> The code in rga-hw.c is used to configure regs accroding to operations.
> 
> The code in rga-buf.c is used to create private mmu table for RGA.
> The tables is stored in a list, and be removed when buffer is cleanup.

Looking at the implementation it seems to be a scatter-gather list, not an 
MMU. Is that right ? Does the hardware documentation refer to it as an MMU ?

> Signed-off-by: Jacob Chen 
> ---
>  drivers/media/platform/Kconfig|  11 +
>  drivers/media/platform/Makefile   |   2 +
>  drivers/media/platform/rockchip-rga/Makefile  |   3 +
>  drivers/media/platform/rockchip-rga/rga-buf.c | 122 
>  drivers/media/platform/rockchip-rga/rga-hw.c  | 652 ++
>  drivers/media/platform/rockchip-rga/rga-hw.h  | 437 
>  drivers/media/platform/rockchip-rga/rga.c | 958 +++
>  drivers/media/platform/rockchip-rga/rga.h | 111 +++
>  8 files changed, 2296 insertions(+)
>  create mode 100644 drivers/media/platform/rockchip-rga/Makefile
>  create mode 100644 drivers/media/platform/rockchip-rga/rga-buf.c
>  create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.c
>  create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.h
>  create mode 100644 drivers/media/platform/rockchip-rga/rga.c
>  create mode 100644 drivers/media/platform/rockchip-rga/rga.h

-- 
Regards,

Laurent Pinchart

[PATCH v2 2/6] [media] rockchip/rga: v4l2 m2m support

[Jacob Chen]

Rockchip RGA is a separate 2D raster graphic acceleration unit. It
accelerates 2D graphics operations, such as point/line drawing, image
scaling, rotation, BitBLT, alpha blending and image blur/sharpness.

The drvier is mostly based on s5p-g2d v4l2 m2m driver.
And supports various operations from the rendering pipeline.
 - copy
 - fast solid color fill
 - rotation
 - flip
 - alpha blending

The code in rga-hw.c is used to configure regs accroding to operations.

The code in rga-buf.c is used to create private mmu table for RGA.
The tables is stored in a list, and be removed when buffer is cleanup.

Signed-off-by: Jacob Chen 
---
 drivers/media/platform/Kconfig|  11 +
 drivers/media/platform/Makefile   |   2 +
 drivers/media/platform/rockchip-rga/Makefile  |   3 +
 drivers/media/platform/rockchip-rga/rga-buf.c | 122 
 drivers/media/platform/rockchip-rga/rga-hw.c  | 652 ++
 drivers/media/platform/rockchip-rga/rga-hw.h  | 437 
 drivers/media/platform/rockchip-rga/rga.c | 958 ++
 drivers/media/platform/rockchip-rga/rga.h | 111 +++
 8 files changed, 2296 insertions(+)
 create mode 100644 drivers/media/platform/rockchip-rga/Makefile
 create mode 100644 drivers/media/platform/rockchip-rga/rga-buf.c
 create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.c
 create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.h
 create mode 100644 drivers/media/platform/rockchip-rga/rga.c
 create mode 100644 drivers/media/platform/rockchip-rga/rga.h

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index c9106e1..8199bcf 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -411,6 +411,17 @@ config VIDEO_RENESAS_VSP1
  To compile this driver as a module, choose M here: the module
  will be called vsp1.
 
+config VIDEO_ROCKCHIP_RGA
+   tristate "Rockchip Raster 2d Grapphic Acceleration Unit"
+   depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
+   depends on ARCH_ROCKCHIP || COMPILE_TEST
+   select VIDEOBUF2_DMA_SG
+   select V4L2_MEM2MEM_DEV
+   default n
+   ---help---
+ This is a v4l2 driver for Rockchip SOC RGA2
+ 2d graphics accelerator.
+
 config VIDEO_TI_VPE
tristate "TI VPE (Video Processing Engine) driver"
depends on VIDEO_DEV && VIDEO_V4L2
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 349ddf6..3bf096f 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -54,6 +54,8 @@ obj-$(CONFIG_VIDEO_RENESAS_FDP1)  += rcar_fdp1.o
 obj-$(CONFIG_VIDEO_RENESAS_JPU)+= rcar_jpu.o
 obj-$(CONFIG_VIDEO_RENESAS_VSP1)   += vsp1/
 
+obj-$(CONFIG_VIDEO_ROCKCHIP_RGA)   += rockchip-rga/
+
 obj-y  += omap/
 
 obj-$(CONFIG_VIDEO_AM437X_VPFE)+= am437x/
diff --git a/drivers/media/platform/rockchip-rga/Makefile 
b/drivers/media/platform/rockchip-rga/Makefile
new file mode 100644
index 000..92fe254
--- /dev/null
+++ b/drivers/media/platform/rockchip-rga/Makefile
@@ -0,0 +1,3 @@
+rockchip-rga-objs := rga.o rga-hw.o rga-buf.o
+
+obj-$(CONFIG_VIDEO_ROCKCHIP_RGA) += rockchip-rga.o
diff --git a/drivers/media/platform/rockchip-rga/rga-buf.c 
b/drivers/media/platform/rockchip-rga/rga-buf.c
new file mode 100644
index 000..efa5686
--- /dev/null
+++ b/drivers/media/platform/rockchip-rga/rga-buf.c
@@ -0,0 +1,122 @@
+/*
+ * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
+ * Author: Jacob Chen 
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "rga-hw.h"
+#include "rga.h"
+
+static int
+rga_queue_setup(struct vb2_queue *vq,
+unsigned int *nbuffers, unsigned int *nplanes,
+unsigned int sizes[], struct device *alloc_devs[])
+{
+   struct rga_ctx *ctx = vb2_get_drv_priv(vq);
+   struct rga_frame *f = rga_get_frame(ctx, vq->type);
+
+   if (IS_ERR(f))
+   return PTR_ERR(f);
+
+   sizes[0] = f->size;
+   *nplanes = 1;
+
+   if (*nbuffers == 0)
+   *nbuffers = 1;
+
+   return 0;
+}
+
+static int rga_buf_prepare(struct vb2_buffer *vb)
+{
+   struct rga_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+   struct rga_frame *f = rga_get_frame(ctx, vb->vb2_queue->type);
+
+   if (IS_ERR(f))
+   return PTR_ERR(f);
+
+   vb2_set_plane_payload(vb, 0, f->size);
+
+   return 0;
+}
+

[PATCH v2 0/6] Add Rockchip RGA V4l2 support

[Jacob Chen]

This patch series add a v4l2 m2m drvier for rockchip RGA direct rendering based 
2d graphics acceleration module.

change in V2:
- generalize the controls.
- map buffers (10-50 us) in every cmd-run rather than in buffer-import to avoid 
get_free_pages failed on
actively used systems.
- remove status in dt-bindings examples.


Jacob Chen (6):
  [media] v4l: add blend modes controls
  [media] rockchip/rga: v4l2 m2m support
  ARM: dts: rockchip: add RGA device node for RK3288
  ARM: dts: rockchip: add RGA device node for RK3399
  ARM: dts: rockchip: enable RGA for rk3288 devices
  dt-bindings: Document the Rockchip RGA bindings

 .../devicetree/bindings/media/rockchip-rga.txt |  35 +
 arch/arm/boot/dts/rk3288-evb.dtsi  |   4 +
 arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi  |   4 +
 arch/arm/boot/dts/rk3288-firefly.dtsi  |   4 +
 arch/arm/boot/dts/rk3288-miqi.dts  |   4 +
 arch/arm/boot/dts/rk3288-popmetal.dts  |   4 +
 arch/arm/boot/dts/rk3288-tinker.dts|   4 +
 arch/arm/boot/dts/rk3288.dtsi  |  13 +
 arch/arm64/boot/dts/rockchip/rk3399.dtsi   |  13 +
 drivers/media/platform/Kconfig |  11 +
 drivers/media/platform/Makefile|   2 +
 drivers/media/platform/rockchip-rga/Makefile   |   3 +
 drivers/media/platform/rockchip-rga/rga-buf.c  | 176 
 drivers/media/platform/rockchip-rga/rga-hw.c   | 456 ++
 drivers/media/platform/rockchip-rga/rga-hw.h   | 434 +
 drivers/media/platform/rockchip-rga/rga.c  | 979 +
 drivers/media/platform/rockchip-rga/rga.h  | 133 +++
 drivers/media/v4l2-core/v4l2-ctrls.c   |  19 +
 include/uapi/linux/v4l2-controls.h |  18 +-
 19 files changed, 2315 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/media/rockchip-rga.txt
 create mode 100644 drivers/media/platform/rockchip-rga/Makefile
 create mode 100644 drivers/media/platform/rockchip-rga/rga-buf.c
 create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.c
 create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.h
 create mode 100644 drivers/media/platform/rockchip-rga/rga.c
 create mode 100644 drivers/media/platform/rockchip-rga/rga.h

-- 
2.7.4

[PATCH v2 3/6] ARM: dts: rockchip: add RGA device node for RK3288

[Jacob Chen]

This patch add the RGA dt config of rk3288 SoC.

Signed-off-by: Yakir Yang 
Signed-off-by: Jacob Chen 
---
 arch/arm/boot/dts/rk3288.dtsi | 13 +
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 1efc2f2..83d025d 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -945,6 +945,19 @@
status = "okay";
};
 
+   rga: rga@ff92 {
+   compatible = "rockchip,rk3288-rga";
+   reg = <0xff92 0x180>;
+   interrupts = ;
+   interrupt-names = "rga";
+   clocks = < ACLK_RGA>, < HCLK_RGA>, < SCLK_RGA>;
+   clock-names = "aclk", "hclk", "sclk";
+   power-domains = < RK3288_PD_VIO>;
+   resets = < SRST_RGA_CORE>, < SRST_RGA_AXI>, < 
SRST_RGA_AHB>;
+   reset-names = "core", "axi", "ahb";
+   status = "disabled";
+   };
+
vopb: vop@ff93 {
compatible = "rockchip,rk3288-vop";
reg = <0xff93 0x19c>;
-- 
2.7.4

[PATCH v2 1/6] [media] v4l: add blend modes controls

[Jacob Chen]

At peresent, we don't have a control for Compositing and Blend.
All drivers are just doing copies while actually many hardwares
supports more functions.

So Adding V4L2 controls for Compositing and Blend, used for for
composting streams.

The values are based on porter duff operations.
Defined in below links.
https://developer.xamarin.com/api/type/Android.Graphics.PorterDuff+Mode/

Signed-off-by: Jacob Chen 
Suggested-by: Nicolas Dufresne 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 19 +++
 include/uapi/linux/v4l2-controls.h   | 18 +-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index b9e08e3..8a235fd 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -216,6 +216,21 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
"Private Packet, IVTV Format",
NULL
};
+   static const char * const blend_modes[] = {
+   "Source",
+   "Source Top",
+   "Source In",
+   "Source Out",
+   "Source Over",
+   "Destination",
+   "Destination Top",
+   "Destination In",
+   "Destination Out",
+   "Destination Over",
+   "Add",
+   "Clear",
+   NULL
+   };
static const char * const camera_power_line_frequency[] = {
"Disabled",
"50 Hz",
@@ -522,6 +537,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
return camera_exposure_metering;
case V4L2_CID_AUTO_FOCUS_RANGE:
return camera_auto_focus_range;
+   case V4L2_CID_BLEND:
+   return blend_modes;
case V4L2_CID_COLORFX:
return colorfx;
case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE:
@@ -655,6 +672,7 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:   return "Min Number of Output 
Buffers";
case V4L2_CID_ALPHA_COMPONENT:  return "Alpha Component";
case V4L2_CID_COLORFX_CBCR: return "Color Effects, CbCr";
+   case V4L2_CID_BLEND:return "Compositing and Blend 
Modes";
 
/* Codec controls */
/* The MPEG controls are applicable to all codec controls
@@ -1033,6 +1051,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum 
v4l2_ctrl_type *type,
case V4L2_CID_MPEG_STREAM_VBI_FMT:
case V4L2_CID_EXPOSURE_AUTO:
case V4L2_CID_AUTO_FOCUS_RANGE:
+   case V4L2_CID_BLEND:
case V4L2_CID_COLORFX:
case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE:
case V4L2_CID_TUNE_PREEMPHASIS:
diff --git a/include/uapi/linux/v4l2-controls.h 
b/include/uapi/linux/v4l2-controls.h
index 0d2e1e0..019fdca 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -140,8 +140,24 @@ enum v4l2_colorfx {
 #define V4L2_CID_ALPHA_COMPONENT   (V4L2_CID_BASE+41)
 #define V4L2_CID_COLORFX_CBCR  (V4L2_CID_BASE+42)
 
+#define V4L2_CID_BLEND (V4L2_CID_BASE+43)
+enum v4l2_blend_mode {
+   V4L2_BLEND_SRC  = 0,
+   V4L2_BLEND_SRCATOP  = 1,
+   V4L2_BLEND_SRCIN= 2,
+   V4L2_BLEND_SRCOUT   = 3,
+   V4L2_BLEND_SRCOVER  = 4,
+   V4L2_BLEND_DST  = 5,
+   V4L2_BLEND_DSTATOP  = 6,
+   V4L2_BLEND_DSTIN= 7,
+   V4L2_BLEND_DSTOUT   = 8,
+   V4L2_BLEND_DSTOVER  = 9,
+   V4L2_BLEND_ADD  = 10,
+   V4L2_BLEND_CLEAR= 11,
+};
+
 /* last CID + 1 */
-#define V4L2_CID_LASTP1 (V4L2_CID_BASE+43)
+#define V4L2_CID_LASTP1 (V4L2_CID_BASE+44)
 
 /* USER-class private control IDs */
 
-- 
2.7.4

[PATCH v2 4/6] ARM: dts: rockchip: add RGA device node for RK3399

[Jacob Chen]

This patch add the RGA dt config of RK3399 SoC.

Signed-off-by: Yakir Yang 
Signed-off-by: Jacob Chen 
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 13 +
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 8e6d1bd..fc33ce5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1056,6 +1056,19 @@
status = "disabled";
};
 
+   rga: rga@ff68 {
+   compatible = "rockchip,rk3399-rga";
+   reg = <0x0 0xff68 0x0 0x1>;
+   interrupts = ;
+   interrupt-names = "rga";
+   clocks = < ACLK_RGA>, < HCLK_RGA>, < SCLK_RGA_CORE>;
+   clock-names = "aclk", "hclk", "sclk";
+   resets = < SRST_RGA_CORE>, < SRST_A_RGA>, < 
SRST_H_RGA>;
+   reset-names = "core", "axi", "ahb";
+   power-domains = < RK3399_PD_RGA>;
+   status = "disabled";
+   };
+
efuse0: efuse@ff69 {
compatible = "rockchip,rk3399-efuse";
reg = <0x0 0xff69 0x0 0x80>;
-- 
2.7.4

Re: [PATCH v2 3/6] ARM: dts: rockchip: add RGA device node for RK3288

[Heiko Stuebner]

Hi Jacob,

Am Samstag, 15. Juli 2017, 14:58:37 CEST schrieb Jacob Chen:
> This patch add the RGA dt config of rk3288 SoC.
> 
> Signed-off-by: Yakir Yang 
> Signed-off-by: Jacob Chen 

from the Signed-off it looks like Yakir was te original author of the
patch. So please fix the authorship or drop the top Signed-off by.

Same for other patches like this.


Thanks
Heiko

> ---
>  arch/arm/boot/dts/rk3288.dtsi | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> index 1efc2f2..83d025d 100644
> --- a/arch/arm/boot/dts/rk3288.dtsi
> +++ b/arch/arm/boot/dts/rk3288.dtsi
> @@ -945,6 +945,19 @@
>   status = "okay";
>   };
>  
> + rga: rga@ff92 {
> + compatible = "rockchip,rk3288-rga";
> + reg = <0xff92 0x180>;
> + interrupts = ;
> + interrupt-names = "rga";
> + clocks = < ACLK_RGA>, < HCLK_RGA>, < SCLK_RGA>;
> + clock-names = "aclk", "hclk", "sclk";
> + power-domains = < RK3288_PD_VIO>;
> + resets = < SRST_RGA_CORE>, < SRST_RGA_AXI>, < 
> SRST_RGA_AHB>;
> + reset-names = "core", "axi", "ahb";
> + status = "disabled";
> + };
> +
>   vopb: vop@ff93 {
>   compatible = "rockchip,rk3288-vop";
>   reg = <0xff93 0x19c>;
> 

Re: [PATCH 10/14] staging:iio:resolver:ad2s1210 fix negative IIO_ANGL_VEL read

[Jonathan Cameron]

On Fri, 14 Jul 2017 11:31:03 +0200
Arnd Bergmann  wrote:

> gcc-7 points out an older regression:
> 
> drivers/staging/iio/resolver/ad2s1210.c: In function 'ad2s1210_read_raw':
> drivers/staging/iio/resolver/ad2s1210.c:515:42: error: '<<' in boolean 
> context, did you mean '<' ? [-Werror=int-in-bool-context]
> 
> The original code had 'unsigned short' here, but incorrectly got
> converted to 'bool'. This reverts the regression and uses a normal
> type instead.
> 
> Fixes: 29148543c521 ("staging:iio:resolver:ad2s1210 minimal chan spec 
> conversion.")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Arnd Bergmann 
Thanks Arnd,

Applied to the fixes-togreg branch of iio.git.

Jonathan
> ---
>  drivers/staging/iio/resolver/ad2s1210.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c 
> b/drivers/staging/iio/resolver/ad2s1210.c
> index a6a8393d6664..3e00df74b18c 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -472,7 +472,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
>long m)
>  {
>   struct ad2s1210_state *st = iio_priv(indio_dev);
> - bool negative;
> + u16 negative;
>   int ret = 0;
>   u16 pos;
>   s16 vel;

[PATCH for 4.13] cec-notifier: small improvements

[Hans Verkuil]

Allow calling cec_notifier_set_phys_addr and
cec_notifier_set_phys_addr_from_edid with a NULL notifier, in which
case these functions do nothing.

Add a cec_notifier_phys_addr_invalidate helper function (the notifier
equivalent of cec_phys_addr_invalidate).

These changes simplify drm CEC driver support.

Signed-off-by: Hans Verkuil 
---
 drivers/media/cec/cec-notifier.c |  6 ++
 include/media/cec-notifier.h | 15 +++
 2 files changed, 21 insertions(+)

diff --git a/drivers/media/cec/cec-notifier.c b/drivers/media/cec/cec-notifier.c
index 74dc1c32080e..08b619d0ea1e 100644
--- a/drivers/media/cec/cec-notifier.c
+++ b/drivers/media/cec/cec-notifier.c
@@ -87,6 +87,9 @@ EXPORT_SYMBOL_GPL(cec_notifier_put);

 void cec_notifier_set_phys_addr(struct cec_notifier *n, u16 pa)
 {
+   if (n == NULL)
+   return;
+
mutex_lock(>lock);
n->phys_addr = pa;
if (n->callback)
@@ -100,6 +103,9 @@ void cec_notifier_set_phys_addr_from_edid(struct 
cec_notifier *n,
 {
u16 pa = CEC_PHYS_ADDR_INVALID;

+   if (n == NULL)
+   return;
+
if (edid && edid->extensions)
pa = cec_get_edid_phys_addr((const u8 *)edid,
EDID_LENGTH * (edid->extensions + 1), NULL);
diff --git a/include/media/cec-notifier.h b/include/media/cec-notifier.h
index 298f996969df..a4f7429c4ae5 100644
--- a/include/media/cec-notifier.h
+++ b/include/media/cec-notifier.h
@@ -57,6 +57,7 @@ void cec_notifier_put(struct cec_notifier *n);
  * @pa: the CEC physical address
  *
  * Set a new CEC physical address.
+ * Does nothing if @n == NULL.
  */
 void cec_notifier_set_phys_addr(struct cec_notifier *n, u16 pa);

@@ -66,6 +67,7 @@ void cec_notifier_set_phys_addr(struct cec_notifier *n, u16 
pa);
  * @edid: the struct edid pointer
  *
  * Parses the EDID to obtain the new CEC physical address and set it.
+ * Does nothing if @n == NULL.
  */
 void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n,
  const struct edid *edid);
@@ -118,4 +120,17 @@ static inline void cec_notifier_unregister(struct 
cec_notifier *n)

 #endif

+/**
+ * cec_notifier_phys_addr_invalidate() - set the physical address to INVALID
+ *
+ * @n: the CEC notifier
+ *
+ * This is a simple helper function to invalidate the physical
+ * address. Does nothing if @n == NULL.
+ */
+static inline void cec_notifier_phys_addr_invalidate(struct cec_notifier *n)
+{
+   cec_notifier_set_phys_addr(n, CEC_PHYS_ADDR_INVALID);
+}
+
 #endif
-- 
2.11.0

[PATCH 4/4] drm/tegra: add cec-notifier support

[Hans Verkuil]

From: Hans Verkuil 

In order to support CEC the HDMI driver has to inform the CEC driver
whenever the physical address changes. So when the EDID is read the
CEC driver has to be informed and whenever the hotplug detect goes
away.

This is done through the cec-notifier framework.

The link between the HDMI driver and the CEC driver is done through
the hdmi_phandle in the tegra-cec node in the device tree.

Signed-off-by: Hans Verkuil 
---
 drivers/gpu/drm/tegra/drm.h| 3 +++
 drivers/gpu/drm/tegra/hdmi.c   | 9 +
 drivers/gpu/drm/tegra/output.c | 6 ++
 3 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 6d6da01282f3..c0a18b60caf1 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -212,6 +212,8 @@ int tegra_dc_state_setup_clock(struct tegra_dc *dc,
   struct clk *clk, unsigned long pclk,
   unsigned int div);
 
+struct cec_notifier;
+
 struct tegra_output {
struct device_node *of_node;
struct device *dev;
@@ -219,6 +221,7 @@ struct tegra_output {
struct drm_panel *panel;
struct i2c_adapter *ddc;
const struct edid *edid;
+   struct cec_notifier *notifier;
unsigned int hpd_irq;
int hpd_gpio;
enum of_gpio_flags hpd_gpio_flags;
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index cda0491ed6bf..fbf14e1efd0e 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -21,6 +21,8 @@
 
 #include 
 
+#include 
+
 #include "hdmi.h"
 #include "drm.h"
 #include "dc.h"
@@ -1720,6 +1722,10 @@ static int tegra_hdmi_probe(struct platform_device *pdev)
return PTR_ERR(hdmi->vdd);
}
 
+   hdmi->output.notifier = cec_notifier_get(>dev);
+   if (hdmi->output.notifier == NULL)
+   return -ENOMEM;
+
hdmi->output.dev = >dev;
 
err = tegra_output_probe(>output);
@@ -1778,6 +1784,9 @@ static int tegra_hdmi_remove(struct platform_device *pdev)
 
tegra_output_remove(>output);
 
+   if (hdmi->output.notifier)
+   cec_notifier_put(hdmi->output.notifier);
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index 595d1ec3e02e..57c052521a44 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -11,6 +11,8 @@
 #include 
 #include "drm.h"
 
+#include 
+
 int tegra_output_connector_get_modes(struct drm_connector *connector)
 {
struct tegra_output *output = connector_to_output(connector);
@@ -33,6 +35,7 @@ int tegra_output_connector_get_modes(struct drm_connector 
*connector)
edid = drm_get_edid(connector, output->ddc);
 
drm_mode_connector_update_edid_property(connector, edid);
+   cec_notifier_set_phys_addr_from_edid(output->notifier, edid);
 
if (edid) {
err = drm_add_edid_modes(connector, edid);
@@ -68,6 +71,9 @@ tegra_output_connector_detect(struct drm_connector 
*connector, bool force)
status = connector_status_connected;
}
 
+   if (status != connector_status_connected)
+   cec_notifier_phys_addr_invalidate(output->notifier);
+
return status;
 }
 
-- 
2.11.0

[PATCH 1/4] dt-bindings: document the tegra CEC bindings

[Hans Verkuil]

From: Hans Verkuil 

This documents the binding for the Tegra CEC module.

Signed-off-by: Hans Verkuil 
---
 .../devicetree/bindings/media/tegra-cec.txt| 26 ++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/tegra-cec.txt

diff --git a/Documentation/devicetree/bindings/media/tegra-cec.txt 
b/Documentation/devicetree/bindings/media/tegra-cec.txt
new file mode 100644
index ..ba0b6071acaa
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/tegra-cec.txt
@@ -0,0 +1,26 @@
+* Tegra HDMI CEC driver
+
+The HDMI CEC module is present in Tegra SoCs and its purpose is to
+handle communication between HDMI connected devices over the CEC bus.
+
+Required properties:
+  - compatible : value should be one of the following:
+   "nvidia,tegra114-cec"
+   "nvidia,tegra124-cec"
+   "nvidia,tegra210-cec"
+  - reg : Physical base address of the IP registers and length of memory
+ mapped region.
+  - interrupts : HDMI CEC interrupt number to the CPU.
+  - clocks : from common clock binding: handle to HDMI CEC clock.
+  - clock-names : from common clock binding: must contain "cec",
+ corresponding to ithe entry in the clocks property.
+  - hdmi-phandle : phandle to the HDMI controller, see also cec.txt.
+
+Example:
+
+tegra_cec {
+   compatible = "nvidia,tegra124-cec";
+   reg = <0x0 0x70015000 0x0 0x1000>;
+   interrupts = ;
+   clocks = <_car TEGRA124_CLK_CEC>;
+   clock-names = "cec";
-- 
2.11.0

[PATCH 3/4] tegra-cec: add Tegra HDMI CEC driver

[Hans Verkuil]

From: Hans Verkuil 

This driver adds support for the Tegra CEC IP. It is based on the
NVIDIA drivers/misc/tegra-cec driver in their 3.10 kernel.

This has been converted to the CEC framework and cleaned up.

Tested with my Jetson TK1 board. It has also been tested with the
Tegra X1 in an embedded product.

Note of warning for the Tegra X2: this SoC supports two HDMI outputs,
but only one CEC adapter and the CEC bus is shared between the
two outputs. This is a design mistake and the CEC adapter can
control only one HDMI output. Never hook up both HDMI outputs
to the CEC bus in a hardware design: this is illegal as per the
CEC specification.

The CEC bus can be shared between multiple inputs and zero or one
outputs, but not between multiple outputs.

Signed-off-by: Hans Verkuil 
---
 MAINTAINERS  |   8 +
 drivers/media/platform/Kconfig   |  11 +
 drivers/media/platform/Makefile  |   2 +
 drivers/media/platform/tegra-cec/Makefile|   1 +
 drivers/media/platform/tegra-cec/tegra_cec.c | 506 +++
 drivers/media/platform/tegra-cec/tegra_cec.h | 127 +++
 6 files changed, 655 insertions(+)
 create mode 100644 drivers/media/platform/tegra-cec/Makefile
 create mode 100644 drivers/media/platform/tegra-cec/tegra_cec.c
 create mode 100644 drivers/media/platform/tegra-cec/tegra_cec.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 7d9bd4a041af..35b393feac52 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1917,6 +1917,14 @@ M:   Lennert Buytenhek 
 L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers)
 S: Maintained
 
+ARM/TEGRA HDMI CEC SUBSYSTEM SUPPORT
+M: Hans Verkuil 
+L: linux-te...@vger.kernel.org
+L: linux-media@vger.kernel.org
+S: Maintained
+F: drivers/media/platform/tegra-cec/
+F: Documentation/devicetree/bindings/media/tegra-cec.txt
+
 ARM/TETON BGA MACHINE SUPPORT
 M: "Mark F. Brown" 
 L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers)
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 1313cd533436..31f54cbdf2e2 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -570,6 +570,17 @@ config VIDEO_STM32_HDMI_CEC
  CEC bus is present in the HDMI connector and enables communication
  between compatible devices.
 
+config VIDEO_TEGRA_HDMI_CEC
+   tristate "Tegra HDMI CEC driver"
+   depends on ARCH_TEGRA || COMPILE_TEST
+   select CEC_CORE
+   select CEC_NOTIFIER
+   ---help---
+ This is a driver for the Tegra HDMI CEC interface. It uses the
+ generic CEC framework interface.
+ The CEC bus is present in the HDMI connector and enables communication
+ between compatible devices.
+
 endif #CEC_PLATFORM_DRIVERS
 
 menuconfig SDR_PLATFORM_DRIVERS
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 9beadc760467..9da73532e556 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -46,6 +46,8 @@ obj-$(CONFIG_VIDEO_STI_HDMI_CEC)  += sti/cec/
 
 obj-$(CONFIG_VIDEO_STI_DELTA)  += sti/delta/
 
+obj-$(CONFIG_VIDEO_TEGRA_HDMI_CEC) += tegra-cec/
+
 obj-y  += stm32/
 
 obj-y   += blackfin/
diff --git a/drivers/media/platform/tegra-cec/Makefile 
b/drivers/media/platform/tegra-cec/Makefile
new file mode 100644
index ..f3d81127589f
--- /dev/null
+++ b/drivers/media/platform/tegra-cec/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_TEGRA_HDMI_CEC) += tegra_cec.o
diff --git a/drivers/media/platform/tegra-cec/tegra_cec.c 
b/drivers/media/platform/tegra-cec/tegra_cec.c
new file mode 100644
index ..346586c3ad6d
--- /dev/null
+++ b/drivers/media/platform/tegra-cec/tegra_cec.c
@@ -0,0 +1,506 @@
+/*
+ * Tegra CEC implementation
+ *
+ * The original 3.10 CEC driver using a custom API:
+ *
+ * Copyright (c) 2012-2015, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * Conversion to the CEC framework and to the mainline kernel:
+ *
+ * Copyright 2016-2017 Cisco Systems, Inc. and/or its affiliates. All rights 
reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 

[PATCH 2/4] ARM: tegra: add CEC support to tegra124.dtsi

[Hans Verkuil]

From: Hans Verkuil 

Add support for the Tegra CEC IP to tegra124.dtsi and enable it on the
Jetson TK1.

Signed-off-by: Hans Verkuil 
---
 arch/arm/boot/dts/tegra124-jetson-tk1.dts |  4 
 arch/arm/boot/dts/tegra124.dtsi   | 12 +++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts 
b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
index 7bacb2954f58..c22c0e6dc3d9 100644
--- a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
+++ b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
@@ -67,6 +67,10 @@
};
};
 
+   tegra_cec {
+   status = "okay";
+   };
+
gpu@0,5700 {
/*
 * Node left disabled on purpose - the bootloader will enable
diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 1b10b14a6abd..df7e9e2925f5 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -123,7 +123,7 @@
nvidia,head = <1>;
};
 
-   hdmi@5428 {
+   hdmi: hdmi@5428 {
compatible = "nvidia,tegra124-hdmi";
reg = <0x0 0x5428 0x0 0x0004>;
interrupts = ;
@@ -851,6 +851,16 @@
status = "disabled";
};
 
+   tegra_cec {
+   compatible = "nvidia,tegra124-cec";
+   reg = <0x0 0x70015000 0x0 0x1000>;
+   interrupts = ;
+   clocks = <_car TEGRA124_CLK_CEC>;
+   clock-names = "cec";
+   hdmi-phandle = <>;
+   status = "disabled";
+   };
+
soctherm: thermal-sensor@700e2000 {
compatible = "nvidia,tegra124-soctherm";
reg = <0x0 0x700e2000 0x0 0x600 /* SOC_THERM reg_base */
-- 
2.11.0

[PATCH 0/4] tegra-cec: add Tegra HDMI CEC support

[Hans Verkuil]

From: Hans Verkuil 

This patch series adds support for the Tegra CEC functionality.

It has two prerequisites:

this cec-notifier patch: https://patchwork.linuxtv.org/patch/42521/

and this workaround: http://www.spinics.net/lists/dri-devel/msg147038.html

A proper fix needs to be found for that workaround, but it is good
enough for testing this patch series.

The first patch documents the CEC bindings, the second adds support
for this to tegra124.dtsi and enables it for the Jetson TK1.

The third patch adds the CEC driver itself and the final patch adds
the cec notifier support to the drm/tegra driver in order to notify
the CEC driver whenever the physical address changes.

I expect that the dts changes apply as well to the Tegra X1/X2 and possibly
other Tegra SoCs, but I can only test this with my Jetson TK1 board.

The dt-bindings and the tegra-cec driver would go in through the media
subsystem, the drm/tegra part through the drm subsystem and the dts
changes through (I guess) the linux-tegra developers. Luckily they are
all independent of one another.

To test this you need the CEC utilities from git://linuxtv.org/v4l-utils.git.

To build this:

git clone git://linuxtv.org/v4l-utils.git
cd v4l-utils
./bootstrap.sh; ./configure
make
sudo make install   # optional, you really only need utils/cec*

To test:

cec-ctl --playback  # configure as playback device
cec-ctl -S  # detect all connected CEC devices

See here for the public CEC API:

https://hverkuil.home.xs4all.nl/spec/uapi/cec/cec-api.html

Regards,

Hans

Hans Verkuil (4):
  dt-bindings: document the tegra CEC bindings
  ARM: tegra: add CEC support to tegra124.dtsi
  tegra-cec: add Tegra HDMI CEC driver
  drm/tegra: add cec-notifier support

 .../devicetree/bindings/media/tegra-cec.txt|  26 ++
 MAINTAINERS|   8 +
 arch/arm/boot/dts/tegra124-jetson-tk1.dts  |   4 +
 arch/arm/boot/dts/tegra124.dtsi|  12 +-
 drivers/gpu/drm/tegra/drm.h|   3 +
 drivers/gpu/drm/tegra/hdmi.c   |   9 +
 drivers/gpu/drm/tegra/output.c |   6 +
 drivers/media/platform/Kconfig |  11 +
 drivers/media/platform/Makefile|   2 +
 drivers/media/platform/tegra-cec/Makefile  |   1 +
 drivers/media/platform/tegra-cec/tegra_cec.c   | 506 +
 drivers/media/platform/tegra-cec/tegra_cec.h   | 127 ++
 12 files changed, 714 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/media/tegra-cec.txt
 create mode 100644 drivers/media/platform/tegra-cec/Makefile
 create mode 100644 drivers/media/platform/tegra-cec/tegra_cec.c
 create mode 100644 drivers/media/platform/tegra-cec/tegra_cec.h

-- 
2.11.0

Re: [PATCH, RESEND 02/14] ata: avoid gcc-7 warning in ata_timing_quantize

[Tejun Heo]

On Fri, Jul 14, 2017 at 11:25:14AM +0200, Arnd Bergmann wrote:
> gcc-7 warns about the result of a constant multiplication used as
> a boolean:
> 
> drivers/ata/libata-core.c: In function 'ata_timing_quantize':
> drivers/ata/libata-core.c:3164:30: warning: '*' in boolean context, suggest 
> '&&' instead [-Wint-in-bool-context]
> 
> This slightly rearranges the macro to simplify the code and avoid
> the warning at the same time.
> 
> Signed-off-by: Arnd Bergmann 

If the whole series will be routed together,

 Acked-by: Tejun Heo 

If not, please let me know.  I'll push it through
libata/for-4.13-fixes.

Thanks!

-- 
tejun

Re: Lots of new warnings with gcc-7.1.1

[Tejun Heo]

Hello,

On Wed, Jul 12, 2017 at 03:31:02PM +0200, Arnd Bergmann wrote:
> > We also have about a bazillion
> >
> > warning: ‘*’ in boolean context, suggest ‘&&’ instead
> >
> > warnings in drivers/ata/libata-core.c, all due to a single macro that
> > uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
> > debatable, but I suspect the macro could easily be changed too.
> >
> > Tejun, would you hate just moving the "multiply by 1000" part _into_
> > that EZ() macro? Something like the attached (UNTESTED!) patch?
> 
> Tejun applied an almost identical patch of mine a while ago, but it seems to
> have gotten lost in the meantime in some rebase:

Yeah, I was scratching my head remembering your patch.  Sorry about
that.  It should have been routed through for-4.12-fixes.

> https://patchwork.kernel.org/patch/9721397/
> https://patchwork.kernel.org/patch/9721399/
> 
> I guess I should have resubmitted the second patch with the suggested
> improvement.

The new one looks good to me.

Thanks.

-- 
tejun

Re: [PATCH 3/3] [media] uvcvideo: skip non-extension unit controls on Oculus Rift Sensors

[Philipp Zabel]

Am Samstag, den 15.07.2017, 12:54 +0300 schrieb Laurent Pinchart:
> Hi Philipp,
> 
> Thank you for the patch.
> 
> On Friday 14 Jul 2017 22:14:24 Philipp Zabel wrote:
> > The Oculus Rift Sensors (DK2 and CV1) allow to configure their sensor chips
> > directly via I2C commands using extension unit controls. The processing and
> > camera unit controls do not function at all.
> 
> Do the processing and camera units they report controls that don't work when 
> exercised ? Who in a sane state of mind could have designed such a terrible 
> product ?

Yes. Without this patch I get a bunch of controls that have no effect
whatsoever.

> If I understand you correctly, this device requires userspace code that knows 
> how to program the sensor (and possibly other chips). If that's the case, is 
> there an open-source implementation of that code publicly available ?

Well, it's all still a bit in the experimentation phase. We have an
implementation to set up the DK2 camera for synchronised exposure
triggered by the Rift DK2 HMD and to read the calibration data from
flash, here:

https://github.com/pH5/ouvrt/blob/master/src/esp570.c
https://github.com/pH5/ouvrt/blob/master/src/mt9v034.c

And an even more rough version to set up the CV1 camera for
synchronised exposure triggered by the Rift CV1 HMD here:

https://github.com/OpenHMD/OpenHMD-RiftPlayground/blob/master/src/main.c

The latter is using libusb, as it needs the variable length SPI data
control.

Do you think adding a pseudo i2c driver for the eSP570/eSP770u webcam
controllers and then exposing the sensor chips as V4L2 subdevices could
be a good idea? We already have a sensor driver for the MT9V034 in the
DK2 USB camera.

regards
Philipp

[PATCH] cec: drop senseless message

[Hans Verkuil]

Especially the '0.10' version number is confusing since CEC_ADAP_G_CAPS
returns a completely different version number.

Signed-off-by: Hans Verkuil 
---
 drivers/media/cec/cec-core.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index b516d599d6c4..57f171161db7 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -386,11 +386,8 @@ EXPORT_SYMBOL_GPL(cec_delete_adapter);
  */
 static int __init cec_devnode_init(void)
 {
-   int ret;
+   int ret = alloc_chrdev_region(_dev_t, 0, CEC_NUM_DEVICES, CEC_NAME);

-   pr_info("Linux cec interface: v0.10\n");
-   ret = alloc_chrdev_region(_dev_t, 0, CEC_NUM_DEVICES,
- CEC_NAME);
if (ret < 0) {
pr_warn("cec: unable to allocate major\n");
return ret;
-- 
2.11.0

Re: [PATCH 1/3] [media] uvcvideo: variable size controls

[Philipp Zabel]

Hi Laurent,

Am Samstag, den 15.07.2017, 12:49 +0300 schrieb Laurent Pinchart:
> Hi Philipp,
> 
> Thank you for the patch.

Thank you for the review.

> On Friday 14 Jul 2017 22:14:22 Philipp Zabel wrote:
> > Some USB webcam controllers have extension unit controls that report
> > different lengths via GET_LEN, depending on internal state.
> 
> If I ever need to hire a hardware designer, I'll make sure to reject any 
> candidate who thinks that creativity is an asset :-(

:)

> If the size changes, could the flags change as well ? Should you issue a 
> GET_INFO too ?

The size-changing eSP770U control always reports the same INFO.
Could this be added as a separate flag in the future if needed, when
the next webcam controller designer feels creative?

> > Add a flag to mark these controls as variable length and issue GET_LEN
> > before GET/SET_CUR transfers to verify the current length.
> 
> What happens if the internal state changes between the GET_LEN and the 
> GET/SET_CUR ?

At least for the Oculus Sensor, as far as I can tell, the length only
changes in reaction to a SET_CUR on another control:
Control 11 is written to set up an SPI transfer, apparently. That 16-
byte control contains a length field in byte 9. The length written to
that field becomes the LEN of control 12, which can then be used to
read or write data.

> > Signed-off-by: Philipp Zabel 
> > ---
> >  drivers/media/usb/uvc/uvc_ctrl.c | 26 +-
> >  include/uapi/linux/uvcvideo.h|  2 ++
> >  2 files changed, 27 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c
> > b/drivers/media/usb/uvc/uvc_ctrl.c index c2ee6e39fd0c..ce69e2c6937d 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -1597,7 +1597,7 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device
> > *dev, struct usb_device_id id;
> > > >     u8 entity;
> > > >     u8 selector;
> > > > -   u8 flags;
> > > > +   u16 flags;
> > > >     };
> > 
> > > >     static const struct uvc_ctrl_fixup fixups[] = {
> > @@ -1799,6 +1799,30 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
> > > >     goto done;
> > > >     }
> > 
> > > > +   if ((ctrl->info.flags & UVC_CTRL_FLAG_VARIABLE_LEN) && 
> > > > reqflags) {
> > > > +   data = kmalloc(2, GFP_KERNEL);
> > > > +   /* Check if the control length has changed */
> > > > +   ret = uvc_query_ctrl(chain->dev, UVC_GET_LEN, 
> > > > xqry->unit,
> > +    chain->dev->intfnum, xqry->selector, 
> 
> data,
> > > > +    2);
> > > > +   size = le16_to_cpup((__le16 *)data);
> > +   kfree(data);
> 
> Now data is not NULL.
> 
> > > > +   if (ret < 0) {
> > > > +   uvc_trace(UVC_TRACE_CONTROL,
> > > > +     "GET_LEN failed on control %pUl/%u 
> > > > (%d).\n",
> > > > +     entity->extension.guidExtensionCode,
> > > > +     xqry->selector, ret);
> > +   goto done;
> 
> And the kfree(data) at the done label will cause a double free.

Thanks, will fix that.

> > +   }
> > > > +   if (ctrl->info.size != size) {
> > > > +   uvc_trace(UVC_TRACE_CONTROL,
> > +     "XU control %pUl/%u queried: len %u -> 
> 
> %u\n",
> > > > +     entity->extension.guidExtensionCode,
> > > > +     xqry->selector, ctrl->info.size, 
> > > > size);
> > > > +   ctrl->info.size = size;
> > > > +   }
> > +   }
> 
> How about moving this code (or part of it at least) to a function that could 
> be shared with uvc_ctrl_fill_xu_info() ?

I'll try. This will come at the cost of a bit more boilerplate.

regards
Philipp

Re: [PATCH v7 2/3] media: i2c: adv748x: add adv748x driver

[Niklas Söderlund]

Hi Kieran,

On 2017-07-14 17:45:02 +0100, Kieran Bingham wrote:
> Hi Niklas
> 
> On 13/07/17 07:28, Niklas Söderlund wrote:
> > 
> > Hi Kieran,
> > 
> > Thanks for your hard work.
> 
> And thank you for your support!
> 
> > On 2017-07-06 12:01:16 +0100, Kieran Bingham wrote:
> >> From: Kieran Bingham 
> >>
> >> Provide support for the ADV7481 and ADV7482.
> >>
> >> The driver is modelled with 4 subdevices to allow simultaneous streaming
> >> from the AFE (Analog front end) and HDMI inputs though two CSI TX
> >> entities.
> >>
> >> The HDMI entity is linked to the TXA CSI bus, whilst the AFE is linked
> >> to the TXB CSI bus.
> >>
> >> The driver is based on a prototype by Koji Matsuoka in the Renesas BSP,
> >> and an earlier rework by Niklas Söderlund.
> >>
> >> Signed-off-by: Kieran Bingham 
> > 
> > Unfortunately I have hit a regression with v7. I use the latest R-Car 
> > Gen3 VIN and CSI-2 patches in conjunction with v6 and v7 of this series.
> > And wile running v4l2-compliance on a VIN which is connected to the CVBS 
> > input it fails, see output at the end.
> 
> Interesting, at first glance I cannot imagine what's breaking here...
> 
> > If i substitute the adv748x driver patch with v6 version v4l2-compliance 
> > works as expected. It also works as expected for the HDMI inputs using 
> > both v6 and v7 of this series. I have tried using both a PAL and NTSC 
> > source so I don't think that the removal of auto detection by the 
> > adv748x driver itself in v7 is to blame, but I have not verified this.
> 
> Is this to do with the change to V4L2_FIELD_ALTERNATE perhaps?

Yes ;-)

> 
> The subdevice will now have a different size which could affect buffer sizes 
> at
> the MMAP? (I'm inferring that the "test MMAP: FAIL" is the issue here)
> 
> Unfortunately - if this is the case - I would push the 'blame' (required 
> change)
> back up to the handling in the RcarVIN driver ... :S
> 
> I'm not trying to defer this back to you - but just thinking out loud :)

Well blame should be placed where it belong, and in this case that is 
with me :-)

> 
> It could be tested quite quickly by changing in adv748x_afe_fill_format()
> 
>  -fmt->field = V4L2_FIELD_ALTERNATE;
>  +fmt->field = V4L2_FIELD_INTERLACED; // (or such)
> 
> (and remember to comment out the fmt->height /= 2; for halving of the size)

Whit this change the regression is gone so the proper fix should be 
implemented in the VIN driver. Thanks for pointing this out!

> 
> 
> I'll see if I can reproduce this on Monday.
> 
> --
> Regards
> 
> Kieran
> 
> 
> > To ease replication let me point out the compliance.sh script in 
> > vin-tests which will replicate the MC setup before running the 
> > v4l2-compliance.
> > 
> > # v4l2-compliance -s -d /dev/video26
> > v4l2-compliance SHA   : [  359.935042] rcar-vin e6ef1000.video: 
> > =  START STATUS  =
> > d16a17abd1d8d788[  359.944098] rcar-vin e6ef1000.video: ==  
> > END STATUS  ==
> > 5ca2f44fb295035278baa89c
> > 
> > Driver Info:
> > Driver name   : rcar_vin
> > Card type : R_Car_VIN
> > Bus info  : platform:e6ef1000.video
> > Driver version: 4.12.0
> > Capabilities  : 0x8521
> > Video Capture
> > Read/Write
> > Streaming
> > Extended Pix Format
> > Device Capabilities
> > Device Caps   : 0x0521
> > Video Capture
> > Read/Write
> > Streaming
> > Extended Pix Format
> > 
> > Compliance test for device /dev/video26 (not using libv4l2):
> > 
> > Required ioctls:
> > test VIDIOC_QUERYCAP: OK
> > 
> > Allow for multiple opens:
> > test second video open: OK
> > test VIDIOC_QUERYCAP: OK
> > test VIDIOC_G/S_PRIORITY: OK
> > test for unlimited opens: OK
> > 
> > Debug ioctls:
> > test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
> > test VIDIOC_LOG_STATUS: OK
> > 
> > Input ioctls:
> > test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
> > test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
> > test VIDIOC_ENUMAUDIO: OK (Not Supported)
> > test VIDIOC_G/S/ENUMINPUT: OK
> > test VIDIOC_G/S_AUDIO: OK (Not Supported)
> > Inputs: 1 Audio Inputs: 0 Tuners: 0
> > 
> > Output ioctls:
> > test VIDIOC_G/S_MODULATOR: OK (Not Supported)
> > test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> > test VIDIOC_ENUMAUDOUT: OK (Not Supported)
> > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
> > test VIDIOC_G/S_AUDOUT: OK (Not Supported)
> > Outputs: 0 Audio Outputs: 0 Modulators: 0
> > 
> > Input/Output configuration ioctls:
> > test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
> > test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
> > test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
> 

Re: [PATCH v2 2/6] [media] rockchip/rga: v4l2 m2m support

[Personnel]

Le samedi 15 juillet 2017 à 12:42 +0300, Laurent Pinchart a écrit :
> Hi Jacob,
> 
> Thank you for the patch.
> 
> On Saturday 15 Jul 2017 14:58:36 Jacob Chen wrote:
> > Rockchip RGA is a separate 2D raster graphic acceleration unit. It
> > accelerates 2D graphics operations, such as point/line drawing, image
> > scaling, rotation, BitBLT, alpha blending and image blur/sharpness.
> > 
> > The drvier is mostly based on s5p-g2d v4l2 m2m driver.
> > And supports various operations from the rendering pipeline.
> >  - copy
> >  - fast solid color fill
> >  - rotation
> >  - flip
> >  - alpha blending
> 
> I notice that you don't support the drawing operations. How do you plan to 
> support them later through the V4L2 M2M API ? I hate stating the obvious, but 
> wouldn't the DRM API be better fit for a graphic accelerator ?

It could fit, maybe, but it really lacks some framework. Also, DRM is
not really meant for M2M operation, and it's also not great for multi-
process. Until recently, there was competing drivers for Exynos, both
implemented in V4L2 and DRM, for similar rational, all DRM ones are
being deprecated/removed.

I think 2D blitters in V4L2 are fine, but they terribly lack something
to differentiate them from converters/scalers when looking up the HW
list. Could be as simple as a capability flag, if I can suggest. For
the reference, the 2D blitter on IMX6 has been used to implement a live
video mixer in GStreamer.

https://bugzilla.gnome.org/show_bug.cgi?id=772766

> 
> Additionally, V4L2 M2M has one source and one destination. How do you 
> implement alpha blending in that case, which by definition requires at least 
> two sources ?

This type of HW only do in-place blits. When using such a node, the
buffer queued on the V4L2_CAPTURE contains the destination image, and
the buffer queued on the V4L2_OUTPUT is the source image.

> 
> > The code in rga-hw.c is used to configure regs accroding to operations.
> > 
> > The code in rga-buf.c is used to create private mmu table for RGA.
> > The tables is stored in a list, and be removed when buffer is cleanup.
> 
> Looking at the implementation it seems to be a scatter-gather list, not an 
> MMU. Is that right ? Does the hardware documentation refer to it as an MMU ?
> 
> > Signed-off-by: Jacob Chen 
> > ---
> >  drivers/media/platform/Kconfig|  11 +
> >  drivers/media/platform/Makefile   |   2 +
> >  drivers/media/platform/rockchip-rga/Makefile  |   3 +
> >  drivers/media/platform/rockchip-rga/rga-buf.c | 122 
> >  drivers/media/platform/rockchip-rga/rga-hw.c  | 652 ++
> >  drivers/media/platform/rockchip-rga/rga-hw.h  | 437 
> >  drivers/media/platform/rockchip-rga/rga.c | 958 +++
> >  drivers/media/platform/rockchip-rga/rga.h | 111 +++
> >  8 files changed, 2296 insertions(+)
> >  create mode 100644 drivers/media/platform/rockchip-rga/Makefile
> >  create mode 100644 drivers/media/platform/rockchip-rga/rga-buf.c
> >  create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.c
> >  create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.h
> >  create mode 100644 drivers/media/platform/rockchip-rga/rga.c
> >  create mode 100644 drivers/media/platform/rockchip-rga/rga.h
> 
> 

Re: [PATCH v2 2/6] [media] rockchip/rga: v4l2 m2m support

[kbuild test robot]

Hi Jacob,

[auto build test WARNING on rockchip/for-next]
[also build test WARNING on v4.12 next-20170714]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Jacob-Chen/v4l-add-blend-modes-controls/20170715-214326
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git 
for-next


coccinelle warnings: (new ones prefixed by >>)

>> drivers/media/platform/rockchip-rga/rga-hw.c:124:2-3: Unneeded semicolon

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

[PATCH] rockchip/rga: fix semicolon.cocci warnings

[kbuild test robot]

drivers/media/platform/rockchip-rga/rga-hw.c:124:2-3: Unneeded semicolon


 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

Fixes: 6f3d6aeab3cb ("rockchip/rga: v4l2 m2m support")
CC: Jacob Chen 
Signed-off-by: Fengguang Wu 
---

 rga-hw.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/media/platform/rockchip-rga/rga-hw.c
+++ b/drivers/media/platform/rockchip-rga/rga-hw.c
@@ -121,7 +121,7 @@ static struct rga_addr_offset *rga_looku
return >right_top;
case RB:
return >right_bottom;
-   };
+   }
 
return NULL;
 }

Re: [PATCH 00/14] ddbridge: bump to ddbridge-0.9.29

[Richard Scobie]

Daniel Scheller wrote:

From: Daniel Scheller 

Preferrably for Linux 4.14 (to get things done).

Hard-depends on the STV0910/STV6111 driver patchset as the diff and the
updated code depends on the driver and the changes involved with the
glue code of the STV/DDCineS2V7 series [1].

Mauro/Media maintainers, this updates drivers/media/pci/ddbridge to the
very latest code that DD carry in their vendor driver package as of
version 0.9.29, in the "once, the big-bang-way is ok" way as discussed at
[2] (compared to the incremental, awkward to do variant since that
involves dissecting all available release archives and having to - try
to - build proper commits out of this, which will always be inaccurate;
a start was done at [3], however - and please understand - I definitely
don't want to continue doing that...)


-snip

Posting another "tested by" for this patch series, in conjunction with 
the recently posted STV0910/STV6111 set that I've been testing longer term


Have been running this series, since it was posted here, several hours 
daily on a dedicated vdr based PVR with a Digital Devices Cine S2 V7A, 
kernel 4.12 and vdr 2.3.8.


MSI interrupts are enabled and no issues to date.

Thanks to Daniel and the reviewers.

Regards,

Richard

[PATCH] media: rc: nuvoton: remove rudimentary transmit functionality

[Heiner Kallweit]

Transmit support in this driver was never tested and based on the code
it can't work. Just one example:
The buffer provided to nvt_tx_ir holds unsigned int values in
micro seconds: First value is for a pulse, second for a pause, etc.
Bytes in this buffer are copied as-is to the chip FIFO what can't work
as the chip-internal format is totally different. See also conversion
done in nvt_process_rx_ir_data.

Even if we would try to fix this we have the issue that we can't test
it. There seems to be no device on the market using IR transmit with
one of the chips supported by this driver.

To facilitate maintenance of the driver I'd propose to remove the
rudimentary transmit support.

Signed-off-by: Heiner Kallweit 
---
 drivers/media/rc/nuvoton-cir.c | 114 ++---
 drivers/media/rc/nuvoton-cir.h |  24 -
 2 files changed, 3 insertions(+), 135 deletions(-)

diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c
index ec4b25bd..7651975b 100644
--- a/drivers/media/rc/nuvoton-cir.c
+++ b/drivers/media/rc/nuvoton-cir.c
@@ -727,70 +727,6 @@ static int nvt_ir_raw_set_wakeup_filter(struct rc_dev *dev,
return ret;
 }
 
-/*
- * nvt_tx_ir
- *
- * 1) clean TX fifo first (handled by AP)
- * 2) copy data from user space
- * 3) disable RX interrupts, enable TX interrupts: TTR & TFU
- * 4) send 9 packets to TX FIFO to open TTR
- * in interrupt_handler:
- * 5) send all data out
- * go back to write():
- * 6) disable TX interrupts, re-enable RX interupts
- *
- * The key problem of this function is user space data may larger than
- * driver's data buf length. So nvt_tx_ir() will only copy TX_BUF_LEN data to
- * buf, and keep current copied data buf num in cur_buf_num. But driver's buf
- * number may larger than TXFCONT (0xff). So in interrupt_handler, it has to
- * set TXFCONT as 0xff, until buf_count less than 0xff.
- */
-static int nvt_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned n)
-{
-   struct nvt_dev *nvt = dev->priv;
-   unsigned long flags;
-   unsigned int i;
-   u8 iren;
-   int ret;
-
-   spin_lock_irqsave(>lock, flags);
-
-   ret = min((unsigned)(TX_BUF_LEN / sizeof(unsigned)), n);
-   nvt->tx.buf_count = (ret * sizeof(unsigned));
-
-   memcpy(nvt->tx.buf, txbuf, nvt->tx.buf_count);
-
-   nvt->tx.cur_buf_num = 0;
-
-   /* save currently enabled interrupts */
-   iren = nvt_cir_reg_read(nvt, CIR_IREN);
-
-   /* now disable all interrupts, save TFU & TTR */
-   nvt_cir_reg_write(nvt, CIR_IREN_TFU | CIR_IREN_TTR, CIR_IREN);
-
-   nvt->tx.tx_state = ST_TX_REPLY;
-
-   nvt_cir_reg_write(nvt, CIR_FIFOCON_TX_TRIGGER_LEV_8 |
- CIR_FIFOCON_RXFIFOCLR, CIR_FIFOCON);
-
-   /* trigger TTR interrupt by writing out ones, (yes, it's ugly) */
-   for (i = 0; i < 9; i++)
-   nvt_cir_reg_write(nvt, 0x01, CIR_STXFIFO);
-
-   spin_unlock_irqrestore(>lock, flags);
-
-   wait_event(nvt->tx.queue, nvt->tx.tx_state == ST_TX_REQUEST);
-
-   spin_lock_irqsave(>lock, flags);
-   nvt->tx.tx_state = ST_TX_NONE;
-   spin_unlock_irqrestore(>lock, flags);
-
-   /* restore enabled interrupts to prior state */
-   nvt_cir_reg_write(nvt, iren, CIR_IREN);
-
-   return ret;
-}
-
 /* dump contents of the last rx buffer we got from the hw rx fifo */
 static void nvt_dump_rx_buf(struct nvt_dev *nvt)
 {
@@ -895,11 +831,6 @@ static void nvt_cir_log_irqs(u8 status, u8 iren)
   CIR_IRSTS_TFU | CIR_IRSTS_GH) ? " ?" : "");
 }
 
-static bool nvt_cir_tx_inactive(struct nvt_dev *nvt)
-{
-   return nvt->tx.tx_state == ST_TX_NONE;
-}
-
 /* interrupt service routine for incoming and outgoing CIR data */
 static irqreturn_t nvt_cir_isr(int irq, void *data)
 {
@@ -952,40 +883,8 @@ static irqreturn_t nvt_cir_isr(int irq, void *data)
 
if (status & CIR_IRSTS_RFO)
nvt_handle_rx_fifo_overrun(nvt);
-
-   else if (status & (CIR_IRSTS_RTR | CIR_IRSTS_PE)) {
-   /* We only do rx if not tx'ing */
-   if (nvt_cir_tx_inactive(nvt))
-   nvt_get_rx_ir_data(nvt);
-   }
-
-   if (status & CIR_IRSTS_TE)
-   nvt_clear_tx_fifo(nvt);
-
-   if (status & CIR_IRSTS_TTR) {
-   unsigned int pos, count;
-   u8 tmp;
-
-   pos = nvt->tx.cur_buf_num;
-   count = nvt->tx.buf_count;
-
-   /* Write data into the hardware tx fifo while pos < count */
-   if (pos < count) {
-   nvt_cir_reg_write(nvt, nvt->tx.buf[pos], CIR_STXFIFO);
-   nvt->tx.cur_buf_num++;
-   /* Disable TX FIFO Trigger Level Reach (TTR) interrupt */
-   } else {
-   tmp = nvt_cir_reg_read(nvt, CIR_IREN);
-   nvt_cir_reg_write(nvt, tmp & ~CIR_IREN_TTR, CIR_IREN);
-   }
-   }
-
-   if 

Re: [PATCH 1/3] [media] uvcvideo: variable size controls (fwd)

[Julia Lawall]

There is a double free on data if the goto is taken on line 1815.

julia

-- Forwarded message --
Date: Sat, 15 Jul 2017 21:07:03 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH 1/3] [media] uvcvideo: variable size controls

CC: kbuild-...@01.org
In-Reply-To: <20170714201424.23592-1-philipp.za...@gmail.com>
TO: Philipp Zabel <philipp.za...@gmail.com>
CC: linux-media@vger.kernel.org, Laurent Pinchart 
<laurent.pinch...@ideasonboard.com>, Philipp Zabel <philipp.za...@gmail.com>
CC: Laurent Pinchart <laurent.pinch...@ideasonboard.com>, Philipp Zabel 
<philipp.za...@gmail.com>

Hi Philipp,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.12 next-20170714]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Philipp-Zabel/uvcvideo-variable-size-controls/20170715-193137
base:   git://linuxtv.org/media_tree.git master
:: branch date: 2 hours ago
:: commit date: 2 hours ago

>> drivers/media/usb/uvc/uvc_ctrl.c:1857:7-11: ERROR: reference preceded by 
>> free on line 1809

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout f06e94cde314fba5859cd6c999dde48e94156c7a
vim +1857 drivers/media/usb/uvc/uvc_ctrl.c

52c58ad6f drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29  1719
8e113595e drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2009-07-01  1720  
int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1721  
struct uvc_xu_control_query *xqry)
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1722  
{
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1723  
struct uvc_entity *entity;
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1724  
struct uvc_control *ctrl;
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1725  
unsigned int i, found = 0;
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1726  
__u32 reqflags;
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1727  
__u16 size;
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1728  
__u8 *data = NULL;
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1729  
int ret;
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1730
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1731  
/* Find the extension unit. */
6241d8ca1 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2009-11-25  1732  
list_for_each_entry(entity, >entities, chain) {
6241d8ca1 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2009-11-25  1733  
if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT &&
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1734  
entity->id == xqry->unit)
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1735  
break;
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1736  
}
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1737
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1738  
if (entity->id != xqry->unit) {
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1739  
uvc_trace(UVC_TRACE_CONTROL, "Extension unit %u not found.\n",
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1740  
xqry->unit);
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1741  
return -ENOENT;
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1742  
}
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1743
52c58ad6f drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29  1744  
/* Find the control and perform delayed initialization if needed. */
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1745  
for (i = 0; i < entity->ncontrols; ++i) {
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1746  
ctrl = >controls[i];
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1747  
if (ctrl->index == xqry->selector - 1) {
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1748  
found = 1;
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent 

[PATCH] pulse8-cec/rainshadow-cec: make adapter name unique

[Hans Verkuil]

The CEC adapter name used by the pulse8-cec and rainshadow-cec USB device 
drivers
was a fixed string, but it should be unique if you connect multiple of these 
devices
to the same computer.

Use dev_name(>dev) instead, which make it unique again.

Signed-off-by: Hans Verkuil 
---
diff --git a/drivers/media/usb/pulse8-cec/pulse8-cec.c 
b/drivers/media/usb/pulse8-cec/pulse8-cec.c
index c843070f24c1..e29d43237046 100644
--- a/drivers/media/usb/pulse8-cec/pulse8-cec.c
+++ b/drivers/media/usb/pulse8-cec/pulse8-cec.c
@@ -656,7 +656,7 @@ static int pulse8_connect(struct serio *serio, struct 
serio_driver *drv)

pulse8->serio = serio;
pulse8->adap = cec_allocate_adapter(_cec_adap_ops, pulse8,
-   "HDMI CEC", caps, 1);
+   dev_name(>dev), caps, 1);
err = PTR_ERR_OR_ZERO(pulse8->adap);
if (err < 0)
goto free_device;
diff --git a/drivers/media/usb/rainshadow-cec/rainshadow-cec.c 
b/drivers/media/usb/rainshadow-cec/rainshadow-cec.c
index f203699e9c1b..7e80eefdf604 100644
--- a/drivers/media/usb/rainshadow-cec/rainshadow-cec.c
+++ b/drivers/media/usb/rainshadow-cec/rainshadow-cec.c
@@ -325,7 +325,7 @@ static int rain_connect(struct serio *serio, struct 
serio_driver *drv)

rain->serio = serio;
rain->adap = cec_allocate_adapter(_cec_adap_ops, rain,
-   "HDMI CEC", caps, 1);
+ dev_name(>dev), caps, 1);
err = PTR_ERR_OR_ZERO(rain->adap);
if (err < 0)
goto free_device;

cron job: media_tree daily build: ERRORS

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Sun Jul 16 05:00:20 CEST 2017
media-tree git hash:2748e76ddb2967c4030171342ebdd3faa6a5e8e8
media_build git hash:   bc1db0a204a87da86349ea5e64ae0d65e945609d
v4l-utils git hash: 8e68406dae2233e811032dc8e7714c09c818e893
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.9.0-164

linux-git-arm-at91: WARNINGS
linux-git-arm-davinci: WARNINGS
linux-git-arm-multi: WARNINGS
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: WARNINGS
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: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: OK
linux-3.12.67-i686: OK
linux-3.13.11-i686: WARNINGS
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: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
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-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: ERRORS
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: WARNINGS
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: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
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
apps: WARNINGS
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH v2 5/6] ARM: dts: rockchip: enable RGA for rk3288 devices

[Jacob Chen]

Hi Laurent,

2017-07-15 17:16 GMT+08:00 Laurent Pinchart :
> Hi Jacob,
>
> Thank you for the patch.
>
> On Saturday 15 Jul 2017 14:58:39 Jacob Chen wrote:
>> Signed-off-by: Jacob Chen 
>> ---
>>  arch/arm/boot/dts/rk3288-evb.dtsi | 4 
>>  arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi | 4 
>>  arch/arm/boot/dts/rk3288-firefly.dtsi | 4 
>>  arch/arm/boot/dts/rk3288-miqi.dts | 4 
>>  arch/arm/boot/dts/rk3288-popmetal.dts | 4 
>>  arch/arm/boot/dts/rk3288-tinker.dts   | 4 
>
> Some boards are missing from this list (Fennec, Phycore, ...) What criteria
> have you used to decide on which ones to enable the RGA ? That should be
> explained in the commit message.
>

Ok.

I just enable the boards i have tested, because i can't make sure it
won't break the other board
because of clocks or power-domains.

>>  6 files changed, 24 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/rk3288-evb.dtsi
>> b/arch/arm/boot/dts/rk3288-evb.dtsi index 4905760..ec12162 100644
>> --- a/arch/arm/boot/dts/rk3288-evb.dtsi
>> +++ b/arch/arm/boot/dts/rk3288-evb.dtsi
>> @@ -379,6 +379,10 @@
>>   };
>>  };
>>
>> + {
>> + status = "okay";
>> +};
>> +
>>   {
>>   status = "okay";
>>  };
>> diff --git a/arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi
>> b/arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi index 8134966..fffa92e2
>> 100644
>> --- a/arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi
>> +++ b/arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi
>> @@ -283,6 +283,10 @@
>>   };
>>  };
>>
>> + {
>> + status = "okay";
>> +};
>> +
>>   {
>>   rockchip,hw-tshut-mode = <0>;
>>   rockchip,hw-tshut-polarity = <0>;
>> diff --git a/arch/arm/boot/dts/rk3288-firefly.dtsi
>> b/arch/arm/boot/dts/rk3288-firefly.dtsi index f520589..74a6ce5 100644
>> --- a/arch/arm/boot/dts/rk3288-firefly.dtsi
>> +++ b/arch/arm/boot/dts/rk3288-firefly.dtsi
>> @@ -500,6 +500,10 @@
>>   };
>>  };
>>
>> + {
>> + status = "okay";
>> +};
>> +
>>   {
>>   vref-supply = <_18>;
>>   status = "okay";
>> diff --git a/arch/arm/boot/dts/rk3288-miqi.dts
>> b/arch/arm/boot/dts/rk3288-miqi.dts index 21326f3..dc5e6bd 100644
>> --- a/arch/arm/boot/dts/rk3288-miqi.dts
>> +++ b/arch/arm/boot/dts/rk3288-miqi.dts
>> @@ -401,6 +401,10 @@
>>   };
>>  };
>>
>> + {
>> + status = "okay";
>> +};
>> +
>>   {
>>   vref-supply = <_18>;
>>   status = "okay";
>> diff --git a/arch/arm/boot/dts/rk3288-popmetal.dts
>> b/arch/arm/boot/dts/rk3288-popmetal.dts index aa1f9ec..362e5aa 100644
>> --- a/arch/arm/boot/dts/rk3288-popmetal.dts
>> +++ b/arch/arm/boot/dts/rk3288-popmetal.dts
>> @@ -490,6 +490,10 @@
>>   };
>>  };
>>
>> + {
>> + status = "okay";
>> +};
>> +
>>   {
>>   rockchip,hw-tshut-mode = <0>;
>>   rockchip,hw-tshut-polarity = <0>;
>> diff --git a/arch/arm/boot/dts/rk3288-tinker.dts
>> b/arch/arm/boot/dts/rk3288-tinker.dts index 525b0e5..1a8c149 100644
>> --- a/arch/arm/boot/dts/rk3288-tinker.dts
>> +++ b/arch/arm/boot/dts/rk3288-tinker.dts
>> @@ -460,6 +460,10 @@
>>   status = "okay";
>>  };
>>
>> + {
>> + status = "okay";
>> +};
>> +
>>   {
>>   vref-supply = <_ldo1>;
>>   status ="okay";
>
> --
> Regards,
>
> Laurent Pinchart
>

[PATCH V3 10/16] [media] dvb-core/dvb_ca_en50221.c: Fixed C++ comments

[Jasmin J.]

From: Jasmin Jessich 

- Changed all C++ style comments ("// ..") to C style ones ("/* .. */").

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 6547f6e..99cf460 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -263,7 +263,7 @@ static int dvb_ca_en50221_check_camstatus(struct 
dvb_ca_private *ca, int slot)
} else {
if ((sl->slot_state == DVB_CA_SLOTSTATE_WAITREADY) &&
(slot_status & DVB_CA_EN50221_POLL_CAM_READY)) {
-   // move to validate state if reset is completed
+   /* move to validate state if reset is completed */
sl->slot_state = DVB_CA_SLOTSTATE_VALIDATE;
}
}
@@ -442,7 +442,7 @@ static int dvb_ca_en50221_read_tuple(struct dvb_ca_private 
*ca, int slot,
}
_address += (_tuple_length * 2);
 
-   // success
+   /* success */
*tuple_type = _tuple_type;
*tuple_length = _tuple_length;
*address = _address;
@@ -476,7 +476,7 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
u16 devid = 0;
 
 
-   // CISTPL_DEVICE_0A
+   /* CISTPL_DEVICE_0A */
status = dvb_ca_en50221_read_tuple(ca, slot, , _type,
   _length, tuple);
if (status < 0)
@@ -486,7 +486,7 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
 
 
 
-   // CISTPL_DEVICE_0C
+   /* CISTPL_DEVICE_0C */
status = dvb_ca_en50221_read_tuple(ca, slot, , _type,
   _length, tuple);
if (status < 0)
@@ -496,7 +496,7 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
 
 
 
-   // CISTPL_VERS_1
+   /* CISTPL_VERS_1 */
status = dvb_ca_en50221_read_tuple(ca, slot, , _type,
   _length, tuple);
if (status < 0)
@@ -506,7 +506,7 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
 
 
 
-   // CISTPL_MANFID
+   /* CISTPL_MANFID */
status = dvb_ca_en50221_read_tuple(ca, slot, , _type,
   _length, tuple);
if (status < 0)
@@ -520,7 +520,7 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
 
 
 
-   // CISTPL_CONFIG
+   /* CISTPL_CONFIG */
status = dvb_ca_en50221_read_tuple(ca, slot, , _type,
   _length, tuple);
if (status < 0)
@@ -562,7 +562,7 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
if (status < 0)
return status;
switch (tuple_type) {
-   case 0x1B:  // CISTPL_CFTABLE_ENTRY
+   case 0x1B:  /* CISTPL_CFTABLE_ENTRY */
if (tuple_length < (2 + 11 + 17))
break;
 
@@ -583,10 +583,10 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
got_cftableentry = 1;
break;
 
-   case 0x14:  // CISTPL_NO_LINK
+   case 0x14:  /* CISTPL_NO_LINK */
break;
 
-   case 0xFF:  // CISTPL_END
+   case 0xFF:  /* CISTPL_END */
end_chain = 1;
break;
 
@@ -603,7 +603,7 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
dprintk("Valid DVB CAM detected MANID:%x DEVID:%x CONFIGBASE:0x%x 
CONFIGOPTION:0x%x\n",
manfid, devid, sl->config_base, sl->config_option);
 
-   // success!
+   /* success! */
return 0;
 }
 
-- 
2.7.4

[PATCH V3 09/16] [media] dvb-core/dvb_ca_en50221.c: Removed unused symbol

[Jasmin J.]

From: Jasmin Jessich 

- The STATUSREG_TXERR definition is not used and it has style
  problems, too.  Removing it seems to solve both issues.

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index ce22bdb..6547f6e 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -76,8 +76,6 @@ MODULE_PARM_DESC(cam_debug, "enable verbose debug messages");
 #define STATUSREG_WE 2 /* write error */
 #define STATUSREG_FR  0x40 /* module free */
 #define STATUSREG_DA  0x80 /* data available */
-#define STATUSREG_TXERR (STATUSREG_RE|STATUSREG_WE)/* general transfer 
error */
-
 
 #define DVB_CA_SLOTSTATE_NONE   0
 #define DVB_CA_SLOTSTATE_UNINITIALISED  1
-- 
2.7.4

[PATCH V3 14/16] [media] dvb-core/dvb_ca_en50221.c: Fixed remaining block comments

[Jasmin J.]

From: Jasmin Jessich 

- Added the missing opening empty comment line.

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 2619822..24e2b0c 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1071,7 +1071,8 @@ static void dvb_ca_en50221_thread_update_delay(struct 
dvb_ca_private *ca)
int curdelay = 1;
int slot;
 
-   /* Beware of too high polling frequency, because one polling
+   /*
+* Beware of too high polling frequency, because one polling
 * call might take several hundred milliseconds until timeout!
 */
for (slot = 0; slot < ca->slot_count; slot++) {
-- 
2.7.4

[PATCH V3 08/16] [media] dvb-core/dvb_ca_en50221.c: Removed useless braces

[Jasmin J.]

From: Jasmin Jessich 

Fixed all:
  WARNING: braces {} are not necessary for single statement blocks

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 30 --
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 678bd6a..ce22bdb 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -242,9 +242,8 @@ static int dvb_ca_en50221_check_camstatus(struct 
dvb_ca_private *ca, int slot)
int cam_changed;
 
/* IRQ mode */
-   if (ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE) {
+   if (ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE)
return (atomic_read(>camchange_count) != 0);
-   }
 
/* poll mode */
slot_status = ca->pub->poll_slot_status(ca->pub, slot, ca->open);
@@ -258,11 +257,10 @@ static int dvb_ca_en50221_check_camstatus(struct 
dvb_ca_private *ca, int slot)
}
 
if (cam_changed) {
-   if (!cam_present_now) {
+   if (!cam_present_now)
sl->camchange_type = DVB_CA_EN50221_CAMCHANGE_REMOVED;
-   } else {
+   else
sl->camchange_type = DVB_CA_EN50221_CAMCHANGE_INSERTED;
-   }
atomic_set(>camchange_count, 1);
} else {
if ((sl->slot_state == DVB_CA_SLOTSTATE_WAITREADY) &&
@@ -314,9 +312,8 @@ static int dvb_ca_en50221_wait_if_status(struct 
dvb_ca_private *ca, int slot,
}
 
/* check for timeout */
-   if (time_after(jiffies, timeout)) {
+   if (time_after(jiffies, timeout))
break;
-   }
 
/* wait for a bit */
usleep_range(1000, 1100);
@@ -786,9 +783,9 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private 
*ca, int slot,
buf[0], (buf[1] & 0x80) == 0, bytes_read);
 
/* wake up readers when a last_fragment is received */
-   if ((buf[1] & 0x80) == 0x00) {
+   if ((buf[1] & 0x80) == 0x00)
wake_up_interruptible(>wait_queue);
-   }
+
status = bytes_read;
 
 exit:
@@ -1676,11 +1673,10 @@ static ssize_t dvb_ca_en50221_io_read(struct file 
*file, char __user *buf,
connection_id = hdr[0];
if (hdr[0] == connection_id) {
if (pktlen < count) {
-   if ((pktlen + fraglen - 2) > count) {
+   if ((pktlen + fraglen - 2) > count)
fraglen = count - pktlen;
-   } else {
+   else
fraglen -= 2;
-   }
 
status =
   dvb_ringbuffer_pkt_read_user(>rx_buffer,
@@ -1818,9 +1814,8 @@ static unsigned int dvb_ca_en50221_io_poll(struct file 
*file, poll_table *wait)
 
dprintk("%s\n", __func__);
 
-   if (dvb_ca_en50221_io_read_condition(ca, , ) == 1) {
+   if (dvb_ca_en50221_io_read_condition(ca, , ) == 1)
mask |= POLLIN;
-   }
 
/* if there is something, return now */
if (mask)
@@ -1829,9 +1824,8 @@ static unsigned int dvb_ca_en50221_io_poll(struct file 
*file, poll_table *wait)
/* wait for something to happen */
poll_wait(file, >wait_queue, wait);
 
-   if (dvb_ca_en50221_io_read_condition(ca, , ) == 1) {
+   if (dvb_ca_en50221_io_read_condition(ca, , ) == 1)
mask |= POLLIN;
-   }
 
return mask;
 }
@@ -1973,9 +1967,9 @@ void dvb_ca_en50221_release(struct dvb_ca_en50221 *pubca)
/* shutdown the thread if there was one */
kthread_stop(ca->thread);
 
-   for (i = 0; i < ca->slot_count; i++) {
+   for (i = 0; i < ca->slot_count; i++)
dvb_ca_en50221_slot_shutdown(ca, i);
-   }
+
dvb_remove_device(ca->dvbdev);
dvb_ca_private_put(ca);
pubca->private = NULL;
-- 
2.7.4

[PATCH V3 04/16] [media] dvb-core/dvb_ca_en50221.c: Fixed block comments

[Jasmin J.]

From: Jasmin Jessich 

Fixed all:
  WARNING: Block comments use * on subsequent lines
Added also the missing opening empty comment line.

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 29 +++--
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 725fdd0..28e6102 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -342,8 +342,10 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private 
*ca, int slot)
/* we'll be determining these during this function */
ca->slot_info[slot].da_irq_supported = 0;
 
-   /* set the host link buffer size temporarily. it will be overwritten 
with the
-* real negotiated size later. */
+   /*
+* set the host link buffer size temporarily. it will be overwritten
+* with the real negotiated size later.
+*/
ca->slot_info[slot].link_buf_size = 2;
 
/* read the buffer size from the CAM */
@@ -796,10 +798,12 @@ static int dvb_ca_en50221_write_data(struct 
dvb_ca_private *ca, int slot,
(ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_LINKINIT))
return ca->pub->write_data(ca->pub, slot, buf, bytes_write);
 
-   /* it is possible we are dealing with a single buffer implementation,
-  thus if there is data available for read or if there is even a read
-  already in progress, we do nothing but awake the kernel thread to
-  process the data if necessary. */
+   /*
+* it is possible we are dealing with a single buffer implementation,
+* thus if there is data available for read or if there is even a read
+* already in progress, we do nothing but awake the kernel thread to
+* process the data if necessary.
+*/
if ((status = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS)) 
< 0)
goto exitnowrite;
if (status & (STATUSREG_DA | STATUSREG_RE)) {
@@ -899,8 +903,10 @@ static int dvb_ca_en50221_slot_shutdown(struct 
dvb_ca_private *ca, int slot)
ca->pub->slot_shutdown(ca->pub, slot);
ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_NONE;
 
-   /* need to wake up all processes to check if they're now
-  trying to write to a defunct CAM */
+   /*
+* need to wake up all processes to check if they're now trying to
+* write to a defunct CAM
+*/
wake_up_interruptible(>wait_queue);
 
dprintk("Slot %i shutdown\n", slot);
@@ -1686,8 +1692,11 @@ static int dvb_ca_en50221_io_open(struct inode *inode, 
struct file *file)
 
if (ca->slot_info[i].slot_state == DVB_CA_SLOTSTATE_RUNNING) {
if (ca->slot_info[i].rx_buffer.data != NULL) {
-   /* it is safe to call this here without locks 
because
-* ca->open == 0. Data is not read in this case 
*/
+   /*
+* it is safe to call this here without locks
+* because ca->open == 0. Data is not read in
+* this case
+*/

dvb_ringbuffer_flush(>slot_info[i].rx_buffer);
}
}
-- 
2.7.4

[PATCH V3 06/16] [media] dvb-core/dvb_ca_en50221.c: Used a helper variable

[Jasmin J.]

From: Jasmin Jessich 

Used a helper variable "struct dvb_ca_slot *sl" instead of
"ca->slot_info[slot]". This reduces the line length and simplifies
code reading.

Fixed also "-strict" checks in this patch:
- Comparison to NULL written as "!".

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 187 ++--
 1 file changed, 106 insertions(+), 81 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index d06cdc7..7207ff5 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -234,13 +234,14 @@ static char *findstr(char *haystack, int hlen, char 
*needle, int nlen)
  */
 static int dvb_ca_en50221_check_camstatus(struct dvb_ca_private *ca, int slot)
 {
+   struct dvb_ca_slot *sl = >slot_info[slot];
int slot_status;
int cam_present_now;
int cam_changed;
 
/* IRQ mode */
if (ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE) {
-   return (atomic_read(>slot_info[slot].camchange_count) != 0);
+   return (atomic_read(>camchange_count) != 0);
}
 
/* poll mode */
@@ -249,22 +250,23 @@ static int dvb_ca_en50221_check_camstatus(struct 
dvb_ca_private *ca, int slot)
cam_present_now = (slot_status & DVB_CA_EN50221_POLL_CAM_PRESENT) ? 1 : 
0;
cam_changed = (slot_status & DVB_CA_EN50221_POLL_CAM_CHANGED) ? 1 : 0;
if (!cam_changed) {
-   int cam_present_old = (ca->slot_info[slot].slot_state != 
DVB_CA_SLOTSTATE_NONE);
+   int cam_present_old = (sl->slot_state != DVB_CA_SLOTSTATE_NONE);
+
cam_changed = (cam_present_now != cam_present_old);
}
 
if (cam_changed) {
if (!cam_present_now) {
-   ca->slot_info[slot].camchange_type = 
DVB_CA_EN50221_CAMCHANGE_REMOVED;
+   sl->camchange_type = DVB_CA_EN50221_CAMCHANGE_REMOVED;
} else {
-   ca->slot_info[slot].camchange_type = 
DVB_CA_EN50221_CAMCHANGE_INSERTED;
+   sl->camchange_type = DVB_CA_EN50221_CAMCHANGE_INSERTED;
}
-   atomic_set(>slot_info[slot].camchange_count, 1);
+   atomic_set(>camchange_count, 1);
} else {
-   if ((ca->slot_info[slot].slot_state == 
DVB_CA_SLOTSTATE_WAITREADY) &&
+   if ((sl->slot_state == DVB_CA_SLOTSTATE_WAITREADY) &&
(slot_status & DVB_CA_EN50221_POLL_CAM_READY)) {
// move to validate state if reset is completed
-   ca->slot_info[slot].slot_state = 
DVB_CA_SLOTSTATE_VALIDATE;
+   sl->slot_state = DVB_CA_SLOTSTATE_VALIDATE;
}
}
 
@@ -333,6 +335,7 @@ static int dvb_ca_en50221_wait_if_status(struct 
dvb_ca_private *ca, int slot,
  */
 static int dvb_ca_en50221_link_init(struct dvb_ca_private *ca, int slot)
 {
+   struct dvb_ca_slot *sl = >slot_info[slot];
int ret;
int buf_size;
u8 buf[2];
@@ -340,13 +343,13 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private 
*ca, int slot)
dprintk("%s\n", __func__);
 
/* we'll be determining these during this function */
-   ca->slot_info[slot].da_irq_supported = 0;
+   sl->da_irq_supported = 0;
 
/*
 * set the host link buffer size temporarily. it will be overwritten
 * with the real negotiated size later.
 */
-   ca->slot_info[slot].link_buf_size = 2;
+   sl->link_buf_size = 2;
 
/* read the buffer size from the CAM */
ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND,
@@ -367,7 +370,7 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private 
*ca, int slot)
buf_size = (buf[0] << 8) | buf[1];
if (buf_size > HOST_LINK_BUF_SIZE)
buf_size = HOST_LINK_BUF_SIZE;
-   ca->slot_info[slot].link_buf_size = buf_size;
+   sl->link_buf_size = buf_size;
buf[0] = buf_size >> 8;
buf[1] = buf_size & 0xff;
dprintk("Chosen link buffer size of %i\n", buf_size);
@@ -459,6 +462,7 @@ static int dvb_ca_en50221_read_tuple(struct dvb_ca_private 
*ca, int slot,
  */
 static int dvb_ca_en50221_parse_attributes(struct dvb_ca_private *ca, int slot)
 {
+   struct dvb_ca_slot *sl;
int address = 0;
int tuple_length;
int tuple_type;
@@ -531,10 +535,10 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
rasz = tuple[0] & 3;
if (tuple_length < (3 + rasz + 14))
return -EINVAL;
-   ca->slot_info[slot].config_base = 0;
-   for (i = 0; i < rasz + 1; i++) {
-   ca->slot_info[slot].config_base |= (tuple[2 + i] << (8 * i));
-   }
+   sl = >slot_info[slot];
+   sl->config_base = 0;
+   for (i = 0; i < rasz + 1; i++)

[PATCH V3 02/16] [media] dvb-core/dvb_ca_en50221.c: New function dvb_ca_en50221_poll_cam_gone

[Jasmin J.]

From: Jasmin Jessich 

The CAM poll code for the budget-av is exactly the same on several
places. Extracting the code to a new function improves maintainability.

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 66 +
 1 file changed, 35 insertions(+), 31 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index e2f35b7..bb6aa0f 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1064,6 +1064,37 @@ static void dvb_ca_en50221_thread_update_delay(struct 
dvb_ca_private *ca)
 }
 
 /**
+ * Poll if the CAM is gone.
+ *
+ * @ca: CA instance.
+ * @slot: Slot to process.
+ * @return: 0 .. no change
+ *  1 .. CAM state changed
+ */
+
+static int dvb_ca_en50221_poll_cam_gone(struct dvb_ca_private *ca, int slot)
+{
+   int changed = 0;
+   int status;
+
+   /*
+* we need this extra check for annoying interfaces like the
+* budget-av
+*/
+   if ((!(ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE)) &&
+   (ca->pub->poll_slot_status)) {
+   status = ca->pub->poll_slot_status(ca->pub, slot, 0);
+   if (!(status &
+   DVB_CA_EN50221_POLL_CAM_PRESENT)) {
+   ca->slot_info[slot].slot_state = DVB_CA_SLOTSTATE_NONE;
+   dvb_ca_en50221_thread_update_delay(ca);
+   changed = 1;
+   }
+   }
+   return changed;
+}
+
+/**
  * Thread state machine for one CA slot to perform the data transfer.
  *
  * @ca: CA instance.
@@ -1074,7 +1105,6 @@ static void dvb_ca_en50221_thread_state_machine(struct 
dvb_ca_private *ca,
 {
struct dvb_ca_slot *sl = >slot_info[slot];
int flags;
-   int status;
int pktcount;
void *rxbuf;
 
@@ -1124,21 +1154,8 @@ static void dvb_ca_en50221_thread_state_machine(struct 
dvb_ca_private *ca,
 
case DVB_CA_SLOTSTATE_VALIDATE:
if (dvb_ca_en50221_parse_attributes(ca, slot) != 0) {
-   /*
-* we need this extra check for annoying interfaces like
-* the budget-av
-*/
-   if ((!(ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE))
-   && (ca->pub->poll_slot_status)) {
-   status = ca->pub->poll_slot_status(ca->pub,
-  slot, 0);
-   if (!(status &
- DVB_CA_EN50221_POLL_CAM_PRESENT)) {
-   sl->slot_state = DVB_CA_SLOTSTATE_NONE;
-   dvb_ca_en50221_thread_update_delay(ca);
-   break;
-   }
-   }
+   if (dvb_ca_en50221_poll_cam_gone(ca, slot))
+   break;
 
pr_err("dvb_ca adapter %d: Invalid PC card inserted 
:(\n",
   ca->dvbdev->adapter->num);
@@ -1187,21 +1204,8 @@ static void dvb_ca_en50221_thread_state_machine(struct 
dvb_ca_private *ca,
 
case DVB_CA_SLOTSTATE_LINKINIT:
if (dvb_ca_en50221_link_init(ca, slot) != 0) {
-   /*
-* we need this extra check for annoying interfaces like
-* the budget-av
-*/
-   if ((!(ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE))
-   && (ca->pub->poll_slot_status)) {
-   status = ca->pub->poll_slot_status(ca->pub,
-  slot, 0);
-   if (!(status &
-   DVB_CA_EN50221_POLL_CAM_PRESENT)) {
-   sl->slot_state = DVB_CA_SLOTSTATE_NONE;
-   dvb_ca_en50221_thread_update_delay(ca);
-   break;
-   }
-   }
+   if (dvb_ca_en50221_poll_cam_gone(ca, slot))
+   break;
 
pr_err("dvb_ca adapter %d: DVB CAM link initialisation 
failed :(\n",
   ca->dvbdev->adapter->num);
-- 
2.7.4

[PATCH V3 03/16] [media] dvb-core/dvb_ca_en50221.c: use usleep_range

[Jasmin J.]

From: Jasmin Jessich 

Fixed all:
  WARNING: msleep < 20ms can sleep for up to 20ms
by using usleep_range.

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index bb6aa0f..725fdd0 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -313,7 +313,7 @@ static int dvb_ca_en50221_wait_if_status(struct 
dvb_ca_private *ca, int slot,
}
 
/* wait for a bit */
-   msleep(1);
+   usleep_range(1000, 1100);
}
 
dprintk("%s failed timeout:%lu\n", __func__, jiffies - start);
@@ -1489,7 +1489,7 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
if (status != -EAGAIN)
goto exit;
 
-   msleep(1);
+   usleep_range(1000, 1100);
}
if (!written) {
status = -EIO;
-- 
2.7.4

[PATCH V3 05/16] [media] dvb-core/dvb_ca_en50221.c: Avoid assignments in ifs

[Jasmin J.]

From: Jasmin Jessich 

Fixed all:
  ERROR: do not use assignment in if condition

Fixed also "-strict" checks in this patch:
- Changed "if (ret != 0)" to "if (ret)".
- Camel case variables have been converted to kernel_case.
- Comparison to NULL written as "!".
- No space is necessary after a cast.

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 166 +++-
 1 file changed, 100 insertions(+), 66 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 28e6102..d06cdc7 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -349,14 +349,18 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private 
*ca, int slot)
ca->slot_info[slot].link_buf_size = 2;
 
/* read the buffer size from the CAM */
-   if ((ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, 
IRQEN | CMDREG_SR)) != 0)
+   ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND,
+IRQEN | CMDREG_SR);
+   if (ret)
return ret;
ret = dvb_ca_en50221_wait_if_status(ca, slot, STATUSREG_DA, HZ);
-   if (ret != 0)
+   if (ret)
return ret;
-   if ((ret = dvb_ca_en50221_read_data(ca, slot, buf, 2)) != 2)
+   ret = dvb_ca_en50221_read_data(ca, slot, buf, 2);
+   if (ret != 2)
return -EIO;
-   if ((ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, 
IRQEN)) != 0)
+   ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, IRQEN);
+   if (ret)
return ret;
 
/* store it, and choose the minimum of our buffer and the CAM's buffer 
size */
@@ -369,13 +373,18 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private 
*ca, int slot)
dprintk("Chosen link buffer size of %i\n", buf_size);
 
/* write the buffer size to the CAM */
-   if ((ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, 
IRQEN | CMDREG_SW)) != 0)
+   ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND,
+IRQEN | CMDREG_SW);
+   if (ret)
return ret;
-   if ((ret = dvb_ca_en50221_wait_if_status(ca, slot, STATUSREG_FR, HZ / 
10)) != 0)
+   ret = dvb_ca_en50221_wait_if_status(ca, slot, STATUSREG_FR, HZ / 10);
+   if (ret)
return ret;
-   if ((ret = dvb_ca_en50221_write_data(ca, slot, buf, 2)) != 2)
+   ret = dvb_ca_en50221_write_data(ca, slot, buf, 2);
+   if (ret != 2)
return -EIO;
-   if ((ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, 
IRQEN)) != 0)
+   ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, IRQEN);
+   if (ret)
return ret;
 
/* success */
@@ -395,42 +404,45 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private 
*ca, int slot)
  * @return 0 on success, nonzero on error.
  */
 static int dvb_ca_en50221_read_tuple(struct dvb_ca_private *ca, int slot,
-int *address, int *tupleType,
-int *tupleLength, u8 *tuple)
+int *address, int *tuple_type,
+int *tuple_length, u8 *tuple)
 {
int i;
-   int _tupleType;
-   int _tupleLength;
+   int _tuple_type;
+   int _tuple_length;
int _address = *address;
 
/* grab the next tuple length and type */
-   if ((_tupleType = ca->pub->read_attribute_mem(ca->pub, slot, _address)) 
< 0)
-   return _tupleType;
-   if (_tupleType == 0xff) {
-   dprintk("END OF CHAIN TUPLE type:0x%x\n", _tupleType);
+   _tuple_type = ca->pub->read_attribute_mem(ca->pub, slot, _address);
+   if (_tuple_type < 0)
+   return _tuple_type;
+   if (_tuple_type == 0xff) {
+   dprintk("END OF CHAIN TUPLE type:0x%x\n", _tuple_type);
*address += 2;
-   *tupleType = _tupleType;
-   *tupleLength = 0;
+   *tuple_type = _tuple_type;
+   *tuple_length = 0;
return 0;
}
-   if ((_tupleLength = ca->pub->read_attribute_mem(ca->pub, slot, _address 
+ 2)) < 0)
-   return _tupleLength;
+   _tuple_length = ca->pub->read_attribute_mem(ca->pub, slot,
+   _address + 2);
+   if (_tuple_length < 0)
+   return _tuple_length;
_address += 4;
 
-   dprintk("TUPLE type:0x%x length:%i\n", _tupleType, _tupleLength);
+   dprintk("TUPLE type:0x%x length:%i\n", _tuple_type, _tuple_length);
 
/* read in the whole tuple */
-   for (i = 0; i < _tupleLength; i++) {
+   for (i = 0; i < _tuple_length; i++) {
tuple[i] = 

[PATCH V3 00/16] Fix coding style in en50221 CAM functions

[Jasmin J.]

From: Jasmin Jessich 

Changes V2 to V3:
Fixed the remarks from Antti Polosaari.
Checked the code with checkpatch.pl -strict and fixed all the findings.

These patch series is the V3 version adapted to the already merged patches
from V1 and other merged patches. It does no longer rename constants (see
V1 of this series), as stated by Mauro as a no-go. It still fixed nearly
all the style issues reported by checkpatch.pl in
dvb-core/dvb_ca_en50221.c. In addtion also the checks have been fixed. I
also renamed the patch title, because "Make checkpatch happy ?" is a bad
title and checkpatch complained about it. I also refactored the state
machine a bit more and added the new function dvb_ca_en50221_poll_cam_gone
in a new patch.
 
Two of the remaining warnings are "WARNING: memory barrier without
comment". I have really no clue why there is a call to "mb()" in that file,
so I can't fill in a good comment.
 
I kept some of the "WARNING: line over 80 characters" findings, which are 
strings for debugging, which shouldn't be split in several lines (will 
give other warning). And some lines, where a change would decrease the
readability. There it would be better to split the functions in new
subroutines, which I currently didn't.
 
And finally one "WARNING: Prefer [subsystem eg: netdev]_dbg", complaining
about the "dprintk" macro. In my opinion it is correctly used and it is
normally disabled anyway.
 
The main problem of the original code was the size of the lines and the
structural complexity of some functions. Beside refactoring of the thread
state machine, I used in nearly every function a helper pointer "sl" (for
"slot" structure) instead the whole structure path. This saved also a lot
of characters in long lines and made the code more readable in my opinion.
 
I split the patch set is small pieces for easier review, compiled each
step and tested the resulting driver on my hardware with the DD DuoFlex CI
(single) card. I took a lot of care in the first two patches to really
keep the code as is without loosing any line during refactoring.

Each patch has been tested for compiling and is therefore bisect save.
I also tested the complete patch set with two CAMs and did several plug-
unplug cycles. And the CAM did decode the program as expected.

Additional note:
In the first patch there are two checks "Logical continuations should be
on the previous line" which can't be fixed easily without braking the 80
col limit. Both of them will be resolved automatically with the 2nd patch
"New function dvb_ca_en50221_poll_cam_gone".

Jasmin Jessich (16):
  [media] dvb-core/dvb_ca_en50221.c: Refactored dvb_ca_en50221_thread
  [media] dvb-core/dvb_ca_en50221.c: New function dvb_ca_en50221_poll_cam_gone
  [media] dvb-core/dvb_ca_en50221.c: use usleep_range
  [media] dvb-core/dvb_ca_en50221.c: Fixed block comments
  [media] dvb-core/dvb_ca_en50221.c: Avoid assignments in ifs
  [media] dvb-core/dvb_ca_en50221.c: Used a helper variable
  [media] dvb-core/dvb_ca_en50221.c: Added line breaks
  [media] dvb-core/dvb_ca_en50221.c: Removed useless braces
  [media] dvb-core/dvb_ca_en50221.c: Removed unused symbol
  [media] dvb-core/dvb_ca_en50221.c: Fixed C++ comments
  [media] dvb-core/dvb_ca_en50221.c: Fixed 80 char limit
  [media] dvb-core/dvb_ca_en50221.c: Fixed typo
  [media] dvb-core/dvb_ca_en50221.c: Fix again wrong EXPORT_SYMBOL order
  [media] dvb-core/dvb_ca_en50221.c: Fixed remaining block comments
  [media] dvb-core/dvb_ca_en50221.c: Fixed style issues on the whole file
  [media] dvb-core/dvb_ca_en50221.c: Fixed multiple blank lines

 drivers/media/dvb-core/dvb_ca_en50221.c | 945 +---
 1 file changed, 512 insertions(+), 433 deletions(-)

-- 
2.7.4

[PATCH V3 01/16] [media] dvb-core/dvb_ca_en50221.c: Refactored dvb_ca_en50221_thread

[Jasmin J.]

From: Jasmin Jessich 

Refactored "dvb_ca_en50221_thread" by moving the state machine into the
new function "dvb_ca_en50221_thread_state_machine". This reduces the
thread function size and reduces the structural complexity and of course
gives us more space to meet the line length goal in the new function.

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 363 +---
 1 file changed, 198 insertions(+), 165 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 17970cd..e2f35b7 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1063,207 +1063,240 @@ static void dvb_ca_en50221_thread_update_delay(struct 
dvb_ca_private *ca)
ca->delay = curdelay;
 }
 
-
-
 /**
- * Kernel thread which monitors CA slots for CAM changes, and performs data 
transfers.
+ * Thread state machine for one CA slot to perform the data transfer.
+ *
+ * @ca: CA instance.
+ * @slot: Slot to process.
  */
-static int dvb_ca_en50221_thread(void *data)
+static void dvb_ca_en50221_thread_state_machine(struct dvb_ca_private *ca,
+   int slot)
 {
-   struct dvb_ca_private *ca = data;
-   int slot;
+   struct dvb_ca_slot *sl = >slot_info[slot];
int flags;
int status;
int pktcount;
void *rxbuf;
 
-   dprintk("%s\n", __func__);
+   mutex_lock(>slot_lock);
 
-   /* choose the correct initial delay */
-   dvb_ca_en50221_thread_update_delay(ca);
+   /* check the cam status + deal with CAMCHANGEs */
+   while (dvb_ca_en50221_check_camstatus(ca, slot)) {
+   /* clear down an old CI slot if necessary */
+   if (sl->slot_state != DVB_CA_SLOTSTATE_NONE)
+   dvb_ca_en50221_slot_shutdown(ca, slot);
 
-   /* main loop */
-   while (!kthread_should_stop()) {
-   /* sleep for a bit */
-   if (!ca->wakeup) {
-   set_current_state(TASK_INTERRUPTIBLE);
-   schedule_timeout(ca->delay);
-   if (kthread_should_stop())
-   return 0;
-   }
-   ca->wakeup = 0;
+   /* if a CAM is NOW present, initialise it */
+   if (sl->camchange_type == DVB_CA_EN50221_CAMCHANGE_INSERTED)
+   sl->slot_state = DVB_CA_SLOTSTATE_UNINITIALISED;
 
-   /* go through all the slots processing them */
-   for (slot = 0; slot < ca->slot_count; slot++) {
-
-   mutex_lock(>slot_info[slot].slot_lock);
-
-   // check the cam status + deal with CAMCHANGEs
-   while (dvb_ca_en50221_check_camstatus(ca, slot)) {
-   /* clear down an old CI slot if necessary */
-   if (ca->slot_info[slot].slot_state != 
DVB_CA_SLOTSTATE_NONE)
-   dvb_ca_en50221_slot_shutdown(ca, slot);
-
-   /* if a CAM is NOW present, initialise it */
-   if (ca->slot_info[slot].camchange_type == 
DVB_CA_EN50221_CAMCHANGE_INSERTED) {
-   ca->slot_info[slot].slot_state = 
DVB_CA_SLOTSTATE_UNINITIALISED;
-   }
+   /* we've handled one CAMCHANGE */
+   dvb_ca_en50221_thread_update_delay(ca);
+   atomic_dec(>camchange_count);
+   }
 
-   /* we've handled one CAMCHANGE */
-   dvb_ca_en50221_thread_update_delay(ca);
-   
atomic_dec(>slot_info[slot].camchange_count);
-   }
+   /* CAM state machine */
+   switch (sl->slot_state) {
+   case DVB_CA_SLOTSTATE_NONE:
+   case DVB_CA_SLOTSTATE_INVALID:
+   /* no action needed */
+   break;
 
-   // CAM state machine
-   switch (ca->slot_info[slot].slot_state) {
-   case DVB_CA_SLOTSTATE_NONE:
-   case DVB_CA_SLOTSTATE_INVALID:
-   // no action needed
-   break;
+   case DVB_CA_SLOTSTATE_UNINITIALISED:
+   sl->slot_state = DVB_CA_SLOTSTATE_WAITREADY;
+   ca->pub->slot_reset(ca->pub, slot);
+   sl->timeout = jiffies + (INIT_TIMEOUT_SECS * HZ);
+   break;
 
-   case DVB_CA_SLOTSTATE_UNINITIALISED:
-   ca->slot_info[slot].slot_state = 
DVB_CA_SLOTSTATE_WAITREADY;
-   ca->pub->slot_reset(ca->pub, slot);
-   ca->slot_info[slot].timeout = jiffies + 
(INIT_TIMEOUT_SECS * HZ);
-   

[PATCH V3 11/16] [media] dvb-core/dvb_ca_en50221.c: Fixed 80 char limit

[Jasmin J.]

From: Jasmin Jessich 

Fixed most of:
  WARNING: line over 80 characters
The remaining lines are printk strings, which should not be split and
lines where I thing they should stay as they are.

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 57 +++--
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 99cf460..8c0c730 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -155,7 +155,10 @@ struct dvb_ca_private {
/* Delay the main thread should use */
unsigned long delay;
 
-   /* Slot to start looking for data to read from in the next user-space 
read operation */
+   /*
+* Slot to start looking for data to read from in the next user-space
+* read operation
+*/
int next_read_slot;
 
/* mutex serializing ioctls */
@@ -225,7 +228,7 @@ static char *findstr(char *haystack, int hlen, char 
*needle, int nlen)
 
 
 
-/* 

 */
+/* ** 
*/
 /* EN50221 physical interface functions */
 
 
@@ -365,7 +368,10 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private 
*ca, int slot)
if (ret)
return ret;
 
-   /* store it, and choose the minimum of our buffer and the CAM's buffer 
size */
+   /*
+* store it, and choose the minimum of our buffer and the CAM's buffer
+* size
+*/
buf_size = (buf[0] << 8) | buf[1];
if (buf_size > HOST_LINK_BUF_SIZE)
buf_size = HOST_LINK_BUF_SIZE;
@@ -435,7 +441,8 @@ static int dvb_ca_en50221_read_tuple(struct dvb_ca_private 
*ca, int slot,
 
/* read in the whole tuple */
for (i = 0; i < _tuple_length; i++) {
-   tuple[i] = ca->pub->read_attribute_mem(ca->pub, slot, _address 
+ (i * 2));
+   tuple[i] = ca->pub->read_attribute_mem(ca->pub, slot,
+  _address + (i * 2));
dprintk("  0x%02x: 0x%02x %c\n",
i, tuple[i] & 0xff,
((tuple[i] > 31) && (tuple[i] < 127)) ? tuple[i] : '.');
@@ -590,7 +597,7 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
end_chain = 1;
break;
 
-   default:/* Unknown tuple type - just skip this tuple 
and move to the next one */
+   default:/* Unknown tuple type - just skip this tuple */
dprintk("dvb_ca: Skipping unknown tuple type:0x%x 
length:0x%x\n",
tuple_type, tuple_length);
break;
@@ -766,7 +773,10 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private 
*ca, int slot,
}
}
 
-   /* OK, add it to the receive buffer, or copy into external buffer if 
supplied */
+   /*
+* OK, add it to the receive buffer, or copy into external buffer if
+* supplied
+*/
if (ebuf == NULL) {
if (!sl->rx_buffer.data) {
status = -EIO;
@@ -918,7 +928,7 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private 
*ca, int slot,
 
 
 
-/* 

 */
+/* ** 
*/
 /* EN50221 higher level functions */
 
 
@@ -955,7 +965,8 @@ static int dvb_ca_en50221_slot_shutdown(struct 
dvb_ca_private *ca, int slot)
  * @slot: Slot concerned.
  * @change_type: One of the DVB_CA_CAMCHANGE_* values.
  */
-void dvb_ca_en50221_camchange_irq(struct dvb_ca_en50221 *pubca, int slot, int 
change_type)
+void dvb_ca_en50221_camchange_irq(struct dvb_ca_en50221 *pubca, int slot,
+ int change_type)
 {
struct dvb_ca_private *ca = pubca->private;
struct dvb_ca_slot *sl = >slot_info[slot];
@@ -1031,7 +1042,7 @@ void dvb_ca_en50221_frda_irq(struct dvb_ca_en50221 
*pubca, int slot)
 EXPORT_SYMBOL(dvb_ca_en50221_frda_irq);
 
 
-/* 

 */
+/* ** 
*/
 /* EN50221 thread functions */
 
 /**
@@ -1351,7 +1362,7 @@ static int dvb_ca_en50221_thread(void *data)
 
 
 
-/* 

 */
+/* ** 
*/
 /* EN50221 IO interface functions */
 
 /**
@@ -1484,7 +1495,10 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
 
dprintk("%s\n", __func__);
 
-   

[PATCH V3 13/16] [media] dvb-core/dvb_ca_en50221.c: Fix again wrong EXPORT_SYMBOL order

[Jasmin J.]

From: Jasmin Jessich 

Some EXPORT_SYMBOL() on this file don't match the name of functions
that precedes them.

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index aba80d8..2619822 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1849,7 +1849,6 @@ static unsigned int dvb_ca_en50221_io_poll(struct file 
*file, poll_table *wait)
 
return mask;
 }
-EXPORT_SYMBOL(dvb_ca_en50221_init);
 
 
 static const struct file_operations dvb_ca_fops = {
@@ -1968,8 +1967,7 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
pubca->private = NULL;
return ret;
 }
-EXPORT_SYMBOL(dvb_ca_en50221_release);
-
+EXPORT_SYMBOL(dvb_ca_en50221_init);
 
 
 /**
@@ -1995,3 +1993,4 @@ void dvb_ca_en50221_release(struct dvb_ca_en50221 *pubca)
dvb_ca_private_put(ca);
pubca->private = NULL;
 }
+EXPORT_SYMBOL(dvb_ca_en50221_release);
-- 
2.7.4

[PATCH V3 12/16] [media] dvb-core/dvb_ca_en50221.c: Fixed typo

[Jasmin J.]

From: Jasmin Jessich 

- "dont" -> "don't"

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 8c0c730..aba80d8 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1304,7 +1304,7 @@ static void dvb_ca_en50221_thread_state_machine(struct 
dvb_ca_private *ca,
 */
if (dvb_ca_en50221_check_camstatus(ca, slot)) {
/*
-* we dont want to sleep on the next iteration
+* we don't want to sleep on the next iteration
 * so we can handle the cam change
 */
ca->wakeup = 1;
@@ -1314,7 +1314,7 @@ static void dvb_ca_en50221_thread_state_machine(struct 
dvb_ca_private *ca,
/* check if we've hit our limit this time */
if (++pktcount >= MAX_RX_PACKETS_PER_ITERATION) {
/*
-* dont sleep; there is likely to be more data
+* don't sleep; there is likely to be more data
 * to read
 */
ca->wakeup = 1;
-- 
2.7.4

[PATCH V3 16/16] [media] dvb-core/dvb_ca_en50221.c: Fixed multiple blank lines

[Jasmin J.]

From: Jasmin Jessich 

- Running "checkpatch.pl -strict -f ..." complained
  * Please don't use multiple blank lines

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 40 -
 1 file changed, 40 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index fa5f5ef..95b3723 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -86,7 +86,6 @@ MODULE_PARM_DESC(cam_debug, "enable verbose debug messages");
 #define DVB_CA_SLOTSTATE_WAITFR 6
 #define DVB_CA_SLOTSTATE_LINKINIT   7
 
-
 /* Information on a CA slot */
 struct dvb_ca_slot {
/* current state of the CAM */
@@ -200,7 +199,6 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private 
*ca, int slot,
 static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
 u8 *ebuf, int ecount);
 
-
 /**
  * Safely find needle in haystack.
  *
@@ -225,12 +223,9 @@ static char *findstr(char *haystack, int hlen, char 
*needle, int nlen)
return NULL;
 }
 
-
-
 /* ** 
*/
 /* EN50221 physical interface functions */
 
-
 /**
  * dvb_ca_en50221_check_camstatus - Check CAM status.
  */
@@ -273,7 +268,6 @@ static int dvb_ca_en50221_check_camstatus(struct 
dvb_ca_private *ca, int slot)
return cam_changed;
 }
 
-
 /**
  * dvb_ca_en50221_wait_if_status - Wait for flags to become set on the STATUS
  *  register on a CAM interface, checking for errors and timeout.
@@ -325,7 +319,6 @@ static int dvb_ca_en50221_wait_if_status(struct 
dvb_ca_private *ca, int slot,
return -ETIMEDOUT;
 }
 
-
 /**
  * dvb_ca_en50221_link_init - Initialise the link layer connection to a CAM.
  *
@@ -455,7 +448,6 @@ static int dvb_ca_en50221_read_tuple(struct dvb_ca_private 
*ca, int slot,
return 0;
 }
 
-
 /**
  * dvb_ca_en50221_parse_attributes - Parse attribute memory of a CAM module,
  * extracting Config register, and checking it is a DVB CAM module.
@@ -481,7 +473,6 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
u16 manfid = 0;
u16 devid = 0;
 
-
/* CISTPL_DEVICE_0A */
status = dvb_ca_en50221_read_tuple(ca, slot, , _type,
   _length, tuple);
@@ -490,8 +481,6 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
if (tuple_type != 0x1D)
return -EINVAL;
 
-
-
/* CISTPL_DEVICE_0C */
status = dvb_ca_en50221_read_tuple(ca, slot, , _type,
   _length, tuple);
@@ -500,8 +489,6 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
if (tuple_type != 0x1C)
return -EINVAL;
 
-
-
/* CISTPL_VERS_1 */
status = dvb_ca_en50221_read_tuple(ca, slot, , _type,
   _length, tuple);
@@ -510,8 +497,6 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
if (tuple_type != 0x15)
return -EINVAL;
 
-
-
/* CISTPL_MANFID */
status = dvb_ca_en50221_read_tuple(ca, slot, , _type,
   _length, tuple);
@@ -524,8 +509,6 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
manfid = (tuple[1] << 8) | tuple[0];
devid = (tuple[3] << 8) | tuple[2];
 
-
-
/* CISTPL_CONFIG */
status = dvb_ca_en50221_read_tuple(ca, slot, , _type,
   _length, tuple);
@@ -613,7 +596,6 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
return 0;
 }
 
-
 /**
  * dvb_ca_en50221_set_configoption - Set CAM's configoption correctly.
  *
@@ -641,7 +623,6 @@ static int dvb_ca_en50221_set_configoption(struct 
dvb_ca_private *ca, int slot)
return 0;
 }
 
-
 /**
  * dvb_ca_en50221_read_data - This function talks to an EN50221 CAM control
  * interface. It reads a buffer of data from the CAM. The data can either
@@ -797,7 +778,6 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private 
*ca, int slot,
return status;
 }
 
-
 /**
  * dvb_ca_en50221_write_data - This function talks to an EN50221 CAM control
  * interface. It writes a buffer of data to a CAM.
@@ -819,7 +799,6 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private 
*ca, int slot,
 
dprintk("%s\n", __func__);
 
-
/* sanity check */
if (bytes_write > sl->link_buf_size)
return -EINVAL;
@@ -923,12 +902,9 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private 
*ca, int slot,
return status;
 }
 
-
-
 /* 

[PATCH V3 07/16] [media] dvb-core/dvb_ca_en50221.c: Added line breaks

[Jasmin J.]

From: Jasmin Jessich 

Fixed all:
  WARNING: Missing a blank line after declarations

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 7207ff5..678bd6a 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -178,7 +178,9 @@ static void dvb_ca_private_free(struct dvb_ca_private *ca)
 
 static void dvb_ca_private_release(struct kref *ref)
 {
-   struct dvb_ca_private *ca = container_of(ref, struct dvb_ca_private, 
refcount);
+   struct dvb_ca_private *ca;
+
+   ca = container_of(ref, struct dvb_ca_private, refcount);
dvb_ca_private_free(ca);
 }
 
@@ -297,8 +299,10 @@ static int dvb_ca_en50221_wait_if_status(struct 
dvb_ca_private *ca, int slot,
start = jiffies;
timeout = jiffies + timeout_hz;
while (1) {
+   int res;
+
/* read the status and check for error */
-   int res = ca->pub->read_cam_control(ca->pub, slot, 
CTRLIF_STATUS);
+   res = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS);
if (res < 0)
return -EIO;
 
-- 
2.7.4

[PATCH V3 15/16] [media] dvb-core/dvb_ca_en50221.c: Fixed style issues on the whole file

[Jasmin J.]

From: Jasmin Jessich 

- Running "checkpatch.pl -strict -f ..." gave more checks to fix.
  * Blank lines aren't necessary after an open brace '{'
  * Comparison to NULL written as "!"
  * CHECK: Blank lines aren't necessary before a close brace '}'

Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 24e2b0c..fa5f5ef 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -89,7 +89,6 @@ MODULE_PARM_DESC(cam_debug, "enable verbose debug messages");
 
 /* Information on a CA slot */
 struct dvb_ca_slot {
-
/* current state of the CAM */
int slot_state;
 
@@ -548,7 +547,7 @@ static int dvb_ca_en50221_parse_attributes(struct 
dvb_ca_private *ca, int slot)
 
/* check it contains the correct DVB string */
dvb_str = findstr((char *)tuple, tuple_length, "DVB_CI_V", 8);
-   if (dvb_str == NULL)
+   if (!dvb_str)
return -EINVAL;
if (tuple_length < ((dvb_str - (char *)tuple) + 12))
return -EINVAL;
@@ -640,7 +639,6 @@ static int dvb_ca_en50221_set_configoption(struct 
dvb_ca_private *ca, int slot)
 
/* fine! */
return 0;
-
 }
 
 
@@ -670,7 +668,7 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private 
*ca, int slot,
dprintk("%s\n", __func__);
 
/* check if we have space for a link buf in the rx_buffer */
-   if (ebuf == NULL) {
+   if (!ebuf) {
int buf_free;
 
if (!sl->rx_buffer.data) {
@@ -688,7 +686,7 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private 
*ca, int slot,
 
if (ca->pub->read_data &&
(sl->slot_state != DVB_CA_SLOTSTATE_LINKINIT)) {
-   if (ebuf == NULL)
+   if (!ebuf)
status = ca->pub->read_data(ca->pub, slot, buf,
sizeof(buf));
else
@@ -699,7 +697,6 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private 
*ca, int slot,
if (status == 0)
goto exit;
} else {
-
/* check if there is data available */
status = ca->pub->read_cam_control(ca->pub, slot,
   CTRLIF_STATUS);
@@ -724,7 +721,7 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private 
*ca, int slot,
bytes_read |= status;
 
/* check it will fit */
-   if (ebuf == NULL) {
+   if (!ebuf) {
if (bytes_read > sl->link_buf_size) {
pr_err("dvb_ca adapter %d: CAM tried to send a 
buffer larger than the link buffer size (%i > %i)!\n",
   ca->dvbdev->adapter->num, bytes_read,
@@ -777,7 +774,7 @@ static int dvb_ca_en50221_read_data(struct dvb_ca_private 
*ca, int slot,
 * OK, add it to the receive buffer, or copy into external buffer if
 * supplied
 */
-   if (ebuf == NULL) {
+   if (!ebuf) {
if (!sl->rx_buffer.data) {
status = -EIO;
goto exit;
@@ -1052,7 +1049,6 @@ EXPORT_SYMBOL(dvb_ca_en50221_frda_irq);
  */
 static void dvb_ca_en50221_thread_wakeup(struct dvb_ca_private *ca)
 {
-
dprintk("%s\n", __func__);
 
ca->wakeup = 1;
@@ -1662,7 +1658,6 @@ static ssize_t dvb_ca_en50221_io_read(struct file *file, 
char __user *buf,
/* wait for some data */
status = dvb_ca_en50221_io_read_condition(ca, , );
if (status == 0) {
-
/* if we're in nonblocking mode, exit immediately */
if (file->f_flags & O_NONBLOCK)
return -EWOULDBLOCK;
-- 
2.7.4

Re: [PATCH v8 5/5] [media] platform: Add Synopsys Designware HDMI RX Controller Driver

[kbuild test robot]

Hi Jose,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.13-rc1 next-20170714]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Jose-Abreu/Synopsys-Designware-HDMI-Video-Capture-Controller-PHY/20170711-092521
base:   git://linuxtv.org/media_tree.git master
config: i386-allyesconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/media/platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_registered':
>> drivers/media/platform/dwc/dw-hdmi-rx.c:1452:9: error: implicit declaration 
>> of function 'v4l2_async_subnotifier_register' 
>> [-Werror=implicit-function-declaration]
 return v4l2_async_subnotifier_register(_dev->sd,
^~~
   drivers/media/platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_unregistered':
>> drivers/media/platform/dwc/dw-hdmi-rx.c:1462:2: error: implicit declaration 
>> of function 'v4l2_async_subnotifier_unregister' 
>> [-Werror=implicit-function-declaration]
 v4l2_async_subnotifier_unregister(_dev->v4l2_notifier);
 ^
   cc1: some warnings being treated as errors

vim +/v4l2_async_subnotifier_register +1452 
drivers/media/platform/dwc/dw-hdmi-rx.c

  1436  
  1437  static int dw_hdmi_registered(struct v4l2_subdev *sd)
  1438  {
  1439  struct dw_hdmi_dev *dw_dev = to_dw_dev(sd);
  1440  int ret;
  1441  
  1442  ret = cec_register_adapter(dw_dev->cec_adap, dw_dev->dev);
  1443  if (ret) {
  1444  dev_err(dw_dev->dev, "failed to register CEC 
adapter\n");
  1445  cec_delete_adapter(dw_dev->cec_adap);
  1446  return ret;
  1447  }
  1448  
  1449  cec_register_cec_notifier(dw_dev->cec_adap, 
dw_dev->cec_notifier);
  1450  dw_dev->registered = true;
  1451  
> 1452  return v4l2_async_subnotifier_register(_dev->sd,
  1453  _dev->v4l2_notifier);
  1454  }
  1455  
  1456  static void dw_hdmi_unregistered(struct v4l2_subdev *sd)
  1457  {
  1458  struct dw_hdmi_dev *dw_dev = to_dw_dev(sd);
  1459  
  1460  cec_unregister_adapter(dw_dev->cec_adap);
  1461  cec_notifier_put(dw_dev->cec_notifier);
> 1462  v4l2_async_subnotifier_unregister(_dev->v4l2_notifier);
  1463  }
  1464  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v2 2/6] [media] rockchip/rga: v4l2 m2m support

[Jacob Chen]

Hi all,

2017-07-16 0:49 GMT+08:00 Personnel :
> Le samedi 15 juillet 2017 à 12:42 +0300, Laurent Pinchart a écrit :
>> Hi Jacob,
>>
>> Thank you for the patch.
>>
>> On Saturday 15 Jul 2017 14:58:36 Jacob Chen wrote:
>> > Rockchip RGA is a separate 2D raster graphic acceleration unit. It
>> > accelerates 2D graphics operations, such as point/line drawing, image
>> > scaling, rotation, BitBLT, alpha blending and image blur/sharpness.
>> >
>> > The drvier is mostly based on s5p-g2d v4l2 m2m driver.
>> > And supports various operations from the rendering pipeline.
>> >  - copy
>> >  - fast solid color fill
>> >  - rotation
>> >  - flip
>> >  - alpha blending
>>
>> I notice that you don't support the drawing operations. How do you plan to
>> support them later through the V4L2 M2M API ? I hate stating the obvious, but
>> wouldn't the DRM API be better fit for a graphic accelerator ?
>
> It could fit, maybe, but it really lacks some framework. Also, DRM is
> not really meant for M2M operation, and it's also not great for multi-
> process. Until recently, there was competing drivers for Exynos, both
> implemented in V4L2 and DRM, for similar rational, all DRM ones are
> being deprecated/removed.
>
> I think 2D blitters in V4L2 are fine, but they terribly lack something
> to differentiate them from converters/scalers when looking up the HW
> list. Could be as simple as a capability flag, if I can suggest. For
> the reference, the 2D blitter on IMX6 has been used to implement a live
> video mixer in GStreamer.
>
> https://bugzilla.gnome.org/show_bug.cgi?id=772766
>

We have write a drm RGA driver.
https://patchwork.kernel.org/patch/8630841/

Here are the reasons that why i rewrite it to V4l2 M2M.
1. V4l2 have a better buffer framework. If it use DRM-GEM to handle buffers,
there will be much redundant cache flush, and we have to add much hack code
to workaround.
2. This driver will be used in rockchip linux project. We mostly use it to
scale/colorconvert/rotate/mix video/camera stream.
A V4L2 M2M drvier can be directly used in gstreamer.

The disadvantages of V4l2 M2M API is that it's not stateless.
It's inconvenient if user change size frequently, but it's OK,
we have not yet need this and I think it's possible to extend. ; )


>>
>> Additionally, V4L2 M2M has one source and one destination. How do you
>> implement alpha blending in that case, which by definition requires at least
>> two sources ?
>
> This type of HW only do in-place blits. When using such a node, the
> buffer queued on the V4L2_CAPTURE contains the destination image, and
> the buffer queued on the V4L2_OUTPUT is the source image.
>

Yep.

>>
>> > The code in rga-hw.c is used to configure regs accroding to operations.
>> >
>> > The code in rga-buf.c is used to create private mmu table for RGA.
>> > The tables is stored in a list, and be removed when buffer is cleanup.
>>
>> Looking at the implementation it seems to be a scatter-gather list, not an
>> MMU. Is that right ? Does the hardware documentation refer to it as an MMU ?
>>

It's a 1-level MMU... We use it like a scatter-gather list,
It's also the reason why we don't use RGA with DRM API.


>> > Signed-off-by: Jacob Chen 
>> > ---
>> >  drivers/media/platform/Kconfig|  11 +
>> >  drivers/media/platform/Makefile   |   2 +
>> >  drivers/media/platform/rockchip-rga/Makefile  |   3 +
>> >  drivers/media/platform/rockchip-rga/rga-buf.c | 122 
>> >  drivers/media/platform/rockchip-rga/rga-hw.c  | 652 ++
>> >  drivers/media/platform/rockchip-rga/rga-hw.h  | 437 
>> >  drivers/media/platform/rockchip-rga/rga.c | 958 
>> > +++
>> >  drivers/media/platform/rockchip-rga/rga.h | 111 +++
>> >  8 files changed, 2296 insertions(+)
>> >  create mode 100644 drivers/media/platform/rockchip-rga/Makefile
>> >  create mode 100644 drivers/media/platform/rockchip-rga/rga-buf.c
>> >  create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.c
>> >  create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.h
>> >  create mode 100644 drivers/media/platform/rockchip-rga/rga.c
>> >  create mode 100644 drivers/media/platform/rockchip-rga/rga.h
>>
>>