linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
@ 2022-12-05 15:23 Francesco Dolcini
  2022-12-05 16:26 ` Marek Vasut
  2022-12-05 16:31 ` Miquel Raynal
  0 siblings, 2 replies; 22+ messages in thread
From: Francesco Dolcini @ 2022-12-05 15:23 UTC (permalink / raw)
  To: Shawn Guo, linux-arm-kernel, Marek Vasut, Miquel Raynal
  Cc: Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable

From: Francesco Dolcini <francesco.dolcini@toradex.com>

This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.

It introduced a boot regression on colibri-imx7, and potentially any
other i.MX7 boards with MTD partition list generated into the fdt by
U-Boot.

While the commit we are reverting here is not obviously wrong, it fixes
only a dt binding checker warning that is non-functional, while it
introduces a boot regression and there is no obvious fix ready.

Cc: stable@vger.kernel.org
Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 arch/arm/boot/dts/imx7s.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 03d2e8544a4e..0fc9e6b8b05d 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -1270,10 +1270,10 @@ dma_apbh: dma-apbh@33000000 {
 			clocks = <&clks IMX7D_NAND_USDHC_BUS_RAWNAND_CLK>;
 		};
 
-		gpmi: nand-controller@33002000 {
+		gpmi: nand-controller@33002000{
 			compatible = "fsl,imx7d-gpmi-nand";
 			#address-cells = <1>;
-			#size-cells = <0>;
+			#size-cells = <1>;
 			reg = <0x33002000 0x2000>, <0x33004000 0x4000>;
 			reg-names = "gpmi-nand", "bch";
 			interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-05 15:23 [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells" Francesco Dolcini
@ 2022-12-05 16:26 ` Marek Vasut
  2022-12-05 17:58   ` Miquel Raynal
  2022-12-08 10:51   ` Miquel Raynal
  2022-12-05 16:31 ` Miquel Raynal
  1 sibling, 2 replies; 22+ messages in thread
From: Marek Vasut @ 2022-12-05 16:26 UTC (permalink / raw)
  To: Francesco Dolcini, Shawn Guo, linux-arm-kernel, Miquel Raynal
  Cc: Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable

On 12/5/22 16:23, Francesco Dolcini wrote:
> From: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.
> 
> It introduced a boot regression on colibri-imx7, and potentially any
> other i.MX7 boards with MTD partition list generated into the fdt by
> U-Boot.
> 
> While the commit we are reverting here is not obviously wrong, it fixes
> only a dt binding checker warning that is non-functional, while it
> introduces a boot regression and there is no obvious fix ready.
> 
> Cc: stable@vger.kernel.org
> Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
> Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
> Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
>   arch/arm/boot/dts/imx7s.dtsi | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index 03d2e8544a4e..0fc9e6b8b05d 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -1270,10 +1270,10 @@ dma_apbh: dma-apbh@33000000 {
>   			clocks = <&clks IMX7D_NAND_USDHC_BUS_RAWNAND_CLK>;
>   		};
>   
> -		gpmi: nand-controller@33002000 {
> +		gpmi: nand-controller@33002000{
>   			compatible = "fsl,imx7d-gpmi-nand";
>   			#address-cells = <1>;
> -			#size-cells = <0>;
> +			#size-cells = <1>;
>   			reg = <0x33002000 0x2000>, <0x33004000 0x4000>;
>   			reg-names = "gpmi-nand", "bch";
>   			interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;

I suspect this fix should eventually be reverted again, once a proper 
fix is agreed upon in the MTD OF parser, right ?

With that:

Acked-by: Marek Vasut <marex@denx.de>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-05 15:23 [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells" Francesco Dolcini
  2022-12-05 16:26 ` Marek Vasut
@ 2022-12-05 16:31 ` Miquel Raynal
  1 sibling, 0 replies; 22+ messages in thread
From: Miquel Raynal @ 2022-12-05 16:31 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Shawn Guo, linux-arm-kernel, Marek Vasut, Francesco Dolcini,
	Rob Herring, Krzysztof Kozlowski, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-mtd, stable

Hi Francesco,

francesco@dolcini.it wrote on Mon,  5 Dec 2022 16:23:27 +0100:

> From: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.
> 
> It introduced a boot regression on colibri-imx7, and potentially any
> other i.MX7 boards with MTD partition list generated into the fdt by
> U-Boot.
> 
> While the commit we are reverting here is not obviously wrong, it fixes
> only a dt binding checker warning that is non-functional, while it
> introduces a boot regression and there is no obvious fix ready.
> 
> Cc: stable@vger.kernel.org
> Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
> Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
> Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

> ---
>  arch/arm/boot/dts/imx7s.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index 03d2e8544a4e..0fc9e6b8b05d 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -1270,10 +1270,10 @@ dma_apbh: dma-apbh@33000000 {
>  			clocks = <&clks IMX7D_NAND_USDHC_BUS_RAWNAND_CLK>;
>  		};
>  
> -		gpmi: nand-controller@33002000 {
> +		gpmi: nand-controller@33002000{
>  			compatible = "fsl,imx7d-gpmi-nand";
>  			#address-cells = <1>;
> -			#size-cells = <0>;
> +			#size-cells = <1>;
>  			reg = <0x33002000 0x2000>, <0x33004000 0x4000>;
>  			reg-names = "gpmi-nand", "bch";
>  			interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;


Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-05 16:26 ` Marek Vasut
@ 2022-12-05 17:58   ` Miquel Raynal
  2022-12-05 18:07     ` Marek Vasut
  2022-12-08 10:51   ` Miquel Raynal
  1 sibling, 1 reply; 22+ messages in thread
From: Miquel Raynal @ 2022-12-05 17:58 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Francesco Dolcini, Shawn Guo, linux-arm-kernel,
	Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable

Hi Marek,

marex@denx.de wrote on Mon, 5 Dec 2022 17:26:53 +0100:

> On 12/5/22 16:23, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > 
> > This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.
> > 
> > It introduced a boot regression on colibri-imx7, and potentially any
> > other i.MX7 boards with MTD partition list generated into the fdt by
> > U-Boot.
> > 
> > While the commit we are reverting here is not obviously wrong, it fixes
> > only a dt binding checker warning that is non-functional, while it
> > introduces a boot regression and there is no obvious fix ready.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
> > Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
> > Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> >   arch/arm/boot/dts/imx7s.dtsi | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> > index 03d2e8544a4e..0fc9e6b8b05d 100644
> > --- a/arch/arm/boot/dts/imx7s.dtsi
> > +++ b/arch/arm/boot/dts/imx7s.dtsi
> > @@ -1270,10 +1270,10 @@ dma_apbh: dma-apbh@33000000 {
> >   			clocks = <&clks IMX7D_NAND_USDHC_BUS_RAWNAND_CLK>;
> >   		};  
> >   > -		gpmi: nand-controller@33002000 {  
> > +		gpmi: nand-controller@33002000{
> >   			compatible = "fsl,imx7d-gpmi-nand";
> >   			#address-cells = <1>;
> > -			#size-cells = <0>;
> > +			#size-cells = <1>;
> >   			reg = <0x33002000 0x2000>, <0x33004000 0x4000>;
> >   			reg-names = "gpmi-nand", "bch";
> >   			interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;  
> 
> I suspect this fix should eventually be reverted again, once a proper fix is agreed upon in the MTD OF parser, right ?

I guess it's time to migrate to a more modern definition, it's not
complex to do, there are plenty of examples. This would be IMHO a
better step ahead rather than just a cell change. Anyway, I don't mind
reverting this once we've sorted this mess out and fixed U-Boot.

Cheers,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-05 17:58   ` Miquel Raynal
@ 2022-12-05 18:07     ` Marek Vasut
  2022-12-05 18:15       ` Francesco Dolcini
  2022-12-05 18:18       ` Miquel Raynal
  0 siblings, 2 replies; 22+ messages in thread
From: Marek Vasut @ 2022-12-05 18:07 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Francesco Dolcini, Shawn Guo, linux-arm-kernel,
	Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable

On 12/5/22 18:58, Miquel Raynal wrote:
> Hi Marek,

Hi,

> marex@denx.de wrote on Mon, 5 Dec 2022 17:26:53 +0100:
> 
>> On 12/5/22 16:23, Francesco Dolcini wrote:
>>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>
>>> This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.
>>>
>>> It introduced a boot regression on colibri-imx7, and potentially any
>>> other i.MX7 boards with MTD partition list generated into the fdt by
>>> U-Boot.
>>>
>>> While the commit we are reverting here is not obviously wrong, it fixes
>>> only a dt binding checker warning that is non-functional, while it
>>> introduces a boot regression and there is no obvious fix ready.
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
>>> Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
>>> Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
>>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>>> ---
>>>    arch/arm/boot/dts/imx7s.dtsi | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
>>> index 03d2e8544a4e..0fc9e6b8b05d 100644
>>> --- a/arch/arm/boot/dts/imx7s.dtsi
>>> +++ b/arch/arm/boot/dts/imx7s.dtsi
>>> @@ -1270,10 +1270,10 @@ dma_apbh: dma-apbh@33000000 {
>>>    			clocks = <&clks IMX7D_NAND_USDHC_BUS_RAWNAND_CLK>;
>>>    		};
>>>    > -		gpmi: nand-controller@33002000 {
>>> +		gpmi: nand-controller@33002000{
>>>    			compatible = "fsl,imx7d-gpmi-nand";
>>>    			#address-cells = <1>;
>>> -			#size-cells = <0>;
>>> +			#size-cells = <1>;
>>>    			reg = <0x33002000 0x2000>, <0x33004000 0x4000>;
>>>    			reg-names = "gpmi-nand", "bch";
>>>    			interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
>>
>> I suspect this fix should eventually be reverted again, once a proper fix is agreed upon in the MTD OF parser, right ?
> 
> I guess it's time to migrate to a more modern definition

Is that the nand-chip@N { status="disabled"; } part ?

>, it's not
> complex to do, there are plenty of examples. This would be IMHO a
> better step ahead rather than just a cell change. Anyway, I don't mind
> reverting this once we've sorted this mess out and fixed U-Boot.

Won't we still have issues with older bootloader versions which paste 
partitions directly into this &gpmi {} node, and which needs to be fixed 
up in the parser in the end ?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-05 18:07     ` Marek Vasut
@ 2022-12-05 18:15       ` Francesco Dolcini
  2022-12-05 18:18       ` Miquel Raynal
  1 sibling, 0 replies; 22+ messages in thread
From: Francesco Dolcini @ 2022-12-05 18:15 UTC (permalink / raw)
  To: Marek Vasut, Miquel Raynal
  Cc: Miquel Raynal, Francesco Dolcini, Shawn Guo, linux-arm-kernel,
	Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable

On Mon, Dec 05, 2022 at 07:07:14PM +0100, Marek Vasut wrote:
> On 12/5/22 18:58, Miquel Raynal wrote:
> > marex@denx.de wrote on Mon, 5 Dec 2022 17:26:53 +0100:
> > , it's not
> > complex to do, there are plenty of examples. This would be IMHO a
> > better step ahead rather than just a cell change. Anyway, I don't mind
> > reverting this once we've sorted this mess out and fixed U-Boot.
> 
> Won't we still have issues with older bootloader versions which paste
> partitions directly into this &gpmi {} node, and which needs to be fixed up
> in the parser in the end ?

Yes, I think so. While I do agree on printk warning and deprecated
functions and use more modern and less problematic stuff, this should
not come at the cost of failing the boot on board using some old U-Boot
version.

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-05 18:07     ` Marek Vasut
  2022-12-05 18:15       ` Francesco Dolcini
@ 2022-12-05 18:18       ` Miquel Raynal
  2022-12-05 18:52         ` Marek Vasut
  1 sibling, 1 reply; 22+ messages in thread
From: Miquel Raynal @ 2022-12-05 18:18 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Francesco Dolcini, Shawn Guo, linux-arm-kernel,
	Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable

Hi Marek,

marex@denx.de wrote on Mon, 5 Dec 2022 19:07:14 +0100:

> On 12/5/22 18:58, Miquel Raynal wrote:
> > Hi Marek,  
> 
> Hi,
> 
> > marex@denx.de wrote on Mon, 5 Dec 2022 17:26:53 +0100:
> >   
> >> On 12/5/22 16:23, Francesco Dolcini wrote:  
> >>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
> >>>
> >>> This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.
> >>>
> >>> It introduced a boot regression on colibri-imx7, and potentially any
> >>> other i.MX7 boards with MTD partition list generated into the fdt by
> >>> U-Boot.
> >>>
> >>> While the commit we are reverting here is not obviously wrong, it fixes
> >>> only a dt binding checker warning that is non-functional, while it
> >>> introduces a boot regression and there is no obvious fix ready.
> >>>
> >>> Cc: stable@vger.kernel.org
> >>> Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
> >>> Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
> >>> Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
> >>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> >>> ---
> >>>    arch/arm/boot/dts/imx7s.dtsi | 4 ++--
> >>>    1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> >>> index 03d2e8544a4e..0fc9e6b8b05d 100644
> >>> --- a/arch/arm/boot/dts/imx7s.dtsi
> >>> +++ b/arch/arm/boot/dts/imx7s.dtsi
> >>> @@ -1270,10 +1270,10 @@ dma_apbh: dma-apbh@33000000 {
> >>>    			clocks = <&clks IMX7D_NAND_USDHC_BUS_RAWNAND_CLK>;
> >>>    		};  
> >>>    > -		gpmi: nand-controller@33002000 {  
> >>> +		gpmi: nand-controller@33002000{
> >>>    			compatible = "fsl,imx7d-gpmi-nand";
> >>>    			#address-cells = <1>;
> >>> -			#size-cells = <0>;
> >>> +			#size-cells = <1>;
> >>>    			reg = <0x33002000 0x2000>, <0x33004000 0x4000>;
> >>>    			reg-names = "gpmi-nand", "bch";
> >>>    			interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;  
> >>
> >> I suspect this fix should eventually be reverted again, once a proper fix is agreed upon in the MTD OF parser, right ?  
> > 
> > I guess it's time to migrate to a more modern definition  
> 
> Is that the nand-chip@N { status="disabled"; } part ?

I would prefer the controller to be disabled if useless, but otherwise
yes.

> 
> >, it's not
> > complex to do, there are plenty of examples. This would be IMHO a
> > better step ahead rather than just a cell change. Anyway, I don't mind
> > reverting this once we've sorted this mess out and fixed U-Boot.  
> 
> Won't we still have issues with older bootloader versions which paste partitions directly into this &gpmi {} node, and which needs to be fixed up in the parser in the end ?

I believe fdt_fixup_mtdparts() should be killed, so we should no longer
have this problem.

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-05 18:18       ` Miquel Raynal
@ 2022-12-05 18:52         ` Marek Vasut
  2022-12-05 19:07           ` Francesco Dolcini
  0 siblings, 1 reply; 22+ messages in thread
From: Marek Vasut @ 2022-12-05 18:52 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Francesco Dolcini, Shawn Guo, linux-arm-kernel,
	Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable

On 12/5/22 19:18, Miquel Raynal wrote:
> Hi Marek,
> 
> marex@denx.de wrote on Mon, 5 Dec 2022 19:07:14 +0100:
> 
>> On 12/5/22 18:58, Miquel Raynal wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> marex@denx.de wrote on Mon, 5 Dec 2022 17:26:53 +0100:
>>>    
>>>> On 12/5/22 16:23, Francesco Dolcini wrote:
>>>>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>>>
>>>>> This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.
>>>>>
>>>>> It introduced a boot regression on colibri-imx7, and potentially any
>>>>> other i.MX7 boards with MTD partition list generated into the fdt by
>>>>> U-Boot.
>>>>>
>>>>> While the commit we are reverting here is not obviously wrong, it fixes
>>>>> only a dt binding checker warning that is non-functional, while it
>>>>> introduces a boot regression and there is no obvious fix ready.
>>>>>
>>>>> Cc: stable@vger.kernel.org
>>>>> Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
>>>>> Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
>>>>> Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
>>>>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>>> ---
>>>>>     arch/arm/boot/dts/imx7s.dtsi | 4 ++--
>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
>>>>> index 03d2e8544a4e..0fc9e6b8b05d 100644
>>>>> --- a/arch/arm/boot/dts/imx7s.dtsi
>>>>> +++ b/arch/arm/boot/dts/imx7s.dtsi
>>>>> @@ -1270,10 +1270,10 @@ dma_apbh: dma-apbh@33000000 {
>>>>>     			clocks = <&clks IMX7D_NAND_USDHC_BUS_RAWNAND_CLK>;
>>>>>     		};
>>>>>     > -		gpmi: nand-controller@33002000 {
>>>>> +		gpmi: nand-controller@33002000{
>>>>>     			compatible = "fsl,imx7d-gpmi-nand";
>>>>>     			#address-cells = <1>;
>>>>> -			#size-cells = <0>;
>>>>> +			#size-cells = <1>;
>>>>>     			reg = <0x33002000 0x2000>, <0x33004000 0x4000>;
>>>>>     			reg-names = "gpmi-nand", "bch";
>>>>>     			interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
>>>>
>>>> I suspect this fix should eventually be reverted again, once a proper fix is agreed upon in the MTD OF parser, right ?
>>>
>>> I guess it's time to migrate to a more modern definition
>>
>> Is that the nand-chip@N { status="disabled"; } part ?
> 
> I would prefer the controller to be disabled if useless, but otherwise
> yes.

ACK, but see below.

>>> , it's not
>>> complex to do, there are plenty of examples. This would be IMHO a
>>> better step ahead rather than just a cell change. Anyway, I don't mind
>>> reverting this once we've sorted this mess out and fixed U-Boot.
>>
>> Won't we still have issues with older bootloader versions which paste partitions directly into this &gpmi {} node, and which needs to be fixed up in the parser in the end ?
> 
> I believe fdt_fixup_mtdparts() should be killed, so we should no longer
> have this problem.

The fdt_fixup_mtdparts is U-Boot code. If contemporary Linux kernel is 
booted with ancient U-Boot, then you would still get defective DT passed 
to Linux, and that should be fixed up by Linux. Removing 
fdt_fixup_mtdparts() from current mainline U-Boot won't solve this problem.

I think this is also what Francesco is trying to convey (please correct 
me if I'm wrong).

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-05 18:52         ` Marek Vasut
@ 2022-12-05 19:07           ` Francesco Dolcini
  2022-12-06 10:16             ` Miquel Raynal
  0 siblings, 1 reply; 22+ messages in thread
From: Francesco Dolcini @ 2022-12-05 19:07 UTC (permalink / raw)
  To: Marek Vasut, Miquel Raynal
  Cc: Francesco Dolcini, Shawn Guo, linux-arm-kernel,
	Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable

On Mon, Dec 05, 2022 at 07:52:08PM +0100, Marek Vasut wrote:
> On 12/5/22 19:18, Miquel Raynal wrote:
> > marex@denx.de wrote on Mon, 5 Dec 2022 19:07:14 +0100:
> > > On 12/5/22 18:58, Miquel Raynal wrote:
> > > > , it's not
> > > > complex to do, there are plenty of examples. This would be IMHO a
> > > > better step ahead rather than just a cell change. Anyway, I don't mind
> > > > reverting this once we've sorted this mess out and fixed U-Boot.
> > > 
> > > Won't we still have issues with older bootloader versions which
> > > paste partitions directly into this &gpmi {} node, and which needs
> > > to be fixed up in the parser in the end ?
> > 
> > I believe fdt_fixup_mtdparts() should be killed, so we should no longer
> > have this problem.
> 
> The fdt_fixup_mtdparts is U-Boot code. If contemporary Linux kernel is
> booted with ancient U-Boot, then you would still get defective DT passed to
> Linux, and that should be fixed up by Linux. Removing fdt_fixup_mtdparts()
> from current mainline U-Boot won't solve this problem.
> 
> I think this is also what Francesco is trying to convey (please correct me
> if I'm wrong).

Yes, exactly, thanks!

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-05 19:07           ` Francesco Dolcini
@ 2022-12-06 10:16             ` Miquel Raynal
  2022-12-06 19:02               ` Marek Vasut
  0 siblings, 1 reply; 22+ messages in thread
From: Miquel Raynal @ 2022-12-06 10:16 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Marek Vasut, Shawn Guo, linux-arm-kernel, Francesco Dolcini,
	Rob Herring, Krzysztof Kozlowski, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-mtd, stable

Hi Francesco,

francesco@dolcini.it wrote on Mon, 5 Dec 2022 20:07:18 +0100:

> On Mon, Dec 05, 2022 at 07:52:08PM +0100, Marek Vasut wrote:
> > On 12/5/22 19:18, Miquel Raynal wrote:  
> > > marex@denx.de wrote on Mon, 5 Dec 2022 19:07:14 +0100:  
> > > > On 12/5/22 18:58, Miquel Raynal wrote:  
> > > > > , it's not
> > > > > complex to do, there are plenty of examples. This would be IMHO a
> > > > > better step ahead rather than just a cell change. Anyway, I don't mind
> > > > > reverting this once we've sorted this mess out and fixed U-Boot.  
> > > > 
> > > > Won't we still have issues with older bootloader versions which
> > > > paste partitions directly into this &gpmi {} node, and which needs
> > > > to be fixed up in the parser in the end ?  
> > > 
> > > I believe fdt_fixup_mtdparts() should be killed, so we should no longer
> > > have this problem.  
> > 
> > The fdt_fixup_mtdparts is U-Boot code. If contemporary Linux kernel is
> > booted with ancient U-Boot, then you would still get defective DT passed to
> > Linux, and that should be fixed up by Linux. Removing fdt_fixup_mtdparts()
> > from current mainline U-Boot won't solve this problem.
> > 
> > I think this is also what Francesco is trying to convey (please correct me
> > if I'm wrong).  

If we can get rid of fdt_fixup_mtdparts(), it means someone has to
create the partitions. I guess the easy way would be to just provide
mtdparts to Linux like all the other boards and let Linux deal with it.
Then we can just assume in Linux that perhaps if the partitions are
invalid (#size-cell is wrong?) then we should just stop their creation
and fallback to another mechanism instead of failing entirely. This way
no need for hackish changes in the parsers and compatibility is still
valid with old U-Boot (if mtdparts was provided on the cmdline, to be
checked). Otherwise we'll have to deal with it in Linux, that's a pity.

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-06 10:16             ` Miquel Raynal
@ 2022-12-06 19:02               ` Marek Vasut
  2022-12-07  7:49                 ` Francesco Dolcini
  0 siblings, 1 reply; 22+ messages in thread
From: Marek Vasut @ 2022-12-06 19:02 UTC (permalink / raw)
  To: Miquel Raynal, Francesco Dolcini
  Cc: Shawn Guo, linux-arm-kernel, Francesco Dolcini, Rob Herring,
	Krzysztof Kozlowski, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, devicetree, linux-mtd, stable

On 12/6/22 11:16, Miquel Raynal wrote:
> Hi Francesco,

Hello everyone,

> francesco@dolcini.it wrote on Mon, 5 Dec 2022 20:07:18 +0100:
> 
>> On Mon, Dec 05, 2022 at 07:52:08PM +0100, Marek Vasut wrote:
>>> On 12/5/22 19:18, Miquel Raynal wrote:
>>>> marex@denx.de wrote on Mon, 5 Dec 2022 19:07:14 +0100:
>>>>> On 12/5/22 18:58, Miquel Raynal wrote:
>>>>>> , it's not
>>>>>> complex to do, there are plenty of examples. This would be IMHO a
>>>>>> better step ahead rather than just a cell change. Anyway, I don't mind
>>>>>> reverting this once we've sorted this mess out and fixed U-Boot.
>>>>>
>>>>> Won't we still have issues with older bootloader versions which
>>>>> paste partitions directly into this &gpmi {} node, and which needs
>>>>> to be fixed up in the parser in the end ?
>>>>
>>>> I believe fdt_fixup_mtdparts() should be killed, so we should no longer
>>>> have this problem.
>>>
>>> The fdt_fixup_mtdparts is U-Boot code. If contemporary Linux kernel is
>>> booted with ancient U-Boot, then you would still get defective DT passed to
>>> Linux, and that should be fixed up by Linux. Removing fdt_fixup_mtdparts()
>>> from current mainline U-Boot won't solve this problem.
>>>
>>> I think this is also what Francesco is trying to convey (please correct me
>>> if I'm wrong).
> 
> If we can get rid of fdt_fixup_mtdparts(), it means someone has to
> create the partitions. I guess the easy way would be to just provide
> mtdparts to Linux like all the other boards and let Linux deal with it.

This is based on an assumption that the platform kernel command line can 
be updated to insert such a workaround. If Francesco cannot update the 
bootloader, the kernel command line may be immutable all the same.

> Then we can just assume in Linux that perhaps if the partitions are
> invalid (#size-cell is wrong?) then we should just stop their creation
> and fallback to another mechanism instead of failing entirely. This way
> no need for hackish changes in the parsers and compatibility is still
> valid with old U-Boot (if mtdparts was provided on the cmdline, to be
> checked). Otherwise we'll have to deal with it in Linux, that's a pity.

I am very much banking toward -- fix it up in the parser, just like any 
other firmware issue. Esp. since the fix up is printing a warning, and 
it is like a 2-liner patch.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-06 19:02               ` Marek Vasut
@ 2022-12-07  7:49                 ` Francesco Dolcini
  0 siblings, 0 replies; 22+ messages in thread
From: Francesco Dolcini @ 2022-12-07  7:49 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Miquel Raynal, Francesco Dolcini, Shawn Guo, linux-arm-kernel,
	Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable

On Tue, Dec 06, 2022 at 08:02:45PM +0100, Marek Vasut wrote:
> On 12/6/22 11:16, Miquel Raynal wrote:
> > Hi Francesco,
> 
> Hello everyone,
> 
> > francesco@dolcini.it wrote on Mon, 5 Dec 2022 20:07:18 +0100:
> > 
> > > On Mon, Dec 05, 2022 at 07:52:08PM +0100, Marek Vasut wrote:
> > > > On 12/5/22 19:18, Miquel Raynal wrote:
> > > > > marex@denx.de wrote on Mon, 5 Dec 2022 19:07:14 +0100:
> > > > > > On 12/5/22 18:58, Miquel Raynal wrote:
> > > > > > > , it's not
> > > > > > > complex to do, there are plenty of examples. This would be IMHO a
> > > > > > > better step ahead rather than just a cell change. Anyway, I don't mind
> > > > > > > reverting this once we've sorted this mess out and fixed U-Boot.
> > > > > > 
> > > > > > Won't we still have issues with older bootloader versions which
> > > > > > paste partitions directly into this &gpmi {} node, and which needs
> > > > > > to be fixed up in the parser in the end ?
> > > > > 
> > > > > I believe fdt_fixup_mtdparts() should be killed, so we should no longer
> > > > > have this problem.
> > > > 
> > > > The fdt_fixup_mtdparts is U-Boot code. If contemporary Linux kernel is
> > > > booted with ancient U-Boot, then you would still get defective DT passed to
> > > > Linux, and that should be fixed up by Linux. Removing fdt_fixup_mtdparts()
> > > > from current mainline U-Boot won't solve this problem.
> > > > 
> > > > I think this is also what Francesco is trying to convey (please correct me
> > > > if I'm wrong).
> > 
> > If we can get rid of fdt_fixup_mtdparts(), it means someone has to
> > create the partitions. I guess the easy way would be to just provide
> > mtdparts to Linux like all the other boards and let Linux deal with it.
> 
> This is based on an assumption that the platform kernel command line can be
> updated to insert such a workaround. If Francesco cannot update the
> bootloader, the kernel command line may be immutable all the same.

Exactly.

What I can do is update the latest stuff and enable people to upgrade, eventually.
But here we are talking about a board that is just generally available
in high volume to a multitude of people since years ... in practice the
vast majority of the users will not upgrade it.

> > Then we can just assume in Linux that perhaps if the partitions are
> > invalid (#size-cell is wrong?) then we should just stop their creation
> > and fallback to another mechanism instead of failing entirely. This way
> > no need for hackish changes in the parsers and compatibility is still
> > valid with old U-Boot (if mtdparts was provided on the cmdline, to be
> > checked). Otherwise we'll have to deal with it in Linux, that's a pity.
> 
> I am very much banking toward -- fix it up in the parser, just like any
> other firmware issue.

I agree again.

> Esp. since the fix up is printing a warning, and it is like a 2-liner
> patch.
Here we might assess if we need more to handle the other weird situation
in which a `partitions{}` node is present, U-Boot ignores it and the
kernel fails to detect the partitions. As of today colibri-imx7 is not
affected by this.

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-05 16:26 ` Marek Vasut
  2022-12-05 17:58   ` Miquel Raynal
@ 2022-12-08 10:51   ` Miquel Raynal
  2022-12-08 11:13     ` Thorsten Leemhuis
                       ` (2 more replies)
  1 sibling, 3 replies; 22+ messages in thread
From: Miquel Raynal @ 2022-12-08 10:51 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Francesco Dolcini, Shawn Guo, linux-arm-kernel,
	Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable, Thorsten Leemhuis

Hi Shawn,

+ Thorsten

marex@denx.de wrote on Mon, 5 Dec 2022 17:26:53 +0100:

> On 12/5/22 16:23, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > 
> > This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.
> > 
> > It introduced a boot regression on colibri-imx7, and potentially any
> > other i.MX7 boards with MTD partition list generated into the fdt by
> > U-Boot.
> > 
> > While the commit we are reverting here is not obviously wrong, it fixes
> > only a dt binding checker warning that is non-functional, while it
> > introduces a boot regression and there is no obvious fix ready.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
> > Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
> > Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
[...]
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
[...]
> Acked-by: Marek Vasut <marex@denx.de>
[...]

As discussed in the above links, boot is broken on imx7 Colibri boards,
this revert was the most quick and straightforward fix we agreed upon
with the hope (~ duty?) it would make it in v6.1. Any chance you could
pick this up rapidly and forward it to Linus? Or should we involve
him directly (Thorsten?).

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-08 10:51   ` Miquel Raynal
@ 2022-12-08 11:13     ` Thorsten Leemhuis
  2022-12-08 13:21     ` Marek Vasut
  2022-12-08 16:40     ` Francesco Dolcini
  2 siblings, 0 replies; 22+ messages in thread
From: Thorsten Leemhuis @ 2022-12-08 11:13 UTC (permalink / raw)
  To: Miquel Raynal, Marek Vasut
  Cc: Francesco Dolcini, Shawn Guo, linux-arm-kernel,
	Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable

On 08.12.22 11:51, Miquel Raynal wrote:
> Hi Shawn,
> 
> + Thorsten
> 
> marex@denx.de wrote on Mon, 5 Dec 2022 17:26:53 +0100:
> 
>> On 12/5/22 16:23, Francesco Dolcini wrote:
>>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>
>>> This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.
>>>
>>> It introduced a boot regression on colibri-imx7, and potentially any
>>> other i.MX7 boards with MTD partition list generated into the fdt by
>>> U-Boot.
>>>
>>> While the commit we are reverting here is not obviously wrong, it fixes
>>> only a dt binding checker warning that is non-functional, while it
>>> introduces a boot regression and there is no obvious fix ready.
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
>>> Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
>>> Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
>>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> [...]
>> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> [...]
>> Acked-by: Marek Vasut <marex@denx.de>
> [...]
> 
> As discussed in the above links, boot is broken on imx7 Colibri boards,
> this revert was the most quick and straightforward fix we agreed upon
> with the hope (~ duty?) it would make it in v6.1. Any chance you could
> pick this up rapidly and forward it to Linus? Or should we involve
> him directly (Thorsten?).

Asking Linus directly often is fine, if it's something urgent and the
maintainer that usually would handle a patch is MIA. But in this
particular case it's likely not the best strategy, as it seems
753395ea1e45 was merged via the ARM soc tree. In that case I'd say the
revert should ideally go through there as well, hence I'd suggest asking
those maintainers (e.g. Arnd and Olof) is the right move at this point
in time (would be something different if today was release day; but even
then it would be wise to have them involved).

Ciao, Thorsten

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-08 10:51   ` Miquel Raynal
  2022-12-08 11:13     ` Thorsten Leemhuis
@ 2022-12-08 13:21     ` Marek Vasut
  2022-12-08 13:49       ` Francesco Dolcini
  2022-12-08 16:40     ` Francesco Dolcini
  2 siblings, 1 reply; 22+ messages in thread
From: Marek Vasut @ 2022-12-08 13:21 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Francesco Dolcini, Shawn Guo, linux-arm-kernel,
	Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable, Thorsten Leemhuis

On 12/8/22 11:51, Miquel Raynal wrote:
> Hi Shawn,

Hi,

> + Thorsten
> 
> marex@denx.de wrote on Mon, 5 Dec 2022 17:26:53 +0100:
> 
>> On 12/5/22 16:23, Francesco Dolcini wrote:
>>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>
>>> This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.
>>>
>>> It introduced a boot regression on colibri-imx7, and potentially any
>>> other i.MX7 boards with MTD partition list generated into the fdt by
>>> U-Boot.
>>>
>>> While the commit we are reverting here is not obviously wrong, it fixes
>>> only a dt binding checker warning that is non-functional, while it
>>> introduces a boot regression and there is no obvious fix ready.
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
>>> Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
>>> Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
>>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> [...]
>> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> [...]
>> Acked-by: Marek Vasut <marex@denx.de>
> [...]
> 
> As discussed in the above links, boot is broken on imx7 Colibri boards,
> this revert was the most quick and straightforward fix we agreed upon
> with the hope (~ duty?) it would make it in v6.1. Any chance you could
> pick this up rapidly and forward it to Linus? Or should we involve
> him directly (Thorsten?).

It seems neither Francesco nor me agree that this is the right approach 
and rather the fix should be the two-liner change to the OF partition 
parser, so maybe this should not be picked ?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-08 13:21     ` Marek Vasut
@ 2022-12-08 13:49       ` Francesco Dolcini
  2022-12-08 13:54         ` Marek Vasut
  0 siblings, 1 reply; 22+ messages in thread
From: Francesco Dolcini @ 2022-12-08 13:49 UTC (permalink / raw)
  To: Marek Vasut, Miquel Raynal
  Cc: Shawn Guo, linux-arm-kernel, Francesco Dolcini, Rob Herring,
	Krzysztof Kozlowski, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, devicetree, linux-mtd, stable,
	Thorsten Leemhuis

Il 8 dicembre 2022 14:21:31 CET, Marek Vasut <marex@denx.de> ha scritto:
>On 12/8/22 11:51, Miquel Raynal wrote:
>> Hi Shawn,
>
>Hi,
>
>> + Thorsten
>> 
>> marex@denx.de wrote on Mon, 5 Dec 2022 17:26:53 +0100:
>> 
>>> On 12/5/22 16:23, Francesco Dolcini wrote:
>>>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>> 
>>>> This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.
>>>> 
>>>> It introduced a boot regression on colibri-imx7, and potentially any
>>>> other i.MX7 boards with MTD partition list generated into the fdt by
>>>> U-Boot.
>>>> 
>>>> While the commit we are reverting here is not obviously wrong, it fixes
>>>> only a dt binding checker warning that is non-functional, while it
>>>> introduces a boot regression and there is no obvious fix ready.
>>>> 
>>>> Cc: stable@vger.kernel.org
>>>> Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
>>>> Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
>>>> Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
>>>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>> [...]
>>> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
>> [...]
>>> Acked-by: Marek Vasut <marex@denx.de>
>> [...]
>> 
>> As discussed in the above links, boot is broken on imx7 Colibri boards,
>> this revert was the most quick and straightforward fix we agreed upon
>> with the hope (~ duty?) it would make it in v6.1. Any chance you could
>> pick this up rapidly and forward it to Linus? Or should we involve
>> him directly (Thorsten?).
>
>It seems neither Francesco nor me agree that this is the right approach and rather the fix should be the two-liner change to the OF partition parser, so maybe this should not be picked ?

I think that the 2 lines change might not be good enough to properly handle the U-Boot generated OF partitions in the general case, even if it fixes my specific issue.

Given that I would do the revert as an immediate first step.

Francesco 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-08 13:49       ` Francesco Dolcini
@ 2022-12-08 13:54         ` Marek Vasut
  2022-12-08 16:26           ` Francesco Dolcini
  0 siblings, 1 reply; 22+ messages in thread
From: Marek Vasut @ 2022-12-08 13:54 UTC (permalink / raw)
  To: Francesco Dolcini, Miquel Raynal
  Cc: Shawn Guo, linux-arm-kernel, Francesco Dolcini, Rob Herring,
	Krzysztof Kozlowski, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, devicetree, linux-mtd, stable,
	Thorsten Leemhuis

On 12/8/22 14:49, Francesco Dolcini wrote:
> Il 8 dicembre 2022 14:21:31 CET, Marek Vasut <marex@denx.de> ha scritto:
>> On 12/8/22 11:51, Miquel Raynal wrote:
>>> Hi Shawn,
>>
>> Hi,
>>
>>> + Thorsten
>>>
>>> marex@denx.de wrote on Mon, 5 Dec 2022 17:26:53 +0100:
>>>
>>>> On 12/5/22 16:23, Francesco Dolcini wrote:
>>>>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>>>
>>>>> This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.
>>>>>
>>>>> It introduced a boot regression on colibri-imx7, and potentially any
>>>>> other i.MX7 boards with MTD partition list generated into the fdt by
>>>>> U-Boot.
>>>>>
>>>>> While the commit we are reverting here is not obviously wrong, it fixes
>>>>> only a dt binding checker warning that is non-functional, while it
>>>>> introduces a boot regression and there is no obvious fix ready.
>>>>>
>>>>> Cc: stable@vger.kernel.org
>>>>> Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
>>>>> Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
>>>>> Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
>>>>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>>> [...]
>>>> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
>>> [...]
>>>> Acked-by: Marek Vasut <marex@denx.de>
>>> [...]
>>>
>>> As discussed in the above links, boot is broken on imx7 Colibri boards,
>>> this revert was the most quick and straightforward fix we agreed upon
>>> with the hope (~ duty?) it would make it in v6.1. Any chance you could
>>> pick this up rapidly and forward it to Linus? Or should we involve
>>> him directly (Thorsten?).
>>
>> It seems neither Francesco nor me agree that this is the right approach and rather the fix should be the two-liner change to the OF partition parser, so maybe this should not be picked ?
> 
> I think that the 2 lines change might not be good enough to properly handle the U-Boot generated OF partitions in the general case, even if it fixes my specific issue.
> 
> Given that I would do the revert as an immediate first step.

Then that's fine, let's do it.

Will you also follow up on the parser fix please ?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-08 13:54         ` Marek Vasut
@ 2022-12-08 16:26           ` Francesco Dolcini
  0 siblings, 0 replies; 22+ messages in thread
From: Francesco Dolcini @ 2022-12-08 16:26 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Francesco Dolcini, Miquel Raynal, Shawn Guo, linux-arm-kernel,
	Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable, Thorsten Leemhuis

On Thu, Dec 08, 2022 at 02:54:43PM +0100, Marek Vasut wrote:
> On 12/8/22 14:49, Francesco Dolcini wrote:
> > Given that I would do the revert as an immediate first step.
> 
> Then that's fine, let's do it.
> 
> Will you also follow up on the parser fix please ?

Yep, I'll try to squeeze some time to work on it in the next weeks.

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-08 10:51   ` Miquel Raynal
  2022-12-08 11:13     ` Thorsten Leemhuis
  2022-12-08 13:21     ` Marek Vasut
@ 2022-12-08 16:40     ` Francesco Dolcini
  2022-12-09 13:30       ` Thorsten Leemhuis
  2 siblings, 1 reply; 22+ messages in thread
From: Francesco Dolcini @ 2022-12-08 16:40 UTC (permalink / raw)
  To: Miquel Raynal, Arnd Bergmann
  Cc: Marek Vasut, Francesco Dolcini, Shawn Guo, linux-arm-kernel,
	Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable, Thorsten Leemhuis

+ Arnd

On Thu, Dec 08, 2022 at 11:51:24AM +0100, Miquel Raynal wrote:
> marex@denx.de wrote on Mon, 5 Dec 2022 17:26:53 +0100:
> > On 12/5/22 16:23, Francesco Dolcini wrote:
> > > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > 
> > > This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.
> > > 
> > > It introduced a boot regression on colibri-imx7, and potentially any
> > > other i.MX7 boards with MTD partition list generated into the fdt by
> > > U-Boot.
> > > 
> > > While the commit we are reverting here is not obviously wrong, it fixes
> > > only a dt binding checker warning that is non-functional, while it
> > > introduces a boot regression and there is no obvious fix ready.
> > > 
> > > Cc: stable@vger.kernel.org
> > > Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
> > > Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
> > > Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
> > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> [...]
> > Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> [...]
> > Acked-by: Marek Vasut <marex@denx.de>
> [...]
> 
> As discussed in the above links, boot is broken on imx7 Colibri boards,
> this revert was the most quick and straightforward fix we agreed upon
> with the hope (~ duty?) it would make it in v6.1. Any chance you could
> pick this up rapidly and forward it to Linus? Or should we involve
> him directly (Thorsten?).

Hello Arnd,
FYI - see Miquel explanation above.

Francesco

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-08 16:40     ` Francesco Dolcini
@ 2022-12-09 13:30       ` Thorsten Leemhuis
  2022-12-09 15:25         ` Arnd Bergmann
  0 siblings, 1 reply; 22+ messages in thread
From: Thorsten Leemhuis @ 2022-12-09 13:30 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Marek Vasut, Shawn Guo, linux-arm-kernel, Francesco Dolcini,
	Rob Herring, Krzysztof Kozlowski, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-mtd, stable, Francesco Dolcini, Miquel Raynal

On 08.12.22 17:40, Francesco Dolcini wrote:
> + Arnd

Arnd, have you seen this? We haven't heard anything from Shawn afaics,
who normally would take care of a patch like this. Hence could you
consider picking up the patch at the start of this thread (e.g.
https://lore.kernel.org/all/20221205152327.26881-1-francesco@dolcini.it/
) and send it to Linus in the next 48 hours? It seems low-risk and fixes
a regression introduced this cycle various people care about. That's why
I'll likely ask Linus to consider picking this up directly before
releasing 6.1, if I don't hear anything from you soon. But I'd prefer if
the patch would go through at least somewhat the proper channels;
alternatively a ACK from you to signal Linus "yeah, pick this up" would
help as well.

Ciao, Thorsten


> On Thu, Dec 08, 2022 at 11:51:24AM +0100, Miquel Raynal wrote:
>> marex@denx.de wrote on Mon, 5 Dec 2022 17:26:53 +0100:
>>> On 12/5/22 16:23, Francesco Dolcini wrote:
>>>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>>
>>>> This reverts commit 753395ea1e45c724150070b5785900b6a44bd5fb.
>>>>
>>>> It introduced a boot regression on colibri-imx7, and potentially any
>>>> other i.MX7 boards with MTD partition list generated into the fdt by
>>>> U-Boot.
>>>>
>>>> While the commit we are reverting here is not obviously wrong, it fixes
>>>> only a dt binding checker warning that is non-functional, while it
>>>> introduces a boot regression and there is no obvious fix ready.
>>>>
>>>> Cc: stable@vger.kernel.org
>>>> Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
>>>> Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
>>>> Link: https://lore.kernel.org/all/20221205144917.6514168a@xps-13/
>>>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>> [...]
>>> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
>> [...]
>>> Acked-by: Marek Vasut <marex@denx.de>
>> [...]
>>
>> As discussed in the above links, boot is broken on imx7 Colibri boards,
>> this revert was the most quick and straightforward fix we agreed upon
>> with the hope (~ duty?) it would make it in v6.1. Any chance you could
>> pick this up rapidly and forward it to Linus? Or should we involve
>> him directly (Thorsten?).
> 
> Hello Arnd,
> FYI - see Miquel explanation above.
> 
> Francesco
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-09 13:30       ` Thorsten Leemhuis
@ 2022-12-09 15:25         ` Arnd Bergmann
  2022-12-09 15:38           ` Miquel Raynal
  0 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2022-12-09 15:25 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: Marek Vasut, Shawn Guo, linux-arm-kernel, Francesco Dolcini,
	Rob Herring, Krzysztof Kozlowski, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-mtd, stable, Francesco Dolcini, Miquel Raynal

On Fri, Dec 9, 2022, at 14:30, Thorsten Leemhuis wrote:
> On 08.12.22 17:40, Francesco Dolcini wrote:
>> + Arnd
>
> Arnd, have you seen this? We haven't heard anything from Shawn afaics,
> who normally would take care of a patch like this. Hence could you
> consider picking up the patch at the start of this thread (e.g.
> https://lore.kernel.org/all/20221205152327.26881-1-francesco@dolcini.it/
> ) and send it to Linus in the next 48 hours? It seems low-risk and fixes
> a regression introduced this cycle various people care about. That's why
> I'll likely ask Linus to consider picking this up directly before
> releasing 6.1, if I don't hear anything from you soon. But I'd prefer if
> the patch would go through at least somewhat the proper channels;
> alternatively a ACK from you to signal Linus "yeah, pick this up" would
> help as well.

Done now.

   Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells"
  2022-12-09 15:25         ` Arnd Bergmann
@ 2022-12-09 15:38           ` Miquel Raynal
  0 siblings, 0 replies; 22+ messages in thread
From: Miquel Raynal @ 2022-12-09 15:38 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Thorsten Leemhuis, Marek Vasut, Shawn Guo, linux-arm-kernel,
	Francesco Dolcini, Rob Herring, Krzysztof Kozlowski,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-mtd, stable, Francesco Dolcini

Hi Arnd,

arnd@arndb.de wrote on Fri, 09 Dec 2022 16:25:28 +0100:

> On Fri, Dec 9, 2022, at 14:30, Thorsten Leemhuis wrote:
> > On 08.12.22 17:40, Francesco Dolcini wrote:  
> >> + Arnd  
> >
> > Arnd, have you seen this? We haven't heard anything from Shawn afaics,
> > who normally would take care of a patch like this. Hence could you
> > consider picking up the patch at the start of this thread (e.g.
> > https://lore.kernel.org/all/20221205152327.26881-1-francesco@dolcini.it/
> > ) and send it to Linus in the next 48 hours? It seems low-risk and fixes
> > a regression introduced this cycle various people care about. That's why
> > I'll likely ask Linus to consider picking this up directly before
> > releasing 6.1, if I don't hear anything from you soon. But I'd prefer if
> > the patch would go through at least somewhat the proper channels;
> > alternatively a ACK from you to signal Linus "yeah, pick this up" would
> > help as well.  
> 
> Done now.
> 
>    Arnd

Thanks a lot.

Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2022-12-09 15:49 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-05 15:23 [PATCH v1] Revert "ARM: dts: imx7: Fix NAND controller size-cells" Francesco Dolcini
2022-12-05 16:26 ` Marek Vasut
2022-12-05 17:58   ` Miquel Raynal
2022-12-05 18:07     ` Marek Vasut
2022-12-05 18:15       ` Francesco Dolcini
2022-12-05 18:18       ` Miquel Raynal
2022-12-05 18:52         ` Marek Vasut
2022-12-05 19:07           ` Francesco Dolcini
2022-12-06 10:16             ` Miquel Raynal
2022-12-06 19:02               ` Marek Vasut
2022-12-07  7:49                 ` Francesco Dolcini
2022-12-08 10:51   ` Miquel Raynal
2022-12-08 11:13     ` Thorsten Leemhuis
2022-12-08 13:21     ` Marek Vasut
2022-12-08 13:49       ` Francesco Dolcini
2022-12-08 13:54         ` Marek Vasut
2022-12-08 16:26           ` Francesco Dolcini
2022-12-08 16:40     ` Francesco Dolcini
2022-12-09 13:30       ` Thorsten Leemhuis
2022-12-09 15:25         ` Arnd Bergmann
2022-12-09 15:38           ` Miquel Raynal
2022-12-05 16:31 ` Miquel Raynal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).