Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] ARM: dts: imx6sx-sabreauto: drop 'regulators' container node
From: Shawn Guo @ 2018-05-04  9:27 UTC (permalink / raw)
  To: linux-arm-kernel

Let's drop the 'regulators' container node by giving VCC_SD3 an unique
name and removing 'reg' property, so that the fixed regulator can be
under root node directly.

Signed-off-by: Shawn Guo <shawnguo@kernel.org>
---
 arch/arm/boot/dts/imx6sx-sabreauto.dts | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/arch/arm/boot/dts/imx6sx-sabreauto.dts b/arch/arm/boot/dts/imx6sx-sabreauto.dts
index 57d1ea0a3f8e..e5c1d6daa5e0 100644
--- a/arch/arm/boot/dts/imx6sx-sabreauto.dts
+++ b/arch/arm/boot/dts/imx6sx-sabreauto.dts
@@ -18,22 +18,15 @@
 		reg = <0x80000000 0x80000000>;
 	};
 
-	regulators {
-		compatible = "simple-bus";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		vcc_sd3: regulator at 0 {
-			compatible = "regulator-fixed";
-			reg = <0>;
-			pinctrl-names = "default";
-			pinctrl-0 = <&pinctrl_vcc_sd3>;
-			regulator-name = "VCC_SD3";
-			regulator-min-microvolt = <3000000>;
-			regulator-max-microvolt = <3000000>;
-			gpio = <&gpio2 11 GPIO_ACTIVE_HIGH>;
-			enable-active-high;
-		};
+	vcc_sd3: regulator-vcc-sd3 {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_vcc_sd3>;
+		regulator-name = "VCC_SD3";
+		regulator-min-microvolt = <3000000>;
+		regulator-max-microvolt = <3000000>;
+		gpio = <&gpio2 11 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
 	};
 };
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 09/10] ARM: dts: sun7i-a20: Add Video Engine and reserved memory nodes
From: Maxime Ripard @ 2018-05-04  9:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e8cd340605ab4db8ebf2888a4fce645e8bc481d0.camel@bootlin.com>

