* [PATCH 0/9] Enable dw-mmc multi-card support @ 2017-10-06 19:21 Liming Sun 2017-10-06 19:21 ` [PATCH 1/9] Revert "Documentation: dw-mshc: deprecate num-slots" Liming Sun ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Liming Sun @ 2017-10-06 19:21 UTC (permalink / raw) To: Ulf Hansson, Rob Herring, Mark Rutland, Jaehoon Chung, Kukjin Kim, Krzysztof Kozlowski Cc: Liming Sun, linux-mmc, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc This series of commits enables the multi-card support for the dw-mmc controller. It includes two parts as below. The first part (patches 1-7) reverts the series of recent commits that removed the multi-card support with comments saying there was no such use case in the real world. Actually this feature is being used in Mellanox Bluefield SoC and has been requested by customers. The second part (patches 8-9) fixes the DesignWare multi-card support according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile Storage Host Databook, 2.70a). It has changes to set the card number into the CMD register to multiplex requests to different cards when working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL registers properly according to the spec, and parse the per-card configuration to match the Linux Documentation (bindings/mmc/synopsys-dw-mshc.txt). Liming Sun (9): Revert "Documentation: dw-mshc: deprecate num-slots" Revert "mmc: dw_mmc: remove the unnecessary slot variable" Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'" Revert "mmc: dw_mmc: remove the 'id' arguments about functions relevant to slot" Revert "mmc: dw_mmc: change the array of slots" Revert "mmc: dw_mmc: remove the loop about finding slots" Revert "mmc: dw_mmc: deprecated the "num-slots" property" mmc: dw_mmc: Support two SD_MMC_CE-ATA cards mmc: dw_mmc: Parse slot-specific configuration .../devicetree/bindings/mmc/synopsys-dw-mshc.txt | 16 +- drivers/mmc/host/dw_mmc-exynos.c | 4 +- drivers/mmc/host/dw_mmc.c | 277 ++++++++++++++++----- drivers/mmc/host/dw_mmc.h | 17 +- 4 files changed, 236 insertions(+), 78 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/9] Revert "Documentation: dw-mshc: deprecate num-slots" 2017-10-06 19:21 [PATCH 0/9] Enable dw-mmc multi-card support Liming Sun @ 2017-10-06 19:21 ` Liming Sun 2017-10-13 19:28 ` Rob Herring 2017-10-16 14:35 ` [PATCH 0/9] Enable dw-mmc multi-card support Liming Sun [not found] ` <cover.1507315433.git.lsun-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 2 siblings, 1 reply; 10+ messages in thread From: Liming Sun @ 2017-10-06 19:21 UTC (permalink / raw) To: Ulf Hansson, Rob Herring, Mark Rutland Cc: Liming Sun, linux-mmc, devicetree, linux-kernel This reverts commit 3f5b4b79d4c0fa71fd7d74c2a44bbc0869c04c9b. The Mellanox BlueField SoC requires multiple slot dw-mmc support. Signed-off-by: Liming Sun <lsun@mellanox.com> --- .../devicetree/bindings/mmc/synopsys-dw-mshc.txt | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt index ef3e5f1..9cb55ca 100644 --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt @@ -12,12 +12,12 @@ Required Properties: * #address-cells: should be 1. * #size-cells: should be 0. -# Slots (DEPRECATED): The slot specific information are contained within - child-nodes with each child-node representing a supported slot. There should - be atleast one child node representing a card slot. The name of the child node - representing the slot is recommended to be slot@n where n is the unique number - of the slot connected to the controller. The following are optional properties - which can be included in the slot child node. +# Slots: The slot specific information are contained within child-nodes with + each child-node representing a supported slot. There should be atleast one + child node representing a card slot. The name of the child node representing + the slot is recommended to be slot@n where n is the unique number of the slot + connected to the controller. The following are optional properties which + can be included in the slot child node. * reg: specifies the physical slot number. The valid values of this property is 0 to (num-slots -1), where num-slots is the value @@ -63,7 +63,7 @@ Optional properties: clock(cclk_out). If it's not specified, max is 200MHZ and min is 400KHz by default. (Use the "max-frequency" instead of "clock-freq-min-max".) -* num-slots (DEPRECATED): specifies the number of slots supported by the controller. +* num-slots: specifies the number of slots supported by the controller. The number of physical slots actually used could be equal or less than the value specified by num-slots. If this property is not specified, the value of num-slot property is assumed to be 1. @@ -124,6 +124,7 @@ board specific portions as listed below. dwmmc0@12200000 { clock-frequency = <400000000>; clock-freq-min-max = <400000 200000000>; + num-slots = <1>; broken-cd; fifo-depth = <0x80>; card-detect-delay = <200>; @@ -138,6 +139,7 @@ board specific portions as listed below. dwmmc0@12200000 { clock-frequency = <400000000>; clock-freq-min-max = <400000 200000000>; + num-slots = <1>; broken-cd; fifo-depth = <0x80>; card-detect-delay = <200>; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/9] Revert "Documentation: dw-mshc: deprecate num-slots" 2017-10-06 19:21 ` [PATCH 1/9] Revert "Documentation: dw-mshc: deprecate num-slots" Liming Sun @ 2017-10-13 19:28 ` Rob Herring 0 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2017-10-13 19:28 UTC (permalink / raw) To: Liming Sun; +Cc: Ulf Hansson, Mark Rutland, linux-mmc, devicetree, linux-kernel On Fri, Oct 06, 2017 at 03:21:24PM -0400, Liming Sun wrote: > This reverts commit 3f5b4b79d4c0fa71fd7d74c2a44bbc0869c04c9b. > > The Mellanox BlueField SoC requires multiple slot dw-mmc support. > > Signed-off-by: Liming Sun <lsun@mellanox.com> > --- > .../devicetree/bindings/mmc/synopsys-dw-mshc.txt | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) Acked-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 0/9] Enable dw-mmc multi-card support 2017-10-06 19:21 [PATCH 0/9] Enable dw-mmc multi-card support Liming Sun 2017-10-06 19:21 ` [PATCH 1/9] Revert "Documentation: dw-mshc: deprecate num-slots" Liming Sun @ 2017-10-16 14:35 ` Liming Sun [not found] ` <cover.1507315433.git.lsun-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 2 siblings, 0 replies; 10+ messages in thread From: Liming Sun @ 2017-10-16 14:35 UTC (permalink / raw) To: Liming Sun, Ulf Hansson, Rob Herring, Mark Rutland, Jaehoon Chung, Kukjin Kim, Krzysztof Kozlowski Cc: linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org So far I saw one comment (an ACK) on PATCH 1/9. There seems no comments on other patches yet. Ulf, (or other maintainers), any comments or suggestions how to move forward for this patch series? Thanks, Liming -----Original Message----- From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Liming Sun Sent: Friday, October 6, 2017 3:21 PM To: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Jaehoon Chung <jh80.chung@samsung.com>; Kukjin Kim <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org> Cc: Liming Sun <lsun@mellanox.com>; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org Subject: [PATCH 0/9] Enable dw-mmc multi-card support This series of commits enables the multi-card support for the dw-mmc controller. It includes two parts as below. The first part (patches 1-7) reverts the series of recent commits that removed the multi-card support with comments saying there was no such use case in the real world. Actually this feature is being used in Mellanox Bluefield SoC and has been requested by customers. The second part (patches 8-9) fixes the DesignWare multi-card support according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile Storage Host Databook, 2.70a). It has changes to set the card number into the CMD register to multiplex requests to different cards when working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL registers properly according to the spec, and parse the per-card configuration to match the Linux Documentation (bindings/mmc/synopsys-dw-mshc.txt). Liming Sun (9): Revert "Documentation: dw-mshc: deprecate num-slots" Revert "mmc: dw_mmc: remove the unnecessary slot variable" Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'" Revert "mmc: dw_mmc: remove the 'id' arguments about functions relevant to slot" Revert "mmc: dw_mmc: change the array of slots" Revert "mmc: dw_mmc: remove the loop about finding slots" Revert "mmc: dw_mmc: deprecated the "num-slots" property" mmc: dw_mmc: Support two SD_MMC_CE-ATA cards mmc: dw_mmc: Parse slot-specific configuration .../devicetree/bindings/mmc/synopsys-dw-mshc.txt | 16 +- drivers/mmc/host/dw_mmc-exynos.c | 4 +- drivers/mmc/host/dw_mmc.c | 277 ++++++++++++++++----- drivers/mmc/host/dw_mmc.h | 17 +- 4 files changed, 236 insertions(+), 78 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <cover.1507315433.git.lsun-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>]
* Re: [PATCH 0/9] Enable dw-mmc multi-card support [not found] ` <cover.1507315433.git.lsun-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> @ 2017-10-17 1:36 ` Shawn Lin 2017-10-17 15:52 ` Liming Sun 0 siblings, 1 reply; 10+ messages in thread From: Shawn Lin @ 2017-10-17 1:36 UTC (permalink / raw) To: Liming Sun, Jaehoon Chung Cc: Ulf Hansson, Rob Herring, Mark Rutland, Kukjin Kim, Krzysztof Kozlowski, shawn.lin-TNX95d0MmH7DzftRWevZcw, linux-mmc-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA On 2017/10/7 3:21, Liming Sun wrote: > This series of commits enables the multi-card support for the dw-mmc > controller. It includes two parts as below. > > The first part (patches 1-7) reverts the series of recent commits that > removed the multi-card support with comments saying there was no such > use case in the real world. Actually this feature is being used in > Mellanox Bluefield SoC and has been requested by customers. Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it. > > The second part (patches 8-9) fixes the DesignWare multi-card support > according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile > Storage Host Databook, 2.70a). It has changes to set the card number > into the CMD register to multiplex requests to different cards when > working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL > registers properly according to the spec, and parse the per-card > configuration to match the Linux Documentation > (bindings/mmc/synopsys-dw-mshc.txt). Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time. > > Liming Sun (9): > Revert "Documentation: dw-mshc: deprecate num-slots" > Revert "mmc: dw_mmc: remove the unnecessary slot variable" > Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'" > Revert "mmc: dw_mmc: remove the 'id' arguments about functions > relevant to slot" > Revert "mmc: dw_mmc: change the array of slots" > Revert "mmc: dw_mmc: remove the loop about finding slots" > Revert "mmc: dw_mmc: deprecated the "num-slots" property" > mmc: dw_mmc: Support two SD_MMC_CE-ATA cards > mmc: dw_mmc: Parse slot-specific configuration > > .../devicetree/bindings/mmc/synopsys-dw-mshc.txt | 16 +- > drivers/mmc/host/dw_mmc-exynos.c | 4 +- > drivers/mmc/host/dw_mmc.c | 277 ++++++++++++++++----- > drivers/mmc/host/dw_mmc.h | 17 +- > 4 files changed, 236 insertions(+), 78 deletions(-) > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 0/9] Enable dw-mmc multi-card support 2017-10-17 1:36 ` Shawn Lin @ 2017-10-17 15:52 ` Liming Sun 2017-10-20 14:06 ` Jaehoon Chung 0 siblings, 1 reply; 10+ messages in thread From: Liming Sun @ 2017-10-17 15:52 UTC (permalink / raw) To: Shawn Lin, Jaehoon Chung Cc: Ulf Hansson, Rob Herring, Mark Rutland, Kukjin Kim, Krzysztof Kozlowski, linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, Chris Metcalf >> Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it. Thanks for the feedback. Yes, earlier the multi-card support was buggy indeed. We spent some time to debug it and got it working. >> Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time. The " num-slots property not found..." log message has already been removed by 8a629d26f back in 2016. Looks like we're good on this one. In dw_mci_probe (), it has code to check pdata->num_slots. If 0, the host->num_slots will be set to 1. So the logic of setting default num_slots seems already there. But correct me if I am wrong. Thanks, Liming -----Original Message----- From: Shawn Lin [mailto:shawn.lin@rock-chips.com] Sent: Monday, October 16, 2017 9:36 PM To: Liming Sun <lsun@mellanox.com>; Jaehoon Chung <jh80.chung@samsung.com> Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin Kim <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; shawn.lin@rock-chips.com; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support On 2017/10/7 3:21, Liming Sun wrote: > This series of commits enables the multi-card support for the dw-mmc > controller. It includes two parts as below. > > The first part (patches 1-7) reverts the series of recent commits that > removed the multi-card support with comments saying there was no such > use case in the real world. Actually this feature is being used in > Mellanox Bluefield SoC and has been requested by customers. Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it. > > The second part (patches 8-9) fixes the DesignWare multi-card support > according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile > Storage Host Databook, 2.70a). It has changes to set the card number > into the CMD register to multiplex requests to different cards when > working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL > registers properly according to the spec, and parse the per-card > configuration to match the Linux Documentation > (bindings/mmc/synopsys-dw-mshc.txt). Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time. > > Liming Sun (9): > Revert "Documentation: dw-mshc: deprecate num-slots" > Revert "mmc: dw_mmc: remove the unnecessary slot variable" > Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'" > Revert "mmc: dw_mmc: remove the 'id' arguments about functions > relevant to slot" > Revert "mmc: dw_mmc: change the array of slots" > Revert "mmc: dw_mmc: remove the loop about finding slots" > Revert "mmc: dw_mmc: deprecated the "num-slots" property" > mmc: dw_mmc: Support two SD_MMC_CE-ATA cards > mmc: dw_mmc: Parse slot-specific configuration > > .../devicetree/bindings/mmc/synopsys-dw-mshc.txt | 16 +- > drivers/mmc/host/dw_mmc-exynos.c | 4 +- > drivers/mmc/host/dw_mmc.c | 277 ++++++++++++++++----- > drivers/mmc/host/dw_mmc.h | 17 +- > 4 files changed, 236 insertions(+), 78 deletions(-) > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/9] Enable dw-mmc multi-card support 2017-10-17 15:52 ` Liming Sun @ 2017-10-20 14:06 ` Jaehoon Chung [not found] ` <f71d94dc-7277-8675-c21e-9c2ec77174e7-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Jaehoon Chung @ 2017-10-20 14:06 UTC (permalink / raw) To: Liming Sun, Shawn Lin Cc: Ulf Hansson, Rob Herring, Mark Rutland, Kukjin Kim, Krzysztof Kozlowski, linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, Chris Metcalf Sorry for late this.. On 10/18/2017 12:52 AM, Liming Sun wrote: >>> Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it. > > Thanks for the feedback. Yes, earlier the multi-card support was buggy indeed. We spent some time to debug it and got it working. > >>> Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time. > > The " num-slots property not found..." log message has already been removed by 8a629d26f back in 2016. Looks like we're good on this one. In dw_mci_probe (), it has code to check pdata->num_slots. If 0, the host->num_slots will be set to 1. So the logic of setting default num_slots seems already there. But correct me if I am wrong. > > Thanks, > Liming > > -----Original Message----- > From: Shawn Lin [mailto:shawn.lin@rock-chips.com] > Sent: Monday, October 16, 2017 9:36 PM > To: Liming Sun <lsun@mellanox.com>; Jaehoon Chung <jh80.chung@samsung.com> > Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin Kim <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; shawn.lin@rock-chips.com; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org > Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support > > > On 2017/10/7 3:21, Liming Sun wrote: >> This series of commits enables the multi-card support for the dw-mmc >> controller. It includes two parts as below. >> >> The first part (patches 1-7) reverts the series of recent commits that >> removed the multi-card support with comments saying there was no such >> use case in the real world. Actually this feature is being used in >> Mellanox Bluefield SoC and has been requested by customers. > > Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 and 9 said, you need them to fix problem for multi-card support, so definitely there was no such use case, and even the code was buggy to support it right? That makes the code hard to read and maintain, so we decide to remove it. Hmm.. Well, if i missed your reply for my removing patch, it's my fault..but i didn't see any reply.. At that time, we didn't see any usage and also now... Are there any patches for using multi slot? e,g) device-tree file or your own driver If there are big benefits to revert them, i don't want to back them..during almost 6 years, never use it.. > >> >> The second part (patches 8-9) fixes the DesignWare multi-card support >> according to the dw-mmc databook (synnopsys: DesignWare Cores Mobile >> Storage Host Databook, 2.70a). It has changes to set the card number >> into the CMD register to multiplex requests to different cards when >> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL >> registers properly according to the spec, and parse the per-card >> configuration to match the Linux Documentation >> (bindings/mmc/synopsys-dw-mshc.txt). the second part seems that it's only support since v2.70a..? > > Havn'e check the databook for details yet, but I think it's ok to re-introduce multi-slot support if a real user benefits from it. But you need a new patch to silent the log "num-slots property not found, assuming 1 slot is available" as we removed all the num-slots from DT at that time. > > >> >> Liming Sun (9): >> Revert "Documentation: dw-mshc: deprecate num-slots" >> Revert "mmc: dw_mmc: remove the unnecessary slot variable" >> Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'" >> Revert "mmc: dw_mmc: remove the 'id' arguments about functions >> relevant to slot" >> Revert "mmc: dw_mmc: change the array of slots" >> Revert "mmc: dw_mmc: remove the loop about finding slots" >> Revert "mmc: dw_mmc: deprecated the "num-slots" property" >> mmc: dw_mmc: Support two SD_MMC_CE-ATA cards >> mmc: dw_mmc: Parse slot-specific configuration >> >> .../devicetree/bindings/mmc/synopsys-dw-mshc.txt | 16 +- >> drivers/mmc/host/dw_mmc-exynos.c | 4 +- >> drivers/mmc/host/dw_mmc.c | 277 ++++++++++++++++----- >> drivers/mmc/host/dw_mmc.h | 17 +- >> 4 files changed, 236 insertions(+), 78 deletions(-) >> > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <f71d94dc-7277-8675-c21e-9c2ec77174e7-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>]
* RE: [PATCH 0/9] Enable dw-mmc multi-card support [not found] ` <f71d94dc-7277-8675-c21e-9c2ec77174e7-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> @ 2017-10-20 15:07 ` Liming Sun 2017-10-25 16:47 ` Ulf Hansson 0 siblings, 1 reply; 10+ messages in thread From: Liming Sun @ 2017-10-20 15:07 UTC (permalink / raw) To: Jaehoon Chung, Shawn Lin Cc: Ulf Hansson, Rob Herring, Mark Rutland, Kukjin Kim, Krzysztof Kozlowski, linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Chris Metcalf Thanks Jaehoon. Please also see my response inline. - Liming > -----Original Message----- > From: Jaehoon Chung [mailto:jh80.chung@samsung.com] > Sent: Friday, October 20, 2017 10:06 AM > To: Liming Sun <lsun@mellanox.com>; Shawn Lin <shawn.lin@rock- > chips.com> > Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring > <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin Kim > <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; linux- > mmc@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- > samsung-soc@vger.kernel.org; Chris Metcalf <cmetcalf@mellanox.com> > Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support > > Sorry for late this.. > > On 10/18/2017 12:52 AM, Liming Sun wrote: > >>> Hrm.... it's so unlucky that your patchset comes a little late. As your patch > 8 and 9 said, you need them to fix problem for multi-card support, so > definitely there was no such use case, and even the code was buggy to > support it right? That makes the code hard to read and maintain, so we > decide to remove it. > > > > Thanks for the feedback. Yes, earlier the multi-card support was buggy > indeed. We spent some time to debug it and got it working. > > > >>> Havn'e check the databook for details yet, but I think it's ok to re- > introduce multi-slot support if a real user benefits from it. But you need a > new patch to silent the log "num-slots property not found, assuming 1 slot is > available" as we removed all the num-slots from DT at that time. > > > > The " num-slots property not found..." log message has already been > removed by 8a629d26f back in 2016. Looks like we're good on this one. In > dw_mci_probe (), it has code to check pdata->num_slots. If 0, the host- > >num_slots will be set to 1. So the logic of setting default num_slots seems > already there. But correct me if I am wrong. > > > > Thanks, > > Liming > > > > -----Original Message----- > > From: Shawn Lin [mailto:shawn.lin@rock-chips.com] > > Sent: Monday, October 16, 2017 9:36 PM > > To: Liming Sun <lsun@mellanox.com>; Jaehoon Chung > > <jh80.chung@samsung.com> > > Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring > > <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Kukjin > Kim > > <kgene@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; > > shawn.lin@rock-chips.com; linux-mmc@vger.kernel.org; > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > > linux-arm-kernel@lists.infradead.org; > > linux-samsung-soc@vger.kernel.org > > Subject: Re: [PATCH 0/9] Enable dw-mmc multi-card support > > > > > > On 2017/10/7 3:21, Liming Sun wrote: > >> This series of commits enables the multi-card support for the dw-mmc > >> controller. It includes two parts as below. > >> > >> The first part (patches 1-7) reverts the series of recent commits > >> that removed the multi-card support with comments saying there was no > >> such use case in the real world. Actually this feature is being used > >> in Mellanox Bluefield SoC and has been requested by customers. > > > > Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 > and 9 said, you need them to fix problem for multi-card support, so definitely > there was no such use case, and even the code was buggy to support it right? > That makes the code hard to read and maintain, so we decide to remove it. > > Hmm.. > Well, if i missed your reply for my removing patch, it's my fault..but i didn't > see any reply.. > At that time, we didn't see any usage and also now... [Liming] It is definitely not anybody's fault. My apology that I should have made it very clear. This is a new SoC which uses two eMMC cards. This feature has been requested by customer. We made it working with patches, then realized that the support for the 2nd card was removed recently from upstream kernel. To avoid reinventing the wheel, a better approach appears to bring back the changes. > > Are there any patches for using multi slot? > e,g) device-tree file or your own driver [Liming] Patch 9/8 and 9/9 are the changes to fix the multi-slot support. We follow the original "Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt" for device configuration. There is no device-tree patches since there is no changes to any of the documentation syntax. Just in case you might be interested, below is the ACPI table we used to include the two slots. It's defined according to synopsys-dw-mshc.txt and seems working. Device (MMC0) { Name (_HID, "PRP0001") Name (_UID, Zero) Name (_CCA, 1) Name (_CRS, ResourceTemplate() { Memory32Fixed(ReadWrite, 0x6008000, 0x400) Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive) { 64 } }) // Common configuration Name(_DSD, Package() { ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Package() { Package () { "compatible", Package () {"snps,dw-mshc"}}, Package () { "fifo-depth", 0x100 }, Package () { "clock-frequency", 24000000 }, Package () { "cap-mmc-highspeed", 1 }, Package () { "card-detect-delay", 200 }, Package () { "num-slots", 2 } } }) // Slot-0 Device (SLT0) { Name(_DSD, Package() { ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Package() { Package () { "reg", 0 }, // physical slot number Package () { "bus-width", 8 } // bus width (8-bit) } }) } // Slot-1 Device (SLT1) { Name(_DSD, Package() { ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Package() { Package () { "reg", 1 }, // physical slot number Package () { "bus-width", 4 } // bus width (4-bit) } }) } } > > If there are big benefits to revert them, i don't want to back them..during > almost 6 years, never use it.. [Liming] Yeah, I could imagine it. Earlier the multi-slot didn't work well at all. We debug it on HW emulation to figure it out. The changes are not big though. We have customers asking for it. It'll be very helpful to re-enable the multi-card support. > > > > >> > >> The second part (patches 8-9) fixes the DesignWare multi-card support > >> according to the dw-mmc databook (synnopsys: DesignWare Cores > Mobile > >> Storage Host Databook, 2.70a). It has changes to set the card number > >> into the CMD register to multiplex requests to different cards when > >> working in SD_MMC_CEATA mode, set the CTYPE / CLKENA / CDTHRCTL > >> registers properly according to the spec, and parse the per-card > >> configuration to match the Linux Documentation > >> (bindings/mmc/synopsys-dw-mshc.txt). > > the second part seems that it's only support since v2.70a..? [Liming] This one is the spec I used as reference for all the changes and testing. I didn't go through previous versions due to the limitation of testing environment I have. As you mentioned, there is no multi-card usage earlier and this SoC is probably the first product to use it in real-world. So adding a version check seems safe (so it won't unnecessarily break anything just in case) . Any suggestions? > > > > > Havn'e check the databook for details yet, but I think it's ok to re-introduce > multi-slot support if a real user benefits from it. But you need a new patch to > silent the log "num-slots property not found, assuming 1 slot is available" as > we removed all the num-slots from DT at that time. > > > > > >> > >> Liming Sun (9): > >> Revert "Documentation: dw-mshc: deprecate num-slots" > >> Revert "mmc: dw_mmc: remove the unnecessary slot variable" > >> Revert "mmc: dw_mmc: use the 'slot' instead of 'cur_slot'" > >> Revert "mmc: dw_mmc: remove the 'id' arguments about functions > >> relevant to slot" > >> Revert "mmc: dw_mmc: change the array of slots" > >> Revert "mmc: dw_mmc: remove the loop about finding slots" > >> Revert "mmc: dw_mmc: deprecated the "num-slots" property" > >> mmc: dw_mmc: Support two SD_MMC_CE-ATA cards > >> mmc: dw_mmc: Parse slot-specific configuration > >> > >> .../devicetree/bindings/mmc/synopsys-dw-mshc.txt | 16 +- > >> drivers/mmc/host/dw_mmc-exynos.c | 4 +- > >> drivers/mmc/host/dw_mmc.c | 277 ++++++++++++++++---- > - > >> drivers/mmc/host/dw_mmc.h | 17 +- > >> 4 files changed, 236 insertions(+), 78 deletions(-) > >> > > > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/9] Enable dw-mmc multi-card support 2017-10-20 15:07 ` Liming Sun @ 2017-10-25 16:47 ` Ulf Hansson 2017-10-25 16:50 ` Ulf Hansson 0 siblings, 1 reply; 10+ messages in thread From: Ulf Hansson @ 2017-10-25 16:47 UTC (permalink / raw) To: Liming Sun Cc: Jaehoon Chung, Shawn Lin, Rob Herring, Mark Rutland, Kukjin Kim, Krzysztof Kozlowski, linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, Chris Metcalf Hi Liming, Sorry for jumping into the discussion like this. [...] >> >> This series of commits enables the multi-card support for the dw-mmc >> >> controller. It includes two parts as below. >> >> >> >> The first part (patches 1-7) reverts the series of recent commits >> >> that removed the multi-card support with comments saying there was no >> >> such use case in the real world. Actually this feature is being used >> >> in Mellanox Bluefield SoC and has been requested by customers. >> > >> > Hrm.... it's so unlucky that your patchset comes a little late. As your patch 8 >> and 9 said, you need them to fix problem for multi-card support, so definitely >> there was no such use case, and even the code was buggy to support it right? >> That makes the code hard to read and maintain, so we decide to remove it. Unfortunate patch 8 and 9 does *not* solve the fundamental problem with multiple slots. You can't solve it without involving the mmc core, as explained in earlier discussions [1]. >> >> Hmm.. >> Well, if i missed your reply for my removing patch, it's my fault..but i didn't >> see any reply.. >> At that time, we didn't see any usage and also now... > > [Liming] It is definitely not anybody's fault. My apology that I should have made it very clear. > This is a new SoC which uses two eMMC cards. This feature has been requested by customer. > We made it working with patches, then realized that the support for the 2nd card was removed recently from upstream kernel. > To avoid reinventing the wheel, a better approach appears to bring back the changes. > >> Having multiple slots of eMMC:s, attached to one single mmc controller, is really a terrible idea, even if one could get it to somewhat work. To deal with multiple slots from the mmc core point of view, we would need to introduce a kind of "bus slot lock", which needs to be taken whenever the core access any of the registered two mmc hosts. However, that then means that the upper block device layers can no longer perform proper I/O scheduling of requests to the eMMC cards. In other words, requests on one eMMC device can then starve requests reaching the other eMMC device. It all falls apart. Perhaps that is a limitation you think is okay to live with, but I am really not so sure it's worth adding "multiple slot" support. [...] Kind regards Uffe ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/9] Enable dw-mmc multi-card support 2017-10-25 16:47 ` Ulf Hansson @ 2017-10-25 16:50 ` Ulf Hansson 0 siblings, 0 replies; 10+ messages in thread From: Ulf Hansson @ 2017-10-25 16:50 UTC (permalink / raw) To: Liming Sun Cc: Jaehoon Chung, Shawn Lin, Rob Herring, Mark Rutland, Kukjin Kim, Krzysztof Kozlowski, linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, Chris Metcalf [...] > Unfortunate patch 8 and 9 does *not* solve the fundamental problem > with multiple slots. You can't solve it without involving the mmc > core, as explained in earlier discussions [1]. > This link got lost, so here it is: [1] https://www.spinics.net/lists/linux-mmc/msg46549.html [...] Kind regards Uffe ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-10-25 16:50 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-06 19:21 [PATCH 0/9] Enable dw-mmc multi-card support Liming Sun 2017-10-06 19:21 ` [PATCH 1/9] Revert "Documentation: dw-mshc: deprecate num-slots" Liming Sun 2017-10-13 19:28 ` Rob Herring 2017-10-16 14:35 ` [PATCH 0/9] Enable dw-mmc multi-card support Liming Sun [not found] ` <cover.1507315433.git.lsun-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 2017-10-17 1:36 ` Shawn Lin 2017-10-17 15:52 ` Liming Sun 2017-10-20 14:06 ` Jaehoon Chung [not found] ` <f71d94dc-7277-8675-c21e-9c2ec77174e7-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> 2017-10-20 15:07 ` Liming Sun 2017-10-25 16:47 ` Ulf Hansson 2017-10-25 16:50 ` Ulf Hansson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).