[PATCH 1/3] driver: add flag to check if cdev is an mci device

[PATCH 1/3] driver: add flag to check if cdev is an mci device

[Marco Felsch]

We need this during mount() to check if the cdev is an mmc/mci device.
Later on we add the feature to pass "root=/dev/mmcblk*" as kernel
command line.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 include/driver.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/driver.h b/include/driver.h
index 0d43b36148..d381aaf78d 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -496,6 +496,7 @@ int cdev_erase(struct cdev *cdev, loff_t count, loff_t offset);
 #define DEVFS_PARTITION_READONLY	(1U << 1)
 #define DEVFS_IS_CHARACTER_DEV		(1U << 3)
 #define DEVFS_PARTITION_FROM_TABLE	(1U << 4)
+#define DEVFS_IS_MCI_DEV		(1U << 5)
 
 struct cdev *devfs_add_partition(const char *devname, loff_t offset,
 		loff_t size, unsigned int flags, const char *name);
@@ -509,6 +510,11 @@ static inline void cdev_create_default_automount(struct cdev *cdev)
 }
 #endif
 
+static inline bool cdev_is_mci_dev(struct cdev *cdev)
+{
+	return !!(cdev->flags & DEVFS_IS_MCI_DEV);
+}
+
 #define DEVFS_PARTITION_APPEND		0
 
 /**
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] mci: mci-core: set the DEVFS_IS_MCI_DEV flag

[Marco Felsch]

Set the new introduced flag to be able to check if the cdev is an
mmc/mci device.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/mci/mci-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 017f25d35f..c0fbcb385f 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -420,6 +420,7 @@ static void mci_part_add(struct mci *mci, uint64_t size,
 	part->size = size;
 	part->blk.cdev.name = name;
 	part->blk.cdev.partname = partname;
+	part->blk.cdev.flags |= DEVFS_IS_MCI_DEV;
 	part->blk.blockbits = SECTOR_SHIFT;
 	part->blk.num_blocks = mci_calc_blk_cnt(size, part->blk.blockbits);
 	part->area_type = area_type;
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] mci: mci-core: set the DEVFS_IS_MCI_DEV flag

[Ahmad Fatoum]

Hello,

On 06.05.21 13:22, Marco Felsch wrote:
> Set the new introduced flag to be able to check if the cdev is an
> mmc/mci device.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/mci/mci-core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index 017f25d35f..c0fbcb385f 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -420,6 +420,7 @@ static void mci_part_add(struct mci *mci, uint64_t size,
>  	part->size = size;
>  	part->blk.cdev.name = name;
>  	part->blk.cdev.partname = partname;
> +	part->blk.cdev.flags |= DEVFS_IS_MCI_DEV;
>  	part->blk.blockbits = SECTOR_SHIFT;
>  	part->blk.num_blocks = mci_calc_blk_cnt(size, part->blk.blockbits);
>  	part->area_type = area_type;

This is called for hardware partitions, like boot and general
purpose partitions.

For boot partitions, the later commit is wrong. I don't know what the default
file name for mmc GPP block partitions is, Jürgen?

You should probably do this only for the MMC user partition (check for
area_type == MMC_BLK_DATA_AREA_MAIN). I think DEVFS_IS_MCI_DEV
should be renamed to DEVFS_IS_MCI_MAIN_DEV to.

Cheers,
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] mci: mci-core: set the DEVFS_IS_MCI_DEV flag

[Marco Felsch]

On 21-05-06 14:06, Ahmad Fatoum wrote:
> Hello,
> 
> On 06.05.21 13:22, Marco Felsch wrote:
> > Set the new introduced flag to be able to check if the cdev is an
> > mmc/mci device.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  drivers/mci/mci-core.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> > index 017f25d35f..c0fbcb385f 100644
> > --- a/drivers/mci/mci-core.c
> > +++ b/drivers/mci/mci-core.c
> > @@ -420,6 +420,7 @@ static void mci_part_add(struct mci *mci, uint64_t size,
> >  	part->size = size;
> >  	part->blk.cdev.name = name;
> >  	part->blk.cdev.partname = partname;
> > +	part->blk.cdev.flags |= DEVFS_IS_MCI_DEV;
> >  	part->blk.blockbits = SECTOR_SHIFT;
> >  	part->blk.num_blocks = mci_calc_blk_cnt(size, part->blk.blockbits);
> >  	part->area_type = area_type;
> 
> This is called for hardware partitions, like boot and general
> purpose partitions.
> 
> For boot partitions, the later commit is wrong. I don't know what the default
> file name for mmc GPP block partitions is, Jürgen?

Arg right, the boot partition name scheme differs. For GPP there should
be seperated devices, don't know that directly.

Should we first add the support for MMC_BLK_DATA_AREA_MAIN only? This is
the most tested path.

> You should probably do this only for the MMC user partition (check for
> area_type == MMC_BLK_DATA_AREA_MAIN). I think DEVFS_IS_MCI_DEV
> should be renamed to DEVFS_IS_MCI_MAIN_DEV to.

^^ Yep, I think so too. Will change that.

Regards,
  Marco

> 
> Cheers,
> Ahmad
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] mci: mci-core: set the DEVFS_IS_MCI_DEV flag

[Ahmad Fatoum]

On 06.05.21 15:07, Marco Felsch wrote:
> On 21-05-06 14:06, Ahmad Fatoum wrote:
>> Hello,
>>
>> On 06.05.21 13:22, Marco Felsch wrote:
>>> Set the new introduced flag to be able to check if the cdev is an
>>> mmc/mci device.
>>>
>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>>> ---
>>>  drivers/mci/mci-core.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
>>> index 017f25d35f..c0fbcb385f 100644
>>> --- a/drivers/mci/mci-core.c
>>> +++ b/drivers/mci/mci-core.c
>>> @@ -420,6 +420,7 @@ static void mci_part_add(struct mci *mci, uint64_t size,
>>>  	part->size = size;
>>>  	part->blk.cdev.name = name;
>>>  	part->blk.cdev.partname = partname;
>>> +	part->blk.cdev.flags |= DEVFS_IS_MCI_DEV;
>>>  	part->blk.blockbits = SECTOR_SHIFT;
>>>  	part->blk.num_blocks = mci_calc_blk_cnt(size, part->blk.blockbits);
>>>  	part->area_type = area_type;
>>
>> This is called for hardware partitions, like boot and general
>> purpose partitions.
>>
>> For boot partitions, the later commit is wrong. I don't know what the default
>> file name for mmc GPP block partitions is, Jürgen?
> 
> Arg right, the boot partition name scheme differs. For GPP there should
> be seperated devices, don't know that directly.
> 
> Should we first add the support for MMC_BLK_DATA_AREA_MAIN only? This is
> the most tested path.

Ack.

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] fs: add linux_rootarg 'root=mmcblkXpN' support

[Marco Felsch]

Since commit fa2d0aa96941 ("mmc: core: Allow setting slot index via
device tree alias") the kernel supports stable mmc device names. Since
the barebox mmc device names matches the one from the kernel we can pass
the 'root=mmcblkXpN' argument on commandline to refer to the correct
boot medium.

This patch adds the support to store the above commandline as
linux_rootarg if enabled. Use the partuuid as fallback since it is not
as unique as the mmcblkXpN scheme. Add a own build option since the
system integrator needs to check if the used kernel contains the above
commit.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
Hi all,

I know that:
8<-----------------
+		if (!str && fsdev->cdev->partuuid[0] != 0)
+			str = basprintf("root=PARTUUID=%s", fsdev->cdev->partuuid);
8<-----------------
is longer than 80char but for the sake of readability I kept that
oneliner. Should we update checkpatch accordingly to match the new linux
rule?

Regards,
  Marco


 common/Kconfig | 15 +++++++++++++++
 fs/fs.c        | 48 ++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index 6b3c1701be..a09a1d9456 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -700,6 +700,21 @@ config FLEXIBLE_BOOTARGS
 	  to replace parts of the bootargs string without reconstructing it
 	  completely.
 
+config MMCBLKDEV_ROOTARG
+	bool
+	prompt "Support 'root=mmcblkXpN' cmdline appending"
+	help
+	  Enable this option to append 'root=mmcblkXpN' to the kernel cmdline
+	  instead of 'root=PARTUUID=XYZ'. Don't enale this option if your used
+	  kernel don't contain commit [1].
+
+	  The appending only happen if barebox 'linux.bootargs.bootm.appendroot'
+	  variable is set or the used blspec entry contains 'linux-appendroot'.
+
+	  [1] fa2d0aa96941 ("mmc: core: Allow setting slot index via device tree
+	      alias")
+
+
 config BAREBOX_UPDATE
 	bool "In-system barebox update infrastructure"
 
diff --git a/fs/fs.c b/fs/fs.c
index 881dc2fca0..8ee1e7febe 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -2831,6 +2831,39 @@ out:
 }
 EXPORT_SYMBOL(chdir);
 
+static char *get_linux_mmcblkdev(struct fs_device_d *fsdev)
+{
+	struct cdev *cdevm, *cdev;
+	int id, partnum;
+	bool found = false;
+
+	cdevm = fsdev->cdev->master;
+	id = of_alias_get_id(cdevm->device_node, "mmc");
+	if (id < 0)
+		return NULL;
+
+	partnum = 1; /* linux partitions are 1 based */
+	list_for_each_entry(cdev, &cdevm->partitions, partition_entry) {
+
+		/*
+		 * Partname is not guaranteed but this partition cdev is listed
+		 * in the partitions list so we need to count it instead of
+		 * skipping it.
+		 */
+		if (cdev->partname &&
+		    !strcmp(cdev->partname, fsdev->cdev->partname)) {
+			found = true;
+			break;
+		}
+		partnum++;
+	}
+
+	if (!found)
+		return NULL;
+
+	return basprintf("root=/dev/mmcblk%dp%d", id, partnum);
+}
+
 /*
  * Mount a device to a directory.
  * We do this by registering a new device on which the filesystem
@@ -2919,11 +2952,18 @@ int mount(const char *device, const char *fsname, const char *pathname,
 
 	fsdev->vfsmount.mnt_root = fsdev->sb.s_root;
 
-	if (!fsdev->linux_rootarg && fsdev->cdev && fsdev->cdev->partuuid[0] != 0) {
-		char *str = basprintf("root=PARTUUID=%s",
-					fsdev->cdev->partuuid);
+	if (!fsdev->linux_rootarg && fsdev->cdev) {
+		char *str = NULL;
+
+		if (IS_ENABLED(CONFIG_MMCBLKDEV_ROOTARG) &&
+		    cdev_is_mci_dev(fsdev->cdev->master))
+			str = get_linux_mmcblkdev(fsdev);
+
+		if (!str && fsdev->cdev->partuuid[0] != 0)
+			str = basprintf("root=PARTUUID=%s", fsdev->cdev->partuuid);
 
-		fsdev_set_linux_rootarg(fsdev, str);
+		if (str)
+			fsdev_set_linux_rootarg(fsdev, str);
 	}
 
 	path_put(&path);
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/3] fs: add linux_rootarg 'root=mmcblkXpN' support

[Ahmad Fatoum]

Hello Marco,

On 06.05.21 13:22, Marco Felsch wrote:
> Since commit fa2d0aa96941 ("mmc: core: Allow setting slot index via
> device tree alias") the kernel supports stable mmc device names. Since
> the barebox mmc device names matches the one from the kernel we can pass
> the 'root=mmcblkXpN' argument on commandline to refer to the correct
> boot medium.
> 
> This patch adds the support to store the above commandline as
> linux_rootarg if enabled. Use the partuuid as fallback since it is not
> as unique as the mmcblkXpN scheme. Add a own build option since the
> system integrator needs to check if the used kernel contains the above
> commit.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> Hi all,
> 
> I know that:
> 8<-----------------
> +		if (!str && fsdev->cdev->partuuid[0] != 0)
> +			str = basprintf("root=PARTUUID=%s", fsdev->cdev->partuuid);
> 8<-----------------
> is longer than 80char but for the sake of readability I kept that
> oneliner. Should we update checkpatch accordingly to match the new linux
> rule?

Yes, I already adhere to the new Linux convention in my barebox code.

> 
> Regards,
>   Marco
> 
> 
>  common/Kconfig | 15 +++++++++++++++
>  fs/fs.c        | 48 ++++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 59 insertions(+), 4 deletions(-)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index 6b3c1701be..a09a1d9456 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -700,6 +700,21 @@ config FLEXIBLE_BOOTARGS
>  	  to replace parts of the bootargs string without reconstructing it
>  	  completely.
>  
> +config MMCBLKDEV_ROOTARG
> +	bool
> +	prompt "Support 'root=mmcblkXpN' cmdline appending"
> +	help
> +	  Enable this option to append 'root=mmcblkXpN' to the kernel cmdline
> +	  instead of 'root=PARTUUID=XYZ'. Don't enale this option if your used
> +	  kernel don't contain commit [1].

Could you add the first mainline Linux release that includes the commit?

> +
> +	  The appending only happen if barebox 'linux.bootargs.bootm.appendroot'
> +	  variable is set or the used blspec entry contains 'linux-appendroot'.
> +
> +	  [1] fa2d0aa96941 ("mmc: core: Allow setting slot index via device tree
> +	      alias")
> +
> +
>  config BAREBOX_UPDATE
>  	bool "In-system barebox update infrastructure"
>  
> diff --git a/fs/fs.c b/fs/fs.c
> index 881dc2fca0..8ee1e7febe 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -2831,6 +2831,39 @@ out:
>  }
>  EXPORT_SYMBOL(chdir);
>  
> +static char *get_linux_mmcblkdev(struct fs_device_d *fsdev)
> +{
> +	struct cdev *cdevm, *cdev;
> +	int id, partnum;
> +	bool found = false;
> +
> +	cdevm = fsdev->cdev->master;
> +	id = of_alias_get_id(cdevm->device_node, "mmc");
> +	if (id < 0)
> +		return NULL;
> +
> +	partnum = 1; /* linux partitions are 1 based */
> +	list_for_each_entry(cdev, &cdevm->partitions, partition_entry) {
> +
> +		/*
> +		 * Partname is not guaranteed but this partition cdev is listed
> +		 * in the partitions list so we need to count it instead of
> +		 * skipping it.
> +		 */
> +		if (cdev->partname &&
> +		    !strcmp(cdev->partname, fsdev->cdev->partname)) {
> +			found = true;

You can return the basprintf() here directly

> +			break;
> +		}
> +		partnum++;
> +	}
> +
> +	if (!found)
> +		return NULL;
> +
> +	return basprintf("root=/dev/mmcblk%dp%d", id, partnum);

Then you can return NULL here and drop found.

> +}
> +
>  /*
>   * Mount a device to a directory.
>   * We do this by registering a new device on which the filesystem
> @@ -2919,11 +2952,18 @@ int mount(const char *device, const char *fsname, const char *pathname,
>  
>  	fsdev->vfsmount.mnt_root = fsdev->sb.s_root;
>  
> -	if (!fsdev->linux_rootarg && fsdev->cdev && fsdev->cdev->partuuid[0] != 0) {
> -		char *str = basprintf("root=PARTUUID=%s",
> -					fsdev->cdev->partuuid);
> +	if (!fsdev->linux_rootarg && fsdev->cdev) {
> +		char *str = NULL;
> +
> +		if (IS_ENABLED(CONFIG_MMCBLKDEV_ROOTARG) &&
> +		    cdev_is_mci_dev(fsdev->cdev->master))
> +			str = get_linux_mmcblkdev(fsdev);
> +
> +		if (!str && fsdev->cdev->partuuid[0] != 0)
> +			str = basprintf("root=PARTUUID=%s", fsdev->cdev->partuuid);
>  
> -		fsdev_set_linux_rootarg(fsdev, str);
> +		if (str)
> +			fsdev_set_linux_rootarg(fsdev, str);

This assumes that barebox aliases align with those of the kernel. I think this
is at least worth a mention in the Kconfig text.

For an alternative, see f382be77f8c0 ("boot: introduce option to pass
barebox-enabled watchdog to systemd"), which parses the aliases out of the
kernel device tree without depending on the barebox ones.

Cheers,
Ahmad


>  	}
>  
>  	path_put(&path);
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/3] fs: add linux_rootarg 'root=mmcblkXpN' support

[Marco Felsch]

Hi Ahmad,

On 21-05-06 14:19, Ahmad Fatoum wrote:
> Hello Marco,
> 
> On 06.05.21 13:22, Marco Felsch wrote:
> > Since commit fa2d0aa96941 ("mmc: core: Allow setting slot index via
> > device tree alias") the kernel supports stable mmc device names. Since
> > the barebox mmc device names matches the one from the kernel we can pass
> > the 'root=mmcblkXpN' argument on commandline to refer to the correct
> > boot medium.
> > 
> > This patch adds the support to store the above commandline as
> > linux_rootarg if enabled. Use the partuuid as fallback since it is not
> > as unique as the mmcblkXpN scheme. Add a own build option since the
> > system integrator needs to check if the used kernel contains the above
> > commit.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> > Hi all,
> > 
> > I know that:
> > 8<-----------------
> > +		if (!str && fsdev->cdev->partuuid[0] != 0)
> > +			str = basprintf("root=PARTUUID=%s", fsdev->cdev->partuuid);
> > 8<-----------------
> > is longer than 80char but for the sake of readability I kept that
> > oneliner. Should we update checkpatch accordingly to match the new linux
> > rule?
> 
> Yes, I already adhere to the new Linux convention in my barebox code.
> 
> > 
> > Regards,
> >   Marco
> > 
> > 
> >  common/Kconfig | 15 +++++++++++++++
> >  fs/fs.c        | 48 ++++++++++++++++++++++++++++++++++++++++++++----
> >  2 files changed, 59 insertions(+), 4 deletions(-)
> > 
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 6b3c1701be..a09a1d9456 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -700,6 +700,21 @@ config FLEXIBLE_BOOTARGS
> >  	  to replace parts of the bootargs string without reconstructing it
> >  	  completely.
> >  
> > +config MMCBLKDEV_ROOTARG
> > +	bool
> > +	prompt "Support 'root=mmcblkXpN' cmdline appending"
> > +	help
> > +	  Enable this option to append 'root=mmcblkXpN' to the kernel cmdline
> > +	  instead of 'root=PARTUUID=XYZ'. Don't enale this option if your used
> > +	  kernel don't contain commit [1].
> 
> Could you add the first mainline Linux release that includes the commit?

Yep, will add it.

> > +
> > +	  The appending only happen if barebox 'linux.bootargs.bootm.appendroot'
> > +	  variable is set or the used blspec entry contains 'linux-appendroot'.
> > +
> > +	  [1] fa2d0aa96941 ("mmc: core: Allow setting slot index via device tree
> > +	      alias")
> > +
> > +
> >  config BAREBOX_UPDATE
> >  	bool "In-system barebox update infrastructure"
> >  
> > diff --git a/fs/fs.c b/fs/fs.c
> > index 881dc2fca0..8ee1e7febe 100644
> > --- a/fs/fs.c
> > +++ b/fs/fs.c
> > @@ -2831,6 +2831,39 @@ out:
> >  }
> >  EXPORT_SYMBOL(chdir);
> >  
> > +static char *get_linux_mmcblkdev(struct fs_device_d *fsdev)
> > +{
> > +	struct cdev *cdevm, *cdev;
> > +	int id, partnum;
> > +	bool found = false;
> > +
> > +	cdevm = fsdev->cdev->master;
> > +	id = of_alias_get_id(cdevm->device_node, "mmc");
> > +	if (id < 0)
> > +		return NULL;
> > +
> > +	partnum = 1; /* linux partitions are 1 based */
> > +	list_for_each_entry(cdev, &cdevm->partitions, partition_entry) {
> > +
> > +		/*
> > +		 * Partname is not guaranteed but this partition cdev is listed
> > +		 * in the partitions list so we need to count it instead of
> > +		 * skipping it.
> > +		 */
> > +		if (cdev->partname &&
> > +		    !strcmp(cdev->partname, fsdev->cdev->partname)) {
> > +			found = true;
> 
> You can return the basprintf() here directly

You're right.

> > +			break;
> > +		}
> > +		partnum++;
> > +	}
> > +
> > +	if (!found)
> > +		return NULL;
> > +
> > +	return basprintf("root=/dev/mmcblk%dp%d", id, partnum);
> 
> Then you can return NULL here and drop found.
> 
> > +}
> > +
> >  /*
> >   * Mount a device to a directory.
> >   * We do this by registering a new device on which the filesystem
> > @@ -2919,11 +2952,18 @@ int mount(const char *device, const char *fsname, const char *pathname,
> >  
> >  	fsdev->vfsmount.mnt_root = fsdev->sb.s_root;
> >  
> > -	if (!fsdev->linux_rootarg && fsdev->cdev && fsdev->cdev->partuuid[0] != 0) {
> > -		char *str = basprintf("root=PARTUUID=%s",
> > -					fsdev->cdev->partuuid);
> > +	if (!fsdev->linux_rootarg && fsdev->cdev) {
> > +		char *str = NULL;
> > +
> > +		if (IS_ENABLED(CONFIG_MMCBLKDEV_ROOTARG) &&
> > +		    cdev_is_mci_dev(fsdev->cdev->master))
> > +			str = get_linux_mmcblkdev(fsdev);
> > +
> > +		if (!str && fsdev->cdev->partuuid[0] != 0)
> > +			str = basprintf("root=PARTUUID=%s", fsdev->cdev->partuuid);
> >  
> > -		fsdev_set_linux_rootarg(fsdev, str);
> > +		if (str)
> > +			fsdev_set_linux_rootarg(fsdev, str);
> 
> This assumes that barebox aliases align with those of the kernel. I think this
> is at least worth a mention in the Kconfig text.

IMHO mention it within the Kconfig should be enough, since in an ideal
world the barebox-dt should be used by kernel as well.

> For an alternative, see f382be77f8c0 ("boot: introduce option to pass
> barebox-enabled watchdog to systemd"), which parses the aliases out of the
> kernel device tree without depending on the barebox ones.

That's interessint as well, but spreads the logic around. Therefore I
would keep my solution an add a note within the Kconfig. Is that OK for
you?

Regards,
  Marco

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] driver: add flag to check if cdev is an mci device

[Ahmad Fatoum]

On 06.05.21 13:22, Marco Felsch wrote:
> We need this during mount() to check if the cdev is an mmc/mci device.
> Later on we add the feature to pass "root=/dev/mmcblk*" as kernel
> command line.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  include/driver.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/driver.h b/include/driver.h
> index 0d43b36148..d381aaf78d 100644
> --- a/include/driver.h
> +++ b/include/driver.h
> @@ -496,6 +496,7 @@ int cdev_erase(struct cdev *cdev, loff_t count, loff_t offset);
>  #define DEVFS_PARTITION_READONLY	(1U << 1)
>  #define DEVFS_IS_CHARACTER_DEV		(1U << 3)
>  #define DEVFS_PARTITION_FROM_TABLE	(1U << 4)
> +#define DEVFS_IS_MCI_DEV		(1U << 5)
>  
>  struct cdev *devfs_add_partition(const char *devname, loff_t offset,
>  		loff_t size, unsigned int flags, const char *name);
> @@ -509,6 +510,11 @@ static inline void cdev_create_default_automount(struct cdev *cdev)
>  }
>  #endif
>  
> +static inline bool cdev_is_mci_dev(struct cdev *cdev)
> +{
> +	return !!(cdev->flags & DEVFS_IS_MCI_DEV);
> +}

Nitpick: cast to bool makes !! superfluous.

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>


> +
>  #define DEVFS_PARTITION_APPEND		0
>  
>  /**
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] driver: add flag to check if cdev is an mci device

[Marco Felsch]

On 21-05-06 13:59, Ahmad Fatoum wrote:
> 
> 
> On 06.05.21 13:22, Marco Felsch wrote:
> > We need this during mount() to check if the cdev is an mmc/mci device.
> > Later on we add the feature to pass "root=/dev/mmcblk*" as kernel
> > command line.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  include/driver.h | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/include/driver.h b/include/driver.h
> > index 0d43b36148..d381aaf78d 100644
> > --- a/include/driver.h
> > +++ b/include/driver.h
> > @@ -496,6 +496,7 @@ int cdev_erase(struct cdev *cdev, loff_t count, loff_t offset);
> >  #define DEVFS_PARTITION_READONLY	(1U << 1)
> >  #define DEVFS_IS_CHARACTER_DEV		(1U << 3)
> >  #define DEVFS_PARTITION_FROM_TABLE	(1U << 4)
> > +#define DEVFS_IS_MCI_DEV		(1U << 5)
> >  
> >  struct cdev *devfs_add_partition(const char *devname, loff_t offset,
> >  		loff_t size, unsigned int flags, const char *name);
> > @@ -509,6 +510,11 @@ static inline void cdev_create_default_automount(struct cdev *cdev)
> >  }
> >  #endif
> >  
> > +static inline bool cdev_is_mci_dev(struct cdev *cdev)
> > +{
> > +	return !!(cdev->flags & DEVFS_IS_MCI_DEV);
> > +}
> 
> Nitpick: cast to bool makes !! superfluous.

Right. I will drop it and add you tag.

> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> 
> > +
> >  #define DEVFS_PARTITION_APPEND		0
> >  
> >  /**
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox