| Message ID | 20201119155025.965941-1-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: ls1028a: make the eMMC and SD card controllers use fixed indices | expand |
Hi Vladimir, Am 2020-11-19 16:50, schrieb Vladimir Oltean: > As the boot order in the kernel continues to change, sometimes it may > happen that the eSDHC controller mmc@2150000 (the one for eMMC) gets > probed before the one at mmc@2140000 (for external SD cards). The > effect > is that the eMMC controller gets the /dev/mmcblk0 name, and the SD card > gets /dev/mmcblk1. Thanks for taking care. > Since the introduction of this SoC, that has never happened in > practice, > even though it was never guaranteed in theory. Setting > "root=/dev/mmcblk0p2" in /proc/cmdline has always caused the kernel to > use the second partition from the SD card as the rootfs. > > Preserve that old behavior by adding some aliases which create naming > consistency: > - the SD card controller uses /dev/mmcblk0 > - the eMMC controller uses /dev/mmcblk1 Could you change this behaviour for the sl28 board(s)? I've always found it counter-intuitive to have mmcblk1 being the eMMC on a board which always have the eMMC populated. All our images uses UUIDs for the "root=" parameter and, technically, the order wasn't specified yet. So I'd like to have the eMMC as /dev/mmcblk0 and the SD card as /dev/mmcblk1. -michael
On Thu, Nov 19, 2020 at 05:08:39PM +0100, Michael Walle wrote: > Could you change this behaviour for the sl28 board(s)? I've always > found it counter-intuitive to have mmcblk1 being the eMMC on a board > which always have the eMMC populated. All our images uses UUIDs for > the "root=" parameter and, technically, the order wasn't specified yet. > > So I'd like to have the eMMC as /dev/mmcblk0 and the SD card as > /dev/mmcblk1. Sure I can do that.
Hi Vladimir, I have already upstreamed a patch for all affected layerscape boards. https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/dt64&id=342ab37ecaf8c1b10dd3ca9a1271db29a6af0705 Please check whether it works for you. Thanks. Best regards, Yangbo Lu > -----Original Message----- > From: Vladimir Oltean <vladimir.oltean@nxp.com> > Sent: Thursday, November 19, 2020 11:50 PM > To: Shawn Guo <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob > Herring <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > devicetree@vger.kernel.org > Cc: Adrian Hunter <adrian.hunter@intel.com>; Ulf Hansson > <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com>; Y.b. Lu > <yangbo.lu@nxp.com>; Michael Walle <michael@walle.cc> > Subject: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > controllers use fixed indices > > As the boot order in the kernel continues to change, sometimes it may > happen that the eSDHC controller mmc@2150000 (the one for eMMC) gets > probed before the one at mmc@2140000 (for external SD cards). The effect > is that the eMMC controller gets the /dev/mmcblk0 name, and the SD card > gets /dev/mmcblk1. > > Since the introduction of this SoC, that has never happened in practice, > even though it was never guaranteed in theory. Setting > "root=/dev/mmcblk0p2" in /proc/cmdline has always caused the kernel to > use the second partition from the SD card as the rootfs. > > Preserve that old behavior by adding some aliases which create naming > consistency: > - the SD card controller uses /dev/mmcblk0 > - the eMMC controller uses /dev/mmcblk1 > > The aliases are parsed by mmc_alloc_host() in drivers/mmc/core/host.c. > > Cc: Ashish Kumar <Ashish.Kumar@nxp.com> > Cc: Yangbo Lu <yangbo.lu@nxp.com> > Cc: Michael Walle <michael@walle.cc> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts | 2 ++ > arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 2 ++ > arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 2 ++ > 3 files changed, 6 insertions(+) > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts > b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts > index 8161dd237971..7d292999f8da 100644 > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts > @@ -23,6 +23,8 @@ aliases { > serial2 = &lpuart1; > spi0 = &fspi; > spi1 = &dspi2; > + mmc0 = &esdhc; > + mmc1 = &esdhc1; > }; > > buttons0 { > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts > b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts > index 13cdc958ba3e..c0786b713791 100644 > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts > @@ -23,6 +23,8 @@ aliases { > gpio2 = &gpio3; > serial0 = &duart0; > serial1 = &duart1; > + mmc0 = &esdhc; > + mmc1 = &esdhc1; > }; > > chosen { > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts > b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts > index 1efb61cff454..c1d1ba459307 100644 > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts > @@ -19,6 +19,8 @@ aliases { > crypto = &crypto; > serial0 = &duart0; > serial1 = &duart1; > + mmc0 = &esdhc; > + mmc1 = &esdhc1; > }; > > chosen { > -- > 2.25.1
On Fri, Nov 20, 2020 at 02:04:02AM +0000, Y.b. Lu wrote: > Hi Vladimir, > > I have already upstreamed a patch for all affected layerscape boards. > https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/dt64&id=342ab37ecaf8c1b10dd3ca9a1271db29a6af0705 > > Please check whether it works for you. Thanks, one can tell that I haven't done my due diligence of checking Shawn's tree first. I'll cherry-pick that patch and carry on with my work. However, the fact still remains that Michael has expressed his opinion regarding mmcblk0 vs mmcblk1. Do you think that we could make the aliases a per-board option instead of per-SoC? Consider that there might even be boards that only use SD card. It would be strange for the block device in that case to be called /dev/mmcblk1.
Am 2020-11-20 10:30, schrieb Vladimir Oltean: > On Fri, Nov 20, 2020 at 02:04:02AM +0000, Y.b. Lu wrote: >> Hi Vladimir, >> >> I have already upstreamed a patch for all affected layerscape boards. >> https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/dt64&id=342ab37ecaf8c1b10dd3ca9a1271db29a6af0705 >> >> Please check whether it works for you. > > Thanks, one can tell that I haven't done my due diligence of checking > Shawn's tree first. I'll cherry-pick that patch and carry on with my > work. > > However, the fact still remains that Michael has expressed his opinion > regarding mmcblk0 vs mmcblk1. Do you think that we could make the > aliases a per-board option instead of per-SoC? Consider that there > might > even be boards that only use SD card. It would be strange for the block > device in that case to be called /dev/mmcblk1. Yangbo, any news on this? I'd like to have this resorted out before it is set in stone for 5.10.
Hi Vladimir, > -----Original Message----- > From: Vladimir Oltean <vladimir.oltean@nxp.com> > Sent: Friday, November 20, 2020 5:30 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob > Herring <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; Ulf > Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com>; > Michael Walle <michael@walle.cc> > Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > controllers use fixed indices > > On Fri, Nov 20, 2020 at 02:04:02AM +0000, Y.b. Lu wrote: > > Hi Vladimir, > > > > I have already upstreamed a patch for all affected layerscape boards. > > > https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/commit/? > h=imx/dt64&id=342ab37ecaf8c1b10dd3ca9a1271db29a6af0705 > > > > Please check whether it works for you. > > Thanks, one can tell that I haven't done my due diligence of checking > Shawn's tree first. I'll cherry-pick that patch and carry on with my > work. > > However, the fact still remains that Michael has expressed his opinion > regarding mmcblk0 vs mmcblk1. Do you think that we could make the > aliases a per-board option instead of per-SoC? Consider that there might > even be boards that only use SD card. It would be strange for the block > device in that case to be called /dev/mmcblk1. I don't think it's a problem in board dts to define board specific thing, like re-defining alias, and disabling any IP it not using. Thanks.
Am 2020-11-24 08:41, schrieb Y.b. Lu: > Hi Vladimir, > >> -----Original Message----- >> From: Vladimir Oltean <vladimir.oltean@nxp.com> >> Sent: Friday, November 20, 2020 5:30 PM >> To: Y.b. Lu <yangbo.lu@nxp.com> >> Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob >> Herring <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; >> Ulf >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com>; >> Michael Walle <michael@walle.cc> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card >> controllers use fixed indices >> >> On Fri, Nov 20, 2020 at 02:04:02AM +0000, Y.b. Lu wrote: >> > Hi Vladimir, >> > >> > I have already upstreamed a patch for all affected layerscape boards. >> > >> https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/commit/? >> h=imx/dt64&id=342ab37ecaf8c1b10dd3ca9a1271db29a6af0705 >> > >> > Please check whether it works for you. >> >> Thanks, one can tell that I haven't done my due diligence of checking >> Shawn's tree first. I'll cherry-pick that patch and carry on with my >> work. >> >> However, the fact still remains that Michael has expressed his opinion >> regarding mmcblk0 vs mmcblk1. Do you think that we could make the >> aliases a per-board option instead of per-SoC? Consider that there >> might >> even be boards that only use SD card. It would be strange for the >> block >> device in that case to be called /dev/mmcblk1. > > I don't think it's a problem in board dts to define board specific > thing, like re-defining alias, and disabling any IP it not using. First, why would you put it in the architecture include anyway? That is really board-specific. That is like you would say, we enable all devices and a board could potentially disable it. TBH it seems that this will fit your reference boards and you don't care about the other ones which uses that include. And as Vladimir pointed out, what do you do if you just have the eMMC on the LS1028A. It will be mmcblk1 unless you do something like the following in the board dts: mmc0 = &esdhc; /delete-property/ mmc1; That is really cumbersome, isnt it? -michael
Hi Michael, > -----Original Message----- > From: Michael Walle <michael@walle.cc> > Sent: Tuesday, November 24, 2020 4:03 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo > <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring > <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; Ulf > Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> > Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > controllers use fixed indices > > Am 2020-11-24 08:41, schrieb Y.b. Lu: > > Hi Vladimir, > > > >> -----Original Message----- > >> From: Vladimir Oltean <vladimir.oltean@nxp.com> > >> Sent: Friday, November 20, 2020 5:30 PM > >> To: Y.b. Lu <yangbo.lu@nxp.com> > >> Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob > >> Herring <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; > >> Ulf > >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com>; > >> Michael Walle <michael@walle.cc> > >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > >> controllers use fixed indices > >> > >> On Fri, Nov 20, 2020 at 02:04:02AM +0000, Y.b. Lu wrote: > >> > Hi Vladimir, > >> > > >> > I have already upstreamed a patch for all affected layerscape boards. > >> > > >> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern > el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fshawnguo%2Flinux.git%2 > Fcommit%2F&data=04%7C01%7Cyangbo.lu%40nxp.com%7C498622ade > e704fc0042008d8904f6184%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 > %7C0%7C637418017917635725%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000 > &sdata=OciS3q%2BmP%2Bz4x1ewPHDigmUkgIZmBgUlRRTm4yaxB7s%3D > &reserved=0? > >> h=imx/dt64&id=342ab37ecaf8c1b10dd3ca9a1271db29a6af0705 > >> > > >> > Please check whether it works for you. > >> > >> Thanks, one can tell that I haven't done my due diligence of checking > >> Shawn's tree first. I'll cherry-pick that patch and carry on with my > >> work. > >> > >> However, the fact still remains that Michael has expressed his opinion > >> regarding mmcblk0 vs mmcblk1. Do you think that we could make the > >> aliases a per-board option instead of per-SoC? Consider that there > >> might > >> even be boards that only use SD card. It would be strange for the > >> block > >> device in that case to be called /dev/mmcblk1. > > > > I don't think it's a problem in board dts to define board specific > > thing, like re-defining alias, and disabling any IP it not using. > > First, why would you put it in the architecture include anyway? That > is really board-specific. That is like you would say, we enable all > devices and a board could potentially disable it. TBH it seems that > this will fit your reference boards and you don't care about the > other ones which uses that include. In soc dtsi, this is giving default alias for two esdhc controllers. This is not board specific. That's natural esdhc0 is mmc0 and esdhc1 is mmc1. > > And as Vladimir pointed out, what do you do if you just have the eMMC > on the LS1028A. It will be mmcblk1 unless you do something like the > following in the board dts: > > mmc0 = &esdhc; > /delete-property/ mmc1; > > That is really cumbersome, isnt it? The soc dtsi gives default alias to make esdhc0 as mmc0, and esdhc1 as mmc1, the use case just needs to consider which esdhc controller is used. That's fixed index for it. No matter how the board is designed, there are two esdhc controllers in soc. It's probed as mmc0 and mmc1. It's use case that should choose the right mmc device. It is not the dts that should be changed to suit use case. If the board owner insists to change alias to make esdhc1 as mmc0, I think no problem. Just do it in board dts to override the default one. Thanks. > > -michael
Am 2020-11-24 09:47, schrieb Y.b. Lu: > Hi Michael, > >> -----Original Message----- >> From: Michael Walle <michael@walle.cc> >> Sent: Tuesday, November 24, 2020 4:03 PM >> To: Y.b. Lu <yangbo.lu@nxp.com> >> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo >> <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring >> <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; >> Ulf >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card >> controllers use fixed indices >> >> Am 2020-11-24 08:41, schrieb Y.b. Lu: >> > Hi Vladimir, >> > >> >> -----Original Message----- >> >> From: Vladimir Oltean <vladimir.oltean@nxp.com> >> >> Sent: Friday, November 20, 2020 5:30 PM >> >> To: Y.b. Lu <yangbo.lu@nxp.com> >> >> Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob >> >> Herring <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; >> >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; >> >> Ulf >> >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; >> >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com>; >> >> Michael Walle <michael@walle.cc> >> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card >> >> controllers use fixed indices >> >> >> >> On Fri, Nov 20, 2020 at 02:04:02AM +0000, Y.b. Lu wrote: >> >> > Hi Vladimir, >> >> > >> >> > I have already upstreamed a patch for all affected layerscape boards. >> >> > >> >> >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern >> el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fshawnguo%2Flinux.git%2 >> Fcommit%2F&data=04%7C01%7Cyangbo.lu%40nxp.com%7C498622ade >> e704fc0042008d8904f6184%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 >> %7C0%7C637418017917635725%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM >> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000 >> &sdata=OciS3q%2BmP%2Bz4x1ewPHDigmUkgIZmBgUlRRTm4yaxB7s%3D >> &reserved=0? >> >> h=imx/dt64&id=342ab37ecaf8c1b10dd3ca9a1271db29a6af0705 >> >> > >> >> > Please check whether it works for you. >> >> >> >> Thanks, one can tell that I haven't done my due diligence of checking >> >> Shawn's tree first. I'll cherry-pick that patch and carry on with my >> >> work. >> >> >> >> However, the fact still remains that Michael has expressed his opinion >> >> regarding mmcblk0 vs mmcblk1. Do you think that we could make the >> >> aliases a per-board option instead of per-SoC? Consider that there >> >> might >> >> even be boards that only use SD card. It would be strange for the >> >> block >> >> device in that case to be called /dev/mmcblk1. >> > >> > I don't think it's a problem in board dts to define board specific >> > thing, like re-defining alias, and disabling any IP it not using. >> >> First, why would you put it in the architecture include anyway? That >> is really board-specific. That is like you would say, we enable all >> devices and a board could potentially disable it. TBH it seems that >> this will fit your reference boards and you don't care about the >> other ones which uses that include. > > In soc dtsi, this is giving default alias for two esdhc controllers. > This is not board specific. > That's natural esdhc0 is mmc0 and esdhc1 is mmc1. How could this be not board specific if there are at least three different use cases the board can choose from - and needs three different configurations: (1) eMMC at /dev/mmcblk0, SD card at /dev/mmcblk1 (2) SD card at /dev/mmcblk0, eMMC at /dev/mmcblk1 (3) no eMMC at all, SD card at /dev/mmcblk0 your include only support (1). If a board needs (2) or (3) it has to override the configuration in the _common_ include. >> And as Vladimir pointed out, what do you do if you just have the eMMC >> on the LS1028A. It will be mmcblk1 unless you do something like the >> following in the board dts: >> >> mmc0 = &esdhc; >> /delete-property/ mmc1; >> >> That is really cumbersome, isnt it? > > The soc dtsi gives default alias to make esdhc0 as mmc0, and esdhc1 as > mmc1, the use case just needs to consider which esdhc controller is > used. That's fixed index for it. > No matter how the board is designed, there are two esdhc controllers > in soc. It's probed as mmc0 and mmc1. > It's use case that should choose the right mmc device. It is not the > dts that should be changed to suit use case. > If the board owner insists to change alias to make esdhc1 as mmc0, I > think no problem. Just do it in board dts to override the default one. Still, why would this be enforced in the common include? What is the advnatage here? I only see disadvantages. -michael
Hi Michael, > -----Original Message----- > From: Michael Walle <michael@walle.cc> > Sent: Tuesday, November 24, 2020 4:55 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo > <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring > <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; Ulf > Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> > Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > controllers use fixed indices > > Am 2020-11-24 09:47, schrieb Y.b. Lu: > > Hi Michael, > > > >> -----Original Message----- > >> From: Michael Walle <michael@walle.cc> > >> Sent: Tuesday, November 24, 2020 4:03 PM > >> To: Y.b. Lu <yangbo.lu@nxp.com> > >> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo > >> <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring > >> <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; > >> Ulf > >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> > >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > >> controllers use fixed indices > >> > >> Am 2020-11-24 08:41, schrieb Y.b. Lu: > >> > Hi Vladimir, > >> > > >> >> -----Original Message----- > >> >> From: Vladimir Oltean <vladimir.oltean@nxp.com> > >> >> Sent: Friday, November 20, 2020 5:30 PM > >> >> To: Y.b. Lu <yangbo.lu@nxp.com> > >> >> Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; > Rob > >> >> Herring <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > >> >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; > >> >> Ulf > >> >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > >> >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com>; > >> >> Michael Walle <michael@walle.cc> > >> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > >> >> controllers use fixed indices > >> >> > >> >> On Fri, Nov 20, 2020 at 02:04:02AM +0000, Y.b. Lu wrote: > >> >> > Hi Vladimir, > >> >> > > >> >> > I have already upstreamed a patch for all affected layerscape boards. > >> >> > > >> >> > >> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern > >> > el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fshawnguo%2Flinux.git%2 > >> > Fcommit%2F&data=04%7C01%7Cyangbo.lu%40nxp.com%7C498622ade > >> > e704fc0042008d8904f6184%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 > >> %7C0%7C637418017917635725%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi > M > >> > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000 > >> > &sdata=OciS3q%2BmP%2Bz4x1ewPHDigmUkgIZmBgUlRRTm4yaxB7s%3D > >> &reserved=0? > >> >> h=imx/dt64&id=342ab37ecaf8c1b10dd3ca9a1271db29a6af0705 > >> >> > > >> >> > Please check whether it works for you. > >> >> > >> >> Thanks, one can tell that I haven't done my due diligence of checking > >> >> Shawn's tree first. I'll cherry-pick that patch and carry on with my > >> >> work. > >> >> > >> >> However, the fact still remains that Michael has expressed his opinion > >> >> regarding mmcblk0 vs mmcblk1. Do you think that we could make the > >> >> aliases a per-board option instead of per-SoC? Consider that there > >> >> might > >> >> even be boards that only use SD card. It would be strange for the > >> >> block > >> >> device in that case to be called /dev/mmcblk1. > >> > > >> > I don't think it's a problem in board dts to define board specific > >> > thing, like re-defining alias, and disabling any IP it not using. > >> > >> First, why would you put it in the architecture include anyway? That > >> is really board-specific. That is like you would say, we enable all > >> devices and a board could potentially disable it. TBH it seems that > >> this will fit your reference boards and you don't care about the > >> other ones which uses that include. > > > > In soc dtsi, this is giving default alias for two esdhc controllers. > > This is not board specific. > > That's natural esdhc0 is mmc0 and esdhc1 is mmc1. > > How could this be not board specific if there are at least three > different use cases the board can choose from - and needs three > different configurations: > > (1) eMMC at /dev/mmcblk0, SD card at /dev/mmcblk1 > (2) SD card at /dev/mmcblk0, eMMC at /dev/mmcblk1 > (3) no eMMC at all, SD card at /dev/mmcblk0 Not matter it's SD card or eMMC card, if it's on esdhc0, use /dev/mmcblk0. Not matter it's SD card or eMMC card, if it's on esdhc1, use /dev/mmcblk1. It's not related to board and card type, it's only related to esdhc interface in use. Thanks. > > your include only support (1). If a board needs (2) or (3) it has to > override the configuration in the _common_ include. > > >> And as Vladimir pointed out, what do you do if you just have the eMMC > >> on the LS1028A. It will be mmcblk1 unless you do something like the > >> following in the board dts: > >> > >> mmc0 = &esdhc; > >> /delete-property/ mmc1; > >> > >> That is really cumbersome, isnt it? > > > > The soc dtsi gives default alias to make esdhc0 as mmc0, and esdhc1 as > > mmc1, the use case just needs to consider which esdhc controller is > > used. That's fixed index for it. > > No matter how the board is designed, there are two esdhc controllers > > in soc. It's probed as mmc0 and mmc1. > > It's use case that should choose the right mmc device. It is not the > > dts that should be changed to suit use case. > > If the board owner insists to change alias to make esdhc1 as mmc0, I > > think no problem. Just do it in board dts to override the default one. > > Still, why would this be enforced in the common include? What is the > advnatage here? I only see disadvantages. > > -michael
Am 2020-11-24 10:02, schrieb Y.b. Lu: > Hi Michael, > >> -----Original Message----- >> From: Michael Walle <michael@walle.cc> >> Sent: Tuesday, November 24, 2020 4:55 PM >> To: Y.b. Lu <yangbo.lu@nxp.com> >> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo >> <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring >> <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; >> Ulf >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card >> controllers use fixed indices >> >> Am 2020-11-24 09:47, schrieb Y.b. Lu: >> > Hi Michael, >> > >> >> -----Original Message----- >> >> From: Michael Walle <michael@walle.cc> >> >> Sent: Tuesday, November 24, 2020 4:03 PM >> >> To: Y.b. Lu <yangbo.lu@nxp.com> >> >> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo >> >> <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring >> >> <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; >> >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; >> >> Ulf >> >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; >> >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> >> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card >> >> controllers use fixed indices >> >> >> >> Am 2020-11-24 08:41, schrieb Y.b. Lu: >> >> > Hi Vladimir, >> >> > >> >> >> -----Original Message----- >> >> >> From: Vladimir Oltean <vladimir.oltean@nxp.com> >> >> >> Sent: Friday, November 20, 2020 5:30 PM >> >> >> To: Y.b. Lu <yangbo.lu@nxp.com> >> >> >> Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; >> Rob >> >> >> Herring <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; >> >> >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; >> >> >> Ulf >> >> >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; >> >> >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com>; >> >> >> Michael Walle <michael@walle.cc> >> >> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card >> >> >> controllers use fixed indices >> >> >> >> >> >> On Fri, Nov 20, 2020 at 02:04:02AM +0000, Y.b. Lu wrote: >> >> >> > Hi Vladimir, >> >> >> > >> >> >> > I have already upstreamed a patch for all affected layerscape boards. >> >> >> > >> >> >> >> >> >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern >> >> >> el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fshawnguo%2Flinux.git%2 >> >> >> Fcommit%2F&data=04%7C01%7Cyangbo.lu%40nxp.com%7C498622ade >> >> >> e704fc0042008d8904f6184%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 >> >> %7C0%7C637418017917635725%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi >> M >> >> >> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000 >> >> >> &sdata=OciS3q%2BmP%2Bz4x1ewPHDigmUkgIZmBgUlRRTm4yaxB7s%3D >> >> &reserved=0? >> >> >> h=imx/dt64&id=342ab37ecaf8c1b10dd3ca9a1271db29a6af0705 >> >> >> > >> >> >> > Please check whether it works for you. >> >> >> >> >> >> Thanks, one can tell that I haven't done my due diligence of checking >> >> >> Shawn's tree first. I'll cherry-pick that patch and carry on with my >> >> >> work. >> >> >> >> >> >> However, the fact still remains that Michael has expressed his opinion >> >> >> regarding mmcblk0 vs mmcblk1. Do you think that we could make the >> >> >> aliases a per-board option instead of per-SoC? Consider that there >> >> >> might >> >> >> even be boards that only use SD card. It would be strange for the >> >> >> block >> >> >> device in that case to be called /dev/mmcblk1. >> >> > >> >> > I don't think it's a problem in board dts to define board specific >> >> > thing, like re-defining alias, and disabling any IP it not using. >> >> >> >> First, why would you put it in the architecture include anyway? That >> >> is really board-specific. That is like you would say, we enable all >> >> devices and a board could potentially disable it. TBH it seems that >> >> this will fit your reference boards and you don't care about the >> >> other ones which uses that include. >> > >> > In soc dtsi, this is giving default alias for two esdhc controllers. >> > This is not board specific. >> > That's natural esdhc0 is mmc0 and esdhc1 is mmc1. >> >> How could this be not board specific if there are at least three >> different use cases the board can choose from - and needs three >> different configurations: >> >> (1) eMMC at /dev/mmcblk0, SD card at /dev/mmcblk1 >> (2) SD card at /dev/mmcblk0, eMMC at /dev/mmcblk1 >> (3) no eMMC at all, SD card at /dev/mmcblk0 > > Not matter it's SD card or eMMC card, if it's on esdhc0, use > /dev/mmcblk0. > Not matter it's SD card or eMMC card, if it's on esdhc1, use > /dev/mmcblk1. > It's not related to board and card type, it's only related to esdhc > interface in use. And what interace is used is board specific, isn't it? >> your include only support (1). If a board needs (2) or (3) it has to >> override the configuration in the _common_ include. >> >> >> And as Vladimir pointed out, what do you do if you just have the eMMC >> >> on the LS1028A. It will be mmcblk1 unless you do something like the >> >> following in the board dts: >> >> >> >> mmc0 = &esdhc; >> >> /delete-property/ mmc1; >> >> >> >> That is really cumbersome, isnt it? >> > >> > The soc dtsi gives default alias to make esdhc0 as mmc0, and esdhc1 as >> > mmc1, the use case just needs to consider which esdhc controller is >> > used. That's fixed index for it. >> > No matter how the board is designed, there are two esdhc controllers >> > in soc. It's probed as mmc0 and mmc1. >> > It's use case that should choose the right mmc device. It is not the >> > dts that should be changed to suit use case. >> > If the board owner insists to change alias to make esdhc1 as mmc0, I >> > think no problem. Just do it in board dts to override the default one. >> >> Still, why would this be enforced in the common include? What is the >> advnatage here? I only see disadvantages. You didn't answer this unfortunately. -michael
Hi Michael, > -----Original Message----- > From: Michael Walle <michael@walle.cc> > Sent: Tuesday, November 24, 2020 5:08 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo > <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring > <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; Ulf > Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> > Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > controllers use fixed indices > > Am 2020-11-24 10:02, schrieb Y.b. Lu: > > Hi Michael, > > > >> -----Original Message----- > >> From: Michael Walle <michael@walle.cc> > >> Sent: Tuesday, November 24, 2020 4:55 PM > >> To: Y.b. Lu <yangbo.lu@nxp.com> > >> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo > >> <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring > >> <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; > >> Ulf > >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> > >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > >> controllers use fixed indices > >> > >> Am 2020-11-24 09:47, schrieb Y.b. Lu: > >> > Hi Michael, > >> > > >> >> -----Original Message----- > >> >> From: Michael Walle <michael@walle.cc> > >> >> Sent: Tuesday, November 24, 2020 4:03 PM > >> >> To: Y.b. Lu <yangbo.lu@nxp.com> > >> >> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo > >> >> <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring > >> >> <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > >> >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; > >> >> Ulf > >> >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > >> >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> > >> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > >> >> controllers use fixed indices > >> >> > >> >> Am 2020-11-24 08:41, schrieb Y.b. Lu: > >> >> > Hi Vladimir, > >> >> > > >> >> >> -----Original Message----- > >> >> >> From: Vladimir Oltean <vladimir.oltean@nxp.com> > >> >> >> Sent: Friday, November 20, 2020 5:30 PM > >> >> >> To: Y.b. Lu <yangbo.lu@nxp.com> > >> >> >> Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li > <leoyang.li@nxp.com>; > >> Rob > >> >> >> Herring <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > >> >> >> devicetree@vger.kernel.org; Adrian Hunter > <adrian.hunter@intel.com>; > >> >> >> Ulf > >> >> >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > >> >> >> linux-kernel@vger.kernel.org; Ashish Kumar > <ashish.kumar@nxp.com>; > >> >> >> Michael Walle <michael@walle.cc> > >> >> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD > card > >> >> >> controllers use fixed indices > >> >> >> > >> >> >> On Fri, Nov 20, 2020 at 02:04:02AM +0000, Y.b. Lu wrote: > >> >> >> > Hi Vladimir, > >> >> >> > > >> >> >> > I have already upstreamed a patch for all affected layerscape > boards. > >> >> >> > > >> >> >> > >> >> > >> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern > >> >> > >> > el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fshawnguo%2Flinux.git%2 > >> >> > >> > Fcommit%2F&data=04%7C01%7Cyangbo.lu%40nxp.com%7C498622ade > >> >> > >> > e704fc0042008d8904f6184%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 > >> >> %7C0%7C637418017917635725%7CUnknown%7CTWFpbGZsb3d8eyJW > Ijoi > >> M > >> >> > >> > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000 > >> >> > >> > &sdata=OciS3q%2BmP%2Bz4x1ewPHDigmUkgIZmBgUlRRTm4yaxB7s%3D > >> >> &reserved=0? > >> >> >> h=imx/dt64&id=342ab37ecaf8c1b10dd3ca9a1271db29a6af0705 > >> >> >> > > >> >> >> > Please check whether it works for you. > >> >> >> > >> >> >> Thanks, one can tell that I haven't done my due diligence of checking > >> >> >> Shawn's tree first. I'll cherry-pick that patch and carry on with my > >> >> >> work. > >> >> >> > >> >> >> However, the fact still remains that Michael has expressed his opinion > >> >> >> regarding mmcblk0 vs mmcblk1. Do you think that we could make > the > >> >> >> aliases a per-board option instead of per-SoC? Consider that there > >> >> >> might > >> >> >> even be boards that only use SD card. It would be strange for the > >> >> >> block > >> >> >> device in that case to be called /dev/mmcblk1. > >> >> > > >> >> > I don't think it's a problem in board dts to define board specific > >> >> > thing, like re-defining alias, and disabling any IP it not using. > >> >> > >> >> First, why would you put it in the architecture include anyway? That > >> >> is really board-specific. That is like you would say, we enable all > >> >> devices and a board could potentially disable it. TBH it seems that > >> >> this will fit your reference boards and you don't care about the > >> >> other ones which uses that include. > >> > > >> > In soc dtsi, this is giving default alias for two esdhc controllers. > >> > This is not board specific. > >> > That's natural esdhc0 is mmc0 and esdhc1 is mmc1. > >> > >> How could this be not board specific if there are at least three > >> different use cases the board can choose from - and needs three > >> different configurations: > >> > >> (1) eMMC at /dev/mmcblk0, SD card at /dev/mmcblk1 > >> (2) SD card at /dev/mmcblk0, eMMC at /dev/mmcblk1 > >> (3) no eMMC at all, SD card at /dev/mmcblk0 > > > > Not matter it's SD card or eMMC card, if it's on esdhc0, use > > /dev/mmcblk0. > > Not matter it's SD card or eMMC card, if it's on esdhc1, use > > /dev/mmcblk1. > > It's not related to board and card type, it's only related to esdhc > > interface in use. > > And what interace is used is board specific, isn't it? Again, for all boards, use /dev/mmcblk0 for card on esdhc0 interface, and /dev/mmcblk1 for card on esdhc1 interface. That's not board specific. > > >> your include only support (1). If a board needs (2) or (3) it has to > >> override the configuration in the _common_ include. > >> > >> >> And as Vladimir pointed out, what do you do if you just have the eMMC > >> >> on the LS1028A. It will be mmcblk1 unless you do something like the > >> >> following in the board dts: > >> >> > >> >> mmc0 = &esdhc; > >> >> /delete-property/ mmc1; > >> >> > >> >> That is really cumbersome, isnt it? > >> > > >> > The soc dtsi gives default alias to make esdhc0 as mmc0, and esdhc1 as > >> > mmc1, the use case just needs to consider which esdhc controller is > >> > used. That's fixed index for it. > >> > No matter how the board is designed, there are two esdhc controllers > >> > in soc. It's probed as mmc0 and mmc1. > >> > It's use case that should choose the right mmc device. It is not the > >> > dts that should be changed to suit use case. > >> > If the board owner insists to change alias to make esdhc1 as mmc0, I > >> > think no problem. Just do it in board dts to override the default one. > >> > >> Still, why would this be enforced in the common include? What is the > >> advnatage here? I only see disadvantages. > > You didn't answer this unfortunately. As I explained, "Again, for all boards, use /dev/mmcblk0 for card on esdhc0 interface, and /dev/mmcblk1 for card on esdhc1 interface. That's not board specific." Without such definition, the index is random for each booting. Thanks. > > -michael
Am 2020-11-24 10:22, schrieb Y.b. Lu: > Hi Michael, > >> -----Original Message----- >> From: Michael Walle <michael@walle.cc> >> Sent: Tuesday, November 24, 2020 5:08 PM >> To: Y.b. Lu <yangbo.lu@nxp.com> >> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo >> <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring >> <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; >> Ulf >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card >> controllers use fixed indices >> >> Am 2020-11-24 10:02, schrieb Y.b. Lu: >> > Hi Michael, >> > >> >> -----Original Message----- >> >> From: Michael Walle <michael@walle.cc> >> >> Sent: Tuesday, November 24, 2020 4:55 PM >> >> To: Y.b. Lu <yangbo.lu@nxp.com> >> >> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo >> >> <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring >> >> <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; >> >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; >> >> Ulf >> >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; >> >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> >> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card >> >> controllers use fixed indices >> >> >> >> Am 2020-11-24 09:47, schrieb Y.b. Lu: >> >> > Hi Michael, >> >> > >> >> >> -----Original Message----- >> >> >> From: Michael Walle <michael@walle.cc> >> >> >> Sent: Tuesday, November 24, 2020 4:03 PM >> >> >> To: Y.b. Lu <yangbo.lu@nxp.com> >> >> >> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo >> >> >> <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring >> >> >> <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; >> >> >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; >> >> >> Ulf >> >> >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; >> >> >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> >> >> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card >> >> >> controllers use fixed indices >> >> >> >> >> >> Am 2020-11-24 08:41, schrieb Y.b. Lu: >> >> >> > Hi Vladimir, >> >> >> > >> >> >> >> -----Original Message----- >> >> >> >> From: Vladimir Oltean <vladimir.oltean@nxp.com> >> >> >> >> Sent: Friday, November 20, 2020 5:30 PM >> >> >> >> To: Y.b. Lu <yangbo.lu@nxp.com> >> >> >> >> Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li >> <leoyang.li@nxp.com>; >> >> Rob >> >> >> >> Herring <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; >> >> >> >> devicetree@vger.kernel.org; Adrian Hunter >> <adrian.hunter@intel.com>; >> >> >> >> Ulf >> >> >> >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; >> >> >> >> linux-kernel@vger.kernel.org; Ashish Kumar >> <ashish.kumar@nxp.com>; >> >> >> >> Michael Walle <michael@walle.cc> >> >> >> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD >> card >> >> >> >> controllers use fixed indices >> >> >> >> >> >> >> >> On Fri, Nov 20, 2020 at 02:04:02AM +0000, Y.b. Lu wrote: >> >> >> >> > Hi Vladimir, >> >> >> >> > >> >> >> >> > I have already upstreamed a patch for all affected layerscape >> boards. >> >> >> >> > >> >> >> >> >> >> >> >> >> >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern >> >> >> >> >> >> el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fshawnguo%2Flinux.git%2 >> >> >> >> >> >> Fcommit%2F&data=04%7C01%7Cyangbo.lu%40nxp.com%7C498622ade >> >> >> >> >> >> e704fc0042008d8904f6184%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 >> >> >> %7C0%7C637418017917635725%7CUnknown%7CTWFpbGZsb3d8eyJW >> Ijoi >> >> M >> >> >> >> >> >> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000 >> >> >> >> >> >> &sdata=OciS3q%2BmP%2Bz4x1ewPHDigmUkgIZmBgUlRRTm4yaxB7s%3D >> >> >> &reserved=0? >> >> >> >> h=imx/dt64&id=342ab37ecaf8c1b10dd3ca9a1271db29a6af0705 >> >> >> >> > >> >> >> >> > Please check whether it works for you. >> >> >> >> >> >> >> >> Thanks, one can tell that I haven't done my due diligence of checking >> >> >> >> Shawn's tree first. I'll cherry-pick that patch and carry on with my >> >> >> >> work. >> >> >> >> >> >> >> >> However, the fact still remains that Michael has expressed his opinion >> >> >> >> regarding mmcblk0 vs mmcblk1. Do you think that we could make >> the >> >> >> >> aliases a per-board option instead of per-SoC? Consider that there >> >> >> >> might >> >> >> >> even be boards that only use SD card. It would be strange for the >> >> >> >> block >> >> >> >> device in that case to be called /dev/mmcblk1. >> >> >> > >> >> >> > I don't think it's a problem in board dts to define board specific >> >> >> > thing, like re-defining alias, and disabling any IP it not using. >> >> >> >> >> >> First, why would you put it in the architecture include anyway? That >> >> >> is really board-specific. That is like you would say, we enable all >> >> >> devices and a board could potentially disable it. TBH it seems that >> >> >> this will fit your reference boards and you don't care about the >> >> >> other ones which uses that include. >> >> > >> >> > In soc dtsi, this is giving default alias for two esdhc controllers. >> >> > This is not board specific. >> >> > That's natural esdhc0 is mmc0 and esdhc1 is mmc1. >> >> >> >> How could this be not board specific if there are at least three >> >> different use cases the board can choose from - and needs three >> >> different configurations: >> >> >> >> (1) eMMC at /dev/mmcblk0, SD card at /dev/mmcblk1 >> >> (2) SD card at /dev/mmcblk0, eMMC at /dev/mmcblk1 >> >> (3) no eMMC at all, SD card at /dev/mmcblk0 >> > >> > Not matter it's SD card or eMMC card, if it's on esdhc0, use >> > /dev/mmcblk0. >> > Not matter it's SD card or eMMC card, if it's on esdhc1, use >> > /dev/mmcblk1. >> > It's not related to board and card type, it's only related to esdhc >> > interface in use. >> >> And what interace is used is board specific, isn't it? > > Again, for all boards, use /dev/mmcblk0 for card on esdhc0 interface, > and /dev/mmcblk1 for card on esdhc1 interface. > That's not board specific. So why don't you enable the devices by default then? That would be the same reasoning, wouldn't it? Or even enable all devices by default. Nobody does that. But the boards themselves enable the devices which _they_ are actually using. >> >> your include only support (1). If a board needs (2) or (3) it has to >> >> override the configuration in the _common_ include. >> >> >> >> >> And as Vladimir pointed out, what do you do if you just have the eMMC >> >> >> on the LS1028A. It will be mmcblk1 unless you do something like the >> >> >> following in the board dts: >> >> >> >> >> >> mmc0 = &esdhc; >> >> >> /delete-property/ mmc1; >> >> >> >> >> >> That is really cumbersome, isnt it? >> >> > >> >> > The soc dtsi gives default alias to make esdhc0 as mmc0, and esdhc1 as >> >> > mmc1, the use case just needs to consider which esdhc controller is >> >> > used. That's fixed index for it. >> >> > No matter how the board is designed, there are two esdhc controllers >> >> > in soc. It's probed as mmc0 and mmc1. >> >> > It's use case that should choose the right mmc device. It is not the >> >> > dts that should be changed to suit use case. >> >> > If the board owner insists to change alias to make esdhc1 as mmc0, I >> >> > think no problem. Just do it in board dts to override the default one. >> >> >> >> Still, why would this be enforced in the common include? What is the >> >> advnatage here? I only see disadvantages. >> >> You didn't answer this unfortunately. > > As I explained, > "Again, for all boards, use /dev/mmcblk0 for card on esdhc0 interface, > and /dev/mmcblk1 for card on esdhc1 interface. > That's not board specific." > > Without such definition, the index is random for each booting. No the question was why to include this into the common header. Not into the board specific ones. -michael
Hi Michael, > -----Original Message----- > From: Michael Walle <michael@walle.cc> > Sent: Tuesday, November 24, 2020 5:43 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo > <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring > <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; Ulf > Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> > Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > controllers use fixed indices > > Am 2020-11-24 10:22, schrieb Y.b. Lu: > > Hi Michael, > > > >> -----Original Message----- > >> From: Michael Walle <michael@walle.cc> > >> Sent: Tuesday, November 24, 2020 5:08 PM > >> To: Y.b. Lu <yangbo.lu@nxp.com> > >> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo > >> <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring > >> <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; > >> Ulf > >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> > >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > >> controllers use fixed indices > >> > >> Am 2020-11-24 10:02, schrieb Y.b. Lu: > >> > Hi Michael, > >> > > >> >> -----Original Message----- > >> >> From: Michael Walle <michael@walle.cc> > >> >> Sent: Tuesday, November 24, 2020 4:55 PM > >> >> To: Y.b. Lu <yangbo.lu@nxp.com> > >> >> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo > >> >> <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring > >> >> <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > >> >> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; > >> >> Ulf > >> >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > >> >> linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> > >> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > >> >> controllers use fixed indices > >> >> > >> >> Am 2020-11-24 09:47, schrieb Y.b. Lu: > >> >> > Hi Michael, > >> >> > > >> >> >> -----Original Message----- > >> >> >> From: Michael Walle <michael@walle.cc> > >> >> >> Sent: Tuesday, November 24, 2020 4:03 PM > >> >> >> To: Y.b. Lu <yangbo.lu@nxp.com> > >> >> >> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Shawn Guo > >> >> >> <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; Rob Herring > >> >> >> <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > >> >> >> devicetree@vger.kernel.org; Adrian Hunter > <adrian.hunter@intel.com>; > >> >> >> Ulf > >> >> >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > >> >> >> linux-kernel@vger.kernel.org; Ashish Kumar > <ashish.kumar@nxp.com> > >> >> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD > card > >> >> >> controllers use fixed indices > >> >> >> > >> >> >> Am 2020-11-24 08:41, schrieb Y.b. Lu: > >> >> >> > Hi Vladimir, > >> >> >> > > >> >> >> >> -----Original Message----- > >> >> >> >> From: Vladimir Oltean <vladimir.oltean@nxp.com> > >> >> >> >> Sent: Friday, November 20, 2020 5:30 PM > >> >> >> >> To: Y.b. Lu <yangbo.lu@nxp.com> > >> >> >> >> Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li > >> <leoyang.li@nxp.com>; > >> >> Rob > >> >> >> >> Herring <robh+dt@kernel.org>; > linux-arm-kernel@lists.infradead.org; > >> >> >> >> devicetree@vger.kernel.org; Adrian Hunter > >> <adrian.hunter@intel.com>; > >> >> >> >> Ulf > >> >> >> >> Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > >> >> >> >> linux-kernel@vger.kernel.org; Ashish Kumar > >> <ashish.kumar@nxp.com>; > >> >> >> >> Michael Walle <michael@walle.cc> > >> >> >> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD > >> card > >> >> >> >> controllers use fixed indices > >> >> >> >> > >> >> >> >> On Fri, Nov 20, 2020 at 02:04:02AM +0000, Y.b. Lu wrote: > >> >> >> >> > Hi Vladimir, > >> >> >> >> > > >> >> >> >> > I have already upstreamed a patch for all affected layerscape > >> boards. > >> >> >> >> > > >> >> >> >> > >> >> >> > >> >> > >> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern > >> >> >> > >> >> > >> > el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fshawnguo%2Flinux.git%2 > >> >> >> > >> >> > >> > Fcommit%2F&data=04%7C01%7Cyangbo.lu%40nxp.com%7C498622ade > >> >> >> > >> >> > >> > e704fc0042008d8904f6184%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 > >> >> >> %7C0%7C637418017917635725%7CUnknown%7CTWFpbGZsb3d8ey > JW > >> Ijoi > >> >> M > >> >> >> > >> >> > >> > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000 > >> >> >> > >> >> > >> > &sdata=OciS3q%2BmP%2Bz4x1ewPHDigmUkgIZmBgUlRRTm4yaxB7s%3D > >> >> >> &reserved=0? > >> >> >> >> h=imx/dt64&id=342ab37ecaf8c1b10dd3ca9a1271db29a6af0705 > >> >> >> >> > > >> >> >> >> > Please check whether it works for you. > >> >> >> >> > >> >> >> >> Thanks, one can tell that I haven't done my due diligence of > checking > >> >> >> >> Shawn's tree first. I'll cherry-pick that patch and carry on with my > >> >> >> >> work. > >> >> >> >> > >> >> >> >> However, the fact still remains that Michael has expressed his > opinion > >> >> >> >> regarding mmcblk0 vs mmcblk1. Do you think that we could make > >> the > >> >> >> >> aliases a per-board option instead of per-SoC? Consider that there > >> >> >> >> might > >> >> >> >> even be boards that only use SD card. It would be strange for the > >> >> >> >> block > >> >> >> >> device in that case to be called /dev/mmcblk1. > >> >> >> > > >> >> >> > I don't think it's a problem in board dts to define board specific > >> >> >> > thing, like re-defining alias, and disabling any IP it not using. > >> >> >> > >> >> >> First, why would you put it in the architecture include anyway? That > >> >> >> is really board-specific. That is like you would say, we enable all > >> >> >> devices and a board could potentially disable it. TBH it seems that > >> >> >> this will fit your reference boards and you don't care about the > >> >> >> other ones which uses that include. > >> >> > > >> >> > In soc dtsi, this is giving default alias for two esdhc controllers. > >> >> > This is not board specific. > >> >> > That's natural esdhc0 is mmc0 and esdhc1 is mmc1. > >> >> > >> >> How could this be not board specific if there are at least three > >> >> different use cases the board can choose from - and needs three > >> >> different configurations: > >> >> > >> >> (1) eMMC at /dev/mmcblk0, SD card at /dev/mmcblk1 > >> >> (2) SD card at /dev/mmcblk0, eMMC at /dev/mmcblk1 > >> >> (3) no eMMC at all, SD card at /dev/mmcblk0 > >> > > >> > Not matter it's SD card or eMMC card, if it's on esdhc0, use > >> > /dev/mmcblk0. > >> > Not matter it's SD card or eMMC card, if it's on esdhc1, use > >> > /dev/mmcblk1. > >> > It's not related to board and card type, it's only related to esdhc > >> > interface in use. > >> > >> And what interace is used is board specific, isn't it? > > > > Again, for all boards, use /dev/mmcblk0 for card on esdhc0 interface, > > and /dev/mmcblk1 for card on esdhc1 interface. > > That's not board specific. > > So why don't you enable the devices by default then? That would be the > same reasoning, wouldn't it? Or even enable all devices by default. > Nobody > does that. But the boards themselves enable the devices which _they_ > are actually using. I explained clearly. Now the point is, I think the alias of mmc0 for esdhc0, mmc1 for esdhc1 is soc specific (no need to change the controller alias in board file), while you think it's totally board specific. Let's wait others' comments. Thanks. > > > >> >> your include only support (1). If a board needs (2) or (3) it has to > >> >> override the configuration in the _common_ include. > >> >> > >> >> >> And as Vladimir pointed out, what do you do if you just have the > eMMC > >> >> >> on the LS1028A. It will be mmcblk1 unless you do something like the > >> >> >> following in the board dts: > >> >> >> > >> >> >> mmc0 = &esdhc; > >> >> >> /delete-property/ mmc1; > >> >> >> > >> >> >> That is really cumbersome, isnt it? > >> >> > > >> >> > The soc dtsi gives default alias to make esdhc0 as mmc0, and esdhc1 as > >> >> > mmc1, the use case just needs to consider which esdhc controller is > >> >> > used. That's fixed index for it. > >> >> > No matter how the board is designed, there are two esdhc controllers > >> >> > in soc. It's probed as mmc0 and mmc1. > >> >> > It's use case that should choose the right mmc device. It is not the > >> >> > dts that should be changed to suit use case. > >> >> > If the board owner insists to change alias to make esdhc1 as mmc0, I > >> >> > think no problem. Just do it in board dts to override the default one. > >> >> > >> >> Still, why would this be enforced in the common include? What is the > >> >> advnatage here? I only see disadvantages. > >> > >> You didn't answer this unfortunately. > > > > As I explained, > > "Again, for all boards, use /dev/mmcblk0 for card on esdhc0 interface, > > and /dev/mmcblk1 for card on esdhc1 interface. > > That's not board specific." > > > > Without such definition, the index is random for each booting. > > No the question was why to include this into the common header. Not into > the board specific ones. > > -michael
Hi Yangbo, On Tue, Nov 24, 2020 at 09:02:57AM +0000, Y.b. Lu wrote: > > Am 2020-11-24 09:47, schrieb Y.b. Lu: > > > Hi Michael, > > >> > I don't think it's a problem in board dts to define board specific > > >> > thing, like re-defining alias, and disabling any IP it not using. > > >> > > >> First, why would you put it in the architecture include anyway? That > > >> is really board-specific. That is like you would say, we enable all > > >> devices and a board could potentially disable it. TBH it seems that > > >> this will fit your reference boards and you don't care about the > > >> other ones which uses that include. > > > > > > In soc dtsi, this is giving default alias for two esdhc controllers. > > > This is not board specific. > > > That's natural esdhc0 is mmc0 and esdhc1 is mmc1. > > > > How could this be not board specific if there are at least three > > different use cases the board can choose from - and needs three > > different configurations: > > > > (1) eMMC at /dev/mmcblk0, SD card at /dev/mmcblk1 > > (2) SD card at /dev/mmcblk0, eMMC at /dev/mmcblk1 > > (3) no eMMC at all, SD card at /dev/mmcblk0 > > Not matter it's SD card or eMMC card, if it's on esdhc0, use /dev/mmcblk0. > Not matter it's SD card or eMMC card, if it's on esdhc1, use /dev/mmcblk1. With the note here that you can't actually connect an SD card to eSDHC1, due to the lack of pins for CD/WP. > It's not related to board and card type, it's only related to esdhc interface in use. I understand the hardware-centric view that you are coming from. It may be natural for you that eSDHC0 is for the SD card and eSDHC1 is for eMMC, because these are the designations in the SoC. But it is also natural for a customer to define the indices according to their schematics and what they use. If, say, there is a board that only uses eMMC, I would expect that for the lay person, no one would even bat an eye if that was called /dev/mmcblk0. Whereas, if it was called /dev/mmcblk1 (and there was no /dev/mmcblk0 in the system), maybe you'd have to come up with some explanations which could be avoided. I am only a passerby when it comes to the MMC subsystem. But in networking/DSA, it is frequent that the board designer comes up with their own numbering scheme, which has nothing to do with the numbering of the chip. Consider this extreme case from arch/arm/boot/dts/ls1021a-tsn.dts: sja1105: ethernet-switch@1 { ports { port@0 { /* ETH5 written on chassis */ label = "swp5"; }; port@1 { /* ETH2 written on chassis */ label = "swp2"; }; port@2 { /* ETH3 written on chassis */ label = "swp3"; }; port@3 { /* ETH4 written on chassis */ label = "swp4"; }; }; }; You just have to go along with how the hardware is being used in the product. I could have insisted that hardware switch port 0 is named as swp0, but that would have not helped anybody.
Hi Vladimir, > -----Original Message----- > From: Vladimir Oltean <vladimir.oltean@nxp.com> > Sent: Tuesday, November 24, 2020 6:31 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: Michael Walle <michael@walle.cc>; Shawn Guo <shawnguo@kernel.org>; > Leo Li <leoyang.li@nxp.com>; Rob Herring <robh+dt@kernel.org>; > linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; Adrian > Hunter <adrian.hunter@intel.com>; Ulf Hansson <ulf.hansson@linaro.org>; > linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Ashish Kumar > <ashish.kumar@nxp.com> > Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > controllers use fixed indices > > Hi Yangbo, > > On Tue, Nov 24, 2020 at 09:02:57AM +0000, Y.b. Lu wrote: > > > Am 2020-11-24 09:47, schrieb Y.b. Lu: > > > > Hi Michael, > > > >> > I don't think it's a problem in board dts to define board specific > > > >> > thing, like re-defining alias, and disabling any IP it not using. > > > >> > > > >> First, why would you put it in the architecture include anyway? That > > > >> is really board-specific. That is like you would say, we enable all > > > >> devices and a board could potentially disable it. TBH it seems that > > > >> this will fit your reference boards and you don't care about the > > > >> other ones which uses that include. > > > > > > > > In soc dtsi, this is giving default alias for two esdhc controllers. > > > > This is not board specific. > > > > That's natural esdhc0 is mmc0 and esdhc1 is mmc1. > > > > > > How could this be not board specific if there are at least three > > > different use cases the board can choose from - and needs three > > > different configurations: > > > > > > (1) eMMC at /dev/mmcblk0, SD card at /dev/mmcblk1 > > > (2) SD card at /dev/mmcblk0, eMMC at /dev/mmcblk1 > > > (3) no eMMC at all, SD card at /dev/mmcblk0 > > > > Not matter it's SD card or eMMC card, if it's on esdhc0, use /dev/mmcblk0. > > Not matter it's SD card or eMMC card, if it's on esdhc1, use /dev/mmcblk1. > > With the note here that you can't actually connect an SD card to eSDHC1, > due to the lack of pins for CD/WP. CD/WP is not essential to support SD card. Both SD/eMMC are supported on both eSDHC controllers. > > > It's not related to board and card type, it's only related to esdhc interface in > use. > > I understand the hardware-centric view that you are coming from. It may > be natural for you that eSDHC0 is for the SD card and eSDHC1 is for eMMC, > because these are the designations in the SoC. Right, from hardware-centric view, it's natural esdhc0 interface is mmc0, and esdhc1 is mmc1. That's what I explained for why I added the alias in common soc dtsi. > > But it is also natural for a customer to define the indices according to > their schematics and what they use. If, say, there is a board that only > uses eMMC, I would expect that for the lay person, no one would even bat > an eye if that was called /dev/mmcblk0. Whereas, if it was called > /dev/mmcblk1 (and there was no /dev/mmcblk0 in the system), maybe you'd > have to come up with some explanations which could be avoided. To make a product friendly to users, it makes sense to define different alias for controller in board dts. But it's not the reason to remove the default/natural alias in soc dtsi for two controllers. What needs to be done after removing them? Add the same to all other board files? This is just my opinion. I don't decide on this:) > > I am only a passerby when it comes to the MMC subsystem. But in > networking/DSA, it is frequent that the board designer comes up with > their own numbering scheme, which has nothing to do with the numbering > of the chip. Consider this extreme case from > arch/arm/boot/dts/ls1021a-tsn.dts: > > sja1105: ethernet-switch@1 { > ports { > port@0 { > /* ETH5 written on chassis */ > label = "swp5"; > }; > > port@1 { > /* ETH2 written on chassis */ > label = "swp2"; > }; > > port@2 { > /* ETH3 written on chassis */ > label = "swp3"; > }; > > port@3 { > /* ETH4 written on chassis */ > label = "swp4"; > }; > }; > }; > > You just have to go along with how the hardware is being used in the > product. I could have insisted that hardware switch port 0 is named as > swp0, but that would have not helped anybody.
On Tue, Nov 24, 2020 at 11:15:19AM +0000, Y.b. Lu wrote: > > > Not matter it's SD card or eMMC card, if it's on esdhc0, use /dev/mmcblk0. > > > Not matter it's SD card or eMMC card, if it's on esdhc1, use /dev/mmcblk1. > > > > With the note here that you can't actually connect an SD card to eSDHC1, > > due to the lack of pins for CD/WP. > > CD/WP is not essential to support SD card. Both SD/eMMC are supported on both eSDHC controllers. Let's keep that discussion separate. While in theory you might be right, I think the real-life complications associated with connecting an eMMC to eSDHC0 and an SD card to eSDHC1 will make everyone avoid that. So in practice they are still single-purpose. > > But it is also natural for a customer to define the indices according to > > their schematics and what they use. If, say, there is a board that only > > uses eMMC, I would expect that for the lay person, no one would even bat > > an eye if that was called /dev/mmcblk0. Whereas, if it was called > > /dev/mmcblk1 (and there was no /dev/mmcblk0 in the system), maybe you'd > > have to come up with some explanations which could be avoided. > > To make a product friendly to users, it makes sense to define different alias for controller in board dts. > But it's not the reason to remove the default/natural alias in soc dtsi for two controllers. > What needs to be done after removing them? Add the same to all other board files? Yes. In fact, this is also the reason why we prefer to have: /soc { esdhc: mmc@2140000 { status = "disabled"; }; }; in fsl-ls1028a.dtsi and &esdhc { status = "okay"; }; in fsl-ls1028a-rdb.dts and not the other way around: /soc { esdhc: mmc@2140000 { status = "okay"; }; }; in fsl-ls1028a.dtsi and &esdhc { status = "disabled"; }; in fsl-ls1028a-rdb.dts So, in line with that, I think that the entity that enables the node should also define the alias. It's weird to use /delete-property/ if it can be avoided.
Hi Vladimir, > -----Original Message----- > From: Vladimir Oltean <vladimir.oltean@nxp.com> > Sent: Tuesday, November 24, 2020 7:28 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: Michael Walle <michael@walle.cc>; Shawn Guo <shawnguo@kernel.org>; > Leo Li <leoyang.li@nxp.com>; Rob Herring <robh+dt@kernel.org>; > linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; Adrian > Hunter <adrian.hunter@intel.com>; Ulf Hansson <ulf.hansson@linaro.org>; > linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Ashish Kumar > <ashish.kumar@nxp.com> > Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > controllers use fixed indices > > On Tue, Nov 24, 2020 at 11:15:19AM +0000, Y.b. Lu wrote: > > > > Not matter it's SD card or eMMC card, if it's on esdhc0, use > /dev/mmcblk0. > > > > Not matter it's SD card or eMMC card, if it's on esdhc1, use > /dev/mmcblk1. > > > > > > With the note here that you can't actually connect an SD card to eSDHC1, > > > due to the lack of pins for CD/WP. > > > > CD/WP is not essential to support SD card. Both SD/eMMC are supported on > both eSDHC controllers. > > Let's keep that discussion separate. While in theory you might be right, > I think the real-life complications associated with connecting an eMMC > to eSDHC0 and an SD card to eSDHC1 will make everyone avoid that. So in > practice they are still single-purpose. You may refer to Layerscape QDS boards. 5 types SDHC adapters with PCIe connecter supporting SD or eMMC could be used on each esdhc interface. Another reason using default mmc0 for esdhc0 and mmc1 for esdhc1, is because that's also the order before esdhc driver introducing asynchronous probe. Distros, bootloaders, and users' cases using fixed index before could avoid issues, and been used as they were. Thanks. > > > > But it is also natural for a customer to define the indices according to > > > their schematics and what they use. If, say, there is a board that only > > > uses eMMC, I would expect that for the lay person, no one would even bat > > > an eye if that was called /dev/mmcblk0. Whereas, if it was called > > > /dev/mmcblk1 (and there was no /dev/mmcblk0 in the system), maybe > you'd > > > have to come up with some explanations which could be avoided. > > > > To make a product friendly to users, it makes sense to define different alias > for controller in board dts. > > But it's not the reason to remove the default/natural alias in soc dtsi for two > controllers. > > What needs to be done after removing them? Add the same to all other > board files? > > Yes. > In fact, this is also the reason why we prefer to have: > /soc { > esdhc: mmc@2140000 { > status = "disabled"; > }; > }; > in fsl-ls1028a.dtsi > and > &esdhc { > status = "okay"; > }; > in fsl-ls1028a-rdb.dts > and not the other way around: > > /soc { > esdhc: mmc@2140000 { > status = "okay"; > }; > }; > in fsl-ls1028a.dtsi > and > &esdhc { > status = "disabled"; > }; > in fsl-ls1028a-rdb.dts > > So, in line with that, I think that the entity that enables the node > should also define the alias. > > It's weird to use /delete-property/ if it can be avoided.
Hi Yangbo, Hi Shawn, Am 2020-11-25 03:59, schrieb Y.b. Lu: >> -----Original Message----- >> From: Vladimir Oltean <vladimir.oltean@nxp.com> >> Sent: Tuesday, November 24, 2020 7:28 PM >> To: Y.b. Lu <yangbo.lu@nxp.com> >> Cc: Michael Walle <michael@walle.cc>; Shawn Guo <shawnguo@kernel.org>; >> Leo Li <leoyang.li@nxp.com>; Rob Herring <robh+dt@kernel.org>; >> linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; >> Adrian >> Hunter <adrian.hunter@intel.com>; Ulf Hansson >> <ulf.hansson@linaro.org>; >> linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Ashish Kumar >> <ashish.kumar@nxp.com> >> Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card >> controllers use fixed indices >> >> On Tue, Nov 24, 2020 at 11:15:19AM +0000, Y.b. Lu wrote: >> > > > Not matter it's SD card or eMMC card, if it's on esdhc0, use >> /dev/mmcblk0. >> > > > Not matter it's SD card or eMMC card, if it's on esdhc1, use >> /dev/mmcblk1. >> > > >> > > With the note here that you can't actually connect an SD card to eSDHC1, >> > > due to the lack of pins for CD/WP. >> > >> > CD/WP is not essential to support SD card. Both SD/eMMC are supported on >> both eSDHC controllers. >> >> Let's keep that discussion separate. While in theory you might be >> right, >> I think the real-life complications associated with connecting an eMMC >> to eSDHC0 and an SD card to eSDHC1 will make everyone avoid that. So >> in >> practice they are still single-purpose. > > You may refer to Layerscape QDS boards. 5 types SDHC adapters with > PCIe connecter supporting SD or eMMC could be used on each esdhc > interface. Just for completeness, on the LS1028A there is definetly one for eMMC and one for SD card. One supports voltage switching and one has a 8bit data bus. But as Vladimir already said, this doesn't matter for this discussion. > Another reason using default mmc0 for esdhc0 and mmc1 for esdhc1, is > because that's also the order before esdhc driver introducing > asynchronous probe. No if there was &esdhc { status = "disabled" }; this would change the block device from /dev/mmcblk0 to /dev/mmcblk1 for the remaining &esdhc1. We are going cirlces here. I guess Shawn (as the soc maintainer) has to step in and decide if a common soc include should contain aliases for nodes which are disabled. That is what it boils down to. All other arguments against having aliases in the common include can be found in this thread. > Distros, bootloaders, and users' cases using fixed index before could > avoid issues, and been used as they were. Nobody argue against having these alias. We are arguing against having them in the common soc include. -michael
On Wed, Nov 25, 2020 at 09:25:23AM +0100, Michael Walle wrote: > We are going cirlces here. I guess Shawn (as the soc maintainer) has to > step in and decide if a common soc include should contain aliases for > nodes which are disabled. That is what it boils down to. > > All other arguments against having aliases in the common include can be > found in this thread. > > > Distros, bootloaders, and users' cases using fixed index before could > > avoid issues, and been used as they were. > > Nobody argue against having these alias. We are arguing against having > them in the common soc include. 342ab37ecaf8 ("arm64: dts: freescale: use fixed index mmcN for layerscape") is dropped from my tree. Shawn
> -----Original Message----- > From: Shawn Guo <shawnguo@kernel.org> > Sent: Monday, November 30, 2020 10:29 PM > To: Michael Walle <michael@walle.cc> > Cc: Y.b. Lu <yangbo.lu@nxp.com>; Vladimir Oltean > <vladimir.oltean@nxp.com>; Leo Li <leoyang.li@nxp.com>; Rob Herring > <robh+dt@kernel.org>; linux-arm-kernel@lists.infradead.org; > devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>; Ulf > Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; > linux-kernel@vger.kernel.org; Ashish Kumar <ashish.kumar@nxp.com> > Subject: Re: [PATCH] arm64: dts: ls1028a: make the eMMC and SD card > controllers use fixed indices > > On Wed, Nov 25, 2020 at 09:25:23AM +0100, Michael Walle wrote: > > We are going cirlces here. I guess Shawn (as the soc maintainer) has to > > step in and decide if a common soc include should contain aliases for > > nodes which are disabled. That is what it boils down to. > > > > All other arguments against having aliases in the common include can be > > found in this thread. > > > > > Distros, bootloaders, and users' cases using fixed index before could > > > avoid issues, and been used as they were. > > > > Nobody argue against having these alias. We are arguing against having > > them in the common soc include. > > 342ab37ecaf8 ("arm64: dts: freescale: use fixed index mmcN for > layerscape") is dropped from my tree. Ok, fine. Let me send board dts patches for NXP development boards instead. Thanks. > > Shawn
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts index 8161dd237971..7d292999f8da 100644 --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts @@ -23,6 +23,8 @@ aliases { serial2 = &lpuart1; spi0 = &fspi; spi1 = &dspi2; + mmc0 = &esdhc; + mmc1 = &esdhc1; }; buttons0 { diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts index 13cdc958ba3e..c0786b713791 100644 --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts @@ -23,6 +23,8 @@ aliases { gpio2 = &gpio3; serial0 = &duart0; serial1 = &duart1; + mmc0 = &esdhc; + mmc1 = &esdhc1; }; chosen { diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts index 1efb61cff454..c1d1ba459307 100644 --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts @@ -19,6 +19,8 @@ aliases { crypto = &crypto; serial0 = &duart0; serial1 = &duart1; + mmc0 = &esdhc; + mmc1 = &esdhc1; }; chosen {
As the boot order in the kernel continues to change, sometimes it may happen that the eSDHC controller mmc@2150000 (the one for eMMC) gets probed before the one at mmc@2140000 (for external SD cards). The effect is that the eMMC controller gets the /dev/mmcblk0 name, and the SD card gets /dev/mmcblk1. Since the introduction of this SoC, that has never happened in practice, even though it was never guaranteed in theory. Setting "root=/dev/mmcblk0p2" in /proc/cmdline has always caused the kernel to use the second partition from the SD card as the rootfs. Preserve that old behavior by adding some aliases which create naming consistency: - the SD card controller uses /dev/mmcblk0 - the eMMC controller uses /dev/mmcblk1 The aliases are parsed by mmc_alloc_host() in drivers/mmc/core/host.c. Cc: Ashish Kumar <Ashish.Kumar@nxp.com> Cc: Yangbo Lu <yangbo.lu@nxp.com> Cc: Michael Walle <michael@walle.cc> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts | 2 ++ arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 2 ++ arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 2 ++ 3 files changed, 6 insertions(+)