* [PATCH v2 1/2] Documentation: mmc: add description for power-delay-ms @ 2018-04-23 8:10 Shawn Lin 2018-04-23 8:10 ` [PATCH v2 2/2] mmc: core: add tunable delay waiting for power to be stable Shawn Lin 2018-04-27 18:54 ` [PATCH v2 1/2] Documentation: mmc: add description for power-delay-ms Rob Herring 0 siblings, 2 replies; 8+ messages in thread From: Shawn Lin @ 2018-04-23 8:10 UTC (permalink / raw) To: Ulf Hansson Cc: Rob Herring, Jaehoon Chung, Wolfram Sang, Adrian Hunter, linux-mmc, devicetree, Shawn Lin power-delay-ms woule be used to substitute the hard-coded 10ms delay waiting for power supply to be stable, specificed by individual platform/board. Default to 10ms as before, if no available. Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- Changes in v2: None Documentation/devicetree/bindings/mmc/mmc.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt index 467cd7b..8db2ba0 100644 --- a/Documentation/devicetree/bindings/mmc/mmc.txt +++ b/Documentation/devicetree/bindings/mmc/mmc.txt @@ -56,6 +56,8 @@ Optional properties: - fixed-emmc-driver-type: for non-removable eMMC, enforce this driver type. The value <n> is the driver type as specified in the eMMC specification (table 206 in spec version 5.1). +- power-delay-ms: Tunable delay waiting for I/O signalling and card power supply + to be stable. Default to 10ms if no available. *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line polarity properties, we have to fix the meaning of the "normal" and "inverted" -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] mmc: core: add tunable delay waiting for power to be stable 2018-04-23 8:10 [PATCH v2 1/2] Documentation: mmc: add description for power-delay-ms Shawn Lin @ 2018-04-23 8:10 ` Shawn Lin 2018-04-23 8:36 ` Adrian Hunter 2018-04-27 18:54 ` [PATCH v2 1/2] Documentation: mmc: add description for power-delay-ms Rob Herring 1 sibling, 1 reply; 8+ messages in thread From: Shawn Lin @ 2018-04-23 8:10 UTC (permalink / raw) To: Ulf Hansson Cc: Rob Herring, Jaehoon Chung, Wolfram Sang, Adrian Hunter, linux-mmc, devicetree, Shawn Lin The hard-coded 10ms delay in mmc_power_up came from commit 79bccc5aefb4 ("mmc: increase power up delay"), which said "The TI controller on Toshiba Tecra M5 needs more time to power up or the cards will init incorrectly or not at all." But it's too engineering solution for a special board but force all platforms to wait for that long time, especially painful for mmc_power_up for eMMC when booting. However, it's added since 2009, and we can't tell if other platforms benefit from it. But in practise, the modern hardware are most likely to have a stable power supply with 1ms after setting it for no matter PMIC or discrete power. And more importnatly, most regulators implement the callback of ->set_voltage_time_sel() for regulator core to wait for specific period of time for the power supply to be stable, which means once regulator_set_voltage_* return, the power should reach the the minimum voltage that works for initialization. Of course, if there are some other ways for host to power the card, we should allow them to argue a suitable delay as well. With this patch, we could assign the delay from firmware, or we could assigne it via ->set_ios() callback from host drivers. Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- Changes in v2: - allow to assign zero delay from firmware suggested by Adrain drivers/mmc/core/core.c | 4 ++-- drivers/mmc/core/host.c | 4 ++++ include/linux/mmc/host.h | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 121ce50..a52e9c2 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1658,7 +1658,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr) * This delay should be sufficient to allow the power supply * to reach the minimum voltage. */ - mmc_delay(10); + mmc_delay(host->ios.power_delay_ms); mmc_pwrseq_post_power_on(host); @@ -1671,7 +1671,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr) * This delay must be at least 74 clock sizes, or 1 ms, or the * time required to reach a stable voltage. */ - mmc_delay(10); + mmc_delay(host->ios.power_delay_ms); } void mmc_power_off(struct mmc_host *host) diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c index 64b03d6..00ddc15 100644 --- a/drivers/mmc/core/host.c +++ b/drivers/mmc/core/host.c @@ -338,6 +338,9 @@ int mmc_of_parse(struct mmc_host *host) host->dsr_req = 0; } + device_property_read_u32(dev, "power-delay-ms", + &host->ios.power_delay_ms); + return mmc_pwrseq_alloc(host); } @@ -403,6 +406,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev) host->max_blk_count = PAGE_SIZE / 512; host->fixed_drv_type = -EINVAL; + host->ios.power_delay_ms = 10; return host; } diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 7c6eaf6..efa9bab 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -22,6 +22,7 @@ struct mmc_ios { unsigned int clock; /* clock rate */ unsigned short vdd; + unsigned int power_delay_ms; /* waiting for stable power */ /* vdd stores the bit number of the selected voltage range from below. */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] mmc: core: add tunable delay waiting for power to be stable 2018-04-23 8:10 ` [PATCH v2 2/2] mmc: core: add tunable delay waiting for power to be stable Shawn Lin @ 2018-04-23 8:36 ` Adrian Hunter 0 siblings, 0 replies; 8+ messages in thread From: Adrian Hunter @ 2018-04-23 8:36 UTC (permalink / raw) To: Shawn Lin, Ulf Hansson Cc: Rob Herring, Jaehoon Chung, Wolfram Sang, linux-mmc, devicetree On 23/04/18 11:10, Shawn Lin wrote: > The hard-coded 10ms delay in mmc_power_up came from > commit 79bccc5aefb4 ("mmc: increase power up delay"), which said "The TI > controller on Toshiba Tecra M5 needs more time to power up or the cards > will init incorrectly or not at all." But it's too engineering solution > for a special board but force all platforms to wait for that long time, > especially painful for mmc_power_up for eMMC when booting. > > However, it's added since 2009, and we can't tell if other platforms > benefit from it. But in practise, the modern hardware are most likely to > have a stable power supply with 1ms after setting it for no matter PMIC > or discrete power. And more importnatly, most regulators implement the > callback of ->set_voltage_time_sel() for regulator core to wait for > specific period of time for the power supply to be stable, which means > once regulator_set_voltage_* return, the power should reach the the > minimum voltage that works for initialization. Of course, if there > are some other ways for host to power the card, we should allow them > to argue a suitable delay as well. > > With this patch, we could assign the delay from firmware, or we could > assigne it via ->set_ios() callback from host drivers. > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> > > --- > > Changes in v2: > - allow to assign zero delay from firmware suggested by Adrain > > drivers/mmc/core/core.c | 4 ++-- > drivers/mmc/core/host.c | 4 ++++ > include/linux/mmc/host.h | 1 + > 3 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 121ce50..a52e9c2 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -1658,7 +1658,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr) > * This delay should be sufficient to allow the power supply > * to reach the minimum voltage. > */ > - mmc_delay(10); > + mmc_delay(host->ios.power_delay_ms); > > mmc_pwrseq_post_power_on(host); > > @@ -1671,7 +1671,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr) > * This delay must be at least 74 clock sizes, or 1 ms, or the > * time required to reach a stable voltage. > */ > - mmc_delay(10); > + mmc_delay(host->ios.power_delay_ms); AFAICT mmc_delay() (calls usleep_range()) does not handle a zero value correctly so maybe make mmc_delay() check for zero and return immediately in that case. > } > > void mmc_power_off(struct mmc_host *host) > diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c > index 64b03d6..00ddc15 100644 > --- a/drivers/mmc/core/host.c > +++ b/drivers/mmc/core/host.c > @@ -338,6 +338,9 @@ int mmc_of_parse(struct mmc_host *host) > host->dsr_req = 0; > } > > + device_property_read_u32(dev, "power-delay-ms", > + &host->ios.power_delay_ms); > + > return mmc_pwrseq_alloc(host); > } > > @@ -403,6 +406,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev) > host->max_blk_count = PAGE_SIZE / 512; > > host->fixed_drv_type = -EINVAL; > + host->ios.power_delay_ms = 10; > > return host; > } > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h > index 7c6eaf6..efa9bab 100644 > --- a/include/linux/mmc/host.h > +++ b/include/linux/mmc/host.h > @@ -22,6 +22,7 @@ > struct mmc_ios { > unsigned int clock; /* clock rate */ > unsigned short vdd; > + unsigned int power_delay_ms; /* waiting for stable power */ > > /* vdd stores the bit number of the selected voltage range from below. */ > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] Documentation: mmc: add description for power-delay-ms 2018-04-23 8:10 [PATCH v2 1/2] Documentation: mmc: add description for power-delay-ms Shawn Lin 2018-04-23 8:10 ` [PATCH v2 2/2] mmc: core: add tunable delay waiting for power to be stable Shawn Lin @ 2018-04-27 18:54 ` Rob Herring 2018-04-28 1:11 ` Shawn Lin 1 sibling, 1 reply; 8+ messages in thread From: Rob Herring @ 2018-04-27 18:54 UTC (permalink / raw) To: Shawn Lin Cc: Ulf Hansson, Jaehoon Chung, Wolfram Sang, Adrian Hunter, linux-mmc, devicetree On Mon, Apr 23, 2018 at 04:10:29PM +0800, Shawn Lin wrote: > power-delay-ms woule be used to substitute the hard-coded 10ms > delay waiting for power supply to be stable, specificed by individual > platform/board. Default to 10ms as before, if no available. > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> > --- > > Changes in v2: None > > Documentation/devicetree/bindings/mmc/mmc.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt > index 467cd7b..8db2ba0 100644 > --- a/Documentation/devicetree/bindings/mmc/mmc.txt > +++ b/Documentation/devicetree/bindings/mmc/mmc.txt > @@ -56,6 +56,8 @@ Optional properties: > - fixed-emmc-driver-type: for non-removable eMMC, enforce this driver type. > The value <n> is the driver type as specified in the eMMC specification > (table 206 in spec version 5.1). > +- power-delay-ms: Tunable delay waiting for I/O signalling and card power supply > + to be stable. Default to 10ms if no available. How long do you need? Would changing the default to say 20ms work and still be short enough others don't care? Can we use the same property as the mmc pwrseq binding defines: post-power-on-delay-ms Rob ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] Documentation: mmc: add description for power-delay-ms 2018-04-27 18:54 ` [PATCH v2 1/2] Documentation: mmc: add description for power-delay-ms Rob Herring @ 2018-04-28 1:11 ` Shawn Lin 2018-05-02 12:46 ` Ulf Hansson 0 siblings, 1 reply; 8+ messages in thread From: Shawn Lin @ 2018-04-28 1:11 UTC (permalink / raw) To: Rob Herring, Ulf Hansson Cc: shawn.lin, Jaehoon Chung, Wolfram Sang, Adrian Hunter, linux-mmc, devicetree Hi Rob and Ulf, On 2018/4/28 2:54, Rob Herring wrote: > On Mon, Apr 23, 2018 at 04:10:29PM +0800, Shawn Lin wrote: ... >> +- power-delay-ms: Tunable delay waiting for I/O signalling and card power supply >> + to be stable. Default to 10ms if no available. > > How long do you need? Would changing the default to say 20ms work and > still be short enough others don't care? It varies from board-2-board, but my boards only need 2ms or less. The hard-coded 10ms was increased from 2ms since 2009, just for fixing one board. And nobody complained 10ms is too short(including me), so it's fine for everybody probably. However, nobody complained it's toooo long as well, expect me, so my best guess is either others don't care it at all, or haven't noticed it. So the intention for this patch, is to save the unnecessary waste for the boot-time sensitive environment, by reducing the delay but don't break anything else. > > Can we use the same property as the mmc pwrseq binding defines: > post-power-on-delay-ms I'm fine with using post-power-on-delay-ms, but it depends on whether using pwrseq_simple. So I need add parsing it in two places for different prupose. Is it ok, or better idea? > > Rob > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] Documentation: mmc: add description for power-delay-ms 2018-04-28 1:11 ` Shawn Lin @ 2018-05-02 12:46 ` Ulf Hansson 2018-05-03 7:26 ` Shawn Lin 0 siblings, 1 reply; 8+ messages in thread From: Ulf Hansson @ 2018-05-02 12:46 UTC (permalink / raw) To: Shawn Lin Cc: Rob Herring, Jaehoon Chung, Wolfram Sang, Adrian Hunter, linux-mmc, devicetree On 28 April 2018 at 03:11, Shawn Lin <shawn.lin@rock-chips.com> wrote: > Hi Rob and Ulf, > > On 2018/4/28 2:54, Rob Herring wrote: >> >> On Mon, Apr 23, 2018 at 04:10:29PM +0800, Shawn Lin wrote: > > > ... > >>> +- power-delay-ms: Tunable delay waiting for I/O signalling and card >>> power supply >>> + to be stable. Default to 10ms if no available. >> >> >> How long do you need? Would changing the default to say 20ms work and >> still be short enough others don't care? > > > It varies from board-2-board, but my boards only need 2ms or less. The > hard-coded 10ms was increased from 2ms since 2009, just for fixing one > board. And nobody complained 10ms is too short(including me), so it's > fine for everybody probably. However, nobody complained it's toooo long > as well, expect me, so my best guess is either others don't care it at > all, or haven't noticed it. So the intention for this patch, is to save > the unnecessary waste for the boot-time sensitive environment, by > reducing the delay but don't break anything else. > > >> >> Can we use the same property as the mmc pwrseq binding defines: >> post-power-on-delay-ms > > > I'm fine with using post-power-on-delay-ms, but it depends on whether > using pwrseq_simple. So I need add parsing it in two places for > different prupose. Is it ok, or better idea? I don't think the parsing is an issue, but that we need to allow two different descriptions of the same property name may be a bit confusing. I would rather keep them separate, but I have no strong onion. Kind regards Uffe ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] Documentation: mmc: add description for power-delay-ms 2018-05-02 12:46 ` Ulf Hansson @ 2018-05-03 7:26 ` Shawn Lin 2018-05-07 13:46 ` Rob Herring 0 siblings, 1 reply; 8+ messages in thread From: Shawn Lin @ 2018-05-03 7:26 UTC (permalink / raw) To: Ulf Hansson, Rob Herring Cc: shawn.lin, Jaehoon Chung, Wolfram Sang, Adrian Hunter, linux-mmc, devicetree On 2018/5/2 20:46, Ulf Hansson wrote: > On 28 April 2018 at 03:11, Shawn Lin <shawn.lin@rock-chips.com> wrote: >> Hi Rob and Ulf, >> >> On 2018/4/28 2:54, Rob Herring wrote: >>> >>> On Mon, Apr 23, 2018 at 04:10:29PM +0800, Shawn Lin wrote: >> >> >> ... >> >>>> +- power-delay-ms: Tunable delay waiting for I/O signalling and card >>>> power supply >>>> + to be stable. Default to 10ms if no available. >>> >>> >>> How long do you need? Would changing the default to say 20ms work and >>> still be short enough others don't care? >> >> >> It varies from board-2-board, but my boards only need 2ms or less. The >> hard-coded 10ms was increased from 2ms since 2009, just for fixing one >> board. And nobody complained 10ms is too short(including me), so it's >> fine for everybody probably. However, nobody complained it's toooo long >> as well, expect me, so my best guess is either others don't care it at >> all, or haven't noticed it. So the intention for this patch, is to save >> the unnecessary waste for the boot-time sensitive environment, by >> reducing the delay but don't break anything else. >> >> >>> >>> Can we use the same property as the mmc pwrseq binding defines: >>> post-power-on-delay-ms >> >> >> I'm fine with using post-power-on-delay-ms, but it depends on whether >> using pwrseq_simple. So I need add parsing it in two places for >> different prupose. Is it ok, or better idea? > > I don't think the parsing is an issue, but that we need to allow two > different descriptions of the same property name may be a bit > confusing. > I would rather keep them separate, but I have no strong onion. > I also would like to keep the separate. Rob, any suggestion? :) > Kind regards > Uffe > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] Documentation: mmc: add description for power-delay-ms 2018-05-03 7:26 ` Shawn Lin @ 2018-05-07 13:46 ` Rob Herring 0 siblings, 0 replies; 8+ messages in thread From: Rob Herring @ 2018-05-07 13:46 UTC (permalink / raw) To: Shawn Lin Cc: Ulf Hansson, Jaehoon Chung, Wolfram Sang, Adrian Hunter, linux-mmc, devicetree On Thu, May 3, 2018 at 2:26 AM, Shawn Lin <shawn.lin@rock-chips.com> wrote: > On 2018/5/2 20:46, Ulf Hansson wrote: >> >> On 28 April 2018 at 03:11, Shawn Lin <shawn.lin@rock-chips.com> wrote: >>> >>> Hi Rob and Ulf, >>> >>> On 2018/4/28 2:54, Rob Herring wrote: >>>> >>>> >>>> On Mon, Apr 23, 2018 at 04:10:29PM +0800, Shawn Lin wrote: >>> >>> >>> >>> ... >>> >>>>> +- power-delay-ms: Tunable delay waiting for I/O signalling and card >>>>> power supply >>>>> + to be stable. Default to 10ms if no available. >>>> >>>> >>>> >>>> How long do you need? Would changing the default to say 20ms work and >>>> still be short enough others don't care? >>> >>> >>> >>> It varies from board-2-board, but my boards only need 2ms or less. The >>> hard-coded 10ms was increased from 2ms since 2009, just for fixing one >>> board. And nobody complained 10ms is too short(including me), so it's >>> fine for everybody probably. However, nobody complained it's toooo long >>> as well, expect me, so my best guess is either others don't care it at >>> all, or haven't noticed it. So the intention for this patch, is to save >>> the unnecessary waste for the boot-time sensitive environment, by >>> reducing the delay but don't break anything else. >>> >>> >>>> >>>> Can we use the same property as the mmc pwrseq binding defines: >>>> post-power-on-delay-ms >>> >>> >>> >>> I'm fine with using post-power-on-delay-ms, but it depends on whether >>> using pwrseq_simple. So I need add parsing it in two places for >>> different prupose. Is it ok, or better idea? >> >> >> I don't think the parsing is an issue, but that we need to allow two >> different descriptions of the same property name may be a bit >> confusing. > > >> I would rather keep them separate, but I have no strong onion. >> > > I also would like to keep the separate. Rob, any suggestion? :) I think it is fine to describe it in 2 places. It just needs to have the same definition in terms of what happens before and after it. Rob ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-05-07 13:46 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-23 8:10 [PATCH v2 1/2] Documentation: mmc: add description for power-delay-ms Shawn Lin 2018-04-23 8:10 ` [PATCH v2 2/2] mmc: core: add tunable delay waiting for power to be stable Shawn Lin 2018-04-23 8:36 ` Adrian Hunter 2018-04-27 18:54 ` [PATCH v2 1/2] Documentation: mmc: add description for power-delay-ms Rob Herring 2018-04-28 1:11 ` Shawn Lin 2018-05-02 12:46 ` Ulf Hansson 2018-05-03 7:26 ` Shawn Lin 2018-05-07 13:46 ` Rob Herring
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.