| Message ID | 20190423090235.17244-1-jbrunet@baylibre.com |
|---|---|
| Headers | show |
| Series | mmc: meson-gx: clean up and tuning update | expand |
On Tue, Apr 23, 2019 at 11:02 AM Jerome Brunet <jbrunet@baylibre.com> wrote: > > This is merely a clean up. It makes sense to only ack raised irqs > instead of acking everything all the time. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> based on reading (and understanding) the code and a test on my Khadas VIM this seems fine so: Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
On Tue, Apr 23, 2019 at 11:02 AM Jerome Brunet <jbrunet@baylibre.com> wrote: > > There is no reason for another device to request the MMC irq. It should > only be used the MMC device, so remove IRQ_SHARED and replace by > IRQ_ONESHOT as we don't the irq to fire again until the irq thread is > done > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> I'm doing the same thing (for the same purpose) in the meson-mx-sdio driver: Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
On Tue, Apr 23, 2019 at 11:03 AM Jerome Brunet <jbrunet@baylibre.com> wrote: > > Activating DDR in the Amlogic mmc controller, among other things, will > divide the output clock by 2. So by activating it with clock on, we are > creating a glitch on the output. > > Instead, let's deal with DDR when the clock output is off, when setting > the clock. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> [boot-tested on Khadas VIM and no obvious regressions could be seen] Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> thank you for fixing this issue from v1!
On Tue, Apr 23, 2019 at 11:03 AM Jerome Brunet <jbrunet@baylibre.com> wrote: > > This remove all the code related to phase settings. Using the Rx phase > for tuning has not been reliable. We had several issues over the past > months, on both v2 and v3 mmc chips After discussing the issue matter > with Amlogic, They suggested to set a phase shift of 180 between Core and > Tx and use signal resampling for the tuning. > > Since we won't be playing with the phase anymore, let's remove all the > clock code related to it and set the appropriate value on init. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> [Khadas VIM now shows the HS200 eMMC] Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> BEFORE (no patches from this series applied): # dmesg | grep mmc1 (no output) AFTER (all 7 patches from this series applied): # dmesg | grep mmc1 [ 2.945155] mmc1: new HS200 MMC card at address 0001 [ 2.957737] mmcblk1: mmc1:0001 AWPD3R 14.6 GiB [ 2.966278] mmcblk1boot0: mmc1:0001 AWPD3R partition 1 4.00 MiB [ 2.976114] mmcblk1boot1: mmc1:0001 AWPD3R partition 2 4.00 MiB [ 2.986354] mmcblk1rpmb: mmc1:0001 AWPD3R partition 3 4.00 MiB, chardev (241:0) Regards Martin
On Tue, 23 Apr 2019 at 11:02, Jerome Brunet <jbrunet@baylibre.com> wrote: > > The purpose of this series is too improve reliability of the amlogic mmc > driver on new (g12a) and old ones (axg, gxl, gxbb, etc...) > > * The 3 first patches are just harmless clean ups. Applied these first three, postponing the the rest until Martin are happy with all of them. Thanks! Kind regards Uffe > * Patch 4 makes sure HS400 can't be enabled, we still have not been able > to crack this modes. > * Patch 5 removes some clock glitches when switching to DDR modes > * Patch 6 and 7 changes the tuning method from Rx phase to signal > resampling. It could have been done in a single patch but the unified > diff was extremely ugly. The change has been split in two patches to > ease review. > > The last tuning update that went through was meant to improve the axg > support. Since then, it was reported to break some other boards, like the > s912 vim2. > > Also with the current tuning method, it was impossible to find phase > settings which would work on all the SoCs, including the new ones. > > After redoing all the tests from scratch, it appeared that Rx phase made > (strangely) almost no difference, especially on g12a and axg. However, it > showed that it is important to have a phase shift between the Core and Tx > clock, 180 works best. > > I discussed the test results with Amlogic. They suggested to use 180/0 or > 0/180 for the Core and Tx phase. For tuning, they suggested to use > signal resampling. > > So far, so good ... here the platform and modes tested: > > NanoPi-K2 (S905): SD UHS SDR50/DDR50, SDIO HS > Odroid-C2 (S905): SD UHS SDR50/DDR50, eMMC DDR52/HS200 (16GB module) > Khadas Vim (S905X): SD HS, SDIO HS, eMMC HS200 > Libretech CC (S905X): SD HS, eMMC HS200 > Khadas Vim2 (S912): SD HS, SDIO HS, eMMC HS200 > S400 (A113D): SDIO UHS SDR104, eMMC DDR52/HS200 > U200 (S905D2): SD HS, eMMC DDR52/HS200 > SEI510 (S905X2): SD HS, eMMC DDR52/HS200 > > Changes since v1 [0]: > * Add missing writel in patch 5 (error when switching width) > * Change patch 3 commit description > > [0]: https://lkml.kernel.org/r/20190417204355.469-1-jbrunet@baylibre.com > > Jerome Brunet (7): > mmc: meson-gx: remove open coded read with timeout > mmc: meson-gx: ack only raised irq > mmc: meson-gx: correct irq flag > mmc: meson-gx: disable HS400 > mmc: meson-gx: avoid clock glitch when switching to DDR modes > mmc: meson-gx: remove Rx phase tuning > mmc: meson-gx: add signal resampling tuning > > drivers/mmc/host/meson-gx-mmc.c | 419 +++++++++----------------------- > 1 file changed, 114 insertions(+), 305 deletions(-) > > -- > 2.20.1 >
Hi Ulf, Hi Kevin, On Mon, Apr 29, 2019 at 12:45 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Tue, 23 Apr 2019 at 11:02, Jerome Brunet <jbrunet@baylibre.com> wrote: > > > > The purpose of this series is too improve reliability of the amlogic mmc > > driver on new (g12a) and old ones (axg, gxl, gxbb, etc...) > > > > * The 3 first patches are just harmless clean ups. > > Applied these first three, postponing the the rest until Martin are > happy with all of them. Thanks! thank you for taking the first three patches! I am fine with everything except the patch description of patch 4 and 7 as noted here: [0] Kevin, can you please also have a look at this series (if you didn't already)? you reviewed earlier changes to the tuning mechanism in this driver. it would be great to know that you're happy with these changes as well. Regards Martin [0] http://lists.infradead.org/pipermail/linux-amlogic/2019-April/011488.html
Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes: > Hi Ulf, Hi Kevin, > > On Mon, Apr 29, 2019 at 12:45 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: >> >> On Tue, 23 Apr 2019 at 11:02, Jerome Brunet <jbrunet@baylibre.com> wrote: >> > >> > The purpose of this series is too improve reliability of the amlogic mmc >> > driver on new (g12a) and old ones (axg, gxl, gxbb, etc...) >> > >> > * The 3 first patches are just harmless clean ups. >> >> Applied these first three, postponing the the rest until Martin are >> happy with all of them. Thanks! > thank you for taking the first three patches! > I am fine with everything except the patch description of patch 4 and > 7 as noted here: [0] > > Kevin, can you please also have a look at this series (if you didn't already)? > you reviewed earlier changes to the tuning mechanism in this driver. > it would be great to know that you're happy with these changes as well. I've reviewed the series, and am happy with it as is, including the changelogs as is. Ulf, for the series, feel free to add: Reviewed-by: Kevin Hilman <khilman@baylibre.com> Kevin
Hi Ulf, On Mon, Apr 29, 2019 at 8:40 PM Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > Hi Ulf, Hi Kevin, > > On Mon, Apr 29, 2019 at 12:45 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > On Tue, 23 Apr 2019 at 11:02, Jerome Brunet <jbrunet@baylibre.com> wrote: > > > > > > The purpose of this series is too improve reliability of the amlogic mmc > > > driver on new (g12a) and old ones (axg, gxl, gxbb, etc...) > > > > > > * The 3 first patches are just harmless clean ups. > > > > Applied these first three, postponing the the rest until Martin are > > happy with all of them. Thanks! > thank you for taking the first three patches! > I am fine with everything except the patch description of patch 4 and > 7 as noted here: [0] I did not understand how HS400 works. based on Jerome's latest explanation I'm fine with the patches as they are Martin