| Message ID | 20220808144517.22967-1-stefan.herbrechtsmeier-oss@weidmueller.com |
|---|---|
| State | Accepted |
| Commit | 2f03a639f36d305800b8befe066f0d3a1616aed0 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | disk: part: remove dependency to ubifs for spl | expand |
Hi Stefan, On Mon, 8 Aug 2022 at 08:45, Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com> wrote: > > From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> > > The spl doesn't support ubifs and thereby doesn't provide the > ubifs_is_mounted function. Remove the dependency to ubifs for the spl. > > Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> > > --- > > disk/part.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/disk/part.c b/disk/part.c > index 79955c7fb0..de1b917e84 100644 > --- a/disk/part.c > +++ b/disk/part.c > @@ -479,7 +479,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, > } > #endif > > -#ifdef CONFIG_CMD_UBIFS > +#if IS_ENABLED(CONFIG_CMD_UBIFS) && !IS_ENABLED(CONFIG_SPL_BUILD) > /* > * Special-case ubi, ubi goes through a mtd, rather than through > * a regular block device. > -- > 2.30.2 > A CMD config should not be used outside cmd/ - can we add a proper CONFIG_UBIFS ? Regards, Simon
Hi Simon, Am 08.08.2022 um 21:26 schrieb Simon Glass: > Hi Stefan, > > On Mon, 8 Aug 2022 at 08:45, Stefan Herbrechtsmeier > <stefan.herbrechtsmeier-oss@weidmueller.com> wrote: >> >> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >> >> The spl doesn't support ubifs and thereby doesn't provide the >> ubifs_is_mounted function. Remove the dependency to ubifs for the spl. >> >> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >> >> --- >> >> disk/part.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/disk/part.c b/disk/part.c >> index 79955c7fb0..de1b917e84 100644 >> --- a/disk/part.c >> +++ b/disk/part.c >> @@ -479,7 +479,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, >> } >> #endif >> >> -#ifdef CONFIG_CMD_UBIFS >> +#if IS_ENABLED(CONFIG_CMD_UBIFS) && !IS_ENABLED(CONFIG_SPL_BUILD) >> /* >> * Special-case ubi, ubi goes through a mtd, rather than through >> * a regular block device. >> -- >> 2.30.2 >> > > A CMD config should not be used outside cmd/ - can we add a proper > CONFIG_UBIFS ? The code use the ubifs_is_mounted function from cmd/ubifs.c file and reference the ubifsmount command from the same file. The drivers/misc/fs_loader.c and fs/fs.c files use functions from the cmd/ubifs.c file too. Regards Stefan
On Mon, Aug 08, 2022 at 04:45:17PM +0200, Stefan Herbrechtsmeier wrote: > From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> > > The spl doesn't support ubifs and thereby doesn't provide the > ubifs_is_mounted function. Remove the dependency to ubifs for the spl. > > Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> Applied to u-boot/master, thanks!
On 8/8/22 16:45, Stefan Herbrechtsmeier wrote: > From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> > > The spl doesn't support ubifs and thereby doesn't provide the > ubifs_is_mounted function. Remove the dependency to ubifs for the spl. > > Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> > > --- > > disk/part.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/disk/part.c b/disk/part.c > index 79955c7fb0..de1b917e84 100644 > --- a/disk/part.c > +++ b/disk/part.c > @@ -479,7 +479,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, > } > #endif > > -#ifdef CONFIG_CMD_UBIFS > +#if IS_ENABLED(CONFIG_CMD_UBIFS) && !IS_ENABLED(CONFIG_SPL_BUILD) This configuration seems strange. The support for a file system should not depend on a command. I think a CONFIG_UBIFS is missing. Best regards Heinrich > /* > * Special-case ubi, ubi goes through a mtd, rather than through > * a regular block device.
Hi Heinrich, Am 29.08.2022 um 17:01 schrieb Heinrich Schuchardt: > On 8/8/22 16:45, Stefan Herbrechtsmeier wrote: >> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >> >> The spl doesn't support ubifs and thereby doesn't provide the >> ubifs_is_mounted function. Remove the dependency to ubifs for the spl. >> >> Signed-off-by: Stefan Herbrechtsmeier >> <stefan.herbrechtsmeier@weidmueller.com> >> >> --- >> >> disk/part.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/disk/part.c b/disk/part.c >> index 79955c7fb0..de1b917e84 100644 >> --- a/disk/part.c >> +++ b/disk/part.c >> @@ -479,7 +479,7 @@ int blk_get_device_part_str(const char *ifname, >> const char *dev_part_str, >> } >> #endif >> >> -#ifdef CONFIG_CMD_UBIFS >> +#if IS_ENABLED(CONFIG_CMD_UBIFS) && !IS_ENABLED(CONFIG_SPL_BUILD) > > This configuration seems strange. The support for a file system should > not depend on a command. I think a CONFIG_UBIFS is missing. The support for the ubi file system depends on the command. The code use the ubifs_is_mounted function from cmd/ubifs.c file and reference the ubifsmount command from the same file. Regards Stefan
Hi Stefan, On Mon, 29 Aug 2022 at 09:53, Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com> wrote: > > Hi Heinrich, > > Am 29.08.2022 um 17:01 schrieb Heinrich Schuchardt: > > On 8/8/22 16:45, Stefan Herbrechtsmeier wrote: > >> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> > >> > >> The spl doesn't support ubifs and thereby doesn't provide the > >> ubifs_is_mounted function. Remove the dependency to ubifs for the spl. > >> > >> Signed-off-by: Stefan Herbrechtsmeier > >> <stefan.herbrechtsmeier@weidmueller.com> > >> > >> --- > >> > >> disk/part.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/disk/part.c b/disk/part.c > >> index 79955c7fb0..de1b917e84 100644 > >> --- a/disk/part.c > >> +++ b/disk/part.c > >> @@ -479,7 +479,7 @@ int blk_get_device_part_str(const char *ifname, > >> const char *dev_part_str, > >> } > >> #endif > >> > >> -#ifdef CONFIG_CMD_UBIFS > >> +#if IS_ENABLED(CONFIG_CMD_UBIFS) && !IS_ENABLED(CONFIG_SPL_BUILD) > > > > This configuration seems strange. The support for a file system should > > not depend on a command. I think a CONFIG_UBIFS is missing. > > The support for the ubi file system depends on the command. The code use > the ubifs_is_mounted function from cmd/ubifs.c file and reference the > ubifsmount command from the same file. Yes it does, but it should not (I think I made the point in another thread but did not understand your response). The UBIFS Kconfig should be separate from CMD_UBIFS. The above can then be: if (CONFIG_IS_ENABLED(UBIFS)) Regards, Simon
Hi, Am 30.08.2022 um 04:29 schrieb Simon Glass: > Hi Stefan, > > On Mon, 29 Aug 2022 at 09:53, Stefan Herbrechtsmeier > <stefan.herbrechtsmeier-oss@weidmueller.com> wrote: >> >> Hi Heinrich, >> >> Am 29.08.2022 um 17:01 schrieb Heinrich Schuchardt: >>> On 8/8/22 16:45, Stefan Herbrechtsmeier wrote: >>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >>>> >>>> The spl doesn't support ubifs and thereby doesn't provide the >>>> ubifs_is_mounted function. Remove the dependency to ubifs for the spl. >>>> >>>> Signed-off-by: Stefan Herbrechtsmeier >>>> <stefan.herbrechtsmeier@weidmueller.com> >>>> >>>> --- >>>> >>>> disk/part.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/disk/part.c b/disk/part.c >>>> index 79955c7fb0..de1b917e84 100644 >>>> --- a/disk/part.c >>>> +++ b/disk/part.c >>>> @@ -479,7 +479,7 @@ int blk_get_device_part_str(const char *ifname, >>>> const char *dev_part_str, >>>> } >>>> #endif >>>> >>>> -#ifdef CONFIG_CMD_UBIFS >>>> +#if IS_ENABLED(CONFIG_CMD_UBIFS) && !IS_ENABLED(CONFIG_SPL_BUILD) >>> >>> This configuration seems strange. The support for a file system should >>> not depend on a command. I think a CONFIG_UBIFS is missing. >> >> The support for the ubi file system depends on the command. The code use >> the ubifs_is_mounted function from cmd/ubifs.c file and reference the >> ubifsmount command from the same file. > > Yes it does, but it should not (I think I made the point in another > thread but did not understand your response). The UBIFS Kconfig should > be separate from CMD_UBIFS. I think we agree that the ubifs implementation isn't optimal. But this patch only fix a problem for SPL if UBIFS is enabled in u-boot proper. The wrong implementation was accept by u-boot 7 years ago. The problem for me is that we no longer use UBI. I fear that a rework of UBI needs a lot of time to to fix a 'should not'. On the other hand I have patches with an unclear status that fix problems / incompatibilities. Regards Stefan
diff --git a/disk/part.c b/disk/part.c index 79955c7fb0..de1b917e84 100644 --- a/disk/part.c +++ b/disk/part.c @@ -479,7 +479,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, } #endif -#ifdef CONFIG_CMD_UBIFS +#if IS_ENABLED(CONFIG_CMD_UBIFS) && !IS_ENABLED(CONFIG_SPL_BUILD) /* * Special-case ubi, ubi goes through a mtd, rather than through * a regular block device.