On Fri, May 04, 2018 at 10:47:44AM +0200, Paul Kocialkowski wrote:
> > > > > +			reg = <0x01c0e000 0x1000>;
> > > > > +			memory-region = <&ve_memory>;
> > > > 
> > > > Since you made the CMA region the default one, you don't need to
> > > > tie
> > > > it to that device in particular (and you can drop it being
> > > > mandatory
> > > > from your binding as well).
> > > 
> > > What if another driver (or the system) claims memory from that zone
> > > and
> > > that the reserved memory ends up not being available for the VPU
> > > anymore?
> > > 
> > > Acccording to the reserved-memory documentation, the reusable
> > > property
> > > (that we need for dmabuf) puts a limitation that the device driver
> > > owning the region must be able to reclaim it back.
> > > 
> > > How does that work out if the CMA region is not tied to a driver in
> > > particular?
> > 
> > I'm not sure to get what you're saying. You have the property
> > linux,cma-default in your reserved region, so the behaviour you
> > described is what you explicitly asked for.
> 
> My point is that I don't see how the driver can claim back (part of) the
> reserved area if the area is not explicitly attached to it.
> 
> Or is that mechanism made in a way that all drivers wishing to use the
> reserved memory area can claim it back from the system, but there is no
> priority (other than first-come first-served) for which drivers claims
> it back in case two want to use the same reserved region (in a scenario
> where there isn't enough memory to allow both drivers)?

This is indeed what happens. Reusable is to let the system use the
reserved memory for things like caches that can easily be dropped when
a driver wants to use the memory in that reserved area. Once that
memory has been allocated, there's no claiming back, unless that
memory segment was freed of course.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180504/41c1fcef/attachment-0001.sig>

^ permalink raw reply

* [PATCH V5 6/6] ARM: imx_v6_v7_defconfig: Select CONFIG_GPIO_MAX732X by default
From: Shawn Guo @ 2018-05-04  9:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1524725898-27974-6-git-send-email-Anson.Huang@nxp.com>

On Thu, Apr 26, 2018 at 02:58:18PM +0800, Anson Huang wrote:
> Enable max7320 IO expander for i.MX platforms.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Applied, thanks.

^ permalink raw reply

* [PATCH v2] pinctrl: msm: fix gpio-hog related boot issues
From: Laxman Dewangan @ 2018-05-04  9:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2265923.XhyVMjVhWR@debian64>



On Thursday 03 May 2018 11:13 PM, Christian Lamparter wrote:
> On Mittwoch, 2. Mai 2018 14:14:39 CEST Linus Walleij wrote:
>> On Thu, Apr 26, 2018 at 11:47 PM, Christian Lamparter
>> <chunkeey@gmail.com> wrote:
>>> On Thu, Apr 26, 2018 11:12:21 CEST Linus Walleij wrote:
>>>> I think I replied in some other mail that I think we need to
>>>> be backwards compatible and it's not too hard to do
>>>> both. (Correct me if I'm wrong.)
>>> I think so too, I looked around and found that the nvidia pinctrl was
>>> doing something similar with of_find_property():
>>> <https://elixir.bootlin.com/linux/v4.17-rc2/source/drivers/pinctrl/tegra/pinctrl-tegra.c#L652>
>>> |       has_prop = of_find_property(np, "gpio-ranges", NULL);
>>>
>>> However this looks kinda funny, since "has_prob" is declared as a bool
>>> and of_find_property() returns a pointer to a "struct property"....
>>> Tell you what: If nobody beats me to it, I'll sent a patch for this after
>>> the pinctrl-msm's gpio-hog has been dealt with. :)
>> Yeah the nVidia driver is one of the oldest and also at the time
>> DT was kind of new. I haven't heard from Stephen for a while
>> but I bet he will pop up, else check with Laxman, he's got
>> a good grip on nVidia pinctrl+GPIO as well.
> All in good time.
>
> But first @Bjorn and @Andy or @David can you please look and
> review v4 "pinctrl: msm: fix gpio-hog related boot issues"
> <https://patchwork.kernel.org/patch/10339129/>

Agree with the patch as by adding gpio-ranges, we add the gpio range 
part of gpiochip_add(), called by 
of_gpiochip_add()->of_gpiochip_add_pin_range(). and so does not need to 
explicitly call of the gpiochip_add_pin_range().

Please add ack in above patch.

Acked-by: Laxman Dewangan <ldewangan@nvidia.com>

^ permalink raw reply

* [PATCH v1] drm/kms/mode: added a new helper for calculating videomode from crtc's display mode
From: Satendra Singh Thakur @ 2018-05-04  9:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525419600-1774-1-git-send-email-satendra.t@samsung.com>

On Thu, May 03, 2018 at 11:36:39 +0100, Liviu Dudau wrote:
> On Thu, May 03, 2018 at 11:28:37AM +0530, Satendra Singh Thakur wrote:
> > 1.
> > -Added a new helper drm_display_mode_crtc_to_videomode
> > -This helper calculates mode parameters like
> >   horizontal front_porch, back_porch, sync length
> >   vertical front_porch, back_porch, sync length
> >   using crtc_* fields of struct drm_display_mode
> > -It uses following fields of crtc mode
> >  horizontal sync start/end, active and total length
> >  vertical sync start/end, active and total length
> > 2.
> > -Most of the driver use user-supplied mode for calculating videomode
> > -However, few drivers use HW (crtc) mode for calculating videomode
> > -This helper will be useful for such drivers
> > 3.
> > -Currently following drivers will be using this new helper
> > -arm hdlcd
> > -atmel hlcdc
> > -exynos 5433 decon
> > -exynos7 decon
> > -exynos fimd
> > 4.
> > -This patch removes related duplicate code from above mentioned drivers
> > 
> > Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
> > Cc: Madhur Verma <madhur.verma@samsung.com>
> > Cc: Hemanshu Srivastava <hemanshu.s@samsung.com>
> > ---
> >  drivers/gpu/drm/arm/hdlcd_crtc.c               |  8 +-------
> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c |  7 +------
> >  drivers/gpu/drm/drm_modes.c                    | 20 ++++++++++++++++++++
> >  drivers/gpu/drm/exynos/exynos5433_drm_decon.c  | 22 ++++++++++------------
> >  drivers/gpu/drm/exynos/exynos7_drm_decon.c     | 23 ++++++++++-------------
> >  drivers/gpu/drm/exynos/exynos_drm_fimd.c       | 22 +++++++++-------------
> >  include/drm/drm_modes.h                        |  2 ++
> >  7 files changed, 53 insertions(+), 51 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > index cf5cbd6..d20e471 100644
> > --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > @@ -130,13 +130,7 @@ static void hdlcd_crtc_mode_set_nofb(struct drm_crtc *crtc)
> >  	struct videomode vm;
> >  	unsigned int polarities, err;
> >  
> > -	vm.vfront_porch = m->crtc_vsync_start - m->crtc_vdisplay;
> > -	vm.vback_porch = m->crtc_vtotal - m->crtc_vsync_end;
> > -	vm.vsync_len = m->crtc_vsync_end - m->crtc_vsync_start;
> > -	vm.hfront_porch = m->crtc_hsync_start - m->crtc_hdisplay;
> > -	vm.hback_porch = m->crtc_htotal - m->crtc_hsync_end;
> > -	vm.hsync_len = m->crtc_hsync_end - m->crtc_hsync_start;
> > -
> > +	drm_display_mode_crtc_to_videomode(m, &vm);
> >  	polarities = HDLCD_POLARITY_DATAEN | HDLCD_POLARITY_DATA;
> >  
> >  	if (m->flags & DRM_MODE_FLAG_PHSYNC)
>
> For the HDLCD part:
> 
> Acked-by: Liviu Dudau <liviu.dudau@arm.com>
> 
> If you're not sending the final version through drm-misc then let me
> know and I can pick up the HDLCD part separately.
>
> Best regards,
> Liviu
>
>

Hi Mr Liviu,
Thanks for accepting the patch.
I don't know how to send patch through drm-misc.
You may please go ahead and pick HDLCD part separately.

Thanks
-Satendra

^ permalink raw reply

* [PATCH] clk: hisilicon: add missing usb3 clocks for Hi3798CV200 SoC
From: sunjg79 at 163.com @ 2018-05-04  8:56 UTC (permalink / raw)
  To: linux-arm-kernel

From: Jianguo Sun <sunjianguo1@huawei.com>

There are two USB3 host controllers on Hi3798CV200 SoC.
This commit adds missing clocks for them.

Signed-off-by: Jianguo Sun <sunjianguo1@huawei.com>
---
 drivers/clk/hisilicon/crg-hi3798cv200.c | 17 +++++++++++++++++
 include/dt-bindings/clock/histb-clock.h |  8 ++++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/clk/hisilicon/crg-hi3798cv200.c b/drivers/clk/hisilicon/crg-hi3798cv200.c
index 743eec1..4fe0b2a 100644
--- a/drivers/clk/hisilicon/crg-hi3798cv200.c
+++ b/drivers/clk/hisilicon/crg-hi3798cv200.c
@@ -186,6 +186,23 @@ static const struct hisi_gate_clock hi3798cv200_gate_clks[] = {
 		CLK_SET_RATE_PARENT, 0xbc, 0, 0 },
 	{ HISTB_USB2_PHY2_REF_CLK, "clk_u2_phy2_ref", "24m",
 		CLK_SET_RATE_PARENT, 0xbc, 2, 0 },
+	/* USB3 */
+	{ HISTB_USB3_BUS_CLK, "clk_u3_bus", NULL,
+		CLK_SET_RATE_PARENT, 0xb0, 0, 0 },
+	{ HISTB_USB3_UTMI_CLK, "clk_u3_utmi", NULL,
+		CLK_SET_RATE_PARENT, 0xb0, 4, 0 },
+	{ HISTB_USB3_PIPE_CLK, "clk_u3_pipe", NULL,
+		CLK_SET_RATE_PARENT, 0xb0, 3, 0 },
+	{ HISTB_USB3_SUSPEND_CLK, "clk_u3_suspend", NULL,
+		CLK_SET_RATE_PARENT, 0xb0, 2, 0 },
+	{ HISTB_USB3_BUS_CLK1, "clk_u3_bus1", NULL,
+		CLK_SET_RATE_PARENT, 0xb0, 16, 0 },
+	{ HISTB_USB3_UTMI_CLK1, "clk_u3_utmi1", NULL,
+		CLK_SET_RATE_PARENT, 0xb0, 20, 0 },
+	{ HISTB_USB3_PIPE_CLK1, "clk_u3_pipe1", NULL,
+		CLK_SET_RATE_PARENT, 0xb0, 19, 0 },
+	{ HISTB_USB3_SUSPEND_CLK1, "clk_u3_suspend1", NULL,
+		CLK_SET_RATE_PARENT, 0xb0, 18, 0 },
 };
 
 static struct hisi_clock_data *hi3798cv200_clk_register(
diff --git a/include/dt-bindings/clock/histb-clock.h b/include/dt-bindings/clock/histb-clock.h
index fab30b3..136de24 100644
--- a/include/dt-bindings/clock/histb-clock.h
+++ b/include/dt-bindings/clock/histb-clock.h
@@ -62,6 +62,14 @@
 #define HISTB_USB2_PHY1_REF_CLK		40
 #define HISTB_USB2_PHY2_REF_CLK		41
 #define HISTB_COMBPHY0_CLK		42
+#define HISTB_USB3_BUS_CLK		43
+#define HISTB_USB3_UTMI_CLK		44
+#define HISTB_USB3_PIPE_CLK		45
+#define HISTB_USB3_SUSPEND_CLK		46
+#define HISTB_USB3_BUS_CLK1		47
+#define HISTB_USB3_UTMI_CLK1		48
+#define HISTB_USB3_PIPE_CLK1		49
+#define HISTB_USB3_SUSPEND_CLK1		50
 
 /* clocks provided by mcu CRG */
 #define HISTB_MCE_CLK			1
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 09/10] ARM: dts: sun7i-a20: Add Video Engine and reserved memory nodes
From: Paul Kocialkowski @ 2018-05-04  8:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e8cd340605ab4db8ebf2888a4fce645e8bc481d0.camel@bootlin.com>

On Fri, 2018-05-04 at 10:47 +0200, Paul Kocialkowski wrote:
> > > > Don't you also need to map the SRAM on the A20?
> > > 
> > > That's a good point, there is currently no syscon handle for A20
> > > (and
> > > also A13). Maybe SRAM is muxed to the VE by default so it "just
> > > works"? 

I just checked on the manual and it appears that SRAM Area C1 is muxed
to the VE at reset, so we can probably keep things as-is until the SRAM
driver is ready to handle explicitly muxing that area to the VE.

-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180504/ea0e5029/attachment.sig>

^ permalink raw reply

* [PATCH v2 05/10] media: v4l: Add definitions for MPEG2 frame format and header metadata
From: Paul Kocialkowski @ 2018-05-04  8:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180419154536.17846-1-paul.kocialkowski@bootlin.com>

On Thu, 2018-04-19 at 17:45 +0200, Paul Kocialkowski wrote:
> Stateless video decoding engines require both the MPEG slices and
> associated metadata from the video stream in order to decode frames.
> 
> This introduces definitions for a new pixel format, describing buffers
> with MPEG2 slice data, as well as a control structure for passing the
> frame header (metadata) to drivers.

While working on this, I came accross Hugues Fruchet's series that also
adds similar definitions for parsed MPEG2 metadata:
https://patchwork.kernel.org/patch/9704707/

Since that version made it to a v6, I will take the time to read the
discussion and see what needs to be changed in my proposal, so that we
can avoid discussing the same points over a year later.

This will most likely not make it to the next revision of the driver
series, so I will keep the format/controls definitions in their v2 state
(despite all the useful comments received) and take the time to properly
rework things in a future revision.

Cheers,

Paul

> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> Signed-off-by: Florent Revest <florent.revest@free-electrons.com>
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++++++
>  drivers/media/v4l2-core/v4l2-ioctl.c |  1 +
>  include/uapi/linux/v4l2-controls.h   | 26 ++++++++++++++++++++++++++
>  include/uapi/linux/videodev2.h       |  3 +++
>  4 files changed, 40 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index ba05a8b9a095..fcdc12b9a9e0 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -761,6 +761,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:		re
> turn "Vertical MV Search Range";
>  	case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER:		re
> turn "Repeat Sequence Header";
>  	case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:		retu
> rn "Force Key Frame";
> +	case V4L2_CID_MPEG_VIDEO_MPEG2_FRAME_HDR:		retu
> rn "MPEG2 Frame Header";
>  
>  	/* VPX controls */
>  	case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:		r
> eturn "VPX Number of Partitions";
> @@ -1152,6 +1153,9 @@ void v4l2_ctrl_fill(u32 id, const char **name,
> enum v4l2_ctrl_type *type,
>  	case V4L2_CID_RDS_TX_ALT_FREQS:
>  		*type = V4L2_CTRL_TYPE_U32;
>  		break;
> +	case V4L2_CID_MPEG_VIDEO_MPEG2_FRAME_HDR:
> +		*type = V4L2_CTRL_TYPE_MPEG2_FRAME_HDR;
> +		break;
>  	default:
>  		*type = V4L2_CTRL_TYPE_INTEGER;
>  		break;
> @@ -1472,6 +1476,9 @@ static int std_validate(const struct v4l2_ctrl
> *ctrl, u32 idx,
>  			return -ERANGE;
>  		return 0;
>  
> +	case V4L2_CTRL_TYPE_MPEG2_FRAME_HDR:
> +		return 0;
> +
>  	default:
>  		return -EINVAL;
>  	}
> @@ -2046,6 +2053,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
> v4l2_ctrl_handler *hdl,
>  	case V4L2_CTRL_TYPE_U32:
>  		elem_size = sizeof(u32);
>  		break;
> +	case V4L2_CTRL_TYPE_MPEG2_FRAME_HDR:
> +		elem_size = sizeof(struct v4l2_ctrl_mpeg2_frame_hdr);
> +		break;
>  	default:
>  		if (type < V4L2_CTRL_COMPOUND_TYPES)
>  			elem_size = sizeof(s32);
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 468c3c65362d..8070203da5d2 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1273,6 +1273,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc
> *fmt)
>  		case V4L2_PIX_FMT_VC1_ANNEX_L:	descr = "VC-1
> (SMPTE 412M Annex L)"; break;
>  		case V4L2_PIX_FMT_VP8:		descr = "VP8";
> break;
>  		case V4L2_PIX_FMT_VP9:		descr = "VP9";
> break;
> +		case V4L2_PIX_FMT_MPEG2_FRAME:	descr = "MPEG2
> Frame"; break;
>  		case V4L2_PIX_FMT_CPIA1:	descr = "GSPCA CPiA
> YUV"; break;
>  		case V4L2_PIX_FMT_WNVA:		descr =
> "WNVA"; break;
>  		case V4L2_PIX_FMT_SN9C10X:	descr = "GSPCA
> SN9C10X"; break;
> diff --git a/include/uapi/linux/v4l2-controls.h
> b/include/uapi/linux/v4l2-controls.h
> index cbbb750d87d1..8431b2a540c7 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -557,6 +557,8 @@ enum v4l2_mpeg_video_mpeg4_profile {
>  };
>  #define V4L2_CID_MPEG_VIDEO_MPEG4_QPEL		(V4L2_CID_MPEG_
> BASE+407)
>  
> +#define
> V4L2_CID_MPEG_VIDEO_MPEG2_FRAME_HDR     (V4L2_CID_MPEG_BASE+450)
> +
>  /*  Control IDs for VP8 streams
>   *  Although VP8 is not part of MPEG we add these controls to the
> MPEG class
>   *  as that class is already handling other video compression
> standards
> @@ -985,4 +987,28 @@ enum v4l2_detect_md_mode {
>  #define V4L2_CID_DETECT_MD_THRESHOLD_GRID	(V4L2_CID_DETECT_CLA
> SS_BASE + 3)
>  #define V4L2_CID_DETECT_MD_REGION_GRID		(V4L2_CID_DETEC
> T_CLASS_BASE + 4)
>  
> +struct v4l2_ctrl_mpeg2_frame_hdr {
> +	__u32 slice_len;
> +	__u32 slice_pos;
> +	enum { MPEG1, MPEG2 } type;
> +
> +	__u16 width;
> +	__u16 height;
> +
> +	enum { PCT_I = 1, PCT_P, PCT_B, PCT_D } picture_coding_type;
> +	__u8 f_code[2][2];
> +
> +	__u8 intra_dc_precision;
> +	__u8 picture_structure;
> +	__u8 top_field_first;
> +	__u8 frame_pred_frame_dct;
> +	__u8 concealment_motion_vectors;
> +	__u8 q_scale_type;
> +	__u8 intra_vlc_format;
> +	__u8 alternate_scan;
> +
> +	__u8 backward_ref_index;
> +	__u8 forward_ref_index;
> +};
> +
>  #endif
> diff --git a/include/uapi/linux/videodev2.h
> b/include/uapi/linux/videodev2.h
> index 31b5728b56e9..4b8336f7bcf0 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -635,6 +635,7 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /*
> SMPTE 421M Annex L compliant stream */
>  #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8
> */
>  #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9
> */
> +#define V4L2_PIX_FMT_MPEG2_FRAME v4l2_fourcc('M', 'G', '2', 'F') /*
> MPEG2 frame */
>  
>  /*  Vendor-specific formats   */
>  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /*
> cpia1 YUV */
> @@ -1586,6 +1587,7 @@ struct v4l2_ext_control {
>  		__u8 __user *p_u8;
>  		__u16 __user *p_u16;
>  		__u32 __user *p_u32;
> +		struct v4l2_ctrl_mpeg2_frame_hdr __user
> *p_mpeg2_frame_hdr;
>  		void __user *ptr;
>  	};
>  } __attribute__ ((packed));
> @@ -1631,6 +1633,7 @@ enum v4l2_ctrl_type {
>  	V4L2_CTRL_TYPE_U8	     = 0x0100,
>  	V4L2_CTRL_TYPE_U16	     = 0x0101,
>  	V4L2_CTRL_TYPE_U32	     = 0x0102,
> +	V4L2_CTRL_TYPE_MPEG2_FRAME_HDR = 0x0109,
>  };
>  
>  /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180504/5e7e40ea/attachment.sig>

^ permalink raw reply

* [PATCH v2 09/10] ARM: dts: sun7i-a20: Add Video Engine and reserved memory nodes
From: Paul Kocialkowski @ 2018-05-04  8:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504084008.h6p4brari3xrbv6l@flea>

Hi,

On Fri, 2018-05-04 at 10:40 +0200, Maxime Ripard wrote:
> On Fri, May 04, 2018 at 09:49:16AM +0200, Paul Kocialkowski wrote:
> > > > +	reserved-memory {
> > > > +		#address-cells = <1>;
> > > > +		#size-cells = <1>;
> > > > +		ranges;
> > > > +
> > > > +		/* Address must be kept in the lower 256 MiBs
> > > > of
> > > > DRAM for VE. */
> > > > +		ve_memory: cma at 4a000000 {
> > > > +			compatible = "shared-dma-pool";
> > > > +			reg = <0x4a000000 0x6000000>;
> > > > +			no-map;
> > > 
> > > I'm not sure why no-map is needed.
> > 
> > In fact, having no-map here would lead to reserving the area as
> > cache-
> > coherent instead of contiguous and thus prevented dmabuf support.
> > Replacing it by "resuable" allows proper CMA reservation.
> > 
> > > And I guess we could use alloc-ranges to make sure the region is
> > > in
> > > the proper memory range, instead of hardcoding it.
> > 
> > As far as I could understand from the documentation, "alloc-ranges"
> > is
> > used for dynamic allocation while only "reg" is used for static
> > allocation. We are currently going with static allocation and thus
> > reserve the whole 96 MiB. Is using dynamic allocation instead
> > desirable
> > here?
> 
> I guess we could turn the question backward. Why do we need a static
> allocation? This isn't a buffer that is always allocated on the same
> area, but rather that we have a range available. So our constraint is
> on the range, nothing else.

That makes sense, I will give it a shot with a range then.

> > > > +			reg = <0x01c0e000 0x1000>;
> > > > +			memory-region = <&ve_memory>;
> > > 
> > > Since you made the CMA region the default one, you don't need to
> > > tie
> > > it to that device in particular (and you can drop it being
> > > mandatory
> > > from your binding as well).
> > 
> > What if another driver (or the system) claims memory from that zone
> > and
> > that the reserved memory ends up not being available for the VPU
> > anymore?
> > 
> > Acccording to the reserved-memory documentation, the reusable
> > property
> > (that we need for dmabuf) puts a limitation that the device driver
> > owning the region must be able to reclaim it back.
> > 
> > How does that work out if the CMA region is not tied to a driver in
> > particular?
> 
> I'm not sure to get what you're saying. You have the property
> linux,cma-default in your reserved region, so the behaviour you
> described is what you explicitly asked for.

My point is that I don't see how the driver can claim back (part of) the
reserved area if the area is not explicitly attached to it.

Or is that mechanism made in a way that all drivers wishing to use the
reserved memory area can claim it back from the system, but there is no
priority (other than first-come first-served) for which drivers claims
it back in case two want to use the same reserved region (in a scenario
where there isn't enough memory to allow both drivers)?

> > > > +
> > > > +			clocks = <&ccu CLK_AHB_VE>, <&ccu
> > > > CLK_VE>,
> > > > +				 <&ccu CLK_DRAM_VE>;
> > > > +			clock-names = "ahb", "mod", "ram";
> > > > +
> > > > +			assigned-clocks = <&ccu CLK_VE>;
> > > > +			assigned-clock-rates = <320000000>;
> > > 
> > > This should be set from within the driver. If it's something that
> > > you
> > > absolutely needed for the device to operate, you have no guarantee
> > > that the clock rate won't change at any point in time after the
> > > device
> > > probe, so that's not a proper solution.
> > > 
> > > And if it's not needed and can be adjusted depending on the
> > > framerate/codec/resolution, then it shouldn't be in the DT either.
> > 
> > Yes, that makes sense.
> > 
> > > Don't you also need to map the SRAM on the A20?
> > 
> > That's a good point, there is currently no syscon handle for A20
> > (and
> > also A13). Maybe SRAM is muxed to the VE by default so it "just
> > works"? 
> > 
> > I'll investigate on this side, also keeping in mind that the actual
> > solution is to use the SRAM controller driver (but that won't make
> > it to
> > v3).
> 
> The SRAM driver is available on the A20, so you should really use that
> instead of a syscon.

The SRAM driver is indeed available for the A20, but still lacks support
for the VE in particular as far as I can see.

Cheers,

-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180504/dd75265f/attachment-0001.sig>

^ permalink raw reply

* [PATCH v7 2/2] PCI: mediatek: Using chained IRQ to setup IRQ handle
From: Marc Zyngier @ 2018-05-04  8:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525412853-24367-3-git-send-email-honghui.zhang@mediatek.com>

On 04/05/18 06:47, honghui.zhang at mediatek.com wrote:
> From: Honghui Zhang <honghui.zhang@mediatek.com>
> 
> Using irq_chip solution to setup IRQs in order to consist
> with IRQ framework.
> 
> Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> Acked-by: Ryder Lee <ryder.lee@mediatek.com>

Thanks for going the extra mile and fixing this driver.

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH 4/5] ARM: dts: imx6sx-sabreauto: add fec support
From: Shawn Guo @ 2018-05-04  8:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <DB5PR04MB132072FEDCAFEDF6592C6A32F58E0@DB5PR04MB1320.eurprd04.prod.outlook.com>

On Thu, Apr 26, 2018 at 06:57:07AM +0000, Anson Huang wrote:
> > > Sorry, I made a mistake here, the MAX7320 IO0 is for adjusting FEC1's
> > > voltage,
> > 
> > In this case you need to pass the 'phy-supply' property inside the fec node and
> > add a regulator that is controlled via MAX7320 IO0 pin.
> 
> The 'phy-supply' is for enabling/disabling phy regulator, but here the MAX7322 IO0 is NOT for
> enabling/disabling PHY regulator, it is for IO voltage switch between 1.5V and 1.8V, our ENET
> IO can work with both 1.5V and 1.8V, so any config is OK for ENET function.
> 
> The 1.5V/1.8V selection is a one time setting thing, that means we only need to
> config it once during boot up, most of i.MX platforms does NOT provide such voltage
> switch function for ENET IO, on this 6SX sabre auto board, it is more like a backup or
> validation purpose. With default settings, ENET's function is NOT impacted at all.
> 
> I think we can add a gpio regulator for it and let the regulator initialization set the GPIO
> Level for fec, such below, with " enable-active-high " present, GPIO will be at LOW and voltage
> is 1.5V, without this property, GPIO will be HIGH and voltage will be 1.8V.
> +               reg_fec: fec_io_supply {
> +                       compatible = "regulator-gpio";
> +                       regulator-name = "1.8V_1.5V_FEC";
> +                       regulator-min-microvolt = <1500000>;
> +                       regulator-max-microvolt = <1800000>;
> +                       states = <1500000 0x0 1800000 0x1>;
> +                       enable-gpio = <&max7322 0 GPIO_ACTIVE_HIGH>;
> +                       vin-supply = <&sw2_reg>;
> +                       enable-active-high;
> +               };

Looking at gpio-regulator bindings doc, I feel that property 'gpios'
rather than 'enable-gpio' should be used to specify the MAX7320 IO0 pin.

- enable-gpio           : GPIO to use to enable/disable the regulator.
- gpios                 : GPIO group used to control voltage.

Shawn

^ permalink raw reply

* [PATCH v2 09/10] ARM: dts: sun7i-a20: Add Video Engine and reserved memory nodes
From: Maxime Ripard @ 2018-05-04  8:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <82057e2f734137a3902d9313c228b01ceb345ee7.camel@bootlin.com>

On Fri, May 04, 2018 at 09:49:16AM +0200, Paul Kocialkowski wrote:
> > > +	reserved-memory {
> > > +		#address-cells = <1>;
> > > +		#size-cells = <1>;
> > > +		ranges;
> > > +
> > > +		/* Address must be kept in the lower 256 MiBs of
> > > DRAM for VE. */
> > > +		ve_memory: cma at 4a000000 {
> > > +			compatible = "shared-dma-pool";
> > > +			reg = <0x4a000000 0x6000000>;
> > > +			no-map;
> > 
> > I'm not sure why no-map is needed.
> 
> In fact, having no-map here would lead to reserving the area as cache-
> coherent instead of contiguous and thus prevented dmabuf support.
> Replacing it by "resuable" allows proper CMA reservation.
> 
> > And I guess we could use alloc-ranges to make sure the region is in
> > the proper memory range, instead of hardcoding it.
> 
> As far as I could understand from the documentation, "alloc-ranges" is
> used for dynamic allocation while only "reg" is used for static
> allocation. We are currently going with static allocation and thus
> reserve the whole 96 MiB. Is using dynamic allocation instead desirable
> here?

I guess we could turn the question backward. Why do we need a static
allocation? This isn't a buffer that is always allocated on the same
area, but rather that we have a range available. So our constraint is
on the range, nothing else.

> > > +			reg = <0x01c0e000 0x1000>;
> > > +			memory-region = <&ve_memory>;
> > 
> > Since you made the CMA region the default one, you don't need to tie
> > it to that device in particular (and you can drop it being mandatory
> > from your binding as well).
> 
> What if another driver (or the system) claims memory from that zone and
> that the reserved memory ends up not being available for the VPU
> anymore?
> 
> Acccording to the reserved-memory documentation, the reusable property
> (that we need for dmabuf) puts a limitation that the device driver
> owning the region must be able to reclaim it back.
> 
> How does that work out if the CMA region is not tied to a driver in
> particular?

I'm not sure to get what you're saying. You have the property
linux,cma-default in your reserved region, so the behaviour you
described is what you explicitly asked for.

> 
> > > +
> > > +			clocks = <&ccu CLK_AHB_VE>, <&ccu CLK_VE>,
> > > +				 <&ccu CLK_DRAM_VE>;
> > > +			clock-names = "ahb", "mod", "ram";
> > > +
> > > +			assigned-clocks = <&ccu CLK_VE>;
> > > +			assigned-clock-rates = <320000000>;
> > 
> > This should be set from within the driver. If it's something that you
> > absolutely needed for the device to operate, you have no guarantee
> > that the clock rate won't change at any point in time after the device
> > probe, so that's not a proper solution.
> > 
> > And if it's not needed and can be adjusted depending on the
> > framerate/codec/resolution, then it shouldn't be in the DT either.
> 
> Yes, that makes sense.
> 
> > Don't you also need to map the SRAM on the A20?
> 
> That's a good point, there is currently no syscon handle for A20 (and
> also A13). Maybe SRAM is muxed to the VE by default so it "just works"? 
> 
> I'll investigate on this side, also keeping in mind that the actual
> solution is to use the SRAM controller driver (but that won't make it to
> v3).

The SRAM driver is available on the A20, so you should really use that
instead of a syscon.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180504/5dcc102b/attachment.sig>

^ permalink raw reply

* [PATCH V4 5/8] soc: mediatek: pwrap: add pwrap for mt6797 SoCs
From: Matthias Brugger @ 2018-05-04  8:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525403052.14792.131.camel@mtkswgap22>



On 05/04/2018 05:04 AM, Sean Wang wrote:
> On Thu, 2018-05-03 at 14:20 +0800, Argus Lin wrote:
>> On Thu, 2018-05-03 at 12:01 +0800, Sean Wang wrote:
>>>  };
> 
> [...]
> 
>>>> @@ -1503,11 +1581,13 @@ static int pwrap_probe(struct platform_device *pdev)
>>>>  	if (IS_ERR(wrp->base))
>>>>  		return PTR_ERR(wrp->base);
>>>>  
>>>> -	wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
>>>> -	if (IS_ERR(wrp->rstc)) {
>>>> -		ret = PTR_ERR(wrp->rstc);
>>>> -		dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
>>>> -		return ret;
>>>> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_RESET)) {
>>>> +		wrp->rstc = devm_reset_control_get(wrp->dev, "pwrap");
>>>
>>> there should be a reset bit present for pwrap on infrasys.
>>>
>>> the specific condition can be dropped when the reset cell is exported from infrasys and then the device has a reference to it.
>> hmm, I think it need to keep here.
>> because after pwrap initialized, it can't be reset alone.
>> It needs to reset PMIC simultaneously, too.
> 
> Reset a pair, either a master or its slave, all had been a part of
> pwrap_init.
> 
> The reset controller provided here is just to reset pwrap device.
> And for its slave reset, it should be done by pwrap_reset_spislave. 
> 
> So for MT6397, it should be able to fall into the same procedure.
> 
>>>
>>>> +		if (IS_ERR(wrp->rstc)) {
>>>> +			ret = PTR_ERR(wrp->rstc);
>>>> +			dev_dbg(wrp->dev, "cannot get pwrap reset: %d\n", ret);
>>>> +			return ret;
>>>> +		}
>>>>  	}
>>>>  
>>>>  	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
>>>> @@ -1549,9 +1629,17 @@ static int pwrap_probe(struct platform_device *pdev)
>>>>  	if (ret)
>>>>  		goto err_out1;
>>>>  
>>>> -	/* Enable internal dynamic clock */
>>>> -	pwrap_writel(wrp, 1, PWRAP_DCM_EN);
>>>> -	pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
>>>> +	/*
>>>> +	 * add dcm capability check
>>>> +	 */
>>>> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {
>>>
>>> the specific condition can be dropped since so far all devices the driver can support are owning PWRAP_CAP_DCM
>> We did not support DCM for future chips.
>> MT6797 is the last one.
>> This why I want to add judgement here.
> 
> The series is only for MT6797 pwrap, so it's fine with only adding these
> things the SoC actually relies on. 
> 
> PWRAP_CAP_DCM should not be added until a new SoC without dcm is really
> introduced.
> 

I agree (and I think I said this already in a previous review).

Regards,
Matthias

>>>
>>>> +		if (wrp->master->type == PWRAP_MT6797)
>>>> +			pwrap_writel(wrp, 3, PWRAP_DCM_EN);
>>>
>>> the setup for MT6797 can be moved into .init_soc_specific callback ?
>>
>> I think put it here is more generally.
>>>
>>>> +		else
>>>> +			pwrap_writel(wrp, 1, PWRAP_DCM_EN);
>>>> +
>>>> +		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
>>>> +	}
>>>>  
>>>>  	/*
>>>>  	 * The PMIC could already be initialized by the bootloader.
>>>> @@ -1580,6 +1668,12 @@ static int pwrap_probe(struct platform_device *pdev)
>>>>  	pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN);
>>>>  	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
>>>>  	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
>>>> +	/*
>>>> +	 * We add INT1 interrupt to handle starvation and request exception
>>>> +	 * If we support it, we should enable them here.
>>>> +	 */
>>>> +	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
>>>> +		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
>>>>  
>>>
>>> if there is no explicitly enabling on INT1, then ISR handling for INT1 is also unnecessary
>>
>> It's ok for me.
>>>
>>>>  	irq = platform_get_irq(pdev, 0);
>>>>  	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,
>>>
>>>
>>>
>>
>>
> 
> 

^ permalink raw reply

* [PATCH 09/17] irqchip/irq-mvebu-icu: support ICU subnodes
From: Miquel Raynal @ 2018-05-04  8:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180502101300.49cc3623@windsurf.home>

Hi Thomas,

On Wed, 2 May 2018 10:13:00 +0200, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:

> Hello Miqu?l,
> 
> On Sat, 21 Apr 2018 15:55:29 +0200, Miquel Raynal wrote:
> > Introduce new bindings for the ICU.  
> 
> Perhaps this should explain *why* we need new bindings.

Sure, I changed the whole message by:

    The ICU can handle several type of interrupt, each of them being
    handled differently on AP side. On CP side, the ICU should be able
    to make the distinction between each interrupt group by pointing to
    the right parent.
    
    This is done through the introduction of new bindings, presenting
    the ICU node as the parent of multiple ICU sub-nodes, each of them
    being an interrupt type with a different interrupt parent. ICU
    interrupt 'clients' now directly point to the right sub-node,
    avoiding the need for the extra ICU_GRP_* parameter.
    
    ICU subnodes are probed automatically with
    devm_platform_populate(). If the node as no child, the probe
    function for NSRs will still be called 'manually' in order to
    preserve backward compatibility with DT using the old binding.
    
> 
> > Each DT subnode of the ICU represents a type of interrupt that should
> > be handled separately. Add the possibility for the ICU to have subnodes
> > and probe each of them automatically with devm_platform_populate(). If
> > the node as no child, the probe function for NSRs will still be called
> > 'manually'.  
> 
>  ... in order to preserve backward compatibility with Device Trees
>  using the old binding.

Added, see above.

> 
> > +static struct mvebu_icu *mvebu_dev_get_drvdata(struct platform_device *pdev)  
> 
> The function should be prefixed by mvebu_icu_, not just mvebu_.

Changed.

> 
> > +{
> > +	struct mvebu_icu *icu;
> > +
> > +	icu = dev_get_drvdata(&pdev->dev);
> > +	if (icu) {
> > +		/* Legacy bindings: get the device data */  
> 
> I find this comment weird, because it doesn't document what the test
> just below is doing.

Refactored a bit the comments in this section.

> 
> > +		if (!icu->legacy_bindings)
> > +			return ERR_PTR(-EINVAL);
> > +	} else {
> > +		/* New bindings: get the parent device (ICU) data */
> > +		icu = dev_get_drvdata(pdev->dev.parent);
> > +		if (!icu)
> > +			return ERR_PTR(-ENODEV);
> > +		if (icu->legacy_bindings)
> > +			return ERR_PTR(-EINVAL);
> > +	}  
> 
> 
> > @@ -144,7 +170,10 @@ mvebu_icu_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> >  		goto free_irqd;
> >  	}
> >  
> > -	icu_irqd->icu_group = fwspec->param[0];
> > +	if (icu->legacy_bindings)
> > +		icu_irqd->icu_group = fwspec->param[0];
> > +	else
> > +		icu_irqd->icu_group = ICU_GRP_NSR;  
> 
> In practice here fwspec->param[0] is always going to be equal to
> ICU_GRP_NSR, but OK, the test makes sense as in a future commit, the
> "else" case will be changed to support SEIs.
> 
> > +static const struct of_device_id mvebu_icu_nsr_of_match[] = {
> > +	{ .compatible = "marvell,cp110-icu-nsr", },
> > +	{},
> > +};
> > +
> > +static struct platform_driver mvebu_icu_nsr_driver = {
> > +	.probe  = mvebu_icu_nsr_probe,
> > +	.driver = {
> > +		.name = "mvebu-icu-nsr",
> > +		.of_match_table = mvebu_icu_nsr_of_match,
> > +	},
> > +};
> > +builtin_platform_driver(mvebu_icu_nsr_driver);  
> 
> I'm not sure why you call this icu_nsr here, and change it later to
> icu_subset. Wouldn't it make sense to call it right away with the final
> name ? Note that this is not a very strong request to change this
> aspect, I'm fine with how it's done today, it's just that I would have
> done it differently.

I thought calling it icu_subset right now would not be very clear as
there are not "ICU subset" other than NSR interrupts, but I changed it,
this is not a big deal.

> 
> Other than that, looks good to me.
> 
> Thomas

Thanks for the review,
Miqu?l


-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* [PATCH 2/2] arm64: Clear the stack
From: Alexander Popov @ 2018-05-04  8:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5b654bb4-64cc-dc64-afd7-135971b54c98@redhat.com>

On 03.05.2018 22:09, Laura Abbott wrote:
> On 05/03/2018 10:33 AM, Alexander Popov wrote:
>> On 03.05.2018 10:19, Mark Rutland wrote:
>>> On Wed, May 02, 2018 at 01:33:26PM -0700, Laura Abbott wrote:
>>>> +	/* Reset the lowest_stack value for the next syscall */
>>>> +	current->thread.lowest_stack = current_stack_pointer;
>>
>> Laura, that might be wrong and introduce huge performance impact.
>>
>> I think, lowest_stack should be reset similarly to the original version.
>>
> 
> Sorry, I'm not understanding here. What's the performance impact and
> what do you mean by original version?

I meant the code for x86:
	/* Reset the lowest_stack value for the next syscall */
	current->thread.lowest_stack = current_top_of_stack() - 256;

...Now when I'm writing about the performance impact, I see that I was wrong
about "huge". Excuse me.

Let me describe the implications of this code change.

So we are at the end of a syscall. We've just erased the used part of the kernel
stack. The current stack pointer is near to the top of stack. On x86_64 I see
that the stack pointer is stack top minus 56 bytes (just before switching onto
the trampoline stack).

I took the idea of resetting lowest_stack to stack top minus 256 from the
original PaX Team's code. It should give the speedup when lowest_stack is not
updated during a syscall (a lot of functions are not instrumented) and we start
to search for the poison value from that reasonable point.

If we speak about the common erase_kstack() code, this code change can break
x86, because this function can be called from the trampoline stack (separate
from the thread stack).

>>>> +}
>>>
>>> Once this function returns, its data is left on the stack. Is that not a problem?
>>>
>>> No strong feelings either way, but it might be worth mentioning in the commit
>>> message.
>>
>> I managed to bypass that with "register" specifier. Although it doesn't give an
>> absolute guarantee.
>>
> 
> I guess I was assuming gcc would be smart enough not to spill stuff
> on the stack. I also intentionally removed the register keyword
> since it wasn't clear gcc does much with it on a modern system? I
> could be completely off base here though so please correct me if
> I'm wrong. It probably is worth documenting what we are assuming about
> the compiler here.

I think having register storage class specifier here is a bit better than
nothing. And yes, I'll add a comment. Right now don't see a better solution.

>>>> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
>>>> index a34e9290a699..25dd2a14560d 100644
>>>> --- a/drivers/firmware/efi/libstub/Makefile
>>>> +++ b/drivers/firmware/efi/libstub/Makefile
>>>> @@ -20,7 +20,8 @@ cflags-$(CONFIG_EFI_ARMSTUB)	+= -I$(srctree)/scripts/dtc/libfdt
>>>>   KBUILD_CFLAGS			:= $(cflags-y) -DDISABLE_BRANCH_PROFILING \
>>>>   				   -D__NO_FORTIFY \
>>>>   				   $(call cc-option,-ffreestanding) \
>>>> -				   $(call cc-option,-fno-stack-protector)
>>>> +				   $(call cc-option,-fno-stack-protector) \
>>>> +				   $(DISABLE_STACKLEAK_PLUGIN)
>>>>   
>>>>   GCOV_PROFILE			:= n
>>>>   KASAN_SANITIZE			:= n
>>>
>>> I believe we'll also need to do this for the KVM hyp code in arch/arm64/kvm/hyp/.
>>
>> Could you please give more details on that? Why STACKLEAK breaks it?
>>
> 
> For reference, I originally added this for the efistub because
> it would not compile.

I guess it was a linkage error, right?

> I did compile this against my Fedora tree which has KVM enabled.

Looked through this big article about ARM, KVM and HYP mode:
https://lwn.net/Articles/557132/

So we have some limited amount of kernel code which runs in HYP mode. Is it only
in arch/arm64/kvm/hyp/ directory?

Mark, could you give a clue what trouble will we have if we call track_stack()
or check_alloca() from that code?

Thanks in advance!

--
Alexander

^ permalink raw reply

* [PATCH] ARM: dts: imx6sx-sabreauto: add egalax touch screen support
From: Shawn Guo @ 2018-05-04  8:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1524813650-19241-1-git-send-email-Anson.Huang@nxp.com>

On Fri, Apr 27, 2018 at 03:20:50PM +0800, Anson Huang wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> Add egalax touch screen support on i2c2 bus.
> 
> Signed-off-by: Haibo Chen <haibo.chen@freescale.com>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> this patch is based on patch (ARM: dts: imx6sx-sabreauto: add debug LED support) I sent out for review.
>  arch/arm/boot/dts/imx6sx-sabreauto.dts | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6sx-sabreauto.dts b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> index 6b94f24..2f6e474 100644
> --- a/arch/arm/boot/dts/imx6sx-sabreauto.dts
> +++ b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> @@ -131,6 +131,12 @@
>  &iomuxc {
>  	imx6x-sabreauto {
>  
> +		pinctrl_egalax_int: egalax_intgrp {

General recommendation: use hyphen instead of underscore in the node
node.

> +			fsl,pins = <
> +				MX6SX_PAD_SD4_RESET_B__GPIO6_IO_22      0x80000000
> +			>;
> +		};
> +
>  		pinctrl_led: ledgrp {
>  			fsl,pins = <
>  				MX6SX_PAD_CSI_PIXCLK__GPIO1_IO_24 0x17059
> @@ -274,6 +280,16 @@
>  	pinctrl-0 = <&pinctrl_i2c2_1>;
>  	status = "okay";
>  
> +	egalax_ts at 04 {

Use generic node name - touchscreen, as recommended by Devicetree
Specification, and drop leading zeros in unit-address.

Shawn

> +		compatible = "eeti,egalax_ts";
> +		reg = <0x04>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_egalax_int>;
> +		interrupt-parent = <&gpio6>;
> +		interrupts = <22 IRQ_TYPE_EDGE_FALLING>;
> +		wakeup-gpios = <&gpio6 22 GPIO_ACTIVE_HIGH>;
> +	};
> +
>  	max7322: gpio at 68 {
>  		compatible = "maxim,max7322";
>  		reg = <0x68>;
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [alsa-devel] [PATCH v7 16/24] ASoC: qdsp6: q6asm: Add support to audio stream apis
From: Srinivas Kandagatla @ 2018-05-04  8:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <f3b3d90b-6504-328e-c9f4-a3b3f085d9dc@codeaurora.org>

Thanks Rohit for testing this!


On 04/05/18 08:11, Rohit Kumar wrote:
> 
> 
> On 5/1/2018 5:38 PM, Srinivas Kandagatla wrote:
>> +static int __q6asm_run(struct audio_client *ac, uint32_t flags,
>> +          uint32_t msw_ts, uint32_t lsw_ts, bool wait)
>> +{
>> +    struct asm_session_cmd_run_v2 *run;
>> +    struct apr_pkt *pkt;
>> +    int pkt_size, rc;
>> +    void *p;
>> +
>> +    pkt_size = APR_HDR_SIZE + sizeof(*run);
>> +    p = kzalloc(pkt_size, GFP_KERNEL);
> Should be GFP_ATOMIC as this API is also called from interrupt context

Yes, these should be a GFP_ATOMIC, Will fix this up in next version.

thanks,
srini

>> +    if (!p)
>> +        return -ENOMEM;
>> +
> [..]
>> +int q6asm_read(struct audio_client *ac)
>> +{
>> +    struct asm_data_cmd_read_v2 *read;
>> +    struct audio_port_data *port;
>> +    struct audio_buffer *ab;
>> +    struct apr_pkt *pkt;
>> +    int pkt_size;
>> +    int rc = 0;
>> +    void *p;
>> +
>> +    if (!(ac->io_mode & ASM_SYNC_IO_MODE))
>> +        return 0;
>> +
>> +    pkt_size = APR_HDR_SIZE + sizeof(*read);
>> +    p = kzalloc(pkt_size, GFP_KERNEL);
> same here. GFP_ATOMIC
>> +    if (!p)
>> +        return -ENOMEM;
> [..]
>> +int q6asm_write_async(struct audio_client *ac, uint32_t len, uint32_t 
>> msw_ts,
>> +               uint32_t lsw_ts, uint32_t flags)
>> +{
>> +    struct asm_data_cmd_write_v2 *write;
>> +    struct audio_port_data *port;
>> +    struct audio_buffer *ab;
>> +    struct apr_pkt *pkt;
>> +    int pkt_size;
>> +    int rc = 0;
>> +    void *p;
>> +
>> +    pkt_size = APR_HDR_SIZE + sizeof(*write);
>> +    p = kzalloc(pkt_size, GFP_KERNEL);
> GFP_ATOMIC
>> +    if (!p)
>> +        return -ENOMEM;
>>
> Thanks,
> Rohit
> 

^ permalink raw reply

* [PATCH v2 05/10] media: v4l: Add definitions for MPEG2 frame format and header metadata
From: Paul Kocialkowski @ 2018-05-04  8:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAAFQd5Dq4OeshtFaoxFK2357+-_=hzh0C7W=zksTWtaDuDCiGg@mail.gmail.com>

Hi,

On Fri, 2018-04-20 at 09:51 +0000, Tomasz Figa wrote:
> Hi Paul,
> 
> On Fri, Apr 20, 2018 at 12:46 AM Paul Kocialkowski <
> paul.kocialkowski at bootlin.com> wrote:
> [snip]
> > +struct v4l2_ctrl_mpeg2_frame_hdr {
> > +       __u32 slice_len;
> > +       __u32 slice_pos;
> > +       enum { MPEG1, MPEG2 } type;
> 
> Is enum suitable for UAPI?

As it turns out, it's not :)

> > +
> > +       __u16 width;
> > +       __u16 height;
> > +
> > +       enum { PCT_I = 1, PCT_P, PCT_B, PCT_D } picture_coding_type;
> 
> Ditto.
> 
> > +       __u8 f_code[2][2];
> > +
> > +       __u8 intra_dc_precision;
> > +       __u8 picture_structure;
> > +       __u8 top_field_first;
> > +       __u8 frame_pred_frame_dct;
> > +       __u8 concealment_motion_vectors;
> > +       __u8 q_scale_type;
> > +       __u8 intra_vlc_format;
> > +       __u8 alternate_scan;
> > +
> > +       __u8 backward_ref_index;
> > +       __u8 forward_ref_index;
> > +};
> > +
> >   #endif
> > diff --git a/include/uapi/linux/videodev2.h
> 
> b/include/uapi/linux/videodev2.h
> > index 31b5728b56e9..4b8336f7bcf0 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -635,6 +635,7 @@ struct v4l2_pix_format {
> >   #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L')
> > /*
> 
> SMPTE 421M Annex L compliant stream */
> >   #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /*
> > VP8 */
> >   #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /*
> > VP9 */
> > +#define V4L2_PIX_FMT_MPEG2_FRAME v4l2_fourcc('M', 'G', '2', 'F') /*
> 
> MPEG2 frame */
> 
> >   /*  Vendor-specific formats   */
> >   #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /*
> > cpia1
> 
> YUV */
> > @@ -1586,6 +1587,7 @@ struct v4l2_ext_control {
> >                  __u8 __user *p_u8;
> >                  __u16 __user *p_u16;
> >                  __u32 __user *p_u32;
> > +               struct v4l2_ctrl_mpeg2_frame_hdr __user
> 
> *p_mpeg2_frame_hdr;
> >                  void __user *ptr;
> >          };
> >   } __attribute__ ((packed));
> > @@ -1631,6 +1633,7 @@ enum v4l2_ctrl_type {
> >          V4L2_CTRL_TYPE_U8            = 0x0100,
> >          V4L2_CTRL_TYPE_U16           = 0x0101,
> >          V4L2_CTRL_TYPE_U32           = 0x0102,
> > +       V4L2_CTRL_TYPE_MPEG2_FRAME_HDR = 0x0109,
> 
> Why 0x0109?

Good catch. I see no reason in particular, so I'll probably make it
0x0103 eventually.

Cheers and thanks for the review!

-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180504/7bc96baf/attachment.sig>

^ permalink raw reply

* [PATCH v1 09/13] drm/kms/mode/sun4i-tv: using helper func drm_display_mode_from_videomode for calculating timing parameters
From: Satendra Singh Thakur @ 2018-05-04  8:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180503110216.63bg3kw7cwppqbzx@flea>

To avoid duplicate logic for horizonal/vertical sync_start/end
helper func drm_display_mode_from_videomode is used

Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
Acked-by: Madhur Verma <madhur.verma@samsung.com>
Cc: Hemanshu Srivastava <hemanshu.s@samsung.com>
---

 v1: Added acked-by fields

 drivers/gpu/drm/sun4i/sun4i_tv.c | 67 +++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c b/drivers/gpu/drm/sun4i/sun4i_tv.c
index b070d52..7ffa930 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
@@ -21,6 +21,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_of.h>
 #include <drm/drm_panel.h>
+#include <video/videomode.h>
 
 #include "sun4i_crtc.h"
 #include "sun4i_drv.h"
@@ -147,16 +148,7 @@ struct tv_mode {
 	u16		front_porch;
 	u16		line_number;
 	u16		vblank_level;
-
-	u32		hdisplay;
-	u16		hfront_porch;
-	u16		hsync_len;
-	u16		hback_porch;
-
-	u32		vdisplay;
-	u16		vfront_porch;
-	u16		vsync_len;
-	u16		vback_porch;
+	struct videomode vm;
 
 	bool		yc_en;
 	bool		dac3_en;
@@ -223,16 +215,16 @@ static const struct tv_mode tv_modes[] = {
 		.back_porch	= 118,
 		.front_porch	= 32,
 		.line_number	= 525,
-
-		.hdisplay	= 720,
-		.hfront_porch	= 18,
-		.hsync_len	= 2,
-		.hback_porch	= 118,
-
-		.vdisplay	= 480,
-		.vfront_porch	= 26,
-		.vsync_len	= 2,
-		.vback_porch	= 17,
+		.vm = {
+			.hactive	= 720,
+			.hfront_porch	= 18,
+			.hsync_len	= 2,
+			.hback_porch	= 118,
+			.vactive	= 480,
+			.vfront_porch	= 26,
+			.vsync_len	= 2,
+			.vback_porch	= 17,
+		},
 
 		.vblank_level	= 240,
 
@@ -249,16 +241,16 @@ static const struct tv_mode tv_modes[] = {
 		.back_porch	= 138,
 		.front_porch	= 24,
 		.line_number	= 625,
-
-		.hdisplay	= 720,
-		.hfront_porch	= 3,
-		.hsync_len	= 2,
-		.hback_porch	= 139,
-
-		.vdisplay	= 576,
-		.vfront_porch	= 28,
-		.vsync_len	= 2,
-		.vback_porch	= 19,
+		.vm = {
+			.hactive	= 720,
+			.hfront_porch	= 3,
+			.hsync_len	= 2,
+			.hback_porch	= 139,
+			.vactive	= 576,
+			.vfront_porch	= 28,
+			.vsync_len	= 2,
+			.vback_porch	= 19,
+		},
 
 		.vblank_level	= 252,
 
@@ -311,9 +303,9 @@ static const struct tv_mode *sun4i_tv_find_tv_by_mode(const struct drm_display_m
 
 		DRM_DEBUG_DRIVER("Comparing mode %s vs %s (X: %d vs %d)",
 				 mode->name, tv_mode->name,
-				 mode->vdisplay, tv_mode->vdisplay);
+				 mode->vdisplay, tv_mode->vm.vactive);
 
-		if (mode->vdisplay == tv_mode->vdisplay)
+		if (mode->vdisplay == tv_mode->vm.vactive)
 			return tv_mode;
 	}
 
@@ -325,19 +317,10 @@ static void sun4i_tv_mode_to_drm_mode(const struct tv_mode *tv_mode,
 {
 	DRM_DEBUG_DRIVER("Creating mode %s\n", mode->name);
 
+	drm_display_mode_from_videomode(&tv_mode->vm, mode);
 	mode->type = DRM_MODE_TYPE_DRIVER;
 	mode->clock = 13500;
 	mode->flags = DRM_MODE_FLAG_INTERLACE;
-
-	mode->hdisplay = tv_mode->hdisplay;
-	mode->hsync_start = mode->hdisplay + tv_mode->hfront_porch;
-	mode->hsync_end = mode->hsync_start + tv_mode->hsync_len;
-	mode->htotal = mode->hsync_end  + tv_mode->hback_porch;
-
-	mode->vdisplay = tv_mode->vdisplay;
-	mode->vsync_start = mode->vdisplay + tv_mode->vfront_porch;
-	mode->vsync_end = mode->vsync_start + tv_mode->vsync_len;
-	mode->vtotal = mode->vsync_end  + tv_mode->vback_porch;
 }
 
 static void sun4i_tv_disable(struct drm_encoder *encoder)
-- 
2.7.4

^ permalink raw reply related

* [PATCH] ARM: dts: da850-lcdk: increase the size of the u-boot nand partition
From: Sekhar Nori @ 2018-05-04  8:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504080835.28632-1-brgl@bgdev.pl>

Hi Bartosz,

On Friday 04 May 2018 01:38 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Current mainline u-boot builds images that are bigger than the 0x80000
> bytes u-boot partition allocated from the NAND memory. Using the free
> space partition results in an unbootable system. Increase the partition
> size to 0xa0000 bytes.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/boot/dts/da850-lcdk.dts | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> index 7de302f890d7..fc8667445892 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -336,11 +336,11 @@
>  				partition at 20000 {
>  					/* The LCDK defaults to booting from this partition */
>  					label = "u-boot";
> -					reg = <0x020000 0x080000>;
> +					reg = <0x020000 0x0a0000>;
>  				};
>  				partition at a0000 {
>  					label = "free space";
> -					reg = <0x0a0000 0>;
> +					reg = <0x0c0000 0>;

Unfortunately, we cannot do this because it introduces incompatibility
(data loss) for users who may have some data stored in "free space".

I should have insisted on a lot of padding for the u-boot partition or
asked not to add partitions to DT at all. For now the only solution is
to use command line partitions. They take precedence over DT partitions.

Thanks,
Sekhar

^ permalink raw reply

* [PATCH] ARM: dts: imx6sx-sabreauto: add debug LED support
From: Shawn Guo @ 2018-05-04  8:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1524812288-799-1-git-send-email-Anson.Huang@nxp.com>

On Fri, Apr 27, 2018 at 02:58:08PM +0800, Anson Huang wrote:
> There is a debug LED(D11) connected to GPIO1_IO24, add
> support for it.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> this patch is based on patch series (ARM: dts: imx6sx-sabreauto: add wdog external reset support).

In this case, you can include these two patches into the series, if you
need to resend the series.

Shawn

>  arch/arm/boot/dts/imx6sx-sabreauto.dts | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6sx-sabreauto.dts b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> index eb1452c..6b94f24 100644
> --- a/arch/arm/boot/dts/imx6sx-sabreauto.dts
> +++ b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> @@ -18,6 +18,18 @@
>  		reg = <0x80000000 0x80000000>;
>  	};
>  
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_led>;
> +
> +		user {
> +			label = "debug";
> +			gpios = <&gpio1 24 GPIO_ACTIVE_HIGH>;
> +			linux,default-trigger = "heartbeat";
> +		};
> +	};
> +
>  	regulators {
>  		compatible = "simple-bus";
>  		#address-cells = <1>;
> @@ -119,6 +131,12 @@
>  &iomuxc {
>  	imx6x-sabreauto {
>  
> +		pinctrl_led: ledgrp {
> +			fsl,pins = <
> +				MX6SX_PAD_CSI_PIXCLK__GPIO1_IO_24 0x17059
> +			>;
> +		};
> +

Try to sort the pinctrl node alphabetically.

Shawn

>  		pinctrl_enet1_1: enet1grp-1 {
>  			fsl,pins = <
>  				MX6SX_PAD_ENET1_MDIO__ENET1_MDIO        0xa0b1
> -- 
> 2.7.4
> 

^ permalink raw reply

* [PATCH v2 05/10] media: v4l: Add definitions for MPEG2 frame format and header metadata
From: Paul Kocialkowski @ 2018-05-04  8:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4e6c4cb7-01b2-3278-4f12-c0ce8cd82ecc@xs4all.nl>

Hi,

On Fri, 2018-04-20 at 15:57 +0200, Hans Verkuil wrote:
> On 04/19/18 17:45, Paul Kocialkowski wrote:
> > Stateless video decoding engines require both the MPEG slices and
> > associated metadata from the video stream in order to decode frames.
> > 
> > This introduces definitions for a new pixel format, describing
> > buffers
> > with MPEG2 slice data, as well as a control structure for passing
> > the
> > frame header (metadata) to drivers.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > Signed-off-by: Florent Revest <florent.revest@free-electrons.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++++++
> >  drivers/media/v4l2-core/v4l2-ioctl.c |  1 +
> >  include/uapi/linux/v4l2-controls.h   | 26
> > ++++++++++++++++++++++++++
> >  include/uapi/linux/videodev2.h       |  3 +++
> >  4 files changed, 40 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> > b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index ba05a8b9a095..fcdc12b9a9e0 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -761,6 +761,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> >  	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:		
> > return "Vertical MV Search Range";
> >  	case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER:		
> > return "Repeat Sequence Header";
> >  	case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:		re
> > turn "Force Key Frame";
> > +	case V4L2_CID_MPEG_VIDEO_MPEG2_FRAME_HDR:		re
> > turn "MPEG2 Frame Header";
> >  
> >  	/* VPX controls */
> >  	case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:		
> > return "VPX Number of Partitions";
> > @@ -1152,6 +1153,9 @@ void v4l2_ctrl_fill(u32 id, const char **name,
> > enum v4l2_ctrl_type *type,
> >  	case V4L2_CID_RDS_TX_ALT_FREQS:
> >  		*type = V4L2_CTRL_TYPE_U32;
> >  		break;
> > +	case V4L2_CID_MPEG_VIDEO_MPEG2_FRAME_HDR:
> > +		*type = V4L2_CTRL_TYPE_MPEG2_FRAME_HDR;
> > +		break;
> >  	default:
> >  		*type = V4L2_CTRL_TYPE_INTEGER;
> >  		break;
> > @@ -1472,6 +1476,9 @@ static int std_validate(const struct v4l2_ctrl
> > *ctrl, u32 idx,
> >  			return -ERANGE;
> >  		return 0;
> >  
> > +	case V4L2_CTRL_TYPE_MPEG2_FRAME_HDR:
> > +		return 0;
> > +
> >  	default:
> >  		return -EINVAL;
> >  	}
> > @@ -2046,6 +2053,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
> > v4l2_ctrl_handler *hdl,
> >  	case V4L2_CTRL_TYPE_U32:
> >  		elem_size = sizeof(u32);
> >  		break;
> > +	case V4L2_CTRL_TYPE_MPEG2_FRAME_HDR:
> > +		elem_size = sizeof(struct
> > v4l2_ctrl_mpeg2_frame_hdr);
> > +		break;
> >  	default:
> >  		if (type < V4L2_CTRL_COMPOUND_TYPES)
> >  			elem_size = sizeof(s32);
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> > b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index 468c3c65362d..8070203da5d2 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1273,6 +1273,7 @@ static void v4l_fill_fmtdesc(struct
> > v4l2_fmtdesc *fmt)
> >  		case V4L2_PIX_FMT_VC1_ANNEX_L:	descr = "VC-1 
> > (SMPTE 412M Annex L)"; break;
> >  		case V4L2_PIX_FMT_VP8:		descr =
> > "VP8"; break;
> >  		case V4L2_PIX_FMT_VP9:		descr =
> > "VP9"; break;
> > +		case V4L2_PIX_FMT_MPEG2_FRAME:	descr =
> > "MPEG2 Frame"; break;
> >  		case V4L2_PIX_FMT_CPIA1:	descr = "GSPCA CPiA
> > YUV"; break;
> >  		case V4L2_PIX_FMT_WNVA:		descr =
> > "WNVA"; break;
> >  		case V4L2_PIX_FMT_SN9C10X:	descr = "GSPCA
> > SN9C10X"; break;
> > diff --git a/include/uapi/linux/v4l2-controls.h
> > b/include/uapi/linux/v4l2-controls.h
> > index cbbb750d87d1..8431b2a540c7 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -557,6 +557,8 @@ enum v4l2_mpeg_video_mpeg4_profile {
> >  };
> >  #define V4L2_CID_MPEG_VIDEO_MPEG4_QPEL		(V4L2_CID_MPE
> > G_BASE+407)
> >  
> > +#define
> > V4L2_CID_MPEG_VIDEO_MPEG2_FRAME_HDR     (V4L2_CID_MPEG_BASE+450)
> > +
> >  /*  Control IDs for VP8 streams
> >   *  Although VP8 is not part of MPEG we add these controls to the
> > MPEG class
> >   *  as that class is already handling other video compression
> > standards
> > @@ -985,4 +987,28 @@ enum v4l2_detect_md_mode {
> >  #define V4L2_CID_DETECT_MD_THRESHOLD_GRID	(V4L2_CID_DETECT_C
> > LASS_BASE + 3)
> >  #define V4L2_CID_DETECT_MD_REGION_GRID		(V4L2_CID_DET
> > ECT_CLASS_BASE + 4)
> >  
> > +struct v4l2_ctrl_mpeg2_frame_hdr {
> > +	__u32 slice_len;
> > +	__u32 slice_pos;
> > +	enum { MPEG1, MPEG2 } type;
> > +
> > +	__u16 width;
> > +	__u16 height;
> > +
> > +	enum { PCT_I = 1, PCT_P, PCT_B, PCT_D }
> > picture_coding_type;
> 
> As someone else already mentioned (I believe): avoid enums. Use __u16
> instead.

Indeed. I have taken a note of that.

> > +	__u8 f_code[2][2];
> > +
> > +	__u8 intra_dc_precision;
> > +	__u8 picture_structure;
> > +	__u8 top_field_first;
> > +	__u8 frame_pred_frame_dct;
> > +	__u8 concealment_motion_vectors;
> > +	__u8 q_scale_type;
> > +	__u8 intra_vlc_format;
> > +	__u8 alternate_scan;
> > +
> > +	__u8 backward_ref_index;
> > +	__u8 forward_ref_index;
> > +};
> 
> Please test that the layout and size of this structure is identical
> for 32 and 64 bit
> architectures, both on Intel and ARM.
> 
> It looks good, but it doesn't hurt to check.

Right, I'll keep that in mind and make sure this is the case.

Cheers and thanks for the review!

Paul

> Regards,
> 
> 	Hans
> 
> > +
> >  #endif
> > diff --git a/include/uapi/linux/videodev2.h
> > b/include/uapi/linux/videodev2.h
> > index 31b5728b56e9..4b8336f7bcf0 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -635,6 +635,7 @@ struct v4l2_pix_format {
> >  #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /*
> > SMPTE 421M Annex L compliant stream */
> >  #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /*
> > VP8 */
> >  #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /*
> > VP9 */
> > +#define V4L2_PIX_FMT_MPEG2_FRAME v4l2_fourcc('M', 'G', '2', 'F') /*
> > MPEG2 frame */
> >  
> >  /*  Vendor-specific formats   */
> >  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /*
> > cpia1 YUV */
> > @@ -1586,6 +1587,7 @@ struct v4l2_ext_control {
> >  		__u8 __user *p_u8;
> >  		__u16 __user *p_u16;
> >  		__u32 __user *p_u32;
> > +		struct v4l2_ctrl_mpeg2_frame_hdr __user
> > *p_mpeg2_frame_hdr;
> >  		void __user *ptr;
> >  	};
> >  } __attribute__ ((packed));
> > @@ -1631,6 +1633,7 @@ enum v4l2_ctrl_type {
> >  	V4L2_CTRL_TYPE_U8	     = 0x0100,
> >  	V4L2_CTRL_TYPE_U16	     = 0x0101,
> >  	V4L2_CTRL_TYPE_U32	     = 0x0102,
> > +	V4L2_CTRL_TYPE_MPEG2_FRAME_HDR = 0x0109,
> >  };
> >  
> >  /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
> > 
> 
> 
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180504/629ee026/attachment.sig>

^ permalink raw reply

* [PATCH v2 05/10] media: v4l: Add definitions for MPEG2 frame format and header metadata
From: Paul Kocialkowski @ 2018-05-04  8:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180424090150.rtbejfjaddd6wysk@paasikivi.fi.intel.com>

Hi,

On Tue, 2018-04-24 at 12:01 +0300, Sakari Ailus wrote:
> Hi Paul,
> 
> On Thu, Apr 19, 2018 at 05:45:31PM +0200, Paul Kocialkowski wrote:
> > Stateless video decoding engines require both the MPEG slices and
> > associated metadata from the video stream in order to decode frames.
> > 
> > This introduces definitions for a new pixel format, describing
> > buffers
> > with MPEG2 slice data, as well as a control structure for passing
> > the
> > frame header (metadata) to drivers.
> 
> What's the typical relationship between MPEG2 slice data and the
> header?
> 
> Are the two always used together, do they originate from the same
> source, for instance? I have to admit I'm not very familiar with
> MPEG...

Yes, the header is closely related to the slice data, as it expresses
the metadata required for properly decoding the slice data. Both are
extracted from the MPEG bitstream and there is dedicated metadata for
each new set of slices that is decoded to a single frame.

> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > Signed-off-by: Florent Revest <florent.revest@free-electrons.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++++++
> >  drivers/media/v4l2-core/v4l2-ioctl.c |  1 +
> >  include/uapi/linux/v4l2-controls.h   | 26
> > ++++++++++++++++++++++++++
> >  include/uapi/linux/videodev2.h       |  3 +++
> >  4 files changed, 40 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> > b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index ba05a8b9a095..fcdc12b9a9e0 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -761,6 +761,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> >  	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:		
> > return "Vertical MV Search Range";
> >  	case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER:		
> > return "Repeat Sequence Header";
> >  	case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:		re
> > turn "Force Key Frame";
> > +	case V4L2_CID_MPEG_VIDEO_MPEG2_FRAME_HDR:		re
> > turn "MPEG2 Frame Header";
> >  
> >  	/* VPX controls */
> >  	case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:		
> > return "VPX Number of Partitions";
> > @@ -1152,6 +1153,9 @@ void v4l2_ctrl_fill(u32 id, const char **name,
> > enum v4l2_ctrl_type *type,
> >  	case V4L2_CID_RDS_TX_ALT_FREQS:
> >  		*type = V4L2_CTRL_TYPE_U32;
> >  		break;
> > +	case V4L2_CID_MPEG_VIDEO_MPEG2_FRAME_HDR:
> > +		*type = V4L2_CTRL_TYPE_MPEG2_FRAME_HDR;
> > +		break;
> >  	default:
> >  		*type = V4L2_CTRL_TYPE_INTEGER;
> >  		break;
> > @@ -1472,6 +1476,9 @@ static int std_validate(const struct v4l2_ctrl
> > *ctrl, u32 idx,
> >  			return -ERANGE;
> >  		return 0;
> >  
> > +	case V4L2_CTRL_TYPE_MPEG2_FRAME_HDR:
> > +		return 0;
> > +
> >  	default:
> >  		return -EINVAL;
> >  	}
> > @@ -2046,6 +2053,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
> > v4l2_ctrl_handler *hdl,
> >  	case V4L2_CTRL_TYPE_U32:
> >  		elem_size = sizeof(u32);
> >  		break;
> > +	case V4L2_CTRL_TYPE_MPEG2_FRAME_HDR:
> > +		elem_size = sizeof(struct
> > v4l2_ctrl_mpeg2_frame_hdr);
> > +		break;
> >  	default:
> >  		if (type < V4L2_CTRL_COMPOUND_TYPES)
> >  			elem_size = sizeof(s32);
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> > b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index 468c3c65362d..8070203da5d2 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1273,6 +1273,7 @@ static void v4l_fill_fmtdesc(struct
> > v4l2_fmtdesc *fmt)
> >  		case V4L2_PIX_FMT_VC1_ANNEX_L:	descr = "VC-1 
> > (SMPTE 412M Annex L)"; break;
> >  		case V4L2_PIX_FMT_VP8:		descr =
> > "VP8"; break;
> >  		case V4L2_PIX_FMT_VP9:		descr =
> > "VP9"; break;
> > +		case V4L2_PIX_FMT_MPEG2_FRAME:	descr =
> > "MPEG2 Frame"; break;
> >  		case V4L2_PIX_FMT_CPIA1:	descr = "GSPCA CPiA
> > YUV"; break;
> >  		case V4L2_PIX_FMT_WNVA:		descr =
> > "WNVA"; break;
> >  		case V4L2_PIX_FMT_SN9C10X:	descr = "GSPCA
> > SN9C10X"; break;
> > diff --git a/include/uapi/linux/v4l2-controls.h
> > b/include/uapi/linux/v4l2-controls.h
> > index cbbb750d87d1..8431b2a540c7 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -557,6 +557,8 @@ enum v4l2_mpeg_video_mpeg4_profile {
> >  };
> >  #define V4L2_CID_MPEG_VIDEO_MPEG4_QPEL		(V4L2_CID_MPE
> > G_BASE+407)
> >  
> > +#define
> > V4L2_CID_MPEG_VIDEO_MPEG2_FRAME_HDR     (V4L2_CID_MPEG_BASE+450)
> > +
> >  /*  Control IDs for VP8 streams
> >   *  Although VP8 is not part of MPEG we add these controls to the
> > MPEG class
> >   *  as that class is already handling other video compression
> > standards
> > @@ -985,4 +987,28 @@ enum v4l2_detect_md_mode {
> >  #define V4L2_CID_DETECT_MD_THRESHOLD_GRID	(V4L2_CID_DETECT_C
> > LASS_BASE + 3)
> >  #define V4L2_CID_DETECT_MD_REGION_GRID		(V4L2_CID_DET
> > ECT_CLASS_BASE + 4)
> >  
> > +struct v4l2_ctrl_mpeg2_frame_hdr {
> > +	__u32 slice_len;
> > +	__u32 slice_pos;
> > +	enum { MPEG1, MPEG2 } type;
> > +
> > +	__u16 width;
> > +	__u16 height;
> > +
> > +	enum { PCT_I = 1, PCT_P, PCT_B, PCT_D }
> > picture_coding_type;
> > +	__u8 f_code[2][2];
> > +
> > +	__u8 intra_dc_precision;
> > +	__u8 picture_structure;
> > +	__u8 top_field_first;
> > +	__u8 frame_pred_frame_dct;
> > +	__u8 concealment_motion_vectors;
> > +	__u8 q_scale_type;
> > +	__u8 intra_vlc_format;
> > +	__u8 alternate_scan;
> > +
> > +	__u8 backward_ref_index;
> > +	__u8 forward_ref_index;
> > +};
> 
> Besides the comments that have been given, please add
> __attribute__((packed)) to the struct definition.

Noted.

> > +
> >  #endif
> > diff --git a/include/uapi/linux/videodev2.h
> > b/include/uapi/linux/videodev2.h
> > index 31b5728b56e9..4b8336f7bcf0 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -635,6 +635,7 @@ struct v4l2_pix_format {
> >  #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /*
> > SMPTE 421M Annex L compliant stream */
> >  #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /*
> > VP8 */
> >  #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /*
> > VP9 */
> > +#define V4L2_PIX_FMT_MPEG2_FRAME v4l2_fourcc('M', 'G', '2', 'F') /*
> > MPEG2 frame */
> 
> This format needs documentation.

Noted too. Thanks for the review!

> >  
> >  /*  Vendor-specific formats   */
> >  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /*
> > cpia1 YUV */
> > @@ -1586,6 +1587,7 @@ struct v4l2_ext_control {
> >  		__u8 __user *p_u8;
> >  		__u16 __user *p_u16;
> >  		__u32 __user *p_u32;
> > +		struct v4l2_ctrl_mpeg2_frame_hdr __user
> > *p_mpeg2_frame_hdr;
> >  		void __user *ptr;
> >  	};
> >  } __attribute__ ((packed));
> > @@ -1631,6 +1633,7 @@ enum v4l2_ctrl_type {
> >  	V4L2_CTRL_TYPE_U8	     = 0x0100,
> >  	V4L2_CTRL_TYPE_U16	     = 0x0101,
> >  	V4L2_CTRL_TYPE_U32	     = 0x0102,
> > +	V4L2_CTRL_TYPE_MPEG2_FRAME_HDR = 0x0109,
> >  };
> >  
> >  /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
> > -- 
> > 2.16.3
> > 
> 
> 
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180504/618ed7c4/attachment-0001.sig>

^ permalink raw reply

* [PATCH 4/4] ARM: dts: imx: drop 'clocks' container for board level clocks
From: Lothar Waßmann @ 2018-05-04  8:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525395446-1953-5-git-send-email-shawnguo@kernel.org>

Hi,

On Fri,  4 May 2018 08:57:26 +0800 Shawn Guo wrote:
> For soc level of clocks which are input to Clock Control Module, clock
> driver expects them to be under 'clocks' container node.  But for board
> level clocks, this container is not really necessary.  Let's drop it and
> use an unique name for fixed rate clock, so that 'reg' property can be
> saved as well.
> 
For imx28-tx28.dts and imx53-tx53.dtsi:
Acked-By: Lothar Wa?mann <LW@KARO-electronics.de>

> Signed-off-by: Shawn Guo <shawnguo@kernel.org>
> ---
>  arch/arm/boot/dts/imx28-tx28.dts             | 13 ++++---------
>  arch/arm/boot/dts/imx53-tx53.dtsi            | 11 +++++------
>  arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts | 15 ++++++---------
>  arch/arm/boot/dts/imx6q-bx50v3.dtsi          | 11 ++++-------
>  4 files changed, 19 insertions(+), 31 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx28-tx28.dts b/arch/arm/boot/dts/imx28-tx28.dts
> index 687186358c18..e4f19f9be6d0 100644
> --- a/arch/arm/boot/dts/imx28-tx28.dts
> +++ b/arch/arm/boot/dts/imx28-tx28.dts
> @@ -140,15 +140,10 @@
>  		regulator-boot-on;
>  	};
>  
> -	clocks {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		mclk: clock at 0 {
> -			compatible = "fixed-clock";
> -			reg = <0>;
> -			#clock-cells = <0>;
> -			clock-frequency = <26000000>;
> -		};
> +	mclk: clock-mclk {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <26000000>;
>  	};
>  
>  	sound {
> diff --git a/arch/arm/boot/dts/imx53-tx53.dtsi b/arch/arm/boot/dts/imx53-tx53.dtsi
> index 77f2ce9f1e41..5dd3dd3a2fd0 100644
> --- a/arch/arm/boot/dts/imx53-tx53.dtsi
> +++ b/arch/arm/boot/dts/imx53-tx53.dtsi
> @@ -67,13 +67,12 @@
>  		ckih1 {
>  			clock-frequency = <0>;
>  		};
> +	};
>  
> -		mclk: clock at 0 {
> -			compatible = "fixed-clock";
> -			reg = <0>;
> -			#clock-cells = <0>;
> -			clock-frequency = <26000000>;
> -		};
> +	mclk: clock-mclk {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <26000000>;
>  	};
>  
>  	gpio-keys {
> diff --git a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> index ea184d108491..3dee3af1a4c1 100644
> --- a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> +++ b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> @@ -72,15 +72,12 @@
>  		stdout-path = "serial0:115200n8";
>  	};
>  
> -	clocks {
> -		/* Fixed crystal dedicated to mcp251x */
> -		clk16m: clk at 1 {
> -			compatible = "fixed-clock";
> -			reg = <1>;
> -			#clock-cells = <0>;
> -			clock-frequency = <16000000>;
> -			clock-output-names = "clk16m";
> -		};
> +	/* Fixed crystal dedicated to mcp251x */
> +	clk16m: clock-16m {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <16000000>;
> +		clock-output-names = "clk16m";
>  	};
>  
>  	gpio-keys {
> diff --git a/arch/arm/boot/dts/imx6q-bx50v3.dtsi b/arch/arm/boot/dts/imx6q-bx50v3.dtsi
> index d0e7dab4d72b..d3cba09be0cb 100644
> --- a/arch/arm/boot/dts/imx6q-bx50v3.dtsi
> +++ b/arch/arm/boot/dts/imx6q-bx50v3.dtsi
> @@ -43,13 +43,10 @@
>  #include "imx6q-ba16.dtsi"
>  
>  / {
> -	clocks {
> -		mclk: clock at 0 {
> -			compatible = "fixed-clock";
> -			reg = <0>;
> -			#clock-cells = <0>;
> -			clock-frequency = <22000000>;
> -		};
> +	mclk: clock-mclk {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <22000000>;
>  	};
>  
>  	gpio-poweroff {


Lothar Wa?mann

^ permalink raw reply

* [RFC PATCH] ARM: dts: imx: replace underscore with hyphen in aliases name
From: Shawn Guo @ 2018-05-04  8:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504074025.GA11186@g751.home>

On Fri, May 04, 2018 at 09:40:25AM +0200, Gary Bisson wrote:
> Hi Shawn,
> 
> On Fri, May 04, 2018 at 09:58:48AM +0800, Shawn Guo wrote:
> > It replaces underscore with hyphen in aliases name to fix DTC
> > alias_paths warning below, which is seen with various i.MX board dts
> > files when W=1 switch is on.
> > 
> >  Warning (alias_paths): /aliases: aliases property name must include only lowercase and '-'
> > 
> > Cc: Lothar Wa?mann <LW@KARO-electronics.de>
> > Cc: Gary Bisson <gary.bisson@boundarydevices.com>
> > Signed-off-by: Shawn Guo <shawnguo@kernel.org>
> > ---
> > Hi Lothar, Gary,
> > 
> > Per my search on kernel code, this change will not break anything on
> > kernel support.  But I'm not really sure about the case of bootloaders
> > running on the boards.  That's why I'm sending it as RFC.
> > 
> > We really want i.MX to be the first-class citizen in the community.
> > After a few cycles efforts on cleaning up DTC warnings, this alias_paths
> > warning is the last one we see with i.MX dts files.  We really want to
> > get rid of it and make i.MX dts be DTC warning free.
> > 
> > So please comment whether we can merge this patch.
> 
> Yes that is fine.
> Reviewed-by: Gary Bisson <gary.bisson@boundarydevices.com>

Thanks, Gary.

Shawn

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox