Re: [PATCH] distroboot: Fix ubifs

From: "Pali Rohár" <pali@kernel.org>
To: Frieder Schrempf <frieder.schrempf@kontron.de>
Cc: u-boot@lists.denx.de, Simon Glass <sjg@chromium.org>,
	Sjoerd Simons <sjoerd.simons@collabora.co.uk>,
	Parthiban Nallathambi <parthiban@linumiz.com>,
	Hiremath Gireesh <Gireesh.Hiremath@in.bosch.com>,
	Marcel Ziswiler <marcel.ziswiler@toradex.com>,
	Patrick Delaunay <patrick.delaunay@foss.st.com>,
	"Derald D. Woods" <woods.technical@gmail.com>,
	Govindaraji Sivanantham <Govindaraji.Sivanantham@in.bosch.com>,
	Navin Sankar Velliangiri <navin@linumiz.com>,
	Martyn Welch <martyn.welch@collabora.com>,
	Patrice Chotard <patrice.chotard@foss.st.com>
Subject: Re: [PATCH] distroboot: Fix ubifs
Date: Tue, 5 Jul 2022 12:15:25 +0200	[thread overview]
Message-ID: <20220705101525.iw3ei54xctobykcr@pali> (raw)
In-Reply-To: <a61c6c35-e798-2606-e916-b2843492cbc7@kontron.de>

On Wednesday 29 June 2022 12:29:40 Frieder Schrempf wrote:
> Am 23.06.22 um 18:09 schrieb Pali Rohár:
> > On Tuesday 31 May 2022 10:32:36 Pali Rohár wrote:
> >> Fix multiple issues in ubifs distroboot code:
> >>
> >> U-Boot supports attaching only one MTD device as UBI at the time. So
> >> always call 'ubifsmount ubi0:${bootubivol}' for mounting UBI volume
> >> ${bootubivol}. Usage of 'ubi${devnum}' is incorrect as 'ubi part'
> >> command attach MTD device always as UBI device ubi0.
> >>
> >> Set distroboot ${bootfstype} variable to ubifs in ubifs_boot command.
> >> Distroboot scripts require ${bootfstype} variable to be properly set and it
> >> is already set for all other boot types.
> >>
> >> Set distroboot ${distro_bootpart} variable to ${bootubivol} value. UBI
> >> device does not have partitions, but has volumes. Distroboot scripts
> >> require something to be set in ${distro_bootpart} variable, so set it to
> >> the UBI volume which is currently mounted by ubifs.
> >>
> >> Set distroboot ${devnum} variable to fixed string "ubi0". ubifs code
> >> differs from the other partition code that it requires "ubi" prefix before
> >> number.
> >>
> >> Explicitly unmount ubifs volume after loading all data from it. This allows
> >> to detach UBI device from MTD device.
> >>
> >> Move definition of MTD device with UBI and UBI volume with ubifs filesystem
> >> from global env variables ${bootubipart} and ${bootubivol} into the
> >> distroboot "func" macro, defined in board include config files. UBIFS
> >> distroboot macros then set ${bootubipart} and ${bootubivol} local variables
> >> for compatibility with existing distroboot scripts.
> >>
> >> This last change allows to define more UBIFS target devices and make it
> >> clear what is boot MTD/UBI device.
> >>
> >> All board include config files are adjusted to use this new scheme of
> >> specifying boot MTD/UBI device.
> >>
> >> Signed-off-by: Pali Rohár <pali@kernel.org>
> > 
> > PING?
> > 
> 
> Sorry, I currently don't have the time to properly review and/or test
> this. Though, in general the changes look good and I can at least provide:
> 
> Acked-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> I think it would help to split this up in smaller chunks, so people can
> review it more easily...

Splitting these changes is hard as it needs to be done atomtically to do
not break boards or do not break compilation.

I think it is better to have one bigger change than lot of smaller which
cause compile or runtime errors until all small changes are applied.