All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Francesco Dolcini <francesco@dolcini.it>
Cc: Marek Vasut <marex@denx.de>, Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	linux-mtd@lists.infradead.org,
	Francesco Dolcini <francesco.dolcini@toradex.com>,
	Shawn Guo <shawnguo@kernel.org>,
	linux-arm-kernel@lists.infradead.org, stable@vger.kernel.org,
	u-boot@lists.denx.de
Subject: Re: [PATCH v1] mtd: parsers: ofpart: Fix parsing when size-cells is 0
Date: Mon, 5 Dec 2022 14:49:17 +0100	[thread overview]
Message-ID: <20221205144917.6514168a@xps-13> (raw)
In-Reply-To: <Y43VdPftDbq6cD2L@francesco-nb.int.toradex.com>

Hi Francesco,

francesco@dolcini.it wrote on Mon, 5 Dec 2022 12:26:44 +0100:

> On Fri, Dec 02, 2022 at 06:08:22PM +0100, Marek Vasut wrote:
> > But here I would say this is a firmware bug and it might have to be handled
> > like a firmware bug, i.e. with fixup in the partition parser. I seem to be
> > changing my opinion here again.  
> 
> I was thinking at this over the weekend, and I came to the following
> ideas:
> 
>  - we need some improvement on the fixup we already have in the
>    partition parser. We cannot ignore the fdt produced by U-Boot - as
>    bad as it is.
>  - the proposed fixup is fine for the immediate need, but it is
>    not going to be enough to cover the general issue with the U-Boot
>    generated partitions. U-Boot might keep generating partitions as direct
>    child of the nand controller even when a partitions{} node is
>    available. In this case the current parser just fails since it looks
>    only into it and it will find it empty.
>  - the current U-Boot only handle partitions{} as a direct child of the
>    nand-controller, the nand-chip is ignored. This is not the way it is
>    supposed to work. U-Boot code would need to be improved.

I've been thinking about it this weekend as well and the current fix
which "just set" s_cell to 1 seems risky for me, it is typically the
type of quick & dirty fix that might even break other board (nobody
knew that U-Boot current logic expected #size-cells to be set in the
DT, what if another "broken" DT expects the opposite...), not
mentioning potential issues with big storages (> 4GiB).

All in all, I really think we should revert the DT change now, reverting
as little to no drawbacks besides a dt_binding_check warning and gives
us time to deal with it properly (both in U-Boot and Linux).

> With all of that said I think that Miquel is right
> 
> > When a patch breaks a board and there is no straight fix, you revert
> > it, then you think harder. That's what I am saying. This is a temporary
> > solution.  
> 
> ?
> 
> Francesco
> 
> 

Thanks,
Miquèl

WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Francesco Dolcini <francesco@dolcini.it>
Cc: Marek Vasut <marex@denx.de>, Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	linux-mtd@lists.infradead.org,
	Francesco Dolcini <francesco.dolcini@toradex.com>,
	Shawn Guo <shawnguo@kernel.org>,
	linux-arm-kernel@lists.infradead.org, stable@vger.kernel.org,
	u-boot@lists.denx.de
Subject: Re: [PATCH v1] mtd: parsers: ofpart: Fix parsing when size-cells is 0
Date: Mon, 5 Dec 2022 14:49:17 +0100	[thread overview]
Message-ID: <20221205144917.6514168a@xps-13> (raw)
In-Reply-To: <Y43VdPftDbq6cD2L@francesco-nb.int.toradex.com>

Hi Francesco,

francesco@dolcini.it wrote on Mon, 5 Dec 2022 12:26:44 +0100:

> On Fri, Dec 02, 2022 at 06:08:22PM +0100, Marek Vasut wrote:
> > But here I would say this is a firmware bug and it might have to be handled
> > like a firmware bug, i.e. with fixup in the partition parser. I seem to be
> > changing my opinion here again.  
> 
> I was thinking at this over the weekend, and I came to the following
> ideas:
> 
>  - we need some improvement on the fixup we already have in the
>    partition parser. We cannot ignore the fdt produced by U-Boot - as
>    bad as it is.
>  - the proposed fixup is fine for the immediate need, but it is
>    not going to be enough to cover the general issue with the U-Boot
>    generated partitions. U-Boot might keep generating partitions as direct
>    child of the nand controller even when a partitions{} node is
>    available. In this case the current parser just fails since it looks
>    only into it and it will find it empty.
>  - the current U-Boot only handle partitions{} as a direct child of the
>    nand-controller, the nand-chip is ignored. This is not the way it is
>    supposed to work. U-Boot code would need to be improved.

I've been thinking about it this weekend as well and the current fix
which "just set" s_cell to 1 seems risky for me, it is typically the
type of quick & dirty fix that might even break other board (nobody
knew that U-Boot current logic expected #size-cells to be set in the
DT, what if another "broken" DT expects the opposite...), not
mentioning potential issues with big storages (> 4GiB).

All in all, I really think we should revert the DT change now, reverting
as little to no drawbacks besides a dt_binding_check warning and gives
us time to deal with it properly (both in U-Boot and Linux).

> With all of that said I think that Miquel is right
> 
> > When a patch breaks a board and there is no straight fix, you revert
> > it, then you think harder. That's what I am saying. This is a temporary
> > solution.  
> 
> ?
> 
> Francesco
> 
> 

Thanks,
Miquèl

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

WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Francesco Dolcini <francesco@dolcini.it>
Cc: Marek Vasut <marex@denx.de>, Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	linux-mtd@lists.infradead.org,
	Francesco Dolcini <francesco.dolcini@toradex.com>,
	Shawn Guo <shawnguo@kernel.org>,
	linux-arm-kernel@lists.infradead.org, stable@vger.kernel.org,
	u-boot@lists.denx.de
Subject: Re: [PATCH v1] mtd: parsers: ofpart: Fix parsing when size-cells is 0
Date: Mon, 5 Dec 2022 14:49:17 +0100	[thread overview]
Message-ID: <20221205144917.6514168a@xps-13> (raw)
In-Reply-To: <Y43VdPftDbq6cD2L@francesco-nb.int.toradex.com>

Hi Francesco,

francesco@dolcini.it wrote on Mon, 5 Dec 2022 12:26:44 +0100:

> On Fri, Dec 02, 2022 at 06:08:22PM +0100, Marek Vasut wrote:
> > But here I would say this is a firmware bug and it might have to be handled
> > like a firmware bug, i.e. with fixup in the partition parser. I seem to be
> > changing my opinion here again.  
> 
> I was thinking at this over the weekend, and I came to the following
> ideas:
> 
>  - we need some improvement on the fixup we already have in the
>    partition parser. We cannot ignore the fdt produced by U-Boot - as
>    bad as it is.
>  - the proposed fixup is fine for the immediate need, but it is
>    not going to be enough to cover the general issue with the U-Boot
>    generated partitions. U-Boot might keep generating partitions as direct
>    child of the nand controller even when a partitions{} node is
>    available. In this case the current parser just fails since it looks
>    only into it and it will find it empty.
>  - the current U-Boot only handle partitions{} as a direct child of the
>    nand-controller, the nand-chip is ignored. This is not the way it is
>    supposed to work. U-Boot code would need to be improved.

I've been thinking about it this weekend as well and the current fix
which "just set" s_cell to 1 seems risky for me, it is typically the
type of quick & dirty fix that might even break other board (nobody
knew that U-Boot current logic expected #size-cells to be set in the
DT, what if another "broken" DT expects the opposite...), not
mentioning potential issues with big storages (> 4GiB).

All in all, I really think we should revert the DT change now, reverting
as little to no drawbacks besides a dt_binding_check warning and gives
us time to deal with it properly (both in U-Boot and Linux).

> With all of that said I think that Miquel is right
> 
> > When a patch breaks a board and there is no straight fix, you revert
> > it, then you think harder. That's what I am saying. This is a temporary
> > solution.  
> 
> ?
> 
> Francesco
> 
> 

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply	other threads:[~2022-12-05 13:49 UTC|newest]

Thread overview: 144+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-02  7:19 [PATCH v1] mtd: parsers: ofpart: Fix parsing when size-cells is 0 Francesco Dolcini
2022-12-02  7:19 ` Francesco Dolcini
2022-12-02  7:19 ` Francesco Dolcini
2022-12-02  9:14 ` Miquel Raynal
2022-12-02  9:14   ` Miquel Raynal
2022-12-02  9:14   ` Miquel Raynal
2022-12-02 10:12   ` Francesco Dolcini
2022-12-02 10:12     ` Francesco Dolcini
2022-12-02 10:12     ` Francesco Dolcini
2022-12-02 10:24     ` Francesco Dolcini
2022-12-02 10:24       ` Francesco Dolcini
2022-12-02 10:24       ` Francesco Dolcini
2022-12-02 10:53       ` Miquel Raynal
2022-12-02 10:53         ` Miquel Raynal
2022-12-02 10:53         ` Miquel Raynal
2022-12-02 11:23         ` Francesco Dolcini
2022-12-02 11:23           ` Francesco Dolcini
2022-12-02 11:23           ` Francesco Dolcini
2022-12-02 14:05           ` Miquel Raynal
2022-12-02 14:05             ` Miquel Raynal
2022-12-02 14:05             ` Miquel Raynal
2022-12-02 14:31             ` Marek Vasut
2022-12-02 14:31               ` Marek Vasut
2022-12-02 14:31               ` Marek Vasut
2022-12-02 15:00               ` Miquel Raynal
2022-12-02 15:00                 ` Miquel Raynal
2022-12-02 15:00                 ` Miquel Raynal
2022-12-02 15:23                 ` Marek Vasut
2022-12-02 15:23                   ` Marek Vasut
2022-12-02 15:23                   ` Marek Vasut
2022-12-02 15:49                   ` Miquel Raynal
2022-12-02 15:49                     ` Miquel Raynal
2022-12-02 15:49                     ` Miquel Raynal
2022-12-02 16:01                     ` Miquel Raynal
2022-12-02 16:01                       ` Miquel Raynal
2022-12-02 16:01                       ` Miquel Raynal
2022-12-02 16:17                     ` Marek Vasut
2022-12-02 16:17                       ` Marek Vasut
2022-12-02 16:17                       ` Marek Vasut
2022-12-02 16:42                       ` Miquel Raynal
2022-12-02 16:42                         ` Miquel Raynal
2022-12-02 16:42                         ` Miquel Raynal
2022-12-02 16:52                         ` Marek Vasut
2022-12-02 16:52                           ` Marek Vasut
2022-12-02 16:52                           ` Marek Vasut
2022-12-02 16:57                           ` Miquel Raynal
2022-12-02 16:57                             ` Miquel Raynal
2022-12-02 16:57                             ` Miquel Raynal
2022-12-02 17:08                             ` Marek Vasut
2022-12-02 17:08                               ` Marek Vasut
2022-12-02 17:08                               ` Marek Vasut
2022-12-05 11:26                               ` Francesco Dolcini
2022-12-05 11:26                                 ` Francesco Dolcini
2022-12-05 11:26                                 ` Francesco Dolcini
2022-12-05 13:49                                 ` Miquel Raynal [this message]
2022-12-05 13:49                                   ` Miquel Raynal
2022-12-05 13:49                                   ` Miquel Raynal
2022-12-05 16:25                                   ` Marek Vasut
2022-12-05 16:25                                     ` Marek Vasut
2022-12-05 16:25                                     ` Marek Vasut
2022-12-15  7:16                                     ` Miquel Raynal
2022-12-15  7:16                                       ` Miquel Raynal
2022-12-15  7:16                                       ` Miquel Raynal
2022-12-15  7:45                                       ` Marek Vasut
2022-12-15  7:45                                         ` Marek Vasut
2022-12-15  7:45                                         ` Marek Vasut
2022-12-15  8:04                                         ` Miquel Raynal
2022-12-15  8:04                                           ` Miquel Raynal
2022-12-15  8:04                                           ` Miquel Raynal
2022-12-16  0:36                                           ` Marek Vasut
2022-12-16  0:36                                             ` Marek Vasut
2022-12-16  0:36                                             ` Marek Vasut
2022-12-16  7:52                                             ` Francesco Dolcini
2022-12-16  7:52                                               ` Francesco Dolcini
2022-12-16  7:52                                               ` Francesco Dolcini
2022-12-16  7:45                                       ` Francesco Dolcini
2022-12-16  7:45                                         ` Francesco Dolcini
2022-12-16  7:45                                         ` Francesco Dolcini
2022-12-16 10:46                                         ` Marek Vasut
2022-12-16 10:46                                           ` Marek Vasut
2022-12-16 10:46                                           ` Marek Vasut
2022-12-16 11:01                                           ` Miquel Raynal
2022-12-16 11:01                                             ` Miquel Raynal
2022-12-16 11:01                                             ` Miquel Raynal
2022-12-16 12:37                                             ` Francesco Dolcini
2022-12-16 12:37                                               ` Francesco Dolcini
2022-12-16 12:37                                               ` Francesco Dolcini
2022-12-16 13:37                                               ` Miquel Raynal
2022-12-16 13:37                                                 ` Miquel Raynal
2022-12-16 13:37                                                 ` Miquel Raynal
2022-12-16 14:32                                                 ` Marek Vasut
2022-12-16 14:32                                                   ` Marek Vasut
2022-12-16 14:32                                                   ` Marek Vasut
2022-12-16 15:35                                                   ` Miquel Raynal
2022-12-16 15:35                                                     ` Miquel Raynal
2022-12-16 15:35                                                     ` Miquel Raynal
2022-12-16 16:30                                                     ` Francesco Dolcini
2022-12-16 16:30                                                       ` Francesco Dolcini
2022-12-16 16:30                                                       ` Francesco Dolcini
2023-01-02  9:40                                                       ` Miquel Raynal
2023-01-02  9:40                                                         ` Miquel Raynal
2023-01-02  9:40                                                         ` Miquel Raynal
2023-01-05 11:33                                                         ` Miquel Raynal
2023-01-05 11:33                                                           ` Miquel Raynal
2023-01-05 11:33                                                           ` Miquel Raynal
2023-01-05 12:47                                                           ` Francesco Dolcini
2023-01-05 12:47                                                             ` Francesco Dolcini
2023-01-05 12:47                                                             ` Francesco Dolcini
2023-01-05 14:51                                                             ` Marek Vasut
2023-01-05 14:51                                                               ` Marek Vasut
2023-01-05 14:51                                                               ` Marek Vasut
2023-01-05 15:03                                                               ` Miquel Raynal
2023-01-05 15:03                                                                 ` Miquel Raynal
2023-01-05 15:03                                                                 ` Miquel Raynal
2022-12-02 17:20                         ` Francesco Dolcini
2022-12-02 17:20                           ` Francesco Dolcini
2022-12-02 17:20                           ` Francesco Dolcini
2022-12-05 11:30                           ` Francesco Dolcini
2022-12-05 11:30                             ` Francesco Dolcini
2022-12-05 11:30                             ` Francesco Dolcini
2022-12-05 15:28                             ` Miquel Raynal
2022-12-05 15:28                               ` Miquel Raynal
2022-12-05 15:28                               ` Miquel Raynal
2022-12-02 16:45                       ` Francesco Dolcini
2022-12-02 16:45                         ` Francesco Dolcini
2022-12-02 16:45                         ` Francesco Dolcini
2022-12-02 17:05                         ` Miquel Raynal
2022-12-02 17:05                           ` Miquel Raynal
2022-12-02 17:05                           ` Miquel Raynal
2022-12-02 15:56               ` Thorsten Leemhuis
2022-12-02 15:56                 ` Thorsten Leemhuis
2022-12-02 15:56                 ` Thorsten Leemhuis
2022-12-04 12:50                 ` Marek Vasut
2022-12-04 12:50                   ` Marek Vasut
2022-12-04 12:50                   ` Marek Vasut
2022-12-04 12:59                   ` Thorsten Leemhuis
2022-12-04 12:59                     ` Thorsten Leemhuis
2022-12-04 12:59                     ` Thorsten Leemhuis
2022-12-04 15:50                     ` Marek Vasut
2022-12-04 15:50                       ` Marek Vasut
2022-12-04 15:50                       ` Marek Vasut
2022-12-02 12:43 ` Greg KH
2022-12-02 12:43   ` Greg KH
2022-12-02 12:43   ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221205144917.6514168a@xps-13 \
    --to=miquel.raynal@bootlin.com \
    --cc=francesco.dolcini@toradex.com \
    --cc=francesco@dolcini.it \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marex@denx.de \
    --cc=richard@nod.at \
    --cc=shawnguo@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=u-boot@lists.denx.de \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.