Linux-mediatek Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v15 4/4] arm: dts: mt2701: Use real clock for UARTs
From: Matthias Brugger @ 2016-11-09 17:20 UTC (permalink / raw)
  To: Erin Lo, Mike Turquette, Stephen Boyd, Rob Herring
  Cc: Arnd Bergmann, Sascha Hauer, Daniel Kurtz, Philipp Zabel,
	devicetree, linux-arm-kernel, linux-kernel, linux-mediatek,
	linux-clk, srv_heupstream, ms.chen, robert.chou
In-Reply-To: <1478245388-1412-5-git-send-email-erin.lo@mediatek.com>



On 11/04/2016 08:43 AM, Erin Lo wrote:
> We used to use a fixed rate clock for the UARTs. Now that we have clock
> support we can associate the correct clocks to the UARTs and drop the
> 26MHz fixed rate UART clock.
>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>

Applied, thanks.

> ---
>  arch/arm/boot/dts/mt2701.dtsi | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
> index c9a8dbf..7eab6f4 100644
> --- a/arch/arm/boot/dts/mt2701.dtsi
> +++ b/arch/arm/boot/dts/mt2701.dtsi
> @@ -73,12 +73,6 @@
>  		#clock-cells = <0>;
>  	};
>
> -	uart_clk: dummy26m {
> -		compatible = "fixed-clock";
> -		clock-frequency = <26000000>;
> -		#clock-cells = <0>;
> -	};
> -
>  	clk26m: oscillator@0 {
>  		compatible = "fixed-clock";
>  		#clock-cells = <0>;
> @@ -186,7 +180,8 @@
>  			     "mediatek,mt6577-uart";
>  		reg = <0 0x11002000 0 0x400>;
>  		interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
> -		clocks = <&uart_clk>;
> +		clocks = <&pericfg CLK_PERI_UART0_SEL>, <&pericfg CLK_PERI_UART0>;
> +		clock-names = "baud", "bus";
>  		status = "disabled";
>  	};
>
> @@ -195,7 +190,8 @@
>  			     "mediatek,mt6577-uart";
>  		reg = <0 0x11003000 0 0x400>;
>  		interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_LOW>;
> -		clocks = <&uart_clk>;
> +		clocks = <&pericfg CLK_PERI_UART1_SEL>, <&pericfg CLK_PERI_UART1>;
> +		clock-names = "baud", "bus";
>  		status = "disabled";
>  	};
>
> @@ -204,7 +200,8 @@
>  			     "mediatek,mt6577-uart";
>  		reg = <0 0x11004000 0 0x400>;
>  		interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_LOW>;
> -		clocks = <&uart_clk>;
> +		clocks = <&pericfg CLK_PERI_UART2_SEL>, <&pericfg CLK_PERI_UART2>;
> +		clock-names = "baud", "bus";
>  		status = "disabled";
>  	};
>
> @@ -213,7 +210,8 @@
>  			     "mediatek,mt6577-uart";
>  		reg = <0 0x11005000 0 0x400>;
>  		interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_LOW>;
> -		clocks = <&uart_clk>;
> +		clocks = <&pericfg CLK_PERI_UART3_SEL>, <&pericfg CLK_PERI_UART3>;
> +		clock-names = "baud", "bus";
>  		status = "disabled";
>  	};
>  };
>

^ permalink raw reply

* Re: [PATCH v2 1/3] binding: irqchip: mtk-cirq: Add binding document
From: Rob Herring @ 2016-11-09 18:26 UTC (permalink / raw)
  To: Youlin Pei
  Cc: Marc Zyngier, Matthias Brugger, Thomas Gleixner, Jason Cooper,
	Mark Rutland, Russell King, linux-kernel, devicetree,
	linux-arm-kernel, linux-mediatek, srv_heupstream, hongkun.cao,
	yong.wu, erin.lo, chieh-jay.liu
In-Reply-To: <1478001122-8664-2-git-send-email-youlin.pei@mediatek.com>

On Tue, Nov 01, 2016 at 07:52:00PM +0800, Youlin Pei wrote:
> This commit adds the device tree binding document for
> the mediatek cirq.
> 
> Signed-off-by: Youlin Pei <youlin.pei@mediatek.com>
> ---
>  .../interrupt-controller/mediatek,cirq.txt         |   30 ++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt b/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
> new file mode 100644
> index 0000000..84e8123
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
> @@ -0,0 +1,30 @@
> +* Mediatek 27xx cirq
> +
> +In Mediatek SOCs, the CIRQ is a low power interrupt controller designed to
> +works outside MCUSYS which comprises with Cortex-Ax cores,CCI and GIC.

s/works/work/

> +The external interrupts (outside MCUSYS) will feed through CIRQ and connect
> +to GIC in MCUSYS. When CIRQ is enabled, it will record the edge-sensitive
> +interrupts and generated a pulse signal to parent interrupt controller when

s/generated/generate/

> +flush command is executed. With CIRQ, MCUSYS can be completely turned off
> +to improve the system power consumption without losing interrupts.
> +
> +Required properties:
> +- compatible: should be: "mediatek,mtk-cirq".

This should be SoC specific. This is fine as a fallback if the same 
block is in many SoCs, but mediatek and mtk is a bit redundant.

> +- interrupt-controller : Identifies the node as an interrupt controller.
> +- #interrupt-cells : Use the same format as specified by GIC in arm,gic.txt.
> +- interrupt-parent: phandle of irq parent for cirq. The parent must
> +  use the same interrupt-cells format as GIC.
> +- reg: Physical base address of the cirq registers and length of memory
> +  mapped region.
> +- mediatek,ext-irq-start: Identifies external irq start number in different
> +  SOCs.

Wouldn't this always be 32 if the GIC is the parent? If 32 is the common 
case, then use the SoC compatible to determine this value.

> +
> +Example:
> +	cirq: interrupt-controller@10204000 {
> +		compatible = "mediatek,mtk-cirq";
> +		interrupt-controller;
> +		#interrupt-cells = <3>;
> +		interrupt-parent = <&sysirq>;
> +		reg = <0 0x10204000 0 0x4000>;
> +		mediatek,ext-irq-start = <32>;
> +	};
> -- 
> 1.7.9.5
> 

^ permalink raw reply

* Re: [PATCH 4/4] dts: arm64: enable mmc3 for supporting sdio feature
From: Yingjoe Chen @ 2016-11-10  2:08 UTC (permalink / raw)
  To: Yong Mao
  Cc: Ulf Hansson, Mark Rutland, devicetree, YH Huang, Nicolas Boichat,
	Mathias Nyman, srv_heupstream, Catalin Marinas, linux-mediatek,
	Will Deacon, Douglas Anderson, linux-kernel, Chunfeng Yun,
	Rob Herring, Geert Uytterhoeven, linux-arm-kernel, Philipp Zabel,
	Matthias Brugger, linux-mmc, Eddie Huang
In-Reply-To: <1478585341-6749-5-git-send-email-yong.mao@mediatek.com>

On Tue, 2016-11-08 at 14:09 +0800, Yong Mao wrote:
> From: yong mao <yong.mao@mediatek.com>
> 
> Add description of mmc3 for supporting sdio feature
> 
> Signed-off-by: Yong Mao <yong.mao@mediatek.com>
> Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8173-evb.dts |   82 +++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
> index 2a7f731..4dbd299 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
> @@ -43,6 +43,14 @@
>  		enable-active-high;
>  	};
>  
> +	sdio_fixed_3v3: fixedregulator@0 {

This should be regulator@1 instead of fixedregulator.

> +		compatible = "regulator-fixed";
> +		regulator-name = "3V3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&pio 85 GPIO_ACTIVE_HIGH>;
> +	};
> +
>  	connector {
>  		compatible = "hdmi-connector";
>  		label = "hdmi";
> @@ -139,6 +147,25 @@
>  	vqmmc-supply = <&mt6397_vmc_reg>;
>  };
>  
> +&mmc3 {
> +	status = "okay";
> +	pinctrl-names = "default", "state_uhs";
> +	pinctrl-0 = <&mmc3_pins_default>;
> +	pinctrl-1 = <&mmc3_pins_uhs>;
> +	bus-width = <4>;
> +	max-frequency = <200000000>;
> +	cap-sd-highspeed;
> +	sd-uhs-sdr50;
> +	sd-uhs-sdr104;
> +	sdr104-clk-delay = <5>;
> +	keep-power-in-suspend;
> +	enable-sdio-wakeup;
> +	cap-sdio-irq;
> +	vmmc-supply = <&sdio_fixed_3v3>;
> +	vqmmc-supply = <&mt6397_vgp3_reg>;
> +	non-removable;
> +};
> +
>  &pio {
>  	disp_pwm0_pins: disp_pwm0_pins {
>  		pins1 {
> @@ -197,6 +224,36 @@
>  		};
>  	};
>  
> +	mmc3_pins_default: mmc3default {

Please keep nodes in &pio sorted, move this one after mmc1_pins_uhs.

> +		pins_dat {
> +			pinmux = <MT8173_PIN_22_MSDC3_DAT0__FUNC_MSDC3_DAT0>,
> +				 <MT8173_PIN_23_MSDC3_DAT1__FUNC_MSDC3_DAT1>,
> +				 <MT8173_PIN_24_MSDC3_DAT2__FUNC_MSDC3_DAT2>,
> +				 <MT8173_PIN_25_MSDC3_DAT3__FUNC_MSDC3_DAT3>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		pins_cmd {
> +			pinmux = <MT8173_PIN_27_MSDC3_CMD__FUNC_MSDC3_CMD>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		pins_clk {
> +			pinmux = <MT8173_PIN_26_MSDC3_CLK__FUNC_MSDC3_CLK>;
> +			bias-pull-down;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +		};
> +
> +		pins_pdn {
> +			pinmux = <MT8173_PIN_85_AUD_DAT_MOSI__FUNC_GPIO85>;
> +			output-low;
> +		};

This one is used in regulator, not really an mmc pin.
Also, you don't need to add node for gpio usage, request_gpio will set
mode for you.
So please remove pins_pdn node.


> +	};
> +
>  	mmc0_pins_uhs: mmc0 {
>  		pins_cmd_dat {
>  			pinmux = <MT8173_PIN_57_MSDC0_DAT0__FUNC_MSDC0_DAT0>,
> @@ -243,6 +300,31 @@
>  			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
>  		};
>  	};
> +
> +	mmc3_pins_uhs: mmc3 {


Please keep nodes in &pio sorted, move this one after mmc1_pins_uhs.


Joe.C

> +		pins_dat {
> +			pinmux = <MT8173_PIN_22_MSDC3_DAT0__FUNC_MSDC3_DAT0>,
> +				 <MT8173_PIN_23_MSDC3_DAT1__FUNC_MSDC3_DAT1>,
> +				 <MT8173_PIN_24_MSDC3_DAT2__FUNC_MSDC3_DAT2>,
> +				 <MT8173_PIN_25_MSDC3_DAT3__FUNC_MSDC3_DAT3>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		pins_cmd {
> +			pinmux = <MT8173_PIN_27_MSDC3_CMD__FUNC_MSDC3_CMD>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		pins_clk {
> +			pinmux = <MT8173_PIN_26_MSDC3_CLK__FUNC_MSDC3_CLK>;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +	};
>  };
>  
>  &pwm0 {

^ permalink raw reply

* Re: [bug report] [media] vcodec: mediatek: Add Mediatek V4L2 Video Decoder Driver
From: Tiffany Lin @ 2016-11-10  4:31 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Dan Carpenter, linux-media, linux-mediatek
In-Reply-To: <9794dab1-94ba-c384-85c5-edb8831810ff@xs4all.nl>

Hi Hans, Dan,

On Wed, 2016-11-09 at 14:45 +0100, Hans Verkuil wrote:
> On 11/09/16 14:28, Dan Carpenter wrote:
> > Hello Tiffany Lin,
> >
> > The patch 590577a4e525: "[media] vcodec: mediatek: Add Mediatek V4L2
> > Video Decoder Driver" from Sep 2, 2016, leads to the following static
> > checker warning:
> >
> > 	drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c:536 vidioc_vdec_qbuf()
> > 	error: buffer overflow 'vq->bufs' 32 <= u32max
> >
> > drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
> >    520  static int vidioc_vdec_qbuf(struct file *file, void *priv,
> >    521                              struct v4l2_buffer *buf)
> >    522  {
> >    523          struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
> >    524          struct vb2_queue *vq;
> >    525          struct vb2_buffer *vb;
> >    526          struct mtk_video_dec_buf *mtkbuf;
> >    527          struct vb2_v4l2_buffer  *vb2_v4l2;
> >    528
> >    529          if (ctx->state == MTK_STATE_ABORT) {
> >    530                  mtk_v4l2_err("[%d] Call on QBUF after unrecoverable error",
> >    531                                  ctx->id);
> >    532                  return -EIO;
> >    533          }
> >    534
> >    535          vq = v4l2_m2m_get_vq(ctx->m2m_ctx, buf->type);
> >    536          vb = vq->bufs[buf->index];
> >
> > Smatch thinks that "buf->index" comes straight from the user without
> > being checked and that this is a buffer overflow.  It seems simple
> > enough to analyse the call tree.
> >
> > __video_do_ioctl()
> > ->  v4l_qbuf()
> >   -> vidioc_vdec_qbuf()
> >
> > It seems like Smatch is correct.  I looked at a different implementation
> > of this and that one wasn't checked either so maybe there is something
> > I am not seeing.
> >
> > This has obvious security implications.  Can someone take a look at
> > this?
> 
> This is indeed wrong.
> 
> The v4l2_m2m_qbuf() call at the end of this function calls in turn 
> vb2_qbuf which
> will check the index. But if you override vidioc_qbuf (or 
> vidioc_prepare), then
> you need to check the index value.
> 
> I double-checked all cases where vidioc_qbuf was set to a 
> driver-specific function
> and this is the only driver that doesn't check the index field. In all 
> other cases
> it is either checked, or it is not used before calling into the vb1/vb2 
> framework
> which checks this.
> 
> So luckily this only concerns this driver.

Thanks for point out this issue.
As Hans' mentioned, our driver access index field in v4l2_buffer before
framework check buffer index.
We will prepare patch for this issue.


best regards,
Tiffany


> 
> Regards,
> 
> 	Hans
> 
> >
> >    537          vb2_v4l2 = container_of(vb, struct vb2_v4l2_buffer, vb2_buf);
> >    538          mtkbuf = container_of(vb2_v4l2, struct mtk_video_dec_buf, vb);
> >    539
> >    540          if ((buf->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) &&
> >    541              (buf->m.planes[0].bytesused == 0)) {
> >    542                  mtkbuf->lastframe = true;
> >    543                  mtk_v4l2_debug(1, "[%d] (%d) id=%d lastframe=%d (%d,%d, %d) vb=%p",
> >    544                           ctx->id, buf->type, buf->index,
> >    545                           mtkbuf->lastframe, buf->bytesused,
> >    546                           buf->m.planes[0].bytesused, buf->length,
> >    547                           vb);
> >    548          }
> >    549
> >    550          return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> >    551  }
> >
> > regards,
> > dan carpenter
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-media" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >

^ permalink raw reply

* [PATCH v1] mtk-vcodec: add index check in decoder vidioc_qbuf
From: Wu-Cheng Li @ 2016-11-10  5:24 UTC (permalink / raw)
  To: pawel, tiffany.lin, andrew-ct.chen, mchehab, matthias.bgg,
	djkurtz, dan.carpenter
  Cc: linux-media, linux-arm-kernel, linux-mediatek, linux-kernel,
	Wu-Cheng Li

From: Wu-Cheng Li <wuchengli@google.com>

This patch adds a buffer index check in decoder vidioc_qbuf.

Wu-Cheng Li (1):
  mtk-vcodec: add index check in decoder vidioc_qbuf.

 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c | 4 ++++
 1 file changed, 4 insertions(+)

-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply

* [PATCH v1] mtk-vcodec: add index check in decoder vidioc_qbuf.
From: Wu-Cheng Li @ 2016-11-10  5:24 UTC (permalink / raw)
  To: pawel, tiffany.lin, andrew-ct.chen, mchehab, matthias.bgg,
	djkurtz, dan.carpenter
  Cc: linux-media, linux-arm-kernel, linux-mediatek, linux-kernel,
	Wu-Cheng Li, Wu-Cheng Li
In-Reply-To: <1478755445-23494-1-git-send-email-wuchengli@chromium.org>

From: Wu-Cheng Li <wuchengli@google.com>

vb2_qbuf will check the buffer index. If a driver overrides
vidioc_qbuf and use the buffer index, the driver needs to check
the index.

Signed-off-by: Wu-Cheng Li <wuchengli@chromium.org>
---
 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
index 0520919..0746592 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
@@ -533,6 +533,10 @@ static int vidioc_vdec_qbuf(struct file *file, void *priv,
 	}
 
 	vq = v4l2_m2m_get_vq(ctx->m2m_ctx, buf->type);
+	if (buf->index >= vq->num_buffers) {
+		mtk_v4l2_debug(1, "buffer index %d out of range", buf->index);
+		return -EINVAL;
+	}
 	vb = vq->bufs[buf->index];
 	vb2_v4l2 = container_of(vb, struct vb2_v4l2_buffer, vb2_buf);
 	mtkbuf = container_of(vb2_v4l2, struct mtk_video_dec_buf, vb);
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related

* Re: [PATCH 1/2] iommu/mediatek: Convert M4Uv2 to iommu_fwspec
From: Joerg Roedel @ 2016-11-10 11:00 UTC (permalink / raw)
  To: Robin Murphy
  Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <0205bf6404b16bdebe8039bfc65570a2a6f9f960.1476704508.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>

On Mon, Oct 17, 2016 at 12:49:20PM +0100, Robin Murphy wrote:
> Our per-device data consists of the M4U instance and firmware-provided
> list of LARB IDs, which is a perfect fit for the generic iommu_fwspec
> machinery. Use that directly as a simpler alternative to the custom
> archdata code.
> 
> CC: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> ---
> 
> These are fairly mechanical cleanups, so I'm pretty confident, but it
> still bears mentioning that they're only compile-tested as I don't have
> the relevant hardware.
> 
> Robin.
> 
>  drivers/iommu/mtk_iommu.c | 75 ++++++++++++-----------------------------------
>  1 file changed, 18 insertions(+), 57 deletions(-)

Applied both, thanks Robin.

^ permalink raw reply

* Re: [PATCH v16 0/5] Mediatek MT8173 CMDQ support
From: Horng-Shyang Liao @ 2016-11-10 11:15 UTC (permalink / raw)
  To: Rob Herring, Jassi Brar, Matthias Brugger
  Cc: Daniel Kurtz, Sascha Hauer, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w, Sascha Hauer,
	Philipp Zabel, Nicolas Boichat, CK HU, cawa cheng, Bibby Hsieh,
	YT Shen, Daoyuan Huang, Damon Chu, Josh-YC Liu, Glory Hung,
	Jiaguang Zhang, Dennis-YC Hsieh
In-Reply-To: <1477999698-6288-1-git-send-email-hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

On Tue, 2016-11-01 at 19:28 +0800, HS Liao wrote:
> Hi,
> 
> This is Mediatek MT8173 Command Queue(CMDQ) driver. The CMDQ is used
> to help write registers with critical time limitation, such as
> updating display configuration during the vblank. It controls Global
> Command Engine (GCE) hardware to achieve this requirement.
> 
> These patches have a build dependency on top of v4.9-rc1.
> 
> Changes since v15:
>  - separate "suspend and resume" patch from "save energy" patch
>  - don't stop running tasks in cmdq_suspend()
>    (i.e. leave no running tasks guarantee to clients)
> 
> Best regards,
> HS Liao
> 
> HS Liao (5):
>   dt-bindings: soc: Add documentation for the MediaTek GCE unit
>   CMDQ: Mediatek CMDQ driver
>   arm64: dts: mt8173: Add GCE node
>   CMDQ: suspend and resume
>   CMDQ: save energy
> 
>  .../devicetree/bindings/mailbox/mtk-gce.txt        |  43 ++
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  10 +
>  drivers/mailbox/Kconfig                            |  10 +
>  drivers/mailbox/Makefile                           |   2 +
>  drivers/mailbox/mtk-cmdq-mailbox.c                 | 632 +++++++++++++++++++++
>  drivers/soc/mediatek/Kconfig                       |  11 +
>  drivers/soc/mediatek/Makefile                      |   1 +
>  drivers/soc/mediatek/mtk-cmdq-helper.c             | 310 ++++++++++
>  include/linux/mailbox/mtk-cmdq-mailbox.h           |  67 +++
>  include/linux/soc/mediatek/mtk-cmdq.h              | 182 ++++++
>  10 files changed, 1268 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/mtk-gce.txt
>  create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c
>  create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c
>  create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h
>  create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h
> 


Hi Jassi, Matthias,

Sorry to disturb you.
Do you have any further comment on CMDQ v16?

Thanks.
HS


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] iommu: convert DT component matching to component_match_add_release()
From: Joerg Roedel @ 2016-11-10 11:25 UTC (permalink / raw)
  To: Russell King
  Cc: Matthias Brugger,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Joerg Roedel,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <E1bwo8o-0005LV-54-eh5Bv4kxaXIk46pC+1QYvQNdhmdF6hFW@public.gmane.org>

On Wed, Oct 19, 2016 at 11:30:34AM +0100, Russell King wrote:
> Convert DT component matching to use component_match_add_release().
> 
> Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
> ---
>  drivers/iommu/mtk_iommu.c    | 8 +++++---
>  drivers/iommu/mtk_iommu.h    | 5 +++++
>  drivers/iommu/mtk_iommu_v1.c | 8 +++++---
>  3 files changed, 15 insertions(+), 6 deletions(-)

Applied, thanks.

^ permalink raw reply

* [PATCH 0/2] mmc: allow mmc_alloc_host() and tmio_mmc_host_alloc()
From: Masahiro Yamada @ 2016-11-10 13:24 UTC (permalink / raw)
  To: linux-mmc-u79uwXL29TY76Z2rM5mHXA
  Cc: Ulf Hansson, Adrian Hunter, Wolfram Sang, Sascha Sommer,
	Johan Hovold, Sonic Zhang, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	Russell King, Jaehoon Chung, Chen-Yu Tsai, Pierre Ossman,
	Harald Welte, Alex Dubov, Masahiro Yamada,
	adi-buildroot-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Michał Mirosław, Rui Miguel Silva,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Ben Dooks,
	Tony Olech, Matthias Brugger, linux-omap-u79uwXL29TY76Z2rM5mHXA


sdhci_alloc_host() returns an error pointer when it fails.
but mmc_alloc_host() cannot.

This series allow to propagate a proper error code
when host-allocation fails.



Masahiro Yamada (2):
  mmc: allow mmc_alloc_host() to return proper error code
  mmc: tmio: allow tmio_mmc_host_alloc() to return proper error code

 drivers/mmc/core/host.c             | 11 ++++++-----
 drivers/mmc/host/android-goldfish.c |  4 ++--
 drivers/mmc/host/atmel-mci.c        |  4 ++--
 drivers/mmc/host/au1xmmc.c          |  4 ++--
 drivers/mmc/host/bfin_sdh.c         |  4 ++--
 drivers/mmc/host/cb710-mmc.c        |  4 ++--
 drivers/mmc/host/davinci_mmc.c      |  4 ++--
 drivers/mmc/host/dw_mmc.c           |  4 ++--
 drivers/mmc/host/jz4740_mmc.c       |  4 ++--
 drivers/mmc/host/mmc_spi.c          |  9 ++++++---
 drivers/mmc/host/mmci.c             |  4 ++--
 drivers/mmc/host/moxart-mmc.c       |  4 ++--
 drivers/mmc/host/mtk-sd.c           |  4 ++--
 drivers/mmc/host/mvsdio.c           |  4 ++--
 drivers/mmc/host/mxcmmc.c           |  4 ++--
 drivers/mmc/host/mxs-mmc.c          |  4 ++--
 drivers/mmc/host/omap.c             |  4 ++--
 drivers/mmc/host/omap_hsmmc.c       |  4 ++--
 drivers/mmc/host/pxamci.c           |  4 ++--
 drivers/mmc/host/rtsx_pci_sdmmc.c   |  4 ++--
 drivers/mmc/host/rtsx_usb_sdmmc.c   |  4 ++--
 drivers/mmc/host/s3cmci.c           |  4 ++--
 drivers/mmc/host/sdhci.c            |  4 ++--
 drivers/mmc/host/sdricoh_cs.c       |  4 ++--
 drivers/mmc/host/sh_mmcif.c         |  4 ++--
 drivers/mmc/host/sh_mobile_sdhi.c   |  4 ++--
 drivers/mmc/host/sunxi-mmc.c        |  4 ++--
 drivers/mmc/host/tifm_sd.c          |  4 ++--
 drivers/mmc/host/tmio_mmc.c         |  4 +++-
 drivers/mmc/host/tmio_mmc_pio.c     |  4 ++--
 drivers/mmc/host/toshsd.c           |  4 ++--
 drivers/mmc/host/usdhi6rol0.c       |  4 ++--
 drivers/mmc/host/ushc.c             |  4 ++--
 drivers/mmc/host/via-sdmmc.c        |  4 ++--
 drivers/mmc/host/vub300.c           |  4 ++--
 drivers/mmc/host/wbsd.c             |  4 ++--
 drivers/mmc/host/wmt-sdmmc.c        |  4 ++--
 drivers/staging/greybus/sdio.c      |  4 ++--
 38 files changed, 85 insertions(+), 79 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH 1/2] mmc: allow mmc_alloc_host() to return proper error code
From: Masahiro Yamada @ 2016-11-10 13:24 UTC (permalink / raw)
  To: linux-mmc-u79uwXL29TY76Z2rM5mHXA
  Cc: Ulf Hansson, Adrian Hunter, Wolfram Sang, Sascha Sommer,
	Johan Hovold, Sonic Zhang, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	Russell King, Jaehoon Chung, Chen-Yu Tsai, Pierre Ossman,
	Harald Welte, Alex Dubov, Masahiro Yamada,
	adi-buildroot-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Michał Mirosław, Rui Miguel Silva,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Ben Dooks,
	Tony Olech, Matthias Brugger, linux-omap-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478784263-18777-1-git-send-email-yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>

Currently, mmc_alloc_host() returns NULL on error, so its callers
cannot return anything but -ENOMEM when it fails, assuming the most
failure cases are due to memory shortage, but it is not true.

Allow mmc_alloc_host() to return an error pointer, then propagate
the proper error code to its callers.

Signed-off-by: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
---

 drivers/mmc/core/host.c             | 11 ++++++-----
 drivers/mmc/host/android-goldfish.c |  4 ++--
 drivers/mmc/host/atmel-mci.c        |  4 ++--
 drivers/mmc/host/au1xmmc.c          |  4 ++--
 drivers/mmc/host/bfin_sdh.c         |  4 ++--
 drivers/mmc/host/cb710-mmc.c        |  4 ++--
 drivers/mmc/host/davinci_mmc.c      |  4 ++--
 drivers/mmc/host/dw_mmc.c           |  4 ++--
 drivers/mmc/host/jz4740_mmc.c       |  4 ++--
 drivers/mmc/host/mmc_spi.c          |  9 ++++++---
 drivers/mmc/host/mmci.c             |  4 ++--
 drivers/mmc/host/moxart-mmc.c       |  4 ++--
 drivers/mmc/host/mtk-sd.c           |  4 ++--
 drivers/mmc/host/mvsdio.c           |  4 ++--
 drivers/mmc/host/mxcmmc.c           |  4 ++--
 drivers/mmc/host/mxs-mmc.c          |  4 ++--
 drivers/mmc/host/omap.c             |  4 ++--
 drivers/mmc/host/omap_hsmmc.c       |  4 ++--
 drivers/mmc/host/pxamci.c           |  4 ++--
 drivers/mmc/host/rtsx_pci_sdmmc.c   |  4 ++--
 drivers/mmc/host/rtsx_usb_sdmmc.c   |  4 ++--
 drivers/mmc/host/s3cmci.c           |  4 ++--
 drivers/mmc/host/sdhci.c            |  4 ++--
 drivers/mmc/host/sdricoh_cs.c       |  4 ++--
 drivers/mmc/host/sh_mmcif.c         |  4 ++--
 drivers/mmc/host/sunxi-mmc.c        |  4 ++--
 drivers/mmc/host/tifm_sd.c          |  4 ++--
 drivers/mmc/host/tmio_mmc_pio.c     |  2 +-
 drivers/mmc/host/toshsd.c           |  4 ++--
 drivers/mmc/host/usdhi6rol0.c       |  4 ++--
 drivers/mmc/host/ushc.c             |  4 ++--
 drivers/mmc/host/via-sdmmc.c        |  4 ++--
 drivers/mmc/host/vub300.c           |  4 ++--
 drivers/mmc/host/wbsd.c             |  4 ++--
 drivers/mmc/host/wmt-sdmmc.c        |  4 ++--
 drivers/staging/greybus/sdio.c      |  4 ++--
 36 files changed, 79 insertions(+), 75 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 98f25ff..b3e13e0 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -349,7 +349,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
 
 	host = kzalloc(sizeof(struct mmc_host) + extra, GFP_KERNEL);
 	if (!host)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	/* scanning will be enabled when we're ready */
 	host->rescan_disable = 1;
@@ -357,7 +357,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
 again:
 	if (!ida_pre_get(&mmc_host_ida, GFP_KERNEL)) {
 		kfree(host);
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 	}
 
 	spin_lock(&mmc_host_lock);
@@ -368,7 +368,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
 		goto again;
 	} else if (err) {
 		kfree(host);
-		return NULL;
+		return ERR_PTR(err);
 	}
 
 	dev_set_name(&host->class_dev, "mmc%d", host->index);
@@ -379,9 +379,10 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
 	device_initialize(&host->class_dev);
 	device_enable_async_suspend(&host->class_dev);
 
-	if (mmc_gpio_alloc(host)) {
+	err = mmc_gpio_alloc(host);
+	if (err < 0) {
 		put_device(&host->class_dev);
-		return NULL;
+		return ERR_PTR(err);
 	}
 
 	spin_lock_init(&host->lock);
diff --git a/drivers/mmc/host/android-goldfish.c b/drivers/mmc/host/android-goldfish.c
index dca5518..7363663 100644
--- a/drivers/mmc/host/android-goldfish.c
+++ b/drivers/mmc/host/android-goldfish.c
@@ -467,8 +467,8 @@ static int goldfish_mmc_probe(struct platform_device *pdev)
 		return -ENXIO;
 
 	mmc = mmc_alloc_host(sizeof(struct goldfish_mmc_host), &pdev->dev);
-	if (mmc == NULL) {
-		ret = -ENOMEM;
+	if (IS_ERR(mmc)) {
+		ret = PTR_ERR(mmc);
 		goto err_alloc_host_failed;
 	}
 
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 0ad8ef5..d0cb112 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -2296,8 +2296,8 @@ static int atmci_init_slot(struct atmel_mci *host,
 	struct atmel_mci_slot		*slot;
 
 	mmc = mmc_alloc_host(sizeof(struct atmel_mci_slot), &host->pdev->dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	slot = mmc_priv(mmc);
 	slot->mmc = mmc;
diff --git a/drivers/mmc/host/au1xmmc.c b/drivers/mmc/host/au1xmmc.c
index ed77fbfa..d97b409 100644
--- a/drivers/mmc/host/au1xmmc.c
+++ b/drivers/mmc/host/au1xmmc.c
@@ -951,9 +951,9 @@ static int au1xmmc_probe(struct platform_device *pdev)
 	int ret, iflag;
 
 	mmc = mmc_alloc_host(sizeof(struct au1xmmc_host), &pdev->dev);
-	if (!mmc) {
+	if (IS_ERR(mmc)) {
 		dev_err(&pdev->dev, "no memory for mmc_host\n");
-		ret = -ENOMEM;
+		ret = PTR_ERR(mmc);
 		goto out0;
 	}
 
diff --git a/drivers/mmc/host/bfin_sdh.c b/drivers/mmc/host/bfin_sdh.c
index 526231e..60c52d1 100644
--- a/drivers/mmc/host/bfin_sdh.c
+++ b/drivers/mmc/host/bfin_sdh.c
@@ -532,8 +532,8 @@ static int sdh_probe(struct platform_device *pdev)
 	}
 
 	mmc = mmc_alloc_host(sizeof(struct sdh_host), &pdev->dev);
-	if (!mmc) {
-		ret = -ENOMEM;
+	if (IS_ERR(mmc)) {
+		ret = PTR_ERR(mmc);
 		goto out;
 	}
 
diff --git a/drivers/mmc/host/cb710-mmc.c b/drivers/mmc/host/cb710-mmc.c
index 1087b4c..79ce871 100644
--- a/drivers/mmc/host/cb710-mmc.c
+++ b/drivers/mmc/host/cb710-mmc.c
@@ -692,8 +692,8 @@ static int cb710_mmc_init(struct platform_device *pdev)
 	u32 val;
 
 	mmc = mmc_alloc_host(sizeof(*reader), cb710_slot_dev(slot));
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	platform_set_drvdata(pdev, mmc);
 
diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index 8fa478c..225b9a8 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -1229,8 +1229,8 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 		return -EBUSY;
 
 	mmc = mmc_alloc_host(sizeof(struct mmc_davinci_host), &pdev->dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	host = mmc_priv(mmc);
 	host->mmc = mmc;	/* Important */
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4fcbc40..4f06528 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2598,8 +2598,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	u32 freq[2];
 
 	mmc = mmc_alloc_host(sizeof(struct dw_mci_slot), host->dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	slot = mmc_priv(mmc);
 	slot->id = id;
diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index 684087d..351dd68 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -998,9 +998,9 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
 	pdata = pdev->dev.platform_data;
 
 	mmc = mmc_alloc_host(sizeof(struct jz4740_mmc_host), &pdev->dev);
-	if (!mmc) {
+	if (IS_ERR(mmc)) {
 		dev_err(&pdev->dev, "Failed to alloc mmc host structure\n");
-		return -ENOMEM;
+		return PTR_ERR(mmc);
 	}
 
 	host = mmc_priv(mmc);
diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index e77d79c..165f73e 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -1333,15 +1333,18 @@ static int mmc_spi_probe(struct spi_device *spi)
 	 * NOTE if many systems use more than one MMC-over-SPI connector
 	 * it'd save some memory to share this.  That's evidently rare.
 	 */
-	status = -ENOMEM;
 	ones = kmalloc(MMC_SPI_BLOCKSIZE, GFP_KERNEL);
-	if (!ones)
+	if (!ones) {
+		status = -ENOMEM;
 		goto nomem;
+	}
 	memset(ones, 0xff, MMC_SPI_BLOCKSIZE);
 
 	mmc = mmc_alloc_host(sizeof(*host), &spi->dev);
-	if (!mmc)
+	if (IS_ERR(mmc)) {
+		status = PTR_ERR(mmc);
 		goto nomem;
+	}
 
 	mmc->ops = &mmc_spi_ops;
 	mmc->max_blk_size = MMC_SPI_BLOCKSIZE;
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index df990bb..5779b57 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1509,8 +1509,8 @@ static int mmci_probe(struct amba_device *dev,
 	}
 
 	mmc = mmc_alloc_host(sizeof(struct mmci_host), &dev->dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	ret = mmci_of_parse(np, mmc);
 	if (ret)
diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index bbad309..f453e55 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -568,9 +568,9 @@ static int moxart_probe(struct platform_device *pdev)
 	u32 i;
 
 	mmc = mmc_alloc_host(sizeof(struct moxart_host), dev);
-	if (!mmc) {
+	if (IS_ERR(mmc)) {
 		dev_err(dev, "mmc_alloc_host failed\n");
-		ret = -ENOMEM;
+		ret = PTR_ERR(mmc);
 		goto out;
 	}
 
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 84e9afc..ef28f64 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -1494,8 +1494,8 @@ static int msdc_drv_probe(struct platform_device *pdev)
 	}
 	/* Allocate MMC host for this device */
 	mmc = mmc_alloc_host(sizeof(struct msdc_host), &pdev->dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	host = mmc_priv(mmc);
 	ret = mmc_of_parse(mmc);
diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index 42296e5..5594f58 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -711,8 +711,8 @@ static int mvsd_probe(struct platform_device *pdev)
 		return -ENXIO;
 
 	mmc = mmc_alloc_host(sizeof(struct mvsd_host), &pdev->dev);
-	if (!mmc) {
-		ret = -ENOMEM;
+	if (IS_ERR(mmc)) {
+		ret = PTR_ERR(mmc);
 		goto out;
 	}
 
diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
index fb3ca82..5bdc644 100644
--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -1018,8 +1018,8 @@ static int mxcmci_probe(struct platform_device *pdev)
 		return -EINVAL;
 
 	mmc = mmc_alloc_host(sizeof(*host), &pdev->dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	host = mmc_priv(mmc);
 
diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index d839147..7f72bb4 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -586,8 +586,8 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 		return irq_err;
 
 	mmc = mmc_alloc_host(sizeof(struct mxs_mmc_host), &pdev->dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	host = mmc_priv(mmc);
 	ssp = &host->ssp;
diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index be3c49f..b1ec63f 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -1227,8 +1227,8 @@ static int mmc_omap_new_slot(struct mmc_omap_host *host, int id)
 	int r;
 
 	mmc = mmc_alloc_host(sizeof(struct mmc_omap_slot), host->dev);
-	if (mmc == NULL)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	slot = mmc_priv(mmc);
 	slot->host = host;
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 5f2f24a..6154b55 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -2024,8 +2024,8 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
 		return PTR_ERR(base);
 
 	mmc = mmc_alloc_host(sizeof(struct omap_hsmmc_host), &pdev->dev);
-	if (!mmc) {
-		ret = -ENOMEM;
+	if (IS_ERR(mmc)) {
+		ret = PTR_ERR(mmc);
 		goto err;
 	}
 
diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index c763b40..9e9b02f 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -652,8 +652,8 @@ static int pxamci_probe(struct platform_device *pdev)
 		return irq;
 
 	mmc = mmc_alloc_host(sizeof(struct pxamci_host), &pdev->dev);
-	if (!mmc) {
-		ret = -ENOMEM;
+	if (IS_ERR(mmc)) {
+		ret = PTR_ERR(mmc);
 		goto out;
 	}
 
diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
index 3ccaa14..551536e 100644
--- a/drivers/mmc/host/rtsx_pci_sdmmc.c
+++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
@@ -1399,8 +1399,8 @@ static int rtsx_pci_sdmmc_drv_probe(struct platform_device *pdev)
 	dev_dbg(&(pdev->dev), ": Realtek PCI-E SDMMC controller found\n");
 
 	mmc = mmc_alloc_host(sizeof(*host), &pdev->dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	host = mmc_priv(mmc);
 	host->pcr = pcr;
diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
index 6e9c0f8..443679f 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -1363,8 +1363,8 @@ static int rtsx_usb_sdmmc_drv_probe(struct platform_device *pdev)
 	dev_dbg(&(pdev->dev), ": Realtek USB SD/MMC controller found\n");
 
 	mmc = mmc_alloc_host(sizeof(*host), &pdev->dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	host = mmc_priv(mmc);
 	host->ucr = ucr;
diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
index c531dee..aacc5cf 100644
--- a/drivers/mmc/host/s3cmci.c
+++ b/drivers/mmc/host/s3cmci.c
@@ -1556,8 +1556,8 @@ static int s3cmci_probe(struct platform_device *pdev)
 	is2440 = platform_get_device_id(pdev)->driver_data;
 
 	mmc = mmc_alloc_host(sizeof(struct s3cmci_host), &pdev->dev);
-	if (!mmc) {
-		ret = -ENOMEM;
+	if (IS_ERR(mmc)) {
+		ret = PTR_ERR(mmc);
 		goto probe_out;
 	}
 
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 71654b9..eb8199e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2946,8 +2946,8 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
 	WARN_ON(dev == NULL);
 
 	mmc = mmc_alloc_host(sizeof(struct sdhci_host) + priv_size, dev);
-	if (!mmc)
-		return ERR_PTR(-ENOMEM);
+	if (IS_ERR(mmc))
+		return ERR_CAST(mmc);
 
 	host = mmc_priv(mmc);
 	host->mmc = mmc;
diff --git a/drivers/mmc/host/sdricoh_cs.c b/drivers/mmc/host/sdricoh_cs.c
index 5ff26ab..6d2f671 100644
--- a/drivers/mmc/host/sdricoh_cs.c
+++ b/drivers/mmc/host/sdricoh_cs.c
@@ -424,9 +424,9 @@ static int sdricoh_init_mmc(struct pci_dev *pci_dev,
 	/* allocate privdata */
 	mmc = pcmcia_dev->priv =
 	    mmc_alloc_host(sizeof(struct sdricoh_host), &pcmcia_dev->dev);
-	if (!mmc) {
+	if (IS_ERR(mmc)) {
 		dev_err(dev, "mmc_alloc_host failed\n");
-		result = -ENOMEM;
+		result = PTR_ERR(mmc);
 		goto unmap_io;
 	}
 	host = mmc_priv(mmc);
diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 9007784..c2affc6 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1432,8 +1432,8 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 		return PTR_ERR(reg);
 
 	mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	ret = mmc_of_parse(mmc);
 	if (ret < 0)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index c0a5c67..a3cb388 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -1212,9 +1212,9 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
 	int ret;
 
 	mmc = mmc_alloc_host(sizeof(struct sunxi_mmc_host), &pdev->dev);
-	if (!mmc) {
+	if (IS_ERR(mmc)) {
 		dev_err(&pdev->dev, "mmc alloc host failed\n");
-		return -ENOMEM;
+		return PTR_ERR(mmc);
 	}
 
 	host = mmc_priv(mmc);
diff --git a/drivers/mmc/host/tifm_sd.c b/drivers/mmc/host/tifm_sd.c
index 93c4b40..b088cb8 100644
--- a/drivers/mmc/host/tifm_sd.c
+++ b/drivers/mmc/host/tifm_sd.c
@@ -958,8 +958,8 @@ static int tifm_sd_probe(struct tifm_dev *sock)
 	}
 
 	mmc = mmc_alloc_host(sizeof(struct tifm_sd), &sock->dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	host = mmc_priv(mmc);
 	tifm_set_drvdata(sock, mmc);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 7005676..18106fc 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -1013,7 +1013,7 @@ struct tmio_mmc_host*
 	struct mmc_host *mmc;
 
 	mmc = mmc_alloc_host(sizeof(struct tmio_mmc_host), &pdev->dev);
-	if (!mmc)
+	if (IS_ERR(mmc))
 		return NULL;
 
 	host = mmc_priv(mmc);
diff --git a/drivers/mmc/host/toshsd.c b/drivers/mmc/host/toshsd.c
index 553ef41..7b086ed 100644
--- a/drivers/mmc/host/toshsd.c
+++ b/drivers/mmc/host/toshsd.c
@@ -617,8 +617,8 @@ static int toshsd_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return ret;
 
 	mmc = mmc_alloc_host(sizeof(struct toshsd_host), &pdev->dev);
-	if (!mmc) {
-		ret = -ENOMEM;
+	if (IS_ERR(mmc)) {
+		ret = PTR_ERR(mmc);
 		goto err;
 	}
 
diff --git a/drivers/mmc/host/usdhi6rol0.c b/drivers/mmc/host/usdhi6rol0.c
index 1bd5f1a..e9d0126 100644
--- a/drivers/mmc/host/usdhi6rol0.c
+++ b/drivers/mmc/host/usdhi6rol0.c
@@ -1753,8 +1753,8 @@ static int usdhi6_probe(struct platform_device *pdev)
 		return -ENODEV;
 
 	mmc = mmc_alloc_host(sizeof(struct usdhi6_host), dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	ret = mmc_regulator_get_supply(mmc);
 	if (ret == -EPROBE_DEFER)
diff --git a/drivers/mmc/host/ushc.c b/drivers/mmc/host/ushc.c
index d2c386f..6937021 100644
--- a/drivers/mmc/host/ushc.c
+++ b/drivers/mmc/host/ushc.c
@@ -427,8 +427,8 @@ static int ushc_probe(struct usb_interface *intf, const struct usb_device_id *id
 	int ret;
 
 	mmc = mmc_alloc_host(sizeof(struct ushc_data), &intf->dev);
-	if (mmc == NULL)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 	ushc = mmc_priv(mmc);
 	usb_set_intfdata(intf, ushc);
 
diff --git a/drivers/mmc/host/via-sdmmc.c b/drivers/mmc/host/via-sdmmc.c
index 63fac78..010bfdb 100644
--- a/drivers/mmc/host/via-sdmmc.c
+++ b/drivers/mmc/host/via-sdmmc.c
@@ -1108,8 +1108,8 @@ static int via_sd_probe(struct pci_dev *pcidev,
 	pci_write_config_byte(pcidev, VIA_CRDR_PCI_DBG_MODE, 0);
 
 	mmc = mmc_alloc_host(sizeof(struct via_crdr_mmc_host), &pcidev->dev);
-	if (!mmc) {
-		ret = -ENOMEM;
+	if (IS_ERR(mmc)) {
+		ret = PTR_ERR(mmc);
 		goto release;
 	}
 
diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
index bb3e0d1..d052c23 100644
--- a/drivers/mmc/host/vub300.c
+++ b/drivers/mmc/host/vub300.c
@@ -2125,8 +2125,8 @@ static int vub300_probe(struct usb_interface *interface,
 	}
 	/* this also allocates memory for our VUB300 mmc host device */
 	mmc = mmc_alloc_host(sizeof(struct vub300_mmc_host), &udev->dev);
-	if (!mmc) {
-		retval = -ENOMEM;
+	if (IS_ERR(mmc)) {
+		retval = PTR_ERR(mmc);
 		dev_err(&udev->dev, "not enough memory for the mmc_host\n");
 		goto error4;
 	}
diff --git a/drivers/mmc/host/wbsd.c b/drivers/mmc/host/wbsd.c
index c3fd16d..40f8fd6 100644
--- a/drivers/mmc/host/wbsd.c
+++ b/drivers/mmc/host/wbsd.c
@@ -1204,8 +1204,8 @@ static int wbsd_alloc_mmc(struct device *dev)
 	 * Allocate MMC structure.
 	 */
 	mmc = mmc_alloc_host(sizeof(struct wbsd_host), dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	host = mmc_priv(mmc);
 	host->mmc = mmc;
diff --git a/drivers/mmc/host/wmt-sdmmc.c b/drivers/mmc/host/wmt-sdmmc.c
index 5af0055..f37f9a4cd 100644
--- a/drivers/mmc/host/wmt-sdmmc.c
+++ b/drivers/mmc/host/wmt-sdmmc.c
@@ -782,9 +782,9 @@ static int wmt_mci_probe(struct platform_device *pdev)
 	}
 
 	mmc = mmc_alloc_host(sizeof(struct wmt_mci_priv), &pdev->dev);
-	if (!mmc) {
+	if (IS_ERR(mmc)) {
 		dev_err(&pdev->dev, "Failed to allocate mmc_host\n");
-		ret = -ENOMEM;
+		ret = PTR_ERR(mmc);
 		goto fail1;
 	}
 
diff --git a/drivers/staging/greybus/sdio.c b/drivers/staging/greybus/sdio.c
index 5649ef1..01c443d 100644
--- a/drivers/staging/greybus/sdio.c
+++ b/drivers/staging/greybus/sdio.c
@@ -768,8 +768,8 @@ static int gb_sdio_probe(struct gbphy_device *gbphy_dev,
 	int ret = 0;
 
 	mmc = mmc_alloc_host(sizeof(*host), &gbphy_dev->dev);
-	if (!mmc)
-		return -ENOMEM;
+	if (IS_ERR(mmc))
+		return PTR_ERR(mmc);
 
 	connection = gb_connection_create(gbphy_dev->bundle,
 					  le16_to_cpu(gbphy_dev->cport_desc->id),
-- 
1.9.1

^ permalink raw reply related

* Re: [bug report] [media] vcodec: mediatek: Add Mediatek V4L2 Video Decoder Driver
From: Dan Carpenter @ 2016-11-10 13:30 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: tiffany.lin, linux-media, linux-mediatek
In-Reply-To: <9794dab1-94ba-c384-85c5-edb8831810ff@xs4all.nl>

On Wed, Nov 09, 2016 at 02:45:21PM +0100, Hans Verkuil wrote:
> On 11/09/16 14:28, Dan Carpenter wrote:
> >Hello Tiffany Lin,
> >
> >The patch 590577a4e525: "[media] vcodec: mediatek: Add Mediatek V4L2
> >Video Decoder Driver" from Sep 2, 2016, leads to the following static
> >checker warning:
> >
> >	drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c:536 vidioc_vdec_qbuf()
> >	error: buffer overflow 'vq->bufs' 32 <= u32max
> >
> >drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
> >   520  static int vidioc_vdec_qbuf(struct file *file, void *priv,
> >   521                              struct v4l2_buffer *buf)
> >   522  {
> >   523          struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
> >   524          struct vb2_queue *vq;
> >   525          struct vb2_buffer *vb;
> >   526          struct mtk_video_dec_buf *mtkbuf;
> >   527          struct vb2_v4l2_buffer  *vb2_v4l2;
> >   528
> >   529          if (ctx->state == MTK_STATE_ABORT) {
> >   530                  mtk_v4l2_err("[%d] Call on QBUF after unrecoverable error",
> >   531                                  ctx->id);
> >   532                  return -EIO;
> >   533          }
> >   534
> >   535          vq = v4l2_m2m_get_vq(ctx->m2m_ctx, buf->type);
> >   536          vb = vq->bufs[buf->index];
> >
> >Smatch thinks that "buf->index" comes straight from the user without
> >being checked and that this is a buffer overflow.  It seems simple
> >enough to analyse the call tree.
> >
> >__video_do_ioctl()
> >->  v4l_qbuf()
> >  -> vidioc_vdec_qbuf()
> >
> >It seems like Smatch is correct.  I looked at a different implementation
> >of this and that one wasn't checked either so maybe there is something
> >I am not seeing.
> >
> >This has obvious security implications.  Can someone take a look at
> >this?
> 
> This is indeed wrong.
> 
> The v4l2_m2m_qbuf() call at the end of this function calls in turn
> vb2_qbuf which
> will check the index.

There could be an issue before you reach v4l2_m2m_qbuf() because we
set "mtkbuf->lastframe = true;" so we could be corrupting random
memory.

I re-reviewed the other function I looked at earlier and that one was OK
though.

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH 0/2] mmc: allow mmc_alloc_host() and tmio_mmc_host_alloc()
From: Greg Kroah-Hartman @ 2016-11-10 13:35 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Ulf Hansson, Adrian Hunter, Wolfram Sang, Sascha Sommer,
	Johan Hovold, Sonic Zhang, devel, Russell King, Jaehoon Chung,
	Chen-Yu Tsai, Pierre Ossman, Harald Welte, Alex Dubov,
	adi-buildroot-devel, Michał Mirosław, linux-mediatek,
	Ben Dooks, Tony Olech, Matthias Brugger, linux-omap,
	linux-arm-kernel, Nicolas Pitre, linux-usb
In-Reply-To: <1478784263-18777-1-git-send-email-yamada.masahiro@socionext.com>

On Thu, Nov 10, 2016 at 10:24:21PM +0900, Masahiro Yamada wrote:
> 
> sdhci_alloc_host() returns an error pointer when it fails.
> but mmc_alloc_host() cannot.
> 
> This series allow to propagate a proper error code
> when host-allocation fails.

Why?  What can we really do about the error except give up?  Why does
having a explicit error value make any difference to the caller, they
can't do anything different, right?

I suggest just leaving it as-is, it's simple, and you don't have to mess
with PTR_ERR() anywhere.

thanks,

greg k-h

^ permalink raw reply

* Re: [v17 2/2] drm/bridge: Add I2C based driver for ps8640 bridge
From: Enric Balletbo Serra @ 2016-11-10 16:39 UTC (permalink / raw)
  To: Jitao Shi
  Cc: David Airlie, Thierry Reding, Matthias Brugger, Mark Rutland,
	stonea168, dri-devel, Andy Yan, Ajay Kumar, Vincent Palatin,
	cawa cheng, bibby.hsieh, CK HU, Russell King,
	devicetree@vger.kernel.org, Sascha Hauer, Pawel Moll,
	Ian Campbell, Inki Dae, Rob Herring, ARM/Mediatek 
In-Reply-To: <1472280263-18177-2-git-send-email-jitao.shi@mediatek.com>

Hi Jitao,

2016-08-27 8:44 GMT+02:00 Jitao Shi <jitao.shi@mediatek.com>:
> This patch adds drm_bridge driver for parade DSI to eDP bridge chip.
>
> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
> Changes since v16:
>  - Disable ps8640 DSI MCS Function.
>  - Rename gpios name more clearly.
>  - Tune the ps8640 power on sequence.
>
> Changes since v15:
>  - Drop drm_connector_(un)register calls from parade ps8640.
>    The main DRM driver mtk_drm_drv now calls
>    drm_connector_register_all() after drm_dev_register() in the
>    mtk_drm_bind() function. That function should iterate over all
>    connectors and call drm_connector_register() for each of them.
>    So, remove drm_connector_(un)register calls from parade ps8640.
>
> Changes since v14:
>  - update copyright info.
>  - change bridge_to_ps8640 and connector_to_ps8640 to inline function.
>  - fix some coding style.
>  - use sizeof as array counter.
>  - use drm_get_edid when read edid.
>  - add mutex when firmware updating.
>
> Changes since v13:
>  - add const on data, ps8640_write_bytes(struct i2c_client *client, const u8 *data, u16 data_len)
>  - fix PAGE2_SW_REST tyro.
>  - move the buf[3] init to entrance of the function.
>
> Changes since v12:
>  - fix hw_chip_id build warning
>
> Changes since v11:
>  - Remove depends on I2C, add DRM depends
>  - Reuse ps8640_write_bytes() in ps8640_write_byte()
>  - Use timer check for polling like the routines in <linux/iopoll.h>
>  - Fix no drm_connector_unregister/drm_connector_cleanup when ps8640_bridge_attach fail
>  - Check the ps8640 hardware id in ps8640_validate_firmware
>  - Remove fw_version check
>  - Move ps8640_validate_firmware before ps8640_enter_bl
>  - Add ddc_i2c unregister when probe fail and ps8640_remove
> ---
>  drivers/gpu/drm/bridge/Kconfig         |   12 +
>  drivers/gpu/drm/bridge/Makefile        |    1 +
>  drivers/gpu/drm/bridge/parade-ps8640.c | 1077 ++++++++++++++++++++++++++++++++
>  3 files changed, 1090 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/parade-ps8640.c
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index b590e67..c59d043 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -50,6 +50,18 @@ config DRM_PARADE_PS8622
>         ---help---
>           Parade eDP-LVDS bridge chip driver.
>
> +config DRM_PARADE_PS8640
> +       tristate "Parade PS8640 MIPI DSI to eDP Converter"
> +       depends on DRM
> +       depends on OF
> +       select DRM_KMS_HELPER
> +       select DRM_MIPI_DSI
> +       select DRM_PANEL
> +       ---help---
> +         Choose this option if you have PS8640 for display
> +         The PS8640 is a high-performance and low-power
> +         MIPI DSI to eDP converter
> +
>  config DRM_SII902X
>         tristate "Silicon Image sii902x RGB/HDMI bridge"
>         depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index efdb07e..3360537 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>  obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
>  obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
> +obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
>  obj-$(CONFIG_DRM_SII902X) += sii902x.o
>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> new file mode 100644
> index 0000000..7d67431
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> @@ -0,0 +1,1077 @@
> +/*
> + * Copyright (c) 2016 MediaTek Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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 <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/firmware.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_graph.h>
> +#include <linux/regulator/consumer.h>
> +#include <asm/unaligned.h>
> +#include <drm/drm_panel.h>
> +
> +#include <drmP.h>
> +#include <drm_atomic_helper.h>
> +#include <drm_crtc_helper.h>
> +#include <drm_crtc.h>
> +#include <drm_edid.h>
> +#include <drm_mipi_dsi.h>
> +
> +#define PAGE1_VSTART           0x6b
> +#define PAGE2_SPI_CFG3         0x82
> +#define I2C_TO_SPI_RESET       0x20
> +#define PAGE2_ROMADD_BYTE1     0x8e
> +#define PAGE2_ROMADD_BYTE2     0x8f
> +#define PAGE2_SWSPI_WDATA      0x90
> +#define PAGE2_SWSPI_RDATA      0x91
> +#define PAGE2_SWSPI_LEN                0x92
> +#define PAGE2_SWSPI_CTL                0x93
> +#define TRIGGER_NO_READBACK    0x05
> +#define TRIGGER_READBACK       0x01
> +#define PAGE2_SPI_STATUS       0x9e
> +#define SPI_READY              0x0c
> +#define PAGE2_GPIO_L           0xa6
> +#define PAGE2_GPIO_H           0xa7
> +#define PS_GPIO9               BIT(1)
> +#define PAGE2_IROM_CTRL                0xb0
> +#define IROM_ENABLE            0xc0
> +#define IROM_DISABLE           0x80
> +#define PAGE2_SW_RESET         0xbc
> +#define SPI_SW_RESET           BIT(7)
> +#define MPU_SW_RESET           BIT(6)
> +#define PAGE2_ENCTLSPI_WR      0xda
> +#define PAGE2_I2C_BYPASS       0xea
> +#define I2C_BYPASS_EN          0xd0
> +#define PAGE2_MCS_EN           0xf3
> +#define MCS_EN                 BIT(0)
> +#define PAGE3_SET_ADD          0xfe
> +#define PAGE3_SET_VAL          0xff
> +#define VDO_CTL_ADD            0x13
> +#define VDO_DIS                        0x18
> +#define VDO_EN                 0x1c
> +#define PAGE4_REV_L            0xf0
> +#define PAGE4_REV_H            0xf1
> +#define PAGE4_CHIP_L           0xf2
> +#define PAGE4_CHIP_H           0xf3
> +
> +/* Firmware */
> +#define PS_FW_NAME             "ps864x_fw.bin"
> +

About the firmware discussion I think that if you want to maintain the
upgrade firmware thing you should also include this patch in the
series.

 https://chromium-review.googlesource.com/#/c/317221/

Otherwise, if this is not really needed I think that remove this from
the driver is the best. Just an opinion, this is something the
maintainer should decide.

> +#define FW_CHIP_ID_OFFSET      0
> +#define FW_VERSION_OFFSET      2
> +#define EDID_I2C_ADDR          0x50
> +
> +#define WRITE_STATUS_REG_CMD   0x01
> +#define READ_STATUS_REG_CMD    0x05
> +#define BUSY                   BIT(0)
> +#define CLEAR_ALL_PROTECT      0x00
> +#define BLK_PROTECT_BITS       0x0c
> +#define STATUS_REG_PROTECT     BIT(7)
> +#define WRITE_ENABLE_CMD       0x06
> +#define CHIP_ERASE_CMD         0xc7
> +#define MAX_DEVS               0x8
> +
> +struct ps8640_info {
> +       u8 family_id;
> +       u8 variant_id;
> +       u16 version;
> +};
> +
> +struct ps8640 {
> +       struct drm_connector connector;
> +       struct drm_bridge bridge;
> +       struct edid *edid;
> +       struct mipi_dsi_device dsi;
> +       struct i2c_client *page[MAX_DEVS];
> +       struct i2c_client *ddc_i2c;
> +       struct regulator_bulk_data supplies[2];
> +       struct drm_panel *panel;
> +       struct gpio_desc *gpio_reset;
> +       struct gpio_desc *gpio_power_down;
> +       struct gpio_desc *gpio_mode_sel;
> +       bool enabled;
> +
> +       /* firmware file info */
> +       struct ps8640_info info;
> +       bool in_fw_update;
> +       /* for firmware update protect */
> +       struct mutex fw_mutex;
> +};
> +
> +static const u8 enc_ctrl_code[6] = { 0xaa, 0x55, 0x50, 0x41, 0x52, 0x44 };
> +static const u8 hw_chip_id[4] = { 0x00, 0x0a, 0x00, 0x30 };
> +
> +static inline struct ps8640 *bridge_to_ps8640(struct drm_bridge *e)
> +{
> +       return container_of(e, struct ps8640, bridge);
> +}
> +
> +static inline struct ps8640 *connector_to_ps8640(struct drm_connector *e)
> +{
> +       return container_of(e, struct ps8640, connector);
> +}
> +
> +static int ps8640_read(struct i2c_client *client, u8 reg, u8 *data,
> +                      u16 data_len)
> +{
> +       int ret;
> +       struct i2c_msg msgs[] = {
> +               {
> +                .addr = client->addr,
> +                .flags = 0,
> +                .len = 1,
> +                .buf = &reg,
> +               },
> +               {
> +                .addr = client->addr,
> +                .flags = I2C_M_RD,
> +                .len = data_len,
> +                .buf = data,
> +               }
> +       };
> +
> +       ret = i2c_transfer(client->adapter, msgs, 2);
> +
> +       if (ret == 2)
> +               return 0;
> +       if (ret < 0)
> +               return ret;
> +       else
> +               return -EIO;
> +}
> +
> +static int ps8640_write_bytes(struct i2c_client *client, const u8 *data,
> +                             u16 data_len)
> +{
> +       int ret;
> +       struct i2c_msg msg;
> +
> +       msg.addr = client->addr;
> +       msg.flags = 0;
> +       msg.len = data_len;
> +       msg.buf = (u8 *)data;
> +
> +       ret = i2c_transfer(client->adapter, &msg, 1);
> +       if (ret == 1)
> +               return 0;
> +       if (ret < 0)
> +               return ret;
> +       else
> +               return -EIO;
> +}
> +
> +static int ps8640_write_byte(struct i2c_client *client, u8 reg,  u8 data)
> +{
> +       u8 buf[] = { reg, data };
> +
> +       return ps8640_write_bytes(client, buf, sizeof(buf));
> +}
> +
> +static void ps8640_get_mcu_fw_version(struct ps8640 *ps_bridge)
> +{
> +       struct i2c_client *client = ps_bridge->page[5];
> +       u8 fw_ver[2];
> +
> +       ps8640_read(client, 0x4, fw_ver, sizeof(fw_ver));
> +       ps_bridge->info.version = (fw_ver[0] << 8) | fw_ver[1];
> +
> +       DRM_INFO_ONCE("ps8640 rom fw version %d.%d\n", fw_ver[0], fw_ver[1]);
> +}
> +
> +static int ps8640_bridge_unmute(struct ps8640 *ps_bridge)
> +{
> +       struct i2c_client *client = ps_bridge->page[3];
> +       u8 vdo_ctrl_buf[3] = { PAGE3_SET_ADD, VDO_CTL_ADD, VDO_EN };
> +
> +       return ps8640_write_bytes(client, vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
> +}
> +
> +static int ps8640_bridge_mute(struct ps8640 *ps_bridge)
> +{
> +       struct i2c_client *client = ps_bridge->page[3];
> +       u8 vdo_ctrl_buf[3] = { PAGE3_SET_ADD, VDO_CTL_ADD, VDO_DIS };
> +
> +       return ps8640_write_bytes(client, vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
> +}
> +
> +static void ps8640_pre_enable(struct drm_bridge *bridge)
> +{
> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> +       struct i2c_client *client = ps_bridge->page[2];
> +       struct i2c_client *page1 = ps_bridge->page[1];
> +       int err;
> +       u8 set_vdo_done, mcs_en, vstart;
> +       ktime_t timeout;
> +
> +       if (ps_bridge->in_fw_update)
> +               return;
> +
> +       if (ps_bridge->enabled)
> +               return;
> +
> +       err = drm_panel_prepare(ps_bridge->panel);
> +       if (err < 0) {
> +               DRM_ERROR("failed to prepare panel: %d\n", err);
> +               return;
> +       }
> +
> +       err = regulator_bulk_enable(ARRAY_SIZE(ps_bridge->supplies),
> +                                   ps_bridge->supplies);
> +       if (err < 0) {
> +               DRM_ERROR("cannot enable regulators %d\n", err);
> +               goto err_panel_unprepare;
> +       }
> +
> +       gpiod_set_value(ps_bridge->gpio_power_down, 1);
> +       gpiod_set_value(ps_bridge->gpio_reset, 0);
> +       usleep_range(2000, 2500);
> +       gpiod_set_value(ps_bridge->gpio_reset, 1);
> +
> +       /*
> +        * Wait for the ps8640 embed mcu ready
> +        * First wait 200ms and then check the mcu ready flag every 20ms
> +        */
> +       msleep(200);
> +
> +       timeout = ktime_add_ms(ktime_get(), 200);
> +       for (;;) {
> +               err = ps8640_read(client, PAGE2_GPIO_H, &set_vdo_done, 1);
> +               if (err < 0) {
> +                       DRM_ERROR("failed read PAGE2_GPIO_H: %d\n", err);
> +                       goto err_regulators_disable;
> +               }
> +               if ((set_vdo_done & PS_GPIO9) == PS_GPIO9)
> +                       break;
> +               if (ktime_compare(ktime_get(), timeout) > 0)
> +                       break;
> +               msleep(20);
> +       }
> +
> +       msleep(50);
> +
> +       ps8640_read(page1, PAGE1_VSTART, &vstart, 1);
> +       DRM_INFO("PS8640 PAGE1.0x6B = 0x%x\n", vstart);
> +
> +       /**
> +        * The Manufacturer Command Set (MCS) is a device dependent interface
> +        * intended for factory programming of the display module default
> +        * parameters. Once the display module is configured, the MCS shall be
> +        * disabled by the manufacturer. Once disabled, all MCS commands are
> +        * ignored by the display interface.
> +        */
> +       ps8640_read(client, PAGE2_MCS_EN, &mcs_en, 1);
> +       ps8640_write_byte(client, PAGE2_MCS_EN, mcs_en & ~MCS_EN);
> +
> +       if (ps_bridge->info.version == 0)
> +               ps8640_get_mcu_fw_version(ps_bridge);
> +
> +       err = ps8640_bridge_unmute(ps_bridge);
> +       if (err)
> +               DRM_ERROR("failed to enable unmutevideo: %d\n", err);
> +       /* Switch access edp panel's edid through i2c */
> +       ps8640_write_byte(client, PAGE2_I2C_BYPASS, I2C_BYPASS_EN);
> +       ps_bridge->enabled = true;
> +
> +       return;
> +
> +err_regulators_disable:
> +       regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
> +                              ps_bridge->supplies);
> +err_panel_unprepare:
> +       drm_panel_unprepare(ps_bridge->panel);
> +}
> +
> +static void ps8640_enable(struct drm_bridge *bridge)
> +{
> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> +       int err;
> +
> +       err = drm_panel_enable(ps_bridge->panel);
> +       if (err < 0)
> +               DRM_ERROR("failed to enable panel: %d\n", err);
> +}
> +
> +static void ps8640_disable(struct drm_bridge *bridge)
> +{
> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> +       int err;
> +
> +       err = drm_panel_disable(ps_bridge->panel);
> +       if (err < 0)
> +               DRM_ERROR("failed to disable panel: %d\n", err);
> +}
> +
> +static void ps8640_post_disable(struct drm_bridge *bridge)
> +{
> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> +       int err;
> +
> +       if (ps_bridge->in_fw_update)
> +               return;
> +
> +       if (!ps_bridge->enabled)
> +               return;
> +
> +       ps_bridge->enabled = false;
> +
> +       err = ps8640_bridge_mute(ps_bridge);
> +       if (err < 0)
> +               DRM_ERROR("failed to unmutevideo: %d\n", err);
> +
> +       gpiod_set_value(ps_bridge->gpio_reset, 0);
> +       gpiod_set_value(ps_bridge->gpio_power_down, 0);
> +       err = regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
> +                                    ps_bridge->supplies);
> +       if (err < 0)
> +               DRM_ERROR("cannot disable regulators %d\n", err);
> +
> +       err = drm_panel_unprepare(ps_bridge->panel);
> +       if (err)
> +               DRM_ERROR("failed to unprepare panel: %d\n", err);
> +}
> +
> +static int ps8640_get_modes(struct drm_connector *connector)
> +{
> +       struct ps8640 *ps_bridge = connector_to_ps8640(connector);
> +       struct edid *edid;
> +       int num_modes = 0;
> +       bool power_off;
> +
> +       if (ps_bridge->edid)
> +               return drm_add_edid_modes(connector, ps_bridge->edid);
> +
> +       power_off = !ps_bridge->enabled;
> +       ps8640_pre_enable(&ps_bridge->bridge);
> +
> +       edid = drm_get_edid(connector, ps_bridge->ddc_i2c->adapter);
> +       if (!edid)
> +               goto out;
> +
> +       ps_bridge->edid = edid;
> +       drm_mode_connector_update_edid_property(connector, ps_bridge->edid);
> +       num_modes = drm_add_edid_modes(connector, ps_bridge->edid);
> +
> +out:
> +       if (power_off)
> +               ps8640_post_disable(&ps_bridge->bridge);
> +
> +       return num_modes;
> +}
> +
> +static struct drm_encoder *ps8640_best_encoder(struct drm_connector *connector)
> +{
> +       struct ps8640 *ps_bridge = connector_to_ps8640(connector);
> +
> +       return ps_bridge->bridge.encoder;
> +}
> +
> +static const struct drm_connector_helper_funcs ps8640_connector_helper_funcs = {
> +       .get_modes = ps8640_get_modes,
> +       .best_encoder = ps8640_best_encoder,
> +};
> +
> +static enum drm_connector_status ps8640_detect(struct drm_connector *connector,
> +                                              bool force)
> +{
> +       return connector_status_connected;
> +}
> +
> +static const struct drm_connector_funcs ps8640_connector_funcs = {
> +       .dpms = drm_atomic_helper_connector_dpms,
> +       .fill_modes = drm_helper_probe_single_connector_modes,
> +       .detect = ps8640_detect,
> +       .reset = drm_atomic_helper_connector_reset,
> +       .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +       .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +int ps8640_bridge_attach(struct drm_bridge *bridge)
> +{
> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> +       struct device *dev = &ps_bridge->page[0]->dev;
> +       struct device_node *port, *in_ep;
> +       struct device_node *dsi_node = NULL;
> +       struct mipi_dsi_host *host = NULL;
> +       int ret;
> +
> +       ret = drm_connector_init(bridge->dev, &ps_bridge->connector,
> +                                &ps8640_connector_funcs,
> +                                DRM_MODE_CONNECTOR_eDP);
> +
> +       if (ret) {
> +               DRM_ERROR("Failed to initialize connector with drm: %d\n", ret);
> +               return ret;
> +       }
> +
> +       drm_connector_helper_add(&ps_bridge->connector,
> +                                &ps8640_connector_helper_funcs);
> +
> +       ps_bridge->connector.dpms = DRM_MODE_DPMS_ON;
> +       drm_mode_connector_attach_encoder(&ps_bridge->connector,
> +                                         bridge->encoder);
> +
> +       if (ps_bridge->panel)
> +               drm_panel_attach(ps_bridge->panel, &ps_bridge->connector);
> +
> +       /* port@0 is ps8640 dsi input port */
> +       port = of_graph_get_port_by_id(dev->of_node, 0);
> +       if (port) {
> +               in_ep = of_get_child_by_name(port, "endpoint");
> +               of_node_put(port);
> +               if (in_ep) {
> +                       dsi_node = of_graph_get_remote_port_parent(in_ep);
> +                       of_node_put(in_ep);
> +               }
> +       }
> +       if (dsi_node) {
> +               host = of_find_mipi_dsi_host_by_node(dsi_node);
> +               of_node_put(dsi_node);
> +               if (!host) {
> +                       ret = -ENODEV;
> +                       goto err;
> +               }
> +       }
> +
> +       ps_bridge->dsi.host = host;
> +       ps_bridge->dsi.mode_flags = MIPI_DSI_MODE_VIDEO |
> +                                    MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> +       ps_bridge->dsi.format = MIPI_DSI_FMT_RGB888;
> +       ps_bridge->dsi.lanes = 4;
> +       ret = mipi_dsi_attach(&ps_bridge->dsi);
> +       if (ret)
> +               goto err;
> +
> +       return 0;
> +err:
> +       if (ps_bridge->panel)
> +               drm_panel_detach(ps_bridge->panel);
> +       drm_connector_cleanup(&ps_bridge->connector);
> +       return ret;
> +}
> +
> +static const struct drm_bridge_funcs ps8640_bridge_funcs = {
> +       .attach = ps8640_bridge_attach,
> +       .disable = ps8640_disable,
> +       .post_disable = ps8640_post_disable,
> +       .pre_enable = ps8640_pre_enable,
> +       .enable = ps8640_enable,
> +};
> +
> +/* Firmware Version is returned as Major.Minor */
> +static ssize_t ps8640_fw_version_show(struct device *dev,
> +                                     struct device_attribute *attr, char *buf)
> +{
> +       struct ps8640 *ps_bridge = dev_get_drvdata(dev);
> +       struct ps8640_info *info = &ps_bridge->info;
> +
> +       return scnprintf(buf, PAGE_SIZE, "%u.%u\n", info->version >> 8,
> +                        info->version & 0xff);
> +}
> +
> +/* Hardware Version is returned as FamilyID.VariantID */
> +static ssize_t ps8640_hw_version_show(struct device *dev,
> +                                     struct device_attribute *attr, char *buf)
> +{
> +       struct ps8640 *ps_bridge = dev_get_drvdata(dev);
> +       struct ps8640_info *info = &ps_bridge->info;
> +
> +       return scnprintf(buf, PAGE_SIZE, "ps%u.%u\n", info->family_id,
> +                        info->variant_id);
> +}
> +
> +static int ps8640_spi_send_cmd(struct ps8640 *ps_bridge, u8 *cmd, u8 cmd_len)
> +{
> +       struct i2c_client *client = ps_bridge->page[2];
> +       u8 i, buf[3] = { PAGE2_SWSPI_LEN, cmd_len - 1, TRIGGER_NO_READBACK };
> +       int ret;
> +
> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
> +       if (ret)
> +               goto err;
> +
> +       /* write command in write port */
> +       for (i = 0; i < cmd_len; i++) {
> +               ret = ps8640_write_byte(client, PAGE2_SWSPI_WDATA, cmd[i]);
> +               if (ret)
> +                       goto err_irom_disable;
> +       }
> +
> +       ret = ps8640_write_bytes(client, buf, sizeof(buf));
> +       if (ret)
> +               goto err_irom_disable;
> +
> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
> +       if (ret)
> +               goto err;
> +
> +       return 0;
> +err_irom_disable:
> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
> +err:
> +       dev_err(&client->dev, "send command err: %d\n", ret);
> +       return ret;
> +}
> +
> +static int ps8640_wait_spi_ready(struct ps8640 *ps_bridge)
> +{
> +       struct i2c_client *client = ps_bridge->page[2];
> +       u8 spi_rdy_st;
> +       ktime_t timeout;
> +
> +       timeout = ktime_add_ms(ktime_get(), 200);
> +       for (;;) {
> +               ps8640_read(client, PAGE2_SPI_STATUS, &spi_rdy_st, 1);
> +               if ((spi_rdy_st & SPI_READY) != SPI_READY)
> +                       break;
> +
> +               if (ktime_compare(ktime_get(), timeout) > 0) {
> +                       dev_err(&client->dev, "wait spi ready timeout\n");
> +                       return -EBUSY;
> +               }
> +
> +               msleep(20);
> +       }
> +
> +       return 0;
> +}
> +
> +static int ps8640_wait_spi_nobusy(struct ps8640 *ps_bridge)
> +{
> +       struct i2c_client *client = ps_bridge->page[2];
> +       u8 spi_status, buf[3] = { PAGE2_SWSPI_LEN, 0, TRIGGER_READBACK };
> +       int ret;
> +       ktime_t timeout;
> +
> +       timeout = ktime_add_ms(ktime_get(), 500);
> +       for (;;) {
> +               /* 0x05 RDSR; Read-Status-Register */
> +               ret = ps8640_write_byte(client, PAGE2_SWSPI_WDATA,
> +                                       READ_STATUS_REG_CMD);
> +               if (ret)
> +                       goto err_send_cmd_exit;
> +
> +               ret = ps8640_write_bytes(client, buf, 3);
> +               if (ret)
> +                       goto err_send_cmd_exit;
> +
> +               /* delay for cmd send */
> +               usleep_range(300, 500);
> +               /* wait for SPI ROM until not busy */
> +               ret = ps8640_read(client, PAGE2_SWSPI_RDATA, &spi_status, 1);
> +               if (ret)
> +                       goto err_send_cmd_exit;
> +
> +               if (!(spi_status & BUSY))
> +                       break;
> +
> +               if (ktime_compare(ktime_get(), timeout) > 0) {
> +                       dev_err(&client->dev, "wait spi no busy timeout: %d\n",
> +                               ret);
> +                       return -EBUSY;
> +               }
> +       }
> +
> +       return 0;
> +
> +err_send_cmd_exit:
> +       dev_err(&client->dev, "send command err: %d\n", ret);
> +       return ret;
> +}
> +
> +static int ps8640_wait_rom_idle(struct ps8640 *ps_bridge)
> +{
> +       struct i2c_client *client = ps_bridge->page[0];
> +       int ret;
> +
> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
> +       if (ret)
> +               goto exit;
> +
> +       ret = ps8640_wait_spi_ready(ps_bridge);
> +       if (ret)
> +               goto err_spi;
> +
> +       ret = ps8640_wait_spi_nobusy(ps_bridge);
> +       if (ret)
> +               goto err_spi;
> +
> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
> +       if (ret)
> +               goto exit;
> +
> +       return 0;
> +
> +err_spi:
> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
> +exit:
> +       dev_err(&client->dev, "wait ps8640 rom idle fail: %d\n", ret);
> +
> +       return ret;
> +}
> +
> +static int ps8640_spi_dl_mode(struct ps8640 *ps_bridge)
> +{
> +       struct i2c_client *client = ps_bridge->page[2];
> +       int ret;
> +
> +       /* switch ps8640 mode to spi dl mode */
> +       if (ps_bridge->gpio_mode_sel)
> +               gpiod_set_value(ps_bridge->gpio_mode_sel, 0);
> +
> +       /* reset spi interface */
> +       ret = ps8640_write_byte(client, PAGE2_SW_RESET,
> +                               SPI_SW_RESET | MPU_SW_RESET);
> +       if (ret)
> +               goto exit;
> +
> +       ret = ps8640_write_byte(client, PAGE2_SW_RESET, MPU_SW_RESET);
> +       if (ret)
> +               goto exit;
> +
> +       return 0;
> +
> +exit:
> +       dev_err(&client->dev, "fail reset spi interface: %d\n", ret);
> +
> +       return ret;
> +}
> +
> +static int ps8640_rom_prepare(struct ps8640 *ps_bridge)
> +{
> +       struct i2c_client *client = ps_bridge->page[2];
> +       struct device *dev = &client->dev;
> +       u8 i, cmd[2];
> +       int ret;
> +
> +       cmd[0] = WRITE_ENABLE_CMD;
> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
> +       if (ret) {
> +               dev_err(dev, "failed enable-write-status-register: %d\n", ret);
> +               return ret;
> +       }
> +
> +       cmd[0] = WRITE_STATUS_REG_CMD;
> +       cmd[1] = CLEAR_ALL_PROTECT;
> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 2);
> +       if (ret) {
> +               dev_err(dev, "fail disable all protection: %d\n", ret);
> +               return ret;
> +       }
> +
> +       /* wait for SPI module ready */
> +       ret = ps8640_wait_rom_idle(ps_bridge);
> +       if (ret) {
> +               dev_err(dev, "fail wait rom idle: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
> +       for (i = 0; i < ARRAY_SIZE(enc_ctrl_code); i++)
> +               ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, enc_ctrl_code[i]);
> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
> +
> +       /* Enable-Write-Status-Register */
> +       cmd[0] = WRITE_ENABLE_CMD;
> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
> +       if (ret) {
> +               dev_err(dev, "fail enable-write-status-register: %d\n", ret);
> +               return ret;
> +       }
> +
> +       /* chip erase command */
> +       cmd[0] = CHIP_ERASE_CMD;
> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
> +       if (ret) {
> +               dev_err(dev, "fail disable all protection: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ret = ps8640_wait_rom_idle(ps_bridge);
> +       if (ret) {
> +               dev_err(dev, "fail wait rom idle: %d\n", ret);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static int ps8640_check_chip_id(struct ps8640 *ps_bridge)
> +{
> +       struct i2c_client *client = ps_bridge->page[4];
> +       u8 buf[4];
> +
> +       ps8640_read(client, PAGE4_REV_L, buf, 4);
> +       return memcmp(buf, hw_chip_id, sizeof(buf));
> +}
> +
> +static int ps8640_validate_firmware(struct ps8640 *ps_bridge,
> +                                   const struct firmware *fw)
> +{
> +       struct i2c_client *client = ps_bridge->page[0];
> +       u16 fw_chip_id;
> +
> +       /*
> +        * Get the chip_id from the firmware. Make sure that it is the
> +        * right controller to do the firmware and config update.
> +        */
> +       fw_chip_id = get_unaligned_le16(fw->data + FW_CHIP_ID_OFFSET);
> +
> +       if (fw_chip_id != 0x8640 && ps8640_check_chip_id(ps_bridge) == 0) {
> +               dev_err(&client->dev,
> +                       "chip id mismatch: fw 0x%x vs. chip 0x8640\n",
> +                       fw_chip_id);
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static int ps8640_write_rom(struct ps8640 *ps_bridge, const struct firmware *fw)
> +{
> +       struct i2c_client *client = ps_bridge->page[0];
> +       struct device *dev = &client->dev;
> +       struct i2c_client *client2 = ps_bridge->page[2];
> +       struct i2c_client *client7 = ps_bridge->page[7];
> +       size_t pos, cpy_len;
> +       u8 buf[257];
> +       int ret;
> +
> +       ps8640_write_byte(client2, PAGE2_SPI_CFG3, I2C_TO_SPI_RESET);
> +       msleep(100);
> +       ps8640_write_byte(client2, PAGE2_SPI_CFG3, 0x00);
> +
> +       for (pos = 0; pos < fw->size; pos += cpy_len) {
> +               buf[0] = PAGE2_ROMADD_BYTE1;
> +               buf[1] = pos >> 8;
> +               buf[2] = pos >> 16;
> +               ret = ps8640_write_bytes(client2, buf, 3);
> +               if (ret)
> +                       goto error;
> +               cpy_len = fw->size >= 256 + pos ? 256 : fw->size - pos;
> +               buf[0] = 0;
> +               memcpy(buf + 1, fw->data + pos, cpy_len);
> +               ret = ps8640_write_bytes(client7, buf, cpy_len + 1);
> +               if (ret)
> +                       goto error;
> +
> +               dev_dbg(dev, "fw update completed %zu / %zu bytes\n", pos,
> +                       fw->size);
> +       }
> +       return 0;
> +
> +error:
> +       dev_err(dev, "failed write external flash, %d\n", ret);
> +       return ret;
> +}
> +
> +static int ps8640_spi_normal_mode(struct ps8640 *ps_bridge)
> +{
> +       u8 cmd[2];
> +       struct i2c_client *client = ps_bridge->page[2];
> +
> +       /* Enable-Write-Status-Register */
> +       cmd[0] = WRITE_ENABLE_CMD;
> +       ps8640_spi_send_cmd(ps_bridge, cmd, 1);
> +
> +       /* protect BPL/BP0/BP1 */
> +       cmd[0] = WRITE_STATUS_REG_CMD;
> +       cmd[1] = BLK_PROTECT_BITS | STATUS_REG_PROTECT;
> +       ps8640_spi_send_cmd(ps_bridge, cmd, 2);
> +
> +       /* wait for SPI rom ready */
> +       ps8640_wait_rom_idle(ps_bridge);
> +
> +       /* disable PS8640 mapping function */
> +       ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, 0x00);
> +
> +       if (ps_bridge->gpio_mode_sel)
> +               gpiod_set_value(ps_bridge->gpio_mode_sel, 1);
> +       return 0;
> +}
> +
> +static int ps8640_enter_bl(struct ps8640 *ps_bridge)
> +{
> +       ps_bridge->in_fw_update = true;
> +       return ps8640_spi_dl_mode(ps_bridge);
> +}
> +
> +static void ps8640_exit_bl(struct ps8640 *ps_bridge, const struct firmware *fw)
> +{
> +       ps8640_spi_normal_mode(ps_bridge);
> +       ps_bridge->in_fw_update = false;
> +}
> +
> +static int ps8640_load_fw(struct ps8640 *ps_bridge, const struct firmware *fw)
> +{
> +       struct i2c_client *client = ps_bridge->page[0];
> +       struct device *dev = &client->dev;
> +       int ret;
> +       bool ps8640_status_backup = ps_bridge->enabled;
> +
> +       ret = ps8640_validate_firmware(ps_bridge, fw);
> +       if (ret)
> +               return ret;
> +
> +       mutex_lock(&ps_bridge->fw_mutex);
> +       if (!ps_bridge->in_fw_update) {
> +               if (!ps8640_status_backup)
> +                       ps8640_pre_enable(&ps_bridge->bridge);
> +
> +               ret = ps8640_enter_bl(ps_bridge);
> +               if (ret)
> +                       goto exit;
> +       }
> +
> +       ret = ps8640_rom_prepare(ps_bridge);
> +       if (ret)
> +               goto exit;
> +
> +       ret = ps8640_write_rom(ps_bridge, fw);
> +
> +exit:
> +       if (ret)
> +               dev_err(dev, "Failed to load firmware, %d\n", ret);
> +
> +       ps8640_exit_bl(ps_bridge, fw);
> +       if (!ps8640_status_backup)
> +               ps8640_post_disable(&ps_bridge->bridge);
> +       mutex_unlock(&ps_bridge->fw_mutex);
> +       return ret;
> +}
> +
> +static ssize_t ps8640_update_fw_store(struct device *dev,
> +                                     struct device_attribute *attr,
> +                                     const char *buf, size_t count)
> +{
> +       struct i2c_client *client = to_i2c_client(dev);
> +       struct ps8640 *ps_bridge = i2c_get_clientdata(client);
> +       const struct firmware *fw;
> +       int error;
> +
> +       error = request_firmware(&fw, PS_FW_NAME, dev);
> +       if (error) {
> +               dev_err(dev, "Unable to open firmware %s: %d\n",
> +                       PS_FW_NAME, error);
> +               return error;
> +       }
> +
> +       error = ps8640_load_fw(ps_bridge, fw);
> +       if (error)
> +               dev_err(dev, "The firmware update failed(%d)\n", error);
> +       else
> +               dev_info(dev, "The firmware update succeeded\n");
> +
> +       release_firmware(fw);
> +       return error ? error : count;
> +}
> +
> +static DEVICE_ATTR(fw_version, S_IRUGO, ps8640_fw_version_show, NULL);
> +static DEVICE_ATTR(hw_version, S_IRUGO, ps8640_hw_version_show, NULL);
> +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, ps8640_update_fw_store);
> +
> +static struct attribute *ps8640_attrs[] = {
> +       &dev_attr_fw_version.attr,
> +       &dev_attr_hw_version.attr,
> +       &dev_attr_update_fw.attr,
> +       NULL
> +};
> +
> +static const struct attribute_group ps8640_attr_group = {
> +       .attrs = ps8640_attrs,
> +};
> +
> +static void ps8640_remove_sysfs_group(void *data)
> +{
> +       struct ps8640 *ps_bridge = data;
> +
> +       sysfs_remove_group(&ps_bridge->page[0]->dev.kobj, &ps8640_attr_group);
> +}
> +
> +static int ps8640_probe(struct i2c_client *client,
> +                       const struct i2c_device_id *id)
> +{
> +       struct device *dev = &client->dev;
> +       struct ps8640 *ps_bridge;
> +       struct device_node *np = dev->of_node;
> +       struct device_node *port, *out_ep;
> +       struct device_node *panel_node = NULL;
> +       int ret;
> +       u32 i;
> +
> +       ps_bridge = devm_kzalloc(dev, sizeof(*ps_bridge), GFP_KERNEL);
> +       if (!ps_bridge)
> +               return -ENOMEM;
> +
> +       /* port@1 is ps8640 output port */
> +       port = of_graph_get_port_by_id(np, 1);
> +       if (port) {
> +               out_ep = of_get_child_by_name(port, "endpoint");
> +               of_node_put(port);
> +               if (out_ep) {
> +                       panel_node = of_graph_get_remote_port_parent(out_ep);
> +                       of_node_put(out_ep);
> +               }
> +       }
> +       if (panel_node) {
> +               ps_bridge->panel = of_drm_find_panel(panel_node);
> +               of_node_put(panel_node);
> +               if (!ps_bridge->panel)
> +                       return -EPROBE_DEFER;
> +       }
> +
> +       mutex_init(&ps_bridge->fw_mutex);
> +       ps_bridge->supplies[0].supply = "vdd33";
> +       ps_bridge->supplies[1].supply = "vdd12";
> +       ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ps_bridge->supplies),
> +                                     ps_bridge->supplies);
> +       if (ret) {
> +               dev_info(dev, "failed to get regulators: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ps_bridge->gpio_mode_sel = devm_gpiod_get_optional(&client->dev,
> +                                                            "mode-sel",
> +                                                            GPIOD_OUT_HIGH);
> +       if (IS_ERR(ps_bridge->gpio_mode_sel)) {
> +               ret = PTR_ERR(ps_bridge->gpio_mode_sel);
> +               dev_err(dev, "cannot get mode-sel %d\n", ret);
> +               return ret;
> +       }
> +
> +       ps_bridge->gpio_power_down = devm_gpiod_get(&client->dev, "sleep",
> +                                              GPIOD_OUT_LOW);
> +       if (IS_ERR(ps_bridge->gpio_power_down)) {
> +               ret = PTR_ERR(ps_bridge->gpio_power_down);
> +               dev_err(dev, "cannot get sleep: %d\n", ret);
> +               return ret;
> +       }
> +
> +       /*
> +        * Request the reset pin low to avoid the bridge being
> +        * initialized prematurely
> +        */
> +       ps_bridge->gpio_reset = devm_gpiod_get(&client->dev, "reset",
> +                                              GPIOD_OUT_LOW);
> +       if (IS_ERR(ps_bridge->gpio_reset)) {
> +               ret = PTR_ERR(ps_bridge->gpio_reset);
> +               dev_err(dev, "cannot get reset: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ps_bridge->bridge.funcs = &ps8640_bridge_funcs;
> +       ps_bridge->bridge.of_node = dev->of_node;
> +
> +       ps_bridge->page[0] = client;
> +       ps_bridge->ddc_i2c = i2c_new_dummy(client->adapter, EDID_I2C_ADDR);
> +       if (!ps_bridge->ddc_i2c) {
> +               dev_err(dev, "failed ddc_i2c dummy device, address%02x\n",
> +                       EDID_I2C_ADDR);
> +               return -EBUSY;
> +       }
> +       /*
> +        * ps8640 uses multiple addresses, use dummy devices for them
> +        * page[0]: for DP control
> +        * page[1]: for VIDEO Bridge
> +        * page[2]: for control top
> +        * page[3]: for DSI Link Control1
> +        * page[4]: for MIPI Phy
> +        * page[5]: for VPLL
> +        * page[6]: for DSI Link Control2
> +        * page[7]: for spi rom mapping
> +        */
> +       for (i = 1; i < MAX_DEVS; i++) {
> +               ps_bridge->page[i] = i2c_new_dummy(client->adapter,
> +                                                  client->addr + i);
> +               if (!ps_bridge->page[i]) {
> +                       dev_err(dev, "failed i2c dummy device, address%02x\n",
> +                               client->addr + i);
> +                       ret = -EBUSY;
> +                       goto exit_dummy;
> +               }
> +       }
> +       i2c_set_clientdata(client, ps_bridge);
> +
> +       ret = sysfs_create_group(&client->dev.kobj, &ps8640_attr_group);
> +       if (ret) {
> +               dev_err(dev, "failed to create sysfs entries: %d\n", ret);
> +               goto exit_dummy;
> +       }
> +
> +       ret = devm_add_action(dev, ps8640_remove_sysfs_group, ps_bridge);
> +       if (ret) {
> +               dev_err(dev, "failed to add sysfs cleanup action: %d\n", ret);
> +               goto exit_remove_sysfs;
> +       }
> +
> +       ret = drm_bridge_add(&ps_bridge->bridge);
> +       if (ret) {
> +               dev_err(dev, "Failed to add bridge: %d\n", ret);
> +               goto exit_remove_sysfs;
> +       }
> +       return 0;
> +
> +exit_remove_sysfs:
> +       sysfs_remove_group(&ps_bridge->page[0]->dev.kobj, &ps8640_attr_group);
> +exit_dummy:
> +       while (--i)
> +               i2c_unregister_device(ps_bridge->page[i]);
> +       i2c_unregister_device(ps_bridge->ddc_i2c);
> +       return ret;
> +}
> +
> +static int ps8640_remove(struct i2c_client *client)
> +{
> +       struct ps8640 *ps_bridge = i2c_get_clientdata(client);
> +       int i = MAX_DEVS;
> +
> +       drm_bridge_remove(&ps_bridge->bridge);
> +       sysfs_remove_group(&ps_bridge->page[0]->dev.kobj, &ps8640_attr_group);
> +       while (--i)
> +               i2c_unregister_device(ps_bridge->page[i]);
> +
> +       i2c_unregister_device(ps_bridge->ddc_i2c);
> +       return 0;
> +}
> +
> +static const struct i2c_device_id ps8640_i2c_table[] = {
> +       { "parade,ps8640", 0 },

I think that you should remove the manufacturer prefix here, note that
the I2C core removes the manufacturer prefix from the compatible field
so it reports to user-space the uevent: i2c:ps8640, but this doesn't
match with the device id which is parade,ps8640. If you build the
driver as a module will not autoload the driver, to fix this just
remove the manufacturer prefix.

> +       {},

nit: add { /* sentinel */ },

> +};
> +MODULE_DEVICE_TABLE(i2c, ps8640_i2c_table);
> +
> +static const struct of_device_id ps8640_match[] = {
> +       { .compatible = "parade,ps8640" },
> +       {},

nit: add { /* sentinel */ },

> +};
> +MODULE_DEVICE_TABLE(of, ps8640_match);
> +
> +static struct i2c_driver ps8640_driver = {
> +       .id_table = ps8640_i2c_table,
> +       .probe = ps8640_probe,
> +       .remove = ps8640_remove,
> +       .driver = {
> +               .name = "parade,ps8640",

Also remove the manufacturer from the name, guess it's more common.

> +               .of_match_table = ps8640_match,
> +       },
> +};
> +module_i2c_driver(ps8640_driver);
> +
> +MODULE_AUTHOR("Jitao Shi <jitao.shi@mediatek.com>");
> +MODULE_AUTHOR("CK Hu <ck.hu@mediatek.com>");
> +MODULE_DESCRIPTION("PARADE ps8640 DSI-eDP converter driver");
> +MODULE_LICENSE("GPL v2");
> --
> 1.7.9.5
>

A part of these minor things.

Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply

* linux-mediatek on IRC freenode
From: Matthias Brugger @ 2016-11-10 16:51 UTC (permalink / raw)
  To: moderated list:ARM/Mediatek SoC support; +Cc: Jiri Kastner

Hi all,

Jiri Kastner created the IRC channel #linux-mediatek on freenode quite 
some time ago.

I just wanted to invite everyone interested in Mediatek SoCs running an 
official Linux kernel to join the channel.

Please feel free to join and help create a vibrant community around 
Mediatek SoCs.

Thanks a lot,
Matthias

^ permalink raw reply

* Re: [PATCH v2 2/3] irqchip: mtk-cirq: Add mediatek mtk-cirq implement
From: Marc Zyngier @ 2016-11-10 18:24 UTC (permalink / raw)
  To: Youlin Pei
  Cc: Rob Herring, Matthias Brugger, Thomas Gleixner, Jason Cooper,
	Mark Rutland, Russell King, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
	hongkun.cao-NuS5LvNUpcJWk0Htik3J/w,
	yong.wu-NuS5LvNUpcJWk0Htik3J/w, erin.lo-NuS5LvNUpcJWk0Htik3J/w,
	chieh-jay.liu-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <1478573833.32099.11.camel@mtksdaap41>

On 08/11/16 02:57, Youlin Pei wrote:
> On Fri, 2016-11-04 at 22:21 +0000, Marc Zyngier wrote:
>> On Fri, Nov 04 2016 at 04:42:57 AM, Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>>> On Tue, 2016-11-01 at 20:49 +0000, Marc Zyngier wrote:
>>>> On Tue, Nov 01 2016 at 11:52:01 AM, Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>>>>> In Mediatek SOCs, the CIRQ is a low power interrupt controller
>>>>> designed to works outside MCUSYS which comprises with Cortex-Ax
>>>>> cores,CCI and GIC.
>>>>>
>>>>> The CIRQ controller is integrated in between MCUSYS( include
>>>>> Cortex-Ax, CCI and GIC ) and interrupt sources as the second
>>>>> level interrupt controller. The external interrupts which outside
>>>>> MCUSYS will feed through CIRQ then bypass to GIC. CIRQ can monitors
>>>>> all edge trigger interupts. When an edge interrupt is triggered,
>>>>> CIRQ can record the status and generate a pulse signal to GIC when
>>>>> flush command executed.
>>>>>
>>>>> When system enters sleep mode, MCUSYS will be turned off to improve
>>>>> power consumption, also GIC is power down. The edge trigger interrupts
>>>>> will be lost in this scenario without CIRQ.
>>>>>
>>>>> This commit provides the CIRQ irqchip implement.
>>>>>
>>>>> Signed-off-by: Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>>>> ---
>>>>>  drivers/irqchip/Makefile       |    2 +-
>>>>>  drivers/irqchip/irq-mtk-cirq.c |  262 ++++++++++++++++++++++++++++++++++++++++
>>>>>  2 files changed, 263 insertions(+), 1 deletion(-)
>>>>>  create mode 100644 drivers/irqchip/irq-mtk-cirq.c
>>>>>
>>>>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>>>>> index e4dbfc8..8f33580 100644
>>>>> --- a/drivers/irqchip/Makefile
>>>>> +++ b/drivers/irqchip/Makefile
>>>>> @@ -60,7 +60,7 @@ obj-$(CONFIG_BCM7120_L2_IRQ)		+= irq-bcm7120-l2.o
>>>>>  obj-$(CONFIG_BRCMSTB_L2_IRQ)		+= irq-brcmstb-l2.o
>>>>>  obj-$(CONFIG_KEYSTONE_IRQ)		+= irq-keystone.o
>>>>>  obj-$(CONFIG_MIPS_GIC)			+= irq-mips-gic.o
>>>>> -obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mtk-sysirq.o
>>>>> +obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mtk-sysirq.o irq-mtk-cirq.o
>>>>>  obj-$(CONFIG_ARCH_DIGICOLOR)		+= irq-digicolor.o
>>>>>  obj-$(CONFIG_RENESAS_H8300H_INTC)	+= irq-renesas-h8300h.o
>>>>>  obj-$(CONFIG_RENESAS_H8S_INTC)		+= irq-renesas-h8s.o
>>>>> diff --git a/drivers/irqchip/irq-mtk-cirq.c b/drivers/irqchip/irq-mtk-cirq.c
>>>>> new file mode 100644
>>>>> index 0000000..fc43ef3
>>>>> --- /dev/null
>>>>> +++ b/drivers/irqchip/irq-mtk-cirq.c
>>>>> @@ -0,0 +1,262 @@
>>>>> +/*
>>>>> + * Copyright (c) 2016 MediaTek Inc.
>>>>> + * Author: Youlin.Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>>>> + *
>>>>> + * This program is free software; you can redistribute it and/or modify
>>>>> + * it under the terms of the GNU General Public License version 2 as
>>>>> + * published by the Free Software Foundation.
>>>>> + *
>>>>> + * 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 <linux/irq.h>
>>>>> +#include <linux/irqchip.h>
>>>>> +#include <linux/irqdomain.h>
>>>>> +#include <linux/of.h>
>>>>> +#include <linux/of_irq.h>
>>>>> +#include <linux/of_address.h>
>>>>> +#include <linux/io.h>
>>>>> +#include <linux/slab.h>
>>>>> +#include <linux/syscore_ops.h>
>>>>> +
>>>>> +#define CIRQ_ACK	0x40
>>>>> +#define CIRQ_MASK_SET	0xc0
>>>>> +#define CIRQ_MASK_CLR	0x100
>>>>> +#define CIRQ_SENS_SET	0x180
>>>>> +#define CIRQ_SENS_CLR	0x1c0
>>>>> +#define CIRQ_POL_SET	0x240
>>>>> +#define CIRQ_POL_CLR	0x280
>>>>> +#define CIRQ_CONTROL	0x300
>>>>> +
>>>>> +#define CIRQ_EN	0x1
>>>>> +#define CIRQ_EDGE	0x2
>>>>> +#define CIRQ_FLUSH	0x4
>>>>> +
>>>>> +#define CIRQ_IRQ_NUM    0x200
>>>>> +
>>>>> +struct mtk_cirq_chip_data {
>>>>> +	void __iomem *base;
>>>>> +	unsigned int ext_irq_start;
>>>>> +};
>>>>> +
>>>>> +static struct mtk_cirq_chip_data *cirq_data;
>>>>
>>>> Are you guaranteed that you'll only ever have a single CIRQ in any
>>>> system?
>>>
>>> In Mediatek's SOC, only hace a single CIRQ.
>>>
>>>>
>>>>> +
>>>>> +static void mtk_cirq_write_mask(struct irq_data *data, unsigned int offset)
>>>>> +{
>>>>> +	struct mtk_cirq_chip_data *chip_data = data->chip_data;
>>>>> +	unsigned int cirq_num = data->hwirq;
>>>>> +	u32 mask = 1 << (cirq_num % 32);
>>>>> +
>>>>> +	writel(mask, chip_data->base + offset + (cirq_num / 32) * 4);
>>>>
>>>> Why can't you use the relaxed accessors?
>>>
>>> It seems that i use wrong function, i will change the writel to
>>> writel_relaxed in next ve
>>>
>>>>
>>>>> +}
>>>>> +
>>>>> +static void mtk_cirq_mask(struct irq_data *data)
>>>>> +{
>>>>> +	mtk_cirq_write_mask(data, CIRQ_MASK_SET);
>>>>> +	irq_chip_mask_parent(data);
>>>>> +}
>>>>> +
>>>>> +static void mtk_cirq_unmask(struct irq_data *data)
>>>>> +{
>>>>> +	mtk_cirq_write_mask(data, CIRQ_MASK_CLR);
>>>>> +	irq_chip_unmask_parent(data);
>>>>> +}
>>>>> +
>>>>> +static void mtk_cirq_eoi(struct irq_data *data)
>>>>> +{
>>>>> +	mtk_cirq_write_mask(data, CIRQ_ACK);
>>>>
>>>> EOI and ACK have very different semantics. What is this write actually
>>>> doing? Also, you're now doing an additional MMIO write on each interrupt
>>>> EOI, doubling its cost. Do you really need to do actually signal the HW
>>>> that we've EOIed an interrupt? I would have hoped that you'd be able to
>>>> put it in "bypass" mode as long as you're not suspending...
>>>>
>>>
>>> When external interrupt happened, CIRQ status register record the status
>>> even CIRQ is not enabled. when execute the flush command, CIRQ will
>>> resend the signals according to the status. So if don't clear the
>>> status, CIRQ will resend the wrong signals. the ACK write operation will
>>> clear the status.
>>
>> But at this time, we haven't suspended yet, and there is nothing to
>> replay. Also, you only enable the edge capture when suspending. So what
>> are you ACKing here? Can't you simply clear everything right when
>> suspending and not do it at all on the fast path?
> 
> I had planned to ACK the status in cirq suspend callback, but
> encountered a problem.
> following is a piece of code from
> http://lxr.free-electrons.com/source/kernel/power/suspend.c#L361
> 
> arch_suspend_disable_irqs(); ---step1
> BUG_ON(!irqs_disabled());
> 
> error = syscore_suspend();
>            |
>            ---cirq suspend(); ---step2
> 
> if ack the status in cirq suspend, the interrupts will be lost which
> happened during step1 to step2.
> 
> So would you mind give me some suggestions about this?
> Thanks a lot!

Right. So maybe there is a way:
- On suspend you can iterate over all the cirq interrupts that have been
recorded
- For each of those, you inspect if it is pending at the GIC level
(using the irq_get_irqchip_state helper)
- if not pending, you Ack it, because it cannot be delivered anymore
- If it is pending, then you know it happened between step 1 and step 2.
- You then have the choice of either going into suspend and waking up
immediately, or failing the suspend.

Thoughts?

	M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 0/2] mmc: allow mmc_alloc_host() and tmio_mmc_host_alloc()
From: Masahiro Yamada @ 2016-11-11  3:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Ulf Hansson, Adrian Hunter, Wolfram Sang, Sascha Sommer,
	Johan Hovold, Sonic Zhang, devel, Russell King, Jaehoon Chung,
	Chen-Yu Tsai, Pierre Ossman, Harald Welte, Alex Dubov,
	adi-buildroot-devel, Michał Mirosław,
	moderated list:ARM/Mediatek SoC support, Ben Dooks, Tony Olech,
	Matthias Brugger, linux-omap, linux-arm-kernel
In-Reply-To: <20161110133559.GA10191@kroah.com>

2016-11-10 22:35 GMT+09:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> On Thu, Nov 10, 2016 at 10:24:21PM +0900, Masahiro Yamada wrote:
>>
>> sdhci_alloc_host() returns an error pointer when it fails.
>> but mmc_alloc_host() cannot.
>>
>> This series allow to propagate a proper error code
>> when host-allocation fails.
>
> Why?  What can we really do about the error except give up?  Why does
> having a explicit error value make any difference to the caller, they
> can't do anything different, right?


The error code is shown in the console, like

  probe of 5a000000.sdhc failed with error -12


The proper error code will give a clue
why the driver failed to probe.


> I suggest just leaving it as-is, it's simple, and you don't have to mess
> with PTR_ERR() anywhere.


Why?

Most of driver just give up probing for any error,
but we still do ERR_PTR()/PTR_ERR() here and there.
I think this patch is the same pattern.

If a function returns NULL on failure, we need to think about
"what is the most common failure case".

Currently, MMC drivers assume -ENOMEM is the best
fit for mmc_alloc_host(), but the assumption is fragile.

Already, mmc_alloc_host() calls a function
that returns not only -ENOMEM, but also -ENOSPC.

In the future, some other failure cases might be
added to mmc_alloc_host().

Once we decide the API returns an error pointer,
drivers just propagate the return value from the API.
This is much more stable implementation.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH v16 0/5] Mediatek MT8173 CMDQ support
From: Jassi Brar @ 2016-11-11  5:45 UTC (permalink / raw)
  To: Horng-Shyang Liao
  Cc: Rob Herring, Matthias Brugger, Daniel Kurtz, Sascha Hauer,
	Devicetree List, Linux Kernel Mailing List,
	linux-arm-kernel@lists.infradead.org, linux-mediatek,
	srv_heupstream, Sascha Hauer, Philipp Zabel, Nicolas Boichat,
	CK HU, cawa cheng, Bibby Hsieh, YT Shen, Daoyuan Huang, Damon Chu
In-Reply-To: <1478776558.15447.2.camel@mtksdaap41>

On Thu, Nov 10, 2016 at 4:45 PM, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:
> On Tue, 2016-11-01 at 19:28 +0800, HS Liao wrote:
>> Hi,
>>
>> This is Mediatek MT8173 Command Queue(CMDQ) driver. The CMDQ is used
>> to help write registers with critical time limitation, such as
>> updating display configuration during the vblank. It controls Global
>> Command Engine (GCE) hardware to achieve this requirement.
>>
>> These patches have a build dependency on top of v4.9-rc1.
>>
>> Changes since v15:
>>  - separate "suspend and resume" patch from "save energy" patch
>>  - don't stop running tasks in cmdq_suspend()
>>    (i.e. leave no running tasks guarantee to clients)
>>
>> Best regards,
>> HS Liao
>>
>> HS Liao (5):
>>   dt-bindings: soc: Add documentation for the MediaTek GCE unit
>>   CMDQ: Mediatek CMDQ driver
>>   arm64: dts: mt8173: Add GCE node
>>   CMDQ: suspend and resume
>>   CMDQ: save energy
>>
>>  .../devicetree/bindings/mailbox/mtk-gce.txt        |  43 ++
>>  arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  10 +
>>  drivers/mailbox/Kconfig                            |  10 +
>>  drivers/mailbox/Makefile                           |   2 +
>>  drivers/mailbox/mtk-cmdq-mailbox.c                 | 632 +++++++++++++++++++++
>>  drivers/soc/mediatek/Kconfig                       |  11 +
>>  drivers/soc/mediatek/Makefile                      |   1 +
>>  drivers/soc/mediatek/mtk-cmdq-helper.c             | 310 ++++++++++
>>  include/linux/mailbox/mtk-cmdq-mailbox.h           |  67 +++
>>  include/linux/soc/mediatek/mtk-cmdq.h              | 182 ++++++
>>  10 files changed, 1268 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/mtk-gce.txt
>>  create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c
>>  create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c
>>  create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h
>>  create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h
>>
>
>
> Hi Jassi, Matthias,
>
> Sorry to disturb you.
>
No, you don't disturb, but the controller driver and protocol driver,
introduced in the same patch, does :)   So does the suspend/resume
support (patch 4&5) added  separately as a patch on top. Please
reorganise the patchset.

Thanks.

^ permalink raw reply

* Re: [v17 2/2] drm/bridge: Add I2C based driver for ps8640 bridge
From: Archit Taneja @ 2016-11-11  6:02 UTC (permalink / raw)
  To: Enric Balletbo Serra, Jitao Shi, djkurtz-F7+t8E8rja9g9hUCZPvPmw
  Cc: David Airlie, Thierry Reding, Matthias Brugger, Mark Rutland,
	stonea168-9Onoh4P/yGk, dri-devel, Andy Yan, Ajay Kumar,
	Vincent Palatin, cawa cheng, bibby.hsieh-NuS5LvNUpcJWk0Htik3J/w,
	CK HU, Russell King,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sascha Hauer,
	Pawel Moll, Ian Campbell, Inki Dae, Rob Herring, ARM/Mediatek 
In-Reply-To: <CAFqH_52A4=GubRjPa8X1=STExXFZy2HdoASEGUBYwqGt0BEFcw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Jitao,

I couldn't locate the original mail, so posting on this thread instead.
Some comments below.

On 11/10/2016 10:09 PM, Enric Balletbo Serra wrote:
> Hi Jitao,
>
> 2016-08-27 8:44 GMT+02:00 Jitao Shi <jitao.shi-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>:
>> This patch adds drm_bridge driver for parade DSI to eDP bridge chip.
>>
>> Signed-off-by: Jitao Shi <jitao.shi-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> ---
>> Changes since v16:
>>  - Disable ps8640 DSI MCS Function.
>>  - Rename gpios name more clearly.
>>  - Tune the ps8640 power on sequence.
>>
>> Changes since v15:
>>  - Drop drm_connector_(un)register calls from parade ps8640.
>>    The main DRM driver mtk_drm_drv now calls
>>    drm_connector_register_all() after drm_dev_register() in the
>>    mtk_drm_bind() function. That function should iterate over all
>>    connectors and call drm_connector_register() for each of them.
>>    So, remove drm_connector_(un)register calls from parade ps8640.
>>
>> Changes since v14:
>>  - update copyright info.
>>  - change bridge_to_ps8640 and connector_to_ps8640 to inline function.
>>  - fix some coding style.
>>  - use sizeof as array counter.
>>  - use drm_get_edid when read edid.
>>  - add mutex when firmware updating.
>>
>> Changes since v13:
>>  - add const on data, ps8640_write_bytes(struct i2c_client *client, const u8 *data, u16 data_len)
>>  - fix PAGE2_SW_REST tyro.
>>  - move the buf[3] init to entrance of the function.
>>
>> Changes since v12:
>>  - fix hw_chip_id build warning
>>
>> Changes since v11:
>>  - Remove depends on I2C, add DRM depends
>>  - Reuse ps8640_write_bytes() in ps8640_write_byte()
>>  - Use timer check for polling like the routines in <linux/iopoll.h>
>>  - Fix no drm_connector_unregister/drm_connector_cleanup when ps8640_bridge_attach fail
>>  - Check the ps8640 hardware id in ps8640_validate_firmware
>>  - Remove fw_version check
>>  - Move ps8640_validate_firmware before ps8640_enter_bl
>>  - Add ddc_i2c unregister when probe fail and ps8640_remove
>> ---
>>  drivers/gpu/drm/bridge/Kconfig         |   12 +
>>  drivers/gpu/drm/bridge/Makefile        |    1 +
>>  drivers/gpu/drm/bridge/parade-ps8640.c | 1077 ++++++++++++++++++++++++++++++++
>>  3 files changed, 1090 insertions(+)
>>  create mode 100644 drivers/gpu/drm/bridge/parade-ps8640.c
>>
>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>> index b590e67..c59d043 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -50,6 +50,18 @@ config DRM_PARADE_PS8622
>>         ---help---
>>           Parade eDP-LVDS bridge chip driver.
>>
>> +config DRM_PARADE_PS8640
>> +       tristate "Parade PS8640 MIPI DSI to eDP Converter"
>> +       depends on DRM
>> +       depends on OF
>> +       select DRM_KMS_HELPER
>> +       select DRM_MIPI_DSI
>> +       select DRM_PANEL
>> +       ---help---
>> +         Choose this option if you have PS8640 for display
>> +         The PS8640 is a high-performance and low-power
>> +         MIPI DSI to eDP converter
>> +
>>  config DRM_SII902X
>>         tristate "Silicon Image sii902x RGB/HDMI bridge"
>>         depends on OF
>> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
>> index efdb07e..3360537 100644
>> --- a/drivers/gpu/drm/bridge/Makefile
>> +++ b/drivers/gpu/drm/bridge/Makefile
>> @@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>>  obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
>>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
>>  obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
>> +obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
>>  obj-$(CONFIG_DRM_SII902X) += sii902x.o
>>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
>> new file mode 100644
>> index 0000000..7d67431
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
>> @@ -0,0 +1,1077 @@
>> +/*
>> + * Copyright (c) 2016 MediaTek Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * 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 <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/firmware.h>
>> +#include <linux/gpio.h>

Not needed.

>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_gpio.h>

The above 2 aren't needed.

>> +#include <linux/of_graph.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <asm/unaligned.h>
>> +#include <drm/drm_panel.h>
>> +
>> +#include <drmP.h>
>> +#include <drm_atomic_helper.h>
>> +#include <drm_crtc_helper.h>
>> +#include <drm_crtc.h>

Not needed.

>> +#include <drm_edid.h>
>> +#include <drm_mipi_dsi.h>
>> +
>> +#define PAGE1_VSTART           0x6b
>> +#define PAGE2_SPI_CFG3         0x82
>> +#define I2C_TO_SPI_RESET       0x20
>> +#define PAGE2_ROMADD_BYTE1     0x8e
>> +#define PAGE2_ROMADD_BYTE2     0x8f
>> +#define PAGE2_SWSPI_WDATA      0x90
>> +#define PAGE2_SWSPI_RDATA      0x91
>> +#define PAGE2_SWSPI_LEN                0x92
>> +#define PAGE2_SWSPI_CTL                0x93
>> +#define TRIGGER_NO_READBACK    0x05
>> +#define TRIGGER_READBACK       0x01
>> +#define PAGE2_SPI_STATUS       0x9e
>> +#define SPI_READY              0x0c
>> +#define PAGE2_GPIO_L           0xa6
>> +#define PAGE2_GPIO_H           0xa7
>> +#define PS_GPIO9               BIT(1)
>> +#define PAGE2_IROM_CTRL                0xb0
>> +#define IROM_ENABLE            0xc0
>> +#define IROM_DISABLE           0x80
>> +#define PAGE2_SW_RESET         0xbc
>> +#define SPI_SW_RESET           BIT(7)
>> +#define MPU_SW_RESET           BIT(6)
>> +#define PAGE2_ENCTLSPI_WR      0xda
>> +#define PAGE2_I2C_BYPASS       0xea
>> +#define I2C_BYPASS_EN          0xd0
>> +#define PAGE2_MCS_EN           0xf3
>> +#define MCS_EN                 BIT(0)
>> +#define PAGE3_SET_ADD          0xfe
>> +#define PAGE3_SET_VAL          0xff
>> +#define VDO_CTL_ADD            0x13
>> +#define VDO_DIS                        0x18
>> +#define VDO_EN                 0x1c
>> +#define PAGE4_REV_L            0xf0
>> +#define PAGE4_REV_H            0xf1
>> +#define PAGE4_CHIP_L           0xf2
>> +#define PAGE4_CHIP_H           0xf3
>> +
>> +/* Firmware */
>> +#define PS_FW_NAME             "ps864x_fw.bin"
>> +
>
> About the firmware discussion I think that if you want to maintain the
> upgrade firmware thing you should also include this patch in the
> series.
>
>  https://chromium-review.googlesource.com/#/c/317221/
>
> Otherwise, if this is not really needed I think that remove this from
> the driver is the best. Just an opinion, this is something the
> maintainer should decide.

Was there a conclusion on this? As Daniel Kurtz suggested, can we drop
the update firmware stuff for now and try to get the functional part
for 4.10?

>
>> +#define FW_CHIP_ID_OFFSET      0
>> +#define FW_VERSION_OFFSET      2
>> +#define EDID_I2C_ADDR          0x50
>> +
>> +#define WRITE_STATUS_REG_CMD   0x01
>> +#define READ_STATUS_REG_CMD    0x05
>> +#define BUSY                   BIT(0)
>> +#define CLEAR_ALL_PROTECT      0x00
>> +#define BLK_PROTECT_BITS       0x0c
>> +#define STATUS_REG_PROTECT     BIT(7)
>> +#define WRITE_ENABLE_CMD       0x06
>> +#define CHIP_ERASE_CMD         0xc7
>> +#define MAX_DEVS               0x8
>> +
>> +struct ps8640_info {
>> +       u8 family_id;
>> +       u8 variant_id;
>> +       u16 version;
>> +};
>> +
>> +struct ps8640 {
>> +       struct drm_connector connector;
>> +       struct drm_bridge bridge;
>> +       struct edid *edid;
>> +       struct mipi_dsi_device dsi;
>> +       struct i2c_client *page[MAX_DEVS];
>> +       struct i2c_client *ddc_i2c;
>> +       struct regulator_bulk_data supplies[2];
>> +       struct drm_panel *panel;
>> +       struct gpio_desc *gpio_reset;
>> +       struct gpio_desc *gpio_power_down;
>> +       struct gpio_desc *gpio_mode_sel;
>> +       bool enabled;
>> +
>> +       /* firmware file info */
>> +       struct ps8640_info info;
>> +       bool in_fw_update;
>> +       /* for firmware update protect */
>> +       struct mutex fw_mutex;
>> +};
>> +
>> +static const u8 enc_ctrl_code[6] = { 0xaa, 0x55, 0x50, 0x41, 0x52, 0x44 };
>> +static const u8 hw_chip_id[4] = { 0x00, 0x0a, 0x00, 0x30 };
>> +
>> +static inline struct ps8640 *bridge_to_ps8640(struct drm_bridge *e)
>> +{
>> +       return container_of(e, struct ps8640, bridge);
>> +}
>> +
>> +static inline struct ps8640 *connector_to_ps8640(struct drm_connector *e)
>> +{
>> +       return container_of(e, struct ps8640, connector);
>> +}
>> +
>> +static int ps8640_read(struct i2c_client *client, u8 reg, u8 *data,
>> +                      u16 data_len)
>> +{
>> +       int ret;
>> +       struct i2c_msg msgs[] = {
>> +               {
>> +                .addr = client->addr,
>> +                .flags = 0,
>> +                .len = 1,
>> +                .buf = &reg,
>> +               },
>> +               {
>> +                .addr = client->addr,
>> +                .flags = I2C_M_RD,
>> +                .len = data_len,
>> +                .buf = data,
>> +               }
>> +       };
>> +
>> +       ret = i2c_transfer(client->adapter, msgs, 2);
>> +
>> +       if (ret == 2)
>> +               return 0;
>> +       if (ret < 0)
>> +               return ret;
>> +       else
>> +               return -EIO;
>> +}
>> +
>> +static int ps8640_write_bytes(struct i2c_client *client, const u8 *data,
>> +                             u16 data_len)
>> +{
>> +       int ret;
>> +       struct i2c_msg msg;
>> +
>> +       msg.addr = client->addr;
>> +       msg.flags = 0;
>> +       msg.len = data_len;
>> +       msg.buf = (u8 *)data;
>> +
>> +       ret = i2c_transfer(client->adapter, &msg, 1);
>> +       if (ret == 1)
>> +               return 0;
>> +       if (ret < 0)
>> +               return ret;
>> +       else
>> +               return -EIO;
>> +}
>> +
>> +static int ps8640_write_byte(struct i2c_client *client, u8 reg,  u8 data)
>> +{
>> +       u8 buf[] = { reg, data };
>> +
>> +       return ps8640_write_bytes(client, buf, sizeof(buf));
>> +}
>> +
>> +static void ps8640_get_mcu_fw_version(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[5];
>> +       u8 fw_ver[2];
>> +
>> +       ps8640_read(client, 0x4, fw_ver, sizeof(fw_ver));
>> +       ps_bridge->info.version = (fw_ver[0] << 8) | fw_ver[1];
>> +
>> +       DRM_INFO_ONCE("ps8640 rom fw version %d.%d\n", fw_ver[0], fw_ver[1]);
>> +}
>> +
>> +static int ps8640_bridge_unmute(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[3];
>> +       u8 vdo_ctrl_buf[3] = { PAGE3_SET_ADD, VDO_CTL_ADD, VDO_EN };
>> +
>> +       return ps8640_write_bytes(client, vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
>> +}
>> +
>> +static int ps8640_bridge_mute(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[3];
>> +       u8 vdo_ctrl_buf[3] = { PAGE3_SET_ADD, VDO_CTL_ADD, VDO_DIS };
>> +
>> +       return ps8640_write_bytes(client, vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
>> +}
>> +
>> +static void ps8640_pre_enable(struct drm_bridge *bridge)
>> +{
>> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +       struct i2c_client *page1 = ps_bridge->page[1];

It's a bit hard to follow what page[3] or page[5] means without going to the
bottom and reading the dummy devices comment. It would be nice to have some
macros here.

>> +       int err;
>> +       u8 set_vdo_done, mcs_en, vstart;
>> +       ktime_t timeout;
>> +
>> +       if (ps_bridge->in_fw_update)
>> +               return;
>> +
>> +       if (ps_bridge->enabled)
>> +               return;
>> +
>> +       err = drm_panel_prepare(ps_bridge->panel);
>> +       if (err < 0) {
>> +               DRM_ERROR("failed to prepare panel: %d\n", err);
>> +               return;
>> +       }
>> +
>> +       err = regulator_bulk_enable(ARRAY_SIZE(ps_bridge->supplies),
>> +                                   ps_bridge->supplies);
>> +       if (err < 0) {
>> +               DRM_ERROR("cannot enable regulators %d\n", err);
>> +               goto err_panel_unprepare;
>> +       }
>> +
>> +       gpiod_set_value(ps_bridge->gpio_power_down, 1);
>> +       gpiod_set_value(ps_bridge->gpio_reset, 0);
>> +       usleep_range(2000, 2500);
>> +       gpiod_set_value(ps_bridge->gpio_reset, 1);
>> +
>> +       /*
>> +        * Wait for the ps8640 embed mcu ready
>> +        * First wait 200ms and then check the mcu ready flag every 20ms
>> +        */
>> +       msleep(200);
>> +
>> +       timeout = ktime_add_ms(ktime_get(), 200);
>> +       for (;;) {
>> +               err = ps8640_read(client, PAGE2_GPIO_H, &set_vdo_done, 1);
>> +               if (err < 0) {
>> +                       DRM_ERROR("failed read PAGE2_GPIO_H: %d\n", err);
>> +                       goto err_regulators_disable;
>> +               }
>> +               if ((set_vdo_done & PS_GPIO9) == PS_GPIO9)
>> +                       break;
>> +               if (ktime_compare(ktime_get(), timeout) > 0)
>> +                       break;
>> +               msleep(20);
>> +       }
>> +
>> +       msleep(50);
>> +
>> +       ps8640_read(page1, PAGE1_VSTART, &vstart, 1);
>> +       DRM_INFO("PS8640 PAGE1.0x6B = 0x%x\n", vstart);
>> +
>> +       /**
>> +        * The Manufacturer Command Set (MCS) is a device dependent interface
>> +        * intended for factory programming of the display module default
>> +        * parameters. Once the display module is configured, the MCS shall be
>> +        * disabled by the manufacturer. Once disabled, all MCS commands are
>> +        * ignored by the display interface.
>> +        */
>> +       ps8640_read(client, PAGE2_MCS_EN, &mcs_en, 1);
>> +       ps8640_write_byte(client, PAGE2_MCS_EN, mcs_en & ~MCS_EN);
>> +
>> +       if (ps_bridge->info.version == 0)
>> +               ps8640_get_mcu_fw_version(ps_bridge);
>> +
>> +       err = ps8640_bridge_unmute(ps_bridge);
>> +       if (err)
>> +               DRM_ERROR("failed to enable unmutevideo: %d\n", err);
>> +       /* Switch access edp panel's edid through i2c */
>> +       ps8640_write_byte(client, PAGE2_I2C_BYPASS, I2C_BYPASS_EN);
>> +       ps_bridge->enabled = true;
>> +
>> +       return;
>> +
>> +err_regulators_disable:
>> +       regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
>> +                              ps_bridge->supplies);
>> +err_panel_unprepare:
>> +       drm_panel_unprepare(ps_bridge->panel);
>> +}
>> +
>> +static void ps8640_enable(struct drm_bridge *bridge)
>> +{
>> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> +       int err;
>> +
>> +       err = drm_panel_enable(ps_bridge->panel);
>> +       if (err < 0)
>> +               DRM_ERROR("failed to enable panel: %d\n", err);
>> +}
>> +
>> +static void ps8640_disable(struct drm_bridge *bridge)
>> +{
>> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> +       int err;
>> +
>> +       err = drm_panel_disable(ps_bridge->panel);
>> +       if (err < 0)
>> +               DRM_ERROR("failed to disable panel: %d\n", err);
>> +}
>> +
>> +static void ps8640_post_disable(struct drm_bridge *bridge)
>> +{
>> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> +       int err;
>> +
>> +       if (ps_bridge->in_fw_update)
>> +               return;
>> +
>> +       if (!ps_bridge->enabled)
>> +               return;
>> +
>> +       ps_bridge->enabled = false;
>> +
>> +       err = ps8640_bridge_mute(ps_bridge);
>> +       if (err < 0)
>> +               DRM_ERROR("failed to unmutevideo: %d\n", err);
>> +
>> +       gpiod_set_value(ps_bridge->gpio_reset, 0);
>> +       gpiod_set_value(ps_bridge->gpio_power_down, 0);
>> +       err = regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
>> +                                    ps_bridge->supplies);
>> +       if (err < 0)
>> +               DRM_ERROR("cannot disable regulators %d\n", err);
>> +
>> +       err = drm_panel_unprepare(ps_bridge->panel);
>> +       if (err)
>> +               DRM_ERROR("failed to unprepare panel: %d\n", err);
>> +}
>> +
>> +static int ps8640_get_modes(struct drm_connector *connector)
>> +{
>> +       struct ps8640 *ps_bridge = connector_to_ps8640(connector);
>> +       struct edid *edid;
>> +       int num_modes = 0;
>> +       bool power_off;
>> +
>> +       if (ps_bridge->edid)
>> +               return drm_add_edid_modes(connector, ps_bridge->edid);
>> +
>> +       power_off = !ps_bridge->enabled;
>> +       ps8640_pre_enable(&ps_bridge->bridge);
>> +
>> +       edid = drm_get_edid(connector, ps_bridge->ddc_i2c->adapter);

See comments related to this in ps8640_probe.

>> +       if (!edid)
>> +               goto out;
>> +
>> +       ps_bridge->edid = edid;
>> +       drm_mode_connector_update_edid_property(connector, ps_bridge->edid);
>> +       num_modes = drm_add_edid_modes(connector, ps_bridge->edid);
>> +
>> +out:
>> +       if (power_off)
>> +               ps8640_post_disable(&ps_bridge->bridge);
>> +
>> +       return num_modes;
>> +}
>> +
>> +static struct drm_encoder *ps8640_best_encoder(struct drm_connector *connector)
>> +{
>> +       struct ps8640 *ps_bridge = connector_to_ps8640(connector);
>> +
>> +       return ps_bridge->bridge.encoder;
>> +}

We can drop the above func.

>> +
>> +static const struct drm_connector_helper_funcs ps8640_connector_helper_funcs = {
>> +       .get_modes = ps8640_get_modes,
>> +       .best_encoder = ps8640_best_encoder,
>> +};
>> +
>> +static enum drm_connector_status ps8640_detect(struct drm_connector *connector,
>> +                                              bool force)
>> +{
>> +       return connector_status_connected;
>> +}
>> +
>> +static const struct drm_connector_funcs ps8640_connector_funcs = {
>> +       .dpms = drm_atomic_helper_connector_dpms,
>> +       .fill_modes = drm_helper_probe_single_connector_modes,
>> +       .detect = ps8640_detect,
>> +       .reset = drm_atomic_helper_connector_reset,
>> +       .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> +       .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +int ps8640_bridge_attach(struct drm_bridge *bridge)
>> +{
>> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> +       struct device *dev = &ps_bridge->page[0]->dev;
>> +       struct device_node *port, *in_ep;
>> +       struct device_node *dsi_node = NULL;
>> +       struct mipi_dsi_host *host = NULL;
>> +       int ret;
>> +
>> +       ret = drm_connector_init(bridge->dev, &ps_bridge->connector,
>> +                                &ps8640_connector_funcs,
>> +                                DRM_MODE_CONNECTOR_eDP);
>> +
>> +       if (ret) {
>> +               DRM_ERROR("Failed to initialize connector with drm: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       drm_connector_helper_add(&ps_bridge->connector,
>> +                                &ps8640_connector_helper_funcs);
>> +
>> +       ps_bridge->connector.dpms = DRM_MODE_DPMS_ON;
>> +       drm_mode_connector_attach_encoder(&ps_bridge->connector,
>> +                                         bridge->encoder);
>> +
>> +       if (ps_bridge->panel)
>> +               drm_panel_attach(ps_bridge->panel, &ps_bridge->connector);
>> +
>> +       /* port@0 is ps8640 dsi input port */
>> +       port = of_graph_get_port_by_id(dev->of_node, 0);
>> +       if (port) {
>> +               in_ep = of_get_child_by_name(port, "endpoint");
>> +               of_node_put(port);

The above 2 funcs can be done by a single func: of_graph_get_endpoint_by_regs().

>> +               if (in_ep) {
>> +                       dsi_node = of_graph_get_remote_port_parent(in_ep);
>> +                       of_node_put(in_ep);
>> +               }
>> +       }
>> +       if (dsi_node) {
>> +               host = of_find_mipi_dsi_host_by_node(dsi_node);
>> +               of_node_put(dsi_node);
>> +               if (!host) {
>> +                       ret = -ENODEV;
>> +                       goto err;
>> +               }
>> +       }
>> +

We haven't created a DSI device for this yet. Don't we need to call
mipi_dsi_device_register_full() here?

>> +       ps_bridge->dsi.host = host;

The code above proceeds even if we don't find a dsi host. In that
case, the host would be a NULL pointer. We shouldn't call
mipi_dsi_attach() with a NULL host. We should have returned earlier with
an error.

>> +       ps_bridge->dsi.mode_flags = MIPI_DSI_MODE_VIDEO |
>> +                                    MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
>> +       ps_bridge->dsi.format = MIPI_DSI_FMT_RGB888;
>> +       ps_bridge->dsi.lanes = 4;
>> +       ret = mipi_dsi_attach(&ps_bridge->dsi);
>> +       if (ret)
>> +               goto err;
>> +
>> +       return 0;
>> +err:
>> +       if (ps_bridge->panel)
>> +               drm_panel_detach(ps_bridge->panel);
>> +       drm_connector_cleanup(&ps_bridge->connector);
>> +       return ret;
>> +}
>> +
>> +static const struct drm_bridge_funcs ps8640_bridge_funcs = {
>> +       .attach = ps8640_bridge_attach,
>> +       .disable = ps8640_disable,
>> +       .post_disable = ps8640_post_disable,
>> +       .pre_enable = ps8640_pre_enable,
>> +       .enable = ps8640_enable,
>> +};
>> +
>> +/* Firmware Version is returned as Major.Minor */
>> +static ssize_t ps8640_fw_version_show(struct device *dev,
>> +                                     struct device_attribute *attr, char *buf)
>> +{
>> +       struct ps8640 *ps_bridge = dev_get_drvdata(dev);
>> +       struct ps8640_info *info = &ps_bridge->info;
>> +
>> +       return scnprintf(buf, PAGE_SIZE, "%u.%u\n", info->version >> 8,
>> +                        info->version & 0xff);
>> +}
>> +
>> +/* Hardware Version is returned as FamilyID.VariantID */
>> +static ssize_t ps8640_hw_version_show(struct device *dev,
>> +                                     struct device_attribute *attr, char *buf)
>> +{
>> +       struct ps8640 *ps_bridge = dev_get_drvdata(dev);
>> +       struct ps8640_info *info = &ps_bridge->info;
>> +
>> +       return scnprintf(buf, PAGE_SIZE, "ps%u.%u\n", info->family_id,
>> +                        info->variant_id);
>> +}
>> +
>> +static int ps8640_spi_send_cmd(struct ps8640 *ps_bridge, u8 *cmd, u8 cmd_len)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +       u8 i, buf[3] = { PAGE2_SWSPI_LEN, cmd_len - 1, TRIGGER_NO_READBACK };
>> +       int ret;
>> +
>> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
>> +       if (ret)
>> +               goto err;
>> +
>> +       /* write command in write port */
>> +       for (i = 0; i < cmd_len; i++) {
>> +               ret = ps8640_write_byte(client, PAGE2_SWSPI_WDATA, cmd[i]);
>> +               if (ret)
>> +                       goto err_irom_disable;
>> +       }
>> +
>> +       ret = ps8640_write_bytes(client, buf, sizeof(buf));
>> +       if (ret)
>> +               goto err_irom_disable;
>> +
>> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> +       if (ret)
>> +               goto err;
>> +
>> +       return 0;
>> +err_irom_disable:
>> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> +err:
>> +       dev_err(&client->dev, "send command err: %d\n", ret);
>> +       return ret;
>> +}
>> +
>> +static int ps8640_wait_spi_ready(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +       u8 spi_rdy_st;
>> +       ktime_t timeout;
>> +
>> +       timeout = ktime_add_ms(ktime_get(), 200);
>> +       for (;;) {
>> +               ps8640_read(client, PAGE2_SPI_STATUS, &spi_rdy_st, 1);
>> +               if ((spi_rdy_st & SPI_READY) != SPI_READY)
>> +                       break;
>> +
>> +               if (ktime_compare(ktime_get(), timeout) > 0) {
>> +                       dev_err(&client->dev, "wait spi ready timeout\n");
>> +                       return -EBUSY;
>> +               }
>> +
>> +               msleep(20);
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int ps8640_wait_spi_nobusy(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +       u8 spi_status, buf[3] = { PAGE2_SWSPI_LEN, 0, TRIGGER_READBACK };
>> +       int ret;
>> +       ktime_t timeout;
>> +
>> +       timeout = ktime_add_ms(ktime_get(), 500);
>> +       for (;;) {
>> +               /* 0x05 RDSR; Read-Status-Register */
>> +               ret = ps8640_write_byte(client, PAGE2_SWSPI_WDATA,
>> +                                       READ_STATUS_REG_CMD);
>> +               if (ret)
>> +                       goto err_send_cmd_exit;
>> +
>> +               ret = ps8640_write_bytes(client, buf, 3);
>> +               if (ret)
>> +                       goto err_send_cmd_exit;
>> +
>> +               /* delay for cmd send */
>> +               usleep_range(300, 500);
>> +               /* wait for SPI ROM until not busy */
>> +               ret = ps8640_read(client, PAGE2_SWSPI_RDATA, &spi_status, 1);
>> +               if (ret)
>> +                       goto err_send_cmd_exit;
>> +
>> +               if (!(spi_status & BUSY))
>> +                       break;
>> +
>> +               if (ktime_compare(ktime_get(), timeout) > 0) {
>> +                       dev_err(&client->dev, "wait spi no busy timeout: %d\n",
>> +                               ret);
>> +                       return -EBUSY;
>> +               }
>> +       }
>> +
>> +       return 0;
>> +
>> +err_send_cmd_exit:
>> +       dev_err(&client->dev, "send command err: %d\n", ret);
>> +       return ret;
>> +}
>> +
>> +static int ps8640_wait_rom_idle(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[0];
>> +       int ret;
>> +
>> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
>> +       if (ret)
>> +               goto exit;
>> +
>> +       ret = ps8640_wait_spi_ready(ps_bridge);
>> +       if (ret)
>> +               goto err_spi;
>> +
>> +       ret = ps8640_wait_spi_nobusy(ps_bridge);
>> +       if (ret)
>> +               goto err_spi;
>> +
>> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> +       if (ret)
>> +               goto exit;
>> +
>> +       return 0;
>> +
>> +err_spi:
>> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> +exit:
>> +       dev_err(&client->dev, "wait ps8640 rom idle fail: %d\n", ret);
>> +
>> +       return ret;
>> +}
>> +
>> +static int ps8640_spi_dl_mode(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +       int ret;
>> +
>> +       /* switch ps8640 mode to spi dl mode */
>> +       if (ps_bridge->gpio_mode_sel)
>> +               gpiod_set_value(ps_bridge->gpio_mode_sel, 0);
>> +
>> +       /* reset spi interface */
>> +       ret = ps8640_write_byte(client, PAGE2_SW_RESET,
>> +                               SPI_SW_RESET | MPU_SW_RESET);
>> +       if (ret)
>> +               goto exit;
>> +
>> +       ret = ps8640_write_byte(client, PAGE2_SW_RESET, MPU_SW_RESET);
>> +       if (ret)
>> +               goto exit;
>> +
>> +       return 0;
>> +
>> +exit:
>> +       dev_err(&client->dev, "fail reset spi interface: %d\n", ret);
>> +
>> +       return ret;
>> +}
>> +
>> +static int ps8640_rom_prepare(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +       struct device *dev = &client->dev;
>> +       u8 i, cmd[2];
>> +       int ret;
>> +
>> +       cmd[0] = WRITE_ENABLE_CMD;
>> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
>> +       if (ret) {
>> +               dev_err(dev, "failed enable-write-status-register: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       cmd[0] = WRITE_STATUS_REG_CMD;
>> +       cmd[1] = CLEAR_ALL_PROTECT;
>> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 2);
>> +       if (ret) {
>> +               dev_err(dev, "fail disable all protection: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       /* wait for SPI module ready */
>> +       ret = ps8640_wait_rom_idle(ps_bridge);
>> +       if (ret) {
>> +               dev_err(dev, "fail wait rom idle: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
>> +       for (i = 0; i < ARRAY_SIZE(enc_ctrl_code); i++)
>> +               ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, enc_ctrl_code[i]);
>> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> +
>> +       /* Enable-Write-Status-Register */
>> +       cmd[0] = WRITE_ENABLE_CMD;
>> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
>> +       if (ret) {
>> +               dev_err(dev, "fail enable-write-status-register: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       /* chip erase command */
>> +       cmd[0] = CHIP_ERASE_CMD;
>> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
>> +       if (ret) {
>> +               dev_err(dev, "fail disable all protection: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       ret = ps8640_wait_rom_idle(ps_bridge);
>> +       if (ret) {
>> +               dev_err(dev, "fail wait rom idle: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int ps8640_check_chip_id(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[4];
>> +       u8 buf[4];
>> +
>> +       ps8640_read(client, PAGE4_REV_L, buf, 4);
>> +       return memcmp(buf, hw_chip_id, sizeof(buf));
>> +}
>> +
>> +static int ps8640_validate_firmware(struct ps8640 *ps_bridge,
>> +                                   const struct firmware *fw)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[0];
>> +       u16 fw_chip_id;
>> +
>> +       /*
>> +        * Get the chip_id from the firmware. Make sure that it is the
>> +        * right controller to do the firmware and config update.
>> +        */
>> +       fw_chip_id = get_unaligned_le16(fw->data + FW_CHIP_ID_OFFSET);
>> +
>> +       if (fw_chip_id != 0x8640 && ps8640_check_chip_id(ps_bridge) == 0) {
>> +               dev_err(&client->dev,
>> +                       "chip id mismatch: fw 0x%x vs. chip 0x8640\n",
>> +                       fw_chip_id);
>> +               return -EINVAL;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int ps8640_write_rom(struct ps8640 *ps_bridge, const struct firmware *fw)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[0];
>> +       struct device *dev = &client->dev;
>> +       struct i2c_client *client2 = ps_bridge->page[2];
>> +       struct i2c_client *client7 = ps_bridge->page[7];
>> +       size_t pos, cpy_len;
>> +       u8 buf[257];
>> +       int ret;
>> +
>> +       ps8640_write_byte(client2, PAGE2_SPI_CFG3, I2C_TO_SPI_RESET);
>> +       msleep(100);
>> +       ps8640_write_byte(client2, PAGE2_SPI_CFG3, 0x00);
>> +
>> +       for (pos = 0; pos < fw->size; pos += cpy_len) {
>> +               buf[0] = PAGE2_ROMADD_BYTE1;
>> +               buf[1] = pos >> 8;
>> +               buf[2] = pos >> 16;
>> +               ret = ps8640_write_bytes(client2, buf, 3);
>> +               if (ret)
>> +                       goto error;
>> +               cpy_len = fw->size >= 256 + pos ? 256 : fw->size - pos;
>> +               buf[0] = 0;
>> +               memcpy(buf + 1, fw->data + pos, cpy_len);
>> +               ret = ps8640_write_bytes(client7, buf, cpy_len + 1);
>> +               if (ret)
>> +                       goto error;
>> +
>> +               dev_dbg(dev, "fw update completed %zu / %zu bytes\n", pos,
>> +                       fw->size);
>> +       }
>> +       return 0;
>> +
>> +error:
>> +       dev_err(dev, "failed write external flash, %d\n", ret);
>> +       return ret;
>> +}
>> +
>> +static int ps8640_spi_normal_mode(struct ps8640 *ps_bridge)
>> +{
>> +       u8 cmd[2];
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +
>> +       /* Enable-Write-Status-Register */
>> +       cmd[0] = WRITE_ENABLE_CMD;
>> +       ps8640_spi_send_cmd(ps_bridge, cmd, 1);
>> +
>> +       /* protect BPL/BP0/BP1 */
>> +       cmd[0] = WRITE_STATUS_REG_CMD;
>> +       cmd[1] = BLK_PROTECT_BITS | STATUS_REG_PROTECT;
>> +       ps8640_spi_send_cmd(ps_bridge, cmd, 2);
>> +
>> +       /* wait for SPI rom ready */
>> +       ps8640_wait_rom_idle(ps_bridge);
>> +
>> +       /* disable PS8640 mapping function */
>> +       ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, 0x00);
>> +
>> +       if (ps_bridge->gpio_mode_sel)
>> +               gpiod_set_value(ps_bridge->gpio_mode_sel, 1);
>> +       return 0;
>> +}
>> +
>> +static int ps8640_enter_bl(struct ps8640 *ps_bridge)
>> +{
>> +       ps_bridge->in_fw_update = true;
>> +       return ps8640_spi_dl_mode(ps_bridge);
>> +}
>> +
>> +static void ps8640_exit_bl(struct ps8640 *ps_bridge, const struct firmware *fw)
>> +{
>> +       ps8640_spi_normal_mode(ps_bridge);
>> +       ps_bridge->in_fw_update = false;
>> +}
>> +
>> +static int ps8640_load_fw(struct ps8640 *ps_bridge, const struct firmware *fw)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[0];
>> +       struct device *dev = &client->dev;
>> +       int ret;
>> +       bool ps8640_status_backup = ps_bridge->enabled;
>> +
>> +       ret = ps8640_validate_firmware(ps_bridge, fw);
>> +       if (ret)
>> +               return ret;
>> +
>> +       mutex_lock(&ps_bridge->fw_mutex);
>> +       if (!ps_bridge->in_fw_update) {
>> +               if (!ps8640_status_backup)
>> +                       ps8640_pre_enable(&ps_bridge->bridge);
>> +
>> +               ret = ps8640_enter_bl(ps_bridge);
>> +               if (ret)
>> +                       goto exit;
>> +       }
>> +
>> +       ret = ps8640_rom_prepare(ps_bridge);
>> +       if (ret)
>> +               goto exit;
>> +
>> +       ret = ps8640_write_rom(ps_bridge, fw);
>> +
>> +exit:
>> +       if (ret)
>> +               dev_err(dev, "Failed to load firmware, %d\n", ret);
>> +
>> +       ps8640_exit_bl(ps_bridge, fw);
>> +       if (!ps8640_status_backup)
>> +               ps8640_post_disable(&ps_bridge->bridge);
>> +       mutex_unlock(&ps_bridge->fw_mutex);
>> +       return ret;
>> +}
>> +
>> +static ssize_t ps8640_update_fw_store(struct device *dev,
>> +                                     struct device_attribute *attr,
>> +                                     const char *buf, size_t count)
>> +{
>> +       struct i2c_client *client = to_i2c_client(dev);
>> +       struct ps8640 *ps_bridge = i2c_get_clientdata(client);
>> +       const struct firmware *fw;
>> +       int error;
>> +
>> +       error = request_firmware(&fw, PS_FW_NAME, dev);
>> +       if (error) {
>> +               dev_err(dev, "Unable to open firmware %s: %d\n",
>> +                       PS_FW_NAME, error);
>> +               return error;
>> +       }
>> +
>> +       error = ps8640_load_fw(ps_bridge, fw);
>> +       if (error)
>> +               dev_err(dev, "The firmware update failed(%d)\n", error);
>> +       else
>> +               dev_info(dev, "The firmware update succeeded\n");
>> +
>> +       release_firmware(fw);
>> +       return error ? error : count;
>> +}
>> +
>> +static DEVICE_ATTR(fw_version, S_IRUGO, ps8640_fw_version_show, NULL);
>> +static DEVICE_ATTR(hw_version, S_IRUGO, ps8640_hw_version_show, NULL);
>> +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, ps8640_update_fw_store);
>> +
>> +static struct attribute *ps8640_attrs[] = {
>> +       &dev_attr_fw_version.attr,
>> +       &dev_attr_hw_version.attr,
>> +       &dev_attr_update_fw.attr,
>> +       NULL
>> +};
>> +
>> +static const struct attribute_group ps8640_attr_group = {
>> +       .attrs = ps8640_attrs,
>> +};
>> +
>> +static void ps8640_remove_sysfs_group(void *data)
>> +{
>> +       struct ps8640 *ps_bridge = data;
>> +
>> +       sysfs_remove_group(&ps_bridge->page[0]->dev.kobj, &ps8640_attr_group);
>> +}
>> +
>> +static int ps8640_probe(struct i2c_client *client,
>> +                       const struct i2c_device_id *id)
>> +{
>> +       struct device *dev = &client->dev;
>> +       struct ps8640 *ps_bridge;
>> +       struct device_node *np = dev->of_node;
>> +       struct device_node *port, *out_ep;
>> +       struct device_node *panel_node = NULL;
>> +       int ret;
>> +       u32 i;
>> +
>> +       ps_bridge = devm_kzalloc(dev, sizeof(*ps_bridge), GFP_KERNEL);
>> +       if (!ps_bridge)
>> +               return -ENOMEM;
>> +
>> +       /* port@1 is ps8640 output port */
>> +       port = of_graph_get_port_by_id(np, 1);
>> +       if (port) {
>> +               out_ep = of_get_child_by_name(port, "endpoint");
>> +               of_node_put(port);
>> +               if (out_ep) {
>> +                       panel_node = of_graph_get_remote_port_parent(out_ep);
>> +                       of_node_put(out_ep);
>> +               }
>> +       }
>> +       if (panel_node) {
>> +               ps_bridge->panel = of_drm_find_panel(panel_node);
>> +               of_node_put(panel_node);
>> +               if (!ps_bridge->panel)
>> +                       return -EPROBE_DEFER;
>> +       }
>> +
>> +       mutex_init(&ps_bridge->fw_mutex);
>> +       ps_bridge->supplies[0].supply = "vdd33";
>> +       ps_bridge->supplies[1].supply = "vdd12";
>> +       ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ps_bridge->supplies),
>> +                                     ps_bridge->supplies);
>> +       if (ret) {
>> +               dev_info(dev, "failed to get regulators: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       ps_bridge->gpio_mode_sel = devm_gpiod_get_optional(&client->dev,
>> +                                                            "mode-sel",
>> +                                                            GPIOD_OUT_HIGH);
>> +       if (IS_ERR(ps_bridge->gpio_mode_sel)) {
>> +               ret = PTR_ERR(ps_bridge->gpio_mode_sel);
>> +               dev_err(dev, "cannot get mode-sel %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       ps_bridge->gpio_power_down = devm_gpiod_get(&client->dev, "sleep",
>> +                                              GPIOD_OUT_LOW);
>> +       if (IS_ERR(ps_bridge->gpio_power_down)) {
>> +               ret = PTR_ERR(ps_bridge->gpio_power_down);
>> +               dev_err(dev, "cannot get sleep: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       /*
>> +        * Request the reset pin low to avoid the bridge being
>> +        * initialized prematurely
>> +        */
>> +       ps_bridge->gpio_reset = devm_gpiod_get(&client->dev, "reset",
>> +                                              GPIOD_OUT_LOW);
>> +       if (IS_ERR(ps_bridge->gpio_reset)) {
>> +               ret = PTR_ERR(ps_bridge->gpio_reset);
>> +               dev_err(dev, "cannot get reset: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       ps_bridge->bridge.funcs = &ps8640_bridge_funcs;
>> +       ps_bridge->bridge.of_node = dev->of_node;
>> +
>> +       ps_bridge->page[0] = client;
>> +       ps_bridge->ddc_i2c = i2c_new_dummy(client->adapter, EDID_I2C_ADDR);

I don't see why we need to create this dummy client. The drm edid helper
drm_get_edid() just needs the i2c adapter to which the client is connected.
It will internally initiate a read form the address EDID_I2C_ADDR.

I guess "drm_get_edid(connector, ps_bridge->page[0]->adapter)" should work.

>> +       if (!ps_bridge->ddc_i2c) {
>> +               dev_err(dev, "failed ddc_i2c dummy device, address%02x\n",
>> +                       EDID_I2C_ADDR);
>> +               return -EBUSY;
>> +       }
>> +       /*
>> +        * ps8640 uses multiple addresses, use dummy devices for them
>> +        * page[0]: for DP control
>> +        * page[1]: for VIDEO Bridge
>> +        * page[2]: for control top
>> +        * page[3]: for DSI Link Control1
>> +        * page[4]: for MIPI Phy
>> +        * page[5]: for VPLL
>> +        * page[6]: for DSI Link Control2

Does this chip support 2 DSI inputs, and we're just exposing one for now?
If so, we should probably revisit the DT bindings, so that port@2 doesn't
need to represent the 2nd DSI link.

Thanks,
Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v16 0/5] Mediatek MT8173 CMDQ support
From: Horng-Shyang Liao @ 2016-11-11  9:34 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Rob Herring, Matthias Brugger, Daniel Kurtz, Sascha Hauer,
	Devicetree List, Linux Kernel Mailing List,
	linux-arm-kernel@lists.infradead.org, linux-mediatek,
	srv_heupstream, Sascha Hauer, Philipp Zabel, Nicolas Boichat,
	CK HU, cawa cheng, Bibby Hsieh, YT Shen, Daoyuan Huang, Damon Chu,
	Jo
In-Reply-To: <CABb+yY3mi3M0xqNao=DRYN6Np0LOBfS669iUa_hWo2w1MmM1sw@mail.gmail.com>

On Fri, 2016-11-11 at 11:15 +0530, Jassi Brar wrote:
> On Thu, Nov 10, 2016 at 4:45 PM, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:
> > On Tue, 2016-11-01 at 19:28 +0800, HS Liao wrote:
> >> Hi,
> >>
> >> This is Mediatek MT8173 Command Queue(CMDQ) driver. The CMDQ is used
> >> to help write registers with critical time limitation, such as
> >> updating display configuration during the vblank. It controls Global
> >> Command Engine (GCE) hardware to achieve this requirement.
> >>
> >> These patches have a build dependency on top of v4.9-rc1.
> >>
> >> Changes since v15:
> >>  - separate "suspend and resume" patch from "save energy" patch
> >>  - don't stop running tasks in cmdq_suspend()
> >>    (i.e. leave no running tasks guarantee to clients)
> >>
> >> Best regards,
> >> HS Liao
> >>
> >> HS Liao (5):
> >>   dt-bindings: soc: Add documentation for the MediaTek GCE unit
> >>   CMDQ: Mediatek CMDQ driver
> >>   arm64: dts: mt8173: Add GCE node
> >>   CMDQ: suspend and resume
> >>   CMDQ: save energy
> >>
> >>  .../devicetree/bindings/mailbox/mtk-gce.txt        |  43 ++
> >>  arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  10 +
> >>  drivers/mailbox/Kconfig                            |  10 +
> >>  drivers/mailbox/Makefile                           |   2 +
> >>  drivers/mailbox/mtk-cmdq-mailbox.c                 | 632 +++++++++++++++++++++
> >>  drivers/soc/mediatek/Kconfig                       |  11 +
> >>  drivers/soc/mediatek/Makefile                      |   1 +
> >>  drivers/soc/mediatek/mtk-cmdq-helper.c             | 310 ++++++++++
> >>  include/linux/mailbox/mtk-cmdq-mailbox.h           |  67 +++
> >>  include/linux/soc/mediatek/mtk-cmdq.h              | 182 ++++++
> >>  10 files changed, 1268 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/mailbox/mtk-gce.txt
> >>  create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c
> >>  create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c
> >>  create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h
> >>  create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h
> >>
> >
> >
> > Hi Jassi, Matthias,
> >
> > Sorry to disturb you.
> >
> No, you don't disturb, but the controller driver and protocol driver,
> introduced in the same patch, does :)   So does the suspend/resume
> support (patch 4&5) added  separately as a patch on top. Please
> reorganise the patchset.
> 
> Thanks.

Hi Jassi,

Do you mean
1. split controller driver and protocol driver as two patches,
2. merge patch 4&5 into one patch, and
3. reorganize the patchset as "(1) binding doc (2) controller driver
   (3) protocol driver (4) devicetree (5) energy patch" ?

Thanks,
HS

^ permalink raw reply

* Re: [PATCH] [media] mtk-mdp: allocate video_device dynamically
From: Hans Verkuil @ 2016-11-11 10:51 UTC (permalink / raw)
  To: Minghsiu Tsai, Hans Verkuil, daniel.thompson, Rob Herring,
	Mauro Carvalho Chehab, Matthias Brugger, Daniel Kurtz,
	Pawel Osciak
  Cc: srv_heupstream, Eddie Huang, Yingjoe Chen, devicetree,
	linux-kernel, linux-arm-kernel, linux-media, linux-mediatek
In-Reply-To: <1478522529-57129-2-git-send-email-minghsiu.tsai@mediatek.com>

Almost correct:

On 11/07/2016 01:42 PM, Minghsiu Tsai wrote:
> It can fix known problems with embedded video_device structs.
> 
> Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> ---
>  drivers/media/platform/mtk-mdp/mtk_mdp_core.h |  2 +-
>  drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c  | 33 ++++++++++++++++-----------
>  2 files changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> index 848569d..ad1cff3 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> @@ -167,7 +167,7 @@ struct mtk_mdp_dev {
>  	struct mtk_mdp_comp		*comp[MTK_MDP_COMP_ID_MAX];
>  	struct v4l2_m2m_dev		*m2m_dev;
>  	struct list_head		ctx_list;
> -	struct video_device		vdev;
> +	struct video_device		*vdev;
>  	struct v4l2_device		v4l2_dev;
>  	struct workqueue_struct		*job_wq;
>  	struct platform_device		*vpu_dev;
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> index 9a747e7..b8dee1c 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> @@ -1236,16 +1236,22 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
>  	int ret;
>  
>  	mdp->variant = &mtk_mdp_default_variant;
> -	mdp->vdev.device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
> -	mdp->vdev.fops = &mtk_mdp_m2m_fops;
> -	mdp->vdev.ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
> -	mdp->vdev.release = video_device_release_empty;
> -	mdp->vdev.lock = &mdp->lock;
> -	mdp->vdev.vfl_dir = VFL_DIR_M2M;
> -	mdp->vdev.v4l2_dev = &mdp->v4l2_dev;
> -	snprintf(mdp->vdev.name, sizeof(mdp->vdev.name), "%s:m2m",
> +	mdp->vdev = video_device_alloc();
> +	if (!mdp->vdev) {
> +		dev_err(dev, "failed to allocate video device\n");
> +		ret = -ENOMEM;
> +		goto err_video_alloc;
> +	}
> +	mdp->vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
> +	mdp->vdev->fops = &mtk_mdp_m2m_fops;
> +	mdp->vdev->ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
> +	mdp->vdev->release = video_device_release;
> +	mdp->vdev->lock = &mdp->lock;
> +	mdp->vdev->vfl_dir = VFL_DIR_M2M;
> +	mdp->vdev->v4l2_dev = &mdp->v4l2_dev;
> +	snprintf(mdp->vdev->name, sizeof(mdp->vdev->name), "%s:m2m",
>  		 MTK_MDP_MODULE_NAME);
> -	video_set_drvdata(&mdp->vdev, mdp);
> +	video_set_drvdata(mdp->vdev, mdp);
>  
>  	mdp->m2m_dev = v4l2_m2m_init(&mtk_mdp_m2m_ops);
>  	if (IS_ERR(mdp->m2m_dev)) {
> @@ -1254,26 +1260,27 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
>  		goto err_m2m_init;
>  	}
>  
> -	ret = video_register_device(&mdp->vdev, VFL_TYPE_GRABBER, 2);
> +	ret = video_register_device(mdp->vdev, VFL_TYPE_GRABBER, 2);
>  	if (ret) {
>  		dev_err(dev, "failed to register video device\n");
>  		goto err_vdev_register;
>  	}
>  
>  	v4l2_info(&mdp->v4l2_dev, "driver registered as /dev/video%d",
> -		  mdp->vdev.num);
> +		  mdp->vdev->num);
>  	return 0;
>  
>  err_vdev_register:
>  	v4l2_m2m_release(mdp->m2m_dev);
>  err_m2m_init:
> -	video_device_release(&mdp->vdev);
> +	video_unregister_device(mdp->vdev);

This should remain video_device_release: the video_register_device call failed, so
the device hasn't been registered. In that case just release the device (i.e.
free the allocated memory).

> +err_video_alloc:
>  
>  	return ret;
>  }
>  
>  void mtk_mdp_unregister_m2m_device(struct mtk_mdp_dev *mdp)
>  {
> -	video_device_release(&mdp->vdev);
> +	video_unregister_device(mdp->vdev);
>  	v4l2_m2m_release(mdp->m2m_dev);
>  }
> 

Regards,

	Hans

^ permalink raw reply

* [PATCH v9 00/10] MT2701 DRM support
From: YT Shen @ 2016-11-11 11:55 UTC (permalink / raw)
  To: dri-devel, Philipp Zabel
  Cc: David Airlie, Matthias Brugger, YT Shen, Daniel Kurtz, Mao Huang,
	CK Hu, Bibby Hsieh, Daniel Vetter, Thierry Reding, Jie Qiu,
	Maxime Ripard, Chris Wilson, shaoming chen, Jitao Shi,
	Boris Brezillon, Dan Carpenter, linux-arm-kernel, linux-mediatek,
	linux-kernel, srv_heupstream

This is MT2701 DRM support PATCH v9, based on 4.9-rc1.
We add DSI interrupt control, transfer function for MIPI DSI panel support.
Most codes are the same, except some register changed.

For example:
 - DISP_OVL address offset changed, color format definition changed.
 - DISP_RDMA fifo size changed.
 - DISP_COLOR offset changed.
 - MIPI_TX setting changed.

We add a new component DDP_COMPONENT_BLS, and the connections are updated.
OVL -> RDMA -> COLOR -> BLS -> DSI
RDMA -> DPI
And we have shadow register support in MT2701.

We remove dts patch from the patch series, which depends on MT2701 CCF and scpsys.

Changes since v8:
- enable 3 DSI interrupts only
- move mtk_dsi_wait_for_irq_done() to the patch of irq control
- use the name BLS in DRM driver part
- move BLS declaration to a separate patch
- update mtk_dsi_switch_to_cmd_mode()
- update mtk_output_dsi_enable() and mtk_output_dsi_disable()

Changes since v7:
- Remove redundant codes
- Move the definition of DDP_COMPONENT_BLS to patch of "drm/mediatek: update display module connections"
- Move _dsi_irq_wait_queue into platform driver data
- Move mtk_dsi_irq_data_clear() to patch of "drm/mediatek: add dsi interrupt control"
- Add more descriptions in the commit messages

Changes since v6:
- Change data type of irq_data to u32
- Rewrite mtk_dsi_host_transfer() for simplify
- Move some MIPI_TX config to patch of "drm/mediatek: add *driver_data for different hardware settings".
- Remove device tree from this patch series

Changes since v5:
- Remove DPI device tree and compatible string
- Use one wait queue to handle interrupt status
- Update the interrupt check flow and DSI_INT_ALL_BITS
- Use same function for host read/write command
- various fixes

Changes since v4:
- Add messages when timeout in mtk_disp_mutex_acquire()
- Add descriptions for DISP_REG_MUTEX registers
- Move connection settings for display modules to a separate patch
- Remove 'mt2701-disp-wdma' because it is unused
- Move cleaning up and renaming to a separate patch
- Use wait_event_interruptible_timeout() to replace polling
- Remove irq_num from mtk_dsi structure
- Remove redundant and debug codes

Changes since v3:
- Add DSI support for MIPI DSI panels
- Update BLS binding to PWM nodes
- Remove ufoe device nodes
- Remove redundant parentheses
- Remove global variable initialization

Changes since v2:
- Rename mtk_ddp_mux_sel to mtk_ddp_sout_sel
- Update mt2701_mtk_ddp_ext components
- Changed to prefix naming
- Reorder the patch series
- Use of_device_get_match_data() to get driver private data
- Use iopoll macros to implement mtk_disp_mutex_acquire()
- Removed empty device tree nodes

Changes since v1:
- Removed BLS bindings and codes, which belong to pwm driver
- Moved mtk_disp_mutex_acquire() just before mtk_crtc_ddp_config()
- Split patch into smaller parts
- Added const keyword to constant structure
- Removed codes for special memory align

Thanks,
yt.shen

YT Shen (8):
  drm/mediatek: rename macros, add chip prefix
  drm/mediatek: add *driver_data for different hardware settings
  drm/mediatek: add shadow register support
  drm/mediatek: add BLS component
  drm/mediatek: update display module connections
  drm/mediatek: cleaning up and refine
  drm/mediatek: update DSI sub driver flow for sending commands to panel
  drm/mediatek: add support for Mediatek SoC MT2701

shaoming chen (2):
  drm/mediatek: add dsi interrupt control
  drm/mediatek: add dsi transfer function

 drivers/gpu/drm/mediatek/mtk_disp_ovl.c     |  33 ++-
 drivers/gpu/drm/mediatek/mtk_disp_rdma.c    |  17 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |  76 +++--
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c      | 138 ++++++---
 drivers/gpu/drm/mediatek/mtk_drm_ddp.h      |   2 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  38 ++-
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  15 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c      |  54 +++-
 drivers/gpu/drm/mediatek/mtk_drm_drv.h      |   9 +
 drivers/gpu/drm/mediatek/mtk_dsi.c          | 429 ++++++++++++++++++++++++----
 drivers/gpu/drm/mediatek/mtk_mipi_tx.c      |  70 +++--
 11 files changed, 715 insertions(+), 166 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH v9 01/10] drm/mediatek: rename macros, add chip prefix
From: YT Shen @ 2016-11-11 11:55 UTC (permalink / raw)
  To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Philipp Zabel
  Cc: David Airlie, Daniel Vetter, Chris Wilson, Thierry Reding,
	Boris Brezillon, Jie Qiu, Mao Huang, Bibby Hsieh, YT Shen,
	yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w, Dan Carpenter, Jitao Shi,
	CK Hu, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Matthias Brugger, shaoming chen,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
	emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sascha Hauer, Maxime Ripard
In-Reply-To: <1478865346-19043-1-git-send-email-yt.shen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

Add MT8173 prefix for hardware related macros.

Signed-off-by: YT Shen <yt.shen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 60 +++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index 17ba935..2fc4321 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -36,21 +36,21 @@
 #define DISP_REG_MUTEX_MOD(n)	(0x2c + 0x20 * (n))
 #define DISP_REG_MUTEX_SOF(n)	(0x30 + 0x20 * (n))
 
-#define MUTEX_MOD_DISP_OVL0		BIT(11)
-#define MUTEX_MOD_DISP_OVL1		BIT(12)
-#define MUTEX_MOD_DISP_RDMA0		BIT(13)
-#define MUTEX_MOD_DISP_RDMA1		BIT(14)
-#define MUTEX_MOD_DISP_RDMA2		BIT(15)
-#define MUTEX_MOD_DISP_WDMA0		BIT(16)
-#define MUTEX_MOD_DISP_WDMA1		BIT(17)
-#define MUTEX_MOD_DISP_COLOR0		BIT(18)
-#define MUTEX_MOD_DISP_COLOR1		BIT(19)
-#define MUTEX_MOD_DISP_AAL		BIT(20)
-#define MUTEX_MOD_DISP_GAMMA		BIT(21)
-#define MUTEX_MOD_DISP_UFOE		BIT(22)
-#define MUTEX_MOD_DISP_PWM0		BIT(23)
-#define MUTEX_MOD_DISP_PWM1		BIT(24)
-#define MUTEX_MOD_DISP_OD		BIT(25)
+#define MT8173_MUTEX_MOD_DISP_OVL0		BIT(11)
+#define MT8173_MUTEX_MOD_DISP_OVL1		BIT(12)
+#define MT8173_MUTEX_MOD_DISP_RDMA0		BIT(13)
+#define MT8173_MUTEX_MOD_DISP_RDMA1		BIT(14)
+#define MT8173_MUTEX_MOD_DISP_RDMA2		BIT(15)
+#define MT8173_MUTEX_MOD_DISP_WDMA0		BIT(16)
+#define MT8173_MUTEX_MOD_DISP_WDMA1		BIT(17)
+#define MT8173_MUTEX_MOD_DISP_COLOR0		BIT(18)
+#define MT8173_MUTEX_MOD_DISP_COLOR1		BIT(19)
+#define MT8173_MUTEX_MOD_DISP_AAL		BIT(20)
+#define MT8173_MUTEX_MOD_DISP_GAMMA		BIT(21)
+#define MT8173_MUTEX_MOD_DISP_UFOE		BIT(22)
+#define MT8173_MUTEX_MOD_DISP_PWM0		BIT(23)
+#define MT8173_MUTEX_MOD_DISP_PWM1		BIT(24)
+#define MT8173_MUTEX_MOD_DISP_OD		BIT(25)
 
 #define MUTEX_SOF_SINGLE_MODE		0
 #define MUTEX_SOF_DSI0			1
@@ -80,21 +80,21 @@ struct mtk_ddp {
 };
 
 static const unsigned int mutex_mod[DDP_COMPONENT_ID_MAX] = {
-	[DDP_COMPONENT_AAL] = MUTEX_MOD_DISP_AAL,
-	[DDP_COMPONENT_COLOR0] = MUTEX_MOD_DISP_COLOR0,
-	[DDP_COMPONENT_COLOR1] = MUTEX_MOD_DISP_COLOR1,
-	[DDP_COMPONENT_GAMMA] = MUTEX_MOD_DISP_GAMMA,
-	[DDP_COMPONENT_OD] = MUTEX_MOD_DISP_OD,
-	[DDP_COMPONENT_OVL0] = MUTEX_MOD_DISP_OVL0,
-	[DDP_COMPONENT_OVL1] = MUTEX_MOD_DISP_OVL1,
-	[DDP_COMPONENT_PWM0] = MUTEX_MOD_DISP_PWM0,
-	[DDP_COMPONENT_PWM1] = MUTEX_MOD_DISP_PWM1,
-	[DDP_COMPONENT_RDMA0] = MUTEX_MOD_DISP_RDMA0,
-	[DDP_COMPONENT_RDMA1] = MUTEX_MOD_DISP_RDMA1,
-	[DDP_COMPONENT_RDMA2] = MUTEX_MOD_DISP_RDMA2,
-	[DDP_COMPONENT_UFOE] = MUTEX_MOD_DISP_UFOE,
-	[DDP_COMPONENT_WDMA0] = MUTEX_MOD_DISP_WDMA0,
-	[DDP_COMPONENT_WDMA1] = MUTEX_MOD_DISP_WDMA1,
+	[DDP_COMPONENT_AAL] = MT8173_MUTEX_MOD_DISP_AAL,
+	[DDP_COMPONENT_COLOR0] = MT8173_MUTEX_MOD_DISP_COLOR0,
+	[DDP_COMPONENT_COLOR1] = MT8173_MUTEX_MOD_DISP_COLOR1,
+	[DDP_COMPONENT_GAMMA] = MT8173_MUTEX_MOD_DISP_GAMMA,
+	[DDP_COMPONENT_OD] = MT8173_MUTEX_MOD_DISP_OD,
+	[DDP_COMPONENT_OVL0] = MT8173_MUTEX_MOD_DISP_OVL0,
+	[DDP_COMPONENT_OVL1] = MT8173_MUTEX_MOD_DISP_OVL1,
+	[DDP_COMPONENT_PWM0] = MT8173_MUTEX_MOD_DISP_PWM0,
+	[DDP_COMPONENT_PWM1] = MT8173_MUTEX_MOD_DISP_PWM1,
+	[DDP_COMPONENT_RDMA0] = MT8173_MUTEX_MOD_DISP_RDMA0,
+	[DDP_COMPONENT_RDMA1] = MT8173_MUTEX_MOD_DISP_RDMA1,
+	[DDP_COMPONENT_RDMA2] = MT8173_MUTEX_MOD_DISP_RDMA2,
+	[DDP_COMPONENT_UFOE] = MT8173_MUTEX_MOD_DISP_UFOE,
+	[DDP_COMPONENT_WDMA0] = MT8173_MUTEX_MOD_DISP_WDMA0,
+	[DDP_COMPONENT_WDMA1] = MT8173_MUTEX_MOD_DISP_WDMA1,
 };
 
 static unsigned int mtk_ddp_mout_en(enum mtk_ddp_comp_id cur,
-- 
1.9.1

^ permalink raw reply related

* [PATCH v9 02/10] drm/mediatek: add *driver_data for different hardware settings
From: YT Shen @ 2016-11-11 11:55 UTC (permalink / raw)
  To: dri-devel, Philipp Zabel
  Cc: David Airlie, Matthias Brugger, YT Shen, Daniel Kurtz, Mao Huang,
	CK Hu, Bibby Hsieh, Daniel Vetter, Thierry Reding, Jie Qiu,
	Maxime Ripard, Chris Wilson, shaoming chen, Jitao Shi,
	Boris Brezillon, Dan Carpenter, linux-arm-kernel, linux-mediatek,
	linux-kernel, srv_heupstream
In-Reply-To: <1478865346-19043-1-git-send-email-yt.shen@mediatek.com>

There are some hardware settings changed, between MT8173 & MT2701:
DISP_OVL address offset changed, color format definition changed.
DISP_RDMA fifo size changed.
DISP_COLOR offset changed.
MIPI_TX pll setting changed.
And add prefix for mtk_ddp_main & mtk_ddp_ext & mutex_mod.

Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c     | 27 ++++++++++++++++-----------
 drivers/gpu/drm/mediatek/mtk_disp_rdma.c    | 11 +++++++++--
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c      | 11 +++++++----
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 27 +++++++++++++++++++++------
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 13 +++++++++++++
 drivers/gpu/drm/mediatek/mtk_drm_drv.c      | 25 ++++++++++++++++++-------
 drivers/gpu/drm/mediatek/mtk_drm_drv.h      |  8 ++++++++
 drivers/gpu/drm/mediatek/mtk_mipi_tx.c      | 24 +++++++++++++++++++++++-
 8 files changed, 115 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 019b7ca..1139834 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -35,13 +35,10 @@
 #define DISP_REG_OVL_PITCH(n)			(0x0044 + 0x20 * (n))
 #define DISP_REG_OVL_RDMA_CTRL(n)		(0x00c0 + 0x20 * (n))
 #define DISP_REG_OVL_RDMA_GMC(n)		(0x00c8 + 0x20 * (n))
-#define DISP_REG_OVL_ADDR(n)			(0x0f40 + 0x20 * (n))
 
 #define	OVL_RDMA_MEM_GMC	0x40402020
 
 #define OVL_CON_BYTE_SWAP	BIT(24)
-#define OVL_CON_CLRFMT_RGB565	(0 << 12)
-#define OVL_CON_CLRFMT_RGB888	(1 << 12)
 #define OVL_CON_CLRFMT_RGBA8888	(2 << 12)
 #define OVL_CON_CLRFMT_ARGB8888	(3 << 12)
 #define	OVL_CON_AEN		BIT(8)
@@ -137,18 +134,18 @@ static void mtk_ovl_layer_off(struct mtk_ddp_comp *comp, unsigned int idx)
 	writel(0x0, comp->regs + DISP_REG_OVL_RDMA_CTRL(idx));
 }
 
-static unsigned int ovl_fmt_convert(unsigned int fmt)
+static unsigned int ovl_fmt_convert(struct mtk_ddp_comp *comp, unsigned int fmt)
 {
 	switch (fmt) {
 	default:
 	case DRM_FORMAT_RGB565:
-		return OVL_CON_CLRFMT_RGB565;
+		return comp->data->ovl.fmt_rgb565;
 	case DRM_FORMAT_BGR565:
-		return OVL_CON_CLRFMT_RGB565 | OVL_CON_BYTE_SWAP;
+		return comp->data->ovl.fmt_rgb565 | OVL_CON_BYTE_SWAP;
 	case DRM_FORMAT_RGB888:
-		return OVL_CON_CLRFMT_RGB888;
+		return comp->data->ovl.fmt_rgb888;
 	case DRM_FORMAT_BGR888:
-		return OVL_CON_CLRFMT_RGB888 | OVL_CON_BYTE_SWAP;
+		return comp->data->ovl.fmt_rgb888 | OVL_CON_BYTE_SWAP;
 	case DRM_FORMAT_RGBX8888:
 	case DRM_FORMAT_RGBA8888:
 		return OVL_CON_CLRFMT_ARGB8888;
@@ -178,7 +175,7 @@ static void mtk_ovl_layer_config(struct mtk_ddp_comp *comp, unsigned int idx,
 	if (!pending->enable)
 		mtk_ovl_layer_off(comp, idx);
 
-	con = ovl_fmt_convert(fmt);
+	con = ovl_fmt_convert(comp, fmt);
 	if (idx != 0)
 		con |= OVL_CON_AEN | OVL_CON_ALPHA;
 
@@ -186,7 +183,8 @@ static void mtk_ovl_layer_config(struct mtk_ddp_comp *comp, unsigned int idx,
 	writel_relaxed(pitch, comp->regs + DISP_REG_OVL_PITCH(idx));
 	writel_relaxed(src_size, comp->regs + DISP_REG_OVL_SRC_SIZE(idx));
 	writel_relaxed(offset, comp->regs + DISP_REG_OVL_OFFSET(idx));
-	writel_relaxed(addr, comp->regs + DISP_REG_OVL_ADDR(idx));
+	writel_relaxed(addr, comp->regs + comp->data->ovl.addr_offset
+					+ idx * 0x20);
 
 	if (pending->enable)
 		mtk_ovl_layer_on(comp, idx);
@@ -270,6 +268,8 @@ static int mtk_disp_ovl_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	priv->ddp_comp.data = of_device_get_match_data(dev);
+
 	platform_set_drvdata(pdev, priv);
 
 	ret = component_add(dev, &mtk_disp_ovl_component_ops);
@@ -286,8 +286,13 @@ static int mtk_disp_ovl_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct mtk_ddp_comp_driver_data mt8173_ovl_driver_data = {
+	.ovl = {0x0f40, 0, 1 << 12}
+};
+
 static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
-	{ .compatible = "mediatek,mt8173-disp-ovl", },
+	{ .compatible = "mediatek,mt8173-disp-ovl",
+	  .data = &mt8173_ovl_driver_data},
 	{},
 };
 MODULE_DEVICE_TABLE(of, mtk_disp_ovl_driver_dt_match);
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
index 0df05f9..b4225e2 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
@@ -123,7 +123,7 @@ static void mtk_rdma_config(struct mtk_ddp_comp *comp, unsigned int width,
 	 */
 	threshold = width * height * vrefresh * 4 * 7 / 1000000;
 	reg = RDMA_FIFO_UNDERFLOW_EN |
-	      RDMA_FIFO_PSEUDO_SIZE(SZ_8K) |
+	      RDMA_FIFO_PSEUDO_SIZE(comp->data->rdma_fifo_pseudo_size) |
 	      RDMA_OUTPUT_VALID_FIFO_THRESHOLD(threshold);
 	writel(reg, comp->regs + DISP_REG_RDMA_FIFO_CON);
 }
@@ -208,6 +208,8 @@ static int mtk_disp_rdma_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	priv->ddp_comp.data = of_device_get_match_data(dev);
+
 	platform_set_drvdata(pdev, priv);
 
 	ret = component_add(dev, &mtk_disp_rdma_component_ops);
@@ -224,8 +226,13 @@ static int mtk_disp_rdma_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct mtk_ddp_comp_driver_data mt8173_rdma_driver_data = {
+	.rdma_fifo_pseudo_size = SZ_8K,
+};
+
 static const struct of_device_id mtk_disp_rdma_driver_dt_match[] = {
-	{ .compatible = "mediatek,mt8173-disp-rdma", },
+	{ .compatible = "mediatek,mt8173-disp-rdma",
+	  .data = &mt8173_rdma_driver_data},
 	{},
 };
 MODULE_DEVICE_TABLE(of, mtk_disp_rdma_driver_dt_match);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index 2fc4321..8030769 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -77,9 +77,10 @@ struct mtk_ddp {
 	struct clk			*clk;
 	void __iomem			*regs;
 	struct mtk_disp_mutex		mutex[10];
+	const unsigned int		*mutex_mod;
 };
 
-static const unsigned int mutex_mod[DDP_COMPONENT_ID_MAX] = {
+static const unsigned int mt8173_mutex_mod[DDP_COMPONENT_ID_MAX] = {
 	[DDP_COMPONENT_AAL] = MT8173_MUTEX_MOD_DISP_AAL,
 	[DDP_COMPONENT_COLOR0] = MT8173_MUTEX_MOD_DISP_COLOR0,
 	[DDP_COMPONENT_COLOR1] = MT8173_MUTEX_MOD_DISP_COLOR1,
@@ -247,7 +248,7 @@ void mtk_disp_mutex_add_comp(struct mtk_disp_mutex *mutex,
 		break;
 	default:
 		reg = readl_relaxed(ddp->regs + DISP_REG_MUTEX_MOD(mutex->id));
-		reg |= mutex_mod[id];
+		reg |= ddp->mutex_mod[id];
 		writel_relaxed(reg, ddp->regs + DISP_REG_MUTEX_MOD(mutex->id));
 		return;
 	}
@@ -273,7 +274,7 @@ void mtk_disp_mutex_remove_comp(struct mtk_disp_mutex *mutex,
 		break;
 	default:
 		reg = readl_relaxed(ddp->regs + DISP_REG_MUTEX_MOD(mutex->id));
-		reg &= ~mutex_mod[id];
+		reg &= ~(ddp->mutex_mod[id]);
 		writel_relaxed(reg, ddp->regs + DISP_REG_MUTEX_MOD(mutex->id));
 		break;
 	}
@@ -326,6 +327,8 @@ static int mtk_ddp_probe(struct platform_device *pdev)
 		return PTR_ERR(ddp->regs);
 	}
 
+	ddp->mutex_mod = of_device_get_match_data(dev);
+
 	platform_set_drvdata(pdev, ddp);
 
 	return 0;
@@ -337,7 +340,7 @@ static int mtk_ddp_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id ddp_driver_dt_match[] = {
-	{ .compatible = "mediatek,mt8173-disp-mutex" },
+	{ .compatible = "mediatek,mt8173-disp-mutex", .data = mt8173_mutex_mod},
 	{},
 };
 MODULE_DEVICE_TABLE(of, ddp_driver_dt_match);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index df33b3c..661a4a0 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -39,9 +39,8 @@
 #define DISP_REG_UFO_START			0x0000
 
 #define DISP_COLOR_CFG_MAIN			0x0400
-#define DISP_COLOR_START			0x0c00
-#define DISP_COLOR_WIDTH			0x0c50
-#define DISP_COLOR_HEIGHT			0x0c54
+#define DISP_COLOR_WIDTH			0x50
+#define DISP_COLOR_HEIGHT			0x54
 
 #define DISP_AAL_EN				0x0000
 #define DISP_AAL_SIZE				0x0030
@@ -107,15 +106,15 @@ static void mtk_color_config(struct mtk_ddp_comp *comp, unsigned int w,
 			     unsigned int h, unsigned int vrefresh,
 			     unsigned int bpc)
 {
-	writel(w, comp->regs + DISP_COLOR_WIDTH);
-	writel(h, comp->regs + DISP_COLOR_HEIGHT);
+	writel(w, comp->regs + comp->data->color_offset + DISP_COLOR_WIDTH);
+	writel(h, comp->regs + comp->data->color_offset + DISP_COLOR_HEIGHT);
 }
 
 static void mtk_color_start(struct mtk_ddp_comp *comp)
 {
 	writel(COLOR_BYPASS_ALL | COLOR_SEQ_SEL,
 	       comp->regs + DISP_COLOR_CFG_MAIN);
-	writel(0x1, comp->regs + DISP_COLOR_START);
+	writel(0x1, comp->regs + comp->data->color_offset);
 }
 
 static void mtk_od_config(struct mtk_ddp_comp *comp, unsigned int w,
@@ -264,6 +263,16 @@ struct mtk_ddp_comp_match {
 	[DDP_COMPONENT_WDMA1]	= { MTK_DISP_WDMA,	1, NULL },
 };
 
+static const struct mtk_ddp_comp_driver_data mt8173_color_driver_data = {
+	.color_offset = 0x0c00,
+};
+
+static const struct of_device_id mtk_disp_color_driver_dt_match[] = {
+	{ .compatible = "mediatek,mt8173-disp-color",
+	  .data = &mt8173_color_driver_data},
+	{},
+};
+
 int mtk_ddp_comp_get_id(struct device_node *node,
 			enum mtk_ddp_comp_type comp_type)
 {
@@ -286,6 +295,7 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node,
 	enum mtk_ddp_comp_type type;
 	struct device_node *larb_node;
 	struct platform_device *larb_pdev;
+	const struct of_device_id *match;
 
 	if (comp_id < 0 || comp_id >= DDP_COMPONENT_ID_MAX)
 		return -EINVAL;
@@ -310,6 +320,11 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node,
 
 	type = mtk_ddp_matches[comp_id].type;
 
+	if (type == MTK_DISP_COLOR) {
+		match = of_match_node(mtk_disp_color_driver_dt_match, node);
+		comp->data = match->data;
+	}
+
 	/* Only DMA capable components need the LARB property */
 	comp->larb_dev = NULL;
 	if (type != MTK_DISP_OVL &&
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index 22a33ee..2f6872a 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -78,6 +78,18 @@ struct mtk_ddp_comp_funcs {
 			  struct drm_crtc_state *state);
 };
 
+struct mtk_ddp_comp_driver_data {
+	union {
+		struct ovl {
+			unsigned int addr_offset;
+			unsigned int fmt_rgb565;
+			unsigned int fmt_rgb888;
+		} ovl;
+		unsigned int rdma_fifo_pseudo_size;
+		unsigned int color_offset;
+	};
+};
+
 struct mtk_ddp_comp {
 	struct clk *clk;
 	void __iomem *regs;
@@ -85,6 +97,7 @@ struct mtk_ddp_comp {
 	struct device *larb_dev;
 	enum mtk_ddp_comp_id id;
 	const struct mtk_ddp_comp_funcs *funcs;
+	const struct mtk_ddp_comp_driver_data *data;
 };
 
 static inline void mtk_ddp_comp_config(struct mtk_ddp_comp *comp,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index cf83f65..5f9b5e8 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -126,7 +126,7 @@ static int mtk_atomic_commit(struct drm_device *drm,
 	.atomic_commit = mtk_atomic_commit,
 };
 
-static const enum mtk_ddp_comp_id mtk_ddp_main[] = {
+static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
 	DDP_COMPONENT_OVL0,
 	DDP_COMPONENT_COLOR0,
 	DDP_COMPONENT_AAL,
@@ -137,7 +137,7 @@ static int mtk_atomic_commit(struct drm_device *drm,
 	DDP_COMPONENT_PWM0,
 };
 
-static const enum mtk_ddp_comp_id mtk_ddp_ext[] = {
+static const enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = {
 	DDP_COMPONENT_OVL1,
 	DDP_COMPONENT_COLOR1,
 	DDP_COMPONENT_GAMMA,
@@ -145,6 +145,13 @@ static int mtk_atomic_commit(struct drm_device *drm,
 	DDP_COMPONENT_DPI0,
 };
 
+static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
+	.main_path = mt8173_mtk_ddp_main,
+	.main_len = ARRAY_SIZE(mt8173_mtk_ddp_main),
+	.ext_path = mt8173_mtk_ddp_ext,
+	.ext_len = ARRAY_SIZE(mt8173_mtk_ddp_ext),
+};
+
 static int mtk_drm_kms_init(struct drm_device *drm)
 {
 	struct mtk_drm_private *private = drm->dev_private;
@@ -187,17 +194,19 @@ static int mtk_drm_kms_init(struct drm_device *drm)
 	 * and each statically assigned to a crtc:
 	 * OVL0 -> COLOR0 -> AAL -> OD -> RDMA0 -> UFOE -> DSI0 ...
 	 */
-	ret = mtk_drm_crtc_create(drm, mtk_ddp_main, ARRAY_SIZE(mtk_ddp_main));
+	ret = mtk_drm_crtc_create(drm, private->data->main_path,
+				  private->data->main_len);
 	if (ret < 0)
 		goto err_component_unbind;
 	/* ... and OVL1 -> COLOR1 -> GAMMA -> RDMA1 -> DPI0. */
-	ret = mtk_drm_crtc_create(drm, mtk_ddp_ext, ARRAY_SIZE(mtk_ddp_ext));
+	ret = mtk_drm_crtc_create(drm, private->data->ext_path,
+				  private->data->ext_len);
 	if (ret < 0)
 		goto err_component_unbind;
 
 	/* Use OVL device for all DMA memory allocations */
-	np = private->comp_node[mtk_ddp_main[0]] ?:
-	     private->comp_node[mtk_ddp_ext[0]];
+	np = private->comp_node[private->data->main_path[0]] ?:
+	     private->comp_node[private->data->ext_path[0]];
 	pdev = of_find_device_by_node(np);
 	if (!pdev) {
 		ret = -ENODEV;
@@ -362,6 +371,7 @@ static int mtk_drm_probe(struct platform_device *pdev)
 
 	mutex_init(&private->commit.lock);
 	INIT_WORK(&private->commit.work, mtk_atomic_work);
+	private->data = of_device_get_match_data(dev);
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	private->config_regs = devm_ioremap_resource(dev, mem);
@@ -512,7 +522,8 @@ static SIMPLE_DEV_PM_OPS(mtk_drm_pm_ops, mtk_drm_sys_suspend,
 			 mtk_drm_sys_resume);
 
 static const struct of_device_id mtk_drm_of_ids[] = {
-	{ .compatible = "mediatek,mt8173-mmsys", },
+	{ .compatible = "mediatek,mt8173-mmsys",
+	  .data = &mt8173_mmsys_driver_data},
 	{ }
 };
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index aa93894..fa0b106 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -28,6 +28,13 @@
 struct drm_property;
 struct regmap;
 
+struct mtk_mmsys_driver_data {
+	const enum mtk_ddp_comp_id *main_path;
+	unsigned int main_len;
+	const enum mtk_ddp_comp_id *ext_path;
+	unsigned int ext_len;
+};
+
 struct mtk_drm_private {
 	struct drm_device *drm;
 	struct device *dma_dev;
@@ -40,6 +47,7 @@ struct mtk_drm_private {
 	void __iomem *config_regs;
 	struct device_node *comp_node[DDP_COMPONENT_ID_MAX];
 	struct mtk_ddp_comp *ddp_comp[DDP_COMPONENT_ID_MAX];
+	const struct mtk_mmsys_driver_data *data;
 
 	struct {
 		struct drm_atomic_state *state;
diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
index 1c366f8..935a8ef 100644
--- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
+++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
@@ -16,6 +16,7 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
 
@@ -87,6 +88,9 @@
 
 #define MIPITX_DSI_PLL_CON2	0x58
 
+#define MIPITX_DSI_PLL_TOP	0x64
+#define RG_DSI_MPPLL_PRESERVE		(0xff << 8)
+
 #define MIPITX_DSI_PLL_PWR	0x68
 #define RG_DSI_MPPLL_SDM_PWR_ON		BIT(0)
 #define RG_DSI_MPPLL_SDM_ISO_EN		BIT(1)
@@ -123,10 +127,15 @@
 #define SW_LNT2_HSTX_PRE_OE		BIT(24)
 #define SW_LNT2_HSTX_OE			BIT(25)
 
+struct mtk_mipitx_data {
+	const u32 data;
+};
+
 struct mtk_mipi_tx {
 	struct device *dev;
 	void __iomem *regs;
 	unsigned int data_rate;
+	const struct mtk_mipitx_data *driver_data;
 	struct clk_hw pll_hw;
 	struct clk *pll;
 };
@@ -243,6 +252,10 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
 	mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
 			       RG_DSI_MPPLL_SDM_SSC_EN);
 
+	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_TOP,
+				RG_DSI_MPPLL_PRESERVE,
+				mipi_tx->driver_data->data);
+
 	return 0;
 }
 
@@ -255,6 +268,9 @@ static void mtk_mipi_tx_pll_unprepare(struct clk_hw *hw)
 	mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
 			       RG_DSI_MPPLL_PLL_EN);
 
+	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_TOP,
+				RG_DSI_MPPLL_PRESERVE, 0);
+
 	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_PWR,
 				RG_DSI_MPPLL_SDM_ISO_EN |
 				RG_DSI_MPPLL_SDM_PWR_ON,
@@ -391,6 +407,7 @@ static int mtk_mipi_tx_probe(struct platform_device *pdev)
 	if (!mipi_tx)
 		return -ENOMEM;
 
+	mipi_tx->driver_data = of_device_get_match_data(dev);
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	mipi_tx->regs = devm_ioremap_resource(dev, mem);
 	if (IS_ERR(mipi_tx->regs)) {
@@ -448,8 +465,13 @@ static int mtk_mipi_tx_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct mtk_mipitx_data mt8173_mipitx_data = {
+	.data = (0 << 8)
+};
+
 static const struct of_device_id mtk_mipi_tx_match[] = {
-	{ .compatible = "mediatek,mt8173-mipi-tx", },
+	{ .compatible = "mediatek,mt8173-mipi-tx",
+	  .data = &mt8173_mipitx_data },
 	{},
 };
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v9 03/10] drm/mediatek: add shadow register support
From: YT Shen @ 2016-11-11 11:55 UTC (permalink / raw)
  To: dri-devel, Philipp Zabel
  Cc: David Airlie, Matthias Brugger, YT Shen, Daniel Kurtz, Mao Huang,
	CK Hu, Bibby Hsieh, Daniel Vetter, Thierry Reding, Jie Qiu,
	Maxime Ripard, Chris Wilson, shaoming chen, Jitao Shi,
	Boris Brezillon, Dan Carpenter, linux-arm-kernel, linux-mediatek,
	linux-kernel, srv_heupstream
In-Reply-To: <1478865346-19043-1-git-send-email-yt.shen@mediatek.com>

We need to acquire mutex before using the resources,
and need to release it after finished.
So we don't need to write registers in the blanking period.

Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 76 ++++++++++++++++++++-------------
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c  | 25 +++++++++++
 drivers/gpu/drm/mediatek/mtk_drm_ddp.h  |  2 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.h  |  1 +
 4 files changed, 75 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 01a21dd..a4f2b3a 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -329,6 +329,42 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc *mtk_crtc)
 	pm_runtime_put(drm->dev);
 }
 
+static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
+{
+	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+	struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
+	struct mtk_ddp_comp *ovl = mtk_crtc->ddp_comp[0];
+	unsigned int i;
+
+	/*
+	 * TODO: instead of updating the registers here, we should prepare
+	 * working registers in atomic_commit and let the hardware command
+	 * queue update module registers on vblank.
+	 */
+	if (state->pending_config) {
+		mtk_ddp_comp_config(ovl, state->pending_width,
+				    state->pending_height,
+				    state->pending_vrefresh, 0);
+
+		state->pending_config = false;
+	}
+
+	if (mtk_crtc->pending_planes) {
+		for (i = 0; i < OVL_LAYER_NR; i++) {
+			struct drm_plane *plane = &mtk_crtc->planes[i];
+			struct mtk_plane_state *plane_state;
+
+			plane_state = to_mtk_plane_state(plane->state);
+
+			if (plane_state->pending.config) {
+				mtk_ddp_comp_layer_config(ovl, i, plane_state);
+				plane_state->pending.config = false;
+			}
+		}
+		mtk_crtc->pending_planes = false;
+	}
+}
+
 static void mtk_drm_crtc_enable(struct drm_crtc *crtc)
 {
 	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
@@ -405,6 +441,7 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
 				      struct drm_crtc_state *old_crtc_state)
 {
 	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+	struct mtk_drm_private *priv = crtc->dev->dev_private;
 	unsigned int pending_planes = 0;
 	int i;
 
@@ -423,6 +460,13 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
 	}
 	if (pending_planes)
 		mtk_crtc->pending_planes = true;
+
+	if (priv->data->shadow_register) {
+		mtk_disp_mutex_acquire(mtk_crtc->mutex);
+		mtk_crtc_ddp_config(crtc);
+		mtk_disp_mutex_release(mtk_crtc->mutex);
+	}
+
 	if (crtc->state->color_mgmt_changed)
 		for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
 			mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
@@ -471,36 +515,10 @@ static int mtk_drm_crtc_init(struct drm_device *drm,
 void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct mtk_ddp_comp *ovl)
 {
 	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
-	struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
-	unsigned int i;
+	struct mtk_drm_private *priv = crtc->dev->dev_private;
 
-	/*
-	 * TODO: instead of updating the registers here, we should prepare
-	 * working registers in atomic_commit and let the hardware command
-	 * queue update module registers on vblank.
-	 */
-	if (state->pending_config) {
-		mtk_ddp_comp_config(ovl, state->pending_width,
-				    state->pending_height,
-				    state->pending_vrefresh, 0);
-
-		state->pending_config = false;
-	}
-
-	if (mtk_crtc->pending_planes) {
-		for (i = 0; i < OVL_LAYER_NR; i++) {
-			struct drm_plane *plane = &mtk_crtc->planes[i];
-			struct mtk_plane_state *plane_state;
-
-			plane_state = to_mtk_plane_state(plane->state);
-
-			if (plane_state->pending.config) {
-				mtk_ddp_comp_layer_config(ovl, i, plane_state);
-				plane_state->pending.config = false;
-			}
-		}
-		mtk_crtc->pending_planes = false;
-	}
+	if (!priv->data->shadow_register)
+		mtk_crtc_ddp_config(crtc);
 
 	mtk_drm_finish_page_flip(mtk_crtc);
 }
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index 8030769..b77d456 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -12,6 +12,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
@@ -32,10 +33,13 @@
 #define DISP_REG_CONFIG_MMSYS_CG_CON0		0x100
 
 #define DISP_REG_MUTEX_EN(n)	(0x20 + 0x20 * (n))
+#define DISP_REG_MUTEX(n)	(0x24 + 0x20 * (n))
 #define DISP_REG_MUTEX_RST(n)	(0x28 + 0x20 * (n))
 #define DISP_REG_MUTEX_MOD(n)	(0x2c + 0x20 * (n))
 #define DISP_REG_MUTEX_SOF(n)	(0x30 + 0x20 * (n))
 
+#define INT_MUTEX				BIT(1)
+
 #define MT8173_MUTEX_MOD_DISP_OVL0		BIT(11)
 #define MT8173_MUTEX_MOD_DISP_OVL1		BIT(12)
 #define MT8173_MUTEX_MOD_DISP_RDMA0		BIT(13)
@@ -300,6 +304,27 @@ void mtk_disp_mutex_disable(struct mtk_disp_mutex *mutex)
 	writel(0, ddp->regs + DISP_REG_MUTEX_EN(mutex->id));
 }
 
+void mtk_disp_mutex_acquire(struct mtk_disp_mutex *mutex)
+{
+	struct mtk_ddp *ddp = container_of(mutex, struct mtk_ddp,
+					   mutex[mutex->id]);
+	u32 tmp;
+
+	writel(1, ddp->regs + DISP_REG_MUTEX_EN(mutex->id));
+	writel(1, ddp->regs + DISP_REG_MUTEX(mutex->id));
+	if (readl_poll_timeout_atomic(ddp->regs + DISP_REG_MUTEX(mutex->id),
+				      tmp, tmp & INT_MUTEX, 1, 10000))
+		pr_err("could not acquire mutex %d\n", mutex->id);
+}
+
+void mtk_disp_mutex_release(struct mtk_disp_mutex *mutex)
+{
+	struct mtk_ddp *ddp = container_of(mutex, struct mtk_ddp,
+					   mutex[mutex->id]);
+
+	writel(0, ddp->regs + DISP_REG_MUTEX(mutex->id));
+}
+
 static int mtk_ddp_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
index 92c1175..f9a7991 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
@@ -37,5 +37,7 @@ void mtk_disp_mutex_remove_comp(struct mtk_disp_mutex *mutex,
 				enum mtk_ddp_comp_id id);
 void mtk_disp_mutex_unprepare(struct mtk_disp_mutex *mutex);
 void mtk_disp_mutex_put(struct mtk_disp_mutex *mutex);
+void mtk_disp_mutex_acquire(struct mtk_disp_mutex *mutex);
+void mtk_disp_mutex_release(struct mtk_disp_mutex *mutex);
 
 #endif /* MTK_DRM_DDP_H */
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index fa0b106..94f8b66 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -33,6 +33,7 @@ struct mtk_mmsys_driver_data {
 	unsigned int main_len;
 	const enum mtk_ddp_comp_id *ext_path;
 	unsigned int ext_len;
+	bool shadow_register;
 };
 
 struct mtk_drm_private {
-- 
1.9.1

^ permalink raw reply related


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