Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 10/12] ARM: imx: Add suspend/resume support for imx6sll
From: Bai Ping @ 2016-12-27  9:47 UTC (permalink / raw)
  To: shawnguo, mturquette, sboyd, robh+dt, mark.rutland, linus.walleij
  Cc: devicetree, kernel, daniel.lezcano, linux-gpio, p.zabel,
	jacky.baip, fabio.estevam, tglx, linux-clk, linux-arm-kernel
In-Reply-To: <1482832070-22668-1-git-send-email-ping.bai@nxp.com>

Add suspend/resume support for imx6sll.

Signed-off-by: Bai Ping <ping.bai@nxp.com>
---
 arch/arm/mach-imx/pm-imx6.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
index 1515e49..2ed4316 100644
--- a/arch/arm/mach-imx/pm-imx6.c
+++ b/arch/arm/mach-imx/pm-imx6.c
@@ -145,6 +145,13 @@ struct imx6_pm_socdata {
 	0x494, 0x4b0,	            /* MODE_CTL, MODE, */
 };
 
+static const u32 imx6sll_mmdc_io_offset[] __initconst = {
+	0x294, 0x298, 0x29c, 0x2a0, /* DQM0 ~ DQM3 */
+	0x544, 0x54c, 0x554, 0x558, /* GPR_B0DS ~ GPR_B3DS */
+	0x530, 0x540, 0x2ac, 0x52c, /* MODE_CTL, MODE, SDCLK_0, GPR_ADDDS */
+	0x2a4, 0x2a8,		    /* SDCKE0, SDCKE1*/
+};
+
 static const struct imx6_pm_socdata imx6q_pm_data __initconst = {
 	.mmdc_compat = "fsl,imx6q-mmdc",
 	.src_compat = "fsl,imx6q-src",
@@ -195,6 +202,15 @@ struct imx6_pm_socdata {
 	.mmdc_io_offset = imx6ul_mmdc_io_offset,
 };
 
+static const struct imx6_pm_socdata imx6sll_pm_data __initconst = {
+	.mmdc_compat = "fsl,imx6sll-mmdc",
+	.src_compat = "fsl,imx6sll-src",
+	.iomuxc_compat = "fsl,imx6sll-iomuxc",
+	.gpc_compat = "fsl,imx6sll-gpc",
+	.pl310_compat = "arm,pl310-cache",
+	.mmdc_io_num = ARRAY_SIZE(imx6sll_mmdc_io_offset),
+	.mmdc_io_offset = imx6sll_mmdc_io_offset,
+};
 /*
  * This structure is for passing necessary data for low level ocram
  * suspend code(arch/arm/mach-imx/suspend-imx6.S), if this struct
@@ -293,9 +309,10 @@ int imx6_set_lpm(enum mxc_cpu_pwr_mode mode)
 		val |= 0x2 << BP_CLPCR_LPM;
 		val &= ~BM_CLPCR_VSTBY;
 		val &= ~BM_CLPCR_SBYOS;
-		if (cpu_is_imx6sl())
+		if (cpu_is_imx6sl() || cpu_is_imx6sll())
 			val |= BM_CLPCR_BYPASS_PMIC_READY;
-		if (cpu_is_imx6sl() || cpu_is_imx6sx() || cpu_is_imx6ul())
+		if (cpu_is_imx6sl() || cpu_is_imx6sx() || cpu_is_imx6ul() ||
+			cpu_is_imx6sll())
 			val |= BM_CLPCR_BYP_MMDC_CH0_LPM_HS;
 		else
 			val |= BM_CLPCR_BYP_MMDC_CH1_LPM_HS;
@@ -310,9 +327,10 @@ int imx6_set_lpm(enum mxc_cpu_pwr_mode mode)
 		val |= 0x3 << BP_CLPCR_STBY_COUNT;
 		val |= BM_CLPCR_VSTBY;
 		val |= BM_CLPCR_SBYOS;
-		if (cpu_is_imx6sl() || cpu_is_imx6sx())
+		if (cpu_is_imx6sl() || cpu_is_imx6sx() || cpu_is_imx6sll())
 			val |= BM_CLPCR_BYPASS_PMIC_READY;
-		if (cpu_is_imx6sl() || cpu_is_imx6sx() || cpu_is_imx6ul())
+		if (cpu_is_imx6sl() || cpu_is_imx6sx() ||
+		    cpu_is_imx6ul() || cpu_is_imx6sll())
 			val |= BM_CLPCR_BYP_MMDC_CH0_LPM_HS;
 		else
 			val |= BM_CLPCR_BYP_MMDC_CH1_LPM_HS;
@@ -373,6 +391,7 @@ static int imx6q_pm_enter(suspend_state_t state)
 			imx6sl_set_wait_clk(true);
 		/* Zzz ... */
 		cpu_do_idle();
+
 		if (cpu_is_imx6sl())
 			imx6sl_set_wait_clk(false);
 		imx_gpc_post_resume();
@@ -632,7 +651,10 @@ void __init imx6dl_pm_init(void)
 
 void __init imx6sl_pm_init(void)
 {
-	imx6_pm_common_init(&imx6sl_pm_data);
+	if (cpu_is_imx6sl())
+		imx6_pm_common_init(&imx6sl_pm_data);
+	else
+		imx6_pm_common_init(&imx6sll_pm_data);
 }
 
 void __init imx6sx_pm_init(void)
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 11/12] ARM: imx: correct i.mx6sll dram io low power mode
From: Bai Ping @ 2016-12-27  9:47 UTC (permalink / raw)
  To: shawnguo, mturquette, sboyd, robh+dt, mark.rutland, linus.walleij
  Cc: devicetree, kernel, daniel.lezcano, linux-gpio, p.zabel,
	jacky.baip, fabio.estevam, tglx, linux-clk, linux-arm-kernel
In-Reply-To: <1482832070-22668-1-git-send-email-ping.bai@nxp.com>

i.MX6SLL has different DRAM IO offset, and it has no
CAS/RAS/ODT/RESET pin now, correct the DRAM IO offset.

To better support all different i.MX6 SoCs and different
DRAM types, introduce a new column to store the low power
settings for DRAM IO, then suspend asm code no need to check
SoC or DRAM type, just get the DRAM IO's low power
settings from OCRAM pm_info and set to each DRAM IO.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Signed-off-by: Bai Ping <ping.bai@nxp.com>
---
 arch/arm/mach-imx/pm-imx6.c      | 17 ++++++++++++++++-
 arch/arm/mach-imx/suspend-imx6.S | 29 +++++++----------------------
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
index 2ed4316..5fb78a9 100644
--- a/arch/arm/mach-imx/pm-imx6.c
+++ b/arch/arm/mach-imx/pm-imx6.c
@@ -230,7 +230,7 @@ struct imx6_cpu_pm_info {
 	struct imx6_pm_base gpc_base;
 	struct imx6_pm_base l2_base;
 	u32 mmdc_io_num; /* Number of MMDC IOs which need saved/restored. */
-	u32 mmdc_io_val[MX6_MAX_MMDC_IO_NUM][2]; /* To save offset and value */
+	u32 mmdc_io_val[MX6_MAX_MMDC_IO_NUM][3]; /* To save offset,value and low power setting */
 } __aligned(8);
 
 void imx6_set_int_mem_clk_lpm(bool enable)
@@ -570,6 +570,21 @@ static int __init imx6q_suspend_init(const struct imx6_pm_socdata *socdata)
 		pm_info->mmdc_io_val[i][1] =
 			readl_relaxed(pm_info->iomuxc_base.vbase +
 			mmdc_offset_array[i]);
+		pm_info->mmdc_io_val[i][2] = 0;
+
+	}
+
+	/* i.MX6SLL has no DRAM RESET pin */
+	if (cpu_is_imx6sll()) {
+		pm_info->mmdc_io_val[pm_info->mmdc_io_num - 2][2] = 0x1000;
+		pm_info->mmdc_io_val[pm_info->mmdc_io_num - 1][2] = 0x1000;
+	} else {
+		if (pm_info->ddr_type == IMX_DDR_TYPE_LPDDR2) {
+			 /* for LPDDR2, CKE0/1 and RESET pin need special setting */
+			pm_info->mmdc_io_val[pm_info->mmdc_io_num - 3][2] = 0x1000;
+			pm_info->mmdc_io_val[pm_info->mmdc_io_num - 2][2] = 0x1000;
+			pm_info->mmdc_io_val[pm_info->mmdc_io_num - 1][2] = 0x80000;
+		}
 	}
 
 	imx6_suspend_in_ocram_fn = fncpy(
diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
index 76ee2ce..c9a26f4 100644
--- a/arch/arm/mach-imx/suspend-imx6.S
+++ b/arch/arm/mach-imx/suspend-imx6.S
@@ -104,7 +104,7 @@
 	add	r7, r7, r0
 1:
 	ldr	r8, [r7], #0x4
-	ldr	r9, [r7], #0x4
+	ldr	r9, [r7], #0x8
 	str	r9, [r11, r8]
 	subs	r6, r6, #0x1
 	bne	1b
@@ -179,7 +179,6 @@ ENTRY(imx6_suspend)
 	ldr	r11, [r0, #PM_INFO_MX6Q_IOMUXC_V_OFFSET]
 	ldr	r6, [r11, #0x0]
 
-	/* use r11 to store the IO address */
 	ldr	r11, [r0, #PM_INFO_MX6Q_SRC_V_OFFSET]
 	/* store physical resume addr and pm_info address. */
 	str	r9, [r11, #MX6Q_SRC_GPR1]
@@ -207,32 +206,18 @@ poll_dvfs_set:
 	ands	r7, r7, #(1 << 25)
 	beq	poll_dvfs_set
 
+	/* use r11 to store the IO address */
 	ldr	r11, [r0, #PM_INFO_MX6Q_IOMUXC_V_OFFSET]
-	ldr	r6, =0x0
-	ldr	r7, [r0, #PM_INFO_MMDC_IO_NUM_OFFSET]
+	ldr	r6, [r0, #PM_INFO_MMDC_IO_NUM_OFFSET]
 	ldr	r8, =PM_INFO_MMDC_IO_VAL_OFFSET
 	add	r8, r8, r0
-	/* LPDDR2's last 3 IOs need special setting */
-	cmp	r3, #IMX_DDR_TYPE_LPDDR2
-	subeq	r7, r7, #0x3
 set_mmdc_io_lpm:
-	ldr	r9, [r8], #0x8
-	str	r6, [r11, r9]
-	subs	r7, r7, #0x1
+	ldr	r7, [r8], #0x8
+	ldr	r9, [r8], #0x4
+	str	r9, [r11, r7]
+	subs	r6, r6, #0x1
 	bne	set_mmdc_io_lpm
 
-	cmp 	r3, #IMX_DDR_TYPE_LPDDR2
-	bne	set_mmdc_io_lpm_done
-	ldr	r6, =0x1000
-	ldr	r9, [r8], #0x8
-	str	r6, [r11, r9]
-	ldr	r9, [r8], #0x8
-	str	r6, [r11, r9]
-	ldr	r6, =0x80000
-	ldr	r9, [r8]
-	str	r6, [r11, r9]
-set_mmdc_io_lpm_done:
-
 	/*
 	 * mask all GPC interrupts before
 	 * enabling the RBC counters to
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 12/12] ARM: configs: enable imx6sll support in defconfig
From: Bai Ping @ 2016-12-27  9:47 UTC (permalink / raw)
  To: shawnguo, mturquette, sboyd, robh+dt, mark.rutland, linus.walleij
  Cc: devicetree, kernel, daniel.lezcano, linux-gpio, p.zabel,
	jacky.baip, fabio.estevam, tglx, linux-clk, linux-arm-kernel
In-Reply-To: <1482832070-22668-1-git-send-email-ping.bai@nxp.com>

Enable i.MX6SLL in imx_v6_v7_defconfig.

Signed-off-by: Bai Ping <ping.bai@nxp.com>
---
 arch/arm/configs/imx_v6_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index cbe7faf..ab8afb3 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -38,6 +38,7 @@ CONFIG_SOC_IMX51=y
 CONFIG_SOC_IMX53=y
 CONFIG_SOC_IMX6Q=y
 CONFIG_SOC_IMX6SL=y
+CONFIG_SOC_IMX6SLL=y
 CONFIG_SOC_IMX6SX=y
 CONFIG_SOC_IMX6UL=y
 CONFIG_SOC_IMX7D=y
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH v3 0/2] Add MediaTek crypto accelerator driver
From: Herbert Xu @ 2016-12-27 10:06 UTC (permalink / raw)
  To: Ryder Lee
  Cc: David S. Miller, Matthias Brugger, devicetree, linux-mediatek,
	linux-kernel, linux-crypto, linux-arm-kernel, Sean Wang, Roy Luo
In-Reply-To: <1482114045-18716-1-git-send-email-ryder.lee@mediatek.com>

On Mon, Dec 19, 2016 at 10:20:43AM +0800, Ryder Lee wrote:
> Hello,
> 
> This adds support for the MediaTek hardware accelerator on
> some SoCs.
> 
> This driver currently implement: 
> - SHA1 and SHA2 family(HMAC) hash algorithms.
> - AES block cipher in CBC/ECB mode with 128/196/256 bits keys.
> 
> Chances since v3:
> -remove unused structure member
> -drop interrupt-parent from DT bindings documentation
> 
> Changes since v2:
> - use byteorder conversion macros and type identifiers for descriptors
> - revise register definition macros to make it more clear
> - revise DT compatiable string
> 
> Changes since v1:
> - remove EXPORT_SYMBOL
> - remove unused PRNG setting
> - sort headers in alphabetical order
> - add a definition for IRQ unmber
> - replace ambiguous definition
> - add more annotation and function comment
> - add COMPILE_TEST in Kconfig
> 
> Ryder Lee (2):
>   Add crypto driver support for some MediaTek chips
>   crypto: mediatek - add DT bindings documentation

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH] ARM: dts: sunxi: Use axp209.dtsi for Olinuxino Lime2
From: Emmanuel Vadot @ 2016-12-27 10:22 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Emmanuel Vadot

Use axp209.dtsi in sun7i-a20-olinuxino-lime2.dts and correct
some regulators.

DCDC2 is used for vdd-cpu so it should never be bellow 1V and above 1.4V
DCDC3 is used for VDD_INT so same as above.
LD01 is used for the RTC, and should have a typical value of 1.3V
LD02 is used for AVCC and should have a typical value of 3.0V
LD03/4 are used for Port-E/Port-G Power pin, and the schematics recommands
to set them to 2.8V as they can be used for CSI0/1.

Signed-off-by: Emmanuel Vadot <manu-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org>
---
 arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 90 ++++++++++++-------------
 1 file changed, 42 insertions(+), 48 deletions(-)

diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
index d5c796c8d16f..5311dbce6fd9 100644
--- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
@@ -112,57 +112,9 @@
 	status = "okay";
 
 	axp209: pmic@34 {
-		compatible = "x-powers,axp209";
 		reg = <0x34>;
 		interrupt-parent = <&nmi_intc>;
 		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
-
-		interrupt-controller;
-		#interrupt-cells = <1>;
-
-		acin-supply = <&reg_axp_ipsout>;
-		vin2-supply = <&reg_axp_ipsout>;
-		vin3-supply = <&reg_axp_ipsout>;
-		ldo24in-supply = <&reg_axp_ipsout>;
-		ldo3in-supply = <&reg_axp_ipsout>;
-
-		regulators {
-			vdd_rtc: ldo1 {
-				regulator-min-microvolt = <1300000>;
-				regulator-max-microvolt = <1300000>;
-				regulator-always-on;
-			};
-
-			avcc: ldo2 {
-				regulator-min-microvolt = <1800000>;
-				regulator-max-microvolt = <3300000>;
-				regulator-always-on;
-			};
-
-			vcc_csi0: ldo3 {
-				regulator-min-microvolt = <700000>;
-				regulator-max-microvolt = <3500000>;
-				regulator-always-on;
-			};
-
-			vcc_csi1: ldo4 {
-				regulator-min-microvolt = <1250000>;
-				regulator-max-microvolt = <3300000>;
-				regulator-always-on;
-			};
-
-			vdd_cpu: dcdc2 {
-				regulator-min-microvolt = <700000>;
-				regulator-max-microvolt = <2275000>;
-				regulator-always-on;
-			};
-
-			vdd_int: dcdc3 {
-				regulator-min-microvolt = <700000>;
-				regulator-max-microvolt = <3500000>;
-				regulator-always-on;
-			};
-		};
 	};
 };
 
@@ -243,6 +195,48 @@
 	status = "okay";
 };
 
+#include "axp209.dtsi"
+
+&reg_dcdc2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1400000>;
+	regulator-name = "vdd-cpu";
+};
+
+&reg_dcdc3 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1400000>;
+	regulator-name = "vdd-int-dll";
+};
+
+&reg_ldo1 {
+	regulator-always-on;
+	regulator-min-microvolt = <1300000>;
+	regulator-max-microvolt = <1300000>;
+	regulator-name = "vdd-rtc";
+};
+
+&reg_ldo2 {
+	regulator-always-on;
+	regulator-min-microvolt = <3000000>;
+	regulator-max-microvolt = <3000000>;
+	regulator-name = "avcc";
+};
+
+&reg_ldo3 {
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+	regulator-name = "vddio-csi0";
+};
+
+&reg_ldo4 {
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+	regulator-name = "vddio-csi1";
+};
+
 &reg_usb0_vbus {
 	pinctrl-0 = <&usb0_vbus_pin_lime2>;
 	gpio = <&pio 2 17 GPIO_ACTIVE_HIGH>;
-- 
2.11.0

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

* [PATCH v3] ARM: dts: sunxi: Add num-cs for A20 spi nodes
From: Emmanuel Vadot @ 2016-12-27 10:28 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Emmanuel Vadot

The spi0 controller on the A20 have up to 4 CS (Chip Select) while the
others three only have 1.
Add the num-cs property to each node.
The current driver doesn't read this property but this is useful for
downstream user of DTS (FreeBSD for example).

Signed-off-by: Emmanuel Vadot <manu-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org>
---
Changes in v3:
  * Put number in bracket

Changes in v2:
  * Explain that driver doesn't support this but that it is useful
  for downstream users of DTS.

arch/arm/boot/dts/sun7i-a20.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 94cf5a1c7172..f67bea5d0710 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -871,6 +871,7 @@
 			status = "disabled";
 			#address-cells = <1>;
 			#size-cells = <0>;
+			num-cs = <4>;
 		};
 
 		spi1: spi@01c06000 {
@@ -885,6 +886,7 @@
 			status = "disabled";
 			#address-cells = <1>;
 			#size-cells = <0>;
+			num-cs = <1>;
 		};
 
 		emac: ethernet@01c0b000 {
@@ -1037,6 +1039,7 @@
 			status = "disabled";
 			#address-cells = <1>;
 			#size-cells = <0>;
+			num-cs = <1>;
 		};
 
 		ahci: sata@01c18000 {
@@ -1079,6 +1082,7 @@
 			status = "disabled";
 			#address-cells = <1>;
 			#size-cells = <0>;
+			num-cs = <1>;
 		};
 
 		pio: pinctrl@01c20800 {
-- 
2.11.0

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

* Re: [PATCH v4 3/5] backlight: lm3533: Support initialization from Device Tree
From: Pavel Machek @ 2016-12-27 10:46 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Lee Jones, Jingoo Han, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Richard Purdie, Jacek Anaszewski,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA, Bjorn Andersson
In-Reply-To: <20161226181153.11271-3-bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

On Mon 2016-12-26 10:11:51, Bjorn Andersson wrote:
> From: Bjorn Andersson <bjorn.andersson-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
> 
> Implement support for initialization of the lm3533 backlight from Device
> Tree.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH v4 3/5] backlight: lm3533: Support initialization from Device Tree
From: Pavel Machek @ 2016-12-27 10:49 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Lee Jones, Jingoo Han, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Richard Purdie, Jacek Anaszewski,
	devicetree, linux-kernel, linux-iio, linux-leds, Bjorn Andersson
In-Reply-To: <20161226181153.11271-3-bjorn.andersson@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 5603 bytes --]

On Mon 2016-12-26 10:11:51, Bjorn Andersson wrote:
> From: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> 
> Implement support for initialization of the lm3533 backlight from Device
> Tree.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Bjorn, you may want to fix your email system:

<bjorn.andersson@sonymobile.com>: host
    seldsegrel01.sonyericsson.com[37.139.156.29] said: 550 Rule
    imposed mailbox
        access for this user is refused: user invalid (in reply to
    RCPT TO command)


									Pavel
> ---
> 
> This patch only depends on the acceptance of the DT binding (doesn't have to be
> merged together with the mfd patch).
> 
> Changes since v3:
> - Moved backlight DT parsing from mfd driver
> - Gave driver its own compatible
> 
>  drivers/video/backlight/lm3533_bl.c | 98 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 95 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
> index 0e2337f367b6..2f132199e604 100644
> --- a/drivers/video/backlight/lm3533_bl.c
> +++ b/drivers/video/backlight/lm3533_bl.c
> @@ -22,6 +22,7 @@
>  
>  
>  #define LM3533_HVCTRLBANK_COUNT		2
> +#define LM3533_BL_DEFAULT_BRIGHTNESS	200
>  #define LM3533_BL_MAX_BRIGHTNESS	255
>  
>  #define LM3533_REG_CTRLBANK_AB_BCONF	0x1a
> @@ -269,6 +270,86 @@ static int lm3533_bl_setup(struct lm3533_bl *bl,
>  	return lm3533_ctrlbank_set_pwm(&bl->cb, pdata->pwm);
>  }
>  
> +static int lm3533_of_parse_pwm_zones(struct device_node *node)
> +{
> +	const char *propname = "ti,pwm-zones";
> +	u32 zones[5];
> +	int count;
> +	int ret;
> +	int i;
> +
> +	count = of_property_count_u32_elems(node, propname);
> +	if (count == -EINVAL)
> +		return 0;
> +	if (count <= 0)
> +		return count;
> +	if (count >= ARRAY_SIZE(zones))
> +		return -EINVAL;
> +
> +	ret = of_property_read_u32_array(node, propname, zones, count);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable pwm input, and enable the selected zones */
> +	ret = BIT(0);
> +	for (i = 0; i < count; i++)
> +		ret |= BIT(zones[i] + 1);
> +
> +	return ret;
> +}
> +
> +static struct lm3533_bl_platform_data *lm3533_bl_of_parse(struct device *dev,
> +							  int *id)
> +{
> +	struct lm3533_bl_platform_data *bl_pdata;
> +	struct device_node *node = dev->of_node;
> +	int ret;
> +	u32 reg;
> +	u32 val;
> +
> +	bl_pdata = devm_kzalloc(dev, sizeof(*bl_pdata), GFP_KERNEL);
> +	if (!bl_pdata)
> +		return NULL;
> +
> +	ret = of_property_read_u32(node, "reg", &reg);
> +	if (ret < 0) {
> +		dev_err(dev, "invalid reg property\n");
> +		return NULL;
> +	}
> +	*id = reg;
> +
> +	ret = of_property_read_string(node, "label",
> +				      (const char **)&bl_pdata->name);
> +	if (ret < 0) {
> +		dev_err(dev, "unable to parse label\n");
> +		return NULL;
> +	}
> +
> +	ret = of_property_read_u32(node, "led-max-microamp", &val);
> +	if (ret < 0) {
> +		dev_err(dev, "unable to parse led-max-microamp\n");
> +		return NULL;
> +	}
> +	bl_pdata->max_current = val;
> +
> +	val = LM3533_BL_DEFAULT_BRIGHTNESS;
> +	ret = of_property_read_u32(node, "default-brightness", &val);
> +	if (ret < 0 && ret != -EINVAL) {
> +		dev_err(dev, "unable to parse default-brightness\n");
> +		return NULL;
> +	}
> +	bl_pdata->default_brightness = val;
> +
> +	ret = lm3533_of_parse_pwm_zones(node);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to parse ti,pwm-zones\n");
> +		return NULL;
> +	}
> +	bl_pdata->pwm = ret;
> +
> +	return bl_pdata;
> +}
> +
>  static int lm3533_bl_probe(struct platform_device *pdev)
>  {
>  	struct lm3533 *lm3533;
> @@ -277,6 +358,7 @@ static int lm3533_bl_probe(struct platform_device *pdev)
>  	struct backlight_device *bd;
>  	struct backlight_properties props;
>  	int ret;
> +	int id;
>  
>  	dev_dbg(&pdev->dev, "%s\n", __func__);
>  
> @@ -284,14 +366,17 @@ static int lm3533_bl_probe(struct platform_device *pdev)
>  	if (!lm3533)
>  		return -EINVAL;
>  
> +	id = pdev->id;
>  	pdata = dev_get_platdata(&pdev->dev);
> +	if (!pdata)
> +		pdata = lm3533_bl_of_parse(&pdev->dev, &id);
>  	if (!pdata) {
>  		dev_err(&pdev->dev, "no platform data\n");
>  		return -EINVAL;
>  	}
>  
> -	if (pdev->id < 0 || pdev->id >= LM3533_HVCTRLBANK_COUNT) {
> -		dev_err(&pdev->dev, "illegal backlight id %d\n", pdev->id);
> +	if (id < 0 || id >= LM3533_HVCTRLBANK_COUNT) {
> +		dev_err(&pdev->dev, "illegal backlight id %d\n", id);
>  		return -EINVAL;
>  	}
>  
> @@ -300,7 +385,7 @@ static int lm3533_bl_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	bl->lm3533 = lm3533;
> -	bl->id = pdev->id;
> +	bl->id = id;
>  
>  	bl->cb.lm3533 = lm3533;
>  	bl->cb.id = lm3533_bl_get_ctrlbank_id(bl);
> @@ -394,10 +479,17 @@ static void lm3533_bl_shutdown(struct platform_device *pdev)
>  	lm3533_ctrlbank_disable(&bl->cb);
>  }
>  
> +static const struct of_device_id lm3533_bl_of_match[] = {
> +	{ .compatible = "ti,lm3533-backlight", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, lm3533_bl_of_match);
> +
>  static struct platform_driver lm3533_bl_driver = {
>  	.driver = {
>  		.name	= "lm3533-backlight",
>  		.pm	= &lm3533_bl_pm_ops,
> +		.of_match_table = lm3533_bl_of_match,
>  	},
>  	.probe		= lm3533_bl_probe,
>  	.remove		= lm3533_bl_remove,

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/2] drm/panel: Add support for S6E3HA2 panel driver on TM2 board
From: Andrzej Hajda @ 2016-12-27 11:36 UTC (permalink / raw)
  To: Hoegeun Kwon, robh-DgEjT+Ai2ygdnm+yROfE0A,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, airlied-cv59FeDIM0c,
	kgene-DgEjT+Ai2ygdnm+yROfE0A, krzk-DgEjT+Ai2ygdnm+yROfE0A
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Donghwa Lee,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	cw00.choi-Sze3O3UU22JBDgjK7y7TUQ, Hyungwon Hwang
In-Reply-To: <1482829548-20842-2-git-send-email-hoegeun.kwon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Hi Hoegeun Kwon,

Thanks for the patch.

On 27.12.2016 10:05, Hoegeun Kwon wrote:
> This patch add support for MIPI-DSI based S6E3HA2 AMOLED panel
> driver. This panel has 1440x2560 resolution in 5.7-inch physical
> panel in the TM2 device.
>
> Signed-off-by: Donghwa Lee <dh09.lee-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Hyungwon Hwang <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> Changes for V2:
>
> - Fixed the samsung,s6e3ha2.txt DT document.
>   - Added active high or low after the description of the GPIOs.
>   - Removed the reg and added a description of the virtual
>     channel number of a DSI peripheral.

I do not understand this change, according to Rob's comment for previous
patch:
> reg doesn't really work here unless this node is a child of the DSI 
> controller node. But if it is a child node, then you don't need the OF 
> graph.

As the panel is controlled via DSI bus the node should be a child of DSI
controller, and therefore 'reg' property is required. Nice novum for me
is that OF graph is not required in that case.

>
>  .../bindings/display/panel/samsung,s6e3ha2.txt     |  62 ++
>  drivers/gpu/drm/panel/Kconfig                      |   6 +
>  drivers/gpu/drm/panel/Makefile                     |   1 +
>  drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c      | 756 +++++++++++++++++++++
>  4 files changed, 825 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
>  create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
>
> diff --git a/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
> new file mode 100644
> index 0000000..6e989ee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
> @@ -0,0 +1,62 @@
> +Samsung S6E3HA2 5.7" 1440x2560 AMOLED panel
> +
> +Required properties:
> +  - compatible: "samsung,s6e3ha2"
> +  - vdd3-supply: core voltage supply
Shouldn't be 'I/O voltage supply' ?

> +  - vci-supply: voltage supply for analog circuits
> +  - reset-gpios: a GPIO spec for the reset pin (active high)

According to specs it is active low, logic in the driver should be also
adjusted.

> +  - enable-gpios: a GPIO spec for the panel enable pin (active high)
> +  - te-gpios: a GPIO spec for the tearing effect synchronization signal
> +    gpio pin (active high)
> +
> +Optional properties:
> +  - display-timings: timings for the connected panel as described by [1]

In case of panels timings should be provided by device driver, not by dts.

> +
> +The device node can contain one 'port' child node with one child
> +'endpoint' node, according to the bindings defined in [2]. This
> +node should describe panel's video bus.

This could be dropped according to Rob's advice.

> +
> +The virtual channel number of a DSI peripheral as described by [3].
> +Must be in the range from 0 to 3.
> +/* peripheral responds to virtual channel 0 */
> +peripheral@0 {
> +	compatible = "...";
> +	reg = <0>;
> +};
> +
> +[1]: Documentation/devicetree/bindings/display/panel/display-timing.txt
> +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt
> +[3]: Documentation/devicetree/bindings/display/mipi-dsi-bus.txt
> +
> +Example:
> +
> +	panel@0 {
> +		...
> +
> +		compatible = "samsung,s6e3ha2";
> +		vdd3-supply = <&ldo27_reg>;
> +		vci-supply = <&ldo28_reg>;
> +		reset-gpios = <&gpg0 0 GPIO_ACTIVE_HIGH>;
> +		enable-gpios = <&gpf1 5 GPIO_ACTIVE_HIGH>;
> +		te-gpios = <&gpf1 3 GPIO_ACTIVE_HIGH>;
> +
> +		display-timings {
> +			timing-0 {
> +				clock-frequency = <0>;
> +				hactive = <1440>;
> +				vactive = <2560>;
> +				hfront-porch = <1>;
> +				hback-porch = <1>;
> +				hsync-len = <1>;
> +				vfront-porch = <1>;
> +				vback-porch = <15>;
> +				vsync-len = <1>;
> +			};
> +		};
> +
> +		port {
> +			dsi_in: endpoint {
> +				remote-endpoint = <&dsi_out>;
> +			};
> +		};
> +	};
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 62aba97..e1a2fcd 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -58,6 +58,12 @@ config DRM_PANEL_SAMSUNG_S6E8AA0
>  	select DRM_MIPI_DSI
>  	select VIDEOMODE_HELPERS
>  
> +config DRM_PANEL_SAMSUNG_S6E3HA2
> +	tristate "Samsung S6E3HA2 DSI video mode panel"
> +	depends on OF
> +	select DRM_MIPI_DSI

It should be "depends on" in case of panels.

> +	select VIDEOMODE_HELPERS
> +
>  config DRM_PANEL_SHARP_LQ101R1SX01
>  	tristate "Sharp LQ101R1SX01 panel"
>  	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index a5c7ec0..993699b 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -4,5 +4,6 @@ obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
>  obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
> +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o

Shoudn't it be sorted?

>  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
> new file mode 100644
> index 0000000..a6ad63b
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
> @@ -0,0 +1,756 @@
> +/*
> + * MIPI-DSI based s6e3ha2 AMOLED 5.7 inch panel driver.
> + *
> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
> + * Donghwa Lee <dh09.lee-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + * Hyungwon Hwang <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + * Hoegeun Kwon <hoegeun.kwon-Sze3O3UU22JBDgjK7y7TUQ@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.
> +*/
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_panel.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +#include <video/mipi_display.h>
> +#include <video/of_videomode.h>
> +#include <video/videomode.h>
> +#include <linux/backlight.h>
> +
> +#define S6E3HA2_MIN_BRIGHTNESS		0
> +#define S6E3HA2_MAX_BRIGHTNESS		100
> +#define S6E3HA2_DEFAULT_BRIGHTNESS	80
> +
> +#define S6E3HA2_NUM_GAMMA_STEPS		46
> +#define S6E3HA2_GAMMA_CMD_CNT		35
> +#define S6E3HA2_VINT_STATUS_MAX		10
> +
> +static const u8 gamma_tbl[S6E3HA2_NUM_GAMMA_STEPS][S6E3HA2_GAMMA_CMD_CNT] = {
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x82, 0x83,
> +	  0x85, 0x88, 0x8b, 0x8b, 0x84, 0x88, 0x82, 0x82, 0x89, 0x86, 0x8c,
> +	  0x94, 0x84, 0xb1, 0xaf, 0x8e, 0xcf, 0xad, 0xc9, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x84, 0x84,
> +	  0x85, 0x87, 0x8b, 0x8a, 0x84, 0x88, 0x82, 0x82, 0x89, 0x86, 0x8a,
> +	  0x93, 0x84, 0xb0, 0xae, 0x8e, 0xc9, 0xa8, 0xc5, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83,
> +	  0x85, 0x86, 0x8a, 0x8a, 0x84, 0x88, 0x81, 0x84, 0x8a, 0x88, 0x8a,
> +	  0x91, 0x84, 0xb1, 0xae, 0x8b, 0xd5, 0xb2, 0xcc, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83,
> +	  0x85, 0x86, 0x8a, 0x8a, 0x84, 0x87, 0x81, 0x84, 0x8a, 0x87, 0x8a,
> +	  0x91, 0x85, 0xae, 0xac, 0x8a, 0xc3, 0xa3, 0xc0, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x85, 0x85,
> +	  0x86, 0x85, 0x88, 0x89, 0x84, 0x89, 0x82, 0x84, 0x87, 0x85, 0x8b,
> +	  0x91, 0x88, 0xad, 0xab, 0x8a, 0xb7, 0x9b, 0xb6, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83,
> +	  0x85, 0x86, 0x89, 0x8a, 0x84, 0x89, 0x83, 0x83, 0x86, 0x84, 0x8b,
> +	  0x90, 0x84, 0xb0, 0xae, 0x8b, 0xce, 0xad, 0xc8, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83,
> +	  0x85, 0x87, 0x89, 0x8a, 0x83, 0x87, 0x82, 0x85, 0x88, 0x87, 0x89,
> +	  0x8f, 0x84, 0xac, 0xaa, 0x89, 0xb1, 0x98, 0xaf, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83,
> +	  0x85, 0x86, 0x88, 0x89, 0x84, 0x88, 0x83, 0x82, 0x85, 0x84, 0x8c,
> +	  0x91, 0x86, 0xac, 0xaa, 0x89, 0xc2, 0xa5, 0xbd, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x84, 0x84,
> +	  0x85, 0x87, 0x89, 0x8a, 0x83, 0x87, 0x82, 0x85, 0x88, 0x87, 0x88,
> +	  0x8b, 0x82, 0xad, 0xaa, 0x8a, 0xc2, 0xa5, 0xbd, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83,
> +	  0x85, 0x86, 0x87, 0x89, 0x84, 0x88, 0x83, 0x82, 0x85, 0x84, 0x8a,
> +	  0x8e, 0x84, 0xae, 0xac, 0x89, 0xda, 0xb7, 0xd0, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x84, 0x84,
> +	  0x85, 0x86, 0x87, 0x89, 0x84, 0x88, 0x83, 0x80, 0x83, 0x82, 0x8b,
> +	  0x8e, 0x85, 0xac, 0xaa, 0x89, 0xc8, 0xaa, 0xc1, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x84, 0x84,
> +	  0x85, 0x86, 0x87, 0x89, 0x81, 0x85, 0x81, 0x84, 0x86, 0x84, 0x8c,
> +	  0x8c, 0x84, 0xa9, 0xa8, 0x87, 0xa3, 0x92, 0xa1, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x84, 0x84,
> +	  0x85, 0x86, 0x87, 0x89, 0x84, 0x86, 0x83, 0x80, 0x83, 0x81, 0x8c,
> +	  0x8d, 0x84, 0xaa, 0xaa, 0x89, 0xce, 0xaf, 0xc5, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x84, 0x84,
> +	  0x85, 0x86, 0x87, 0x89, 0x81, 0x83, 0x80, 0x83, 0x85, 0x85, 0x8c,
> +	  0x8c, 0x84, 0xa8, 0xa8, 0x88, 0xb5, 0x9f, 0xb0, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x84, 0x84,
> +	  0x86, 0x86, 0x87, 0x88, 0x81, 0x83, 0x80, 0x83, 0x85, 0x85, 0x8c,
> +	  0x8b, 0x84, 0xab, 0xa8, 0x86, 0xd4, 0xb4, 0xc9, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x84, 0x84,
> +	  0x86, 0x86, 0x87, 0x88, 0x81, 0x83, 0x80, 0x84, 0x84, 0x85, 0x8b,
> +	  0x8a, 0x83, 0xa6, 0xa5, 0x84, 0xbb, 0xa4, 0xb3, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x84, 0x84,
> +	  0x86, 0x85, 0x86, 0x86, 0x82, 0x85, 0x81, 0x82, 0x83, 0x84, 0x8e,
> +	  0x8b, 0x83, 0xa4, 0xa3, 0x8a, 0xa1, 0x93, 0x9d, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x83, 0x83,
> +	  0x85, 0x86, 0x87, 0x87, 0x82, 0x85, 0x81, 0x82, 0x82, 0x84, 0x8e,
> +	  0x8b, 0x83, 0xa4, 0xa2, 0x86, 0xc1, 0xa9, 0xb7, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x83, 0x83,
> +	  0x85, 0x86, 0x87, 0x87, 0x82, 0x85, 0x81, 0x82, 0x82, 0x84, 0x8d,
> +	  0x89, 0x82, 0xa2, 0xa1, 0x84, 0xa7, 0x98, 0xa1, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x83, 0x83,
> +	  0x85, 0x86, 0x87, 0x87, 0x82, 0x85, 0x81, 0x83, 0x83, 0x85, 0x8c,
> +	  0x87, 0x7f, 0xa2, 0x9d, 0x88, 0x8d, 0x88, 0x8b, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xbb, 0x00, 0xc5, 0x00, 0xb4, 0x87, 0x86, 0x86, 0x84, 0x83,
> +	  0x86, 0x87, 0x87, 0x87, 0x80, 0x82, 0x7f, 0x86, 0x86, 0x88, 0x8a,
> +	  0x84, 0x7e, 0x9d, 0x9c, 0x82, 0x8d, 0x88, 0x8b, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xbd, 0x00, 0xc7, 0x00, 0xb7, 0x87, 0x85, 0x85, 0x84, 0x83,
> +	  0x86, 0x86, 0x86, 0x88, 0x81, 0x83, 0x80, 0x83, 0x84, 0x85, 0x8a,
> +	  0x85, 0x7e, 0x9c, 0x9b, 0x85, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xc0, 0x00, 0xca, 0x00, 0xbb, 0x87, 0x86, 0x85, 0x83, 0x83,
> +	  0x85, 0x86, 0x86, 0x88, 0x81, 0x83, 0x80, 0x84, 0x85, 0x86, 0x89,
> +	  0x83, 0x7d, 0x9c, 0x99, 0x87, 0x7b, 0x7b, 0x7c, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xc4, 0x00, 0xcd, 0x00, 0xbe, 0x87, 0x86, 0x85, 0x83, 0x83,
> +	  0x86, 0x85, 0x85, 0x87, 0x81, 0x82, 0x80, 0x82, 0x82, 0x83, 0x8a,
> +	  0x85, 0x7f, 0x9f, 0x9b, 0x86, 0xb4, 0xa1, 0xac, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xc7, 0x00, 0xd0, 0x00, 0xc2, 0x87, 0x85, 0x85, 0x83, 0x82,
> +	  0x85, 0x85, 0x85, 0x86, 0x82, 0x83, 0x80, 0x82, 0x82, 0x84, 0x87,
> +	  0x86, 0x80, 0x9e, 0x9a, 0x87, 0xa7, 0x98, 0xa1, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xca, 0x00, 0xd2, 0x00, 0xc5, 0x87, 0x85, 0x84, 0x82, 0x82,
> +	  0x84, 0x85, 0x85, 0x86, 0x81, 0x82, 0x7f, 0x82, 0x82, 0x84, 0x88,
> +	  0x86, 0x81, 0x9d, 0x98, 0x86, 0x8d, 0x88, 0x8b, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xce, 0x00, 0xd6, 0x00, 0xca, 0x86, 0x85, 0x84, 0x83, 0x83,
> +	  0x85, 0x84, 0x84, 0x85, 0x81, 0x82, 0x80, 0x81, 0x81, 0x82, 0x89,
> +	  0x86, 0x81, 0x9c, 0x97, 0x86, 0xa7, 0x98, 0xa1, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xd1, 0x00, 0xd9, 0x00, 0xce, 0x86, 0x84, 0x83, 0x83, 0x82,
> +	  0x85, 0x85, 0x85, 0x86, 0x81, 0x83, 0x81, 0x82, 0x82, 0x83, 0x86,
> +	  0x83, 0x7f, 0x99, 0x95, 0x86, 0xbb, 0xa4, 0xb3, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xd4, 0x00, 0xdb, 0x00, 0xd1, 0x86, 0x85, 0x83, 0x83, 0x82,
> +	  0x85, 0x84, 0x84, 0x85, 0x80, 0x83, 0x82, 0x80, 0x80, 0x81, 0x87,
> +	  0x84, 0x81, 0x98, 0x93, 0x85, 0xae, 0x9c, 0xa8, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xd8, 0x00, 0xde, 0x00, 0xd6, 0x86, 0x84, 0x83, 0x81, 0x81,
> +	  0x83, 0x85, 0x85, 0x85, 0x82, 0x83, 0x81, 0x81, 0x81, 0x83, 0x86,
> +	  0x84, 0x80, 0x98, 0x91, 0x85, 0x7b, 0x7b, 0x7c, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xdc, 0x00, 0xe2, 0x00, 0xda, 0x85, 0x84, 0x83, 0x82, 0x82,
> +	  0x84, 0x84, 0x84, 0x85, 0x81, 0x82, 0x82, 0x80, 0x80, 0x81, 0x83,
> +	  0x82, 0x7f, 0x99, 0x93, 0x86, 0x94, 0x8b, 0x92, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xdf, 0x00, 0xe5, 0x00, 0xde, 0x85, 0x84, 0x82, 0x82, 0x82,
> +	  0x84, 0x83, 0x83, 0x84, 0x81, 0x81, 0x80, 0x83, 0x82, 0x84, 0x82,
> +	  0x81, 0x7f, 0x99, 0x92, 0x86, 0x7b, 0x7b, 0x7c, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xe4, 0x00, 0xe9, 0x00, 0xe3, 0x84, 0x83, 0x82, 0x81, 0x81,
> +	  0x82, 0x83, 0x83, 0x84, 0x80, 0x81, 0x80, 0x83, 0x83, 0x84, 0x80,
> +	  0x81, 0x7c, 0x99, 0x92, 0x87, 0xa1, 0x93, 0x9d, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xe4, 0x00, 0xe9, 0x00, 0xe3, 0x85, 0x84, 0x83, 0x81, 0x81,
> +	  0x82, 0x82, 0x82, 0x83, 0x80, 0x81, 0x80, 0x81, 0x80, 0x82, 0x83,
> +	  0x82, 0x80, 0x91, 0x8d, 0x83, 0x9a, 0x90, 0x96, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xe4, 0x00, 0xe9, 0x00, 0xe3, 0x84, 0x83, 0x82, 0x81, 0x81,
> +	  0x82, 0x83, 0x83, 0x84, 0x80, 0x81, 0x80, 0x81, 0x80, 0x82, 0x83,
> +	  0x81, 0x7f, 0x91, 0x8c, 0x82, 0x8d, 0x88, 0x8b, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xe4, 0x00, 0xe9, 0x00, 0xe3, 0x84, 0x83, 0x82, 0x81, 0x81,
> +	  0x82, 0x83, 0x83, 0x83, 0x82, 0x82, 0x81, 0x81, 0x80, 0x82, 0x82,
> +	  0x82, 0x7f, 0x94, 0x89, 0x84, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xe4, 0x00, 0xe9, 0x00, 0xe3, 0x84, 0x83, 0x82, 0x81, 0x81,
> +	  0x82, 0x83, 0x83, 0x83, 0x82, 0x82, 0x81, 0x81, 0x80, 0x82, 0x83,
> +	  0x82, 0x7f, 0x91, 0x85, 0x81, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xe4, 0x00, 0xe9, 0x00, 0xe3, 0x84, 0x83, 0x82, 0x81, 0x81,
> +	  0x82, 0x83, 0x83, 0x83, 0x80, 0x80, 0x7f, 0x83, 0x82, 0x84, 0x83,
> +	  0x82, 0x7f, 0x90, 0x84, 0x81, 0x9a, 0x90, 0x96, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xe4, 0x00, 0xe9, 0x00, 0xe3, 0x84, 0x83, 0x82, 0x80, 0x80,
> +	  0x82, 0x83, 0x83, 0x83, 0x80, 0x80, 0x7f, 0x80, 0x80, 0x81, 0x81,
> +	  0x82, 0x83, 0x7e, 0x80, 0x7c, 0xa4, 0x97, 0x9f, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xe9, 0x00, 0xec, 0x00, 0xe8, 0x84, 0x83, 0x82, 0x81, 0x81,
> +	  0x82, 0x82, 0x82, 0x83, 0x7f, 0x7f, 0x7f, 0x81, 0x80, 0x82, 0x83,
> +	  0x83, 0x84, 0x79, 0x7c, 0x79, 0xb1, 0xa0, 0xaa, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xed, 0x00, 0xf0, 0x00, 0xec, 0x83, 0x83, 0x82, 0x80, 0x80,
> +	  0x81, 0x82, 0x82, 0x82, 0x7f, 0x7f, 0x7e, 0x81, 0x81, 0x82, 0x80,
> +	  0x81, 0x81, 0x84, 0x84, 0x83, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xf1, 0x00, 0xf4, 0x00, 0xf1, 0x83, 0x82, 0x82, 0x80, 0x80,
> +	  0x81, 0x82, 0x82, 0x82, 0x80, 0x80, 0x80, 0x80, 0x80, 0x81, 0x7d,
> +	  0x7e, 0x7f, 0x84, 0x84, 0x83, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xf6, 0x00, 0xf7, 0x00, 0xf5, 0x82, 0x82, 0x81, 0x80, 0x80,
> +	  0x80, 0x82, 0x82, 0x82, 0x80, 0x80, 0x80, 0x7f, 0x7f, 0x7f, 0x82,
> +	  0x82, 0x82, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x00, 0xfa, 0x00, 0xfb, 0x00, 0xfa, 0x81, 0x81, 0x81, 0x80, 0x80,
> +	  0x80, 0x82, 0x82, 0x82, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80,
> +	  0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x80, 0x80, 0x80, 0x80, 0x80,
> +	  0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80,
> +	  0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 },
> +	{ 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x80, 0x80, 0x80, 0x80, 0x80,
> +	  0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80,
> +	  0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00,
> +	  0x00, 0x00 }
> +};
> +
> +unsigned char VINT_TABLE[S6E3HA2_VINT_STATUS_MAX] = {

Lower case names.

> +	0x18, 0x19, 0x1a, 0x1b, 0x1c,
> +	0x1d, 0x1e, 0x1f, 0x20, 0x21
> +};
> +
> +struct s6e3ha2 {
> +	struct device *dev;
> +	struct drm_panel panel;
> +	struct backlight_device *bl_dev;
> +
> +	struct regulator_bulk_data supplies[2];
> +	struct gpio_desc *reset_gpio;
> +	struct gpio_desc *enable_gpio;
> +	u32 power_on_delay;
> +	u32 reset_delay;
> +	struct videomode vm;
> +
> +	/* This field is tested by functions directly accessing DSI bus before
> +	 * transfer, transfer is skipped if it is set. In case of transfer
> +	 * failure or unexpected response the field is set to error value.
> +	 * Such construct allows to eliminate many checks in higher level
> +	 * functions.
> +	 */
> +	int error;
> +};
> +
> +static int  s6e3ha2_clear_error(struct s6e3ha2 *ctx)
> +{
> +	int ret = ctx->error;
> +
> +	ctx->error = 0;
> +	return ret;
> +}
> +
> +static void s6e3ha2_dcs_write(struct s6e3ha2 *ctx, const void *data, size_t len)
> +{
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	ssize_t ret;
> +
> +	if (ctx->error < 0)
> +		return;
> +
> +	ret = mipi_dsi_dcs_write_buffer(dsi, data, len);
> +	if (ret < 0) {
> +		dev_err(ctx->dev, "error %zd writing dcs seq: %*ph\n",
> +						ret, (int)len, data);
> +		ctx->error = ret;
> +	}
> +}
> +
> +#define s6e3ha2_dcs_write_seq_static(ctx, seq...) do {	\
> +	static const u8 d[] = { seq };			\
> +	s6e3ha2_dcs_write(ctx, d, ARRAY_SIZE(d));	\
> +} while (0)
> +
> +static void s6e3ha2_test_key_on_f0(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xf0, 0x5a, 0x5a);
> +}
> +
> +static void s6e3ha2_test_key_off_f0(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xf0, 0xa5, 0xa5);
> +}
> +
> +static void s6e3ha2_test_key_on_fc(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xfc, 0x5a, 0x5a);
> +}
> +
> +static void s6e3ha2_test_key_off_fc(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xfc, 0xa5, 0xa5);
> +}
> +
> +static void s6e3ha2_single_dsi_set(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xf2, 0x67);
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xf9, 0x09);
> +}
> +
> +static void s6e3ha2_freq_calibration(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xfd, 0x1c);
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xfe, 0x20, 0x39);
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xfe, 0xa0);
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xfe, 0x20);
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xce, 0x03, 0x3b, 0x12, 0x62,
> +		0x40, 0x80, 0xc0, 0x28, 0x28, 0x28, 0x28, 0x39, 0xc5);
> +}
> +
> +static void s6e3ha2_aor_control(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xb2, 0x03, 0x10);
> +}
> +
> +static void s6e3ha2_caps_elvss_set(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xb6, 0x9c, 0x0a);
> +}
> +
> +static void s6e3ha2_acl_off(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0x55, 0x00);
> +}
> +
> +static void s6e3ha2_acl_off_opr(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xb5, 0x40);
> +}
> +
> +static void s6e3ha2_test_global(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xb0, 0x07);
> +}
> +
> +static void s6e3ha2_test(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xb8, 0x19);
> +}
> +
> +static void s6e3ha2_touch_hsync_on1(struct s6e3ha2 *ctx) {
> +	s6e3ha2_dcs_write_seq_static(ctx,
> +			0xbd, 0x33, 0x11, 0x02, 0x16, 0x02, 0x16);
> +}
> +
> +static void s6e3ha2_pentile_control(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xc0, 0x00, 0x00, 0xd8, 0xd8);
> +}
> +
> +static void s6e3ha2_poc_global(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xb0, 0x20);
> +}
> +
> +static void s6e3ha2_poc_setting(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xfe, 0x08);
> +}
> +
> +static void s6e3ha2_pcd_set_off(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xcc, 0x40, 0x51);
> +}
> +
> +static void s6e3ha2_err_fg_set(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xed, 0x44);
> +}
> +
> +static void s6e3ha2_hbm_off(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0x53, 0x00);
> +}
> +
> +static void s6e3ha2_te_start_setting(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xb9, 0x10, 0x09, 0xff, 0x00, 0x09);
> +}
> +
> +static void s6e3ha2_gamma_update(struct s6e3ha2 *ctx)
> +{
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xf7, 0x03);
> +	ndelay(100); /* need for 100ns delay */
> +	s6e3ha2_dcs_write_seq_static(ctx, 0xf7, 0x00);
> +}
> +
> +static int s6e3ha2_get_brightness(struct backlight_device *bl_dev)
> +{
> +	return bl_dev->props.brightness;
> +}
> +
> +static void s6e3ha2_set_vint(struct s6e3ha2 *ctx) {

Curly bracket should be on new line.

> +	struct backlight_device *bl_dev = ctx->bl_dev;
> +	unsigned int brightness = bl_dev->props.brightness;
> +	unsigned char data[] = { 0xf4, 0x8b,
> +			VINT_TABLE[brightness * (S6E3HA2_VINT_STATUS_MAX - 1) /
> +			S6E3HA2_MAX_BRIGHTNESS] };
> +
> +	s6e3ha2_dcs_write(ctx, data, 3);
> +}
> +
> +static unsigned int s6e3ha2_get_brightness_index(unsigned int brightness)
> +{
> +	return (brightness * (S6E3HA2_NUM_GAMMA_STEPS - 1)) /
> +		S6E3HA2_MAX_BRIGHTNESS;
> +}
> +
> +static int s6e3ha2_update_gamma(struct s6e3ha2 *ctx, unsigned int brightness)
> +{
> +	struct backlight_device *bl_dev = ctx->bl_dev;
> +	unsigned int index = s6e3ha2_get_brightness_index(brightness);
> +	u8 data[S6E3HA2_GAMMA_CMD_CNT + 1] = { 0xca, };
> +
> +	memcpy(data + 1, gamma_tbl + index, S6E3HA2_GAMMA_CMD_CNT);
> +	s6e3ha2_dcs_write(ctx, data, ARRAY_SIZE(data));
> +
> +	s6e3ha2_gamma_update(ctx);
> +	bl_dev->props.brightness = brightness;
> +
> +	return 0;
> +}
> +
> +static int s6e3ha2_set_brightness(struct backlight_device *bl_dev)
> +{
> +	struct s6e3ha2 *ctx = (struct s6e3ha2 *)bl_get_data(bl_dev);
> +	unsigned int brightness = bl_dev->props.brightness;
> +
> +	if (brightness < S6E3HA2_MIN_BRIGHTNESS ||
> +		brightness > bl_dev->props.max_brightness) {
> +		dev_err(ctx->dev, "Invalid brightness: %u\n", brightness);
> +		return -EINVAL;
> +	}
> +
> +	if (bl_dev->props.power > FB_BLANK_NORMAL) {
> +		dev_err(ctx->dev,
> +			"panel must be at least in fb blank normal state\n");

This message adds unnecessary noise to the logs, it should be definitely
removed.
I do not know how this callback should react here, it would be good to
verify it in documentation of backlight interface, or other drivers.

> +		return -EPERM;
> +	}
> +
> +	s6e3ha2_test_key_on_f0(ctx);
> +	s6e3ha2_update_gamma(ctx, brightness);
> +	s6e3ha2_aor_control(ctx);
> +	s6e3ha2_set_vint(ctx);
> +	s6e3ha2_test_key_off_f0(ctx);
> +
> +	return ctx->error;

I think 'return s6e3ha2_clear_error(ctx);' would be better here, but
this is just my suggestion.
By the way, backlight framework is not synchronized with drm framework,
so there are possible races here.

> +}
> +
> +static const struct backlight_ops s6e3ha2_bl_ops = {
> +	.get_brightness = s6e3ha2_get_brightness,
> +	.update_status = s6e3ha2_set_brightness,
> +};
> +
> +static void s6e3ha2_panel_init(struct s6e3ha2 *ctx)
> +{
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	u32 reset_delay = ctx->reset_delay * 1000;
> +
> +	mipi_dsi_dcs_exit_sleep_mode(dsi);
> +	usleep_range(reset_delay, reset_delay + 1000);
> +
> +	s6e3ha2_test_key_on_f0(ctx);
> +	s6e3ha2_single_dsi_set(ctx);
> +	s6e3ha2_test_key_on_fc(ctx);
> +	s6e3ha2_freq_calibration(ctx);
> +	s6e3ha2_test_key_off_fc(ctx);
> +	s6e3ha2_test_key_off_f0(ctx);
> +}
> +
> +static int s6e3ha2_power_off(struct s6e3ha2 *ctx)
> +{
> +	return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +}
> +
> +static int s6e3ha2_disable(struct drm_panel *panel)
> +{
> +	struct s6e3ha2 *ctx = container_of(panel, struct s6e3ha2, panel);
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +
> +	mipi_dsi_dcs_enter_sleep_mode(dsi);
> +	if (ctx->error != 0)
> +		goto err;
> +
> +	mipi_dsi_dcs_set_display_off(dsi);
> +	if (ctx->error != 0)
> +		goto err;
> +
> +	msleep(40);
> +	ctx->bl_dev->props.power = FB_BLANK_NORMAL;
> +
> +	return 0;
> +err:
> +	return ctx->error;
> +}
> +
> +static int s6e3ha2_unprepare(struct drm_panel *panel)
> +{
> +	struct s6e3ha2 *ctx = container_of(panel, struct s6e3ha2, panel);
> +	int ret;
> +
> +	ret = s6e3ha2_clear_error(ctx);
> +	if (!ret)
> +		ctx->bl_dev->props.power = FB_BLANK_POWERDOWN;
> +
> +	return s6e3ha2_power_off(ctx);
> +}
> +
> +static int s6e3ha2_power_on(struct s6e3ha2 *ctx)
> +{
> +	u32 reset_delay = ctx->reset_delay * 1000;
> +	int ret;
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		return ret;
> +
> +	msleep(ctx->power_on_delay);
> +
> +	gpiod_set_value(ctx->enable_gpio, 0);
> +	usleep_range(reset_delay, reset_delay + 1000);
> +	gpiod_set_value(ctx->enable_gpio, 1);
> +
> +	gpiod_set_value(ctx->reset_gpio, 0);
> +	usleep_range(reset_delay, reset_delay + 1000);
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +	usleep_range(reset_delay, reset_delay + 1000);
> +
> +	return 0;
> +}
> +static int s6e3ha2_prepare(struct drm_panel *panel)
> +{
> +	struct s6e3ha2 *ctx = container_of(panel, struct s6e3ha2, panel);
> +	int ret;
> +
> +	ret = s6e3ha2_power_on(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	s6e3ha2_panel_init(ctx);
> +
> +	ret = s6e3ha2_clear_error(ctx);
> +	if (ret < 0) {
> +		s6e3ha2_power_off(ctx);
> +		return ret;
> +	}
> +
> +	ctx->bl_dev->props.power = FB_BLANK_NORMAL;
> +
> +	return 0;
> +}
> +
> +static int s6e3ha2_enable(struct drm_panel *panel)
> +{
> +	struct s6e3ha2 *ctx = container_of(panel, struct s6e3ha2, panel);
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +
> +	/* common setting */
> +	mipi_dsi_dcs_set_tear_on(dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> +
> +	s6e3ha2_test_key_on_f0(ctx);
> +	s6e3ha2_test_key_on_fc(ctx);
> +	s6e3ha2_touch_hsync_on1(ctx);
> +	s6e3ha2_pentile_control(ctx);
> +	s6e3ha2_poc_global(ctx);
> +	s6e3ha2_poc_setting(ctx);
> +	s6e3ha2_test_key_off_fc(ctx);
> +
> +	/* pcd setting off for TB */
> +	s6e3ha2_pcd_set_off(ctx);
> +	s6e3ha2_err_fg_set(ctx);
> +	s6e3ha2_te_start_setting(ctx);
> +
> +	/* brightness setting */
> +	s6e3ha2_set_brightness(ctx->bl_dev);
> +	s6e3ha2_aor_control(ctx);
> +	s6e3ha2_caps_elvss_set(ctx);
> +	s6e3ha2_gamma_update(ctx);
> +	s6e3ha2_acl_off(ctx);
> +	s6e3ha2_acl_off_opr(ctx);
> +	s6e3ha2_hbm_off(ctx);
> +
> +	/* elvss temp compensation */
> +	s6e3ha2_test_global(ctx);
> +	s6e3ha2_test(ctx);
> +	s6e3ha2_test_key_off_f0(ctx);
> +
> +	mipi_dsi_dcs_set_display_on(dsi);
> +	if (ctx->error != 0)
> +		return ctx->error;
> +
> +	ctx->bl_dev->props.power = FB_BLANK_UNBLANK;
> +
> +	return 0;
> +}
> +
> +static int s6e3ha2_get_modes(struct drm_panel *panel)
> +{
> +	struct drm_connector *connector = panel->connector;
> +	struct s6e3ha2 *ctx = container_of(panel, struct s6e3ha2, panel);
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_create(connector->dev);
> +	if (!mode) {
> +		DRM_ERROR("failed to create a new display mode\n");
> +		return 0;
> +	}
> +
> +	drm_display_mode_from_videomode(&ctx->vm, mode);
> +	mode->vrefresh = 60;
> +	mode->width_mm = 71;
> +	mode->height_mm = 125;
> +	connector->display_info.width_mm = mode->width_mm;
> +	connector->display_info.height_mm = mode->height_mm;
> +
> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +	drm_mode_probed_add(connector, mode);
> +
> +	return 1;
> +}
> +
> +static const struct drm_panel_funcs s6e3ha2_drm_funcs = {
> +	.disable = s6e3ha2_disable,
> +	.unprepare = s6e3ha2_unprepare,
> +	.prepare = s6e3ha2_prepare,
> +	.enable = s6e3ha2_enable,
> +	.get_modes = s6e3ha2_get_modes,
> +};
> +
> +static int s6e3ha2_parse_dt(struct s6e3ha2 *ctx)
> +{
> +	struct device *dev = ctx->dev;
> +	struct device_node *np = dev->of_node;
> +	int ret;
> +
> +	ret = of_get_videomode(dev->of_node, &ctx->vm, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	of_property_read_u32(np, "power-on-delay", &ctx->power_on_delay);
> +	of_property_read_u32(np, "reset-delay", &ctx->reset_delay);

These props are not described in bindings, anyway I think these
properties similarly to display-timings are panel specific, so they
should be hardcoded into the driver.

> +
> +	return 0;
> +}
> +
> +static int s6e3ha2_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct s6e3ha2 *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +
> +	ctx->dev = dev;
> +
> +	dsi->lanes = 4;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS;
> +
> +	ret = s6e3ha2_parse_dt(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ctx->supplies[0].supply = "vdd3";
> +	ctx->supplies[1].supply = "vci";
> +
> +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
> +				      ctx->supplies);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to get regulators: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(ctx->reset_gpio)) {
> +		dev_err(dev, "cannot get reset-gpios %ld\n",
> +			PTR_ERR(ctx->reset_gpio));
> +		return PTR_ERR(ctx->reset_gpio);
> +	}
> +
> +	ctx->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH);
> +	if (IS_ERR(ctx->enable_gpio)) {
> +		dev_err(dev, "cannot get enable-gpios %ld\n",
> +			PTR_ERR(ctx->enable_gpio));
> +		return PTR_ERR(ctx->enable_gpio);
> +	}
> +
> +	ctx->bl_dev = backlight_device_register("s6e3ha2", dev, ctx,
> +						&s6e3ha2_bl_ops, NULL);
> +	if (IS_ERR(ctx->bl_dev)) {
> +		dev_err(dev, "failed to register backlight device\n");
> +		return PTR_ERR(ctx->bl_dev);
> +	}
> +
> +	ctx->bl_dev->props.max_brightness = S6E3HA2_MAX_BRIGHTNESS;
> +	ctx->bl_dev->props.brightness = S6E3HA2_DEFAULT_BRIGHTNESS;
> +	ctx->bl_dev->props.power = FB_BLANK_POWERDOWN;
> +
> +	drm_panel_init(&ctx->panel);
> +	ctx->panel.dev = dev;
> +	ctx->panel.funcs = &s6e3ha2_drm_funcs;
> +
> +	ret = drm_panel_add(&ctx->panel);
> +	if (ret < 0)
> +		goto unregister_backlight;
> +
> +	ret = mipi_dsi_attach(dsi);
> +	if (ret < 0)
> +		goto remove_panel;
> +
> +	return ret;
> +
> +remove_panel:
> +	drm_panel_remove(&ctx->panel);
> +
> +unregister_backlight:
> +	backlight_device_unregister(ctx->bl_dev);
> +
> +	return ret;
> +}
> +
> +static int s6e3ha2_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct s6e3ha2 *ctx = mipi_dsi_get_drvdata(dsi);
> +
> +	mipi_dsi_detach(dsi);
> +	drm_panel_remove(&ctx->panel);
> +	backlight_device_unregister(ctx->bl_dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id s6e3ha2_of_match[] = {
> +	{ .compatible = "samsung,s6e3ha2" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, s6e3ha2_of_match);
> +
> +static struct mipi_dsi_driver s6e3ha2_driver = {
> +	.probe = s6e3ha2_probe,
> +	.remove = s6e3ha2_remove,
> +	.driver = {
> +		.name = "panel-samsung-s6e3ha2",
> +		.of_match_table = s6e3ha2_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(s6e3ha2_driver);
> +
> +MODULE_AUTHOR("Donghwa Lee <dh09.lee-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>");
> +MODULE_AUTHOR("Hyungwon Hwang <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>");
> +MODULE_AUTHOR("Hoegeun Kwon <hoegeun.kwon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>");
> +MODULE_DESCRIPTION("MIPI-DSI based s6e3ha2 AMOLED Panel Driver");
> +MODULE_LICENSE("GPL v2");


--
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 v2 05/12] Document: dt: binding: imx: update pinctrl doc for imx6sll
From: Linus Walleij @ 2016-12-27 12:59 UTC (permalink / raw)
  To: Bai Ping
  Cc: Shawn Guo, Michael Turquette, Stephen Boyd, Rob Herring,
	Mark Rutland, Sascha Hauer, Fabio Estevam, Daniel Lezcano,
	Thomas Gleixner, Philipp Zabel, linux-clk,
	devicetree@vger.kernel.org, linux-gpio@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, jacky.baip
In-Reply-To: <1482832070-22668-6-git-send-email-ping.bai@nxp.com>

On Tue, Dec 27, 2016 at 10:47 AM, Bai Ping <ping.bai@nxp.com> wrote:

> Add pinctrl binding doc update for imx6sll.
>
> Signed-off-by: Bai Ping <ping.bai@nxp.com>

I have to push back on this a bit.

> +Please refer to fsl,imx-pinctrl.txt in this directory for common binding part
> +and usage.

I understand that it is building on top of the old i.MX bindings and that
it has some kind of "tradition" coming with it.

At the same time, the i.MX bindings came about before we had the
generic pin control bindings defined.

> +CONFIG bits definition:
> +PAD_CTL_LVE                    (1 << 22)
> +PAD_CTL_HYS                     (1 << 16)
> +PAD_CTL_PUS_100K_DOWN           (0 << 14)
> +PAD_CTL_PUS_47K_UP              (1 << 14)
> +PAD_CTL_PUS_100K_UP             (2 << 14)
> +PAD_CTL_PUS_22K_UP              (3 << 14)
> +PAD_CTL_PUE                     (1 << 13)
> +PAD_CTL_PKE                     (1 << 12)
> +PAD_CTL_ODE                     (1 << 11)
> +PAD_CTL_SPEED_LOW               (0 << 6)
> +PAD_CTL_SPEED_MED               (1 << 6)
> +PAD_CTL_SPEED_HIGH              (3 << 6)
> +PAD_CTL_DSE_DISABLE             (0 << 3)
> +PAD_CTL_DSE_260ohm              (1 << 3)
> +PAD_CTL_DSE_130ohm              (2 << 3)
> +PAD_CTL_DSE_87ohm               (3 << 3)
> +PAD_CTL_DSE_65ohm               (4 << 3)
> +PAD_CTL_DSE_52ohm               (5 << 3)
> +PAD_CTL_DSE_43ohm               (6 << 3)
> +PAD_CTL_DSE_37ohm               (7 << 3)
> +PAD_CTL_SRE_FAST                (1 << 0)
> +PAD_CTL_SRE_SLOW                (0 << 0)

A whole slew of these if not all correspond to the generic bindings.

I would consider augmenting the code in the driver to handle the generic
bindings *in addition* to the old legacy bindings, and use those over these
random custom bits.

Read drivers using CONFIG_GENERIC_PINCONF as an inspiration.

For example see commit
cefbf1a1b29531a970bc2908a50a75d6474fcc38
"pinctrl: sunxi: Support generic binding"
from Maxime Ripard, where he does a similar thing for sunxi.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v2 06/12] driver: pinctrl: imx: Add pinctrl driver support for imx6sll
From: Linus Walleij @ 2016-12-27 13:01 UTC (permalink / raw)
  To: Bai Ping
  Cc: Mark Rutland, devicetree@vger.kernel.org, Philipp Zabel,
	Michael Turquette, Daniel Lezcano, Stephen Boyd,
	linux-gpio@vger.kernel.org, Rob Herring, Sascha Hauer, jacky.baip,
	Fabio Estevam, Thomas Gleixner, Shawn Guo, linux-clk,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <1482832070-22668-7-git-send-email-ping.bai@nxp.com>

On Tue, Dec 27, 2016 at 10:47 AM, Bai Ping <ping.bai@nxp.com> wrote:

> Add pinctrl driver support for imx6sll.
>
> Signed-off-by: Bai Ping <ping.bai@nxp.com>
(...)
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -514,6 +514,7 @@ config SOC_IMX6SL
>
>  config SOC_IMX6SLL
>         bool "i.MX6 SoloLiteLite support"
> +       select PINCTRL_IMX6SLL

This needs to be separate so I can merge the pure pin control patch
to my tree.

Apart from that, look into using generic pinconf.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v7 0/2] Add support for Omnivision OV5647
From: Ramiro Oliveira @ 2016-12-27 13:59 UTC (permalink / raw)
  To: mchehab-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	linux-0h96xk9xTtrk1uMJSBkQmQ, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
	dheitmueller-eb9eJ82Ua7k9XoPSrs7Ehg,
	slongerbeam-Re5JQEeQqe8AvxtiuMwx3w, lars-Qo5EllUWu/uELgA04lAiVw,
	robert.jarzmik-GANU6spQydw, pavel-+ZI9xUNit7I,
	pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
	sakari.ailus-VuQAYsv1563Yd54FQh9/CA, mark.rutland-5wv7dgnIgG8,
	Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w,
	CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w

Hello,

This patch adds support for the Omnivision OV5647 sensor.

At the moment it only supports 640x480 in Raw 8.

This is the seventh version of the OV5647 camera driver patchset.

v7:
 - Remove "0x" and leading 0 from DT documentation examples

v6:
 - Add example to DT documentation
 - Remove data-lanes and clock-lane property from DT
 - Add external clock property to DT
 - Order includes
 - Remove unused variables and functions
 - Add external clock handling
 - Add power on counter
 - Change from g/s_parm to g/s_frame_interval

v5:
 - Refactor code 
 - Change comments
 - Add missing error handling in some functions

v4: 
 - Add correct license
 - Revert debugging info to generic infrastructure
 - Turn defines into enums
 - Correct code style issues
 - Remove unused defines
 - Make sure all errors where being handled
 - Rename some functions to make code more readable
 - Add some debugging info

v3: 
 - No changes. Re-submitted due to lack of responses

v2: 
 - Corrections in DT documentation

Ramiro Oliveira (2):
  Add OV5647 device tree documentation
  Add support for OV5647 sensor.

 .../devicetree/bindings/media/i2c/ov5647.txt       |  35 +
 MAINTAINERS                                        |   7 +
 drivers/media/i2c/Kconfig                          |  12 +
 drivers/media/i2c/Makefile                         |   1 +
 drivers/media/i2c/ov5647.c                         | 718 +++++++++++++++++++++
 5 files changed, 773 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
 create mode 100644 drivers/media/i2c/ov5647.c

-- 
2.11.0


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

* [PATCH v7 1/2] Add OV5647 device tree documentation
From: Ramiro Oliveira @ 2016-12-27 13:59 UTC (permalink / raw)
  To: mchehab, linux-kernel, linux-media, robh+dt, devicetree
  Cc: davem, gregkh, geert+renesas, akpm, linux, hverkuil, dheitmueller,
	slongerbeam, lars, robert.jarzmik, pavel, pali.rohar,
	sakari.ailus, mark.rutland, Ramiro.Oliveira, CARLOS.PALMINHA
In-Reply-To: <cover.1482846784.git.roliveir@synopsys.com>

Create device tree bindings documentation.

Signed-off-by: Ramiro Oliveira <roliveir@synopsys.com>
---
 .../devicetree/bindings/media/i2c/ov5647.txt       | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.txt b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
new file mode 100644
index 000000000000..57fd40036c26
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
@@ -0,0 +1,35 @@
+Omnivision OV5647 raw image sensor
+---------------------------------
+
+OV5647 is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces
+and CCI (I2C compatible) control bus.
+
+Required properties:
+
+- compatible		: "ovti,ov5647".
+- reg			: I2C slave address of the sensor.
+- clocks		: Reference to the xclk clock.
+- clock-names		: Should be "xclk".
+- clock-frequency	: Frequency of the xclk clock.
+
+The common video interfaces bindings (see video-interfaces.txt) should be
+used to specify link to the image data receiver. The OV5647 device
+node should contain one 'port' child node with an 'endpoint' subnode.
+
+Example:
+
+	i2c@2000 {
+		...
+		ov: camera@36 {
+			compatible = "ovti,ov5647";
+			reg = <0x36>;
+			clocks = <&camera_clk>;
+			clock-names = "xclk";
+			clock-frequency = <30000000>;
+			port {
+				camera_1: endpoint {
+					remote-endpoint = <&csi1_ep1>;
+				};
+			};
+		};
+	};
-- 
2.11.0

^ permalink raw reply related

* [PATCH v7 2/2] Add support for OV5647 sensor.
From: Ramiro Oliveira @ 2016-12-27 13:59 UTC (permalink / raw)
  To: mchehab, linux-kernel, linux-media, robh+dt, devicetree
  Cc: davem, gregkh, geert+renesas, akpm, linux, hverkuil, dheitmueller,
	slongerbeam, lars, robert.jarzmik, pavel, pali.rohar,
	sakari.ailus, mark.rutland, Ramiro.Oliveira, CARLOS.PALMINHA
In-Reply-To: <cover.1482846784.git.roliveir@synopsys.com>

Modes supported:
 - 640x480 RAW 8

Signed-off-by: Ramiro Oliveira <roliveir@synopsys.com>
---
 MAINTAINERS                |   7 +
 drivers/media/i2c/Kconfig  |  12 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/ov5647.c | 718 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 738 insertions(+)
 create mode 100644 drivers/media/i2c/ov5647.c

diff --git a/MAINTAINERS b/MAINTAINERS
index cfff2c9e3d94..346bd8d55703 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9095,6 +9095,13 @@ M:	Harald Welte <laforge@gnumonks.org>
 S:	Maintained
 F:	drivers/char/pcmcia/cm4040_cs.*
 
+OMNIVISION OV5647 SENSOR DRIVER
+M:	Ramiro Oliveira <roliveir@synopsys.com>
+L:	linux-media@vger.kernel.org
+T:	git git://linuxtv.org/media_tree.git
+S:	Maintained
+F:	drivers/media/i2c/ov5647.c
+
 OMNIVISION OV7670 SENSOR DRIVER
 M:	Jonathan Corbet <corbet@lwn.net>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index b979ea148251..155878b25566 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -531,6 +531,18 @@ config VIDEO_OV2659
 	  To compile this driver as a module, choose M here: the
 	  module will be called ov2659.
 
+config VIDEO_OV5647
+	tristate "OmniVision OV5647 sensor support"
+	depends on OF
+	depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+	depends on MEDIA_CAMERA_SUPPORT
+	---help---
+	  This is a Video4Linux2 sensor-level driver for the OmniVision
+	  OV5647 camera.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ov5647.
+
 config VIDEO_OV7640
 	tristate "OmniVision OV7640 sensor support"
 	depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 92773b2e6225..0d9014c7ab43 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -82,3 +82,4 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)	+= ml86v7667.o
 obj-$(CONFIG_VIDEO_OV2659)	+= ov2659.o
 obj-$(CONFIG_VIDEO_TC358743)	+= tc358743.o
+obj-$(CONFIG_VIDEO_OV5647)	+= ov5647.o
diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
new file mode 100644
index 000000000000..c2828650d3a3
--- /dev/null
+++ b/drivers/media/i2c/ov5647.c
@@ -0,0 +1,718 @@
+/*
+ * A V4L2 driver for OmniVision OV5647 cameras.
+ *
+ * Based on Samsung S5K6AAFX SXGA 1/6" 1.3M CMOS Image Sensor driver
+ * Copyright (C) 2011 Sylwester Nawrocki <s.nawrocki@samsung.com>
+ *
+ * Based on Omnivision OV7670 Camera Driver
+ * Copyright (C) 2006-7 Jonathan Corbet <corbet@lwn.net>
+ *
+ * Copyright (C) 2016, Synopsys, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed .as is. WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/videodev2.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-image-sizes.h>
+#include <media/v4l2-mediabus.h>
+#include <media/v4l2-of.h>
+
+#define SENSOR_NAME "ov5647"
+
+#define OV5647_SW_RESET		0x1003
+#define OV5647_REG_CHIPID_H	0x300A
+#define OV5647_REG_CHIPID_L	0x300B
+
+#define REG_TERM 0xfffe
+#define VAL_TERM 0xfe
+#define REG_DLY  0xffff
+
+#define OV5647_ROW_START		0x01
+#define OV5647_ROW_START_MIN		0
+#define OV5647_ROW_START_MAX		2004
+#define OV5647_ROW_START_DEF		54
+
+#define OV5647_COLUMN_START		0x02
+#define OV5647_COLUMN_START_MIN		0
+#define OV5647_COLUMN_START_MAX		2750
+#define OV5647_COLUMN_START_DEF		16
+
+#define OV5647_WINDOW_HEIGHT		0x03
+#define OV5647_WINDOW_HEIGHT_MIN	2
+#define OV5647_WINDOW_HEIGHT_MAX	2006
+#define OV5647_WINDOW_HEIGHT_DEF	1944
+
+#define OV5647_WINDOW_WIDTH		0x04
+#define OV5647_WINDOW_WIDTH_MIN		2
+#define OV5647_WINDOW_WIDTH_MAX		2752
+#define OV5647_WINDOW_WIDTH_DEF		2592
+
+struct regval_list {
+	u16 addr;
+	u8 data;
+};
+
+struct cfg_array {
+	struct regval_list *regs;
+	int size;
+};
+
+struct ov5647 {
+	struct device			*dev;
+	struct v4l2_subdev		sd;
+	struct media_pad		pad;
+	struct mutex			lock;
+	struct v4l2_mbus_framefmt	format;
+	unsigned int			width;
+	unsigned int			height;
+	int				power_count;
+	struct clk			*xclk;
+	/* External clock frequency currently supported is 30MHz */
+	u32				xclk_freq;
+};
+
+static inline struct ov5647 *to_state(struct v4l2_subdev *sd)
+{
+	return container_of(sd, struct ov5647, sd);
+}
+
+static struct regval_list sensor_oe_disable_regs[] = {
+	{0x3000, 0x00},
+	{0x3001, 0x00},
+	{0x3002, 0x00},
+};
+
+static struct regval_list sensor_oe_enable_regs[] = {
+	{0x3000, 0x0f},
+	{0x3001, 0xff},
+	{0x3002, 0xe4},
+};
+
+static struct regval_list ov5647_640x480[] = {
+	{0x0100, 0x00},
+	{0x0103, 0x01},
+	{0x3034, 0x08},
+	{0x3035, 0x21},
+	{0x3036, 0x46},
+	{0x303c, 0x11},
+	{0x3106, 0xf5},
+	{0x3821, 0x07},
+	{0x3820, 0x41},
+	{0x3827, 0xec},
+	{0x370c, 0x0f},
+	{0x3612, 0x59},
+	{0x3618, 0x00},
+	{0x5000, 0x06},
+	{0x5001, 0x01},
+	{0x5002, 0x41},
+	{0x5003, 0x08},
+	{0x5a00, 0x08},
+	{0x3000, 0x00},
+	{0x3001, 0x00},
+	{0x3002, 0x00},
+	{0x3016, 0x08},
+	{0x3017, 0xe0},
+	{0x3018, 0x44},
+	{0x301c, 0xf8},
+	{0x301d, 0xf0},
+	{0x3a18, 0x00},
+	{0x3a19, 0xf8},
+	{0x3c01, 0x80},
+	{0x3b07, 0x0c},
+	{0x380c, 0x07},
+	{0x380d, 0x68},
+	{0x380e, 0x03},
+	{0x380f, 0xd8},
+	{0x3814, 0x31},
+	{0x3815, 0x31},
+	{0x3708, 0x64},
+	{0x3709, 0x52},
+	{0x3808, 0x02},
+	{0x3809, 0x80},
+	{0x380a, 0x01},
+	{0x380b, 0xE0},
+	{0x3801, 0x00},
+	{0x3802, 0x00},
+	{0x3803, 0x00},
+	{0x3804, 0x0a},
+	{0x3805, 0x3f},
+	{0x3806, 0x07},
+	{0x3807, 0xa1},
+	{0x3811, 0x08},
+	{0x3813, 0x02},
+	{0x3630, 0x2e},
+	{0x3632, 0xe2},
+	{0x3633, 0x23},
+	{0x3634, 0x44},
+	{0x3636, 0x06},
+	{0x3620, 0x64},
+	{0x3621, 0xe0},
+	{0x3600, 0x37},
+	{0x3704, 0xa0},
+	{0x3703, 0x5a},
+	{0x3715, 0x78},
+	{0x3717, 0x01},
+	{0x3731, 0x02},
+	{0x370b, 0x60},
+	{0x3705, 0x1a},
+	{0x3f05, 0x02},
+	{0x3f06, 0x10},
+	{0x3f01, 0x0a},
+	{0x3a08, 0x01},
+	{0x3a09, 0x27},
+	{0x3a0a, 0x00},
+	{0x3a0b, 0xf6},
+	{0x3a0d, 0x04},
+	{0x3a0e, 0x03},
+	{0x3a0f, 0x58},
+	{0x3a10, 0x50},
+	{0x3a1b, 0x58},
+	{0x3a1e, 0x50},
+	{0x3a11, 0x60},
+	{0x3a1f, 0x28},
+	{0x4001, 0x02},
+	{0x4004, 0x02},
+	{0x4000, 0x09},
+	{0x4837, 0x24},
+	{0x4050, 0x6e},
+	{0x4051, 0x8f},
+	{0x0100, 0x01},
+};
+
+/**
+ * @short I2C Write operation
+ * @param[in] i2c_client I2C client
+ * @param[in] reg register address
+ * @param[in] val value to write
+ * @return Error code
+ */
+static int ov5647_write(struct v4l2_subdev *sd, u16 reg, u8 val)
+{
+	int ret;
+	unsigned char data[3] = { reg >> 8, reg & 0xff, val};
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+	ret = i2c_master_send(client, data, 3);
+	if (ret != 3) {
+		dev_dbg(&client->dev, "%s: i2c write error, reg: %x\n",
+				__func__, reg);
+		return ret < 0 ? ret : -EIO;
+	}
+	return 0;
+}
+
+/**
+ * @short I2C Read operation
+ * @param[in] i2c_client I2C client
+ * @param[in] reg register address
+ * @param[out] val value read
+ * @return Error code
+ */
+static int ov5647_read(struct v4l2_subdev *sd, u16 reg, u8 *val)
+{
+	int ret;
+	unsigned char data_w[2] = { reg >> 8, reg & 0xff };
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+
+	ret = i2c_master_send(client, data_w, 2);
+
+	if (ret < 2) {
+		dev_dbg(&client->dev, "%s: i2c read error, reg: %x\n",
+			__func__, reg);
+		return ret < 0 ? ret : -EIO;
+	}
+
+
+	ret = i2c_master_recv(client, val, 1);
+
+	if (ret < 1) {
+		dev_dbg(&client->dev, "%s: i2c read error, reg: %x\n",
+				__func__, reg);
+		return ret < 0 ? ret : -EIO;
+	}
+	return 0;
+}
+
+static int ov5647_write_array(struct v4l2_subdev *sd,
+				struct regval_list *regs, int array_size)
+{
+	int i = 0;
+	int ret = 0;
+
+	if (!regs)
+		return -EINVAL;
+
+	while (i < array_size) {
+		ret = ov5647_write(sd, regs->addr, regs->data);
+		if (ret < 0)
+			return ret;
+		i++;
+		regs++;
+	}
+	return 0;
+}
+
+static void ov5647_set_virtual_channel(struct v4l2_subdev *sd, int channel)
+{
+	u8 channel_id;
+
+	ov5647_read(sd, 0x4814, &channel_id);
+	channel_id &= ~(3 << 6);
+	ov5647_write(sd, 0x4814, channel_id | (channel << 6));
+}
+
+void ov5647_stream_on(struct v4l2_subdev *sd)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+	ov5647_write(sd, 0x4202, 0x00);
+	dev_dbg(&client->dev, "Stream on");
+	ov5647_write(sd, 0x300D, 0x00);
+}
+
+void ov5647_stream_off(struct v4l2_subdev *sd)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+	ov5647_write(sd, 0x4202, 0x0f);
+	dev_dbg(&client->dev, "Stream off");
+	ov5647_write(sd, 0x300D, 0x01);
+}
+
+/**
+ * @short Set SW standby
+ * @param[in] sd v4l2 sd
+ * @param[in] stanby standby mode status (on or off)
+ * @return Error code
+ */
+static int set_sw_standby(struct v4l2_subdev *sd, bool standby)
+{
+	int ret;
+	unsigned char rdval;
+
+	ret = ov5647_read(sd, 0x0100, &rdval);
+	if (ret != 0)
+		return ret;
+
+	if (standby)
+		rdval &= 0xfe;
+	else
+		rdval |= 0x01;
+
+	ret = ov5647_write(sd, 0x0100, rdval);
+
+	return ret;
+}
+
+/**
+ * @short Initialize sensor
+ * @param[in] sd v4l2 subdev
+ * @param[in] val not used
+ * @return Error code
+ */
+static int __sensor_init(struct v4l2_subdev *sd)
+{
+	int ret;
+	u8 resetval;
+	u8 rdval;
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+	dev_dbg(&client->dev, "sensor init\n");
+
+	ret = ov5647_read(sd, 0x0100, &rdval);
+	if (ret != 0)
+		return ret;
+
+	ov5647_write(sd, 0x4800, 0x25);
+	ov5647_stream_off(sd);
+
+	ret = ov5647_write_array(sd, ov5647_640x480,
+					ARRAY_SIZE(ov5647_640x480));
+	if (ret < 0) {
+		dev_err(&client->dev, "write sensor_default_regs error\n");
+		return ret;
+	}
+
+	ov5647_set_virtual_channel(sd, 0);
+
+	ov5647_read(sd, 0x0100, &resetval);
+	if (!(resetval & 0x01)) {
+		dev_err(&client->dev, "Device was in SW standby");
+		ov5647_write(sd, 0x0100, 0x01);
+	}
+
+	ov5647_write(sd, 0x4800, 0x04);
+	ov5647_stream_on(sd);
+
+	return 0;
+}
+
+/**
+ * @short Control sensor power state
+ * @param[in] sd v4l2 subdev
+ * @param[in] on Sensor power
+ * @return Error code
+ */
+static int sensor_power(struct v4l2_subdev *sd, int on)
+{
+	int ret;
+	struct ov5647 *ov5647 = to_state(sd);
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+	ret = 0;
+	mutex_lock(&ov5647->lock);
+
+	if (on && !ov5647->power_count)	{
+		dev_dbg(&client->dev, "OV5647 power on\n");
+
+		clk_set_rate(ov5647->xclk, ov5647->xclk_freq);
+
+		ret = clk_prepare_enable(ov5647->xclk);
+		if (ret < 0) {
+			dev_err(ov5647->dev, "clk prepare enable failed\n");
+			goto out;
+		}
+
+		ret = ov5647_write_array(sd, sensor_oe_enable_regs,
+				ARRAY_SIZE(sensor_oe_enable_regs));
+		if (ret < 0) {
+			clk_disable_unprepare(ov5647->xclk);
+			dev_err(&client->dev,
+				"write sensor_oe_enable_regs error\n");
+			goto out;
+		}
+
+		ret = __sensor_init(sd);
+		if (ret < 0) {
+			clk_disable_unprepare(ov5647->xclk);
+			dev_err(&client->dev,
+				"Camera not available, check Power\n");
+			goto out;
+		}
+	} else if (!on && ov5647->power_count == 1) {
+		dev_dbg(&client->dev, "OV5647 power off\n");
+
+		dev_dbg(&client->dev, "disable oe\n");
+		ret = ov5647_write_array(sd, sensor_oe_disable_regs,
+				ARRAY_SIZE(sensor_oe_disable_regs));
+
+		if (ret < 0)
+			dev_dbg(&client->dev, "disable oe failed\n");
+
+		ret = set_sw_standby(sd, true);
+
+		if (ret < 0)
+			dev_dbg(&client->dev, "soft stby failed\n");
+
+		clk_disable_unprepare(ov5647->xclk);
+	}
+
+	/* Update the power count. */
+	ov5647->power_count += on ? 1 : -1;
+	WARN_ON(ov5647->power_count < 0);
+
+out:
+	mutex_unlock(&ov5647->lock);
+
+	return ret;
+}
+
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+/**
+ * @short Get register value
+ * @param[in] sd v4l2 subdev
+ * @param[in] reg register struct
+ * @return Error code
+ */
+static int sensor_get_register(struct v4l2_subdev *sd,
+				struct v4l2_dbg_register *reg)
+{
+	unsigned char val = 0;
+	int ret;
+
+	ret = ov5647_read(sd, reg->reg & 0xff, &val);
+	if (ret != 0)
+		return ret;
+
+	reg->val = val;
+	reg->size = 1;
+
+	return ret;
+}
+
+/**
+ * @short Set register value
+ * @param[in] sd v4l2 subdev
+ * @param[in] reg register struct
+ * @return Error code
+ */
+static int sensor_set_register(struct v4l2_subdev *sd,
+				const struct v4l2_dbg_register *reg)
+{
+	return ov5647_write(sd, reg->reg & 0xff, reg->val & 0xff);
+}
+#endif
+
+/**
+ * @short Subdev core operations registration
+ */
+static const struct v4l2_subdev_core_ops sensor_core_ops = {
+	.s_power		= sensor_power,
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+	.g_register		= sensor_get_register,
+	.s_register		= sensor_set_register,
+#endif
+};
+
+static int enum_mbus_code(struct v4l2_subdev *sd,
+				struct v4l2_subdev_pad_config *cfg,
+				struct v4l2_subdev_mbus_code_enum *code)
+{
+	if (code->index > 0)
+		return -EINVAL;
+
+	code->code = MEDIA_BUS_FMT_SBGGR8_1X8;
+
+	return 0;
+}
+
+static const struct v4l2_subdev_pad_ops subdev_pad_ops = {
+	.enum_mbus_code = enum_mbus_code,
+};
+
+
+/**
+ * @short Subdev operations registration
+ *
+ */
+static const struct v4l2_subdev_ops subdev_ops = {
+	.core		= &sensor_core_ops,
+	.pad		= &subdev_pad_ops,
+};
+
+/**
+ * @short Detect camera version and model
+ * @param[in] sd v4l2 subdev
+ * @return Error code
+ */
+static int ov5647_detect(struct v4l2_subdev *sd)
+{
+	unsigned char v;
+	int ret;
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+	ret = ov5647_write(sd, OV5647_SW_RESET, 0x01);
+	if (ret < 0)
+		return ret;
+	ret = ov5647_read(sd, OV5647_REG_CHIPID_H, &v);
+	if (ret < 0)
+		return ret;
+	if (v != 0x56) {
+		dev_err(&client->dev, "Wrong model version detected");
+		return -ENODEV;
+	}
+	ret = ov5647_read(sd, OV5647_REG_CHIPID_L, &v);
+	if (ret < 0)
+		return ret;
+	if (v != 0x47) {
+		dev_err(&client->dev, "Wrong model version detected");
+		return -ENODEV;
+	}
+
+	ret = ov5647_write(sd, OV5647_SW_RESET, 0x00);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+/**
+ * @short Detect if camera is registered
+ * @param[in] sd v4l2 subdev
+ * @return Error code
+ */
+static int ov5647_registered(struct v4l2_subdev *sd)
+{
+	return 0;
+}
+
+/**
+ * @short Open device
+ * @param[in] sd v4l2 subdev
+ * @param[in] fh v4l2 file handler
+ * @return Error code
+ */
+static int ov5647_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	struct v4l2_mbus_framefmt *format =
+				v4l2_subdev_get_try_format(sd, fh->pad, 0);
+	struct v4l2_rect *crop =
+				v4l2_subdev_get_try_crop(sd, fh->pad, 0);
+
+	crop->left = OV5647_COLUMN_START_DEF;
+	crop->top = OV5647_ROW_START_DEF;
+	crop->width = OV5647_WINDOW_WIDTH_DEF;
+	crop->height = OV5647_WINDOW_HEIGHT_DEF;
+
+	format->code = MEDIA_BUS_FMT_SBGGR8_1X8;
+
+	format->width = OV5647_WINDOW_WIDTH_DEF;
+	format->height = OV5647_WINDOW_HEIGHT_DEF;
+	format->field = V4L2_FIELD_NONE;
+	format->colorspace = V4L2_COLORSPACE_SRGB;
+
+	return sensor_power(sd, true);
+}
+
+/**
+ * @short Open device
+ * @param[in] sd v4l2 subdev
+ * @param[in] fh v4l2 file handler
+ * @return Error code
+ */
+static int ov5647_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	return sensor_power(sd, false);
+}
+
+/**
+ * @short Subdev internal operations registration
+ *
+ */
+static const struct v4l2_subdev_internal_ops ov5647_subdev_internal_ops = {
+	.registered = ov5647_registered,
+	.open = ov5647_open,
+	.close = ov5647_close,
+};
+
+/**
+ * @short Initialization routine - Entry point of the driver
+ * @param[in] client pointer to the i2c client structure
+ * @param[in] id pointer to the i2c device id structure
+ * @return 0 on success and a negative number on failure
+ */
+static int ov5647_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct ov5647 *sensor;
+	int ret;
+	struct v4l2_subdev *sd;
+
+	dev_info(&client->dev, "Installing OmniVision OV5647 camera driver\n");
+
+	sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
+	if (sensor == NULL)
+		return -ENOMEM;
+
+	/* get system clock (xclk) */
+	sensor->xclk = devm_clk_get(dev, "xclk");
+	if (IS_ERR(sensor->xclk)) {
+		dev_err(dev, "could not get xclk");
+		return PTR_ERR(sensor->xclk);
+	}
+
+	ret = of_property_read_u32(dev->of_node, "clock-frequency",
+				    &sensor->xclk_freq);
+	if (ret) {
+		dev_err(dev, "could not get xclk frequency\n");
+		return ret;
+	}
+
+	mutex_init(&sensor->lock);
+	sensor->dev = dev;
+
+	sd = &sensor->sd;
+	v4l2_i2c_subdev_init(sd, client, &subdev_ops);
+	sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+
+	sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
+	sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
+	ret = media_entity_pads_init(&sd->entity, 1, &sensor->pad);
+	if (ret < 0)
+		goto mutex_remove;
+
+	ret = ov5647_detect(sd);
+	if (ret < 0)
+		goto error;
+
+	ret = v4l2_async_register_subdev(sd);
+	if (ret < 0)
+		goto error;
+
+	return 0;
+error:
+	media_entity_cleanup(&sd->entity);
+mutex_remove:
+	mutex_destroy(&sensor->lock);
+	return ret;
+}
+
+/**
+ * @short Exit routine - Exit point of the driver
+ * @param[in] client pointer to the i2c client structure
+ * @return 0 on success and a negative number on failure
+ */
+static int ov5647_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct ov5647 *ov5647 = to_state(sd);
+
+	v4l2_async_unregister_subdev(&ov5647->sd);
+	media_entity_cleanup(&ov5647->sd.entity);
+	v4l2_device_unregister_subdev(sd);
+	mutex_destroy(&ov5647->lock);
+
+	return 0;
+}
+
+static const struct i2c_device_id ov5647_id[] = {
+	{ "ov5647", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ov5647_id);
+
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id ov5647_of_match[] = {
+	{ .compatible = "ovti,ov5647" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ov5647_of_match);
+#endif
+
+/**
+ * @short i2c driver structure
+ */
+static struct i2c_driver ov5647_driver = {
+	.driver = {
+		.of_match_table = of_match_ptr(ov5647_of_match),
+		.owner	= THIS_MODULE,
+		.name	= "ov5647",
+	},
+	.probe		= ov5647_probe,
+	.remove		= ov5647_remove,
+	.id_table	= ov5647_id,
+};
+
+module_i2c_driver(ov5647_driver);
+
+MODULE_AUTHOR("Ramiro Oliveira <roliveir@synopsys.com>");
+MODULE_DESCRIPTION("A low-level driver for OmniVision ov5647 sensors");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH v4 4/4] clk: rockchip: add clock controller for rk3328
From: Heiko Stuebner @ 2016-12-27 14:05 UTC (permalink / raw)
  To: Elaine Zhang
  Cc: mark.rutland-5wv7dgnIgG8, huangtao-TNX95d0MmH7DzftRWevZcw,
	xf-TNX95d0MmH7DzftRWevZcw, mturquette-rdvid1DuHRBWk0Htik3J/w,
	sboyd-sgV2jX0FEOL9JmXXK+q4OQ, xxx-TNX95d0MmH7DzftRWevZcw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-clk-u79uwXL29TY76Z2rM5mHXA, cl-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <1482820373-10186-5-git-send-email-zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

Hi Elaine,

Am Dienstag, 27. Dezember 2016, 14:32:53 CET schrieb Elaine Zhang:
> Add the clock tree definition for the new rk3328 SoC.

looks good in general, I have some styling nitpicks below and would like
the grf-clocks solved differently, also explained below:


> diff --git a/drivers/clk/rockchip/clk-rk3328.c
> b/drivers/clk/rockchip/clk-rk3328.c new file mode 100644
> index 000000000000..9958ce7d0dcd
> --- /dev/null
> +++ b/drivers/clk/rockchip/clk-rk3328.c
> +static struct rockchip_clk_branch rk3328_clk_branches[] __initdata = {

[...]

> +	/*
> +	 * Clock-Architecture Diagram 8
> +	 */

blank line between two comment blocks please.
Same for similar setups in the code above.

> +	/* PD_GMAC */
> +
> +	COMPOSITE(ACLK_GMAC, "aclk_gmac", mux_2plls_hdmiphy_p, 0,
> +		  RK3328_CLKSEL_CON(35), 6, 2, MFLAGS, 0, 5, DFLAGS,
> +		  RK3328_CLKGATE_CON(3), 2, GFLAGS),
> +	COMPOSITE_NOMUX(PCLK_GMAC, "pclk_gmac", "aclk_gmac", 0,
> +			RK3328_CLKSEL_CON(25), 8, 3, DFLAGS,
> +			RK3328_CLKGATE_CON(9), 0, GFLAGS),
> +
> +	COMPOSITE(SCLK_MAC2IO_SRC, "clk_mac2io_src", mux_2plls_p, 0,
> +		  RK3328_CLKSEL_CON(27), 7, 1, MFLAGS, 0, 5, DFLAGS,
> +		  RK3328_CLKGATE_CON(3), 1, GFLAGS),
> +	GATE(SCLK_MAC2IO_REF, "clk_mac2io_ref", "clk_mac2io", 0,
> +	     RK3328_CLKGATE_CON(9), 7, GFLAGS),
> +	GATE(SCLK_MAC2IO_RX, "clk_mac2io_rx", "clk_mac2io", 0,
> +	     RK3328_CLKGATE_CON(9), 4, GFLAGS),
> +	GATE(SCLK_MAC2IO_TX, "clk_mac2io_tx", "clk_mac2io", 0,
> +	     RK3328_CLKGATE_CON(9), 5, GFLAGS),
> +	GATE(SCLK_MAC2IO_REFOUT, "clk_mac2io_refout", "clk_mac2io", 0,
> +	     RK3328_CLKGATE_CON(9), 6, GFLAGS),
> +	COMPOSITE(SCLK_MAC2IO_OUT, "clk_mac2io_out", mux_2plls_p, 0,
> +		  RK3328_CLKSEL_CON(27), 15, 1, MFLAGS, 8, 5, DFLAGS,
> +		  RK3328_CLKGATE_CON(3), 5, GFLAGS),
> +
> +	COMPOSITE(SCLK_MAC2PHY_SRC, "clk_mac2phy_src", mux_2plls_p, 0,
> +		  RK3328_CLKSEL_CON(26), 7, 1, MFLAGS, 0, 5, DFLAGS,
> +		  RK3328_CLKGATE_CON(3), 0, GFLAGS),
> +	GATE(SCLK_MAC2PHY_REF, "clk_mac2phy_ref", "clk_mac2phy", 0,
> +	     RK3328_CLKGATE_CON(9), 3, GFLAGS),
> +	GATE(SCLK_MAC2PHY_RXTX, "clk_mac2phy_rxtx", "clk_mac2phy", 0,
> +	     RK3328_CLKGATE_CON(9), 1, GFLAGS),
> +	COMPOSITE_NOMUX(SCLK_MAC2PHY_OUT, "clk_mac2phy_out", "clk_mac2phy", 0,
> +			RK3328_CLKSEL_CON(26), 8, 2, DFLAGS,
> +			RK3328_CLKGATE_CON(9), 2, GFLAGS),

Please look at the other clock drivers for indentation. I.e. don't align with 
the "(", but instead use the same indent everywhere.

This makes things like the register address always sit in the same position
and thus makes it easier to read the clock diagram.


> +	FACTOR(0, "xin12m", "xin24m", 0, 1, 2),
> +
> +	/*
> +	 * Clock-Architecture Diagram 9
> +	 */
> +

For the following long list of gates, just make it one line per gate and do 
not line-break them ... see other clock drivers for reference.

This long gate list is very uniform (= only gates) and reducing the number
of lines needed makes it easier to parse for those.

> +	/* PD_VOP */
> +	GATE(ACLK_RGA, "aclk_rga", "aclk_rga_pre", 0,
> +	     RK3328_CLKGATE_CON(21), 10, GFLAGS),
> +	GATE(0, "aclk_rga_niu", "aclk_rga_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(22), 3, GFLAGS),
> +	GATE(ACLK_VOP, "aclk_vop", "aclk_vop_pre", 0,
> +	     RK3328_CLKGATE_CON(21), 2, GFLAGS),
> +	GATE(0, "aclk_vop_niu", "aclk_vop_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(21), 4, GFLAGS),
> +
> +	GATE(ACLK_IEP, "aclk_iep", "aclk_vio_pre", 0,
> +	     RK3328_CLKGATE_CON(21), 6, GFLAGS),
> +	GATE(ACLK_CIF, "aclk_cif", "aclk_vio_pre", 0,
> +	     RK3328_CLKGATE_CON(21), 8, GFLAGS),
> +	GATE(ACLK_HDCP, "aclk_hdcp", "aclk_vio_pre", 0,
> +	     RK3328_CLKGATE_CON(21), 15, GFLAGS),
> +	GATE(0, "aclk_vio_niu", "aclk_vio_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(22), 2, GFLAGS),
> +
> +	GATE(HCLK_VOP, "hclk_vop", "hclk_vio_pre", 0,
> +	     RK3328_CLKGATE_CON(21), 3, GFLAGS),
> +	GATE(0, "hclk_vop_niu", "hclk_vio_pre", 0,
> +	     RK3328_CLKGATE_CON(21), 5, GFLAGS),
> +	GATE(HCLK_IEP, "hclk_iep", "hclk_vio_pre", 0,
> +	     RK3328_CLKGATE_CON(21), 7, GFLAGS),
> +	GATE(HCLK_CIF, "hclk_cif", "hclk_vio_pre", 0,
> +	     RK3328_CLKGATE_CON(21), 9, GFLAGS),
> +	GATE(HCLK_RGA, "hclk_rga", "hclk_vio_pre", 0,
> +	     RK3328_CLKGATE_CON(21), 11, GFLAGS),
> +	GATE(0, "hclk_ahb1tom", "hclk_vio_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(21), 12, GFLAGS),
> +	GATE(0, "pclk_vio_h2p", "hclk_vio_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(21), 13, GFLAGS),
> +	GATE(0, "hclk_vio_h2p", "hclk_vio_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(21), 14, GFLAGS),
> +	GATE(HCLK_HDCP, "hclk_hdcp", "hclk_vio_pre", 0,
> +	     RK3328_CLKGATE_CON(22), 0, GFLAGS),
> +	GATE(HCLK_VIO, "hclk_vio", "hclk_vio_pre", 0,
> +	     RK3328_CLKGATE_CON(22), 1, GFLAGS),
> +	GATE(PCLK_HDMI, "pclk_hdmi", "hclk_vio_pre", 0,
> +	     RK3328_CLKGATE_CON(22), 4, GFLAGS),
> +	GATE(PCLK_HDCP, "pclk_hdcp", "hclk_vio_pre", 0,
> +	     RK3328_CLKGATE_CON(22), 5, GFLAGS),
> +
> +	/* PD_PERI */
> +	GATE(0, "aclk_peri_noc", "aclk_peri", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(19), 11, GFLAGS),
> +	GATE(ACLK_USB3OTG, "aclk_usb3otg", "aclk_peri", 0,
> +	     RK3328_CLKGATE_CON(19), 4, GFLAGS),
> +
> +	GATE(HCLK_SDMMC, "hclk_sdmmc", "hclk_peri", 0,
> +	     RK3328_CLKGATE_CON(19), 0, GFLAGS),
> +	GATE(HCLK_SDIO, "hclk_sdio", "hclk_peri", 0,
> +	     RK3328_CLKGATE_CON(19), 1, GFLAGS),
> +	GATE(HCLK_EMMC, "hclk_emmc", "hclk_peri", 0,
> +	     RK3328_CLKGATE_CON(19), 2, GFLAGS),
> +	GATE(HCLK_SDMMC_EXT, "hclk_sdmmc_ext", "hclk_peri", 0,
> +	     RK3328_CLKGATE_CON(19), 15, GFLAGS),
> +	GATE(HCLK_HOST0, "hclk_host0", "hclk_peri", 0,
> +	     RK3328_CLKGATE_CON(19), 6, GFLAGS),
> +	GATE(HCLK_HOST0_ARB, "hclk_host0_arb", "hclk_peri", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(19), 7, GFLAGS),
> +	GATE(HCLK_OTG, "hclk_otg", "hclk_peri", 0,
> +	     RK3328_CLKGATE_CON(19), 8, GFLAGS),
> +	GATE(HCLK_OTG_PMU, "hclk_otg_pmu", "hclk_peri", 0,
> +	     RK3328_CLKGATE_CON(19), 9, GFLAGS),
> +	GATE(0, "hclk_peri_niu", "hclk_peri", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(19), 12, GFLAGS),
> +	GATE(0, "pclk_peri_niu", "hclk_peri", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(19), 13, GFLAGS),
> +
> +	/* PD_GMAC */
> +	GATE(ACLK_MAC2PHY, "aclk_mac2phy", "aclk_gmac", 0,
> +	     RK3328_CLKGATE_CON(26), 0, GFLAGS),
> +	GATE(ACLK_MAC2IO, "aclk_mac2io", "aclk_gmac", 0,
> +	     RK3328_CLKGATE_CON(26), 2, GFLAGS),
> +	GATE(0, "aclk_gmac_niu", "aclk_gmac", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(26), 4, GFLAGS),
> +	GATE(PCLK_MAC2PHY, "pclk_mac2phy", "pclk_gmac", 0,
> +	     RK3328_CLKGATE_CON(26), 1, GFLAGS),
> +	GATE(PCLK_MAC2IO, "pclk_mac2io", "pclk_gmac", 0,
> +	     RK3328_CLKGATE_CON(26), 3, GFLAGS),
> +	GATE(0, "pclk_gmac_niu", "pclk_gmac", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(26), 5, GFLAGS),
> +
> +	/* PD_BUS */
> +	GATE(0, "aclk_bus_niu", "aclk_bus_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(15), 12, GFLAGS),
> +	GATE(ACLK_DCF, "aclk_dcf", "aclk_bus_pre", 0,
> +	     RK3328_CLKGATE_CON(15), 11, GFLAGS),
> +	GATE(ACLK_TSP, "aclk_tsp", "aclk_bus_pre", 0,
> +	     RK3328_CLKGATE_CON(17), 12, GFLAGS),
> +	GATE(0, "aclk_intmem", "aclk_bus_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(15), 0, GFLAGS),
> +	GATE(ACLK_DMAC, "aclk_dmac_bus", "aclk_bus_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(15), 1, GFLAGS),
> +
> +	GATE(0, "hclk_rom", "hclk_bus_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(15), 2, GFLAGS),
> +	GATE(HCLK_I2S0_8CH, "hclk_i2s0_8ch", "hclk_bus_pre", 0,
> +	     RK3328_CLKGATE_CON(15), 3, GFLAGS),
> +	GATE(HCLK_I2S1_8CH, "hclk_i2s1_8ch", "hclk_bus_pre", 0,
> +	     RK3328_CLKGATE_CON(15), 4, GFLAGS),
> +	GATE(HCLK_I2S2_2CH, "hclk_i2s2_2ch", "hclk_bus_pre", 0,
> +	     RK3328_CLKGATE_CON(15), 5, GFLAGS),
> +	GATE(HCLK_SPDIF_8CH, "hclk_spdif_8ch", "hclk_bus_pre", 0,
> +	     RK3328_CLKGATE_CON(15), 6, GFLAGS),
> +	GATE(HCLK_TSP, "hclk_tsp", "hclk_bus_pre", 0,
> +	     RK3328_CLKGATE_CON(17), 11, GFLAGS),
> +	GATE(HCLK_CRYPTO_MST, "hclk_crypto_mst", "hclk_bus_pre", 0,
> +	     RK3328_CLKGATE_CON(15), 7, GFLAGS),
> +	GATE(HCLK_CRYPTO_SLV, "hclk_crypto_slv", "hclk_bus_pre", 0,
> +	     RK3328_CLKGATE_CON(15), 8, GFLAGS),
> +	GATE(0, "hclk_bus_niu", "hclk_bus_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(15), 13, GFLAGS),
> +	GATE(HCLK_PDM, "hclk_pdm", "hclk_bus_pre", 0,
> +	     RK3328_CLKGATE_CON(28), 0, GFLAGS),
> +
> +	GATE(0, "pclk_bus_niu", "pclk_bus", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(15), 14, GFLAGS),
> +	GATE(0, "pclk_efuse", "pclk_bus", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(15), 9, GFLAGS),
> +	GATE(0, "pclk_otp", "pclk_bus", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(28), 4, GFLAGS),
> +	GATE(PCLK_I2C0, "pclk_i2c0", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(15), 10, GFLAGS),
> +	GATE(PCLK_I2C1, "pclk_i2c1", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 0, GFLAGS),
> +	GATE(PCLK_I2C2, "pclk_i2c2", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 1, GFLAGS),
> +	GATE(PCLK_I2C3, "pclk_i2c3", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 2, GFLAGS),
> +	GATE(PCLK_TIMER, "pclk_timer0", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 3, GFLAGS),
> +	GATE(0, "pclk_stimer", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 4, GFLAGS),
> +	GATE(PCLK_SPI, "pclk_spi", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 5, GFLAGS),
> +	GATE(PCLK_PWM, "pclk_rk_pwm", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 6, GFLAGS),
> +	GATE(PCLK_GPIO0, "pclk_gpio0", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 7, GFLAGS),
> +	GATE(PCLK_GPIO1, "pclk_gpio1", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 8, GFLAGS),
> +	GATE(PCLK_GPIO2, "pclk_gpio2", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 9, GFLAGS),
> +	GATE(PCLK_GPIO3, "pclk_gpio3", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 10, GFLAGS),
> +	GATE(PCLK_UART0, "pclk_uart0", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 11, GFLAGS),
> +	GATE(PCLK_UART1, "pclk_uart1", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 12, GFLAGS),
> +	GATE(PCLK_UART2, "pclk_uart2", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 13, GFLAGS),
> +	GATE(PCLK_TSADC, "pclk_tsadc", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 14, GFLAGS),
> +	GATE(PCLK_DCF, "pclk_dcf", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(16), 15, GFLAGS),
> +	GATE(PCLK_GRF, "pclk_grf", "pclk_bus", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(17), 0, GFLAGS),
> +	GATE(0, "pclk_cru", "pclk_bus", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(17), 4, GFLAGS),
> +	GATE(0, "pclk_sgrf", "pclk_bus", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(17), 6, GFLAGS),
> +	GATE(0, "pclk_sim", "pclk_bus", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(17), 10, GFLAGS),
> +	GATE(PCLK_SARADC, "pclk_saradc", "pclk_bus", 0,
> +	     RK3328_CLKGATE_CON(17), 15, GFLAGS),
> +	GATE(0, "pclk_pmu", "pclk_bus", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(28), 3, GFLAGS),
> +
> +	GATE(PCLK_USB3PHY_OTG, "pclk_usb3phy_otg", "pclk_phy_pre", 0,
> +	     RK3328_CLKGATE_CON(28), 1, GFLAGS),
> +	GATE(PCLK_USB3PHY_PIPE, "pclk_usb3phy_pipe", "pclk_phy_pre", 0,
> +	     RK3328_CLKGATE_CON(28), 2, GFLAGS),
> +	GATE(PCLK_USB3_GRF, "pclk_usb3_grf", "pclk_phy_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(17), 2, GFLAGS),
> +	GATE(PCLK_USB2_GRF, "pclk_usb2_grf", "pclk_phy_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(17), 14, GFLAGS),
> +	GATE(0, "pclk_ddrphy", "pclk_phy_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(17), 13, GFLAGS),
> +	GATE(0, "pclk_acodecphy", "pclk_phy_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(17), 5, GFLAGS),
> +	GATE(PCLK_HDMIPHY, "pclk_hdmiphy", "pclk_phy_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(17), 7, GFLAGS),
> +	GATE(0, "pclk_vdacphy", "pclk_phy_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(17), 8, GFLAGS),
> +	GATE(0, "pclk_phy_niu", "pclk_phy_pre", CLK_IGNORE_UNUSED,
> +	     RK3328_CLKGATE_CON(15), 15, GFLAGS),




> +static void __init rk3328_clk_init(struct device_node *np)
> +{
> +	struct rockchip_clk_provider *ctx;
> +	void __iomem *reg_base;
> +
> +	reg_base = of_iomap(np, 0);
> +	if (!reg_base) {
> +		pr_err("%s: could not map cru region\n", __func__);
> +		return;
> +	}
> +
> +	rk3328_cru_base = reg_base;
> +
> +	ctx = rockchip_clk_init(np, reg_base, CLK_NR_CLKS);
> +	if (IS_ERR(ctx)) {
> +		pr_err("%s: rockchip clk init failed\n", __func__);
> +		iounmap(reg_base);
> +		return;
> +	}
> +
> +	rockchip_clk_register_plls(ctx, rk3328_pll_clks,
> +				   ARRAY_SIZE(rk3328_pll_clks),
> +				   RK3328_GRF_SOC_STATUS0);
> +	rockchip_clk_register_branches(ctx, rk3328_clk_branches,
> +				       ARRAY_SIZE(rk3328_clk_branches));
> +	rockchip_clk_protect_critical(rk3328_critical_clocks,
> +				      ARRAY_SIZE(rk3328_critical_clocks));
> +
> +	rockchip_clk_register_armclk(ctx, ARMCLK, "armclk",
> +				     mux_armclk_p, ARRAY_SIZE(mux_armclk_p),
> +				     &rk3328_cpuclk_data, rk3328_cpuclk_rates,
> +				     ARRAY_SIZE(rk3328_cpuclk_rates));
> +
> +	rockchip_register_softrst(np, 11, reg_base + RK3328_SOFTRST_CON(0),
> +				  ROCKCHIP_SOFTRST_HIWORD_MASK);
> +
> +	rockchip_register_restart_notifier(ctx, RK3328_GLB_SRST_FST, NULL);
> +
> +	rockchip_clk_of_add_provider(np, ctx);
> +
> +	atomic_notifier_chain_register(&panic_notifier_list,
> +				       &rk3328_clk_panic_block);

please drop this notifier and asorted code above for dumping cru registers.


> +}
> +
> +CLK_OF_DECLARE(rk3328_cru, "rockchip,rk3328-cru", rk3328_clk_init);
> +
> +static void __init rk3328_grf_clk_init(struct device_node *np)
> +{
> +	struct rockchip_clk_provider *ctx;
> +	void __iomem *reg_base;
> +
> +	reg_base = of_iomap(np, 0);
> +	if (!reg_base) {
> +		pr_err("%s: could not map cru pmu region\n", __func__);
> +		return;
> +	}
> +
> +	ctx = rockchip_clk_init(np, reg_base, CLKGRF_NR_CLKS);
> +	if (IS_ERR(ctx)) {
> +		pr_err("%s: rockchip pmu clk init failed\n", __func__);
> +		return;
> +	}
> +
> +	rockchip_clk_register_branches(ctx, rk3328_clk_grf_branches,
> +				       ARRAY_SIZE(rk3328_clk_grf_branches));
> +
> +	rockchip_clk_of_add_provider(np, ctx);
> +}

We definitly don't want to bind against the regular GRF node. This causes a 
double mapping of the region and in general this isn't the clock-controller 
and just contains 2 clocks controls that somehow ended up in the GRF :-) .

Instead please take a look at the muxgrf clock patches I Cc'ed you on some 
minutes ago.


> +CLK_OF_DECLARE(rk3328_cru_grf, "rockchip,rk3328-grf", rk3328_grf_clk_init);
> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h index
> 06acb7e0911f..7225997f8d52 100644
> --- a/drivers/clk/rockchip/clk.h
> +++ b/drivers/clk/rockchip/clk.h
> @@ -91,6 +91,28 @@
>  #define RK3288_EMMC_CON0		0x218
>  #define RK3288_EMMC_CON1		0x21c
> 
> +#define RK3328_PLL_CON(x)		RK2928_PLL_CON(x)
> +#define RK3328_CLKSEL_CON(x)		((x) * 0x4 + 0x100)
> +#define RK3328_CLKGATE_CON(x)		((x) * 0x4 + 0x200)
> +#define RK3328_GRFCLKSEL_CON(x)		((x) * 0x4 + 0x100)
> +#define RK3328_GLB_SRST_FST		0x9c
> +#define RK3328_GLB_SRST_SND		0x98
> +#define RK3328_SOFTRST_CON(x)		((x) * 0x4 + 0x300)
> +#define RK3328_MODE_CON			0x80
> +#define RK3328_MISC_CON			0x84
-----
> +#define RK3328_DIV_ACLKM_MASK		0x7
> +#define RK3328_DIV_ACLKM_SHIFT		4
> +#define RK3328_DIV_PCLK_DBG_MASK	0xf
> +#define RK3328_DIV_PCLK_DBG_SHIFT	0
-----
please move these to the armclk defintion in the controller driver
where already the other definitions are


Thanks
Heiko

^ permalink raw reply

* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Geoff Lansberry @ 2016-12-27 14:18 UTC (permalink / raw)
  To: Mark Greer
  Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Justin Bronder, Jaret Cantu
In-Reply-To: <20161224172439.GA15103-luAo+O/VEmrlveNOaEYElw@public.gmane.org>

Mark - I will split this off soon.

In the meantime - here is some more info about how we use it.

We do use NFC structures.    I did find an interesting clue in that
there are certain bottles that cause neard to segfault,  I'm not sure
what is different about them.  We write a string, like
"coppola_chardonnay_2015" to the bottles.  Come to think of it, I
haven't done anything special to make that an ndef record, just
assumed that it would happen by default, I'll look into this further.
  Also, I've been running neard with --plugin nfctype2. Just in case
the problem was happening due to cycling through other tag types.   It
didn't seem to make any difference, but I have not gone back to
default.

Geoff
Geoff Lansberry


Engineering Guy
Kuvée, Inc
125 Kingston St., 3rd Floor
Boston, MA 02111
1-617-290-1118 (m)
geoff.lansberry (skype)
http://www.kuvee.com



On Sat, Dec 24, 2016 at 12:24 PM, Mark Greer <mgreer-luAo+O/VEmrlveNOaEYElw@public.gmane.org> wrote:
> On Sat, Dec 24, 2016 at 11:17:18AM -0500, Geoff Lansberry wrote:
>> Mark - I'm sorry, but I did not write this code, and therefore was not
>> able to accurately describe it.   It is fixing a different issue, not
>> the neard segfault that we are still chasing. Last week Jaret Cantu
>> sent a separate email explaining the purpose of the code, which had
>> you copied, did you see that?
>
> Hm, no, I didn't.  I received an email from Justin Bronder but not from
> Jaret Cantu.  Justin's email did help but is still pretty high-level.
> We need a clear understanding as to what is happening in the digital
> layer and the driver to know how execution is getting into a block of
> error handling code that should never be executed.  Once we understand
> that we can start thinking about what the best fix is.
>
>> Does it explain why it was done to
>> your satisfaction?   I've asked him to join in on the effort to push
>> the change upstream, however he will not be available until the new
>> year.
>
> I expect that it would help if he joins.  After the holidays is fine -
> I think many people are taking it easy for the next week or so, anyway.
>
>> I know you did suggest that we split off that change from the others,
>> and if now is the time to do that, let me know.   If you don't have
>> the email from Jaret, also please let me know and I will forward it to
>> you.
>
> I think it would help you if you split it off because the first two patches
> have a good chance of being accepted but this one doesn't (yet).  If you
> separate the them, it will make it easier for Samuel to take the first two
> (or he may take the first two anyway but its always good to make it as
> easy maintainers as you can).
>
> Mark
> --

^ permalink raw reply

* Re: [PATCH 7/9] pinctrl: samsung: Add property to mark pad state as suitable for power down
From: Krzysztof Kozlowski @ 2016-12-27 15:33 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Krzysztof Kozlowski, linux-gpio, linux-arm-kernel, linux-pm,
	linux-samsung-soc, Sylwester Nawrocki, Linus Walleij, Tomasz Figa,
	Ulf Hansson, Bartlomiej Zolnierkiewicz, devicetree
In-Reply-To: <bd48fe32-5296-fc23-dbb8-d212292d76cd@samsung.com>

On Tue, Dec 27, 2016 at 11:30:57AM +0100, Marek Szyprowski wrote:
> Hi Krzysztof,
> 
> 
> On 2016-12-25 20:19, Krzysztof Kozlowski wrote:
> > On Fri, Dec 23, 2016 at 01:24:47PM +0100, Marek Szyprowski wrote:
> > > Add support for special property "samsung,off-state", which indicates a special
> > > state suitable for device's "sleep" state. Its pin values/properties should
> > > match the configuration in power down mode. It indicates that pin controller
> > > can notify runtime power management subsystem, that it is ready for runtime
> > > suspend if its all pins are configured for such state. This in turn might
> > > allow to turn respective power domain off to reduce power consumption.
> > > 
> > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > ---
> > >   Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt | 8 ++++++++
> > >   drivers/pinctrl/samsung/pinctrl-samsung.c                     | 4 ++++
> > >   drivers/pinctrl/samsung/pinctrl-samsung.h                     | 1 +
> > >   3 files changed, 13 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> > > index b7bd2e12a269..354eea0e7798 100644
> > > --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> > > +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> > > @@ -105,6 +105,7 @@ Required Properties:
> > >     - samsung,pin-drv: Drive strength configuration.
> > >     - samsung,pin-pud-pdn: Pull up/down configuration in power down mode.
> > >     - samsung,pin-drv-pdn: Drive strength configuration in power down mode.
> > > +  - samsung,off-state: Mark this configuration as suitable for bank power off.
> > >     The values specified by these config properties should be derived from the
> > >     hardware manual and these values are programmed as-is into the pin
> > > @@ -113,6 +114,13 @@ Required Properties:
> > >     Note: A child should include atleast a pin function selection property or
> > >     pin configuration property (one or more) or both.
> > > +  Note: Special property "samsung,off-state" indicates that this state can
> > > +  be used for device's "sleep" pins state. Its pin values/properties should
> > > +  match the configuration in power down mode.
> > Why power down values cannot be used for sleep state? Why you need
> > separate pin control state? If pins values should match power down
> > configuration, then they could just be added to default state, couldn't
> > they?
> 
> Separate sleep state is needed because of the pin control bindings and
> design.
> 
> A separate sleep state is required to let pin control client driver (in this
> case
> Exynos I2S driver) let to choose when it is okay to switch pads into sleep
> state and I see no other way to achieve this.

Maybe the pinctrl API should be extended for this? Doing this in DTS
just for purpose of passing information between drivers (consumer and
provider) looks odd.

Anyway, you are proposing a new binding so please Cc devicetree mailing
list and device tree maintainers.

> > In the patch 2/9, existing configuration:
> > 716         i2s0_bus: i2s0-bus {
> > (...)
> > 719                 samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
> > 720                 samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
> > 721                 samsung,pin-drv = <EXYNOS5420_PIN_DRV_LV1>;
> > 722         };
> > 
> > additional configuration:
> > +       i2s0_bus_slp: i2s0-bus-slp {
> > +               samsung,pin-function = <EXYNOS_PIN_FUNC_INPUT>;
> > +               samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
> > +               samsung,pin-drv = <EXYNOS5420_PIN_DRV_LV1>;
> > +               samsung,pin-con-pdn = <EXYNOS_PIN_PDN_INPUT>;
> > +               samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
> > +               samsung,off-state;
> > +       };
> 
> I agree that it would be possible to get rid of "samsung,off-state" property
> and
> just detect off state when func/pud pair matches power down func/pud.
> 
> > > It indicates that pin control
> > > +  can notify runtime power management subsystem, that it is ready for runtime
> > > +  suspend if its all pins are configured for such state. This in turn might
> > > +  allow to turn respective power domain off to reduce power consumption.
> > What do you mean by "notifying RPM subsystem"? Either this is
> > description of hardware in certain mode (sleep state) or this is not
> > device tree property.
> 
> Maybe I wrote the description with a view too limited to the kernel
> operation
> perspective, but if we remove the need to mark state as off, the above
> description
> will not be needed.

But still it wouldn't be describing any hardware property, would it?

Best regards,
Krzysztof

^ permalink raw reply

* [PATCH] ARM, ARM64: dts: drop "arm, amba-bus" in favor of "simple-bus" part 3
From: Masahiro Yamada @ 2016-12-27 15:51 UTC (permalink / raw)
  To: arm
  Cc: David Brown, Rob Herring, linux-samsung-soc, Rob Herring,
	Arnd Bergmann, devicetree, linux-arm-msm, Will Deacon,
	linux-kernel, Krzysztof Kozlowski, Russell King, Masahiro Yamada,
	Kukjin Kim, linux-arm-kernel, Catalin Marinas, Olof Johansson,
	Mark Rutland, Andy Gross, linux-soc, Javier Martinez Canillas

Tree-wide replacement was done by commit 2ef7d5f342c1 ("ARM, ARM64:
dts: drop "arm,amba-bus" in favor of "simple-bus"), then the 2nd
round by commit 15b7cc78f095 ("arm64: dts: drop "arm,amba-bus" in
favor of "simple-bus" part 2").

Here, some new users have appeared for Linux v4.10-rc1.  Eliminate
them now.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Hi Arnd, Olof,

Can you pick this up for v4.10 fixes?

If we carry arm,amba-bus until the release, we will need to
take more time to deprecate it.


 arch/arm/boot/dts/qcom-mdm9615.dtsi        | 2 +-
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-mdm9615.dtsi b/arch/arm/boot/dts/qcom-mdm9615.dtsi
index 5ae4ec5..c852b69 100644
--- a/arch/arm/boot/dts/qcom-mdm9615.dtsi
+++ b/arch/arm/boot/dts/qcom-mdm9615.dtsi
@@ -357,7 +357,7 @@
 		};
 
 		amba {
-			compatible = "arm,amba-bus";
+			compatible = "simple-bus";
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges;
diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 64226d5..135890c 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -1367,7 +1367,7 @@
 		};
 
 		amba {
-			compatible = "arm,amba-bus";
+			compatible = "simple-bus";
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges;
-- 
2.7.4

^ permalink raw reply related

* Re: [RFC PATCH 3/6] ARM64: dts: exynos5433: add the pcie_phy node for PCIe
From: Krzysztof Kozlowski @ 2016-12-27 16:11 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
	robh+dt, mark.rutland, kgene, krzk, javier, kishon, will.deacon,
	catalin.marinas, cpgs
In-Reply-To: <20161226052029.10552-4-jh80.chung@samsung.com>

On Mon, Dec 26, 2016 at 02:20:26PM +0900, Jaehoon Chung wrote:
> To use the generic PHY framework, adds the pcie_phy node.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> index 64226d5..2a15f18 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> @@ -805,6 +805,11 @@
>  			reg = <0x145f0000 0x1038>;
>  		};
>  
> +		syscon_fsys: syscon@156f0000 {
> +			compatible = "syscon";
> +			reg = <0x156f0000 0x1044>;
> +		};
> +
>  		gsc_0: video-scaler@13C00000 {
>  			compatible = "samsung,exynos5433-gsc";
>  			reg = <0x13c00000 0x1000>;
> @@ -1443,6 +1448,15 @@
>  				status = "disabled";
>  			};
>  		};
> +
> +		pcie_phy: pcie-phy@15680000 {
> +			#phy-cells = <0>;
> +			compatible = "samsung,exynos5433-pcie-phy";

Mostly we use the convention of compatible being first property.

> +			reg = <0x15680000 0x1000>;
> +			samsung,pmureg-phandle = <&pmu_system_controller>;
> +			samsung,fsys-sysreg = <&syscon_fsys>;
> +			status = "okay";

Why do you need to put status=okay here?

Best regards,
Krzysztof

> +		};
>  	};
>  
>  	timer: timer {
> -- 
> 2.10.2
> 

^ permalink raw reply

* Re: [RFC PATCH 5/6] Documentation: pci: add the exynos5433-pcie binding
From: Krzysztof Kozlowski @ 2016-12-27 16:19 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
	robh+dt, mark.rutland, kgene, krzk, javier, kishon, will.deacon,
	catalin.marinas, cpgs
In-Reply-To: <20161226052029.10552-6-jh80.chung@samsung.com>

On Mon, Dec 26, 2016 at 02:20:28PM +0900, Jaehoon Chung wrote:
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  .../devicetree/bindings/pci/exynos5433-pcie.txt    | 36 ++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/exynos5433-pcie.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/exynos5433-pcie.txt b/Documentation/devicetree/bindings/pci/exynos5433-pcie.txt
> new file mode 100644
> index 0000000..932a847
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/exynos5433-pcie.txt
> @@ -0,0 +1,36 @@
> +* Samsung Exynos5433 PCIe interface
> +
> +This PCIe host controller is based on the Synopsis Designware PCIe IP

Synopsys.

> +and thus inherits all the common properties defined in designware-pcie.txt.
> +
> +Required properties:
> +- compatible: "samsung,exynos5433-pcie"
> +- reg: base addresses and lengths of the pcie controller,
> +	the phy controller, additional register for the phy controller.

You mentioned three regs but the example contains four of them. Is the
config comming from snps,dw-pcie?

> +- reg-names: Must be "elbi", "phy" and "dbi" for each regs

Again, three here, four in example.

> +- interrupt-names: Must be "intr" for legacy interrupt pin.
> +
> +Other common properites refer to
> +	Documentation/devicetree/binding/pci/designware-pcie.txt
> +
> +Example:
> +
> +	pcie: pcie@15700000 {
> +		compatible ="samsung,exynos5433-pcie", "snps,dw-pcie";
                            ^
			    space needed
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		interrupts = <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "intr";
> +		clocks = <&cmu_fsys CLK_PCIE>, <&cmu_fsys CLK_PCLK_PCIE_PHY>;
> +		clock-names = "pcie", "pcie_bus";
> +		num-lanes = <1>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pcie_bus>;
> +		reg = <0x156b0000 0x1000>, <0x15680000 0x1000>,
> +		    <0x15700000 0x1000>, <0x0c000000 0x1000>;

Indentation here looks wrong. You indented it with spaces after tabs...
but not to align with line before.

Beside that, fine with me:
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof


> +		reg-names = "elbi", "phy", "dbi", "config";
> +		ranges = <0x81000000 0 0	  0x0c001000 0 0x00010000
> +			  0x82000000 0 0x0c011000 0x0c011000 0 0x3feefff>;
> +		status = "disabled";
> +	};
> -- 
> 2.10.2
> 

^ permalink raw reply

* Re: [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2
From: Krzysztof Kozlowski @ 2016-12-27 16:32 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
	robh+dt, mark.rutland, kgene, krzk, javier, kishon, will.deacon,
	catalin.marinas, cpgs
In-Reply-To: <20161226052029.10552-7-jh80.chung@samsung.com>

On Mon, Dec 26, 2016 at 02:20:29PM +0900, Jaehoon Chung wrote:
> Add the Exxynos5433 pcie node for TM2.
> This pcie device is used for supporting WiFi.
> 
> And some gpios are already requested from pinctrl. so it doesn't need to
> initialize.
> GPJ2-0 is used for supplying to WiFi PCIe chip.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |  7 +++++++
>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts      | 11 +++++++++--
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi         | 23 ++++++++++++++++++++++
>  3 files changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
> index ad71247..3e8b728 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
> @@ -183,6 +183,13 @@
>  		interrupt-controller;
>  		#interrupt-cells = <2>;
>  	};
> +
> +	pcie_wlanen: pcie-wlanen {
> +		samsung,pins = "gpj2-0";
> +		samsung,pin-function = <0>;
> +		samsung,pin-pud = <3>;
> +		samsung,pin-drv = <3>;
> +	};
>  };
>  
>  &pinctrl_finger {
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> index f21bdc2..c84a2ad 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> @@ -737,6 +737,15 @@
>  	bus-width = <4>;
>  };
>  
> +&pcie {
> +	assigned-clocks = <&cmu_fsys CLK_MOUT_SCLK_PCIE_100_USER>,
> +			<&cmu_top CLK_MOUT_SCLK_PCIE_100>;
> +	assigned-clock-parents = <&cmu_top CLK_SCLK_PCIE_100_FSYS>,
> +			<&cmu_top CLK_MOUT_BUS_PLL_USER>;
> +	assigned-clock-rates = <0>, <100000000>;
> +	status = "okay";
> +};
> +
>  &pinctrl_alive {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&initial_alive>;
> @@ -836,7 +845,6 @@
>  	pinctrl-0 = <&initial_ese>;
>  
>  	initial_ese: initial-state {
> -		PIN(IN, gpj2-0, DOWN, LV1);
>  		PIN(IN, gpj2-1, DOWN, LV1);
>  		PIN(IN, gpj2-2, DOWN, LV1);
>  	};
> @@ -851,7 +859,6 @@
>  		PIN(IN, gpr3-1, DOWN, LV1);
>  		PIN(IN, gpr3-2, DOWN, LV1);
>  		PIN(IN, gpr3-3, DOWN, LV1);
> -		PIN(IN, gpr3-7, NONE, LV1);
>  	};
>  };
>  
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> index 2a15f18..da287f4 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> @@ -1457,6 +1457,29 @@
>  			samsung,fsys-sysreg = <&syscon_fsys>;
>  			status = "okay";
>  		};
> +
> +		pcie: pcie@15700000 {
> +			compatible = "samsung,exynos5433-pcie", "snps,dw-pcie";
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			device_type = "pci";
> +			interrupts = <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "intr";
> +			clocks = <&cmu_fsys CLK_PCIE>,
> +			       <&cmu_fsys CLK_PCLK_PCIE_PHY>;

Here and in the 'reg' property - indentation looks weird. Tabs+spaces
but not aligned. Either you use spaces to align... or just don't care
and use tabs. I prefer consistency and below the 'ranges' property is
aligned.

> +			clock-names = "pcie", "pcie_bus";
> +			num-lanes = <1>;
> +			pinctrl-names = "default";
> +			phys = <&pcie_phy>;
> +			phy-names = "pcie-phy";
> +			pinctrl-0 = <&pcie_bus &pcie_wlanen>;
> +			reg = <0x156b0000 0x1000>, <0x15700000 0x1000>,
> +			    <0x0c000000 0x1000>;
> +			reg-names = "elbi", "dbi", "config";

This does not match the bindings documentation.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2
From: Krzysztof Kozlowski @ 2016-12-27 16:33 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, kgene-DgEjT+Ai2ygdnm+yROfE0A,
	Krzysztof Kozlowski, Javier Martinez Canillas, kishon-l0cyMroinI0,
	will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	cpgs-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <20161227163200.6noed454fmtgozrv@kozik-lap>

On Tue, Dec 27, 2016 at 6:32 PM, Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Dec 26, 2016 at 02:20:29PM +0900, Jaehoon Chung wrote:
>> Add the Exxynos5433 pcie node for TM2.
>> This pcie device is used for supporting WiFi.
>>
>> And some gpios are already requested from pinctrl. so it doesn't need to
>> initialize.
>> GPJ2-0 is used for supplying to WiFi PCIe chip.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---


Ahhh, and one more comment - please add missing 'dts' prefix in the subject.

BR,
Krzysztof
--
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: [RESEND PATCH v2] arm64: dts: rockchip: add u2phy clock for ehci and ohci of rk3399
From: Heiko Stuebner @ 2016-12-27 17:04 UTC (permalink / raw)
  To: Xing Zheng
  Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
	dianders-F7+t8E8rja9g9hUCZPvPmw, wxt-TNX95d0MmH7DzftRWevZcw,
	shawn.lin-TNX95d0MmH7DzftRWevZcw,
	briannorris-F7+t8E8rja9g9hUCZPvPmw, jay.xu-TNX95d0MmH7DzftRWevZcw,
	zhangqing-TNX95d0MmH7DzftRWevZcw, david.wu-TNX95d0MmH7DzftRWevZcw,
	wulf-TNX95d0MmH7DzftRWevZcw, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dianders-hpIqsD4AKlfQT0dZR+AlfA,
	frank.wang-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <1482316865-2769-1-git-send-email-zhengxing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

Am Mittwoch, 21. Dezember 2016, 18:41:05 CET schrieb Xing Zheng:
> From: William wu <wulf-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> We found that the suspend process was blocked when it run into
> ehci/ohci module due to clk-480m of usb2-phy was disabled.
> 
> The root cause is that usb2-phy suspended earlier than ehci/ohci
> (usb2-phy will be auto suspended if no devices plug-in). and the
> clk-480m provided by it was disabled if no module used. However,
> some suspend process related ehci/ohci are base on this clock,
> so we should refer it into ehci/ohci driver to prevent this case.
> 
> The u2phy clock flow like this:
> ===
>       u2phy ________________
> 
>            |                |    |-----> UTMI_CLK ---------> | EHCI |
> 
> OSC_24M ---|---> PHY_PLL----|----|
> 
>            |________^_______|    |-----> 480M_CLK ---|G|---> |
>            |USBPHY_480M_SRC| ----> USBPHY_480M for SoC
>                    GRF
> ===
> 
> Signed-off-by: William wu <wulf-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Signed-off-by: Xing Zheng <zhengxing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

applied for 4.11 with Doug's Review-tag


Thanks
Heiko
--
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

* [PATCHv2 0/5] Add generic pinctrl helpers for managing groups and function
From: Tony Lindgren @ 2016-12-27 17:19 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Haojian Zhuang, Masahiro Yamada, Grygorii Strashko,
	Nishanth Menon, linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA

Hi all,

Here are some changes to add generic helpers for managing pinctrl groups and
functions.

Regards,

Tony

Changes since v1:

- Updates to the first patch in the series to use delayed work for pinctrl
  hogs based on what was discussed on the mailing list, mostly to pass
  pctldev to the parsers and add pinctrl_dt_has_hogs() check

Tony Lindgren (5):
  pinctrl: core: Use delayed work for hogs
  pinctrl: core: Add generic pinctrl functions for managing groups
  pinctrl: core: Add generic pinctrl functions for managing groups
  pinctrl: single: Use generic pinctrl helpers for managing groups
  pinctrl: single: Use generic pinmux helpers for managing functions

 drivers/pinctrl/Kconfig          |  11 +-
 drivers/pinctrl/core.c           | 270 +++++++++++++++++++++++++++++++-----
 drivers/pinctrl/core.h           |  67 +++++++++
 drivers/pinctrl/devicetree.c     |  28 +++-
 drivers/pinctrl/devicetree.h     |  12 +-
 drivers/pinctrl/pinctrl-single.c | 290 ++++-----------------------------------
 drivers/pinctrl/pinmux.c         | 173 +++++++++++++++++++++++
 drivers/pinctrl/pinmux.h         |  42 ++++++
 8 files changed, 591 insertions(+), 302 deletions(-)

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

* [PATCH 1/5] pinctrl: core: Use delayed work for hogs
From: Tony Lindgren @ 2016-12-27 17:19 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Haojian Zhuang, Masahiro Yamada, Grygorii Strashko,
	Nishanth Menon, linux-gpio, devicetree, linux-kernel, linux-omap
In-Reply-To: <20161227172003.6517-1-tony@atomide.com>

Having the pin control framework call pin controller functions
before it's probe has finished is not nice as the pin controller
device driver does not yet have struct pinctrl_dev handle.

Let's fix this issue by adding deferred work for late init. This is
needed to be able to add pinctrl generic helper functions that expect
to know struct pinctrl_dev handle. Note that we now need to call
create_pinctrl() directly as we don't want to add the pin controller
to the list of controllers until the hogs are claimed. We also need
to pass the pinctrl_dev to the device tree parser functions as they
otherwise won't find the right controller at this point.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/pinctrl/core.c       | 90 ++++++++++++++++++++++++++++----------------
 drivers/pinctrl/core.h       |  2 +
 drivers/pinctrl/devicetree.c | 28 +++++++++++---
 drivers/pinctrl/devicetree.h | 12 +++++-
 4 files changed, 93 insertions(+), 39 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -720,7 +720,8 @@ static struct pinctrl_state *create_state(struct pinctrl *p,
 	return state;
 }
 
-static int add_setting(struct pinctrl *p, struct pinctrl_map const *map)
+static int add_setting(struct pinctrl *p, struct pinctrl_dev *pctldev,
+		       struct pinctrl_map const *map)
 {
 	struct pinctrl_state *state;
 	struct pinctrl_setting *setting;
@@ -744,7 +745,11 @@ static int add_setting(struct pinctrl *p, struct pinctrl_map const *map)
 
 	setting->type = map->type;
 
-	setting->pctldev = get_pinctrl_dev_from_devname(map->ctrl_dev_name);
+	if (pctldev)
+		setting->pctldev = pctldev;
+	else
+		setting->pctldev =
+			get_pinctrl_dev_from_devname(map->ctrl_dev_name);
 	if (setting->pctldev == NULL) {
 		kfree(setting);
 		/* Do not defer probing of hogs (circular loop) */
@@ -800,7 +805,8 @@ static struct pinctrl *find_pinctrl(struct device *dev)
 
 static void pinctrl_free(struct pinctrl *p, bool inlist);
 
-static struct pinctrl *create_pinctrl(struct device *dev)
+static struct pinctrl *create_pinctrl(struct device *dev,
+				      struct pinctrl_dev *pctldev)
 {
 	struct pinctrl *p;
 	const char *devname;
@@ -823,7 +829,7 @@ static struct pinctrl *create_pinctrl(struct device *dev)
 	INIT_LIST_HEAD(&p->states);
 	INIT_LIST_HEAD(&p->dt_maps);
 
-	ret = pinctrl_dt_to_map(p);
+	ret = pinctrl_dt_to_map(p, pctldev);
 	if (ret < 0) {
 		kfree(p);
 		return ERR_PTR(ret);
@@ -838,7 +844,7 @@ static struct pinctrl *create_pinctrl(struct device *dev)
 		if (strcmp(map->dev_name, devname))
 			continue;
 
-		ret = add_setting(p, map);
+		ret = add_setting(p, pctldev, map);
 		/*
 		 * At this point the adding of a setting may:
 		 *
@@ -899,7 +905,7 @@ struct pinctrl *pinctrl_get(struct device *dev)
 		return p;
 	}
 
-	return create_pinctrl(dev);
+	return create_pinctrl(dev, NULL);
 }
 EXPORT_SYMBOL_GPL(pinctrl_get);
 
@@ -1738,6 +1744,46 @@ static int pinctrl_check_ops(struct pinctrl_dev *pctldev)
 }
 
 /**
+ * pinctrl_late_init() - finish pin controller device registration
+ * @work: work struct
+ */
+static void pinctrl_late_init(struct work_struct *work)
+{
+	struct pinctrl_dev *pctldev;
+
+	pctldev = container_of(work, struct pinctrl_dev, late_init.work);
+
+	pctldev->p = create_pinctrl(pctldev->dev, pctldev);
+	if (!IS_ERR(pctldev->p)) {
+		kref_get(&pctldev->p->users);
+		pctldev->hog_default =
+			pinctrl_lookup_state(pctldev->p, PINCTRL_STATE_DEFAULT);
+		if (IS_ERR(pctldev->hog_default)) {
+			dev_dbg(pctldev->dev,
+				"failed to lookup the default state\n");
+		} else {
+			if (pinctrl_select_state(pctldev->p,
+						 pctldev->hog_default))
+				dev_err(pctldev->dev,
+					"failed to select default state\n");
+		}
+
+		pctldev->hog_sleep =
+			pinctrl_lookup_state(pctldev->p,
+					     PINCTRL_STATE_SLEEP);
+		if (IS_ERR(pctldev->hog_sleep))
+			dev_dbg(pctldev->dev,
+				"failed to lookup the sleep state\n");
+	}
+
+	mutex_lock(&pinctrldev_list_mutex);
+	list_add_tail(&pctldev->node, &pinctrldev_list);
+	mutex_unlock(&pinctrldev_list_mutex);
+
+	pinctrl_init_device_debugfs(pctldev);
+}
+
+/**
  * pinctrl_register() - register a pin controller device
  * @pctldesc: descriptor for this pin controller
  * @dev: parent device for this pin controller
@@ -1766,6 +1812,7 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
 	pctldev->driver_data = driver_data;
 	INIT_RADIX_TREE(&pctldev->pin_desc_tree, GFP_KERNEL);
 	INIT_LIST_HEAD(&pctldev->gpio_ranges);
+	INIT_DELAYED_WORK(&pctldev->late_init, pinctrl_late_init);
 	pctldev->dev = dev;
 	mutex_init(&pctldev->mutex);
 
@@ -1800,32 +1847,10 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
 		goto out_err;
 	}
 
-	mutex_lock(&pinctrldev_list_mutex);
-	list_add_tail(&pctldev->node, &pinctrldev_list);
-	mutex_unlock(&pinctrldev_list_mutex);
-
-	pctldev->p = pinctrl_get(pctldev->dev);
-
-	if (!IS_ERR(pctldev->p)) {
-		pctldev->hog_default =
-			pinctrl_lookup_state(pctldev->p, PINCTRL_STATE_DEFAULT);
-		if (IS_ERR(pctldev->hog_default)) {
-			dev_dbg(dev, "failed to lookup the default state\n");
-		} else {
-			if (pinctrl_select_state(pctldev->p,
-						pctldev->hog_default))
-				dev_err(dev,
-					"failed to select default state\n");
-		}
-
-		pctldev->hog_sleep =
-			pinctrl_lookup_state(pctldev->p,
-						    PINCTRL_STATE_SLEEP);
-		if (IS_ERR(pctldev->hog_sleep))
-			dev_dbg(dev, "failed to lookup the sleep state\n");
-	}
-
-	pinctrl_init_device_debugfs(pctldev);
+	if (pinctrl_dt_has_hogs(pctldev))
+		schedule_delayed_work(&pctldev->late_init, 0);
+	else
+		pinctrl_late_init(&pctldev->late_init.work);
 
 	return pctldev;
 
@@ -1848,6 +1873,7 @@ void pinctrl_unregister(struct pinctrl_dev *pctldev)
 	if (pctldev == NULL)
 		return;
 
+	cancel_delayed_work_sync(&pctldev->late_init);
 	mutex_lock(&pctldev->mutex);
 	pinctrl_remove_device_debugfs(pctldev);
 	mutex_unlock(&pctldev->mutex);
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -33,6 +33,7 @@ struct pinctrl_gpio_range;
  * @p: result of pinctrl_get() for this device
  * @hog_default: default state for pins hogged by this device
  * @hog_sleep: sleep state for pins hogged by this device
+ * @late_init: delayed work for pin controller to finish registration
  * @mutex: mutex taken on each pin controller specific action
  * @device_root: debugfs root for this device
  */
@@ -47,6 +48,7 @@ struct pinctrl_dev {
 	struct pinctrl *p;
 	struct pinctrl_state *hog_default;
 	struct pinctrl_state *hog_sleep;
+	struct delayed_work late_init;
 	struct mutex mutex;
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *device_root;
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -100,11 +100,12 @@ struct pinctrl_dev *of_pinctrl_get(struct device_node *np)
 	return get_pinctrl_dev_from_of_node(np);
 }
 
-static int dt_to_map_one_config(struct pinctrl *p, const char *statename,
+static int dt_to_map_one_config(struct pinctrl *p,
+				struct pinctrl_dev *pctldev,
+				const char *statename,
 				struct device_node *np_config)
 {
 	struct device_node *np_pctldev;
-	struct pinctrl_dev *pctldev;
 	const struct pinctrl_ops *ops;
 	int ret;
 	struct pinctrl_map *map;
@@ -121,7 +122,8 @@ static int dt_to_map_one_config(struct pinctrl *p, const char *statename,
 			/* OK let's just assume this will appear later then */
 			return -EPROBE_DEFER;
 		}
-		pctldev = get_pinctrl_dev_from_of_node(np_pctldev);
+		if (!pctldev)
+			pctldev = get_pinctrl_dev_from_of_node(np_pctldev);
 		if (pctldev)
 			break;
 		/* Do not defer probing of hogs (circular loop) */
@@ -166,7 +168,22 @@ static int dt_remember_dummy_state(struct pinctrl *p, const char *statename)
 	return dt_remember_or_free_map(p, statename, NULL, map, 1);
 }
 
-int pinctrl_dt_to_map(struct pinctrl *p)
+bool pinctrl_dt_has_hogs(struct pinctrl_dev *pctldev)
+{
+	struct device_node *np;
+	struct property *prop;
+	int size;
+
+	np = pctldev->dev->of_node;
+	if (!np)
+		return false;
+
+	prop = of_find_property(np, "pinctrl-0", &size);
+
+	return prop ? true : false;
+}
+
+int pinctrl_dt_to_map(struct pinctrl *p, struct pinctrl_dev *pctldev)
 {
 	struct device_node *np = p->dev->of_node;
 	int state, ret;
@@ -233,7 +250,8 @@ int pinctrl_dt_to_map(struct pinctrl *p)
 			}
 
 			/* Parse the node */
-			ret = dt_to_map_one_config(p, statename, np_config);
+			ret = dt_to_map_one_config(p, pctldev, statename,
+						   np_config);
 			of_node_put(np_config);
 			if (ret < 0)
 				goto err;
diff --git a/drivers/pinctrl/devicetree.h b/drivers/pinctrl/devicetree.h
--- a/drivers/pinctrl/devicetree.h
+++ b/drivers/pinctrl/devicetree.h
@@ -20,8 +20,10 @@ struct of_phandle_args;
 
 #ifdef CONFIG_OF
 
+bool pinctrl_dt_has_hogs(struct pinctrl_dev *pctldev);
+
 void pinctrl_dt_free_maps(struct pinctrl *p);
-int pinctrl_dt_to_map(struct pinctrl *p);
+int pinctrl_dt_to_map(struct pinctrl *p, struct pinctrl_dev *pctldev);
 
 int pinctrl_count_index_with_args(const struct device_node *np,
 				  const char *list_name);
@@ -32,7 +34,13 @@ int pinctrl_parse_index_with_args(const struct device_node *np,
 
 #else
 
-static inline int pinctrl_dt_to_map(struct pinctrl *p)
+static inline bool pinctrl_dt_has_hogs(struct pinctrl_dev *pctldev)
+{
+	return false;
+}
+
+static inline int pinctrl_dt_to_map(struct pinctrl *p,
+				    struct pinctrl_dev *pctldev)
 {
 	return 0;
 }
-- 
2.11.0

^ permalink raw reply


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