* [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller @ 2018-07-12 3:02 ` Chen-Yu Tsai 0 siblings, 0 replies; 16+ messages in thread From: Chen-Yu Tsai @ 2018-07-12 3:02 UTC (permalink / raw) To: Ulf Hansson, Maxime Ripard Cc: Chen-Yu Tsai, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw The eMMC controller is also a new timing mode controller, but it doesn't have the timing mode switch. It does however have signal delay and calibration controls, typical of Allwinner MMC controllers that support the new timing mode. Enable the new timing mode setting for the A64 eMMC controller. This also enables MMC HS-DDR modes, which gives higher throughput for eMMC chips that support it, and can deliver such throughput. Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> --- drivers/mmc/host/sunxi-mmc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c index 8e7f3e35ee3d..13201bddfcc3 100644 --- a/drivers/mmc/host/sunxi-mmc.c +++ b/drivers/mmc/host/sunxi-mmc.c @@ -1166,6 +1166,7 @@ static const struct sunxi_mmc_cfg sun50i_a64_emmc_cfg = { .idma_des_size_bits = 13, .clk_delays = NULL, .can_calibrate = true, + .needs_new_timings = true, }; static const struct of_device_id sunxi_mmc_of_match[] = { -- 2.18.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller @ 2018-07-12 3:02 ` Chen-Yu Tsai 0 siblings, 0 replies; 16+ messages in thread From: Chen-Yu Tsai @ 2018-07-12 3:02 UTC (permalink / raw) To: linux-arm-kernel The eMMC controller is also a new timing mode controller, but it doesn't have the timing mode switch. It does however have signal delay and calibration controls, typical of Allwinner MMC controllers that support the new timing mode. Enable the new timing mode setting for the A64 eMMC controller. This also enables MMC HS-DDR modes, which gives higher throughput for eMMC chips that support it, and can deliver such throughput. Signed-off-by: Chen-Yu Tsai <wens@csie.org> --- drivers/mmc/host/sunxi-mmc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c index 8e7f3e35ee3d..13201bddfcc3 100644 --- a/drivers/mmc/host/sunxi-mmc.c +++ b/drivers/mmc/host/sunxi-mmc.c @@ -1166,6 +1166,7 @@ static const struct sunxi_mmc_cfg sun50i_a64_emmc_cfg = { .idma_des_size_bits = 13, .clk_delays = NULL, .can_calibrate = true, + .needs_new_timings = true, }; static const struct of_device_id sunxi_mmc_of_match[] = { -- 2.18.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
[parent not found: <20180712030225.15681-1-wens-jdAy2FN1RRM@public.gmane.org>]
* Re: [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller 2018-07-12 3:02 ` Chen-Yu Tsai @ 2018-07-12 7:19 ` Maxime Ripard -1 siblings, 0 replies; 16+ messages in thread From: Maxime Ripard @ 2018-07-12 7:19 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Ulf Hansson, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw [-- Attachment #1: Type: text/plain, Size: 1154 bytes --] Hi, On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: > The eMMC controller is also a new timing mode controller, but it doesn't > have the timing mode switch. It does however have signal delay and > calibration controls, typical of Allwinner MMC controllers that support > the new timing mode. > > Enable the new timing mode setting for the A64 eMMC controller. This > also enables MMC HS-DDR modes, which gives higher throughput for eMMC > chips that support it, and can deliver such throughput. > > Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> That doesn't look right. The datasheet explicitly mentions that this bit doesn't apply to the eMMC controller, and the BSP is doing the same: https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c vs https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c And I definitely remember having HS-DDR working back when I added the a64 eMMC support. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller @ 2018-07-12 7:19 ` Maxime Ripard 0 siblings, 0 replies; 16+ messages in thread From: Maxime Ripard @ 2018-07-12 7:19 UTC (permalink / raw) To: linux-arm-kernel Hi, On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: > The eMMC controller is also a new timing mode controller, but it doesn't > have the timing mode switch. It does however have signal delay and > calibration controls, typical of Allwinner MMC controllers that support > the new timing mode. > > Enable the new timing mode setting for the A64 eMMC controller. This > also enables MMC HS-DDR modes, which gives higher throughput for eMMC > chips that support it, and can deliver such throughput. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> That doesn't look right. The datasheet explicitly mentions that this bit doesn't apply to the eMMC controller, and the BSP is doing the same: https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c vs https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c And I definitely remember having HS-DDR working back when I added the a64 eMMC support. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180712/438c9ea8/attachment.sig> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller 2018-07-12 7:19 ` Maxime Ripard @ 2018-07-12 10:17 ` Chen-Yu Tsai -1 siblings, 0 replies; 16+ messages in thread From: Chen-Yu Tsai @ 2018-07-12 10:17 UTC (permalink / raw) To: Maxime Ripard Cc: Ulf Hansson, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel, linux-sunxi On Thu, Jul 12, 2018 at 3:19 PM, Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: > Hi, > > On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: >> The eMMC controller is also a new timing mode controller, but it doesn't >> have the timing mode switch. It does however have signal delay and >> calibration controls, typical of Allwinner MMC controllers that support >> the new timing mode. >> >> Enable the new timing mode setting for the A64 eMMC controller. This >> also enables MMC HS-DDR modes, which gives higher throughput for eMMC >> chips that support it, and can deliver such throughput. >> >> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> > > That doesn't look right. The datasheet explicitly mentions that this > bit doesn't apply to the eMMC controller, and the BSP is doing the same: > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c > > vs > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c You mean the bit in SDXC_REG_SD_NTSR? Yes I know that doesn't exist for the eMMC controller. I mentioned this in the commit message. It doesn't exist, and writes to it become a no-op. Would a comment, or comments, help with making this clear? > And I definitely remember having HS-DDR working back when I added the > a64 eMMC support. Well it doesn't at the moment. My BPI-M64 reports: [ 1.634276] mmc2: new high speed MMC card at address 0001 And with the patch: [ 1.632552] mmc2: new DDR MMC card at address 0001 Regards ChenYu ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller @ 2018-07-12 10:17 ` Chen-Yu Tsai 0 siblings, 0 replies; 16+ messages in thread From: Chen-Yu Tsai @ 2018-07-12 10:17 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 12, 2018 at 3:19 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > Hi, > > On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: >> The eMMC controller is also a new timing mode controller, but it doesn't >> have the timing mode switch. It does however have signal delay and >> calibration controls, typical of Allwinner MMC controllers that support >> the new timing mode. >> >> Enable the new timing mode setting for the A64 eMMC controller. This >> also enables MMC HS-DDR modes, which gives higher throughput for eMMC >> chips that support it, and can deliver such throughput. >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > That doesn't look right. The datasheet explicitly mentions that this > bit doesn't apply to the eMMC controller, and the BSP is doing the same: > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c > > vs > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c You mean the bit in SDXC_REG_SD_NTSR? Yes I know that doesn't exist for the eMMC controller. I mentioned this in the commit message. It doesn't exist, and writes to it become a no-op. Would a comment, or comments, help with making this clear? > And I definitely remember having HS-DDR working back when I added the > a64 eMMC support. Well it doesn't at the moment. My BPI-M64 reports: [ 1.634276] mmc2: new high speed MMC card at address 0001 And with the patch: [ 1.632552] mmc2: new DDR MMC card at address 0001 Regards ChenYu ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <CAGb2v65rE2nQbyeiSPAyTzw-n-EQsUwQABx3nrvXTu4JogFPBQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller 2018-07-12 10:17 ` Chen-Yu Tsai @ 2018-07-17 15:15 ` Maxime Ripard -1 siblings, 0 replies; 16+ messages in thread From: Maxime Ripard @ 2018-07-17 15:15 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Ulf Hansson, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 1760 bytes --] On Thu, Jul 12, 2018 at 06:17:23PM +0800, Chen-Yu Tsai wrote: > On Thu, Jul 12, 2018 at 3:19 PM, Maxime Ripard > <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: > > Hi, > > > > On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: > >> The eMMC controller is also a new timing mode controller, but it doesn't > >> have the timing mode switch. It does however have signal delay and > >> calibration controls, typical of Allwinner MMC controllers that support > >> the new timing mode. > >> > >> Enable the new timing mode setting for the A64 eMMC controller. This > >> also enables MMC HS-DDR modes, which gives higher throughput for eMMC > >> chips that support it, and can deliver such throughput. > >> > >> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> > > > > That doesn't look right. The datasheet explicitly mentions that this > > bit doesn't apply to the eMMC controller, and the BSP is doing the same: > > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c > > > > vs > > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c > > You mean the bit in SDXC_REG_SD_NTSR? Yes I know that doesn't exist > for the eMMC controller. I mentioned this in the commit message. It > doesn't exist, and writes to it become a no-op. > > Would a comment, or comments, help with making this clear? Ah right. Maybe we should move the calibration under can_calibrate though, or create another boolean for this? Putting it under has_new_timings while the SoC doesn't use it looks very confusing. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller @ 2018-07-17 15:15 ` Maxime Ripard 0 siblings, 0 replies; 16+ messages in thread From: Maxime Ripard @ 2018-07-17 15:15 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 12, 2018 at 06:17:23PM +0800, Chen-Yu Tsai wrote: > On Thu, Jul 12, 2018 at 3:19 PM, Maxime Ripard > <maxime.ripard@bootlin.com> wrote: > > Hi, > > > > On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: > >> The eMMC controller is also a new timing mode controller, but it doesn't > >> have the timing mode switch. It does however have signal delay and > >> calibration controls, typical of Allwinner MMC controllers that support > >> the new timing mode. > >> > >> Enable the new timing mode setting for the A64 eMMC controller. This > >> also enables MMC HS-DDR modes, which gives higher throughput for eMMC > >> chips that support it, and can deliver such throughput. > >> > >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > > > That doesn't look right. The datasheet explicitly mentions that this > > bit doesn't apply to the eMMC controller, and the BSP is doing the same: > > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c > > > > vs > > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c > > You mean the bit in SDXC_REG_SD_NTSR? Yes I know that doesn't exist > for the eMMC controller. I mentioned this in the commit message. It > doesn't exist, and writes to it become a no-op. > > Would a comment, or comments, help with making this clear? Ah right. Maybe we should move the calibration under can_calibrate though, or create another boolean for this? Putting it under has_new_timings while the SoC doesn't use it looks very confusing. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180717/b9e9ebd4/attachment.sig> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller 2018-07-17 15:15 ` Maxime Ripard @ 2018-07-17 15:43 ` Chen-Yu Tsai -1 siblings, 0 replies; 16+ messages in thread From: Chen-Yu Tsai @ 2018-07-17 15:43 UTC (permalink / raw) To: Maxime Ripard Cc: Ulf Hansson, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel, linux-sunxi On Tue, Jul 17, 2018 at 11:15 PM, Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: > On Thu, Jul 12, 2018 at 06:17:23PM +0800, Chen-Yu Tsai wrote: >> On Thu, Jul 12, 2018 at 3:19 PM, Maxime Ripard >> <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: >> > Hi, >> > >> > On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: >> >> The eMMC controller is also a new timing mode controller, but it doesn't >> >> have the timing mode switch. It does however have signal delay and >> >> calibration controls, typical of Allwinner MMC controllers that support >> >> the new timing mode. >> >> >> >> Enable the new timing mode setting for the A64 eMMC controller. This >> >> also enables MMC HS-DDR modes, which gives higher throughput for eMMC >> >> chips that support it, and can deliver such throughput. >> >> >> >> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> >> > >> > That doesn't look right. The datasheet explicitly mentions that this >> > bit doesn't apply to the eMMC controller, and the BSP is doing the same: >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c >> > >> > vs >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c >> >> You mean the bit in SDXC_REG_SD_NTSR? Yes I know that doesn't exist >> for the eMMC controller. I mentioned this in the commit message. It >> doesn't exist, and writes to it become a no-op. >> >> Would a comment, or comments, help with making this clear? > > Ah right. Maybe we should move the calibration under can_calibrate > though, or create another boolean for this? > > Putting it under has_new_timings while the SoC doesn't use it looks > very confusing. IIRC we don't support calibration anyway. This boolean simply signals the usage of the new timing mode, whether by choice, or because it is the only mode the controller supports. ChenYu ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller @ 2018-07-17 15:43 ` Chen-Yu Tsai 0 siblings, 0 replies; 16+ messages in thread From: Chen-Yu Tsai @ 2018-07-17 15:43 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jul 17, 2018 at 11:15 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > On Thu, Jul 12, 2018 at 06:17:23PM +0800, Chen-Yu Tsai wrote: >> On Thu, Jul 12, 2018 at 3:19 PM, Maxime Ripard >> <maxime.ripard@bootlin.com> wrote: >> > Hi, >> > >> > On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: >> >> The eMMC controller is also a new timing mode controller, but it doesn't >> >> have the timing mode switch. It does however have signal delay and >> >> calibration controls, typical of Allwinner MMC controllers that support >> >> the new timing mode. >> >> >> >> Enable the new timing mode setting for the A64 eMMC controller. This >> >> also enables MMC HS-DDR modes, which gives higher throughput for eMMC >> >> chips that support it, and can deliver such throughput. >> >> >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> > >> > That doesn't look right. The datasheet explicitly mentions that this >> > bit doesn't apply to the eMMC controller, and the BSP is doing the same: >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c >> > >> > vs >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c >> >> You mean the bit in SDXC_REG_SD_NTSR? Yes I know that doesn't exist >> for the eMMC controller. I mentioned this in the commit message. It >> doesn't exist, and writes to it become a no-op. >> >> Would a comment, or comments, help with making this clear? > > Ah right. Maybe we should move the calibration under can_calibrate > though, or create another boolean for this? > > Putting it under has_new_timings while the SoC doesn't use it looks > very confusing. IIRC we don't support calibration anyway. This boolean simply signals the usage of the new timing mode, whether by choice, or because it is the only mode the controller supports. ChenYu ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <CAGb2v64NRVaWpbDqiX0JX7_6uvypUpNEecPUj-sSjaK5X-hdQA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller 2018-07-17 15:43 ` Chen-Yu Tsai @ 2018-07-18 15:22 ` Maxime Ripard -1 siblings, 0 replies; 16+ messages in thread From: Maxime Ripard @ 2018-07-18 15:22 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Ulf Hansson, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 2479 bytes --] On Tue, Jul 17, 2018 at 11:43:03PM +0800, Chen-Yu Tsai wrote: > On Tue, Jul 17, 2018 at 11:15 PM, Maxime Ripard > <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: > > On Thu, Jul 12, 2018 at 06:17:23PM +0800, Chen-Yu Tsai wrote: > >> On Thu, Jul 12, 2018 at 3:19 PM, Maxime Ripard > >> <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: > >> > Hi, > >> > > >> > On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: > >> >> The eMMC controller is also a new timing mode controller, but it doesn't > >> >> have the timing mode switch. It does however have signal delay and > >> >> calibration controls, typical of Allwinner MMC controllers that support > >> >> the new timing mode. > >> >> > >> >> Enable the new timing mode setting for the A64 eMMC controller. This > >> >> also enables MMC HS-DDR modes, which gives higher throughput for eMMC > >> >> chips that support it, and can deliver such throughput. > >> >> > >> >> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> > >> > > >> > That doesn't look right. The datasheet explicitly mentions that this > >> > bit doesn't apply to the eMMC controller, and the BSP is doing the same: > >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c > >> > > >> > vs > >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c > >> > >> You mean the bit in SDXC_REG_SD_NTSR? Yes I know that doesn't exist > >> for the eMMC controller. I mentioned this in the commit message. It > >> doesn't exist, and writes to it become a no-op. > >> > >> Would a comment, or comments, help with making this clear? > > > > Ah right. Maybe we should move the calibration under can_calibrate > > though, or create another boolean for this? > > > > Putting it under has_new_timings while the SoC doesn't use it looks > > very confusing. > > IIRC we don't support calibration anyway. This boolean simply signals > the usage of the new timing mode, whether by choice, or because it is > the only mode the controller supports. This is not the semantic I had in mind when I introduced it. The original intent was to set the new timing bit all the time for SoCs. If we want to change that semantic, then we also need to make sure what this bit means is documented properly. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller @ 2018-07-18 15:22 ` Maxime Ripard 0 siblings, 0 replies; 16+ messages in thread From: Maxime Ripard @ 2018-07-18 15:22 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jul 17, 2018 at 11:43:03PM +0800, Chen-Yu Tsai wrote: > On Tue, Jul 17, 2018 at 11:15 PM, Maxime Ripard > <maxime.ripard@bootlin.com> wrote: > > On Thu, Jul 12, 2018 at 06:17:23PM +0800, Chen-Yu Tsai wrote: > >> On Thu, Jul 12, 2018 at 3:19 PM, Maxime Ripard > >> <maxime.ripard@bootlin.com> wrote: > >> > Hi, > >> > > >> > On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: > >> >> The eMMC controller is also a new timing mode controller, but it doesn't > >> >> have the timing mode switch. It does however have signal delay and > >> >> calibration controls, typical of Allwinner MMC controllers that support > >> >> the new timing mode. > >> >> > >> >> Enable the new timing mode setting for the A64 eMMC controller. This > >> >> also enables MMC HS-DDR modes, which gives higher throughput for eMMC > >> >> chips that support it, and can deliver such throughput. > >> >> > >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > >> > > >> > That doesn't look right. The datasheet explicitly mentions that this > >> > bit doesn't apply to the eMMC controller, and the BSP is doing the same: > >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c > >> > > >> > vs > >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c > >> > >> You mean the bit in SDXC_REG_SD_NTSR? Yes I know that doesn't exist > >> for the eMMC controller. I mentioned this in the commit message. It > >> doesn't exist, and writes to it become a no-op. > >> > >> Would a comment, or comments, help with making this clear? > > > > Ah right. Maybe we should move the calibration under can_calibrate > > though, or create another boolean for this? > > > > Putting it under has_new_timings while the SoC doesn't use it looks > > very confusing. > > IIRC we don't support calibration anyway. This boolean simply signals > the usage of the new timing mode, whether by choice, or because it is > the only mode the controller supports. This is not the semantic I had in mind when I introduced it. The original intent was to set the new timing bit all the time for SoCs. If we want to change that semantic, then we also need to make sure what this bit means is documented properly. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180718/a0d51ed7/attachment.sig> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller 2018-07-18 15:22 ` Maxime Ripard @ 2018-07-30 9:27 ` Chen-Yu Tsai -1 siblings, 0 replies; 16+ messages in thread From: Chen-Yu Tsai @ 2018-07-30 9:27 UTC (permalink / raw) To: Maxime Ripard Cc: Ulf Hansson, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel, linux-sunxi On Wed, Jul 18, 2018 at 11:22 PM, Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: > On Tue, Jul 17, 2018 at 11:43:03PM +0800, Chen-Yu Tsai wrote: >> On Tue, Jul 17, 2018 at 11:15 PM, Maxime Ripard >> <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: >> > On Thu, Jul 12, 2018 at 06:17:23PM +0800, Chen-Yu Tsai wrote: >> >> On Thu, Jul 12, 2018 at 3:19 PM, Maxime Ripard >> >> <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: >> >> > Hi, >> >> > >> >> > On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: >> >> >> The eMMC controller is also a new timing mode controller, but it doesn't >> >> >> have the timing mode switch. It does however have signal delay and >> >> >> calibration controls, typical of Allwinner MMC controllers that support >> >> >> the new timing mode. >> >> >> >> >> >> Enable the new timing mode setting for the A64 eMMC controller. This >> >> >> also enables MMC HS-DDR modes, which gives higher throughput for eMMC >> >> >> chips that support it, and can deliver such throughput. >> >> >> >> >> >> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> >> >> > >> >> > That doesn't look right. The datasheet explicitly mentions that this >> >> > bit doesn't apply to the eMMC controller, and the BSP is doing the same: >> >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c >> >> > >> >> > vs >> >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c >> >> >> >> You mean the bit in SDXC_REG_SD_NTSR? Yes I know that doesn't exist >> >> for the eMMC controller. I mentioned this in the commit message. It >> >> doesn't exist, and writes to it become a no-op. >> >> >> >> Would a comment, or comments, help with making this clear? >> > >> > Ah right. Maybe we should move the calibration under can_calibrate >> > though, or create another boolean for this? >> > >> > Putting it under has_new_timings while the SoC doesn't use it looks >> > very confusing. >> >> IIRC we don't support calibration anyway. This boolean simply signals >> the usage of the new timing mode, whether by choice, or because it is >> the only mode the controller supports. > > This is not the semantic I had in mind when I introduced it. The > original intent was to set the new timing bit all the time for > SoCs. If we want to change that semantic, then we also need to make > sure what this bit means is documented properly. In the driver: /* hardware only supports new timing mode */ bool needs_new_timings; /* hardware can switch between old and new timing modes */ bool has_timings_switch; So if the A64 / H6 eMMC controller only supports / is stuck with the new timing mode, that surely fits the description of the first one, right? As for setting the new timing bit all the time, yes it is set, but it's a no-op. Would a comment clarifying this at the point the hardware bit is set suffice? Thanks ChenYu ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller @ 2018-07-30 9:27 ` Chen-Yu Tsai 0 siblings, 0 replies; 16+ messages in thread From: Chen-Yu Tsai @ 2018-07-30 9:27 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jul 18, 2018 at 11:22 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > On Tue, Jul 17, 2018 at 11:43:03PM +0800, Chen-Yu Tsai wrote: >> On Tue, Jul 17, 2018 at 11:15 PM, Maxime Ripard >> <maxime.ripard@bootlin.com> wrote: >> > On Thu, Jul 12, 2018 at 06:17:23PM +0800, Chen-Yu Tsai wrote: >> >> On Thu, Jul 12, 2018 at 3:19 PM, Maxime Ripard >> >> <maxime.ripard@bootlin.com> wrote: >> >> > Hi, >> >> > >> >> > On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: >> >> >> The eMMC controller is also a new timing mode controller, but it doesn't >> >> >> have the timing mode switch. It does however have signal delay and >> >> >> calibration controls, typical of Allwinner MMC controllers that support >> >> >> the new timing mode. >> >> >> >> >> >> Enable the new timing mode setting for the A64 eMMC controller. This >> >> >> also enables MMC HS-DDR modes, which gives higher throughput for eMMC >> >> >> chips that support it, and can deliver such throughput. >> >> >> >> >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> >> > >> >> > That doesn't look right. The datasheet explicitly mentions that this >> >> > bit doesn't apply to the eMMC controller, and the BSP is doing the same: >> >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c >> >> > >> >> > vs >> >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c >> >> >> >> You mean the bit in SDXC_REG_SD_NTSR? Yes I know that doesn't exist >> >> for the eMMC controller. I mentioned this in the commit message. It >> >> doesn't exist, and writes to it become a no-op. >> >> >> >> Would a comment, or comments, help with making this clear? >> > >> > Ah right. Maybe we should move the calibration under can_calibrate >> > though, or create another boolean for this? >> > >> > Putting it under has_new_timings while the SoC doesn't use it looks >> > very confusing. >> >> IIRC we don't support calibration anyway. This boolean simply signals >> the usage of the new timing mode, whether by choice, or because it is >> the only mode the controller supports. > > This is not the semantic I had in mind when I introduced it. The > original intent was to set the new timing bit all the time for > SoCs. If we want to change that semantic, then we also need to make > sure what this bit means is documented properly. In the driver: /* hardware only supports new timing mode */ bool needs_new_timings; /* hardware can switch between old and new timing modes */ bool has_timings_switch; So if the A64 / H6 eMMC controller only supports / is stuck with the new timing mode, that surely fits the description of the first one, right? As for setting the new timing bit all the time, yes it is set, but it's a no-op. Would a comment clarifying this at the point the hardware bit is set suffice? Thanks ChenYu ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <CAGb2v67XaWoBGX2Tn_J8LfBJiJja0yfRbM+QyHPLnXvQtBuGGw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller 2018-07-30 9:27 ` Chen-Yu Tsai @ 2018-07-31 14:19 ` Maxime Ripard -1 siblings, 0 replies; 16+ messages in thread From: Maxime Ripard @ 2018-07-31 14:19 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Ulf Hansson, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 3351 bytes --] On Mon, Jul 30, 2018 at 05:27:43PM +0800, Chen-Yu Tsai wrote: > On Wed, Jul 18, 2018 at 11:22 PM, Maxime Ripard > <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: > > On Tue, Jul 17, 2018 at 11:43:03PM +0800, Chen-Yu Tsai wrote: > >> On Tue, Jul 17, 2018 at 11:15 PM, Maxime Ripard > >> <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: > >> > On Thu, Jul 12, 2018 at 06:17:23PM +0800, Chen-Yu Tsai wrote: > >> >> On Thu, Jul 12, 2018 at 3:19 PM, Maxime Ripard > >> >> <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: > >> >> > Hi, > >> >> > > >> >> > On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: > >> >> >> The eMMC controller is also a new timing mode controller, but it doesn't > >> >> >> have the timing mode switch. It does however have signal delay and > >> >> >> calibration controls, typical of Allwinner MMC controllers that support > >> >> >> the new timing mode. > >> >> >> > >> >> >> Enable the new timing mode setting for the A64 eMMC controller. This > >> >> >> also enables MMC HS-DDR modes, which gives higher throughput for eMMC > >> >> >> chips that support it, and can deliver such throughput. > >> >> >> > >> >> >> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> > >> >> > > >> >> > That doesn't look right. The datasheet explicitly mentions that this > >> >> > bit doesn't apply to the eMMC controller, and the BSP is doing the same: > >> >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c > >> >> > > >> >> > vs > >> >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c > >> >> > >> >> You mean the bit in SDXC_REG_SD_NTSR? Yes I know that doesn't exist > >> >> for the eMMC controller. I mentioned this in the commit message. It > >> >> doesn't exist, and writes to it become a no-op. > >> >> > >> >> Would a comment, or comments, help with making this clear? > >> > > >> > Ah right. Maybe we should move the calibration under can_calibrate > >> > though, or create another boolean for this? > >> > > >> > Putting it under has_new_timings while the SoC doesn't use it looks > >> > very confusing. > >> > >> IIRC we don't support calibration anyway. This boolean simply signals > >> the usage of the new timing mode, whether by choice, or because it is > >> the only mode the controller supports. > > > > This is not the semantic I had in mind when I introduced it. The > > original intent was to set the new timing bit all the time for > > SoCs. If we want to change that semantic, then we also need to make > > sure what this bit means is documented properly. > > In the driver: > > /* hardware only supports new timing mode */ > bool needs_new_timings; > > /* hardware can switch between old and new timing modes */ > bool has_timings_switch; > > So if the A64 / H6 eMMC controller only supports / is stuck with the new > timing mode, that surely fits the description of the first one, right? I guess so, yes > As for setting the new timing bit all the time, yes it is set, but it's > a no-op. Would a comment clarifying this at the point the hardware bit > is set suffice? Yep. Thanks! Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller @ 2018-07-31 14:19 ` Maxime Ripard 0 siblings, 0 replies; 16+ messages in thread From: Maxime Ripard @ 2018-07-31 14:19 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 30, 2018 at 05:27:43PM +0800, Chen-Yu Tsai wrote: > On Wed, Jul 18, 2018 at 11:22 PM, Maxime Ripard > <maxime.ripard@bootlin.com> wrote: > > On Tue, Jul 17, 2018 at 11:43:03PM +0800, Chen-Yu Tsai wrote: > >> On Tue, Jul 17, 2018 at 11:15 PM, Maxime Ripard > >> <maxime.ripard@bootlin.com> wrote: > >> > On Thu, Jul 12, 2018 at 06:17:23PM +0800, Chen-Yu Tsai wrote: > >> >> On Thu, Jul 12, 2018 at 3:19 PM, Maxime Ripard > >> >> <maxime.ripard@bootlin.com> wrote: > >> >> > Hi, > >> >> > > >> >> > On Thu, Jul 12, 2018 at 11:02:25AM +0800, Chen-Yu Tsai wrote: > >> >> >> The eMMC controller is also a new timing mode controller, but it doesn't > >> >> >> have the timing mode switch. It does however have signal delay and > >> >> >> calibration controls, typical of Allwinner MMC controllers that support > >> >> >> the new timing mode. > >> >> >> > >> >> >> Enable the new timing mode setting for the A64 eMMC controller. This > >> >> >> also enables MMC HS-DDR modes, which gives higher throughput for eMMC > >> >> >> chips that support it, and can deliver such throughput. > >> >> >> > >> >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > >> >> > > >> >> > That doesn't look right. The datasheet explicitly mentions that this > >> >> > bit doesn't apply to the eMMC controller, and the BSP is doing the same: > >> >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-1.c > >> >> > > >> >> > vs > >> >> > https://github.com/longsleep/linux-pine64/blob/lichee-dev-v3.10.65/drivers/mmc/host/sunxi-mmc-sun50iw1p1-2.c > >> >> > >> >> You mean the bit in SDXC_REG_SD_NTSR? Yes I know that doesn't exist > >> >> for the eMMC controller. I mentioned this in the commit message. It > >> >> doesn't exist, and writes to it become a no-op. > >> >> > >> >> Would a comment, or comments, help with making this clear? > >> > > >> > Ah right. Maybe we should move the calibration under can_calibrate > >> > though, or create another boolean for this? > >> > > >> > Putting it under has_new_timings while the SoC doesn't use it looks > >> > very confusing. > >> > >> IIRC we don't support calibration anyway. This boolean simply signals > >> the usage of the new timing mode, whether by choice, or because it is > >> the only mode the controller supports. > > > > This is not the semantic I had in mind when I introduced it. The > > original intent was to set the new timing bit all the time for > > SoCs. If we want to change that semantic, then we also need to make > > sure what this bit means is documented properly. > > In the driver: > > /* hardware only supports new timing mode */ > bool needs_new_timings; > > /* hardware can switch between old and new timing modes */ > bool has_timings_switch; > > So if the A64 / H6 eMMC controller only supports / is stuck with the new > timing mode, that surely fits the description of the first one, right? I guess so, yes > As for setting the new timing bit all the time, yes it is set, but it's > a no-op. Would a comment clarifying this at the point the hardware bit > is set suffice? Yep. Thanks! Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180731/67794c1d/attachment.sig> ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-07-31 14:19 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-12 3:02 [PATCH] mmc: sunxi: Use new timing mode for A64 eMMC controller Chen-Yu Tsai 2018-07-12 3:02 ` Chen-Yu Tsai [not found] ` <20180712030225.15681-1-wens-jdAy2FN1RRM@public.gmane.org> 2018-07-12 7:19 ` Maxime Ripard 2018-07-12 7:19 ` Maxime Ripard 2018-07-12 10:17 ` Chen-Yu Tsai 2018-07-12 10:17 ` Chen-Yu Tsai [not found] ` <CAGb2v65rE2nQbyeiSPAyTzw-n-EQsUwQABx3nrvXTu4JogFPBQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-07-17 15:15 ` Maxime Ripard 2018-07-17 15:15 ` Maxime Ripard 2018-07-17 15:43 ` Chen-Yu Tsai 2018-07-17 15:43 ` Chen-Yu Tsai [not found] ` <CAGb2v64NRVaWpbDqiX0JX7_6uvypUpNEecPUj-sSjaK5X-hdQA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-07-18 15:22 ` Maxime Ripard 2018-07-18 15:22 ` Maxime Ripard 2018-07-30 9:27 ` Chen-Yu Tsai 2018-07-30 9:27 ` Chen-Yu Tsai [not found] ` <CAGb2v67XaWoBGX2Tn_J8LfBJiJja0yfRbM+QyHPLnXvQtBuGGw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-07-31 14:19 ` Maxime Ripard 2018-07-31 14:19 ` Maxime Ripard
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.