| Message ID | 56FA5C56.9080009@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jaehoon, On Tue, 29 Mar 2016 19:43:34 +0900 Jaehoon Chung wrote: > This patch is just RFC. I want to know opinions. > > Now, index of mmcblk is allocated in accordance with probing time. > If want to use the mmcblk1 for some device, it can use alias. > > aliases { > mmc0 = &mmc0; /* mmcblk0 for eMMC */ > mmc1 = &mmc2; /* mmcblk1 for SD */ > mmc2 = &mmc1; /* mmcblk2 for SDIO*/ > }; I like this feature and we do need it. In the past, we have to put emmc dt node before the sd node, but on our platform, the register base of sdhc host for emmc is higher than the one for sd, putting emmc dt node before sd looks a bit strange, but we have no better solutions. Thanks, Jisheng > > If there are no corresponding values, it might be allocated with > existing scheme. > > Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> > --- > Documentation/devicetree/bindings/mmc/mmc.txt | 11 +++++++++++ > drivers/mmc/card/block.c | 17 ++++++++++++++++- > 2 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt > index ed23b9b..e37ea29 100644 > --- a/Documentation/devicetree/bindings/mmc/mmc.txt > +++ b/Documentation/devicetree/bindings/mmc/mmc.txt > @@ -71,6 +71,10 @@ Optional SDIO properties: > - wakeup-source: Enables wake up of host system on SDIO IRQ assertion > (Legacy property supported: "enable-sdio-wakeup") > > +Aliases (Optional): > +- If you want to use the fixed index for block device like mmcblkX, should be > +represented in the aliases node using following format "mmc(X)". > +(X is an unique number for the alias.) > > MMC power sequences: > -------------------- > @@ -145,3 +149,10 @@ mmc3: mmc@01c12000 { > interrupt-names = "host-wake"; > }; > }; > + > +Example with aliases nodes: > + > +aliases { > + mmc0 = &mmc0; /* Fixed to mmcblk0 for &mmc0 */ > + mmc1 = &mmc2; /* Fixed to mmcblk1 for &mmc2 */ > +}; > diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c > index 3bdbe50..6a40de5 100644 > --- a/drivers/mmc/card/block.c > +++ b/drivers/mmc/card/block.c > @@ -35,6 +35,7 @@ > #include <linux/capability.h> > #include <linux/compat.h> > #include <linux/pm_runtime.h> > +#include <linux/of.h> > > #include <linux/mmc/ioctl.h> > #include <linux/mmc/card.h> > @@ -2190,6 +2191,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card, > { > struct mmc_blk_data *md; > int devidx, ret; > + int idx = 0; > > devidx = find_first_zero_bit(dev_use, max_devices); > if (devidx >= max_devices) > @@ -2209,7 +2211,20 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card, > * index anymore so we keep track of a name index. > */ > if (!subname) { > - md->name_idx = find_first_zero_bit(name_use, max_devices); > + if (card->dev.parent->parent->of_node) > + idx = of_alias_get_id(card->dev.parent->parent->of_node, > + "mmc"); > + > + if (idx < 0) > + md->name_idx = find_first_zero_bit(name_use, > + max_devices); > + else { > + if (test_bit(idx, name_use)) > + md->name_idx = find_first_zero_bit(name_use, > + max_devices); > + else > + md->name_idx = (unsigned int)idx; > + } > __set_bit(md->name_idx, name_use); > } else > md->name_idx = ((struct mmc_blk_data *) -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
? 2016/3/29 18:49, Jisheng Zhang ??: > Hi Jaehoon, > > On Tue, 29 Mar 2016 19:43:34 +0900 Jaehoon Chung wrote: > >> This patch is just RFC. I want to know opinions. >> >> Now, index of mmcblk is allocated in accordance with probing time. >> If want to use the mmcblk1 for some device, it can use alias. >> >> aliases { >> mmc0 = &mmc0; /* mmcblk0 for eMMC */ >> mmc1 = &mmc2; /* mmcblk1 for SD */ >> mmc2 = &mmc1; /* mmcblk2 for SDIO*/ >> }; > > I like this feature and we do need it. In the past, we have to put emmc dt node > before the sd node, but on our platform, the register base of sdhc host for emmc > is higher than the one for sd, putting emmc dt node before sd looks a bit > strange, but we have no better solutions. > :) That is also what I suffered currently. I have to prepare two seperate firmware with two diff fstab files to assign the mount point since sometime I need to boot from sd, but sometimes from emmc.... With this patch, I only need to dtc a new dtb... quite simple. > Thanks, > Jisheng > >> >> If there are no corresponding values, it might be allocated with >> existing scheme. >> >> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> >> --- >> Documentation/devicetree/bindings/mmc/mmc.txt | 11 +++++++++++ >> drivers/mmc/card/block.c | 17 ++++++++++++++++- >> 2 files changed, 27 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt >> index ed23b9b..e37ea29 100644 >> --- a/Documentation/devicetree/bindings/mmc/mmc.txt >> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt >> @@ -71,6 +71,10 @@ Optional SDIO properties: >> - wakeup-source: Enables wake up of host system on SDIO IRQ assertion >> (Legacy property supported: "enable-sdio-wakeup") >> >> +Aliases (Optional): >> +- If you want to use the fixed index for block device like mmcblkX, should be >> +represented in the aliases node using following format "mmc(X)". >> +(X is an unique number for the alias.) >> >> MMC power sequences: >> -------------------- >> @@ -145,3 +149,10 @@ mmc3: mmc@01c12000 { >> interrupt-names = "host-wake"; >> }; >> }; >> + >> +Example with aliases nodes: >> + >> +aliases { >> + mmc0 = &mmc0; /* Fixed to mmcblk0 for &mmc0 */ >> + mmc1 = &mmc2; /* Fixed to mmcblk1 for &mmc2 */ >> +}; >> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c >> index 3bdbe50..6a40de5 100644 >> --- a/drivers/mmc/card/block.c >> +++ b/drivers/mmc/card/block.c >> @@ -35,6 +35,7 @@ >> #include <linux/capability.h> >> #include <linux/compat.h> >> #include <linux/pm_runtime.h> >> +#include <linux/of.h> >> >> #include <linux/mmc/ioctl.h> >> #include <linux/mmc/card.h> >> @@ -2190,6 +2191,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card, >> { >> struct mmc_blk_data *md; >> int devidx, ret; >> + int idx = 0; >> >> devidx = find_first_zero_bit(dev_use, max_devices); >> if (devidx >= max_devices) >> @@ -2209,7 +2211,20 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card, >> * index anymore so we keep track of a name index. >> */ >> if (!subname) { >> - md->name_idx = find_first_zero_bit(name_use, max_devices); >> + if (card->dev.parent->parent->of_node) >> + idx = of_alias_get_id(card->dev.parent->parent->of_node, >> + "mmc"); >> + >> + if (idx < 0) >> + md->name_idx = find_first_zero_bit(name_use, >> + max_devices); >> + else { >> + if (test_bit(idx, name_use)) >> + md->name_idx = find_first_zero_bit(name_use, >> + max_devices); >> + else >> + md->name_idx = (unsigned int)idx; >> + } >> __set_bit(md->name_idx, name_use); >> } else >> md->name_idx = ((struct mmc_blk_data *) > > > >
Hi All, Are there any other opinion for this RFC patch? Best Regards, Jaehoon Chung On 03/30/2016 09:15 AM, Shawn Lin wrote: > ? 2016/3/29 18:49, Jisheng Zhang ??: >> Hi Jaehoon, >> >> On Tue, 29 Mar 2016 19:43:34 +0900 Jaehoon Chung wrote: >> >>> This patch is just RFC. I want to know opinions. >>> >>> Now, index of mmcblk is allocated in accordance with probing time. >>> If want to use the mmcblk1 for some device, it can use alias. >>> >>> aliases { >>> mmc0 = &mmc0; /* mmcblk0 for eMMC */ >>> mmc1 = &mmc2; /* mmcblk1 for SD */ >>> mmc2 = &mmc1; /* mmcblk2 for SDIO*/ >>> }; >> >> I like this feature and we do need it. In the past, we have to put emmc dt node >> before the sd node, but on our platform, the register base of sdhc host for emmc >> is higher than the one for sd, putting emmc dt node before sd looks a bit >> strange, but we have no better solutions. >> > > :) That is also what I suffered currently. I have to prepare two > seperate firmware with two diff fstab files to assign the mount point > since sometime I need to boot from sd, but sometimes from emmc.... > With this patch, I only need to dtc a new dtb... quite simple. > >> Thanks, >> Jisheng >> >>> >>> If there are no corresponding values, it might be allocated with >>> existing scheme. >>> >>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> >>> --- >>> Documentation/devicetree/bindings/mmc/mmc.txt | 11 +++++++++++ >>> drivers/mmc/card/block.c | 17 ++++++++++++++++- >>> 2 files changed, 27 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt >>> index ed23b9b..e37ea29 100644 >>> --- a/Documentation/devicetree/bindings/mmc/mmc.txt >>> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt >>> @@ -71,6 +71,10 @@ Optional SDIO properties: >>> - wakeup-source: Enables wake up of host system on SDIO IRQ assertion >>> (Legacy property supported: "enable-sdio-wakeup") >>> >>> +Aliases (Optional): >>> +- If you want to use the fixed index for block device like mmcblkX, should be >>> +represented in the aliases node using following format "mmc(X)". >>> +(X is an unique number for the alias.) >>> >>> MMC power sequences: >>> -------------------- >>> @@ -145,3 +149,10 @@ mmc3: mmc@01c12000 { >>> interrupt-names = "host-wake"; >>> }; >>> }; >>> + >>> +Example with aliases nodes: >>> + >>> +aliases { >>> + mmc0 = &mmc0; /* Fixed to mmcblk0 for &mmc0 */ >>> + mmc1 = &mmc2; /* Fixed to mmcblk1 for &mmc2 */ >>> +}; >>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c >>> index 3bdbe50..6a40de5 100644 >>> --- a/drivers/mmc/card/block.c >>> +++ b/drivers/mmc/card/block.c >>> @@ -35,6 +35,7 @@ >>> #include <linux/capability.h> >>> #include <linux/compat.h> >>> #include <linux/pm_runtime.h> >>> +#include <linux/of.h> >>> >>> #include <linux/mmc/ioctl.h> >>> #include <linux/mmc/card.h> >>> @@ -2190,6 +2191,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card, >>> { >>> struct mmc_blk_data *md; >>> int devidx, ret; >>> + int idx = 0; >>> >>> devidx = find_first_zero_bit(dev_use, max_devices); >>> if (devidx >= max_devices) >>> @@ -2209,7 +2211,20 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card, >>> * index anymore so we keep track of a name index. >>> */ >>> if (!subname) { >>> - md->name_idx = find_first_zero_bit(name_use, max_devices); >>> + if (card->dev.parent->parent->of_node) >>> + idx = of_alias_get_id(card->dev.parent->parent->of_node, >>> + "mmc"); >>> + >>> + if (idx < 0) >>> + md->name_idx = find_first_zero_bit(name_use, >>> + max_devices); >>> + else { >>> + if (test_bit(idx, name_use)) >>> + md->name_idx = find_first_zero_bit(name_use, >>> + max_devices); >>> + else >>> + md->name_idx = (unsigned int)idx; >>> + } >>> __set_bit(md->name_idx, name_use); >>> } else >>> md->name_idx = ((struct mmc_blk_data *) >> >> >> >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4 April 2016 at 04:08, Jaehoon Chung <jh80.chung@samsung.com> wrote: > Hi All, > > Are there any other opinion for this RFC patch? > > Best Regards, > Jaehoon Chung > > On 03/30/2016 09:15 AM, Shawn Lin wrote: >> ? 2016/3/29 18:49, Jisheng Zhang ??: >>> Hi Jaehoon, >>> >>> On Tue, 29 Mar 2016 19:43:34 +0900 Jaehoon Chung wrote: >>> >>>> This patch is just RFC. I want to know opinions. >>>> >>>> Now, index of mmcblk is allocated in accordance with probing time. >>>> If want to use the mmcblk1 for some device, it can use alias. >>>> >>>> aliases { >>>> mmc0 = &mmc0; /* mmcblk0 for eMMC */ >>>> mmc1 = &mmc2; /* mmcblk1 for SD */ >>>> mmc2 = &mmc1; /* mmcblk2 for SDIO*/ >>>> }; >>> >>> I like this feature and we do need it. In the past, we have to put emmc dt node >>> before the sd node, but on our platform, the register base of sdhc host for emmc >>> is higher than the one for sd, putting emmc dt node before sd looks a bit >>> strange, but we have no better solutions. >>> >> >> :) That is also what I suffered currently. I have to prepare two >> seperate firmware with two diff fstab files to assign the mount point >> since sometime I need to boot from sd, but sometimes from emmc.... >> With this patch, I only need to dtc a new dtb... quite simple. >> >>> Thanks, >>> Jisheng I am not immediately opposed to this patch, although let me think a bit more about it. What I would like to understand is why UUID/PARTUUID isn't working for your case. Did you try to use that? Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/04/2016 09:14 PM, Ulf Hansson wrote: > On 4 April 2016 at 04:08, Jaehoon Chung <jh80.chung@samsung.com> wrote: >> Hi All, >> >> Are there any other opinion for this RFC patch? >> >> Best Regards, >> Jaehoon Chung >> >> On 03/30/2016 09:15 AM, Shawn Lin wrote: >>> ? 2016/3/29 18:49, Jisheng Zhang ??: >>>> Hi Jaehoon, >>>> >>>> On Tue, 29 Mar 2016 19:43:34 +0900 Jaehoon Chung wrote: >>>> >>>>> This patch is just RFC. I want to know opinions. >>>>> >>>>> Now, index of mmcblk is allocated in accordance with probing time. >>>>> If want to use the mmcblk1 for some device, it can use alias. >>>>> >>>>> aliases { >>>>> mmc0 = &mmc0; /* mmcblk0 for eMMC */ >>>>> mmc1 = &mmc2; /* mmcblk1 for SD */ >>>>> mmc2 = &mmc1; /* mmcblk2 for SDIO*/ >>>>> }; >>>> >>>> I like this feature and we do need it. In the past, we have to put emmc dt node >>>> before the sd node, but on our platform, the register base of sdhc host for emmc >>>> is higher than the one for sd, putting emmc dt node before sd looks a bit >>>> strange, but we have no better solutions. >>>> >>> >>> :) That is also what I suffered currently. I have to prepare two >>> seperate firmware with two diff fstab files to assign the mount point >>> since sometime I need to boot from sd, but sometimes from emmc.... >>> With this patch, I only need to dtc a new dtb... quite simple. >>> >>>> Thanks, >>>> Jisheng > > I am not immediately opposed to this patch, although let me think a > bit more about it. > > What I would like to understand is why UUID/PARTUUID isn't working for > your case. Did you try to use that? I have used with UUID/PARTUUID/PARTLABEL and etc.. I think it's not problem. But that is why some guys wants to use the fixed index. (Almost all SoCs are still using "root=mmcblk0pX" in bootloader.) I'm considering more what is better. :) Best Regards, Jaehoon Chung > > Kind regards > Uffe > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Tue, Apr 5, 2016 at 7:00 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >> I am not immediately opposed to this patch, although let me think a >> bit more about it. >> >> What I would like to understand is why UUID/PARTUUID isn't working for >> your case. Did you try to use that? > > I have used with UUID/PARTUUID/PARTLABEL and etc.. I think it's not problem. > But that is why some guys wants to use the fixed index. > (Almost all SoCs are still using "root=mmcblk0pX" in bootloader.) > > I'm considering more what is better. :) I like the idea of consistent numbering here. IMHO that solves a few different problems: 1. For poor, feeble-minded humans like me, have sane numbering for devices helps a lot. When grepping through dmesg it's terribly handy if a given SDMMC device has a consistent number. I know that I can do "dmesg | grep mmc0" or "dmesg | grep mmcblk0" to find info about the eMMC. I know that I can do "dmesg | grep mmc1" to find info about the SD card slot. I don't want it to matter which one probed first, I don't want it to matter if I'm working on a variant of the hardware that has the SD card slot disabled, and I don't want to care what my boot device was. Worrying about what device number I got increases my cognitive load. 2. There are cases where it's not trivially easy during development to use the UUID. Specifically I work a lot with coreboot / depthcharge as a BIOS. When configured properly, that BIOS has a nice feature to allow you to fetch the kernel and kernel command line from TFTP by pressing Ctrl-N. In this particular case the BIOS doesn't actually know which disk I'd like for my root filesystem, so it's not so easy for it to put the right UUID into the command line. For this purpose, knowing that "mmcblk0" will always refer to eMMC is handy. Note that I would personally prefer that the old patches to solve this problem were resurrected. Those old patches: * Also ensure consistent numbering for "mmc", not just "mmcblk" * Are slightly cleaner in the "mmcblk". I've just rebased and cleaned up those patches and included the Documentation from here. I'll post them now. -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt index ed23b9b..e37ea29 100644 --- a/Documentation/devicetree/bindings/mmc/mmc.txt +++ b/Documentation/devicetree/bindings/mmc/mmc.txt @@ -71,6 +71,10 @@ Optional SDIO properties: - wakeup-source: Enables wake up of host system on SDIO IRQ assertion (Legacy property supported: "enable-sdio-wakeup") +Aliases (Optional): +- If you want to use the fixed index for block device like mmcblkX, should be +represented in the aliases node using following format "mmc(X)". +(X is an unique number for the alias.) MMC power sequences: -------------------- @@ -145,3 +149,10 @@ mmc3: mmc@01c12000 { interrupt-names = "host-wake"; }; }; + +Example with aliases nodes: + +aliases { + mmc0 = &mmc0; /* Fixed to mmcblk0 for &mmc0 */ + mmc1 = &mmc2; /* Fixed to mmcblk1 for &mmc2 */ +}; diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index 3bdbe50..6a40de5 100644 --- a/drivers/mmc/card/block.c +++ b/drivers/mmc/card/block.c @@ -35,6 +35,7 @@ #include <linux/capability.h> #include <linux/compat.h> #include <linux/pm_runtime.h> +#include <linux/of.h> #include <linux/mmc/ioctl.h> #include <linux/mmc/card.h> @@ -2190,6 +2191,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card, { struct mmc_blk_data *md; int devidx, ret; + int idx = 0; devidx = find_first_zero_bit(dev_use, max_devices); if (devidx >= max_devices) @@ -2209,7 +2211,20 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card, * index anymore so we keep track of a name index. */ if (!subname) { - md->name_idx = find_first_zero_bit(name_use, max_devices); + if (card->dev.parent->parent->of_node) + idx = of_alias_get_id(card->dev.parent->parent->of_node, + "mmc"); + + if (idx < 0) + md->name_idx = find_first_zero_bit(name_use, + max_devices); + else { + if (test_bit(idx, name_use)) + md->name_idx = find_first_zero_bit(name_use, + max_devices); + else + md->name_idx = (unsigned int)idx; + } __set_bit(md->name_idx, name_use); } else md->name_idx = ((struct mmc_blk_data *)
This patch is just RFC. I want to know opinions. Now, index of mmcblk is allocated in accordance with probing time. If want to use the mmcblk1 for some device, it can use alias. aliases { mmc0 = &mmc0; /* mmcblk0 for eMMC */ mmc1 = &mmc2; /* mmcblk1 for SD */ mmc2 = &mmc1; /* mmcblk2 for SDIO*/ }; If there are no corresponding values, it might be allocated with existing scheme. Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> --- Documentation/devicetree/bindings/mmc/mmc.txt | 11 +++++++++++ drivers/mmc/card/block.c | 17 ++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-)