[v2,00/40] Tegra SDHCI add support for HS200 and UHS signaling
mbox series

Message ID 1533924522-1037-1-git-send-email-avienamo@nvidia.com
Headers show
Series
  • Tegra SDHCI add support for HS200 and UHS signaling
Related show

Message

Aapo Vienamo Aug. 10, 2018, 6:08 p.m. UTC
Hi all,

This series implements support for faster signaling modes on Tegra
SDHCI controllers. This series consist of several parts: changes
requried for 1.8 V signaling and pad control, pad calibration, and
tuning. Following earlies patch sets have been merged into this
larger set: "Tegra PMC pinctrl pad configuration", "Tegra SDHCI enable
1.8 V signaling on Tegar210 and Tegra186", "Tegra SDHCI update the
padautocal procedure". Also the patches for enabling SDHCI tuning
are added.

Changelog:
v2:
	- Fix grammar in PMC device tree bindings docs
	- Remove a stray line from tegra sdhci bindings
	- Cosmetic changes to PMC pinctrl driver
	- Fix a typo in "soc/tegra: pmc: Implement
	  tegra_io_pad_is_powered()" commit message
	- Declare mask and value on the same line in
	  tegra_io_pad_is_powered()
	- Move the call to tegra_sdhci_is_pad_and_regulator_valid() to
	  inside the if condition in tegra_sdhci_reset()
	- Use usleep_range() in tegra_sdhci_configure_cal_pad()
	- Move sdhci_writel() out of the enable if-else body in
	  tegra_sdhci_configure_cal_pad()
	- Add a delay before starting polling in
	  tegra_sdhci_pad_autocalib()
	- Use usleep_range() in tegra_sdhci_set_tap()
	- Rename orig_enabled to status in
	  tegra_sdhci_configure_card_clk()
	- Fix if condition wrapping alignment in tegra_sdhci_set_tap()

v1:
	- Probe the regulator voltage capabilities to determine whether pinctrl
	  is needed in tegra_sdhci_r eset
	- Don't remove tegra_sdhci_voltage_switch()
	- Use dev_warn() in tegra_sdhci_init_pinctrl_info()
	- Don't change start_signal_voltage_switch callback if pinctrl info
	  invalid
	- Only call udelay(1) on enable in tegra_sdhci_configure_cal_pad()
	- Add nvidia, prefix to pad autocal offset dt props in the example

See the original patch sets for earlier changelogs.

Aapo Vienamo (40):
  dt-bindings: Add Tegra PMC pad configuration bindings
  dt-bindings: mmc: tegra: Add pad voltage control properties
  dt-bindings: Add Tegra SDHCI pad pdpu offset bindings
  dt-bindings: mmc: Add Tegra SDHCI sampling trimmer values
  soc/tegra: pmc: Fix pad voltage configuration for Tegra186
  soc/tegra: pmc: Factor out DPD register bit calculation
  soc/tegra: pmc: Implement tegra_io_pad_is_powered()
  soc/tegra: pmc: Use X macro to generate IO pad tables
  soc/tegra: pmc: Remove public pad voltage APIs
  soc/tegra: pmc: Implement pad configuration via pinctrl
  mmc: sdhci: Add a quirk to skip clearing the transfer mode register on
    tuning
  mmc: tegra: Reconfigure pad voltages during voltage switching
  mmc: tegra: Poll for calibration completion
  mmc: tegra: Set calibration pad voltage reference
  mmc: tegra: Power on the calibration pad
  mmc: tegra: Disable card clock during pad calibration
  mmc: tegra: Program pad autocal offsets from dt
  mmc: tegra: Perform pad calibration after voltage switch
  mmc: tegra: Enable pad calibration on Tegra210 and Tegra186
  mmc: tegra: Add a workaround for tap value change glitch
  mmc: tegra: Parse default trim and tap from dt
  mmc: tegra: Configure default tap values
  mmc: tegra: Configure default trim value on reset
  mmc: tegra: Use standard SDHCI tuning on Tegra210 and Tegra186
  mmc: sdhci: Add a quirk to disable card clock during tuning
  mmc: tegra: Enable workaround for tuning transfer mode bug
  mmc: tegra: Set SDHCI_QUIRK2_TUNE_DIS_CARD_CLK on Tegra210
  mmc: tegra: Enable UHS and HS200 modes for Tegra210
  mmc: tegra: Enable UHS and HS200 modes for Tegra186
  arm64: dts: Add Tegra210 sdmmc pinctrl voltage states
  arm64: dts: Add Tegra186 sdmmc pinctrl voltage states
  arm64: dts: tegra210-p2180: Allow ldo2 to go down to 1.8 V
  arm64: dts: tegra210-p2180: Correct sdmmc4 vqmmc-supply
  arm64: dts: tegra210-p2597: Remove no-1-8-v from sdmmc1
  arm64: dts: tegra186: Add sdmmc pad auto calibration offsets
  arm64: dts: tegra210: Add sdmmc pad auto calibration offsets
  arm64: dts: tegra210: Add SDHCI tap and trim values
  arm64: dts: tegra186: Add SDHCI tap and trim values
  arm64: dts: tegra186: Assign clocks for sdmmc1 and sdmmc4
  arm64: dts: tegra210: Assign clocks for sdmmc1 and sdmmc4

 .../bindings/arm/tegra/nvidia,tegra186-pmc.txt     |  93 ++++
 .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      | 103 ++++
 .../bindings/mmc/nvidia,tegra20-sdhci.txt          |  68 +++
 arch/arm64/boot/dts/nvidia/tegra186.dtsi           |  74 +++
 arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi     |  12 +-
 arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi     |   1 -
 arch/arm64/boot/dts/nvidia/tegra210.dtsi           |  55 ++
 drivers/mmc/host/sdhci-tegra.c                     | 556 +++++++++++++++++++--
 drivers/mmc/host/sdhci.c                           |  21 +
 drivers/mmc/host/sdhci.h                           |   4 +
 drivers/soc/tegra/pmc.c                            | 511 ++++++++++++++-----
 include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h |  18 +
 include/soc/tegra/pmc.h                            |  20 +-
 13 files changed, 1324 insertions(+), 212 deletions(-)
 create mode 100644 include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h

Comments

Thierry Reding Aug. 23, 2018, 8:47 a.m. UTC | #1
On Fri, Aug 10, 2018 at 09:08:02PM +0300, Aapo Vienamo wrote:
> Hi all,
> 
> This series implements support for faster signaling modes on Tegra
> SDHCI controllers. This series consist of several parts: changes
> requried for 1.8 V signaling and pad control, pad calibration, and
> tuning. Following earlies patch sets have been merged into this
> larger set: "Tegra PMC pinctrl pad configuration", "Tegra SDHCI enable
> 1.8 V signaling on Tegar210 and Tegra186", "Tegra SDHCI update the
> padautocal procedure". Also the patches for enabling SDHCI tuning
> are added.
> 
> Changelog:
> v2:
> 	- Fix grammar in PMC device tree bindings docs
> 	- Remove a stray line from tegra sdhci bindings
> 	- Cosmetic changes to PMC pinctrl driver
> 	- Fix a typo in "soc/tegra: pmc: Implement
> 	  tegra_io_pad_is_powered()" commit message
> 	- Declare mask and value on the same line in
> 	  tegra_io_pad_is_powered()
> 	- Move the call to tegra_sdhci_is_pad_and_regulator_valid() to
> 	  inside the if condition in tegra_sdhci_reset()
> 	- Use usleep_range() in tegra_sdhci_configure_cal_pad()
> 	- Move sdhci_writel() out of the enable if-else body in
> 	  tegra_sdhci_configure_cal_pad()
> 	- Add a delay before starting polling in
> 	  tegra_sdhci_pad_autocalib()
> 	- Use usleep_range() in tegra_sdhci_set_tap()
> 	- Rename orig_enabled to status in
> 	  tegra_sdhci_configure_card_clk()
> 	- Fix if condition wrapping alignment in tegra_sdhci_set_tap()
> 
> v1:
> 	- Probe the regulator voltage capabilities to determine whether pinctrl
> 	  is needed in tegra_sdhci_r eset
> 	- Don't remove tegra_sdhci_voltage_switch()
> 	- Use dev_warn() in tegra_sdhci_init_pinctrl_info()
> 	- Don't change start_signal_voltage_switch callback if pinctrl info
> 	  invalid
> 	- Only call udelay(1) on enable in tegra_sdhci_configure_cal_pad()
> 	- Add nvidia, prefix to pad autocal offset dt props in the example
> 
> See the original patch sets for earlier changelogs.
> 
> Aapo Vienamo (40):
>   dt-bindings: Add Tegra PMC pad configuration bindings
>   dt-bindings: mmc: tegra: Add pad voltage control properties
>   dt-bindings: Add Tegra SDHCI pad pdpu offset bindings
>   dt-bindings: mmc: Add Tegra SDHCI sampling trimmer values
>   soc/tegra: pmc: Fix pad voltage configuration for Tegra186
>   soc/tegra: pmc: Factor out DPD register bit calculation
>   soc/tegra: pmc: Implement tegra_io_pad_is_powered()
>   soc/tegra: pmc: Use X macro to generate IO pad tables
>   soc/tegra: pmc: Remove public pad voltage APIs
>   soc/tegra: pmc: Implement pad configuration via pinctrl
>   mmc: sdhci: Add a quirk to skip clearing the transfer mode register on
>     tuning
>   mmc: tegra: Reconfigure pad voltages during voltage switching
>   mmc: tegra: Poll for calibration completion
>   mmc: tegra: Set calibration pad voltage reference
>   mmc: tegra: Power on the calibration pad
>   mmc: tegra: Disable card clock during pad calibration
>   mmc: tegra: Program pad autocal offsets from dt
>   mmc: tegra: Perform pad calibration after voltage switch
>   mmc: tegra: Enable pad calibration on Tegra210 and Tegra186
>   mmc: tegra: Add a workaround for tap value change glitch
>   mmc: tegra: Parse default trim and tap from dt
>   mmc: tegra: Configure default tap values
>   mmc: tegra: Configure default trim value on reset
>   mmc: tegra: Use standard SDHCI tuning on Tegra210 and Tegra186
>   mmc: sdhci: Add a quirk to disable card clock during tuning
>   mmc: tegra: Enable workaround for tuning transfer mode bug
>   mmc: tegra: Set SDHCI_QUIRK2_TUNE_DIS_CARD_CLK on Tegra210
>   mmc: tegra: Enable UHS and HS200 modes for Tegra210
>   mmc: tegra: Enable UHS and HS200 modes for Tegra186
>   arm64: dts: Add Tegra210 sdmmc pinctrl voltage states
>   arm64: dts: Add Tegra186 sdmmc pinctrl voltage states
>   arm64: dts: tegra210-p2180: Allow ldo2 to go down to 1.8 V
>   arm64: dts: tegra210-p2180: Correct sdmmc4 vqmmc-supply
>   arm64: dts: tegra210-p2597: Remove no-1-8-v from sdmmc1
>   arm64: dts: tegra186: Add sdmmc pad auto calibration offsets
>   arm64: dts: tegra210: Add sdmmc pad auto calibration offsets
>   arm64: dts: tegra210: Add SDHCI tap and trim values
>   arm64: dts: tegra186: Add SDHCI tap and trim values
>   arm64: dts: tegra186: Assign clocks for sdmmc1 and sdmmc4
>   arm64: dts: tegra210: Assign clocks for sdmmc1 and sdmmc4
> 
>  .../bindings/arm/tegra/nvidia,tegra186-pmc.txt     |  93 ++++
>  .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      | 103 ++++
>  .../bindings/mmc/nvidia,tegra20-sdhci.txt          |  68 +++
>  arch/arm64/boot/dts/nvidia/tegra186.dtsi           |  74 +++
>  arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi     |  12 +-
>  arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi     |   1 -
>  arch/arm64/boot/dts/nvidia/tegra210.dtsi           |  55 ++
>  drivers/mmc/host/sdhci-tegra.c                     | 556 +++++++++++++++++++--
>  drivers/mmc/host/sdhci.c                           |  21 +
>  drivers/mmc/host/sdhci.h                           |   4 +
>  drivers/soc/tegra/pmc.c                            | 511 ++++++++++++++-----
>  include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h |  18 +
>  include/soc/tegra/pmc.h                            |  20 +-
>  13 files changed, 1324 insertions(+), 212 deletions(-)
>  create mode 100644 include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h

Hi Ulf, Adrian,

I've been running some tests on this on various Tegra boards and this
all seems to work fine. I'm also pretty happy with how this turned out,
so the whole series is:

Acked-by: Thierry Reding <treding@nvidia.com>

In order to get this merged, I think it'd be best for you guys to pick
up the MMC patches, since they are all independent of the rest. There is
a runtime dependency on the PMC bits for the faster modes, but things
will fallback to the status quo if those bits are not enabled. The only
dependency here is between the PMC and DTS changes, which I usually pick
up into the Tegra tree anyway, so I can sort that out there.

Since I've already gone through the process of splitting up the series,
I can offer to rebase the MMC patches on top of v4.19-rc1 when it's out
and send a pull request for the MMC patches. Alternatively I can further
split that branch up into two parts, one with the two patches that touch
the core and another with the remainder, if that's what you prefer. Or
you can of course feel free to apply all of the MMC patches yourselves
with my above Acked-by if you want to.

Any thought on the above?

Thanks,
Thierry
Ulf Hansson Aug. 23, 2018, 10:42 a.m. UTC | #2
On 23 August 2018 at 10:47, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Fri, Aug 10, 2018 at 09:08:02PM +0300, Aapo Vienamo wrote:
>> Hi all,
>>
>> This series implements support for faster signaling modes on Tegra
>> SDHCI controllers. This series consist of several parts: changes
>> requried for 1.8 V signaling and pad control, pad calibration, and
>> tuning. Following earlies patch sets have been merged into this
>> larger set: "Tegra PMC pinctrl pad configuration", "Tegra SDHCI enable
>> 1.8 V signaling on Tegar210 and Tegra186", "Tegra SDHCI update the
>> padautocal procedure". Also the patches for enabling SDHCI tuning
>> are added.
>>
>> Changelog:
>> v2:
>>       - Fix grammar in PMC device tree bindings docs
>>       - Remove a stray line from tegra sdhci bindings
>>       - Cosmetic changes to PMC pinctrl driver
>>       - Fix a typo in "soc/tegra: pmc: Implement
>>         tegra_io_pad_is_powered()" commit message
>>       - Declare mask and value on the same line in
>>         tegra_io_pad_is_powered()
>>       - Move the call to tegra_sdhci_is_pad_and_regulator_valid() to
>>         inside the if condition in tegra_sdhci_reset()
>>       - Use usleep_range() in tegra_sdhci_configure_cal_pad()
>>       - Move sdhci_writel() out of the enable if-else body in
>>         tegra_sdhci_configure_cal_pad()
>>       - Add a delay before starting polling in
>>         tegra_sdhci_pad_autocalib()
>>       - Use usleep_range() in tegra_sdhci_set_tap()
>>       - Rename orig_enabled to status in
>>         tegra_sdhci_configure_card_clk()
>>       - Fix if condition wrapping alignment in tegra_sdhci_set_tap()
>>
>> v1:
>>       - Probe the regulator voltage capabilities to determine whether pinctrl
>>         is needed in tegra_sdhci_r eset
>>       - Don't remove tegra_sdhci_voltage_switch()
>>       - Use dev_warn() in tegra_sdhci_init_pinctrl_info()
>>       - Don't change start_signal_voltage_switch callback if pinctrl info
>>         invalid
>>       - Only call udelay(1) on enable in tegra_sdhci_configure_cal_pad()
>>       - Add nvidia, prefix to pad autocal offset dt props in the example
>>
>> See the original patch sets for earlier changelogs.
>>
>> Aapo Vienamo (40):
>>   dt-bindings: Add Tegra PMC pad configuration bindings
>>   dt-bindings: mmc: tegra: Add pad voltage control properties
>>   dt-bindings: Add Tegra SDHCI pad pdpu offset bindings
>>   dt-bindings: mmc: Add Tegra SDHCI sampling trimmer values
>>   soc/tegra: pmc: Fix pad voltage configuration for Tegra186
>>   soc/tegra: pmc: Factor out DPD register bit calculation
>>   soc/tegra: pmc: Implement tegra_io_pad_is_powered()
>>   soc/tegra: pmc: Use X macro to generate IO pad tables
>>   soc/tegra: pmc: Remove public pad voltage APIs
>>   soc/tegra: pmc: Implement pad configuration via pinctrl
>>   mmc: sdhci: Add a quirk to skip clearing the transfer mode register on
>>     tuning
>>   mmc: tegra: Reconfigure pad voltages during voltage switching
>>   mmc: tegra: Poll for calibration completion
>>   mmc: tegra: Set calibration pad voltage reference
>>   mmc: tegra: Power on the calibration pad
>>   mmc: tegra: Disable card clock during pad calibration
>>   mmc: tegra: Program pad autocal offsets from dt
>>   mmc: tegra: Perform pad calibration after voltage switch
>>   mmc: tegra: Enable pad calibration on Tegra210 and Tegra186
>>   mmc: tegra: Add a workaround for tap value change glitch
>>   mmc: tegra: Parse default trim and tap from dt
>>   mmc: tegra: Configure default tap values
>>   mmc: tegra: Configure default trim value on reset
>>   mmc: tegra: Use standard SDHCI tuning on Tegra210 and Tegra186
>>   mmc: sdhci: Add a quirk to disable card clock during tuning
>>   mmc: tegra: Enable workaround for tuning transfer mode bug
>>   mmc: tegra: Set SDHCI_QUIRK2_TUNE_DIS_CARD_CLK on Tegra210
>>   mmc: tegra: Enable UHS and HS200 modes for Tegra210
>>   mmc: tegra: Enable UHS and HS200 modes for Tegra186
>>   arm64: dts: Add Tegra210 sdmmc pinctrl voltage states
>>   arm64: dts: Add Tegra186 sdmmc pinctrl voltage states
>>   arm64: dts: tegra210-p2180: Allow ldo2 to go down to 1.8 V
>>   arm64: dts: tegra210-p2180: Correct sdmmc4 vqmmc-supply
>>   arm64: dts: tegra210-p2597: Remove no-1-8-v from sdmmc1
>>   arm64: dts: tegra186: Add sdmmc pad auto calibration offsets
>>   arm64: dts: tegra210: Add sdmmc pad auto calibration offsets
>>   arm64: dts: tegra210: Add SDHCI tap and trim values
>>   arm64: dts: tegra186: Add SDHCI tap and trim values
>>   arm64: dts: tegra186: Assign clocks for sdmmc1 and sdmmc4
>>   arm64: dts: tegra210: Assign clocks for sdmmc1 and sdmmc4
>>
>>  .../bindings/arm/tegra/nvidia,tegra186-pmc.txt     |  93 ++++
>>  .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      | 103 ++++
>>  .../bindings/mmc/nvidia,tegra20-sdhci.txt          |  68 +++
>>  arch/arm64/boot/dts/nvidia/tegra186.dtsi           |  74 +++
>>  arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi     |  12 +-
>>  arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi     |   1 -
>>  arch/arm64/boot/dts/nvidia/tegra210.dtsi           |  55 ++
>>  drivers/mmc/host/sdhci-tegra.c                     | 556 +++++++++++++++++++--
>>  drivers/mmc/host/sdhci.c                           |  21 +
>>  drivers/mmc/host/sdhci.h                           |   4 +
>>  drivers/soc/tegra/pmc.c                            | 511 ++++++++++++++-----
>>  include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h |  18 +
>>  include/soc/tegra/pmc.h                            |  20 +-
>>  13 files changed, 1324 insertions(+), 212 deletions(-)
>>  create mode 100644 include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h
>
> Hi Ulf, Adrian,
>
> I've been running some tests on this on various Tegra boards and this
> all seems to work fine. I'm also pretty happy with how this turned out,
> so the whole series is:
>
> Acked-by: Thierry Reding <treding@nvidia.com>
>
> In order to get this merged, I think it'd be best for you guys to pick
> up the MMC patches, since they are all independent of the rest. There is
> a runtime dependency on the PMC bits for the faster modes, but things
> will fallback to the status quo if those bits are not enabled. The only
> dependency here is between the PMC and DTS changes, which I usually pick
> up into the Tegra tree anyway, so I can sort that out there.

