Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] drm/mediatek: fixed the calc method of data rate per lane
From: CK Hu @ 2016-10-26  9:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477472395-6451-1-git-send-email-jitao.shi@mediatek.com>

Hi, Jitao:

On Wed, 2016-10-26 at 16:59 +0800, Jitao Shi wrote:
> Tune dsi frame rate by pixel clock, dsi add some extra signal (i.e. Tlpx,
> Ths-prepare, Ths-zero, Ths-trail,Ths-exit) when enter and exit LP mode, this
> signal will cause h-time larger than normal and reduce FPS.
> Need to multiply a coefficient to offset the extra signal's effect.
> coefficient = ((htotal*bpp/lane_number)+Tlpx+Ths_prep+Ths_zero+Ths_trail+
>                 Ths_exit)/(htotal*bpp/lane_number))
> 

One check-patch warning in commit message:

WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description
(prefer a maximum 75 chars per line)
#7:
    Tune dsi frame rate by pixel clock, dsi add some extra signal (i.e.
Tlpx,

> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
> Change since v2:
>  - move phy timing back to dsi_phy_timconfig.
> 
> Change since v1:
>  - phy_timing2 and phy_timing3 refer clock cycle time.
>  - define values of LPX HS_PRPR HS_ZERO HS_TRAIL TA_GO TA_SURE TA_GET DA_HS_EXIT
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c |   74 +++++++++++++++++++++++++-----------
>  1 file changed, 51 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 28b2044..8b3b38a 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -85,16 +85,16 @@
>  #define LD0_WAKEUP_EN			BIT(2)
>  
>  #define DSI_PHY_TIMECON0	0x110
> -#define LPX				(0xff << 0)
> -#define HS_PRPR				(0xff << 8)
> -#define HS_ZERO				(0xff << 16)
> -#define HS_TRAIL			(0xff << 24)
> +#define LPX				(5 << 0)
> +#define HS_PRPR				(6 << 8)
> +#define HS_ZERO				(10 << 16)
> +#define HS_TRAIL			(8 << 24)
>  
>  #define DSI_PHY_TIMECON1	0x114
> -#define TA_GO				(0xff << 0)
> -#define TA_SURE				(0xff << 8)
> -#define TA_GET				(0xff << 16)
> -#define DA_HS_EXIT			(0xff << 24)
> +#define TA_GO				(20 << 0)
> +#define TA_SURE				(7 << 8)
> +#define TA_GET				(25 << 16)
> +#define DA_HS_EXIT			(7 << 24)
>  
>  #define DSI_PHY_TIMECON2	0x118
>  #define CONT_DET			(0xff << 0)
> @@ -161,20 +161,18 @@ static void mtk_dsi_mask(struct mtk_dsi *dsi, u32 offset, u32 mask, u32 data)
>  static void dsi_phy_timconfig(struct mtk_dsi *dsi)
>  {
>  	u32 timcon0, timcon1, timcon2, timcon3;
> -	unsigned int ui, cycle_time;
> -	unsigned int lpx;
> +	u32 ui, cycle_time;
>  
>  	ui = 1000 / dsi->data_rate + 0x01;
>  	cycle_time = 8000 / dsi->data_rate + 0x01;
> -	lpx = 5;
>  
> -	timcon0 = (8 << 24) | (0xa << 16) | (0x6 << 8) | lpx;
> -	timcon1 = (7 << 24) | (5 * lpx << 16) | ((3 * lpx) / 2) << 8 |
> -		  (4 * lpx);
> +	timcon0 = LPX | HS_PRPR | HS_ZERO | HS_TRAIL;
> +	timcon1 = 4 * LPX | (3 * LPX / 2) << 8 | 5 * LPX << 16 | DA_HS_EXIT;
>  	timcon2 = ((NS_TO_CYCLE(0x64, cycle_time) + 0xa) << 24) |
> -		  (NS_TO_CYCLE(0x150, cycle_time) << 16);
> -	timcon3 = (2 * lpx) << 16 | NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
> -		   NS_TO_CYCLE(0x40, cycle_time);
> +		      (NS_TO_CYCLE(0x150, cycle_time) << 16);

I think this line may align to '(' in the right of '='.

> +	timcon3 = (2 * LPX) << 16 |
> +		      NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
> +		      NS_TO_CYCLE(0x40, cycle_time);

I think these two lines may align to '(' in the right of '='.

Regards,
CK
     I. 
>  
>  	writel(timcon0, dsi->regs + DSI_PHY_TIMECON0);
>  	writel(timcon1, dsi->regs + DSI_PHY_TIMECON1);
> @@ -202,19 +200,49 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
>  {
>  	struct device *dev = dsi->dev;
>  	int ret;
> +	u64 bit_clock, total_bits;
> +	u32 htotal, htotal_bits, bit_per_pixel, overhead_cycles, overhead_bits;
>  
>  	if (++dsi->refcount != 1)
>  		return 0;
>  
> +	switch (dsi->format) {
> +	case MIPI_DSI_FMT_RGB565:
> +		bit_per_pixel = 16;
> +		break;
> +	case MIPI_DSI_FMT_RGB666_PACKED:
> +		bit_per_pixel = 18;
> +		break;
> +	case MIPI_DSI_FMT_RGB666:
> +	case MIPI_DSI_FMT_RGB888:
> +	default:
> +		bit_per_pixel = 24;
> +		break;
> +	}
>  	/**
> -	 * data_rate = (pixel_clock / 1000) * pixel_dipth * mipi_ratio;
> -	 * pixel_clock unit is Khz, data_rata unit is MHz, so need divide 1000.
> -	 * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
> -	 * we set mipi_ratio is 1.05.
> +	 * data_rate = (pixel_clock) * bit_per_pixel * mipi_ratio / lane_num;
> +	 * vm.pixelclock is Khz, data_rata unit is Hz, so need to multiply 1000
> +	 * mipi_ratio is (htotal * byte_per_pixel / lane_num + Tlpx + Ths_prep
> +	 *		  + Thstrail + Ths_exit + Ths_zero) /
> +	 *		 (htotal * byte_per_pixel /lane_number)
>  	 */
> -	dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
> +	bit_clock = dsi->vm.pixelclock * 1000 * bit_per_pixel;
> +	htotal = dsi->vm.hactive + dsi->vm.hback_porch + dsi->vm.hfront_porch +
> +		 dsi->vm.hsync_len;
> +	htotal_bits = htotal * bit_per_pixel;
> +
> +	/**
> +	 * overhead = lpx + hs_prepare + hs_zero + hs_trail + hs_exit
> +	 */
> +	overhead_cycles = LPX + (HS_PRPR >> 8) + (HS_ZERO >> 16) +
> +			  (HS_TRAIL >> 24) + (DA_HS_EXIT >> 24);
> +	overhead_bits = overhead_cycles * dsi->lanes * 8;
> +	total_bits = htotal_bits + overhead_bits;
> +
> +	dsi->data_rate = DIV_ROUND_UP_ULL(bit_clock * total_bits,
> +					  htotal_bits * dsi->lanes);
>  
> -	ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
> +	ret = clk_set_rate(dsi->hs_clk, dsi->data_rate);
>  	if (ret < 0) {
>  		dev_err(dev, "Failed to set data rate: %d\n", ret);
>  		goto err_refcount;

^ permalink raw reply

* [PATCH] [media] media: mtk-mdp: mark PM functions as __maybe_unused
From: Arnd Bergmann @ 2016-10-26  9:43 UTC (permalink / raw)
  To: linux-arm-kernel

A previous patch tried to fix a build error, but introduced another
warning:

drivers/media/platform/mtk-mdp/mtk_mdp_core.c:71:13: error: ?mtk_mdp_clock_off? defined but not used [-Werror=unused-function]
drivers/media/platform/mtk-mdp/mtk_mdp_core.c:62:13: error: ?mtk_mdp_clock_on? defined but not used [-Werror=unused-function]

This marks all the PM functions as __maybe_unused and removes
the #ifdef around them, as that will always do the right thing.

Fixes: 1b06fcf56aa6 ("[media] media: mtk-mdp: fix build error")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
index 40a229d8a1f5..d4c740263457 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
@@ -233,8 +233,7 @@ static int mtk_mdp_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-static int mtk_mdp_pm_suspend(struct device *dev)
+static int __maybe_unused mtk_mdp_pm_suspend(struct device *dev)
 {
 	struct mtk_mdp_dev *mdp = dev_get_drvdata(dev);
 
@@ -243,7 +242,7 @@ static int mtk_mdp_pm_suspend(struct device *dev)
 	return 0;
 }
 
-static int mtk_mdp_pm_resume(struct device *dev)
+static int __maybe_unused mtk_mdp_pm_resume(struct device *dev)
 {
 	struct mtk_mdp_dev *mdp = dev_get_drvdata(dev);
 
@@ -251,10 +250,8 @@ static int mtk_mdp_pm_resume(struct device *dev)
 
 	return 0;
 }
-#endif /* CONFIG_PM */
 
-#ifdef CONFIG_PM_SLEEP
-static int mtk_mdp_suspend(struct device *dev)
+static int __maybe_unused mtk_mdp_suspend(struct device *dev)
 {
 	if (pm_runtime_suspended(dev))
 		return 0;
@@ -262,14 +259,13 @@ static int mtk_mdp_suspend(struct device *dev)
 	return mtk_mdp_pm_suspend(dev);
 }
 
-static int mtk_mdp_resume(struct device *dev)
+static int __maybe_unused mtk_mdp_resume(struct device *dev)
 {
 	if (pm_runtime_suspended(dev))
 		return 0;
 
 	return mtk_mdp_pm_resume(dev);
 }
-#endif /* CONFIG_PM_SLEEP */
 
 static const struct dev_pm_ops mtk_mdp_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(mtk_mdp_suspend, mtk_mdp_resume)
-- 
2.9.0

^ permalink raw reply related

* [PATCH 00/10] arm64: move thread_info off of the task stack
From: Mark Rutland @ 2016-10-26  9:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2db24d48-3a8a-11c2-9b2c-26a51a3baa36@redhat.com>

On Tue, Oct 25, 2016 at 05:46:39PM -0700, Laura Abbott wrote:
> On 10/24/2016 11:09 AM, Mark Rutland wrote:
> >On Mon, Oct 24, 2016 at 10:58:10AM -0700, Laura Abbott wrote:
> >>On 10/24/2016 10:48 AM, Mark Rutland wrote:
> >>>On Mon, Oct 24, 2016 at 10:38:59AM -0700, Laura Abbott wrote:
> >>>>On 10/19/2016 12:10 PM, Mark Rutland wrote:
> >>>>>[3] https://git.kernel.org/cgit/linux/kernel/git/mark/linux.git/log/?h=arm64/ti-stack-split
> >>>
> >>>>I pulled the arm64/ti-stack-split branch on top of a Fedora
> >>>>tree and ran back-to-back kernel RPM builds for a long weekend.
> >>>>It's still going as of this morning so you can take that as a
> >>>>
> >>>>Tested-by: Laura Abbott <labbott@redhat.com>
> >>>
> >>>Thanks! That's much appreciated!
> >>>
> >>>Just to check, did you grab the version with entry.S fixes rolled in
> >>>(where the head is 657f54256c427fec)?
> >>
> >>Ah I did not. That came in after I started the test. I'll start
> >>another run with the new version.
> >
> >Sorry about that; thanks for respinning!
 
> While not as long running, I gave it another spin and it seemed to
> work fine as well.

Thanks, that's much appreciated!

I've applied your Tested-by to the series.

Thanks,
Mark.

^ permalink raw reply

* [PATCH v6 2/5] ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration.
From: Sekhar Nori @ 2016-10-26  9:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477451211-31979-3-git-send-email-david@lechnology.com>

On Wednesday 26 October 2016 08:36 AM, David Lechner wrote:
> The CFGCHIP registers are used by a number of devices, so using a syscon
> device to share them. The first consumer of this will by the phy-da8xx-usb

"will be the .." 

Also, in subject line, "platform device" instead of "platform 
declaration". Also lose the period at at the end of subject line

> driver.
> 
> Signed-off-by: David Lechner <david@lechnology.com>

Applied to v4.10/soc. Here is the final commit text.

---
ARM: davinci: da8xx: Add CFGCHIP syscon platform device

The CFGCHIP registers are used by a number of devices, so use a syscon
device to share them. The first consumer of this will by the phy-da8xx-usb
driver.

Add the syscon device and register it.

Signed-off-by: David Lechner <david@lechnology.com>
[nsekhar at ti.com: minor commit message fixes]
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---

Thanks,
Sekhar

^ permalink raw reply

* [PATCH] ARM: dts: imx: Fix "ERROR: code indent should use tabs where possible"
From: Jagan Teki @ 2016-10-26 10:01 UTC (permalink / raw)
  To: linux-arm-kernel

Fixed code indent tabs in respetcive imx23, imx51, imx53, imx6dl, imx6q
and imx6sx dtsi and dts files.

Signed-off-by: Jagan Teki <jagan@openedev.com>
---
 arch/arm/boot/dts/imx23.dtsi                 |  2 +-
 arch/arm/boot/dts/imx50.dtsi                 | 44 +++++++++---------
 arch/arm/boot/dts/imx51.dtsi                 | 44 +++++++++---------
 arch/arm/boot/dts/imx53.dtsi                 | 68 ++++++++++++++--------------
 arch/arm/boot/dts/imx6dl-riotboard.dts       |  2 +-
 arch/arm/boot/dts/imx6dl-tx6dl-comtft.dts    |  2 +-
 arch/arm/boot/dts/imx6dl-tx6u-801x.dts       |  2 +-
 arch/arm/boot/dts/imx6q-phytec-pbab01.dts    |  2 +-
 arch/arm/boot/dts/imx6q-tx6q-1010-comtft.dts |  2 +-
 arch/arm/boot/dts/imx6q-tx6q-1010.dts        |  2 +-
 arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts |  2 +-
 arch/arm/boot/dts/imx6q-tx6q-1020.dts        |  2 +-
 arch/arm/boot/dts/imx6sx-sdb.dtsi            |  8 ++--
 arch/arm/boot/dts/imx6sx.dtsi                |  6 +--
 14 files changed, 94 insertions(+), 94 deletions(-)

diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi
index 440ee9a..8e1543f 100644
--- a/arch/arm/boot/dts/imx23.dtsi
+++ b/arch/arm/boot/dts/imx23.dtsi
@@ -464,7 +464,7 @@
 				reg = <0x80038000 0x2000>;
 				status = "disabled";
 			};
-                };
+		};
 
 		apbx at 80040000 {
 			compatible = "simple-bus";
diff --git a/arch/arm/boot/dts/imx50.dtsi b/arch/arm/boot/dts/imx50.dtsi
index 8fe8bee..92a03bc 100644
--- a/arch/arm/boot/dts/imx50.dtsi
+++ b/arch/arm/boot/dts/imx50.dtsi
@@ -103,8 +103,8 @@
 					reg = <0x50004000 0x4000>;
 					interrupts = <1>;
 					clocks = <&clks IMX5_CLK_ESDHC1_IPG_GATE>,
-					         <&clks IMX5_CLK_DUMMY>,
-					         <&clks IMX5_CLK_ESDHC1_PER_GATE>;
+						 <&clks IMX5_CLK_DUMMY>,
+						 <&clks IMX5_CLK_ESDHC1_PER_GATE>;
 					clock-names = "ipg", "ahb", "per";
 					bus-width = <4>;
 					status = "disabled";
@@ -115,8 +115,8 @@
 					reg = <0x50008000 0x4000>;
 					interrupts = <2>;
 					clocks = <&clks IMX5_CLK_ESDHC2_IPG_GATE>,
-					         <&clks IMX5_CLK_DUMMY>,
-					         <&clks IMX5_CLK_ESDHC2_PER_GATE>;
+						 <&clks IMX5_CLK_DUMMY>,
+						 <&clks IMX5_CLK_ESDHC2_PER_GATE>;
 					clock-names = "ipg", "ahb", "per";
 					bus-width = <4>;
 					status = "disabled";
@@ -127,7 +127,7 @@
 					reg = <0x5000c000 0x4000>;
 					interrupts = <33>;
 					clocks = <&clks IMX5_CLK_UART3_IPG_GATE>,
-					         <&clks IMX5_CLK_UART3_PER_GATE>;
+						 <&clks IMX5_CLK_UART3_PER_GATE>;
 					clock-names = "ipg", "per";
 					status = "disabled";
 				};
@@ -139,7 +139,7 @@
 					reg = <0x50010000 0x4000>;
 					interrupts = <36>;
 					clocks = <&clks IMX5_CLK_ECSPI1_IPG_GATE>,
-					         <&clks IMX5_CLK_ECSPI1_PER_GATE>;
+						 <&clks IMX5_CLK_ECSPI1_PER_GATE>;
 					clock-names = "ipg", "per";
 					status = "disabled";
 				};
@@ -164,8 +164,8 @@
 					reg = <0x50020000 0x4000>;
 					interrupts = <3>;
 					clocks = <&clks IMX5_CLK_ESDHC3_IPG_GATE>,
-					         <&clks IMX5_CLK_DUMMY>,
-					         <&clks IMX5_CLK_ESDHC3_PER_GATE>;
+						 <&clks IMX5_CLK_DUMMY>,
+						 <&clks IMX5_CLK_ESDHC3_PER_GATE>;
 					clock-names = "ipg", "ahb", "per";
 					bus-width = <4>;
 					status = "disabled";
@@ -176,8 +176,8 @@
 					reg = <0x50024000 0x4000>;
 					interrupts = <4>;
 					clocks = <&clks IMX5_CLK_ESDHC4_IPG_GATE>,
-					         <&clks IMX5_CLK_DUMMY>,
-					         <&clks IMX5_CLK_ESDHC4_PER_GATE>;
+						 <&clks IMX5_CLK_DUMMY>,
+						 <&clks IMX5_CLK_ESDHC4_PER_GATE>;
 					clock-names = "ipg", "ahb", "per";
 					bus-width = <4>;
 					status = "disabled";
@@ -279,7 +279,7 @@
 				reg = <0x53fa0000 0x4000>;
 				interrupts = <39>;
 				clocks = <&clks IMX5_CLK_GPT_IPG_GATE>,
-				         <&clks IMX5_CLK_GPT_HF_GATE>;
+					 <&clks IMX5_CLK_GPT_HF_GATE>;
 				clock-names = "ipg", "per";
 			};
 
@@ -298,7 +298,7 @@
 				compatible = "fsl,imx50-pwm", "fsl,imx27-pwm";
 				reg = <0x53fb4000 0x4000>;
 				clocks = <&clks IMX5_CLK_PWM1_IPG_GATE>,
-				         <&clks IMX5_CLK_PWM1_HF_GATE>;
+					 <&clks IMX5_CLK_PWM1_HF_GATE>;
 				clock-names = "ipg", "per";
 				interrupts = <61>;
 			};
@@ -308,7 +308,7 @@
 				compatible = "fsl,imx50-pwm", "fsl,imx27-pwm";
 				reg = <0x53fb8000 0x4000>;
 				clocks = <&clks IMX5_CLK_PWM2_IPG_GATE>,
-				         <&clks IMX5_CLK_PWM2_HF_GATE>;
+					 <&clks IMX5_CLK_PWM2_HF_GATE>;
 				clock-names = "ipg", "per";
 				interrupts = <94>;
 			};
@@ -318,7 +318,7 @@
 				reg = <0x53fbc000 0x4000>;
 				interrupts = <31>;
 				clocks = <&clks IMX5_CLK_UART1_IPG_GATE>,
-				         <&clks IMX5_CLK_UART1_PER_GATE>;
+					 <&clks IMX5_CLK_UART1_PER_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -328,7 +328,7 @@
 				reg = <0x53fc0000 0x4000>;
 				interrupts = <32>;
 				clocks = <&clks IMX5_CLK_UART2_IPG_GATE>,
-				         <&clks IMX5_CLK_UART2_PER_GATE>;
+					 <&clks IMX5_CLK_UART2_PER_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -383,7 +383,7 @@
 				reg = <0x53ff0000 0x4000>;
 				interrupts = <13>;
 				clocks = <&clks IMX5_CLK_UART4_IPG_GATE>,
-				         <&clks IMX5_CLK_UART4_PER_GATE>;
+					 <&clks IMX5_CLK_UART4_PER_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -401,7 +401,7 @@
 				reg = <0x63f90000 0x4000>;
 				interrupts = <86>;
 				clocks = <&clks IMX5_CLK_UART5_IPG_GATE>,
-				         <&clks IMX5_CLK_UART5_PER_GATE>;
+					 <&clks IMX5_CLK_UART5_PER_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -420,7 +420,7 @@
 				reg = <0x63fac000 0x4000>;
 				interrupts = <37>;
 				clocks = <&clks IMX5_CLK_ECSPI2_IPG_GATE>,
-				         <&clks IMX5_CLK_ECSPI2_PER_GATE>;
+					 <&clks IMX5_CLK_ECSPI2_PER_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -430,7 +430,7 @@
 				reg = <0x63fb0000 0x4000>;
 				interrupts = <6>;
 				clocks = <&clks IMX5_CLK_SDMA_GATE>,
-				         <&clks IMX5_CLK_SDMA_GATE>;
+					 <&clks IMX5_CLK_SDMA_GATE>;
 				clock-names = "ipg", "ahb";
 				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx50.bin";
 			};
@@ -442,7 +442,7 @@
 				reg = <0x63fc0000 0x4000>;
 				interrupts = <38>;
 				clocks = <&clks IMX5_CLK_CSPI_IPG_GATE>,
-				         <&clks IMX5_CLK_CSPI_IPG_GATE>;
+					 <&clks IMX5_CLK_CSPI_IPG_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -492,8 +492,8 @@
 				reg = <0x63fec000 0x4000>;
 				interrupts = <87>;
 				clocks = <&clks IMX5_CLK_FEC_GATE>,
-				         <&clks IMX5_CLK_FEC_GATE>,
-				         <&clks IMX5_CLK_FEC_GATE>;
+					 <&clks IMX5_CLK_FEC_GATE>,
+					 <&clks IMX5_CLK_FEC_GATE>;
 				clock-names = "ipg", "ahb", "ptp";
 				status = "disabled";
 			};
diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index f46fe9b..d8efdab 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -130,8 +130,8 @@
 			reg = <0x40000000 0x20000000>;
 			interrupts = <11 10>;
 			clocks = <&clks IMX5_CLK_IPU_GATE>,
-			         <&clks IMX5_CLK_IPU_DI0_GATE>,
-			         <&clks IMX5_CLK_IPU_DI1_GATE>;
+				 <&clks IMX5_CLK_IPU_DI0_GATE>,
+				 <&clks IMX5_CLK_IPU_DI1_GATE>;
 			clock-names = "bus", "di0", "di1";
 			resets = <&src 2>;
 
@@ -169,8 +169,8 @@
 					reg = <0x70004000 0x4000>;
 					interrupts = <1>;
 					clocks = <&clks IMX5_CLK_ESDHC1_IPG_GATE>,
-					         <&clks IMX5_CLK_DUMMY>,
-					         <&clks IMX5_CLK_ESDHC1_PER_GATE>;
+						 <&clks IMX5_CLK_DUMMY>,
+						 <&clks IMX5_CLK_ESDHC1_PER_GATE>;
 					clock-names = "ipg", "ahb", "per";
 					status = "disabled";
 				};
@@ -180,8 +180,8 @@
 					reg = <0x70008000 0x4000>;
 					interrupts = <2>;
 					clocks = <&clks IMX5_CLK_ESDHC2_IPG_GATE>,
-					         <&clks IMX5_CLK_DUMMY>,
-					         <&clks IMX5_CLK_ESDHC2_PER_GATE>;
+						 <&clks IMX5_CLK_DUMMY>,
+						 <&clks IMX5_CLK_ESDHC2_PER_GATE>;
 					clock-names = "ipg", "ahb", "per";
 					bus-width = <4>;
 					status = "disabled";
@@ -192,7 +192,7 @@
 					reg = <0x7000c000 0x4000>;
 					interrupts = <33>;
 					clocks = <&clks IMX5_CLK_UART3_IPG_GATE>,
-					         <&clks IMX5_CLK_UART3_PER_GATE>;
+						 <&clks IMX5_CLK_UART3_PER_GATE>;
 					clock-names = "ipg", "per";
 					status = "disabled";
 				};
@@ -204,7 +204,7 @@
 					reg = <0x70010000 0x4000>;
 					interrupts = <36>;
 					clocks = <&clks IMX5_CLK_ECSPI1_IPG_GATE>,
-					         <&clks IMX5_CLK_ECSPI1_PER_GATE>;
+						 <&clks IMX5_CLK_ECSPI1_PER_GATE>;
 					clock-names = "ipg", "per";
 					status = "disabled";
 				};
@@ -229,8 +229,8 @@
 					reg = <0x70020000 0x4000>;
 					interrupts = <3>;
 					clocks = <&clks IMX5_CLK_ESDHC3_IPG_GATE>,
-					         <&clks IMX5_CLK_DUMMY>,
-					         <&clks IMX5_CLK_ESDHC3_PER_GATE>;
+						 <&clks IMX5_CLK_DUMMY>,
+						 <&clks IMX5_CLK_ESDHC3_PER_GATE>;
 					clock-names = "ipg", "ahb", "per";
 					bus-width = <4>;
 					status = "disabled";
@@ -241,8 +241,8 @@
 					reg = <0x70024000 0x4000>;
 					interrupts = <4>;
 					clocks = <&clks IMX5_CLK_ESDHC4_IPG_GATE>,
-					         <&clks IMX5_CLK_DUMMY>,
-					         <&clks IMX5_CLK_ESDHC4_PER_GATE>;
+						 <&clks IMX5_CLK_DUMMY>,
+						 <&clks IMX5_CLK_ESDHC4_PER_GATE>;
 					clock-names = "ipg", "ahb", "per";
 					bus-width = <4>;
 					status = "disabled";
@@ -364,7 +364,7 @@
 				reg = <0x73fa0000 0x4000>;
 				interrupts = <39>;
 				clocks = <&clks IMX5_CLK_GPT_IPG_GATE>,
-				         <&clks IMX5_CLK_GPT_HF_GATE>;
+					 <&clks IMX5_CLK_GPT_HF_GATE>;
 				clock-names = "ipg", "per";
 			};
 
@@ -378,7 +378,7 @@
 				compatible = "fsl,imx51-pwm", "fsl,imx27-pwm";
 				reg = <0x73fb4000 0x4000>;
 				clocks = <&clks IMX5_CLK_PWM1_IPG_GATE>,
-				         <&clks IMX5_CLK_PWM1_HF_GATE>;
+					 <&clks IMX5_CLK_PWM1_HF_GATE>;
 				clock-names = "ipg", "per";
 				interrupts = <61>;
 			};
@@ -388,7 +388,7 @@
 				compatible = "fsl,imx51-pwm", "fsl,imx27-pwm";
 				reg = <0x73fb8000 0x4000>;
 				clocks = <&clks IMX5_CLK_PWM2_IPG_GATE>,
-				         <&clks IMX5_CLK_PWM2_HF_GATE>;
+					 <&clks IMX5_CLK_PWM2_HF_GATE>;
 				clock-names = "ipg", "per";
 				interrupts = <94>;
 			};
@@ -398,7 +398,7 @@
 				reg = <0x73fbc000 0x4000>;
 				interrupts = <31>;
 				clocks = <&clks IMX5_CLK_UART1_IPG_GATE>,
-				         <&clks IMX5_CLK_UART1_PER_GATE>;
+					 <&clks IMX5_CLK_UART1_PER_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -408,7 +408,7 @@
 				reg = <0x73fc0000 0x4000>;
 				interrupts = <32>;
 				clocks = <&clks IMX5_CLK_UART2_IPG_GATE>,
-				         <&clks IMX5_CLK_UART2_PER_GATE>;
+					 <&clks IMX5_CLK_UART2_PER_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -456,7 +456,7 @@
 				reg = <0x83fac000 0x4000>;
 				interrupts = <37>;
 				clocks = <&clks IMX5_CLK_ECSPI2_IPG_GATE>,
-				         <&clks IMX5_CLK_ECSPI2_PER_GATE>;
+					 <&clks IMX5_CLK_ECSPI2_PER_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -466,7 +466,7 @@
 				reg = <0x83fb0000 0x4000>;
 				interrupts = <6>;
 				clocks = <&clks IMX5_CLK_SDMA_GATE>,
-				         <&clks IMX5_CLK_SDMA_GATE>;
+					 <&clks IMX5_CLK_SDMA_GATE>;
 				clock-names = "ipg", "ahb";
 				#dma-cells = <3>;
 				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx51.bin";
@@ -479,7 +479,7 @@
 				reg = <0x83fc0000 0x4000>;
 				interrupts = <38>;
 				clocks = <&clks IMX5_CLK_CSPI_IPG_GATE>,
-				         <&clks IMX5_CLK_CSPI_IPG_GATE>;
+					 <&clks IMX5_CLK_CSPI_IPG_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -582,8 +582,8 @@
 				reg = <0x83fec000 0x4000>;
 				interrupts = <87>;
 				clocks = <&clks IMX5_CLK_FEC_GATE>,
-				         <&clks IMX5_CLK_FEC_GATE>,
-				         <&clks IMX5_CLK_FEC_GATE>;
+					 <&clks IMX5_CLK_FEC_GATE>,
+					 <&clks IMX5_CLK_FEC_GATE>;
 				clock-names = "ipg", "ahb", "ptp";
 				status = "disabled";
 			};
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index 0777b41..88f9e09e 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -131,8 +131,8 @@
 			reg = <0x18000000 0x08000000>;
 			interrupts = <11 10>;
 			clocks = <&clks IMX5_CLK_IPU_GATE>,
-			         <&clks IMX5_CLK_IPU_DI0_GATE>,
-			         <&clks IMX5_CLK_IPU_DI1_GATE>;
+				 <&clks IMX5_CLK_IPU_DI0_GATE>,
+				 <&clks IMX5_CLK_IPU_DI1_GATE>;
 			clock-names = "bus", "di0", "di1";
 			resets = <&src 2>;
 
@@ -199,8 +199,8 @@
 					reg = <0x50004000 0x4000>;
 					interrupts = <1>;
 					clocks = <&clks IMX5_CLK_ESDHC1_IPG_GATE>,
-					         <&clks IMX5_CLK_DUMMY>,
-					         <&clks IMX5_CLK_ESDHC1_PER_GATE>;
+						 <&clks IMX5_CLK_DUMMY>,
+						 <&clks IMX5_CLK_ESDHC1_PER_GATE>;
 					clock-names = "ipg", "ahb", "per";
 					bus-width = <4>;
 					status = "disabled";
@@ -211,8 +211,8 @@
 					reg = <0x50008000 0x4000>;
 					interrupts = <2>;
 					clocks = <&clks IMX5_CLK_ESDHC2_IPG_GATE>,
-					         <&clks IMX5_CLK_DUMMY>,
-					         <&clks IMX5_CLK_ESDHC2_PER_GATE>;
+						 <&clks IMX5_CLK_DUMMY>,
+						 <&clks IMX5_CLK_ESDHC2_PER_GATE>;
 					clock-names = "ipg", "ahb", "per";
 					bus-width = <4>;
 					status = "disabled";
@@ -223,7 +223,7 @@
 					reg = <0x5000c000 0x4000>;
 					interrupts = <33>;
 					clocks = <&clks IMX5_CLK_UART3_IPG_GATE>,
-					         <&clks IMX5_CLK_UART3_PER_GATE>;
+						 <&clks IMX5_CLK_UART3_PER_GATE>;
 					clock-names = "ipg", "per";
 					dmas = <&sdma 42 4 0>, <&sdma 43 4 0>;
 					dma-names = "rx", "tx";
@@ -237,7 +237,7 @@
 					reg = <0x50010000 0x4000>;
 					interrupts = <36>;
 					clocks = <&clks IMX5_CLK_ECSPI1_IPG_GATE>,
-					         <&clks IMX5_CLK_ECSPI1_PER_GATE>;
+						 <&clks IMX5_CLK_ECSPI1_PER_GATE>;
 					clock-names = "ipg", "per";
 					status = "disabled";
 				};
@@ -264,8 +264,8 @@
 					reg = <0x50020000 0x4000>;
 					interrupts = <3>;
 					clocks = <&clks IMX5_CLK_ESDHC3_IPG_GATE>,
-					         <&clks IMX5_CLK_DUMMY>,
-					         <&clks IMX5_CLK_ESDHC3_PER_GATE>;
+						 <&clks IMX5_CLK_DUMMY>,
+						 <&clks IMX5_CLK_ESDHC3_PER_GATE>;
 					clock-names = "ipg", "ahb", "per";
 					bus-width = <4>;
 					status = "disabled";
@@ -276,8 +276,8 @@
 					reg = <0x50024000 0x4000>;
 					interrupts = <4>;
 					clocks = <&clks IMX5_CLK_ESDHC4_IPG_GATE>,
-					         <&clks IMX5_CLK_DUMMY>,
-					         <&clks IMX5_CLK_ESDHC4_PER_GATE>;
+						 <&clks IMX5_CLK_DUMMY>,
+						 <&clks IMX5_CLK_ESDHC4_PER_GATE>;
 					clock-names = "ipg", "ahb", "per";
 					bus-width = <4>;
 					status = "disabled";
@@ -419,7 +419,7 @@
 				reg = <0x53fa0000 0x4000>;
 				interrupts = <39>;
 				clocks = <&clks IMX5_CLK_GPT_IPG_GATE>,
-				         <&clks IMX5_CLK_GPT_HF_GATE>;
+					 <&clks IMX5_CLK_GPT_HF_GATE>;
 				clock-names = "ipg", "per";
 			};
 
@@ -440,11 +440,11 @@
 				reg = <0x53fa8008 0x4>;
 				gpr = <&gpr>;
 				clocks = <&clks IMX5_CLK_LDB_DI0_SEL>,
-				         <&clks IMX5_CLK_LDB_DI1_SEL>,
-				         <&clks IMX5_CLK_IPU_DI0_SEL>,
-				         <&clks IMX5_CLK_IPU_DI1_SEL>,
-				         <&clks IMX5_CLK_LDB_DI0_GATE>,
-				         <&clks IMX5_CLK_LDB_DI1_GATE>;
+					 <&clks IMX5_CLK_LDB_DI1_SEL>,
+					 <&clks IMX5_CLK_IPU_DI0_SEL>,
+					 <&clks IMX5_CLK_IPU_DI1_SEL>,
+					 <&clks IMX5_CLK_LDB_DI0_GATE>,
+					 <&clks IMX5_CLK_LDB_DI1_GATE>;
 				clock-names = "di0_pll", "di1_pll",
 					      "di0_sel", "di1_sel",
 					      "di0", "di1";
@@ -486,7 +486,7 @@
 				compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
 				reg = <0x53fb4000 0x4000>;
 				clocks = <&clks IMX5_CLK_PWM1_IPG_GATE>,
-				         <&clks IMX5_CLK_PWM1_HF_GATE>;
+					 <&clks IMX5_CLK_PWM1_HF_GATE>;
 				clock-names = "ipg", "per";
 				interrupts = <61>;
 			};
@@ -496,7 +496,7 @@
 				compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
 				reg = <0x53fb8000 0x4000>;
 				clocks = <&clks IMX5_CLK_PWM2_IPG_GATE>,
-				         <&clks IMX5_CLK_PWM2_HF_GATE>;
+					 <&clks IMX5_CLK_PWM2_HF_GATE>;
 				clock-names = "ipg", "per";
 				interrupts = <94>;
 			};
@@ -506,7 +506,7 @@
 				reg = <0x53fbc000 0x4000>;
 				interrupts = <31>;
 				clocks = <&clks IMX5_CLK_UART1_IPG_GATE>,
-				         <&clks IMX5_CLK_UART1_PER_GATE>;
+					 <&clks IMX5_CLK_UART1_PER_GATE>;
 				clock-names = "ipg", "per";
 				dmas = <&sdma 18 4 0>, <&sdma 19 4 0>;
 				dma-names = "rx", "tx";
@@ -518,7 +518,7 @@
 				reg = <0x53fc0000 0x4000>;
 				interrupts = <32>;
 				clocks = <&clks IMX5_CLK_UART2_IPG_GATE>,
-				         <&clks IMX5_CLK_UART2_PER_GATE>;
+					 <&clks IMX5_CLK_UART2_PER_GATE>;
 				clock-names = "ipg", "per";
 				dmas = <&sdma 12 4 0>, <&sdma 13 4 0>;
 				dma-names = "rx", "tx";
@@ -530,7 +530,7 @@
 				reg = <0x53fc8000 0x4000>;
 				interrupts = <82>;
 				clocks = <&clks IMX5_CLK_CAN1_IPG_GATE>,
-				         <&clks IMX5_CLK_CAN1_SERIAL_GATE>;
+					 <&clks IMX5_CLK_CAN1_SERIAL_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -540,7 +540,7 @@
 				reg = <0x53fcc000 0x4000>;
 				interrupts = <83>;
 				clocks = <&clks IMX5_CLK_CAN2_IPG_GATE>,
-				         <&clks IMX5_CLK_CAN2_SERIAL_GATE>;
+					 <&clks IMX5_CLK_CAN2_SERIAL_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -603,7 +603,7 @@
 				reg = <0x53ff0000 0x4000>;
 				interrupts = <13>;
 				clocks = <&clks IMX5_CLK_UART4_IPG_GATE>,
-				         <&clks IMX5_CLK_UART4_PER_GATE>;
+					 <&clks IMX5_CLK_UART4_PER_GATE>;
 				clock-names = "ipg", "per";
 				dmas = <&sdma 2 4 0>, <&sdma 3 4 0>;
 				dma-names = "rx", "tx";
@@ -635,7 +635,7 @@
 				reg = <0x63f90000 0x4000>;
 				interrupts = <86>;
 				clocks = <&clks IMX5_CLK_UART5_IPG_GATE>,
-				         <&clks IMX5_CLK_UART5_PER_GATE>;
+					 <&clks IMX5_CLK_UART5_PER_GATE>;
 				clock-names = "ipg", "per";
 				dmas = <&sdma 16 4 0>, <&sdma 17 4 0>;
 				dma-names = "rx", "tx";
@@ -656,7 +656,7 @@
 				reg = <0x63fac000 0x4000>;
 				interrupts = <37>;
 				clocks = <&clks IMX5_CLK_ECSPI2_IPG_GATE>,
-				         <&clks IMX5_CLK_ECSPI2_PER_GATE>;
+					 <&clks IMX5_CLK_ECSPI2_PER_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -666,7 +666,7 @@
 				reg = <0x63fb0000 0x4000>;
 				interrupts = <6>;
 				clocks = <&clks IMX5_CLK_SDMA_GATE>,
-				         <&clks IMX5_CLK_SDMA_GATE>;
+					 <&clks IMX5_CLK_SDMA_GATE>;
 				clock-names = "ipg", "ahb";
 				#dma-cells = <3>;
 				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx53.bin";
@@ -679,7 +679,7 @@
 				reg = <0x63fc0000 0x4000>;
 				interrupts = <38>;
 				clocks = <&clks IMX5_CLK_CSPI_IPG_GATE>,
-				         <&clks IMX5_CLK_CSPI_IPG_GATE>;
+					 <&clks IMX5_CLK_CSPI_IPG_GATE>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};
@@ -755,8 +755,8 @@
 				reg = <0x63fec000 0x4000>;
 				interrupts = <87>;
 				clocks = <&clks IMX5_CLK_FEC_GATE>,
-				         <&clks IMX5_CLK_FEC_GATE>,
-				         <&clks IMX5_CLK_FEC_GATE>;
+					 <&clks IMX5_CLK_FEC_GATE>,
+					 <&clks IMX5_CLK_FEC_GATE>;
 				clock-names = "ipg", "ahb", "ptp";
 				status = "disabled";
 			};
@@ -766,7 +766,7 @@
 				reg = <0x63ff0000 0x1000>;
 				interrupts = <92>;
 				clocks = <&clks IMX5_CLK_TVE_GATE>,
-				         <&clks IMX5_CLK_IPU_DI1_SEL>;
+					 <&clks IMX5_CLK_IPU_DI1_SEL>;
 				clock-names = "tve", "di_sel";
 				status = "disabled";
 
@@ -782,7 +782,7 @@
 				reg = <0x63ff4000 0x1000>;
 				interrupts = <9>;
 				clocks = <&clks IMX5_CLK_VPU_REFERENCE_GATE>,
-				         <&clks IMX5_CLK_VPU_GATE>;
+					 <&clks IMX5_CLK_VPU_GATE>;
 				clock-names = "per", "ahb";
 				resets = <&src 1>;
 				iram = <&ocram>;
@@ -793,7 +793,7 @@
 				reg = <0x63ff8000 0x4000>;
 				interrupts = <19 20>;
 				clocks = <&clks IMX5_CLK_SAHARA_IPG_GATE>,
-				         <&clks IMX5_CLK_SAHARA_IPG_GATE>;
+					 <&clks IMX5_CLK_SAHARA_IPG_GATE>;
 				clock-names = "ipg", "ahb";
 			};
 		};
diff --git a/arch/arm/boot/dts/imx6dl-riotboard.dts b/arch/arm/boot/dts/imx6dl-riotboard.dts
index 75d7343..2cb7282 100644
--- a/arch/arm/boot/dts/imx6dl-riotboard.dts
+++ b/arch/arm/boot/dts/imx6dl-riotboard.dts
@@ -390,7 +390,7 @@
 				MX6QDL_PAD_RGMII_RD3__RGMII_RD3		0x1b030		/* AR8035 pin strapping: MODE#3: pull up */
 				MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL	0x130b0		/* AR8035 pin strapping: MODE#0: pull down */
 				MX6QDL_PAD_GPIO_16__ENET_REF_CLK	0x4001b0a8	/* GPIO16 -> AR8035 25MHz */
-			        MX6QDL_PAD_EIM_D31__GPIO3_IO31		0x130b0		/* RGMII_nRST */
+				MX6QDL_PAD_EIM_D31__GPIO3_IO31		0x130b0		/* RGMII_nRST */
 				MX6QDL_PAD_ENET_TX_EN__GPIO1_IO28	0x180b0		/* AR8035 interrupt */
 				MX6QDL_PAD_GPIO_6__ENET_IRQ		0x000b1
 			>;
diff --git a/arch/arm/boot/dts/imx6dl-tx6dl-comtft.dts b/arch/arm/boot/dts/imx6dl-tx6dl-comtft.dts
index 063fe75..aac42ac 100644
--- a/arch/arm/boot/dts/imx6dl-tx6dl-comtft.dts
+++ b/arch/arm/boot/dts/imx6dl-tx6dl-comtft.dts
@@ -105,7 +105,7 @@
 				pixelclk-active = <1>;
 			};
 		};
-        };
+	};
 };
 
 &can1 {
diff --git a/arch/arm/boot/dts/imx6dl-tx6u-801x.dts b/arch/arm/boot/dts/imx6dl-tx6u-801x.dts
index b7a7284..d1f1298 100644
--- a/arch/arm/boot/dts/imx6dl-tx6u-801x.dts
+++ b/arch/arm/boot/dts/imx6dl-tx6u-801x.dts
@@ -199,7 +199,7 @@
 				pixelclk-active = <0>;
 			};
 		};
-        };
+	};
 };
 
 &ipu1_di0_disp0 {
diff --git a/arch/arm/boot/dts/imx6q-phytec-pbab01.dts b/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
index c139ac0..1f47713 100644
--- a/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
+++ b/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
@@ -23,5 +23,5 @@
 };
 
 &sata {
-        status = "okay";
+	status = "okay";
 };
diff --git a/arch/arm/boot/dts/imx6q-tx6q-1010-comtft.dts b/arch/arm/boot/dts/imx6q-tx6q-1010-comtft.dts
index 65e95ae..71746ed 100644
--- a/arch/arm/boot/dts/imx6q-tx6q-1010-comtft.dts
+++ b/arch/arm/boot/dts/imx6q-tx6q-1010-comtft.dts
@@ -105,7 +105,7 @@
 				pixelclk-active = <1>;
 			};
 		};
-        };
+	};
 };
 
 &can1 {
diff --git a/arch/arm/boot/dts/imx6q-tx6q-1010.dts b/arch/arm/boot/dts/imx6q-tx6q-1010.dts
index 20cd0e7..f9cd21a 100644
--- a/arch/arm/boot/dts/imx6q-tx6q-1010.dts
+++ b/arch/arm/boot/dts/imx6q-tx6q-1010.dts
@@ -199,7 +199,7 @@
 				pixelclk-active = <0>;
 			};
 		};
-        };
+	};
 };
 
 &ipu1_di0_disp0 {
diff --git a/arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts b/arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts
index 9ed243b..959ff3fb 100644
--- a/arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts
+++ b/arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts
@@ -105,7 +105,7 @@
 				pixelclk-active = <1>;
 			};
 		};
-        };
+	};
 };
 
 &can1 {
diff --git a/arch/arm/boot/dts/imx6q-tx6q-1020.dts b/arch/arm/boot/dts/imx6q-tx6q-1020.dts
index 347b531..b49133d 100644
--- a/arch/arm/boot/dts/imx6q-tx6q-1020.dts
+++ b/arch/arm/boot/dts/imx6q-tx6q-1020.dts
@@ -199,7 +199,7 @@
 				pixelclk-active = <0>;
 			};
 		};
-        };
+	};
 };
 
 &ds1339 {
diff --git a/arch/arm/boot/dts/imx6sx-sdb.dtsi b/arch/arm/boot/dts/imx6sx-sdb.dtsi
index 9d70cfd..7327bcd 100644
--- a/arch/arm/boot/dts/imx6sx-sdb.dtsi
+++ b/arch/arm/boot/dts/imx6sx-sdb.dtsi
@@ -192,10 +192,10 @@
 };
 
 &i2c4 {
-        clock-frequency = <100000>;
-        pinctrl-names = "default";
-        pinctrl-0 = <&pinctrl_i2c4>;
-        status = "okay";
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c4>;
+	status = "okay";
 
 	codec: wm8962 at 1a {
 		compatible = "wlf,wm8962";
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 9526c38..bd9fe67 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -858,7 +858,7 @@
 				fsl,num-tx-queues=<3>;
 				fsl,num-rx-queues=<3>;
 				status = "disabled";
-                        };
+			};
 
 			mlb: mlb at 0218c000 {
 				reg = <0x0218c000 0x4000>;
@@ -1181,7 +1181,7 @@
 				fsl,adck-max-frequency = <30000000>, <40000000>,
 							 <20000000>;
 				status = "disabled";
-                        };
+			};
 
 			adc2: adc at 02284000 {
 				compatible = "fsl,imx6sx-adc", "fsl,vf610-adc";
@@ -1192,7 +1192,7 @@
 				fsl,adck-max-frequency = <30000000>, <40000000>,
 							 <20000000>;
 				status = "disabled";
-                        };
+			};
 
 			wdog3: wdog at 02288000 {
 				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size
From: Mark Brown @ 2016-10-26 10:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477465231-17704-1-git-send-email-mr.nuke.me@gmail.com>

On Wed, Oct 26, 2016 at 12:00:30AM -0700, Alexandru Gagniuc wrote:
> This is the seventh attempt to get this patch in. I was prompted to look
> into this again as someone recently remarked:

Please don't send cover letters for single patches, if there is anything
that needs saying put it in the changelog of the patch or after the ---
if it's administrative stuff.  This reduces mail volume and ensures that 
any important information is recorded in the changelog rather than being
lost. 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161026/c5b06fb0/attachment.sig>

^ permalink raw reply

* [PATCH v2] modversions: treat symbol CRCs as 32 bit quantities on 64 bit archs
From: Michael Ellerman @ 2016-10-26 10:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476956263-1787-1-git-send-email-ard.biesheuvel@linaro.org>

Hi Ard,

I like the concept, but ...

Ard Biesheuvel <ard.biesheuvel@linaro.org> writes:
> The symbol CRCs are emitted as ELF symbols, which allows us to easily
> populate the kcrctab sections by relying on the linker to associate
> each kcrctab slot with the correct value.
>
> This has two downsides:
> - given that the CRCs are treated as pointers, we waste 4 bytes for
>   each CRC on 64 bit architectures,
> - on architectures that support runtime relocation, a relocation entry is
>   emitted for each CRC value, which may take up 24 bytes of __init space
>   (on ELF64 systems)
>
> This comes down to a x8 overhead in [uncompressed] kernel size. In addition,
> each relocation has to be reverted before the CRC value can be used.
>
> Switching to explicit 32 bit values on 64 bit architectures fixes both
> issues, since 32 bit values are not treated as relocatable quantities on
> ELF64 systems, even if the value ultimately resolves to a linker supplied
> value.

Are we sure that part is true? ("not treated as relocatable")

A quick test build on powerpc gives me:

  WARNING: 6829 bad relocations
  c000000000ca3748 R_PPC64_ADDR16    *ABS*+0x0000000013f53da6
  c000000000ca374a R_PPC64_ADDR16    *ABS*+0x00000000f7272059
  c000000000ca374c R_PPC64_ADDR16    *ABS*+0x0000000002013d36
  c000000000ca374e R_PPC64_ADDR16    *ABS*+0x00000000a59dffc8
  ...

Which is coming from our relocs_check.sh script, which checks that the
generated relocations are ones we know how to handle.

And when I try to boot it I get:

  virtio: disagrees about version of symbol module_layout
  virtio: disagrees about version of symbol module_layout
  scsi_mod: disagrees about version of symbol module_layout

And it can't find my root file system (unsurprisingly as it's on scsi).

Will try and investigate more tomorrow.

cheers

^ permalink raw reply

* [linux-sunxi] [PATCH RESEND 1/2] dt: bindings: add allwinner,otg-routed property for phy-sun4i-usb
From: Hans de Goede @ 2016-10-26 10:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4534561477471963@web8g.yandex.ru>

Hi,

On 26-10-16 10:52, Icenowy Zheng wrote:
>
>
> 26.10.2016, 16:28, "Hans de Goede" <hdegoede@redhat.com>:
>> Hi,
>>
>> On 25-10-16 06:11, Icenowy Zheng wrote:
>>>  On some newer Allwinner SoCs (H3 or A64), the PHY0 can be either routed to
>>>  the MUSB controller (which is an OTG controller) or the OHCI/EHCI pair
>>>  (which is a Host-only controller, but more stable and easy to implement).
>>>
>>>  This property marks whether on a certain board which controller should be
>>>  attached to the PHY.
>>>
>>>  Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>>
>> Icenowy, I appreciate your work on this, but we really need full otg
>> support with dynamic switching rather then hardwiring the routing, so
>> this cannot go in as is.
>
> Now I have both PHY0 controllers' drivers.
>
> In the tree of https://github.com/Icenowy/linux/tree/ice-a64-v6.1 , I have already
> enabled MUSB controller.
>
> And this patchset is for those prefer a stable USB host implement to dual-role
> implementation. MUSB is a good UDC, but not a good host controller. My USB
> sound card cannot work on MUSB on A33. Even connecting a R8's MUSB (Serial
> Gadget) to an A33's MUSB cannot work.

The idea is for dual-role setups to used the MUSB in gadget mode and the EHCI/OHCI
pair when in host mode. So for otg setups you would runtime change the mux
from one controller to the other based on the id pin value.

Take a look at drivers/phy/phy-sun4i-usb.c, around line 512:

	if (id_det != data->id_det) {
		...
	}

This deals with id_det changes (including the initial id_det "change"
for hardwired host-only ports). This currently assumes that the musb
will be used for host mode too, we will want to change this to
something like this:

	if (id_det != data->id_det) {
		if (data->cfg->separate_phy0_host_controller) {
			if (id_det) {
				/* Change to gadget mode (id_det == 1), switch phy mux to musb */
				actual code to switch phy mux to musb...
			} else {
				/* Change to host mode (id_det == 0), switch phy mux to ehci/ohci */
				actual code to switch phy mux to ehci/ohci...
			}
		}
		/* old code */
	}

Note this will then still rely on the musb code to actually turn
the regulator on, so you do need to have the musb driver build and
loaded. This can be fixed but lets start with the above.

If you combine this with dr_mode = "host"; in the dts, then
sun4i_usb_phy0_get_id_det() will return 0 so on its first run
sun4i_usb_phy0_id_vbus_det_scan() will throw the mux to the ehci/ohci
and everything should work as you want without needing the custom
"allwinner,otg-routed" property, and we should be more or less
ready to support full otg on other boards.

Regards,

Hans





>
> See the IRC log between Andre and me,
> https://irclog.whitequark.org/linux-sunxi/2016-10-24#18012695; .
>
>>
>> NACK.
>>
>> Regards,
>>
>> Hans
>>
>>>  ---
>>>   Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>>  diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
>>>  index 287150d..a63c766 100644
>>>  --- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
>>>  +++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
>>>  @@ -36,6 +36,12 @@ Optional properties:
>>>   - usb1_vbus-supply : regulator phandle for controller usb1 vbus
>>>   - usb2_vbus-supply : regulator phandle for controller usb2 vbus
>>>
>>>  +Optional properties for H3 or A64 SoCs:
>>>  +- allwinner,otg-routed : USB0 (OTG) PHY is routed to OHCI/EHCI pair rather than
>>>  + MUSB. (boolean, if this property is set, the OHCI/EHCI
>>>  + controllers at PHY0 should be enabled and the MUSB
>>>  + controller must *NOT* be enabled)
>>>  +
>>>   Example:
>>>           usbphy: phy at 0x01c13400 {
>>>                   #phy-cells = <1>;

^ permalink raw reply

* [PATCH v6 4/5] ARM: DTS: da850: Add cfgchip syscon node
From: Sekhar Nori @ 2016-10-26 10:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477451211-31979-5-git-send-email-david@lechnology.com>

On Wednesday 26 October 2016 08:36 AM, David Lechner wrote:
> Add a syscon node for the SoC CFGCHIPn registers. This is needed for
> the new usb phy driver.
> 
> Signed-off-by: David Lechner <david@lechnology.com>

Applied to v4.10/dt. Changed "DTS" in subject line to small case. ARM
device-tree patches have been following that.

Thanks
Sekhar

^ permalink raw reply

* [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI
From: Mark Rutland @ 2016-10-26 10:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <VI1PR04MB161582C628C5EA4CD08B15C6E8AB0@VI1PR04MB1615.eurprd04.prod.outlook.com>

On Wed, Oct 26, 2016 at 06:55:22AM +0000, M.H. Lian wrote:
> Hi Robin,
> 
> Please see my comments inline.
> 
> Thanks,
> Minghuan
> 
> > -----Original Message-----
> > From: Robin Murphy [mailto:robin.murphy at arm.com]
> > Sent: Tuesday, October 25, 2016 9:01 PM
> > To: M.H. Lian <minghuan.lian@nxp.com>; linux-arm-
> > kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> > devicetree at vger.kernel.org
> > Cc: Marc Zyngier <marc.zyngier@arm.com>; Stuart Yoder
> > <stuart.yoder@nxp.com>; Leo Li <leoyang.li@nxp.com>; Scott Wood
> > <scott.wood@nxp.com>; Shawn Guo <shawnguo@kernel.org>; Mingkai Hu
> > <mingkai.hu@nxp.com>
> > Subject: Re: [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG
> > MSI
> > 
> > On 25/10/16 13:35, Minghuan Lian wrote:
> > > 1. The different version of a SoC may have different MSI
> > > implementation. But compatible "fsl,<soc-name>-msi" can not describe
> > > the SoC version.
> > 
> > Can't it?
> > 
> > 	compatible = "fsl-ls1043a-rev11-msi";
> > 
> > Oh, I guess it can!
> > 
> > Joking aside, if there are multiple versions of a piece of hardware which
> > require *different* treatment by drivers, then it is obviously wrong to use
> > the same compatible string because *they are not compatible*.
> > 
> [Minghuan Lian]  Yes, but Rev1.0 and Rev1.1 SoC will use the same dts files.
> We cannot create different dts files for each revision of the same kind of SoC.

... why?

The DT should describe the hardware; if hardware differs then it should
have a different DT.

> It means that there are different variants in the different versions
> of the same SoC that will use the same compatible string.

Why can you not add a string for each variant, in addition to the SoC
string? We do that elsewhere.

> So I have to use SoC match interface to get the versions.
> 
> I'm too radical. I do not want to first check SoC family via
> compatible string and then check revision via SoC match or SVR.

You can have *both* in the the compatible string list, e.g. at the top
level:

	compatible = "vendor,soc-rev", "vendor-soc";

For devices which differ, this can be encoded similarly in the device
compatible string list.

> I selected the "SoC match" like the following to get the related information at only one place. 
> 
> static struct soc_device_attribute soc_msi_matches[] = {
> 	{ .family = "QorIQ LS1021A",
> 	  .data = &ls1021_msi_cfg },
> 	{ .family = "QorIQ LS1012A",
> 	  .data = &ls1021_msi_cfg },
> 	{ .family = "QorIQ LS1043A", .revision = "1.0",
> 	  .data = &ls1021_msi_cfg },
> 	{ .family = "QorIQ LS1043A", .revision = "1.1",
> 	  .data = &ls1043_rev11_msi_cfg },
> 	{ .family = "QorIQ LS1046A",
> 	  .data = &ls1046_msi_cfg },
> 	{ },
> };
> 
> I will remain the SoC related compatible and try to describe the
> difference via some kind of the property.

As commented on the driver side, this should be described with DT
properties on the devices.

Thanks,
Mark.

^ permalink raw reply

* [PATCH v6 5/5] ARM: DTS: da850: Add usb phy node
From: Sekhar Nori @ 2016-10-26 10:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477451211-31979-6-git-send-email-david@lechnology.com>

On Wednesday 26 October 2016 08:36 AM, David Lechner wrote:
> Add a node for the new usb phy driver.

changed this to:

    Add a node for usb phy device. This device
    controls both the USB 1.1 and USB 2.0 PHYs.

mainly because the node is for the device, not the driver.

> 
> Signed-off-by: David Lechner <david@lechnology.com>

Applied to v4.10/dt

Thanks,
Sekhar

^ permalink raw reply

* [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI
From: Mark Rutland @ 2016-10-26 10:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477398945-22774-1-git-send-email-Minghuan.Lian@nxp.com>

On Tue, Oct 25, 2016 at 08:35:40PM +0800, Minghuan Lian wrote:
> 1. The different version of a SoC may have different MSI
> implementation. But compatible "fsl,<soc-name>-msi" can not describe
> the SoC version. 

Surely, "fsl,<soc-name>-<rev>-msi" can describe this?

If the hardware differs, it needs a new compatible string.

If there's some configuration value that varies across revisions (e.g.
number of slots), you can add a proeprty to describe that explciitly.

> The MSI driver will use SoC match interface to get
> SoC type and version instead of compatible string. So all MSI node
> can use the common compatible "fsl,ls-scfg-msi" and the original
> compatible is unnecessary.
> 
> 2. Layerscape SoCs may have one or several MSI controllers.
> In order to increase MSI interrupt number of a PCIe, the patch
> moves all MSI node into the parent node "msi-controller". So a
> PCIe can request MSI from all the MSI controllers.

This is not necessary, and does not represent a real block of hardware.
So NAK for this approach.

The msi-parent property can contain a list of MSI controllers. See the
examples in
Documentation/devicetree/bindings/interrupt-controller/msi.txt.
Likewise, the msi-map property can map to a number of MSI controllers.

If the core code can only consider one at a time, then that's an issue
to be addressed in core code, not one to be bodged around in bindings.

> 
> Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
> ---
>  .../interrupt-controller/fsl,ls-scfg-msi.txt       | 57 +++++++++++++++++++---
>  1 file changed, 49 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> index 9e38949..29f95fd 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> @@ -1,18 +1,28 @@
>  * Freescale Layerscape SCFG PCIe MSI controller
>  
> +Layerscape SoCs may have one or multiple MSI controllers.
> +Each MSI controller must be showed as a child node.
> +
>  Required properties:
>  
> -- compatible: should be "fsl,<soc-name>-msi" to identify
> -	      Layerscape PCIe MSI controller block such as:
> -              "fsl,1s1021a-msi"
> -              "fsl,1s1043a-msi"
> +- compatible: should be "fsl,ls-scfg-msi"

This breaks old DTBs, and throws away information which you describe
above as valuable. So another NAK for that.

> +- #address-cells: must be 2
> +- #size-cells: must be 2
> +- ranges: allows valid 1:1 translation between child's address space and
> +	  parent's address space
>  - msi-controller: indicates that this is a PCIe MSI controller node
> +
> +Required child node:
> +A child node must exist to represent the MSI controller.
> +The following are properties specific to those nodes:

Also, as above, the approach of gathering MSI controllers in this manner
is wrong.

Thanks,
Mark.

^ permalink raw reply

* [PATCH 3/6] arm64: dts: ls1043a: update MSI and PCIe node
From: Mark Rutland @ 2016-10-26 10:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477398945-22774-3-git-send-email-Minghuan.Lian@nxp.com>

On Tue, Oct 25, 2016 at 08:35:42PM +0800, Minghuan Lian wrote:
> 3. The rev1.1 of LS1043a moves PCIe INTB/C/D interrupts to MSI controller.

[...]

> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> index 41e5dc1..5295bb9 100644

> -			interrupt-map = <0000 0 0 1 &gic 0 110 0x4>,
> -					<0000 0 0 2 &gic 0 111 0x4>,
> -					<0000 0 0 3 &gic 0 112 0x4>,
> -					<0000 0 0 4 &gic 0 113 0x4>;
> +			interrupt-map = <0000 0 0 1 &gic 0 110 0x4>;

[...]

> -			interrupt-map = <0000 0 0 1 &gic 0 120  0x4>,
> -					<0000 0 0 2 &gic 0 121 0x4>,
> -					<0000 0 0 3 &gic 0 122 0x4>,
> -					<0000 0 0 4 &gic 0 123 0x4>;
> +			interrupt-map = <0000 0 0 1 &gic 0 120  0x4>;
>  		};

[...]

> -			interrupt-map = <0000 0 0 1 &gic 0 154 0x4>,
> -					<0000 0 0 2 &gic 0 155 0x4>,
> -					<0000 0 0 3 &gic 0 156 0x4>,
> -					<0000 0 0 4 &gic 0 157 0x4>;
> +			interrupt-map = <0000 0 0 1 &gic 0 154 0x4>;

Per the description above, this breaks revisions prior to 1.1.

Please, split the rev 1.1 changes into a new DTS. Share the common parts
in a common dtsi.

Thanks,
Mark.

^ permalink raw reply

* [PATCH 5/6] arm64: dts: ls1043a: update gic dts node
From: Mark Rutland @ 2016-10-26 10:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477398945-22774-5-git-send-email-Minghuan.Lian@nxp.com>

On Tue, Oct 25, 2016 at 08:35:44PM +0800, Minghuan Lian wrote:
> From: Gong Qianyu <Qianyu.Gong@nxp.com>
> 
> In order to support kvm, rev1.1 LS1043a GIC register has been
> changed to align as 64K. The patch updates GIC node according to
> the rev1.1 hardware.
> 
> Signed-off-by: Gong Qianyu <Qianyu.Gong@nxp.com>
> Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> index 5295bb9..da1809d 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> @@ -144,10 +144,10 @@
>  		compatible = "arm,gic-400";
>  		#interrupt-cells = <3>;
>  		interrupt-controller;
> -		reg = <0x0 0x1401000 0 0x1000>, /* GICD */
> -		      <0x0 0x1402000 0 0x2000>, /* GICC */
> -		      <0x0 0x1404000 0 0x2000>, /* GICH */
> -		      <0x0 0x1406000 0 0x2000>; /* GICV */
> +		reg = <0x0 0x1410000 0 0x10000>, /* GICD */
> +		      <0x0 0x1420000 0 0x20000>, /* GICC */
> +		      <0x0 0x1440000 0 0x20000>, /* GICH */
> +		      <0x0 0x1460000 0 0x20000>; /* GICV */

... this breaks HW prior to rev1.1, then.

Thanks,
Mark.

^ permalink raw reply

* [PATCH] ARM: davinci: da850: Fix pwm name matching
From: Sekhar Nori @ 2016-10-26 10:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477418044-24603-1-git-send-email-david@lechnology.com>

On Tuesday 25 October 2016 11:24 PM, David Lechner wrote:
> This fixes pwm name matching for DA850 familiy devices. When using device
> tree, the da850_auxdata_lookup[] table caused pwm devices to have the exact
> same name, which caused errors when trying to register the devices.
> 
> The names for clock matching in da850_clks[] also have to be updated to
> to exactly match in order for the clock lookup to work correctly.
> 
> Signed-off-by: David Lechner <david@lechnology.com>

Applied to "fixes" branch. Will send upstream after testing a bit and
also waiting to see if anyone else has any comments.

Thanks,
Sekhar

^ permalink raw reply

* [PATCH] iommu/arm-smmu: Expunge redundant iommu_present() checks
From: Robin Murphy @ 2016-10-26 10:37 UTC (permalink / raw)
  To: linux-arm-kernel

The first thing bus_set_iommu() does is check if iommu_ops are already
installed on the given bus, and immediately return -EBUSY if so. Since
the return value makes no difference as we ignore it anyway, there is
no need to redundantly duplicate that check by explicitly calling
iommu_present() beforehand.

This does bring the slight change that we may now end up calling
pci_request_acs() multiple times, but as that does nothing but set a
variable to 1, the impact should be effectively zero.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/arm-smmu.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index c841eb7a1a74..ef978db2bb54 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2000,17 +2000,13 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	arm_smmu_device_reset(smmu);
 
 	/* Oh, for a proper bus abstraction */
-	if (!iommu_present(&platform_bus_type))
-		bus_set_iommu(&platform_bus_type, &arm_smmu_ops);
+	bus_set_iommu(&platform_bus_type, &arm_smmu_ops);
 #ifdef CONFIG_ARM_AMBA
-	if (!iommu_present(&amba_bustype))
-		bus_set_iommu(&amba_bustype, &arm_smmu_ops);
+	bus_set_iommu(&amba_bustype, &arm_smmu_ops);
 #endif
 #ifdef CONFIG_PCI
-	if (!iommu_present(&pci_bus_type)) {
-		pci_request_acs();
-		bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
-	}
+	pci_request_acs();
+	bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
 #endif
 	return 0;
 }
-- 
1.9.1

^ permalink raw reply related

* [PATCH v14 1/9] clocksource/drivers/arm_arch_timer: Move enums and defines to header file
From: Mark Rutland @ 2016-10-26 10:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CADyBb7shWfCQycmo0GcheX2NSsrjYBpzdtYFqymvDNCKi0QM2A@mail.gmail.com>

On Wed, Oct 26, 2016 at 04:31:55PM +0800, Fu Wei wrote:
> On 20 October 2016 at 22:45, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Thu, Sep 29, 2016 at 02:17:09AM +0800, fu.wei at linaro.org wrote:
> >>  #include <linux/timecounter.h>
> >>  #include <linux/types.h>
> >>
> >> +#define ARCH_CP15_TIMER                      BIT(0)
> >> +#define ARCH_MEM_TIMER                       BIT(1)
> >
> > If we're going to expose these in a header, it would be better to rename
> > them to something that makes their usage/meaning clear. These should
> > probably be ARCH_TIMER_TYPE_{CP15,MEM}.

> > With those changes (regardless of the ARCH_TIMER_TYPE_* bits):
> >
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> 
> For ARCH_TIMER_TYPE_*, maybe I should add a patch at the end of this
> patchset to fix it, OK ?

Sure. If you could put that *earlier* in the patchset it would be
preferable so as to minimize churn.

Thanks,
Mark.

^ permalink raw reply

* [PATCH 2/5] ARM: davinci: Don't append git rev to local version
From: Sekhar Nori @ 2016-10-26 10:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <bf8cac01-910d-5afa-e840-f0d7ee96d22a@lechnology.com>

On Monday 24 October 2016 08:45 PM, David Lechner wrote:
> On 10/24/2016 06:35 AM, Sekhar Nori wrote:
>> On Saturday 22 October 2016 12:06 AM, David Lechner wrote:
>>> In the davinci default configuration, don't append the git revision to
>>> the local kernel version by. This seems like the more desirable default
>>> value.
>>
>> Why? To the contrary I actually quite like the fact that the git commit
>> is appended to version string. Makes it easy for me to cross-check that
>> I am booting the right image.
>>
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>
>> Thanks,
>> Sekhar
>>
> 
> Each time you make a commit, you get a new version, which installs
> another copy of the kernel modules on the device. This will fill up the
> SD card if you are making many commits.

Right, but thats easily fixable by removing existing modules before
installing new ones.

> Also, if someone wants to build the mainline kernel using the default
> configuration, it seems odd to have a git revision tacked on to the end
> even though you made no revisions.

If you checkout a tag and build, then no commit information is added.
Which I guess is what most end users will do.

I don't see this done in other defconfigs like omap2plus and multi_v7 as
well. I would like to keep it similar for davinci.

Thanks,
Sekhar

^ permalink raw reply

* [PATCH v14 1/9] clocksource/drivers/arm_arch_timer: Move enums and defines to header file
From: Fu Wei @ 2016-10-26 10:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161026105144.GF19965@leverpostej>

Hi Mark

On 26 October 2016 at 18:51, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Oct 26, 2016 at 04:31:55PM +0800, Fu Wei wrote:
>> On 20 October 2016 at 22:45, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Thu, Sep 29, 2016 at 02:17:09AM +0800, fu.wei at linaro.org wrote:
>> >>  #include <linux/timecounter.h>
>> >>  #include <linux/types.h>
>> >>
>> >> +#define ARCH_CP15_TIMER                      BIT(0)
>> >> +#define ARCH_MEM_TIMER                       BIT(1)
>> >
>> > If we're going to expose these in a header, it would be better to rename
>> > them to something that makes their usage/meaning clear. These should
>> > probably be ARCH_TIMER_TYPE_{CP15,MEM}.
>
>> > With those changes (regardless of the ARCH_TIMER_TYPE_* bits):
>> >
>> > Acked-by: Mark Rutland <mark.rutland@arm.com>
>>
>> For ARCH_TIMER_TYPE_*, maybe I should add a patch at the end of this
>> patchset to fix it, OK ?
>
> Sure. If you could put that *earlier* in the patchset it would be
> preferable so as to minimize churn.

OK, NP, will do

>
> Thanks,
> Mark.



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat

^ permalink raw reply

* [PATCH v6 00/16] ACPI IORT ARM SMMU support
From: Lorenzo Pieralisi @ 2016-10-26 11:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161018160414.1228-1-lorenzo.pieralisi@arm.com>

Rafael, Joerg (and anyone else CC'ed),

On Tue, Oct 18, 2016 at 05:03:58PM +0100, Lorenzo Pieralisi wrote:
> This patch series is v6 of a previous posting:
> 
> https://lkml.org/lkml/2016/9/9/418
> 
> v5 -> v6
> 	- Rebased against v4.9-rc1
> 	- Changed FWNODE_IOMMU to FWNODE_ACPI_STATIC
> 	- Moved platform devices creation into IORT code
> 	- Updated fwnode handling
> 	- Added default dma masks initialization

Any comments on v6 ? Patches touching generic ACPI code
are {1, 2, 7}, patch 4 updates the IOMMU of_iommu_{set/get}_ops()
API to make it work on ACPI systems too, by replacing the
device_node with a fwnode_handle pointer as look-up token;
the remainder of patches are ARM specific and creates the
infrastructure to probe ARM SMMU devices through ACPI,
ARM IORT table in particular. Given the generic bits changes
above I would not leave it to late -rc to reach an agreement
please, thank you.

Cheers,
Lorenzo

> v4 -> v5
> 	- Added SMMUv1/v2 support
> 	- Rebased against v4.8-rc5 and dependencies series
> 	- Consolidated IORT platform devices creation
> 
> v3 -> v4
> 	- Added single mapping API (for IORT named components)
> 	- Fixed arm_smmu_iort_xlate() return value
> 	- Reworked fwnode registration and platform device creation
> 	  ordering to fix probe ordering dependencies
> 	- Added code to keep device_node ref count with new iommu
> 	  fwspec API
> 	- Added patch to make iommu_fwspec arch agnostic
> 	- Dropped RFC status
> 	- Rebased against v4.8-rc2
> 
> v2 -> v3
> 	- Rebased on top of dependencies series [1][2][3](v4.7-rc3)
> 	- Added back reliance on ACPI early probing infrastructure
> 	- Patch[1-3] merged through other dependent series
> 	- Added back IOMMU fwnode generalization
> 	- Move SMMU v3 static functions configuration to IORT code
> 	- Implemented generic IOMMU fwspec API
> 	- Added code to implement fwnode platform device look-up
> 
> v1 -> v2:
> 	- Rebased on top of dependencies series [1][2][3](v4.7-rc1)
> 	- Removed IOMMU fwnode generalization
> 	- Implemented ARM SMMU v3 ACPI probing instead of ARM SMMU v2
> 	  owing to patch series dependencies [1]
> 	- Moved platform device creation logic to IORT code to
> 	  generalize its usage for ARM SMMU v1-v2-v3 components
> 	- Removed reliance on ACPI early device probing
> 	- Created IORT specific iommu_xlate() translation hook leaving
> 	  OF code unchanged according to v1 reviews
> 
> The ACPI IORT table provides information that allows instantiating
> ARM SMMU devices and carrying out id mappings between components on
> ARM based systems (devices, IOMMUs, interrupt controllers).
> 
> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf
> 
> Building on basic IORT support, this patchset enables ARM SMMUs support
> on ACPI systems.
> 
> Most of the code is aimed at building the required generic ACPI
> infrastructure to create and enable IOMMU components and to bring
> the IOMMU infrastructure for ACPI on par with DT, which is going to
> make future ARM SMMU components easier to integrate.
> 
> PATCH (1) adds a FWNODE_ACPI_STATIC type to the struct fwnode_handle type.
>           It is required to attach a fwnode identifier to platform
>           devices allocated/detected through static ACPI table entries
>           (ie IORT tables entries).
>           IOMMU devices have to have an identifier to look them up
>           eg IOMMU core layer carrying out id translation. This can be
>           done through a fwnode_handle (ie IOMMU platform devices created
>           out of IORT tables are not ACPI devices hence they can't be
>           allocated as such, otherwise they would have a fwnode_handle of
>           type FWNODE_ACPI).
> 
> PATCH (2) makes use of the ACPI early probing API to add a linker script
>           section for probing devices via IORT ACPI kernel code.
> 
> PATCH (3) provides IORT support for registering IOMMU IORT node through
>           their fwnode handle.
> 
> PATCH (4) make of_iommu_{set/get}_ops() functions DT agnostic.
> 
> PATCH (5) convert ARM SMMU driver to use fwnode instead of of_node as
>           look-up and iommu_ops retrieval token.
> 
> PATCH (6) convert ARM SMMU v3 driver to use fwnode instead of of_node as
>           look-up and iommu_ops retrieval token.
> 
> PATCH (7) implements the of_dma_configure() API in ACPI world -
>           acpi_dma_configure() - and patches PCI and ACPI core code to
>           start making use of it.
> 
> PATCH (8) provides an IORT function to detect existence of specific type
>           of IORT components.
> 
> PATCH (9) creates the kernel infrastructure required to create ARM SMMU
>           platform devices for IORT nodes.
> 
> PATCH (10) refactors the ARM SMMU v3 driver so that the init functions are
>            split in a way that groups together code that probes through DT
>            and code that carries out HW registers FW agnostic probing, in
>            preparation for adding the ACPI probing path.
> 
> PATCH (11) adds ARM SMMU v3 IORT IOMMU operations to create and probe
>            ARM SMMU v3 components.
> 
> PATCH (12) refactors the ARM SMMU v1/v2 driver so that the init functions
>            are split in a way that groups together code that probes
>            through DT and code that carries out HW registers FW agnostic
>            probing, in preparation for adding the ACPI probing path.
> 
> PATCH (13) adds ARM SMMU v1/v2 IORT IOMMU operations to create and
>            probe ARM SMMU v1/v2 components.
> 
> PATCH (14) Extend the IORT iort_node_map_rid() to work on a type mask
>            instead of a single type so that the translation API can
>            be used on a range of components.
> 
> PATCH (15) Add IORT API to carry out id mappings for components that do
>            do not have an input identifier/RIDs (ie named components).
> 
> PATCH (16) provides IORT infrastructure to carry out IOMMU configuration
>            for devices and hook it up to the previously introduced ACPI
>            DMA configure API.
> 
> This patchset is provided for review/testing purposes here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git acpi/iort-smmu-v6
> 
> Tested on Juno and FVP models for ARM SMMU v1 and v3 probing path.
> 
> Lorenzo Pieralisi (16):
>   drivers: acpi: add FWNODE_ACPI_STATIC fwnode type
>   drivers: acpi: iort: introduce linker section for IORT entries probing
>   drivers: acpi: iort: add support for IOMMU fwnode registration
>   drivers: iommu: make of_iommu_set/get_ops() DT agnostic
>   drivers: iommu: arm-smmu: convert struct device of_node to fwnode
>     usage
>   drivers: iommu: arm-smmu-v3: convert struct device of_node to fwnode
>     usage
>   drivers: acpi: implement acpi_dma_configure
>   drivers: acpi: iort: add node match function
>   drivers: acpi: iort: add support for ARM SMMU platform devices
>     creation
>   drivers: iommu: arm-smmu-v3: split probe functions into DT/generic
>     portions
>   drivers: iommu: arm-smmu-v3: add IORT configuration
>   drivers: iommu: arm-smmu: split probe functions into DT/generic
>     portions
>   drivers: iommu: arm-smmu: add IORT configuration
>   drivers: acpi: iort: replace rid map type with type mask
>   drivers: acpi: iort: add single mapping function
>   drivers: acpi: iort: introduce iort_iommu_configure
> 
>  drivers/acpi/arm64/iort.c         | 586 +++++++++++++++++++++++++++++++++++++-
>  drivers/acpi/glue.c               |   4 +-
>  drivers/acpi/scan.c               |  45 +++
>  drivers/iommu/arm-smmu-v3.c       | 105 +++++--
>  drivers/iommu/arm-smmu.c          | 155 ++++++++--
>  drivers/iommu/iommu.c             |  43 +++
>  drivers/iommu/of_iommu.c          |  39 ---
>  drivers/pci/probe.c               |   3 +-
>  include/acpi/acpi_bus.h           |   2 +
>  include/asm-generic/vmlinux.lds.h |   1 +
>  include/linux/acpi.h              |  26 ++
>  include/linux/acpi_iort.h         |  14 +
>  include/linux/fwnode.h            |   3 +-
>  include/linux/iommu.h             |  14 +
>  include/linux/of_iommu.h          |  12 +-
>  15 files changed, 951 insertions(+), 101 deletions(-)
> 
> -- 
> 2.10.0
> 

^ permalink raw reply

* [PATCH 3/5] ARM: davinci: enable gpio poweroff in default config
From: Sekhar Nori @ 2016-10-26 11:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477075018-20176-4-git-send-email-david@lechnology.com>

On Saturday 22 October 2016 12:06 AM, David Lechner wrote:
> The gpio-poweroff driver is needed by LEGO MINDSTORMS EV3 (AM1808 based
> board).
> 
> Signed-off-by: David Lechner <david@lechnology.com>

For defconfig patches, we have been using the davinci_all_defconfig
prefix (see git log --oneline arch/arm/configs/davinci_all_defconfig).

Applied to v4.10/defconfig with subject line changed to:

"ARM: davinci_all_defconfig: enable gpio poweroff driver"

Thanks,
Sekhar

^ permalink raw reply

* [PATCH v14 4/9] acpi/arm64: Add GTDT table parse driver
From: Fu Wei @ 2016-10-26 11:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161020163719.GC27598@leverpostej>

Hi Mark,

On 21 October 2016 at 00:37, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi,
>
> As a heads-up, on v4.9-rc1 I see conflicts at least against
> arch/arm64/Kconfig. Luckily git am -3 seems to be able to fix that up
> automatically, but this will need to be rebased before the next posting
> and/or merging.
>
> On Thu, Sep 29, 2016 at 02:17:12AM +0800, fu.wei at linaro.org wrote:
>> +static int __init map_gt_gsi(u32 interrupt, u32 flags)
>> +{
>> +     int trigger, polarity;
>> +
>> +     if (!interrupt)
>> +             return 0;
>
> Urgh.
>
> Only the secure interrupt (which we do not need) is optional in this
> manner, and (hilariously), zero appears to also be a valid GSIV, per
> figure 5-24 in the ACPI 6.1 spec.
>
> So, I think that:
>
> (a) we should not bother parsing the secure interrupt

If I understand correctly, from this point of view, kernel don't
handle the secure interrupt.
But the current arm_arch_timer driver still enable/disable/request
PHYS_SECURE_PPI
with PHYS_NONSECURE_PPI.
That means we still need to parse the secure interrupt.
Please correct me, if I misunderstand something? :-)

> (b) we should drop the check above

yes, if zero is a valid GSIV, this makes sense.

> (c) we should report the spec issue to the ASWG
>
>> +/*
>> + * acpi_gtdt_c3stop - got c3stop info from GTDT
>> + *
>> + * Returns 1 if the timer is powered in deep idle state, 0 otherwise.
>> + */
>> +bool __init acpi_gtdt_c3stop(void)
>> +{
>> +     struct acpi_table_gtdt *gtdt = acpi_gtdt_desc.gtdt;
>> +
>> +     return !(gtdt->non_secure_el1_flags & ACPI_GTDT_ALWAYS_ON);
>> +}
>
> It looks like this can differ per interrupt. Shouldn't we check the
> appropriate one?

yes, I think you are right.

>
>> +int __init acpi_gtdt_init(struct acpi_table_header *table)
>> +{
>> +     void *start;
>> +     struct acpi_table_gtdt *gtdt;
>> +
>> +     gtdt = container_of(table, struct acpi_table_gtdt, header);
>> +
>> +     acpi_gtdt_desc.gtdt = gtdt;
>> +     acpi_gtdt_desc.gtdt_end = (void *)table + table->length;
>> +
>> +     if (table->revision < 2) {
>> +             pr_debug("Revision:%d doesn't support Platform Timers.\n",
>> +                      table->revision);
>> +             return 0;
>> +     }
>> +
>> +     if (!gtdt->platform_timer_count) {
>> +             pr_debug("No Platform Timer.\n");
>> +             return 0;
>> +     }
>> +
>> +     start = (void *)gtdt + gtdt->platform_timer_offset;
>> +     if (start < (void *)table + sizeof(struct acpi_table_gtdt)) {
>> +             pr_err(FW_BUG "Failed to retrieve timer info from firmware: invalid data.\n");
>> +             return -EINVAL;
>> +     }
>> +     acpi_gtdt_desc.platform_timer_start = start;
>> +
>> +     return gtdt->platform_timer_count;
>> +}
>
> This is never used as anything other than a status code.
>
> Just return zero; we haven't parsed the platform timers themselves at
> this point anyway.

Sorry, in my driver, I use this return value to inform driver

 negative number : error
0 : we don't have platform timer in GTDT table.
positive number: the number of platform timer we have in GTDT table.

please correct me, if I am doing it in wrong way. Thanks :-)


>
> Thanks,
> Mark.



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat

^ permalink raw reply

* [PATCH] ARM: imx6: Fix GPC probe error path
From: Fabio Estevam @ 2016-10-26 11:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <39c26009-8e50-d182-dbfd-49f44cb9402a@roeck-us.net>

Hi Guenter,

On Wed, Oct 26, 2016 at 12:35 AM, Guenter Roeck <linux@roeck-us.net> wrote:

> Looking into this some more, it turns out that
> of_genpd_add_provider_onecell()
> now returns an error if one of the provided power domains does not exist.
> In this case, the "ARM" power domain does not exist. I don't see where it is
> created, so it may well be that this now fails for all imx6 boards with
> multi_v7_defconfig. Looking into kernelci.org test results, this is
> confirmed
> for at least imx6dl-riotboard. Overall I think it is quite safe to assume
> that all imx6 boards crash with mainline kernels and multi_v7_defconfig.
>
> The change can be tracked down to commit 0159ec67076 ("PM / Domains: Verify
> the PM domain is present when adding a provider"). Adding everyone in the
> commit log for feedback.

Yes, this is the same conclusion I got. Please check:
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=eef0b282bb586259d35548851cf6a4ce847bb804

^ permalink raw reply

* [PATCH 1/5] ARM: davinci: Compile MMC in kernel
From: Sekhar Nori @ 2016-10-26 11:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477075018-20176-2-git-send-email-david@lechnology.com>

On Saturday 22 October 2016 12:06 AM, David Lechner wrote:
> This changes the davinci default configuration to compile the davinci
> MMC driver into the kernel. This allows booting from an SD card without
> requiring an initrd containing the kernel module.
> 
> Signed-off-by: David Lechner <david@lechnology.com>

"build" is more relevant here than "compile" so I changed that. And also
used davinci_all_defconfig as detailed in last e-mail.

Applied to v4.10/defconfig

Thanks,
Sekhar

^ permalink raw reply

* Add Allwinner Q8 tablets hardware manager
From: Hans de Goede @ 2016-10-26 11:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161024173900.GP15620@leverpostej>

Hi,

On 24-10-16 19:39, Mark Rutland wrote:
> On Fri, Oct 14, 2016 at 09:53:31AM +0200, Hans de Goede wrote:
>> Hi Rob, Mark, et al.,
>
> Hi Hans,
>
> Apologies for the delay in replying to this.

No worries, I believe that 1 week is actually a pretty good
turn around time, esp. directly after a conference.

> I'd like to be clear that I do understand that there is a problem that
> needs to be addressed here. However, I do not believe that the *current*
> in-kernel approach is correct. More on that below.

Ok.

>> Mark, I know that we discussed this at ELCE and you clearly indicated
>> that according to you this does not belong in the kernel. I was a bit
>> surprised by this part of the discussion.
>>
>> I had posted a RFC earlier and Rob had indicated that given that the q8
>> tablets are a special case, as my code uses actual probing rather then some
>> pre-arranged id mechanism with say an eeprom, that doing this in a
>> non-generic manner would be ok for my special case.
>
> To some extent, Rob and I may have differing views here; I'm not
> entirely sure what Rob's view is, and I cannot talk on his behalf. I
> certainly must apologise for having not commented on said RFC, however.
>
>> So on to why I believe that the kernel is the best place to do this, at least
>> for my special use-case:
>>
>> 1. Configurability
>>
>> Since the q8 tablets do not have any id mechanism I'm probing i2c busses to
>> generate i2c client dt nodes with the right address and compatible.
>> Some info in these nodes (e.g. touchscreen firmware-name) is not probable at
>> all and gets set by heuristics. This heuristics may get things wrong.
>> So my current implementation offers kernel cmdline options to override this.
>
> As I mentioned at ELCE, one major concern I have with this approach is
> this probing, which in part relies on a collection of heuristics.

This is quite use-case specific, anyways, the probing is a 2 step process:

1) Identify which hardware there is in the tablet, this is pretty
reliable, we only detect a fix set of known possible touchscreens
and accelerometers, at known addresses and almost all have an id
register to check.

2) Determine *defaults* for various none probable settings, like guessing
which firmware to load into the touchscreen controllers, as there are
at least 2 ways the gsl1680 is wired up on these tablets and this
requires 2 different firmware files. This uses heuristics, to, as said,
determine the defaults all of the non-probable bits are overidable
through config options (currently kernel module options). Getting these
wrong is not dangerous to the hardware, but will work in a non-functional
or misbehaving (wrong coordinates) touchscreen.

Note with the models I've access to so far the heuristics score 100%
but I'm not sure how representative the 16 models I've access to are
(they are all different and have been bought over a span of multiple
years).

> I'm worried that this is very fragile, and sets us up for having to
> maintain a much larger collection of heuristics and/or a database of
> particular boards in future. This is fragile, defeats much of the gain
> from DT.

I understand your worries, as said I'm confident the actual probing
is safe and getting the heuristics wrong will result in misbehavior,
but not in any hardware damage or such.

> Worse, this could be completely incompatible with some arbitrary board
> that comes by in future,

I assume you mean an arbitrary q8 tablet here, as the probe code does
bind by board/machine compatible, so for a really arbitrary board
this code will never activate.

> because the kernel assumed something that was
> not true, which it would not have done if things were explicitly
> described to the kernel.

I understand your worry, but moving the probing code to say u-boot
will not change any of this, the kernel will get the explicit
description created by the u-boot probe code, but it would be
just as wrong.

So maybe we need to answer 2 questions in a row:

1) Do we want such probe code at all ?

My answer to this is yes, these (cheap) tablets are interesting to
e.g. the maker community and I would like them to run mainline
(and run mainline well), but given the way there are put together
this require some code to dynamically adapt to the batch of the
month somewhere

2) Where do we put this code ?

If we agree on 1 (I assume we do) then this becomes the real
question, at which point your worries about the kernel assuming
something which is not true because the probe code got it wrong
may become true regardless where the code lives.

So wrt this worries is all I can do is ask you to trust me to
not mess things up, just like we all trust driver authors, etc.
all the time to not mess things up.

> As I mentioned at ELCE, I'm not opposed to the concept of the kernel
> applying overlays or fixups based on a well-defined set of criteria;
> having some of that embedded in the DT itself would be remarkably
> helpful.  However, I am very much not keen on loosely defined criteria as
> with here, as it couples the DT and kernel, and creates problems longer
> term, as described above.

Right, so again I think we need to split the discussion in 2 steps:

1) How do we apply the fixups, currently I'm using free-form changes
done from C-code. I can envision moving to something like the quirk
mechanism suggested by Pantelis in the past. Note this is not a perfect
fit for my rather corner-case use-case, but I can understand that in
general you want the variants to be described in dt, and activated
in some way, rather then have c-code make free-form changes to the dt

2) How do we select which fixups to apply. Again I can understand
you wanting some well defined mechanism for this, but again my
use-case is special and simply does not allow for this as there
is no id-eeprom to read or some such.

>> Although having to specify kernel cmdline options is not the plug and play
>> experience I want to give end-users most distros do have documentation on
>> how to do this and doing this is relatively easy for end-users. Moving this
>> to the bootloader means moving to some bootloader specific config mechanism
>> which is going to be quite hard (and possibly dangerous) for users to use.
>
> I have to ask, why is it more dangerous?

Because for normal end users meddling with the bootloader / with u-boot's
environment is like flashing a PC BIOS. Most (non technical) end users will
want to install u-boot once (or not at all) and then just have it work.

> Perhaps more difficult, but that can be solved,

More difficult means not doable for many users.

> if the manual
> corrections are simply a set of options to be passed to the kernel, I
> don't see why the bootloader cannot pick this up.
>
>> 2. Upgradability
>>
>> Most users treat the bootloader like they treat an x86 machine BIOS/EFI,
>> they will never upgrade it unless they really have to. This means that it
>> will be very difficult to get users to actual benefit from bug-fixes /
>> improvements done to the probing code. Where as the kernel on boards running
>> e.g. Debian or Fedora gets regular updates together with the rest of the
>> system.
>
> Given that DTBs are supposed to remain supported, users should find
> themselves with a system that continues to work, but may not have all
> the bells and whistles it could, much like elsewhere.
>
> While it's true that we have issues in this area, I don't think that's
> an argument for putting things into the kernel for this specific set of
> boards.

It is an argument to put much of the dynamic (dt) hardware support in
the kernel in general.

>> 3. Infrastructure
>>
>> The kernel simply has better infrastructure for doing these kind of things.
>
> At least on the DT front, a lot of work has gone into improving the
> infrastructure, e.g. the work that Free Electrons have done [1]. I
> appreciate that the infrastructure for things like poking SPI may not be
> as advanced.
>
> Which bits of infrastructure do you find lacking today?

Nothing really specific (I've not yet tried porting the probe code
to u-boot), but I just find working within the kernel easier in general,
since there really just is a lot more infrastructure. Note I'm the
upstream u-boot maintainer for the allwinner SoC support, so this
is not due to me being unfamiliar with u-boot.

>> Yes we could improve the bootloader, but the kernel is also improving
>> and likely at a faster rate. Besides that the purpose of the
>> bootloader is mostly to be simple and small, load the kernel and get
>> out of the way, not to be a general purpose os kernel. So it will
>> simply always have less infrastructure and that is a good thing,
>> otherwise we will be writing another general purpose os which is a
>> waste of effort.
>
> I think this conflates a number of details. Yes, we'd like firmware and
> bootloaders to be small, and yes, their infrastructure and feature
> support will be smaller than a general purpose OS.
>
> That doesn't imply that they cannot have features necessary to boostrap
> an OS.
>
> It's also not strictly necessary that the firmware or bootloader have
> the capability to do all this probing, as that could be contained in
> another part (e.g. a U-Boot application which is run once to detect the
> board details, logging this into a file).
>
> It's also possible to ship an intermediary stage (e.g. like the
> impedance matcher) which can be upgradeable independently of the kernel.

Yes there are other solutions, but they all involve a lot more
moving pieces (and thus will break) then a single isolated .c file
in the kernel, which is all this series adds.

Esp the intermediate solution just adds a ton of complexity with 0
gain.

>> 4. This is not a new board file
>>
>> Mark, at ELCE I got the feeling that your biggest worry / objection is
>> that this will bring back board files, but that is not the case, if you
>> look at the actual code it is nothing like a board-file at all. Where a
>> board file was instantiating device after device after device from c-code,
>> with maybe a few if-s in there to deal with board revisions. This code is
>> all about figuring out which accelerometer and touchscreen there are,
>> to then add nodes to the dt for this 2 devices, almost all the code is
>> probing, the actual dt modifying code is tiny and no direct device
>> instantiation is done at all.
>
> Sorry, but I must disagree. This code:
>
> (a) identifies a set of boards based on a top-level compatible string.
>     i.e. it's sole purpose is to handle those boards.
>
> (b) assumes non-general properties of those boards (e.g. that poking
>     certain SPI endpoints is safe).
>
> (c) applies arbitrary properties to the DT, applying in-built knowledge
>     of those boards (in addition to deep structural knowledge of the
>     DTB in question).
>
> To me, given the implicit knowledge, that qualifies as a "board file".
>
> As I mentioned at ELCE, if this had no knowledge of the boards in
> question, I would be less concerned. e.g. if there was a well-defined
> identification mechanism, describe in the DT, with fixups also defined
> in the DT.

And as I tried to explain before, for this specific use-case describing
all this board specific knowledge in a generic manner in dt is simply
impossible, unless we add a turing complete language to dt aka aml.

I've a feeling that you're mixing this, rather special, use-case with
the more generic use-case of daughter-boards for various small-board-computers
I agree that for the SBC use-case it makes sense to try and come up with
a shared core / dt bindings for much of this. Note that even this boards
will still need a board (or board-family) specific method for getting
the actual id from a daughter-board, but this board specific code could
then pass the id to some more general hw-manager core which starts applying
dt changes based on the id. But this assumes there is a single id to
uniquely identify the extensions, which in my case there simply is not.

>> 5. This is an exception, not the rule
>>
>> Yes this is introducing board (family of boards) specific c-code into the
>> kernel, so in a way it is reminiscent of board files. But sometimes this is
>> necessary, just look at all the vendor / platform specific code in the kernel
>> in drivers/platform/x86, or all the places where DMI strings are used to
>> uniquely identify a x86 board and adjust behavior.
>>
>> But this really is the exception, not the rule. I've written/upstreamed a
>> number of drivers for q8 tablets hardware and if you look at e.g. the
>> silead touchscreen driver then in linux-next this is already used for 5
>> ARM dts files where no board specific C-code is involved at all.
>>
>> So this again is very different from the board file era, where C-code
>> had to be used to fill device specific platform-data structs, here all
>> necessary info is contained in the dt-binding and there are many users
>> who do not need any board specific C-code in the kernel at all.
>>
>> So dt is working as it should and is avoiding board specific C-code for
>> the majority of the cases. But sometimes hardware is not as we ideally
>> would like it to be; and for those *exceptions* we are sometimes going
>> to need C-code in the kernel, just like there is "board" specific C-code
>> in the x86 code.
>
> Your talk convinced me that we're both going to see more variation
> within this family of boards, and that we'll see more families of boards
> following a similar patter. Given that, I think that we need a more
> general solution, as I commented on the RFC.
>
> That doesn't necessarily mean that this can't happen in the kernel, but
> it certainly needs to be more strictly defined, e.g. with match criteria
> and fixups explicit in the DTB.

The only answer I've to: "with match criteria and fixups explicit in the DTB"
is: ok, give my a turing complete language inside DTB then, anything else
will not suffice. So either we are doomed to reinvent ACPI; or we must
accept some board(family) specific C-code in the kernel.

Regards,

Hans

^ 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