Great, thanks for clarifying the dependency.

>
> Since I've already gone through the process of splitting up the series,
> I can offer to rebase the MMC patches on top of v4.19-rc1 when it's out
> and send a pull request for the MMC patches.

That sounds convenient for me. Please also include the corresponding
dt doc changes for mmc, as I am normally queuing these via my mmc
tree.

Although, I think it's better to wait until Adrian has given his ack
for the series.

> Alternatively I can further
> split that branch up into two parts, one with the two patches that touch
> the core and another with the remainder, if that's what you prefer. Or
> you can of course feel free to apply all of the MMC patches yourselves
> with my above Acked-by if you want to.
>
> Any thought on the above?

Let's see how it goes, if there are any struggles, I don't have a
problem to cherry-pick the mmc parts of the series.

Kind regards
Uffe
Thierry Reding Aug. 27, 2018, 10:10 a.m. UTC | #3
On Fri, Aug 10, 2018 at 09:08:02PM +0300, Aapo Vienamo wrote:
> Hi all,
> 
> This series implements support for faster signaling modes on Tegra
> SDHCI controllers. This series consist of several parts: changes
> requried for 1.8 V signaling and pad control, pad calibration, and
> tuning. Following earlies patch sets have been merged into this
> larger set: "Tegra PMC pinctrl pad configuration", "Tegra SDHCI enable
> 1.8 V signaling on Tegar210 and Tegra186", "Tegra SDHCI update the
> padautocal procedure". Also the patches for enabling SDHCI tuning
> are added.
> 
> Changelog:
> v2:
> 	- Fix grammar in PMC device tree bindings docs
> 	- Remove a stray line from tegra sdhci bindings
> 	- Cosmetic changes to PMC pinctrl driver
> 	- Fix a typo in "soc/tegra: pmc: Implement
> 	  tegra_io_pad_is_powered()" commit message
> 	- Declare mask and value on the same line in
> 	  tegra_io_pad_is_powered()
> 	- Move the call to tegra_sdhci_is_pad_and_regulator_valid() to
> 	  inside the if condition in tegra_sdhci_reset()
> 	- Use usleep_range() in tegra_sdhci_configure_cal_pad()
> 	- Move sdhci_writel() out of the enable if-else body in
> 	  tegra_sdhci_configure_cal_pad()
> 	- Add a delay before starting polling in
> 	  tegra_sdhci_pad_autocalib()
> 	- Use usleep_range() in tegra_sdhci_set_tap()
> 	- Rename orig_enabled to status in
> 	  tegra_sdhci_configure_card_clk()
> 	- Fix if condition wrapping alignment in tegra_sdhci_set_tap()
> 
> v1:
> 	- Probe the regulator voltage capabilities to determine whether pinctrl
> 	  is needed in tegra_sdhci_r eset
> 	- Don't remove tegra_sdhci_voltage_switch()
> 	- Use dev_warn() in tegra_sdhci_init_pinctrl_info()
> 	- Don't change start_signal_voltage_switch callback if pinctrl info
> 	  invalid
> 	- Only call udelay(1) on enable in tegra_sdhci_configure_cal_pad()
> 	- Add nvidia, prefix to pad autocal offset dt props in the example
> 
> See the original patch sets for earlier changelogs.
> 
> Aapo Vienamo (40):
>   dt-bindings: Add Tegra PMC pad configuration bindings
>   dt-bindings: mmc: tegra: Add pad voltage control properties
>   dt-bindings: Add Tegra SDHCI pad pdpu offset bindings
>   dt-bindings: mmc: Add Tegra SDHCI sampling trimmer values
>   soc/tegra: pmc: Fix pad voltage configuration for Tegra186
>   soc/tegra: pmc: Factor out DPD register bit calculation
>   soc/tegra: pmc: Implement tegra_io_pad_is_powered()
>   soc/tegra: pmc: Use X macro to generate IO pad tables
>   soc/tegra: pmc: Remove public pad voltage APIs
>   soc/tegra: pmc: Implement pad configuration via pinctrl
>   mmc: sdhci: Add a quirk to skip clearing the transfer mode register on
>     tuning
>   mmc: tegra: Reconfigure pad voltages during voltage switching
>   mmc: tegra: Poll for calibration completion
>   mmc: tegra: Set calibration pad voltage reference
>   mmc: tegra: Power on the calibration pad
>   mmc: tegra: Disable card clock during pad calibration
>   mmc: tegra: Program pad autocal offsets from dt
>   mmc: tegra: Perform pad calibration after voltage switch
>   mmc: tegra: Enable pad calibration on Tegra210 and Tegra186
>   mmc: tegra: Add a workaround for tap value change glitch
>   mmc: tegra: Parse default trim and tap from dt
>   mmc: tegra: Configure default tap values
>   mmc: tegra: Configure default trim value on reset
>   mmc: tegra: Use standard SDHCI tuning on Tegra210 and Tegra186
>   mmc: sdhci: Add a quirk to disable card clock during tuning
>   mmc: tegra: Enable workaround for tuning transfer mode bug
>   mmc: tegra: Set SDHCI_QUIRK2_TUNE_DIS_CARD_CLK on Tegra210
>   mmc: tegra: Enable UHS and HS200 modes for Tegra210
>   mmc: tegra: Enable UHS and HS200 modes for Tegra186
>   arm64: dts: Add Tegra210 sdmmc pinctrl voltage states
>   arm64: dts: Add Tegra186 sdmmc pinctrl voltage states
>   arm64: dts: tegra210-p2180: Allow ldo2 to go down to 1.8 V
>   arm64: dts: tegra210-p2180: Correct sdmmc4 vqmmc-supply
>   arm64: dts: tegra210-p2597: Remove no-1-8-v from sdmmc1
>   arm64: dts: tegra186: Add sdmmc pad auto calibration offsets
>   arm64: dts: tegra210: Add sdmmc pad auto calibration offsets
>   arm64: dts: tegra210: Add SDHCI tap and trim values
>   arm64: dts: tegra186: Add SDHCI tap and trim values
>   arm64: dts: tegra186: Assign clocks for sdmmc1 and sdmmc4
>   arm64: dts: tegra210: Assign clocks for sdmmc1 and sdmmc4
> 
>  .../bindings/arm/tegra/nvidia,tegra186-pmc.txt     |  93 ++++
>  .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      | 103 ++++
>  .../bindings/mmc/nvidia,tegra20-sdhci.txt          |  68 +++
>  arch/arm64/boot/dts/nvidia/tegra186.dtsi           |  74 +++
>  arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi     |  12 +-
>  arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi     |   1 -
>  arch/arm64/boot/dts/nvidia/tegra210.dtsi           |  55 ++
>  drivers/mmc/host/sdhci-tegra.c                     | 556 +++++++++++++++++++--
>  drivers/mmc/host/sdhci.c                           |  21 +
>  drivers/mmc/host/sdhci.h                           |   4 +
>  drivers/soc/tegra/pmc.c                            | 511 ++++++++++++++-----
>  include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h |  18 +
>  include/soc/tegra/pmc.h                            |  20 +-
>  13 files changed, 1324 insertions(+), 212 deletions(-)
>  create mode 100644 include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h

Adrian,

these patches are also a prerequisite for the UHS signalling changes in
the other two series that Aapo posted. It'd be great if you could take a
quick look and weigh in.

Thanks,
Thierry
Adrian Hunter Aug. 27, 2018, 10:26 a.m. UTC | #4
On 27/08/18 13:10, Thierry Reding wrote:
> On Fri, Aug 10, 2018 at 09:08:02PM +0300, Aapo Vienamo wrote:
>> Hi all,
>>
>> This series implements support for faster signaling modes on Tegra
>> SDHCI controllers. This series consist of several parts: changes
>> requried for 1.8 V signaling and pad control, pad calibration, and
>> tuning. Following earlies patch sets have been merged into this
>> larger set: "Tegra PMC pinctrl pad configuration", "Tegra SDHCI enable
>> 1.8 V signaling on Tegar210 and Tegra186", "Tegra SDHCI update the
>> padautocal procedure". Also the patches for enabling SDHCI tuning
>> are added.
>>
>> Changelog:
>> v2:
>> 	- Fix grammar in PMC device tree bindings docs
>> 	- Remove a stray line from tegra sdhci bindings
>> 	- Cosmetic changes to PMC pinctrl driver
>> 	- Fix a typo in "soc/tegra: pmc: Implement
>> 	  tegra_io_pad_is_powered()" commit message
>> 	- Declare mask and value on the same line in
>> 	  tegra_io_pad_is_powered()
>> 	- Move the call to tegra_sdhci_is_pad_and_regulator_valid() to
>> 	  inside the if condition in tegra_sdhci_reset()
>> 	- Use usleep_range() in tegra_sdhci_configure_cal_pad()
>> 	- Move sdhci_writel() out of the enable if-else body in
>> 	  tegra_sdhci_configure_cal_pad()
>> 	- Add a delay before starting polling in
>> 	  tegra_sdhci_pad_autocalib()
>> 	- Use usleep_range() in tegra_sdhci_set_tap()
>> 	- Rename orig_enabled to status in
>> 	  tegra_sdhci_configure_card_clk()
>> 	- Fix if condition wrapping alignment in tegra_sdhci_set_tap()
>>
>> v1:
>> 	- Probe the regulator voltage capabilities to determine whether pinctrl
>> 	  is needed in tegra_sdhci_r eset
>> 	- Don't remove tegra_sdhci_voltage_switch()
>> 	- Use dev_warn() in tegra_sdhci_init_pinctrl_info()
>> 	- Don't change start_signal_voltage_switch callback if pinctrl info
>> 	  invalid
>> 	- Only call udelay(1) on enable in tegra_sdhci_configure_cal_pad()
>> 	- Add nvidia, prefix to pad autocal offset dt props in the example
>>
>> See the original patch sets for earlier changelogs.
>>
>> Aapo Vienamo (40):
>>   dt-bindings: Add Tegra PMC pad configuration bindings
>>   dt-bindings: mmc: tegra: Add pad voltage control properties
>>   dt-bindings: Add Tegra SDHCI pad pdpu offset bindings
>>   dt-bindings: mmc: Add Tegra SDHCI sampling trimmer values
>>   soc/tegra: pmc: Fix pad voltage configuration for Tegra186
>>   soc/tegra: pmc: Factor out DPD register bit calculation
>>   soc/tegra: pmc: Implement tegra_io_pad_is_powered()
>>   soc/tegra: pmc: Use X macro to generate IO pad tables
>>   soc/tegra: pmc: Remove public pad voltage APIs
>>   soc/tegra: pmc: Implement pad configuration via pinctrl
>>   mmc: sdhci: Add a quirk to skip clearing the transfer mode register on
>>     tuning
>>   mmc: tegra: Reconfigure pad voltages during voltage switching
>>   mmc: tegra: Poll for calibration completion
>>   mmc: tegra: Set calibration pad voltage reference
>>   mmc: tegra: Power on the calibration pad
>>   mmc: tegra: Disable card clock during pad calibration
>>   mmc: tegra: Program pad autocal offsets from dt
>>   mmc: tegra: Perform pad calibration after voltage switch
>>   mmc: tegra: Enable pad calibration on Tegra210 and Tegra186
>>   mmc: tegra: Add a workaround for tap value change glitch
>>   mmc: tegra: Parse default trim and tap from dt
>>   mmc: tegra: Configure default tap values
>>   mmc: tegra: Configure default trim value on reset
>>   mmc: tegra: Use standard SDHCI tuning on Tegra210 and Tegra186
>>   mmc: sdhci: Add a quirk to disable card clock during tuning
>>   mmc: tegra: Enable workaround for tuning transfer mode bug
>>   mmc: tegra: Set SDHCI_QUIRK2_TUNE_DIS_CARD_CLK on Tegra210
>>   mmc: tegra: Enable UHS and HS200 modes for Tegra210
>>   mmc: tegra: Enable UHS and HS200 modes for Tegra186
>>   arm64: dts: Add Tegra210 sdmmc pinctrl voltage states
>>   arm64: dts: Add Tegra186 sdmmc pinctrl voltage states
>>   arm64: dts: tegra210-p2180: Allow ldo2 to go down to 1.8 V
>>   arm64: dts: tegra210-p2180: Correct sdmmc4 vqmmc-supply
>>   arm64: dts: tegra210-p2597: Remove no-1-8-v from sdmmc1
>>   arm64: dts: tegra186: Add sdmmc pad auto calibration offsets
>>   arm64: dts: tegra210: Add sdmmc pad auto calibration offsets
>>   arm64: dts: tegra210: Add SDHCI tap and trim values
>>   arm64: dts: tegra186: Add SDHCI tap and trim values
>>   arm64: dts: tegra186: Assign clocks for sdmmc1 and sdmmc4
>>   arm64: dts: tegra210: Assign clocks for sdmmc1 and sdmmc4
>>
>>  .../bindings/arm/tegra/nvidia,tegra186-pmc.txt     |  93 ++++
>>  .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      | 103 ++++
>>  .../bindings/mmc/nvidia,tegra20-sdhci.txt          |  68 +++
>>  arch/arm64/boot/dts/nvidia/tegra186.dtsi           |  74 +++
>>  arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi     |  12 +-
>>  arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi     |   1 -
>>  arch/arm64/boot/dts/nvidia/tegra210.dtsi           |  55 ++
>>  drivers/mmc/host/sdhci-tegra.c                     | 556 +++++++++++++++++++--
>>  drivers/mmc/host/sdhci.c                           |  21 +
>>  drivers/mmc/host/sdhci.h                           |   4 +
>>  drivers/soc/tegra/pmc.c                            | 511 ++++++++++++++-----
>>  include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h |  18 +
>>  include/soc/tegra/pmc.h                            |  20 +-
>>  13 files changed, 1324 insertions(+), 212 deletions(-)
>>  create mode 100644 include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h
> 
> Adrian,
> 
> these patches are also a prerequisite for the UHS signalling changes in
> the other two series that Aapo posted. It'd be great if you could take a
> quick look and weigh in.

Sure, I'll try and have a look today.
Adrian Hunter Aug. 27, 2018, 11:43 a.m. UTC | #5
On 27/08/18 13:26, Adrian Hunter wrote:
> On 27/08/18 13:10, Thierry Reding wrote:
>> On Fri, Aug 10, 2018 at 09:08:02PM +0300, Aapo Vienamo wrote:
>>> Hi all,
>>>
>>> This series implements support for faster signaling modes on Tegra
>>> SDHCI controllers. This series consist of several parts: changes
>>> requried for 1.8 V signaling and pad control, pad calibration, and
>>> tuning. Following earlies patch sets have been merged into this
>>> larger set: "Tegra PMC pinctrl pad configuration", "Tegra SDHCI enable
>>> 1.8 V signaling on Tegar210 and Tegra186", "Tegra SDHCI update the
>>> padautocal procedure". Also the patches for enabling SDHCI tuning
>>> are added.
>>>
>>> Changelog:
>>> v2:
>>> 	- Fix grammar in PMC device tree bindings docs
>>> 	- Remove a stray line from tegra sdhci bindings
>>> 	- Cosmetic changes to PMC pinctrl driver
>>> 	- Fix a typo in "soc/tegra: pmc: Implement
>>> 	  tegra_io_pad_is_powered()" commit message
>>> 	- Declare mask and value on the same line in
>>> 	  tegra_io_pad_is_powered()
>>> 	- Move the call to tegra_sdhci_is_pad_and_regulator_valid() to
>>> 	  inside the if condition in tegra_sdhci_reset()
>>> 	- Use usleep_range() in tegra_sdhci_configure_cal_pad()
>>> 	- Move sdhci_writel() out of the enable if-else body in
>>> 	  tegra_sdhci_configure_cal_pad()
>>> 	- Add a delay before starting polling in
>>> 	  tegra_sdhci_pad_autocalib()
>>> 	- Use usleep_range() in tegra_sdhci_set_tap()
>>> 	- Rename orig_enabled to status in
>>> 	  tegra_sdhci_configure_card_clk()
>>> 	- Fix if condition wrapping alignment in tegra_sdhci_set_tap()
>>>
>>> v1:
>>> 	- Probe the regulator voltage capabilities to determine whether pinctrl
>>> 	  is needed in tegra_sdhci_r eset
>>> 	- Don't remove tegra_sdhci_voltage_switch()
>>> 	- Use dev_warn() in tegra_sdhci_init_pinctrl_info()
>>> 	- Don't change start_signal_voltage_switch callback if pinctrl info
>>> 	  invalid
>>> 	- Only call udelay(1) on enable in tegra_sdhci_configure_cal_pad()
>>> 	- Add nvidia, prefix to pad autocal offset dt props in the example
>>>
>>> See the original patch sets for earlier changelogs.
>>>
>>> Aapo Vienamo (40):
>>>   dt-bindings: Add Tegra PMC pad configuration bindings
>>>   dt-bindings: mmc: tegra: Add pad voltage control properties
>>>   dt-bindings: Add Tegra SDHCI pad pdpu offset bindings
>>>   dt-bindings: mmc: Add Tegra SDHCI sampling trimmer values
>>>   soc/tegra: pmc: Fix pad voltage configuration for Tegra186
>>>   soc/tegra: pmc: Factor out DPD register bit calculation
>>>   soc/tegra: pmc: Implement tegra_io_pad_is_powered()
>>>   soc/tegra: pmc: Use X macro to generate IO pad tables
>>>   soc/tegra: pmc: Remove public pad voltage APIs
>>>   soc/tegra: pmc: Implement pad configuration via pinctrl
>>>   mmc: sdhci: Add a quirk to skip clearing the transfer mode register on
>>>     tuning
>>>   mmc: tegra: Reconfigure pad voltages during voltage switching
>>>   mmc: tegra: Poll for calibration completion
>>>   mmc: tegra: Set calibration pad voltage reference
>>>   mmc: tegra: Power on the calibration pad
>>>   mmc: tegra: Disable card clock during pad calibration
>>>   mmc: tegra: Program pad autocal offsets from dt
>>>   mmc: tegra: Perform pad calibration after voltage switch
>>>   mmc: tegra: Enable pad calibration on Tegra210 and Tegra186
>>>   mmc: tegra: Add a workaround for tap value change glitch
>>>   mmc: tegra: Parse default trim and tap from dt
>>>   mmc: tegra: Configure default tap values
>>>   mmc: tegra: Configure default trim value on reset
>>>   mmc: tegra: Use standard SDHCI tuning on Tegra210 and Tegra186
>>>   mmc: sdhci: Add a quirk to disable card clock during tuning
>>>   mmc: tegra: Enable workaround for tuning transfer mode bug
>>>   mmc: tegra: Set SDHCI_QUIRK2_TUNE_DIS_CARD_CLK on Tegra210
>>>   mmc: tegra: Enable UHS and HS200 modes for Tegra210
>>>   mmc: tegra: Enable UHS and HS200 modes for Tegra186
>>>   arm64: dts: Add Tegra210 sdmmc pinctrl voltage states
>>>   arm64: dts: Add Tegra186 sdmmc pinctrl voltage states
>>>   arm64: dts: tegra210-p2180: Allow ldo2 to go down to 1.8 V
>>>   arm64: dts: tegra210-p2180: Correct sdmmc4 vqmmc-supply
>>>   arm64: dts: tegra210-p2597: Remove no-1-8-v from sdmmc1
>>>   arm64: dts: tegra186: Add sdmmc pad auto calibration offsets
>>>   arm64: dts: tegra210: Add sdmmc pad auto calibration offsets
>>>   arm64: dts: tegra210: Add SDHCI tap and trim values
>>>   arm64: dts: tegra186: Add SDHCI tap and trim values
>>>   arm64: dts: tegra186: Assign clocks for sdmmc1 and sdmmc4
>>>   arm64: dts: tegra210: Assign clocks for sdmmc1 and sdmmc4
>>>
>>>  .../bindings/arm/tegra/nvidia,tegra186-pmc.txt     |  93 ++++
>>>  .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      | 103 ++++
>>>  .../bindings/mmc/nvidia,tegra20-sdhci.txt          |  68 +++
>>>  arch/arm64/boot/dts/nvidia/tegra186.dtsi           |  74 +++
>>>  arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi     |  12 +-
>>>  arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi     |   1 -
>>>  arch/arm64/boot/dts/nvidia/tegra210.dtsi           |  55 ++
>>>  drivers/mmc/host/sdhci-tegra.c                     | 556 +++++++++++++++++++--
>>>  drivers/mmc/host/sdhci.c                           |  21 +
>>>  drivers/mmc/host/sdhci.h                           |   4 +
>>>  drivers/soc/tegra/pmc.c                            | 511 ++++++++++++++-----
>>>  include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h |  18 +
>>>  include/soc/tegra/pmc.h                            |  20 +-
>>>  13 files changed, 1324 insertions(+), 212 deletions(-)
>>>  create mode 100644 include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h
>>
>> Adrian,
>>
>> these patches are also a prerequisite for the UHS signalling changes in
>> the other two series that Aapo posted. It'd be great if you could take a
>> quick look and weigh in.
> 
> Sure, I'll try and have a look today.

Apart from the 2 new quirks which I made comments on, the rest seems fine.
Marcel Ziswiler Aug. 27, 2018, 2:10 p.m. UTC | #6
On Fri, 2018-08-10 at 21:08 +0300, Aapo Vienamo wrote:
> Hi all,
> 
> This series implements support for faster signaling modes on Tegra
> SDHCI controllers. This series consist of several parts: changes
> requried for 1.8 V signaling and pad control, pad calibration, and
> tuning. Following earlies patch sets have been merged into this
> larger set: "Tegra PMC pinctrl pad configuration", "Tegra SDHCI
> enable
> 1.8 V signaling on Tegar210 and Tegra186", "Tegra SDHCI update the
> padautocal procedure". Also the patches for enabling SDHCI tuning
> are added.

I tried your tkln/hs200 branch on Colibri T20, Apalis/Colibri T30 and
Apalis TK1. It at least does not seem to make things any worse but
HS200 on TK1 still seems to behave strangely. During boot I do get the
following message (mmc0 being the SDHCI instance of one of them SD card
slots):

[    3.238360] mmc0: Internal clock never stabilised.
[    3.243183] mmc0: sdhci: ============ SDHCI REGISTER DUMP
===========
[    3.249649] mmc0: sdhci: Sys addr:  0x00000000 |
Version:  0x00000303
[    3.256138] mmc0: sdhci: Blk size:  0x00000000 | Blk
cnt:  0x00000000
[    3.262657] mmc0: sdhci: Argument:  0x00000000 | Trn mode:
0x00000000
[    3.269119] mmc0: sdhci: Present:   0x01fb00f0 | Host ctl:
0x00000000
[    3.275580] mmc0: sdhci: Power:     0x0000000f | Blk
gap:  0x00000000
[    3.282041] mmc0: sdhci: Wake-up:   0x00000000 |
Clock:    0x00000401
[    3.288485] mmc0: sdhci: Timeout:   0x00000000 | Int stat:
0x00000000
[    3.295037] mmc0: sdhci: Int enab:  0x00ff0003 | Sig enab:
0x00fc0003
[    3.301559] mmc0: sdhci: AC12 err:  0x00000000 | Slot int:
0x00000000
[    3.308022] mmc0: sdhci: Caps:      0x376fd080 |
Caps_1:   0x10000f70
[    3.314527] mmc0: sdhci: Cmd:       0x00000000 | Max curr:
0x00000000
[    3.321159] mmc0: sdhci: Resp[0]:   0x00000000 |
Resp[1]:  0x00000000
[    3.327642] mmc0: sdhci: Resp[2]:   0x00000000 |
Resp[3]:  0x00000000
[    3.334144] mmc0: sdhci: Host ctl2: 0x00000000
[    3.338613] mmc0: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
0x00000000
[    3.345110] mmc0: sdhci:
============================================

And it subsequently stalls waiting for interrupt for more than 8
seconds before continuing to mount the rootfs as follows (mmc2 being
the SDHCI instance of the eMMC):

[    4.874017] tegra-hdmi 54280000.hdmi: cannot set audio to 48000 Hz
at 297000000 Hz pixel clock
[   13.930136] mmc2: Timeout waiting for hardware interrupt.
[   13.935603] mmc2: sdhci: ============ SDHCI REGISTER DUMP
===========
[   13.942071] mmc2: sdhci: Sys addr:  0x00000000 |
Version:  0x00000303
[   13.948511] mmc2: sdhci: Blk size:  0x00007080 | Blk
cnt:  0x00000001
[   13.954948] mmc2: sdhci: Argument:  0x00000000 | Trn mode:
0x00000013
[   13.961385] mmc2: sdhci: Present:   0x01fb00f0 | Host ctl:
0x00000031
[   13.967821] mmc2: sdhci: Power:     0x00000001 | Blk
gap:  0x00000000
[   13.974263] mmc2: sdhci: Wake-up:   0x00000000 |
Clock:    0x00000007
[   13.980692] mmc2: sdhci: Timeout:   0x0000000e | Int stat:
0x00000000
[   13.987119] mmc2: sdhci: Int enab:  0x02ff000b | Sig enab:
0x02fc000b
[   13.993546] mmc2: sdhci: AC12 err:  0x00000000 | Slot int:
0x00000000
[   13.999974] mmc2: sdhci: Caps:      0x376fd080 |
Caps_1:   0x10000f70
[   14.006415] mmc2: sdhci: Cmd:       0x0000153a | Max curr:
0x00000000
[   14.012845] mmc2: sdhci: Resp[0]:   0x00000b00 |
Resp[1]:  0x048062bf
[   14.019272] mmc2: sdhci: Resp[2]:   0x314a8000 |
Resp[3]:  0x00000240
[   14.025697] mmc2: sdhci: Host ctl2: 0x0000000b
[   14.030132] mmc2: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
0xfbc6b208
[   14.036561] mmc2: sdhci:
============================================
[   14.044332] mmc2: new HS200 MMC card at address 0001
[   14.050656] mmcblk2: mmc2:0001 016G30 14.7 GiB
[   14.056376] mmcblk2boot0: mmc2:0001 016G30 partition 1 4.00 MiB
[   14.063563] mmcblk2boot1: mmc2:0001 016G30 partition 2 4.00 MiB
[   14.069589] mmcblk2rpmb: mmc2:0001 016G30 partition 3 4.00 MiB,
chardev (247:0)
[   14.078260]  mmcblk2: p1 p2

After that it actually seems to work quite nicely:

root@apalis-tk1-mainline:~# cat /sys/kernel/debug/mmc2/ios
clock:          200000000 Hz
actual clock:   163200000 Hz
vdd:            21 (3.3 ~ 3.4 V)
bus mode:       2 (push-pull)
chip select:    0 (don't care)
power mode:     2 (on)
bus width:      3 (8 bits)
timing spec:    9 (mmc HS200)
signal voltage: 1 (1.80 V)
driver type:    0 (driver type B)
root@apalis-tk1-mainline:~# hdparm -t /dev/mmcblk2

/dev/mmcblk2:
 Timing buffered disk reads: 408 MB in  3.01 seconds = 135.39 MB/sec

Have you ever observed similar behaviour? What could cause this?

Anybody else tried it on TK1?
Thierry Reding Aug. 27, 2018, 3:50 p.m. UTC | #7
On Mon, Aug 27, 2018 at 02:10:58PM +0000, Marcel Ziswiler wrote:
> On Fri, 2018-08-10 at 21:08 +0300, Aapo Vienamo wrote:
> > Hi all,
> > 
> > This series implements support for faster signaling modes on Tegra
> > SDHCI controllers. This series consist of several parts: changes
> > requried for 1.8 V signaling and pad control, pad calibration, and
> > tuning. Following earlies patch sets have been merged into this
> > larger set: "Tegra PMC pinctrl pad configuration", "Tegra SDHCI
> > enable
> > 1.8 V signaling on Tegar210 and Tegra186", "Tegra SDHCI update the
> > padautocal procedure". Also the patches for enabling SDHCI tuning
> > are added.
> 
> I tried your tkln/hs200 branch on Colibri T20, Apalis/Colibri T30 and
> Apalis TK1. It at least does not seem to make things any worse but
> HS200 on TK1 still seems to behave strangely. During boot I do get the
> following message (mmc0 being the SDHCI instance of one of them SD card
> slots):
> 
> [    3.238360] mmc0: Internal clock never stabilised.
> [    3.243183] mmc0: sdhci: ============ SDHCI REGISTER DUMP
> ===========
> [    3.249649] mmc0: sdhci: Sys addr:  0x00000000 |
> Version:  0x00000303
> [    3.256138] mmc0: sdhci: Blk size:  0x00000000 | Blk
> cnt:  0x00000000
> [    3.262657] mmc0: sdhci: Argument:  0x00000000 | Trn mode:
> 0x00000000
> [    3.269119] mmc0: sdhci: Present:   0x01fb00f0 | Host ctl:
> 0x00000000
> [    3.275580] mmc0: sdhci: Power:     0x0000000f | Blk
> gap:  0x00000000
> [    3.282041] mmc0: sdhci: Wake-up:   0x00000000 |
> Clock:    0x00000401
> [    3.288485] mmc0: sdhci: Timeout:   0x00000000 | Int stat:
> 0x00000000
> [    3.295037] mmc0: sdhci: Int enab:  0x00ff0003 | Sig enab:
> 0x00fc0003
> [    3.301559] mmc0: sdhci: AC12 err:  0x00000000 | Slot int:
> 0x00000000
> [    3.308022] mmc0: sdhci: Caps:      0x376fd080 |
> Caps_1:   0x10000f70
> [    3.314527] mmc0: sdhci: Cmd:       0x00000000 | Max curr:
> 0x00000000
> [    3.321159] mmc0: sdhci: Resp[0]:   0x00000000 |
> Resp[1]:  0x00000000
> [    3.327642] mmc0: sdhci: Resp[2]:   0x00000000 |
> Resp[3]:  0x00000000
> [    3.334144] mmc0: sdhci: Host ctl2: 0x00000000
> [    3.338613] mmc0: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
> 0x00000000
> [    3.345110] mmc0: sdhci:
> ============================================
> 
> And it subsequently stalls waiting for interrupt for more than 8
> seconds before continuing to mount the rootfs as follows (mmc2 being
> the SDHCI instance of the eMMC):
> 
> [    4.874017] tegra-hdmi 54280000.hdmi: cannot set audio to 48000 Hz
> at 297000000 Hz pixel clock
> [   13.930136] mmc2: Timeout waiting for hardware interrupt.
> [   13.935603] mmc2: sdhci: ============ SDHCI REGISTER DUMP
> ===========
> [   13.942071] mmc2: sdhci: Sys addr:  0x00000000 |
> Version:  0x00000303
> [   13.948511] mmc2: sdhci: Blk size:  0x00007080 | Blk
> cnt:  0x00000001
> [   13.954948] mmc2: sdhci: Argument:  0x00000000 | Trn mode:
> 0x00000013
> [   13.961385] mmc2: sdhci: Present:   0x01fb00f0 | Host ctl:
> 0x00000031
> [   13.967821] mmc2: sdhci: Power:     0x00000001 | Blk
> gap:  0x00000000
> [   13.974263] mmc2: sdhci: Wake-up:   0x00000000 |
> Clock:    0x00000007
> [   13.980692] mmc2: sdhci: Timeout:   0x0000000e | Int stat:
> 0x00000000
> [   13.987119] mmc2: sdhci: Int enab:  0x02ff000b | Sig enab:
> 0x02fc000b
> [   13.993546] mmc2: sdhci: AC12 err:  0x00000000 | Slot int:
> 0x00000000
> [   13.999974] mmc2: sdhci: Caps:      0x376fd080 |
> Caps_1:   0x10000f70
> [   14.006415] mmc2: sdhci: Cmd:       0x0000153a | Max curr:
> 0x00000000
> [   14.012845] mmc2: sdhci: Resp[0]:   0x00000b00 |
> Resp[1]:  0x048062bf
> [   14.019272] mmc2: sdhci: Resp[2]:   0x314a8000 |
> Resp[3]:  0x00000240
> [   14.025697] mmc2: sdhci: Host ctl2: 0x0000000b
> [   14.030132] mmc2: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
> 0xfbc6b208
> [   14.036561] mmc2: sdhci:
> ============================================
> [   14.044332] mmc2: new HS200 MMC card at address 0001
> [   14.050656] mmcblk2: mmc2:0001 016G30 14.7 GiB
> [   14.056376] mmcblk2boot0: mmc2:0001 016G30 partition 1 4.00 MiB
> [   14.063563] mmcblk2boot1: mmc2:0001 016G30 partition 2 4.00 MiB
> [   14.069589] mmcblk2rpmb: mmc2:0001 016G30 partition 3 4.00 MiB,
> chardev (247:0)
> [   14.078260]  mmcblk2: p1 p2
> 
> After that it actually seems to work quite nicely:
> 
> root@apalis-tk1-mainline:~# cat /sys/kernel/debug/mmc2/ios
> clock:          200000000 Hz
> actual clock:   163200000 Hz
> vdd:            21 (3.3 ~ 3.4 V)
> bus mode:       2 (push-pull)
> chip select:    0 (don't care)
> power mode:     2 (on)
> bus width:      3 (8 bits)
> timing spec:    9 (mmc HS200)
> signal voltage: 1 (1.80 V)
> driver type:    0 (driver type B)
> root@apalis-tk1-mainline:~# hdparm -t /dev/mmcblk2
> 
> /dev/mmcblk2:
>  Timing buffered disk reads: 408 MB in  3.01 seconds = 135.39 MB/sec
> 
> Have you ever observed similar behaviour? What could cause this?
> 
> Anybody else tried it on TK1?

I can't reproduce this on Jetson TK1. Things boot normally and I don't
see the controllers switch into HS200 for either eMMC or SD card.

Did you update the device tree to enable HS200 on Apalis?

Aapo, isn't there a mechanism to prevent HS200 on devices where it
hasn't explicitly been tested (and enabled)? I thought we had discussed
that this was going to depend on pinmux DTS entries, but I can't find
the actual code where any of that would be checked. I only see checking
NVQUIRK_NEEDS_PAD_CONTROL in tegra_sdhci_is_pad_and_regulator_valid(),
but since we don't enable that quirk on Tegra124, it would seem to me
that we always enable fast modes on boards that don't need pad control.
Is that really what we want?

Also, if the above is correct, then why am I not seeing faster modes
getting enabled on Jetson TK1?

Thierry
Thierry Reding Aug. 27, 2018, 3:57 p.m. UTC | #8
On Mon, Aug 27, 2018 at 05:50:53PM +0200, Thierry Reding wrote:
> On Mon, Aug 27, 2018 at 02:10:58PM +0000, Marcel Ziswiler wrote:
> > On Fri, 2018-08-10 at 21:08 +0300, Aapo Vienamo wrote:
> > > Hi all,
> > > 
> > > This series implements support for faster signaling modes on Tegra
> > > SDHCI controllers. This series consist of several parts: changes
> > > requried for 1.8 V signaling and pad control, pad calibration, and
> > > tuning. Following earlies patch sets have been merged into this
> > > larger set: "Tegra PMC pinctrl pad configuration", "Tegra SDHCI
> > > enable
> > > 1.8 V signaling on Tegar210 and Tegra186", "Tegra SDHCI update the
> > > padautocal procedure". Also the patches for enabling SDHCI tuning
> > > are added.
> > 
> > I tried your tkln/hs200 branch on Colibri T20, Apalis/Colibri T30 and
> > Apalis TK1. It at least does not seem to make things any worse but
> > HS200 on TK1 still seems to behave strangely. During boot I do get the
> > following message (mmc0 being the SDHCI instance of one of them SD card
> > slots):
> > 
> > [    3.238360] mmc0: Internal clock never stabilised.
> > [    3.243183] mmc0: sdhci: ============ SDHCI REGISTER DUMP
> > ===========
> > [    3.249649] mmc0: sdhci: Sys addr:  0x00000000 |
> > Version:  0x00000303
> > [    3.256138] mmc0: sdhci: Blk size:  0x00000000 | Blk
> > cnt:  0x00000000
> > [    3.262657] mmc0: sdhci: Argument:  0x00000000 | Trn mode:
> > 0x00000000
> > [    3.269119] mmc0: sdhci: Present:   0x01fb00f0 | Host ctl:
> > 0x00000000
> > [    3.275580] mmc0: sdhci: Power:     0x0000000f | Blk
> > gap:  0x00000000
> > [    3.282041] mmc0: sdhci: Wake-up:   0x00000000 |
> > Clock:    0x00000401
> > [    3.288485] mmc0: sdhci: Timeout:   0x00000000 | Int stat:
> > 0x00000000
> > [    3.295037] mmc0: sdhci: Int enab:  0x00ff0003 | Sig enab:
> > 0x00fc0003
> > [    3.301559] mmc0: sdhci: AC12 err:  0x00000000 | Slot int:
> > 0x00000000
> > [    3.308022] mmc0: sdhci: Caps:      0x376fd080 |
> > Caps_1:   0x10000f70
> > [    3.314527] mmc0: sdhci: Cmd:       0x00000000 | Max curr:
> > 0x00000000
> > [    3.321159] mmc0: sdhci: Resp[0]:   0x00000000 |
> > Resp[1]:  0x00000000
> > [    3.327642] mmc0: sdhci: Resp[2]:   0x00000000 |
> > Resp[3]:  0x00000000
> > [    3.334144] mmc0: sdhci: Host ctl2: 0x00000000
> > [    3.338613] mmc0: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
> > 0x00000000
> > [    3.345110] mmc0: sdhci:
> > ============================================
> > 
> > And it subsequently stalls waiting for interrupt for more than 8
> > seconds before continuing to mount the rootfs as follows (mmc2 being
> > the SDHCI instance of the eMMC):
> > 
> > [    4.874017] tegra-hdmi 54280000.hdmi: cannot set audio to 48000 Hz
> > at 297000000 Hz pixel clock
> > [   13.930136] mmc2: Timeout waiting for hardware interrupt.
> > [   13.935603] mmc2: sdhci: ============ SDHCI REGISTER DUMP
> > ===========
> > [   13.942071] mmc2: sdhci: Sys addr:  0x00000000 |
> > Version:  0x00000303
> > [   13.948511] mmc2: sdhci: Blk size:  0x00007080 | Blk
> > cnt:  0x00000001
> > [   13.954948] mmc2: sdhci: Argument:  0x00000000 | Trn mode:
> > 0x00000013
> > [   13.961385] mmc2: sdhci: Present:   0x01fb00f0 | Host ctl:
> > 0x00000031
> > [   13.967821] mmc2: sdhci: Power:     0x00000001 | Blk
> > gap:  0x00000000
> > [   13.974263] mmc2: sdhci: Wake-up:   0x00000000 |
> > Clock:    0x00000007
> > [   13.980692] mmc2: sdhci: Timeout:   0x0000000e | Int stat:
> > 0x00000000
> > [   13.987119] mmc2: sdhci: Int enab:  0x02ff000b | Sig enab:
> > 0x02fc000b
> > [   13.993546] mmc2: sdhci: AC12 err:  0x00000000 | Slot int:
> > 0x00000000
> > [   13.999974] mmc2: sdhci: Caps:      0x376fd080 |
> > Caps_1:   0x10000f70
> > [   14.006415] mmc2: sdhci: Cmd:       0x0000153a | Max curr:
> > 0x00000000
> > [   14.012845] mmc2: sdhci: Resp[0]:   0x00000b00 |
> > Resp[1]:  0x048062bf
> > [   14.019272] mmc2: sdhci: Resp[2]:   0x314a8000 |
> > Resp[3]:  0x00000240
> > [   14.025697] mmc2: sdhci: Host ctl2: 0x0000000b
> > [   14.030132] mmc2: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
> > 0xfbc6b208
> > [   14.036561] mmc2: sdhci:
> > ============================================
> > [   14.044332] mmc2: new HS200 MMC card at address 0001
> > [   14.050656] mmcblk2: mmc2:0001 016G30 14.7 GiB
> > [   14.056376] mmcblk2boot0: mmc2:0001 016G30 partition 1 4.00 MiB
> > [   14.063563] mmcblk2boot1: mmc2:0001 016G30 partition 2 4.00 MiB
> > [   14.069589] mmcblk2rpmb: mmc2:0001 016G30 partition 3 4.00 MiB,
> > chardev (247:0)
> > [   14.078260]  mmcblk2: p1 p2
> > 
> > After that it actually seems to work quite nicely:
> > 
> > root@apalis-tk1-mainline:~# cat /sys/kernel/debug/mmc2/ios
> > clock:          200000000 Hz
> > actual clock:   163200000 Hz
> > vdd:            21 (3.3 ~ 3.4 V)
> > bus mode:       2 (push-pull)
> > chip select:    0 (don't care)
> > power mode:     2 (on)
> > bus width:      3 (8 bits)
> > timing spec:    9 (mmc HS200)
> > signal voltage: 1 (1.80 V)
> > driver type:    0 (driver type B)
> > root@apalis-tk1-mainline:~# hdparm -t /dev/mmcblk2
> > 
> > /dev/mmcblk2:
> >  Timing buffered disk reads: 408 MB in  3.01 seconds = 135.39 MB/sec
> > 
> > Have you ever observed similar behaviour? What could cause this?
> > 
> > Anybody else tried it on TK1?
> 
> I can't reproduce this on Jetson TK1. Things boot normally and I don't
> see the controllers switch into HS200 for either eMMC or SD card.
> 
> Did you update the device tree to enable HS200 on Apalis?
> 
> Aapo, isn't there a mechanism to prevent HS200 on devices where it
> hasn't explicitly been tested (and enabled)? I thought we had discussed
> that this was going to depend on pinmux DTS entries, but I can't find
> the actual code where any of that would be checked. I only see checking
> NVQUIRK_NEEDS_PAD_CONTROL in tegra_sdhci_is_pad_and_regulator_valid(),
> but since we don't enable that quirk on Tegra124, it would seem to me
> that we always enable fast modes on boards that don't need pad control.
> Is that really what we want?
> 
> Also, if the above is correct, then why am I not seeing faster modes
> getting enabled on Jetson TK1?

I also just ran into what seems to be a typo. Do we want the below fix?

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index ef18a0c6ed3a..a6053e5004da 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -336,7 +336,7 @@ static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
                        misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_DDR50;
                if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR104)
                        misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDR104;
-               if (soc_data->nvquirks & SDHCI_MISC_CTRL_ENABLE_SDR50)
+               if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR50)
                        clk_ctrl |= SDHCI_CLOCK_CTRL_SDR50_TUNING_OVERRIDE;
        }
Aapo Vienamo Aug. 27, 2018, 4:27 p.m. UTC | #9
On Mon, 27 Aug 2018 17:50:53 +0200
Thierry Reding <thierry.reding@gmail.com> wrote:

> On Mon, Aug 27, 2018 at 02:10:58PM +0000, Marcel Ziswiler wrote:
> > On Fri, 2018-08-10 at 21:08 +0300, Aapo Vienamo wrote:  
> > > Hi all,
> > > 
> > > This series implements support for faster signaling modes on Tegra
> > > SDHCI controllers. This series consist of several parts: changes
> > > requried for 1.8 V signaling and pad control, pad calibration, and
> > > tuning. Following earlies patch sets have been merged into this
> > > larger set: "Tegra PMC pinctrl pad configuration", "Tegra SDHCI
> > > enable
> > > 1.8 V signaling on Tegar210 and Tegra186", "Tegra SDHCI update the
> > > padautocal procedure". Also the patches for enabling SDHCI tuning
> > > are added.  
> > 
> > I tried your tkln/hs200 branch on Colibri T20, Apalis/Colibri T30 and
> > Apalis TK1. It at least does not seem to make things any worse but
> > HS200 on TK1 still seems to behave strangely. During boot I do get the
> > following message (mmc0 being the SDHCI instance of one of them SD card
> > slots):
> > 
> > [    3.238360] mmc0: Internal clock never stabilised.
> > [    3.243183] mmc0: sdhci: ============ SDHCI REGISTER DUMP
> > ===========
> > [    3.249649] mmc0: sdhci: Sys addr:  0x00000000 |
> > Version:  0x00000303
> > [    3.256138] mmc0: sdhci: Blk size:  0x00000000 | Blk
> > cnt:  0x00000000
> > [    3.262657] mmc0: sdhci: Argument:  0x00000000 | Trn mode:
> > 0x00000000
> > [    3.269119] mmc0: sdhci: Present:   0x01fb00f0 | Host ctl:
> > 0x00000000
> > [    3.275580] mmc0: sdhci: Power:     0x0000000f | Blk
> > gap:  0x00000000
> > [    3.282041] mmc0: sdhci: Wake-up:   0x00000000 |
> > Clock:    0x00000401
> > [    3.288485] mmc0: sdhci: Timeout:   0x00000000 | Int stat:
> > 0x00000000
> > [    3.295037] mmc0: sdhci: Int enab:  0x00ff0003 | Sig enab:
> > 0x00fc0003
> > [    3.301559] mmc0: sdhci: AC12 err:  0x00000000 | Slot int:
> > 0x00000000
> > [    3.308022] mmc0: sdhci: Caps:      0x376fd080 |
> > Caps_1:   0x10000f70
> > [    3.314527] mmc0: sdhci: Cmd:       0x00000000 | Max curr:
> > 0x00000000
> > [    3.321159] mmc0: sdhci: Resp[0]:   0x00000000 |
> > Resp[1]:  0x00000000
> > [    3.327642] mmc0: sdhci: Resp[2]:   0x00000000 |
> > Resp[3]:  0x00000000
> > [    3.334144] mmc0: sdhci: Host ctl2: 0x00000000
> > [    3.338613] mmc0: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
> > 0x00000000
> > [    3.345110] mmc0: sdhci:
> > ============================================
> > 
> > And it subsequently stalls waiting for interrupt for more than 8
> > seconds before continuing to mount the rootfs as follows (mmc2 being
> > the SDHCI instance of the eMMC):
> > 
> > [    4.874017] tegra-hdmi 54280000.hdmi: cannot set audio to 48000 Hz
> > at 297000000 Hz pixel clock
> > [   13.930136] mmc2: Timeout waiting for hardware interrupt.
> > [   13.935603] mmc2: sdhci: ============ SDHCI REGISTER DUMP
> > ===========
> > [   13.942071] mmc2: sdhci: Sys addr:  0x00000000 |
> > Version:  0x00000303
> > [   13.948511] mmc2: sdhci: Blk size:  0x00007080 | Blk
> > cnt:  0x00000001
> > [   13.954948] mmc2: sdhci: Argument:  0x00000000 | Trn mode:
> > 0x00000013
> > [   13.961385] mmc2: sdhci: Present:   0x01fb00f0 | Host ctl:
> > 0x00000031
> > [   13.967821] mmc2: sdhci: Power:     0x00000001 | Blk
> > gap:  0x00000000
> > [   13.974263] mmc2: sdhci: Wake-up:   0x00000000 |
> > Clock:    0x00000007
> > [   13.980692] mmc2: sdhci: Timeout:   0x0000000e | Int stat:
> > 0x00000000
> > [   13.987119] mmc2: sdhci: Int enab:  0x02ff000b | Sig enab:
> > 0x02fc000b
> > [   13.993546] mmc2: sdhci: AC12 err:  0x00000000 | Slot int:
> > 0x00000000
> > [   13.999974] mmc2: sdhci: Caps:      0x376fd080 |
> > Caps_1:   0x10000f70
> > [   14.006415] mmc2: sdhci: Cmd:       0x0000153a | Max curr:
> > 0x00000000
> > [   14.012845] mmc2: sdhci: Resp[0]:   0x00000b00 |
> > Resp[1]:  0x048062bf
> > [   14.019272] mmc2: sdhci: Resp[2]:   0x314a8000 |
> > Resp[3]:  0x00000240
> > [   14.025697] mmc2: sdhci: Host ctl2: 0x0000000b
> > [   14.030132] mmc2: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
> > 0xfbc6b208
> > [   14.036561] mmc2: sdhci:
> > ============================================
> > [   14.044332] mmc2: new HS200 MMC card at address 0001
> > [   14.050656] mmcblk2: mmc2:0001 016G30 14.7 GiB
> > [   14.056376] mmcblk2boot0: mmc2:0001 016G30 partition 1 4.00 MiB
> > [   14.063563] mmcblk2boot1: mmc2:0001 016G30 partition 2 4.00 MiB
> > [   14.069589] mmcblk2rpmb: mmc2:0001 016G30 partition 3 4.00 MiB,
> > chardev (247:0)
> > [   14.078260]  mmcblk2: p1 p2
> > 
> > After that it actually seems to work quite nicely:
> > 
> > root@apalis-tk1-mainline:~# cat /sys/kernel/debug/mmc2/ios
> > clock:          200000000 Hz
> > actual clock:   163200000 Hz
> > vdd:            21 (3.3 ~ 3.4 V)
> > bus mode:       2 (push-pull)
> > chip select:    0 (don't care)
> > power mode:     2 (on)
> > bus width:      3 (8 bits)
> > timing spec:    9 (mmc HS200)
> > signal voltage: 1 (1.80 V)
> > driver type:    0 (driver type B)
> > root@apalis-tk1-mainline:~# hdparm -t /dev/mmcblk2
> > 
> > /dev/mmcblk2:
> >  Timing buffered disk reads: 408 MB in  3.01 seconds = 135.39 MB/sec
> > 
> > Have you ever observed similar behaviour? What could cause this?
> > 
> > Anybody else tried it on TK1?  
> 
> I can't reproduce this on Jetson TK1. Things boot normally and I don't
> see the controllers switch into HS200 for either eMMC or SD card.
> 
> Did you update the device tree to enable HS200 on Apalis?
> 
> Aapo, isn't there a mechanism to prevent HS200 on devices where it
> hasn't explicitly been tested (and enabled)? I thought we had discussed
> that this was going to depend on pinmux DTS entries, but I can't find
> the actual code where any of that would be checked. I only see checking
> NVQUIRK_NEEDS_PAD_CONTROL in tegra_sdhci_is_pad_and_regulator_valid(),
> but since we don't enable that quirk on Tegra124, it would seem to me
> that we always enable fast modes on boards that don't need pad control.
> Is that really what we want?

The HS200 mode is enabled by setting NVQUIRK_ENABLE_SDR104 in nvquirks.
This quirk bit shouldn't be currently set on Tegra124.

> Also, if the above is correct, then why am I not seeing faster modes
> getting enabled on Jetson TK1?

I spent last week looking into it. One really weird issue I found was
that when the tuning command is sent, the timeout in sdhci_send_tuning()
sometimes expires. This seems to happen only on certain SD cards and
only if the tap values are modified faster than 10 ms apart. 10 ms is a
really long time which makes this really weird.

Downstream has reimplemented a register polling version of
mmc_send_tuning(), supposedly to avoid interrupt overheads, which oddly
enough seems to work.

 -Aapo
Marcel Ziswiler Aug. 27, 2018, 9:35 p.m. UTC | #10
On Mon, 2018-08-27 at 17:50 +0200, Thierry Reding wrote:
> On Mon, Aug 27, 2018 at 02:10:58PM +0000, Marcel Ziswiler wrote:
> > On Fri, 2018-08-10 at 21:08 +0300, Aapo Vienamo wrote:
> > > Hi all,
> > > 
> > > This series implements support for faster signaling modes on
> > > Tegra
> > > SDHCI controllers. This series consist of several parts: changes
> > > requried for 1.8 V signaling and pad control, pad calibration,
> > > and
> > > tuning. Following earlies patch sets have been merged into this
> > > larger set: "Tegra PMC pinctrl pad configuration", "Tegra SDHCI
> > > enable
> > > 1.8 V signaling on Tegar210 and Tegra186", "Tegra SDHCI update
> > > the
> > > padautocal procedure". Also the patches for enabling SDHCI tuning
> > > are added.
> > 
> > I tried your tkln/hs200 branch on Colibri T20, Apalis/Colibri T30
> > and
> > Apalis TK1. It at least does not seem to make things any worse but
> > HS200 on TK1 still seems to behave strangely. During boot I do get
> > the
> > following message (mmc0 being the SDHCI instance of one of them SD
> > card
> > slots):
> > 
> > [    3.238360] mmc0: Internal clock never stabilised.
> > [    3.243183] mmc0: sdhci: ============ SDHCI REGISTER DUMP
> > ===========
> > [    3.249649] mmc0: sdhci: Sys addr:  0x00000000 |
> > Version:  0x00000303
> > [    3.256138] mmc0: sdhci: Blk size:  0x00000000 | Blk
> > cnt:  0x00000000
> > [    3.262657] mmc0: sdhci: Argument:  0x00000000 | Trn mode:
> > 0x00000000
> > [    3.269119] mmc0: sdhci: Present:   0x01fb00f0 | Host ctl:
> > 0x00000000
> > [    3.275580] mmc0: sdhci: Power:     0x0000000f | Blk
> > gap:  0x00000000
> > [    3.282041] mmc0: sdhci: Wake-up:   0x00000000 |
> > Clock:    0x00000401
> > [    3.288485] mmc0: sdhci: Timeout:   0x00000000 | Int stat:
> > 0x00000000
> > [    3.295037] mmc0: sdhci: Int enab:  0x00ff0003 | Sig enab:
> > 0x00fc0003
> > [    3.301559] mmc0: sdhci: AC12 err:  0x00000000 | Slot int:
> > 0x00000000
> > [    3.308022] mmc0: sdhci: Caps:      0x376fd080 |
> > Caps_1:   0x10000f70
> > [    3.314527] mmc0: sdhci: Cmd:       0x00000000 | Max curr:
> > 0x00000000
> > [    3.321159] mmc0: sdhci: Resp[0]:   0x00000000 |
> > Resp[1]:  0x00000000
> > [    3.327642] mmc0: sdhci: Resp[2]:   0x00000000 |
> > Resp[3]:  0x00000000
> > [    3.334144] mmc0: sdhci: Host ctl2: 0x00000000
> > [    3.338613] mmc0: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
> > 0x00000000
> > [    3.345110] mmc0: sdhci:
> > ============================================
> > 
> > And it subsequently stalls waiting for interrupt for more than 8
> > seconds before continuing to mount the rootfs as follows (mmc2
> > being
> > the SDHCI instance of the eMMC):
> > 
> > [    4.874017] tegra-hdmi 54280000.hdmi: cannot set audio to 48000
> > Hz
> > at 297000000 Hz pixel clock
> > [   13.930136] mmc2: Timeout waiting for hardware interrupt.
> > [   13.935603] mmc2: sdhci: ============ SDHCI REGISTER DUMP
> > ===========
> > [   13.942071] mmc2: sdhci: Sys addr:  0x00000000 |
> > Version:  0x00000303
> > [   13.948511] mmc2: sdhci: Blk size:  0x00007080 | Blk
> > cnt:  0x00000001
> > [   13.954948] mmc2: sdhci: Argument:  0x00000000 | Trn mode:
> > 0x00000013
> > [   13.961385] mmc2: sdhci: Present:   0x01fb00f0 | Host ctl:
> > 0x00000031
> > [   13.967821] mmc2: sdhci: Power:     0x00000001 | Blk
> > gap:  0x00000000
> > [   13.974263] mmc2: sdhci: Wake-up:   0x00000000 |
> > Clock:    0x00000007
> > [   13.980692] mmc2: sdhci: Timeout:   0x0000000e | Int stat:
> > 0x00000000
> > [   13.987119] mmc2: sdhci: Int enab:  0x02ff000b | Sig enab:
> > 0x02fc000b
> > [   13.993546] mmc2: sdhci: AC12 err:  0x00000000 | Slot int:
> > 0x00000000
> > [   13.999974] mmc2: sdhci: Caps:      0x376fd080 |
> > Caps_1:   0x10000f70
> > [   14.006415] mmc2: sdhci: Cmd:       0x0000153a | Max curr:
> > 0x00000000
> > [   14.012845] mmc2: sdhci: Resp[0]:   0x00000b00 |
> > Resp[1]:  0x048062bf
> > [   14.019272] mmc2: sdhci: Resp[2]:   0x314a8000 |
> > Resp[3]:  0x00000240
> > [   14.025697] mmc2: sdhci: Host ctl2: 0x0000000b
> > [   14.030132] mmc2: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
> > 0xfbc6b208
> > [   14.036561] mmc2: sdhci:
> > ============================================
> > [   14.044332] mmc2: new HS200 MMC card at address 0001
> > [   14.050656] mmcblk2: mmc2:0001 016G30 14.7 GiB
> > [   14.056376] mmcblk2boot0: mmc2:0001 016G30 partition 1 4.00 MiB
> > [   14.063563] mmcblk2boot1: mmc2:0001 016G30 partition 2 4.00 MiB
> > [   14.069589] mmcblk2rpmb: mmc2:0001 016G30 partition 3 4.00 MiB,
> > chardev (247:0)
> > [   14.078260]  mmcblk2: p1 p2
> > 
> > After that it actually seems to work quite nicely:
> > 
> > root@apalis-tk1-mainline:~# cat /sys/kernel/debug/mmc2/ios
> > clock:          200000000 Hz
> > actual clock:   163200000 Hz
> > vdd:            21 (3.3 ~ 3.4 V)
> > bus mode:       2 (push-pull)
> > chip select:    0 (don't care)
> > power mode:     2 (on)
> > bus width:      3 (8 bits)
> > timing spec:    9 (mmc HS200)
> > signal voltage: 1 (1.80 V)
> > driver type:    0 (driver type B)
> > root@apalis-tk1-mainline:~# hdparm -t /dev/mmcblk2
> > 
> > /dev/mmcblk2:
> >  Timing buffered disk reads: 408 MB in  3.01 seconds = 135.39
> > MB/sec
> > 
> > Have you ever observed similar behaviour? What could cause this?
> > 
> > Anybody else tried it on TK1?
> 
> I can't reproduce this on Jetson TK1. Things boot normally and I
> don't
> see the controllers switch into HS200 for either eMMC or SD card.
> 
> Did you update the device tree to enable HS200 on Apalis?

Yes, sorry. I forgot to mention that I added the mmc-hs200-1_8v
property to my eMMC's sdhci node.

> Aapo, isn't there a mechanism to prevent HS200 on devices where it
> hasn't explicitly been tested (and enabled)? I thought we had
> discussed
> that this was going to depend on pinmux DTS entries, but I can't find
> the actual code where any of that would be checked. I only see
> checking
> NVQUIRK_NEEDS_PAD_CONTROL in
> tegra_sdhci_is_pad_and_regulator_valid(),
> but since we don't enable that quirk on Tegra124, it would seem to me
> that we always enable fast modes on boards that don't need pad
> control.
> Is that really what we want?
> 
> Also, if the above is correct, then why am I not seeing faster modes
> getting enabled on Jetson TK1?
> 
> Thierry